mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(prompter): batch confirm ignores a vestigial top-level project slug (#380)
* fix(prompter): batch confirm ignores a vestigial top-level project slug The MegaTask confirm-batch 400'd with "Invalid project_id UUID: roboco-api". The intake agent authors each draft's project as the repo slug it read, and the panel only nulls the top-level project_id when the CEO toggles that draft's picker — so an untouched draft carried the slug through to create_task_from_draft, whose eager _resolve_uuid_field(project_id) raised on the non-UUID and rejected the whole batch (guard-release from the prior fix surfaced it as a clean 400 instead of a wedged 500). A batch root-subtask targets its repos via the the_work per-cell map (the panel fills those with real project UUIDs); the top-level project_id/product_id is vestigial for it. Strip it from a sub-draft when its cell map carries the real target — a legacy no-the_work draft keeps its panel-filled top-level UUID, and scope validation (which already runs off the cell map) is unchanged. Adds a repro test with two cell-map drafts that also carry leftover top-level slugs (roboco-api / roboco-panel); they now confirm instead of 400-ing. * fix(tests): conventions PR integration test honors the #375 workspace-scope guard #375 added a containment guard to open_conventions_pr (workspace_path must sit under {workspaces_root}/{slug}); the unit test was updated but this integration test still seeded a bare tmp_path/repo, so open_conventions_pr returned None and test_open_conventions_pr_commits_locally_without_remote failed on master. Anchor workspaces_root at the test dir and place the repo under the project's slug. * refactor(prompter): extract batch sub-draft sanitize (xenon rank B) The inline vestigial-target strip pushed _build_confirm_batch to cyclomatic rank C (over the --max-absolute B gate). Move the assigned_to + top-level project/product stripping into a pure _batch_subtask_draft helper; behavior is unchanged, _build_confirm_batch drops back under the limit. --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -827,12 +827,7 @@ class PrompterService:
|
||||
# sequence = its wave index and the umbrella as parent.
|
||||
task_of: dict[int, UUID] = {}
|
||||
for idx, draft in enumerate(drafts):
|
||||
# Root-subtasks are coordination roots: assignment is the
|
||||
# PM-activation flow's call, not a draft-carried field. Strip a
|
||||
# hallucinated/injected assigned_to so a board-role uuid can't
|
||||
# deadlock the umbrella (board roles have no dev delivery verbs).
|
||||
sub_draft = dict(draft)
|
||||
sub_draft.pop("assigned_to", None)
|
||||
sub_draft = _batch_subtask_draft(draft)
|
||||
sub = await self.create_task_from_draft(
|
||||
sub_draft,
|
||||
agent_id,
|
||||
@@ -1087,6 +1082,30 @@ def _cell_teams(the_work: list[Any]) -> list[str]:
|
||||
return seen
|
||||
|
||||
|
||||
def _batch_subtask_draft(draft: dict[str, Any]) -> dict[str, Any]:
|
||||
"""A batch root-subtask's draft: a copy with the non-targeting fields dropped.
|
||||
|
||||
``assigned_to`` is stripped — assignment is the PM-activation flow's call, not
|
||||
a draft-carried field, and a hallucinated/injected board-role uuid would
|
||||
deadlock the umbrella (board roles have no dev delivery verbs). The top-level
|
||||
``project_id``/``product_id`` is stripped when the ``the_work`` per-cell map
|
||||
carries the real target: the intake agent authors the top-level as the repo
|
||||
SLUG it read (e.g. ``"roboco-api"``), which the panel only clears on a manual
|
||||
picker toggle, so an untouched draft would reach ``create_task_from_draft``'s
|
||||
UUID parse and 400 the whole batch. A legacy no-``the_work`` draft keeps its
|
||||
panel-filled top-level target.
|
||||
"""
|
||||
out = dict(draft)
|
||||
out.pop("assigned_to", None)
|
||||
if any(
|
||||
isinstance(cell, dict) and cell.get("project_id")
|
||||
for cell in (out.get("the_work") or [])
|
||||
):
|
||||
out.pop("project_id", None)
|
||||
out.pop("product_id", None)
|
||||
return out
|
||||
|
||||
|
||||
def _draft_cell_map(draft: dict[str, Any]) -> list[tuple[Team, UUID]]:
|
||||
"""The draft's ad-hoc per-cell project map.
|
||||
|
||||
|
||||
@@ -73,6 +73,59 @@ async def test_confirm_live_batch_panel_the_work_shape(db_session: Any) -> None:
|
||||
assert len(ids) == len(drafts)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_confirm_live_batch_ignores_vestigial_top_level_slug(
|
||||
db_session: Any,
|
||||
) -> None:
|
||||
"""A batch draft whose the_work cell-map carries the real project UUID but
|
||||
that also still has the intake agent's top-level project_id SLUG (the panel
|
||||
only nulls it on a manual picker toggle) must NOT 400 the whole batch on the
|
||||
UUID parse — the cell map is authoritative. Repro for the live 400
|
||||
"Invalid project_id UUID: roboco-api"."""
|
||||
project1, ceo_id = await _seed_project_and_ceo(db_session)
|
||||
project2 = await _seed_second_project(db_session, ceo_id)
|
||||
service = get_prompter_service(db=db_session)
|
||||
|
||||
drafts: list[dict[str, Any]] = [
|
||||
{
|
||||
"title": "A: backend work",
|
||||
"acceptance_criteria": ["a"],
|
||||
"project_id": "roboco-api", # agent's slug, panel left it in place
|
||||
"the_work": [
|
||||
{
|
||||
"team": "backend",
|
||||
"summary": "s",
|
||||
"items": ["u"],
|
||||
"project_id": str(project1),
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
"title": "B: frontend work",
|
||||
"acceptance_criteria": ["b"],
|
||||
"project_id": "roboco-panel", # slug on the other draft too
|
||||
"the_work": [
|
||||
{
|
||||
"team": "frontend",
|
||||
"summary": "s",
|
||||
"items": ["u"],
|
||||
"project_id": str(project2),
|
||||
},
|
||||
],
|
||||
},
|
||||
]
|
||||
with patch("roboco.services.prompter.redis.from_url", return_value=_FakeRedis()):
|
||||
result = await service.confirm_live_batch(
|
||||
"Slug batch",
|
||||
drafts,
|
||||
ceo_id,
|
||||
project_ids=[project1, project2],
|
||||
route="main_pm",
|
||||
session_id=f"sess-slug-{uuid4().hex[:8]}",
|
||||
)
|
||||
assert len(result["root_subtask_ids"]) == len(drafts)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_confirm_live_batch_panel_multicell_root_subtask(db_session: Any) -> None:
|
||||
"""A batch draft that targets TWO cells (backend+frontend, one repo each) →
|
||||
|
||||
Reference in New Issue
Block a user