From 54d320a61f8f9a0032d4d49f80248350fad3a39c Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Thu, 9 Jul 2026 04:07:04 +0000 Subject: fix(api): reject accepting a BLOCKED task directly The nested-subtask-completion fix (ad00e18) necessarily added BLOCKED -> COMPLETED to task.ValidTransition so executor.Pool.maybeUnblockParent can complete a subtask whose own children all finished. That widened acceptTask's gate (shared by POST /api/tasks/{id}/accept and the chatbot MCP accept_task tool, which validates only via task.ValidTransition) to also permit accepting ANY BLOCKED task directly -- one still awaiting ask_user, or with genuinely incomplete subtasks -- bypassing the completion invariant this session's fix was built to protect. Caught by local subagent review of that fix. Not live in production: the deployed claudomator service is still on an earlier commit. --- internal/api/taskops.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'internal/api/taskops.go') diff --git a/internal/api/taskops.go b/internal/api/taskops.go index 2b133ac..a2668f3 100644 --- a/internal/api/taskops.go +++ b/internal/api/taskops.go @@ -36,7 +36,9 @@ type badRequestError struct{ msg string } func (e *badRequestError) Error() string { return e.msg } -func badRequestf(format string, a ...any) error { return &badRequestError{msg: fmt.Sprintf(format, a...)} } +func badRequestf(format string, a ...any) error { + return &badRequestError{msg: fmt.Sprintf(format, a...)} +} // submitTaskSpec describes a task a chatbot wants created and immediately run. type submitTaskSpec struct { @@ -127,6 +129,16 @@ func (s *Server) acceptTask(ctx context.Context, id string, actor event.Actor) e if err != nil { return errTaskNotFound } + // BLOCKED -> COMPLETED is a valid task.ValidTransition (added so + // internal/executor.Pool's maybeUnblockParent can complete a subtask + // whose own children all finished), but that transition must only ever + // happen via that internal mechanism -- never via a direct accept call, + // which would bypass the subtask-completion invariant (or silently + // foreclose an open ask_user question) for a task that isn't actually + // done yet. + if t.State == task.StateBlocked { + return conflictf("task cannot be accepted while BLOCKED (awaiting subtasks or a question)") + } if !task.ValidTransition(t.State, task.StateCompleted) { return conflictf("task cannot be accepted from state %s", t.State) } -- cgit v1.2.3