diff --git a/panel/public/roboco-logo-nobg.png b/panel/public/roboco-logo-nobg.png deleted file mode 100644 index ac66224b..00000000 Binary files a/panel/public/roboco-logo-nobg.png and /dev/null differ diff --git a/panel/public/roboco-logo.png b/panel/public/roboco-logo.png index d10e4993..0a75eb74 100644 Binary files a/panel/public/roboco-logo.png and b/panel/public/roboco-logo.png differ diff --git a/panel/src/app/(dashboard)/communications/[sessionId]/page.tsx b/panel/src/app/(dashboard)/communications/[sessionId]/page.tsx index 6f43e717..db633dc3 100644 --- a/panel/src/app/(dashboard)/communications/[sessionId]/page.tsx +++ b/panel/src/app/(dashboard)/communications/[sessionId]/page.tsx @@ -154,7 +154,7 @@ function SessionDetailContent() { if (!session) { return (
- + - +
@@ -97,7 +97,7 @@ export default function JournalEntryPage({ params }: JournalEntryPageProps) { {/* Header */}
- + @@ -143,7 +143,7 @@ export default function JournalEntryPage({ params }: JournalEntryPageProps) { Related Task
- + - + - +
diff --git a/panel/src/app/apple-icon.png b/panel/src/app/apple-icon.png index d10e4993..cde929b4 100644 Binary files a/panel/src/app/apple-icon.png and b/panel/src/app/apple-icon.png differ diff --git a/panel/src/app/icon.png b/panel/src/app/icon.png index ac66224b..718e6d4d 100644 Binary files a/panel/src/app/icon.png and b/panel/src/app/icon.png differ diff --git a/panel/src/components/agents/agent-card.tsx b/panel/src/components/agents/agent-card.tsx index ec20f1b3..89ab0a1d 100644 --- a/panel/src/components/agents/agent-card.tsx +++ b/panel/src/components/agents/agent-card.tsx @@ -74,7 +74,7 @@ export function AgentCard({ agent, agentStatus, usageRow }: AgentCardProps) { {isActive && ( <> - + View Details diff --git a/panel/src/components/agents/agent-status-cards.tsx b/panel/src/components/agents/agent-status-cards.tsx index 40ef49d7..08ef14bf 100644 --- a/panel/src/components/agents/agent-status-cards.tsx +++ b/panel/src/components/agents/agent-status-cards.tsx @@ -30,6 +30,7 @@ export function AgentStatusCards({ agent }: AgentStatusCardsProps) { {agent.task_id ? ( diff --git a/panel/src/components/agents/waiting-agents-alert.tsx b/panel/src/components/agents/waiting-agents-alert.tsx index ef8157e9..c5d10632 100644 --- a/panel/src/components/agents/waiting-agents-alert.tsx +++ b/panel/src/components/agents/waiting-agents-alert.tsx @@ -36,7 +36,9 @@ export function WaitingAgentsAlert({ waitingAgents }: WaitingAgentsAlertProps) { ))} diff --git a/panel/src/components/auditor/flagged-item.tsx b/panel/src/components/auditor/flagged-item.tsx index ba353ce4..4e627b7f 100644 --- a/panel/src/components/auditor/flagged-item.tsx +++ b/panel/src/components/auditor/flagged-item.tsx @@ -76,7 +76,7 @@ export function FlaggedItem({ {formatTime(flag.created_at)} {flag.related_task_id && ( - + Task #{flag.related_task_id.slice(0, 8)} @@ -88,7 +88,7 @@ export function FlaggedItem({ {!isResolved && (
{flag.related_task_id && ( - + diff --git a/panel/src/components/communications/communications-view.tsx b/panel/src/components/communications/communications-view.tsx index 3d4f6f2c..ad19e5aa 100644 --- a/panel/src/components/communications/communications-view.tsx +++ b/panel/src/components/communications/communications-view.tsx @@ -29,7 +29,7 @@ export function CommunicationsView() {

- +
diff --git a/panel/src/components/communications/message-item.tsx b/panel/src/components/communications/message-item.tsx index 6fa9b874..d1279091 100644 --- a/panel/src/components/communications/message-item.tsx +++ b/panel/src/components/communications/message-item.tsx @@ -56,7 +56,7 @@ export function MessageItem({ message }: MessageItemProps) { )} {/* Related Task */} {message.task_id && ( - + Task #{message.task_id.slice(0, 8)} diff --git a/panel/src/components/dashboard/active-blockers-panel.tsx b/panel/src/components/dashboard/active-blockers-panel.tsx index 5907d994..5fc25a68 100644 --- a/panel/src/components/dashboard/active-blockers-panel.tsx +++ b/panel/src/components/dashboard/active-blockers-panel.tsx @@ -66,7 +66,7 @@ export function ActiveBlockersPanel({ ) : (
{blockedTasks.map((task) => ( - +
\uD83D\uDD34
@@ -91,7 +91,7 @@ export function ActiveBlockersPanel({
)}
- +
)}
- +
@@ -231,7 +234,7 @@ export function CeoApprovalQueue({ className }: CeoApprovalQueueProps) { )}
- + @@ -348,6 +351,7 @@ export function CeoApprovalQueue({ className }: CeoApprovalQueueProps) {

)} Refresh - + diff --git a/panel/src/components/dashboard/pr-review-queue.tsx b/panel/src/components/dashboard/pr-review-queue.tsx index a1de8a99..9f93eca3 100644 --- a/panel/src/components/dashboard/pr-review-queue.tsx +++ b/panel/src/components/dashboard/pr-review-queue.tsx @@ -166,6 +166,7 @@ export function PrReviewQueue({ className }: PrReviewQueueProps) {
@@ -202,7 +203,11 @@ export function PrReviewQueue({ className }: PrReviewQueueProps) { )}
- + diff --git a/panel/src/components/dashboard/quick-actions-bar.tsx b/panel/src/components/dashboard/quick-actions-bar.tsx index 73472edf..e26699f3 100644 --- a/panel/src/components/dashboard/quick-actions-bar.tsx +++ b/panel/src/components/dashboard/quick-actions-bar.tsx @@ -17,42 +17,42 @@ export function QuickActionsBar() {
- + - + - + - + - + - + diff --git a/panel/src/components/prompter/board-review-sent-card.tsx b/panel/src/components/prompter/board-review-sent-card.tsx index 59b4618b..f585de79 100644 --- a/panel/src/components/prompter/board-review-sent-card.tsx +++ b/panel/src/components/prompter/board-review-sent-card.tsx @@ -73,6 +73,7 @@ export function BoardReviewSentCard({ diff --git a/panel/src/components/tasks/task-detail/task-metadata.tsx b/panel/src/components/tasks/task-detail/task-metadata.tsx index 6beca612..f219a98b 100644 --- a/panel/src/components/tasks/task-detail/task-metadata.tsx +++ b/panel/src/components/tasks/task-detail/task-metadata.tsx @@ -507,6 +507,7 @@ export function TaskMetadata({ task }: TaskMetadataProps) {
{task.project_id && project ? ( diff --git a/panel/src/components/tasks/task-detail/work-session-card.tsx b/panel/src/components/tasks/task-detail/work-session-card.tsx index eb7b94d9..d72c11b2 100644 --- a/panel/src/components/tasks/task-detail/work-session-card.tsx +++ b/panel/src/components/tasks/task-detail/work-session-card.tsx @@ -231,7 +231,7 @@ export function WorkSessionCard({ taskId }: WorkSessionCardProps) { {/* View Full Session Link */}
- + diff --git a/panel/src/lib/api/tasks.ts b/panel/src/lib/api/tasks.ts index 77bcc808..1ae6bd0f 100644 --- a/panel/src/lib/api/tasks.ts +++ b/panel/src/lib/api/tasks.ts @@ -31,8 +31,66 @@ export interface BoardReviewEntry { timestamp: string | null; } +// Wire shape of GET /tasks/summary (backend TaskSummaryResponse) — exactly +// the fields list views render; everything fat stays on GET /tasks/{id}. +interface TaskSummaryWire { + id: string; + title: string; + status: TaskStatus; + priority: number; + team: Team; + assigned_to: string | null; + created_at: string; + updated_at: string | null; + estimated_complexity: Complexity; + nature: TaskNature; + task_type: TaskType; + sequence: number; + parent_task_id: string | null; + batch_id: string | null; + project_id: string | null; + product_id: string | null; + branch_name: string | null; + pr_number: number | null; + pr_url: string | null; + pr_created: boolean; + docs_complete: boolean; + completed_at: string | null; + board_review_complete: boolean; + description_snippet: string | null; +} + +// Normalize a summary into the Task shape so list consumers keep their +// types. Defaulted fields are never rendered by list views (verified in the +// 2026-07-02 audit); anything needing them must fetch the full task. +const summaryToTask = (s: TaskSummaryWire): Task => ({ + ...s, + description: s.description_snippet ?? "", + acceptance_criteria: [], + created_by: "", + dependency_ids: [], + blocker_ids: [], + claimed_at: null, + started_at: null, + target_date: null, + pm_approvals: {}, + plan: null, + checkpoints: [], + progress_updates: [], + commits: [], + dev_notes: null, + qa_notes: null, + auditor_notes: null, + quick_context: null, + self_verified: false, + qa_verified: null, + sessions: [], +}); + export const tasksApi = { - // List tasks with optional filters + // List tasks with optional filters — served by the trimmed summary route + // (~50x lighter than the full TaskResponse list that measured 2MB and made + // every page slow, 2026-07-02). Detail views fetch the full task via get(). list: async (filters?: TaskFilters): Promise => { if (isMockMode()) { let tasks = [...mockTasks]; @@ -49,10 +107,31 @@ export const tasksApi = { if (filters?.status) params.append("status", filters.status); if (filters?.team) params.append("team", filters.team); if (filters?.limit) params.append("limit", String(filters.limit)); - if (filters?.offset) params.append("offset", String(filters.offset)); - const url = "/tasks?" + params.toString(); - const { data } = await api.get(url); + const url = "/tasks/summary?" + params.toString(); + const { data } = await api.get(url); + return data.map(summaryToTask); + }, + + // Full-fat list for consumers that render fields beyond the summary + // (the CEO approval queue shows quick_context). Hits the heavy /tasks + // route — keep the filter narrow and the limit small. + listFull: async (filters?: TaskFilters): Promise => { + if (isMockMode()) { + let tasks = [...mockTasks]; + if (filters?.status) { + tasks = tasks.filter((t) => t.status === filters.status); + } + if (filters?.team) { + tasks = tasks.filter((t) => t.team === filters.team); + } + return tasks; + } + const params = new URLSearchParams(); + if (filters?.status) params.append("status", filters.status); + if (filters?.team) params.append("team", filters.team); + params.append("limit", String(filters?.limit ?? 100)); + const { data } = await api.get("/tasks?" + params.toString()); return data; }, diff --git a/roboco/api/routes/tasks.py b/roboco/api/routes/tasks.py index 6f1d9e2c..510b1bdf 100644 --- a/roboco/api/routes/tasks.py +++ b/roboco/api/routes/tasks.py @@ -38,11 +38,13 @@ from roboco.api.schemas.tasks import ( TaskCountResponse, TaskResponse, TaskSessionLinkResponse, + TaskSummaryResponse, TaskUpdate, TeamTasksQuery, ValidTransitionsResponse, enrich_task_with_context, task_list_to_response, + task_list_to_summary_response, task_to_response, transform_update_data, ) @@ -619,22 +621,63 @@ async def list_tasks( elif effective_team: tasks = await service.list_by_team(effective_team, limit=limit) elif status: - tasks = await service.list_by_status(status) + # list_by_status has no limit param — slice so the status-only + # branch can't return the whole table (it silently skipped the + # declared limit until 2026-07-02). + tasks = (await service.list_by_status(status))[:limit] else: tasks = await service.list_all(limit) return task_list_to_response(tasks) +@router.get("/summary", response_model=list[TaskSummaryResponse]) +async def list_tasks_summary( + db: DbSession, + agent: CurrentAgentContext, + team: Team | None = None, + status: TaskStatus | None = None, + limit: Annotated[int, Query(ge=1, le=1000)] = 500, +) -> list[TaskSummaryResponse]: + """List tasks as trimmed summaries for panel list views. + + Same filters and view permissions as the full list, ~50x lighter per + task: no description/plan/progress/commits/notes. The panel task tree + needs the whole set at once, so the default limit is higher than the + full route's. + """ + service = get_task_service(db) + permissions = get_permission_service() + + effective_team = team + if not permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL): + if agent.team: + effective_team = agent.team + else: + return [] + + if effective_team and status: + tasks = await service.list_by_team(effective_team, status, limit) + elif effective_team: + tasks = await service.list_by_team(effective_team, limit=limit) + elif status: + tasks = (await service.list_by_status(status))[:limit] + else: + tasks = await service.list_all(limit) + + return task_list_to_summary_response(tasks) + + @router.get("/my", response_model=list[TaskResponse]) async def get_my_tasks( db: DbSession, agent: CurrentAgentContext, status: TaskStatus | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get tasks assigned to the current agent.""" service = get_task_service(db) - tasks = await service.list_by_assignee(agent.agent_id, status) + tasks = (await service.list_by_assignee(agent.agent_id, status))[:limit] return task_list_to_response(tasks) @@ -644,6 +687,7 @@ async def get_pending_tasks( agent: CurrentAgentContext, permissions: PermissionServiceDep, team: Team | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get pending tasks available to claim.""" service = get_task_service(db) @@ -652,7 +696,7 @@ async def get_pending_tasks( 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_pending(effective_team) + tasks = (await service.list_pending(effective_team))[:limit] return task_list_to_response(tasks) @@ -662,6 +706,7 @@ async def get_blocked_tasks( agent: CurrentAgentContext, permissions: PermissionServiceDep, team: Team | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get blocked tasks.""" service = get_task_service(db) @@ -670,7 +715,7 @@ async def get_blocked_tasks( 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_blocked(effective_team) + tasks = (await service.list_blocked(effective_team))[:limit] return task_list_to_response(tasks) @@ -680,6 +725,7 @@ async def get_awaiting_qa_tasks( agent: CurrentAgentContext, permissions: PermissionServiceDep, team: Team | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get tasks awaiting QA review.""" service = get_task_service(db) @@ -688,7 +734,7 @@ async def get_awaiting_qa_tasks( 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_qa(effective_team) + tasks = (await service.list_awaiting_qa(effective_team))[:limit] return task_list_to_response(tasks) @@ -698,6 +744,7 @@ async def get_awaiting_docs_tasks( agent: CurrentAgentContext, permissions: PermissionServiceDep, team: Team | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get tasks awaiting documentation.""" service = get_task_service(db) @@ -706,7 +753,7 @@ async def get_awaiting_docs_tasks( 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_docs(effective_team) + tasks = (await service.list_awaiting_docs(effective_team))[:limit] return task_list_to_response(tasks) @@ -783,6 +830,7 @@ async def get_awaiting_pm_review_tasks( agent: CurrentAgentContext, permissions: PermissionServiceDep, team: Team | None = None, + limit: Annotated[int, Query(ge=1, le=500)] = 200, ) -> list[TaskResponse]: """Get tasks awaiting PM review.""" service = get_task_service(db) @@ -791,7 +839,7 @@ async def get_awaiting_pm_review_tasks( 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) + tasks = (await service.list_awaiting_pm_review(effective_team))[:limit] return task_list_to_response(tasks) @@ -818,7 +866,7 @@ async def get_awaiting_ceo_approval_tasks( ) service = get_task_service(db) - tasks = await service.list_awaiting_ceo_approval() + tasks = (await service.list_awaiting_ceo_approval())[:200] return task_list_to_response(tasks) @@ -845,7 +893,7 @@ async def get_external_pr_reviews( detail="Only PMs and management can view the PR-review queue", ) service = get_task_service(db) - tasks = await service.list_external_pr_reviews() + tasks = (await service.list_external_pr_reviews())[:200] return task_list_to_response(tasks) @@ -1138,7 +1186,7 @@ async def get_subtasks( ) -> list[TaskResponse]: """Get subtasks of a task.""" service = get_task_service(db) - tasks = await service.get_subtasks(task_id) + tasks = (await service.get_subtasks(task_id))[:500] return task_list_to_response(tasks) @@ -1149,7 +1197,7 @@ async def get_descendants( ) -> list[TaskResponse]: """Get ALL descendants of a task (recursive - children, grandchildren, etc.).""" service = get_task_service(db) - tasks = await service.get_all_descendants(task_id) + tasks = (await service.get_all_descendants(task_id))[:500] return task_list_to_response(tasks) diff --git a/roboco/api/schemas/tasks.py b/roboco/api/schemas/tasks.py index 2f6d3c66..011e7d62 100644 --- a/roboco/api/schemas/tasks.py +++ b/roboco/api/schemas/tasks.py @@ -382,7 +382,13 @@ class TaskResponse(BaseModel): class TaskSummaryResponse(BaseModel): - """Lightweight task response for list views.""" + """Lightweight task response for list views. + + Carries exactly what the panel's list surfaces render — the task tree + (parent/sequence), kanban card (type/snippet), and git badge (pr/branch) + — and none of the fat columns (description, plan, progress_updates, + commits, notes). Full payloads stay on /tasks/{id}. + """ id: UUID title: str @@ -394,10 +400,67 @@ class TaskSummaryResponse(BaseModel): updated_at: datetime | None estimated_complexity: Complexity nature: TaskNature + task_type: TaskType + sequence: int + parent_task_id: UUID | None = None + batch_id: UUID | None = None + project_id: UUID | None = None + product_id: UUID | None = None + branch_name: str | None = None + pr_number: int | None = None + pr_url: str | None = None + pr_created: bool = False + docs_complete: bool = False + # Client-side velocity metrics filter on completion time; the CEO + # approval queue gates its button on board_review_complete. + completed_at: datetime | None = None + board_review_complete: bool = False + description_snippet: str | None = None model_config = ConfigDict(from_attributes=True) +_SUMMARY_SNIPPET_LEN = 200 + + +def task_to_summary_response(task: "TaskTable") -> TaskSummaryResponse: + """Trimmed list-view conversion — no fat JSON columns serialized.""" + snippet = (task.description or "")[:_SUMMARY_SNIPPET_LEN] or None + return TaskSummaryResponse( + id=require_uuid(task.id), + title=task.title, + status=task.status, + priority=task.priority, + team=task.team, + assigned_to=to_python_uuid(task.assigned_to), + created_at=task.created_at, + updated_at=task.updated_at, + estimated_complexity=task.estimated_complexity, + nature=task.nature, + task_type=task.task_type, + sequence=task.sequence, + parent_task_id=to_python_uuid(task.parent_task_id), + batch_id=to_python_uuid(task.batch_id), + project_id=to_python_uuid(task.project_id), + product_id=to_python_uuid(task.product_id), + branch_name=getattr(task, "branch_name", None), + pr_number=getattr(task, "pr_number", None), + pr_url=getattr(task, "pr_url", None), + pr_created=task.pr_created, + docs_complete=task.docs_complete, + completed_at=task.completed_at, + board_review_complete=task.board_review_complete, + description_snippet=snippet, + ) + + +def task_list_to_summary_response( + tasks: list["TaskTable"], +) -> list[TaskSummaryResponse]: + """Convert list of TaskTable to trimmed summaries.""" + return [task_to_summary_response(t) for t in tasks] + + class ProgressRequest(BaseModel): """Request to add progress update. diff --git a/roboco/foundation/policy/agent_loop.py b/roboco/foundation/policy/agent_loop.py index 461258c7..565953a5 100644 --- a/roboco/foundation/policy/agent_loop.py +++ b/roboco/foundation/policy/agent_loop.py @@ -42,6 +42,13 @@ class BudgetPolicy: # unblock journal-decision gate). After this many consecutive resets the # gap stops counting as progress and strikes accrue, so the loop gate fires. pm_respawn_max_tracing_resets: int = 3 + # A status CHANGE normally resets the unproductive counter (forward + # progress). That reset is also bounded for REVISITED statuses: an + # A<->B oscillation (blocked <-> in_progress) changes status on every + # spawn yet advances nothing — 2026-07-02 a dev looped for two hours + # (8 spawns) without ever tripping the gate. A status never seen before + # on this (agent, task) still always fully resets. + pm_respawn_max_revisit_resets: int = 2 verb_retry_max_per_minute: int = 3 # default cap for verbs not in VERB_RETRY_LIMITS diff --git a/roboco/foundation/policy/lifecycle.py b/roboco/foundation/policy/lifecycle.py index 1d8ba231..843e8958 100644 --- a/roboco/foundation/policy/lifecycle.py +++ b/roboco/foundation/policy/lifecycle.py @@ -429,7 +429,7 @@ _ATOMIC_ACTIONS: dict[str, ActionSpec] = { allowed_task_types=None, preconditions=(), self_review_block=False, - needs_team_match=False, + needs_team_match=True, ), # claim's source_statuses is the UNION across all roles — see CLAIM_RULES # for per-role authority. Both tables are authoritative; a validator @@ -494,7 +494,7 @@ _ATOMIC_ACTIONS: dict[str, ActionSpec] = { allowed_task_types=None, preconditions=(), self_review_block=False, - needs_team_match=False, + needs_team_match=True, ), "pause": ActionSpec( name="pause", @@ -514,7 +514,7 @@ _ATOMIC_ACTIONS: dict[str, ActionSpec] = { allowed_task_types=None, preconditions=(), self_review_block=False, - needs_team_match=False, + needs_team_match=True, ), "submit_verification": ActionSpec( name="submit_verification", @@ -1767,7 +1767,7 @@ def can_invoke_action( # service layer, so a consumer trusting the spec gate alone let a backend # dev claim a frontend task). When the caller supplies the agent's team via # Context, enforce it here; absent, defer to the service layer. - rejection = _check_team_match(spec_action, task, ctx) + rejection = _check_team_match(spec_action, task, ctx, role) if rejection is not None: return rejection if action == "claim": @@ -1777,18 +1777,39 @@ def can_invoke_action( return Decision.allow() +# Org-wide actors act across cells by design: the Main PM absorbs every +# cell's escalations, the board PR reviewer gates root PRs on the main_pm +# team, and CEO / board decisions are global. Cell-scoped roles (developer, +# qa, documenter, cell_pm) are the ones a dispatch misroute can weaponize — +# live 2026-07-02 a frontend cell PM blocked, escalated, and briefly held a +# backend task through exactly this gap. +_ORG_WIDE_ROLES: frozenset[Role] = frozenset( + { + Role.MAIN_PM, + Role.CEO, + Role.PRODUCT_OWNER, + Role.HEAD_MARKETING, + Role.AUDITOR, + Role.PR_REVIEWER, + } +) + + def _check_team_match( - spec_action: ActionSpec, task: Any, ctx: Context + spec_action: ActionSpec, task: Any, ctx: Context, role: Role | None = None ) -> Decision | None: """Reject a cross-team action when the caller's team is known. ``needs_team_match`` was enforced only at the service layer, so a consumer trusting the spec gate alone let a backend dev claim a frontend task. When the caller supplies the agent's team via Context, enforce it here; absent, - defer to the service layer (backward compatible). + defer to the service layer (backward compatible). Org-wide roles + (``_ORG_WIDE_ROLES``) are exempt. """ if not spec_action.needs_team_match: return None + if role is not None and role in _ORG_WIDE_ROLES: + return None agent_team = getattr(ctx, "agent_team", None) if agent_team is None: return None diff --git a/roboco/mcp/flow_server.py b/roboco/mcp/flow_server.py index c9bc6006..52dc54fa 100644 --- a/roboco/mcp/flow_server.py +++ b/roboco/mcp/flow_server.py @@ -816,6 +816,10 @@ def delegate( acceptance_criteria: StrList, estimated_complexity: str = "medium", covers_parent_criteria: StrList | None = None, + intends_to_touch: StrList | None = None, + adds_migration: bool = False, + touches_shared: bool = False, + depends_on: StrList | None = None, ) -> dict[str, Any]: """PM: create a subtask of parent_task_id. @@ -835,6 +839,14 @@ def delegate( EVERY parent criterion is claimed by a subtask and satisfied before the parent rolls up — split the parent's criteria across subtasks so their union covers all of them. + intends_to_touch: Collision surface — file paths/globs this subtask + will modify. REQUIRED for task_type="code": the sibling collision + DAG can only sequence what is declared. + adds_migration: True if the subtask adds a DB migration (migration + adders are chained serially). + touches_shared: True if the subtask edits a shared surface. + depends_on: Task UUIDs this subtask must wait for — wired verbatim as + dependency edges (use for ordering the surface rules would miss). """ return _post( _role_path("delegate"), @@ -849,6 +861,10 @@ def delegate( "acceptance_criteria": acceptance_criteria, "estimated_complexity": estimated_complexity, "covers_parent_criteria": covers_parent_criteria, + "intends_to_touch": intends_to_touch, + "adds_migration": adds_migration, + "touches_shared": touches_shared, + "depends_on": depends_on, }, ) diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 340dbe2b..fc100c3c 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -599,7 +599,9 @@ GATEWAY_ENABLED_ROLES: frozenset[str] = frozenset( ) -def _build_manifest_for_agent(agent_id: str, model: str) -> Path | None: +def _build_manifest_for_agent( + agent_id: str, model: str, workspace_path: str | None = None +) -> Path | None: """Write a SpawnManifest for developer-role agents; return the host path. Returns ``None`` for roles outside ``GATEWAY_ENABLED_ROLES`` so callers @@ -608,6 +610,12 @@ def _build_manifest_for_agent(agent_id: str, model: str) -> Path | None: Args: agent_id: Agent slug (e.g. ``be-dev-1``). model: Resolved model name passed to ``SpawnInputs.agent_model``. + workspace_path: The task-resolved workspace (project clone or per-task + worktree) — the SAME path the container ``-w`` uses. Without it + the manifest falls back to the agent's roboco-project workspace, + which is WRONG for any other project's task (live 2026-07-02: + be-dev-2's manifest pointed at /data/workspaces/roboco while the + task lived in guard-core-saas-backend). Returns: Absolute host path to the written JSON file, or ``None``. @@ -631,14 +639,18 @@ def _build_manifest_for_agent(agent_id: str, model: str) -> Path | None: raw_uuid = AGENT_UUIDS.get(agent_id) agent_uuid = UUID(raw_uuid) if raw_uuid else __import__("uuid").uuid4() - workspace_path = Path(settings.workspaces_root) / "roboco" / team / agent_id + resolved_workspace = ( + Path(workspace_path) + if workspace_path + else Path(settings.workspaces_root) / "roboco" / team / agent_id + ) manifest = build_for_role( SpawnInputs( agent_id=agent_uuid, role=role, team=team, - workspace_path=workspace_path, + workspace_path=resolved_workspace, agent_model=model, ) ) @@ -2601,7 +2613,13 @@ class AgentOrchestrator: # Spawn manifest + gateway flag — developer role only in Phase 1. # _build_manifest_for_agent writes the JSON file to the host and # returns the path; other roles get None and the gateway flag stays off. - manifest_host_path = _build_manifest_for_agent(config.agent_id, subagent_model) + # workspace_path mirrors the container -w (same resolver) so the + # manifest never claims a different directory than the shell. + manifest_host_path = _build_manifest_for_agent( + config.agent_id, + subagent_model, + workspace_path=AgentOrchestrator._resolve_workspace_cwd(config), + ) if manifest_host_path: cmd.extend( [ @@ -2621,6 +2639,27 @@ class AgentOrchestrator: ) _ROLES_WITH_CELL_WORKSPACE: ClassVar[frozenset[str]] = frozenset({"documenter"}) + @staticmethod + def _resolve_workspace_cwd(config: AgentConfig) -> str | None: + """The task-resolved workspace path for this spawn, or None. + + Single source of truth consumed by BOTH the container ``-w`` and the + spawn manifest's ``workspace_path`` — they must agree, or the agent's + prompt claims one directory while its shell sits in another (live + 2026-07-02: manifest said the roboco workspace for a guard-core task). + """ + role = get_agent_role(config.agent_id) or "developer" + team = get_agent_team(config.agent_id) or "" + project = _resolve_project_slug_from_git_context(config.git_context) + if role in AgentOrchestrator._ROLES_WITH_AGENT_WORKSPACE: + # Per-task worktree when the task has a branch (F123), else the + # clone root. _agent_cwd_path is the SAME formula the Edit/Write + # allowlist is built from, so -w and the allowlist match exactly. + return _agent_cwd_path(project, team, config.agent_id, config.git_context) + if role in AgentOrchestrator._ROLES_WITH_CELL_WORKSPACE: + return _cell_workspace_path(project, team) + return None + @staticmethod def _append_workspace_cwd(cmd: list[str], config: AgentConfig) -> None: """Set the container -w to the agent or cell workspace by role.""" @@ -2630,25 +2669,13 @@ class AgentOrchestrator: # the workspace clone. Without this, container WORKDIR (/app from the # Dockerfile) shadows the workspace and every file op fails. # - # Mirror the workspace-path selection in _get_role_permissions exactly: + # Workspace selection lives in _resolve_workspace_cwd: # - developer / product_owner / head_marketing: per-agent workspace # - documenter: cell workspace # - qa / cell_pm / main_pm / auditor: no write workspace → omit -w - role = get_agent_role(config.agent_id) or "developer" - team = get_agent_team(config.agent_id) or "" - project = _resolve_project_slug_from_git_context(config.git_context) - if role in AgentOrchestrator._ROLES_WITH_AGENT_WORKSPACE: - # Per-task worktree when the task has a branch (F123), else the - # clone root. _agent_cwd_path is the SAME formula the Edit/Write - # allowlist is built from, so -w and the allowlist match exactly. - cmd.extend( - [ - "-w", - _agent_cwd_path(project, team, config.agent_id, config.git_context), - ] - ) - elif role in AgentOrchestrator._ROLES_WITH_CELL_WORKSPACE: - cmd.extend(["-w", _cell_workspace_path(project, team)]) + workspace = AgentOrchestrator._resolve_workspace_cwd(config) + if workspace is not None: + cmd.extend(["-w", workspace]) @staticmethod def _append_agent_auth_env(cmd: list[str], config: AgentConfig) -> None: @@ -9791,6 +9818,58 @@ Start now: evidence(task_id="{task_id}") # Use foundation's default; keep the local name for back-compat. _PM_RESPAWN_MAX_UNPRODUCTIVE = _AGENT_LOOP_BUDGET.pm_respawn_max_unproductive _PM_RESPAWN_MAX_TRACING_RESETS = _AGENT_LOOP_BUDGET.pm_respawn_max_tracing_resets + _PM_RESPAWN_MAX_REVISIT_RESETS = _AGENT_LOOP_BUDGET.pm_respawn_max_revisit_resets + + def _respawn_status_change_resets( + self, + key: tuple[str, Any], + record: dict[str, Any], + current_status: Any, + now: datetime, + ) -> bool: + """Handle a status CHANGE; True when it resets the strike counter. + + A status never seen on this (agent, task) is genuine forward progress + and fully resets, exactly as before. A REVISITED status — the A<->B + oscillation (blocked <-> in_progress) that changes status on every + spawn while advancing nothing (2026-07-02: a dev looped 2h/8 spawns + without tripping the gate) — gets a bounded reset budget mirroring + tracing_resets, after which strikes accrue. seen_statuses is + in-memory only (not a tracker column): after a restart it rebuilds + from observed statuses, which can only under-gate briefly — never + over-gate. + """ + agent_slug, task_id = key + seen = record.get("seen_statuses") or [record.get("last_status")] + if current_status not in seen: + self._pm_respawn_tracker[key] = { + "count": 1, + "last_status": current_status, + "last_check": now, + "seen_statuses": [*seen, current_status], + } + self._schedule_respawn_persist( + agent_slug, str(task_id), self._pm_respawn_tracker[key] + ) + return True + record["last_status"] = current_status + revisits = record.get("revisit_resets", 0) + if revisits < self._PM_RESPAWN_MAX_REVISIT_RESETS: + record["revisit_resets"] = revisits + 1 + record["count"] = 1 + record["last_check"] = now + record["notified"] = False + self._schedule_respawn_persist(agent_slug, str(task_id), record) + return True + logger.warning( + "PM respawn status ping-pong budget exhausted — " + "revisited statuses no longer reset the strike counter", + agent_id=agent_slug, + task_id=str(task_id), + task_status=current_status, + revisit_resets=revisits, + ) + return False async def _pm_respawn_should_gate( self, agent_slug: str, task: dict[str, Any] @@ -9827,16 +9906,21 @@ Start now: evidence(task_id="{task_id}") current_status = task.get("status") record = self._pm_respawn_tracker.get(key) now = datetime.now(UTC) - if record is None or record.get("last_status") != current_status: + if record is None: self._pm_respawn_tracker[key] = { "count": 1, "last_status": current_status, "last_check": now, + "seen_statuses": [current_status], } self._schedule_respawn_persist( agent_slug, str(task_id), self._pm_respawn_tracker[key] ) return False + if record.get("last_status") != current_status and ( + self._respawn_status_change_resets(key, record, current_status, now) + ): + return False # Same status as last spawn — could be a stuck loop OR a # rule-following retry. A tracing_gap normally means the agent is # advancing through a verb chain, so reset the strike counter — but @@ -10884,20 +10968,16 @@ Never `commit`, never write code, never run `git`. PMs coordinate. # QA already running, they'll pick up on scan continue - # Respawn circuit breaker — before claiming, so a wedged QA task - # doesn't churn claims while the gate is open. + # Respawn circuit breaker — same progress-aware gate as every + # other task-keyed spawn path. if await self._pm_respawn_should_gate(agent_id, task): continue - # Claim the task for QA agent BEFORE spawning - if not await self._claim_task_for_agent(client, task["id"], agent_id): - logger.warning( - "Failed to claim awaiting_qa task for QA", - task_id=task["id"], - agent_id=agent_id, - ) - continue - - # Spawn QA agent with task assignment + # NO pre-claim (matches _spawn_assigned_qa and the external-PR + # reviewer dispatch): the transitioning claim moved the task to + # 'claimed' before the agent existed, stranding the QA whose own + # claim_review/pass_review demand awaiting_qa (live 2026-07-02, + # ba7b751c). The agent claims itself via claim_review; the + # _is_agent_active guard prevents a double-spawn across ticks. await self.spawn_agent( agent_id=agent_id, task_id=task["id"], diff --git a/roboco/security.py b/roboco/security.py index 5b2f73f8..6bbd1587 100644 --- a/roboco/security.py +++ b/roboco/security.py @@ -1,4 +1,4 @@ -"""RoboCo HTTP security layer — fastapi-guard 7.2.0 / guard-core 3.3.0. +"""RoboCo HTTP security layer — fastapi-guard 7.2.2 / guard-core 3.3.0. A ``SecurityMiddleware`` + per-route decorator layer, gated by ``settings.guard_enabled`` (default off). Importing this module is always safe: diff --git a/roboco/services/git.py b/roboco/services/git.py index 6c6f89b3..55343297 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -4035,6 +4035,24 @@ class GitService(BaseService): unmerged = [line for line in cherry.stdout.splitlines() if line.startswith("+")] if not unmerged: return None + # Squash-merge relief: cherry can't patch-match N child commits against + # the one squashed commit, but every commit (incl. the squash) carries + # the [taskid8] prefix — a marker commit on the parent proves the child + # landed (live false positive 2026-07-02: 3 squash-merged children). + marker = await self._run_git( + workspace, + [ + "log", + f"origin/{parent_branch}", + "--grep", + rf"\[{str(child.id)[:8]}\]", + "--oneline", + "-1", + ], + check=False, + ) + if marker.returncode == 0 and marker.stdout.strip(): + return None return { "task_id": str(child.id)[:8], "title": str(getattr(child, "title", ""))[:80], @@ -4341,10 +4359,26 @@ class GitService(BaseService): await self._run_git( workspace, ["fetch", "origin", branch_name], check=False, token=token ) - if await self._ref_exists(workspace, branch_name): + origin_ref = f"origin/{branch_name}" + local_exists = await self._ref_exists(workspace, branch_name) + origin_exists = await self._ref_exists(workspace, origin_ref) + if local_exists and origin_exists: + # An assembled branch advances on ORIGIN when child PRs merge on + # GitHub, while the inspecting clone's local ref stays parked — a + # diff off the stale local ref re-flags work that already landed + # (live 2026-07-02: two false pr_fails on the S6 cell PR). Prefer + # origin when the local ref is strictly behind it; a local ref + # that is ahead (unpushed) or diverged keeps priority. + behind = await self._run_git( + workspace, + ["merge-base", "--is-ancestor", branch_name, origin_ref], + check=False, + ) + return origin_ref if behind.returncode == 0 else branch_name + if local_exists: return branch_name - if await self._ref_exists(workspace, f"origin/{branch_name}"): - return f"origin/{branch_name}" + if origin_exists: + return origin_ref return branch_name async def diff( diff --git a/tests/foundation/test_team_match_context.py b/tests/foundation/test_team_match_context.py new file mode 100644 index 00000000..7b5133a7 --- /dev/null +++ b/tests/foundation/test_team_match_context.py @@ -0,0 +1,104 @@ +"""Team-match must actually fire: the spec gate rejects cross-team actors. + +Live 2026-07-02: a frontend cell PM was dispatched onto a BACKEND task's +review, then blocked it, escalated it, and briefly held it — a 40-minute +ownership tug-of-war. Seventeen ActionSpecs carry needs_team_match=True and +_check_team_match enforces it — but only when the caller supplies +Context.agent_team, which no choreographer site did, so the gate sat in its +permissive fallback forever. These tests pin the policy behavior the +choreographer sweep wires up. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any, cast +from uuid import uuid4 + +from roboco.foundation.policy.lifecycle import ( + Context, + Role, + can_invoke_intent, +) +from roboco.models.base import TaskStatus + + +def _task(**overrides: Any) -> Any: + base: dict[str, Any] = { + "id": uuid4(), + "status": TaskStatus.IN_PROGRESS, + "team": "backend", + "assigned_to": None, + "task_type": "code", + } + base.update(overrides) + return cast("Any", SimpleNamespace(**base)) + + +def test_cross_team_developer_is_rejected_when_team_supplied() -> None: + decision = can_invoke_intent( + Role.DEVELOPER, + "i_am_blocked", + _task(team="backend", status=TaskStatus.IN_PROGRESS, task_type="code"), + Context(actor_id=uuid4(), agent_team="frontend"), + ) + assert not decision.allowed + assert "team" in (decision.message or "").lower() + + +def test_cross_team_cell_pm_resume_is_rejected() -> None: + decision = can_invoke_intent( + Role.CELL_PM, + "resume", + _task(team="backend", status=TaskStatus.PAUSED), + Context(actor_id=uuid4(), agent_team="frontend"), + ) + assert not decision.allowed + assert "team" in (decision.message or "").lower() + + +def test_same_team_developer_is_allowed() -> None: + decision = can_invoke_intent( + Role.DEVELOPER, + "i_am_blocked", + _task(team="backend", status=TaskStatus.IN_PROGRESS, task_type="code"), + Context(actor_id=uuid4(), agent_team="backend"), + ) + assert decision.allowed + + +def test_missing_team_keeps_permissive_fallback() -> None: + """Absent agent_team defers to the service layer (backward compatible).""" + decision = can_invoke_intent( + Role.DEVELOPER, + "i_am_blocked", + _task(team="backend", status=TaskStatus.IN_PROGRESS, task_type="code"), + Context(actor_id=uuid4()), + ) + assert decision.allowed + + +def test_org_wide_roles_are_exempt_cross_team() -> None: + """Main PM handles every cell's escalations; the exemption keeps that.""" + for role, verb, status in ( + (Role.MAIN_PM, "resume", TaskStatus.PAUSED), + (Role.MAIN_PM, "unblock", TaskStatus.BLOCKED), + ): + decision = can_invoke_intent( + role, + verb, + _task(team="backend", status=status), + Context(actor_id=uuid4(), agent_team="main_pm"), + ) + assert decision.allowed, f"{role} {verb} must stay org-wide" + + +def test_cross_team_cell_pm_unblock_is_rejected() -> None: + decision = can_invoke_intent( + Role.CELL_PM, + "unblock", + _task(team="backend", status=TaskStatus.BLOCKED), + Context(actor_id=uuid4(), agent_team="frontend"), + ) + assert not decision.allowed + assert "team" in (decision.message or "").lower() diff --git a/tests/unit/api/test_task_summary_response.py b/tests/unit/api/test_task_summary_response.py new file mode 100644 index 00000000..75818850 --- /dev/null +++ b/tests/unit/api/test_task_summary_response.py @@ -0,0 +1,144 @@ +"""Task list summary mode — trimmed payloads for panel list views. + +The panel fetched /api/tasks unbounded and full-fat (2MB measured live, +2026-07-02): every list row shipped description, plan, progress_updates, +commits, notes. TaskSummaryResponse existed but was dead code. These tests +pin the wired-up summary path: the converter carries exactly the fields +list views render (tree, kanban card, git badge), excludes the fat columns, +and the /summary route is registered before /{task_id} so it can't be +swallowed by the UUID path match. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from types import SimpleNamespace +from typing import TYPE_CHECKING, Any, cast +from unittest.mock import AsyncMock, MagicMock, patch +from uuid import uuid4 + +import pytest +from roboco.api.routes import tasks as routes_mod +from roboco.api.routes.tasks import router +from roboco.api.schemas.tasks import ( + _SUMMARY_SNIPPET_LEN, + task_list_to_summary_response, + task_to_summary_response, +) +from roboco.models.base import Complexity, TaskNature, TaskStatus, TaskType, Team + +if TYPE_CHECKING: + from roboco.db.tables import TaskTable + +_LIMIT = 2 + + +def _stub_task(**overrides: Any) -> TaskTable: + base: dict[str, Any] = { + "id": uuid4(), + "title": "t", + "description": "d" * (_SUMMARY_SNIPPET_LEN * 2 + 100), + "status": TaskStatus.PENDING, + "priority": 3, + "sequence": 1, + "nature": TaskNature.TECHNICAL, + "task_type": TaskType.CODE, + "team": Team.BACKEND, + "assigned_to": uuid4(), + "parent_task_id": uuid4(), + "batch_id": None, + "project_id": uuid4(), + "product_id": None, + "branch_name": "feature/backend/x", + "pr_number": 42, + "pr_url": "https://github.com/x/y/pull/42", + "pr_created": True, + "docs_complete": False, + "created_at": datetime.now(UTC), + "updated_at": datetime.now(UTC), + "completed_at": datetime.now(UTC), + "board_review_complete": True, + "estimated_complexity": Complexity.MEDIUM, + } + base.update(overrides) + return cast("TaskTable", SimpleNamespace(**base)) + + +def test_summary_carries_every_list_view_field() -> None: + t = _stub_task() + s = task_to_summary_response(t) + assert (s.id, s.title, s.status) == (t.id, "t", TaskStatus.PENDING) + assert s.parent_task_id == t.parent_task_id # tree build + assert s.sequence == 1 and s.task_type is TaskType.CODE # kanban card + assert (s.pr_number, s.pr_created, s.docs_complete) == ( + 42, + True, + False, + ) # git badge + assert s.branch_name == "feature/backend/x" + assert s.project_id == t.project_id and s.product_id is None + # velocity metrics filter on completion time; the CEO approval queue + # gates on board_review_complete — both burned as gaps on 2026-07-02 + assert s.completed_at == t.completed_at + assert s.board_review_complete is True + + +def test_summary_excludes_fat_fields_and_truncates_snippet() -> None: + s = task_to_summary_response(_stub_task()) + dump = s.model_dump() + for fat in ( + "description", + "plan", + "progress_updates", + "commits", + "quick_context", + "checkpoints", + "notes_structured", + "dev_notes", + "acceptance_criteria", + ): + assert fat not in dump, f"summary must not carry {fat}" + assert len(s.description_snippet or "") == _SUMMARY_SNIPPET_LEN + + +def test_summary_snippet_none_safe() -> None: + assert ( + task_to_summary_response(_stub_task(description=None)).description_snippet + is None + ) + assert ( + task_to_summary_response(_stub_task(description="")).description_snippet is None + ) + + +def test_summary_list_converter() -> None: + stubs = [_stub_task() for _ in range(_LIMIT)] + assert len(task_list_to_summary_response(stubs)) == len(stubs) + + +def test_summary_route_registered_before_task_id_route() -> None: + """/tasks/summary must not be swallowed by /tasks/{task_id} UUID parsing.""" + paths = [getattr(r, "path", "") for r in router.routes] + assert "/summary" in paths + assert paths.index("/summary") < paths.index("/{task_id}") + + +@pytest.mark.asyncio +async def test_summary_route_status_branch_respects_limit() -> None: + service = AsyncMock() + service.list_by_status.return_value = [_stub_task() for _ in range(_LIMIT * 3)] + permissions = MagicMock() + permissions.can_perform_task_action.return_value = True + agent = MagicMock(team=Team.BACKEND) + with ( + patch.object(routes_mod, "get_task_service", return_value=service), + patch.object(routes_mod, "get_permission_service", return_value=permissions), + ): + out = await routes_mod.list_tasks_summary( + db=MagicMock(), + agent=agent, + team=None, + status=TaskStatus.PENDING, + limit=_LIMIT, + ) + assert len(out) == _LIMIT diff --git a/tests/unit/mcp_servers/test_flow_server_delegate_surface_parity.py b/tests/unit/mcp_servers/test_flow_server_delegate_surface_parity.py new file mode 100644 index 00000000..8ad6f4ac --- /dev/null +++ b/tests/unit/mcp_servers/test_flow_server_delegate_surface_parity.py @@ -0,0 +1,147 @@ +"""The delegate MCP tool must be able to send every field the gate demands. + +Original bug (2026-07-02 live): TASK_AT_DELEGATE required ``intends_to_touch`` +on code delegations, but the MCP ``delegate`` tool had no such parameter — +PMs were rejected 4x with ``incomplete_input``, could never comply, and +blocked/escalated. Fleet-wide code-delegation wall. + +Invariant: every FieldRequirement in TASK_AT_DELEGATE (and TASK_AT_CREATE, +which it extends) is either a parameter of the MCP delegate tool or +server-resolved (never demanded from the caller). +""" + +from __future__ import annotations + +import importlib +import inspect +import json +from typing import TYPE_CHECKING, Any +from unittest.mock import MagicMock + +import pytest +from roboco.foundation.policy.task_completeness import TASK_AT_DELEGATE + +if TYPE_CHECKING: + import types + from pathlib import Path + +# Fields the choreographer resolves server-side; the tool never sends them. +_SERVER_RESOLVED = {"project_id"} + + +def _pm_manifest() -> dict[str, object]: + return { + "agent_id": "00000000-0000-0000-0000-000000000098", + "role": "cell_pm", + "team": "frontend", + "workspace_path": "/tmp/test", + "flow_tools": ["delegate", "i_am_idle"], + "do_tools": [], + "read_tools": [], + "write_tools": [], + "bash_allowed": True, + "subagent_allowed": False, + "subagent_model": None, + "env": {}, + } + + +@pytest.fixture() +def flow_module_pm(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> types.ModuleType: + manifest_path = tmp_path / "tool-manifest.json" + manifest_path.write_text(json.dumps(_pm_manifest())) + monkeypatch.setenv("ROBOCO_AGENT_ID", "00000000-0000-0000-0000-000000000098") + monkeypatch.setenv("ROBOCO_AGENT_ROLE", "cell_pm") + monkeypatch.setenv("ROBOCO_ORCHESTRATOR_URL", "http://test-orchestrator:8000") + monkeypatch.setenv("ROBOCO_SDK_URL", "http://test-sdk:9000") + monkeypatch.setenv("ROBOCO_TOOL_MANIFEST_PATH", str(manifest_path)) + + import roboco.mcp.flow_server as srv + + importlib.reload(srv) + return srv + + +def test_delegate_tool_covers_every_gate_required_field( + flow_module_pm: types.ModuleType, +) -> None: + """Every TASK_AT_DELEGATE FieldRequirement is a delegate() parameter.""" + params = set(inspect.signature(flow_module_pm.delegate).parameters) + required = {req.field for req in TASK_AT_DELEGATE.requires} + missing = required - params - _SERVER_RESOLVED + assert not missing, ( + f"TASK_AT_DELEGATE demands fields the MCP delegate tool cannot send: " + f"{sorted(missing)}. A PM rejected with incomplete_input for these " + f"can NEVER comply — add them to flow_server.delegate and forward " + f"them in the payload." + ) + + +# Choreographer plan-depth gates hard-reject with `missing=[...]` naming these +# fields (_pm_sub_tasks_gate for i_will_plan; the dev rich-plan gate for +# i_will_work_on). The named tool must be able to send every one of them, or +# the rejected agent can never comply — the delegate/intends_to_touch wall. +_PLAN_GATE_FIELDS: dict[str, set[str]] = { + "i_will_plan": {"plan", "approach", "sub_tasks"}, + "i_will_work_on": {"plan", "steps", "technical_considerations", "risks"}, +} + + +def test_plan_gate_fields_are_tool_parameters( + flow_module_pm: types.ModuleType, +) -> None: + """Every field a plan gate can demand exists on the corresponding tool.""" + for verb, required in _PLAN_GATE_FIELDS.items(): + params = set(inspect.signature(getattr(flow_module_pm, verb)).parameters) + missing = required - params + assert not missing, ( + f"{verb} gate demands fields the MCP tool cannot send: " + f"{sorted(missing)} — same class as the delegate wall." + ) + + +def test_delegate_forwards_collision_surface_in_payload( + flow_module_pm: types.ModuleType, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The surface fields actually reach the POST body (not just the signature).""" + captured: dict[str, Any] = {} + + def _client_factory(*_a: object, **_kw: object) -> MagicMock: + client = MagicMock() + client.__enter__ = MagicMock(return_value=client) + client.__exit__ = MagicMock(return_value=False) + + def _post(url: str, **kwargs: object) -> MagicMock: + captured["url"] = url + captured["json"] = kwargs.get("json") + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = {"status": "ok"} + return resp + + client.post = _post + return client + + monkeypatch.setattr(flow_module_pm.httpx, "Client", _client_factory) + flow_module_pm.delegate( + parent_task_id="00000000-0000-0000-0000-000000000001", + title="t", + description="a description well over twenty chars", + assigned_to="fe-dev-1", + team="frontend", + task_type="code", + nature="technical", + acceptance_criteria=["done"], + intends_to_touch=["frontend/src/components/behavioral-content.tsx"], + adds_migration=False, + touches_shared=True, + depends_on=["00000000-0000-0000-0000-000000000002"], + ) + body = captured["json"] + assert body["intends_to_touch"] == [ + "frontend/src/components/behavioral-content.tsx" + ] + assert body["adds_migration"] is False + assert body["touches_shared"] is True + assert body["depends_on"] == ["00000000-0000-0000-0000-000000000002"] diff --git a/tests/unit/runtime/test_dispatch_qa_no_preclaim.py b/tests/unit/runtime/test_dispatch_qa_no_preclaim.py new file mode 100644 index 00000000..36ed11a6 --- /dev/null +++ b/tests/unit/runtime/test_dispatch_qa_no_preclaim.py @@ -0,0 +1,53 @@ +"""QA dispatch must not pre-claim the review task. + +Live 2026-07-02 (ba7b751c): the unassigned-QA branch claimed the task +BEFORE spawning (awaiting_qa -> claimed via the transitioning claim), then +spawned a QA agent whose own verbs demand awaiting_qa — claim_review bounced +("cannot claim from 'claimed'"), pass_review bounced, and the agent gave up +and unclaimed. The assigned-QA branch and the external-PR reviewer dispatch +both already spawn WITHOUT pre-claiming (the agent claims itself via +claim_review); the unassigned branch must match. +""" + +from __future__ import annotations + +from typing import Any, cast +from unittest.mock import AsyncMock, MagicMock +from uuid import uuid4 + +import pytest +from roboco.runtime.orchestrator import AgentOrchestrator + + +def _orch() -> tuple[AgentOrchestrator, AsyncMock, AsyncMock]: + orch = AgentOrchestrator.__new__(AgentOrchestrator) + orch._pm_respawn_tracker = {} + orch._bg_tasks = set() + any_orch = cast("Any", orch) + any_orch._is_task_handled_this_tick = lambda _tid: False + any_orch._select_agent_for_cell = lambda _team, _role: "be-qa" + any_orch._is_agent_active = lambda _slug: False + any_orch._pm_respawn_should_gate = AsyncMock(return_value=False) + any_orch._build_qa_prompt = lambda _t: "review it" + any_orch._task_git_context = lambda _t: None + claim = AsyncMock(return_value=True) + spawn = AsyncMock() + any_orch._claim_task_for_agent = claim + any_orch.spawn_agent = spawn + return orch, claim, spawn + + +@pytest.mark.asyncio +async def test_unassigned_qa_dispatch_spawns_without_preclaim() -> None: + orch, claim, spawn = _orch() + task = {"id": str(uuid4()), "team": "backend", "assigned_to": None} + cast("Any", orch)._fetch_tasks = AsyncMock(return_value=[task]) + + await orch._dispatch_qa_work(MagicMock()) + + claim.assert_not_awaited() + spawn.assert_awaited_once() + spawn_call = spawn.await_args + assert spawn_call is not None + assert spawn_call.kwargs["task_id"] == task["id"] + assert spawn_call.kwargs["agent_id"] == "be-qa" diff --git a/tests/unit/runtime/test_orchestrator_manifest.py b/tests/unit/runtime/test_orchestrator_manifest.py index e36d53a9..153d8a37 100644 --- a/tests/unit/runtime/test_orchestrator_manifest.py +++ b/tests/unit/runtime/test_orchestrator_manifest.py @@ -221,3 +221,40 @@ class TestBuildManifestForAgent: assert result is not None assert nested.exists() assert result.exists() + + +class TestManifestWorkspacePath: + """workspace_path must be the TASK-resolved workspace, not the roboco default. + + Live 2026-07-02: be-dev-2's manifest said /data/workspaces/roboco/... while + its task lived in guard-core-saas-backend — an agent trusting the manifest + hunts for its files in the wrong repository. + """ + + def test_workspace_override_reaches_manifest(self, tmp_path: Path) -> None: + worktree = ( + "/data/workspaces/guard-core-saas-backend/backend/be-dev-1" + "/.worktrees/abc12345" + ) + with patch("roboco.runtime.orchestrator.settings") as mock_settings: + mock_settings.manifest_host_dir = str(tmp_path) + mock_settings.workspaces_root = str(tmp_path / "workspaces") + + result = _build_manifest_for_agent( + "be-dev-1", "claude-sonnet-5", workspace_path=worktree + ) + + assert result is not None + data = json.loads(result.read_text()) + assert data["workspace_path"] == worktree + + def test_no_override_keeps_roboco_default(self, tmp_path: Path) -> None: + with patch("roboco.runtime.orchestrator.settings") as mock_settings: + mock_settings.manifest_host_dir = str(tmp_path) + mock_settings.workspaces_root = str(tmp_path / "workspaces") + + result = _build_manifest_for_agent("be-dev-1", "claude-sonnet-5") + + assert result is not None + data = json.loads(result.read_text()) + assert data["workspace_path"].endswith("workspaces/roboco/backend/be-dev-1") diff --git a/tests/unit/runtime/test_respawn_gate_oscillation.py b/tests/unit/runtime/test_respawn_gate_oscillation.py new file mode 100644 index 00000000..198ef016 --- /dev/null +++ b/tests/unit/runtime/test_respawn_gate_oscillation.py @@ -0,0 +1,94 @@ +"""The respawn breaker must not be fooled by status ping-pong. + +Live 2026-07-02: a dev looped blocked -> in_progress -> blocked for two hours +(8 spawns, 30 gateway rejections) and the breaker never tripped — every +status CHANGE fully reset the strike counter, and an A<->B oscillation +changes status on every spawn. A revisited status now gets a bounded reset +budget (mirroring tracing_resets); genuinely new statuses keep the full +reset so forward progress is never punished. +""" + +from __future__ import annotations + +from typing import Any, cast +from unittest.mock import AsyncMock, patch +from uuid import uuid4 + +import pytest +from roboco.runtime.orchestrator import AgentOrchestrator + + +def _new_orchestrator() -> AgentOrchestrator: + orch = AgentOrchestrator.__new__(AgentOrchestrator) + orch._pm_respawn_tracker = {} + orch._bg_tasks = set() + cast("Any", orch)._schedule_respawn_persist = lambda *_a, **_k: None + return orch + + +def _quiet_audit() -> AsyncMock: + audit = AsyncMock() + audit.has_recent_tracing_gap = AsyncMock(return_value=False) + return audit + + +@pytest.mark.asyncio +async def test_status_ping_pong_eventually_trips_the_gate() -> None: + """blocked <-> in_progress oscillation accrues strikes past the budget.""" + orch = _new_orchestrator() + task_id = str(uuid4()) + statuses = ["blocked", "in_progress"] * 6 + results = [] + with ( + patch("roboco.services.audit.get_audit_service", return_value=_quiet_audit()), + patch( + "roboco.services.notification.NotificationService", + return_value=AsyncMock(), + ), + ): + for status in statuses: + results.append( + await orch._pm_respawn_should_gate( + "be-dev-1", {"id": task_id, "status": status} + ) + ) + assert any(results), ( + "an A<->B status oscillation never accumulated strikes — the exact " + "2026-07-02 two-hour loop the breaker exists to stop" + ) + + +@pytest.mark.asyncio +async def test_forward_progress_through_new_statuses_never_gates() -> None: + orch = _new_orchestrator() + task_id = str(uuid4()) + lifecycle = ["pending", "claimed", "in_progress", "verifying", "awaiting_qa"] + with ( + patch("roboco.services.audit.get_audit_service", return_value=_quiet_audit()), + patch( + "roboco.services.notification.NotificationService", + return_value=AsyncMock(), + ), + ): + for status in lifecycle: + assert not await orch._pm_respawn_should_gate( + "be-dev-1", {"id": task_id, "status": status} + ), f"forward progress into {status} must not gate" + + +@pytest.mark.asyncio +async def test_single_revisit_within_budget_does_not_gate() -> None: + """A legitimate revision cycle (one revisit) stays under the budget.""" + orch = _new_orchestrator() + task_id = str(uuid4()) + with ( + patch("roboco.services.audit.get_audit_service", return_value=_quiet_audit()), + patch( + "roboco.services.notification.NotificationService", + return_value=AsyncMock(), + ), + ): + for status in ["in_progress", "awaiting_qa", "in_progress", "awaiting_qa"]: + assert not await orch._pm_respawn_should_gate( + "be-dev-1", {"id": task_id, "status": status} + ), "one revision round-trip must not trip the breaker" diff --git a/tests/unit/services/test_git_cherry_squash_merge_relief.py b/tests/unit/services/test_git_cherry_squash_merge_relief.py new file mode 100644 index 00000000..a2a0273d --- /dev/null +++ b/tests/unit/services/test_git_cherry_squash_merge_relief.py @@ -0,0 +1,95 @@ +"""_cherry_unmerged_entry must not flag squash-merged children as missing. + +Live false positive (2026-07-02): three children of the S6 cell task were +squash-merged (PRs #176/#185/#190) — their commits sat at the assembled +branch tip, yet ``git cherry`` reported every individual child commit as +unmerged (a squash rewrites N patches into one patch-id) and the assembly +integrity guard refused every legitimate submit_up. + +Relief: every commit — including the squash commit — carries the +``[taskid8]`` prefix, so a marker-bearing commit on the parent proves the +child landed. A child with no marker on the parent stays flagged (the +original incident #11 the guard exists for). +""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace +from typing import Any +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest +from roboco.services.git import GitService + + +def _svc_with_git_responses( + responses: dict[str, SimpleNamespace], +) -> tuple[GitService, list[list[str]]]: + """GitService with _run_git stubbed by subcommand name; records calls.""" + svc = GitService.__new__(GitService) + calls: list[list[str]] = [] + + async def _run_git( + _workspace: Path, args: list[str], **_kw: Any + ) -> SimpleNamespace: + calls.append(args) + return responses[args[0]] + + svc_any: Any = svc + svc_any._run_git = _run_git + return svc, calls + + +def _child() -> MagicMock: + return MagicMock( + id=uuid4(), branch_name="feature/frontend/root--cell--child", title="t" + ) + + +@pytest.mark.asyncio +async def test_squash_merged_child_with_task_marker_is_not_flagged() -> None: + """cherry says unmerged, but the [taskid8] squash commit is on the parent.""" + svc, calls = _svc_with_git_responses( + { + "rev-parse": SimpleNamespace(returncode=0, stdout="abc\n"), + "cherry": SimpleNamespace(returncode=0, stdout="+ aaa\n+ bbb\n"), + "log": SimpleNamespace( + returncode=0, stdout="4771bd71 [deadbeef] title (#190)\n" + ), + } + ) + entry = await svc._cherry_unmerged_entry(Path("/tmp"), "parent", _child()) + assert entry is None + log_call = next(c for c in calls if c[0] == "log") + assert any("\\[" in arg for arg in log_call) # grep pattern escapes the bracket + + +@pytest.mark.asyncio +async def test_genuinely_missing_child_stays_flagged() -> None: + """No marker commit on the parent → the original #11 catch still fires.""" + child = _child() + svc, _calls = _svc_with_git_responses( + { + "rev-parse": SimpleNamespace(returncode=0, stdout="abc\n"), + "cherry": SimpleNamespace(returncode=0, stdout="+ aaa\n"), + "log": SimpleNamespace(returncode=0, stdout=""), + } + ) + entry = await svc._cherry_unmerged_entry(Path("/tmp"), "parent", child) + assert entry == {"task_id": str(child.id)[:8], "title": "t", "unmerged": 1} + + +@pytest.mark.asyncio +async def test_cherry_clean_short_circuits_without_marker_probe() -> None: + """No + lines from cherry → merged; the log probe is never run.""" + svc, calls = _svc_with_git_responses( + { + "rev-parse": SimpleNamespace(returncode=0, stdout="abc\n"), + "cherry": SimpleNamespace(returncode=0, stdout="- aaa\n"), + } + ) + entry = await svc._cherry_unmerged_entry(Path("/tmp"), "parent", _child()) + assert entry is None + assert not any(c[0] == "log" for c in calls) diff --git a/tests/unit/services/test_git_resolve_head_ref_stale_local.py b/tests/unit/services/test_git_resolve_head_ref_stale_local.py new file mode 100644 index 00000000..b962e191 --- /dev/null +++ b/tests/unit/services/test_git_resolve_head_ref_stale_local.py @@ -0,0 +1,73 @@ +"""_resolve_head_ref must not diff off a stale local ref. + +Live incident (2026-07-02): the S6 cell branch advanced on ORIGIN as child +PRs squash-merged on GitHub, but the assignee clone's local ref stayed +parked pre-merge. ``diff()`` preferred the local ref, so the PR-gate +reviewer's evidence diff re-flagged work that had already landed — two +false ``pr_fail`` verdicts on a clean PR. + +Rule: when both refs exist and the local ref is STRICTLY BEHIND origin, +use ``origin/``; a local ref that is ahead (unpushed commits) or +diverged keeps priority, and single-ref cases are unchanged. +""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest +from roboco.services.git import GitService + +_BRANCH = "feature/frontend/root--cell" +_ORIGIN = f"origin/{_BRANCH}" + + +def _svc(*, refs: set[str], ancestor_rc: int) -> tuple[GitService, list[list[str]]]: + svc = GitService.__new__(GitService) + calls: list[list[str]] = [] + + async def _run_git( + _workspace: Path, args: list[str], **_kw: Any + ) -> SimpleNamespace: + calls.append(args) + if args[0] == "merge-base": + return SimpleNamespace(returncode=ancestor_rc, stdout="") + return SimpleNamespace(returncode=0, stdout="") + + async def _ref_exists(_workspace: Path, ref: str) -> bool: + return ref in refs + + svc_any: Any = svc + svc_any._run_git = _run_git + svc_any._ref_exists = _ref_exists + return svc, calls + + +@pytest.mark.asyncio +async def test_local_behind_origin_resolves_to_origin() -> None: + svc, calls = _svc(refs={_BRANCH, _ORIGIN}, ancestor_rc=0) + ref = await svc._resolve_head_ref(Path("/tmp"), _BRANCH) + assert ref == _ORIGIN + ancestor = next(c for c in calls if c[0] == "merge-base") + assert ancestor == ["merge-base", "--is-ancestor", _BRANCH, _ORIGIN] + + +@pytest.mark.asyncio +async def test_local_ahead_or_diverged_keeps_local() -> None: + svc, _calls = _svc(refs={_BRANCH, _ORIGIN}, ancestor_rc=1) + assert await svc._resolve_head_ref(Path("/tmp"), _BRANCH) == _BRANCH + + +@pytest.mark.asyncio +async def test_only_local_ref_unchanged() -> None: + svc, calls = _svc(refs={_BRANCH}, ancestor_rc=1) + assert await svc._resolve_head_ref(Path("/tmp"), _BRANCH) == _BRANCH + assert not any(c[0] == "merge-base" for c in calls) + + +@pytest.mark.asyncio +async def test_only_origin_ref_unchanged() -> None: + svc, _calls = _svc(refs={_ORIGIN}, ancestor_rc=1) + assert await svc._resolve_head_ref(Path("/tmp"), _BRANCH) == _ORIGIN diff --git a/uv.lock b/uv.lock index 41d7fa4a..45dd3f82 100644 --- a/uv.lock +++ b/uv.lock @@ -3158,11 +3158,11 @@ wheels = [ [[package]] name = "stevedore" -version = "5.8.0" +version = "5.9.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/e9/88/35e4d27d9177d7df76d060e0a18f69c6c5794c96960c94042e20a12c8ba2/stevedore-5.8.0.tar.gz", hash = "sha256:b49867b32ca3016e94100e68dbf26e72aa7b8708d0a3f73c08aeb220370ac715", size = 514710, upload-time = "2026-05-18T09:15:27.731Z" } +sdist = { url = "https://files.pythonhosted.org/packages/d7/dd/04d56c2a5232358df41f3d0f0e31833d378b6c8ed7803a6b1b7867b0eba6/stevedore-5.9.0.tar.gz", hash = "sha256:abbd0af7a38a8bbb1d6adea2e35b17609cf004eaac323e88a8d8963640dd2b3c", size = 514850, upload-time = "2026-07-02T11:38:08.509Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/f5/ac/19f9941c74add59d17694930ec8105d5eddeee4ce56dd8632b765ca16d6c/stevedore-5.8.0-py3-none-any.whl", hash = "sha256:88eede9e66ca80e34085b9174e2327da2c61ac91f24f70e41c3ad76e4bb4872b", size = 54553, upload-time = "2026-05-18T09:15:25.82Z" }, + { url = "https://files.pythonhosted.org/packages/62/8d/008761f6e1000600e5303db30d05724bdcf3d2d186cbb59fac79b52e39ed/stevedore-5.9.0-py3-none-any.whl", hash = "sha256:e520945d4c257700eddc1eb1d79df04b2ea578eef185e0e3fa5b442fc848d3f7", size = 54463, upload-time = "2026-07-02T11:38:07.43Z" }, ] [[package]] @@ -3394,11 +3394,11 @@ wheels = [ [[package]] name = "typing-extensions" -version = "4.15.0" +version = "4.16.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/72/94/1a15dd82efb362ac84269196e94cf00f187f7ed21c242792a923cdb1c61f/typing_extensions-4.15.0.tar.gz", hash = "sha256:0cea48d173cc12fa28ecabc3b837ea3cf6f38c6d1136f85cbaaf598984861466", size = 109391, upload-time = "2025-08-25T13:49:26.313Z" } +sdist = { url = "https://files.pythonhosted.org/packages/f6/cc/6253133b5bb138fc3306cebfbda2c520f545d36b5be2c7255cc528bb45d6/typing_extensions-4.16.0.tar.gz", hash = "sha256:dc983d19a509c94dba722ee6abd33940f7c05a89e243c47e907eb4db6f1a43e5", size = 113555, upload-time = "2026-07-02T08:40:05.92Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/18/67/36e9267722cc04a6b9f15c7f3441c2363321a3ea07da7ae0c0707beb2a9c/typing_extensions-4.15.0-py3-none-any.whl", hash = "sha256:f0fa19c6845758ab08074a0cfa8b7aecb71c999ca73d62883bc25cc018c4e548", size = 44614, upload-time = "2025-08-25T13:49:24.86Z" }, + { url = "https://files.pythonhosted.org/packages/49/d3/b8441a820a491ddfc024b0b0cf0393375b75ea13866d9c66727e54c2fc80/typing_extensions-4.16.0-py3-none-any.whl", hash = "sha256:481caa481374e813c1b176ada14e97f1f67a4539ce9cfeb3f350d78d6370c2e8", size = 45571, upload-time = "2026-07-02T08:40:04.659Z" }, ] [[package]]