diff --git a/CLAUDE.md b/CLAUDE.md index c618db94..aeebdbf1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,6 +120,8 @@ On a Python workspace, `WorkspaceService` runs `uv sync --extra dev` (not plain Because the clone is shared across a dev's tasks, a **fresh claim** git-resets the workspace to a clean tree (`git reset --hard`) before checking out the new task's branch — discarding abandoned uncommitted cruft from a finished task while preserving all commits and the gitignored `.venv`. A resume short-circuits before this, so committed work is never reset. +Terminal completion and cancellation also force-delete the task's local branch ref and its `.previews/` video-render dir in the assignee's clone (alongside the existing worktree removal), skipping any branch that coincides with an environment-ladder rung; a PM/CEO can additionally sweep older backlog branches project-wide via `POST /git/branches/cleanup` or the Git page's "Clean Up Stale Branches" button. + ## Git Workflow ### Branch Naming Convention diff --git a/docs/map/_complete_map.md b/docs/map/_complete_map.md index d1c5f83d..d3e16799 100644 --- a/docs/map/_complete_map.md +++ b/docs/map/_complete_map.md @@ -2446,7 +2446,7 @@ The FastAPI surface of RoboCo: every HTTP route under `roboco/api/routes/` (the | roboco/api/routes/dashboard.py | CEO/auditor/kanban/metrics/agents/activity dashboards. | | roboco/api/routes/tasks.py | Task CRUD + lifecycle transitions (claim/start/verify/qa/complete...). | | roboco/api/routes/work_session.py | Work-session list/commit/files/PR/merge/complete/abandon. | -| roboco/api/routes/git.py | Per-project git status/log/diff/commit/push/PR/rebase. | +| roboco/api/routes/git.py | Per-project git status/log/diff/commit/push/PR/rebase/branch-cleanup sweep. | | roboco/api/routes/project.py | Project CRUD + workspace/sync/access + conventions. | | roboco/api/routes/product.py | Product CRUD. | | roboco/api/routes/optimal.py | RAG: kb/search, rag/query, mentor/ask, learnings, decisions, review. | @@ -2502,6 +2502,7 @@ The FastAPI surface of RoboCo: every HTTP route under `roboco/api/routes/` (the | GET/POST | /api/roadmap/cycles, /cycles/{id}/items/{id}/{approve,reject} | roadmap.py | `require_ceo_role` (agent context) | | GET/POST | /api/auth/status (always), /auth/login, /auth/logout (mounted only when `cloud_auth_enabled`) | auth/routes.py | none (status) / FastAPI Users cookie login | | GET/POST/PUT/DELETE | /api/projects, /{id}/conventions, /workspace, /sync | project.py | agent context | +| POST | /api/git/branches/cleanup | git.py | agent context, PM/CEO role-gated like `/rebase`; rate-limit 5/60 — cursor-resumable stale-branch sweep, `GitBranchCleanupRequest`/`Response` (wave 2, open PR #548) | | POST | /api/v1/flow/developer/{give_me_work,i_will_work_on,open_pr,i_am_done,unclaim,resume,sync_branch} | flow_dev.py | `require_dev` (role + HMAC) | | POST | /api/v1/flow/qa/{claim_review,pass_review,fail_review} | flow_qa.py | `require_qa` | | POST | /api/v1/flow/cell_pm/{delegate,submit_up,complete,triage,unblock,reassign} | flow_cell_pm.py | `require_cell_pm` | @@ -2666,6 +2667,7 @@ roboco/api/ > - `da563487` Wave 2 (#297) — adds the CEO-only `/api/a2a/chat/admin/{conversations,conversations/{id}/messages,conversations/{id}/reply}` routes (`_require_ceo`) for the A2A live view + reply-as-CEO. > - `876e19b3` Wave 2c (#298) — adds `/api/a2a/chat/admin/pairs` (the switchboard, same `_require_ceo` gate); tightens `/api/tasks` PATCH so cell/main PM roles get a content-only field allowlist instead of the unrestricted CEO/Board/Auditor admin bypass (`_pm_editor_scope` / `_enforce_pm_lighter_fields`, `roboco/api/routes/tasks.py:256,278`) — closes an over-permission hole where PM identities could edit any-team tasks via the ASSIGN-holding bypass. > - `637c75dc` (2026-07-17, PR #546, "wave-1 quick wins") fix(api): normalize agent UUID to slug at the orchestrator route boundary — `_validated_agent_id` now also calls `_resolve_to_slug` after its path-injection checks, so a caller-supplied DB UUID (e.g. from the panel) resolves to the canonical slug before spawn/stop/status/resolve-wait/mark-waiting address the runtime, fixing UUID-named containers and registry misses. +> - (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) adds `POST /api/git/branches/cleanup` (PM/CEO role-gated like `/rebase`, rate-limit 5/60) + `GitBranchCleanupRequest`/`GitBranchCleanupResponse` schemas — cursor-resumable sweep of terminal tasks' remote+local branches, backing a confirm-dialog button on the panel Git page. ## Regression Risks @@ -3605,7 +3607,10 @@ The slice is structurally sound: the SAVEPOINT boundary, intermediate-None INVAL | `escalate_to_ceo` | method | task.py:5064 | `awaiting_pm_review→awaiting_ceo_approval`; gained `actor_agent_id: UUID | None = None` param (stamped as `audit_agent_id` so the transition row attributes to the specific PM/Board agent, not just the role). | | `ceo_approve` | method | task.py:5146 | CEO merges then approves; `awaiting_ceo_approval→completed`. | | `ceo_reject` | method | task.py:5414 | Reject → `needs_revision` (dev) or `pending` (branchless root via admin_set_status). | -| `_remove_task_worktree_on_terminal` | method | task.py:5601 | Best-effort worktree cleanup on complete/ceo_approve; no-op for branchless. | +| `_delete_task_branch_best_effort` | method | task.py:6726 | Cancel-path cleanup: remote branch delete + `_remove_task_worktree_best_effort(force_branch_delete=True)`; skipped once branch is unset. | +| `_remove_task_worktree_best_effort` | method | task.py:6767 | Shared worktree+local-branch+previews cleanup called by both cancel and terminal paths; force-deletes the local branch ref unless it's an environment-ladder rung (`effective_environments`). | +| `_cleanup_task_previews_best_effort` | method | task.py:6804 | `rmtree` the task's `.previews/{task8}` video-render dir; path-containment-checked against the project workspace dir before deleting. | +| `_remove_task_worktree_on_terminal` | method | task.py:6829 | Best-effort worktree + local-branch (force `-D`, squash-merge is never an ancestor) + previews cleanup on complete/ceo_approve; no-op for branchless. | | `cancel` | method | task.py:5644 | Cascade-cancel descendants through the validator. | | `reassign` / `reassign_active_claim` | method | task.py:7657 / 7807 | Reassignment with Board/Main-PM diversion guards. | | `pr_pass` / `pr_fail` | method | task.py:8100 / 8137 | In-path PR-review gate verdicts. | @@ -3645,7 +3650,7 @@ stateDiagram-v2 - TaskService - State core: `_validate_and_set_status`, `_emit_status_transition_audit`, `admin_set_status`, `_restore_block_ownership`, `_emit_admin_override_audit` - Create/shape: `create`, `_validate_parent_depth`, `_validate_batch_membership`, `activate` - - Branch/worktree: `_ensure_branch_for_task`, `_auto_create_branch`, `_remove_task_worktree*` + - Branch/worktree: `_ensure_branch_for_task`, `_auto_create_branch`, `_delete_task_branch_best_effort`, `_remove_task_worktree*`, `_cleanup_task_previews_best_effort` - Claim: `claim`, `_finalize_claim`, `_inject_proactive_context`, `acquire_*_lock` - Lifecycle verbs: `start`, `block*`, `unblock`, `pause`, `resume`, `submit_for_qa`, `pass_qa`, `fail_qa`, `docs_complete`, `submit_for_pm_review` - Completion: `complete`, `_apply_complete_approval_chain`, `ceo_approve`, `ceo_reject`, `cancel` @@ -3684,6 +3689,7 @@ stateDiagram-v2 - Branchless/umbrella/external-review tasks are exempt from the branch gate inside `GitContext` (task.py:597-611); umbrella is also exempt from the `awaiting_pm_review→awaiting_ceo_approval` pr_number gate. - `complete()` requires PR merged (`_assert_pr_merged_for_complete`) EXCEPT branchless roots; `ceo_approve` separately checks `work_session.pr_status=="merged"` and refuses otherwise. - Background indexing/learning/cleanup tasks are tracked on `self._background_tasks` and are best-effort — a failure never blocks the transition. +- Both cancel and terminal-completion now force-delete (`-D`) the task's LOCAL branch ref in the assignee's clone alongside the worktree — a completed task's PR was squash-merged (its local ref is never an ancestor of base, so a "safe" `-d` refuses unconditionally) and a cancelled task's work is discarded by decision, so the ref is spent either way. Skipped when the branch name coincides with an environment-ladder rung (`effective_environments`), which outlives any one task. ## Drift from CLAUDE.md - CLAUDE.md states ceo_reject "~4779 skips _validate_and_set_status in branchless path". Actual: branchless branch of `ceo_reject` is at task.py:5488 and routes through `admin_set_status` (which DOES emit audit at task.py:2100). The non-branchless branch DOES call `_validate_and_set_status` (task.py:5461). No audit gap — the line reference is stale. @@ -3696,6 +3702,8 @@ stateDiagram-v2 - `3aff6e04` Chore: Close gaps (#285) — follow-on gap close (worktree-on-terminal cleanup F123 Phase C, escalation audit emit, rework routing hardening). > Post-snapshot updates (since 2026-06-29): `20f1f9ba` admin_set_status: thread actor_id/actor_role into `_apply_pre_block_restore`; blocked→pending/in_progress restore now attributes the audit row to the admin actor (not the restored owner) and emits a `task.admin_override` row (forced=False, restore=True) independent of the force flag. `b3558d4e` complexity: extract `_restore_block_ownership` (line 8526) + `_emit_admin_override_audit` (line 8555) from `_apply_pre_block_restore` — no behavior change, splits a C-rank block for the xenon gate. `0e7674af` escalate_to_ceo gains `actor_agent_id: UUID | None = None` param stamped as audit_agent_id; push_branch / create_pr / create_root_pr / escalate_to_ceo side-effect handlers in the verb runner now forward actor_agent_id (was dropped, causing wrong workspace or role-only audit attribution). +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Local branch refs stop leaking: `_delete_task_branch_best_effort`/`_remove_task_worktree_on_terminal` now also force-delete the assignee's local branch ref (via new `WorkspaceService.delete_local_branch`) and rmtree the task's `.previews/{task8}` video-preview dir, both skipped for environment-ladder rungs. See the `worksession-git` section for the paired `GitService.cleanup_stale_branches` sweep. ## Regression Risks @@ -3795,6 +3803,11 @@ This slice is the git substrate every delivery agent works on. `GitService` runs | `GitService.sync_task_branch` | method | git.py:3847 | Task-keyed rebase through dev `sync_branch` verb (pre-PR) | | `GitService.is_behind_base` | method | git.py:3889 | `(behind, ahead)` counts for i_am_done submit gate | | `GitService.close_pull_request` | method | git.py:3940 | Close superseded PR + optional comment + branch cleanup (idempotent) | +| `GitService._delete_remote_branch_best_effort` | method | git.py:3608 | Best-effort remote delete; skips main/master/develop + open-dependent-PR branches; returns `bool` (issued vs skipped/failed) | +| `GitService.delete_task_branch` | method | git.py:3671 | Cancel-path remote branch delete; chokepoint for the environment-ladder skip (`effective_environments`) so a task's `branch_name` can never collide-delete a ladder rung; returns `bool` | +| `GitService.cleanup_stale_branches` | method | git.py:3711 | `POST /git/branches/cleanup` backing sweep: terminal (completed/cancelled) tasks' branches, remote (`delete_task_branch`) + local force-delete in the assignee's clone; capped 200/call, cursor-resumable | +| `GitService._stale_branch_window` | method | git.py:3777 | One deterministic `ORDER BY id` window of sweep candidates; ladder rungs excluded from results but still advance the cursor | +| `GitService._cleanup_one_stale_branch` | method | git.py:3810 | Per-branch remote+local delete for one sweep candidate; raises on unexpected failure so the caller's try/except counts it as an error | | `GitService.pr_target` | method | git.py:4021 | Return PR base branch (project_id scoped) | | `GitService.create_pr` | method | git.py:3418 | Branch-keyed open PR (gateway path; ensures base on remote) | | `GitService._record_pr_atomically` | method | git.py:2601 | Atomic pr_number/url write to task | @@ -3915,6 +3928,7 @@ roboco/ │ │ _pr_is_merged / _auto_complete_on_merge / _first_allowed_merge_method │ ├── branch cleanup: _delete_remote_branch_best_effort / _delete_pr_branch_best_effort │ │ delete_task_branch / _branch_has_open_dependents +│ │ cleanup_stale_branches / _stale_branch_window / _cleanup_one_stale_branch (sweep) │ ├── rebase/sync: rebase_onto_base / rebase_pr_for_task / sync_task_branch / is_behind_base │ ├── close: close_pull_request │ ├── quality: run_pre_submit_quality_gate / toolchain_status_for_task / _fast_gate_commands @@ -3954,7 +3968,7 @@ External: ## Entry Points -- **HTTP routes** (`roboco/api/routes/git.py`): `get_status`, `log`, `diff`, `commit_for_task`, `push_for_task`, `create_branch_for_task`, `checkout_branch_for_agent`, `create_pr_for_task`, `merge_pr_for_task`, `pull`, `fetch`, `rebase` — all construct via `get_git_service(db)`. +- **HTTP routes** (`roboco/api/routes/git.py`): `get_status`, `log`, `diff`, `commit_for_task`, `push_for_task`, `create_branch_for_task`, `checkout_branch_for_agent`, `create_pr_for_task`, `merge_pr_for_task`, `pull`, `fetch`, `rebase`, `cleanup_stale_branches` (`POST /git/branches/cleanup`, PM/CEO role-gated like `/rebase`, rate-limit 5/60) — all construct via `get_git_service(db)`. - **HTTP routes** (`roboco/api/routes/tasks.py:253`): `get_git_service` for task-scoped git. - **Gateway Choreographer** (`roboco/services/gateway/choreographer/`): - `_verb_runner._do_pr_merge` → `pr_merge` @@ -3999,6 +4013,8 @@ Module-level tunables (not env): `_SLOW_GIT_OP_MS=5000`, `_CI_RUN_WINDOW=20`, `_ - **Conventions validator fails closed** (`could_not_run=True` blocks submit) on resolution error / timeout / non-zero exit; branchless + no-changed-files fail open. - **`_assert_on_task_branch` never discards work** — it does `checkout`, not `reset --hard`, to preserve a resumed agent's unpushed commits. - **CEO-only master merge**: `pr_merge` refuses `target == default_branch` for agents; only `merge_pr_for_task` (CEO role-gated from `awaiting_ceo_approval`) may merge to master. +- **`cleanup_stale_branches` cursor is required, not optional**: task rows never change as a side effect of the sweep (unlike, say, a queue that drains), so a repeat call with no `after_cursor` re-scans the identical first 200-row window forever instead of progressing. `_stale_branch_window` still advances the cursor past ladder-rung rows even though they're excluded from `candidates`, so `truncated` can't false-negative when a rung lands inside the window. +- **Local branch delete in the sweep is always `force=True`** (`-D`) regardless of completed vs cancelled — a completed task's PR was squash-merged, so its local ref is never an ancestor of base and a "safe" `-d` would refuse every single candidate. ## Drift from CLAUDE.md @@ -4020,6 +4036,8 @@ Baseline: `fd10cc862c2020b3f639cdb686d427b0198a2441` (master tip before the metr | `3aff6e04` (Close gaps) | Same mega-commit (the PR body is identical — #285 is the merge closure of the #283 batch); the in-scope file deltas are the same set of additions. No additional logic change to these files beyond what #283 listed. | > Post-snapshot updates (since 2026-06-29): `536bbb64` (Chore/all/logical gaps sweep #286 — closed regression risks #108 and #109 in this slice: `_merge_with_retry` now falls back to a permitted merge method on 405 via `_first_allowed_merge_method` before raising `MergeConflictError`; `close_pull_request` default flipped to `delete_branch=False`, choreographer caller now passes `delete_branch=True` explicitly). `00513399` ([bug] push_branch — `push_branch(branch_name)` now passes `branch=branch_name` to `self.push()` so the gateway `open_pr` path pushes the actual named task branch rather than the clone root's current checkout; fixes the "No commits between" 422 → `i_am_blocked` wedge observed in the F123 per-worktree model). `2759edf7` ([B-REL] release executor — added `_CiRunQuery` dataclass at git.py:241 to bundle per-project CI-fetch inputs; `get_latest_ci_conclusion` and `_fetch_latest_ci_run` now accept an optional `head_sha` so the release CI gate polls a specific release commit's own run rather than branch-latest; `settings.release_ci_workflow` config flag added). `69071030` ([chore] work-session-routes — added `WorkSessionService.task_team_for_session` helper (route layer PM cell-ownership check for `merge_pr`); route layer now stamps `merged_by` from the authenticated caller rather than the request body — `WorkSessionService.merge_pr` signature is unchanged, but `MergePRRequest` schema dropped `merged_by` field). +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Local branch refs stop leaking alongside remote ones: `delete_task_branch` now also skips environment-ladder rungs (previously only the remote-delete's own main/master/develop guard existed) and returns `bool`; new `cleanup_stale_branches` + `_stale_branch_window` + `_cleanup_one_stale_branch` back a PM/CEO-only `POST /git/branches/cleanup` sweep of terminal tasks' remote+local branches, exposed as a confirm-dialog button on the panel Git page. See the `task-service` section for the paired per-task reap at cancel/completion and the `workspace` section for the new `WorkspaceService.delete_local_branch` primitive both routes share. ## Regression Risks @@ -4082,6 +4100,7 @@ WorkspaceService manages the per-agent git clone layout under {workspaces_root}/ | WorkspaceService._fetch_branch_ref | method | roboco/services/workspace.py:613 | Token-aware git fetch origin into clone_root; best-effort (never raises); used by ensure_worktree_self_heal (536bbb64) | | WorkspaceService.ensure_worktree_self_heal | method | roboco/services/workspace.py:671 | Orchestrator spawn-time chokepoint: re-attaches a per-task worktree after clone vanished (redeploy/disk loss); fetches branch ref from origin when the local ref is absent after a re-clone, then delegates to ensure_worktree (536bbb64) | | WorkspaceService.remove_worktree | method | roboco/services/workspace.py:733 | Best-effort git worktree remove --force + prune; no-op if gone (cancel/terminal/reaper evict) | +| WorkspaceService.delete_local_branch | method | roboco/services/workspace.py:787 | Best-effort `git branch -d/-D ` in a clone; never raises; skips main/master/develop/empty (mirrors GitService._delete_remote_branch_best_effort); callers run it AFTER remove_worktree (a still-checked-out branch refuses) | | WorkspaceService.resolve_workspace | method | roboco/services/workspace.py:745 | Look up agent (UUID or slug) -> team+slug -> workspace path; default team BACKEND | | WorkspaceService._lookup_agent_or_raise | method | roboco/services/workspace.py:787 | Find agent by UUID or slug; raise WorkspaceError if missing | | WorkspaceService._is_workspace_healthy | staticmethod | roboco/services/workspace.py:806 | True only if .git exists AND has HEAD + objects/ (rejects stub clones) | @@ -4178,7 +4197,7 @@ WorkspaceService slice +-- WorkspaceError +-- WorkspaceService | +-- Path math: get_workspace_path / get_clone_root_path / get_worktree_path -| +-- Worktree ops: _clone_root_default_branch / _park_clone_root_off_branch / _worktree_git / _link_shared_venv / ensure_worktree / ensure_worktree_for_resume / _fetch_branch_ref / ensure_worktree_self_heal / remove_worktree +| +-- Worktree ops: _clone_root_default_branch / _park_clone_root_off_branch / _worktree_git / _link_shared_venv / ensure_worktree / ensure_worktree_for_resume / _fetch_branch_ref / ensure_worktree_self_heal / remove_worktree / delete_local_branch | +-- Agent lookup: resolve_workspace / _lookup_agent_or_raise | +-- Health + refs: _is_workspace_healthy / _prune_broken_refs / _fetch_origin_best_effort | +-- Token: _resolve_git_token / _read_clone_token @@ -4203,6 +4222,7 @@ WorkspaceService slice | ensure_worktree_for_resume | roboco/services/workspace.py | GitService._ensure_worktree_for_commit (commit/rebase paths) | | ensure_worktree_self_heal | roboco/services/workspace.py | orchestrator._ensure_worktree_before_spawn before -w container launch (replaces the former ensure_worktree_for_resume call there; handles vanished clones + missing branch refs) | | remove_worktree | roboco/services/workspace.py | TaskService terminal/cancel paths + claim-rollback (mid-claim failure) | +| delete_local_branch | roboco/services/workspace.py | TaskService terminal/cancel paths (right after remove_worktree) + GitService.cleanup_stale_branches sweep | | ensure_read_clone | roboco/services/workspace.py | ConventionsService.scaffold/effective-map reads (project-level conventions metadata) | | dry_upgrade_changes_lockfile | roboco/services/workspace.py | DepUpdateEngine periodic probe loop | | fetch_branch_for_inspection | roboco/services/workspace.py | gateway content_actions (QA/Documenter/PM need to read a dev branch) | @@ -4236,6 +4256,7 @@ WorkspaceService slice - dry_upgrade_changes_lockfile holds the read-clone lock only for the local clone step, then releases it before the upgrade runs. The tiny gap between ensure_read_clone releasing and the probe re-acquiring is safe only because any concurrent _sync_read_clone completes under the lock first — a future change that interleaves could race. - get_workspace_path raises WorkspaceError if team is None rather than producing a literal 'None' segment; resolve_workspace falls back to Team.BACKEND when agent.team is falsy — agents missing a team silently land under backend/. - fetch_branch_for_inspection reuses workspace_clone_timeout (300s) for a single-branch fetch, not the shorter refresh timeout — a hung remote blocks the QA/Doc verb for 5 minutes. +- delete_local_branch only detaches the ref; remove_worktree only detaches the worktree. Callers MUST run remove_worktree first — `git branch -d/-D` refuses a branch still checked out elsewhere in the clone (the worktree). Skipping the order silently no-ops the branch delete (check=False swallows the refusal). ## Drift from CLAUDE.md @@ -4258,6 +4279,8 @@ WorkspaceService slice | 0f7d6929 | [F-fix] gate the worktree .venv symlink on the clone-root venv existing | _link_shared_venv now no-ops when clone_root/.venv does not yet exist (instead of dangling a symlink), so uv no longer errors or silently re-syncs a worktree-local venv in the near-zero gap before install_dev_deps provisions the clone-root venv. A later ensure self-heals the link. | > Post-snapshot updates (since 2026-06-29): 5 commits touched workspace.py. (1) 9faf2763 [hotfix] strip VIRTUAL_ENV + UV_PROJECT_ENVIRONMENT from _uv_subprocess_env so workspace uv calls stop warning about the image-baked /app/.venv pin. (2) cfe725da [hotfix] worktree: clone root left on the task branch caused fatal "already checked out" on every worktree add re-dispatch — added _clone_root_default_branch + _park_clone_root_off_branch; ensure_worktree and ensure_worktree_for_resume now call _park_clone_root_off_branch before the add to restore the F123 invariant. (3) 536bbb64 (logical-gap sweep PR#286) added _fetch_branch_ref + ensure_worktree_self_heal: the orchestrator's _ensure_worktree_before_spawn now calls ensure_worktree_self_heal instead of bare ensure_worktree_for_resume so a vanished clone (redeploy/disk loss) that left no local branch ref recovers the pushed commits from origin before re-attaching. (4) 3aff6e04 and 15effce0 (gap-fill PRs #285/#283) contributed earlier worktree + dep-probe plumbing (the _clone_local_into / _probe_lockfile_on_clone split already captured in the baseline). +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Added `delete_local_branch` (line 787) so `TaskService`'s cancel/terminal-completion cleanup and `GitService.cleanup_stale_branches` can reap a spent local branch ref, not just the worktree — previously every task an agent ever claimed leaked a permanent `refs/heads/{branch}` in that agent's clone. ## Regression Risks @@ -8037,6 +8060,7 @@ The Next.js 16 control panel (`panel/`, package `roboco-panel` v0.14.0) is the s | A2A Live (switchboard + reply + New DM) | `app/(dashboard)/a2a/page.tsx` + `components/a2a/*` | CEO watches every agent-to-agent conversation live: default org-chart switchboard (pair cards grouped by cell/PM-chain/board, pulsing on fresh `a2a.message` frames) or the classic conversation list; drill-in shows the transcript + a reply composer that lets the CEO chime into a watched thread as itself (task-linked conversations only). "New DM" opens a fresh CEO-owned 1:1 with any DM-capable agent (no task link needed); the recipient is woken via the a2a_request dispatch path if offline, and the CEO's own threads render with `A2ADirectComposer` instead of the reply composer | | Project Settings / Conventions | `components/projects/edit-project-dialog.tsx` + `components/conventions/conventions-tab.tsx` | Per-project `.roboco/conventions.yml` map + health; Save / Restore via PR | | Usage Dashboard | `components/dashboard/usage-overview-panel.tsx` + `hooks/use-usage.ts` | Token/cost totals; live WS snapshot with HTTP-polling fallback | +| Git | `app/(dashboard)/git/page.tsx` | Repository / Work Sessions tabs (business-page tab idiom, `?tab=`); `GitBrowser` (status/branches/log/diff + actions incl. confirm-gated "Clean Up Stale Branches") and `WorkSessionsView` (active sessions, search/status filter kept LOCAL not in URL params); old `/work-sessions` route now redirects to `/git?tab=sessions` | | Kanban | `components/kanban/{core,views}/*` | dnd-kit drag board; dev/qa/pm/pr-review views; drag routes through admin status-override with bypass-precondition prompt | | Task Detail | `components/tasks/task-detail/*` | Tabbed: overview, plan, progress, commits, sessions, notes, dependencies, AC, action dialogs | | AI Providers | `app/(dashboard)/settings/ai-providers/page.tsx` + `components/settings/ai-routing-card.tsx` | Per-slug/role/global model routing | @@ -8075,6 +8099,11 @@ The Next.js 16 control panel (`panel/`, package `roboco-panel` v0.14.0) is the s | `RequiredNotesDialog` | comp | `components/ui/required-notes-dialog.tsx` | Reusable notes-gated confirm; submit disabled on empty/whitespace | | `CommandCenter` | comp | `components/dashboard/command-center.tsx` | Overview page body; composes all dashboard cards | | `DeliveryTabContent` | comp | `components/metrics/delivery-tab.tsx` | Cycle-time/bottleneck/rework/scorecard panels | +| `GitActionsPanel` | comp | `components/git/git-actions-panel.tsx` | Commit/push/PR/rebase actions + destructive-confirm "Clean Up Stale Branches" (`AlertDialog`) | +| `useCleanupBranches` / `handleCleanupBranches` | hook | `hooks/use-git.ts` / `hooks/use-git-browser.ts` | Mutation over `POST /git/branches/cleanup`; the browser hook tracks a per-project cursor ref so a repeat click resumes a truncated sweep instead of re-scanning the first window | +| `WorkSessionsView` | comp | `components/work-sessions/work-sessions-view.tsx` | Git page's "Work Sessions" tab body; search/status filters are LOCAL `useState`, not URL params | +| `SessionTrendChart` | comp | `components/work-sessions/session-trend-chart.tsx` | Active-session start-time histogram (hourly/daily bucketing); honestly labeled active-only, no history beyond `GET /work-sessions` | +| `CostTrendChart` / `SpendTrendChart` | comp | `components/dashboard/cost-trend-chart.tsx` / `components/business/spend-trend-chart.tsx` | Daily-spend area charts off `GET /usage/time-series`; 7d on Overview (`CommandCenter`), 30d on the Business scorecard (`CompanyScorecardCard`) | ## Data Flow Browser → nginx :3000 → (panel Next.js server for pages; `/api/*` and `/ws/*` proxied to `orchestrator:8000`). All client calls use relative URLs: `API_URL="/api"` (axios `baseURL`) and `WS_URL="/ws"` (`getWebSocketUrl`) — no CORS because the browser sees one origin. When cloud auth is armed (`ROBOCO_CLOUD_AUTH_ENABLED`), every navigation to a `(dashboard)` route first runs `proxy.ts` (Next 16's rename of `middleware.ts`), which probes `/auth/status` directly against the docker-internal orchestrator URL (not through nginx) and redirects to `/login` when no `roboco_session` cookie is present; a probe failure/timeout fails OPEN to "cloud auth off" so a slow/unreachable backend never blocks navigation. The login page (`(auth)/login/page.tsx`) posts credentials via `authApi.login` (OAuth2 form body, FastAPI Users' cookie route) and the session cookie rides back on the response. The shared axios client DEFAULTS `X-Agent-ID=` + `X-Agent-Role=CEO_ROLE` headers for API authorization — `has()`/`set()`, not a flat overwrite, so a call that already set its own headers (the CEO-DM composer's `X-Agent-ID: "ceo"`, needed literally by its route) keeps them. Live events flow: orchestrator `StreamEventBus` → `websocket_bridge` → per-resource `/ws/{agents,notifications,system}` sockets → panel `useWebSocket` hooks → zustand stores / TanStack Query cache. Usage snapshots (`USAGE_SNAPSHOT`) and rate-limit lifecycle (`RATE_LIMIT_HIT/LIFTED`) arrive on the single shared `/ws/system` stream mounted in providers; on any non-`connected` state the usage store clears its snapshot so the panel falls back to HTTP-polling summary until a fresh frame lands. The A2A page's `useA2ALiveStream` is a second, independent consumer of that same shared `/ws/system` connection (not a new socket): every persisted A2A message publishes an `a2a.message` frame, which the page uses purely to invalidate-on-frame (REST via `a2aApi` stays the source of truth for full message bodies, since the frame's excerpt is capped) and to drive the switchboard's 45s pulse fade on the matching pair card. @@ -8208,6 +8237,7 @@ Deliberately **not** on this card (compose/env-coupled, unsafe for a runtime tog - **Switchboard "peeked pair" state**: a pair with `conversation_id: null` (never talked) has nothing to select via `?conversation=`, so `page.tsx` tracks it separately (`peekedPair`) and renders its own empty state — don't conflate this with the ordinary `selectedId` empty-state path when touching the drill-in panel. - **`a2aApi.createConversation`/`sendCeoMessage` must pass `X-Agent-ID: "ceo"` explicitly** (via axios per-call `headers`) — the backend routes they hit resolve the caller's identity from that raw header rather than a DB lookup, so the client's *default* `CEO_AGENT_ID` (a UUID) would persist as `agent_a`/`from_agent` and break every downstream `"ceo"`-string check (reply-budget gate, reply-composer recipient exclusion, admin pairing). `client.ts`'s interceptor uses `has()`/`set()` (case-insensitive) specifically so this per-call override isn't clobbered — `AxiosHeaders` bracket access is case-sensitive and would have silently lost a lowercase key. - **`A2ANewDmDialog`'s `AgentSelector` uses `excludeRoles`**, a new prop that drops roles from the roster before grouping (not just filters within a group) — used here to exclude the CEO itself plus every role without `read_a2a` on its manifest (auditor, pr_reviewer, prompter, secretary), since a DM to one of them would be a black hole no one ever reads. +- **Every URL param write forks `ScrollRestoration`'s route key and force-scrolls `
` to top** — `WorkSessionsView`'s search/status filters learned this the hard way (a per-keystroke `q=` param bounced the page) and moved to local `useState`; the Git page's own `?tab=` switch is fine since it's a deliberate, infrequent navigation, not per-keystroke. Don't route per-keystroke or high-frequency filter state through `router.replace`/`push` on this page. ## Drift from CLAUDE.md - CLAUDE.md says panel lives at `roboco/panel/` inside this repo — confirmed (no longer a separate `roboco-panel` project). No drift. @@ -8237,6 +8267,7 @@ Deliberately **not** on this card (compose/env-coupled, unsafe for a runtime tog > - `ca07c83f` + `40b1a586` (2026-07-17, PR #546) — scroll-bounce fix: `scroll-restoration.tsx`'s route key now strips UI-only params before comparing (`UI_ONLY_PARAMS=["expanded"]`, exported `buildRouteKey`) so a tasks-page row expand/collapse no longer forks/resets the saved scroll position; new floating `ScrollJumpButtons` (`components/scroll-jump-buttons.tsx`, mounted as a `
` sibling in `(dashboard)/layout.tsx`) re-observes `
`'s children via `MutationObserver` across a Suspense fallback→content swap so the `ResizeObserver` never watches a detached fallback node; the dead, unfiltered duplicate `hooks/use-scroll-restoration.ts` was deleted; `agent-utils.ts` `AGENT_NAMES` gains `system: "System"` for backend-authored notifications/events. > - `d83104e9` + `9a08cb3e` (2026-07-17, PR #546) — `ai-routing-card.tsx` confirm/toast copy now reads "Role/global routing now on … — per-agent pins kept" (was "All agents now on … Clears any overrides"), matching the backend fix that mode switches no longer wipe the whole `model_assignments` table — see `docs/map/support-services.md`. > - **Wave 3** (2026-07-17, branch `feature/wave-3-a2a-ceo`, PR #547) — CEO New-DM composer: `a2a-new-dm-dialog.tsx` (opens a fresh CEO-owned 1:1, `AgentSelector`'s new `excludeRoles` prop) + `a2a-direct-composer.tsx` (posts in a CEO-owned thread, no task link needed) wired into `page.tsx`'s composer-selection branch (CEO-owned thread → direct composer; task-linked watched thread → reply composer; else read-only). `use-a2a-live.ts` adds `useCreateCeoConversation`/`useSendCeoMessage`; `lib/api/a2a.ts` adds `createConversation`/`sendCeoMessage` (both force `X-Agent-ID: "ceo"` per-call). `client.ts`'s header injection changed from an unconditional overwrite to a `has()`/`set()` default so a per-call override survives. Backend: `A2AService._maybe_wake_ceo_recipient` wakes an offline `read_a2a`-capable recipient of a CEO DM via the `a2a_request` dispatch path — see `docs/map/a2a-audit-journal-permissions.md`. Same branch also scrubbed "message the CEO" recipes from `docs/rag`/`agents/prompts` (agents are never taught to DM the CEO — reply-only). +> - (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Wave 2 hygiene + charts: `/work-sessions` route now redirects to `/git?tab=sessions` (moved under Git as a "Work Sessions" tab, `git-page.tsx` gains a `Tabs`); `GitActionsPanel` gains a confirm-gated "Clean Up Stale Branches" button (`useCleanupBranches`, cursor-resumable); `WorkSessionsView`'s filters moved from URL params to local state (ScrollRestoration bounce fix); new `SessionTrendChart` / `CostTrendChart` / `SpendTrendChart`. ## Regression Risks diff --git a/docs/map/api-routes-schemas.md b/docs/map/api-routes-schemas.md index 3ddaaca0..e9165151 100644 --- a/docs/map/api-routes-schemas.md +++ b/docs/map/api-routes-schemas.md @@ -19,7 +19,7 @@ The FastAPI surface of RoboCo: every HTTP route under `roboco/api/routes/` (the | roboco/api/routes/dashboard.py | CEO/auditor/kanban/metrics/agents/activity dashboards. | | roboco/api/routes/tasks.py | Task CRUD + lifecycle transitions (claim/start/verify/qa/complete...). | | roboco/api/routes/work_session.py | Work-session list/commit/files/PR/merge/complete/abandon. | -| roboco/api/routes/git.py | Per-project git status/log/diff/commit/push/PR/rebase. | +| roboco/api/routes/git.py | Per-project git status/log/diff/commit/push/PR/rebase/branch-cleanup sweep. | | roboco/api/routes/project.py | Project CRUD + workspace/sync/access + conventions. | | roboco/api/routes/product.py | Product CRUD. | | roboco/api/routes/optimal.py | RAG: kb/search, rag/query, mentor/ask, learnings, decisions, review. | @@ -75,6 +75,7 @@ The FastAPI surface of RoboCo: every HTTP route under `roboco/api/routes/` (the | GET/POST | /api/roadmap/cycles, /cycles/{id}/items/{id}/{approve,reject} | roadmap.py | `require_ceo_role` (agent context) | | GET/POST | /api/auth/status (always), /auth/login, /auth/logout (mounted only when `cloud_auth_enabled`) | auth/routes.py | none (status) / FastAPI Users cookie login | | GET/POST/PUT/DELETE | /api/projects, /{id}/conventions, /workspace, /sync | project.py | agent context | +| POST | /api/git/branches/cleanup | git.py | agent context, PM/CEO role-gated like `/rebase`; rate-limit 5/60 — cursor-resumable stale-branch sweep, `GitBranchCleanupRequest`/`Response` (wave 2, open PR #548) | | POST | /api/v1/flow/developer/{give_me_work,i_will_work_on,open_pr,i_am_done,unclaim,resume,sync_branch} | flow_dev.py | `require_dev` (role + HMAC) | | POST | /api/v1/flow/qa/{claim_review,pass_review,fail_review} | flow_qa.py | `require_qa` | | POST | /api/v1/flow/cell_pm/{delegate,submit_up,complete,triage,unblock,reassign} | flow_cell_pm.py | `require_cell_pm` | @@ -239,6 +240,7 @@ roboco/api/ > - `da563487` Wave 2 (#297) — adds the CEO-only `/api/a2a/chat/admin/{conversations,conversations/{id}/messages,conversations/{id}/reply}` routes (`_require_ceo`) for the A2A live view + reply-as-CEO. > - `876e19b3` Wave 2c (#298) — adds `/api/a2a/chat/admin/pairs` (the switchboard, same `_require_ceo` gate); tightens `/api/tasks` PATCH so cell/main PM roles get a content-only field allowlist instead of the unrestricted CEO/Board/Auditor admin bypass (`_pm_editor_scope` / `_enforce_pm_lighter_fields`, `roboco/api/routes/tasks.py:256,278`) — closes an over-permission hole where PM identities could edit any-team tasks via the ASSIGN-holding bypass. > - `637c75dc` (2026-07-17, PR #546, "wave-1 quick wins") fix(api): normalize agent UUID to slug at the orchestrator route boundary — `_validated_agent_id` now also calls `_resolve_to_slug` after its path-injection checks, so a caller-supplied DB UUID (e.g. from the panel) resolves to the canonical slug before spawn/stop/status/resolve-wait/mark-waiting address the runtime, fixing UUID-named containers and registry misses. +> - (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) adds `POST /api/git/branches/cleanup` (PM/CEO role-gated like `/rebase`, rate-limit 5/60) + `GitBranchCleanupRequest`/`GitBranchCleanupResponse` schemas — cursor-resumable sweep of terminal tasks' remote+local branches, backing a confirm-dialog button on the panel Git page. ## Regression Risks diff --git a/docs/map/panel.md b/docs/map/panel.md index 2eab969b..e23fffa1 100644 --- a/docs/map/panel.md +++ b/docs/map/panel.md @@ -58,6 +58,7 @@ The Next.js 16 control panel (`panel/`, package `roboco-panel` v0.14.0) is the s | A2A Live (switchboard + reply + New DM) | `app/(dashboard)/a2a/page.tsx` + `components/a2a/*` | CEO watches every agent-to-agent conversation live: default org-chart switchboard (pair cards grouped by cell/PM-chain/board, pulsing on fresh `a2a.message` frames) or the classic conversation list; drill-in shows the transcript + a reply composer that lets the CEO chime into a watched thread as itself (task-linked conversations only). "New DM" opens a fresh CEO-owned 1:1 with any DM-capable agent (no task link needed); the recipient is woken via the a2a_request dispatch path if offline, and the CEO's own threads render with `A2ADirectComposer` instead of the reply composer | | Project Settings / Conventions | `components/projects/edit-project-dialog.tsx` + `components/conventions/conventions-tab.tsx` | Per-project `.roboco/conventions.yml` map + health; Save / Restore via PR | | Usage Dashboard | `components/dashboard/usage-overview-panel.tsx` + `hooks/use-usage.ts` | Token/cost totals; live WS snapshot with HTTP-polling fallback | +| Git | `app/(dashboard)/git/page.tsx` | Repository / Work Sessions tabs (business-page tab idiom, `?tab=`); `GitBrowser` (status/branches/log/diff + actions incl. confirm-gated "Clean Up Stale Branches") and `WorkSessionsView` (active sessions, search/status filter kept LOCAL not in URL params); old `/work-sessions` route now redirects to `/git?tab=sessions` | | Kanban | `components/kanban/{core,views}/*` | dnd-kit drag board; dev/qa/pm/pr-review views; drag routes through admin status-override with bypass-precondition prompt | | Task Detail | `components/tasks/task-detail/*` | Tabbed: overview, plan, progress, commits, sessions, notes, dependencies, **findings**, AC, action dialogs | | AI Providers | `app/(dashboard)/settings/ai-providers/page.tsx` + `components/settings/ai-routing-card.tsx` | Per-slug/role/global model routing | @@ -96,6 +97,11 @@ The Next.js 16 control panel (`panel/`, package `roboco-panel` v0.14.0) is the s | `RequiredNotesDialog` | comp | `components/ui/required-notes-dialog.tsx` | Reusable notes-gated confirm; submit disabled on empty/whitespace | | `CommandCenter` | comp | `components/dashboard/command-center.tsx` | Overview page body; composes all dashboard cards | | `DeliveryTabContent` | comp | `components/metrics/delivery-tab.tsx` | Cycle-time/bottleneck/rework/scorecard panels | +| `GitActionsPanel` | comp | `components/git/git-actions-panel.tsx` | Commit/push/PR/rebase actions + destructive-confirm "Clean Up Stale Branches" (`AlertDialog`) | +| `useCleanupBranches` / `handleCleanupBranches` | hook | `hooks/use-git.ts` / `hooks/use-git-browser.ts` | Mutation over `POST /git/branches/cleanup`; the browser hook tracks a per-project cursor ref so a repeat click resumes a truncated sweep instead of re-scanning the first window | +| `WorkSessionsView` | comp | `components/work-sessions/work-sessions-view.tsx` | Git page's "Work Sessions" tab body; search/status filters are LOCAL `useState`, not URL params | +| `SessionTrendChart` | comp | `components/work-sessions/session-trend-chart.tsx` | Active-session start-time histogram (hourly/daily bucketing); honestly labeled active-only, no history beyond `GET /work-sessions` | +| `CostTrendChart` / `SpendTrendChart` | comp | `components/dashboard/cost-trend-chart.tsx` / `components/business/spend-trend-chart.tsx` | Daily-spend area charts off `GET /usage/time-series`; 7d on Overview (`CommandCenter`), 30d on the Business scorecard (`CompanyScorecardCard`) | ## Data Flow Browser → nginx :3000 → (panel Next.js server for pages; `/api/*` and `/ws/*` proxied to `orchestrator:8000`). All client calls use relative URLs: `API_URL="/api"` (axios `baseURL`) and `WS_URL="/ws"` (`getWebSocketUrl`) — no CORS because the browser sees one origin. When cloud auth is armed (`ROBOCO_CLOUD_AUTH_ENABLED`), every navigation to a `(dashboard)` route first runs `proxy.ts` (Next 16's rename of `middleware.ts`), which probes `/auth/status` directly against the docker-internal orchestrator URL (not through nginx) and redirects to `/login` when no `roboco_session` cookie is present; a probe failure/timeout fails OPEN to "cloud auth off" so a slow/unreachable backend never blocks navigation. The login page (`(auth)/login/page.tsx`) posts credentials via `authApi.login` (OAuth2 form body, FastAPI Users' cookie route) and the session cookie rides back on the response. The shared axios client DEFAULTS `X-Agent-ID=` + `X-Agent-Role=CEO_ROLE` headers for API authorization — `has()`/`set()`, not a flat overwrite, so a call that already set its own headers (the CEO-DM composer's `X-Agent-ID: "ceo"`, needed literally by its route) keeps them. Live events flow: orchestrator `StreamEventBus` → `websocket_bridge` → per-resource `/ws/{agents,notifications,system}` sockets → panel `useWebSocket` hooks → zustand stores / TanStack Query cache. Usage snapshots (`USAGE_SNAPSHOT`) and rate-limit lifecycle (`RATE_LIMIT_HIT/LIFTED`) arrive on the single shared `/ws/system` stream mounted in providers; on any non-`connected` state the usage store clears its snapshot so the panel falls back to HTTP-polling summary until a fresh frame lands. The A2A page's `useA2ALiveStream` is a second, independent consumer of that same shared `/ws/system` connection (not a new socket): every persisted A2A message publishes an `a2a.message` frame, which the page uses purely to invalidate-on-frame (REST via `a2aApi` stays the source of truth for full message bodies, since the frame's excerpt is capped) and to drive the switchboard's 45s pulse fade on the matching pair card. @@ -229,6 +235,7 @@ Deliberately **not** on this card (compose/env-coupled, unsafe for a runtime tog - **Switchboard "peeked pair" state**: a pair with `conversation_id: null` (never talked) has nothing to select via `?conversation=`, so `page.tsx` tracks it separately (`peekedPair`) and renders its own empty state — don't conflate this with the ordinary `selectedId` empty-state path when touching the drill-in panel. - **`a2aApi.createConversation`/`sendCeoMessage` must pass `X-Agent-ID: "ceo"` explicitly** (via axios per-call `headers`) — the backend routes they hit resolve the caller's identity from that raw header rather than a DB lookup, so the client's *default* `CEO_AGENT_ID` (a UUID) would persist as `agent_a`/`from_agent` and break every downstream `"ceo"`-string check (reply-budget gate, reply-composer recipient exclusion, admin pairing). `client.ts`'s interceptor uses `has()`/`set()` (case-insensitive) specifically so this per-call override isn't clobbered — `AxiosHeaders` bracket access is case-sensitive and would have silently lost a lowercase key. - **`A2ANewDmDialog`'s `AgentSelector` uses `excludeRoles`**, a new prop that drops roles from the roster before grouping (not just filters within a group) — used here to exclude the CEO itself plus every role without `read_a2a` on its manifest (auditor, pr_reviewer, prompter, secretary), since a DM to one of them would be a black hole no one ever reads. +- **Every URL param write forks `ScrollRestoration`'s route key and force-scrolls `
` to top** — `WorkSessionsView`'s search/status filters learned this the hard way (a per-keystroke `q=` param bounced the page) and moved to local `useState`; the Git page's own `?tab=` switch is fine since it's a deliberate, infrequent navigation, not per-keystroke. Don't route per-keystroke or high-frequency filter state through `router.replace`/`push` on this page. ## Drift from CLAUDE.md - CLAUDE.md says panel lives at `roboco/panel/` inside this repo — confirmed (no longer a separate `roboco-panel` project). No drift. @@ -258,6 +265,7 @@ Deliberately **not** on this card (compose/env-coupled, unsafe for a runtime tog > - `ca07c83f` + `40b1a586` (2026-07-17, PR #546) — scroll-bounce fix: `scroll-restoration.tsx`'s route key now strips UI-only params before comparing (`UI_ONLY_PARAMS=["expanded"]`, exported `buildRouteKey`) so a tasks-page row expand/collapse no longer forks/resets the saved scroll position; new floating `ScrollJumpButtons` (`components/scroll-jump-buttons.tsx`, mounted as a `
` sibling in `(dashboard)/layout.tsx`) re-observes `
`'s children via `MutationObserver` across a Suspense fallback→content swap so the `ResizeObserver` never watches a detached fallback node; the dead, unfiltered duplicate `hooks/use-scroll-restoration.ts` was deleted; `agent-utils.ts` `AGENT_NAMES` gains `system: "System"` for backend-authored notifications/events. > - `d83104e9` + `9a08cb3e` (2026-07-17, PR #546) — `ai-routing-card.tsx` confirm/toast copy now reads "Role/global routing now on … — per-agent pins kept" (was "All agents now on … Clears any overrides"), matching the backend fix that mode switches no longer wipe the whole `model_assignments` table — see `docs/map/support-services.md`. > - **Wave 3** (2026-07-17, branch `feature/wave-3-a2a-ceo`, PR #547) — CEO New-DM composer: `a2a-new-dm-dialog.tsx` (opens a fresh CEO-owned 1:1, `AgentSelector`'s new `excludeRoles` prop) + `a2a-direct-composer.tsx` (posts in a CEO-owned thread, no task link needed) wired into `page.tsx`'s composer-selection branch (CEO-owned thread → direct composer; task-linked watched thread → reply composer; else read-only). `use-a2a-live.ts` adds `useCreateCeoConversation`/`useSendCeoMessage`; `lib/api/a2a.ts` adds `createConversation`/`sendCeoMessage` (both force `X-Agent-ID: "ceo"` per-call). `client.ts`'s header injection changed from an unconditional overwrite to a `has()`/`set()` default so a per-call override survives. Backend: `A2AService._maybe_wake_ceo_recipient` wakes an offline `read_a2a`-capable recipient of a CEO DM via the `a2a_request` dispatch path — see `docs/map/a2a-audit-journal-permissions.md`. Same branch also scrubbed "message the CEO" recipes from `docs/rag`/`agents/prompts` (agents are never taught to DM the CEO — reply-only). +> - (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Wave 2 hygiene + charts: `/work-sessions` route now redirects to `/git?tab=sessions` (moved under Git as a "Work Sessions" tab, `git-page.tsx` gains a `Tabs`); `GitActionsPanel` gains a confirm-gated "Clean Up Stale Branches" button (`useCleanupBranches`, cursor-resumable); `WorkSessionsView`'s filters moved from URL params to local state (ScrollRestoration bounce fix); new `SessionTrendChart` / `CostTrendChart` / `SpendTrendChart`. ## Regression Risks diff --git a/docs/map/task-service.md b/docs/map/task-service.md index 0bbb0f85..d38abda5 100644 --- a/docs/map/task-service.md +++ b/docs/map/task-service.md @@ -55,7 +55,10 @@ | `escalate_to_ceo` | method | task.py:5064 | `awaiting_pm_review→awaiting_ceo_approval`; gained `actor_agent_id: UUID | None = None` param (stamped as `audit_agent_id` so the transition row attributes to the specific PM/Board agent, not just the role). | | `ceo_approve` | method | task.py:5146 | CEO merges then approves; `awaiting_ceo_approval→completed`. | | `ceo_reject` | method | task.py:5414 | Reject → `needs_revision` (dev) or `pending` (branchless root via admin_set_status); now validates `reason` (`reject_trivial` — previously an uncaught Pydantic error could 500 on empty/trivial input) and inserts one `origin=ceo` Finding onto the revision-findings ledger; the branchless-root path manually bumps `revision_count` + emits `task.ceo_reject` since it skips `_emit_status_transition_audit`. See `docs/map/review-findings.md`. | -| `_remove_task_worktree_on_terminal` | method | task.py:5601 | Best-effort worktree cleanup on complete/ceo_approve; no-op for branchless. | +| `_delete_task_branch_best_effort` | method | task.py:6726 | Cancel-path cleanup: remote branch delete + `_remove_task_worktree_best_effort(force_branch_delete=True)`; skipped once branch is unset. | +| `_remove_task_worktree_best_effort` | method | task.py:6767 | Shared worktree+local-branch+previews cleanup called by both cancel and terminal paths; force-deletes the local branch ref unless it's an environment-ladder rung (`effective_environments`). | +| `_cleanup_task_previews_best_effort` | method | task.py:6804 | `rmtree` the task's `.previews/{task8}` video-render dir; path-containment-checked against the project workspace dir before deleting. | +| `_remove_task_worktree_on_terminal` | method | task.py:6829 | Best-effort worktree + local-branch (force `-D`, squash-merge is never an ancestor) + previews cleanup on complete/ceo_approve; no-op for branchless. | | `cancel` | method | task.py:5644 | Cascade-cancel descendants through the validator. | | `reassign` / `reassign_active_claim` | method | task.py:7657 / 7807 | Reassignment with Board/Main-PM diversion guards. | | `pr_pass` / `pr_fail` | method | task.py:8100 / 8137 | In-path PR-review gate verdicts; `pr_fail` transitions to `needs_revision`, then calls `_alert_auditor_of_rework` after flush to emit a best-effort auditor rework ALERT. | @@ -96,7 +99,7 @@ stateDiagram-v2 - TaskService - State core: `_validate_and_set_status`, `_emit_status_transition_audit`, `admin_set_status`, `_restore_block_ownership`, `_emit_admin_override_audit` - Create/shape: `create`, `_validate_parent_depth`, `_validate_batch_membership`, `activate` - - Branch/worktree: `_ensure_branch_for_task`, `_auto_create_branch`, `_remove_task_worktree*` + - Branch/worktree: `_ensure_branch_for_task`, `_auto_create_branch`, `_delete_task_branch_best_effort`, `_remove_task_worktree*`, `_cleanup_task_previews_best_effort` - Claim: `claim`, `_validate_claim_preconditions`, `_claim_blocked_by_sequence`, `_claim_blocked_by_dependencies`, `_finalize_claim`, `_apply_dependency_lineage`, `_inject_proactive_context`, `acquire_*_lock` - Lifecycle verbs: `start`, `block*`, `unblock`, `pause`, `resume`, `submit_for_qa`, `pass_qa`, `fail_qa`, `docs_complete`, `submit_for_pm_review` - Completion: `complete`, `_apply_complete_approval_chain`, `ceo_approve`, `ceo_reject`, `cancel` @@ -138,6 +141,7 @@ stateDiagram-v2 - The sequence gate (`_claim_blocked_by_sequence`) is enforced ONLY in `_validate_claim_preconditions`, i.e. inside `claim` itself — both the gateway claim verbs AND the orchestrator's raw dispatch claim cross it because they both funnel through `TaskService.claim`, unlike the pre-#382 dependency gate which briefly lived only on the gateway side. Any future claim path that bypasses `TaskService.claim` (a raw `admin_set_status`, for instance) does NOT get sequence enforcement. - `_apply_dependency_lineage` is scoped to SAME-REPO dependencies only (`dep_task.project_id != ctx.project.id` short-circuits) — a cross-repo dependency edge (e.g. a MegaTask root-subtask in another project) has no shared git history to merge and is silently skipped; the dependency TIMING gate still holds the claim regardless of repo. - `TaskTable.orchestration_markers` is generic `JSON`, not `JSONB`. Any SQL predicate on a marker key must use `.as_string()` (or the JSON dialect's generic comparator), not `.astext`, which is JSONB-only and raises `AttributeError` at compile time. `list_open_docs_sync_tasks(version=...)` at task.py:1596 is the current example; the inline comment records the rationale. +- Both cancel and terminal-completion now force-delete (`-D`) the task's LOCAL branch ref in the assignee's clone alongside the worktree — a completed task's PR was squash-merged (its local ref is never an ancestor of base, so a "safe" `-d` refuses unconditionally) and a cancelled task's work is discarded by decision, so the ref is spent either way. Skipped when the branch name coincides with an environment-ladder rung (`effective_environments`), which outlives any one task. ## Drift from CLAUDE.md - CLAUDE.md states ceo_reject "~4779 skips _validate_and_set_status in branchless path". Actual: branchless branch of `ceo_reject` is at task.py:5488 and routes through `admin_set_status` (which DOES emit audit at task.py:2100). The non-branchless branch DOES call `_validate_and_set_status` (task.py:5461). No audit gap — the line reference is stale. @@ -152,6 +156,8 @@ stateDiagram-v2 > Post-snapshot updates (since 2026-06-29): `20f1f9ba` admin_set_status: thread actor_id/actor_role into `_apply_pre_block_restore`; blocked→pending/in_progress restore now attributes the audit row to the admin actor (not the restored owner) and emits a `task.admin_override` row (forced=False, restore=True) independent of the force flag. `b3558d4e` complexity: extract `_restore_block_ownership` (line 8526) + `_emit_admin_override_audit` (line 8555) from `_apply_pre_block_restore` — no behavior change, splits a C-rank block for the xenon gate. `0e7674af` escalate_to_ceo gains `actor_agent_id: UUID | None = None` param stamped as audit_agent_id; push_branch / create_pr / create_root_pr / escalate_to_ceo side-effect handlers in the verb runner now forward actor_agent_id (was dropped, causing wrong workspace or role-only audit attribution). `8f3f4236` (#452) "sequence is the bar" — adds `_claim_blocked_by_sequence` + `_validate_claim_preconditions` wiring, `stamp_wave_sequence` (replacing a raw per-sibling delegation ordinal), and migration 069 (`tasks.parent_task_id` index, the sibling probe's hot path). `f2834cf5` (#466) adds `_apply_dependency_lineage`/`_merge_one_dependency`, called from `_create_branch_in_project` right after a fresh branch cut. `61e00832` (PR #492) added `_alert_auditor_of_rework()` and invoked it from `fail_qa`, `pr_fail`, and `request_changes` after each transition to `needs_revision`, wiring the reactive auditor ALERT path. `f6c75237` (PR #509) restored those `_alert_auditor_of_rework()` calls after they were accidentally deleted by the docs-sync PR: all three call sites now dispatch the alert immediately after `await self.session.flush()` so the `needs_revision` transition row is committed before the auditor notification is created. The same commit also changed the descendant-traversal casts in `_supersede_replacement_landed` and `get_all_descendants`, but it used `cast(UUID, child.id)` with a scoped `# noqa: TC006` and `child.id` with a `# type: ignore[arg-type]`, respectively. `e4b7dd0f` / PR #511 reverted those two cast regressions to the preferred string-literal form `cast('UUID', child.id)` with no lint or type suppression, leaving `DOCS_SYNC_SOURCE` and `list_open_docs_sync_tasks` untouched. > > (uncommitted, branch `feature/findings-ledger`, 2026-07-11) Revision-findings ledger: `_audit_events_for` (task.py:997) gains `task.request_changes` (agent_role `cell_pm`/`main_pm`) and `task.ceo_reject` (agent_role `ceo`) branches alongside the existing `task.qa_fail`/`task.pr_fail`; `ceo_reject` gains reason validation + a ledger `Finding` insert (see above); `qa_fail` and `request_changes` drop their raw `dev_notes` appends (the mirror-column data-loss bug) in favor of the ledger + a structured note. Full detail: `docs/map/review-findings.md`. +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Local branch refs stop leaking: `_delete_task_branch_best_effort`/`_remove_task_worktree_on_terminal` now also force-delete the assignee's local branch ref (via new `WorkspaceService.delete_local_branch`) and rmtree the task's `.previews/{task8}` video-preview dir, both skipped for environment-ladder rungs. See `docs/map/worksession-git.md` for the paired `GitService.cleanup_stale_branches` sweep. ## Regression Risks diff --git a/docs/map/worksession-git.md b/docs/map/worksession-git.md index 51b75b5f..8c94552e 100644 --- a/docs/map/worksession-git.md +++ b/docs/map/worksession-git.md @@ -83,6 +83,11 @@ This slice is the git substrate every delivery agent works on. `GitService` runs | `GitService.sync_task_branch` | method | git.py:3847 | Task-keyed rebase through dev `sync_branch` verb (pre-PR) | | `GitService.is_behind_base` | method | git.py:3889 | `(behind, ahead)` counts for i_am_done submit gate | | `GitService.close_pull_request` | method | git.py:3940 | Close superseded PR + optional comment + branch cleanup (idempotent) | +| `GitService._delete_remote_branch_best_effort` | method | git.py:3608 | Best-effort remote delete; skips main/master/develop + open-dependent-PR branches; returns `bool` (issued vs skipped/failed) | +| `GitService.delete_task_branch` | method | git.py:3671 | Cancel-path remote branch delete; chokepoint for the environment-ladder skip (`effective_environments`) so a task's `branch_name` can never collide-delete a ladder rung; returns `bool` | +| `GitService.cleanup_stale_branches` | method | git.py:3711 | `POST /git/branches/cleanup` backing sweep: terminal (completed/cancelled) tasks' branches, remote (`delete_task_branch`) + local force-delete in the assignee's clone; capped 200/call, cursor-resumable | +| `GitService._stale_branch_window` | method | git.py:3777 | One deterministic `ORDER BY id` window of sweep candidates; ladder rungs excluded from results but still advance the cursor | +| `GitService._cleanup_one_stale_branch` | method | git.py:3810 | Per-branch remote+local delete for one sweep candidate; raises on unexpected failure so the caller's try/except counts it as an error | | `GitService.pr_target` | method | git.py:4021 | Return PR base branch (project_id scoped) | | `GitService.create_pr` | method | git.py:3418 | Branch-keyed open PR (gateway path; ensures base on remote) | | `GitService._record_pr_atomically` | method | git.py:2601 | Atomic pr_number/url write to task | @@ -203,6 +208,7 @@ roboco/ │ │ _pr_is_merged / _auto_complete_on_merge / _first_allowed_merge_method │ ├── branch cleanup: _delete_remote_branch_best_effort / _delete_pr_branch_best_effort │ │ delete_task_branch / _branch_has_open_dependents +│ │ cleanup_stale_branches / _stale_branch_window / _cleanup_one_stale_branch (sweep) │ ├── rebase/sync: rebase_onto_base / rebase_pr_for_task / sync_task_branch / is_behind_base │ ├── close: close_pull_request │ ├── quality: run_pre_submit_quality_gate / toolchain_status_for_task / _fast_gate_commands @@ -242,7 +248,7 @@ External: ## Entry Points -- **HTTP routes** (`roboco/api/routes/git.py`): `get_status`, `log`, `diff`, `commit_for_task`, `push_for_task`, `create_branch_for_task`, `checkout_branch_for_agent`, `create_pr_for_task`, `merge_pr_for_task`, `pull`, `fetch`, `rebase` — all construct via `get_git_service(db)`. +- **HTTP routes** (`roboco/api/routes/git.py`): `get_status`, `log`, `diff`, `commit_for_task`, `push_for_task`, `create_branch_for_task`, `checkout_branch_for_agent`, `create_pr_for_task`, `merge_pr_for_task`, `pull`, `fetch`, `rebase`, `cleanup_stale_branches` (`POST /git/branches/cleanup`, PM/CEO role-gated like `/rebase`, rate-limit 5/60) — all construct via `get_git_service(db)`. - **HTTP routes** (`roboco/api/routes/tasks.py:253`): `get_git_service` for task-scoped git. - **Gateway Choreographer** (`roboco/services/gateway/choreographer/`): - `_verb_runner._do_pr_merge` → `pr_merge` @@ -287,6 +293,8 @@ Module-level tunables (not env): `_SLOW_GIT_OP_MS=5000`, `_CI_RUN_WINDOW=20`, `_ - **Conventions validator fails closed** (`could_not_run=True` blocks submit) on resolution error / timeout / non-zero exit; branchless + no-changed-files fail open. - **`_assert_on_task_branch` never discards work** — it does `checkout`, not `reset --hard`, to preserve a resumed agent's unpushed commits. - **CEO-only master merge**: `pr_merge` refuses `target == default_branch` for agents; only `merge_pr_for_task` (CEO role-gated from `awaiting_ceo_approval`) may merge to master. `default_branch` resolves through the env-ladder head rung (`_project_default_branch` → `head_branch(project)`), not the raw `projects.default_branch` column. +- **`cleanup_stale_branches` cursor is required, not optional**: task rows never change as a side effect of the sweep (unlike, say, a queue that drains), so a repeat call with no `after_cursor` re-scans the identical first 200-row window forever instead of progressing. `_stale_branch_window` still advances the cursor past ladder-rung rows even though they're excluded from `candidates`, so `truncated` can't false-negative when a rung lands inside the window. +- **Local branch delete in the sweep is always `force=True`** (`-D`) regardless of completed vs cancelled — a completed task's PR was squash-merged, so its local ref is never an ancestor of base and a "safe" `-d` would refuse every single candidate. ## Drift from CLAUDE.md @@ -308,6 +316,8 @@ Baseline: `fd10cc862c2020b3f639cdb686d427b0198a2441` (master tip before the metr | `3aff6e04` (Close gaps) | Same mega-commit (the PR body is identical — #285 is the merge closure of the #283 batch); the in-scope file deltas are the same set of additions. No additional logic change to these files beyond what #283 listed. | > Post-snapshot updates (since 2026-06-29): `536bbb64` (Chore/all/logical gaps sweep #286 — closed regression risks #108 and #109 in this slice: `_merge_with_retry` now falls back to a permitted merge method on 405 via `_first_allowed_merge_method` before raising `MergeConflictError`; `close_pull_request` default flipped to `delete_branch=False`, choreographer caller now passes `delete_branch=True` explicitly). `00513399` ([bug] push_branch — `push_branch(branch_name)` now passes `branch=branch_name` to `self.push()` so the gateway `open_pr` path pushes the actual named task branch rather than the clone root's current checkout; fixes the "No commits between" 422 → `i_am_blocked` wedge observed in the F123 per-worktree model). `2759edf7` ([B-REL] release executor — added `_CiRunQuery` dataclass at git.py:241 to bundle per-project CI-fetch inputs; `get_latest_ci_conclusion` and `_fetch_latest_ci_run` now accept an optional `head_sha` so the release CI gate polls a specific release commit's own run rather than branch-latest; `settings.release_ci_workflow` config flag added). `69071030` ([chore] work-session-routes — added `WorkSessionService.task_team_for_session` helper (route layer PM cell-ownership check for `merge_pr`); route layer now stamps `merged_by` from the authenticated caller rather than the request body — `WorkSessionService.merge_pr` signature is unchanged, but `MergePRRequest` schema dropped `merged_by` field). +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Local branch refs stop leaking alongside remote ones: `delete_task_branch` now also skips environment-ladder rungs (previously only the remote-delete's own main/master/develop guard existed) and returns `bool`; new `cleanup_stale_branches` + `_stale_branch_window` + `_cleanup_one_stale_branch` back a PM/CEO-only `POST /git/branches/cleanup` sweep of terminal tasks' remote+local branches, exposed as a confirm-dialog button on the panel Git page. See `docs/map/task-service.md` for the paired per-task reap at cancel/completion and `docs/map/workspace.md` for the new `WorkspaceService.delete_local_branch` primitive both routes share. ## Regression Risks diff --git a/docs/map/workspace.md b/docs/map/workspace.md index 534a2116..fcd640d7 100644 --- a/docs/map/workspace.md +++ b/docs/map/workspace.md @@ -39,6 +39,7 @@ WorkspaceService manages the per-agent git clone layout under {workspaces_root}/ | WorkspaceService._fetch_branch_ref | method | roboco/services/workspace.py:613 | Token-aware git fetch origin into clone_root; best-effort (never raises); used by ensure_worktree_self_heal (536bbb64) | | WorkspaceService.ensure_worktree_self_heal | method | roboco/services/workspace.py:671 | Orchestrator spawn-time chokepoint: re-attaches a per-task worktree after clone vanished (redeploy/disk loss); fetches branch ref from origin when the local ref is absent after a re-clone, then delegates to ensure_worktree (536bbb64) | | WorkspaceService.remove_worktree | method | roboco/services/workspace.py:733 | Best-effort git worktree remove --force + prune; no-op if gone (cancel/terminal/reaper evict) | +| WorkspaceService.delete_local_branch | method | roboco/services/workspace.py:787 | Best-effort `git branch -d/-D ` in a clone; never raises; skips main/master/develop/empty (mirrors GitService._delete_remote_branch_best_effort); callers run it AFTER remove_worktree (a still-checked-out branch refuses) | | WorkspaceService.resolve_workspace | method | roboco/services/workspace.py:745 | Look up agent (UUID or slug) -> team+slug -> workspace path; default team BACKEND | | WorkspaceService._lookup_agent_or_raise | method | roboco/services/workspace.py:787 | Find agent by UUID or slug; raise WorkspaceError if missing | | WorkspaceService._is_workspace_healthy | staticmethod | roboco/services/workspace.py:806 | True only if .git exists AND has HEAD + objects/ (rejects stub clones) | @@ -135,7 +136,7 @@ WorkspaceService slice +-- WorkspaceError +-- WorkspaceService | +-- Path math: get_workspace_path / get_clone_root_path / get_worktree_path -| +-- Worktree ops: _clone_root_default_branch / _park_clone_root_off_branch / _worktree_git / _link_shared_venv / ensure_worktree / ensure_worktree_for_resume / _fetch_branch_ref / ensure_worktree_self_heal / remove_worktree +| +-- Worktree ops: _clone_root_default_branch / _park_clone_root_off_branch / _worktree_git / _link_shared_venv / ensure_worktree / ensure_worktree_for_resume / _fetch_branch_ref / ensure_worktree_self_heal / remove_worktree / delete_local_branch | +-- Agent lookup: resolve_workspace / _lookup_agent_or_raise | +-- Health + refs: _is_workspace_healthy / _prune_broken_refs / _fetch_origin_best_effort | +-- Token: _resolve_git_token / _read_clone_token @@ -160,6 +161,7 @@ WorkspaceService slice | ensure_worktree_for_resume | roboco/services/workspace.py | GitService._ensure_worktree_for_commit (commit/rebase paths) | | ensure_worktree_self_heal | roboco/services/workspace.py | orchestrator._ensure_worktree_before_spawn before -w container launch (replaces the former ensure_worktree_for_resume call there; handles vanished clones + missing branch refs) | | remove_worktree | roboco/services/workspace.py | TaskService terminal/cancel paths + claim-rollback (mid-claim failure) | +| delete_local_branch | roboco/services/workspace.py | TaskService terminal/cancel paths (right after remove_worktree) + GitService.cleanup_stale_branches sweep | | ensure_read_clone | roboco/services/workspace.py | ConventionsService.scaffold/effective-map reads (project-level conventions metadata) | | dry_upgrade_changes_lockfile | roboco/services/workspace.py | DepUpdateEngine periodic probe loop | | fetch_branch_for_inspection | roboco/services/workspace.py | gateway content_actions (QA/Documenter/PM need to read a dev branch) | @@ -193,6 +195,7 @@ WorkspaceService slice - dry_upgrade_changes_lockfile holds the read-clone lock only for the local clone step, then releases it before the upgrade runs. The tiny gap between ensure_read_clone releasing and the probe re-acquiring is safe only because any concurrent _sync_read_clone completes under the lock first — a future change that interleaves could race. - get_workspace_path raises WorkspaceError if team is None rather than producing a literal 'None' segment; resolve_workspace falls back to Team.BACKEND when agent.team is falsy — agents missing a team silently land under backend/. - fetch_branch_for_inspection reuses workspace_clone_timeout (300s) for a single-branch fetch, not the shorter refresh timeout — a hung remote blocks the QA/Doc verb for 5 minutes. +- delete_local_branch only detaches the ref; remove_worktree only detaches the worktree. Callers MUST run remove_worktree first — `git branch -d/-D` refuses a branch still checked out elsewhere in the clone (the worktree). Skipping the order silently no-ops the branch delete (check=False swallows the refusal). ## Drift from CLAUDE.md @@ -217,6 +220,8 @@ WorkspaceService slice > Post-snapshot updates (since 2026-06-29): 5 commits touched workspace.py. (1) 9faf2763 [hotfix] strip VIRTUAL_ENV + UV_PROJECT_ENVIRONMENT from _uv_subprocess_env so workspace uv calls stop warning about the image-baked /app/.venv pin. (2) cfe725da [hotfix] worktree: clone root left on the task branch caused fatal "already checked out" on every worktree add re-dispatch — added _clone_root_default_branch + _park_clone_root_off_branch; ensure_worktree and ensure_worktree_for_resume now call _park_clone_root_off_branch before the add to restore the F123 invariant. (3) 536bbb64 (logical-gap sweep PR#286) added _fetch_branch_ref + ensure_worktree_self_heal: the orchestrator's _ensure_worktree_before_spawn now calls ensure_worktree_self_heal instead of bare ensure_worktree_for_resume so a vanished clone (redeploy/disk loss) that left no local branch ref recovers the pushed commits from origin before re-attaching. (4) 3aff6e04 and 15effce0 (gap-fill PRs #285/#283) contributed earlier worktree + dep-probe plumbing (the _clone_local_into / _probe_lockfile_on_clone split already captured in the baseline). > > Further post-snapshot update (#534, env-branches ladder): `ensure_workspace`'s fresh-clone branch and `ensure_read_clone` both resolve their target branch via `roboco.models.env_branches.head_branch(project)` — the env-ladder's head rung — instead of reading `project.default_branch` directly. A project with no declared ladder resolves to the identical `default_branch` value via the read-time shim, so this is behavior-preserving until the CEO declares a real ladder in the panel. +> +> (open PR #548, branch `feature/wave-2-hygiene-charts`, 2026-07-17) Added `delete_local_branch` (line 787) so `TaskService`'s cancel/terminal-completion cleanup and `GitService.cleanup_stale_branches` can reap a spent local branch ref, not just the worktree — previously every task an agent ever claimed leaked a permanent `refs/heads/{branch}` in that agent's clone. ## Regression Risks diff --git a/docs/rag/architecture/workspaces.md b/docs/rag/architecture/workspaces.md index bb14b608..7c718faa 100644 --- a/docs/rag/architecture/workspaces.md +++ b/docs/rag/architecture/workspaces.md @@ -76,7 +76,7 @@ Your agent clone is **shared across all your tasks**, but each **claimed task** - **Git ops split by kind**: checkout/HEAD-moving ops (`create_branch`, `commit`, `rebase`, `checkout`) target the worktree; branch-by-name ops (`push`, `pull`, `fetch`, `pr_merge`, `diff`) run from the clone root. You never do either by hand — the verbs resolve the worktree for you. - **One active WorkSession per task** is enforced both in the service layer and by a DB unique index — a re-claim (pool release, reaper unclaim, escalation redirect) supersedes any prior agent's stale session for that task. - **Claim rollback** (a mid-claim failure) `worktree remove --force`s the worktree so a retry doesn't collide with a stale one. -- **Terminal completion** (`complete` / `ceo_approve`) removes the assignee's worktree best-effort, so finished tasks don't accumulate. A `needs_revision` bounce keeps the worktree — you need it back. The stale-claim reaper does **not** remove the worktree; it routes the task to `pending` for a re-claim that reuses it. +- **Terminal completion** (`complete` / `ceo_approve`) and **cancellation** remove the assignee's worktree AND force-delete the now-spent local branch ref in the clone, so finished/cancelled tasks don't leak either on disk. A `needs_revision` bounce keeps both — you need the branch back. The stale-claim reaper does **not** remove the worktree or branch; it routes the task to `pending` for a re-claim that reuses them. A PM/CEO can also run a backlog stale-branch sweep from the panel's Git page for older completed/cancelled tasks whose ref survived from before this reaping existed. You do not manage any of this. The verbs do. The only thing you must know: **your cwd is the worktree for your current task, not the clone root** — so relative paths and `uv run` resolve against your task's checkout. diff --git a/panel/src/app/(dashboard)/git/page.tsx b/panel/src/app/(dashboard)/git/page.tsx index 23976590..00d8626e 100644 --- a/panel/src/app/(dashboard)/git/page.tsx +++ b/panel/src/app/(dashboard)/git/page.tsx @@ -1,5 +1,99 @@ -import { GitBrowser } from "@/components/git"; +"use client"; +import { Suspense } from "react"; +import { useRouter, useSearchParams } from "next/navigation"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@/components/ui/tooltip"; +import { Skeleton } from "@/components/ui/skeleton"; +import { GitBrowser } from "@/components/git"; +import { WorkSessionsView } from "@/components/work-sessions"; + +interface TabDef { + value: "repository" | "sessions"; + label: string; + hint: string; +} + +const TAB_DEFS: TabDef[] = [ + { + value: "repository", + label: "Repository", + hint: "Browse status, branches, log, and diffs; run git actions", + }, + { + value: "sessions", + label: "Work Sessions", + hint: "Active agent work sessions — branch, commits, and PR per task", + }, +]; + +const TAB_VALUES = TAB_DEFS.map((t) => t.value); +type TabValue = (typeof TAB_VALUES)[number]; + +function isValidTab(value: string | null): value is TabValue { + return TAB_VALUES.includes(value as TabValue); +} + +function GitPageContent() { + const router = useRouter(); + const searchParams = useSearchParams(); + + const rawTab = searchParams.get("tab"); + const activeTab: TabValue = isValidTab(rawTab) ? rawTab : "repository"; + + const handleTabChange = (value: string) => { + const params = new URLSearchParams(searchParams.toString()); + params.set("tab", value); + router.replace(`/git?${params.toString()}`); + }; + + return ( + + + {TAB_DEFS.map((tab) => ( + + + {/* TooltipTrigger's asChild Slot merge clobbers TabsTrigger's + own data-state; re-stamp it so the active style survives. */} + + {tab.label} + + + {tab.hint} + + ))} + + + + + + + + + + + ); +} + +// Wrap in Suspense for useSearchParams export default function GitPage() { - return ; + return ( + + + + + } + > + + + ); } diff --git a/panel/src/app/(dashboard)/work-sessions/page.tsx b/panel/src/app/(dashboard)/work-sessions/page.tsx index d346f915..ffbd0570 100644 --- a/panel/src/app/(dashboard)/work-sessions/page.tsx +++ b/panel/src/app/(dashboard)/work-sessions/page.tsx @@ -1,157 +1,6 @@ -"use client"; +import { redirect } from "next/navigation"; -import { Suspense, useMemo, useCallback, useEffect } from "react"; -import { useSearchParams, useRouter } from "next/navigation"; -import { useWorkSessions } from "@/hooks/use-work-sessions"; -import { WorkSessionStatus } from "@/types"; -import { OfflineState } from "@/components/ui/offline-state"; -import { - WorkSessionTable, - WorkSessionFilters, -} from "@/components/work-sessions"; -import { Skeleton } from "@/components/ui/skeleton"; -import { usePageRefresh } from "@/hooks"; - -function WorkSessionsPageContent() { - const router = useRouter(); - const searchParams = useSearchParams(); - - // Read state from URL params - const searchQuery = searchParams.get("q") || ""; - const statusParam = searchParams.get("status"); - const statusFilter = useMemo( - () => - (statusParam?.split(",").filter(Boolean) as WorkSessionStatus[]) || [], - [statusParam], - ); - - // Update URL params - const updateParams = useCallback( - (updates: Record) => { - const params = new URLSearchParams(searchParams.toString()); - Object.entries(updates).forEach(([key, value]) => { - if (value) { - params.set(key, value); - } else { - params.delete(key); - } - }); - const query = params.toString(); - router.push(query ? `/work-sessions?${query}` : "/work-sessions"); - }, - [router, searchParams], - ); - - const handleSearchChange = useCallback( - (value: string) => { - updateParams({ q: value || null }); - }, - [updateParams], - ); - - const handleStatusChange = useCallback( - (value: WorkSessionStatus[]) => { - updateParams({ status: value.length > 0 ? value.join(",") : null }); - }, - [updateParams], - ); - - // Fetch work sessions - const { data: sessions, isLoading, error, refetch } = useWorkSessions(); - - const { register, unregister, refresh } = usePageRefresh(); - - useEffect(() => { - const cb = () => { - void refetch(); - }; - register(cb); - return () => unregister(cb); - }, [register, unregister, refetch]); - - // Filter sessions client-side for search and multi-select status filter - const filteredSessions = useMemo(() => { - if (!sessions) return []; - - return sessions.filter((session) => { - // Search filter - match branch name - if ( - searchQuery && - !session.branch_name.toLowerCase().includes(searchQuery.toLowerCase()) - ) { - return false; - } - - // Status filter (if any selected, session must match one of them) - if (statusFilter.length > 0 && !statusFilter.includes(session.status)) { - return false; - } - - return true; - }); - }, [sessions, searchQuery, statusFilter]); - - // Check if it's a connection error (backend not running) - const isOffline = - error && - (error.message?.includes("Network Error") || - error.message?.includes("ECONNREFUSED") || - (error as { code?: string })?.code === "ERR_NETWORK"); - - return ( -
- {/* Header */} -
-
-

