From 0b853598e46769b9b19afb045a67caf5669f2e80 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Thu, 2 Jul 2026 11:20:57 +0800 Subject: [PATCH 1/2] feat(tui): FILES sidebar panel with click-to-select and file drill-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a FILES section to the right context sidebar showing the files the session has touched — newest first, with A/M status badges, aggregated +/− diffstats, and a pulsing row for the file being written right now. Clicking a row selects the file (accent marker, edit cards tint in the chat, transcript scrolls to the latest edit); a second click opens a drill-in view: the chat column swaps to the file's stacked edit cards, `f` toggles to the syntax-highlighted on-disk file with markers on session-added lines, `d` back to diff, Esc returns to the chat. The roster is fed from two sources: changedFiles carried by write_file/edit_file/apply_patch results (persisted per session, so it survives /resume), and a git sweep — baseline snapshot at startup, re-check after command tools and at turn end — so files created through bash/exec_command scaffolding or subagents show up too. --- internal/tui/file_view.go | 224 +++++++++++++++++ internal/tui/file_view_test.go | 213 +++++++++++++++++ internal/tui/files_git_sweep.go | 217 +++++++++++++++++ internal/tui/files_git_sweep_test.go | 201 ++++++++++++++++ internal/tui/files_panel.go | 346 +++++++++++++++++++++++++++ internal/tui/files_panel_test.go | 232 ++++++++++++++++++ internal/tui/hover.go | 7 + internal/tui/model.go | 79 +++++- internal/tui/render_cache.go | 3 + internal/tui/rendering.go | 13 + internal/tui/session.go | 35 ++- internal/tui/sidebar.go | 22 ++ internal/tui/transcript.go | 5 + internal/tui/transcript_selection.go | 23 ++ 14 files changed, 1606 insertions(+), 14 deletions(-) create mode 100644 internal/tui/file_view.go create mode 100644 internal/tui/file_view_test.go create mode 100644 internal/tui/files_git_sweep.go create mode 100644 internal/tui/files_git_sweep_test.go create mode 100644 internal/tui/files_panel.go create mode 100644 internal/tui/files_panel_test.go diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go new file mode 100644 index 000000000..194554e3b --- /dev/null +++ b/internal/tui/file_view.go @@ -0,0 +1,224 @@ +// file_view.go is the drill-in view for a touched file, opened from the FILES +// sidebar section (files_panel.go). It reuses the subchat pattern: while +// active, the chat column's body swaps to this file's content — the sidebar, +// composer, and scroll engine keep working unchanged (transcriptBodyItems is +// the single source the viewport, renderer, and hit-testers all read, so +// swapping there keeps every consumer consistent). Two modes: +// +// diff (default) — the file's edit cards from this session, full-depth +// full — the file as it stands on disk, syntax highlighted, with +// gutter markers on the lines this session's diffs added +// +// d/f switch modes (with an empty composer), Esc returns to the chat at the +// scroll position it was left at. +package tui + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// fileViewMaxLines caps the full-file mode so a giant generated file can't +// freeze a render; the tail collapses into a "… N more lines" trailer. +const fileViewMaxLines = 4000 + +const ( + fileViewDiff = iota + fileViewFull +) + +// fileViewState manages the drill-in view for a touched file. When active, the +// transcript body swaps to the file's diff/content instead of the chat rows. +type fileViewState struct { + active bool + path string // workspace-relative, as carried by changedFiles + mode int // fileViewDiff | fileViewFull + // parentScrollOffset preserves the chat scroll position so closing the view + // returns to the same spot (mirrors subchatState). + parentScrollOffset int +} + +// openFileView activates the drill-in for path in diff mode. Opening from an +// already-open view (clicking another FILES row) keeps the original saved chat +// scroll position rather than saving the file view's own offset as "parent". +func (m model) openFileView(path string) model { + if !m.fileView.active { + m.fileView.parentScrollOffset = m.chatScrollOffset + } + m.fileView.active = true + m.fileView.path = path + m.fileView.mode = fileViewDiff + // A file only the git sweep knows about (bash/subagent mutation) has no edit + // cards to stack — open straight on the full file instead of a placeholder. + if len(m.fileViewResultRows()) == 0 { + m.fileView.mode = fileViewFull + } + m.chatScrollOffset = 0 + m = m.clearHover() // bodyY numbering differs between the file body and the chat + return m +} + +// exitFileView deactivates the view and restores the chat scroll position. +func (m model) exitFileView() model { + if !m.fileView.active { + return m + } + m.chatScrollOffset = m.fileView.parentScrollOffset + m.fileView = fileViewState{} + m = m.clearHover() + return m +} + +// setFileViewMode switches diff/full, resetting the scroll to the bottom-anchored +// start since the two bodies have unrelated heights. +func (m model) setFileViewMode(mode int) model { + if !m.fileView.active || m.fileView.mode == mode { + return m + } + m.fileView.mode = mode + m.chatScrollOffset = 0 + return m +} + +// fileViewNavBar renders the single-line header shown in place of the pinned +// title bar while the view is active: the path plus the key hints. One line +// exactly, so every scrollableTranscriptFrame computed against it agrees with +// the title bar's geometry. +func (m model) fileViewNavBar(width int) string { + mode := "diff" + other := "f full" + if m.fileView.mode == fileViewFull { + mode = "full" + other = "d diff" + } + left := zeroTheme.accent.Render("← "+truncatePathLeft(m.fileView.path, maxInt(8, width/2))) + + zeroTheme.faint.Render(" · "+mode) + right := zeroTheme.faint.Render(other + " · esc back") + return fitStyledLine(joinHeaderLine(left, right, width), width) +} + +// fileViewBodyItems builds the body items the transcript machinery renders +// while the view is active — one pre-rendered block, so scrolling and height +// accounting flow through the exact same path as chat rows. +func (m model) fileViewBodyItems(width int) []transcriptBodyItem { + var block string + if m.fileView.mode == fileViewFull { + block = m.renderFileViewFull(width) + } else { + block = m.renderFileViewDiff(width) + } + return []transcriptBodyItem{transcriptBlockBodyItem(transcriptBodyItemRow, -1, block)} +} + +// fileViewResultRows returns the transcript's tool-result rows that touched the +// viewed file, in chronological order. +func (m model) fileViewResultRows() []transcriptRow { + var rows []transcriptRow + for _, row := range m.transcript { + if row.kind != rowToolResult { + continue + } + for _, p := range row.changedFiles { + if p == m.fileView.path { + rows = append(rows, row) + break + } + } + } + return rows +} + +// renderFileViewDiff renders the session's edits to the file as its full-depth +// tool cards (bodyCap 0, the detailed-view depth), stacked chronologically — +// the same cards the chat shows, so the diffs read identically in both places. +func (m model) renderFileViewDiff(width int) string { + rows := m.fileViewResultRows() + if len(rows) == 0 { + return zeroTheme.faint.Render("No recorded edits for this file in this session.") + } + rc := buildRowContext(m.transcript) + opts := cardRenderOptions{bodyCap: 0, cwd: m.cwd} + var b strings.Builder + for i, row := range rows { + if i > 0 { + b.WriteString("\n\n") + } + if len(rows) > 1 { + b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("edit %d of %d", i+1, len(rows)))) + b.WriteString("\n") + } + b.WriteString(m.renderRowModeUncached(row, width, rc, opts)) + } + return b.String() +} + +// renderFileViewFull renders the file as it currently stands on disk, syntax +// highlighted, with a line-number gutter and an accent ▎ marker on the lines +// this session's diffs added (matched by exact text — an approximation that +// tolerates later drift; a stale marker just doesn't highlight). +func (m model) renderFileViewFull(width int) string { + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + data, err := os.ReadFile(target) + if err != nil { + return zeroTheme.faint.Render("Could not read file: " + err.Error()) + } + lines := strings.Split(strings.TrimSuffix(string(data), "\n"), "\n") + truncated := 0 + if len(lines) > fileViewMaxLines { + truncated = len(lines) - fileViewMaxLines + lines = lines[:fileViewMaxLines] + } + + changed := m.fileViewChangedLines() + gutterW := len(fmt.Sprintf("%d", len(lines))) + textBudget := maxInt(8, width-gutterW-3) // gutter + space + marker column + // Highlight with an effectively-infinite measure so the highlighter never + // wraps — output lines stay 1:1 with file lines and the gutter numbering + // can't desync. Each line is then truncated to the column budget below. + display, ok := highlightCodeForPath(lines, m.fileView.path, 1<<20, nil) + if !ok || len(display) != len(lines) { + display = lines // no lexer for this path: render plain + } + + var b strings.Builder + for i, line := range display { + line = fitStyledLine(line, textBudget) + if i > 0 { + b.WriteString("\n") + } + marker := " " + if changed[strings.TrimSpace(lines[i])] { + marker = zeroTheme.accent.Render("▎") + } + b.WriteString(zeroTheme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) + b.WriteString(marker) + b.WriteString(line) + } + if truncated > 0 { + b.WriteString("\n") + b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… %d more lines (file truncated for display)", truncated))) + } + return b.String() +} + +// fileViewChangedLines collects the trimmed text of every line the session's +// diffs ADDED to the viewed file, for the full-mode gutter markers. +func (m model) fileViewChangedLines() map[string]bool { + changed := map[string]bool{} + for _, row := range m.fileViewResultRows() { + for _, line := range strings.Split(row.detail, "\n") { + if !strings.HasPrefix(line, "+") || strings.HasPrefix(line, "+++") { + continue + } + if text := strings.TrimSpace(strings.TrimPrefix(line, "+")); text != "" { + changed[text] = true + } + } + } + return changed +} diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go new file mode 100644 index 000000000..9136346ec --- /dev/null +++ b/internal/tui/file_view_test.go @@ -0,0 +1,213 @@ +package tui + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/sessions" + "github.com/Gitlawb/zero/internal/tools" +) + +// TestFileViewOpenExitRestoresScroll: opening saves the chat scroll position, +// resets it for the file body, and Esc restores it; switching files while open +// keeps the ORIGINAL saved position (not the file view's own). +func TestFileViewOpenExitRestoresScroll(t *testing.T) { + m := filesPanelTestModel() + m.chatScrollOffset = 12 + + m = m.openFileView("web/app.js") + if !m.fileView.active || m.fileView.mode != fileViewDiff { + t.Fatalf("open should activate in diff mode: %+v", m.fileView) + } + if m.chatScrollOffset != 0 || m.fileView.parentScrollOffset != 12 { + t.Fatalf("open should reset scroll and save the parent offset: offset=%d saved=%d", m.chatScrollOffset, m.fileView.parentScrollOffset) + } + + m.chatScrollOffset = 5 // scrolled within the file body + m = m.openFileView("internal/tui/sidebar.go") + if m.fileView.parentScrollOffset != 12 { + t.Fatalf("switching files must keep the original parent offset, got %d", m.fileView.parentScrollOffset) + } + + m = m.exitFileView() + if m.fileView.active || m.chatScrollOffset != 12 { + t.Fatalf("exit should restore the chat scroll: active=%v offset=%d", m.fileView.active, m.chatScrollOffset) + } +} + +// TestFileViewEscAndModeKeys: Esc exits the view via the model's key handler; +// d/f switch modes while the composer is empty and never while typing. +func TestFileViewEscAndModeKeys(t *testing.T) { + m := filesPanelTestModel() + m = m.openFileView("web/app.js") + + updated, _ := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + m = updated.(model) + if m.fileView.mode != fileViewFull { + t.Fatal("f should switch to full mode") + } + updated, _ = m.Update(tea.KeyPressMsg{Code: 'd', Text: "d"}) + m = updated.(model) + if m.fileView.mode != fileViewDiff { + t.Fatal("d should switch back to diff mode") + } + + // With text in the composer, d/f type as normal characters. + m.input.SetValue("say") + updated, _ = m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + m = updated.(model) + if m.fileView.mode != fileViewDiff { + t.Fatal("f while typing must not hijack the composer") + } + m.input.SetValue("") + + updated, _ = m.Update(tea.KeyPressMsg{Code: tea.KeyEsc}) + m = updated.(model) + if m.fileView.active { + t.Fatal("Esc should exit the file view") + } +} + +// TestFileViewDiffBody: diff mode stacks the file's edit cards chronologically +// with "edit N of M" labels; a file with no recorded edits shows the quiet +// placeholder. +func TestFileViewDiffBody(t *testing.T) { + m := filesPanelTestModel() + m = m.openFileView("internal/tui/sidebar.go") + body := plainRender(t, m.renderFileViewDiff(78)) + if !strings.Contains(body, "edit 1 of 2") || !strings.Contains(body, "edit 2 of 2") { + t.Fatalf("expected chronological edit labels:\n%s", body) + } + if !strings.Contains(body, "added one") || !strings.Contains(body, "three") { + t.Errorf("expected both diffs' content:\n%s", body) + } + + m.fileView.path = "never/touched.go" + if got := plainRender(t, m.renderFileViewDiff(78)); !strings.Contains(got, "No recorded edits") { + t.Errorf("untouched file should show the placeholder, got:\n%s", got) + } +} + +// TestFileViewFullBody: full mode shows the on-disk content with line numbers +// and marks session-added lines with the gutter marker; a missing file degrades +// to a readable error line. +func TestFileViewFullBody(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "app.js"), []byte("let a = 1\nlet untouched = 0\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, tool: "write_file", id: "w9", status: tools.StatusOK, + detail: "+let a = 1", + changedFiles: []string{"app.js"}, + }) + m = m.openFileView("app.js") + m = m.setFileViewMode(fileViewFull) + + body := m.renderFileViewFull(78) + plain := plainRender(t, body) + if !strings.Contains(plain, "1 ") || !strings.Contains(plain, "let untouched = 0") { + t.Fatalf("full view should show numbered file content:\n%s", plain) + } + lines := strings.Split(plain, "\n") + if len(lines) != 2 { + t.Fatalf("one rendered line per file line, got %d:\n%s", len(lines), plain) + } + if !strings.Contains(lines[0], "▎") { + t.Errorf("session-added line should carry the gutter marker: %q", lines[0]) + } + if strings.Contains(lines[1], "▎") { + t.Errorf("untouched line must not carry the marker: %q", lines[1]) + } + + m.fileView.path = "gone.js" + if got := plainRender(t, m.renderFileViewFull(78)); !strings.Contains(got, "Could not read file") { + t.Errorf("missing file should degrade to an error line, got:\n%s", got) + } +} + +// TestFileViewSwapsTranscriptBody: while active, transcriptBodyItems returns +// the file body (a single block) instead of the chat rows, and the pinned +// title bar swaps to the one-line nav bar — the geometry every frame consumer +// relies on. +func TestFileViewSwapsTranscriptBody(t *testing.T) { + m := filesPanelTestModel() + m = m.openFileView("internal/tui/sidebar.go") + + items := m.transcriptBodyItems(m.chatColumnWidth(), "") + if len(items) != 1 { + t.Fatalf("file view should swap the body to a single block item, got %d items", len(items)) + } + nav := plainRender(t, m.pinnedTitleBar(m.chatColumnWidth())) + if !strings.Contains(nav, "sidebar.go") || !strings.Contains(nav, "esc back") { + t.Fatalf("nav bar should show the path and key hints: %q", nav) + } + if lines := len(viewLines(m.fileViewNavBar(m.chatColumnWidth()))); lines != 1 { + t.Fatalf("nav bar must be exactly one line (title-bar geometry), got %d", lines) + } + + // The whole view renders without panicking in both modes and shows the nav. + if view := plainRender(t, m.transcriptView()); !strings.Contains(view, "esc back") { + t.Fatal("transcript view should carry the file nav bar") + } +} + +// TestSubchatEntryClosesFileView: drilling into an AGENTS row while a file view +// is open closes the file view first (the subchat owns the single-column view). +func TestSubchatEntryClosesFileView(t *testing.T) { + store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()}) + if _, err := store.Create(sessions.CreateInput{SessionID: "sess-1"}); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.sessionStore = store + m.swarmSessionMap = map[string]string{"subagent-1": "sess-1"} + m.transcript = append(m.transcript, + transcriptRow{kind: rowToolCall, tool: "swarm_spawn", detail: "build it", runID: 1}, + transcriptRow{kind: rowToolResult, tool: "swarm_spawn", detail: "Spawned subagent as task subagent-1 on team default.", runID: 1}, + ) + m.activeRunID = 1 + m = m.openFileView("web/app.js") + + width := sidebarWidth(m.width) + agents := m.sidebarAgentSelectables(width) + if len(agents) == 0 { + t.Fatal("expected a clickable agent row") + } + click := testMouseClick(tea.MouseLeft, m.chatColumnWidth()+3, agents[0].lineOffset) + updated, _, handled := m.handleTranscriptSelectionMouse(click) + if !handled { + t.Fatal("agent row click should be handled") + } + if updated.fileView.active { + t.Fatal("entering the subchat should close the file view") + } + if !updated.subchat.active { + t.Fatal("subchat should be active") + } +} + +// TestChangedFilesRehydration: a persisted tool-result payload's changedFiles +// restores onto the rehydrated transcript row, so the FILES panel survives +// /resume. +func TestChangedFilesRehydration(t *testing.T) { + events := []sessions.Event{{ + Type: sessions.EventToolResult, + Payload: json.RawMessage(`{"toolCallId":"t1","name":"edit_file","status":"ok","output":"+x","changedFiles":["pkg/a.go","pkg/b.go"]}`), + }} + rows := transcriptRowsFromSessionEvents(events) + if len(rows) != 1 { + t.Fatalf("expected 1 row, got %d", len(rows)) + } + got := rows[0].changedFiles + if len(got) != 2 || got[0] != "pkg/a.go" || got[1] != "pkg/b.go" { + t.Fatalf("changedFiles not rehydrated: %v", got) + } +} diff --git a/internal/tui/files_git_sweep.go b/internal/tui/files_git_sweep.go new file mode 100644 index 000000000..14385a477 --- /dev/null +++ b/internal/tui/files_git_sweep.go @@ -0,0 +1,217 @@ +// files_git_sweep.go fills the FILES sidebar's blind spot: mutations that +// bypass the file tools entirely — bash/exec_command scaffolding (npm create, +// go generate, heredoc writes) and subagents editing the shared workspace. None +// of those produce a changedFiles-carrying tool result, so the transcript-derived +// roster (files_panel.go) never sees them. The sweep asks git instead: a +// baseline `git status --porcelain` snapshot is taken at startup (Init), and a +// re-run after each command tool result / turn end reports any NEWLY dirty +// paths, which merge into the roster with a diffstat from `git diff --numstat`. +// Pre-existing dirty state stays in the baseline and never shows; a non-git +// workspace fails the first command and the sweep silently stays off. +package tui + +import ( + "context" + "strconv" + "strings" + "time" + + tea "charm.land/bubbletea/v2" +) + +// gitSweepTimeout bounds each background git invocation so a hung index lock +// can never stall message delivery (the command runs off the update goroutine). +const gitSweepTimeout = 3 * time.Second + +// gitSweepFile is one workspace file git reports as dirty: workspace-relative +// path (git's own output convention, matching changedFiles), whether it is new +// (untracked/added), and the tracked diffstat when known (0/0 for untracked). +type gitSweepFile struct { + path string + created bool + adds int + dels int +} + +// gitSweepMsg carries one sweep's result. baseline marks the startup snapshot +// (recorded as "pre-existing, never show") vs a live re-check (merged into the +// roster). ok is false when git failed (not a repo, no git binary) — the +// handler then marks the sweep unavailable rather than retrying every turn. +type gitSweepMsg struct { + baseline bool + ok bool + files []gitSweepFile +} + +// gitSweepCmd runs the status+numstat pair off the update goroutine. cwd is the +// workspace root the TUI resolves paths against. +func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd { + return func() tea.Msg { + if parent == nil { + parent = context.Background() + } + ctx, cancel := context.WithTimeout(parent, gitSweepTimeout) + defer cancel() + // --untracked-files=all enumerates files inside new directories; plain + // porcelain collapses them to "dir/", which is useless as a FILES row. + status, err := defaultPRCommandRunner(ctx, cwd, "git", "status", "--porcelain", "--untracked-files=all") + if err != nil { + return gitSweepMsg{baseline: baseline, ok: false} + } + files := parseGitPorcelain(status) + // Diffstat for tracked modifications; untracked files have no diff to + // stat. Best-effort: a failure (e.g. unborn HEAD in a fresh repo) keeps + // the file list with zero counts rather than dropping the sweep. + if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat"); err == nil { + stats := parseGitNumstat(numstat) + for i := range files { + if counts, ok := stats[files[i].path]; ok { + files[i].adds, files[i].dels = counts[0], counts[1] + } + } + } + return gitSweepMsg{baseline: baseline, ok: true, files: files} + } +} + +// parseGitPorcelain parses `git status --porcelain` v1 output into sweep files. +// Format: two status columns, a space, then the path ("XY path"); renames show +// "old -> new" (keep the new side); untracked entries are "?? path". +func parseGitPorcelain(out string) []gitSweepFile { + var files []gitSweepFile + for _, line := range strings.Split(out, "\n") { + if len(line) < 4 { + continue + } + code, path := line[:2], strings.TrimSpace(line[3:]) + if to, _, found := cutRename(path); found { + path = to + } + path = unquoteGitPath(path) + if path == "" { + continue + } + files = append(files, gitSweepFile{ + path: path, + created: code == "??" || strings.Contains(code, "A"), + }) + } + return files +} + +// cutRename splits a porcelain rename value "old -> new", returning the new +// path. found is false for ordinary (non-rename) paths. +func cutRename(path string) (string, string, bool) { + if idx := strings.Index(path, " -> "); idx >= 0 { + return path[idx+4:], path[:idx], true + } + return "", "", false +} + +// unquoteGitPath strips the quotes git wraps around paths with special +// characters ("web/my file.js"). Escapes inside are left as-is — such a path +// still identifies the file well enough for a sidebar row. +func unquoteGitPath(path string) string { + if len(path) >= 2 && strings.HasPrefix(path, `"`) && strings.HasSuffix(path, `"`) { + return path[1 : len(path)-1] + } + return path +} + +// parseGitNumstat parses `git diff --numstat` output ("added\tdeleted\tpath") +// into path → [added, deleted]. Binary files report "-" and are skipped. +func parseGitNumstat(out string) map[string][2]int { + stats := map[string][2]int{} + for _, line := range strings.Split(out, "\n") { + parts := strings.SplitN(line, "\t", 3) + if len(parts) != 3 { + continue + } + adds, errA := strconv.Atoi(parts[0]) + dels, errD := strconv.Atoi(parts[1]) + if errA != nil || errD != nil { + continue + } + path := parts[2] + if to, _, found := cutRename(path); found { + path = to + } + stats[unquoteGitPath(path)] = [2]int{adds, dels} + } + return stats +} + +// handleGitSweepMsg folds a sweep result into the model: the baseline snapshot +// records what was already dirty before this TUI session (those paths never +// show), a live sweep upserts newly dirty paths into gitTouched in first-seen +// order. A failed sweep marks git unavailable so no further sweeps are issued. +func (m model) handleGitSweepMsg(msg gitSweepMsg) model { + m.gitSweepInFlight = false + if !msg.ok { + m.gitSweepUnavailable = true + if m.gitFileBaseline == nil { + m.gitFileBaseline = map[string]bool{} + } + return m + } + if msg.baseline { + baseline := make(map[string]bool, len(msg.files)) + for _, f := range msg.files { + baseline[f.path] = true + } + m.gitFileBaseline = baseline + return m + } + for _, f := range msg.files { + if m.gitFileBaseline[f.path] { + continue + } + found := false + for i := range m.gitTouched { + if m.gitTouched[i].path == f.path { + m.gitTouched[i] = f + found = true + break + } + } + if !found { + // Copy-on-append: model copies share the backing array, so an in-place + // append from two update branches could alias. Rebuilding is cheap at + // sidebar scale. + m.gitTouched = append(append([]gitSweepFile(nil), m.gitTouched...), f) + } + } + return m +} + +// maybeGitSweep issues a live sweep when one is useful and none is running: +// the baseline exists (Init's snapshot answered), git works here, and the +// workspace is known. Returns the (possibly nil) command to batch. +func (m model) maybeGitSweep() (model, tea.Cmd) { + if m.gitSweepInFlight || m.gitSweepUnavailable || m.gitFileBaseline == nil || strings.TrimSpace(m.cwd) == "" { + return m, nil + } + m.gitSweepInFlight = true + return m, gitSweepCmd(m.ctx, m.cwd, false) +} + +// gitTouchedFiles adapts the sweep results to the roster's touchedFile shape, +// for merging under the transcript-derived entries (files_panel.go). No +// transcript row backs them (lastRowIndex -1), so selecting one skips the +// scroll/tint and the drill-in opens on the full file. +func (m model) gitTouchedFiles() []touchedFile { + if len(m.gitTouched) == 0 { + return nil + } + files := make([]touchedFile, 0, len(m.gitTouched)) + for _, f := range m.gitTouched { + files = append(files, touchedFile{ + path: f.path, + created: f.created, + adds: f.adds, + dels: f.dels, + lastRowIndex: -1, + }) + } + return files +} diff --git a/internal/tui/files_git_sweep_test.go b/internal/tui/files_git_sweep_test.go new file mode 100644 index 000000000..68c80d16f --- /dev/null +++ b/internal/tui/files_git_sweep_test.go @@ -0,0 +1,201 @@ +package tui + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestParseGitPorcelain(t *testing.T) { + out := " M web/app.js\n?? web/new.css\nA docs/added.md\nR old.go -> pkg/new.go\n?? \"web/my file.js\"\n\n" + files := parseGitPorcelain(out) + if len(files) != 5 { + t.Fatalf("expected 5 entries, got %d: %+v", len(files), files) + } + byPath := map[string]gitSweepFile{} + for _, f := range files { + byPath[f.path] = f + } + if f := byPath["web/app.js"]; f.created { + t.Error("modified file must not read as created") + } + if f := byPath["web/new.css"]; !f.created { + t.Error("untracked file should read as created") + } + if f := byPath["docs/added.md"]; !f.created { + t.Error("index-added file should read as created") + } + if _, ok := byPath["pkg/new.go"]; !ok { + t.Errorf("rename should keep the new path: %+v", files) + } + if _, ok := byPath["web/my file.js"]; !ok { + t.Errorf("quoted path should unquote: %+v", files) + } +} + +func TestParseGitNumstat(t *testing.T) { + stats := parseGitNumstat("12\t3\tweb/app.js\n-\t-\tassets/logo.png\n4\t0\told.go => pkg/new.go\n1\t1\tR\told.go -> pkg/new.go\n") + if got := stats["web/app.js"]; got != [2]int{12, 3} { + t.Errorf("web/app.js = %v, want [12 3]", got) + } + if _, ok := stats["assets/logo.png"]; ok { + t.Error("binary '-' entries must be skipped") + } +} + +// TestGitSweepMergeAndBaseline: the baseline snapshot hides pre-existing dirty +// paths; live sweeps upsert only new ones; a failed sweep latches unavailable. +func TestGitSweepMergeAndBaseline(t *testing.T) { + m := model{} + m = m.handleGitSweepMsg(gitSweepMsg{baseline: true, ok: true, files: []gitSweepFile{{path: "dirty-before.go"}}}) + if !m.gitFileBaseline["dirty-before.go"] { + t.Fatal("baseline should record pre-existing dirty paths") + } + + m = m.handleGitSweepMsg(gitSweepMsg{ok: true, files: []gitSweepFile{ + {path: "dirty-before.go", adds: 9}, + {path: "kanban/board.tsx", created: true, adds: 120}, + }}) + if len(m.gitTouched) != 1 || m.gitTouched[0].path != "kanban/board.tsx" { + t.Fatalf("only newly dirty paths should merge: %+v", m.gitTouched) + } + + // Re-sweep updates stats in place, no duplicate. + m = m.handleGitSweepMsg(gitSweepMsg{ok: true, files: []gitSweepFile{{path: "kanban/board.tsx", created: true, adds: 150, dels: 2}}}) + if len(m.gitTouched) != 1 || m.gitTouched[0].adds != 150 { + t.Fatalf("re-sweep should upsert stats: %+v", m.gitTouched) + } + + failed := model{} + failed = failed.handleGitSweepMsg(gitSweepMsg{baseline: true, ok: false}) + if !failed.gitSweepUnavailable { + t.Fatal("a failed sweep should latch unavailable") + } + if _, cmd := failed.maybeGitSweep(); cmd != nil { + t.Fatal("no further sweeps once unavailable") + } +} + +// TestMaybeGitSweepGating: no sweep before the baseline answers, none while one +// is in flight, and the in-flight flag sets when one is issued. +func TestMaybeGitSweepGating(t *testing.T) { + m := model{cwd: "/tmp"} + if _, cmd := m.maybeGitSweep(); cmd != nil { + t.Fatal("no baseline yet: sweep must not run") + } + m.gitFileBaseline = map[string]bool{} + next, cmd := m.maybeGitSweep() + if cmd == nil || !next.gitSweepInFlight { + t.Fatal("with a baseline, a sweep should be issued and marked in flight") + } + if _, again := next.maybeGitSweep(); again != nil { + t.Fatal("single-flight: no second sweep while one runs") + } +} + +// TestTouchedFilesMergesGitSweep: git-discovered files append below the +// transcript-derived entries, deduped by path (a tool-result entry wins). +func TestTouchedFilesMergesGitSweep(t *testing.T) { + m := filesPanelTestModel() + m.gitTouched = []gitSweepFile{ + {path: "kanban/board.tsx", created: true, adds: 120}, + {path: "web/app.js", adds: 999}, // duplicate of a transcript entry + } + files := m.touchedFiles() + var kanban *touchedFile + appCount := 0 + for i := range files { + if files[i].path == "kanban/board.tsx" { + kanban = &files[i] + } + if files[i].path == "web/app.js" { + appCount++ + if files[i].adds == 999 { + t.Error("transcript-derived entry should win over the git duplicate") + } + } + } + if kanban == nil || !kanban.created || kanban.lastRowIndex != -1 { + t.Fatalf("git-only file should merge with created badge and no row anchor: %+v", files) + } + if appCount != 1 { + t.Fatalf("web/app.js should appear exactly once, got %d", appCount) + } +} + +// TestOpenFileViewGitOnlyFallsBackToFull: a file with no edit cards (git sweep +// discovery) opens straight on the full-file view instead of an empty diff. +func TestOpenFileViewGitOnlyFallsBackToFull(t *testing.T) { + m := filesPanelTestModel() + m.gitTouched = []gitSweepFile{{path: "kanban/board.tsx", created: true}} + m = m.openFileView("kanban/board.tsx") + if m.fileView.mode != fileViewFull { + t.Fatal("git-only file should open in full mode") + } + m = m.exitFileView() + m = m.openFileView("web/app.js") + if m.fileView.mode != fileViewDiff { + t.Fatal("a file with edit cards still opens in diff mode") + } +} + +// TestGitSweepCmdAgainstRealRepo: end-to-end against a real git repo — the +// baseline sees pre-existing dirt, a post-mutation sweep reports the new file +// as created and the modified file with its numstat. +func TestGitSweepCmdAgainstRealRepo(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + dir := t.TempDir() + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + write := func(name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + run("init", "-q") + write("tracked.txt", "one\ntwo\n") + run("add", ".") + run("commit", "-q", "-m", "seed") + write("pre-existing.txt", "dirt\n") // dirty BEFORE the TUI "opens" + + baseline := gitSweepCmd(nil, dir, true)().(gitSweepMsg) + if !baseline.ok || len(baseline.files) != 1 || baseline.files[0].path != "pre-existing.txt" { + t.Fatalf("baseline should see only the pre-existing dirt: %+v", baseline) + } + + // The "agent" now scaffolds via the shell. + write("scaffolded.txt", "hello\n") + write("tracked.txt", "one\ntwo\nthree\nfour\n") + + sweep := gitSweepCmd(nil, dir, false)().(gitSweepMsg) + if !sweep.ok { + t.Fatal("sweep failed") + } + byPath := map[string]gitSweepFile{} + for _, f := range sweep.files { + byPath[f.path] = f + } + if f, ok := byPath["scaffolded.txt"]; !ok || !f.created { + t.Fatalf("scaffolded file should report created: %+v", sweep.files) + } + if f := byPath["tracked.txt"]; f.adds != 2 || f.dels != 0 { + t.Fatalf("tracked.txt numstat = +%d −%d, want +2 −0", f.adds, f.dels) + } + + // Not-a-repo → ok=false (the sweep latches off). + if msg := gitSweepCmd(nil, t.TempDir(), false)().(gitSweepMsg); msg.ok { + t.Fatal("a non-repo should report ok=false") + } +} diff --git a/internal/tui/files_panel.go b/internal/tui/files_panel.go new file mode 100644 index 000000000..f25a1b05c --- /dev/null +++ b/internal/tui/files_panel.go @@ -0,0 +1,346 @@ +// files_panel.go renders the FILES section of the right context sidebar: the +// workspace files this session has touched, newest first, with an A/M badge and +// a +added/−removed diffstat per file, plus a pulsing row for the file whose +// write is streaming right now. Like the swarm roster (sidebar.go), the touched +// set is not separate model state — it is recovered on demand from the +// transcript's tool-result rows (their changedFiles), so it survives resume for +// free and can never drift from what the chat shows. +// +// Interaction (see handleTranscriptSelectionMouse): the first click on a row +// SELECTS the file — its edit cards tint in the chat and the transcript scrolls +// to the most recent one; a second click (or a click while the drill-in is +// already open) opens the file view (file_view.go). Esc clears the selection. +package tui + +import ( + "fmt" + "strings" + + tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/tools" +) + +// maxSidebarFiles caps the FILES rows so the section stays a glanceable set, +// not a scrolling log; older files collapse into a "+N more" trailer. +const maxSidebarFiles = 6 + +// touchedFile is one workspace file the session has mutated, aggregated across +// every tool-result row that touched it. +type touchedFile struct { + path string + adds int // total added lines across its diffs + dels int // total removed lines across its diffs + edits int // number of mutations + created bool // first touch created the file (write_file "Created …") + failed bool // the latest touch errored + lastRowIndex int // transcript index of the most recent result touching it +} + +// touchedFiles recovers the session's touched-file roster from the transcript, +// most recently touched first. Recomputed on demand (the value-receiver model +// can't persist a registry from View), mirroring swarmSpawnedAgents; the scan is +// a single pass over the transcript per render, same order as the sidebar's +// other sections. +func (m model) touchedFiles() []touchedFile { + var files []touchedFile + index := map[string]int{} + for i, row := range m.transcript { + if row.kind != rowToolResult || len(row.changedFiles) == 0 { + continue + } + adds, dels := planDiffStat(row.detail) + for _, path := range row.changedFiles { + if path == "" { + continue + } + at, seen := index[path] + if !seen { + index[path] = len(files) + files = append(files, touchedFile{ + path: path, + created: resultRowCreatedFile(row), + lastRowIndex: i, + }) + at = len(files) - 1 + } + files[at].adds += adds + files[at].dels += dels + files[at].edits++ + files[at].failed = row.status == tools.StatusError + files[at].lastRowIndex = i + } + } + // Most recently touched first: the file being worked on now belongs at the + // top, like the sidebar's ACTIVITY feed. + for left, right := 0, len(files)-1; left < right; left, right = left+1, right-1 { + files[left], files[right] = files[right], files[left] + } + // Merge the git sweep's discoveries (bash/subagent mutations that carry no + // changedFiles — files_git_sweep.go) below the transcript-derived entries, + // skipping paths a tool result already reported. + for _, f := range m.gitTouchedFiles() { + if _, seen := index[f.path]; seen { + continue + } + files = append(files, f) + } + return files +} + +// resultRowCreatedFile reports whether a tool-result row reads as a file +// creation ("tool result: write_file ok Created x (…)"), so the FILES row can +// badge it A rather than M. Heuristic on the confirmation text write_file +// already emits; a miss just shows M, never breaks anything. +func resultRowCreatedFile(row transcriptRow) bool { + return strings.Contains(row.text, " Created ") +} + +// liveEditingPath is the path of the file whose mutating tool call is streaming +// its arguments RIGHT NOW (the same live write streamingToolCallView previews), +// or "" when no mutation is in flight. It renders as a pulsing row pinned atop +// the FILES section. +func (m model) liveEditingPath() string { + if m.streamCallDecoder == nil { + return "" + } + switch m.streamCallName { + case "write_file", "edit_file", "apply_patch": + return m.streamCallDecoder.path + } + return "" +} + +// fileHit marks a rendered FILES row (by its ABSOLUTE index inside the rendered +// sidebar) that is clickable, carrying the file it refers to. +type fileHit struct { + lineOffset int + path string +} + +// sidebarFilesHeader renders the FILES section header with the touched count. +func (m model) sidebarFilesHeader(width int) string { + n := len(m.touchedFiles()) + if n == 0 { + return sidebarHeader("FILES", width) + } + return sidebarHeaderWithCount("FILES", fmt.Sprintf("%d", n), zeroTheme.muted, width) +} + +// sidebarFileLines renders the FILES section body: the live "writing" pulse row +// first, then the touched files newest-first, capped at maxSidebarFiles with a +// "+N more" trailer. Returns nil when the session has touched nothing (the +// caller shows a placeholder). The paired hits slice records each clickable +// row's index WITHIN the returned lines. +func (m model) sidebarFileLines(width int) ([]string, []fileHit) { + files := m.touchedFiles() + live := m.liveEditingPath() + if len(files) == 0 && live == "" { + return nil, nil + } + room := maxInt(4, width-3) + var lines []string + var hits []fileHit + + // The file being written this instant: an accent pulse, not yet clickable + // (its result row — the diff — doesn't exist until the call completes). + if live != "" { + lines = append(lines, " "+zeroTheme.accent.Render("●")+" "+ + zeroTheme.ink.Render(truncatePathLeft(live, room))) + } + + shown := 0 + for _, f := range files { + if f.path == live { + continue // already shown as the live row + } + if shown >= maxSidebarFiles { + lines = append(lines, " "+zeroTheme.faint.Render(fmt.Sprintf("+%d more", len(files)-shown))) + break + } + shown++ + hits = append(hits, fileHit{lineOffset: len(lines), path: f.path}) + lines = append(lines, m.renderFileRow(f, room)) + } + return lines, hits +} + +// renderFileRow renders one touched file: status badge, left-truncated path, +// and the +/− diffstat, with the selected file carrying an accent marker so the +// selection reads in the sidebar as well as in the chat tint. +func (m model) renderFileRow(f touchedFile, room int) string { + badge := zeroTheme.muted.Render("M") + switch { + case f.failed: + badge = zeroTheme.red.Render("✗") + case f.created: + badge = zeroTheme.green.Render("A") + } + // Reserve the diffstat's width so the path truncates around it. + stat := "" + if f.adds > 0 || f.dels > 0 { + stat = fmt.Sprintf(" +%d −%d", f.adds, f.dels) + } + pathRoom := maxInt(4, room-len(stat)) + pathStyle := zeroTheme.muted + lead := " " + if m.selectedFile == f.path { + lead = zeroTheme.accent.Render("▸") + pathStyle = zeroTheme.ink + } + line := lead + badge + " " + pathStyle.Render(truncatePathLeft(f.path, pathRoom)) + if stat != "" { + line += zeroTheme.faintest.Render(stat) + } + return line +} + +// sidebarFileSelectables returns the clickable FILES rows with their ABSOLUTE +// index inside the rendered sidebar. The FILES section renders after PLAN in +// renderContextSidebar: AGENTS header + body, blank + PLAN header + body, then +// blank + FILES header, then the file rows. The offset accounting mirrors +// sidebarPlanSelectables exactly, extended one section down. +func (m model) sidebarFileSelectables(width int) []fileHit { + lines, hits := m.sidebarFileLines(width) + if len(lines) == 0 { + return nil + } + agentBody := len(m.sidebarAgentLines(width)) + if agentBody == 0 { + agentBody = 1 // the "no agents spawned" placeholder occupies one line + } + planBody := len(m.sidebarPlanLines(width)) + if planBody == 0 { + planBody = 1 // the "no active plan" placeholder occupies one line + } + base := 1 + agentBody + 2 + planBody + 2 // sections above + (blank + FILES header) + for i := range hits { + hits[i].lineOffset += base + } + return hits +} + +// fileRowAtMouse maps a left-click in the context sidebar to a touched file, +// mirroring planStepAtMouse's column/x gate. Rows truncated away by the +// sidebar's height budget (or colliding with the token floor line) never match. +func (m model) fileRowAtMouse(msg tea.MouseMsg) (string, bool) { + if !m.sidebarActive() { + return "", false + } + if m.setup.visible || m.providerWizard != nil || m.mcpAddWizard != nil || m.mcpManager != nil || m.picker != nil || m.suggestionsActive() { + return "", false + } + sidebarW := sidebarWidth(m.width) + if sidebarW <= 0 { + return "", false + } + x0 := m.chatColumnWidth() + 3 // " │ " divider between the columns + x, y := mouseX(msg), mouseY(msg) + if x < x0 || x >= x0+sidebarW { + return "", false + } + for _, hit := range m.sidebarFileSelectables(sidebarW) { + if hit.lineOffset == y && hit.lineOffset < m.height-1 && hit.path != "" { + return hit.path, true + } + } + return "", false +} + +// rowTouchesSelectedFile reports whether a transcript row's mutation touched +// the currently selected file, so its card renders with the selection tint. +func (m model) rowTouchesSelectedFile(row transcriptRow) bool { + if m.selectedFile == "" || row.kind != rowToolResult { + return false + } + for _, path := range row.changedFiles { + if path == m.selectedFile { + return true + } + } + return false +} + +// lastRowIndexForFile returns the transcript index of the most recent +// tool-result row that touched path, or -1. +func (m model) lastRowIndexForFile(path string) int { + for i := len(m.transcript) - 1; i >= 0; i-- { + row := m.transcript[i] + if row.kind != rowToolResult { + continue + } + for _, p := range row.changedFiles { + if p == path { + return i + } + } + } + return -1 +} + +// selectFile marks path as the selected file and scrolls the transcript so its +// most recent edit card is in view; the card tint comes from the renderers +// reading selectedFile (rowTouchesSelectedFile). +func (m model) selectFile(path string) model { + m.selectedFile = path + if offset, ok := m.scrollOffsetForTranscriptRow(m.lastRowIndexForFile(path)); ok { + m.chatScrollOffset = offset + if offset == 0 { + m.chatBodyLines = 0 + } + } + return m +} + +// scrollOffsetForTranscriptRow computes the chatScrollOffset that places the +// given transcript row's first rendered line at the top of the viewport (with +// one line of breathing room), using the same layout metrics the scroll engine +// itself uses. ok is false outside alt-screen or when the row isn't rendered +// (e.g. skipped/collapsed). +func (m model) scrollOffsetForTranscriptRow(rowIndex int) (int, bool) { + if rowIndex < 0 || !m.altScreen || m.height <= 0 { + return 0, false + } + width := m.chatColumnWidth() + items := m.transcriptBodyItems(width, "") + metrics := measureTranscriptBodyItems(items, m.transcriptBodyHeights) + startY := -1 + for _, span := range metrics.spans { + if span.kind == transcriptBodyItemRow && span.rowIndex == rowIndex { + startY = span.startY + break + } + } + if startY < 0 { + return 0, false + } + frame := m.scrollableTranscriptFrame(m.pinnedTitleBar(width), m.footerView(width)) + viewport := transcriptViewportForLayout(metrics, frame, m.chatScrollOffset) + // The offset counts lines below the fold: window.start == total-height-offset, + // so placing startY at the window top (minus one context line) means + // offset = total - height - (startY - 1), clamped to the scroll range. + offset := metrics.totalLines() - frame.bodyRect.height - (startY - 1) + return clampInt(offset, 0, viewport.maxOffset()), true +} + +// truncatePathLeft fits a path into room cells by dropping leading components +// ("internal/tui/sidebar.go" → "…/tui/sidebar.go"), keeping the filename — the +// part that identifies the file — intact for as long as possible. +func truncatePathLeft(path string, room int) string { + if room <= 0 { + return "" + } + if len(path) <= room { + return path + } + const ellipsis = "…/" + parts := strings.Split(path, "/") + for start := 1; start < len(parts); start++ { + candidate := ellipsis + strings.Join(parts[start:], "/") + if len(candidate) <= room { + return candidate + } + } + // Even "…/name" overflows: fall back to a right-truncated basename. + return truncateRunes(ellipsis+parts[len(parts)-1], room) +} diff --git a/internal/tui/files_panel_test.go b/internal/tui/files_panel_test.go new file mode 100644 index 000000000..8c9b96e3f --- /dev/null +++ b/internal/tui/files_panel_test.go @@ -0,0 +1,232 @@ +package tui + +import ( + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/tools" +) + +// filesPanelTestModel is sidebarTestModel plus a couple of touched files: one +// created, one edited twice (the second touch most recent). +func filesPanelTestModel() model { + m := sidebarTestModel() + m.transcript = append(m.transcript, + transcriptRow{kind: rowToolResult, tool: "write_file", id: "f1", status: tools.StatusOK, + text: "tool result: write_file ok Created web/app.js (10 lines).", + detail: "+let a = 1\n+let b = 2", + changedFiles: []string{"web/app.js"}}, + transcriptRow{kind: rowToolResult, tool: "edit_file", id: "f2", status: tools.StatusOK, + text: "tool result: edit_file ok Edited internal/tui/sidebar.go.", + detail: "+added one\n-removed one", + changedFiles: []string{"internal/tui/sidebar.go"}}, + transcriptRow{kind: rowToolResult, tool: "edit_file", id: "f3", status: tools.StatusOK, + text: "tool result: edit_file ok Edited internal/tui/sidebar.go.", + detail: "+two\n+three\n-gone", + changedFiles: []string{"internal/tui/sidebar.go"}}, + ) + return m +} + +// TestTouchedFilesAggregates: the roster is recovered from tool-result rows, +// newest-touch first, with per-file diffstat totals, edit counts, and the +// created badge from write_file's confirmation text. +func TestTouchedFilesAggregates(t *testing.T) { + m := filesPanelTestModel() + files := m.touchedFiles() + if len(files) != 2 { + t.Fatalf("expected 2 touched files, got %d: %+v", len(files), files) + } + // sidebar.go was touched last, so it lists first. + if files[0].path != "internal/tui/sidebar.go" { + t.Fatalf("most recently touched file should list first, got %q", files[0].path) + } + if files[0].adds != 3 || files[0].dels != 2 || files[0].edits != 2 { + t.Errorf("sidebar.go diffstat = +%d −%d over %d edits, want +3 −2 over 2", files[0].adds, files[0].dels, files[0].edits) + } + if files[0].created { + t.Error("edited file must not read as created") + } + if !files[1].created { + t.Error("write_file 'Created …' result should mark the file created") + } + if files[0].lastRowIndex != len(m.transcript)-1 { + t.Errorf("lastRowIndex = %d, want the final transcript row", files[0].lastRowIndex) + } +} + +// TestSidebarFileLinesRenderAndOverflow: rows carry the badge + left-truncated +// path + diffstat; beyond maxSidebarFiles the tail collapses into "+N more"; +// hits index only real file rows. +func TestSidebarFileLinesRenderAndOverflow(t *testing.T) { + m := filesPanelTestModel() + lines, hits := m.sidebarFileLines(34) + joined := plainRender(t, strings.Join(lines, "\n")) + if !strings.Contains(joined, "sidebar.go") || !strings.Contains(joined, "app.js") { + t.Fatalf("file rows missing paths:\n%s", joined) + } + if !strings.Contains(joined, "+3 −2") { + t.Errorf("expected aggregated diffstat +3 −2:\n%s", joined) + } + if !strings.Contains(joined, "A") || !strings.Contains(joined, "M") { + t.Errorf("expected A and M badges:\n%s", joined) + } + if len(hits) != 2 { + t.Fatalf("expected 2 clickable hits, got %d", len(hits)) + } + if hits[0].path != "internal/tui/sidebar.go" { + t.Errorf("first hit should be the newest file, got %q", hits[0].path) + } + + // Overflow: 8 distinct files -> maxSidebarFiles rows + a "+2 more" trailer. + over := sidebarTestModel() + for _, name := range []string{"a", "b", "c", "d", "e", "f", "g", "h"} { + over.transcript = append(over.transcript, transcriptRow{ + kind: rowToolResult, tool: "edit_file", id: name, status: tools.StatusOK, + changedFiles: []string{name + ".go"}, + }) + } + overLines, overHits := over.sidebarFileLines(34) + if len(overHits) != maxSidebarFiles { + t.Fatalf("expected %d clickable rows, got %d", maxSidebarFiles, len(overHits)) + } + if got := plainRender(t, overLines[len(overLines)-1]); !strings.Contains(got, "+2 more") { + t.Errorf("expected '+2 more' trailer, got %q", got) + } +} + +// TestSidebarFilesLivePulseRow: while a mutating tool call streams its args, +// the file it is writing shows as a pinned (unclickable) pulse row. +func TestSidebarFilesLivePulseRow(t *testing.T) { + m := filesPanelTestModel() + m.streamCallName = "write_file" + m.streamCallDecoder = newStreamingDecoder() + m.streamCallDecoder.feed(`{"path":"web/new.css","content":"body{}`) + + lines, hits := m.sidebarFileLines(34) + if got := plainRender(t, lines[0]); !strings.Contains(got, "web/new.css") { + t.Fatalf("live write should pin its path atop FILES, got %q", got) + } + for _, hit := range hits { + if hit.path == "web/new.css" { + t.Error("the live row must not be clickable (no result exists yet)") + } + } + // A non-mutating streaming call shows no pulse row. + m.streamCallName = "read_file" + if m.liveEditingPath() != "" { + t.Error("read_file must not read as a live edit") + } +} + +// TestSidebarFileSelectablesMatchRenderedRows: the hit offsets computed by +// sidebarFileSelectables must land exactly on the rendered FILES rows in +// renderContextSidebar's output — the same invariant the mouse hit-test relies +// on. Asserted against the real render, not a re-derivation of the math. +func TestSidebarFileSelectablesMatchRenderedRows(t *testing.T) { + m := filesPanelTestModel() + width := sidebarWidth(m.width) + rendered := m.renderContextSidebar(width, m.height) + hits := m.sidebarFileSelectables(width) + if len(hits) == 0 { + t.Fatal("expected clickable FILES rows") + } + for _, hit := range hits { + if hit.lineOffset >= len(rendered) { + t.Fatalf("hit offset %d beyond rendered sidebar (%d lines)", hit.lineOffset, len(rendered)) + } + line := plainRender(t, rendered[hit.lineOffset]) + base := hit.path[strings.LastIndex(hit.path, "/")+1:] + if !strings.Contains(line, base) { + t.Errorf("hit for %q points at line %d which reads %q", hit.path, hit.lineOffset, line) + } + } +} + +// TestFileRowClickSelectsThenOpens: the first sidebar click on a file selects +// it (tint + scroll state, no view swap); the second click on the same file +// opens the drill-in; clicking another file while the view is open switches it. +func TestFileRowClickSelectsThenOpens(t *testing.T) { + m := filesPanelTestModel() + width := sidebarWidth(m.width) + hits := m.sidebarFileSelectables(width) + if len(hits) < 2 { + t.Fatal("expected two clickable FILES rows") + } + x := m.chatColumnWidth() + 3 + click := func(hit fileHit) tea.MouseMsg { return testMouseClick(tea.MouseLeft, x, hit.lineOffset) } + + m1, _, handled := m.handleTranscriptSelectionMouse(click(hits[0])) + if !handled { + t.Fatal("FILES row click should be handled") + } + if m1.selectedFile != hits[0].path || m1.fileView.active { + t.Fatalf("first click should select (not open): selected=%q active=%v", m1.selectedFile, m1.fileView.active) + } + + m2, _, _ := m1.handleTranscriptSelectionMouse(click(hits[0])) + if !m2.fileView.active || m2.fileView.path != hits[0].path { + t.Fatalf("second click should open the file view: %+v", m2.fileView) + } + + m3, _, _ := m2.handleTranscriptSelectionMouse(click(hits[1])) + if !m3.fileView.active || m3.fileView.path != hits[1].path { + t.Fatalf("clicking another file with the view open should switch it: %+v", m3.fileView) + } + if m3.selectedFile != hits[1].path { + t.Errorf("switching the view should move the selection too, got %q", m3.selectedFile) + } +} + +// TestRowTouchesSelectedFileTint: only tool-result rows whose changedFiles +// carry the selected path tint, and the render cache key differs across +// selection states so the tint can't be served stale. +func TestRowTouchesSelectedFileTint(t *testing.T) { + m := filesPanelTestModel() + row := m.transcript[len(m.transcript)-1] + if m.rowTouchesSelectedFile(row) { + t.Fatal("nothing selected: no row should tint") + } + m.selectedFile = "internal/tui/sidebar.go" + if !m.rowTouchesSelectedFile(row) { + t.Fatal("selected file's edit row should tint") + } + other := m.transcript[len(m.transcript)-3] // web/app.js + if m.rowTouchesSelectedFile(other) { + t.Fatal("an unrelated file's row must not tint") + } + + rc := buildRowContext(m.transcript) + opts := cardRenderOptions{bodyCap: cardBodyMaxLines, cwd: m.cwd} + selectedKey, _ := m.renderRowCacheKey(row, 80, rc, opts, false) + m.selectedFile = "" + plainKey, _ := m.renderRowCacheKey(row, 80, rc, opts, false) + if selectedKey == plainKey { + t.Error("render cache key must change with the selection state") + } +} + +// TestEscClearsFileSelectionThenNothing: Esc drops an active file selection +// (before any run-level action) and is consumed doing so. +func TestEscClearsFileSelectionThenNothing(t *testing.T) { + m := filesPanelTestModel() + m.selectedFile = "web/app.js" + updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEsc}) + if got := updated.(model).selectedFile; got != "" { + t.Fatalf("Esc should clear the file selection, still %q", got) + } +} + +func TestTruncatePathLeft(t *testing.T) { + if got := truncatePathLeft("internal/tui/sidebar.go", 40); got != "internal/tui/sidebar.go" { + t.Errorf("short path should pass through, got %q", got) + } + if got := truncatePathLeft("internal/tui/sidebar.go", 18); got != "…/tui/sidebar.go" { + t.Errorf("want …/tui/sidebar.go, got %q", got) + } + if got := truncatePathLeft("internal/tui/sidebar.go", 14); got != "…/sidebar.go" { + t.Errorf("want …/sidebar.go, got %q", got) + } +} diff --git a/internal/tui/hover.go b/internal/tui/hover.go index ae142a4de..5ec530e1a 100644 --- a/internal/tui/hover.go +++ b/internal/tui/hover.go @@ -15,6 +15,9 @@ const ( // hoverPlanStep: a plan step row in the sidebar's PLAN section, identified by // step index. hoverPlanStep + // hoverFileRow: a touched-file row in the sidebar's FILES section, identified + // by path. + hoverFileRow ) // hoverTarget identifies the single clickable row (if any) currently under the @@ -34,6 +37,7 @@ type hoverTarget struct { bodyY int // hoverTranscript sessionID string // hoverSidebarAgent stepIndex int // hoverPlanStep + filePath string // hoverFileRow } // mouseHover reports whether msg is a plain cursor-movement event with NO button @@ -59,6 +63,9 @@ func (m model) updateHoverTarget(msg tea.MouseMsg) model { if stepIndex, ok := m.planStepAtMouse(msg); ok { return m.withHover(hoverTarget{kind: hoverPlanStep, stepIndex: stepIndex}) } + if path, ok := m.fileRowAtMouse(msg); ok { + return m.withHover(hoverTarget{kind: hoverFileRow, filePath: path}) + } if line, ok := m.transcriptLineAtMouse(msg); ok { // A permission option reuses its OWN existing keyboard-cursor highlight // (see hoverPermissionOption) rather than the m.hover mechanism, so there's diff --git a/internal/tui/model.go b/internal/tui/model.go index e55c43a4b..e386d841f 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -211,6 +211,22 @@ type model struct { // sidebar; when set, the chat reflows to full width. Distinct from the // availability conditions in sidebarAvailable (geometry / mode / overlays). sidebarHidden bool + // selectedFile is the touched file selected by clicking its FILES sidebar + // row: its edit cards tint in the chat (rowTouchesSelectedFile) and a second + // click opens the drill-in file view. "" when nothing is selected; Esc clears. + selectedFile string + // fileView is the drill-in view for a touched file (file_view.go): while + // active the chat column's body shows the file's diff/content instead of the + // transcript, mirroring the subchat drill-in. + fileView fileViewState + // Git-sweep state (files_git_sweep.go): the startup snapshot of already-dirty + // paths (nil until Init's sweep answers), the newly dirty files discovered by + // live sweeps (bash/subagent mutations that carry no changedFiles), the + // single-flight guard, and the "not a git repo / no git" latch. + gitFileBaseline map[string]bool + gitTouched []gitSweepFile + gitSweepInFlight bool + gitSweepUnavailable bool // swarmDoneAt records when each swarm member was first seen finished (done/ // failed) in a swarm_status report, so the sidebar can linger it briefly with a // fading ✓ before dropping it (a smooth exit, not an abrupt pop). Stamped in the @@ -803,6 +819,12 @@ func (m model) Init() tea.Cmd { // terminal's unprompted push — mirrors the RequestBackgroundColor request // below for the same reason. cmds = append(cmds, tea.RequestWindowSize) + // Baseline git snapshot for the FILES sidebar sweep: whatever is already + // dirty when the TUI opens is pre-existing state, not this session's work + // (files_git_sweep.go). Async; a non-git workspace just disables the sweep. + if strings.TrimSpace(m.cwd) != "" { + cmds = append(cmds, gitSweepCmd(m.ctx, m.cwd, true)) + } // In auto mode, ask the terminal for its background color; the reply arrives // as tea.BackgroundColorMsg and selects light vs dark (see updateModel). if m.themeMode == themeAuto { @@ -1058,6 +1080,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m.handleCtrlC() case keyCtrl(msg, 'o'): return m.toggleDetailedTranscript(), nil + case m.fileView.active && m.composerValue() == "" && (keyText(msg) == "d" || keyText(msg) == "f"): + // Mode toggle for the file drill-in, only while the composer is empty + // so mid-sentence typing is never hijacked. + if keyText(msg) == "f" { + return m.setFileViewMode(fileViewFull), nil + } + return m.setFileViewMode(fileViewDiff), nil case keyCtrl(msg, 'e'): // Release/recapture the mouse so the user can drag-select and copy text // natively (mouse capture otherwise intercepts terminal selection). @@ -1082,6 +1111,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m = m.clearHover() // bodyY numbering differs between subchat and the parent transcript return m, nil } + // File drill-in exits on Esc (returns to the chat at its saved scroll + // position); the file stays selected so a second Esc clears that. + if m.fileView.active { + return m.exitFileView(), nil + } if m.mcpCommandCancel != nil { m.cancelMCPCommand() if m.mcpAddWizard != nil { @@ -1138,6 +1172,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.suggestionsActive() { return m.dismissSuggestions(), nil } + // A selected FILES row clears before anything run-related: the + // selection is a passive highlight, so Esc dropping it is cheap and + // expected (mirrors how editors clear selection on Esc). + if m.selectedFile != "" { + m.selectedFile = "" + return m, nil + } if m.hasQueuedMessage() { return m.clearQueuedMessage(), nil } @@ -1812,8 +1853,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } m, recapCmd = m.maybeRecapTurn(msg.runID, finalAnswer) } + // End-of-turn git sweep: catch file mutations the tool stream couldn't + // report (bash scaffolding, subagent edits) so the FILES sidebar is + // complete once the turn settles. + var sweepCmd tea.Cmd + m, sweepCmd = m.maybeGitSweep() next, queuedCmd := m.launchQueuedMessageIfReady() - return next, tea.Batch(titleCmd, recapCmd, queuedCmd) + return next, tea.Batch(titleCmd, recapCmd, sweepCmd, queuedCmd) case sessionTitleGeneratedMsg: return m.handleSessionTitleGenerated(msg) case recapGeneratedMsg: @@ -1964,6 +2010,14 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.transcript = collapseRepeatedStatusCard(m.transcript, msg.row) m.transcript = appendTranscriptRow(m.transcript, msg.row) m = m.captureStepWork(msg.row) + // A finished command tool may have mutated files git can see but no + // changedFiles reports (npm create, heredoc writes, subagent edits) — + // re-sweep so the FILES sidebar picks them up mid-turn. + if msg.row.kind == rowToolResult && isPlanCommandTool(msg.row.tool) { + var sweep tea.Cmd + m, sweep = m.maybeGitSweep() + return m, sweep + } return m, nil case swarmSessionsMsg: // Merge completed swarm members' session ids so their AGENTS sidebar rows @@ -1994,6 +2048,8 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { case prStateMsg: m.prState = msg.state return m, nil + case gitSweepMsg: + return m.handleGitSweepMsg(msg), nil case prWatcherStartedMsg: if msg.stop == nil { return m, nil @@ -2198,6 +2254,12 @@ func (m model) pinnedTitleBar(width int) string { if !m.altScreen || m.height <= 0 { return "" } + // The file drill-in replaces the title bar with its nav line (path + key + // hints). Both are exactly one line, and every frame computation routes + // through here, so the swap never desyncs the viewport geometry. + if m.fileView.active { + return m.fileViewNavBar(width) + } return m.titleBar(width) } @@ -4298,13 +4360,14 @@ func (m model) runAgentWithOptions(runID int, runCtx context.Context, prompt str } } row := transcriptRow{ - kind: rowToolResult, - id: effectiveToolRowID(result.ToolCallID, callSeq[result.ToolCallID]), - text: toolResultRowText(result), - tool: result.Name, - status: result.Status, - detail: toolResultDetail(result), - runID: runID, + kind: rowToolResult, + id: effectiveToolRowID(result.ToolCallID, callSeq[result.ToolCallID]), + text: toolResultRowText(result), + tool: result.Name, + status: result.Status, + detail: toolResultDetail(result), + runID: runID, + changedFiles: result.ChangedFiles, } // A Task result is shown by the specialist card, and update_plan by the // plan panel/sidebar, so skip both redundant transcript rows. diff --git a/internal/tui/render_cache.go b/internal/tui/render_cache.go index 18885d79a..0c8937392 100644 --- a/internal/tui/render_cache.go +++ b/internal/tui/render_cache.go @@ -164,6 +164,9 @@ func (m model) renderRowCacheKey(row transcriptRow, width int, rc rowContext, op appendRenderCacheField(&b, strconv.FormatBool(row.final)) appendRenderCacheField(&b, strconv.Itoa(row.turnTools)) appendRenderCacheField(&b, strconv.FormatInt(int64(row.turnElapsed), 10)) + // The FILES selection tints this row's card border, so selecting/deselecting + // a file must miss the cache entry rendered under the other state. + appendRenderCacheField(&b, strconv.FormatBool(m.rowTouchesSelectedFile(row))) key := rcKey(row.runID, row.id) appendRenderCacheField(&b, rc.hints[key]) diff --git a/internal/tui/rendering.go b/internal/tui/rendering.go index 2a9e37f9f..48e34da7c 100644 --- a/internal/tui/rendering.go +++ b/internal/tui/rendering.go @@ -167,6 +167,10 @@ type cardRenderOptions struct { bodyCap int cwd string expanded bool + // fileSelected marks a tool-result card whose mutation touched the file + // selected in the FILES sidebar; the card border tints accent so the + // selection reads in the transcript. + fileSelected bool } // flushCardBodyMaxLines is the body cap for cards flushed to scrollback. The @@ -210,6 +214,10 @@ func (m model) renderRowMode(row transcriptRow, width int, rc rowContext, flush } func (m model) renderRowModeUncached(row transcriptRow, width int, rc rowContext, opts cardRenderOptions) string { + // Resolved per-row (not at the opts construction sites) so every render + // path — live, flush, detailed — carries the FILES selection tint; the + // cache key includes the same predicate (renderRowCacheKey). + opts.fileSelected = m.rowTouchesSelectedFile(row) switch row.kind { case rowUser: return renderUserRow(row, width) @@ -1390,6 +1398,11 @@ func renderToolResultCard(row transcriptRow, width int, rc rowContext, opts card glyph := zeroTheme.green.Render("•") nameStyle := zeroTheme.green borderStyle := zeroTheme.line + if opts.fileSelected { + // The selected FILES row's edit card: accent border, same as the + // sidebar's ▸ marker, so click → highlight reads as one gesture. + borderStyle = zeroTheme.accent + } if failed { glyph = zeroTheme.red.Render("•") nameStyle = zeroTheme.red diff --git a/internal/tui/session.go b/internal/tui/session.go index bd047a79f..4332f7600 100644 --- a/internal/tui/session.go +++ b/internal/tui/session.go @@ -444,12 +444,13 @@ func transcriptRowsFromSessionEvents(events []sessions.Event) []transcriptRow { } output := payloadString(payload, "output") rows = append(rows, transcriptRow{ - kind: rowToolResult, - id: effectiveToolRowID(id, callSeq[id]), - text: fmt.Sprintf("tool result: %s %s %s", name, status, truncateTUIOutput(output, tuiToolOutputLimit)), - tool: name, - status: status, - detail: output, + kind: rowToolResult, + id: effectiveToolRowID(id, callSeq[id]), + text: fmt.Sprintf("tool result: %s %s %s", name, status, truncateTUIOutput(output, tuiToolOutputLimit)), + tool: name, + status: status, + detail: output, + changedFiles: payloadStringSlice(payload, "changedFiles"), }) case sessions.EventError: if message := payloadString(payload, "message"); message != "" { @@ -619,6 +620,28 @@ func payloadString(payload map[string]any, key string) string { } } +// payloadStringSlice reads a []string persisted into a session payload (JSON +// round-trips it as []any), skipping non-string entries. Nil when absent. +func payloadStringSlice(payload map[string]any, key string) []string { + switch typed := payload[key].(type) { + case []string: + return typed + case []any: + out := make([]string, 0, len(typed)) + for _, v := range typed { + if s, ok := v.(string); ok && s != "" { + out = append(out, s) + } + } + if len(out) == 0 { + return nil + } + return out + default: + return nil + } +} + func payloadBool(payload map[string]any, key string) bool { value := payload[key] switch typed := value.(type) { diff --git a/internal/tui/sidebar.go b/internal/tui/sidebar.go index 9755dd156..a38ca4239 100644 --- a/internal/tui/sidebar.go +++ b/internal/tui/sidebar.go @@ -97,6 +97,9 @@ func (m model) sidebarHasContent() bool { if len(m.sidebarSpecialists()) > 0 || len(m.swarmSpawnedAgents()) > 0 { return true } + if len(m.touchedFiles()) > 0 { + return true + } return !m.plan.isEmpty() } @@ -587,6 +590,19 @@ func (m model) renderContextSidebar(width, height int) []string { lines = append(lines, planLines...) } + // FILES section: the files this session has touched (files_panel.go). + // Rendered BELOW the plan steps so it never shifts sidebarPlanSelectables' + // click offsets; its own hits (sidebarFileSelectables) account for the + // sections above it. + add("") + add(m.sidebarFilesHeader(width)) + fileLines, _ := m.sidebarFileLines(width) + if len(fileLines) == 0 { + add(sidebarPlaceholder("no files touched", width)) + } else { + lines = append(lines, fileLines...) + } + // ACTIVITY section: recent completed work + a live "generating…" pulse. Shown // BELOW the plan steps so it never shifts sidebarPlanSelectables' click offsets, // and budgeted (height-1 minus what's used) so it clips ITSELF from the bottom @@ -651,6 +667,12 @@ func (m model) hoveredSidebarLineOffset(width int) (int, bool) { return hit.lineOffset, true } } + case hoverFileRow: + for _, hit := range m.sidebarFileSelectables(width) { + if hit.path == m.hover.filePath { + return hit.lineOffset, true + } + } } return 0, false } diff --git a/internal/tui/transcript.go b/internal/tui/transcript.go index 0dd3505ba..431af311a 100644 --- a/internal/tui/transcript.go +++ b/internal/tui/transcript.go @@ -41,6 +41,11 @@ type transcriptRow struct { askUser *agent.AskUserRequest expanded bool // collapsible transcript rows, e.g. provider thoughts + // changedFiles lists the workspace-relative paths a mutating tool result + // wrote (from tools.Result.ChangedFiles; restored from the session payload on + // resume). The sidebar FILES section derives its roster from these. + changedFiles []string + // specialistInfo holds the specialist card data for rowSpecialist rows. // Nil for all other row kinds. specialistInfo *specialistInfo diff --git a/internal/tui/transcript_selection.go b/internal/tui/transcript_selection.go index 35e5f4aa8..f858da4a4 100644 --- a/internal/tui/transcript_selection.go +++ b/internal/tui/transcript_selection.go @@ -211,6 +211,12 @@ func (m model) renderHoverHighlight(rendered string, selectable []transcriptSele } func (m model) transcriptBodyItems(width int, emptyOverlay string) []transcriptBodyItem { + // File drill-in: the chat column's body swaps to the viewed file's + // diff/content. Swapping HERE (the single source every consumer reads) keeps + // the viewport, scroll engine, renderer, and mouse hit-tests consistent. + if m.fileView.active { + return m.fileViewBodyItems(width) + } items := []transcriptBodyItem{} // Transcript ROWS render at the full chat width; row/status glyphs provide // structure without adding another body margin. Block items (title bar, empty @@ -1242,6 +1248,9 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd, // session, reusing the specialist-card subchat path. Checked before the // transcript hit-test since the sidebar is outside the chat column. if hit, ok := m.sidebarLineAtMouse(msg); ok { + // The subchat drill-in owns the whole (single-column) view; a file + // drill-in can't meaningfully stay open behind it. + m = m.exitFileView() if errMsg := m.subchat.enter(m.sessionStore, hit.sessionID, hit.title, m.chatScrollOffset); errMsg != "" { m = m.appendSystemNotice(errMsg) } @@ -1252,10 +1261,24 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd, // A click on a PLAN step row drops a transcript card listing the file // changes captured while that step was in progress. if stepIndex, ok := m.planStepAtMouse(msg); ok { + // The card lands in the chat transcript; close the file drill-in so + // it isn't appended invisibly behind the swapped body. + m = m.exitFileView() var cmd tea.Cmd m, cmd = m.openPlanStepDetail(stepIndex) return m, cmd, true } + // A click on a FILES row: first click selects the file (its edit cards + // tint and the chat scrolls to the most recent one); a click on the + // already-selected file — or any FILES click while the drill-in is open — + // opens/switches the file view. + if path, ok := m.fileRowAtMouse(msg); ok { + if m.fileView.active || m.selectedFile == path { + m.selectedFile = path + return m.openFileView(path), nil, true + } + return m.selectFile(path), nil, true + } line, ok := m.transcriptLineAtMouse(msg) if !ok { if m.transcriptSelection.active { From b8d23f1ce28583125bca6a260f1e57506c5c2ca3 Mon Sep 17 00:00:00 2001 From: Gnanam Date: Thu, 2 Jul 2026 12:03:27 +0530 Subject: [PATCH 2/2] fix(tui): address FILES sidebar review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes for the FILES panel / drill-in / git sweep: - touchedFiles: split multi-file diffs per file (perFileDiffStats) so an apply_patch spanning several files no longer charges the whole patch's +/- totals to every file it touched; order by lastRowIndex descending so a re-touched file lists first (reversing first-seen order kept it in its original slot). - sidebarFileLines: filter the live-writing row out before computing the "+N more" trailer so the skipped entry no longer inflates the count. - git sweep: run `git diff --numstat -z` and parse the NUL-terminated rename records (counts + preimage + postimage) — plain --numstat brace-mangles renamed paths ("src/{old => new}"), so their diffstat never matched a porcelain path and renamed files stayed at 0/0; gitTouchedFiles now yields newest detection first to match the roster's ordering. - file view: stream the full-mode read and stop at fileViewMaxLines (os.ReadFile loaded a multi-GB file wholesale before truncating — the exact freeze the cap exists to prevent); re-opening the file already being viewed is a no-op so a stray re-click can't bounce full mode back to diff or reset scroll; skip sub-4-char lines in the changed-line markers so common braces/returns don't mark unrelated occurrences. - keys: gate the drill-in's d/f and Esc handling on noBlockingModal so a permission prompt / ask-user / wizard keeps its own key handling. - sidebar: a live in-flight write counts as content, so the FILES pulse shows for the session's first mutation. - tests: numstat test now feeds real -z record shapes and asserts the rename postimage key; new coverage for per-file stats, re-touch ordering, live-row overflow, same-file reopen, truncation, modal gating, live-write content, and a real-repo rename sweep. --- internal/tui/file_view.go | 47 +++++++++++---- internal/tui/file_view_test.go | 75 +++++++++++++++++++++++ internal/tui/files_git_sweep.go | 48 ++++++++++----- internal/tui/files_git_sweep_test.go | 44 +++++++++++++- internal/tui/files_panel.go | 90 ++++++++++++++++++++++++---- internal/tui/files_panel_test.go | 85 ++++++++++++++++++++++++++ internal/tui/model.go | 12 ++-- internal/tui/sidebar.go | 4 +- 8 files changed, 365 insertions(+), 40 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 194554e3b..7e544d70b 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -14,6 +14,7 @@ package tui import ( + "bufio" "fmt" "os" "path/filepath" @@ -43,7 +44,13 @@ type fileViewState struct { // openFileView activates the drill-in for path in diff mode. Opening from an // already-open view (clicking another FILES row) keeps the original saved chat // scroll position rather than saving the file view's own offset as "parent". +// Re-opening the file that is ALREADY being viewed is a no-op: a stray +// re-click must not bounce the user from full mode back to diff or reset +// their scroll position. func (m model) openFileView(path string) model { + if m.fileView.active && m.fileView.path == path { + return m + } if !m.fileView.active { m.fileView.parentScrollOffset = m.chatScrollOffset } @@ -163,15 +170,30 @@ func (m model) renderFileViewFull(width int) string { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - data, err := os.ReadFile(target) + // Stream the read and stop at the cap: os.ReadFile would load a multi-GB + // file wholesale before any truncation, which is the exact render freeze + // fileViewMaxLines exists to prevent. + file, err := os.Open(target) if err != nil { return zeroTheme.faint.Render("Could not read file: " + err.Error()) } - lines := strings.Split(strings.TrimSuffix(string(data), "\n"), "\n") - truncated := 0 - if len(lines) > fileViewMaxLines { - truncated = len(lines) - fileViewMaxLines - lines = lines[:fileViewMaxLines] + defer file.Close() + var lines []string + truncated := false + scanner := bufio.NewScanner(file) + scanner.Buffer(make([]byte, 0, 64*1024), 16*1024*1024) + for scanner.Scan() { + if len(lines) == fileViewMaxLines { + truncated = true + break + } + lines = append(lines, scanner.Text()) + } + if err := scanner.Err(); err != nil { + if len(lines) == 0 { + return zeroTheme.faint.Render("Could not read file: " + err.Error()) + } + truncated = true // e.g. a single over-long line mid-file: show what we have } changed := m.fileViewChangedLines() @@ -199,15 +221,20 @@ func (m model) renderFileViewFull(width int) string { b.WriteString(marker) b.WriteString(line) } - if truncated > 0 { + if truncated { + // No exact remaining-line count: computing one would require reading the + // rest of the file, defeating the bounded read above. b.WriteString("\n") - b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… %d more lines (file truncated for display)", truncated))) + b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) } return b.String() } // fileViewChangedLines collects the trimmed text of every line the session's -// diffs ADDED to the viewed file, for the full-mode gutter markers. +// diffs ADDED to the viewed file, for the full-mode gutter markers. Very short +// lines ("}", "return", ")") are skipped: text-matching them would mark every +// unrelated occurrence across the file, which misleads far more than a missing +// marker on a brace line ever could. func (m model) fileViewChangedLines() map[string]bool { changed := map[string]bool{} for _, row := range m.fileViewResultRows() { @@ -215,7 +242,7 @@ func (m model) fileViewChangedLines() map[string]bool { if !strings.HasPrefix(line, "+") || strings.HasPrefix(line, "+++") { continue } - if text := strings.TrimSpace(strings.TrimPrefix(line, "+")); text != "" { + if text := strings.TrimSpace(strings.TrimPrefix(line, "+")); len(text) >= 4 { changed[text] = true } } diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index 9136346ec..ebc62adb8 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -2,6 +2,7 @@ package tui import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -9,6 +10,7 @@ import ( tea "charm.land/bubbletea/v2" + "github.com/Gitlawb/zero/internal/agent" "github.com/Gitlawb/zero/internal/sessions" "github.com/Gitlawb/zero/internal/tools" ) @@ -211,3 +213,76 @@ func TestChangedFilesRehydration(t *testing.T) { t.Fatalf("changedFiles not rehydrated: %v", got) } } + +// TestOpenFileViewSamePathIsNoOp: re-clicking the FILES row of the file already +// being viewed must not clobber the user's mode or scroll — the old +// unconditional openFileView bounced full mode back to diff and reset scroll. +func TestOpenFileViewSamePathIsNoOp(t *testing.T) { + m := filesPanelTestModel() + m = m.openFileView("web/app.js") + m = m.setFileViewMode(fileViewFull) + m.chatScrollOffset = 7 // scrolled within the file body + + m = m.openFileView("web/app.js") + if m.fileView.mode != fileViewFull { + t.Fatal("re-opening the same file must keep full mode") + } + if m.chatScrollOffset != 7 { + t.Fatalf("re-opening the same file must keep the scroll, got %d", m.chatScrollOffset) + } + // A DIFFERENT file still switches (and resets to diff mode as documented). + m = m.openFileView("internal/tui/sidebar.go") + if m.fileView.path != "internal/tui/sidebar.go" || m.fileView.mode != fileViewDiff { + t.Fatalf("opening another file should switch views: %+v", m.fileView) + } +} + +// TestFileViewFullBodyTruncatesLongFile: the full view stops reading at +// fileViewMaxLines (bounded read — a giant file must not be loaded wholesale) +// and appends the truncation trailer. +func TestFileViewFullBodyTruncatesLongFile(t *testing.T) { + dir := t.TempDir() + var b strings.Builder + for i := 0; i < fileViewMaxLines+50; i++ { + fmt.Fprintf(&b, "line %d\n", i) + } + if err := os.WriteFile(filepath.Join(dir, "big.txt"), []byte(b.String()), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.gitTouched = []gitSweepFile{{path: "big.txt"}} + m = m.openFileView("big.txt") + + plain := plainRender(t, m.renderFileViewFull(80)) + lines := strings.Split(plain, "\n") + if len(lines) != fileViewMaxLines+1 { // capped content + the trailer line + t.Fatalf("rendered %d lines, want %d content lines + 1 trailer", len(lines), fileViewMaxLines) + } + if !strings.Contains(lines[len(lines)-1], "truncated") { + t.Fatalf("expected the truncation trailer, got %q", lines[len(lines)-1]) + } +} + +// TestFileViewKeysDeferToBlockingModal: with a permission prompt up, Esc and +// the d/f shortcuts belong to the prompt — the drill-in must not swallow them +// (Esc exiting the view instead of reaching the prompt's deny handling). +func TestFileViewKeysDeferToBlockingModal(t *testing.T) { + m := filesPanelTestModel() + m = m.openFileView("web/app.js") + m.pendingPermission = &pendingPermissionPrompt{ + request: agent.PermissionRequest{ToolName: "write_file"}, + decide: func(agent.PermissionDecision) {}, + } + + updated, _ := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + m = updated.(model) + if m.fileView.mode != fileViewDiff { + t.Fatal("f with a permission prompt up must not switch file-view modes") + } + updated, _ = m.Update(tea.KeyPressMsg{Code: tea.KeyEsc}) + m = updated.(model) + if !m.fileView.active { + t.Fatal("Esc with a permission prompt up must not exit the file view") + } +} diff --git a/internal/tui/files_git_sweep.go b/internal/tui/files_git_sweep.go index 14385a477..8c6678dfb 100644 --- a/internal/tui/files_git_sweep.go +++ b/internal/tui/files_git_sweep.go @@ -60,9 +60,13 @@ func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd { } files := parseGitPorcelain(status) // Diffstat for tracked modifications; untracked files have no diff to - // stat. Best-effort: a failure (e.g. unborn HEAD in a fresh repo) keeps - // the file list with zero counts rather than dropping the sweep. - if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat"); err == nil { + // stat. -z makes the output machine-parseable: NUL-terminated records, no + // path quoting, and renames as explicit preimage/postimage fields instead + // of the brace-mangled "src/{old => new}" a plain --numstat emits (which + // would never match a porcelain path). Best-effort: a failure (e.g. unborn + // HEAD in a fresh repo) keeps the file list with zero counts rather than + // dropping the sweep. + if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat", "-z"); err == nil { stats := parseGitNumstat(numstat) for i := range files { if counts, ok := stats[files[i].path]; ok { @@ -118,12 +122,18 @@ func unquoteGitPath(path string) string { return path } -// parseGitNumstat parses `git diff --numstat` output ("added\tdeleted\tpath") -// into path → [added, deleted]. Binary files report "-" and are skipped. +// parseGitNumstat parses `git diff --numstat -z` output into path → [added, +// deleted]. -z records are NUL-terminated "added\tdeleted\tpath"; a rename/copy +// instead ends the counts record with an EMPTY path ("added\tdeleted\t") and +// appends two more NUL-terminated fields, preimage then postimage — the stats +// key is the postimage (the path porcelain/changedFiles report). Paths are +// never quoted or brace-mangled in -z mode. Binary files report "-" counts and +// are skipped. func parseGitNumstat(out string) map[string][2]int { stats := map[string][2]int{} - for _, line := range strings.Split(out, "\n") { - parts := strings.SplitN(line, "\t", 3) + fields := strings.Split(out, "\x00") + for i := 0; i < len(fields); i++ { + parts := strings.SplitN(fields[i], "\t", 3) if len(parts) != 3 { continue } @@ -133,10 +143,17 @@ func parseGitNumstat(out string) map[string][2]int { continue } path := parts[2] - if to, _, found := cutRename(path); found { - path = to + if path == "" { + // Rename/copy record: the next two fields are preimage, postimage. + if i+2 >= len(fields) { + break // truncated output — drop the partial record + } + path = fields[i+2] + i += 2 + } + if path != "" { + stats[path] = [2]int{adds, dels} } - stats[unquoteGitPath(path)] = [2]int{adds, dels} } return stats } @@ -196,15 +213,18 @@ func (m model) maybeGitSweep() (model, tea.Cmd) { } // gitTouchedFiles adapts the sweep results to the roster's touchedFile shape, -// for merging under the transcript-derived entries (files_panel.go). No -// transcript row backs them (lastRowIndex -1), so selecting one skips the -// scroll/tint and the drill-in opens on the full file. +// for merging under the transcript-derived entries (files_panel.go), newest +// detection first — gitTouched upserts in first-seen order, so iterating it in +// reverse keeps the merged FILES tail consistent with the roster's +// newest-first ordering. No transcript row backs them (lastRowIndex -1), so +// selecting one skips the scroll/tint and the drill-in opens on the full file. func (m model) gitTouchedFiles() []touchedFile { if len(m.gitTouched) == 0 { return nil } files := make([]touchedFile, 0, len(m.gitTouched)) - for _, f := range m.gitTouched { + for i := len(m.gitTouched) - 1; i >= 0; i-- { + f := m.gitTouched[i] files = append(files, touchedFile{ path: f.path, created: f.created, diff --git a/internal/tui/files_git_sweep_test.go b/internal/tui/files_git_sweep_test.go index 68c80d16f..484392e39 100644 --- a/internal/tui/files_git_sweep_test.go +++ b/internal/tui/files_git_sweep_test.go @@ -34,14 +34,36 @@ func TestParseGitPorcelain(t *testing.T) { } } +// TestParseGitNumstat feeds the exact `git diff --numstat -z` record shapes: +// plain "added\tdeleted\tpath\0", binary "-\t-\tpath\0", and the rename form +// "added\tdeleted\t\0preimage\0postimage\0" — the stats must key the postimage +// (the path porcelain/changedFiles report), which the old newline parser lost +// (plain --numstat brace-mangles renames, so they never matched a roster path). func TestParseGitNumstat(t *testing.T) { - stats := parseGitNumstat("12\t3\tweb/app.js\n-\t-\tassets/logo.png\n4\t0\told.go => pkg/new.go\n1\t1\tR\told.go -> pkg/new.go\n") + out := "12\t3\tweb/app.js\x00" + + "-\t-\tassets/logo.png\x00" + + "4\t0\t\x00old.go\x00pkg/new.go\x00" + + "1\t1\tweb/my file.js\x00" // -z paths are never quoted + stats := parseGitNumstat(out) if got := stats["web/app.js"]; got != [2]int{12, 3} { t.Errorf("web/app.js = %v, want [12 3]", got) } if _, ok := stats["assets/logo.png"]; ok { t.Error("binary '-' entries must be skipped") } + if got := stats["pkg/new.go"]; got != [2]int{4, 0} { + t.Errorf("renamed file must key the postimage path = %v, want [4 0]", got) + } + if _, ok := stats["old.go"]; ok { + t.Error("the rename preimage must not appear as its own entry") + } + if got := stats["web/my file.js"]; got != [2]int{1, 1} { + t.Errorf("path with spaces = %v, want [1 1] (no quoting in -z mode)", got) + } + // Truncated rename record (missing postimage) must not panic or mis-key. + if got := parseGitNumstat("4\t0\t\x00old.go"); len(got) != 0 { + t.Errorf("truncated rename record should be dropped, got %v", got) + } } // TestGitSweepMergeAndBaseline: the baseline snapshot hides pre-existing dirty @@ -194,6 +216,26 @@ func TestGitSweepCmdAgainstRealRepo(t *testing.T) { t.Fatalf("tracked.txt numstat = +%d −%d, want +2 −0", f.adds, f.dels) } + // A rename WITH edits: porcelain reports the new path, and the -z numstat + // rename record must resolve to the same (postimage) path so the diffstat + // actually attaches — the plain --numstat form brace-mangles the path and + // the counts were silently lost. + run("add", ".") + run("commit", "-q", "-m", "second") + run("mv", "tracked.txt", "renamed.txt") + write("renamed.txt", "one\ntwo\nthree\nfour\nfive\n") + renameSweep := gitSweepCmd(nil, dir, false)().(gitSweepMsg) + if !renameSweep.ok { + t.Fatal("rename sweep failed") + } + renamed := map[string]gitSweepFile{} + for _, f := range renameSweep.files { + renamed[f.path] = f + } + if f, ok := renamed["renamed.txt"]; !ok || f.adds != 1 { + t.Fatalf("renamed file should carry its numstat under the new path, got %+v", renameSweep.files) + } + // Not-a-repo → ok=false (the sweep latches off). if msg := gitSweepCmd(nil, t.TempDir(), false)().(gitSweepMsg); msg.ok { t.Fatal("a non-repo should report ok=false") diff --git a/internal/tui/files_panel.go b/internal/tui/files_panel.go index f25a1b05c..01723d0f6 100644 --- a/internal/tui/files_panel.go +++ b/internal/tui/files_panel.go @@ -14,6 +14,7 @@ package tui import ( "fmt" + "sort" "strings" tea "charm.land/bubbletea/v2" @@ -50,6 +51,13 @@ func (m model) touchedFiles() []touchedFile { continue } adds, dels := planDiffStat(row.detail) + // A multi-file result (apply_patch spanning several files) must not charge + // the WHOLE patch's totals to every file it touched — split the diff by its + // per-file headers and attribute each file its own counts. + var perFile map[string][2]int + if len(row.changedFiles) > 1 { + perFile = perFileDiffStats(row.detail) + } for _, path := range row.changedFiles { if path == "" { continue @@ -64,18 +72,27 @@ func (m model) touchedFiles() []touchedFile { }) at = len(files) - 1 } - files[at].adds += adds - files[at].dels += dels + fileAdds, fileDels := adds, dels + if perFile != nil { + // Headers were found: use this file's own counts. A path the split + // couldn't attribute shows 0/0 rather than the inflated patch totals. + counts := perFile[path] + fileAdds, fileDels = counts[0], counts[1] + } + files[at].adds += fileAdds + files[at].dels += fileDels files[at].edits++ files[at].failed = row.status == tools.StatusError files[at].lastRowIndex = i } } // Most recently touched first: the file being worked on now belongs at the - // top, like the sidebar's ACTIVITY feed. - for left, right := 0, len(files)-1; left < right; left, right = left+1, right-1 { - files[left], files[right] = files[right], files[left] - } + // top, like the sidebar's ACTIVITY feed. Sorted by last touch (not reversed + // first-seen order): a file touched early and touched again last must list + // first. Stable, so same-row files keep their changedFiles order. + sort.SliceStable(files, func(i, j int) bool { + return files[i].lastRowIndex > files[j].lastRowIndex + }) // Merge the git sweep's discoveries (bash/subagent mutations that carry no // changedFiles — files_git_sweep.go) below the transcript-derived entries, // skipping paths a tool result already reported. @@ -88,6 +105,53 @@ func (m model) touchedFiles() []touchedFile { return files } +// perFileDiffStats splits a (possibly multi-file) unified diff into per-path +// +added/−removed counts, keyed by the workspace-relative path from each +// section's "+++ b/…" header (the "--- a/…" old side when the new side is +// /dev/null, i.e. a deletion). Returns nil when the detail carries no file +// headers at all (write_file/edit_file previews may be bare hunks) — the caller +// then falls back to whole-detail totals, which are correct for a single file. +func perFileDiffStats(detail string) map[string][2]int { + stats := map[string][2]int{} + current, oldPath := "", "" + normalize := func(line string, prefix string) string { + path := strings.TrimSpace(line[4:]) + if tab := strings.IndexByte(path, '\t'); tab >= 0 { + path = path[:tab] // unified-diff convention: optional timestamp after a tab + } + path = unquoteGitPath(path) + if path == "/dev/null" { + return "" + } + return strings.TrimPrefix(path, prefix) + } + for _, line := range strings.Split(detail, "\n") { + switch { + case strings.HasPrefix(line, "--- "): + oldPath = normalize(line, "a/") + case strings.HasPrefix(line, "+++ "): + current = normalize(line, "b/") + if current == "" { + current = oldPath // deletion: charge the removed lines to the old path + } + case current == "": + // Lines before the first header (or in an unattributable section). + case strings.HasPrefix(line, "+"): + counts := stats[current] + counts[0]++ + stats[current] = counts + case strings.HasPrefix(line, "-"): + counts := stats[current] + counts[1]++ + stats[current] = counts + } + } + if len(stats) == 0 { + return nil + } + return stats +} + // resultRowCreatedFile reports whether a tool-result row reads as a file // creation ("tool result: write_file ok Created x (…)"), so the FILES row can // badge it A rather than M. Heuristic on the confirmation text write_file @@ -149,13 +213,19 @@ func (m model) sidebarFileLines(width int) ([]string, []fileHit) { zeroTheme.ink.Render(truncatePathLeft(live, room))) } - shown := 0 + // Filter the live row out FIRST so the "+N more" trailer counts only rows + // that could actually render — counting the skipped live entry would inflate + // the overflow by one. + visible := files[:0:0] for _, f := range files { - if f.path == live { - continue // already shown as the live row + if f.path != live { + visible = append(visible, f) } + } + shown := 0 + for _, f := range visible { if shown >= maxSidebarFiles { - lines = append(lines, " "+zeroTheme.faint.Render(fmt.Sprintf("+%d more", len(files)-shown))) + lines = append(lines, " "+zeroTheme.faint.Render(fmt.Sprintf("+%d more", len(visible)-shown))) break } shown++ diff --git a/internal/tui/files_panel_test.go b/internal/tui/files_panel_test.go index 8c9b96e3f..909771703 100644 --- a/internal/tui/files_panel_test.go +++ b/internal/tui/files_panel_test.go @@ -230,3 +230,88 @@ func TestTruncatePathLeft(t *testing.T) { t.Errorf("want …/sidebar.go, got %q", got) } } + +// TestTouchedFilesPerFileDiffStat: a multi-file result (apply_patch spanning +// several files) must charge each file its OWN +/- counts from the per-file +// diff sections — not the whole patch's totals to every file it touched. +func TestTouchedFilesPerFileDiffStat(t *testing.T) { + m := sidebarTestModel() + patch := "diff --git a/web/one.js b/web/one.js\n" + + "--- a/web/one.js\n+++ b/web/one.js\n@@ -1,1 +1,2 @@\n+added one\n+added two\n" + + "diff --git a/web/two.js b/web/two.js\n" + + "--- a/web/two.js\n+++ b/web/two.js\n@@ -1,2 +1,1 @@\n-gone one\n-gone two\n-gone three\n" + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, tool: "apply_patch", id: "p1", status: tools.StatusOK, + detail: patch, + changedFiles: []string{"web/one.js", "web/two.js"}, + }) + byPath := map[string]touchedFile{} + for _, f := range m.touchedFiles() { + byPath[f.path] = f + } + if f := byPath["web/one.js"]; f.adds != 2 || f.dels != 0 { + t.Errorf("web/one.js = +%d −%d, want its own +2 −0 (not the patch totals)", f.adds, f.dels) + } + if f := byPath["web/two.js"]; f.adds != 0 || f.dels != 3 { + t.Errorf("web/two.js = +%d −%d, want its own +0 −3 (not the patch totals)", f.adds, f.dels) + } +} + +// TestTouchedFilesRetouchOrdersByLastTouch: touch a, then b, then a again — a +// must list first. The old "reverse first-seen order" put b first because a's +// slot kept its original position. +func TestTouchedFilesRetouchOrdersByLastTouch(t *testing.T) { + m := sidebarTestModel() + for _, step := range []struct{ id, path string }{ + {"r1", "a.go"}, {"r2", "b.go"}, {"r3", "a.go"}, + } { + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, tool: "edit_file", id: step.id, status: tools.StatusOK, + changedFiles: []string{step.path}, + }) + } + files := m.touchedFiles() + if len(files) != 2 || files[0].path != "a.go" { + t.Fatalf("the re-touched file must list first, got %+v", files) + } +} + +// TestSidebarFileLinesOverflowExcludesLiveRow: when the live-writing file is +// also a touched file, it renders as the pulse row and is skipped from the +// list — the "+N more" trailer must not count that skipped entry. +func TestSidebarFileLinesOverflowExcludesLiveRow(t *testing.T) { + m := sidebarTestModel() + for _, name := range []string{"a", "b", "c", "d", "e", "f", "g", "h"} { + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, tool: "edit_file", id: name, status: tools.StatusOK, + changedFiles: []string{name + ".go"}, + }) + } + // h.go (a touched file) is ALSO the live write: 7 listable files remain, + // 6 render, so the trailer is +1 — not +2 (which counted the skipped live row). + m.streamCallName = "edit_file" + m.streamCallDecoder = newStreamingDecoder() + m.streamCallDecoder.feed(`{"path":"h.go","content":"x`) + + lines, _ := m.sidebarFileLines(34) + trailer := plainRender(t, lines[len(lines)-1]) + if !strings.Contains(trailer, "+1 more") { + t.Fatalf("overflow must exclude the live row from the count, got %q", trailer) + } +} + +// TestSidebarHasContentForLiveWrite: the sidebar counts an in-flight first +// write as content, so the FILES pulse is visible before any result row lands. +func TestSidebarHasContentForLiveWrite(t *testing.T) { + m := sidebarTestModel() + m.plan.steps = nil // drop the helper's seeded plan: no agents/plan/files now + if m.sidebarHasContent() { + t.Fatal("session without agents/plan/files should have no sidebar content") + } + m.streamCallName = "write_file" + m.streamCallDecoder = newStreamingDecoder() + m.streamCallDecoder.feed(`{"path":"web/new.css","content":"body{}`) + if !m.sidebarHasContent() { + t.Fatal("a live in-flight write must count as sidebar content") + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index e386d841f..0a67d97ae 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1080,9 +1080,10 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m.handleCtrlC() case keyCtrl(msg, 'o'): return m.toggleDetailedTranscript(), nil - case m.fileView.active && m.composerValue() == "" && (keyText(msg) == "d" || keyText(msg) == "f"): + case m.fileView.active && m.noBlockingModal() && m.composerValue() == "" && (keyText(msg) == "d" || keyText(msg) == "f"): // Mode toggle for the file drill-in, only while the composer is empty - // so mid-sentence typing is never hijacked. + // (so mid-sentence typing is never hijacked) and no modal is up (so a + // permission prompt / ask-user / wizard keeps its own key handling). if keyText(msg) == "f" { return m.setFileViewMode(fileViewFull), nil } @@ -1112,8 +1113,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } // File drill-in exits on Esc (returns to the chat at its saved scroll - // position); the file stays selected so a second Esc clears that. - if m.fileView.active { + // position); the file stays selected so a second Esc clears that. Only + // with no blocking modal up: Esc on a permission prompt / ask-user / + // wizard must reach THAT surface's deny/cancel handling below, not + // silently close the drill-in behind it. + if m.fileView.active && m.noBlockingModal() { return m.exitFileView(), nil } if m.mcpCommandCancel != nil { diff --git a/internal/tui/sidebar.go b/internal/tui/sidebar.go index a38ca4239..f43d014f3 100644 --- a/internal/tui/sidebar.go +++ b/internal/tui/sidebar.go @@ -97,7 +97,9 @@ func (m model) sidebarHasContent() bool { if len(m.sidebarSpecialists()) > 0 || len(m.swarmSpawnedAgents()) > 0 { return true } - if len(m.touchedFiles()) > 0 { + if len(m.touchedFiles()) > 0 || m.liveEditingPath() != "" { + // A live in-flight write counts before its result row exists, so the + // FILES pulse for the session's first mutation isn't hidden. return true } return !m.plan.isEmpty()