mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): root PR base resolves the project's env ladder, not literal master
A parentless root's PR base / merge target now resolves through resolve_parent_branch to the project's panel-configured head rung — submit_root passed a hardcoded 'master', which on a main-default repo made _ensure_base_on_remote silently create a spurious master branch and land the assembled root PR there. Literal master survives only as the no-project string-derivation fallback.
This commit is contained in:
@@ -290,13 +290,17 @@ class VerbRunner:
|
|||||||
)
|
)
|
||||||
|
|
||||||
async def _do_create_root_pr(self, task: Any, agent: Any) -> Any:
|
async def _do_create_root_pr(self, task: Any, agent: Any) -> Any:
|
||||||
# Root→master PR for the in-path gate's root level (submit_root). The
|
from roboco.services.gateway.merge_chain import resolve_parent_branch
|
||||||
# base is always master and is_root_pr marks it for the CEO-merge path.
|
|
||||||
# The PM opening the master PR is the assigned_to-may-be-None case
|
# Root PR for the in-path gate's root level (submit_root). The base
|
||||||
# create_pr's actor_agent_id exists for.
|
# is the project's head rung (panel-configured env ladder; "master"
|
||||||
|
# only as the no-project string fallback) and is_root_pr marks it
|
||||||
|
# for the CEO-merge path. The PM opening it is the
|
||||||
|
# assigned_to-may-be-None case create_pr's actor_agent_id exists for.
|
||||||
|
parent = await resolve_parent_branch(task, self.task_service)
|
||||||
return await self.git_service.create_pr(
|
return await self.git_service.create_pr(
|
||||||
task.branch_name,
|
task.branch_name,
|
||||||
parent="master",
|
parent=parent,
|
||||||
is_root_pr=True,
|
is_root_pr=True,
|
||||||
actor_agent_id=agent.id,
|
actor_agent_id=agent.id,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -76,8 +76,12 @@ async def resolve_parent_branch(task: Any, task_service: Any) -> str:
|
|||||||
created — the merge then has no valid target and the cell↔Main-PM loop
|
created — the merge then has no valid target and the cell↔Main-PM loop
|
||||||
wedges. In that case fall back to the child task's own project
|
wedges. In that case fall back to the child task's own project
|
||||||
default branch (e.g. master), which is what the child branch was actually
|
default branch (e.g. master), which is what the child branch was actually
|
||||||
cut from. Only when there is genuinely no project to consult do we fall
|
cut from. A PARENTLESS root resolves the same way: its branch was cut
|
||||||
back to pure string derivation.
|
from the project's head rung (panel-configured env ladder), so that rung
|
||||||
|
is the root PR base / merge target — never a literal ``master``, which
|
||||||
|
on a ``main``-default repo silently targets a branch the project doesn't
|
||||||
|
use. Only when there is genuinely no project to consult do we fall back
|
||||||
|
to pure string derivation.
|
||||||
"""
|
"""
|
||||||
parent_id = getattr(task, "parent_task_id", None)
|
parent_id = getattr(task, "parent_task_id", None)
|
||||||
if parent_id is not None:
|
if parent_id is not None:
|
||||||
@@ -91,6 +95,10 @@ async def resolve_parent_branch(task: Any, task_service: Any) -> str:
|
|||||||
default_branch = await _project_default_branch(task, task_service)
|
default_branch = await _project_default_branch(task, task_service)
|
||||||
if default_branch is not None:
|
if default_branch is not None:
|
||||||
return default_branch
|
return default_branch
|
||||||
|
else:
|
||||||
|
default_branch = await _project_default_branch(task, task_service)
|
||||||
|
if default_branch is not None:
|
||||||
|
return default_branch
|
||||||
return parent_branch_for(task.branch_name)
|
return parent_branch_for(task.branch_name)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -72,10 +72,23 @@ class TestResolveParentBranch:
|
|||||||
task_service.get.assert_awaited_once_with(root_id)
|
task_service.get.assert_awaited_once_with(root_id)
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_falls_back_when_no_parent(self) -> None:
|
async def test_parentless_root_uses_project_head_rung(self) -> None:
|
||||||
|
# A parentless root's branch was cut from the project's head rung
|
||||||
|
# (panel-configured env ladder), so that rung is the PR base / merge
|
||||||
|
# target — never a literal "master", which on a main-default repo
|
||||||
|
# silently targets a branch the project doesn't use.
|
||||||
task = MagicMock(parent_task_id=None, branch_name="feature/backend/ROOT0001")
|
task = MagicMock(parent_task_id=None, branch_name="feature/backend/ROOT0001")
|
||||||
task_service = AsyncMock()
|
task_service = AsyncMock()
|
||||||
# No parent → root→master.
|
task_service.project_default_branch_for_task = AsyncMock(return_value="main")
|
||||||
|
assert await resolve_parent_branch(task, task_service) == "main"
|
||||||
|
task_service.get.assert_not_called()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_parentless_root_falls_back_to_string_when_no_project(self) -> None:
|
||||||
|
task = MagicMock(parent_task_id=None, branch_name="feature/backend/ROOT0001")
|
||||||
|
task_service = AsyncMock()
|
||||||
|
task_service.project_default_branch_for_task = AsyncMock(return_value=None)
|
||||||
|
# No project to consult → string derivation's master fallback.
|
||||||
assert await resolve_parent_branch(task, task_service) == "master"
|
assert await resolve_parent_branch(task, task_service) == "master"
|
||||||
task_service.get.assert_not_called()
|
task_service.get.assert_not_called()
|
||||||
|
|
||||||
|
|||||||
@@ -363,6 +363,7 @@ async def test_runner_forwards_actor_agent_id_to_create_root_pr() -> None:
|
|||||||
task_svc.submit_for_review = AsyncMock(
|
task_svc.submit_for_review = AsyncMock(
|
||||||
return_value=MagicMock(status="awaiting_pr_review")
|
return_value=MagicMock(status="awaiting_pr_review")
|
||||||
)
|
)
|
||||||
|
task_svc.project_default_branch_for_task = AsyncMock(return_value="main")
|
||||||
git_svc = AsyncMock()
|
git_svc = AsyncMock()
|
||||||
git_svc.create_pr = AsyncMock(return_value={"pr_number": 7})
|
git_svc.create_pr = AsyncMock(return_value={"pr_number": 7})
|
||||||
runner = VerbRunner(task_service=task_svc, git_service=git_svc)
|
runner = VerbRunner(task_service=task_svc, git_service=git_svc)
|
||||||
@@ -381,6 +382,9 @@ async def test_runner_forwards_actor_agent_id_to_create_root_pr() -> None:
|
|||||||
|
|
||||||
assert git_svc.create_pr.call_args.kwargs.get("is_root_pr") is True
|
assert git_svc.create_pr.call_args.kwargs.get("is_root_pr") is True
|
||||||
assert git_svc.create_pr.call_args.kwargs.get("actor_agent_id") == agent.id
|
assert git_svc.create_pr.call_args.kwargs.get("actor_agent_id") == agent.id
|
||||||
|
assert git_svc.create_pr.call_args.kwargs.get("parent") == "main", (
|
||||||
|
"root PR base must be the project's head rung, not a literal master"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user