From 6b21ea6f9978648e136697dff50b6310549919fd Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Thu, 26 Feb 2026 00:31:47 +0530 Subject: [PATCH] feat(boards): add 'comment_required_for_review' rule and update related logic --- backend/app/api/tasks.py | 27 +++++- backend/app/models/boards.py | 1 + backend/app/schemas/boards.py | 2 + backend/app/services/openclaw/provisioning.py | 1 + ..._comment_required_for_review_board_rule.py | 43 ++++++++++ backend/templates/BOARD_HEARTBEAT.md.j2 | 1 + backend/templates/README.md | 1 + backend/tests/test_board_schema.py | 3 + backend/tests/test_task_agent_permissions.py | 85 ++++++++++++++++++- .../src/api/generated/model/boardCreate.ts | 1 + frontend/src/api/generated/model/boardRead.ts | 1 + .../src/api/generated/model/boardUpdate.ts | 1 + .../src/app/boards/[boardId]/edit/page.tsx | 41 +++++++++ 13 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 backend/migrations/versions/f1b2c3d4e5a6_add_comment_required_for_review_board_rule.py diff --git a/backend/app/api/tasks.py b/backend/app/api/tasks.py index d846b8f8..e5014c5f 100644 --- a/backend/app/api/tasks.py +++ b/backend/app/api/tasks.py @@ -259,6 +259,19 @@ async def _require_review_before_done_when_enabled( raise _review_required_for_done_error() +async def _require_comment_for_review_when_enabled( + session: AsyncSession, + *, + board_id: UUID, +) -> bool: + requires_comment = ( + await session.exec( + select(col(Board.comment_required_for_review)).where(col(Board.id) == board_id), + ) + ).first() + return bool(requires_comment) + + async def _require_no_pending_approval_for_status_change_when_enabled( session: AsyncSession, *, @@ -2088,6 +2101,9 @@ async def _lead_apply_status( *, update: _TaskUpdateInput, ) -> None: + if update.actor.actor_type != "agent" or update.actor.agent is None: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + lead_agent = update.actor.agent if "status" not in update.updates: return if update.task.status != "review": @@ -2112,7 +2128,7 @@ async def _lead_apply_status( session, task_id=update.task.id, board_id=update.board_id, - lead_agent_id=update.actor.agent.id, + lead_agent_id=lead_agent.id, ) update.task.in_progress_at = None update.task.status = target_status @@ -2581,9 +2597,12 @@ async def _finalize_updated_task( update.task.updated_at = utcnow() status_raw = update.updates.get("status") - # Entering review requires either a new comment or a valid recent one to - # ensure reviewers get context on readiness. - if status_raw == "review": + # Entering review can require a new comment or valid recent context when + # the board-level rule is enabled. + if status_raw == "review" and await _require_comment_for_review_when_enabled( + session, + board_id=update.board_id, + ): comment_text = (update.comment or "").strip() review_comment_author = update.task.assigned_agent_id or update.previous_assigned review_comment_since = ( diff --git a/backend/app/models/boards.py b/backend/app/models/boards.py index 5d26340b..2023c9c3 100644 --- a/backend/app/models/boards.py +++ b/backend/app/models/boards.py @@ -41,6 +41,7 @@ class Board(TenantScoped, table=True): goal_source: str | None = None require_approval_for_done: bool = Field(default=True) require_review_before_done: bool = Field(default=False) + comment_required_for_review: bool = Field(default=False) block_status_changes_with_pending_approval: bool = Field(default=False) only_lead_can_change_status: bool = Field(default=False) max_agents: int = Field(default=1) diff --git a/backend/app/schemas/boards.py b/backend/app/schemas/boards.py index c954a1d5..3d2cdef2 100644 --- a/backend/app/schemas/boards.py +++ b/backend/app/schemas/boards.py @@ -31,6 +31,7 @@ class BoardBase(SQLModel): goal_source: str | None = None require_approval_for_done: bool = True require_review_before_done: bool = False + comment_required_for_review: bool = False block_status_changes_with_pending_approval: bool = False only_lead_can_change_status: bool = False max_agents: int = Field(default=1, ge=0) @@ -75,6 +76,7 @@ class BoardUpdate(SQLModel): goal_source: str | None = None require_approval_for_done: bool | None = None require_review_before_done: bool | None = None + comment_required_for_review: bool | None = None block_status_changes_with_pending_approval: bool | None = None only_lead_can_change_status: bool | None = None max_agents: int | None = Field(default=None, ge=0) diff --git a/backend/app/services/openclaw/provisioning.py b/backend/app/services/openclaw/provisioning.py index f80a72a4..a1665c63 100644 --- a/backend/app/services/openclaw/provisioning.py +++ b/backend/app/services/openclaw/provisioning.py @@ -363,6 +363,7 @@ def _build_context( "board_goal_confirmed": str(board.goal_confirmed).lower(), "board_rule_require_approval_for_done": str(board.require_approval_for_done).lower(), "board_rule_require_review_before_done": str(board.require_review_before_done).lower(), + "board_rule_comment_required_for_review": str(board.comment_required_for_review).lower(), "board_rule_block_status_changes_with_pending_approval": str( board.block_status_changes_with_pending_approval ).lower(), diff --git a/backend/migrations/versions/f1b2c3d4e5a6_add_comment_required_for_review_board_rule.py b/backend/migrations/versions/f1b2c3d4e5a6_add_comment_required_for_review_board_rule.py new file mode 100644 index 00000000..ff530d45 --- /dev/null +++ b/backend/migrations/versions/f1b2c3d4e5a6_add_comment_required_for_review_board_rule.py @@ -0,0 +1,43 @@ +"""add comment-required-for-review board rule + +Revision ID: f1b2c3d4e5a6 +Revises: e3a1b2c4d5f6 +Create Date: 2026-02-25 00:00:00.000000 + +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "f1b2c3d4e5a6" +down_revision = "e3a1b2c4d5f6" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + board_columns = {column["name"] for column in inspector.get_columns("boards")} + if "comment_required_for_review" not in board_columns: + op.add_column( + "boards", + sa.Column( + "comment_required_for_review", + sa.Boolean(), + nullable=False, + server_default=sa.false(), + ), + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + board_columns = {column["name"] for column in inspector.get_columns("boards")} + if "comment_required_for_review" in board_columns: + op.drop_column("boards", "comment_required_for_review") diff --git a/backend/templates/BOARD_HEARTBEAT.md.j2 b/backend/templates/BOARD_HEARTBEAT.md.j2 index 2c6ab234..5e9ef6bd 100644 --- a/backend/templates/BOARD_HEARTBEAT.md.j2 +++ b/backend/templates/BOARD_HEARTBEAT.md.j2 @@ -162,6 +162,7 @@ Before execution: ### Board Rule Snapshot - `require_review_before_done`: `{{ board_rule_require_review_before_done }}` - `require_approval_for_done`: `{{ board_rule_require_approval_for_done }}` +- `comment_required_for_review`: `{{ board_rule_comment_required_for_review }}` - `block_status_changes_with_pending_approval`: `{{ board_rule_block_status_changes_with_pending_approval }}` - `only_lead_can_change_status`: `{{ board_rule_only_lead_can_change_status }}` - `max_agents`: `{{ board_rule_max_agents }}` diff --git a/backend/templates/README.md b/backend/templates/README.md index dff27cf1..819f36eb 100644 --- a/backend/templates/README.md +++ b/backend/templates/README.md @@ -133,6 +133,7 @@ This avoids relying on startup hooks to populate `api/openapi.json`. - `workspace_path` - `board_rule_require_approval_for_done` - `board_rule_require_review_before_done` +- `board_rule_comment_required_for_review` - `board_rule_block_status_changes_with_pending_approval` - `board_rule_only_lead_can_change_status` - `board_rule_max_agents` diff --git a/backend/tests/test_board_schema.py b/backend/tests/test_board_schema.py index f0aa4516..a66a07f5 100644 --- a/backend/tests/test_board_schema.py +++ b/backend/tests/test_board_schema.py @@ -86,6 +86,7 @@ def test_board_rule_toggles_have_expected_defaults() -> None: ) assert created.require_approval_for_done is True assert created.require_review_before_done is False + assert created.comment_required_for_review is False assert created.block_status_changes_with_pending_approval is False assert created.only_lead_can_change_status is False assert created.max_agents == 1 @@ -93,12 +94,14 @@ def test_board_rule_toggles_have_expected_defaults() -> None: updated = BoardUpdate( require_approval_for_done=False, require_review_before_done=True, + comment_required_for_review=True, block_status_changes_with_pending_approval=True, only_lead_can_change_status=True, max_agents=3, ) assert updated.require_approval_for_done is False assert updated.require_review_before_done is True + assert updated.comment_required_for_review is True assert updated.block_status_changes_with_pending_approval is True assert updated.only_lead_can_change_status is True assert updated.max_agents == 3 diff --git a/backend/tests/test_task_agent_permissions.py b/backend/tests/test_task_agent_permissions.py index 60811f5f..44e3e7a8 100644 --- a/backend/tests/test_task_agent_permissions.py +++ b/backend/tests/test_task_agent_permissions.py @@ -756,7 +756,89 @@ async def test_non_lead_agent_comment_in_review_without_status_does_not_reassign @pytest.mark.asyncio -async def test_non_lead_agent_moves_to_review_without_comment_or_recent_comment_fails() -> None: +async def test_non_lead_agent_moves_to_review_without_comment_when_rule_disabled() -> None: + engine = await _make_engine() + try: + async with await _make_session(engine) as session: + org_id = uuid4() + board_id = uuid4() + gateway_id = uuid4() + worker_id = uuid4() + lead_id = uuid4() + task_id = uuid4() + + session.add(Organization(id=org_id, name="org")) + session.add( + Gateway( + id=gateway_id, + organization_id=org_id, + name="gateway", + url="https://gateway.local", + workspace_root="/tmp/workspace", + ), + ) + session.add( + Board( + id=board_id, + organization_id=org_id, + name="board", + slug="board", + gateway_id=gateway_id, + comment_required_for_review=False, + ), + ) + session.add( + Agent( + id=worker_id, + name="worker", + board_id=board_id, + gateway_id=gateway_id, + status="online", + ), + ) + session.add( + Agent( + id=lead_id, + name="Lead Agent", + board_id=board_id, + gateway_id=gateway_id, + status="online", + is_board_lead=True, + ), + ) + session.add( + Task( + id=task_id, + board_id=board_id, + title="assigned task", + description="", + status="in_progress", + assigned_agent_id=worker_id, + in_progress_at=utcnow(), + ), + ) + await session.commit() + + task = (await session.exec(select(Task).where(col(Task.id) == task_id))).first() + assert task is not None + actor = (await session.exec(select(Agent).where(col(Agent.id) == worker_id))).first() + assert actor is not None + + updated = await tasks_api.update_task( + payload=TaskUpdate(status="review"), + task=task, + session=session, + actor=ActorContext(actor_type="agent", agent=actor), + ) + + assert updated.status == "review" + assert updated.assigned_agent_id == lead_id + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_non_lead_agent_moves_to_review_without_comment_or_recent_comment_fails_when_rule_enabled() -> None: engine = await _make_engine() try: async with await _make_session(engine) as session: @@ -783,6 +865,7 @@ async def test_non_lead_agent_moves_to_review_without_comment_or_recent_comment_ name="board", slug="board", gateway_id=gateway_id, + comment_required_for_review=True, ), ) session.add( diff --git a/frontend/src/api/generated/model/boardCreate.ts b/frontend/src/api/generated/model/boardCreate.ts index 44163dac..b518bd82 100644 --- a/frontend/src/api/generated/model/boardCreate.ts +++ b/frontend/src/api/generated/model/boardCreate.ts @@ -23,6 +23,7 @@ export interface BoardCreate { goal_source?: string | null; require_approval_for_done?: boolean; require_review_before_done?: boolean; + comment_required_for_review?: boolean; block_status_changes_with_pending_approval?: boolean; only_lead_can_change_status?: boolean; /** @minimum 0 */ diff --git a/frontend/src/api/generated/model/boardRead.ts b/frontend/src/api/generated/model/boardRead.ts index f7482496..15f49b9a 100644 --- a/frontend/src/api/generated/model/boardRead.ts +++ b/frontend/src/api/generated/model/boardRead.ts @@ -23,6 +23,7 @@ export interface BoardRead { goal_source?: string | null; require_approval_for_done?: boolean; require_review_before_done?: boolean; + comment_required_for_review?: boolean; block_status_changes_with_pending_approval?: boolean; only_lead_can_change_status?: boolean; /** @minimum 0 */ diff --git a/frontend/src/api/generated/model/boardUpdate.ts b/frontend/src/api/generated/model/boardUpdate.ts index 19c57fed..3a1b359f 100644 --- a/frontend/src/api/generated/model/boardUpdate.ts +++ b/frontend/src/api/generated/model/boardUpdate.ts @@ -23,6 +23,7 @@ export interface BoardUpdate { goal_source?: string | null; require_approval_for_done?: boolean | null; require_review_before_done?: boolean | null; + comment_required_for_review?: boolean | null; block_status_changes_with_pending_approval?: boolean | null; only_lead_can_change_status?: boolean | null; max_agents?: number | null; diff --git a/frontend/src/app/boards/[boardId]/edit/page.tsx b/frontend/src/app/boards/[boardId]/edit/page.tsx index 966a0702..2248a681 100644 --- a/frontend/src/app/boards/[boardId]/edit/page.tsx +++ b/frontend/src/app/boards/[boardId]/edit/page.tsx @@ -291,6 +291,9 @@ export default function EditBoardPage() { const [requireReviewBeforeDone, setRequireReviewBeforeDone] = useState< boolean | undefined >(undefined); + const [commentRequiredForReview, setCommentRequiredForReview] = useState< + boolean | undefined + >(undefined); const [ blockStatusChangesWithPendingApproval, setBlockStatusChangesWithPendingApproval, @@ -504,6 +507,8 @@ export default function EditBoardPage() { requireApprovalForDone ?? baseBoard?.require_approval_for_done ?? true; const resolvedRequireReviewBeforeDone = requireReviewBeforeDone ?? baseBoard?.require_review_before_done ?? false; + const resolvedCommentRequiredForReview = + commentRequiredForReview ?? baseBoard?.comment_required_for_review ?? false; const resolvedBlockStatusChangesWithPendingApproval = blockStatusChangesWithPendingApproval ?? baseBoard?.block_status_changes_with_pending_approval ?? @@ -588,6 +593,7 @@ export default function EditBoardPage() { setObjective(updated.objective ?? ""); setRequireApprovalForDone(updated.require_approval_for_done ?? true); setRequireReviewBeforeDone(updated.require_review_before_done ?? false); + setCommentRequiredForReview(updated.comment_required_for_review ?? false); setBlockStatusChangesWithPendingApproval( updated.block_status_changes_with_pending_approval ?? false, ); @@ -656,6 +662,7 @@ export default function EditBoardPage() { : resolvedObjective.trim() || null, require_approval_for_done: resolvedRequireApprovalForDone, require_review_before_done: resolvedRequireReviewBeforeDone, + comment_required_for_review: resolvedCommentRequiredForReview, block_status_changes_with_pending_approval: resolvedBlockStatusChangesWithPendingApproval, only_lead_can_change_status: resolvedOnlyLeadCanChangeStatus, @@ -1016,6 +1023,40 @@ export default function EditBoardPage() { +
+ + + + Require comment for review + + + Require a task comment when moving status to{" "} + review. + + +