Fix problems found in QA pass of build 1792795001

docs/operations.md

@@ -88,7 +88,7 @@ Vendor setup and protocol quirks are in [Claude Code](claude-protocol.md), [Code
 
 - A turn whose final message asks the human something in prose is bounced once, telling the agent to use `ask_human`. For Claude the bounce is a Stop hook that blocks the stop; Claude Code reports the block as a "Stop hook error" notification, which is expected. An agent told explicitly to ask in plain text will refuse the bounce.
 - A turn that asks a question has no summary; everything the human needs belongs in the question's context. Other turns use their final message as the summary; don't ask agents to write a second recap.
-- A review covers the whole task against the default branch, so a newer one makes older ones moot. Findings reach the task's agent together, once the whole review workflow finishes; an individual reviewer finishing is not a completed review.
+- A review covers the whole task against the default branch (a subtask against its parent's branch, which it merges into), so a newer one makes older ones moot. Findings reach the task's agent together, once the whole review workflow finishes; an individual reviewer finishing is not a completed review.
 - An audit checks only the newest commit of the default branch, one at a time per project, so it doesn't run on every commit but the newest one is always audited eventually. byre never fetches for an audit, so a commit pushed outside byre is audited only once a byre merge has fetched it.
 - A team task that comes up with some members failing to start isn't repaired; discard it and create it again.
 

internal/agents/lead.go

@@ -310,6 +310,9 @@ func (m *Manager) leadTurnEnded(ctx context.Context, l *live, res endResult) err
 	if handoff {
 		payload["handoff"] = true
 	}
+	if interrupted {
+		payload["interrupted"] = true
+	}
 	err := m.emit(ctx, l, agentActor, "turn.ended", payload)
 	if err != nil {
 		return err

internal/agents/manager.go

@@ -1003,11 +1003,15 @@ func (m *Manager) pump(l *live) {
 	m.mu.Lock()
 	delete(m.live, l.id)
 	closing := m.closing
-	quiet := closing || l.stopping
+	stopping := l.stopping
 	interrupted := l.interrupted
 	m.mu.Unlock()
 
-	orphaned := len(l.subagents) > 0 && !closing
+	if closing {
+		return
+	}
+
+	orphaned := len(l.subagents) > 0
 	if orphaned {
 		err := m.store.SetSubagents(ctx, l.id, 0, "byre")
 		if err != nil {
@@ -1015,8 +1019,8 @@ func (m *Manager) pump(l *live) {
 		}
 	}
 
-	if quiet {
-		m.logUndelivered(ctx, l.id, "agent process ended while closing or stopping")
+	if stopping {
+		m.logUndelivered(ctx, l.id, "agent process ended while stopping")
 		return
 	}
 
@@ -1259,6 +1263,9 @@ func (m *Manager) turnEnded(ctx context.Context, l *live, res endResult) error {
 	if l.member != "" {
 		payload["member"] = l.member
 	}
+	if interrupted {
+		payload["interrupted"] = true
+	}
 	owner := m.ownsTask(l.id)
 	if owner {
 		defer m.endMergeTurn(l.id, false)

internal/agents/review.go

@@ -119,9 +119,13 @@ func (m *Manager) beginReviews(ctx context.Context, ref store.SnapshotRef) error
 	if project.ReviewWorkflow == "" {
 		return finished(api.ReviewStateFailed, "", fmt.Sprintf("project %s has no review workflow", project.Name))
 	}
-	changes, err := git.Compare(ctx, project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
+	base, err := m.reviewBase(ctx, task, project)
 	if err != nil {
-		return finished(api.ReviewStateFailed, project.ReviewWorkflow, fmt.Sprintf("byre couldn't compare snapshot #%d with %s: %v", snap.N, project.DefaultBranch, err))
+		return err
+	}
+	changes, err := git.Compare(ctx, project.RepoPath, base, snap.CommitSha, snap.TreeSha, "")
+	if err != nil {
+		return finished(api.ReviewStateFailed, project.ReviewWorkflow, fmt.Sprintf("byre couldn't compare snapshot #%d with %s: %v", snap.N, base, err))
 	}
 	if len(changes.Files) == 0 {
 		slog.Info("review skipped, snapshot has no diff", "task", task.Id, "snapshot", snap.N)
@@ -232,7 +236,11 @@ func (m *Manager) launchReviewer(ctx context.Context, task api.Task, project api
 }
 
 func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.Project, snap api.Snapshot, def api.WorkflowDefinition, stage string, inputs []api.Review) (string, error) {
-	changes, err := git.Compare(ctx, project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
+	base, err := m.reviewBase(ctx, task, project)
+	if err != nil {
+		return "", err
+	}
+	changes, err := git.Compare(ctx, project.RepoPath, base, snap.CommitSha, snap.TreeSha, "")
 	if err != nil {
 		return "", err
 	}
@@ -244,7 +252,7 @@ func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.P
 	b := &strings.Builder{}
 	fmt.Fprintf(b, "Review the work of the agent on the byre task %q, as reviewer %q of the project's review workflow %q.\n\n%s\n\n", task.Title, stage, project.ReviewWorkflow, strings.TrimSpace(workflow.Get(def, stage).Prompt))
 	fmt.Fprintf(b, "It is snapshot #%d of the task, commit %s, checked out in your working directory. The change is all of the task's work, `git diff %s %s` (from the merge-base with %s); %d files changed:\n",
-		snap.N, snap.CommitSha, changes.Base, snap.CommitSha, project.DefaultBranch, len(changes.Files))
+		snap.N, snap.CommitSha, changes.Base, snap.CommitSha, base, len(changes.Files))
 	for i, f := range changes.Files {
 		if i == maxReviewFiles {
 			fmt.Fprintf(b, "- … and %d more\n", len(changes.Files)-maxReviewFiles)
@@ -262,6 +270,17 @@ func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.P
 	return b.String(), nil
 }
 
+func (m *Manager) reviewBase(ctx context.Context, task api.Task, project api.Project) (string, error) {
+	if task.ParentTaskId == nil || task.WorkflowStage != nil {
+		return project.DefaultBranch, nil
+	}
+	parent, err := m.store.GetTask(ctx, *task.ParentTaskId)
+	if err != nil {
+		return "", err
+	}
+	return parent.Branch, nil
+}
+
 func quote(s string) string {
 	lines := strings.Split(strings.TrimSpace(s), "\n")
 	for i, l := range lines {

internal/agents/snapshots.go

@@ -269,12 +269,12 @@ func (m *Manager) takeSnapshotTool(l *live) mcp.Tool {
 			if err != nil {
 				return "", err
 			}
-			return "Worktree committed to the task's branch, and snapshot requested. If the project has a review workflow, its reviewers check it against the default branch; their findings come to you together once the workflow finishes.", nil
+			return "Worktree committed to the task's branch, and snapshot requested. If the project has a review workflow, its reviewers check it against the default branch (a subtask's against its parent's branch); their findings come to you together once the workflow finishes.", nil
 		},
 	}
 }
 
-const takeSnapshotDescription = "Commit and snapshot a task's worktree now: byre commits the whole worktree, uncommitted work included, to the task's branch, and returns once that commit is made. It then snapshots that commit on byre's own ref for the task (refs/byre/tasks/<task>/snap/<n>). If the project has a review workflow, its reviewers check all of the task's work against the default branch, using each stage's model and instructions; findings arrive together once the workflow finishes. " +
+const takeSnapshotDescription = "Commit and snapshot a task's worktree now: byre commits the whole worktree, uncommitted work included, to the task's branch, and returns once that commit is made. It then snapshots that commit on byre's own ref for the task (refs/byre/tasks/<task>/snap/<n>). If the project has a review workflow, its reviewers check all of the task's work against the default branch (a subtask's against its parent's branch), using each stage's model and instructions; findings arrive together once the workflow finishes. " +
 	"byre already does this at the end of every turn that changes files; call this to review work mid-turn, or to retry a review that failed (it snapshots even when nothing changed since the last snapshot)."
 
 func (m *Manager) dropSnapshots(repo, taskID string) error {

internal/procgroup/procgroup.go

@@ -3,6 +3,8 @@ package procgroup
 import (
 	"context"
 	"os/exec"
+	"strconv"
+	"strings"
 	"syscall"
 )
 
@@ -25,6 +27,41 @@ func group(cmd *exec.Cmd) *Cmd {
 	return &Cmd{Cmd: cmd}
 }
 
+// Kill also kills descendants that left the group: agents run their commands in sessions of their own.
 func (cmd *Cmd) Kill() error {
-	return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+	pids := descendants(cmd.Process.Pid)
+	err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+	for _, pid := range pids {
+		syscall.Kill(pid, syscall.SIGKILL)
+	}
+	return err
+}
+
+func descendants(root int) []int {
+	out, err := exec.Command("ps", "-A", "-o", "pid=,ppid=").Output()
+	if err != nil {
+		return nil
+	}
+	children := map[int][]int{}
+	for _, line := range strings.Split(string(out), "\n") {
+		f := strings.Fields(line)
+		if len(f) != 2 {
+			continue
+		}
+		pid, err1 := strconv.Atoi(f[0])
+		ppid, err2 := strconv.Atoi(f[1])
+		if err1 != nil || err2 != nil {
+			continue
+		}
+		children[ppid] = append(children[ppid], pid)
+	}
+	pids := []int{}
+	queue := children[root]
+	for len(queue) > 0 {
+		pid := queue[0]
+		queue = queue[1:]
+		pids = append(pids, pid)
+		queue = append(queue, children[pid]...)
+	}
+	return pids
 }

mac/Sources/Byre/Chat.swift

@@ -56,6 +56,7 @@ final class ChatModel {
     @ObservationIgnored private var members: [String: String] = [:]
     private(set) var scrollRequest: (seq: Int, id: Int?, anchor: UnitPoint)?
     private(set) var fetching = false
+    private(set) var stopping = false
     private(set) var older: [ByreEvent]?
     @ObservationIgnored var stuck = true
     @ObservationIgnored var prepended = false
@@ -396,6 +397,9 @@ final class ChatModel {
         if let q = groups.first(where: { $0.id == "questions" }) {
             return flagged(q)
         }
+        if task.state == .running && stopping {
+            return ChatStatus(label: "Stopping…", help: "Waiting for the agent to stop its turn\(end)", tone: .working, group: nil)
+        }
         if let busy = task.state == .running ? store.taskState(task) : store.progress(task) {
             return ChatStatus(label: busy.label, help: "\(busy.title)\(end)", tone: .working, group: nil)
         }
@@ -555,7 +559,11 @@ final class ChatModel {
     }
 
     func interrupt(_ sessionId: String, _ overlays: Overlays) {
+        stopping = true
         Task {
+            defer {
+                stopping = false
+            }
             do {
                 _ = try await store.api.client.interruptSession(path: .init(sessionId: sessionId)).accepted
             } catch {
@@ -1895,6 +1903,7 @@ private struct SnapshotRow: View {
         HStack(spacing: 8) {
             Rectangle().fill(Palette.border).frame(height: 1)
             Text("snapshot #\(snap.n)")
+                .fixedSize()
             if !inherited {
                 Button("Changes") {
                     store.pinSnapshot(snap.n)

mac/Sources/Byre/TranscriptBuilder.swift

@@ -910,6 +910,9 @@ final class TranscriptBuilder {
             if bool(p, "handoff") {
                 return note([.text("handoff written; continuing in a new session with a fresh context")])
             }
+            if bool(p, "interrupted") {
+                return note([.text("turn stopped")])
+            }
             return Row(.turnEnd(error: bool(p, "is_error") ? "turn ended: \(jsString(p["subtype"]).replacingOccurrences(of: "_", with: " "))" : nil), taskId: rowTask)
 
         case "session.created":