Merge remote-tracking branch 'origin/main' into timbre-2

Structured sessions spawned with auto_approve lost the flag on daemon
restart because it lived only in the in-memory SessionRecord. Add it to
the Session struct (sidecar JSON) so recovery reads it back instead of
hardcoding false.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

crates/maestro-cli/src/sessions.rs

@@ -345,6 +345,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 

crates/maestro-daemon/src/session.rs

@@ -1404,6 +1404,7 @@ impl SessionManager {
             last_focused_at: None,
             acp_session_id: None,
             view: req.view,
+            auto_approve: req.auto_approve,
         };
 
         if let Err(e) = self.inner.sidecar.write(&session) {
@@ -1678,6 +1679,7 @@ impl SessionManager {
                     // Unknown on adoption without a sidecar; matches every
                     // other field's forward-compat default.
                     view: maestro_protocol::SessionView::Terminal,
+                    auto_approve: false,
                 },
             };
             let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
@@ -1715,7 +1717,7 @@ impl SessionManager {
                     acp_events: None,
                     structured_activity_watermark: None,
                     acp_event_pump: None,
-                    auto_approve: false,
+                    auto_approve: session.auto_approve,
                     pending_prompt: None,
                 },
             );
@@ -1894,15 +1896,13 @@ impl SessionManager {
                     acp_events,
                     structured_activity_watermark: None,
                     acp_event_pump: None,
-                    // Recovery cannot know the original preset's flag; the
-                    // operator re-enables it by restarting with the preset.
-                    auto_approve: false,
+                    auto_approve: session.auto_approve,
                     pending_prompt: None,
                 },
             );
             if let Some((acp, events)) = event_pump {
                 let done = start_acp_event_pump(&self.inner, id, &acp, events.clone());
-                start_acp_permission_pump(&self.inner, id, &acp, events, false);
+                start_acp_permission_pump(&self.inner, id, &acp, events, session.auto_approve);
                 if let Some(record) = self.inner.sessions.lock().await.get_mut(&id) {
                     record.acp_event_pump = Some(done);
                 }
@@ -3023,12 +3023,7 @@ async fn settle_worktree_on_exit(
 /// gone from the authoritative in-memory map, so a stray FIFO or sidecar can't
 /// resurrect it. `kill_tmux` is `true` for an explicit kill and `false` for the
 /// liveness reaper (tmux already lost the session).
-async fn cleanup_record(
-    inner: &Arc<Inner>,
-    id: Uuid,
-    record: SessionRecord,
-    kill_tmux: bool,
-) {
+async fn cleanup_record(inner: &Arc<Inner>, id: Uuid, record: SessionRecord, kill_tmux: bool) {
     if kill_tmux {
         let tmux = inner.tmux.clone();
         let _ = tmux_blocking(move || tmux.kill_session(id)).await;

crates/maestro-daemon/tests/sidecar.rs

@@ -34,6 +34,7 @@ fn sample(id: Uuid) -> Session {
         last_focused_at: None,
         acp_session_id: None,
         view: SessionView::Terminal,
+        auto_approve: false,
     }
 }
 

crates/maestro-protocol/src/display.rs

@@ -744,6 +744,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 

crates/maestro-protocol/src/lib.rs

@@ -132,6 +132,12 @@ pub struct Session {
     /// the two are minted by different processes with different lifetimes.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub acp_session_id: Option<String>,
+    /// When `true` the daemon auto-approves every ACP permission request for
+    /// this session instead of surfacing them to the client. Set at spawn time
+    /// from the preset or the `CreateSessionRequest`; persisted so it survives
+    /// a daemon restart.
+    #[serde(default)]
+    pub auto_approve: bool,
 }
 
 /// A session's process shape (#133): a plain terminal, or a structured agent
@@ -1054,6 +1060,7 @@ mod tests {
             last_focused_at: None,
             view: SessionView::Terminal,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 

crates/maestro-testing/src/lib.rs

@@ -50,6 +50,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
         blocked_work: None,
         last_focused_at: None,
         acp_session_id: None,
+        auto_approve: false,
     }
 }
 

crates/maestro-tui/src/app.rs

@@ -2216,6 +2216,7 @@ mod tests {
                     blocked_work: None,
                     last_focused_at: None,
                     acp_session_id: None,
+                    auto_approve: false,
                 })
                 .collect(),
         });
@@ -2399,6 +2400,7 @@ mod tests {
                 blocked_work: None,
                 last_focused_at: None,
                 acp_session_id: None,
+                auto_approve: false,
             }],
         });
         app.dispatch(Action::SidebarNext);
@@ -2428,6 +2430,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 
@@ -3590,6 +3593,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: focused,
             acp_session_id: None,
