From efd0a5e3cd32d0d42bb41c6c7accf8e366a9098f Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 27 Aug 2026 18:22:53 +0400 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_011xsXyr5J1RACo71YeKG3Pu --- internal/ui/ui.go | 13 ++++++++++ internal/ui/ui_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/internal/ui/ui.go b/internal/ui/ui.go index d5eb55f..f32b5b1 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -422,11 +422,24 @@ func (s Server) action(w http.ResponseWriter, r *http.Request, id, action string http.Error(w, "unknown action", 404) return } + if body == nil { + body = map[string]any{} + } if action == "block" { // A browser-created block is an explicit operator decision. Preserve // that fact even if its prose happens to contain a system keyword. body["block_reason"] = string(domain.BlockReasonOperator) } + // The store fences release/block/complete on a leased task against the + // live lease (validateTransition, the "TaskBlocked" case). That fence + // exists to stop a *stale* writer, not the operator, and the browser never + // had a way to satisfy it: all three actions returned 409 on exactly the + // tasks the UI offered them for. Carry the lease we just read. The version + // CAS on the append still rejects a racing write. + if t.Lease != nil { + body["harness_id"] = t.Lease.HarnessID + body["lease_epoch"] = t.Lease.Epoch + } b, _ := json.Marshal(body) e := domain.Event{ID: domain.NewID(), Type: typ, TaskID: id, Version: t.Version + 1, Payload: b, Surface: string(authz.Web)} if err := s.Store.Append(e); err != nil { diff --git a/internal/ui/ui_test.go b/internal/ui/ui_test.go index 7106413..2fe4f7c 100644 --- a/internal/ui/ui_test.go +++ b/internal/ui/ui_test.go @@ -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()) + } +}