mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
#160 — panel /roboco-logo.png "received null": next/image optimizer fails for static public assets in Next.js standalone mode. Added `unoptimized` to the sidebar logo Image so it serves the static file directly (validated on panel rebuild). #161 — QA/doc evidence pr_diff_summary empty: A leaf dev branch's parent_branch_for is the cell-PM branch, which is never pushed (only devs push their leaf branch). diff against a non-existent origin/<parent> returned empty. Added GitService._resolve_diff_base + _default_branch_ref + _ref_exists: diff/list_changed_files fall back to the repo default branch (origin/HEAD → master/main) when origin/<parent> is absent. #162 — claim_doc_task BRANCH_MISMATCH loop: The documenter's clone is separate from the dev's; the task branch already existed (dev created it) so no checkout ran in the doc workspace — roboco_docs_write / commit failed BRANCH_MISMATCH and the doc looped. Fixes: (a) new GitService.checkout_branch_in_agent_workspace; claim_doc_task checks out the task branch into the doc clone (best-effort — a checkout hiccup never fails the claim). (b) BRANCH_MISMATCH remediate now lists all four role claim verbs (i_will_work_on / i_will_plan / claim_doc_task / claim_review). (d) give_me_work next-hint is role+status aware via _claim_verb_hint (doc→claim_doc_task, qa→claim_review, pm→i_will_plan, else dev). Facet (c) (i_am_blocked "Not Found" for doc) was only reachable via the stuck-without-checkout path; primary fix removes it. Smoke-11 reached dev→QA→doc (deepest ever) and validated the prior 6 fixes (panel flood gone, #158/#159/#157 confirmed). These three clear the doc-phase blockers found in that run.
This commit is contained in:
@@ -72,6 +72,7 @@ export function Sidebar() {
|
|||||||
width={32}
|
width={32}
|
||||||
height={32}
|
height={32}
|
||||||
priority
|
priority
|
||||||
|
unoptimized
|
||||||
className="h-8 w-8 rounded"
|
className="h-8 w-8 rounded"
|
||||||
/>
|
/>
|
||||||
<span className="font-semibold text-lg">RoboCo</span>
|
<span className="font-semibold text-lg">RoboCo</span>
|
||||||
|
|||||||
@@ -542,6 +542,26 @@ class Choreographer:
|
|||||||
|
|
||||||
# --- Phase 1 (developer) verbs ---
|
# --- Phase 1 (developer) verbs ---
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _claim_verb_hint(role: str, task: Any) -> str:
|
||||||
|
"""Role + status aware 'how to start this task' hint.
|
||||||
|
|
||||||
|
Task #162 facet (d): give_me_work hard-coded
|
||||||
|
``i_will_work_on(...)`` for every role/status. A documenter
|
||||||
|
handed an awaiting_documentation task (or QA an awaiting_qa
|
||||||
|
task) was told to call a dev verb it doesn't have — it looped.
|
||||||
|
Map to the verb that actually claims the task for this role.
|
||||||
|
"""
|
||||||
|
tid = str(getattr(task, "id", ""))
|
||||||
|
status = str(getattr(task, "status", ""))
|
||||||
|
if status == "awaiting_documentation":
|
||||||
|
return f"call claim_doc_task(task_id='{tid}') to start"
|
||||||
|
if status == "awaiting_qa":
|
||||||
|
return f"call claim_review(task_id='{tid}') to start"
|
||||||
|
if role in ("cell_pm", "main_pm", "product_owner", "head_marketing"):
|
||||||
|
return f"call i_will_plan(task_id='{tid}', plan='<plan>') to start"
|
||||||
|
return f"call i_will_work_on(task_id='{tid}', plan='<plan>') to start"
|
||||||
|
|
||||||
async def give_me_work(self, agent_id: UUID) -> Envelope:
|
async def give_me_work(self, agent_id: UUID) -> Envelope:
|
||||||
"""Return the agent's most-actionable task or signal idle."""
|
"""Return the agent's most-actionable task or signal idle."""
|
||||||
agent = await self._deps.task.agent_for(agent_id)
|
agent = await self._deps.task.agent_for(agent_id)
|
||||||
@@ -558,7 +578,7 @@ class Choreographer:
|
|||||||
return Envelope.ok(
|
return Envelope.ok(
|
||||||
status=str(t.status),
|
status=str(t.status),
|
||||||
task_id=str(t.id),
|
task_id=str(t.id),
|
||||||
next=f"call i_will_work_on(task_id='{t.id}', plan='<plan>') to start",
|
next=self._claim_verb_hint(role, t),
|
||||||
context_briefing=await self._briefing_for(agent_id, t.id),
|
context_briefing=await self._briefing_for(agent_id, t.id),
|
||||||
).with_introspection(task=t, role=role)
|
).with_introspection(task=t, role=role)
|
||||||
assigned = await self._deps.task.list_assigned_for_agent(agent_id)
|
assigned = await self._deps.task.list_assigned_for_agent(agent_id)
|
||||||
@@ -567,7 +587,7 @@ class Choreographer:
|
|||||||
return Envelope.ok(
|
return Envelope.ok(
|
||||||
status=str(t.status),
|
status=str(t.status),
|
||||||
task_id=str(t.id),
|
task_id=str(t.id),
|
||||||
next=f"call i_will_work_on(task_id='{t.id}', plan='<plan>') to start",
|
next=self._claim_verb_hint(role, t),
|
||||||
context_briefing=await self._briefing_for(agent_id, t.id),
|
context_briefing=await self._briefing_for(agent_id, t.id),
|
||||||
).with_introspection(task=t, role=role)
|
).with_introspection(task=t, role=role)
|
||||||
paused = await self._deps.task.list_paused_for_agent(agent_id)
|
paused = await self._deps.task.list_paused_for_agent(agent_id)
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ requires.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import contextlib
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
@@ -165,6 +166,16 @@ class DocMixin(_Base):
|
|||||||
# "start") but doc_claim is the runtime-correct specialized form
|
# "start") but doc_claim is the runtime-correct specialized form
|
||||||
# that keeps status at AWAITING_DOCUMENTATION. See module docstring.
|
# that keeps status at AWAITING_DOCUMENTATION. See module docstring.
|
||||||
t = await self.task.doc_claim(doc_agent_id, task_id)
|
t = await self.task.doc_claim(doc_agent_id, task_id)
|
||||||
|
# Task #162: the documenter's clone is separate from the dev's;
|
||||||
|
# the task branch already exists (dev created it) so no checkout
|
||||||
|
# ran in the doc's workspace. Put the doc on the task branch now
|
||||||
|
# so roboco_docs_write / commit don't fail BRANCH_MISMATCH.
|
||||||
|
# Best-effort — a checkout hiccup must not fail the claim.
|
||||||
|
if t.branch_name:
|
||||||
|
with contextlib.suppress(Exception):
|
||||||
|
await self.git.checkout_branch_in_agent_workspace(
|
||||||
|
t.branch_name, actor_agent_id=doc_agent_id
|
||||||
|
)
|
||||||
ev = await self._claim_doc_evidence(t, task_id)
|
ev = await self._claim_doc_evidence(t, task_id)
|
||||||
return Envelope.ok(
|
return Envelope.ok(
|
||||||
status=str(t.status),
|
status=str(t.status),
|
||||||
|
|||||||
+95
-20
@@ -536,11 +536,13 @@ class GitService(BaseService):
|
|||||||
if current_branch and current_branch != task_branch:
|
if current_branch and current_branch != task_branch:
|
||||||
raise ValidationError(
|
raise ValidationError(
|
||||||
f"BRANCH_MISMATCH: Workspace is on '{current_branch}' but "
|
f"BRANCH_MISMATCH: Workspace is on '{current_branch}' but "
|
||||||
f"task requires '{task_branch}'. Branches are auto-checked-"
|
f"task requires '{task_branch}'. The branch is checked out "
|
||||||
f"out when you call `i_will_work_on(task_id)` (devs) or "
|
f"into your clone by your role's claim verb: "
|
||||||
f"`i_will_plan(task_id, plan)` (PMs) — call your role's "
|
f"`i_will_work_on(task_id)` (devs), "
|
||||||
f"verb on the right task instead of switching branches by "
|
f"`i_will_plan(task_id, plan)` (PMs), "
|
||||||
f"hand."
|
f"`claim_doc_task(task_id)` (documenters), "
|
||||||
|
f"`claim_review(task_id)` (QA). Re-call your role's claim "
|
||||||
|
f"verb on this task instead of switching branches by hand."
|
||||||
)
|
)
|
||||||
|
|
||||||
async def _link_commit_to_task(
|
async def _link_commit_to_task(
|
||||||
@@ -1815,6 +1817,30 @@ class GitService(BaseService):
|
|||||||
workspace_agent_id = self._resolve_workspace_agent_id(task, actor_agent_id)
|
workspace_agent_id = self._resolve_workspace_agent_id(task, actor_agent_id)
|
||||||
return await self.get_workspace(project.slug, agent_id=workspace_agent_id)
|
return await self.get_workspace(project.slug, agent_id=workspace_agent_id)
|
||||||
|
|
||||||
|
async def checkout_branch_in_agent_workspace(
|
||||||
|
self,
|
||||||
|
branch_name: str,
|
||||||
|
*,
|
||||||
|
actor_agent_id: UUID,
|
||||||
|
) -> None:
|
||||||
|
"""Check out `branch_name` into the actor's own clone.
|
||||||
|
|
||||||
|
Task #162: dev/PM workspaces land on the right branch because
|
||||||
|
``_auto_create_branch`` runs ``git checkout -b`` in the dev's
|
||||||
|
clone at claim time. The documenter's clone is a *separate*
|
||||||
|
workspace; when it claims an awaiting_documentation task the
|
||||||
|
branch already exists (created by the dev) so no checkout ever
|
||||||
|
ran in the doc's clone — it stayed on the default branch and
|
||||||
|
``roboco_docs_write`` / ``commit`` failed with BRANCH_MISMATCH.
|
||||||
|
This puts the doc's workspace on the task branch (fetch +
|
||||||
|
tracking-branch create). Best-effort: a checkout failure must
|
||||||
|
not break the claim itself — the caller surfaces a remediation.
|
||||||
|
"""
|
||||||
|
workspace = await self._workspace_for_branch(
|
||||||
|
branch_name, actor_agent_id=actor_agent_id
|
||||||
|
)
|
||||||
|
await self.checkout(workspace, branch_name)
|
||||||
|
|
||||||
async def push_branch(
|
async def push_branch(
|
||||||
self,
|
self,
|
||||||
branch_name: str,
|
branch_name: str,
|
||||||
@@ -2102,6 +2128,61 @@ class GitService(BaseService):
|
|||||||
)
|
)
|
||||||
return str(base_ref)
|
return str(base_ref)
|
||||||
|
|
||||||
|
async def _ref_exists(self, workspace: Any, ref: str) -> bool:
|
||||||
|
"""True iff `ref` resolves in `workspace` (e.g. 'origin/<branch>')."""
|
||||||
|
result = await self._run_git(
|
||||||
|
workspace,
|
||||||
|
["rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"],
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
return result.returncode == 0
|
||||||
|
|
||||||
|
async def _default_branch_ref(self, workspace: Any) -> str:
|
||||||
|
"""Resolve the repo's default remote branch ref.
|
||||||
|
|
||||||
|
Tries ``origin/HEAD`` (the canonical pointer), then common
|
||||||
|
defaults. Always returns a usable ref string; falls back to
|
||||||
|
``origin/master`` so the diff command is still well-formed even
|
||||||
|
on a misconfigured remote (an empty/garbage diff is recoverable;
|
||||||
|
a malformed git invocation is not).
|
||||||
|
"""
|
||||||
|
head = await self._run_git(
|
||||||
|
workspace,
|
||||||
|
["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"],
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
target = head.stdout.strip()
|
||||||
|
if head.returncode == 0 and target:
|
||||||
|
# refs/remotes/origin/HEAD -> refs/remotes/origin/<name>
|
||||||
|
return target.replace("refs/remotes/", "", 1)
|
||||||
|
for candidate in ("origin/master", "origin/main"):
|
||||||
|
await self._run_git(
|
||||||
|
workspace,
|
||||||
|
["fetch", "origin", candidate.split("/", 1)[1]],
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if await self._ref_exists(workspace, candidate):
|
||||||
|
return candidate
|
||||||
|
return "origin/master"
|
||||||
|
|
||||||
|
async def _resolve_diff_base(self, workspace: Any, branch_name: str) -> str:
|
||||||
|
"""Best diff base for `branch_name` when no explicit base is given.
|
||||||
|
|
||||||
|
Task #161: a leaf dev branch's ``parent_branch_for`` is the
|
||||||
|
cell-PM branch (``feature/{team}/{root}--{cellpm}``) which is
|
||||||
|
NEVER pushed — only devs push their own leaf branch. Diffing
|
||||||
|
against a non-existent ``origin/<parent>`` returns an empty
|
||||||
|
diff, so QA / docs see nothing. Fall back to the repo default
|
||||||
|
branch when the computed parent ref is absent on origin.
|
||||||
|
"""
|
||||||
|
from roboco.services.gateway.merge_chain import parent_branch_for
|
||||||
|
|
||||||
|
parent = parent_branch_for(branch_name)
|
||||||
|
await self._run_git(workspace, ["fetch", "origin", parent], check=False)
|
||||||
|
if await self._ref_exists(workspace, f"origin/{parent}"):
|
||||||
|
return f"origin/{parent}"
|
||||||
|
return await self._default_branch_ref(workspace)
|
||||||
|
|
||||||
async def diff(
|
async def diff(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
@@ -2112,24 +2193,20 @@ class GitService(BaseService):
|
|||||||
"""Return the git diff for `branch_name` against `base`.
|
"""Return the git diff for `branch_name` against `base`.
|
||||||
|
|
||||||
When `base` is omitted, diffs against the branch's parent (per
|
When `base` is omitted, diffs against the branch's parent (per
|
||||||
`parent_branch_for`) which is what the choreographer/PR-review
|
`parent_branch_for`), falling back to the repo default branch
|
||||||
path wants. Content_actions evidence path can pass `HEAD~1` to
|
when that parent was never pushed (Task #161). Content_actions
|
||||||
get just the latest change diff for incremental review.
|
evidence path can pass `HEAD~1` for an incremental diff.
|
||||||
|
|
||||||
``actor_agent_id`` resolves the workspace via the caller's clone
|
``actor_agent_id`` resolves the workspace via the caller's clone
|
||||||
when ``task.assigned_to`` is None (audit D-40) — important for
|
when ``task.assigned_to`` is None (audit D-40) — important for
|
||||||
QA reviewing post-submit_qa.
|
QA reviewing post-submit_qa.
|
||||||
"""
|
"""
|
||||||
from roboco.services.gateway.merge_chain import parent_branch_for
|
|
||||||
|
|
||||||
workspace = await self._workspace_for_branch(
|
workspace = await self._workspace_for_branch(
|
||||||
branch_name, actor_agent_id=actor_agent_id
|
branch_name, actor_agent_id=actor_agent_id
|
||||||
)
|
)
|
||||||
if base is None:
|
if base is None:
|
||||||
parent = parent_branch_for(branch_name)
|
base_ref = await self._resolve_diff_base(workspace, branch_name)
|
||||||
# Make sure the parent ref exists locally before diffing.
|
diff_args = ["diff", f"{base_ref}...{branch_name}"]
|
||||||
await self._run_git(workspace, ["fetch", "origin", parent], check=False)
|
|
||||||
diff_args = ["diff", f"origin/{parent}...{branch_name}"]
|
|
||||||
else:
|
else:
|
||||||
diff_args = ["diff", f"{base}...{branch_name}"]
|
diff_args = ["diff", f"{base}...{branch_name}"]
|
||||||
diff_result = await self._run_git(workspace, diff_args, check=False)
|
diff_result = await self._run_git(workspace, diff_args, check=False)
|
||||||
@@ -2149,17 +2226,15 @@ class GitService(BaseService):
|
|||||||
authoritative git state — independent of whether the agent
|
authoritative git state — independent of whether the agent
|
||||||
ever called the legacy ``add_files_modified`` HTTP endpoint
|
ever called the legacy ``add_files_modified`` HTTP endpoint
|
||||||
(which the gateway commit() does not call). Empty paths are
|
(which the gateway commit() does not call). Empty paths are
|
||||||
skipped; output preserves git's order.
|
skipped; output preserves git's order. Same Task #161 default-
|
||||||
|
branch fallback as ``diff``.
|
||||||
"""
|
"""
|
||||||
from roboco.services.gateway.merge_chain import parent_branch_for
|
|
||||||
|
|
||||||
workspace = await self._workspace_for_branch(
|
workspace = await self._workspace_for_branch(
|
||||||
branch_name, actor_agent_id=actor_agent_id
|
branch_name, actor_agent_id=actor_agent_id
|
||||||
)
|
)
|
||||||
if base is None:
|
if base is None:
|
||||||
parent = parent_branch_for(branch_name)
|
base_ref = await self._resolve_diff_base(workspace, branch_name)
|
||||||
await self._run_git(workspace, ["fetch", "origin", parent], check=False)
|
args = ["diff", "--name-only", f"{base_ref}...{branch_name}"]
|
||||||
args = ["diff", "--name-only", f"origin/{parent}...{branch_name}"]
|
|
||||||
else:
|
else:
|
||||||
args = ["diff", "--name-only", f"{base}...{branch_name}"]
|
args = ["diff", "--name-only", f"{base}...{branch_name}"]
|
||||||
result = await self._run_git(workspace, args, check=False)
|
result = await self._run_git(workspace, args, check=False)
|
||||||
|
|||||||
@@ -0,0 +1,185 @@
|
|||||||
|
"""Task #162: claim_doc_task checks out the task branch + verb hints.
|
||||||
|
|
||||||
|
Smoke-11: be-doc claimed an awaiting_documentation task but its clone
|
||||||
|
stayed on the default branch (the branch was created in the dev's
|
||||||
|
separate clone). roboco_docs_write / commit failed BRANCH_MISMATCH and
|
||||||
|
the doc looped (i_am_blocked Not Found, give_me_work pointed at a dev
|
||||||
|
verb). Primary fix: claim_doc_task checks out the task branch into the
|
||||||
|
documenter's workspace. Facet (d): give_me_work's next-hint is now
|
||||||
|
role + status aware.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from datetime import UTC, datetime
|
||||||
|
from typing import Any
|
||||||
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
from uuid import uuid4
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||||
|
|
||||||
|
|
||||||
|
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||||
|
base: dict[str, Any] = {
|
||||||
|
"task": AsyncMock(),
|
||||||
|
"work_session": AsyncMock(),
|
||||||
|
"git": AsyncMock(),
|
||||||
|
"a2a": AsyncMock(),
|
||||||
|
"journal": AsyncMock(),
|
||||||
|
"audit": AsyncMock(),
|
||||||
|
"evidence_repo": AsyncMock(),
|
||||||
|
"messaging": AsyncMock(),
|
||||||
|
}
|
||||||
|
base.update(overrides)
|
||||||
|
repo = base["evidence_repo"]
|
||||||
|
for m in (
|
||||||
|
"list_unread_a2a",
|
||||||
|
"list_unread_mentions",
|
||||||
|
"list_pending_notifications",
|
||||||
|
"task_metadata_gaps",
|
||||||
|
"recent_team_activity",
|
||||||
|
"blockers_in_lane",
|
||||||
|
"journal_highlights_for_task",
|
||||||
|
):
|
||||||
|
getattr(repo, m).return_value = []
|
||||||
|
_ldef = base["journal"].latest_decision_at.return_value
|
||||||
|
if type(_ldef).__name__ in ("MagicMock", "AsyncMock"):
|
||||||
|
base["journal"].latest_decision_at.return_value = datetime.now(UTC)
|
||||||
|
return ChoreographerDeps(**base)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Facet (d): give_me_work next-hint is role + status aware
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _task(status: str) -> MagicMock:
|
||||||
|
t = MagicMock()
|
||||||
|
t.id = uuid4()
|
||||||
|
t.status = status
|
||||||
|
return t
|
||||||
|
|
||||||
|
|
||||||
|
def test_claim_verb_hint_doc_for_awaiting_documentation() -> None:
|
||||||
|
hint = Choreographer._claim_verb_hint("documenter", _task("awaiting_documentation"))
|
||||||
|
assert "claim_doc_task" in hint
|
||||||
|
assert "i_will_work_on" not in hint
|
||||||
|
|
||||||
|
|
||||||
|
def test_claim_verb_hint_qa_for_awaiting_qa() -> None:
|
||||||
|
hint = Choreographer._claim_verb_hint("qa", _task("awaiting_qa"))
|
||||||
|
assert "claim_review" in hint
|
||||||
|
assert "i_will_work_on" not in hint
|
||||||
|
|
||||||
|
|
||||||
|
def test_claim_verb_hint_pm_for_planning() -> None:
|
||||||
|
hint = Choreographer._claim_verb_hint("cell_pm", _task("pending"))
|
||||||
|
assert "i_will_plan" in hint
|
||||||
|
|
||||||
|
|
||||||
|
def test_claim_verb_hint_dev_default() -> None:
|
||||||
|
hint = Choreographer._claim_verb_hint("developer", _task("pending"))
|
||||||
|
assert "i_will_work_on" in hint
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Primary: claim_doc_task checks out the task branch into doc workspace
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_claim_doc_task_checks_out_branch() -> None:
|
||||||
|
"""After doc_claim, the task branch is checked out into the
|
||||||
|
documenter's own clone so roboco_docs_write / commit don't
|
||||||
|
BRANCH_MISMATCH."""
|
||||||
|
doc_id = uuid4()
|
||||||
|
task_id = uuid4()
|
||||||
|
branch = "feature/backend/root1234--cellpm56--dev78901"
|
||||||
|
|
||||||
|
t_initial = MagicMock(
|
||||||
|
id=task_id,
|
||||||
|
status="awaiting_documentation",
|
||||||
|
assigned_to=None,
|
||||||
|
task_type="documentation",
|
||||||
|
team="backend",
|
||||||
|
branch_name=branch,
|
||||||
|
quick_context=None,
|
||||||
|
documents=[],
|
||||||
|
commits=[{"sha": "abc123", "message": "[x] work"}],
|
||||||
|
pr_number=7,
|
||||||
|
pr_url="https://github.com/x/y/pull/7",
|
||||||
|
dev_notes="done",
|
||||||
|
acceptance_criteria_status=[],
|
||||||
|
work_session_id=uuid4(),
|
||||||
|
)
|
||||||
|
t_claimed = MagicMock(
|
||||||
|
**{
|
||||||
|
**t_initial.__dict__,
|
||||||
|
"assigned_to": doc_id,
|
||||||
|
"status": "awaiting_documentation",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.get.return_value = t_initial
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="documenter", team="backend")
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
task_svc.list_paused_for_agent.return_value = []
|
||||||
|
task_svc.doc_claim.return_value = t_claimed
|
||||||
|
git_svc = AsyncMock()
|
||||||
|
git_svc.diff.return_value = "diff"
|
||||||
|
git_svc.list_changed_files.return_value = ["README.md"]
|
||||||
|
|
||||||
|
deps = _make_deps(task=task_svc, git=git_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.claim_doc_task(doc_id, task_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] is None, body
|
||||||
|
git_svc.checkout_branch_in_agent_workspace.assert_awaited_once_with(
|
||||||
|
branch, actor_agent_id=doc_id
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_claim_doc_task_checkout_failure_does_not_break_claim() -> None:
|
||||||
|
"""A checkout hiccup must not fail the claim — the doc still gets
|
||||||
|
an ok envelope and can retry / escalate from a claimed state."""
|
||||||
|
doc_id = uuid4()
|
||||||
|
task_id = uuid4()
|
||||||
|
branch = "feature/backend/root1234--cellpm56--dev78901"
|
||||||
|
t_initial = MagicMock(
|
||||||
|
id=task_id,
|
||||||
|
status="awaiting_documentation",
|
||||||
|
assigned_to=None,
|
||||||
|
task_type="documentation",
|
||||||
|
team="backend",
|
||||||
|
branch_name=branch,
|
||||||
|
quick_context=None,
|
||||||
|
documents=[],
|
||||||
|
commits=[{"sha": "abc", "message": "[x] w"}],
|
||||||
|
pr_number=7,
|
||||||
|
pr_url="u",
|
||||||
|
dev_notes="d",
|
||||||
|
acceptance_criteria_status=[],
|
||||||
|
work_session_id=uuid4(),
|
||||||
|
)
|
||||||
|
t_claimed = MagicMock(**{**t_initial.__dict__, "assigned_to": doc_id})
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.get.return_value = t_initial
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="documenter", team="backend")
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
task_svc.list_paused_for_agent.return_value = []
|
||||||
|
task_svc.doc_claim.return_value = t_claimed
|
||||||
|
git_svc = AsyncMock()
|
||||||
|
git_svc.checkout_branch_in_agent_workspace.side_effect = RuntimeError("fetch fail")
|
||||||
|
git_svc.diff.return_value = ""
|
||||||
|
git_svc.list_changed_files.return_value = []
|
||||||
|
|
||||||
|
deps = _make_deps(task=task_svc, git=git_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.claim_doc_task(doc_id, task_id)
|
||||||
|
# Claim still succeeds despite checkout raising.
|
||||||
|
assert env.as_dict()["error"] is None
|
||||||
@@ -0,0 +1,90 @@
|
|||||||
|
"""Task #161: GitService diff base falls back to default branch.
|
||||||
|
|
||||||
|
A leaf dev branch's parent (per parent_branch_for) is the cell-PM
|
||||||
|
branch feature/{team}/{root}--{cellpm}, which is NEVER pushed — only
|
||||||
|
devs push their own leaf branch. Diffing against a non-existent
|
||||||
|
origin/<parent> returns an empty diff, so QA / docs saw nothing.
|
||||||
|
_resolve_diff_base must fall back to the repo default branch when the
|
||||||
|
parent ref is absent on origin.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Any
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from roboco.services.git import GitService
|
||||||
|
|
||||||
|
|
||||||
|
def _git_service() -> GitService:
|
||||||
|
return GitService.__new__(GitService)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_resolve_diff_base_uses_parent_when_pushed() -> None:
|
||||||
|
"""When origin/<parent> exists, use it (normal case)."""
|
||||||
|
svc = _git_service()
|
||||||
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
||||||
|
svc._ref_exists = AsyncMock(return_value=True) # type: ignore[method-assign]
|
||||||
|
ws = Path("/tmp/ws")
|
||||||
|
|
||||||
|
base = await svc._resolve_diff_base(
|
||||||
|
ws, "feature/backend/root1234--cellpm56--dev78901"
|
||||||
|
)
|
||||||
|
# parent_branch_for strips the last --segment.
|
||||||
|
assert base == "origin/feature/backend/root1234--cellpm56"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_resolve_diff_base_falls_back_when_parent_absent() -> None:
|
||||||
|
"""When origin/<parent> does NOT exist (cell-PM branch never pushed),
|
||||||
|
fall back to the repo default branch via origin/HEAD."""
|
||||||
|
svc = _git_service()
|
||||||
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
||||||
|
# parent ref absent → _ref_exists False for the parent check.
|
||||||
|
svc._ref_exists = AsyncMock(return_value=False) # type: ignore[method-assign]
|
||||||
|
svc._default_branch_ref = AsyncMock( # type: ignore[method-assign]
|
||||||
|
return_value="origin/master"
|
||||||
|
)
|
||||||
|
ws = Path("/tmp/ws")
|
||||||
|
|
||||||
|
base = await svc._resolve_diff_base(
|
||||||
|
ws, "feature/backend/root1234--cellpm56--dev78901"
|
||||||
|
)
|
||||||
|
assert base == "origin/master"
|
||||||
|
svc._default_branch_ref.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_default_branch_ref_prefers_origin_head() -> None:
|
||||||
|
"""origin/HEAD symbolic-ref is the canonical default-branch pointer."""
|
||||||
|
svc = _git_service()
|
||||||
|
|
||||||
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
||||||
|
if args[:2] == ["symbolic-ref", "--quiet"]:
|
||||||
|
return type(
|
||||||
|
"R", (), {"returncode": 0, "stdout": "refs/remotes/origin/main\n"}
|
||||||
|
)()
|
||||||
|
return type("R", (), {"returncode": 1, "stdout": ""})()
|
||||||
|
|
||||||
|
svc._run_git = fake_run # type: ignore[method-assign]
|
||||||
|
ref = await svc._default_branch_ref(Path("/tmp/ws"))
|
||||||
|
assert ref == "origin/main"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_default_branch_ref_fallback_when_no_head() -> None:
|
||||||
|
"""No origin/HEAD → probe origin/master then origin/main; final
|
||||||
|
hard fallback is origin/master so the git invocation stays valid."""
|
||||||
|
svc = _git_service()
|
||||||
|
|
||||||
|
async def fake_run(_ws: Any, _args: list[str], **_kw: Any) -> Any:
|
||||||
|
# symbolic-ref fails; fetches succeed but ref never verifies.
|
||||||
|
return type("R", (), {"returncode": 1, "stdout": ""})()
|
||||||
|
|
||||||
|
svc._run_git = fake_run # type: ignore[method-assign]
|
||||||
|
svc._ref_exists = AsyncMock(return_value=False) # type: ignore[method-assign]
|
||||||
|
ref = await svc._default_branch_ref(Path("/tmp/ws"))
|
||||||
|
assert ref == "origin/master"
|
||||||
Reference in New Issue
Block a user