mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(tasks): merge cross-lineage dependency content at branch cut (#466)
* fix(tasks): merge cross-lineage dependency content at branch cut The dependency gate enforced timing but never content: a dependent's fresh branch could miss a same-repo dependency's merged work when that merge landed outside the branch's own ancestor chain (cross-cell edges under one root, same-repo batch cross-root edges). After a successful branch cut, each dependency's real merge target (resolve_parent_branch) is fetched and, unless already an ancestor, merged into the new branch; conflicts abort cleanly (branch stays at its cut point, warning + an accumulating task marker note) and never fail the claim. Cross-repo dependencies are skipped — no shared history. Zero git work for the no-deps common case; resumes never re-enter (branch creation only). * chore(foundation): regenerate lifecycle artifacts; reflow inherited prose * [lineage] mypy-clean mock idioms in the lineage orchestration tests --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
@@ -0,0 +1,185 @@
|
||||
"""``GitService.merge_dependency_lineage`` — the cross-subtree/cross-cell
|
||||
dependency-lineage gap.
|
||||
|
||||
The claim-time dependency gate (``TaskService._claim_blocked_by_dependencies``)
|
||||
only enforces TIMING — a dependency must be terminal before the dependent task
|
||||
can claim — it never checks whether the dependency's MERGED content is
|
||||
reachable from the dependent's freshly cut branch. A cross-subtree/cross-cell
|
||||
edge (e.g. a UX cell task merged into the UX cell branch, depended on by a
|
||||
sibling frontend cell task cut from the frontend cell branch) can complete on
|
||||
a branch the dependent never descends from.
|
||||
|
||||
This backfills it at branch-cut time: fetch the dependency's merge-target
|
||||
branch, no-op if it is already an ancestor of the fresh branch (the common,
|
||||
transitively-safe case), otherwise merge it in. On a real conflict the merge
|
||||
is aborted and the branch is left exactly at its cut point — a content
|
||||
assist, never a gate, so it always returns a status rather than raising.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.git import GitService
|
||||
|
||||
_WORKSPACE = Path("/tmp/fake-ws")
|
||||
_TASK_ID = uuid4()
|
||||
_WORKTREE = _WORKSPACE / ".worktrees" / str(_TASK_ID)[:8]
|
||||
_BRANCH = "feature/frontend/root--fe-cell"
|
||||
_SOURCE = "feature/ux_ui/root--ux-cell"
|
||||
_ORIGIN_SOURCE = f"origin/{_SOURCE}"
|
||||
|
||||
|
||||
def _git_service() -> GitService:
|
||||
svc = GitService.__new__(GitService)
|
||||
svc.log = MagicMock()
|
||||
return svc
|
||||
|
||||
|
||||
def _result(returncode: int = 0, stdout: str = "") -> Any:
|
||||
r = MagicMock()
|
||||
r.returncode = returncode
|
||||
r.stdout = stdout
|
||||
return r
|
||||
|
||||
|
||||
def _drive(
|
||||
svc: GitService, responses: dict[tuple[str, ...], Any]
|
||||
) -> list[tuple[Path, list[str]]]:
|
||||
"""Stub _run_git to answer by args-prefix; _ensure_worktree_for_commit +
|
||||
_token_for_project are no-ops so only merge mechanics are under test."""
|
||||
calls: list[tuple[Path, list[str]]] = []
|
||||
|
||||
async def _run(workspace: Path, args: list[str], **_kw: object) -> Any:
|
||||
calls.append((Path(workspace), list(args)))
|
||||
for prefix, resp in responses.items():
|
||||
if tuple(args[: len(prefix)]) == prefix:
|
||||
return resp
|
||||
return _result()
|
||||
|
||||
object.__setattr__(svc, "_run_git", _run)
|
||||
object.__setattr__(svc, "_token_for_project", AsyncMock(return_value="tok"))
|
||||
object.__setattr__(svc, "_ensure_worktree_for_commit", AsyncMock())
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_already_ancestor_is_a_no_op() -> None:
|
||||
"""source_branch already reachable from branch_name: no merge, no push —
|
||||
the common, transitively-safe case (same-parent siblings, root waves)."""
|
||||
svc = _git_service()
|
||||
calls = _drive(
|
||||
svc,
|
||||
{
|
||||
("rev-parse", "--verify", "--quiet"): _result(returncode=0),
|
||||
("merge-base", "--is-ancestor"): _result(returncode=0),
|
||||
},
|
||||
)
|
||||
|
||||
result = await svc.merge_dependency_lineage(
|
||||
_WORKSPACE, _TASK_ID, _BRANCH, _SOURCE, project_slug="roboco-api"
|
||||
)
|
||||
|
||||
assert result == {"status": "already_ancestor"}
|
||||
assert not [c for c in calls if c[1][:1] == ["merge"]], (
|
||||
"an already-ancestor source must never be merged"
|
||||
)
|
||||
assert not [c for c in calls if c[1][:1] == ["push"]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_ref_short_circuits_before_touching_worktree() -> None:
|
||||
"""source_branch not on origin: no worktree touched, no merge attempted."""
|
||||
svc = _git_service()
|
||||
calls = _drive(svc, {("rev-parse", "--verify", "--quiet"): _result(returncode=1)})
|
||||
|
||||
result = await svc.merge_dependency_lineage(
|
||||
_WORKSPACE, _TASK_ID, _BRANCH, _SOURCE, project_slug="roboco-api"
|
||||
)
|
||||
|
||||
assert result == {"status": "missing_ref"}
|
||||
ensure = object.__getattribute__(svc, "_ensure_worktree_for_commit")
|
||||
ensure.assert_not_awaited()
|
||||
assert not [c for c in calls if c[1][:1] == ["merge-base"]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_outside_lineage_merges_and_pushes_from_the_worktree() -> None:
|
||||
"""source_branch NOT an ancestor: merged into the worktree and pushed —
|
||||
the actual gap fix (cross-cell dependency content backfilled)."""
|
||||
svc = _git_service()
|
||||
calls = _drive(
|
||||
svc,
|
||||
{
|
||||
("rev-parse", "--verify", "--quiet"): _result(returncode=0),
|
||||
("merge-base", "--is-ancestor"): _result(returncode=1),
|
||||
("merge", "--no-edit"): _result(returncode=0),
|
||||
("push", "origin"): _result(returncode=0),
|
||||
},
|
||||
)
|
||||
|
||||
result = await svc.merge_dependency_lineage(
|
||||
_WORKSPACE, _TASK_ID, _BRANCH, _SOURCE, project_slug="roboco-api"
|
||||
)
|
||||
|
||||
assert result == {"status": "merged"}
|
||||
merge_calls = [c for c in calls if c[1][:1] == ["merge"]]
|
||||
push_calls = [c for c in calls if c[1][:1] == ["push"]]
|
||||
assert merge_calls and all(ws == _WORKTREE for ws, _ in merge_calls), (
|
||||
"the merge must run in the task's worktree, not the shared clone"
|
||||
)
|
||||
assert push_calls and push_calls[0][1] == ["push", "origin", _BRANCH]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_merge_conflict_aborts_and_reports_files() -> None:
|
||||
"""A real conflict: abort, leave the branch at its cut point, never push."""
|
||||
svc = _git_service()
|
||||
calls = _drive(
|
||||
svc,
|
||||
{
|
||||
("rev-parse", "--verify", "--quiet"): _result(returncode=0),
|
||||
("merge-base", "--is-ancestor"): _result(returncode=1),
|
||||
("merge", "--no-edit"): _result(returncode=1),
|
||||
("diff", "--name-only", "--diff-filter=U"): _result(
|
||||
stdout="src/a.py\nsrc/b.py\n"
|
||||
),
|
||||
("merge", "--abort"): _result(returncode=0),
|
||||
},
|
||||
)
|
||||
|
||||
result = await svc.merge_dependency_lineage(
|
||||
_WORKSPACE, _TASK_ID, _BRANCH, _SOURCE, project_slug="roboco-api"
|
||||
)
|
||||
|
||||
assert result == {"status": "conflict", "files": ["src/a.py", "src/b.py"]}
|
||||
assert not [c for c in calls if c[1][:1] == ["push"]], (
|
||||
"a conflicted merge must never push"
|
||||
)
|
||||
abort_calls = [c for c in calls if c[1] == ["merge", "--abort"]]
|
||||
assert abort_calls, "a failed merge must be aborted, restoring the branch"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_merge_succeeds_but_push_fails_is_reported_distinctly() -> None:
|
||||
"""A clean local merge that can't reach origin is not silently "merged"."""
|
||||
svc = _git_service()
|
||||
_drive(
|
||||
svc,
|
||||
{
|
||||
("rev-parse", "--verify", "--quiet"): _result(returncode=0),
|
||||
("merge-base", "--is-ancestor"): _result(returncode=1),
|
||||
("merge", "--no-edit"): _result(returncode=0),
|
||||
("push", "origin"): _result(returncode=1),
|
||||
},
|
||||
)
|
||||
|
||||
result = await svc.merge_dependency_lineage(
|
||||
_WORKSPACE, _TASK_ID, _BRANCH, _SOURCE, project_slug="roboco-api"
|
||||
)
|
||||
|
||||
assert result == {"status": "merged_push_failed"}
|
||||
@@ -0,0 +1,277 @@
|
||||
"""``TaskService._apply_dependency_lineage`` — orchestration of the
|
||||
cross-subtree/cross-cell dependency-lineage gap fix at the branch-cut seam.
|
||||
|
||||
The dependency gate (``_claim_blocked_by_dependencies``) enforces TIMING —
|
||||
a task can't claim until every ``dependency_ids`` entry is terminal — but
|
||||
never CONTENT: a dependency's merged work can sit on a branch this task's
|
||||
freshly cut branch never descends from (a cross-subtree/cross-cell edge, or
|
||||
a batch cross-root edge). ``_apply_dependency_lineage`` runs right after
|
||||
branch creation, resolves each dependency's real merge target via
|
||||
``merge_chain.resolve_parent_branch``, and delegates the ancestor check +
|
||||
merge to ``GitService.merge_dependency_lineage`` (covered separately in
|
||||
``test_git_dependency_lineage_merge.py``). This is best-effort by
|
||||
construction: nothing here may ever raise back into the claim.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.foundation.policy.content import markers
|
||||
from roboco.services.task import TaskService, _LineageCutContext
|
||||
|
||||
_MERGE_CHAIN_RESOLVE = "roboco.services.gateway.merge_chain.resolve_parent_branch"
|
||||
_WORKSPACE = Path("/tmp/ws")
|
||||
_BRANCH = "feature/frontend/root--fe-cell"
|
||||
|
||||
|
||||
def _service() -> TaskService:
|
||||
svc = TaskService.__new__(TaskService)
|
||||
svc.log = MagicMock()
|
||||
session = MagicMock()
|
||||
session.flush = AsyncMock()
|
||||
svc.session = session
|
||||
return svc
|
||||
|
||||
|
||||
def _task(**over: Any) -> MagicMock:
|
||||
task = MagicMock(
|
||||
id=over.pop("id", uuid4()),
|
||||
project_id=over.pop("project_id", uuid4()),
|
||||
dependency_ids=over.pop("dependency_ids", []),
|
||||
orchestration_markers=over.pop("orchestration_markers", None),
|
||||
branch_name=over.pop("branch_name", _BRANCH),
|
||||
)
|
||||
for key, value in over.items():
|
||||
setattr(task, key, value)
|
||||
return task
|
||||
|
||||
|
||||
def _ctx(**over: Any) -> _LineageCutContext:
|
||||
git_service = over.pop("git_service", None) or MagicMock()
|
||||
project = over.pop("project", None) or MagicMock(id=uuid4(), slug="roboco-api")
|
||||
return _LineageCutContext(
|
||||
git_service=git_service,
|
||||
workspace=over.pop("workspace", _WORKSPACE),
|
||||
project=project,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_dependencies_does_no_work() -> None:
|
||||
"""Zero dependency_ids: no DB lookups, no git calls, no flush."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
any_svc.get = AsyncMock()
|
||||
task = _task(dependency_ids=[])
|
||||
ctx = _ctx()
|
||||
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
any_svc.get.assert_not_awaited()
|
||||
ctx.git_service.merge_dependency_lineage.assert_not_called()
|
||||
any_svc.session.flush.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_same_project_dependency_resolves_source_and_calls_merge() -> None:
|
||||
"""A same-repo dependency: resolve its real merge target (the branch its
|
||||
PR merged into) and hand it to GitService for the ancestor check/merge."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
dep_id = uuid4()
|
||||
dep_task = _task(id=dep_id, project_id=project.id)
|
||||
task = _task(project_id=project.id, dependency_ids=[dep_id])
|
||||
any_svc.get = AsyncMock(return_value=dep_task)
|
||||
|
||||
git_service = MagicMock()
|
||||
any_git_service: Any = git_service
|
||||
any_git_service.merge_dependency_lineage = AsyncMock(
|
||||
return_value={"status": "merged"}
|
||||
)
|
||||
ctx = _ctx(git_service=git_service, project=project)
|
||||
|
||||
with patch(
|
||||
_MERGE_CHAIN_RESOLVE,
|
||||
AsyncMock(return_value="feature/ux_ui/root--ux-cell"),
|
||||
):
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
any_git_service.merge_dependency_lineage.assert_awaited_once_with(
|
||||
_WORKSPACE,
|
||||
task.id,
|
||||
_BRANCH,
|
||||
"feature/ux_ui/root--ux-cell",
|
||||
project_slug="roboco-api",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cross_project_dependency_is_skipped() -> None:
|
||||
"""A dependency in a DIFFERENT repo has no shared git history — skip it
|
||||
rather than attempt a meaningless cross-repo merge."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
dep_id = uuid4()
|
||||
dep_task = _task(id=dep_id, project_id=uuid4()) # different project
|
||||
task = _task(project_id=project.id, dependency_ids=[dep_id])
|
||||
any_svc.get = AsyncMock(return_value=dep_task)
|
||||
ctx = _ctx(project=project)
|
||||
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
ctx.git_service.merge_dependency_lineage.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_dependency_task_is_skipped() -> None:
|
||||
"""A dependency id that no longer resolves to a task: skip, don't crash."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
task = _task(project_id=project.id, dependency_ids=[uuid4()])
|
||||
any_svc.get = AsyncMock(return_value=None)
|
||||
ctx = _ctx(project=project)
|
||||
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
ctx.git_service.merge_dependency_lineage.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_conflict_status_notes_transition_and_warns_but_does_not_raise() -> None:
|
||||
"""A real conflict never fails the claim: it logs + leaves a task note
|
||||
naming the dependency and its branch so a human follows up."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
dep_id = uuid4()
|
||||
dep_task = _task(id=dep_id, project_id=project.id)
|
||||
task = _task(project_id=project.id, dependency_ids=[dep_id])
|
||||
any_svc.get = AsyncMock(return_value=dep_task)
|
||||
|
||||
git_service = MagicMock()
|
||||
any_git_service: Any = git_service
|
||||
any_git_service.merge_dependency_lineage = AsyncMock(
|
||||
return_value={"status": "conflict", "files": ["src/a.py"]}
|
||||
)
|
||||
ctx = _ctx(git_service=git_service, project=project)
|
||||
|
||||
with patch(
|
||||
_MERGE_CHAIN_RESOLVE,
|
||||
AsyncMock(return_value="feature/ux_ui/root--ux-cell"),
|
||||
):
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
note = markers.get_transition_note(task, "dependency_lineage_conflict")
|
||||
assert note is not None
|
||||
assert str(dep_id) in note
|
||||
assert "src/a.py" in note
|
||||
assert "feature/ux_ui/root--ux-cell" in note
|
||||
svc.log.warning.assert_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_second_conflict_appends_rather_than_overwrites() -> None:
|
||||
"""Two conflicting dependencies on one branch: both surface, not just
|
||||
the last one — set_transition_note's per-event dict would otherwise
|
||||
silently drop the first."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
dep_a, dep_b = uuid4(), uuid4()
|
||||
dep_task_a = _task(id=dep_a, project_id=project.id)
|
||||
dep_task_b = _task(id=dep_b, project_id=project.id)
|
||||
task = _task(project_id=project.id, dependency_ids=[dep_a, dep_b])
|
||||
any_svc.get = AsyncMock(side_effect=[dep_task_a, dep_task_b])
|
||||
|
||||
git_service = MagicMock()
|
||||
any_git_service: Any = git_service
|
||||
any_git_service.merge_dependency_lineage = AsyncMock(
|
||||
side_effect=[
|
||||
{"status": "conflict", "files": ["a.py"]},
|
||||
{"status": "conflict", "files": ["b.py"]},
|
||||
]
|
||||
)
|
||||
ctx = _ctx(git_service=git_service, project=project)
|
||||
|
||||
with patch(
|
||||
_MERGE_CHAIN_RESOLVE,
|
||||
AsyncMock(side_effect=["feature/x/one", "feature/x/two"]),
|
||||
):
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
note = markers.get_transition_note(task, "dependency_lineage_conflict")
|
||||
assert note is not None
|
||||
assert str(dep_a) in note
|
||||
assert str(dep_b) in note
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_merge_error_is_swallowed_never_raised() -> None:
|
||||
"""This is a claim-time content assist, never a gate: any unexpected
|
||||
failure (network, resolver bug) is logged and swallowed."""
|
||||
svc = _service()
|
||||
any_svc: Any = svc
|
||||
project = MagicMock(id=uuid4(), slug="roboco-api")
|
||||
dep_id = uuid4()
|
||||
dep_task = _task(id=dep_id, project_id=project.id)
|
||||
task = _task(project_id=project.id, dependency_ids=[dep_id])
|
||||
any_svc.get = AsyncMock(return_value=dep_task)
|
||||
|
||||
git_service = MagicMock()
|
||||
any_git_service: Any = git_service
|
||||
any_git_service.merge_dependency_lineage = AsyncMock(
|
||||
side_effect=RuntimeError("boom")
|
||||
)
|
||||
ctx = _ctx(git_service=git_service, project=project)
|
||||
|
||||
with patch(_MERGE_CHAIN_RESOLVE, AsyncMock(return_value="feature/x/one")):
|
||||
await svc._apply_dependency_lineage(task, ctx)
|
||||
|
||||
svc.log.warning.assert_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_branch_in_project_wires_apply_dependency_lineage() -> None:
|
||||
"""Wiring check: after a successful branch cut, _create_branch_in_project
|
||||
calls the lineage step with the CONCRETE project just resolved — correct
|
||||
both for a plain task and inside the coordination-root per-project loop
|
||||
(_ensure_coordination_root_branches calls this once per spanned repo)."""
|
||||
svc = _service()
|
||||
task = _task(branch_name=None, dependency_ids=[uuid4()])
|
||||
project = MagicMock(id=task.project_id, slug="roboco-api")
|
||||
|
||||
object.__setattr__(svc, "_resolve_parent_branch", AsyncMock(return_value=None))
|
||||
object.__setattr__(svc, "_resolve_team_dir", MagicMock(return_value="backend"))
|
||||
|
||||
git_service = MagicMock()
|
||||
any_git_service: Any = git_service
|
||||
any_git_service.get_workspace = AsyncMock(return_value=_WORKSPACE)
|
||||
any_git_service.create_branch = AsyncMock(return_value=("feature/x", "master"))
|
||||
|
||||
applied: list[tuple[Any, _LineageCutContext]] = []
|
||||
|
||||
async def _fake_apply(t: Any, ctx: _LineageCutContext) -> None:
|
||||
applied.append((t, ctx))
|
||||
|
||||
object.__setattr__(svc, "_apply_dependency_lineage", _fake_apply)
|
||||
|
||||
with patch(
|
||||
"roboco.services.git.get_git_service", MagicMock(return_value=git_service)
|
||||
):
|
||||
out = await svc._create_branch_in_project(task, uuid4(), project)
|
||||
|
||||
assert out == "feature/x"
|
||||
assert len(applied) == 1
|
||||
got_task, got_ctx = applied[0]
|
||||
assert got_task is task
|
||||
assert got_ctx.git_service is git_service
|
||||
assert got_ctx.workspace == _WORKSPACE
|
||||
assert got_ctx.project is project
|
||||
Reference in New Issue
Block a user