+            auto_approve: false,
         };
         let older = Utc::now() - chrono::Duration::seconds(60);
         app.catalog.apply_event(CatalogEvent::Snapshot {
@@ -3870,6 +3874,7 @@ mod tests {
                 blocked_work: None,
                 last_focused_at: None,
                 acp_session_id: None,
+                auto_approve: false,
             }],
         });
         app.dispatch(Action::KillHighlighted);
@@ -3907,6 +3912,7 @@ mod tests {
                 blocked_work: None,
                 last_focused_at: None,
                 acp_session_id: None,
+                auto_approve: false,
             }],
         });
     }

crates/maestro-tui/src/catalog.rs

@@ -225,6 +225,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 

crates/maestro-tui/src/command_bar.rs

@@ -811,6 +811,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 

crates/maestro-tui/src/sidebar.rs

@@ -502,6 +502,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         }
     }
 
@@ -1272,6 +1273,7 @@ mod tests {
             blocked_work: None,
             last_focused_at: None,
             acp_session_id: None,
+            auto_approve: false,
         };
         let sb = Sidebar {
             sessions: vec![s],

docs/STRUCTURED-VIEW.md

@@ -189,14 +189,17 @@ choices, not daemon ones.
   presentation for `Notice`. `Plan` is the exception: both clients omit it
   from the transcript; the TUI may show the current plan in its status line,
   while the web client deliberately has no stats or plan rail.
-- The web client groups two or more tool rows after the same `User` row into
-  one collapsed turn activity block. `AgentText` and `AgentThought` rows stay
-  outside the group as standalone transcript items — the operator sees all
-  reasoning without expanding. Resolved approval rows between tools are
-  absorbed into the group. The collapsed header reads "N tool calls · Label
-  'arg'" where the label and argument come from the last tool row's
-  `splitToolTitle` — the operator sees which command is current at a glance.
-  Opening the group reveals tool rows in chronological order.
+- The web client groups each contiguous run of two or more tool rows into a
+  collapsed activity block. An `AgentText` or `AgentThought` row between
+  tools splits them into separate groups, so the timeline reads
+  text → group → text → group rather than lumping every tool in a turn into
+  one box. Text and thought rows stay outside groups as standalone transcript
+  items — the operator sees all reasoning without expanding. Resolved
+  approval rows between tools within a run are absorbed into the group. The
+  collapsed header reads "N tool calls · Label 'arg'" where the label and
+  argument come from the last tool row's `splitToolTitle` — the operator
+  sees which command is current at a glance. Opening the group reveals tool
+  rows in chronological order.
 - The web client's user, agent, thought, tool-detail, and notice text blocks
   copy their complete source text when clicked or keyboard-activated.
 - The web client consumes `available_commands_update` notices as the live

web/src/components/structured/model.test.ts

@@ -115,8 +115,8 @@ describe("activity grouping", () => {
   it("groups a turn's tools once there are two of them", () => {
     const rows = [text("user:1", "user", "go"), tool("tool:1"), tool("tool:2")];
     const groups = activityGroupsByTurn(rows, null, false);
-    expect([...groups.keys()]).toEqual(["turn:user:1"]);
-    expect(groups.get("turn:user:1")).toMatchObject({ toolCount: 2 });
+    expect([...groups.keys()]).toEqual(["turn:user:1:0"]);
+    expect(groups.get("turn:user:1:0")).toMatchObject({ toolCount: 2 });
   });
 
   it("leaves a single tool call standalone", () => {
@@ -124,7 +124,7 @@ describe("activity grouping", () => {
     expect(activityGroupsByTurn(rows, null, false).size).toBe(0);
   });
 
-  it("keeps intermediate text outside the group as standalone items", () => {
+  it("splits tools into separate runs when text appears between them", () => {
     const rows = [
       text("user:1", "user", "go"),
       tool("tool:1"),
@@ -132,9 +132,23 @@ describe("activity grouping", () => {
       tool("tool:2"),
       text("agent:2", "agent_text", "done"),
     ];
-    const group = activityGroupsByTurn(rows, null, false).get("turn:user:1")!;
-    expect(group.rows.map((r) => r.id)).toEqual(["tool:1", "tool:2"]);
-    expect(group.toolCount).toBe(2);
+    // Each run has only one tool, so neither qualifies for grouping.
+    expect(activityGroupsByTurn(rows, null, false).size).toBe(0);
+  });
+
+  it("groups contiguous tools separated by text into distinct groups", () => {
+    const rows = [
+      text("user:1", "user", "go"),
+      tool("tool:1"),
+      tool("tool:2"),
+      text("agent:1", "agent_text", "working"),
+      tool("tool:3"),
+      tool("tool:4"),
+    ];
+    const groups = activityGroupsByTurn(rows, null, false);
+    expect([...groups.keys()]).toEqual(["turn:user:1:0", "turn:user:1:1"]);
+    expect(groups.get("turn:user:1:0")!.rows.map((r) => r.id)).toEqual(["tool:1", "tool:2"]);
+    expect(groups.get("turn:user:1:1")!.rows.map((r) => r.id)).toEqual(["tool:3", "tool:4"]);
   });
 
   it("keeps thoughts outside the group as standalone items", () => {
@@ -145,7 +159,7 @@ describe("activity grouping", () => {
       text("thought:1", "agent_thought", "hmm"),
       text("thought:2", "agent_thought", "aha"),
     ];
-    const group = activityGroupsByTurn(rows, null, false).get("turn:user:1")!;
+    const group = activityGroupsByTurn(rows, null, false).get("turn:user:1:0")!;
     expect(group.rows.map((r) => r.id)).toEqual(["tool:1", "tool:2"]);
   });
 
@@ -155,7 +169,7 @@ describe("activity grouping", () => {
       row("tool:1", "tool_call", { title: "Read(src/main.ts)", status: "completed" }),
       row("tool:2", "tool_call", { title: "Execute(git status)", kind: "execute", status: "completed" }),
     ];
-    const group = activityGroupsByTurn(rows, null, false).get("turn:user:1")!;
+    const group = activityGroupsByTurn(rows, null, false).get("turn:user:1:0")!;
     expect(group.lastToolLabel).toBe("Execute");
     expect(group.lastToolArg).toBe("git status");
   });
@@ -169,7 +183,7 @@ describe("activity grouping", () => {
       row("approval:2", "approval"),
       tool("tool:3"),
     ];
-    const group = activityGroupsByTurn(rows, "approval:2", false).get("turn:user:1")!;
+    const group = activityGroupsByTurn(rows, "approval:2", false).get("turn:user:1:0")!;
     expect(group.rows.map((r) => r.id)).toEqual(["tool:1", "approval:1", "tool:2", "tool:3"]);
   });
 });
@@ -180,11 +194,31 @@ describe("transcript render list", () => {
     const items = transcriptItems(rows, null, false);
     expect(items.map((item) => (item.type === "row" ? item.row.id : item.id))).toEqual([
       "user:1",
-      "turn:user:1",
+      "turn:user:1:0",
       "agent:1",
     ]);
   });
 
+  it("interleaves groups and text in timeline order", () => {
+    const rows = [
+      text("user:1", "user", "go"),
+      tool("tool:1"),
+      tool("tool:2"),
+      text("agent:1", "agent_text", "working"),
+      tool("tool:3"),
+      tool("tool:4"),
+      text("agent:2", "agent_text", "done"),
+    ];
+    const items = transcriptItems(rows, null, false);
+    expect(items.map((item) => (item.type === "row" ? item.row.id : item.id))).toEqual([
+      "user:1",
+      "turn:user:1:0",
+      "agent:1",
+      "turn:user:1:1",
+      "agent:2",
+    ]);
+  });
+
   it("omits plan rows, state-only notices and standalone resolved approvals", () => {
     const rows = [
       text("user:1", "user", "go"),

web/src/components/structured/model.ts

@@ -155,57 +155,68 @@ export interface ActivityGroup {
 }
 
 /**
- * Group tool rows (and resolved approvals between them) by turn. Text and
- * thought rows stay outside the group as standalone transcript items — the
- * operator sees all reasoning without expanding anything.
+ * Group contiguous tool rows (and resolved approvals between them) into
+ * collapsible blocks. An `AgentText` or `AgentThought` row between tools
+ * splits the run — each contiguous stretch of tools becomes its own group,
+ * so the timeline reads text → group → text → group rather than lumping
+ * every tool in a turn into one box.
  *
- * Returns groups keyed by `turn:<userId>`.
+ * A group must contain at least two tools to qualify; a single tool stays
+ * standalone. Returns groups keyed by `turn:<userId>:<seq>`.
  */
 export function activityGroupsByTurn(
   rows: TranscriptRow[],
   pendingApprovalId: string | null,
   _turnActive: boolean,
 ): Map<string, ActivityGroup> {
-  // First: count tools per turn to know which turns qualify for grouping.
-  const toolCounts = new Map<string, number>();
-  let turnId = "before-first-message";
-  for (const row of rows) {
-    if (row.kind === "user") turnId = row.id;
-    if (row.kind !== "tool_call" && row.kind !== "tool_diff") continue;
-    const gid = `turn:${turnId}`;
-    toolCounts.set(gid, (toolCounts.get(gid) ?? 0) + 1);
-  }
-  // Second: build groups for turns with 2+ tools.
   const groups = new Map<string, ActivityGroup>();
-  turnId = "before-first-message";
-  let pending: TranscriptRow[] = [];
-  let currentGid = `turn:${turnId}`;
+  let turnId = "before-first-message";
+  let runSeq = 0;
+  let runTools: TranscriptRow[] = [];
+  let runPending: TranscriptRow[] = [];
+
+  const flushRun = () => {
+    if (runTools.length >= 2) {
+      const gid = `turn:${turnId}:${runSeq}`;
+      const tool = (runTools[runTools.length - 1].data ?? {}) as { title?: string; name?: string | null; kind?: string };
+      const { label, arg } = splitToolTitle(tool.title, tool.name ?? undefined, tool.kind);
+      groups.set(gid, {
+        toolCount: runTools.length,
+        rows: [...runTools],
+        lastToolLabel: label,
+        lastToolArg: arg,
+      });
+      runSeq++;
+    }
+    runTools = [];
+    runPending = [];
+  };
+
   for (const row of rows) {
     if (row.kind === "user") {
+      flushRun();
       turnId = row.id;
-      currentGid = `turn:${turnId}`;
-      pending = [];
+      runSeq = 0;
+      continue;
+    }
+    // Text and thought rows break the current tool run.
+    if (row.kind === "agent_text" || row.kind === "agent_thought") {
+      flushRun();
       continue;
     }
-    const count = toolCounts.get(currentGid) ?? 0;
-    if (count < 2) continue;
     const isResolvedApproval = row.kind === "approval" && row.id !== pendingApprovalId;
     if (isResolvedApproval) {
-      pending.push(row);
+      runPending.push(row);
       continue;
     }
     if (row.kind === "tool_call" || row.kind === "tool_diff") {
-      const group = groups.get(currentGid) ?? { toolCount: count, rows: [], lastToolLabel: "", lastToolArg: "" };
-      group.rows.push(...pending);
-      pending = [];
-      group.rows.push(row);
-      const tool = (row.data ?? {}) as { title?: string; name?: string | null; kind?: string };
-      const { label, arg } = splitToolTitle(tool.title, tool.name ?? undefined, tool.kind);
-      group.lastToolLabel = label;
-      group.lastToolArg = arg;
-      groups.set(currentGid, group);
+      // Flush any pending resolved approvals into the run before this tool.
+      runTools.push(...runPending);
+      runPending = [];
+      runTools.push(row);
     }
   }
+  flushRun();
 
   return groups;
 }
@@ -217,10 +228,10 @@ export type TranscriptItem =
   | { type: "group"; id: string; group: ActivityGroup };
 
 /**
- * The transcript in render order: rows, with each turn's tool activity folded
- * into a single group item where one exists. Resolved approvals not absorbed
- * into a group are dropped — the tool row itself already identifies the
- * action. `plan` rows and state-only notices are dropped too (both clients
+ * The transcript in render order: rows, with each contiguous tool stretch
+ * folded into a single group item where one exists. Resolved approvals not
+ * absorbed into a group are dropped — the tool row itself already identifies
+ * the action. `plan` rows and state-only notices are dropped too (both clients
  * omit `Plan` from the transcript). A turn footer only survives as the final
  * item, so old footers stop reading like transcript content.
  */
@@ -230,21 +241,19 @@ export function transcriptItems(
   turnActive: boolean,
 ): TranscriptItem[] {
   const groups = activityGroupsByTurn(rows, pendingApprovalId, turnActive);
-  const grouped = new Set<string>();
-  for (const group of groups.values()) for (const row of group.rows) grouped.add(row.id);
+  // Reverse index: row id → group id that contains it.
+  const rowToGroup = new Map<string, string>();
+  for (const [groupId, group] of groups) for (const r of group.rows) rowToGroup.set(r.id, groupId);
 
   const items: TranscriptItem[] = [];
-  let turnId = "before-first-message";
   const emitted = new Set<string>();
   for (const row of rows) {
-    if (row.kind === "user") turnId = row.id;
-    const groupId = `turn:${turnId}`;
-    const group = groups.get(groupId);
-    if (group && grouped.has(row.id)) {
+    const groupId = rowToGroup.get(row.id);
+    if (groupId !== undefined) {
       // The group renders once, at its first member's position.
       if (!emitted.has(groupId)) {
         emitted.add(groupId);
-        items.push({ type: "group", id: groupId, group });
+        items.push({ type: "group", id: groupId, group: groups.get(groupId)! });
       }
       continue;
     }