feat(boards): add 'comment_required_for_review' rule and update related logic
This commit is contained in:
@@ -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 = (
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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")
|
||||
@@ -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 }}`
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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() {
|
||||
</span>
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-start gap-3 rounded-lg border border-slate-200 px-3 py-3">
|
||||
<button
|
||||
type="button"
|
||||
role="switch"
|
||||
aria-checked={resolvedCommentRequiredForReview}
|
||||
aria-label="Require comment for review"
|
||||
onClick={() =>
|
||||
setCommentRequiredForReview(!resolvedCommentRequiredForReview)
|
||||
}
|
||||
disabled={isLoading}
|
||||
className={`mt-0.5 inline-flex h-6 w-11 shrink-0 items-center rounded-full border transition ${
|
||||
resolvedCommentRequiredForReview
|
||||
? "border-emerald-600 bg-emerald-600"
|
||||
: "border-slate-300 bg-slate-200"
|
||||
} ${isLoading ? "cursor-not-allowed opacity-60" : "cursor-pointer"}`}
|
||||
>
|
||||
<span
|
||||
className={`inline-block h-5 w-5 rounded-full bg-white shadow-sm transition ${
|
||||
resolvedCommentRequiredForReview
|
||||
? "translate-x-5"
|
||||
: "translate-x-0.5"
|
||||
}`}
|
||||
/>
|
||||
</button>
|
||||
<span className="space-y-1">
|
||||
<span className="block text-sm font-medium text-slate-900">
|
||||
Require comment for review
|
||||
</span>
|
||||
<span className="block text-xs text-slate-600">
|
||||
Require a task comment when moving status to{" "}
|
||||
<code>review</code>.
|
||||
</span>
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-start gap-3 rounded-lg border border-slate-200 px-3 py-3">
|
||||
<button
|
||||
type="button"
|
||||
|
||||
Reference in New Issue
Block a user