mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(api): require substantive audit notes on human task-decision endpoints
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.
This commit is contained in:
@@ -1084,11 +1084,19 @@ async def docs_complete(
|
|||||||
|
|
||||||
Transitions task from awaiting_documentation to awaiting_pm_review.
|
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)
|
service = get_task_service(db)
|
||||||
try:
|
try:
|
||||||
task = await service.docs_complete_for_task(
|
task = await service.docs_complete_for_task(task_id, agent, notes=data.notes)
|
||||||
task_id, agent, notes=(data.notes if data else None)
|
|
||||||
)
|
|
||||||
except ServiceError as e:
|
except ServiceError as e:
|
||||||
raise _translate_error(e) from e
|
raise _translate_error(e) from e
|
||||||
return task_to_response(task)
|
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",
|
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)
|
task = await service.submit_for_pm_review(task_id, agent.role.value, notes)
|
||||||
if not task:
|
if not task:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@@ -1159,6 +1177,16 @@ async def complete_task(
|
|||||||
If force_with_cancelled=True, PM can complete despite cancelled subtasks.
|
If force_with_cancelled=True, PM can complete despite cancelled subtasks.
|
||||||
Requires justification. Does NOT apply to pending/in_progress 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)
|
service = get_task_service(db)
|
||||||
try:
|
try:
|
||||||
task = await service.complete_task_for_agent(
|
task = await service.complete_task_for_agent(
|
||||||
@@ -1166,7 +1194,7 @@ async def complete_task(
|
|||||||
agent,
|
agent,
|
||||||
permissions,
|
permissions,
|
||||||
force_with_cancelled=(data.force_with_cancelled if data else False),
|
force_with_cancelled=(data.force_with_cancelled if data else False),
|
||||||
justification=(data.justification if data else None),
|
justification=justification,
|
||||||
)
|
)
|
||||||
except ServiceError as e:
|
except ServiceError as e:
|
||||||
raise _translate_error(e) from e
|
raise _translate_error(e) from e
|
||||||
|
|||||||
@@ -2209,6 +2209,7 @@ async def test_submit_pm_review_service_returns_none(task_client: dict) -> None:
|
|||||||
mock_factory.return_value = instance
|
mock_factory.return_value = instance
|
||||||
response = await task_client["client"].post(
|
response = await task_client["client"].post(
|
||||||
f"/api/tasks/{task.id}/submit-pm-review",
|
f"/api/tasks/{task.id}/submit-pm-review",
|
||||||
|
json={"notes": "Ready for PM review — all criteria met."},
|
||||||
headers=_HDR,
|
headers=_HDR,
|
||||||
)
|
)
|
||||||
assert response.status_code == HTTPStatus.BAD_REQUEST
|
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
|
mock_factory.return_value = instance
|
||||||
response = await task_client["client"].post(
|
response = await task_client["client"].post(
|
||||||
f"/api/tasks/{task.id}/complete",
|
f"/api/tasks/{task.id}/complete",
|
||||||
json={"force_with_cancelled": False},
|
json={
|
||||||
|
"force_with_cancelled": False,
|
||||||
|
"justification": "All acceptance criteria met; merging.",
|
||||||
|
},
|
||||||
headers=_HDR,
|
headers=_HDR,
|
||||||
)
|
)
|
||||||
assert response.status_code == HTTPStatus.OK
|
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)
|
# cancel: service returns None branch (1162-1167)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user