Let the operator act on a leased task from the browser

The store fences TaskReleased, TaskBlocked and TaskCompleted on a leased task against the live harness_id and lease_epoch. The UI action handler sent neither, so all three returned 409 on exactly the tasks the UI listed them as enabled for. Found live: eight block attempts against a stuck run at a stable version, all 409 "task version conflict". The fence is there to reject a stale writer, not the operator. Carry the lease read at the top of the handler. The version CAS on the append still rejects a racing write. Also initialise body when the request carries none. The block path wrote block_reason into a nil map. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xsXyr5J1RACo71YeKG3Pu
2026-08-27 18:22:53 +04:00
parent dea56e4bcd
commit efd0a5e3cd
2 changed files with 70 additions and 0 deletions
@@ -130,3 +130,60 @@ func TestCoordinatorCaptureRevisionTracksContentNotTime(t *testing.T) {
 		t.Fatal("revision 0 means \"no revision\" to federation.Queue")
 	}
 }
+
+// TestOperatorBlockReachesALeasedTask guards the fence the browser could never
+// satisfy: the store rejects TaskBlocked on a leased task unless the payload
+// carries the live harness_id and lease_epoch, and the UI action handler sent
+// neither. Every block/release/complete the UI advertised on a leased task
+// returned 409, which left an operator with no lifecycle action at all on the
+// tasks that most needed one.
+func TestOperatorBlockReachesALeasedTask(t *testing.T) {
+	s, err := store.Open(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	h := Server{Store: s}.Handler()
+	req := httptest.NewRequest(http.MethodPost, "/v1/ui/tasks", bytes.NewBufferString(`{"source":"web","external_id":"blk","project":"demo","title":"Leased"}`))
+	r := httptest.NewRecorder()
+	h.ServeHTTP(r, req)
+	if r.Code != http.StatusOK {
+		t.Fatalf("create status=%d body=%s", r.Code, r.Body.String())
+	}
+	id := s.Tasks()[0].ID
+	if _, err := s.Lease(id, "wpc-claude", time.Hour); err != nil {
+		t.Fatal(err)
+	}
+	req = httptest.NewRequest(http.MethodPost, "/v1/ui/tasks/"+id+"/actions/block", bytes.NewBufferString(`{"blocker":"parked by the operator"}`))
+	r = httptest.NewRecorder()
+	h.ServeHTTP(r, req)
+	if r.Code != http.StatusOK {
+		t.Fatalf("block status=%d body=%s", r.Code, r.Body.String())
+	}
+	task, ok := s.Task(id)
+	if !ok || task.State != domain.StateBlocked {
+		t.Fatalf("state=%q ok=%v, want blocked", task.State, ok)
+	}
+	if task.BlockReason != domain.BlockReasonOperator {
+		t.Fatalf("block_reason=%q, want %q", task.BlockReason, domain.BlockReasonOperator)
+	}
+}
+
+// A missing request body must not panic the action handler. Decode leaves the
+// map nil, and the block path writes into it.
+func TestActionWithoutABodyDoesNotPanic(t *testing.T) {
+	s, err := store.Open(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	h := Server{Store: s}.Handler()
+	req := httptest.NewRequest(http.MethodPost, "/v1/ui/tasks", bytes.NewBufferString(`{"source":"web","external_id":"nobody","project":"demo","title":"No body"}`))
+	r := httptest.NewRecorder()
+	h.ServeHTTP(r, req)
+	id := s.Tasks()[0].ID
+	req = httptest.NewRequest(http.MethodPost, "/v1/ui/tasks/"+id+"/actions/block", nil)
+	r = httptest.NewRecorder()
+	h.ServeHTTP(r, req)
+	if r.Code != http.StatusConflict || !strings.Contains(r.Body.String(), "blocker required") {
+		t.Fatalf("block status=%d body=%s, want 409 blocker required", r.Code, r.Body.String())
+	}
+}