chat: a late task start does not claim failure

docs/changes/unreleased/1539-chat-late-task-start.md

@@ -0,0 +1,8 @@
+---
+kind: fixed
+title: a late task start is an uncertain receipt
+pr: 1539
+surface: [chat, remote]
+invalidates:
+  - "A task start that did not answer within the connection's wait was reported as a refusal. It is now an uncertain receipt; the task may already be running, stays visible through its task updates, and is not retried automatically."
+---

internal/manual/chat/tasks.md

@@ -1,5 +1,12 @@
 # Work that runs on its own — tasks, and finding out what one actually did
 
+## The engine is slow to answer when starting a task
+
+If the task start takes longer than the connection's wait, the chat says
+`the engine has not confirmed the start — the task may already be running`. This is an
+uncertain receipt, not a refusal. The task can appear in the task rail when the
+engine reports it. Check the running work before starting the same brief again.
+
 ## What a task is
 
 A task is one self-contained piece of work handed off to run on its own while the

internal/remote/client.go

@@ -1548,7 +1548,7 @@ func (a *Agent) Cancel(id string) (string, error) {
 // brief is written beside the work (internal/session's task_shape.go), so there
 // is nothing on the far side worth a longer wait.
 func (a *Agent) StartTask(ctx context.Context, brief string, solo bool) (uint64, string, string, error) {
-	payload, err := a.c.call(ctx, MethodTaskStart, TaskStartArgs{Brief: brief, Solo: solo})
+	payload, err := a.callTaskStart(ctx, MethodTaskStart, TaskStartArgs{Brief: brief, Solo: solo})
 	if err != nil {
 		return 0, "", "", err
 	}
@@ -1579,7 +1579,7 @@ func (a *Agent) Delegates() session.DelegateReport {
 // and returns the same receipt StartTask does. It is an ordinary call with the
 // ordinary deadline: the engine admits the run at once.
 func (a *Agent) StartDelegate(ctx context.Context, name, brief string) (uint64, string, string, error) {
-	payload, err := a.c.call(ctx, MethodDelegateStart, DelegateStartArgs{Name: name, Brief: brief})
+	payload, err := a.callTaskStart(ctx, MethodDelegateStart, DelegateStartArgs{Name: name, Brief: brief})
 	if err != nil {
 		return 0, "", "", err
 	}
@@ -1594,7 +1594,7 @@ func (a *Agent) StartDelegate(ctx context.Context, name, brief string) (uint64,
 // (`/task --best`, `/task --cheap`); the engine's router reads it for this task
 // and nothing after it.
 func (a *Agent) StartTaskEffort(ctx context.Context, brief string, solo bool, effort string) (uint64, string, string, error) {
-	payload, err := a.c.call(ctx, MethodTaskStart, TaskStartArgs{Brief: brief, Solo: solo, Effort: effort})
+	payload, err := a.callTaskStart(ctx, MethodTaskStart, TaskStartArgs{Brief: brief, Solo: solo, Effort: effort})
 	if err != nil {
 		return 0, "", "", err
 	}
@@ -1605,6 +1605,15 @@ func (a *Agent) StartTaskEffort(ctx context.Context, brief string, solo bool, ef
 	return started.ID, started.Title, started.Note, nil
 }
 
+// A start whose answer never arrived may already have created work on the engine.
+func (a *Agent) callTaskStart(ctx context.Context, method string, args any) (json.RawMessage, error) {
+	payload, answered, err := a.c.callAnswered(ctx, method, args, callDeadline)
+	if err != nil && !answered {
+		return nil, unanswered{said: err}
+	}
+	return payload, err
+}
+
 // RedoStronger runs a task again on the engine machine with a stronger crew
 // (`/redo stronger`); row 0 is the newest task the conversation started.
 func (a *Agent) RedoStronger(ctx context.Context, row uint64) (uint64, string, error) {

internal/remote/task_start_unanswered_test.go

@@ -0,0 +1,110 @@
+package remote
+
+import (
+	"context"
+	"errors"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/session"
+)
+
+type slowTaskStartAgent struct {
+	*railAgent
+	accepted chan struct{}
+	release  chan struct{}
+	starts   int
+}
+
+func (a *slowTaskStartAgent) StartTask(context.Context, string, bool) (uint64, string, string, error) {
+	a.mu.Lock()
+	a.starts++
+	a.mu.Unlock()
+	a.land(taskEvent(7, "accepted work", session.TaskRunning))
+	close(a.accepted)
+	<-a.release
+	return 7, "accepted work", "", nil
+}
+
+func (a *slowTaskStartAgent) StartTaskEffort(ctx context.Context, brief string, solo bool, _ string) (uint64, string, string, error) {
+	return a.StartTask(ctx, brief, solo)
+}
+
+func (a *slowTaskStartAgent) StartDelegate(ctx context.Context, _ string, brief string) (uint64, string, string, error) {
+	return a.StartTask(ctx, brief, true)
+}
+
+func TestALateTaskStartKeepsOneAcceptedTaskOnTheHostedRail(t *testing.T) {
+	for _, entry := range []struct {
+		name  string
+		start func(*Agent, context.Context) (uint64, string, string, error)
+	}{
+		{"ordinary", func(a *Agent, ctx context.Context) (uint64, string, string, error) {
+			return a.StartTask(ctx, "accepted work", true)
+		}},
+		{"effort", func(a *Agent, ctx context.Context) (uint64, string, string, error) {
+			return a.StartTaskEffort(ctx, "accepted work", true, "best")
+		}},
+		{"delegate", func(a *Agent, ctx context.Context) (uint64, string, string, error) {
+			return a.StartDelegate(ctx, "builder", "accepted work")
+		}},
+	} {
+		t.Run(entry.name, func(t *testing.T) {
+			far := &slowTaskStartAgent{
+				railAgent: &railAgent{fakeAgent: &fakeAgent{}},
+				accepted:  make(chan struct{}), release: make(chan struct{}),
+			}
+			var releaseOnce sync.Once
+			defer func() { releaseOnce.Do(func() { close(far.release) }) }()
+			loop, err := Loopback(Hello{Version: Version}, Options{Boot: func(Hello) (*Engine, error) {
+				return &Engine{Agent: far, Workspace: "/srv/app", SessionFile: "/srv/app/j.jsonl"}, nil
+			}})
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(func() { _ = loop.Close() })
+			lane, stop := loop.Client.Agent().WatchTaskUpdates()
+			t.Cleanup(stop)
+			waitFor(t, "the task lane to open", func() bool { return far.opened() == 1 })
+
+			ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+			t.Cleanup(cancel)
+			result := make(chan error, 1)
+			go func() {
+				_, _, _, err := entry.start(loop.Client.Agent(), ctx)
+				result <- err
+			}()
+			select {
+			case <-far.accepted:
+			case <-ctx.Done():
+				t.Fatal("the engine did not accept the task before the deadline")
+			}
+			if err := <-result; !errors.Is(err, session.ErrSendUnanswered) {
+				t.Fatalf("accepted start without its receipt = %v, want an uncertain result", err)
+			}
+			row := nextTask(t, lane)
+			if row.Task == nil || row.Task.ID != 7 || row.Task.State != session.TaskRunning {
+				t.Fatalf("accepted task disappeared after its receipt timed out: %+v", row)
+			}
+			releaseOnce.Do(func() { close(far.release) })
+			if _, err := loop.Client.Ping(); err != nil {
+				t.Fatalf("the late receipt broke the connection: %v", err)
+			}
+			far.mu.Lock()
+			defer far.mu.Unlock()
+			if far.starts != 1 || len(far.roster) != 1 {
+				t.Fatalf("start attempts = %d, tasks = %d, want one of each", far.starts, len(far.roster))
+			}
+		})
+	}
+}
+
+func TestAnEngineRefusalToStartATaskIsStillDefinite(t *testing.T) {
+	client, engine := newEngine(t)
+	engine.fails[MethodTaskStart] = "the planner did not answer in time"
+	_, _, _, err := client.Agent().StartTask(context.Background(), "work", true)
+	if err == nil || errors.Is(err, session.ErrSendUnanswered) {
+		t.Fatalf("engine refusal = %v, want a definite failure", err)
+	}
+}

internal/tui3/app.go

@@ -5139,7 +5139,7 @@ func (a *app) route(msg tea.Msg) (tea.Model, tea.Cmd) {
 			return a, nil
 		}
 		if msg.err != nil {
-			a.note("could not start the task · " + msg.err.Error())
+			a.note(taskStartFailureNote(msg.err))
 		} else {
 			// WHAT LANDED, AND WHAT IT IS CALLED (payload.go). The id is how a
 			// person names this node to any other command on the surface and the

internal/tui3/taskcommand.go

@@ -2,6 +2,7 @@ package tui3
 
 import (
 	"context"
+	"errors"
 	"strconv"
 	"strings"
 	"time"
@@ -93,6 +94,18 @@ type taskStartedMsg struct {
 	conv string
 }
 
+const taskStartLateNote = "the engine has not confirmed the start — the task may already be running"
+
+func taskStartFailureNote(err error) string {
+	if errors.Is(err, session.ErrSendUnanswered) {
+		return taskStartLateNote
+	}
+	if err == nil {
+		return "could not start the task"
+	}
+	return "could not start the task · " + err.Error()
+}
+
 func (a *app) runTaskCommand(arg string) tea.Cmd {
 	// The word was typed, bare or with a brief (notice.go).
 	a.noticeEvent(eventTaskTyped)

internal/tui3/taskcommand_test.go

@@ -28,6 +28,25 @@ type taskCommandFake struct {
 	err  error
 }
 
+func TestTaskStartLateAnswerDoesNotSayTheTaskFailedToStart(t *testing.T) {
+	note := taskStartFailureNote(session.ErrSendUnanswered)
+	if note != taskStartLateNote {
+		t.Fatalf("late start note = %q, want %q", note, taskStartLateNote)
+	}
+	if strings.Contains(note, "could not start") {
+		t.Fatalf("late start was reported as a start failure: %q", note)
+	}
+	f := &taskCommandFake{Agent: &fakeAgent{model: "m"}, err: session.ErrSendUnanswered}
+	a := newTestApp(f)
+	_, _ = a.Update(a.runTaskCommand("accepted work")())
+	if got := lastNote(t, a); got != taskStartLateNote {
+		t.Fatalf("task command displayed %q, want %q", got, taskStartLateNote)
+	}
+	if got := taskStartFailureNote(errors.New("the planner did not answer in time")); got != "could not start the task · the planner did not answer in time" {
+		t.Fatalf("a definite refusal was changed to uncertainty: %q", got)
+	}
+}
+
 func (f *taskCommandFake) StartTask(_ context.Context, brief string, solo bool) (uint64, string, string, error) {
 	f.singleCalls++
 	f.brief, f.solo = brief, solo