Work Sessions

-

- Track git branches and pull requests for active work -

-
-
- - {/* Filters - Sticky */} -
- -
- - {/* Content */} - {isOffline ? ( - void refresh()} - /> - ) : ( - - )} -
- ); -} - -// Wrap in Suspense for useSearchParams +// The work-sessions surface moved under /git as its "Work Sessions" tab. export default function WorkSessionsPage() { - return ( - -
-
- - -
-
- - - - } - > - -
- ); + redirect("/git?tab=sessions"); } diff --git a/panel/src/components/business/__tests__/company-scorecard-card.test.tsx b/panel/src/components/business/__tests__/company-scorecard-card.test.tsx index da0e4db2..80395e13 100644 --- a/panel/src/components/business/__tests__/company-scorecard-card.test.tsx +++ b/panel/src/components/business/__tests__/company-scorecard-card.test.tsx @@ -26,6 +26,17 @@ vi.mock("@/lib/api/cockpit", () => ({ }, })); +// The spend trend chart pulls its own series via useUsageTimeSeries — a +// hook-level mock (not the raw react-query one above, which only controls +// the single cockpit-summary useQuery call) so SpendTrendChart never sees +// the mocked CockpitSummary object where it expects an array. +vi.mock("@/hooks/use-usage", () => ({ + useUsageTimeSeries: () => ({ + data: [], + isLoading: false, + }), +})); + // --------------------------------------------------------------------------- // Import component AFTER mocks are set up // --------------------------------------------------------------------------- @@ -160,6 +171,17 @@ describe("CompanyScorecardCard", () => { expect(screen.getByText("Done (30 d)")).toBeInTheDocument(); }); + // ------------------------------------------------------------------------- + // The spend-trend chart is wired into the Spend section + // ------------------------------------------------------------------------- + it("renders the daily spend trend chart alongside the spend summary", () => { + setQueryState({ data: buildSummary() }); + + render(); + + expect(screen.getByText("Daily Spend (30d)")).toBeInTheDocument(); + }); + // ------------------------------------------------------------------------- // AC2 Scenario 4: Spend — 'No budget cap set' when cap is null // ------------------------------------------------------------------------- diff --git a/panel/src/components/business/__tests__/spend-trend-chart.test.tsx b/panel/src/components/business/__tests__/spend-trend-chart.test.tsx new file mode 100644 index 00000000..96ecdb2a --- /dev/null +++ b/panel/src/components/business/__tests__/spend-trend-chart.test.tsx @@ -0,0 +1,37 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { SpendTrendChart } from "../spend-trend-chart"; +import type { UsageTimePoint } from "@/types"; + +function buildPoint(overrides: Partial = {}): UsageTimePoint { + return { + bucket: new Date().toISOString(), + tokens_input: 1000, + tokens_output: 500, + total_tokens: 1500, + cost_usd: 4.56, + ...overrides, + }; +} + +describe("SpendTrendChart", () => { + it("renders the card title", () => { + render(); + expect(screen.getByText("Daily Spend (30d)")).toBeInTheDocument(); + }); + + it("shows an empty state when there is no data", () => { + render(); + expect(screen.getByText("No spend data")).toBeInTheDocument(); + }); + + it("shows an empty state when data is undefined", () => { + render(); + expect(screen.getByText("No spend data")).toBeInTheDocument(); + }); + + it("does not show the empty state while loading", () => { + render(); + expect(screen.queryByText("No spend data")).not.toBeInTheDocument(); + }); +}); diff --git a/panel/src/components/business/company-scorecard-card.tsx b/panel/src/components/business/company-scorecard-card.tsx index 2e56fac5..7547bbe6 100644 --- a/panel/src/components/business/company-scorecard-card.tsx +++ b/panel/src/components/business/company-scorecard-card.tsx @@ -12,6 +12,9 @@ import { import { Skeleton } from "@/components/ui/skeleton"; import { OfflineState } from "@/components/ui/offline-state"; import { HelpTip } from "@/components/ui/help-tip"; +import { useUsageTimeSeries } from "@/hooks/use-usage"; +import { SpendTrendChart } from "./spend-trend-chart"; +import type { UsageTimePoint } from "@/types"; // --------------------------------------------------------------------------- // Loading skeleton — three grouped skeleton blocks @@ -131,9 +134,15 @@ function DeliverySection({ delivery }: DeliverySectionProps) { interface SpendSectionProps { spend: CockpitSummary["spend"]; + spendTrend: UsageTimePoint[] | undefined; + spendTrendLoading: boolean; } -function SpendSection({ spend }: SpendSectionProps) { +function SpendSection({ + spend, + spendTrend, + spendTrendLoading, +}: SpendSectionProps) { const { monthly_budget_cap_usd, spend_30d_usd, @@ -190,6 +199,7 @@ function SpendSection({ spend }: SpendSectionProps) { )} + ); } @@ -266,9 +276,15 @@ function StubObjectivesSection() { interface ScorecardBodyProps { data: CockpitSummary; + spendTrend: UsageTimePoint[] | undefined; + spendTrendLoading: boolean; } -function ScorecardBody({ data }: ScorecardBodyProps) { +function ScorecardBody({ + data, + spendTrend, + spendTrendLoading, +}: ScorecardBodyProps) { return ( @@ -277,7 +293,11 @@ function ScorecardBody({ data }: ScorecardBodyProps) { - + @@ -294,6 +314,8 @@ export function CompanyScorecardCard() { queryKey: ["cockpit-summary"], queryFn: cockpitApi.summary, }); + const { data: spendTrend, isLoading: spendTrendLoading } = + useUsageTimeSeries("30d"); if (isLoading) return ; @@ -307,5 +329,11 @@ export function CompanyScorecardCard() { ); } - return ; + return ( + + ); } diff --git a/panel/src/components/business/spend-trend-chart.tsx b/panel/src/components/business/spend-trend-chart.tsx new file mode 100644 index 00000000..15d5d0b2 --- /dev/null +++ b/panel/src/components/business/spend-trend-chart.tsx @@ -0,0 +1,91 @@ +"use client"; + +import { + BarChart, + Bar, + XAxis, + YAxis, + CartesianGrid, + Tooltip, + ResponsiveContainer, +} from "recharts"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; +import { HelpTip } from "@/components/ui/help-tip"; +import type { UsageTimePoint } from "@/types"; + +interface SpendTrendChartProps { + data: UsageTimePoint[] | undefined; + isLoading: boolean; +} + +function formatBucket(bucket: string): string { + const d = new Date(bucket); + return d.getMonth() + 1 + "/" + d.getDate(); +} + +function fmtCost(n: number): string { + return "$" + n.toFixed(2); +} + +/** + * Daily-spend breakdown behind the Scorecard's "30-day spend" figure — + * reuses GET /usage/time-series (period=30d, daily buckets), the same + * series-shaped endpoint the Overview page's CostTrendChart draws from. + */ +export function SpendTrendChart({ data, isLoading }: SpendTrendChartProps) { + const chartData = (data ?? []).map((p) => ({ + day: formatBucket(p.bucket), + Spend: p.cost_usd, + })); + + return ( + + + + Daily Spend (30d) + + + + {isLoading ? ( + + ) : chartData.length === 0 ? ( +

