Make task state store-owned and signal its changes to the lead

internal/agents/lead.go

@@ -15,7 +15,7 @@ import (
 )
 
 const leadGuidance = `You are the human's lead agent in byre, shown in the left pane of byre's web UI. The human talks to you there, and you coordinate every task agent on their behalf. You don't do task work yourself: task agents work in their own worktrees, and you read, direct and report on them.
-- Signals: byre sends you, as messages from byre, what happens across tasks: questions and errors that task agents file, turns they finish with their final message, questions the human answered, tasks created, starting to merge, merged or discarded. Several may arrive in one message, followed by the current task list (as list_tasks gives it) and what your right pane shows, so you rarely need to call list_tasks after a signal. The human doesn't see signals. Messages from the human arrive separately.
+- Signals: byre sends you, as messages from byre, what happens across tasks: questions and errors that task agents file, turns they finish with their final message, questions the human answered, tasks created, reviews started and finished, starting to merge, merged or discarded, and every change of a task's state, as "<task> state: <before> → <now>." listed first among what happened at the same moment. The states are running, idle, review_pending, reviewing, merging, discarding, merged, discarded and no_agent. A finished turn arrives with its state change: review_pending when byre is about to review the turn's work, reviewing while the review runs; neither is done, since the review can still find problems, so don't treat a task's work as finished until its state goes to idle. Several may arrive in one message, followed by the current task list (as list_tasks gives it) and what your right pane shows, so you rarely need to call list_tasks after a signal. The human doesn't see signals. Messages from the human arrive separately.
 - Don't respond to what the human just did. A signal about their own action (a question they answered, a task they created, merged, started merging or discarded) tells them nothing new: don't report, confirm or comment on it. Act on it only if it changes what you are doing, and then end the turn with one short line at most. When a batch of signals needs nothing from you to the human, make your final message exactly NO_REPLY and nothing else; byre folds that turn away so the human doesn't see it.
 - "Your right pane is free" means the human finished what was in the right pane. Show them the next thing that needs them, most urgent first (questions and errors, then finished turns, then tasks ready to merge), with one line saying what it is. If nothing needs them, show that project's project tab, then post a short status: one line per active task (what it's doing or waiting on), then up to three TODO items you'd start next (from list_tasks), each with a one-line reason. Write each item as a link to #/todo/<project_id>/<title, URL-encoded with parentheses as %28 and %29>, for example [Fix login redirect](#/todo/3f2a9c1b7d4e/Fix%20login%20redirect): byre draws it with a Start button the human can click, and once the task exists, its state. Start one with start_todo only when the human picks it. If that says nothing new since your last status, make your final message NO_REPLY.
 - Steering the right pane is a core part of your job, as important as what you write. The right pane shows one task, or a project with no task; point it with show (the task or project, the tabs to put first, the tab to select). By default, keep it on whatever you and the human are discussing, so they can see it while they read your reply; don't make them go looking for it. Call show in the same turn you bring something up:
@@ -315,20 +315,27 @@ func (m *Manager) warnLeadContext(ctx context.Context, l *live) error {
 	return m.store.AddLeadInput(ctx, store.LeadInput{Owner: l.owner, Kind: store.LeadSignal, Actor: "byre", Text: text})
 }
 
-func (m *Manager) Observe(ev api.Event) {
-	switch ev.Kind {
-	case "inbox.created", "inbox.updated", "task.created", "task.merge_started", "task.merged", "task.discarded", "turn.ended", "task.workflow_stage", "task.workflow_finished", "review.finished":
-	case "user.message":
-		if ev.TaskId == nil || !humanActor(ev.Actor) {
-			return
+func (m *Manager) Observe(events []api.Event) {
+	batch := []api.Event{}
+	for _, ev := range events {
+		switch ev.Kind {
+		case "inbox.created", "inbox.updated", "task.created", "task.merge_started", "task.merged", "task.discarded", "turn.ended", "task.workflow_stage", "task.workflow_finished", "review.started", "review.finished", "task.state":
+		case "user.message":
+			if ev.TaskId == nil || !humanActor(ev.Actor) {
+				continue
+			}
+		default:
+			continue
 		}
-	default:
+		batch = append(batch, ev)
+	}
+	if len(batch) == 0 {
 		return
 	}
 	select {
-	case m.signals <- ev:
+	case m.signals <- batch:
 	default:
-		slog.Warn("lead signal dropped", "kind", ev.Kind, "seq", ev.Seq)
+		slog.Warn("lead signal dropped", "kind", batch[0].Kind, "seq", batch[0].Seq)
 	}
 }
 
@@ -337,39 +344,56 @@ func (m *Manager) signalLoop() {
 		select {
 		case <-m.watchStop:
 			return
-		case ev := <-m.signals:
-			err := m.signal(context.Background(), ev)
+		case batch := <-m.signals:
+			err := m.signal(context.Background(), batch)
 			if err != nil {
-				slog.Error("signal lead", "kind", ev.Kind, "seq", ev.Seq, "err", err)
+				slog.Error("signal lead", "kind", batch[0].Kind, "seq", batch[0].Seq, "err", err)
 			}
 		}
 	}
 }
 
-func (m *Manager) signal(ctx context.Context, ev api.Event) error {
+func (m *Manager) signal(ctx context.Context, batch []api.Event) error {
 	owners, err := m.store.LeadOwners(ctx)
 	if err != nil {
 		return err
 	}
 
-	text, err := m.signalText(ctx, ev)
-	if err != nil {
-		return err
-	}
-
-	for _, owner := range owners {
-		if ev.Actor == leadActor(owner) {
-			continue
-		}
-		freed, err := m.freedText(ctx, owner, ev)
+	texts := []string{}
+	for _, ev := range batch {
+		text, err := m.signalText(ctx, ev)
 		if err != nil {
 			return err
 		}
-		lines := strings.TrimSpace(text + "\n" + freed)
-		if lines == "" {
+		texts = append(texts, text)
+	}
+
+	for _, owner := range owners {
+		states := []string{}
+		lines := []string{}
+		for i, ev := range batch {
+			if ev.Actor == leadActor(owner) {
+				continue
+			}
+			freed, err := m.freedText(ctx, owner, ev)
+			if err != nil {
+				return err
+			}
+			text := strings.TrimSpace(texts[i] + "\n" + freed)
+			if text == "" {
+				continue
+			}
+			if ev.Kind == "task.state" {
+				states = append(states, text)
+				continue
+			}
+			lines = append(lines, text)
+		}
+		text := strings.Join(append(states, lines...), "\n")
+		if text == "" {
 			continue
 		}
-		err = m.store.AddLeadInput(ctx, store.LeadInput{Owner: owner, Kind: store.LeadSignal, Actor: "byre", Text: lines})
+		err = m.store.AddLeadInput(ctx, store.LeadInput{Owner: owner, Kind: store.LeadSignal, Actor: "byre", Text: text})
 		if err != nil {
 			return err
 		}
@@ -486,14 +510,23 @@ func (m *Manager) signalText(ctx context.Context, ev api.Event) (string, error)
 	case "task.workflow_finished":
 		return fmt.Sprintf("%s finished workflow %q:\n%s", m.taskLabel(ctx, deref(ev.TaskId)), ev.Payload["workflow"], clip(fmt.Sprint(ev.Payload["output"]), 1500)), nil
 
-	case "review.finished":
+	case "review.started", "review.finished":
 		r := api.Review{}
 		err := remarshal(ev.Payload, &r)
 		if err != nil {
 			return "", err
 		}
+		if ev.Kind == "review.started" {
+			return fmt.Sprintf("%s started a review of snapshot %d.", m.taskLabel(ctx, r.TaskId), r.SnapshotN), nil
+		}
 		return fmt.Sprintf("%s %s", m.taskLabel(ctx, r.TaskId), reviewLine(r)), nil
 
+	case "task.state":
+		if ev.Payload["from"] == "" {
+			return fmt.Sprintf("%s state: %v.", m.taskLabel(ctx, deref(ev.TaskId)), ev.Payload["to"]), nil
+		}
+		return fmt.Sprintf("%s state: %v → %v.", m.taskLabel(ctx, deref(ev.TaskId)), ev.Payload["from"], ev.Payload["to"]), nil
+
 	case "user.message":
 		return fmt.Sprintf("%s sent %s a message; its agent is working on it.", ev.Actor, m.taskLabel(ctx, deref(ev.TaskId))), nil
 

internal/agents/leadtools.go

@@ -710,16 +710,13 @@ func (m *Manager) listTasksText(ctx context.Context) (string, error) {
 			if t.TodoItem != nil {
 				started[*t.TodoItem] = true
 			}
-			agent := "no agent"
+			agent := string(t.State)
 			sess, found, err := m.liveSession(ctx, t.Id)
 			if err != nil {
 				return "", err
 			}
-			if found {
-				agent = fmt.Sprintf("agent %s", sess.State)
-				if sess.Model != "" {
-					agent += " on " + sess.Model
-				}
+			if found && sess.Model != "" {
+				agent += "; agent on " + sess.Model
 			}
 			open := 0
 			for _, i := range items {

internal/agents/manager.go

@@ -51,7 +51,7 @@ type Manager struct {
 
 	deliverMu sync.Mutex
 	leadMu    sync.Mutex
-	signals   chan api.Event
+	signals   chan []api.Event
 
 	snapMu sync.Mutex
 	snaps  map[string]*snapState
@@ -114,7 +114,7 @@ func New(st *store.Store, worktreeRoot, leadRoot, codexHome string) *Manager {
 		paneMoved:    map[string]time.Time{},
 		startedTodo:  map[string]time.Time{},
 		finishing:    map[string]bool{},
-		signals:      make(chan api.Event, 1024),
+		signals:      make(chan []api.Event, 1024),
 		snaps:        map[string]*snapState{},
 		mergeHolders: map[string]string{},
 		bad:          newBadLines(),
@@ -310,7 +310,7 @@ func (m *Manager) Recover(ctx context.Context) error {
 			if derr != nil {
 				return derr
 			}
-			next, serr := m.store.EndTurn(ctx, sess.Id, "byre", map[string]any{"error": err.Error()})
+			next, serr := m.store.EndTurn(ctx, sess.Id, "byre", map[string]any{"error": err.Error()}, nil, false)
 			if serr != nil {
 				return serr
 			}
@@ -660,7 +660,7 @@ func (m *Manager) pump(l *live) {
 			slog.Error("drop stage output", "session", l.id, "err", err)
 		}
 	}
-	next, err := m.store.EndTurn(ctx, l.id, agentActor, map[string]any{"error": "agent process exited mid-turn"})
+	next, err := m.store.EndTurn(ctx, l.id, agentActor, map[string]any{"error": "agent process exited mid-turn"}, nil, false)
 	if err != nil {
 		slog.Error("record session exit", "session", l.id, "err", err)
 	}
@@ -848,18 +848,10 @@ func (m *Manager) turnEnded(ctx context.Context, l *live, res endResult) error {
 	case finished && !compacting && !asked:
 		payload["summary"] = true
 	}
-	err = m.emit(ctx, l, agentActor, "turn.ended", payload)
-	if err != nil {
-		return err
-	}
 	discard, err := m.store.GetDiscard(ctx, l.taskID)
 	if err != nil {
 		return err
 	}
-	go m.closeMergedSubtasks(l.taskID, l.dir)
-	if payload["summary"] == true && discard.State == store.DiscardNone {
-		go m.reportToParent(l.taskID, res.result)
-	}
 	mergingBy := ""
 	if finished && !handoff {
 		mergingBy, err = m.store.MergingBy(ctx, l.taskID)
@@ -867,8 +859,11 @@ func (m *Manager) turnEnded(ctx context.Context, l *live, res endResult) error {
 			return err
 		}
 	}
-	if discard.State == store.DiscardNone {
-		m.snapshotSoon(l, api.SnapshotTriggerTurnEnd, 0, mergingBy == "")
+	snapshot := discard.State == store.DiscardNone
+	review := snapshot && mergingBy == ""
+	go m.closeMergedSubtasks(l.taskID, l.dir)
+	if payload["summary"] == true && snapshot {
+		go m.reportToParent(l.taskID, res.result)
 	}
 
 	if res.window > 0 {
@@ -900,10 +895,14 @@ func (m *Manager) turnEnded(ctx context.Context, l *live, res endResult) error {
 		}
 	}
 
-	next, err := m.store.EndTurn(ctx, l.id, agentActor, nil)
+	turn := m.event(l, agentActor, "turn.ended", payload)
+	next, err := m.store.EndTurn(ctx, l.id, agentActor, nil, &turn, review)
 	if err != nil {
 		return err
 	}
+	if snapshot {
+		m.snapshotSoon(l, api.SnapshotTriggerTurnEnd, 0, review)
+	}
 
 	switch next {
 	case store.DiscardClosing:

internal/agents/review.go

@@ -471,6 +471,10 @@ func (m *Manager) cancelReviews(ctx context.Context, task api.Task, project api.
 }
 
 func (m *Manager) recoverReviews(ctx context.Context) error {
+	err := m.store.ClearReviewRequests(ctx)
+	if err != nil {
+		return err
+	}
 	running, err := m.store.RunningReviews(ctx)
 	if err != nil {
 		return err

internal/agents/snapshots.go

@@ -43,10 +43,12 @@ func (m *Manager) snapshotSoon(l *live, trigger api.SnapshotTrigger, seq int64,
 
 	st := m.snapStateLocked(l.taskID)
 	if st.closed {
+		m.dropRequest(l.taskID, &snapRequest{review: review})
 		return
 	}
 	req := &snapRequest{trigger: trigger, seq: seq, review: review}
 	if st.running {
+		m.dropRequest(l.taskID, st.pending)
 		st.pending = req
 		return
 	}
@@ -66,8 +68,13 @@ func (m *Manager) runSnapshots(l *live, st *snapState, req *snapRequest) {
 				slog.Error("take snapshot", "task", l.taskID, "trigger", req.trigger, "err", err)
 			}
 			if req.review {
-				go m.afterSnapshot(snap)
+				go func() {
+					m.afterSnapshot(snap)
+					m.endReviewRequest(l.taskID)
+				}()
 			}
+		} else {
+			m.dropRequest(l.taskID, req)
 		}
 		st.mu.Unlock()
 
@@ -75,6 +82,7 @@ func (m *Manager) runSnapshots(l *live, st *snapState, req *snapRequest) {
 		req = st.pending
 		st.pending = nil
 		if req == nil || st.closed {
+			m.dropRequest(l.taskID, req)
 			st.running = false
 			m.snapMu.Unlock()
 			return
@@ -83,6 +91,19 @@ func (m *Manager) runSnapshots(l *live, st *snapState, req *snapRequest) {
 	}
 }
 
+func (m *Manager) dropRequest(taskID string, req *snapRequest) {
+	if req != nil && req.review {
+		m.endReviewRequest(taskID)
+	}
+}
+
+func (m *Manager) endReviewRequest(taskID string) {
+	err := m.store.EndReviewRequest(context.Background(), taskID)
+	if err != nil {
+		slog.Error("end review request", "task", taskID, "err", err)
+	}
+}
+
 func (m *Manager) takeSnapshot(ctx context.Context, l *live, trigger api.SnapshotTrigger, seq int64) (*api.Snapshot, error) {
 	if seq == 0 {
 		var err error