diff --git a/internal/tmux/classify_test.go b/internal/tmux/classify_test.go new file mode 100644 index 0000000..4c2d07c --- /dev/null +++ b/internal/tmux/classify_test.go @@ -0,0 +1,193 @@ +package tmux + +import ( + "reflect" + "sort" + "testing" +) + +func TestClassifyClaudeProcs_DirectChildAndOrphan(t *testing.T) { + // Pane shell PID 100 spawned a direct claude (PID 200). + // PID 300 is an orphan (parent isn't a pane and isn't another claude). + procs := []ClaudeProc{ + {PID: 200, PPID: 100, Args: "claude --session-id abc"}, + {PID: 300, PPID: 1, Args: "claude --session-id def"}, + } + panePIDs := map[int]bool{100: true} + + direct, orphan := classifyClaudeProcs(procs, panePIDs) + if got, want := direct[100], "claude --session-id abc"; got != want { + t.Fatalf("direct[100] = %q, want %q", got, want) + } + if len(orphan) != 1 || orphan[0].PID != 300 { + t.Fatalf("expected orphan PID 300; got %+v", orphan) + } +} + +func TestClassifyClaudeProcs_DropsSubagent(t *testing.T) { + // Pane shell PID 100 -> claude PID 200 -> subagent PID 201 (PPID=200). + // The subagent must NOT appear in either direct or orphan, otherwise the + // fallback would mark an unrelated past session on the same path as LIVE. + procs := []ClaudeProc{ + {PID: 200, PPID: 100, Args: "claude --session-id main"}, + {PID: 201, PPID: 200, Args: "claude --session-id agent-1"}, + {PID: 202, PPID: 200, Args: "claude --session-id agent-2"}, + } + panePIDs := map[int]bool{100: true} + + direct, orphan := classifyClaudeProcs(procs, panePIDs) + if got, want := direct[100], "claude --session-id main"; got != want { + t.Fatalf("direct[100] = %q, want %q", got, want) + } + if len(orphan) != 0 { + gotPIDs := make([]int, len(orphan)) + for i, o := range orphan { + gotPIDs[i] = o.PID + } + sort.Ints(gotPIDs) + t.Fatalf("subagents must be excluded; got orphan PIDs %v", gotPIDs) + } + if len(direct) != 1 { + t.Fatalf("direct map should hold exactly 1 entry, got %d", len(direct)) + } +} + +func TestClassifyClaudeProcs_NestedSubagent(t *testing.T) { + // Three-level chain: pane -> claude (200) -> subagent (201) -> nested (202). + // Both 201 and 202 must be dropped. + procs := []ClaudeProc{ + {PID: 200, PPID: 100, Args: "claude main"}, + {PID: 201, PPID: 200, Args: "claude sub-1"}, + {PID: 202, PPID: 201, Args: "claude sub-2"}, + } + panePIDs := map[int]bool{100: true} + + direct, orphan := classifyClaudeProcs(procs, panePIDs) + if len(direct) != 1 || direct[100] != "claude main" { + t.Fatalf("direct = %v, want only main claude", direct) + } + if len(orphan) != 0 { + t.Fatalf("nested subagents must be excluded; got %d orphans", len(orphan)) + } +} + +func TestClassifyClaudeProcs_StandaloneOrphan(t *testing.T) { + // nohup'd / pane-closed claude with PPID=1 is a legitimate orphan and + // should still be classified as orphan (not subagent). + procs := []ClaudeProc{ + {PID: 500, PPID: 1, Args: "claude --session-id standalone"}, + } + panePIDs := map[int]bool{100: true, 200: true} + + direct, orphan := classifyClaudeProcs(procs, panePIDs) + if len(direct) != 0 { + t.Fatalf("direct should be empty, got %v", direct) + } + wantOrphan := []ClaudeProc{{PID: 500, PPID: 1, Args: "claude --session-id standalone"}} + if !reflect.DeepEqual(orphan, wantOrphan) { + t.Fatalf("orphan = %+v, want %+v", orphan, wantOrphan) + } +} + +func TestClassifyClaudeProcsByAncestry_CcproxyWrappedClaude(t *testing.T) { + // pane shell (100) -> ccproxy (150) -> claude (200). + // The ancestry walk must attribute claude to pane shell 100 even though + // the immediate PPID is ccproxy, not the pane. + procs := []ClaudeProc{ + {PID: 200, PPID: 150, Args: "claude --settings ..."}, + } + panePIDs := map[int]bool{100: true} + ppidOf := map[int]int{ + 200: 150, + 150: 100, + 100: 1, + } + direct, orphan := classifyClaudeProcsByAncestry(procs, panePIDs, ppidOf) + if got, want := direct[100], "claude --settings ..."; got != want { + t.Fatalf("direct[100] = %q, want %q", got, want) + } + if len(orphan) != 0 { + t.Fatalf("ccproxy-wrapped claude must not be orphaned; got %+v", orphan) + } +} + +func TestClassifyClaudeProcsByAncestry_SubagentUnderWrappedClaude(t *testing.T) { + // pane (100) -> ccproxy (150) -> main claude (200) -> sub claude (201). + // Sub must be dropped because the chain passes through another claude + // before reaching the pane. + procs := []ClaudeProc{ + {PID: 200, PPID: 150, Args: "main"}, + {PID: 201, PPID: 200, Args: "sub"}, + } + panePIDs := map[int]bool{100: true} + ppidOf := map[int]int{ + 200: 150, + 201: 200, + 150: 100, + 100: 1, + } + direct, orphan := classifyClaudeProcsByAncestry(procs, panePIDs, ppidOf) + if got, want := direct[100], "main"; got != want { + t.Fatalf("direct[100] = %q, want %q", got, want) + } + if len(orphan) != 0 { + t.Fatalf("subagent must be dropped; got %+v", orphan) + } +} + +func TestClassifyClaudeProcsByAncestry_TrueOrphan(t *testing.T) { + procs := []ClaudeProc{{PID: 300, PPID: 1, Args: "lingering"}} + panePIDs := map[int]bool{100: true} + ppidOf := map[int]int{300: 1, 100: 1} + direct, orphan := classifyClaudeProcsByAncestry(procs, panePIDs, ppidOf) + if len(direct) != 0 { + t.Fatalf("no pane in chain; direct should be empty, got %v", direct) + } + if len(orphan) != 1 || orphan[0].PID != 300 { + t.Fatalf("expected lingering claude in orphans; got %+v", orphan) + } +} + +func TestClassifyClaudeProcsByAncestry_CycleDefense(t *testing.T) { + procs := []ClaudeProc{{PID: 400, PPID: 401, Args: "loop"}} + panePIDs := map[int]bool{100: true} + ppidOf := map[int]int{400: 401, 401: 400} + direct, orphan := classifyClaudeProcsByAncestry(procs, panePIDs, ppidOf) + if len(direct) != 0 { + t.Fatalf("cycle without pane should produce no direct; got %v", direct) + } + if len(orphan) != 1 { + t.Fatalf("cycle without pane should yield 1 orphan; got %+v", orphan) + } +} + +func TestClassifyClaudeProcsByAncestry_RehomeScenario(t *testing.T) { + // The user-reported case: pane 1 has ccproxy-wrapped claude; two other + // claude processes (e.g. leftovers from previous pane-1 sessions) are + // orphans whose cwds happen to match current-window paths. Only the + // pane-1-owned claude must show up under directByPaneShell. The orphans + // must NOT silently inherit the current-window pane shell. + procs := []ClaudeProc{ + {PID: 200, PPID: 150, Args: "claude main"}, + {PID: 300, PPID: 1, Args: "claude orphan-a"}, + {PID: 400, PPID: 1, Args: "claude orphan-b"}, + } + panePIDs := map[int]bool{100: true} + ppidOf := map[int]int{ + 200: 150, + 150: 100, + 100: 1, + 300: 1, + 400: 1, + } + direct, orphan := classifyClaudeProcsByAncestry(procs, panePIDs, ppidOf) + if got, want := direct[100], "claude main"; got != want { + t.Fatalf("direct[100] = %q, want %q", got, want) + } + if len(direct) != 1 { + t.Fatalf("only pane 1's claude should be direct; got %d entries (%v)", len(direct), direct) + } + if len(orphan) != 2 { + t.Fatalf("two unattributable claudes should be orphans; got %+v", orphan) + } +} diff --git a/internal/tmux/live.go b/internal/tmux/live.go index d60cbaa..b576d9b 100644 --- a/internal/tmux/live.go +++ b/internal/tmux/live.go @@ -78,22 +78,20 @@ func markLiveSessionsTmux(sessions []session.Session) { return } - // Separate direct-child procs (ppid matches a pane) from orphaned (ppid=1) + // Walk every claude process up its PPID chain to find which tmux pane + // shell — if any — owns it. This handles claudes that aren't direct + // children of a pane shell (e.g. wrapped by ccproxy/teen/sudo). Subagents + // (claude under another claude) and true orphans (no pane in the ancestor + // chain) are filtered out of pane attribution. panePIDs := make(map[int]bool, len(panes)) for _, p := range panes { panePIDs[p.PID] = true } - directByPPID := make(map[int]string) // ppid → args (for pane-matched procs) - var orphaned []ClaudeProc // ppid=1 or ppid not matching any pane - for _, cp := range allProcs { - if panePIDs[cp.PPID] { - directByPPID[cp.PPID] = cp.Args - } else { - orphaned = append(orphaned, cp) - } - } + ppidOf := batchPPIDMap() + directByPaneShell, orphaned := classifyClaudeProcsByAncestry(allProcs, panePIDs, ppidOf) - // Build pane PID → claude args map (direct children) + // Build pane PID → claude args map. Only panes whose shell owns a real + // claude (per the PPID walk) get a cps entry. type claudeMatch struct { args string windowName string @@ -102,7 +100,7 @@ func markLiveSessionsTmux(sessions []session.Session) { } var cps []claudeMatch for _, p := range panes { - if args, ok := directByPPID[p.PID]; ok { + if args, ok := directByPaneShell[p.PID]; ok { absPath, _ := filepath.Abs(p.Path) if absPath != "" { inCur := currentKey != "" && p.Session+"|"+p.Window == currentKey @@ -111,11 +109,15 @@ func markLiveSessionsTmux(sessions []session.Session) { } } - // For orphaned claude procs, resolve their cwd via lsof and match to sessions + // True orphans (no tmux pane in their ancestry) are still alive but don't + // belong to any visible window. Mark them LIVE via their cwd, but never + // mark them as belonging to the current window — even when their cwd + // matches a pane in this window, the orphan itself isn't in this window. if len(orphaned) > 0 { + _ = currentWindowPaths // retained for documentation; orphans deliberately ignore it orphanCwds := resolveOrphanCwds(orphaned) for _, oc := range orphanCwds { - cps = append(cps, claudeMatch{args: oc.args, path: oc.cwd, currentWindow: currentWindowPaths[oc.cwd]}) + cps = append(cps, claudeMatch{args: oc.args, path: oc.cwd, currentWindow: false}) } } @@ -168,6 +170,122 @@ type ClaudeProc struct { Args string } +// classifyClaudeProcsByAncestry walks each claude process up its PPID chain +// using ppidOf and classifies it as: +// - direct: the chain reaches a tmux pane shell PID without first passing +// through another claude → return value `directByPaneShell` maps that +// pane shell PID → claude args. +// - subagent: the chain passes through another claude before reaching a +// pane shell → silently dropped (the parent claude already attributes +// the work to its own session). +// - true orphan: the chain ends at init (PPID 0 / 1) or a dead process +// without ever reaching a pane shell → returned in `orphaned`. +// +// The PPID walk is bounded by len(ppidOf) iterations to defend against +// cycles in a corrupt process map. When ppidOf is empty (lookup unavailable) +// the function falls back to the immediate PPID — only direct pane children +// and direct subagents are recognised. +func classifyClaudeProcsByAncestry(procs []ClaudeProc, panePIDs map[int]bool, ppidOf map[int]int) (directByPaneShell map[int]string, orphaned []ClaudeProc) { + claudePIDs := make(map[int]bool, len(procs)) + for _, cp := range procs { + claudePIDs[cp.PID] = true + } + directByPaneShell = make(map[int]string) + + walkOwningPane := func(startPID int) (paneShellPID int, isSubagent bool) { + cur := startPID + if ppid, ok := ppidOf[cur]; ok { + cur = ppid + } else { + // No process tree available; only direct parent is known via procs. + for _, cp := range procs { + if cp.PID == startPID { + cur = cp.PPID + break + } + } + } + // Bound the walk so a corrupt cycle can't hang us. + steps := len(ppidOf) + len(procs) + 4 + for i := 0; i < steps && cur > 1; i++ { + if cur != startPID && claudePIDs[cur] { + return 0, true + } + if panePIDs[cur] { + return cur, false + } + next, ok := ppidOf[cur] + if !ok { + return 0, false + } + cur = next + } + return 0, false + } + + for _, cp := range procs { + paneShell, sub := walkOwningPane(cp.PID) + if sub { + continue + } + if paneShell != 0 { + directByPaneShell[paneShell] = cp.Args + continue + } + orphaned = append(orphaned, cp) + } + return directByPaneShell, orphaned +} + +// classifyClaudeProcs partitions claude processes by their immediate PPID. +// This is the cheap pre-walk classification kept for backwards compatibility +// with callers that only inspect direct parent relationships. New code should +// prefer classifyClaudeProcsByAncestry. +func classifyClaudeProcs(procs []ClaudeProc, panePIDs map[int]bool) (directByPPID map[int]string, orphaned []ClaudeProc) { + claudePIDs := make(map[int]bool, len(procs)) + for _, cp := range procs { + claudePIDs[cp.PID] = true + } + directByPPID = make(map[int]string) + for _, cp := range procs { + if panePIDs[cp.PPID] { + directByPPID[cp.PPID] = cp.Args + continue + } + if claudePIDs[cp.PPID] { + continue // subagent of another claude + } + orphaned = append(orphaned, cp) + } + return directByPPID, orphaned +} + +// batchPPIDMap returns a pid → ppid map for every process visible to ps. +// Used by classifyClaudeProcsByAncestry to walk past intermediate wrappers +// such as ccproxy / tee / sudo when looking for the owning pane shell. +// Returns an empty map if the lookup fails — callers should still degrade +// gracefully (treating claudes as direct or orphan based on immediate PPID). +func batchPPIDMap() map[int]int { + out, err := exec.Command("ps", "-e", "-o", "pid=,ppid=").Output() + if err != nil { + return map[int]int{} + } + m := make(map[int]int) + for _, line := range strings.Split(string(out), "\n") { + fields := strings.Fields(strings.TrimSpace(line)) + if len(fields) < 2 { + continue + } + pid, err1 := strconv.Atoi(fields[0]) + ppid, err2 := strconv.Atoi(fields[1]) + if err1 != nil || err2 != nil { + continue + } + m[pid] = ppid + } + return m +} + // BatchFindClaudeProcs finds all claude processes and maps parent PID → args. // When multiple processes share ppid=1 (orphaned/reparented), they are stored // in the OrphanedProcs slice instead to avoid map key collisions. diff --git a/internal/tui/app.go b/internal/tui/app.go index cc20d1c..b629242 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -3687,6 +3687,7 @@ func (a *App) doRefresh() tea.Cmd { oldLive[i] = liveState{a.sessions[i].IsLive, a.sessions[i].IsResponding} a.sessions[i].IsLive = false a.sessions[i].IsResponding = false + a.sessions[i].IsCurrentWindow = false } tmux.MarkLiveSessions(a.sessions) for i := range a.sessions { diff --git a/internal/tui/conversation.go b/internal/tui/conversation.go index 50cdf31..3844641 100644 --- a/internal/tui/conversation.go +++ b/internal/tui/conversation.go @@ -6,6 +6,7 @@ import ( "io" "log" "os" + "regexp" "sort" "strconv" "strings" @@ -417,9 +418,22 @@ func (a *App) handleConversationKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { a.liveTail = false a.conv.split.BottomAlign = false if a.conv.task.ID != "" || a.conv.agent.ShortID != "" || a.conv.cron.ID != "" { - return a.popNavFrame() + // Pop one level back into the originating conv view and + // re-open the preview pane. The next ESC will close that + // preview before considering further navigation, so the + // user lands cleanly in the parent conv view instead of + // skipping past it to the session list. + m, cmd := a.popNavFrame() + if app, ok := m.(*App); ok { + app.conv.split.Show = true + app.conv.split.CacheKey = "" + app.updateConvPreview() + } + return m, cmd } - a.state = viewSessions + // Plain conv view (no drilldown, preview already closed): stay + // in the conv list. ESC never auto-exits to the session list; + // use `left` for that explicit navigation. return a, nil } case "enter": @@ -473,6 +487,21 @@ func (a *App) handleConversationKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return a.openAgentConversation(agents[0]) } } + // Tasks without a real ID (e.g. TaskCreate-only items built from + // the tool input alone) can't be filtered by ID. Falling through + // to openTaskConversation would mismatch and surface an unrelated + // task's content. Open the parent message in msgFull instead so + // the user lands on the turn that defined the task. + if item.task.ID == "" { + items := a.convList.Items() + if item.parentIdx >= 0 && item.parentIdx < len(items) { + if parent, ok := items[item.parentIdx].(convItem); ok && parent.kind == convMsg { + a.pushNavFrame() + return a.openMsgFullForEntry(parent.merged) + } + } + return a, nil + } // Otherwise drill into task — show conversation entries related to this task a.pushNavFrame() return a.openTaskConversation(item.task) @@ -754,16 +783,15 @@ func (a *App) updateConvPreview() { return } - var entry session.Entry - var sourceEntries []session.Entry + var build previewBuild switch item.kind { case convMsg: - entry = item.merged.entry + build.Fallback = item.merged.entry if item.merged.startIdx >= 0 && item.merged.endIdx < len(a.conv.messages) && item.merged.startIdx <= item.merged.endIdx { - sourceEntries = append([]session.Entry(nil), a.conv.messages[item.merged.startIdx:item.merged.endIdx+1]...) + build.Sources = append([]session.Entry(nil), a.conv.messages[item.merged.startIdx:item.merged.endIdx+1]...) } case convAgent: - entry = buildAgentPreviewEntry(item.agent) + build = buildAgentPreview(item.agent) case convSessionMeta: switch item.sessionMeta { case "memory": @@ -791,18 +819,23 @@ func (a *App) updateConvPreview() { return } if item.bgTaskID != "" { - entry = a.buildBgJobPreviewEntry(item.bgTaskID) + build = a.buildBgJobPreview(item.bgTaskID) } else { - entry = a.buildTaskPreviewEntry(item.task) + build = a.buildTaskPreview(item.task) } } - if a.conv.leftPaneMode != convPaneTree { - if a.conv.rightPaneMode == previewText { - entry = buildCompactEntry(entry, sourceEntries) - } else if a.conv.rightPaneMode == previewTool { - entry = buildStandardEntryWithSources(entry, sourceEntries) - } + // Mode transformation is uniform: every preview kind expresses itself as + // a previewBuild, and the three transformers consume it the same way. + var entry session.Entry + var blockSrcIdx []int + switch a.conv.rightPaneMode { + case previewText: + entry, blockSrcIdx = compactPreview(build) + case previewTool: + entry, blockSrcIdx = standardPreview(build) + default: + entry, blockSrcIdx = verbosePreview(build) } cacheKey := fmt.Sprintf("%s:%d:%x", baseKey, len(entry.Content), entryContentHash(entry.Content)) @@ -816,6 +849,7 @@ func (a *App) updateConvPreview() { sp.CacheKey = cacheKey if sp.Folds != nil { sp.Folds.ResetWithPrefs(entry, sp.TypeFoldPrefs, sp.TypeFmtPrefs) + sp.Folds.BlockSourceIdx = blockSrcIdx sp.Folds.HideHooks = a.conv.rightPaneMode == previewTool if sp.Folds.BlockFilter != "" { sp.Folds.BlockVisible = applyBlockFilter(sp.Folds.BlockFilter, entry) @@ -834,6 +868,7 @@ func (a *App) updateConvPreview() { sp.CacheKey = cacheKey if sp.Folds != nil { sp.Folds.GrowBlocks(entry, oldBC, sp.TypeFoldPrefs, sp.TypeFmtPrefs) + sp.Folds.BlockSourceIdx = blockSrcIdx sp.Folds.HideHooks = a.conv.rightPaneMode == previewTool } sp.RefreshFoldPreview(a.width, a.splitRatio) @@ -868,34 +903,35 @@ func entryContentHash(blocks []session.ContentBlock) uint64 { return h } -func buildCompactEntry(entry session.Entry, sourceEntries []session.Entry) session.Entry { - if len(sourceEntries) == 0 { - blocks := make([]session.ContentBlock, 0, len(entry.Content)) - first := true - for _, b := range entry.Content { - if b.Type != "text" { - continue - } - text := strings.TrimSpace(session.StripXMLTags(b.Text)) - if text == "" { - continue - } - if !first { - text = "[separator]\n\n" + text - } - blocks = append(blocks, session.ContentBlock{Type: "text", Text: text}) - first = false - } - if len(blocks) == 0 { - blocks = append(blocks, session.ContentBlock{Type: "text", Text: "(no text content)"}) - } - entry.Content = blocks - return entry - } - - blocks := make([]session.ContentBlock, 0, len(sourceEntries)*2) - first := true - for _, raw := range sourceEntries { +// previewBuild captures everything a preview transformer needs. Every preview +// kind — flat conversation turn, task, agent, background job — produces one +// of these, and the compact / standard / verbose transformers consume them +// uniformly so there's no per-kind special casing downstream. +// +// - Header : optional descriptor (task subject, agent id, bg command). +// Prepended as a single text block in every mode so the user +// always sees the context. +// - Sources : per-turn raw entries; compact and standard summarise each. +// - Fallback : pre-flattened entry used for verbose mode (and as the +// cache-key carrier even when Sources is empty). +type previewBuild struct { + Header string + Sources []session.Entry + Fallback session.Entry +} + +// compactPreview emits one text block per turn (plus an optional header block +// at the top). Returns the entry and a parallel slice mapping each output +// block to its source-entry index (-1 for the synthetic header). +func compactPreview(b previewBuild) (session.Entry, []int) { + blocks := make([]session.ContentBlock, 0, len(b.Sources)+1) + var srcIdx []int + if b.Header != "" { + blocks = append(blocks, session.ContentBlock{Type: "text", Text: strings.TrimSpace(b.Header)}) + srcIdx = append(srcIdx, -1) + } + first := len(blocks) == 0 + for i, raw := range b.Sources { text := compactPreviewMessageText(raw) if strings.TrimSpace(text) == "" { continue @@ -904,34 +940,47 @@ func buildCompactEntry(entry session.Entry, sourceEntries []session.Entry) sessi text = "[separator]\n\n" + text } blocks = append(blocks, session.ContentBlock{Type: "text", Text: text}) + srcIdx = append(srcIdx, i) first = false } if len(blocks) == 0 { blocks = append(blocks, session.ContentBlock{Type: "text", Text: "(no text content)"}) - } - entry.Content = blocks - return entry -} - -func buildStandardEntryWithSources(entry session.Entry, sourceEntries []session.Entry) session.Entry { - if len(sourceEntries) == 0 { - sourceEntries = []session.Entry{{ - Role: entry.Role, - Timestamp: entry.Timestamp, - Content: append([]session.ContentBlock(nil), entry.Content...), + srcIdx = append(srcIdx, -1) + } + out := b.Fallback + out.Content = blocks + return out, srcIdx +} + +// standardPreview emits a text summary + artifact rows (file / change / url) +// per turn, with the optional header as the first block. +func standardPreview(b previewBuild) (session.Entry, []int) { + sources := b.Sources + if len(sources) == 0 { + // No per-turn breakdown — treat the fallback entry as a single turn so + // we still get text-summary + artifact extraction. + sources = []session.Entry{{ + Role: b.Fallback.Role, + Timestamp: b.Fallback.Timestamp, + Content: append([]session.ContentBlock(nil), b.Fallback.Content...), }} } - blocks := make([]session.ContentBlock, 0, len(sourceEntries)*4) - firstSection := true - for _, raw := range sourceEntries { + blocks := make([]session.ContentBlock, 0, len(sources)*4+1) + var srcIdx []int + if b.Header != "" { + blocks = append(blocks, session.ContentBlock{Type: "text", Text: strings.TrimSpace(b.Header)}) + srcIdx = append(srcIdx, -1) + } + firstSection := len(blocks) == 0 + for i, raw := range sources { sectionBlocks := make([]session.ContentBlock, 0, len(raw.Content)+4) if msg := previewMessageText(raw); msg != "" { sectionBlocks = append(sectionBlocks, session.ContentBlock{Type: "text", Text: msg}) } - for _, b := range raw.Content { - if b.Type == "image" { - sectionBlocks = append(sectionBlocks, b) + for _, blk := range raw.Content { + if blk.Type == "image" { + sectionBlocks = append(sectionBlocks, blk) } } for _, item := range extract.BlockFilePaths(raw.Content) { @@ -950,22 +999,57 @@ func buildStandardEntryWithSources(entry session.Entry, sourceEntries []session. if len(sectionBlocks) == 0 { continue } - for i := range sectionBlocks { - if !firstSection && i == 0 && sectionBlocks[i].Type == "text" { - sectionBlocks[i].Text = "[separator]\n\n" + sectionBlocks[i].Text + for j := range sectionBlocks { + if !firstSection && j == 0 && sectionBlocks[j].Type == "text" { + sectionBlocks[j].Text = "[separator]\n\n" + sectionBlocks[j].Text } } blocks = append(blocks, sectionBlocks...) + for range sectionBlocks { + srcIdx = append(srcIdx, i) + } firstSection = false } if len(blocks) == 0 { blocks = append(blocks, session.ContentBlock{Type: "text", Text: "(no content)"}) + srcIdx = append(srcIdx, -1) } - entry.Content = blocks - return entry + out := b.Fallback + out.Content = blocks + return out, srcIdx } -func buildStandardEntry(entry session.Entry) session.Entry { - return buildStandardEntryWithSources(entry, nil) + +// verbosePreview returns the pre-flattened fallback entry unchanged and a +// parallel source-index map (when the fallback's block count matches the +// concatenated Sources content shape). The mapping is nil for synthetic +// fallbacks that insert their own header/divider blocks; anchor matching +// then falls back to numeric block index, which is fine for verbose. +func verbosePreview(b previewBuild) (session.Entry, []int) { + srcIdx := computeVerboseBlockSources(b.Fallback, b.Sources) + return b.Fallback, srcIdx +} + +// computeVerboseBlockSources maps each block of the entry back to its +// source-entry index by walking sources in order. Returns nil when the merged +// content shape doesn't match the source content totals. +func computeVerboseBlockSources(entry session.Entry, sources []session.Entry) []int { + if len(sources) == 0 { + return nil + } + total := 0 + for _, src := range sources { + total += len(src.Content) + } + if total != len(entry.Content) { + return nil + } + out := make([]int, 0, len(entry.Content)) + for i, src := range sources { + for range src.Content { + out = append(out, i) + } + } + return out } func renderPreviewHeader(entry session.Entry, textW int) string { @@ -1387,15 +1471,49 @@ func (a *App) setConvPreviewText(content string) { } type convPreviewAnchor struct { - baseKey string - blockText string - blockType string - viewportY int - blockIndex int + baseKey string + blockText string + blockCore string + blockType string + toolName string // for tool_use blocks or text summaries like "[ToolName]" + toolOrdinal int // 0-based occurrence index among same-name tool blocks before the cursor + sourceIdx int // source-entry index for this block (-1 = unknown) + viewportY int + blockIndex int +} + +// previewBlockToolRef returns the tool name carried by a block — directly from +// tool_use blocks, or extracted from "[ToolName]" summaries embedded in text +// blocks. Returns "" if the block does not reference a single tool. +func previewBlockToolRef(block session.ContentBlock) string { + if block.Type == "tool_use" { + return block.ToolName + } + if block.Type != "text" { + return "" + } + // Strip "[separator]\n\nROLE HH:MM:SS\n" decorations first so the + // "[ToolName]" summary isn't shadowed by a leading "[separator]" match. + core := previewBlockCore(strings.TrimSpace(session.StripXMLTags(block.Text))) + if core == "" { + return "" + } + names := previewBlockToolNames(core) + if len(names) != 1 { + return "" + } + name := names[0] + // Skip pseudo-tags ("[file] /path", "[url] http://...", "[change] /path") + // emitted by standardPreview — they are not real tool names. + switch name { + case "file", "url", "change", "separator": + return "" + } + return name } func captureConvPreviewAnchor(sp *SplitPane, baseKey string) convPreviewAnchor { - anchor := convPreviewAnchor{baseKey: baseKey, blockIndex: -1} + anchor := convPreviewAnchor{baseKey: baseKey, blockIndex: -1, sourceIdx: -1} if sp == nil { return anchor } @@ -1409,10 +1527,72 @@ func captureConvPreviewAnchor(sp *SplitPane, baseKey string) convPreviewAnchor { block := sp.Folds.Entry.Content[sp.Folds.BlockCursor] anchor.blockType = block.Type anchor.blockText = strings.TrimSpace(session.StripXMLTags(block.Text)) + anchor.blockCore = previewBlockCore(anchor.blockText) + anchor.toolName = previewBlockToolRef(block) + if anchor.toolName != "" { + // Count preceding blocks that reference the same tool so we can pick + // the same occurrence in the new mode. + for i := 0; i < sp.Folds.BlockCursor; i++ { + if previewBlockToolRef(sp.Folds.Entry.Content[i]) == anchor.toolName { + anchor.toolOrdinal++ + } + } + } + if len(sp.Folds.BlockSourceIdx) == len(sp.Folds.Entry.Content) { + anchor.sourceIdx = sp.Folds.BlockSourceIdx[sp.Folds.BlockCursor] + } anchor.blockIndex = sp.Folds.BlockCursor return anchor } +// previewHeaderRE matches a role-header line like "USER" or "ASSISTANT 12:00:03" +// produced by compactPreviewMessageText / previewMessageText. +var previewHeaderRE = regexp.MustCompile(`^[A-Z][A-Z_]*(\s+\d{2}:\d{2}:\d{2})?$`) + +// previewToolSummaryRE extracts tool names from a previewMessageText summary +// such as "[TaskUpdate]", "[[TaskUpdate]]", or "[Read×3, Edit]". +var previewToolSummaryRE = regexp.MustCompile(`\[+([^\[\]]+)\]+`) + +// previewBlockToolNames returns the tool names referenced inside a "[Name, Name×N]" +// style summary. Returns nil when no summary is present. +func previewBlockToolNames(text string) []string { + m := previewToolSummaryRE.FindStringSubmatch(text) + if len(m) < 2 { + return nil + } + parts := strings.Split(m[1], ",") + names := make([]string, 0, len(parts)) + for _, p := range parts { + name := strings.TrimSpace(p) + if idx := strings.Index(name, "×"); idx > 0 { + name = strings.TrimSpace(name[:idx]) + } + if name != "" { + names = append(names, name) + } + } + return names +} + +// previewBlockCore strips mode-specific decorations from a block's text so the +// same logical block matches across compact / standard / verbose modes. +// Compact and standard wrap each turn with a "[separator]\n\n" prefix and a +// "ROLE HH:MM:SS\n" header; verbose uses the raw content unchanged. +func previewBlockCore(text string) string { + s := strings.TrimSpace(text) + s = strings.TrimPrefix(s, "[separator]") + s = strings.TrimLeft(s, "\n") + if idx := strings.IndexByte(s, '\n'); idx > 0 { + if previewHeaderRE.MatchString(s[:idx]) { + s = s[idx+1:] + } + } else if previewHeaderRE.MatchString(s) { + // Header-only block (e.g. user tool_result entry with no body). + return "" + } + return strings.TrimSpace(s) +} + func restoreConvPreviewAnchor(sp *SplitPane, anchor convPreviewAnchor) { if sp == nil || sp.Folds == nil || len(sp.Folds.Entry.Content) == 0 { return @@ -1426,15 +1606,83 @@ func restoreConvPreviewAnchor(sp *SplitPane, anchor convPreviewAnchor) { break } } - if best < 0 && anchor.blockText != "" { + // Match on stripped-decoration text so a block selected in compact/standard + // (which carries a "[separator]\n\nROLE HH:MM:SS\n" prefix) still matches + // the same logical block in verbose mode (which has only the raw text). + if best < 0 && anchor.blockCore != "" { for i, block := range sp.Folds.Entry.Content { + if block.Type != anchor.blockType { + continue + } text := strings.TrimSpace(session.StripXMLTags(block.Text)) - if text == anchor.blockText { + if previewBlockCore(text) == anchor.blockCore { best = i break } } } + // Source-idx bridge: blocks built from the same source entry should map + // to each other across modes. Prefer text-conversation blocks (the user + // asked tool/result blocks to fall back to the nearest plain-text turn + // when leaving verbose for compact/standard). + srcMap := sp.Folds.BlockSourceIdx + hasSrc := anchor.sourceIdx >= 0 && len(srcMap) == len(sp.Folds.Entry.Content) + if best < 0 && hasSrc { + // Same source, same type. + for i, src := range srcMap { + if src == anchor.sourceIdx && sp.Folds.Entry.Content[i].Type == anchor.blockType { + best = i + break + } + } + // Same source, text block (preferred conversation representation). + if best < 0 { + for i, src := range srcMap { + if src == anchor.sourceIdx && sp.Folds.Entry.Content[i].Type == "text" { + best = i + break + } + } + } + // Same source, any block. + if best < 0 { + for i, src := range srcMap { + if src == anchor.sourceIdx { + best = i + break + } + } + } + // Nearest preceding source that has a text block (handles verbose + // tool_use -> compact, where the tool's own source produced no text). + if best < 0 { + bestSrc := -1 + for i, src := range srcMap { + if src >= 0 && src <= anchor.sourceIdx && src > bestSrc && sp.Folds.Entry.Content[i].Type == "text" { + best = i + bestSrc = src + } + } + } + } + // Tool-name bridge: standard mode collapses a tool-only turn into a single + // "[ToolName]" text summary, while verbose has the underlying tool_use + // blocks. Pick the same Nth occurrence by source order. + if best < 0 && anchor.toolName != "" { + var matches []int + for i, block := range sp.Folds.Entry.Content { + if previewBlockToolRef(block) == anchor.toolName { + matches = append(matches, i) + } + } + if len(matches) > 0 { + ord := anchor.toolOrdinal + if ord >= len(matches) { + ord = len(matches) - 1 + } + best = matches[ord] + } + } if best < 0 && anchor.blockIndex >= 0 { best = min(anchor.blockIndex, len(sp.Folds.Entry.Content)-1) } @@ -1454,9 +1702,10 @@ func restoreConvPreviewAnchor(sp *SplitPane, anchor convPreviewAnchor) { sp.ScrollToBlock() } -// buildAgentPreviewEntry builds a synthetic Entry from an agent's messages -// so the preview can use fold/unfold block cursor like regular messages. -func buildAgentPreviewEntry(agent session.Subagent) session.Entry { +// buildAgentPreview assembles the agent preview build (header + raw conv +// entries + flattened fallback). External callers that only need the +// flattened entry can use buildAgentPreviewEntry as a thin wrapper. +func buildAgentPreview(agent session.Subagent) previewBuild { entries, err := session.LoadMessages(agent.FilePath) if err == nil && len(entries) > 0 { entries = filterAgentContextEntries(entries) @@ -1472,7 +1721,17 @@ func buildAgentPreviewEntry(agent session.Subagent) session.Entry { if agent.FirstPrompt != "" { header += "\nPrompt: " + agent.FirstPrompt } - return buildConversationPreviewEntry(header, agent.Timestamp, entries) + return previewBuild{ + Header: header, + Sources: entries, + Fallback: buildConversationPreviewEntry(header, agent.Timestamp, entries), + } +} + +// buildAgentPreviewEntry is a backwards-compatible shim returning just the +// flattened fallback entry. New code should use buildAgentPreview directly. +func buildAgentPreviewEntry(agent session.Subagent) session.Entry { + return buildAgentPreview(agent).Fallback } func convPreviewBaseKey(item convItem) string { @@ -1671,15 +1930,29 @@ func extractBgTaskEntries(merged []mergedMsg, taskID string) []session.Entry { return entries } -func (a *App) buildBgJobPreviewEntry(taskID string) session.Entry { +// buildBgJobPreview assembles the bg-job preview build: header (job id + +// command), per-turn sources, and a flattened fallback entry. Callers that +// only need the fallback entry can use buildBgJobPreviewEntry as a shim. +func (a *App) buildBgJobPreview(taskID string) previewBuild { header := fmt.Sprintf("Background Job: %s", taskID) if cmd := buildBgTaskMap(a.conv.merged)[taskID]; cmd != "" { header += "\nCommand: " + cmd } - return buildConversationPreviewEntry(header, time.Time{}, extractBgTaskEntries(a.conv.merged, taskID)) + raws := extractBgTaskEntries(a.conv.merged, taskID) + return previewBuild{ + Header: header, + Sources: raws, + Fallback: buildConversationPreviewEntry(header, time.Time{}, raws), + } } -func (a *App) buildTaskPreviewEntry(task session.TaskItem) session.Entry { +func (a *App) buildBgJobPreviewEntry(taskID string) session.Entry { + return a.buildBgJobPreview(taskID).Fallback +} + +// buildTaskPreview assembles the task preview build: header (id + subject + +// status + description), per-turn sources, and a flattened fallback entry. +func (a *App) buildTaskPreview(task session.TaskItem) previewBuild { header := "Task" if task.ID != "" { header += ": " + task.ID @@ -1693,7 +1966,16 @@ func (a *App) buildTaskPreviewEntry(task session.TaskItem) session.Entry { if task.Description != "" { header += "\n\n" + task.Description } - return buildConversationPreviewEntry(header, time.Time{}, extractTaskEntries(a.conv.messages, task.ID)) + raws := extractTaskEntries(a.conv.messages, task.ID) + return previewBuild{ + Header: header, + Sources: raws, + Fallback: buildConversationPreviewEntry(header, time.Time{}, raws), + } +} + +func (a *App) buildTaskPreviewEntry(task session.TaskItem) session.Entry { + return a.buildTaskPreview(task).Fallback } func extractCronEntries(entries []session.Entry, cron session.CronItem) []session.Entry { diff --git a/internal/tui/conversation_render.go b/internal/tui/conversation_render.go index 90739b3..4b445da 100644 --- a/internal/tui/conversation_render.go +++ b/internal/tui/conversation_render.go @@ -657,14 +657,26 @@ func buildConvItems(sess session.Session, merged []mergedMsg, agents []session.S continue } if subject != "" { - // TaskCreate: show subject directly + // TaskCreate: show subject directly. Resolve the task ID + // from the session's task list by subject so the preview + // and Enter drilldown can target the right entries — + // without an ID, downstream filtering (extractTaskEntries + // / openTaskConversation) can't distinguish this task + // from any other ID-less TaskCreate match. label := icon + " " + subject if len(label) > 50 { label = label[:47] + "..." } + resolvedID := "" + for _, t := range tasks { + if t.Subject == subject { + resolvedID = t.ID + break + } + } items = append(items, convItem{ kind: convTask, - task: session.TaskItem{Subject: label}, + task: session.TaskItem{Subject: label, ID: resolvedID}, indent: 1, parentIdx: parentIdx, }) diff --git a/internal/tui/conversation_ux_test.go b/internal/tui/conversation_ux_test.go index 9ecd128..3738c3a 100644 --- a/internal/tui/conversation_ux_test.go +++ b/internal/tui/conversation_ux_test.go @@ -893,7 +893,10 @@ func TestBuildCompactEntrySkipsToolOnlyTurns(t *testing.T) { makeTextEntry("assistant", base.Add(2*time.Second), "Found the issue in main.go"), } - entry := buildCompactEntry(session.Entry{Role: "assistant"}, sourceEntries) + entry, _ := compactPreview(previewBuild{ + Sources: sourceEntries, + Fallback: session.Entry{Role: "assistant"}, + }) if got := len(entry.Content); got != 2 { t.Fatalf("compact entry block count = %d, want 2", got) } @@ -962,6 +965,243 @@ func TestModeSwitchPreservesNearestSelection(t *testing.T) { } } +func TestModeCyclePreservesOriginalSelection(t *testing.T) { + base := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + entries := []session.Entry{ + makeTextEntry("user", base, "Question one"), + { + Role: "assistant", + Timestamp: base.Add(time.Second), + Content: []session.ContentBlock{ + {Type: "text", Text: "FIRST marker xyz1"}, + {Type: "tool_use", ToolName: "Read", ID: "r1", ToolInput: `{"file_path":"/tmp/a.go"}`}, + {Type: "tool_use", ToolName: "Read", ID: "r2", ToolInput: `{"file_path":"/tmp/b.go"}`}, + }, + }, + { + Role: "user", + Timestamp: base.Add(2 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_result", ID: "r1", Text: "package a"}, + {Type: "tool_result", ID: "r2", Text: "package b"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(3 * time.Second), + Content: []session.ContentBlock{ + {Type: "text", Text: "MIDDLE marker pqr3"}, + {Type: "tool_use", ToolName: "Read", ID: "r3", ToolInput: `{"file_path":"/tmp/c.go"}`}, + {Type: "tool_use", ToolName: "Read", ID: "r4", ToolInput: `{"file_path":"/tmp/d.go"}`}, + }, + }, + { + Role: "user", + Timestamp: base.Add(4 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_result", ID: "r3", Text: "package c"}, + {Type: "tool_result", ID: "r4", Text: "package d"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(5 * time.Second), + Content: []session.ContentBlock{ + {Type: "text", Text: "LAST marker abc2"}, + }, + }, + } + app := setupConvApp(t, entries, 160, 40) + app.conv.split.Focus = true + app.conv.rightPaneMode = previewText + selectConvItemBy(t, app, func(ci convItem) bool { + return ci.kind == convMsg && ci.merged.entry.Role == "assistant" + }) + app.updateConvPreview() + + // In compact mode select the block containing the "pqr3" (middle) marker. + target := -1 + for i, b := range app.conv.split.Folds.Entry.Content { + if b.Type == "text" && strings.Contains(b.Text, "pqr3") { + target = i + break + } + } + if target < 0 { + t.Fatalf("expected compact preview to contain marker pqr3, blocks=%+v", app.conv.split.Folds.Entry.Content) + } + app.conv.split.Folds.BlockCursor = target + app.conv.split.RefreshFoldPreview(app.width, app.splitRatio) + + // Cycle: compact -> standard -> verbose -> compact. At every step the + // selected block must remain the "pqr3" marker block — across modes the + // per-block text format differs (compact/standard wrap each turn with a + // "[separator]\n\nROLE HH:MM:SS\n" prefix, verbose uses the raw content), + // so a naive exact-text match plus blockIndex fallback corrupts the + // cursor in verbose and propagates the error back through compact. + for _, step := range []struct { + mode int + label string + }{ + {previewTool, "standard"}, + {previewHook, "verbose"}, + {previewText, "compact"}, + } { + app.setConvDetailLevel(step.mode) + bc := app.conv.split.Folds.BlockCursor + if bc < 0 || bc >= len(app.conv.split.Folds.Entry.Content) { + t.Fatalf("[%s] cursor out of range: %d (n=%d)", step.label, bc, len(app.conv.split.Folds.Entry.Content)) + } + got := app.conv.split.Folds.Entry.Content[bc].Text + if !strings.Contains(got, "pqr3") { + t.Fatalf("[%s] expected cursor on pqr3 block; got %q", step.label, got) + } + } +} + +func TestStandardToVerbosePreservesToolOnlyTurn(t *testing.T) { + base := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + entries := []session.Entry{ + makeTextEntry("user", base, "Please plan the work"), + { + Role: "assistant", + Timestamp: base.Add(time.Second), + Content: []session.ContentBlock{ + {Type: "tool_use", ToolName: "Edit", ID: "e1", ToolInput: `{"file_path":"/tmp/a.go"}`}, + }, + }, + { + Role: "user", + Timestamp: base.Add(2 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_result", ID: "e1", Text: "edited"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(3 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_use", ToolName: "TaskUpdate", ID: "t1", ToolInput: `{"taskId":"1","status":"completed"}`}, + }, + }, + { + Role: "user", + Timestamp: base.Add(4 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_result", ID: "t1", Text: "ok"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(5 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_use", ToolName: "Edit", ID: "e2", ToolInput: `{"file_path":"/tmp/b.go"}`}, + }, + }, + } + app := setupConvApp(t, entries, 160, 40) + app.conv.split.Focus = true + app.conv.rightPaneMode = previewTool + selectConvItemBy(t, app, func(ci convItem) bool { + return ci.kind == convMsg && ci.merged.entry.Role == "assistant" + }) + app.updateConvPreview() + + // In standard mode the tool-only turn shows up as a "[TaskUpdate]" text summary. + target := -1 + for i, b := range app.conv.split.Folds.Entry.Content { + if b.Type == "text" && strings.Contains(b.Text, "[TaskUpdate]") { + target = i + break + } + } + if target < 0 { + t.Fatalf("expected standard preview to summarize TaskUpdate turn; blocks=%+v", app.conv.split.Folds.Entry.Content) + } + app.conv.split.Folds.BlockCursor = target + app.conv.split.RefreshFoldPreview(app.width, app.splitRatio) + + app.setConvDetailLevel(previewHook) + + bc := app.conv.split.Folds.BlockCursor + if bc < 0 || bc >= len(app.conv.split.Folds.Entry.Content) { + t.Fatalf("verbose cursor out of range: %d (n=%d)", bc, len(app.conv.split.Folds.Entry.Content)) + } + got := app.conv.split.Folds.Entry.Content[bc] + if got.Type != "tool_use" || got.ToolName != "TaskUpdate" { + t.Fatalf("verbose cursor should land on the TaskUpdate tool_use block, got type=%s name=%q", got.Type, got.ToolName) + } +} + +func TestVerboseToolToCompactFallsBackToPrecedingText(t *testing.T) { + base := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + entries := []session.Entry{ + makeTextEntry("user", base, "kick off"), + { + Role: "assistant", + Timestamp: base.Add(time.Second), + Content: []session.ContentBlock{ + {Type: "text", Text: "Plan: marker conv-A"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(2 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_use", ToolName: "TaskUpdate", ID: "t1", ToolInput: `{}`}, + }, + }, + { + Role: "user", + Timestamp: base.Add(3 * time.Second), + Content: []session.ContentBlock{ + {Type: "tool_result", ID: "t1", Text: "ok"}, + }, + }, + { + Role: "assistant", + Timestamp: base.Add(4 * time.Second), + Content: []session.ContentBlock{ + {Type: "text", Text: "Next: marker conv-B"}, + }, + }, + } + app := setupConvApp(t, entries, 160, 40) + app.conv.split.Focus = true + app.conv.rightPaneMode = previewHook + selectConvItemBy(t, app, func(ci convItem) bool { + return ci.kind == convMsg && ci.merged.entry.Role == "assistant" + }) + app.updateConvPreview() + + // In verbose, select the TaskUpdate tool_use block (sandwiched between two + // text turns). Compact mode drops tool-only turns entirely, so the cursor + // should fall back to the nearest preceding plain-text turn ("conv-A"). + target := -1 + for i, b := range app.conv.split.Folds.Entry.Content { + if b.Type == "tool_use" && b.ToolName == "TaskUpdate" { + target = i + break + } + } + if target < 0 { + t.Fatalf("expected verbose preview to contain TaskUpdate tool_use; blocks=%+v", app.conv.split.Folds.Entry.Content) + } + app.conv.split.Folds.BlockCursor = target + app.conv.split.RefreshFoldPreview(app.width, app.splitRatio) + + app.setConvDetailLevel(previewText) + + bc := app.conv.split.Folds.BlockCursor + if bc < 0 || bc >= len(app.conv.split.Folds.Entry.Content) { + t.Fatalf("compact cursor out of range: %d (n=%d)", bc, len(app.conv.split.Folds.Entry.Content)) + } + got := app.conv.split.Folds.Entry.Content[bc] + if got.Type != "text" || !strings.Contains(got.Text, "conv-A") { + t.Fatalf("compact cursor should fall back to preceding 'conv-A' text turn, got type=%s text=%q", got.Type, got.Text) + } +} + func TestBuildEntityTreeUsesCompactLabels(t *testing.T) { merged := []mergedMsg{{ entry: session.Entry{ @@ -1069,16 +1309,95 @@ func TestTreeBgJobPreviewShowsCommandAndOutput(t *testing.T) { } app := setupTreeConvApp(t, entries, nil, nil, 160, 50) - app.conv.rightPaneMode = previewTool selectConvItemBy(t, app, func(ci convItem) bool { return ci.bgTaskID == "bg-1" }) - app.updateConvPreview() - if got := len(app.conv.split.Folds.Entry.Content); got < 3 { - t.Fatalf("background job tree preview should include command and output blocks, got %d", got) + // Standard mode: command lives in the synthetic header. Like regular + // conversation standard previews, tool_result bodies aren't expanded here. + app.setConvDetailLevel(previewTool) + stdText := entryFullText(app.conv.split.Folds.Entry) + if !strings.Contains(stdText, "Command: npm test --watch --runInBand") { + t.Fatalf("standard bg job preview should include command in header; got %q", stdText) + } + + // Verbose mode keeps the full synthetic entry, including the underlying + // tool_use / tool_result blocks for the bg job. + app.setConvDetailLevel(previewHook) + verbose := app.conv.split.Folds.Entry.Content + var verboseFull strings.Builder + hasToolResult := false + for _, b := range verbose { + verboseFull.WriteString(b.Text) + verboseFull.WriteByte('\n') + if b.Type == "tool_result" { + hasToolResult = true + } } - text := entryFullText(app.conv.split.Folds.Entry) - if !strings.Contains(text, "Command: npm test --watch --runInBand") { - t.Fatalf("background job preview should include command text, got %q", text) + if !strings.Contains(verboseFull.String(), "Command: npm test --watch --runInBand") { + t.Fatalf("verbose bg job preview should include command; got %q", verboseFull.String()) + } + if !hasToolResult { + t.Fatalf("verbose bg job preview should include tool_result block; got blocks=%+v", verbose) + } + if !strings.Contains(verboseFull.String(), "all tests passed") { + t.Fatalf("verbose bg job preview should retain tool_result output text; got %q", verboseFull.String()) + } +} + +func TestTreeTaskCompactModeFiltersToTextOnly(t *testing.T) { + base := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + entries := []session.Entry{ + { + Role: "assistant", + Timestamp: base, + Content: []session.ContentBlock{ + {Type: "text", Text: "Starting refactor with marker xyz"}, + {Type: "tool_use", ToolName: "TaskUpdate", ToolInput: `{"taskId":"99","status":"in_progress"}`}, + {Type: "tool_use", ToolName: "Edit", ToolInput: `{"file_path":"/tmp/a.go"}`}, + }, + }, + } + tasks := []session.TaskItem{{ID: "99", Subject: "Demo task", Status: "in_progress", Description: "verify mode distinction"}} + + app := setupTreeConvApp(t, entries, tasks, nil, 160, 50) + selectConvItemBy(t, app, func(ci convItem) bool { return ci.kind == convTask && ci.task.ID == "99" }) + + // Verbose: keeps the rich synthetic entry. Tool-use blocks survive. + app.setConvDetailLevel(previewHook) + verboseContent := app.conv.split.Folds.Entry.Content + hasToolUse := false + for _, b := range verboseContent { + if b.Type == "tool_use" { + hasToolUse = true + break + } + } + if !hasToolUse { + t.Fatalf("verbose tree task preview should retain tool_use blocks, blocks=%+v", verboseContent) + } + + // Compact: text-only filter. No tool_use blocks should remain. + app.setConvDetailLevel(previewText) + compactContent := app.conv.split.Folds.Entry.Content + for _, b := range compactContent { + if b.Type != "text" { + t.Fatalf("compact tree task preview should keep text blocks only, got %s in %+v", b.Type, compactContent) + } + } + if !strings.Contains(entryFullText(app.conv.split.Folds.Entry), "Starting refactor with marker xyz") { + t.Fatalf("compact tree task preview should retain conversation text; got %q", entryFullText(app.conv.split.Folds.Entry)) + } + + // Switching back to verbose restores the rich view (tool_use returns). + app.setConvDetailLevel(previewHook) + hasToolUseAfter := false + for _, b := range app.conv.split.Folds.Entry.Content { + if b.Type == "tool_use" { + hasToolUseAfter = true + break + } + } + if !hasToolUseAfter { + t.Fatalf("verbose tree task preview should retain tool_use after round-trip, blocks=%+v", app.conv.split.Folds.Entry.Content) } } @@ -1119,6 +1438,111 @@ func TestTreeTaskPreviewShowsActivityLog(t *testing.T) { } } +func TestPopNavFrameRestoresParentConvPosition(t *testing.T) { + app := setupConvApp(t, testEntries(), 160, 50) + app.state = viewConversation + + if items := app.convList.Items(); len(items) < 2 { + t.Fatalf("test needs at least 2 conv items; got %d", len(items)) + } + originalIdx := 1 + app.convList.Select(originalIdx) + originalItems := app.conv.items + + // Simulate the drilldown push (cursor + parent state captured). + app.pushNavFrame() + + // Mutate state as a real task drilldown would: cursor at 0, different items, + // task.ID set. popNavFrame should undo all of this. + app.conv.task = session.TaskItem{ID: "task-x"} + app.conv.items = []convItem{originalItems[0]} // truncated synthetic list + app.rebuildConversationList(0) + + app.popNavFrame() + if app.state != viewConversation { + t.Fatalf("popNavFrame should leave us in viewConversation; got %v", app.state) + } + if app.conv.task.ID != "" { + t.Fatalf("popNavFrame should clear conv.task; got %+v", app.conv.task) + } + if got := len(app.conv.items); got != len(originalItems) { + t.Fatalf("conv.items should be restored (%d items); got %d", len(originalItems), got) + } + if got := app.convList.Index(); got != originalIdx { + t.Fatalf("cursor should restore to original idx=%d; got %d", originalIdx, got) + } +} + +func TestEscFromTaskDrilldownReturnsToParentConv(t *testing.T) { + // Reproduce the user-reported flow: enter a session conv, drill into a + // task, then press ESC twice. The first ESC should land back in the parent + // session conversation (not the session list), and a second ESC should + // still leave us in the parent conv view — it must not skip past it. + app := setupConvApp(t, testEntries(), 160, 50) + app.state = viewConversation + originalIdx := 1 + app.convList.Select(originalIdx) + + // Simulate the drilldown the case "enter" handler does. + app.pushNavFrame() + app.conv.task = session.TaskItem{ID: "task-x", Subject: "drilldown probe"} + // Preview hidden when ESC fires reproduces the actual failing flow. + app.conv.split.Show = false + + app = pressKey(app, "esc") + + if app.state != viewConversation { + t.Fatalf("first ESC from task drilldown should stay in viewConversation; got state=%v", app.state) + } + if app.conv.task.ID != "" { + t.Fatalf("first ESC should clear conv.task (pop); got %+v", app.conv.task) + } + if !app.conv.split.Show { + t.Fatalf("first ESC should re-open the preview pane in the parent conv view") + } + + // Second ESC closes the (re-opened) preview but stays in the parent + // conv view. The drilldown→pop ESC must not skip past the parent into + // the session list in one keystroke. + app = pressKey(app, "esc") + if app.state != viewConversation { + t.Fatalf("second ESC should still leave us in viewConversation; got state=%v", app.state) + } + if app.conv.split.Show { + t.Fatalf("second ESC should close the preview in the parent conv view") + } + + // Third ESC (parent conv, preview closed) stays in the conv list — ESC + // never auto-exits the conv view. + app = pressKey(app, "esc") + if app.state != viewConversation { + t.Fatalf("third ESC must NOT exit to session list; got state=%v", app.state) + } +} + +func TestEscFromPlainConvStaysInConvList(t *testing.T) { + // In a plain session conv view (no drilldown): + // - preview shown → ESC closes it (stay in conv list) + // - preview closed → ESC stays in conv list (no auto-exit; `left` + // is the explicit exit key) + app := setupConvApp(t, testEntries(), 160, 50) + app.state = viewConversation + app.conv.split.Show = true + + app = pressKey(app, "esc") + if app.state != viewConversation { + t.Fatalf("first ESC should keep us in viewConversation; got state=%v", app.state) + } + if app.conv.split.Show { + t.Fatalf("first ESC should close the preview") + } + + app = pressKey(app, "esc") + if app.state != viewConversation { + t.Fatalf("second ESC must NOT exit to session list; got state=%v", app.state) + } +} + func TestEscClosesPreview(t *testing.T) { app := setupConvApp(t, testEntries(), 160, 50) app.conv.split.Show = true @@ -1440,7 +1864,7 @@ func TestBuildStandardEntryPlacesArtifactsNearRelatedText(t *testing.T) { {Type: "text", Text: "And here is the summary"}, }, } - preview := buildStandardEntry(entry) + preview, _ := standardPreview(previewBuild{Fallback: entry}) textIdx := -1 fileIdx := -1 artifactsHeaderIdx := -1 diff --git a/internal/tui/msgfull.go b/internal/tui/msgfull.go index 84a9f6d..afd36ef 100644 --- a/internal/tui/msgfull.go +++ b/internal/tui/msgfull.go @@ -14,16 +14,19 @@ import ( // navFrame stores state for navigating back from an agent drill-down. type navFrame struct { - sess session.Session - messages []session.Entry - merged []mergedMsg - agents []session.Subagent - items []convItem - listIdx int // cursor position to restore - agent session.Subagent - task session.TaskItem - cron session.CronItem - fromView viewState // which view pushed this frame + sess session.Session + messages []session.Entry + merged []mergedMsg + agents []session.Subagent + items []convItem + treeItems []convItem // tree-mode items (parallel to items; needed because list index is mode-specific) + leftPaneMode int // flat vs tree at push time — cursor index is meaningless without it + rightPaneMode int // compact/standard/verbose at push time + listIdx int // cursor position to restore, indexes into items or treeItems per leftPaneMode + agent session.Subagent + task session.TaskItem + cron session.CronItem + fromView viewState // which view pushed this frame } // openMsgFullForEntry opens viewMessageFull for a specific merged message. @@ -482,16 +485,19 @@ func (a *App) renderMsgFullSearchHintBox() string { // pushNavFrame saves current conversation state onto the nav stack. func (a *App) pushNavFrame() { frame := navFrame{ - sess: a.conv.sess, - messages: a.conv.messages, - merged: a.conv.merged, - agents: a.conv.agents, - items: a.conv.items, - listIdx: a.convList.Index(), - agent: a.conv.agent, - task: a.conv.task, - cron: a.conv.cron, - fromView: a.state, + sess: a.conv.sess, + messages: a.conv.messages, + merged: a.conv.merged, + agents: a.conv.agents, + items: a.conv.items, + treeItems: a.conv.treeItems, + leftPaneMode: a.conv.leftPaneMode, + rightPaneMode: a.conv.rightPaneMode, + listIdx: a.convList.Index(), + agent: a.conv.agent, + task: a.conv.task, + cron: a.conv.cron, + fromView: a.state, } a.navStack = append(a.navStack, frame) } @@ -538,18 +544,18 @@ func (a *App) popNavFrame() (tea.Model, tea.Cmd) { a.conv.merged = frame.merged a.conv.agents = frame.agents a.conv.items = frame.items + a.conv.treeItems = frame.treeItems + a.conv.leftPaneMode = frame.leftPaneMode + a.conv.rightPaneMode = frame.rightPaneMode a.conv.agent = frame.agent a.conv.task = frame.task a.conv.cron = frame.cron a.msgFull.allMessages = false - contentH := ContentHeight(a.height) - a.convList = newConvList(a.conv.items, a.conv.split.ListWidth(a.width, a.splitRatio), contentH) - a.conv.split.List = &a.convList - - if frame.listIdx < len(a.conv.items) { - a.convList.Select(frame.listIdx) - } + // rebuildConversationList respects leftPaneMode (flat vs tree) so the + // cursor index lands on the correct slice — frame.listIdx was captured + // from whichever pane mode was active at push time. + a.rebuildConversationList(frame.listIdx) a.conv.split.CacheKey = "" a.state = viewConversation a.updateConvPreview() diff --git a/internal/tui/splitpane.go b/internal/tui/splitpane.go index d0737e4..86e9eb4 100644 --- a/internal/tui/splitpane.go +++ b/internal/tui/splitpane.go @@ -44,15 +44,16 @@ type SplitPane struct { // FoldState holds fold/unfold and block cursor state for previews // that render structured content blocks. type FoldState struct { - Collapsed foldSet - Formatted foldSet - Entry session.Entry - BlockCursor int - BlockStarts []int - BlockVisible []bool // nil = all visible; non-nil = per-block visibility - BlockFilter string // current filter expression (empty = no filter) - HideHooks bool // true = suppress hook badges/details in render - Selected foldSet // block indices selected for copy + Collapsed foldSet + Formatted foldSet + Entry session.Entry + BlockCursor int + BlockStarts []int + BlockVisible []bool // nil = all visible; non-nil = per-block visibility + BlockFilter string // current filter expression (empty = no filter) + HideHooks bool // true = suppress hook badges/details in render + Selected foldSet // block indices selected for copy + BlockSourceIdx []int // parallel to Entry.Content; -1 = unknown source } // ListWidth returns the list width given total width and split ratio.