Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions codeframe/core/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,7 @@ class ApprovalResult:
def approve_tasks(
workspace: Workspace,
excluded_task_ids: Optional[list[str]] = None,
included_task_ids: Optional[list[str]] = None,
) -> ApprovalResult:
"""Approve tasks for execution by transitioning them to READY status.

Expand All @@ -1238,21 +1239,52 @@ def approve_tasks(

Args:
workspace: Target workspace
excluded_task_ids: Optional list of task IDs to exclude from approval
excluded_task_ids: Task IDs to exclude; everything else in BACKLOG is
approved. This is the original, exclusion-shaped contract.
included_task_ids: Approve exactly these and nothing else (#1146). The
intuitive shape, and mutually exclusive with the one above — a call
passing both is ambiguous, so it raises rather than picking one.
An id here that is not in BACKLOG raises too: silently approving
fewer tasks than asked for is the failure this parameter exists to
prevent.

Returns:
ApprovalResult with counts and IDs

Raises:
ValueError: both lists given, or an included id is not an approvable
BACKLOG task

Example:
result = approve_tasks(workspace, excluded_task_ids=["task-1", "task-2"])
print(f"Approved {result.approved_count} tasks")
if result.approved_count > 0:
batch = start_approved_batch(workspace, result.approved_task_ids)
"""
excluded = set(excluded_task_ids or [])
if excluded_task_ids and included_task_ids:
raise ValueError(
"Pass excluded_task_ids or included_task_ids, not both — "
"they mean opposite things and there is no safe way to combine them"
)

# Get all BACKLOG tasks
backlog_tasks = tasks.list_tasks(workspace, status=TaskStatus.BACKLOG)
# Get all BACKLOG tasks. limit=None is load-bearing: list_tasks defaults to
# 100 (#743), so "approve everything" silently approved the first 100 of a
# larger backlog, and the inclusion path would 422 a perfectly valid id that
# happened to sort past the cap. Caught in review on #1146; the exclusion
# path had the same defect all along.
backlog_tasks = tasks.list_tasks(workspace, status=TaskStatus.BACKLOG, limit=None)

if included_task_ids is not None:
wanted = set(included_task_ids)
approvable = {t.id for t in backlog_tasks}
unknown = sorted(wanted - approvable)
if unknown:
raise ValueError(
"not approvable (unknown, or not in BACKLOG): " + ", ".join(unknown)
)
excluded = approvable - wanted
else:
excluded = set(excluded_task_ids or [])

approved_ids = []
excluded_ids = []
Expand Down
45 changes: 38 additions & 7 deletions codeframe/ui/routers/tasks_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from fastapi import APIRouter, Depends, HTTPException, Query, Request
from fastapi.concurrency import run_in_threadpool
from fastapi.responses import StreamingResponse
from pydantic import BaseModel, Field, model_validator
from pydantic import BaseModel, ConfigDict, Field, model_validator

from codeframe.core.workspace import Workspace
from codeframe.lib.rate_limiter import rate_limit_ai, rate_limit_standard
Expand All @@ -43,11 +43,27 @@


class ApproveTasksRequest(BaseModel):
"""Request for task approval."""
"""Request for task approval.

Accepts either shape (#1146). It used to accept only ``excluded_task_ids``,
and Pydantic's default is to DROP unknown fields — so the intuitive
``{"task_ids": [...]}`` returned 200 having approved the entire backlog,
the exact inverse of the caller's intent, in silence. ``extra="forbid"``
closes that even for a field neither name covers.
"""

model_config = ConfigDict(extra="forbid")

excluded_task_ids: list[str] = Field(
default_factory=list,
description="Task IDs to exclude from approval",
description="Task IDs to exclude; every other BACKLOG task is approved",
)
task_ids: Optional[list[str]] = Field(
default=None,
description=(
"Approve exactly these tasks and nothing else. Mutually exclusive "
"with excluded_task_ids."
),
)
start_execution: bool = Field(
default=False,
Expand All @@ -65,6 +81,15 @@ def _validate_engine(self) -> "ApproveTasksRequest":
raise ValueError(f"engine must be one of: {', '.join(valid)}")
return self

@model_validator(mode="after")
def _validate_selection(self) -> "ApproveTasksRequest":
if self.task_ids is not None and self.excluded_task_ids:
raise ValueError(
"Pass task_ids or excluded_task_ids, not both — they mean "
"opposite things"
)
return self


class ApproveTasksResponse(BaseModel):
"""Response for task approval."""
Expand Down Expand Up @@ -666,10 +691,16 @@ async def approve_tasks_endpoint(
"""
try:
# Approve tasks (transition BACKLOG → READY)
result = runtime.approve_tasks(
workspace,
excluded_task_ids=body.excluded_task_ids,
)
try:
result = runtime.approve_tasks(
workspace,
excluded_task_ids=body.excluded_task_ids,
included_task_ids=body.task_ids,
)
except ValueError as e:
# A named task that is not approvable. 422, not 500: the request is
# well-formed and the server is fine — the ids are wrong.
raise HTTPException(status_code=422, detail=api_error(str(e), "INVALID_TASK_IDS"))

batch_id = None
message = f"Approved {result.approved_count} task(s)."
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/test_batch_execution_offload.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ async def test_approve_with_execution_also_returns_immediately(
monkeypatch.setattr(
tasks_v2.runtime,
"approve_tasks",
lambda ws, excluded_task_ids=None: type(
lambda ws, excluded_task_ids=None, included_task_ids=None: type(
"R",
(),
{
Expand Down Expand Up @@ -267,7 +267,7 @@ async def test_approve_is_guarded_too(self, app, slow_batch, monkeypatch):
monkeypatch.setattr(
tasks_v2.runtime,
"approve_tasks",
lambda ws, excluded_task_ids=None: type(
lambda ws, excluded_task_ids=None, included_task_ids=None: type(
"R",
(),
{
Expand Down
Loading