From 5120b5ce81746ba162c56093348000f07cf6a45b Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 24 May 2026 07:05:33 +0200 Subject: [PATCH] fix(api): require substantive audit notes on human task-decision endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit/tracing rule: every human decision must record its rationale. These panel-facing routes accepted empty/absent notes, leaving no trail: - docs-complete: now requires notes (>=20) — what was documented - submit-pm-review: now requires notes (>=20) — what is ready for review - complete: now requires justification (>=20) — why the task is done Mirrors the existing pass-qa / ceo-approve notes gates (checked after the 404/403 so not-found and forbidden still take precedence). submit-qa is left as-is: it already gates on commits + PR + progress_updates + self_verified, and the panel collects no extra note there to drop. Tests updated to send notes; added complete-without-justification reject. --- roboco/api/routes/tasks.py | 38 ++++++++++++++++++++++---- tests/integration/test_tasks_routes.py | 20 +++++++++++++- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/roboco/api/routes/tasks.py b/roboco/api/routes/tasks.py index 24794aee..78c073ca 100644 --- a/roboco/api/routes/tasks.py +++ b/roboco/api/routes/tasks.py @@ -1084,11 +1084,19 @@ async def docs_complete( Transitions task from awaiting_documentation to awaiting_pm_review. """ + # Audit: the documenter must record what was documented, so the next + # reader knows what exists. No note → empty trail, so reject. + if not data or not data.notes or len(data.notes.strip()) < _MIN_NOTES_CHARS: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=( + "DOC_NOTES_REQUIRED: docs-complete must include notes (>=20 " + "chars) describing what was documented and where." + ), + ) service = get_task_service(db) try: - task = await service.docs_complete_for_task( - task_id, agent, notes=(data.notes if data else None) - ) + task = await service.docs_complete_for_task(task_id, agent, notes=data.notes) except ServiceError as e: raise _translate_error(e) from e return task_to_response(task) @@ -1122,7 +1130,17 @@ async def submit_for_pm_review( detail="Only the assigned agent can submit for PM review", ) - notes = data.notes if data else None + # Audit: the submitter must record what is ready for review. + if not data or not data.notes or len(data.notes.strip()) < _MIN_NOTES_CHARS: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=( + "PM_REVIEW_NOTES_REQUIRED: submit-pm-review must include notes " + "(>=20 chars) summarizing what is ready for the PM to review." + ), + ) + + notes = data.notes task = await service.submit_for_pm_review(task_id, agent.role.value, notes) if not task: raise HTTPException( @@ -1159,6 +1177,16 @@ async def complete_task( If force_with_cancelled=True, PM can complete despite cancelled subtasks. Requires justification. Does NOT apply to pending/in_progress subtasks. """ + # Audit: completing a task is a decision that must carry its rationale. + justification = data.justification if data else None + if not justification or len(justification.strip()) < _MIN_NOTES_CHARS: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=( + "COMPLETE_JUSTIFICATION_REQUIRED: complete must include a " + "justification (>=20 chars) recording why the task is done." + ), + ) service = get_task_service(db) try: task = await service.complete_task_for_agent( @@ -1166,7 +1194,7 @@ async def complete_task( agent, permissions, force_with_cancelled=(data.force_with_cancelled if data else False), - justification=(data.justification if data else None), + justification=justification, ) except ServiceError as e: raise _translate_error(e) from e diff --git a/tests/integration/test_tasks_routes.py b/tests/integration/test_tasks_routes.py index 881da802..23fc66c2 100644 --- a/tests/integration/test_tasks_routes.py +++ b/tests/integration/test_tasks_routes.py @@ -2209,6 +2209,7 @@ async def test_submit_pm_review_service_returns_none(task_client: dict) -> None: mock_factory.return_value = instance response = await task_client["client"].post( f"/api/tasks/{task.id}/submit-pm-review", + json={"notes": "Ready for PM review — all criteria met."}, headers=_HDR, ) assert response.status_code == HTTPStatus.BAD_REQUEST @@ -2230,12 +2231,29 @@ async def test_complete_task_success(task_client: dict) -> None: mock_factory.return_value = instance response = await task_client["client"].post( f"/api/tasks/{task.id}/complete", - json={"force_with_cancelled": False}, + json={ + "force_with_cancelled": False, + "justification": "All acceptance criteria met; merging.", + }, headers=_HDR, ) assert response.status_code == HTTPStatus.OK +@pytest.mark.asyncio +async def test_complete_without_justification_rejected(task_client: dict) -> None: + """Audit: completing a task must carry its rationale (>= 20 chars).""" + task = _seed_task(task_client) + await task_client["db"].flush() + response = await task_client["client"].post( + f"/api/tasks/{task.id}/complete", + json={"force_with_cancelled": False}, + headers=_HDR, + ) + assert response.status_code == HTTPStatus.BAD_REQUEST + assert "JUSTIFICATION_REQUIRED" in response.json()["detail"] + + # --------------------------------------------------------------------------- # cancel: service returns None branch (1162-1167) # ---------------------------------------------------------------------------