+ No spend data +

+ ) : ( + + + + + + [ + fmtCost(typeof value === "number" ? value : 0), + "Spend", + ]} + contentStyle={{ fontSize: 12 }} + /> + + + + )} +
+
+ ); +} diff --git a/panel/src/components/dashboard/__tests__/command-center.test.tsx b/panel/src/components/dashboard/__tests__/command-center.test.tsx index f4ff84be..ca09b5c1 100644 --- a/panel/src/components/dashboard/__tests__/command-center.test.tsx +++ b/panel/src/components/dashboard/__tests__/command-center.test.tsx @@ -32,6 +32,13 @@ vi.mock("@/hooks/use-tasks", () => ({ refetch: vi.fn(), }), })); +vi.mock("@/hooks/use-usage", () => ({ + useUsageTimeSeries: () => ({ + data: undefined, + isLoading: false, + refetch: vi.fn(), + }), +})); vi.mock("../team-health-cards", () => ({ TeamHealthCards: () =>
TeamHealthCardsStub
, @@ -87,6 +94,9 @@ vi.mock("../usage-overview-panel", () => ({ vi.mock("../scorecard-overview-panel", () => ({ ScorecardOverviewPanel: () =>
ScorecardOverviewPanelStub
, })); +vi.mock("../cost-trend-chart", () => ({ + CostTrendChart: () =>
CostTrendChartStub
, +})); import { CommandCenter } from "../command-center"; @@ -106,6 +116,7 @@ describe("CommandCenter", () => { "AuditorAlertsPanelStub", "UsageOverviewPanelStub", "ScorecardOverviewPanelStub", + "CostTrendChartStub", "TeamHealthCardsStub", "CeoApprovalQueueStub", "StrategySignalsPanelStub", diff --git a/panel/src/components/dashboard/__tests__/cost-trend-chart.test.tsx b/panel/src/components/dashboard/__tests__/cost-trend-chart.test.tsx new file mode 100644 index 00000000..073bacbc --- /dev/null +++ b/panel/src/components/dashboard/__tests__/cost-trend-chart.test.tsx @@ -0,0 +1,37 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { CostTrendChart } from "../cost-trend-chart"; +import type { UsageTimePoint } from "@/types"; + +function buildPoint(overrides: Partial = {}): UsageTimePoint { + return { + bucket: new Date().toISOString(), + tokens_input: 1000, + tokens_output: 500, + total_tokens: 1500, + cost_usd: 1.23, + ...overrides, + }; +} + +describe("CostTrendChart", () => { + it("renders the card title", () => { + render(); + expect(screen.getByText("Spend Trend (7d)")).toBeInTheDocument(); + }); + + it("shows an empty state when there is no data", () => { + render(); + expect(screen.getByText("No usage data")).toBeInTheDocument(); + }); + + it("shows an empty state when data is undefined", () => { + render(); + expect(screen.getByText("No usage data")).toBeInTheDocument(); + }); + + it("does not show the empty state while loading", () => { + render(); + expect(screen.queryByText("No usage data")).not.toBeInTheDocument(); + }); +}); diff --git a/panel/src/components/dashboard/command-center.tsx b/panel/src/components/dashboard/command-center.tsx index 5961b971..a44ec93d 100644 --- a/panel/src/components/dashboard/command-center.tsx +++ b/panel/src/components/dashboard/command-center.tsx @@ -7,6 +7,7 @@ import { useRecentActivity, } from "@/hooks/use-dashboard"; import { useTasks } from "@/hooks/use-tasks"; +import { useUsageTimeSeries } from "@/hooks/use-usage"; import { usePageRefresh } from "@/hooks"; import { TeamHealthCards } from "./team-health-cards"; import { KeyMetricsPanel } from "./key-metrics-panel"; @@ -25,6 +26,7 @@ import type { Activity } from "./activity-item"; import { Button } from "@/components/ui/button"; import { UsageOverviewPanel } from "./usage-overview-panel"; import { ScorecardOverviewPanel } from "./scorecard-overview-panel"; +import { CostTrendChart } from "./cost-trend-chart"; import { Tooltip, TooltipContent, @@ -62,6 +64,11 @@ export function CommandCenter() { isError: errorActivity, refetch: refetchActivity, } = useRecentActivity(24); + const { + data: costTrend, + isLoading: loadingCostTrend, + refetch: refetchCostTrend, + } = useUsageTimeSeries("7d"); const { register, unregister } = usePageRefresh(); @@ -79,6 +86,9 @@ export function CommandCenter() { () => { void refetchActivity(); }, + () => { + void refetchCostTrend(); + }, ]; callbacks.forEach((cb) => register(cb)); return () => { @@ -91,6 +101,7 @@ export function CommandCenter() { refetchFlags, refetchTasks, refetchActivity, + refetchCostTrend, ]); const hasError = errorOverview || errorFlags || errorTasks || errorActivity; @@ -156,6 +167,8 @@ export function CommandCenter() { + + {/* Section 2: Team Health (team cards + Task Intake + Secretary) */}
diff --git a/panel/src/components/dashboard/cost-trend-chart.tsx b/panel/src/components/dashboard/cost-trend-chart.tsx new file mode 100644 index 00000000..ea4549b0 --- /dev/null +++ b/panel/src/components/dashboard/cost-trend-chart.tsx @@ -0,0 +1,109 @@ +"use client"; + +import { + AreaChart, + Area, + XAxis, + YAxis, + CartesianGrid, + Tooltip, + ResponsiveContainer, +} from "recharts"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; +import { HelpTip } from "@/components/ui/help-tip"; +import type { UsageTimePoint } from "@/types"; + +interface CostTrendChartProps { + data: UsageTimePoint[] | undefined; + isLoading: boolean; +} + +function formatBucket(bucket: string): string { + const d = new Date(bucket); + return d.getMonth() + 1 + "/" + d.getDate(); +} + +function fmtCost(n: number): string { + return "$" + n.toFixed(2); +} + +/** + * Compact daily spend trend for the Command Center landing page — reuses + * GET /usage/time-series (period=7d, daily buckets) so the CEO sees where + * the current-period totals in UsageOverviewPanel came from at a glance. + */ +export function CostTrendChart({ data, isLoading }: CostTrendChartProps) { + const chartData = (data ?? []).map((p) => ({ + day: formatBucket(p.bucket), + Cost: p.cost_usd, + })); + + return ( + + + + Spend Trend (7d) + + + + {isLoading ? ( + + ) : chartData.length === 0 ? ( +

+ No usage data +

+ ) : ( + + + + + + + + + + + + [ + fmtCost(typeof value === "number" ? value : 0), + "Cost", + ]} + contentStyle={{ fontSize: 12 }} + /> + + + + )} +
+
+ ); +} diff --git a/panel/src/components/git/git-actions-panel.tsx b/panel/src/components/git/git-actions-panel.tsx index f24f99ca..ee8bba64 100644 --- a/panel/src/components/git/git-actions-panel.tsx +++ b/panel/src/components/git/git-actions-panel.tsx @@ -36,6 +36,7 @@ import { Download, RefreshCcw, GitGraph, + Trash2, } from "lucide-react"; import { HelpTip } from "@/components/ui/help-tip"; @@ -51,6 +52,7 @@ interface GitActionsPanelProps { onPull: () => void; onFetch: () => void; onRebase: (targetBranch: string) => void; + onCleanupBranches: () => void; isCommitting: boolean; isPushing: boolean; isCreatingPR: boolean; @@ -58,6 +60,7 @@ interface GitActionsPanelProps { isPulling: boolean; isFetching: boolean; isRebasing: boolean; + isCleaningUpBranches: boolean; } export function GitActionsPanel({ @@ -72,6 +75,7 @@ export function GitActionsPanel({ onPull, onFetch, onRebase, + onCleanupBranches, isCommitting, isPushing, isCreatingPR, @@ -79,6 +83,7 @@ export function GitActionsPanel({ isPulling, isFetching, isRebasing, + isCleaningUpBranches, }: GitActionsPanelProps) { void _agentId; // Reserved for future use const [showCommitDialog, setShowCommitDialog] = useState(false); @@ -513,6 +518,48 @@ export function GitActionsPanel({ + {/* Cleanup Stale Branches — destructive, requires confirmation */} + + + + + + + + + + + Clean up stale branches? + + Deletes the remote + local branch of every completed or + cancelled task in {projectSlug}. The default + branch and every environment-ladder rung are always skipped. + This action cannot be undone. + + + + Cancel + + Clean Up + + + + + {/* Status Summary */} {status && (
diff --git a/panel/src/components/git/git-browser.tsx b/panel/src/components/git/git-browser.tsx index 702a99e1..65458bc6 100644 --- a/panel/src/components/git/git-browser.tsx +++ b/panel/src/components/git/git-browser.tsx @@ -48,6 +48,7 @@ function GitBrowserContent() { handlePull, handleFetch, handleRebase, + handleCleanupBranches, isCommitting, isPushing, isCreatingPR, @@ -57,6 +58,7 @@ function GitBrowserContent() { isRebasing, isCheckingOut, isCreatingBranch, + isCleaningUpBranches, } = useGitBrowser(); if (isOffline) { @@ -139,6 +141,7 @@ function GitBrowserContent() { onPull={handlePull} onFetch={handleFetch} onRebase={handleRebase} + onCleanupBranches={handleCleanupBranches} isCommitting={isCommitting} isPushing={isPushing} isCreatingPR={isCreatingPR} @@ -146,6 +149,7 @@ function GitBrowserContent() { isPulling={isPulling} isFetching={isFetching} isRebasing={isRebasing} + isCleaningUpBranches={isCleaningUpBranches} />
diff --git a/panel/src/components/work-sessions/__tests__/session-trend-chart.test.tsx b/panel/src/components/work-sessions/__tests__/session-trend-chart.test.tsx new file mode 100644 index 00000000..e86efea7 --- /dev/null +++ b/panel/src/components/work-sessions/__tests__/session-trend-chart.test.tsx @@ -0,0 +1,39 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { SessionTrendChart } from "../session-trend-chart"; +import { WorkSessionStatus } from "@/types"; +import type { WorkSessionSummary } from "@/types"; + +function buildSession(overrides: Partial = {}): WorkSessionSummary { + return { + id: "session-1", + task_id: "11111111-1111-1111-1111-111111111111", + branch_name: "feature/backend/ABC12345", + status: WorkSessionStatus.ACTIVE, + started_at: new Date().toISOString(), + has_pr: false, + ...overrides, + }; +} + +describe("SessionTrendChart", () => { + it("renders the card title", () => { + render(); + expect(screen.getByText("Active Session Starts")).toBeInTheDocument(); + }); + + it("shows an empty state when there are no sessions", () => { + render(); + expect(screen.getByText("No active sessions")).toBeInTheDocument(); + }); + + it("shows an empty state when sessions is undefined", () => { + render(); + expect(screen.getByText("No active sessions")).toBeInTheDocument(); + }); + + it("does not show the empty state while loading", () => { + render(); + expect(screen.queryByText("No active sessions")).not.toBeInTheDocument(); + }); +}); diff --git a/panel/src/components/work-sessions/index.ts b/panel/src/components/work-sessions/index.ts index aac10946..c153b233 100644 --- a/panel/src/components/work-sessions/index.ts +++ b/panel/src/components/work-sessions/index.ts @@ -1,2 +1,4 @@ export { WorkSessionTable } from "./work-session-table"; export { WorkSessionFilters } from "./work-session-filters"; +export { SessionTrendChart } from "./session-trend-chart"; +export { WorkSessionsView } from "./work-sessions-view"; diff --git a/panel/src/components/work-sessions/session-trend-chart.tsx b/panel/src/components/work-sessions/session-trend-chart.tsx new file mode 100644 index 00000000..f0517d78 --- /dev/null +++ b/panel/src/components/work-sessions/session-trend-chart.tsx @@ -0,0 +1,128 @@ +"use client"; + +import { useMemo } from "react"; +import { + BarChart, + Bar, + XAxis, + YAxis, + CartesianGrid, + Tooltip, + ResponsiveContainer, +} from "recharts"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; +import { HelpTip } from "@/components/ui/help-tip"; +import type { WorkSessionSummary } from "@/types"; + +interface SessionTrendChartProps { + sessions: WorkSessionSummary[] | undefined; + isLoading: boolean; +} + +const HOURLY_SPAN_MS = 36 * 60 * 60 * 1000; + +interface Bucket { + key: string; + label: string; + count: number; +} + +/** + * Buckets session `started_at` timestamps by hour (span <= 36h) or by day + * (wider span), mirroring the hourly/daily switch usage-time-series-chart + * applies for its period-selected data. + */ +function bucketSessions(sessions: WorkSessionSummary[]): Bucket[] { + if (sessions.length === 0) return []; + + const times = sessions.map((s) => new Date(s.started_at).getTime()); + const spanMs = Math.max(...times) - Math.min(...times); + const hourly = spanMs <= HOURLY_SPAN_MS; + + const counts = new Map(); + for (const s of sessions) { + const d = new Date(s.started_at); + const key = hourly + ? `${d.getFullYear()}-${d.getMonth()}-${d.getDate()}-${d.getHours()}` + : `${d.getFullYear()}-${d.getMonth()}-${d.getDate()}`; + counts.set(key, (counts.get(key) ?? 0) + 1); + } + + const buckets: Bucket[] = Array.from(counts.entries()).map(([key, count]) => { + const [y, m, day, hour] = key.split("-").map(Number); + const d = new Date(y, m, day, hour ?? 0); + const label = hourly + ? d.getHours().toString().padStart(2, "0") + ":00" + : d.getMonth() + 1 + "/" + d.getDate(); + return { key, label, count }; + }); + + buckets.sort((a, b) => a.key.localeCompare(b.key)); + return buckets; +} + +/** + * Session-start volume trend for the Work Sessions page. `GET /work-sessions` + * (unfiltered, as this page calls it) returns only currently ACTIVE sessions + * — there is no tokens/cost/duration field on WorkSessionSummary (that data + * lives in agent_spawn_sessions, a different table) and no historical depth + * beyond whatever is active right now. So this charts what's honestly here: + * a start-time distribution of the active sessions already on the page, + * labeled accordingly rather than presented as a full history. + */ +export function SessionTrendChart({ + sessions, + isLoading, +}: SessionTrendChartProps) { + const buckets = useMemo(() => bucketSessions(sessions ?? []), [sessions]); + + return ( + + + + Active Session Starts + + + + {isLoading ? ( + + ) : buckets.length === 0 ? ( +

+ No active sessions +

+ ) : ( + + + + + + [ + typeof value === "number" ? value : 0, + "Sessions started", + ]} + contentStyle={{ fontSize: 12 }} + /> + + + + )} +
+
+ ); +} diff --git a/panel/src/components/work-sessions/work-sessions-view.tsx b/panel/src/components/work-sessions/work-sessions-view.tsx new file mode 100644 index 00000000..72280943 --- /dev/null +++ b/panel/src/components/work-sessions/work-sessions-view.tsx @@ -0,0 +1,104 @@ +"use client"; + +import { useMemo, useState, useEffect } from "react"; +import { useWorkSessions } from "@/hooks/use-work-sessions"; +import { WorkSessionStatus } from "@/types"; +import { OfflineState } from "@/components/ui/offline-state"; +import { WorkSessionTable } from "./work-session-table"; +import { WorkSessionFilters } from "./work-session-filters"; +import { SessionTrendChart } from "./session-trend-chart"; +import { usePageRefresh } from "@/hooks"; + +/** + * Work-sessions content, rendered as the "Work Sessions" tab of /git. + * Filter state is LOCAL, deliberately not URL params: every URL write forks + * ScrollRestoration's route key and force-scrolls
to top, so a + * per-keystroke q= param made typing in the search box bounce the page. + */ +export function WorkSessionsView() { + const [searchQuery, setSearchQuery] = useState(""); + const [statusFilter, setStatusFilter] = useState([]); + + const handleSearchChange = setSearchQuery; + const handleStatusChange = setStatusFilter; + + // Fetch work sessions + const { data: sessions, isLoading, error, refetch } = useWorkSessions(); + + const { register, unregister, refresh } = usePageRefresh(); + + useEffect(() => { + const cb = () => { + void refetch(); + }; + register(cb); + return () => unregister(cb); + }, [register, unregister, refetch]); + + // Filter sessions client-side for search and multi-select status filter + const filteredSessions = useMemo(() => { + if (!sessions) return []; + + return sessions.filter((session) => { + // Search filter - match branch name + if ( + searchQuery && + !session.branch_name.toLowerCase().includes(searchQuery.toLowerCase()) + ) { + return false; + } + + // Status filter (if any selected, session must match one of them) + if (statusFilter.length > 0 && !statusFilter.includes(session.status)) { + return false; + } + + return true; + }); + }, [sessions, searchQuery, statusFilter]); + + // Check if it's a connection error (backend not running) + const isOffline = + error && + (error.message?.includes("Network Error") || + error.message?.includes("ECONNREFUSED") || + (error as { code?: string })?.code === "ERR_NETWORK"); + + return ( +
+ {/* Header */} +
+
+

Work Sessions

+

+ Track git branches and pull requests for active work +

+
+
+ + {/* Filters - Sticky */} +
+ +
+ + {/* Content */} + {isOffline ? ( + void refresh()} + /> + ) : ( + <> + + + + )} +
+ ); +} diff --git a/panel/src/hooks/__tests__/use-git-browser.test.tsx b/panel/src/hooks/__tests__/use-git-browser.test.tsx index 026dbe99..f48d926b 100644 --- a/panel/src/hooks/__tests__/use-git-browser.test.tsx +++ b/panel/src/hooks/__tests__/use-git-browser.test.tsx @@ -73,6 +73,7 @@ function buildMutations(overrides: Record = {}) { pull: { mutateAsync: vi.fn(), isPending: false }, fetch: { mutateAsync: vi.fn(), isPending: false }, rebase: { mutateAsync: vi.fn(), isPending: false }, + cleanupBranches: { mutateAsync: vi.fn(), isPending: false }, ...overrides, }; } @@ -312,4 +313,41 @@ describe("useGitBrowser", () => { expect(result.current.isCommitting).toBe(true); expect(result.current.isPushing).toBe(false); }); + + it("cleans up branches for the current project and shows a count toast", async () => { + const mutateAsync = vi.fn(() => + Promise.resolve({ + remote_deleted: 3, + local_deleted: 2, + skipped: 1, + errors: 0, + truncated: false, + }), + ); + mockUseGitOperations.mockReturnValue( + buildMutations({ cleanupBranches: { mutateAsync, isPending: false } }), + ); + + const { result } = renderHook(() => useGitBrowser()); + await result.current.handleCleanupBranches(); + + await waitFor(() => + expect(mutateAsync).toHaveBeenCalledWith({ project_slug: "roboco" }), + ); + expect(mockToastSuccess).toHaveBeenCalledWith( + "Cleaned up branches: 3 remote, 2 local, 1 skipped, 0 errors", + ); + }); + + it("shows an error toast when branch cleanup fails", async () => { + const mutateAsync = vi.fn(() => Promise.reject(new Error("boom"))); + mockUseGitOperations.mockReturnValue( + buildMutations({ cleanupBranches: { mutateAsync, isPending: false } }), + ); + + const { result } = renderHook(() => useGitBrowser()); + await result.current.handleCleanupBranches(); + + await waitFor(() => expect(mockToastError).toHaveBeenCalledWith("boom")); + }); }); diff --git a/panel/src/hooks/use-git-browser.ts b/panel/src/hooks/use-git-browser.ts index bc1de13b..9b32abae 100644 --- a/panel/src/hooks/use-git-browser.ts +++ b/panel/src/hooks/use-git-browser.ts @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useRef } from "react"; import { useRouter, useSearchParams } from "next/navigation"; import { toast } from "sonner"; import { useProjects } from "@/hooks/use-projects"; @@ -45,6 +45,7 @@ export interface UseGitBrowserResult { handlePull: () => Promise; handleFetch: () => Promise; handleRebase: (targetBranch: string) => Promise; + handleCleanupBranches: () => Promise; isCommitting: boolean; isPushing: boolean; isCreatingPR: boolean; @@ -54,6 +55,7 @@ export interface UseGitBrowserResult { isRebasing: boolean; isCheckingOut: boolean; isCreatingBranch: boolean; + isCleaningUpBranches: boolean; } /** @@ -162,8 +164,12 @@ export function useGitBrowser(): UseGitBrowserResult { pull, fetch, rebase, + cleanupBranches, } = useGitOperations(); + // Resume point for a capped stale-branch sweep, per project. + const cleanupCursorRef = useRef<{ slug: string; cursor: string } | null>(null); + const handleCheckout = useCallback( async (branch: string) => { try { @@ -317,6 +323,35 @@ export function useGitBrowser(): UseGitBrowserResult { [projectSlug, taskId, rebase], ); + const handleCleanupBranches = useCallback(async () => { + try { + // Resume a capped sweep from where the last click stopped — without + // the cursor the backend re-scans the identical first window forever. + const cursor = + cleanupCursorRef.current?.slug === projectSlug + ? cleanupCursorRef.current.cursor + : undefined; + const result = await cleanupBranches.mutateAsync({ + project_slug: projectSlug, + ...(cursor ? { after_cursor: cursor } : {}), + }); + cleanupCursorRef.current = + result.truncated && result.next_cursor + ? { slug: projectSlug, cursor: result.next_cursor } + : null; + const truncatedNote = result.truncated + ? " (cap reached — click again to continue where it stopped)" + : ""; + toast.success( + `Cleaned up branches: ${result.remote_deleted} remote, ` + + `${result.local_deleted} local, ${result.skipped} skipped, ` + + `${result.errors} errors${truncatedNote}`, + ); + } catch (error) { + toast.error(getErrorMessage(error)); + } + }, [projectSlug, cleanupBranches]); + const isOffline = !!projectsError && (projectsError.message?.includes("Network Error") || @@ -349,6 +384,7 @@ export function useGitBrowser(): UseGitBrowserResult { handlePull, handleFetch, handleRebase, + handleCleanupBranches, isCommitting: commit.isPending, isPushing: push.isPending, isCreatingPR: createPR.isPending, @@ -358,5 +394,6 @@ export function useGitBrowser(): UseGitBrowserResult { isRebasing: rebase.isPending, isCheckingOut: checkout.isPending, isCreatingBranch: createBranch.isPending, + isCleaningUpBranches: cleanupBranches.isPending, }; } diff --git a/panel/src/hooks/use-git.ts b/panel/src/hooks/use-git.ts index bf563cb9..da8f0370 100644 --- a/panel/src/hooks/use-git.ts +++ b/panel/src/hooks/use-git.ts @@ -30,6 +30,8 @@ import type { GitFetchResponse, GitRebaseRequest, GitRebaseResponse, + GitBranchCleanupRequest, + GitBranchCleanupResponse, } from "@/types/git"; // ============================================================================= @@ -331,6 +333,27 @@ export function useGitRebase() { }); } +/** + * Sweep a project's terminal-task branches (remote + local, PM/CEO only) + */ +export function useCleanupBranches() { + const queryClient = useQueryClient(); + + return useMutation< + GitBranchCleanupResponse, + Error, + GitBranchCleanupRequest + >({ + mutationFn: (request) => gitApi.cleanupBranches(request), + onSuccess: (_, variables) => { + // Invalidate branches — the sweep may have deleted several. + queryClient.invalidateQueries({ + queryKey: [...gitKeys.all, "branches", variables.project_slug], + }); + }, + }); +} + // ============================================================================= // Bundled Hook for Git Operations // ============================================================================= @@ -348,6 +371,7 @@ export function useGitOperations() { const pull = useGitPull(); const fetch = useGitFetch(); const rebase = useGitRebase(); + const cleanupBranches = useCleanupBranches(); return { commit, @@ -359,5 +383,6 @@ export function useGitOperations() { pull, fetch, rebase, + cleanupBranches, }; } diff --git a/panel/src/lib/api/git.ts b/panel/src/lib/api/git.ts index d5d70a0f..8ad6b16f 100644 --- a/panel/src/lib/api/git.ts +++ b/panel/src/lib/api/git.ts @@ -30,6 +30,8 @@ import type { GitFetchResponse, GitRebaseRequest, GitRebaseResponse, + GitBranchCleanupRequest, + GitBranchCleanupResponse, } from "@/types/git"; // ============================================================================= @@ -358,4 +360,28 @@ export const gitApi = { const { data } = await api.post("/git/rebase", request); return data; }, + + /** + * Sweep a project's terminal-task branches (remote + local, PM/CEO only) + */ + cleanupBranches: async ( + request: GitBranchCleanupRequest, + ): Promise => { + if (isMockMode()) { + return { + project_slug: request.project_slug, + remote_deleted: 3, + local_deleted: 3, + skipped: 0, + errors: 0, + truncated: false, + next_cursor: null, + }; + } + const { data } = await api.post( + "/git/branches/cleanup", + request, + ); + return data; + }, }; diff --git a/panel/src/types/git.ts b/panel/src/types/git.ts index 7456b823..f0de1012 100644 --- a/panel/src/types/git.ts +++ b/panel/src/types/git.ts @@ -198,3 +198,19 @@ export interface GitRebaseResponse { conflict: boolean; conflicted_files: string[]; } + +export interface GitBranchCleanupRequest { + project_slug: string; + /** Resume point from a prior truncated sweep's next_cursor. */ + after_cursor?: string; +} + +export interface GitBranchCleanupResponse { + project_slug: string; + remote_deleted: number; + local_deleted: number; + skipped: number; + errors: number; + truncated: boolean; + next_cursor: string | null; +} diff --git a/roboco/api/routes/git.py b/roboco/api/routes/git.py index 1c15a087..f969fa7d 100644 --- a/roboco/api/routes/git.py +++ b/roboco/api/routes/git.py @@ -31,6 +31,8 @@ from roboco.api.deps import CurrentAgentContext, DbSession from roboco.api.schemas.git import ( BranchInfo, CommitInfo, + GitBranchCleanupRequest, + GitBranchCleanupResponse, GitBranchListResponse, GitCheckoutRequest, GitCheckoutResponse, @@ -763,3 +765,60 @@ async def rebase_branch( conflict=conflict, conflicted_files=conflicted_files, ) + + +@router.post("/branches/cleanup", response_model=GitBranchCleanupResponse) +@guard_deco.rate_limit(requests=5, window=60) +@guard_deco.max_request_size(size_bytes=65536) +@guard_deco.block_clouds() +@guard_deco.content_type_filter(["application/json"]) +async def cleanup_stale_branches( + data: GitBranchCleanupRequest, + db: DbSession, + agent: CurrentAgentContext, +) -> GitBranchCleanupResponse: + """Sweep a project's terminal-task branches (PM/CEO only). + + Deletes the remote + local branch of every completed/cancelled task in + the project (capped per call — see ``GitService.cleanup_stale_branches``), + skipping the default branch and any environment-ladder rung so a live + integration/prod branch is never touched. Role-gated identically to + ``/rebase`` — a history-affecting bulk operation shouldn't be open to + developers either. Purely a read + external-git-op endpoint: no task rows + are mutated, so there's nothing for this request to commit. + """ + if agent.role not in _REBASE_ALLOWED_ROLES: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=( + f"BRANCH_CLEANUP_ROLE_RESTRICTED: Role '{agent.role}' is not " + "permitted to sweep branches. Only CEO and PM roles (cell_pm, " + "main_pm) may use this endpoint." + ), + ) + project_slug = await _resolve_project_slug(data.project_slug, db) + git_service = get_git_service(db) + + try: + ( + remote_deleted, + local_deleted, + skipped, + errors, + truncated, + next_cursor, + ) = await git_service.cleanup_stale_branches( + project_slug, after_task_id=data.after_cursor + ) + except _TranslatableError as e: + raise _translate_error(e) from e + + return GitBranchCleanupResponse( + project_slug=project_slug, + remote_deleted=remote_deleted, + local_deleted=local_deleted, + skipped=skipped, + errors=errors, + truncated=truncated, + next_cursor=next_cursor, + ) diff --git a/roboco/api/schemas/git.py b/roboco/api/schemas/git.py index 410f6a76..970d8ccd 100644 --- a/roboco/api/schemas/git.py +++ b/roboco/api/schemas/git.py @@ -334,6 +334,42 @@ class GitRebaseResponse(BaseModel): conflicted_files: list[str] = [] +# ============================================================================= +# BRANCH CLEANUP +# ============================================================================= + + +class GitBranchCleanupRequest(BaseModel): + """Request to sweep a project's terminal-task branches. + + ``after_cursor`` resumes a capped sweep from a prior response's + ``next_cursor`` — task rows never change as a sweep side effect, so + without it a repeat call re-scans the same first window forever. + """ + + project_slug: str + after_cursor: UUID | None = None + + +class GitBranchCleanupResponse(BaseModel): + """Counts from a stale-branch cleanup sweep. + + ``local_deleted`` counts an attempted local delete (assignee/clone + resolved), not a confirmed one — the underlying ``git branch -D`` is + itself best-effort. ``truncated`` is True when more terminal-task + branches existed than the per-call cap; ``next_cursor`` is then the + resume point to pass back as ``after_cursor``. + """ + + project_slug: str + remote_deleted: int = 0 + local_deleted: int = 0 + skipped: int = 0 + errors: int = 0 + truncated: bool = False + next_cursor: str | None = None + + # ============================================================================= # GATEWAY-LAYER LIGHTWEIGHT SCHEMAS # diff --git a/roboco/services/git.py b/roboco/services/git.py index 48df2128..81d68147 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -42,7 +42,7 @@ if TYPE_CHECKING: GitCreatePRRequest, GitMergePRRequest, ) - from roboco.db.tables import TaskTable + from roboco.db.tables import ProjectTable, TaskTable from roboco.config import settings from roboco.exceptions import ( GitCommandError, @@ -53,7 +53,7 @@ from roboco.exceptions import ( from roboco.foundation.policy import lifecycle from roboco.foundation.policy.pr_labels import CONVENTIONS_PR_LABELS, derive_pr_labels from roboco.models.base import AgentRole, TaskStatus -from roboco.models.env_branches import head_branch +from roboco.models.env_branches import effective_environments, head_branch from roboco.services.base import ( BaseService, NotFoundError, @@ -3607,16 +3607,19 @@ class GitService(BaseService): async def _delete_remote_branch_best_effort( self, owner: str, repo: str, branch: str, git_token: str - ) -> None: + ) -> bool: """Best-effort: delete a remote branch by name. Silently swallows errors — cleanup is not critical. Skips branches that look like project defaults (main / master / develop) and any branch that still has open dependent PRs (an active integration target — deleting it - would strand in-flight child work). + would strand in-flight child work). Returns True if the delete request + was issued with no transport error, False on any skip/failure — callers + that only fire-and-forget can ignore it; the branch-cleanup sweep uses + it to report counts. """ if branch in ("main", "master", "develop", ""): - return + return False if await self._branch_has_open_dependents(owner, repo, branch, git_token): self.log.info( "branch delete skipped: open dependent PRs target it as base", @@ -3624,7 +3627,7 @@ class GitService(BaseService): owner=owner, repo=repo, ) - return + return False try: async with httpx.AsyncClient(timeout=10.0) as client: await client.delete( @@ -3635,8 +3638,9 @@ class GitService(BaseService): "X-GitHub-Api-Version": "2022-11-28", }, ) + return True except httpx.HTTPError: - return + return False async def _delete_pr_branch_best_effort( self, owner: str, repo: str, pr_number: int, git_token: str @@ -3664,15 +3668,25 @@ class GitService(BaseService): except httpx.HTTPError: return - async def delete_task_branch(self, project_slug: str, branch_name: str) -> None: + async def delete_task_branch(self, project_slug: str, branch_name: str) -> bool: """Delete a remote task branch after cancel/discard. Best-effort. Called by `TaskService` on cancellation so abandoned task - branches don't accumulate on the remote. + branches don't accumulate on the remote. Returns whether the delete + was actually issued (see ``_delete_remote_branch_best_effort``). + + This is the chokepoint every task-scoped remote-delete call routes + through, so the environment-ladder guard lives here rather than only + at each caller: ``_delete_remote_branch_best_effort``'s own + main/master/develop skip predates the env-ladder model and doesn't + know about it (it's a generic branch-delete primitive also used by + the merged-PR source-branch cleanup, which never targets a ladder + branch by construction) — a task's ``branch_name`` could otherwise + coincide with a ladder rung and get deleted out from under it. """ git_token = await self._token_for_project(project_slug) if not git_token: - return + return False # Resolve remote from any workspace — branch deletion only needs # the owner/repo, not a checkout. Use a service-root probe path # if no agent workspace is available. @@ -3680,14 +3694,148 @@ class GitService(BaseService): project_service = get_project_service(self.session) project = await project_service.get_by_slug(project_slug) if not project or not project.git_url: - return + return False + if branch_name in {r.branch for r in effective_environments(project)}: + return False owner, repo = self._parse_git_url(project.git_url) except Exception: - return - await self._delete_remote_branch_best_effort( + return False + return await self._delete_remote_branch_best_effort( owner, repo, branch_name, git_token ) + # Per-call cap on the stale-branch sweep so one request can't hang on an + # unbounded fan-out of remote-delete calls. + _CLEANUP_BRANCH_LIMIT = 200 + + async def cleanup_stale_branches( + self, project_slug: str, after_task_id: UUID | None = None + ) -> tuple[int, int, int, int, bool, str | None]: + """Sweep a project's terminal tasks and delete their spent branches. + + Candidates are TERMINAL (completed/cancelled) tasks with a + ``branch_name`` that isn't an environment-ladder rung (a ladder branch + outlives any one task — see ``roboco.models.env_branches``). Capped at + ``_CLEANUP_BRANCH_LIMIT`` per call; the window is deterministic + (``ORDER BY id``) and cursor-resumable via ``after_task_id`` — task + rows never change as a side effect of the sweep, so without a cursor a + repeat call would re-scan the identical first window forever instead + of progressing past the cap. Ladder-branch rows still advance the + cursor (processed-as-excluded), so ``truncated`` can't go false- + negative when rungs land inside the window. Per branch, best-effort: + remote delete (the same guarded ``delete_task_branch`` cancel already + uses — main/master/develop and open-dependent-PR branches are skipped + there too) and, in the assignee's clone, a force local delete (a + completed task's branch was squash-merged, so a safe ``-d`` would + refuse unconditionally; a cancelled one's work is discarded by + decision). + + Returns ``(remote_deleted, local_deleted, skipped, errors, truncated, + next_cursor)`` — ``next_cursor`` is the last processed task id when + truncated, to pass back as ``after_task_id``. ``local_deleted`` counts + a local delete as ATTEMPTED (assignee/clone resolved), not confirmed — + the underlying ``git branch -D`` is itself best-effort and reports no + outcome. ``skipped`` counts branches with no resolvable assignee/clone + (nothing to locally clean up, though the remote delete may still have + run); ``errors`` counts branches that raised unexpectedly while + resolving the assignee's workspace. + """ + project_service = get_project_service(self.session) + project = await project_service.get_by_slug(project_slug) + if not project: + return (0, 0, 0, 0, False, None) + + candidates, truncated, next_cursor = await self._stale_branch_window( + project, after_task_id + ) + + remote_deleted = local_deleted = skipped = errors = 0 + workspace_service = get_workspace_service(self.session) + for task in candidates: + branch = str(task.branch_name) + try: + remote_ok, local_attempted = await self._cleanup_one_stale_branch( + project_slug, task, branch, workspace_service + ) + except Exception as e: + errors += 1 + self.log.warning( + "Stale-branch cleanup skipped for branch", + project_slug=project_slug, + branch=branch, + error=str(e), + ) + continue + remote_deleted += int(remote_ok) + if local_attempted: + local_deleted += 1 + else: + skipped += 1 + + return (remote_deleted, local_deleted, skipped, errors, truncated, next_cursor) + + async def _stale_branch_window( + self, project: ProjectTable, after_task_id: UUID | None + ) -> tuple[list[TaskTable], bool, str | None]: + """Fetch one deterministic, cursor-resumable window of terminal-task + branch-cleanup candidates for ``cleanup_stale_branches``. + + Returns ``(candidates, truncated, next_cursor)`` — ladder-branch rows + stay in the window (and so still advance the cursor) but are excluded + from ``candidates``, matching the caller's docstring. + """ + from sqlalchemy import select + + from roboco.db.tables import TaskTable + + ladder_branches = {rung.branch for rung in effective_environments(project)} + query = ( + select(TaskTable) + .where(TaskTable.project_id == project.id) + .where(TaskTable.branch_name.is_not(None)) + .where(TaskTable.status.in_([TaskStatus.COMPLETED, TaskStatus.CANCELLED])) + .order_by(TaskTable.id) + .limit(self._CLEANUP_BRANCH_LIMIT + 1) + ) + if after_task_id is not None: + query = query.where(TaskTable.id > after_task_id) + result = await self.session.execute(query) + window = list(result.scalars().all()) + truncated = len(window) > self._CLEANUP_BRANCH_LIMIT + window = window[: self._CLEANUP_BRANCH_LIMIT] + next_cursor = str(window[-1].id) if truncated and window else None + candidates = [t for t in window if str(t.branch_name) not in ladder_branches] + return candidates, truncated, next_cursor + + async def _cleanup_one_stale_branch( + self, + project_slug: str, + task: TaskTable, + branch: str, + workspace_service: WorkspaceService, + ) -> tuple[bool, bool]: + """Delete one candidate's remote + local branch. + + Returns ``(remote_deleted, local_attempted)`` — see + ``cleanup_stale_branches`` for what each means. Raises on an + unexpected failure so the caller's per-branch try/except counts it. + """ + remote_deleted = await self.delete_task_branch(project_slug, branch) + + assignee = task.assignee + if assignee is None or assignee.team is None or assignee.slug is None: + return remote_deleted, False + + clone_root = workspace_service.get_clone_root_path( + project_slug, assignee.team, assignee.slug + ) + # force for every terminal candidate: a completed task's PR was + # squash-merged (its local ref is never an ancestor of the base, so + # -d refuses unconditionally), a cancelled one's work is discarded + # by decision — the ref is spent either way. + await workspace_service.delete_local_branch(clone_root, branch, force=True) + return remote_deleted, True + async def _first_allowed_merge_method( self, owner: str, diff --git a/roboco/services/task.py b/roboco/services/task.py index af9aa777..bd3408f6 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -6,6 +6,7 @@ Handles status transitions, assignments, and queries. """ import asyncio +import shutil from dataclasses import dataclass from datetime import UTC, datetime, timedelta from pathlib import Path @@ -60,7 +61,7 @@ from roboco.models.base import ( TaskType, Team, ) -from roboco.models.env_branches import head_branch +from roboco.models.env_branches import effective_environments, head_branch from roboco.models.permissions import AgentContext, TaskAction from roboco.models.task import TaskCreateRequest from roboco.models.work_session import WorkSessionCreate @@ -6723,30 +6724,36 @@ class TaskService(BaseService): await ws_service.abandon(require_uuid(task.work_session_id), reason=reason) async def _delete_task_branch_best_effort(self, task: TaskTable) -> None: - """Delete the task's remote branch + per-task worktree on cancel. + """Delete the task's remote + local branch and per-task worktree on + cancel. Best-effort, never raises. Skipped for tasks that didn't make it to a branch yet, or whose PR already merged (merge path deletes the source branch). The worktree at ``{clone_root}/.worktrees/{task-short}/`` is removed from the assignee's clone so cancelled tasks don't leak full - working trees on disk (F123). The stale-claim reaper must NOT call this - — it routes to ``pending`` for a re-claim that reuses the worktree. + working trees on disk (F123); the local branch ref (force-deleted — + cancelled work is discarded on purpose) and the task's video-preview + dir are cleaned up alongside it. The stale-claim reaper must NOT call + this — it routes to ``pending`` for a re-claim that reuses the + worktree. """ branch = task.branch_name if not branch: return try: project_result = await self.session.execute( - select(ProjectTable.slug).where(ProjectTable.id == task.project_id) + select(ProjectTable).where(ProjectTable.id == task.project_id) ) - project_slug = project_result.scalar_one_or_none() - if not project_slug: + project = project_result.scalar_one_or_none() + if not project: return from roboco.services.git import get_git_service git_service = get_git_service(self.session) - await git_service.delete_task_branch(project_slug, str(branch)) - await self._remove_task_worktree_best_effort(task, project_slug) + await git_service.delete_task_branch(project.slug, str(branch)) + await self._remove_task_worktree_best_effort( + task, project, force_branch_delete=True + ) except Exception as e: # Cleanup is best-effort — don't fail the cancel if the # remote is unreachable or the branch is already gone. @@ -6758,46 +6765,95 @@ class TaskService(BaseService): ) async def _remove_task_worktree_best_effort( - self, task: TaskTable, project_slug: str + self, task: TaskTable, project: ProjectTable, *, force_branch_delete: bool ) -> None: - """Remove the per-task worktree from the assignee's clone. Never raises. + """Remove the per-task worktree + local branch ref, and rmtree the + task's video-preview dir. Never raises. - No-op when the task has no resolvable assignee (pooled/unassigned at - cancel) or the assignee carries no team (can't form a clone path). + The worktree/local-branch step no-ops when the task has no resolvable + assignee (pooled/unassigned at cancel) or the assignee carries no team + (can't form a clone path); previews cleanup still runs regardless + (project-scoped, not clone-scoped). The local branch is skipped + entirely when it's still an environment-ladder rung — a ladder branch + outlives any one task. """ assignee = task.assignee - if assignee is None or assignee.team is None or assignee.slug is None: - return - from roboco.services.workspace import get_workspace_service + if ( + assignee is not None + and assignee.team is not None + and assignee.slug is not None + ): + from roboco.services.workspace import get_workspace_service - ws_service = get_workspace_service(self.session) - clone_root = ws_service.get_clone_root_path( - project_slug, assignee.team, assignee.slug - ) - worktree = clone_root / ".worktrees" / str(task.id)[:8] - await ws_service.remove_worktree(clone_root, worktree) + ws_service = get_workspace_service(self.session) + clone_root = ws_service.get_clone_root_path( + project.slug, assignee.team, assignee.slug + ) + worktree = clone_root / ".worktrees" / str(task.id)[:8] + await ws_service.remove_worktree(clone_root, worktree) + + branch = task.branch_name + ladder_branches = {r.branch for r in effective_environments(project)} + if branch and str(branch) not in ladder_branches: + await ws_service.delete_local_branch( + clone_root, str(branch), force=force_branch_delete + ) + + self._cleanup_task_previews_best_effort(task, project.slug) + + def _cleanup_task_previews_best_effort( + self, task: TaskTable, project_slug: str + ) -> None: + """Best-effort rmtree of the task's video-render preview dir. + + Nothing else prunes it (``_render_extract_frames`` writes frames but + no cleanup runs on task end), so a video-authoring task's previews + would otherwise leak on disk forever. Guards the resolved path stays + under the project's workspace dir before deleting anything. + """ + from roboco.config import settings + + project_dir = Path(settings.workspaces_root) / project_slug + previews_dir = project_dir / ".previews" / str(task.id)[:8] + try: + if not previews_dir.exists(): + return + if not previews_dir.resolve().is_relative_to(project_dir.resolve()): + return + shutil.rmtree(previews_dir) + except Exception as e: + self.log.warning( + "Preview cleanup skipped", task_id=str(task.id), error=str(e) + ) async def _remove_task_worktree_on_terminal(self, task: TaskTable) -> None: - """Best-effort per-task worktree removal on terminal completion. + """Best-effort per-task worktree + local-branch removal on terminal + completion. - Mirrors the cancel-path cleanup but WITHOUT deleting the remote branch - (the merge path already deleted it). A completed/merged task would - otherwise leak its worktree on disk until the whole agent is deleted - (F123). Best-effort: never raises, so a cleanup failure can't block - completion. No-op for branchless tasks (no worktree was ever cut). - Terminal-only by call site — earlier review states may bounce - ``needs_revision`` and need the worktree back. + Force-deletes the local branch ref (``-D``), same as the cancel path: + completion implies the PR already merged remotely (the merge path + deleted the remote branch), and the default merge method is SQUASH — + the local ref's commits are never ancestors of the base, so a "safe" + ``-d`` refuses every time and the ref would leak forever. The ref is + spent either way once the task is terminal. A completed task would + otherwise leak its worktree + branch ref on disk until the whole + agent is deleted (F123). Best-effort: never raises, so a cleanup + failure can't block completion. No-op for branchless tasks (no + worktree was ever cut). Terminal-only by call site — earlier review + states may bounce ``needs_revision`` and need the worktree back. """ if not task.branch_name: return try: result = await self.session.execute( - select(ProjectTable.slug).where(ProjectTable.id == task.project_id) + select(ProjectTable).where(ProjectTable.id == task.project_id) ) - project_slug = result.scalar_one_or_none() - if not project_slug: + project = result.scalar_one_or_none() + if not project: return - await self._remove_task_worktree_best_effort(task, project_slug) + await self._remove_task_worktree_best_effort( + task, project, force_branch_delete=True + ) except OSError as e: # FS/permission failure (stuck mount, perms) — systemic, not # task-specific. Track the streak and escalate a CEO alert once diff --git a/roboco/services/workspace.py b/roboco/services/workspace.py index c4054e01..e1becb87 100644 --- a/roboco/services/workspace.py +++ b/roboco/services/workspace.py @@ -784,6 +784,29 @@ class WorkspaceService: ) self._worktree_git(clone_root, ["worktree", "prune"], check=False) + async def delete_local_branch( + self, clone_root: Path, branch: str, *, force: bool + ) -> None: + """Delete a local branch ref in a clone. Best-effort, never raises. + + ``remove_worktree`` only detaches the worktree — the local + ``refs/heads/{branch}`` ref survives, so every task an agent ever + claimed leaks a permanent branch ref in that agent's clone. Callers + run this right after ``remove_worktree`` (the worktree must be gone + first, or ``git branch -d/-D`` refuses a branch still checked out + elsewhere in the clone). + + ``force=False`` uses ``-d`` (refuses an unmerged branch — a clean skip, + not an error, via ``check=False``); ``force=True`` uses ``-D``. A + missing branch is likewise a clean skip. Never touches main/master/ + develop or an empty branch name — mirrors + ``GitService._delete_remote_branch_best_effort``'s default-branch guard. + """ + if branch in ("main", "master", "develop", ""): + return + flag = "-D" if force else "-d" + self._worktree_git(clone_root, ["branch", flag, branch], check=False) + async def resolve_workspace( self, project_slug: str, diff --git a/tests/integration/test_git_cleanup_stale_branches.py b/tests/integration/test_git_cleanup_stale_branches.py new file mode 100644 index 00000000..089ab49f --- /dev/null +++ b/tests/integration/test_git_cleanup_stale_branches.py @@ -0,0 +1,302 @@ +"""GitService.cleanup_stale_branches — the branch-cleanup sweep's selection ++ per-branch delete logic (backs the panel's "Clean up stale branches" +button / POST /git/branches/cleanup). + +Real DB (ProjectTable/TaskTable/AgentTable) so the terminal-only + env-ladder +selection runs for real; the two external-I/O boundaries are mocked so the +test never makes a real GitHub API call or runs a real git subprocess: +``_delete_remote_branch_best_effort`` (network) and +``roboco.services.git.get_workspace_service`` (local clone/subprocess). +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, MagicMock +from uuid import UUID, uuid4 + +import pytest +import pytest_asyncio +from roboco.db.tables import AgentTable, ProjectTable, TaskTable +from roboco.models.base import ( + AgentRole, + AgentStatus, + TaskNature, + TaskStatus, + TaskType, + Team, +) +from roboco.services.git import GitService + +if TYPE_CHECKING: + from collections.abc import AsyncIterator + + from sqlalchemy.ext.asyncio import AsyncSession + + +@pytest_asyncio.fixture +async def cleanup_setup( + db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> AsyncIterator[dict[str, Any]]: + agent = AgentTable( + id=uuid4(), + name="Dev", + slug=f"be-dev-{uuid4().hex[:8]}", + role=AgentRole.DEVELOPER, + team=Team.BACKEND, + status=AgentStatus.ACTIVE, + model_config={}, + system_prompt="dev", + capabilities=[], + permissions={}, + metrics={}, + ) + db_session.add(agent) + await db_session.flush() + project = ProjectTable( + id=uuid4(), + name="P", + slug=f"p-{uuid4().hex[:8]}", + git_url="https://github.com/acme/repo.git", + default_branch="master", + assigned_cell=Team.BACKEND, + created_by=agent.id, + ) + db_session.add(project) + await db_session.flush() + + svc = GitService(db_session) + monkeypatch.setattr(svc, "_token_for_project", AsyncMock(return_value="tok")) + # Remote delete: no real HTTP — a bare True/False signal is all + # cleanup_stale_branches consumes from it. + monkeypatch.setattr( + svc, "_delete_remote_branch_best_effort", AsyncMock(return_value=True) + ) + ws_svc = MagicMock() + ws_svc.get_clone_root_path = MagicMock( + return_value=f"/data/workspaces/{project.slug}/backend/{agent.slug}" + ) + ws_svc.delete_local_branch = AsyncMock() + monkeypatch.setattr("roboco.services.git.get_workspace_service", lambda _s: ws_svc) + + yield { + "svc": svc, + "db": db_session, + "agent": agent, + "project": project, + "ws_svc": ws_svc, + } + + +def _task( + setup: dict[str, Any], + *, + branch: str, + status: TaskStatus, + assigned: bool = True, +) -> TaskTable: + task = TaskTable( + id=uuid4(), + title="t", + description="d", + status=status, + task_type=TaskType.CODE, + nature=TaskNature.TECHNICAL, + team=Team.BACKEND, + project_id=setup["project"].id, + created_by=setup["agent"].id, + assigned_to=setup["agent"].id if assigned else None, + acceptance_criteria=["ac"], + branch_name=branch, + ) + setup["db"].add(task) + return task + + +@pytest.mark.asyncio +async def test_only_terminal_tasks_are_candidates( + cleanup_setup: dict[str, Any], +) -> None: + _task(cleanup_setup, branch="feature/backend/pending", status=TaskStatus.PENDING) + _task( + cleanup_setup, + branch="feature/backend/inprogress", + status=TaskStatus.IN_PROGRESS, + ) + _task( + cleanup_setup, branch="feature/backend/completed", status=TaskStatus.COMPLETED + ) + _task( + cleanup_setup, branch="feature/backend/cancelled", status=TaskStatus.CANCELLED + ) + await cleanup_setup["db"].flush() + + result = await cleanup_setup["svc"].cleanup_stale_branches( + cleanup_setup["project"].slug + ) + + # Only the 2 terminal tasks are candidates — pending/in_progress untouched. + assert result == (2, 2, 0, 0, False, None) + + +@pytest.mark.asyncio +async def test_env_ladder_branch_is_excluded(cleanup_setup: dict[str, Any]) -> None: + project = cleanup_setup["project"] + project.environments = [ + {"name": "head", "branch": "develop"}, + {"name": "prod", "branch": "master"}, + ] + await cleanup_setup["db"].flush() + + # A terminal task whose branch happens to equal a ladder rung must never + # be touched — the ladder outlives any one task. + _task(cleanup_setup, branch="develop", status=TaskStatus.COMPLETED) + _task( + cleanup_setup, branch="feature/backend/real-task", status=TaskStatus.COMPLETED + ) + await cleanup_setup["db"].flush() + + result = await cleanup_setup["svc"].cleanup_stale_branches(project.slug) + + assert result == (1, 1, 0, 0, False, None) + cleanup_setup["ws_svc"].delete_local_branch.assert_awaited_once() + call = cleanup_setup["ws_svc"].delete_local_branch.await_args + assert call is not None + assert call.args[1] == "feature/backend/real-task" + + +@pytest.mark.asyncio +async def test_cancelled_task_force_deletes_local_branch( + cleanup_setup: dict[str, Any], +) -> None: + _task(cleanup_setup, branch="feature/backend/x", status=TaskStatus.CANCELLED) + await cleanup_setup["db"].flush() + + await cleanup_setup["svc"].cleanup_stale_branches(cleanup_setup["project"].slug) + + cleanup_setup["ws_svc"].delete_local_branch.assert_awaited_once() + call = cleanup_setup["ws_svc"].delete_local_branch.await_args + assert call is not None + assert call.kwargs["force"] is True + + +@pytest.mark.asyncio +async def test_completed_task_also_force_deletes_local_branch( + cleanup_setup: dict[str, Any], +) -> None: + # Completed ⇒ the PR already squash-merged, so the local ref is spent but + # never an ancestor of the base — a "safe" -d would refuse every time. + _task(cleanup_setup, branch="feature/backend/x", status=TaskStatus.COMPLETED) + await cleanup_setup["db"].flush() + + await cleanup_setup["svc"].cleanup_stale_branches(cleanup_setup["project"].slug) + + cleanup_setup["ws_svc"].delete_local_branch.assert_awaited_once() + call = cleanup_setup["ws_svc"].delete_local_branch.await_args + assert call is not None + assert call.kwargs["force"] is True + + +@pytest.mark.asyncio +async def test_no_assignee_skips_local_delete_but_still_attempts_remote( + cleanup_setup: dict[str, Any], +) -> None: + _task( + cleanup_setup, + branch="feature/backend/orphan", + status=TaskStatus.COMPLETED, + assigned=False, + ) + await cleanup_setup["db"].flush() + + result = await cleanup_setup["svc"].cleanup_stale_branches( + cleanup_setup["project"].slug + ) + + assert result == (1, 0, 1, 0, False, None) + cleanup_setup["ws_svc"].delete_local_branch.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_candidate_set_truncated_past_the_cap( + cleanup_setup: dict[str, Any], monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(GitService, "_CLEANUP_BRANCH_LIMIT", 2) + for i in range(3): + _task( + cleanup_setup, + branch=f"feature/backend/task-{i}", + status=TaskStatus.COMPLETED, + ) + await cleanup_setup["db"].flush() + + result = await cleanup_setup["svc"].cleanup_stale_branches( + cleanup_setup["project"].slug + ) + + assert result[:5] == (2, 2, 0, 0, True) + assert result[5] is not None # resume cursor for the next call + + +@pytest.mark.asyncio +async def test_capped_sweep_progresses_with_cursor( + cleanup_setup: dict[str, Any], monkeypatch: pytest.MonkeyPatch +) -> None: + """Repeated sweeps must cover NEW branches, not re-scan the same window — + task rows never change as a sweep side effect, so only the cursor moves + the window forward.""" + monkeypatch.setattr(GitService, "_CLEANUP_BRANCH_LIMIT", 2) + for i in range(5): + _task( + cleanup_setup, + branch=f"feature/backend/task-{i}", + status=TaskStatus.COMPLETED, + ) + await cleanup_setup["db"].flush() + svc = cleanup_setup["svc"] + slug = cleanup_setup["project"].slug + ws = cleanup_setup["ws_svc"] + + first = await svc.cleanup_stale_branches(slug) + assert first[4] is True and first[5] is not None + first_branches = {call.args[1] for call in ws.delete_local_branch.await_args_list} + ws.delete_local_branch.reset_mock() + + await svc.cleanup_stale_branches(slug, after_task_id=UUID(first[5])) + second_branches = {call.args[1] for call in ws.delete_local_branch.await_args_list} + + assert second_branches, "second sweep processed nothing" + assert first_branches.isdisjoint(second_branches), ( + f"second sweep re-touched {first_branches & second_branches}" + ) + + +@pytest.mark.asyncio +async def test_unknown_project_returns_zeroed_result( + cleanup_setup: dict[str, Any], +) -> None: + result = await cleanup_setup["svc"].cleanup_stale_branches("does-not-exist") + assert result == (0, 0, 0, 0, False, None) + + +@pytest.mark.asyncio +async def test_delete_task_branch_refuses_env_ladder_rung_directly( + cleanup_setup: dict[str, Any], +) -> None: + """The chokepoint guard, not just the sweep's candidate filter: even a + direct ``delete_task_branch`` call (e.g. the cancel-path caller in + task.py) must refuse a branch that is an environment-ladder rung — the + generic ``_delete_remote_branch_best_effort`` primitive's own + main/master/develop skip predates the ladder model and doesn't know it.""" + project = cleanup_setup["project"] + project.environments = [ + {"name": "head", "branch": "develop"}, + {"name": "prod", "branch": "master"}, + ] + await cleanup_setup["db"].flush() + + ok = await cleanup_setup["svc"].delete_task_branch(project.slug, "develop") + + assert ok is False + remote_delete = cleanup_setup["svc"]._delete_remote_branch_best_effort + remote_delete.assert_not_awaited() diff --git a/tests/integration/test_git_routes.py b/tests/integration/test_git_routes.py index 806ea23b..c3bf6ef4 100644 --- a/tests/integration/test_git_routes.py +++ b/tests/integration/test_git_routes.py @@ -1002,5 +1002,78 @@ async def test_merge_pr_without_task_id_no_422(git_client: dict) -> None: assert response.status_code == HTTPStatus.OK +# --------------------------------------------------------------------------- +# branches/cleanup +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_cleanup_branches_success(pm_git_client: dict) -> None: + with patch("roboco.api.routes.git.get_git_service") as mock_get: + svc = AsyncMock() + svc.cleanup_stale_branches = AsyncMock(return_value=(3, 2, 1, 0, False, None)) + mock_get.return_value = svc + response = await pm_git_client["client"].post( + "/api/git/branches/cleanup", + json={"project_slug": pm_git_client["project"].slug}, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.OK + data = response.json() + assert ( + data["remote_deleted"], + data["local_deleted"], + data["skipped"], + data["errors"], + data["truncated"], + ) == (3, 2, 1, 0, False) + svc.cleanup_stale_branches.assert_awaited_once_with( + pm_git_client["project"].slug, after_task_id=None + ) + + +@pytest.mark.asyncio +async def test_cleanup_branches_reports_truncation(pm_git_client: dict) -> None: + with patch("roboco.api.routes.git.get_git_service") as mock_get: + svc = AsyncMock() + svc.cleanup_stale_branches = AsyncMock( + return_value=(200, 190, 0, 0, True, "0" * 32) + ) + mock_get.return_value = svc + response = await pm_git_client["client"].post( + "/api/git/branches/cleanup", + json={"project_slug": pm_git_client["project"].slug}, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.OK + assert response.json()["truncated"] is True + + +@pytest.mark.asyncio +async def test_cleanup_branches_developer_gets_403(git_client: dict) -> None: + """git_client carries a DEVELOPER-role agent — same role gate as /rebase.""" + with patch("roboco.api.routes.git.get_git_service") as mock_get: + svc = AsyncMock() + mock_get.return_value = svc + response = await git_client["client"].post( + "/api/git/branches/cleanup", + json={"project_slug": git_client["project"].slug}, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.FORBIDDEN + assert "BRANCH_CLEANUP_ROLE_RESTRICTED" in response.json()["detail"] + mock_get.assert_not_called() + + +@pytest.mark.asyncio +async def test_cleanup_branches_project_not_found(pm_git_client: dict) -> None: + response = await pm_git_client["client"].post( + "/api/git/branches/cleanup", + json={"project_slug": "does-not-exist"}, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.NOT_FOUND + + # Re-export to keep import alive (TC reorders imports) _ = SimpleNamespace diff --git a/tests/unit/services/test_task_cancel_worktree_cleanup.py b/tests/unit/services/test_task_cancel_worktree_cleanup.py index 94e001af..e8d8620e 100644 --- a/tests/unit/services/test_task_cancel_worktree_cleanup.py +++ b/tests/unit/services/test_task_cancel_worktree_cleanup.py @@ -50,7 +50,17 @@ def _task(*, branch: str | None, assignee: MagicMock | None) -> MagicMock: def _project_result(slug: str | None) -> MagicMock: result = MagicMock() - result.scalar_one_or_none.return_value = slug + # `_delete_task_branch_best_effort` now fetches the full project row (not + # just the slug column) so it can resolve the environment ladder before + # deleting a local branch. `environments=None` degenerates to a + # single-branch ladder off `default_branch` (never equal to a task branch + # in these tests, so the ladder-exclusion guard is a no-op here). + project = ( + MagicMock(slug=slug, environments=None, default_branch="master") + if slug + else None + ) + result.scalar_one_or_none.return_value = project return result @@ -71,6 +81,7 @@ async def test_cancel_removes_worktree_for_assignee() -> None: ws_svc = MagicMock() ws_svc.get_clone_root_path = MagicMock(return_value=clone) ws_svc.remove_worktree = AsyncMock() + ws_svc.delete_local_branch = AsyncMock() with ( patch( @@ -97,6 +108,10 @@ async def test_cancel_removes_worktree_for_assignee() -> None: f"remove must target the task worktree {clone}/.worktrees/{short}; " f"got {args[1]}" ) + # Cancel force-deletes the local branch ref (-D) — the work is discarded. + ws_svc.delete_local_branch.assert_awaited_once_with( + clone, "feature/backend/abc12345", force=True + ) @pytest.mark.asyncio diff --git a/tests/unit/services/test_workspace_worktree_lifecycle.py b/tests/unit/services/test_workspace_worktree_lifecycle.py index 93fda610..062baf29 100644 --- a/tests/unit/services/test_workspace_worktree_lifecycle.py +++ b/tests/unit/services/test_workspace_worktree_lifecycle.py @@ -485,3 +485,89 @@ async def test_self_heal_recovers_clone_root_left_on_task_branch(clone: Path) -> assert (wt / ".git").is_file() assert _git(wt, "rev-parse", "--abbrev-ref", "HEAD").strip() == branch assert _git(clone, "rev-parse", "--abbrev-ref", "HEAD").strip() == "main" + + +# --------------------------------------------------------------------------- +# delete_local_branch — cleans up the branch ref remove_worktree leaves behind +# (worktree removal never deletes refs/heads/{branch}; every claimed task +# leaked a permanent local branch ref until this). +# --------------------------------------------------------------------------- + + +async def test_delete_local_branch_skips_default_branches(clone: Path) -> None: + svc = _service() + for branch in ("main", "master", "develop", ""): + await svc.delete_local_branch(clone, branch, force=True) + # "main" is the clone's actual current branch — still checked out, untouched. + assert _git(clone, "rev-parse", "--abbrev-ref", "HEAD").strip() == "main" + + +async def test_delete_local_branch_removes_merged_branch(clone: Path) -> None: + svc = _service() + _git(clone, "branch", "feature/merged") # no new commits -> already merged + assert _ref_exists(clone, "refs/heads/feature/merged") + + await svc.delete_local_branch(clone, "feature/merged", force=False) + + assert not _ref_exists(clone, "refs/heads/feature/merged") + + +async def test_delete_local_branch_not_merged_skips_without_force(clone: Path) -> None: + svc = _service() + _git(clone, "checkout", "-b", "feature/unmerged") + (clone / "work.txt").write_text("x") + _git(clone, "add", "work.txt") + _git(clone, "commit", "-m", "unmerged work") + _git(clone, "checkout", "main") + assert _ref_exists(clone, "refs/heads/feature/unmerged") + + await svc.delete_local_branch(clone, "feature/unmerged", force=False) + + # `-d` refuses an unmerged branch — a clean skip, not an error, ref stays. + assert _ref_exists(clone, "refs/heads/feature/unmerged") + + +async def test_delete_local_branch_force_deletes_unmerged(clone: Path) -> None: + svc = _service() + _git(clone, "checkout", "-b", "feature/unmerged") + (clone / "work.txt").write_text("x") + _git(clone, "add", "work.txt") + _git(clone, "commit", "-m", "unmerged work") + _git(clone, "checkout", "main") + assert _ref_exists(clone, "refs/heads/feature/unmerged") + + await svc.delete_local_branch(clone, "feature/unmerged", force=True) + + assert not _ref_exists(clone, "refs/heads/feature/unmerged") + + +async def test_delete_local_branch_squash_merged_needs_force(clone: Path) -> None: + # RoboCo's default merge method is SQUASH: the branch's commits are never + # ancestors of the base afterwards, so `-d` refuses even though the work + # fully landed. This is why every terminal-path caller passes force=True — + # with -d the completed-task ref would leak forever. + svc = _service() + _git(clone, "checkout", "-b", "feature/squashed") + (clone / "work.txt").write_text("x") + _git(clone, "add", "work.txt") + _git(clone, "commit", "-m", "feature work") + _git(clone, "checkout", "main") + _git(clone, "merge", "--squash", "feature/squashed") + _git(clone, "commit", "-m", "squash-merge feature") + assert (clone / "work.txt").exists() # the work landed on main + + await svc.delete_local_branch(clone, "feature/squashed", force=False) + assert _ref_exists(clone, "refs/heads/feature/squashed") # -d refused + + await svc.delete_local_branch(clone, "feature/squashed", force=True) + assert not _ref_exists(clone, "refs/heads/feature/squashed") + + +async def test_delete_local_branch_missing_branch_is_clean_skip(clone: Path) -> None: + svc = _service() + await svc.delete_local_branch( + clone, "feature/never-existed", force=False + ) # no error + await svc.delete_local_branch( + clone, "feature/never-existed", force=True + ) # no error diff --git a/tests/unit/services/test_worktree_cleanup_on_complete.py b/tests/unit/services/test_worktree_cleanup_on_complete.py index 0b38b5ff..c9f5a3d4 100644 --- a/tests/unit/services/test_worktree_cleanup_on_complete.py +++ b/tests/unit/services/test_worktree_cleanup_on_complete.py @@ -75,7 +75,7 @@ async def test_complete_removes_assignee_worktree_best_effort() -> None: result = await svc.complete(task.id) assert result is task - remove.assert_awaited_once_with(task, "roboco-api") + remove.assert_awaited_once_with(task, "roboco-api", force_branch_delete=True) @pytest.mark.asyncio @@ -149,7 +149,7 @@ async def test_ceo_approve_removes_assignee_worktree_best_effort() -> None: result = await svc.ceo_approve(task.id) assert result is task - remove.assert_awaited_once_with(task, "roboco-api") + remove.assert_awaited_once_with(task, "roboco-api", force_branch_delete=True) @pytest.mark.asyncio