diff --git a/tests/integration/test_git_routes.py b/tests/integration/test_git_routes.py index c6a29c74..e5a5e0f4 100644 --- a/tests/integration/test_git_routes.py +++ b/tests/integration/test_git_routes.py @@ -267,6 +267,37 @@ async def test_log_with_branch_success(git_client: dict) -> None: assert [c["author"] for c in commits] == ["me", "you"] +@pytest.mark.asyncio +async def test_log_resolves_through_head_ref_not_bare_branch( + git_client: dict, +) -> None: + """The route must route the requested branch through + ``_resolve_head_ref`` (fetch + prefer origin) instead of handing git the + bare branch name straight off whatever this clone happens to have on + disk — this clone is the CALLER's own, never the branch owner's, and a + left-over local ref from an earlier inspection can be pinned stale + (live 2026-07-24: a QA clone read a commit 5 review rounds old).""" + log_result = MagicMock() + log_result.returncode = 0 + log_result.stdout = "" + with patch("roboco.api.routes.git.get_git_service") as mock_get: + svc = AsyncMock() + svc.get_workspace = AsyncMock(return_value="/tmp/ws") + svc._token_for_branch = AsyncMock(return_value="tok") + svc._resolve_head_ref = AsyncMock(return_value="origin/feature/x") + svc._run_git = AsyncMock(return_value=log_result) + mock_get.return_value = svc + response = await git_client["client"].get( + f"/api/git/log?project_slug={git_client['project'].slug}&branch=feature/x", + headers=_HDR, + ) + assert response.status_code == HTTPStatus.OK + svc._resolve_head_ref.assert_awaited_once_with("/tmp/ws", "feature/x", token="tok") + svc._run_git.assert_awaited_once() + logged_args = svc._run_git.await_args.args[1] + assert logged_args[-1] == "origin/feature/x" + + @pytest.mark.asyncio async def test_log_no_branch_fetches_current(git_client: dict) -> None: log_result = MagicMock() @@ -325,7 +356,7 @@ async def test_log_service_error(git_client: dict) -> None: @pytest.mark.asyncio async def test_branches_local_only(git_client: dict) -> None: branch_result = MagicMock() - branch_result.stdout = "main|abc123\nfeature/x|def456\n" + branch_result.stdout = "refs/heads/main|abc123\nrefs/heads/feature/x|def456\n" with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws") @@ -337,12 +368,27 @@ async def test_branches_local_only(git_client: dict) -> None: headers=_HDR, ) assert response.status_code == HTTPStatus.OK + names = {b["name"]: b for b in response.json()["branches"]} + assert names["main"]["is_remote"] is False + assert names["feature/x"]["is_remote"] is False + # include_remote=False (default) never prunes. + svc.prune_remote_best_effort.assert_not_awaited() @pytest.mark.asyncio async def test_branches_with_remote(git_client: dict) -> None: + """Regression: `%(refname)` renders a remote-tracking ref as + `refs/remotes/origin/` (real git never emits the old stub's + `remotes/origin/` shape) — it must classify as remote with the + `refs/remotes/origin/` prefix stripped down to the bare branch name, and + the symbolic `origin/HEAD` ref must be dropped, not surfaced as a fake + branch named "HEAD".""" branch_result = MagicMock() - branch_result.stdout = "main|abc123\nremotes/origin/feature/y|def456\n" + branch_result.stdout = ( + "refs/heads/main|abc123\n" + "refs/remotes/origin/feature/y|def456\n" + "refs/remotes/origin/HEAD|abc123\n" + ) with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws") @@ -355,6 +401,11 @@ async def test_branches_with_remote(git_client: dict) -> None: headers=_HDR, ) assert response.status_code == HTTPStatus.OK + names = {b["name"]: b for b in response.json()["branches"]} + assert names["feature/y"]["is_remote"] is True + assert "origin/feature/y" not in names + assert "HEAD" not in names + svc.prune_remote_best_effort.assert_awaited_once_with("/tmp/ws") @pytest.mark.asyncio @@ -362,7 +413,7 @@ async def test_branches_skips_empty_lines(git_client: dict) -> None: """Line 246: empty line in branch output triggers continue.""" branch_result = MagicMock() # Embed an empty line between two branches. - branch_result.stdout = "main|abc\n\nfeature/x|def\n" + branch_result.stdout = "refs/heads/main|abc\n\nrefs/heads/feature/x|def\n" with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws") @@ -1030,5 +1081,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/gateway/test_choreographer_qa.py b/tests/unit/gateway/test_choreographer_qa.py index 429dc6d7..7f731c29 100644 --- a/tests/unit/gateway/test_choreographer_qa.py +++ b/tests/unit/gateway/test_choreographer_qa.py @@ -384,6 +384,94 @@ async def test_pass_review_succeeds_and_transitions() -> None: a2a_svc.send.assert_awaited_once() +@pytest.mark.asyncio +async def test_pass_review_rejects_without_criteria_verified_when_acs_present() -> None: + """A task with real acceptance criteria demands criteria_verified — a + gestalt "looks good" notes string alone is no longer enough.""" + qa_id = uuid4() + task_id = uuid4() + t = _qa_owned_task( + task_id, qa_id, acceptance_criteria=["returns 200", "includes timestamp"] + ) + task_svc = AsyncMock() + task_svc.get.return_value = t + task_svc.agent_for.return_value = _qa_agent_mock(qa_id) + journal_svc = AsyncMock() + journal_svc.has_learning_for_task.return_value = True + deps = _make_deps(task=task_svc, journal=journal_svc) + c = Choreographer(deps) + + notes = "x" * 100 + env = await c.pass_review(qa_id, task_id, notes=notes) + body = env.as_dict() + assert body["error"] == "invalid_state", body + assert "returns 200" in body["message"] + assert "includes timestamp" in body["message"] + + +@pytest.mark.asyncio +async def test_pass_review_renders_criteria_verified_into_notes() -> None: + """Happy path: every AC matched + evidenced renders '[AC] ...' lines into + the persisted qa_notes and the transition still fires.""" + qa_id = uuid4() + task_id = uuid4() + t = _qa_owned_task( + task_id, qa_id, acceptance_criteria=["returns 200", "includes timestamp"] + ) + after = MagicMock( + id=task_id, + status="awaiting_documentation", + assigned_to=qa_id, + team="backend", + pr_url="https://x/pr/8", + qa_evidence_inspected=True, + ) + task_svc = AsyncMock() + task_svc.get.return_value = t + task_svc.agent_for.return_value = _qa_agent_mock(qa_id) + task_svc.qa_pass.return_value = after + task_svc.documenter_for_team.return_value = MagicMock(id=uuid4()) + task_svc.session = MagicMock() + task_svc.session.begin_nested = MagicMock( + return_value=MagicMock( + __aenter__=AsyncMock(return_value=None), + __aexit__=AsyncMock(return_value=False), + ) + ) + _stub_empty_ledger(task_svc.session) + journal_svc = AsyncMock() + journal_svc.has_learning_for_task.return_value = True + a2a_svc = AsyncMock() + deps = _make_deps(task=task_svc, journal=journal_svc, a2a=a2a_svc) + c = Choreographer(deps) + + notes = ( + "Reviewed PR carefully. Rendered every scene and checked each frame " + "against the brief before approving." + ) + env = await c.pass_review( + qa_id, + task_id, + notes=notes, + criteria_verified=[ + {"criterion": "returns 200", "evidence": "test_healthz asserts 200"}, + { + "criterion": "includes timestamp", + "evidence": "frame diff shows ts field at README.md line 12", + }, + ], + ) + assert env.error is None, env.as_dict() + assert env.status == "awaiting_documentation" + task_svc.qa_pass.assert_awaited_once() + persisted_notes = task_svc.qa_pass.call_args.args[2] + assert "[AC] returns 200 — verified: test_healthz asserts 200" in persisted_notes + assert ( + "[AC] includes timestamp — verified: frame diff shows ts field at " + "README.md line 12" in persisted_notes + ) + + @pytest.mark.asyncio async def test_pass_review_not_assigned_returns_not_authorized() -> None: qa_id = uuid4() @@ -464,6 +552,34 @@ async def test_fail_review_requires_at_least_one_issue() -> None: assert "finding" in body["message"].lower() +@pytest.mark.asyncio +async def test_fail_review_rejects_prose_file_names_evidence_in_remediate() -> None: + qa_id = uuid4() + task_id = uuid4() + t = _qa_owned_task(task_id, qa_id) + task_svc = AsyncMock() + task_svc.get.return_value = t + task_svc.agent_for.return_value = _qa_agent_mock(qa_id) + journal_svc = AsyncMock() + journal_svc.has_learning_for_task.return_value = True + deps = _make_deps(task=task_svc, journal=journal_svc) + c = Choreographer(deps) + + findings = [ + { + "file": "PR #676 description", + "severity": "major", + "expected": "matches the acceptance criteria", + "actual": "diverges from the acceptance criteria", + } + ] + env = await c.fail_review(qa_id, task_id, findings=findings) + body = env.as_dict() + assert body["error"] == "invalid_state" + assert "evidence" in body["remediate"] + assert "file" in body["remediate"] + + @pytest.mark.asyncio async def test_fail_review_not_assigned_returns_not_authorized() -> None: qa_id = uuid4()