From 558beb9703e724ff7bfcf4d73edd703ee42a1e5e Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Mon, 2 Nov 2026 10:07:39 +0100 Subject: [PATCH 1/6] perf(tui): bounded file read, LRU rendering cache, and size caps for full-file view - Bound full-file view reads (4000 lines, 1 MiB total, 4 KiB per line) with async loading and an LRU render cache capped at 64 entries and 8 MiB. - Refresh both BTW surfaces when a tool result (such as write_stdin) carries both exact changed paths and an unknown-scope or aggregated directory summary outside Git. - Gate Run details FILES mouse hit-testing on Run details being the visible foreground overlay so higher-priority modals (speech API-key prompt, leader help) block clicks behind them. (cherry picked from commit 100d847aafe1958180c7082cb014f80434adf01b) --- internal/tui/btw.go | 75 +- internal/tui/btw_test.go | 67 + internal/tui/export_test.go | 9 + internal/tui/file_view.go | 1311 ++++++++- internal/tui/file_view_test.go | 3897 +++++++++++++++++++++++++- internal/tui/file_view_unix_test.go | 55 + internal/tui/files_git_sweep_test.go | 4 +- internal/tui/files_panel.go | 52 +- internal/tui/files_panel_test.go | 192 ++ internal/tui/flush_test.go | 2 +- internal/tui/model.go | 255 +- internal/tui/mouse.go | 5 + internal/tui/run_details.go | 57 +- internal/tui/run_details_test.go | 28 + internal/tui/session_controls.go | 38 +- internal/tui/session_test.go | 2 +- internal/tui/syntax_highlight.go | 16 +- internal/tui/theme_select.go | 3 + 18 files changed, 5926 insertions(+), 142 deletions(-) create mode 100644 internal/tui/file_view_unix_test.go diff --git a/internal/tui/btw.go b/internal/tui/btw.go index d83559c12..ca228a53a 100644 --- a/internal/tui/btw.go +++ b/internal/tui/btw.go @@ -91,6 +91,20 @@ func (m model) handleBTWCommand(question string) (model, tea.Cmd) { parent.flushQueue = nil side := parent + // The side surface must own its file-view lifecycle. Because `side` is a + // value copy of `parent`, its liveSeq pointer would otherwise alias the + // hidden parent's: closing or switching the view in BTW would revoke the + // parent's in-flight load. Detach it and give the side a fresh lifetime so + // inherited markers cannot masquerade as the parent's snapshot. + side.fileView.liveSeq = nil + if side.fileView.active { + side.fileView.lifetimeToken = nextFileViewLifetimeToken() + side.fileView.loadedToken = [16]byte{} + side.fileView.loadedSeq = 0 + side.fileView.loadedRev = 0 + side.fileView.snapshotReady = false + side.fileView.renderedContent = "" + } side.activeSession = fork side.sessionEvents = events side.transcript = initialTranscript() @@ -156,10 +170,16 @@ func (m model) handleBTWCommand(question string) (model, tea.Cmd) { sideRunIDBase: side.runID, } + var btwFileViewCmd tea.Cmd + if side.fileView.active && side.fileView.mode == fileViewFull { + side, btwFileViewCmd = side.startFileViewLoadCmd(side.chatColumnWidth()) + } + if question == "" { - return side, nil + return side, btwFileViewCmd } - return side.launchPrompt(question) + next, launchCmd := side.launchPrompt(question) + return next, batchCommands(btwFileViewCmd, launchCmd) } func (m model) leaveBTW() (model, tea.Cmd) { @@ -175,6 +195,10 @@ func (m model) leaveBTW() (model, tea.Cmd) { if m.compactInFlight { return m.appendSystemNotice("BTW compaction is still running. Wait for it to finish before returning."), nil } + // The side surface is being discarded. Revoke its file-view lifetime token so + // a load still queued behind BTW cannot run or land after the view closes. + // The restored parent keeps its own request and snapshot untouched. + m.revokeFileViewRequest() m, _ = m.clearLoopsForSessionSwitch() parent := *m.btw.parent parent.goalContinuationsSuspended = false @@ -201,7 +225,9 @@ func (m model) leaveBTW() (model, tea.Cmd) { parent.resetFlushFrontier("· returned from btw ·") var goalCmd tea.Cmd parent, goalCmd = parent.launchGoalContinuationIfReady() - return parent, batchCommands(sweepCmd, spinnerCmd, goalCmd) + var fileCmd tea.Cmd + parent, fileCmd = parent.recoverInvalidatedFileView() + return parent, batchCommands(sweepCmd, spinnerCmd, goalCmd, fileCmd) } func btwCommandUnavailable(command parsedCommand) bool { @@ -236,9 +262,9 @@ func btwCommandUnavailable(command parsedCommand) bool { // messages have no run ID, so normal BTW routing intentionally leaves them on // the visible side surface; copying the layout fields prevents stale wrapping // after the parent is restored. -func (m model) resizeBTWParent(msg tea.WindowSizeMsg) model { +func (m model) resizeBTWParent(msg tea.WindowSizeMsg) (model, tea.Cmd) { if !m.btw.active || m.btw.parent == nil { - return m + return m, nil } parent := *m.btw.parent parent.width = msg.Width @@ -247,8 +273,15 @@ func (m model) resizeBTWParent(msg tea.WindowSizeMsg) model { parent.lineAges = nil parent.lastStreamActivity = parent.now() parent.input.SetWidth(maxInt(20, chatWidth(msg.Width)-14)) + // The parent's full-file view wrapped at the old width; reload it at the new + // one. Its completion has no run ID, so routeBTWParentMessage matches it by + // lifetime token and delivers it to the hidden parent. + var cmd tea.Cmd + if parent.fileView.active && parent.fileView.mode == fileViewFull { + parent, cmd = parent.startFileViewLoadCmd(parent.chatColumnWidth()) + } m.btw.parent = &parent - return m + return m, cmd } func btwTitle(parent string) string { @@ -272,6 +305,15 @@ func (m model) routeBTWParentMessage(msg tea.Msg) (model, tea.Cmd, bool) { } return m.routeBTWMessageToParent(msg) } + if loaded, ok := msg.(fileViewLoadedMsg); ok { + // File-view completions carry no run ID. Deliver the hidden parent's + // load (matched by lifetime token) so it does not stay stuck on + // "Loading…" while BTW is active; side-owned loads fall through. + if m.btw.parent.fileView.active && m.btw.parent.fileView.lifetimeToken == loaded.lifetimeToken { + return m.routeBTWMessageToParent(msg) + } + return m, nil, false + } runID, ok := btwMessageRunID(msg) if !ok || runID <= 0 || runID >= m.btw.sideRunIDBase { return m, nil, false @@ -279,6 +321,21 @@ func (m model) routeBTWParentMessage(msg tea.Msg) (model, tea.Cmd, bool) { return m.routeBTWMessageToParent(msg) } +// refreshHiddenParentFileView reloads the hidden parent's full-file view when a +// side-surface mutation advanced the shared path revision. The load completion +// carries the parent's lifetime token, so routeBTWParentMessage delivers it +// back to the hidden model without waiting for a git sweep or for /btw return. +func (m model) refreshHiddenParentFileView() (model, tea.Cmd) { + if !m.btw.active || m.btw.parent == nil { + return m, nil + } + parent := *m.btw.parent + var cmd tea.Cmd + parent, cmd = parent.recoverInvalidatedFileView() + m.btw.parent = &parent + return m, cmd +} + func (m model) routeBTWMessageToParent(msg tea.Msg) (model, tea.Cmd, bool) { parentNext, cmd := m.btw.parent.updateModel(msg) parent, ok := parentNext.(model) @@ -296,7 +353,9 @@ func (m model) routeBTWMessageToParent(msg tea.Msg) (model, tea.Cmd, bool) { case agentResponseMsg: m.btw.parentNeedsInput = parent.pendingPermission != nil || parent.pendingAskUser != nil } - return m, cmd, true + var fileCmd tea.Cmd + m, fileCmd = m.recoverInvalidatedFileView() + return m, batchCommands(cmd, fileCmd), true } func btwMessageRunID(msg tea.Msg) (int, bool) { @@ -327,6 +386,8 @@ func btwMessageRunID(msg tea.Msg) (int, bool) { return typed.runID, true case specialistProgressMsg: return typed.runID, true + case unknownScopeMutationMsg: + return typed.runID, true case swarmSessionsMsg: return typed.runID, true case permissionRequestMsg: diff --git a/internal/tui/btw_test.go b/internal/tui/btw_test.go index 4425cd0ed..f5f2d44b1 100644 --- a/internal/tui/btw_test.go +++ b/internal/tui/btw_test.go @@ -3,6 +3,7 @@ package tui import ( "bytes" "context" + "errors" "os" "path/filepath" "strings" @@ -535,3 +536,69 @@ func TestBTWCtrlCDuringRunDoesNotClearDraft(t *testing.T) { t.Fatalf("missing in-flight return guidance: %#v", got.transcript) } } + +// TestBTWLeaveRevokesSideFileLoad verifies that leaving BTW revokes the side +// surface's file-view lifetime so a still-queued side load cannot run or land +// after the view closes, without cancelling the restored parent's own load. +func TestBTWLeaveRevokesSideFileLoad(t *testing.T) { + resetFileViewCacheForTest() + m := newBTWTestModel(t) + dir := t.TempDir() + m.cwd = dir + name := "side_view.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package side\n"), 0o644); err != nil { + t.Fatal(err) + } + // Open a full view so the side surface inherits an active file view. + m, parentLoadCmd := m.openFileView(name) + if parentLoadCmd == nil { + t.Fatal("expected a pending parent load command") + } + parentLiveSeq := m.fileView.liveSeq + if parentLiveSeq == nil { + t.Fatal("parent load token missing") + } + parentSeq := parentLiveSeq.Load() + + // Enter BTW: the side surface detaches and schedules its own load. + side, sideLoadCmd := m.handleBTWCommand("") + if !side.btw.active || side.btw.parent == nil { + t.Fatal("expected an active BTW conversation") + } + if sideLoadCmd == nil { + t.Fatal("side surface should schedule its own file load") + } + sideLiveSeq := side.fileView.liveSeq + if sideLiveSeq == nil || sideLiveSeq == parentLiveSeq { + t.Fatal("side load must own a detached lifetime token") + } + sideSeq := sideLiveSeq.Load() + + // Return while the side load is still queued. + parent, _ := side.leaveBTW() + if sideLiveSeq.Load() == sideSeq { + t.Fatal("leaveBTW must revoke the side file-view request") + } + if parentLiveSeq.Load() != parentSeq || parentLiveSeq == sideLiveSeq { + t.Fatal("leaveBTW must not revoke the restored parent's request") + } + + // The held side command must now be a superseded no-op. + held := sideLoadCmd() + loaded, ok := held.(fileViewLoadedMsg) + if !ok { + t.Fatalf("held side command produced %T, want fileViewLoadedMsg", held) + } + if !errors.Is(loaded.err, errFileViewSuperseded) { + t.Fatalf("queued side load must be superseded after leaving BTW, got %v", loaded.err) + } + + // The parent's own load still completes and settles its snapshot. + parent = deliverCommandMessages(t, parent, parentLoadCmd) + if parent.fileView.loading { + t.Fatal("parent load must remain valid after the side revocation") + } + if !strings.Contains(plainRender(t, parent.renderFileViewFull(80)), "package side") { + t.Fatal("restored parent must keep its snapshot") + } +} diff --git a/internal/tui/export_test.go b/internal/tui/export_test.go index 6a3a6ed69..f55d7c4d2 100644 --- a/internal/tui/export_test.go +++ b/internal/tui/export_test.go @@ -128,6 +128,15 @@ func (c *staticRenderCache) stats() renderCacheStats { return c.statsData } +func fileViewCacheStatsForTest() fileViewCacheStats { + return defaultFileViewCache.stats() +} + +func resetFileViewCacheForTest() { + defaultFileViewCache.clear() + defaultFileViewCache.resetStats() +} + func renderSelectableList(options selectableListOptions) string { if len(options.Items) == 0 { return "" diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 7e544d70b..8074f1c24 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -15,30 +15,1097 @@ package tui import ( "bufio" + "container/list" + "crypto/sha256" + "errors" "fmt" + "io" "os" "path/filepath" + "sort" "strings" + "sync" + "sync/atomic" + "syscall" + "time" + + tea "charm.land/bubbletea/v2" ) // 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 +// freeze a render; the tail collapses into a "… more lines (file truncated at N for display)" trailer. +const ( + fileViewMaxLines = 4000 + fileViewMaxBytes = 1 << 20 // 1 MiB total read budget per file + fileViewMaxLineBytes = 4096 // 4 KiB max line length budget + defaultFileViewCacheMaxEntries = 64 + // defaultFileViewCacheMaxBytes bounds the total retained bytes of the file + // view cache so wide renders of large files cannot grow without limit + // (measured at tens to hundreds of MiB before this cap). It counts the stable + // source+display payload plus every rendered variant. + defaultFileViewCacheMaxBytes = 8 << 20 // 8 MiB + fileViewMaxRenderVariants = 4 // max rendered variants (width/fingerprint) per cached file entry + fileViewMaxPathRevisions = 1024 // hard cap on retained per-path revision counters + fileViewMinPathRevisions = 256 // floor for the revision bound on small caches + fileViewLoadingPlaceholder = "Loading…" +) const ( fileViewDiff = iota fileViewFull ) +var ( + fileViewLifetimeTS atomic.Uint64 + fileViewLifetimeSeq atomic.Uint32 +) + +// nextFileViewLifetimeToken produces a monotonic, time-ordered 128-bit UUIDv7 (RFC 9562) +// to uniquely identify the lifecycle of a file view session without heap allocations. +func nextFileViewLifetimeToken() [16]byte { + nowMs := uint64(time.Now().UnixMilli()) + for { + last := fileViewLifetimeTS.Load() + if nowMs > last { + if fileViewLifetimeTS.CompareAndSwap(last, nowMs) { + fileViewLifetimeSeq.Store(0) + break + } + } else { + nowMs = last + break + } + } + seq := fileViewLifetimeSeq.Add(1) + var u [16]byte + u[0] = byte(nowMs >> 40) + u[1] = byte(nowMs >> 32) + u[2] = byte(nowMs >> 24) + u[3] = byte(nowMs >> 16) + u[4] = byte(nowMs >> 8) + u[5] = byte(nowMs) + u[6] = 0x70 | byte((seq>>8)&0x0F) // Version 7 + u[7] = byte(seq & 0xFF) + u[8] = 0x80 | byte((seq>>16)&0x3F) // RFC 9562 variant + u[9] = byte(seq >> 24) + return u +} + +// fileViewCacheStats tracks disk I/O, Chroma highlighting, and cache hits/misses. +type fileViewCacheStats struct { + DiskReads int + HighlightCalls int + CacheHits int + CacheMisses int + Evictions int + ThemeClears int + RenderEvictions int + SkippedOversized int +} + +type fileViewCachedEntry struct { + targetPath string + displayPath string + modTime time.Time + size int64 + sourceRev uint64 + sourceHash [sha256.Size]byte + sourceBytes int + lines []string + display []string + truncated bool + omittedLines bool + + rendersMu sync.RWMutex + renderKeys []string // LRU order: oldest at index 0, most recent at end + renders map[string]string // key: "width:changedLinesFingerprint" -> formatted ANSI string +} + +// byteSize is the total retained payload for the entry: the stable source and +// display lines plus every rendered variant. The reader uses it to enforce the +// cache-wide byte budget. +func (e *fileViewCachedEntry) byteSize() int { + n := e.sourceBytes + e.rendersMu.RLock() + for k, v := range e.renders { + n += len(k) + len(v) + } + e.rendersMu.RUnlock() + return n +} + +// fileViewSourceHash fingerprints the bounded source lines so a metadata-only +// change (cp -p, rsync -t, tar -xp restoring mtime and size) cannot serve a +// stale entry. Lines are joined with a separator the reader never emits inside +// a line, so shifted boundaries cannot collide. +func fileViewSourceHash(lines []string) [sha256.Size]byte { + h := sha256.New() + for i, line := range lines { + if i > 0 { + h.Write([]byte{'\n'}) + } + h.Write([]byte(line)) + } + var out [sha256.Size]byte + copy(out[:], h.Sum(nil)) + return out +} + +func (e *fileViewCachedEntry) getRender(key string) (string, bool) { + e.rendersMu.RLock() + val, ok := e.renders[key] + e.rendersMu.RUnlock() + if !ok { + return "", false + } + e.rendersMu.Lock() + for i, k := range e.renderKeys { + if k == key { + e.renderKeys = append(append(e.renderKeys[:i], e.renderKeys[i+1:]...), key) + break + } + } + e.rendersMu.Unlock() + return val, true +} + +// putRender stores a formatted variant and returns the change in retained +// bytes (positive on growth, negative when an old variant is dropped or +// replaced). The caller folds the delta into the cache-wide budget. +func (e *fileViewCachedEntry) putRender(key string, val string) int { + e.rendersMu.Lock() + defer e.rendersMu.Unlock() + if e.renders == nil { + e.renders = make(map[string]string) + } + delta := 0 + if prev, ok := e.renders[key]; ok { + delta = len(val) - len(prev) + } else { + for len(e.renders) >= fileViewMaxRenderVariants { + if len(e.renderKeys) > 0 { + oldKey := e.renderKeys[0] + e.renderKeys = e.renderKeys[1:] + delta -= len(oldKey) + len(e.renders[oldKey]) + delete(e.renders, oldKey) + } else { + for k, v := range e.renders { + delta -= len(k) + len(v) + delete(e.renders, k) + break + } + } + } + e.renderKeys = append(e.renderKeys, key) + delta += len(key) + len(val) + } + e.renders[key] = val + return delta +} + +type fileViewRenderCache struct { + mu sync.Mutex + maxEntries int + maxBytes int + retained int + items map[string]*list.Element // targetPath -> *list.Element containing *fileViewCachedEntry + lru *list.List + gen int + pathRevisions map[string]uint64 + revOrder *list.List // LRU of revision keys: front = most recently invalidated path + revElems map[string]*list.Element // targetPath -> *list.Element containing the path string + revEpochFloor uint64 // minimum required revision for any path absent from pathRevisions + statsData fileViewCacheStats +} + +var defaultFileViewCache = newFileViewRenderCache(defaultFileViewCacheMaxEntries, defaultFileViewCacheMaxBytes) + +func newFileViewRenderCache(maxEntries int, maxBytes int) *fileViewRenderCache { + return &fileViewRenderCache{ + maxEntries: maxEntries, + maxBytes: maxBytes, + items: make(map[string]*list.Element), + lru: list.New(), + pathRevisions: make(map[string]uint64), + revOrder: list.New(), + revElems: make(map[string]*list.Element), + } +} + +func (c *fileViewRenderCache) invalidatePath(targetPath string) uint64 { + c.mu.Lock() + defer c.mu.Unlock() + rev := c.pathRevisions[targetPath] + if rev < c.revEpochFloor { + rev = c.revEpochFloor + } + rev++ + c.pathRevisions[targetPath] = rev + c.touchRevisionLocked(targetPath) + c.trimRevisionsLocked() + c.removeItemLocked(targetPath) + return rev +} + +func (c *fileViewRenderCache) requiredRevision(targetPath string) uint64 { + c.mu.Lock() + defer c.mu.Unlock() + return c.requiredRevisionLocked(targetPath) +} + +// requiredRevisionLocked returns targetPath's revision, or the epoch floor when +// its metadata has been evicted. Any request older than an evicted revision is +// therefore rejected without needing the dead key to stay resident. +func (c *fileViewRenderCache) requiredRevisionLocked(targetPath string) uint64 { + if rev, ok := c.pathRevisions[targetPath]; ok { + return rev + } + return c.revEpochFloor +} + +// maxPathRevisions bounds how many per-path revision counters are retained. It +// scales with the entry budget but never drops below a small floor and never +// grows past a hard cap. +func (c *fileViewRenderCache) maxPathRevisions() int { + limit := c.maxEntries * 4 + if limit < fileViewMinPathRevisions { + limit = fileViewMinPathRevisions + } + if limit > fileViewMaxPathRevisions { + limit = fileViewMaxPathRevisions + } + return limit +} + +// touchRevisionLocked moves targetPath to the front of the revision LRU, +// inserting it when new. +func (c *fileViewRenderCache) touchRevisionLocked(targetPath string) { + if c.revOrder == nil { + c.revOrder = list.New() + } + if c.revElems == nil { + c.revElems = make(map[string]*list.Element) + } + if elem, ok := c.revElems[targetPath]; ok { + c.revOrder.MoveToFront(elem) + return + } + c.revElems[targetPath] = c.revOrder.PushFront(targetPath) +} + +// dropRevisionLocked removes a path's revision metadata and advances the epoch +// floor so requests captured before the removal cannot commit. +func (c *fileViewRenderCache) dropRevisionLocked(targetPath string) { + rev, ok := c.pathRevisions[targetPath] + if !ok { + return + } + delete(c.pathRevisions, targetPath) + if c.revElems != nil { + if elem, ok := c.revElems[targetPath]; ok { + c.revOrder.Remove(elem) + delete(c.revElems, targetPath) + } + } + if rev > c.revEpochFloor { + c.revEpochFloor = rev + } +} + +// trimRevisionsLocked evicts the oldest revision keys once the map exceeds the +// bound, advancing the epoch floor as it goes. +func (c *fileViewRenderCache) trimRevisionsLocked() { + limit := c.maxPathRevisions() + for len(c.pathRevisions) > limit { + back := c.revOrder.Back() + if back == nil { + return + } + c.dropRevisionLocked(back.Value.(string)) + } +} + +// removeItemLocked drops targetPath from the LRU and folds its bytes back out +// of retained. It intentionally leaves pathRevisions alone: an invalidated path +// stays dirty until its entry is evicted by capacity or the cache is purged. +func (c *fileViewRenderCache) removeItemLocked(targetPath string) { + elem, ok := c.items[targetPath] + if !ok { + return + } + c.retained -= elem.Value.(*fileViewCachedEntry).byteSize() + c.lru.Remove(elem) + delete(c.items, targetPath) +} + +// evictOverflowLocked trims the LRU until both the entry count and the retained +// byte budget fit. Evicting an entry also drops its path revision: the path is no +// longer cached, so keeping the revision would only leak one key per file. +func (c *fileViewRenderCache) evictOverflowLocked() { + for len(c.items) > c.maxEntries || (c.maxBytes > 0 && c.retained > c.maxBytes) { + back := c.lru.Back() + if back == nil { + c.retained = 0 + return + } + backEntry := back.Value.(*fileViewCachedEntry) + c.retained -= backEntry.byteSize() + delete(c.items, backEntry.targetPath) + c.lru.Remove(back) + c.dropRevisionLocked(backEntry.targetPath) + c.statsData.Evictions++ + } +} + +// commitRenderPut applies a variant's byte delta and re-trims the cache. The +// entry is only accounted for while it is still the resident cache value for +// its path: a concurrent purge, eviction, or replacement must not leak bytes +// into c.retained from a detached entry. +func (c *fileViewRenderCache) commitRenderPut(entry *fileViewCachedEntry, key string, val string) { + c.mu.Lock() + defer c.mu.Unlock() + elem, ok := c.items[entry.targetPath] + if !ok || elem.Value.(*fileViewCachedEntry) != entry { + return + } + delta := entry.putRender(key, val) + c.retained += delta + c.evictOverflowLocked() +} + +func (c *fileViewRenderCache) evictPath(targetPath string) { + c.mu.Lock() + c.removeItemLocked(targetPath) + c.mu.Unlock() +} + +func (c *fileViewRenderCache) purgeLocked() { + c.gen++ + c.items = make(map[string]*list.Element) + c.lru.Init() + c.retained = 0 + c.pathRevisions = make(map[string]uint64) + c.revOrder = list.New() + c.revElems = make(map[string]*list.Element) + c.revEpochFloor = 0 +} + +func (c *fileViewRenderCache) clear() { + c.mu.Lock() + defer c.mu.Unlock() + c.purgeLocked() + c.statsData.ThemeClears++ +} + +func (c *fileViewRenderCache) invalidateUnknownScope() { + c.mu.Lock() + defer c.mu.Unlock() + c.purgeLocked() +} + +func (c *fileViewRenderCache) generation() int { + c.mu.Lock() + defer c.mu.Unlock() + return c.gen +} + +func (c *fileViewRenderCache) resetStats() { + c.mu.Lock() + defer c.mu.Unlock() + c.statsData = fileViewCacheStats{} +} + +// recordHit notes a snapshot served entirely from resident state. The accepted +// snapshot bypasses the LRU lookup, so it accounts for its own hit to keep the +// no-I/O contract observable in the cache statistics. +func (c *fileViewRenderCache) recordHit() { + c.mu.Lock() + c.statsData.CacheHits++ + c.mu.Unlock() +} + +func (c *fileViewRenderCache) stats() fileViewCacheStats { + c.mu.Lock() + defer c.mu.Unlock() + return c.statsData +} + +type fileViewReadResult struct { + lines []string + modTime time.Time + size int64 + truncated bool + omittedLines bool + err error +} + +type fileViewByteCounter struct { + r io.Reader + n int +} + +func (c *fileViewByteCounter) Read(p []byte) (int, error) { + got, err := c.r.Read(p) + c.n += got + return got, err +} + +func (c *fileViewByteCounter) delivered(buf *bufio.Reader) int { + n := c.n - buf.Buffered() + if n < 0 { + return 0 + } + return n +} + +func stripFileViewLineEnding(b []byte) []byte { + if n := len(b); n > 0 && b[n-1] == '\n' { + b = b[:n-1] + } + if n := len(b); n > 0 && b[n-1] == '\r' { + b = b[:n-1] + } + return b +} + +func readFileViewBounded(path string, maxLines int, maxLineBytes int, maxTotalBytes int) fileViewReadResult { + return readFileViewBoundedCancellable(path, maxLines, maxLineBytes, maxTotalBytes, nil, 0) +} + +func readFileViewBoundedCancellable(path string, maxLines int, maxLineBytes int, maxTotalBytes int, liveSeq *atomic.Uint64, seq uint64) fileViewReadResult { + if fileViewSuperseded(liveSeq, seq) { + return fileViewReadResult{err: errFileViewSuperseded} + } + + file, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + if err != nil { + return fileViewReadResult{err: err} + } + defer file.Close() + + stat, err := file.Stat() + if err != nil { + return fileViewReadResult{err: err} + } + if !stat.Mode().IsRegular() { + return fileViewReadResult{err: fmt.Errorf("cannot read non-regular file (mode %s)", stat.Mode().String())} + } + if fileViewSuperseded(liveSeq, seq) { + return fileViewReadResult{err: errFileViewSuperseded} + } + + var lines []string + truncated := false + omittedLines := false + counter := &fileViewByteCounter{r: io.LimitReader(file, int64(maxTotalBytes)+1)} + reader := bufio.NewReader(counter) + + moreSource := func() bool { + if reader.Buffered() > 0 { + return true + } + _, err := reader.Peek(1) + return err == nil + } + + for len(lines) < maxLines { + if fileViewSuperseded(liveSeq, seq) { + return fileViewReadResult{err: errFileViewSuperseded} + } + if fileViewInsideDiskRead != nil { + fileViewInsideDiskRead() + if fileViewSuperseded(liveSeq, seq) { + return fileViewReadResult{err: errFileViewSuperseded} + } + } + start := counter.delivered(reader) + if start >= maxTotalBytes { + if moreSource() { + truncated = true + omittedLines = true + } + break + } + + var raw []byte + for { + if fileViewSuperseded(liveSeq, seq) { + return fileViewReadResult{err: errFileViewSuperseded} + } + frag, err := reader.ReadSlice('\n') + raw = append(raw, frag...) + if errors.Is(err, bufio.ErrBufferFull) { + continue + } + if err != nil && !errors.Is(err, io.EOF) { + if len(lines) == 0 && len(raw) == 0 { + return fileViewReadResult{err: err} + } + truncated = true + omittedLines = true + break + } + break + } + + if len(raw) == 0 { + break + } + + consumed := counter.delivered(reader) + budget := maxTotalBytes - start + if budget < 0 { + budget = 0 + } + kept := raw + if len(kept) > budget { + kept = kept[:budget] + truncated = true + omittedLines = true + } + display := stripFileViewLineEnding(kept) + lineTruncated := false + if len(display) > maxLineBytes { + display = display[:maxLineBytes] + lineTruncated = true + truncated = true + } + if consumed > maxTotalBytes { + truncated = true + omittedLines = true + } + if lineTruncated { + truncated = true + } + lines = append(lines, string(display)) + if consumed >= maxTotalBytes { + if moreSource() { + truncated = true + omittedLines = true + } + break + } + if len(raw) > 0 && raw[len(raw)-1] != '\n' && !moreSource() { + break + } + } + + if !omittedLines && len(lines) >= maxLines && moreSource() { + truncated = true + omittedLines = true + } + + return fileViewReadResult{ + lines: lines, + modTime: stat.ModTime(), + size: stat.Size(), + truncated: truncated, + omittedLines: omittedLines, + } +} + +// canonicalChangedLineKey normalizes a line for changed-line matching: +// - Replaces tabs with 4 spaces (matching sanitizeRawFileLine) +// - Strips ANSI escape sequences and non-printable control characters +// - Trims leading and trailing whitespace +func canonicalChangedLineKey(s string) string { + var out strings.Builder + for _, r := range s { + if r == '\t' { + out.WriteString(" ") + } else if r == '\r' || r == '\n' { + continue + } else if r < 32 || r == 0x7f || (r >= 0x80 && r <= 0x9f) { + if r == '\x1b' { + out.WriteString("^[") + } + } else { + out.WriteRune(r) + } + } + return strings.TrimSpace(out.String()) +} + +func changedLinesFingerprint(changed map[string]bool) string { + if len(changed) == 0 { + return "" + } + keys := make([]string, 0, len(changed)) + for k, v := range changed { + if v { + keys = append(keys, k) + } + } + sort.Strings(keys) + return strings.Join(keys, "\x00") +} + +func formatFileViewLines(lines []string, display []string, changed map[string]bool, truncated bool, omittedLines bool, width int, theme tuiTheme) string { + gutterW := len(fmt.Sprintf("%d", len(lines))) + textBudget := maxInt(8, width-gutterW-3) // gutter + space + marker column + + var b strings.Builder + for i, line := range display { + line = fitStyledLine(line, textBudget) + if i > 0 { + b.WriteString("\n") + } + marker := " " + if changed != nil && len(lines) > i && changed[canonicalChangedLineKey(lines[i])] { + marker = theme.accent.Render("▎") + } + b.WriteString(theme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) + b.WriteString(marker) + b.WriteString(line) + } + 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") + if omittedLines { + b.WriteString(theme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) + } else { + b.WriteString(theme.faint.Render("… (line content truncated at display limit)")) + } + } + return b.String() +} + +// peekRenderOnly looks up an already-formatted variant in memory. It performs +// strictly 0 I/O and 0 string formatting/allocations, guaranteeing O(1) instantaneous +// access on the View() drawing path. +func (c *fileViewRenderCache) peekRenderOnly(targetPath string, width int, changedFingerprint string, loadedSeq, desiredSeq uint64, loadedRev, reqRev uint64) (string, bool) { + if loadedSeq != desiredSeq || loadedRev < reqRev { + return "", false + } + c.mu.Lock() + elem, ok := c.items[targetPath] + if !ok { + c.mu.Unlock() + return "", false + } + entry := elem.Value.(*fileViewCachedEntry) + if entry.sourceRev < reqRev { + c.mu.Unlock() + return "", false + } + c.lru.MoveToFront(elem) + c.statsData.CacheHits++ + c.mu.Unlock() + + renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) + return entry.getRender(renderKey) +} + +// loadAndRender performs the bounded read, Chroma highlighting, and formatting. +// The bounded source is always read: mtime/size are kept only as a hint because +// `cp -p`, `rsync -t` and `tar -xp` can restore matching metadata over different +// bytes, so the content hash decides freshness. On a matching metadata+hash pair +// the highlight/format work is reused (re-formatting only for a new width +// variant). It is intended to run from a tea.Cmd / background worker, off the +// View path. +var errFileViewSuperseded = errors.New("file view request superseded") + +var ( + fileViewInsideLoad func() + fileViewBeforeCacheHitFormat func() + fileViewBeforeDiskRead func() + fileViewInsideDiskRead func() + fileViewBeforeHighlight func() + fileViewInsideHighlight func() + fileViewBeforeFormat func() + fileViewBeforeCacheCommit func() + fileViewHighlightMu sync.Mutex +) + +func fileViewSuperseded(liveSeq *atomic.Uint64, seq uint64) bool { + return liveSeq != nil && liveSeq.Load() != seq +} + +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) { + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + if fileViewInsideLoad != nil { + fileViewInsideLoad() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + stat, err := os.Stat(targetPath) + if err != nil { + c.evictPath(targetPath) + rendered := theme.faint.Render("Could not read file: " + err.Error()) + return rendered, err + } + if !stat.Mode().IsRegular() { + c.evictPath(targetPath) + readErr := fmt.Errorf("cannot display non-regular file %s (mode %s)", displayPath, stat.Mode().String()) + rendered := theme.faint.Render("Could not read file: " + readErr.Error()) + return rendered, readErr + } + + modTime := stat.ModTime() + size := stat.Size() + renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) + + c.mu.Lock() + if c.gen != reqGen { + c.mu.Unlock() + return "", errors.New("request superseded by cache invalidation") + } + c.statsData.DiskReads++ + c.mu.Unlock() + + if fileViewBeforeDiskRead != nil { + fileViewBeforeDiskRead() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + readRes := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq) + if readRes.err != nil && len(readRes.lines) == 0 { + c.evictPath(targetPath) + rendered := theme.faint.Render("Could not read file: " + readRes.err.Error()) + return rendered, readRes.err + } + sourceHash := fileViewSourceHash(readRes.lines) + + c.mu.Lock() + if c.gen != reqGen { + c.mu.Unlock() + return "", errors.New("request superseded by cache invalidation") + } + + curRequiredRev := c.requiredRevisionLocked(targetPath) + if reqSourceRev < curRequiredRev { + reqSourceRev = curRequiredRev + } + + if elem, ok := c.items[targetPath]; ok { + entry := elem.Value.(*fileViewCachedEntry) + forceReload := entry.sourceRev < reqSourceRev + if !forceReload && + entry.modTime.Equal(modTime) && + entry.size == size && + entry.displayPath == displayPath && + entry.sourceHash == sourceHash && + entry.truncated == readRes.truncated && + entry.omittedLines == readRes.omittedLines { + if fileViewSuperseded(liveSeq, seq) { + c.mu.Unlock() + return "", errFileViewSuperseded + } + c.statsData.CacheHits++ + c.lru.MoveToFront(elem) + c.mu.Unlock() + + if rendered, ok := entry.getRender(renderKey); ok { + return rendered, nil + } + + if fileViewBeforeCacheHitFormat != nil { + fileViewBeforeCacheHitFormat() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + // Re-format for the new width or changed markers using cached display and lines + rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width, theme) + if fileViewBeforeCacheCommit != nil { + fileViewBeforeCacheCommit() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + c.commitRenderPut(entry, renderKey, rendered) + return rendered, nil + } + } + + if fileViewSuperseded(liveSeq, seq) { + c.mu.Unlock() + return "", errFileViewSuperseded + } + + c.statsData.CacheMisses++ + c.mu.Unlock() + + cleanLines := make([]string, len(readRes.lines)) + for i, l := range readRes.lines { + cleanLines[i] = sanitizeRawFileLine(l) + } + + if fileViewBeforeHighlight != nil { + fileViewBeforeHighlight() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + display, ok := func() ([]string, bool) { + fileViewHighlightMu.Lock() + defer fileViewHighlightMu.Unlock() + if fileViewSuperseded(liveSeq, seq) { + return nil, false + } + if fileViewInsideHighlight != nil { + fileViewInsideHighlight() + if fileViewSuperseded(liveSeq, seq) { + return nil, false + } + } + c.mu.Lock() + c.statsData.HighlightCalls++ + c.mu.Unlock() + return highlightCodeForPathWithTheme(cleanLines, displayPath, 1<<20, nil, theme) + }() + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + if !ok || len(display) != len(cleanLines) { + display = cleanLines + } + + if fileViewBeforeFormat != nil { + fileViewBeforeFormat() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + rendered := formatFileViewLines(cleanLines, display, changed, readRes.truncated, readRes.omittedLines, width, theme) + + if fileViewBeforeCacheCommit != nil { + fileViewBeforeCacheCommit() + } + if fileViewSuperseded(liveSeq, seq) { + return "", errFileViewSuperseded + } + + sourceBytes := 0 + for _, l := range cleanLines { + sourceBytes += len(l) + } + for _, d := range display { + sourceBytes += len(d) + } + + entry := &fileViewCachedEntry{ + targetPath: targetPath, + displayPath: displayPath, + modTime: readRes.modTime, + size: readRes.size, + sourceRev: reqSourceRev, + sourceHash: sourceHash, + sourceBytes: sourceBytes, + lines: cleanLines, + display: display, + truncated: readRes.truncated, + omittedLines: readRes.omittedLines, + renderKeys: []string{renderKey}, + renders: map[string]string{renderKey: rendered}, + } + + c.mu.Lock() + if c.gen != reqGen { + c.mu.Unlock() + return "", errors.New("request superseded by cache invalidation") + } + if fileViewSuperseded(liveSeq, seq) { + c.mu.Unlock() + return "", errFileViewSuperseded + } + if c.maxEntries <= 0 || c.maxBytes <= 0 || entry.byteSize() > c.maxBytes { + c.statsData.SkippedOversized++ + c.removeItemLocked(targetPath) + c.mu.Unlock() + return rendered, nil + } + + if elem, ok := c.items[targetPath]; ok { + existing := elem.Value.(*fileViewCachedEntry) + if existing.modTime.After(readRes.modTime) && existing.sourceRev > reqSourceRev { + c.mu.Unlock() + return rendered, nil + } + c.removeItemLocked(targetPath) + } + c.items[targetPath] = c.lru.PushFront(entry) + c.retained += entry.byteSize() + c.evictOverflowLocked() + c.mu.Unlock() + + return rendered, nil +} + +// sanitizeRawFileLine strips or transforms raw control characters and terminal escape sequences +// when syntax highlighting is bypassed or unavailable, preventing terminal screen corruption. +func sanitizeRawFileLine(s string) string { + var out strings.Builder + for _, r := range s { + if r == '\t' { + out.WriteString(" ") + } else if r == '\r' || r == '\n' { + continue + } else if r < 32 || r == 0x7f || (r >= 0x80 && r <= 0x9f) { + if r == '\x1b' { + out.WriteString("^[") + } + } else { + out.WriteRune(r) + } + } + return out.String() +} + +func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { + fingerprint := changedLinesFingerprint(changed) + rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, fingerprint, c.generation(), zeroTheme, 0, nil, 0) + return rendered +} + +// fileViewLoadedMsg delivers the result of an asynchronous file read & render. +type fileViewLoadedMsg struct { + lifetimeToken [16]byte + seq uint64 + requiredSourceRev uint64 + generation int + targetPath string + displayPath string + width int + fingerprint string + rendered string + err error +} + +func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, seq uint64, gen int, reqRev uint64, theme tuiTheme, liveSeq *atomic.Uint64) tea.Cmd { + return func() tea.Msg { + if fileViewSuperseded(liveSeq, seq) { + return fileViewLoadedMsg{lifetimeToken: token, seq: seq, requiredSourceRev: reqRev, generation: gen, targetPath: targetPath, displayPath: displayPath, width: width, fingerprint: fingerprint, err: errFileViewSuperseded} + } + rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq) + return fileViewLoadedMsg{ + lifetimeToken: token, + seq: seq, + requiredSourceRev: reqRev, + generation: gen, + targetPath: targetPath, + displayPath: displayPath, + width: width, + fingerprint: fingerprint, + rendered: rendered, + err: err, + } + } +} + // 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). + active bool + path string // workspace-relative, as carried by changedFiles + mode int // fileViewDiff | fileViewFull parentScrollOffset int + // preservedScrollOffset holds the reader's latest scroll intent while an + // async reload swaps the body for the one-line loading placeholder. Scroll + // actions and prompt submission update it during the pending window, and + // handleFileViewLoaded reconciles it against the real body. + preservedScrollOffset int + // completedBodyLines is the rendered body height of the last accepted + // snapshot, including screen wrapping. A loading placeholder never updates it. + completedBodyLines int + + // View session lifetime identity (UUIDv7 RFC 9562 0-alloc) + lifetimeToken [16]byte + + // Monotonically advancing desired snapshot sequence & requested parameters + desiredSeq uint64 + requiredSourceRev uint64 + desiredWidth int + desiredFingerprint string + desiredGen int + + // Authoritative completed snapshot (only valid when loadedSeq == desiredSeq) + renderedContent string + loadedPath string + loadedWidth int + loadedGen int + loadedFingerprint string + loadedToken [16]byte + loadedSeq uint64 + loadedRev uint64 + loading bool + hasError bool + snapshotReady bool + liveSeq *atomic.Uint64 +} + +func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { + return m.startFileViewLoad(width, false) +} + +func (m model) startFileViewRefreshCmd(width int) (model, tea.Cmd) { + return m.startFileViewLoad(width, true) +} + +// refreshFileViewMarkers reloads an open full-file view so its gutter markers +// and fingerprint are recomputed against the current transcript after a session +// or transcript transition. No-op unless a full view is open. +func (m model) refreshFileViewMarkers() (model, tea.Cmd) { + if m.fileView.active && m.fileView.mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil +} + +func (m model) startFileViewLoad(width int, refreshSource bool) (model, tea.Cmd) { + if !m.fileView.active || m.fileView.mode != fileViewFull || m.fileView.path == "" { + return m, nil + } + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + if refreshSource { + rev := defaultFileViewCache.invalidatePath(target) + if rev > m.fileView.requiredSourceRev { + m.fileView.requiredSourceRev = rev + } else { + m.fileView.requiredSourceRev++ + } + } + curReq := defaultFileViewCache.requiredRevision(target) + if curReq > m.fileView.requiredSourceRev { + m.fileView.requiredSourceRev = curReq + } + + // Remember where the reader is before the body temporarily becomes the + // loading placeholder; syncChatScroll holds it and handleFileViewLoaded + // reconciles it against the real body. Intentional resets (new file, + // diff/full switch) zero chatScrollOffset before reaching here. + m.fileView.preservedScrollOffset = m.chatScrollOffset + m.fileView.desiredSeq++ + m.fileView.desiredWidth = width + m.fileView.loading = true + m.fileView.snapshotReady = false + if m.fileView.liveSeq == nil { + m.fileView.liveSeq = new(atomic.Uint64) + } + seq := m.fileView.desiredSeq + m.fileView.liveSeq.Store(seq) + token := m.fileView.lifetimeToken + gen := defaultFileViewCache.generation() + changed := m.fileViewChangedLines() + fingerprint := changedLinesFingerprint(changed) + m.fileView.desiredFingerprint = fingerprint + m.fileView.desiredGen = gen + reqRev := m.fileView.requiredSourceRev + theme := zeroTheme + return m, loadFileViewCmd(target, m.fileView.path, width, changed, fingerprint, token, seq, gen, reqRev, theme, m.fileView.liveSeq) } // openFileView activates the drill-in for path in diff mode. Opening from an @@ -47,16 +1114,36 @@ type fileViewState struct { // 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 { +func (m model) revokeFileViewRequest() { + if m.fileView.liveSeq != nil { + m.fileView.liveSeq.Add(1) + } +} + +func (m model) openFileView(path string) (model, tea.Cmd) { if m.fileView.active && m.fileView.path == path { - return m + return m, nil + } + if m.fileView.active { + m.revokeFileViewRequest() } if !m.fileView.active { m.fileView.parentScrollOffset = m.chatScrollOffset } + target := path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } m.fileView.active = true m.fileView.path = path m.fileView.mode = fileViewDiff + m.fileView.lifetimeToken = nextFileViewLifetimeToken() + m.fileView.renderedContent = "" + m.fileView.loadedToken = [16]byte{} + m.fileView.loadedSeq = 0 + m.fileView.loadedRev = 0 + m.fileView.requiredSourceRev = defaultFileViewCache.requiredRevision(target) + m.fileView.hasError = false // 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 { @@ -64,7 +1151,10 @@ func (m model) openFileView(path string) model { } m.chatScrollOffset = 0 m = m.clearHover() // bodyY numbering differs between the file body and the chat - return m + if m.fileView.mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil } // exitFileView deactivates the view and restores the chat scroll position. @@ -72,6 +1162,7 @@ func (m model) exitFileView() model { if !m.fileView.active { return m } + m.revokeFileViewRequest() m.chatScrollOffset = m.fileView.parentScrollOffset m.fileView = fileViewState{} m = m.clearHover() @@ -80,13 +1171,75 @@ func (m model) exitFileView() model { // 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 { +func (m model) setFileViewMode(mode int) (model, tea.Cmd) { if !m.fileView.active || m.fileView.mode == mode { - return m + return m, nil + } + if m.fileView.mode == fileViewFull && mode != fileViewFull { + m.revokeFileViewRequest() } m.fileView.mode = mode m.chatScrollOffset = 0 - return m + if mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil +} + +func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { + if !m.fileView.active || m.fileView.mode != fileViewFull { + return m, nil + } + if errors.Is(msg.err, errFileViewSuperseded) { + return m, nil + } + // 1. Session lifetime identity match + if m.fileView.lifetimeToken != msg.lifetimeToken || m.fileView.path != msg.displayPath { + return m, nil + } + // 2. Exact desired snapshot match: reject superseded / out-of-order completions without any side effects + if msg.seq != m.fileView.desiredSeq || + msg.width != m.fileView.desiredWidth || + msg.fingerprint != m.fileView.desiredFingerprint { + // Out-of-order or obsolete sequence: drop without modifying state or triggering retries + return m, nil + } + + // 3. Current sequence with invalid theme generation: request a retry inheriting current source revision requirement + if msg.generation != defaultFileViewCache.generation() { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + + // 4. Source revision monotonic check: if required revision was advanced while in flight, request a refresh + if msg.requiredSourceRev < m.fileView.requiredSourceRev { + return m.startFileViewRefreshCmd(m.chatColumnWidth()) + } + + m.fileView.loading = false + m.fileView.snapshotReady = true + m.fileView.renderedContent = msg.rendered + m.fileView.loadedPath = msg.displayPath + m.fileView.loadedWidth = msg.width + m.fileView.loadedGen = msg.generation + m.fileView.loadedFingerprint = msg.fingerprint + m.fileView.loadedToken = msg.lifetimeToken + m.fileView.loadedSeq = msg.seq + m.fileView.loadedRev = msg.requiredSourceRev + m.fileView.hasError = (msg.err != nil) + // Reconcile the held reading offset against the real body now that the + // loading placeholder is gone. Apply the height delta exactly once to + // preserve the absolute position, while offset zero keeps following the tail. + current, maxOffset := m.chatScrollMetrics() + if m.fileView.preservedScrollOffset > 0 { + m.chatScrollOffset = clampInt(m.fileView.preservedScrollOffset+current-m.fileView.completedBodyLines, 0, maxOffset) + if m.chatScrollOffset > 0 { + m.chatBodyLines = current + } else { + m.chatBodyLines = 0 + } + } + m.fileView.completedBodyLines = current + return m, nil } // fileViewNavBar renders the single-line header shown in place of the pinned @@ -165,69 +1318,28 @@ func (m model) renderFileViewDiff(width int) string { // 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). +// It is strictly non-blocking and performs O(1) lookup without invoking formatters. func (m model) renderFileViewFull(width int) string { + if m.fileView.hasError { + return m.fileView.renderedContent + } target := m.fileView.path if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, 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()) + if m.fileView.snapshotReady && + m.fileView.loadedPath == m.fileView.path && + m.fileView.loadedSeq == m.fileView.desiredSeq && + m.fileView.loadedRev >= m.fileView.requiredSourceRev && + m.fileView.loadedGen == defaultFileViewCache.generation() && + m.fileView.loadedToken == m.fileView.lifetimeToken { + defaultFileViewCache.recordHit() + return m.fileView.renderedContent } - 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 + if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.desiredFingerprint, m.fileView.loadedSeq, m.fileView.desiredSeq, m.fileView.loadedRev, m.fileView.requiredSourceRev); ok { + return cached } - - 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 { - // 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("… more lines (file truncated at %d for display)", len(lines)))) - } - return b.String() + return zeroTheme.faint.Render(fileViewLoadingPlaceholder) } // fileViewChangedLines collects the trimmed text of every line the session's @@ -242,10 +1354,67 @@ func (m model) fileViewChangedLines() map[string]bool { if !strings.HasPrefix(line, "+") || strings.HasPrefix(line, "+++") { continue } - if text := strings.TrimSpace(strings.TrimPrefix(line, "+")); len(text) >= 4 { - changed[text] = true + raw := strings.TrimPrefix(line, "+") + if key := canonicalChangedLineKey(raw); len(key) >= 4 { + changed[key] = true } } } return changed } + +// toolResultMayMutateUnknownScope reports tools that can change workspace files +// but do not reliably report the affected paths (ChangedFiles). Their completion +// must trigger the conservative unknown-scope refresh so an open file view +// cannot keep rendering a stale snapshot. Task/TaskOutput successful results are +// suppressed from the transcript, so their completion boundaries carry this +// refresh separately. +func toolResultMayMutateUnknownScope(name string) bool { + switch name { + case "bash", "exec_command", "terminal_session", "write_stdin", + "swarm_collect", "swarm_status", "Task", "TaskOutput": + return true + } + return false +} + +// invalidateFileViewForUnknownMutation performs the conservative refresh used +// when a completed action may have changed workspace files but no exact path is +// known: purge the shared render cache and, when a full file view is open, +// schedule an async reload so its snapshot cannot stay stale. +func (m model) invalidateFileViewForUnknownMutation() (model, tea.Cmd) { + defaultFileViewCache.invalidateUnknownScope() + var selfCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, selfCmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + } + var peerCmd tea.Cmd + m, peerCmd = m.refreshHiddenParentFileView() + return m, batchCommands(selfCmd, peerCmd) +} + +// recoverInvalidatedFileView schedules a snapshot for a surface whose shared +// cache generation changed, or whose open path's required revision advanced, +// while another BTW surface handled the invalidation. Path-reporting tools bump +// only that revision: outside a git workspace no sweep follows, so the other +// surface must notice the revision itself and reload immediately. +func (m model) recoverInvalidatedFileView() (model, tea.Cmd) { + if !m.fileView.active || m.fileView.mode != fileViewFull { + return m, nil + } + generation := m.fileView.loadedGen + if m.fileView.loading { + generation = m.fileView.desiredGen + } + if generation != defaultFileViewCache.generation() { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + if defaultFileViewCache.requiredRevision(target) > m.fileView.requiredSourceRev { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil +} diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index caf4f4a49..d70bf1d16 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -1,21 +1,50 @@ package tui import ( + "context" "encoding/json" + "errors" "fmt" + "go/ast" + "go/parser" + "go/token" "os" "path/filepath" "strconv" "strings" + "sync" + "sync/atomic" "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/Twigpine/zero/internal/agent" + "github.com/Twigpine/zero/internal/execution" "github.com/Twigpine/zero/internal/sessions" "github.com/Twigpine/zero/internal/tools" ) +func testOpenFile(m model, path string) model { + next, cmd := m.openFileView(path) + if cmd != nil { + msg := cmd() + updated, _ := next.Update(msg) + return updated.(model) + } + return next +} + +func testSetMode(m model, mode int) model { + next, cmd := m.setFileViewMode(mode) + if cmd != nil { + msg := cmd() + updated, _ := next.Update(msg) + return updated.(model) + } + return next +} + // 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). @@ -23,7 +52,7 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { m := filesPanelTestModel() m.chatScrollOffset = 12 - m = m.openFileView("web/app.js") + 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) } @@ -32,7 +61,7 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { } m.chatScrollOffset = 5 // scrolled within the file body - m = m.openFileView("internal/tui/sidebar.go") + 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) } @@ -47,15 +76,23 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { // 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") + m, _ = m.openFileView("web/app.js") - updated, _ := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + updated, cmd := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) m = updated.(model) + if cmd != nil { + updated, _ = m.Update(cmd()) + 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"}) + updated, cmd = m.Update(tea.KeyPressMsg{Code: 'd', Text: "d"}) m = updated.(model) + if cmd != nil { + updated, _ = m.Update(cmd()) + m = updated.(model) + } if m.fileView.mode != fileViewDiff { t.Fatal("d should switch back to diff mode") } @@ -81,7 +118,7 @@ func TestFileViewEscAndModeKeys(t *testing.T) { // placeholder. func TestFileViewDiffBody(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("internal/tui/sidebar.go") + 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) @@ -111,8 +148,8 @@ func TestFileViewFullBody(t *testing.T) { detail: "+let a = 1", changedFiles: []string{"app.js"}, }) - m = m.openFileView("app.js") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "app.js") + m = testSetMode(m, fileViewFull) body := m.renderFileViewFull(78) plain := plainRender(t, body) @@ -131,6 +168,11 @@ func TestFileViewFullBody(t *testing.T) { } m.fileView.path = "gone.js" + m, cmd := m.startFileViewLoadCmd(78) + if cmd != nil { + updated, _ := m.Update(cmd()) + m = updated.(model) + } 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) } @@ -142,7 +184,7 @@ func TestFileViewFullBody(t *testing.T) { // relies on. func TestFileViewSwapsTranscriptBody(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("internal/tui/sidebar.go") + m, _ = m.openFileView("internal/tui/sidebar.go") items := m.transcriptBodyItems(m.chatColumnWidth(), "", false) if len(items) != 1 { @@ -177,7 +219,7 @@ func TestSidebarAgentClickIsIgnoredWithoutRail(t *testing.T) { 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") + m, _ = m.openFileView("web/app.js") width := sidebarWidth(m.width) agents := m.sidebarAgentSelectables(width) @@ -246,11 +288,11 @@ func TestResumedFileEditUsesPersistedDisplayPreview(t *testing.T) { // 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 = testOpenFile(m, "web/app.js") + m = testSetMode(m, fileViewFull) m.chatScrollOffset = 7 // scrolled within the file body - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if m.fileView.mode != fileViewFull { t.Fatal("re-opening the same file must keep full mode") } @@ -258,7 +300,7 @@ func TestOpenFileViewSamePathIsNoOp(t *testing.T) { 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") + 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) } @@ -279,7 +321,7 @@ func TestFileViewFullBodyTruncatesLongFile(t *testing.T) { m := filesPanelTestModel() m.cwd = dir m.gitTouched = []gitSweepFile{{path: "big.txt"}} - m = m.openFileView("big.txt") + m = testOpenFile(m, "big.txt") plain := plainRender(t, m.renderFileViewFull(80)) lines := strings.Split(plain, "\n") @@ -297,7 +339,7 @@ func TestFileViewFullBodyTruncatesLongFile(t *testing.T) { func TestDetailedTranscriptClosesFileView(t *testing.T) { m := filesPanelTestModel() m.altScreen = true - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if !m.fileView.active { t.Fatal("sanity check: openFileView should activate the file view") @@ -324,7 +366,7 @@ func TestDetailedTranscriptClosesFileView(t *testing.T) { func TestDetailedTranscriptStaysClosedOnSecondToggle(t *testing.T) { m := filesPanelTestModel() m.altScreen = true - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") updated, _ := m.Update(testKeyCtrl('o')) m = updated.(model) @@ -345,7 +387,7 @@ func TestDetailedTranscriptStaysClosedOnSecondToggle(t *testing.T) { // (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, _ = m.openFileView("web/app.js") m.pendingPermission = &pendingPermissionPrompt{ request: agent.PermissionRequest{ToolName: "write_file"}, decide: func(agent.PermissionDecision) {}, @@ -362,3 +404,3822 @@ func TestFileViewKeysDeferToBlockingModal(t *testing.T) { t.Fatal("Esc with a permission prompt up must not exit the file view") } } + +// TestFileViewRepeatedViewNoDiskIOOrHighlighting proves that repeated calls +// to render the full file view do not perform disk I/O or Chroma syntax +// highlighting after the initial load, and that modifying the file on disk +// properly invalidates and triggers a reload. +func TestFileViewRepeatedViewNoDiskIOOrHighlighting(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "sample.go") + content := "package main\n\nfunc main() {\n\tprintln(\"hello world\")\n}\n" + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "sample.go") + m = testSetMode(m, fileViewFull) + + // First render: Misses cache during testSetMode cmd execution, performed 1 disk read and 1 highlight call + firstRender := m.renderFileViewFull(80) + if !strings.Contains(firstRender, "hello world") { + t.Fatalf("first render missing content: %s", firstRender) + } + + statsAfterFirst := fileViewCacheStatsForTest() + if statsAfterFirst.DiskReads != 1 { + t.Fatalf("expected 1 disk read on initial view, got %d", statsAfterFirst.DiskReads) + } + if statsAfterFirst.HighlightCalls != 1 { + t.Fatalf("expected 1 highlight call on initial view, got %d", statsAfterFirst.HighlightCalls) + } + + // Repeated renders (e.g. 10 frames during typing/scrolling/resize) + for i := 0; i < 10; i++ { + rendered := m.renderFileViewFull(80) + if rendered != firstRender { + t.Fatalf("subsequent render %d mismatch", i) + } + } + + statsAfterRepeated := fileViewCacheStatsForTest() + if statsAfterRepeated.DiskReads != 1 { + t.Fatalf("repeated View calls must not trigger disk reads, got %d", statsAfterRepeated.DiskReads) + } + if statsAfterRepeated.HighlightCalls != 1 { + t.Fatalf("repeated View calls must not trigger Chroma highlighting, got %d", statsAfterRepeated.HighlightCalls) + } + if statsAfterRepeated.CacheHits != statsAfterFirst.CacheHits+10 { + t.Fatalf("expected 10 additional cache hits, got %d (before: %d)", statsAfterRepeated.CacheHits, statsAfterFirst.CacheHits) + } + + // Same byte length as `content`, so only mtime can invalidate the entry. + newContent := "package main\n\nfunc main() {\n\tprintln(\"HELLO WORLD\")\n}\n" + if len(newContent) != len(content) { + t.Fatalf("test setup: newContent length %d must match original length %d", len(newContent), len(content)) + } + if err := os.WriteFile(filePath, []byte(newContent), 0o644); err != nil { + t.Fatal(err) + } + future := time.Now().Add(time.Hour) + if err := os.Chtimes(filePath, future, future); err != nil { + t.Fatal(err) + } + + // Re-trigger load command after file update + m, cmd := m.startFileViewLoadCmd(80) + if cmd != nil { + updated, _ := m.Update(cmd()) + m = updated.(model) + } + + updatedRender := m.renderFileViewFull(80) + if !strings.Contains(updatedRender, "HELLO WORLD") { + t.Fatalf("expected updated content after disk mutation, got: %s", updatedRender) + } + + statsAfterUpdate := fileViewCacheStatsForTest() + if statsAfterUpdate.DiskReads != 2 { + t.Fatalf("expected 2 disk reads after file change, got %d", statsAfterUpdate.DiskReads) + } + if statsAfterUpdate.HighlightCalls != 2 { + t.Fatalf("expected 2 highlight calls after file change, got %d", statsAfterUpdate.HighlightCalls) + } +} + +// TestFileViewMaxBytesBudgetTruncation verifies that files exceeding the +// total byte budget (fileViewMaxBytes) are truncated and display the trailer. +func TestFileViewMaxBytesBudgetTruncation(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_bytes.txt") + + // Generate a file with total size ~1.5 MB (> fileViewMaxBytes of 1 MiB) + line := strings.Repeat("a", 500) + "\n" + numLines := (fileViewMaxBytes / 500) + 100 + var sb strings.Builder + for i := 0; i < numLines; i++ { + sb.WriteString(line) + } + if err := os.WriteFile(filePath, []byte(sb.String()), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "giant_bytes.txt") + m = testSetMode(m, fileViewFull) + + body := m.renderFileViewFull(80) + plain := plainRender(t, body) + + if !strings.Contains(plain, "truncated") { + t.Fatalf("expected truncation trailer for file exceeding byte budget, got:\n%s", plain) + } + + renderedLines := strings.Split(plain, "\n") + if len(renderedLines) >= numLines { + t.Fatalf("rendered line count %d should be strictly less than total file lines %d", len(renderedLines), numLines) + } +} + +// TestFileViewMaxLineBytesBudgetTruncation verifies that single overlong lines +// exceeding fileViewMaxLineBytes are clamped without crashing or unbounded memory. +func TestFileViewMaxLineBytesBudgetTruncation(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_line.js") + + // Generate a single overlong line of 50,000 bytes (> fileViewMaxLineBytes of 4096) + giantLine := "let data = \"" + strings.Repeat("x", 50000) + "\";\nlet next = 1;\n" + if err := os.WriteFile(filePath, []byte(giantLine), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "giant_line.js") + m = testSetMode(m, fileViewFull) + + body := m.renderFileViewFull(80) + plain := plainRender(t, body) + + if !strings.Contains(plain, "let next = 1") { + t.Fatalf("expected next line to be readable after overlong line truncation, got:\n%s", plain) + } + if !strings.Contains(plain, "truncated") { + t.Fatalf("expected truncation trailer for overlong line, got:\n%s", plain) + } +} + +// TestFileViewCacheEviction verifies that the LRU cache caps entry count to +// defaultFileViewCacheMaxEntries. +func TestFileViewCacheEviction(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + numFiles := defaultFileViewCacheMaxEntries + 10 + for i := 0; i < numFiles; i++ { + fname := fmt.Sprintf("file_%d.txt", i) + if err := os.WriteFile(filepath.Join(dir, fname), []byte(fmt.Sprintf("content %d\n", i)), 0o644); err != nil { + t.Fatal(err) + } + } + + m := filesPanelTestModel() + m.cwd = dir + for i := 0; i < numFiles; i++ { + fname := fmt.Sprintf("file_%d.txt", i) + m = testOpenFile(m, fname) + m = testSetMode(m, fileViewFull) + _ = m.renderFileViewFull(80) + } + + defaultFileViewCache.mu.Lock() + cachedCount := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if cachedCount > defaultFileViewCacheMaxEntries { + t.Fatalf("cache size %d exceeded maxEntries %d", cachedCount, defaultFileViewCacheMaxEntries) + } +} + +// TestFileViewClearOnThemeChange verifies that switching themes clears the +// file view cache so updated palette styles are applied. +func TestFileViewClearOnThemeChange(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "code.go") + if err := os.WriteFile(filePath, []byte("package main\nfunc main() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "code.go") + m = testSetMode(m, fileViewFull) + _ = m.renderFileViewFull(80) + + defaultFileViewCache.mu.Lock() + entriesBefore := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if entriesBefore == 0 { + t.Fatal("expected cached entries before theme switch") + } + + applyTheme(themeLight, false) + + defaultFileViewCache.mu.Lock() + entriesAfter := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if entriesAfter != 0 { + t.Fatalf("expected cache to be cleared after theme change, got %d entries", entriesAfter) + } +} + +// TestReadFileViewBounded_GiantSingleLineStopsAtBudget verifies that reading a +// multi-megabyte physical line without newlines stops immediately at maxTotalBytes +// rather than reading through to EOF or loading the entire line into memory. +func TestReadFileViewBounded_GiantSingleLineStopsAtBudget(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_single_line.txt") + + // 5 MiB single line without any newlines + totalSize := 5 * 1024 * 1024 + giantContent := strings.Repeat("A", totalSize) + if err := os.WriteFile(filePath, []byte(giantContent), 0o644); err != nil { + t.Fatal(err) + } + + maxTotalBytes := 1 << 20 // 1 MiB budget + maxLineBytes := 4096 // 4 KiB line cap + maxLines := 4000 + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if !res.truncated { + t.Fatal("expected truncated=true when reading 5 MiB single line with 1 MiB budget") + } + + if len(res.lines) != 1 { + t.Fatalf("expected exactly 1 truncated line, got %d", len(res.lines)) + } + + if len(res.lines[0]) > maxLineBytes { + t.Fatalf("retained line length %d exceeded per-line cap %d", len(res.lines[0]), maxLineBytes) + } +} + +// TestReadFileViewBounded_ExactMaxBytesNotTruncated verifies that a file exactly +// equal in size to maxTotalBytes without extra unread bytes is read completely +// without an erroneous truncation flag or trailer. +func TestReadFileViewBounded_ExactMaxBytesNotTruncated(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "exact_budget.txt") + + maxTotalBytes := 1024 * 1024 // 1 MiB + maxLineBytes := 4096 + maxLines := 4000 + + // Construct exactly 1024 * 1024 bytes with 512 lines of 2048 bytes (2047 chars + '\n') + lineLen := 2048 + numLines := maxTotalBytes / lineLen + remainder := maxTotalBytes % lineLen + + var sb strings.Builder + for i := 0; i < numLines; i++ { + sb.WriteString(strings.Repeat("B", lineLen-1) + "\n") + } + if remainder > 0 { + sb.WriteString(strings.Repeat("C", remainder)) + } + + content := []byte(sb.String()) + if len(content) != maxTotalBytes { + t.Fatalf("generated content size %d != %d", len(content), maxTotalBytes) + } + + if err := os.WriteFile(filePath, content, 0o644); err != nil { + t.Fatal(err) + } + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if res.truncated { + t.Fatal("expected truncated=false for file exactly matching maxTotalBytes with no trailing data") + } + + totalReadLen := 0 + for _, l := range res.lines { + totalReadLen += len(l) + } + if totalReadLen == 0 { + t.Fatal("expected lines to be populated") + } +} + +// TestReadFileViewBounded_ExactMaxBytesUnterminatedLineTruncated verifies that a single +// unterminated physical line of exactly maxTotalBytes is correctly marked truncated=true +// when the line length exceeds maxLineBytes. +func TestReadFileViewBounded_ExactMaxBytesUnterminatedLineTruncated(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "exact_single_line_unterminated.txt") + + maxTotalBytes := 1024 * 1024 // 1 MiB + maxLineBytes := 4096 + maxLines := 4000 + + // 1 MiB continuous single line without any newlines + content := []byte(strings.Repeat("X", maxTotalBytes)) + if err := os.WriteFile(filePath, content, 0o644); err != nil { + t.Fatal(err) + } + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if !res.truncated { + t.Fatal("expected truncated=true for 1 MiB single unterminated line clipped at maxLineBytes") + } + + if len(res.lines) != 1 { + t.Fatalf("expected exactly 1 line, got %d", len(res.lines)) + } + if len(res.lines[0]) > maxLineBytes { + t.Fatalf("retained line length %d exceeded maxLineBytes %d", len(res.lines[0]), maxLineBytes) + } +} + +func TestReadFileViewBounded_SourceByteBudgetCountsDelimiters(t *testing.T) { + tests := []struct { + name string + content string + maxLines int + maxLineBytes int + maxTotalBytes int + wantLines []string + wantTrunc bool + wantOmitted bool + wantTrailer string + }{ + { + name: "empty file", + content: "", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 1, + wantLines: nil, + }, + { + name: "budget 1 two LF empty lines", + content: "\n\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 1, + wantLines: []string{""}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "single LF exact budget", + content: "\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 1, + wantLines: []string{""}, + }, + { + name: "LF content line exact budget", + content: "ab\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 3, + wantLines: []string{"ab"}, + }, + { + name: "LF second line one byte over", + content: "ab\ncd\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 3, + wantLines: []string{"ab"}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "CRLF two empty lines budget 2", + content: "\r\n\r\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 2, + wantLines: []string{""}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "CRLF exact budget", + content: "ab\r\n", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 4, + wantLines: []string{"ab"}, + }, + { + name: "CRLF one byte over", + content: "ab\r\ncd", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 3, + wantLines: []string{"ab"}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "exact budget unterminated last line", + content: "abc", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 3, + wantLines: []string{"abc"}, + }, + { + name: "unterminated last line one byte over", + content: "abcd", + maxLines: 4000, + maxLineBytes: 4096, + maxTotalBytes: 3, + wantLines: []string{"abc"}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "overlong physical line clipped then next line shown", + content: strings.Repeat("x", 20) + "\nnext\n", + maxLines: 4000, + maxLineBytes: 8, + maxTotalBytes: 100, + wantLines: []string{strings.Repeat("x", 8), "next"}, + wantTrunc: true, + wantTrailer: "line content truncated", + }, + { + name: "overlong physical line newline counts against byte budget", + content: strings.Repeat("x", 10) + "\ny\n", + maxLines: 4000, + maxLineBytes: 4, + maxTotalBytes: 11, + wantLines: []string{strings.Repeat("x", 4)}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "line count cap with leftover", + content: "a\nb\nc\nd\n", + maxLines: 2, + maxLineBytes: 4096, + maxTotalBytes: 100, + wantLines: []string{"a", "b"}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + { + name: "line count cap exact file", + content: "a\nb\n", + maxLines: 2, + maxLineBytes: 4096, + maxTotalBytes: 100, + wantLines: []string{"a", "b"}, + }, + { + name: "byte budget binds before line count cap on empty LF lines", + content: "\n\n\n\n", + maxLines: 10, + maxLineBytes: 4096, + maxTotalBytes: 2, + wantLines: []string{"", ""}, + wantTrunc: true, + wantOmitted: true, + wantTrailer: "more lines", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "input.txt") + if err := os.WriteFile(path, []byte(tc.content), 0o644); err != nil { + t.Fatal(err) + } + + res := readFileViewBounded(path, tc.maxLines, tc.maxLineBytes, tc.maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + if res.truncated != tc.wantTrunc { + t.Fatalf("truncated=%v, want %v (lines=%q)", res.truncated, tc.wantTrunc, res.lines) + } + if res.omittedLines != tc.wantOmitted { + t.Fatalf("omittedLines=%v, want %v (lines=%q)", res.omittedLines, tc.wantOmitted, res.lines) + } + if len(res.lines) != len(tc.wantLines) { + t.Fatalf("retained %d lines %q, want %d %q", len(res.lines), res.lines, len(tc.wantLines), tc.wantLines) + } + for i := range tc.wantLines { + if res.lines[i] != tc.wantLines[i] { + t.Fatalf("line %d = %q, want %q", i, res.lines[i], tc.wantLines[i]) + } + } + + plain := plainRender(t, formatFileViewLines(res.lines, res.lines, nil, res.truncated, res.omittedLines, 80, zeroTheme)) + if tc.wantTrailer == "" { + if strings.Contains(plain, "truncated") { + t.Fatalf("expected no trailer, got %q", plain) + } + } else if !strings.Contains(plain, tc.wantTrailer) { + t.Fatalf("expected trailer %q in %q", tc.wantTrailer, plain) + } + }) + } +} + +// TestFileViewCache_RenderVariantsBoundedUnderResize verifies that varying width +// and changed-line fingerprints cannot grow an entry's renders map beyond +// fileViewMaxRenderVariants, even under concurrent access from multiple goroutines. +func TestFileViewCache_RenderVariantsBoundedUnderResize(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "resize_test.go") + if err := os.WriteFile(filePath, []byte("package main\n\nfunc main() {\n\tprintln(\"hello\")\n}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "resize_test.go") + m = testSetMode(m, fileViewFull) + + // Execute mixed-width getOrRender calls concurrently from multiple goroutines + var wg sync.WaitGroup + workers := 8 + callsPerWorker := 20 + + for w := 0; w < workers; w++ { + wg.Add(1) + go func(workerID int) { + defer wg.Done() + for i := 0; i < callsPerWorker; i++ { + width := 40 + ((workerID*callsPerWorker + i) % 50) + changed := map[string]bool{} + if width%2 == 0 { + changed[fmt.Sprintf("marker_%d_%d", workerID, width)] = true + } + _ = defaultFileViewCache.getOrRender(filePath, "resize_test.go", width, changed) + } + }(w) + } + wg.Wait() + + defaultFileViewCache.mu.Lock() + elem, ok := defaultFileViewCache.items[filePath] + defaultFileViewCache.mu.Unlock() + + if !ok || elem == nil { + t.Fatal("expected cached entry for file") + } + + entry := elem.Value.(*fileViewCachedEntry) + entry.rendersMu.RLock() + variantCount := len(entry.renders) + keyCount := len(entry.renderKeys) + entry.rendersMu.RUnlock() + + if variantCount > fileViewMaxRenderVariants { + t.Fatalf("variant count %d exceeded maximum limit %d", variantCount, fileViewMaxRenderVariants) + } + if keyCount > fileViewMaxRenderVariants { + t.Fatalf("renderKeys count %d exceeded maximum limit %d", keyCount, fileViewMaxRenderVariants) + } +} + +// TestFileViewAsyncCacheMissLifecycle exercises the cache-miss lifecycle through +// the actual View/Update boundary: +// 1. Initial full-mode activation returns a command while View() renders the +// loading placeholder without performing disk I/O or Chroma work. +// 2. The command executes asynchronously and returns fileViewLoadedMsg. +// 3. Update() applies the message to the model. +// 4. View() renders the loaded, formatted content. +func TestFileViewAsyncCacheMissLifecycle(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "async_sample.go") + content := "package main\n\nfunc AsyncWork() string {\n\treturn \"done\"\n}\n" + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Step 1: Open file with no edit rows (opens in full mode directly with async cmd) + m, cmd := m.openFileView("async_sample.go") + if cmd == nil { + t.Fatal("expected async load command on cache miss") + } + + // View before cmd completes must render loading placeholder with 0 disk I/O or highlighting + initialView := m.renderFileViewFull(80) + if !strings.Contains(initialView, "Loading…") { + t.Fatalf("expected loading placeholder before command completes, got:\n%s", initialView) + } + statsBefore := fileViewCacheStatsForTest() + if statsBefore.DiskReads != 0 || statsBefore.HighlightCalls != 0 { + t.Fatalf("View() must not stat/read/highlight directly: %+v", statsBefore) + } + + // Step 2: Execute command asynchronously + msg := cmd() + loadedMsg, ok := msg.(fileViewLoadedMsg) + if !ok { + t.Fatalf("expected fileViewLoadedMsg, got %T", msg) + } + if loadedMsg.err != nil { + t.Fatalf("unexpected load error: %v", loadedMsg.err) + } + + // Step 3: Update model with loaded message + updated, _ := m.Update(loadedMsg) + m = updated.(model) + + // Step 4: View now renders the loaded content + loadedView := m.renderFileViewFull(80) + if !strings.Contains(loadedView, "AsyncWork") { + t.Fatalf("expected loaded content in view, got:\n%s", loadedView) + } + statsAfter := fileViewCacheStatsForTest() + if statsAfter.DiskReads != 1 || statsAfter.HighlightCalls != 1 { + t.Fatalf("expected exactly 1 disk read and 1 highlight call, got: %+v", statsAfter) + } + + // Also verify switching from diff mode to full mode triggers the cmd + appFile := filepath.Join(dir, "web", "app.js") + if err := os.MkdirAll(filepath.Join(dir, "web"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(appFile, []byte("let webApp = true;\n"), 0o644); err != nil { + t.Fatal(err) + } + mDiff, cmdDiff := m.openFileView("web/app.js") + if cmdDiff != nil || mDiff.fileView.mode != fileViewDiff { + t.Fatalf("file with edit cards must open in diff mode with nil cmd: mode=%d, cmd=%v", mDiff.fileView.mode, cmdDiff) + } + mFull, cmdFull := mDiff.setFileViewMode(fileViewFull) + if cmdFull == nil || mFull.fileView.mode != fileViewFull { + t.Fatal("switching to full mode must return load command") + } + updated, _ = mFull.Update(cmdFull()) + mFull = updated.(model) + if !strings.Contains(mFull.renderFileViewFull(80), "webApp") { + t.Fatalf("expected loaded webApp content, got: %s", mFull.renderFileViewFull(80)) + } +} + +// TestFileViewAsyncDiscardSupersededResult verifies that if a user switches files +// or exits the view while an async load is in flight, the completed message from +// the old file is safely discarded and does not overwrite the active view. +func TestFileViewAsyncDiscardSupersededResult(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "fileA.txt") + fileB := filepath.Join(dir, "fileB.txt") + if err := os.WriteFile(fileA, []byte("Content of File A\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(fileB, []byte("Content of File B\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Start loading File A + m, cmdA := m.openFileView("fileA.txt") + if cmdA == nil { + t.Fatal("expected cmd for fileA") + } + + // User switches to File B before cmdA returns + m, cmdB := m.openFileView("fileB.txt") + if cmdB == nil { + t.Fatal("expected cmd for fileB") + } + + // Now cmdA completes and its message is dispatched + msgA := cmdA() + updated, _ := m.Update(msgA) + m = updated.(model) + + // File A's result must be discarded because active file is File B + viewWhileB := m.renderFileViewFull(80) + if strings.Contains(viewWhileB, "Content of File A") { + t.Fatalf("stale File A result must not paint over File B: %s", viewWhileB) + } + + // Now cmdB completes and is dispatched + msgB := cmdB() + updated, _ = m.Update(msgB) + m = updated.(model) + + viewFinal := m.renderFileViewFull(80) + if !strings.Contains(viewFinal, "Content of File B") { + t.Fatalf("expected File B content, got: %s", viewFinal) + } +} + +// TestFileViewAsyncDiscardOnModeSwitchOrExit verifies that if a view exits or +// switches back to diff mode, completed async loads are safely ignored. +func TestFileViewAsyncDiscardOnModeSwitchOrExit(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "discard_mode.txt") + if err := os.WriteFile(fileA, []byte("Some content\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m, cmdA := m.openFileView("discard_mode.txt") + if cmdA == nil { + t.Fatal("expected cmd") + } + + // Exit file view before command returns + m = m.exitFileView() + if m.fileView.active { + t.Fatal("view should be inactive") + } + + // Now deliver the message + msgA := cmdA() + updated, _ := m.Update(msgA) + m = updated.(model) + + if m.fileView.active { + t.Fatal("discarded message must not re-activate file view") + } +} + +// TestFileViewAsyncDiscardOnThemeInvalidation verifies that if a theme switch occurs +// while an async load is in flight, the old theme's completion message is discarded +// and a fresh load command for the new theme generation is triggered. +func TestFileViewAsyncDiscardOnThemeInvalidation(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "theme_test.go") + if err := os.WriteFile(fileA, []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m, cmd := m.openFileView("theme_test.go") + if cmd == nil { + t.Fatal("expected cmd") + } + + // Invalidate cache by switching theme before cmd completes + applyTheme(themeLight, false) + + // Now old cmd completes with stale generation + msg := cmd() + updated, retryCmd := m.Update(msg) + m = updated.(model) + + // The message must have been rejected due to generation mismatch + if m.fileView.renderedContent != "" { + t.Fatalf("expected empty renderedContent after invalidation, got %q", m.fileView.renderedContent) + } + if retryCmd == nil { + t.Fatal("expected retry command for new generation after invalidation") + } + + // Executing the retry command loads the file under the new generation + retryMsg := retryCmd() + updated, _ = m.Update(retryMsg) + m = updated.(model) + + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatalf("expected file content loaded after retry, got: %s", m.renderFileViewFull(80)) + } + if m.fileView.loadedGen != defaultFileViewCache.generation() { + t.Fatalf("loadedGen %d != cache generation %d", m.fileView.loadedGen, defaultFileViewCache.generation()) + } +} + +// TestFileViewThemeSwitchWhileLoaded verifies that switching themes invalidates +// the loaded generation and allows immediate reloading for the new palette. +func TestFileViewThemeSwitchWhileLoaded(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "switch_test.go") + if err := os.WriteFile(fileA, []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m = testOpenFile(m, "switch_test.go") + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatal("file should be loaded initially") + } + + // Switch theme: generation advances, cache is cleared + applyTheme(themeLight, false) + + // renderFileViewFull must not return the stale dark-theme content + staleCheck := m.renderFileViewFull(80) + if strings.Contains(staleCheck, "package") { + t.Fatalf("stale renderedContent must not be rendered after generation increment: %s", staleCheck) + } + + // Starting a new load command re-populates for the new theme + m, reloadCmd := m.startFileViewLoadCmd(80) + if reloadCmd == nil { + t.Fatal("expected reload command") + } + updated, _ := m.Update(reloadCmd()) + m = updated.(model) + + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatalf("expected reloaded content for new theme, got: %s", m.renderFileViewFull(80)) + } +} + +// TestFileViewLifecycle_OpenToLoad tests the full model update flow from open to async load completion. +func TestFileViewLifecycle_OpenToLoad(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "app.go") + if err := os.WriteFile(filePath, []byte("package app\nfunc Run() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Open file via model action + m, cmd := m.openFileView("app.go") + if !m.fileView.active || m.fileView.mode != fileViewFull { + t.Fatal("file view should be active in full mode for new file") + } + if cmd == nil { + t.Fatal("expected async load command on open") + } + + // View displays loading placeholder before completion + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), fileViewLoadingPlaceholder) { + t.Fatalf("expected loading placeholder, got: %s", plainRender(t, m.renderFileViewFull(80))) + } + + // Process load completion + updated, _ := m.Update(cmd()) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "package app") || !strings.Contains(rendered, "func Run()") { + t.Fatalf("expected loaded content, got: %s", rendered) + } + if m.fileView.hasError { + t.Fatal("expected no error") + } +} + +// TestFileViewLifecycle_RapidResizeCoalesced verifies that repeated resize events +// do not cause race conditions or synchronous render spikes, and the latest resize wins. +func TestFileViewLifecycle_RapidResizeCoalesced(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "resize.go") + if err := os.WriteFile(filePath, []byte("package resize\nconst BigWidth = true\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "resize.go") + + var cmds []tea.Cmd + for w := 40; w <= 120; w += 10 { + var cmd tea.Cmd + m, cmd = m.startFileViewLoadCmd(w) + if cmd != nil { + cmds = append(cmds, cmd) + } + } + + // Deliver the latest resize command completion + lastCmd := cmds[len(cmds)-1] + updated, _ := m.Update(lastCmd()) + m = updated.(model) + + if m.fileView.loadedWidth != 120 { + t.Fatalf("expected loadedWidth 120, got %d", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(120)), "package resize") { + t.Fatalf("expected content for width 120, got: %s", plainRender(t, m.renderFileViewFull(120))) + } +} + +// TestFileViewLifecycle_ThemeSwitchReloadsActiveView tests that selecting a +// registered theme in production immediately triggers a reload for the active +// file view, and that the returned command structure actually delivers the +// reloaded snapshot under the new cache generation. +func TestFileViewLifecycle_ThemeSwitchReloadsActiveView(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "theme_active.go") + if err := os.WriteFile(filePath, []byte("package theme\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "theme_active.go") + + generationBefore := defaultFileViewCache.generation() + + // "dune" is a registered palette (unlike the retired "light" preference), so + // the dispatch clears the theme's palette and advances the cache generation. + cmdAction := parsedCommand{kind: commandTheme, text: "dune"} + updated, dispatchCmd := m.dispatchCommand(cmdAction) + m = updated.(model) + + if dispatchCmd == nil { + t.Fatal("expected a command on the active file view after a valid theme change") + } + if generationAfter := defaultFileViewCache.generation(); generationAfter <= generationBefore { + t.Fatalf("valid theme switch must advance the cache generation: before=%d after=%d", generationBefore, generationAfter) + } + + // dispatchCommand returns tea.Batch(noticeCmd, loadCmd); unwrap the batch so + // the load completion is delivered to Update instead of being dropped. + m = deliverCommandMessages(t, m, dispatchCmd) + + if m.fileView.loading { + t.Fatal("active file view must leave the loading state after the reload completes") + } + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package theme") { + t.Fatalf("expected reloaded theme content, got: %s", got) + } + if m.fileView.loadedGen != defaultFileViewCache.generation() { + t.Fatalf("expected loadedGen %d, got %d", defaultFileViewCache.generation(), m.fileView.loadedGen) + } +} + +// TestFileViewLifecycle_DirectToolMutationTriggersRefresh tests that tool result +// rows from write_file or edit_file directly refresh an active file view snapshot. +func TestFileViewLifecycle_DirectToolMutationTriggersRefresh(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "live_edit.go") + if err := os.WriteFile(filePath, []byte("version 1\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "live_edit.go") + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "version 1") { + t.Fatal("initial load should have version 1") + } + + // Directly modify file on disk + if err := os.WriteFile(filePath, []byte("version 2 modified\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Dispatch tool result for write_file affecting live_edit.go + toolRow := transcriptRow{ + kind: rowToolResult, + tool: "write_file", + changedFiles: []string{"live_edit.go"}, + detail: "+version 2 modified", + } + updated, reloadCmd := m.Update(agentRowMsg{runID: m.activeRunID, row: toolRow}) + m = updated.(model) + + if reloadCmd == nil { + t.Fatal("expected reload command on direct tool mutation for active file") + } + + // Execute reload + updated, _ = m.Update(reloadCmd()) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "version 2 modified") { + t.Fatalf("expected version 2 after tool mutation reload, got: %s", rendered) + } +} + +// TestFileViewLifecycle_DeletionOverrulesStaleCache tests that when a file is deleted, +// the reload failure evicts the former cache entry and immediately displays the error. +func TestFileViewLifecycle_DeletionOverrulesStaleCache(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "deleted.go") + if err := os.WriteFile(filePath, []byte("package deleted\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "deleted.go") + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package deleted") { + t.Fatal("initial load failed") + } + + // Delete the file + if err := os.Remove(filePath); err != nil { + t.Fatal(err) + } + + // Force reload + m, reloadCmd := m.startFileViewLoadCmd(80) + if reloadCmd == nil { + t.Fatal("expected reload command") + } + + updated, _ := m.Update(reloadCmd()) + m = updated.(model) + + if !m.fileView.hasError { + t.Fatal("expected hasError to be true after deletion") + } + + rendered := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(rendered, "package deleted") { + t.Fatalf("stale cache must not be shown after deletion, got: %s", rendered) + } + if !strings.Contains(rendered, "Could not read file") { + t.Fatalf("expected error message in view, got: %s", rendered) + } +} + +// TestFileViewLifecycle_LateCompletionAcrossReopenDiscarded tests that if a file view +// is exited and the same path is reopened, any late-arriving completion from the first +// session is discarded and cannot populate the new session. +func TestFileViewLifecycle_LateCompletionAcrossReopenDiscarded(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "reopen.go") + if err := os.WriteFile(filePath, []byte("original content\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Session 1: open and get command + m, cmd1 := m.openFileView("reopen.go") + if cmd1 == nil { + t.Fatal("expected cmd1") + } + + // Exit session 1 + m = m.exitFileView() + if m.fileView.active { + t.Fatal("view should be inactive") + } + + // Modify file on disk before session 2 + if err := os.WriteFile(filePath, []byte("new session content\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Session 2: reopen same path + m, cmd2 := m.openFileView("reopen.go") + if cmd2 == nil { + t.Fatal("expected cmd2") + } + + // Late completion from session 1 arrives + msg1 := cmd1() + updated, _ := m.Update(msg1) + m = updated.(model) + + // Session 1 message must have been discarded: view is still waiting on session 2 + if m.fileView.renderedContent != "" { + t.Fatalf("late completion from session 1 must be discarded, got: %s", m.fileView.renderedContent) + } + + // Session 2 completion arrives + msg2 := cmd2() + updated, _ = m.Update(msg2) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "new session content") { + t.Fatalf("expected new session content, got: %s", rendered) + } +} + +// TestFileViewLifecycle_ReverseOrderResizeCompletions verifies that when two resize events +// schedule requests A (earlier) and B (later), and B completes before A, the subsequent +// late arrival of A is discarded and does not overwrite B's width or rendered snapshot. +func TestFileViewLifecycle_ReverseOrderResizeCompletions(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "resize_order.go") + if err := os.WriteFile(filePath, []byte("package resize_order\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "resize_order.go") + + // Schedule Request A for width 60 + m, cmdA := m.startFileViewLoadCmd(60) + if cmdA == nil { + t.Fatal("expected cmdA") + } + + // Schedule Request B for width 100 + m, cmdB := m.startFileViewLoadCmd(100) + if cmdB == nil { + t.Fatal("expected cmdB") + } + + // Message B completes first + msgB := cmdB() + updated, _ := m.Update(msgB) + m = updated.(model) + + if m.fileView.loadedWidth != 100 { + t.Fatalf("expected loadedWidth 100 after B completes, got %d", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package resize_order") { + t.Fatalf("expected width 100 content rendered, got: %s", plainRender(t, m.renderFileViewFull(100))) + } + + // Message A arrives late (reverse-order) + msgA := cmdA() + updated, _ = m.Update(msgA) + m = updated.(model) + + // State MUST remain B (width 100), not overwritten by A (width 60) + if m.fileView.loadedWidth != 100 { + t.Fatalf("late completion A must NOT overwrite loadedWidth, got %d (want 100)", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package resize_order") { + t.Fatalf("expected width 100 content still visible, got: %s", plainRender(t, m.renderFileViewFull(100))) + } +} + +// TestFileViewLifecycle_ReverseOrderToolMutationCompletions verifies that when two tool +// mutations trigger requests A (version 1) and B (version 2), and B completes before A, +// the subsequent arrival of A cannot revert the visible snapshot back to version 1. +func TestFileViewLifecycle_ReverseOrderToolMutationCompletions(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "tool_order.go") + if err := os.WriteFile(filePath, []byte("initial\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "tool_order.go") + + // Mutation A modifies file to v1 + if err := os.WriteFile(filePath, []byte("version 1 state\n"), 0o644); err != nil { + t.Fatal(err) + } + rowA := transcriptRow{ + kind: rowToolResult, + tool: "write_file", + changedFiles: []string{"tool_order.go"}, + detail: "+version 1 state", + } + updated, cmdA := m.Update(agentRowMsg{runID: m.activeRunID, row: rowA}) + m = updated.(model) + if cmdA == nil { + t.Fatal("expected cmdA for mutation A") + } + + // Capture A while v1 is still on disk so msgA contains the genuine v1 snapshot + msgA := cmdA() + + // Mutation B immediately modifies file to v2 before A is applied + if err := os.WriteFile(filePath, []byte("version 2 state\n"), 0o644); err != nil { + t.Fatal(err) + } + rowB := transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + changedFiles: []string{"tool_order.go"}, + detail: "+version 2 state", + } + updated, cmdB := m.Update(agentRowMsg{runID: m.activeRunID, row: rowB}) + m = updated.(model) + if cmdB == nil { + t.Fatal("expected cmdB for mutation B") + } + + // B completes first + msgB := cmdB() + updated, _ = m.Update(msgB) + m = updated.(model) + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "version 2 state") { + t.Fatalf("expected version 2 state after B completes, got: %s", plainRender(t, m.renderFileViewFull(80))) + } + + // A arrives late (reverse-order delivery of stale snapshot) + updated, _ = m.Update(msgA) + m = updated.(model) + + // View MUST remain version 2, never reverted by A + rendered := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(rendered, "version 1 state") { + t.Fatalf("stale version 1 completion must NOT overwrite version 2, got: %s", rendered) + } + if !strings.Contains(rendered, "version 2 state") { + t.Fatalf("expected version 2 state still visible, got: %s", rendered) + } +} + +func TestFileViewSanitizesControlSequencesInHighlightedSource(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + // Recognized Go source with OSC 52 clipboard hijacking sequence in a comment + // and ANSI CSI styling codes inside a string literal. + goContent := `package main + +// \x1b]52;c;cGFzc3dvcmQ=\x07 malicious comment +func main() { + const escape = "\x1b[31;1mred\x1b[0m" +} +` + // Replace raw escape escapes with actual byte values 0x1b and 0x07 + goContent = strings.ReplaceAll(goContent, `\x1b`, "\x1b") + goContent = strings.ReplaceAll(goContent, `\x07`, "\x07") + + filePath := filepath.Join(dir, "malicious.go") + if err := os.WriteFile(filePath, []byte(goContent), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "malicious.go") + + rendered := m.renderFileViewFull(80) + + // Verify that raw hostile control sequences never reach the rendered output + if strings.Contains(rendered, "\x1b]52;") { + t.Fatalf("rendered output must not contain raw OSC 52 sequence, got: %q", rendered) + } + if strings.Contains(rendered, "\x07") { + t.Fatalf("rendered output must not contain BEL character, got: %q", rendered) + } + if strings.Contains(rendered, "\x1b[31;1m") { + t.Fatalf("rendered output must not contain raw source-supplied CSI sequence, got: %q", rendered) + } + + // Verify that the sanitized printable representation is present and styled + plain := plainRender(t, rendered) + if !strings.Contains(plain, "^[") { + t.Fatalf("expected sanitized escape prefix '^[' in plain view, got: %q", plain) + } + if !strings.Contains(plain, "malicious comment") { + t.Fatalf("expected safe comment text in plain view, got: %q", plain) + } +} + +func TestFileViewLifecycle_ResizeRoundTripDoesNotReuseStaleCache(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "roundtrip.go") + if err := os.WriteFile(filePath, []byte("package roundtrip\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "roundtrip.go") + + m, cmd80 := m.startFileViewLoadCmd(80) + if cmd80 == nil { + t.Fatal("expected first width-80 load") + } + updated, _ := m.Update(cmd80()) + m = updated.(model) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package roundtrip") { + t.Fatal("expected committed width-80 content") + } + + m, cmd100 := m.startFileViewLoadCmd(100) + if cmd100 == nil { + t.Fatal("expected width-100 load") + } + m, cmd80b := m.startFileViewLoadCmd(80) + if cmd80b == nil { + t.Fatal("expected second width-80 load") + } + + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, fileViewLoadingPlaceholder) { + t.Fatalf("80→100→80 must stay on loading until desiredSeq completes, got: %s", got) + } + if strings.Contains(got, "package roundtrip") { + t.Fatalf("must not reuse cached width 80 from an earlier seq, got: %s", got) + } + + updated, _ = m.Update(cmd80b()) + m = updated.(model) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package roundtrip") { + t.Fatalf("expected content after matching seq completes, got: %s", plainRender(t, m.renderFileViewFull(80))) + } +} + +func TestFileViewLifecycle_EmptyFileIsCompletedSnapshot(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "empty.go"), []byte(""), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "empty.go") + got := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(got, fileViewLoadingPlaceholder) { + t.Fatalf("empty completed snapshot must not stay on loading, got %q", got) + } + if !m.fileView.snapshotReady { + t.Fatal("snapshotReady must be set for an empty file") + } +} + +func TestFileViewLifecycle_ShellEscapeReloadsFullView(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + path := filepath.Join(dir, "shell.go") + if err := os.WriteFile(path, []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "shell.go") + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + oldTime := info.ModTime() + if err := os.WriteFile(path, []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, oldTime, oldTime); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(bashResultMsg{output: "ok"}) + m = updated.(model) + if cmd == nil { + t.Fatal("bashResultMsg must schedule a file-view reload") + } + updated, _ = m.Update(cmd()) + m = updated.(model) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("expected reloaded content after shell escape, got %s", got) + } +} + +func TestFileViewLifecycle_SupersededResizeSkipsWork(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "coalesce.go"), []byte("package coalesce\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + var cmd0 tea.Cmd + m, cmd0 = m.openFileView("coalesce.go") + if cmd0 == nil { + t.Fatal("expected initial load") + } + var cmd1, cmd2 tea.Cmd + m, cmd1 = m.startFileViewLoadCmd(60) + m, cmd2 = m.startFileViewLoadCmd(100) + _ = cmd0() + _ = cmd1() + msg := cmd2() + updated, _ := m.Update(msg) + m = updated.(model) + stats := fileViewCacheStatsForTest() + if stats.DiskReads != 1 { + t.Fatalf("superseded loads must not re-read, DiskReads=%d", stats.DiskReads) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package coalesce") { + t.Fatal("latest width must still load") + } +} + +func TestFilesPanelSecondActivationOpensFullViewThroughUpdate(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "web"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "web", "app.js"), []byte("let a = 1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.runDetailsOpen = true + m, cmd := m.selectFile("web/app.js") + if cmd != nil { + t.Fatal("first FILES activation must only select") + } + if m.fileView.active { + t.Fatal("first FILES activation must not open the file view") + } + if m.selectedFile != "web/app.js" { + t.Fatalf("selectedFile = %q", m.selectedFile) + } + + updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEnter}) + m = updated.(model) + if !m.fileView.active || m.fileView.path != "web/app.js" { + t.Fatal("Enter on the selected FILES row must call openFileView") + } + if m.runDetailsOpen { + t.Fatal("drilling in must close run details") + } + + updated, cmd = m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + m = updated.(model) + if m.fileView.mode != fileViewFull { + t.Fatal("f must switch to full mode") + } + if cmd == nil { + t.Fatal("full mode must schedule an async load") + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), fileViewLoadingPlaceholder) { + t.Fatal("expected loading before async result") + } + updated, _ = m.Update(cmd()) + m = updated.(model) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "let a = 1") { + t.Fatalf("expected loaded file content, got %s", plainRender(t, m.renderFileViewFull(80))) + } +} + +func TestFileViewLifecycle_SupersedeStopsInFlightWork(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "stall.go"), []byte("package stall\n"), 0o644); err != nil { + t.Fatal(err) + } + entered := make(chan struct{}) + release := make(chan struct{}) + fileViewInsideLoad = func() { + select { + case <-entered: + default: + close(entered) + } + <-release + } + defer func() { fileViewInsideLoad = nil }() + + m := filesPanelTestModel() + m.cwd = dir + m, cmdA := m.openFileView("stall.go") + if cmdA == nil { + t.Fatal("expected initial load") + } + doneA := make(chan tea.Msg, 1) + go func() { doneA <- cmdA() }() + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("load A did not enter worker") + } + m, cmdB := m.startFileViewLoadCmd(100) + if cmdB == nil { + t.Fatal("expected superseding load") + } + close(release) + msgA := <-doneA + loaded, ok := msgA.(fileViewLoadedMsg) + if !ok { + t.Fatalf("msgA type %T", msgA) + } + if !errors.Is(loaded.err, errFileViewSuperseded) { + t.Fatalf("in-flight A must stop, err=%v", loaded.err) + } + msgB := cmdB() + updated, _ := m.Update(msgB) + m = updated.(model) + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package stall") { + t.Fatal("latest request must complete") + } +} + +func TestFileViewLifecycle_SupersededHighlightMustNotClobberCache(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + path := filepath.Join(dir, "clobber.go") + if err := os.WriteFile(path, []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "clobber.go") + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + oldTime := info.ModTime() + if err := os.WriteFile(path, []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, oldTime, oldTime); err != nil { + t.Fatal(err) + } + + entered := make(chan struct{}) + release := make(chan struct{}) + var n int32 + fileViewBeforeCacheCommit = func() { + if atomic.AddInt32(&n, 1) == 1 { + close(entered) + <-release + } + } + defer func() { fileViewBeforeCacheCommit = nil }() + + m, cmdA := m.startFileViewRefreshCmd(80) + if cmdA == nil { + t.Fatal("expected refresh A") + } + doneA := make(chan tea.Msg, 1) + go func() { doneA <- cmdA() }() + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("A did not reach cache commit") + } + m, cmdB := m.startFileViewRefreshCmd(80) + if cmdB == nil { + t.Fatal("expected refresh B") + } + updated, _ := m.Update(cmdB()) + m = updated.(model) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package new") { + t.Fatal("B must be accepted before releasing A") + } + close(release) + msgA := <-doneA + loaded, ok := msgA.(fileViewLoadedMsg) + if !ok { + t.Fatalf("msgA type %T", msgA) + } + if !errors.Is(loaded.err, errFileViewSuperseded) { + t.Fatalf("A must not commit, err=%v", loaded.err) + } + got := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(got, "package old") { + t.Fatalf("stale highlight must not clobber cache, got %s", got) + } + if !strings.Contains(got, "package new") { + t.Fatalf("expected package new, got %s", got) + } +} + +func TestFileViewExitRevokesWorkerToken(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "leave.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package p\nfunc B() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m, cmd := m.openFileView(name) + if cmd == nil { + t.Fatal("load cmd") + } + token := m.fileView.liveSeq + if token == nil { + t.Fatal("liveSeq") + } + seq := token.Load() + m = m.exitFileView() + if token.Load() == seq { + t.Fatal("exitFileView must revoke the token the worker still holds") + } +} + +func TestFileViewCacheHitRespectsLiveSeq(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "hit.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package p\nfunc A() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m, cmd := m.openFileView(name) + if cmd == nil { + t.Fatal("cache miss must yield load cmd") + } + updated, _ := m.Update(cmd()) + m = updated.(model) + if m.fileView.liveSeq == nil { + t.Fatal("liveSeq") + } + seq := m.fileView.liveSeq.Load() + live := m.fileView.liveSeq + fileViewBeforeCacheCommit = func() { live.Store(seq + 1) } + defer func() { fileViewBeforeCacheCommit = nil }() + target := filepath.Join(dir, name) + gen := defaultFileViewCache.generation() + _, err := defaultFileViewCache.loadAndRender(target, name, m.fileView.loadedWidth+17, nil, m.fileView.loadedFingerprint, gen, zeroTheme, 0, live, seq) + if !errors.Is(err, errFileViewSuperseded) { + t.Fatalf("cache-hit put after supersede: err=%v", err) + } +} + +func TestFileViewPlanToolRefreshesWhenGitSweepNil(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "mut.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 7 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m = testOpenFile(m, name) + if err := os.WriteFile(filepath.Join(dir, name), []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(agentRowMsg{ + runID: 7, + row: transcriptRow{ + kind: rowToolResult, + tool: "bash", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + m = updated.(model) + if cmd == nil { + t.Fatal("bash/plan mutation must refresh the file view even when maybeGitSweep is nil") + } + msg := cmd() + if batch, ok := msg.(tea.BatchMsg); ok { + for _, c := range batch { + if c == nil { + continue + } + updated, _ = m.Update(c()) + m = updated.(model) + } + } else { + updated, _ = m.Update(msg) + m = updated.(model) + } + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("stale view after git-nil mutation: %s", got) + } +} + +func TestFileViewTransitionMatrix_MutationClosedDiffFull(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "sample.go" + filePath := filepath.Join(dir, name) + + // Phase 1: Closed mutation with equal size & equal mtime + if err := os.WriteFile(filePath, []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + + // Seed cache + m = testOpenFile(m, name) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package old") { + t.Fatal("initial seed must show package old") + } + m = m.exitFileView() + + // Mutate on disk with same length ("package new\n" has 12 bytes == "package old\n") + fi, _ := os.Stat(filePath) + oldTime := fi.ModTime() + if err := os.WriteFile(filePath, []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filePath, oldTime, oldTime) + + // Deliver mutation row while closed + updated, _ := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + m = updated.(model) + + // Re-open and switch to full mode: must not reuse stale cache entry + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("reopening after closed mutation must render package new, got: %s", got) + } + + // Phase 2: Diff mode mutation with equal metadata + if err := os.WriteFile(filePath, []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filePath, oldTime, oldTime) + m = testSetMode(m, fileViewFull) // refreshed to old + m = testSetMode(m, fileViewDiff) + + // Mutate while in diff mode + if err := os.WriteFile(filePath, []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filePath, oldTime, oldTime) + updated, _ = m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + m = updated.(model) + + // Switch back to full mode: must not reuse stale cache entry + m = testSetMode(m, fileViewFull) + got = plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("switching to full after diff mutation must render package new, got: %s", got) + } + + // Phase 3: Full mode live mutation + if err := os.WriteFile(filePath, []byte("package live\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + m = updated.(model) + if cmd == nil { + t.Fatal("full mode mutation must yield refresh cmd") + } + updated, _ = m.Update(cmd()) + m = updated.(model) + got = plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package live") { + t.Fatalf("full mode live mutation must render package live, got: %s", got) + } +} + +func TestFileViewTransitionMatrix_RefreshResizeThemeOrdering(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "order.go" + filePath := filepath.Join(dir, name) + + if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + m = testOpenFile(m, name) + + // Mutate to v2 with equal size & mtime + fi, _ := os.Stat(filePath) + oldTime := fi.ModTime() + if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filePath, oldTime, oldTime) + + // 1. Tool mutation schedules refresh + updated, refreshCmd := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + m = updated.(model) + if refreshCmd == nil { + t.Fatal("mutation must schedule refresh") + } + + // 2. Before refreshCmd completes, a resize event arrives (advances desiredSeq with refreshSource=false) + updated, resizeCmd := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40}) + m = updated.(model) + if resizeCmd == nil { + t.Fatal("resize must yield load cmd") + } + + // 3. Obsolete refreshCmd returns: must be rejected + oldRefreshMsg := refreshCmd() + updated, _ = m.Update(oldRefreshMsg) + m = updated.(model) + + // 4. Winning resizeCmd completes: MUST inherit requiredSourceRev and render v2 + resizeMsg := resizeCmd() + updated, _ = m.Update(resizeMsg) + m = updated.(model) + + got := plainRender(t, m.renderFileViewFull(120)) + if !strings.Contains(got, "package v2") { + t.Fatalf("resize following mutation must render package v2, got: %s", got) + } +} + +func TestFileViewTransitionMatrix_DeterministicStageHooksSupersession(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "hooks.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package hooks\nfunc Work() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + hooks := []struct { + name string + isCacheHit bool + set func(fn func()) + }{ + {"InsideLoad", false, func(fn func()) { fileViewInsideLoad = fn }}, + {"BeforeCacheHitFormat", true, func(fn func()) { fileViewBeforeCacheHitFormat = fn }}, + {"BeforeDiskRead", false, func(fn func()) { fileViewBeforeDiskRead = fn }}, + {"BeforeHighlight", false, func(fn func()) { fileViewBeforeHighlight = fn }}, + {"BeforeFormat", false, func(fn func()) { fileViewBeforeFormat = fn }}, + {"BeforeCacheCommit", false, func(fn func()) { fileViewBeforeCacheCommit = fn }}, + } + + for _, tc := range hooks { + t.Run(tc.name, func(t *testing.T) { + resetFileViewCacheForTest() + m := filesPanelTestModel() + m.cwd = dir + + if tc.isCacheHit { + // Seed cache first + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + } + + m, cmd := m.openFileView(name) + if tc.isCacheHit { + var loadCmd tea.Cmd + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth() + 15) // width variant + cmd = loadCmd + } + if cmd == nil { + t.Fatal("load cmd") + } + live := m.fileView.liveSeq + seq := m.fileView.desiredSeq + + executedHook := false + tc.set(func() { + executedHook = true + live.Store(seq + 1) // supersede A with B + }) + defer tc.set(nil) + + msg := cmd() + if !executedHook { + t.Fatalf("hook %s was never executed", tc.name) + } + loaded := msg.(fileViewLoadedMsg) + if !errors.Is(loaded.err, errFileViewSuperseded) { + t.Fatalf("hook %s: expected errFileViewSuperseded, got %v", tc.name, loaded.err) + } + + // Delivery of superseded message must be a complete no-op + updated, nextCmd := m.handleFileViewLoaded(loaded) + if nextCmd != nil { + t.Fatalf("hook %s: superseded message must not schedule retry", tc.name) + } + if updated.fileView.snapshotReady { + t.Fatalf("hook %s: superseded message must not mark snapshotReady", tc.name) + } + }) + } +} + +func TestFileViewTransitionMatrix_CurrentCompletionFollowedByObsoleteCompletion(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "race.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package race\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m, cmd1 := m.openFileView(name) + if cmd1 == nil { + t.Fatal("cmd1") + } + + // Resize to advance sequence from 1 to 2 + updated, cmd2 := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) + m = updated.(model) + if cmd2 == nil { + t.Fatal("cmd2") + } + + // Sequence 2 completes first + msg2 := cmd2() + updated, _ = m.Update(msg2) + m = updated.(model) + if !m.fileView.snapshotReady || m.fileView.loadedSeq != 2 { + t.Fatalf("seq 2 must be loaded, ready=%v seq=%d", m.fileView.snapshotReady, m.fileView.loadedSeq) + } + + // Sequence 1 arrives late: must be a complete no-op + msg1 := cmd1() + updated, lateCmd := m.Update(msg1) + m = updated.(model) + if lateCmd != nil { + t.Fatal("late completion must not schedule retry") + } + if !m.fileView.snapshotReady || m.fileView.loadedSeq != 2 { + t.Fatalf("late completion must not degrade state: ready=%v seq=%d", m.fileView.snapshotReady, m.fileView.loadedSeq) + } +} + +func TestFileView_ChangedLineMatchingWithTabsAndControlChars(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "tabmatch.go" + content := "package main\n\nvar Field\t= 1\nvar Clean = 2\n" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + detail: "--- a/tabmatch.go\n+++ b/tabmatch.go\n@@ -1,2 +1,3 @@\n+var Field\t= 1\n", + }) + + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + rendered := m.renderFileViewFull(80) + + // The rendered source displays 4 spaces instead of raw tab, and retains the accent gutter marker ▎ + if strings.Contains(rendered, "\t") { + t.Fatal("rendered output must not contain raw tab characters") + } + if !strings.Contains(rendered, "▎") { + t.Fatalf("added line with interior tab must retain accent gutter marker, got:\n%s", rendered) + } +} + +func TestRunDetails_KeyboardActivationStrictRowTargets(t *testing.T) { + m := filesPanelTestModel() + m.altScreen = true + m.width = 80 + m.height = 30 + m.runDetailsOpen = true + + // Build a transcript with 7 files touched so files 0..3 are visible, file 4 is overflow trailer ("… more in transcript"), files 5..6 hidden + for i := 0; i < 7; i++ { + name := fmt.Sprintf("file_%d.go", i) + m.transcript = append(m.transcript, transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }) + } + + // 1. Visible file (file_6.go) can be activated on Enter + m.selectedFile = "file_6.go" + updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEnter}) + res := updated.(model) + if !res.fileView.active || res.fileView.path != "file_6.go" { + t.Fatalf("visible file in run details must be activated on Enter: active=%v path=%s", res.fileView.active, res.fileView.path) + } + + // Reset file view + m = res.exitFileView() + m.runDetailsOpen = true + + // 2. Hidden / overflow file (file_0.go) must NOT be activated on Enter + m.selectedFile = "file_0.go" + updated, _ = m.Update(tea.KeyPressMsg{Code: tea.KeyEnter}) + res = updated.(model) + if res.fileView.active { + t.Fatalf("overflow hidden file (file_0.go) must not be activated on Enter from run details modal") + } +} + +func TestFileView_RewindReturnsAsyncCommandAndDoesNotBlockUpdate(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "target.go") + if err := os.WriteFile(filePath, []byte("package before_rewind\n"), 0o644); err != nil { + t.Fatal(err) + } + + store := testSessionStore(t) + session, err := store.Create(sessions.CreateInput{Title: "RewindTest"}) + if err != nil { + t.Fatal(err) + } + appendTestEvent(t, store, session.SessionID, sessions.EventSessionCheckpoint, map[string]any{"tool": "write_file", "files": []any{}}) + + m := filesPanelTestModel() + m.cwd = dir + m.sessionStore = store + m.activeSession = session + m.activeRunID = 1 + + // Open file in full view + m = testOpenFile(m, "target.go") + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package before_rewind") { + t.Fatal("expected initial view to show before_rewind") + } + + // Overwrite on disk to simulate restored file + if err := os.WriteFile(filePath, []byte("package after_rewind\n"), 0o644); err != nil { + t.Fatal(err) + } + + m, summary, rewindCmd := m.handleRewindCommand("latest") + if !strings.Contains(summary, "Rewound") { + t.Fatalf("expected summary to indicate rewound, got %q", summary) + } + if rewindCmd == nil { + t.Fatal("handleRewindCommand must return an async tea.Cmd when fileView is active in full mode") + } + + // Model in Update must NOT have synchronously rendered the new file yet: + // it must NOT display package after_rewind until Bubble Tea delivers fileViewLoadedMsg. + if strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package after_rewind") { + t.Fatal("expected view to not synchronously show after_rewind before async tea.Cmd is executed") + } + + // Now execute rewindCmd as Bubble Tea runtime would + msg := rewindCmd() + loadedMsg, ok := msg.(fileViewLoadedMsg) + if !ok { + t.Fatalf("expected fileViewLoadedMsg, got %T", msg) + } + updated, _ := m.Update(loadedMsg) + m = updated.(model) + + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package after_rewind") { + t.Fatalf("rewind reload must render after_rewind, got: %s", got) + } +} + +func TestFileView_RewindPartialFailure_ActiveAndClosedView(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "partial_fail.go") + if err := os.WriteFile(filePath, []byte("package before_partial\n"), 0o644); err != nil { + t.Fatal(err) + } + + store := testSessionStore(t) + session, err := store.Create(sessions.CreateInput{Title: "PartialFailRewind"}) + if err != nil { + t.Fatal(err) + } + appendTestEvent(t, store, session.SessionID, sessions.EventSessionCheckpoint, map[string]any{"tool": "write_file", "files": []any{}}) + + m := filesPanelTestModel() + m.cwd = dir + m.sessionStore = store + m.activeSession = session + m.activeRunID = 1 + + // 1. Active view: open and cache + m = testOpenFile(m, "partial_fail.go") + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package before_partial") { + t.Fatal("expected before_partial") + } + + // Restored file written to disk + if err := os.WriteFile(filePath, []byte("package after_partial_restored\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Invalidate session ID so ApplyRewind fails after disk mutation + m.activeSession.SessionID = "non_existent_session" + + // Invoke handleRewindCommand directly: ApplyRewind will fail! + m, summary, refreshCmd := m.handleRewindCommand("0") + if !strings.Contains(summary, "Rewind\n") { + t.Fatalf("expected error summary, got: %s", summary) + } + if refreshCmd == nil { + t.Fatal("expected refreshCmd on ApplyRewind error") + } + + msg := refreshCmd() + loaded, ok := msg.(fileViewLoadedMsg) + if !ok { + t.Fatalf("expected fileViewLoadedMsg, got %T", msg) + } + updated, _ := m.Update(loaded) + m = updated.(model) + + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package after_partial_restored") { + t.Fatalf("expected after_partial_restored, got: %s", got) + } + + // 2. Closed view: close view, write v3, trigger another failed rewind, reopen + m = m.exitFileView() + if err := os.WriteFile(filePath, []byte("package after_partial_v3\n"), 0o644); err != nil { + t.Fatal(err) + } + + m, _, _ = m.handleRewindCommand("0") + + m = testOpenFile(m, "partial_fail.go") + m = testSetMode(m, fileViewFull) + got = plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package after_partial_v3") { + t.Fatalf("expected after_partial_v3 on reopen, got: %s", got) + } +} + +func TestFileView_UnknownScopeCommandMutationClosedView(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "equal_len.go" + filePath := filepath.Join(dir, name) + + // Seed file with "package old\n" (12 bytes) + if err := os.WriteFile(filePath, []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + + // Open and cache + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package old") { + t.Fatal("expected initial render of package old") + } + + // Close view + m = m.exitFileView() + + // Mutate on disk with identical size (12 bytes) and restored mtime + fi, _ := os.Stat(filePath) + oldModTime := fi.ModTime() + if err := os.WriteFile(filePath, []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filePath, oldModTime, oldModTime) + + // Deliver an unknown-scope tool result (bash command) while view is closed + updated, _ := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "bash", + status: tools.StatusOK, + detail: "sed -i 's/old/new/g' equal_len.go", + }, + }) + m = updated.(model) + + // Reopen view: must NOT serve cached package old + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("reopening after closed unknown-scope mutation must render package new, got: %s", got) + } +} + +func TestFileView_NoTOCTOUFileOpenInTUI(t *testing.T) { + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, "file_view.go", nil, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + + // Verify that inside readFileViewBoundedCancellable, os.Stat is NOT called before os.OpenFile + ast.Inspect(node, func(n ast.Node) bool { + fn, ok := n.(*ast.FuncDecl) + if !ok || fn.Name.Name != "readFileViewBoundedCancellable" { + return true + } + + var sawOpenFile bool + var sawPreStat bool + + ast.Inspect(fn.Body, func(inner ast.Node) bool { + call, ok := inner.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return true + } + if ident.Name == "os" && sel.Sel.Name == "Stat" && !sawOpenFile { + sawPreStat = true + } + if ident.Name == "os" && sel.Sel.Name == "OpenFile" { + sawOpenFile = true + } + return true + }) + + if sawPreStat { + t.Fatal("TOCTOU vulnerability detected: os.Stat called before os.OpenFile in readFileViewBoundedCancellable") + } + if !sawOpenFile { + t.Fatal("expected os.OpenFile with O_NONBLOCK in readFileViewBoundedCancellable") + } + return false + }) +} + +func TestFileView_SupersededInsideDiskReadAndHighlight(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "cancel_test.go") + if err := os.WriteFile(filePath, []byte("package cancel\nfunc Foo() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + cache := newFileViewRenderCache(10, defaultFileViewCacheMaxBytes) + theme := zeroTheme + var liveSeq atomic.Uint64 + liveSeq.Store(1) + + // 1. Superseded during disk read loop + fileViewInsideDiskRead = func() { + liveSeq.Store(2) // supersede request 1 + } + defer func() { fileViewInsideDiskRead = nil }() + + _, err := cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 1) + if !errors.Is(err, errFileViewSuperseded) { + t.Fatalf("expected errFileViewSuperseded on inside-disk-read supersession, got: %v", err) + } + fileViewInsideDiskRead = nil + + // 2. Superseded before highlight + liveSeq.Store(10) + fileViewBeforeHighlight = func() { + liveSeq.Store(11) // supersede request 10 + } + defer func() { fileViewBeforeHighlight = nil }() + + _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 10) + if !errors.Is(err, errFileViewSuperseded) { + t.Fatalf("expected errFileViewSuperseded on before-highlight supersession, got: %v", err) + } + fileViewBeforeHighlight = nil + + // 3. Superseded inside highlight (under fileViewHighlightMu) + liveSeq.Store(20) + fileViewInsideHighlight = func() { + liveSeq.Store(21) // supersede request 20 + } + defer func() { fileViewInsideHighlight = nil }() + + _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 20) + if !errors.Is(err, errFileViewSuperseded) { + t.Fatalf("expected errFileViewSuperseded on inside-highlight supersession, got: %v", err) + } + if cache.stats().HighlightCalls != 0 { + t.Fatalf("expected 0 HighlightCalls on superseded request, got: %d", cache.stats().HighlightCalls) + } + fileViewInsideHighlight = nil +} + +func TestFileView_DispatchCommandRewindReturnsPromptlyWithCmd(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "prompt_test.go") + if err := os.WriteFile(filePath, []byte("package before_dispatch\n"), 0o644); err != nil { + t.Fatal(err) + } + + store := testSessionStore(t) + session, err := store.Create(sessions.CreateInput{Title: "DispatchRewind"}) + if err != nil { + t.Fatal(err) + } + appendTestEvent(t, store, session.SessionID, sessions.EventSessionCheckpoint, map[string]any{"tool": "write_file", "files": []any{}}) + + m := newModel(context.Background(), Options{SessionStore: store}) + m.cwd = dir + m.activeSession = session + m.activeRunID = 1 + + // Open in full view + m = testOpenFile(m, "prompt_test.go") + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package before_dispatch") { + t.Fatal("expected view to display before_dispatch") + } + + // Restored file content on disk + if err := os.WriteFile(filePath, []byte("package after_dispatch\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Type /rewind and press enter + m.input.SetValue("/rewind latest") + updated, cmd := m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + + if cmd == nil { + t.Fatal("expected /rewind via dispatchCommand to return a non-nil async tea.Cmd") + } + + // Before executing cmd, full view must NOT have synchronously loaded the new content: + if strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package after_dispatch") { + t.Fatal("expected view to not show after_dispatch before async command is delivered") + } + + // Now execute cmd and deliver + msg := cmd() + loadedMsg, ok := msg.(fileViewLoadedMsg) + if !ok { + t.Fatalf("expected fileViewLoadedMsg from rewind tea.Cmd, got %T", msg) + } + updated, _ = m.Update(loadedMsg) + m = updated.(model) + + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package after_dispatch") { + t.Fatalf("expected view to update to after_dispatch after delivery, got: %s", got) + } +} + +func TestFileView_CommandAndSweepCompletionOrders(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "order_test.go" + filePath := filepath.Join(dir, name) + + // Order 1: Command tool result arrives -> Git sweep arrives later + if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package v1") { + t.Fatal("expected v1") + } + + // Mutate on disk to v2 + if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Step 1: command tool result + updated, cmd1 := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "bash", + status: tools.StatusOK, + detail: "echo 'package v2' > order_test.go", + }, + }) + m = updated.(model) + if cmd1 != nil { + msg := cmd1() + if loaded, ok := msg.(fileViewLoadedMsg); ok { + updated, _ = m.Update(loaded) + m = updated.(model) + } + } + + // Step 2: git sweep arrives later + updated, cmd2 := m.Update(gitSweepMsg{ + ok: true, + files: []gitSweepFile{{path: name, adds: 1}}, + }) + m = updated.(model) + if cmd2 != nil { + msg := cmd2() + if loaded, ok := msg.(fileViewLoadedMsg); ok { + updated, _ = m.Update(loaded) + m = updated.(model) + } + } + + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package v2") { + t.Fatalf("expected package v2 after command->sweep order, got: %s", got) + } + + // Order 2: Git sweep arrives first -> Command tool result arrives second + if err := os.WriteFile(filePath, []byte("package v3\n"), 0o644); err != nil { + t.Fatal(err) + } + + updated, cmdA := m.Update(gitSweepMsg{ + ok: true, + files: []gitSweepFile{{path: name, adds: 1}}, + }) + m = updated.(model) + if cmdA != nil { + msg := cmdA() + if loaded, ok := msg.(fileViewLoadedMsg); ok { + updated, _ = m.Update(loaded) + m = updated.(model) + } + } + + updated, cmdB := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{ + kind: rowToolResult, + tool: "bash", + status: tools.StatusOK, + detail: "echo 'package v3' > order_test.go", + }, + }) + m = updated.(model) + if cmdB != nil { + msg := cmdB() + if loaded, ok := msg.(fileViewLoadedMsg); ok { + updated, _ = m.Update(loaded) + m = updated.(model) + } + } + + got = plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package v3") { + t.Fatalf("expected package v3 after sweep->command order, got: %s", got) + } +} + +// TestFileViewCacheByteBudget verifies the cache-wide byte cap: retained stays +// within maxBytes, eviction fires before the budget is exceeded, and the named +// default bound is pinned so a silent regression is caught. +func TestFileViewCacheByteBudget(t *testing.T) { + if defaultFileViewCacheMaxBytes != 8<<20 { + t.Fatalf("defaultFileViewCacheMaxBytes = %d, want %d", defaultFileViewCacheMaxBytes, 8<<20) + } + defaultFileViewCache.mu.Lock() + pinned := defaultFileViewCache.maxBytes + defaultFileViewCache.mu.Unlock() + if pinned != defaultFileViewCacheMaxBytes { + t.Fatalf("default cache maxBytes = %d, want %d", pinned, defaultFileViewCacheMaxBytes) + } + + dir := t.TempDir() + const maxBytes = 48 * 1024 + cache := newFileViewRenderCache(64, maxBytes) + + payload := strings.Repeat("abcdefghij\n", 400) // ~4 KiB per file + for i := 0; i < 5; i++ { + name := fmt.Sprintf("byte_cap_%d.txt", i) + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(payload), 0o644); err != nil { + t.Fatal(err) + } + if _, err := cache.loadAndRender(path, name, 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + t.Fatalf("load %d: %v", i, err) + } + } + + cache.mu.Lock() + retained := cache.retained + entries := len(cache.items) + cache.mu.Unlock() + if retained > maxBytes { + t.Fatalf("retained %d exceeded maxBytes %d", retained, maxBytes) + } + if entries == 0 { + t.Fatal("expected at least one cached entry") + } + if evictions := cache.stats().Evictions; evictions == 0 { + t.Fatal("expected byte-budget eviction to fire") + } +} + +// TestFileViewCacheRefusesOversizedEntry verifies a single entry larger than the +// byte budget is returned to the caller but never retained. +func TestFileViewCacheRefusesOversizedEntry(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "oversized.txt") + if err := os.WriteFile(path, []byte(strings.Repeat("z", 4096)), 0o644); err != nil { + t.Fatal(err) + } + cache := newFileViewRenderCache(64, 512) + rendered, err := cache.loadAndRender(path, "oversized.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("load: %v", err) + } + if !strings.Contains(rendered, "zzz") { + t.Fatal("oversized entry must still be returned to the caller") + } + cache.mu.Lock() + entries := len(cache.items) + retained := cache.retained + cache.mu.Unlock() + if entries != 0 || retained != 0 { + t.Fatalf("oversized entry must not be cached: entries=%d retained=%d", entries, retained) + } + if stats := cache.stats(); stats.SkippedOversized != 1 { + t.Fatalf("SkippedOversized = %d, want 1", stats.SkippedOversized) + } +} + +// TestFileViewCacheEvictionDropsPathRevision verifies that LRU eviction removes +// the evicted path's revision key, so revisions cannot leak one key per file. +func TestFileViewCacheEvictionDropsPathRevision(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "rev_a.txt") + b := filepath.Join(dir, "rev_b.txt") + for _, p := range []string{a, b} { + if err := os.WriteFile(p, []byte("alpha\n"), 0o644); err != nil { + t.Fatal(err) + } + } + cache := newFileViewRenderCache(1, defaultFileViewCacheMaxBytes) + + // Give a a revision, then load it so the entry and its revision coexist. + cache.invalidatePath(a) + if _, err := cache.loadAndRender(a, "rev_a.txt", 80, nil, "", cache.generation(), zeroTheme, cache.requiredRevision(a), nil, 0); err != nil { + t.Fatalf("load a: %v", err) + } + cache.mu.Lock() + _, hasA := cache.pathRevisions[a] + cache.mu.Unlock() + if !hasA { + t.Fatal("expected revision key for a before eviction") + } + + // Loading b with maxEntries=1 evicts a and must drop a's revision. + if _, err := cache.loadAndRender(b, "rev_b.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + t.Fatalf("load b: %v", err) + } + cache.mu.Lock() + _, hasA = cache.pathRevisions[a] + orphans := len(cache.pathRevisions) + cache.mu.Unlock() + if hasA { + t.Fatal("evicted path a must not keep a revision key") + } + if orphans != 0 { + t.Fatalf("expected no orphan revision keys, got %d", orphans) + } +} + +// TestFileViewCachePurgeResetsPathRevisions verifies clear() resets the revision +// map instead of bumping dead keys forever, so /clear cannot leak one key per +// abandoned tool result. +func TestFileViewCachePurgeResetsPathRevisions(t *testing.T) { + cache := newFileViewRenderCache(8, defaultFileViewCacheMaxBytes) + cache.invalidatePath("one.go") + cache.invalidatePath("two.go") + cache.mu.Lock() + before := len(cache.pathRevisions) + cache.mu.Unlock() + if before != 2 { + t.Fatalf("expected 2 revision keys, got %d", before) + } + + cache.clear() + cache.mu.Lock() + after := len(cache.pathRevisions) + cache.mu.Unlock() + if after != 0 { + t.Fatalf("clear() must drop revision keys, got %d", after) + } + if rev := cache.invalidatePath("one.go"); rev != 1 { + t.Fatalf("revision after purge = %d, want 1", rev) + } +} + +// TestFileViewCacheFreshnessBySourceHash verifies that metadata (mtime+size) is +// only a hint: when an overwrite restores the same metadata with different +// bytes, the cached render must not be served. +func TestFileViewCacheFreshnessBySourceHash(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "fresh.go") + original := "package main\n// AAAA\n" + updated := "package main\n// BBBB\n" + if len(original) != len(updated) { + t.Fatal("test setup: contents must have equal length") + } + if err := os.WriteFile(path, []byte(original), 0o644); err != nil { + t.Fatal(err) + } + stamp := time.Unix(1700000000, 0) + if err := os.Chtimes(path, stamp, stamp); err != nil { + t.Fatal(err) + } + + cache := newFileViewRenderCache(64, defaultFileViewCacheMaxBytes) + first, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("first load: %v", err) + } + if !strings.Contains(first, "AAAA") { + t.Fatalf("first render missing original content: %q", first) + } + + if err := os.WriteFile(path, []byte(updated), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, stamp, stamp); err != nil { + t.Fatal(err) + } + + second, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("second load: %v", err) + } + if strings.Contains(second, "AAAA") || !strings.Contains(second, "BBBB") { + t.Fatalf("stale render served after same-metadata overwrite: %q", second) + } + if stats := cache.stats(); stats.HighlightCalls != 2 { + t.Fatalf("HighlightCalls = %d, want 2 (hash mismatch must force a miss)", stats.HighlightCalls) + } +} + +// deliverCommandMessages executes a tea.Cmd tree and feeds every produced +// message back into the model through Update, unwrapping tea.BatchMsg so a +// batched dispatch (notice + reload) reaches its load completion. Commands that +// Update schedules in response are intentionally not followed. +func deliverCommandMessages(t *testing.T, m model, cmd tea.Cmd) model { + t.Helper() + if cmd == nil { + return m + } + pending := []tea.Cmd{cmd} + for len(pending) > 0 { + current := pending[0] + pending = pending[1:] + if current == nil { + continue + } + msg := current() + if msg == nil { + continue + } + if batch, ok := msg.(tea.BatchMsg); ok { + pending = append(pending, batch...) + continue + } + updated, _ := m.Update(msg) + m = updated.(model) + } + return m +} + +// TestFileViewLifecycle_OversizedReplacementEvictsObsoleteEntry verifies that an +// oversized replacement is still the displayed snapshot: the obsolete resident +// entry is removed and cannot win the next cache lookup, and the oversized +// admission is counted without being retained. +func TestFileViewLifecycle_OversizedReplacementEvictsObsoleteEntry(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + name := "oversized_lifecycle.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package original\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, name) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package original") { + t.Fatal("expected the seed file to be cached and displayed") + } + m = m.exitFileView() + + // 4000 * 78 == 312000 bytes: inside the 1 MiB read budget, but the retained + // source plus its highlighted and rendered payload exceeds the cache budget. + line := "var a = 123; var b = 456; var c = 789; var d = 123; var e = 456; var f = 789;\n" + payload := strings.Repeat(line, 4000) + if len(payload) != 312000 { + t.Fatalf("test payload = %d bytes, want 312000", len(payload)) + } + if err := os.WriteFile(filePath, []byte(payload), 0o644); err != nil { + t.Fatal(err) + } + + statsBefore := fileViewCacheStatsForTest() + m = testOpenFile(m, name) + + displayed := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(displayed, "package original") { + t.Fatal("the obsolete cached entry must not override the oversized replacement") + } + if !strings.Contains(displayed, "var a = 123") { + t.Fatal("the oversized replacement must be the displayed snapshot") + } + statsAfter := fileViewCacheStatsForTest() + if statsAfter.SkippedOversized <= statsBefore.SkippedOversized { + t.Fatalf("SkippedOversized must increase: before=%d after=%d", statsBefore.SkippedOversized, statsAfter.SkippedOversized) + } + defaultFileViewCache.mu.Lock() + _, resident := defaultFileViewCache.items[filePath] + defaultFileViewCache.mu.Unlock() + if resident { + t.Fatal("an oversized replacement must not remain resident") + } +} + +// TestFileViewLifecycle_PendingRefreshPreservesViewport verifies that a pending +// full-file reload keeps the reader's scroll position, and that completion +// reconciles that position against the real body instead of leaving it at the +// loading placeholder's zero height. +func TestFileViewLifecycle_PendingRefreshPreservesViewport(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + name := "viewport.go" + var body strings.Builder + for i := 0; i < 200; i++ { + fmt.Fprintf(&body, "line %d\n", i) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body.String()), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m.altScreen = true + m.width = 100 + m.height = 30 + m = testOpenFile(m, name) + if !m.fileView.snapshotReady { + t.Fatal("expected the initial full-file snapshot") + } + + m.chatScrollOffset = 50 + updated, batchCmd := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) + m = updated.(model) + if batchCmd == nil { + t.Fatal("resize on an open full view must schedule a reload") + } + if !m.fileView.loading { + t.Fatal("reload should be pending after the resize") + } + if m.chatScrollOffset != 50 { + t.Fatalf("pending reload must preserve the reading position: got %d, want 50", m.chatScrollOffset) + } + + m = deliverCommandMessages(t, m, batchCmd) + if m.fileView.loading { + t.Fatal("expected the reload to complete") + } + if m.chatScrollOffset != 50 { + t.Fatalf("completed reload must preserve the reading position: got %d, want 50", m.chatScrollOffset) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "line 199") { + t.Fatal("expected the completed body to render the file tail") + } +} + +// TestFileViewLifecycle_BTWPreservesParentLoadOwnership verifies that a parent +// file load completing while a BTW conversation is visible is routed to the +// hidden parent, so returning restores a settled snapshot rather than a stalled +// "Loading…" view. +func TestFileViewLifecycle_BTWPreservesParentLoadOwnership(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + name := "parent_view.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package parent\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := newBTWTestModel(t) + m.cwd = dir + + // Open a full view but keep its load command pending in the parent. + m, parentLoadCmd := m.openFileView(name) + if parentLoadCmd == nil { + t.Fatal("expected a pending parent load command") + } + if !m.fileView.loading { + t.Fatal("parent view should be loading") + } + + // Enter BTW; the hidden parent retains ownership of its pending load. + side, _ := m.handleBTWCommand("") + if !side.btw.active || side.btw.parent == nil { + t.Fatal("expected an active BTW conversation") + } + + // The parent's load completes while BTW is visible. It must update the hidden + // parent, not the side surface. + updated, _ := side.Update(parentLoadCmd()) + side = updated.(model) + if side.btw.parent == nil { + t.Fatal("BTW lost the parent snapshot") + } + if side.btw.parent.fileView.loading { + t.Fatal("parent load completion must settle the hidden parent's loading state") + } + if !side.btw.parent.fileView.snapshotReady { + t.Fatal("hidden parent must integrate its snapshot before returning") + } + + restored, _ := side.leaveBTW() + if restored.fileView.loading { + t.Fatal("restored parent must not be stuck loading") + } + got := plainRender(t, restored.renderFileViewFull(80)) + if !strings.Contains(got, "package parent") { + t.Fatalf("restored parent must render its snapshot, got: %s", got) + } + if strings.Contains(got, fileViewLoadingPlaceholder) { + t.Fatal("restored parent must not show the loading placeholder") + } +} + +// TestFileViewLifecycle_RevisionMetadataBounded verifies that per-path revision +// counters for never-cached paths stay bounded, and that retiring a revision +// cannot make an older request appear current (ABA protection). +func TestFileViewLifecycle_RevisionMetadataBounded(t *testing.T) { + dir := t.TempDir() + warm := filepath.Join(dir, "revision_warm.go") + if err := os.WriteFile(warm, []byte("package warm\n"), 0o644); err != nil { + t.Fatal(err) + } + + cache := newFileViewRenderCache(defaultFileViewCacheMaxEntries, defaultFileViewCacheMaxBytes) + oldRev := cache.invalidatePath(warm) + if _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { + t.Fatalf("warm load: %v", err) + } + + for i := 0; i < 10000; i++ { + cache.invalidatePath(filepath.Join(dir, fmt.Sprintf("never_cached_%d.go", i))) + } + + cache.mu.Lock() + revisionCount := len(cache.pathRevisions) + epochFloor := cache.revEpochFloor + cache.mu.Unlock() + if revisionCount > fileViewMaxPathRevisions { + t.Fatalf("pathRevisions = %d, exceeds hard cap %d", revisionCount, fileViewMaxPathRevisions) + } + if revisionCount > cache.maxPathRevisions() { + t.Fatalf("pathRevisions = %d, exceeds policy bound %d", revisionCount, cache.maxPathRevisions()) + } + + if got := cache.requiredRevision(warm); got <= oldRev { + t.Fatalf("retired revision must not read as current: required=%d old=%d", got, oldRev) + } + if epochFloor <= oldRev { + t.Fatalf("epoch floor %d must advance past retired revision %d", epochFloor, oldRev) + } + + // A request captured before the retirement must not reuse the still-resident + // entry; it has to re-render under the advanced authority. + highlightBefore := cache.stats().HighlightCalls + if _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { + t.Fatalf("stale load: %v", err) + } + if highlightAfter := cache.stats().HighlightCalls; highlightAfter != highlightBefore+1 { + t.Fatalf("stale request must not commit a cache hit: HighlightCalls %d -> %d", highlightBefore, highlightAfter) + } +} + +// TestFileViewLifecycle_TruncationStateEquivalence verifies that two versions +// with identical size, timestamp, and retained source hash but a different +// truncation status are not interchangeable: the clipped version keeps its +// warning, and the warning disappears again when clipping stops. +func TestFileViewLifecycle_TruncationStateEquivalence(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + name := "truncation_state.go" + filePath := filepath.Join(dir, name) + stamp := time.Unix(1700000000, 0) + fill := strings.Repeat("x", fileViewMaxLineBytes) + + // Version A: CRLF-terminated, so the line is exactly at the limit and not clipped. + if err := os.WriteFile(filePath, []byte(fill+"\r\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(filePath, stamp, stamp); err != nil { + t.Fatal(err) + } + first, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("first load: %v", err) + } + if strings.Contains(plainRender(t, first), "truncated") { + t.Fatal("an unclipped line must not display a truncation warning") + } + + // Version B: the same 4098 bytes and timestamp, but the physical line now + // exceeds the per-line display limit while the retained line and hash do not. + if err := os.WriteFile(filePath, []byte(fill+"Z\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(filePath, stamp, stamp); err != nil { + t.Fatal(err) + } + second, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("second load: %v", err) + } + if !strings.Contains(plainRender(t, second), "truncated") { + t.Fatal("a clipped replacement must display its truncation warning") + } + + // Reverse: going back to the unclipped CRLF form must drop the warning. + if err := os.WriteFile(filePath, []byte(fill+"\r\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(filePath, stamp, stamp); err != nil { + t.Fatal(err) + } + third, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + if err != nil { + t.Fatalf("third load: %v", err) + } + if strings.Contains(plainRender(t, third), "truncated") { + t.Fatal("a previously cached warning must disappear when clipping stops") + } +} + +// TestFileViewLifecycle_VariantAccountingConsistency verifies that same-key +// variant rewrites do not charge the key twice, and that a detached or purged +// entry cannot leak bytes into the cache's retained counter. +func TestFileViewLifecycle_VariantAccountingConsistency(t *testing.T) { + // Same-key replacement accounts for the difference, not the key again. + entry := &fileViewCachedEntry{} + key := "80:" + current := strings.Repeat("a", 100) + if delta := entry.putRender(key, current); delta != len(key)+len(current) { + t.Fatalf("initial put delta = %d, want %d", delta, len(key)+len(current)) + } + for i := 0; i < 10; i++ { + next := strings.Repeat("b", 40+i) + delta := entry.putRender(key, next) + if want := len(next) - len(current); delta != want { + t.Fatalf("same-key rewrite %d delta = %d, want %d", i, delta, want) + } + current = next + } + if got, want := entry.byteSize(), len(key)+len(current); got != want { + t.Fatalf("entry byteSize = %d, want %d", got, want) + } + + resetFileViewCacheForTest() + dir := t.TempDir() + path := filepath.Join(dir, "variant_accounting.go") + if err := os.WriteFile(path, []byte("package accounting\n"), 0o644); err != nil { + t.Fatal(err) + } + cache := newFileViewRenderCache(64, defaultFileViewCacheMaxBytes) + if _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + t.Fatalf("seed load: %v", err) + } + + cache.mu.Lock() + elem := cache.items[path] + retainedBefore := cache.retained + var residentBytes int + if elem != nil { + residentBytes = elem.Value.(*fileViewCachedEntry).byteSize() + } + cache.mu.Unlock() + if elem == nil { + t.Fatal("expected a resident entry to seed the accounting check") + } + if retainedBefore != residentBytes { + t.Fatalf("retained = %d, want resident payload %d", retainedBefore, residentBytes) + } + + // Evicting the entry detaches it without destroying the value the caller holds. + cache.evictPath(path) + cache.mu.Lock() + afterEvict := cache.retained + _, resident := cache.items[path] + cache.mu.Unlock() + if afterEvict != 0 || resident { + t.Fatalf("eviction must release and detach the entry: retained=%d resident=%v", afterEvict, resident) + } + cache.commitRenderPut(elem.Value.(*fileViewCachedEntry), "80:detached", strings.Repeat("z", 512)) + cache.mu.Lock() + polluted := cache.retained + _, readmitted := cache.items[path] + cache.mu.Unlock() + if polluted != afterEvict { + t.Fatalf("a detached entry must not charge retained: %d -> %d", afterEvict, polluted) + } + if readmitted { + t.Fatal("a detached entry must not be re-admitted by commitRenderPut") + } + + // A purge detaches every entry the same way. + if _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + t.Fatalf("reload: %v", err) + } + cache.mu.Lock() + purged := cache.items[path].Value.(*fileViewCachedEntry) + cache.mu.Unlock() + cache.clear() + cache.commitRenderPut(purged, "80:purged", strings.Repeat("y", 512)) + cache.mu.Lock() + retainedAfterPurge := cache.retained + purgedItems := len(cache.items) + cache.mu.Unlock() + if retainedAfterPurge != 0 || purgedItems != 0 { + t.Fatalf("a purged entry must not charge retained: retained=%d items=%d", retainedAfterPurge, purgedItems) + } +} + +func TestFileViewReviewBTWInvalidation(t *testing.T) { + for _, parentResult := range []bool{false, true} { + scenario := "side_result_after_closing_inherited_view" + if parentResult { + scenario = "parent_result_with_distinct_visible_side_file" + } + t.Run(scenario, func(t *testing.T) { + resetFileViewCacheForTest() + m := newBTWTestModel(t) + m.cwd = t.TempDir() + write := func(name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(m.cwd, name), []byte(content), 0600); err != nil { + t.Fatal(err) + } + } + write("parent.txt", "parent before\n") + write("side.txt", "side before\n") + m.activeRunID = 1 + m = testOpenFile(m, "parent.txt") + side, cmd := m.handleBTWCommand("") + if !side.btw.active { + t.Fatal("fork failed") + } + side = deliverCommandMessages(t, side, cmd) + side = side.exitFileView() + if parentResult { + side = testOpenFile(side, "side.txt") + } + write("parent.txt", "parent after\n") + write("side.txt", "side after\n") + var msg tea.Msg = bashResultMsg{output: "done"} + if parentResult { + msg = agentRowMsg{runID: 1, row: transcriptRow{kind: rowToolResult, tool: "bash", status: tools.StatusOK}} + } + next, cmd := side.Update(msg) + side = deliverCommandMessages(t, next.(model), cmd) + if parentResult && !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "side after") { + t.Fatal("visible side did not recover after parent result") + } + parent, cmd := side.leaveBTW() + parent = deliverCommandMessages(t, parent, cmd) + if !strings.Contains(plainRender(t, parent.renderFileViewFull(80)), "parent after") { + t.Fatal("restored parent did not recover") + } + }) + } +} + +func TestFileViewReviewAbsolutePosition(t *testing.T) { + for _, count := range []int{220, 60, 200} { + t.Run(fmt.Sprint(count), func(t *testing.T) { + resetFileViewCacheForTest() + m := filesPanelTestModel() + m.cwd = t.TempDir() + m.altScreen = true + m.width = 100 + m.height = 30 + write := func(n int) { + t.Helper() + var body strings.Builder + for i := 0; i < n; i++ { + fmt.Fprintf(&body, "numbered-line-%03d\n", i) + } + if err := os.WriteFile(filepath.Join(m.cwd, "lines.txt"), []byte(body.String()), 0600); err != nil { + t.Fatal(err) + } + } + write(200) + m = testOpenFile(m, "lines.txt") + // The shell is still running; the reader scrolls before its result arrives. + m.pending = true + m = m.scrollChat(50) + m = m.syncChatScroll() + before := plainRender(t, m.View().Content) + if !strings.Contains(before, "numbered-line-149") { + t.Fatal("scrolled viewport must visibly include line 149 before refresh") + } + write(count) + next, cmd := m.Update(bashResultMsg{output: "done"}) + m = deliverCommandMessages(t, next.(model), cmd) + want := maxInt(0, 50+count-200) + if m.chatScrollOffset != want { + t.Fatalf("offset=%d want=%d", m.chatScrollOffset, want) + } + after := plainRender(t, m.View().Content) + if count >= 200 { + for i := 0; i < 200; i++ { + line := fmt.Sprintf("numbered-line-%03d", i) + if strings.Contains(before, line) != strings.Contains(after, line) { + t.Fatalf("visible line changed: %s", line) + } + } + } else if !strings.Contains(after, "numbered-line-059") { + t.Fatal("shrink must clamp to tail") + } + next, cmd = m.Update(tea.WindowSizeMsg{Width: 90, Height: 30}) + m = deliverCommandMessages(t, next.(model), cmd) + if m.chatScrollOffset != want { + t.Fatalf("same height resize changed offset to %d", m.chatScrollOffset) + } + }) + } +} + +func TestFileViewReviewMarkerReplacement(t *testing.T) { + for _, entry := range []string{"compact", "resume", "picker"} { + t.Run(entry, func(t *testing.T) { + resetFileViewCacheForTest() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", home) + t.Setenv("XDG_CACHE_HOME", home) + t.Setenv("XDG_STATE_HOME", home) + t.Setenv("APPDATA", home) + t.Setenv("LOCALAPPDATA", home) + m := newBTWTestModel(t) + m.cwd = t.TempDir() + if err := os.WriteFile(filepath.Join(m.cwd, "marked.go"), []byte("var markedValue = 42\n"), 0600); err != nil { + t.Fatal(err) + } + preview := "--- a/marked.go\n+++ b/marked.go\n@@ -1,1 +1,1 @@\n-old value\n+var markedValue = 42" + _, err := m.sessionStore.AppendEvent(m.activeSession.SessionID, sessions.AppendEventInput{Type: sessions.EventToolResult, Payload: map[string]any{"changedFiles": []string{"marked.go"}, "toolCallId": "edit-review", "name": "edit_file", "status": "ok", "output": "Successfully edited marked.go (replaced 1 occurrence).", "displayPreview": preview}}) + if err != nil { + t.Fatal(err) + } + m, text := m.handleResumeCommand(m.activeSession.SessionID) + if text != "" { + t.Fatal(text) + } + m = testOpenFile(m, "marked.go") + m = testSetMode(m, fileViewFull) + marker := func(m model) bool { return strings.Contains(plainRender(t, m.renderFileViewFull(80)), "▎") } + if !marker(m) { + t.Fatal("initial gutter marker missing") + } + var next tea.Model + var cmd tea.Cmd + if entry == "compact" { + m, cmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + stale := cmd() + m.compactInFlight = true + next, cmd = m.Update(compactResultMsg{hasSessionSnapshot: true, activeSession: m.activeSession, transcript: initialTranscript()}) + m = deliverCommandMessages(t, next.(model), cmd) + next, _ = m.Update(stale) + m = next.(model) + if marker(m) || len(m.fileViewChangedLines()) != 0 { + t.Fatal("compaction retained obsolete gutter") + } + } else { + next, cmd = m.dispatchCommand(parsedCommand{kind: commandClear}) + m = deliverCommandMessages(t, next.(model), cmd) + if marker(m) { + t.Fatal("clear retained marker") + } + m, cmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + stale := cmd() + if entry == "resume" { + next, cmd = m.dispatchCommand(parsedCommand{kind: commandResume, text: m.activeSession.SessionID}) + } else { + m.picker = &commandPicker{kind: pickerSession, items: []pickerItem{{Value: m.activeSession.SessionID}}} + next, cmd = m.choosePicker() + } + m = deliverCommandMessages(t, next.(model), cmd) + next, _ = m.Update(stale) + m = next.(model) + if !marker(m) || !m.fileViewChangedLines()["var markedValue = 42"] { + t.Fatal("same-session resume did not restore gutter") + } + } + }) + } +} + +// TestFileViewSpecialistCompletionRefreshesUnknownScope covers the hidden +// successful Task result: specialistCompleteMsg is the only boundary that sees +// the child's mutation, so it must conservatively reload an open full view even +// outside a git workspace. +func TestFileViewSpecialistCompletionRefreshesUnknownScope(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "specialist_mut.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 9 + // Non-git workspace: the end-of-turn sweep cannot help here. + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m = testOpenFile(m, name) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package old") { + t.Fatal("initial snapshot must render package old") + } + if err := os.WriteFile(filepath.Join(dir, name), []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(specialistCompleteMsg{ + runID: 9, + toolCallID: "call-specialist", + childSessionID: "sess-specialist", + status: specialistCompleted, + }) + m = updated.(model) + if cmd == nil { + t.Fatal("specialist completion must schedule a conservative refresh") + } + m = deliverCommandMessages(t, m, cmd) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("hidden specialist mutation left a stale snapshot: %s", got) + } +} + +// TestFileViewTerminalSessionResultRefreshesUnknownScope covers tool results +// that mutate the workspace but report no ChangedFiles. +func TestFileViewTerminalSessionResultRefreshesUnknownScope(t *testing.T) { + for _, tool := range []string{"terminal_session", "write_stdin", "swarm_collect", "swarm_status"} { + t.Run(tool, func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "terminal_mut.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 11 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m = testOpenFile(m, name) + if err := os.WriteFile(filepath.Join(dir, name), []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(agentRowMsg{ + runID: 11, + row: transcriptRow{ + kind: rowToolResult, + tool: tool, + status: tools.StatusOK, + }, + }) + m = updated.(model) + if cmd == nil { + t.Fatalf("%s result with no changedFiles must still invalidate the open view", tool) + } + m = deliverCommandMessages(t, m, cmd) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("%s mutation left a stale snapshot: %s", tool, got) + } + }) + } +} + +// TestFileViewTaskOutputSuppressedResultRefreshesUnknownScope covers the hidden +// TaskOutput success boundary delivered as unknownScopeMutationMsg. +func TestFileViewTaskOutputSuppressedResultRefreshesUnknownScope(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "taskoutput_mut.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 13 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m = testOpenFile(m, name) + if !toolResultCardSuppressedInTranscript("TaskOutput", tools.StatusOK) || !toolResultMayMutateUnknownScope("TaskOutput") { + t.Fatal("TaskOutput must be classified as a suppressed unknown-scope mutator") + } + if err := os.WriteFile(filepath.Join(dir, name), []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(unknownScopeMutationMsg{runID: 13}) + m = updated.(model) + if cmd == nil { + t.Fatal("suppressed TaskOutput completion must schedule a conservative refresh") + } + m = deliverCommandMessages(t, m, cmd) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("hidden TaskOutput mutation left a stale snapshot: %s", got) + } +} + +// TestFileViewTurnEndRefreshesInNonGitWorkspace verifies the end-of-turn +// fallback: maybeGitSweep does nothing outside git, but an open view still +// reloads so an unreported mutation cannot stay stale. +func TestFileViewTurnEndRefreshesInNonGitWorkspace(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "turn_end.go" + if err := os.WriteFile(filepath.Join(dir, name), []byte("package old\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 17 + m.pending = true + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m = testOpenFile(m, name) + if err := os.WriteFile(filepath.Join(dir, name), []byte("package new\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(agentResponseMsg{ + runID: 17, + rows: []transcriptRow{{kind: rowAssistant, text: "done", final: true}}, + }) + m = updated.(model) + if cmd == nil { + t.Fatal("turn end must schedule a conservative refresh even without git") + } + m = deliverCommandMessages(t, m, cmd) + got := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(got, "package new") { + t.Fatalf("turn-end mutation left a stale snapshot outside git: %s", got) + } +} + +// TestFileViewPendingReloadAcceptsScrollInput verifies that scrolling while an +// async reload is in flight updates the pending intent against the last +// completed body, so a Page Down is not discarded by the one-line placeholder. +func TestFileViewPendingReloadAcceptsScrollInput(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "pending_scroll.go" + var body strings.Builder + for i := 0; i < 200; i++ { + fmt.Fprintf(&body, "line %d\n", i) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body.String()), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.altScreen = true + m.width = 100 + m.height = 30 + m = testOpenFile(m, name) + if !m.fileView.snapshotReady { + t.Fatal("expected the initial full-file snapshot") + } + m.chatScrollOffset = 50 + // Trigger an async reload via resize, exactly like the review scenario. + updated, batchCmd := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) + m = updated.(model) + if !m.fileView.loading { + t.Fatal("reload should be pending after the resize") + } + before := m.fileView.preservedScrollOffset + // Page Down while the reload is in flight: intent must move toward the tail. + updated, _ = m.Update(testKey(tea.KeyPgDown)) + m = updated.(model) + if m.fileView.preservedScrollOffset >= before { + t.Fatalf("Page Down during a pending reload must lower the intent: before=%d after=%d", before, m.fileView.preservedScrollOffset) + } + if m.chatScrollOffset != m.fileView.preservedScrollOffset { + t.Fatalf("syncChatScroll must hold the updated intent: offset=%d intent=%d", m.chatScrollOffset, m.fileView.preservedScrollOffset) + } + want := m.fileView.preservedScrollOffset + m = deliverCommandMessages(t, m, batchCmd) + if m.fileView.loading { + t.Fatal("expected the reload to complete") + } + if m.chatScrollOffset != want && want > 0 { + t.Fatalf("completed reload must apply the latest intent: got %d, want %d", m.chatScrollOffset, want) + } +} + +// TestFileViewPendingReloadPromptSubmissionReturnsToBottom verifies that a real +// prompt submitted while a reload is pending records the bottom intent, so +// completion cannot restore the reader's pre-submission position. +func TestFileViewPendingReloadPromptSubmissionReturnsToBottom(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "pending_submit.go" + var body strings.Builder + for i := 0; i < 200; i++ { + fmt.Fprintf(&body, "line %d\n", i) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body.String()), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.altScreen = true + m.width = 100 + m.height = 30 + m = testOpenFile(m, name) + m.chatScrollOffset = 50 + updated, batchCmd := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) + m = updated.(model) + if !m.fileView.loading { + t.Fatal("reload should be pending after the resize") + } + m.input.SetValue("/help") + updated, _ = m.handleSubmit() + m = updated.(model) + if m.fileView.preservedScrollOffset != 0 { + t.Fatalf("prompt submission during a pending reload must record the bottom intent, got %d", m.fileView.preservedScrollOffset) + } + if m.chatScrollOffset != 0 { + t.Fatalf("prompt submission must snap the viewport to the bottom, got %d", m.chatScrollOffset) + } + m = deliverCommandMessages(t, m, batchCmd) + if m.chatScrollOffset != 0 { + t.Fatalf("completed reload must respect the bottom intent, got %d", m.chatScrollOffset) + } +} + +// TestFileViewCrossSurfacePathMutationOutsideGit covers the review matrix for a +// workspace with no git sweep: a path-reporting tool result on one surface must +// reload the other surface's open full-file view immediately, without a turn-end +// fallback or a git scan. +func TestFileViewCrossSurfacePathMutationOutsideGit(t *testing.T) { + const ( + before = "package before\n" + after = "package after\n" + ) + writeFile := func(t *testing.T, dir, name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + nonGit := func(t *testing.T, dir string) model { + t.Helper() + m := newBTWTestModel(t) + m.cwd = dir + m.width = 100 + m.height = 30 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + return m + } + for _, tool := range []string{"edit_file", "write_file", "apply_patch"} { + t.Run("parent_"+tool+"_refreshes_side", func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "side_shown.go" + writeFile(t, dir, name, before) + m := nonGit(t, dir) + m.pending = true + m.runID = 4 + m.activeRunID = 4 + side, cmd := m.handleBTWCommand("") + if cmd != nil { + t.Fatal("bare /btw must not start work") + } + if !side.btw.active || side.btw.parent == nil || side.btw.parent.activeRunID != 4 { + t.Fatal("parent run was not preserved") + } + side = testOpenFile(side, name) + if side.fileView.mode != fileViewFull { + t.Fatal("side must open the untouched file in full mode") + } + if !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "package before") { + t.Fatal("side snapshot must show the pre-mutation content") + } + writeFile(t, dir, name, after) + updated, cmd := side.Update(agentRowMsg{ + runID: 4, + row: transcriptRow{ + kind: rowToolResult, + tool: tool, + status: tools.StatusOK, + changedFiles: []string{"other.go", name}, + }, + }) + side = updated.(model) + if cmd == nil { + t.Fatalf("%s on the parent must schedule a side reload without a git sweep", tool) + } + side = deliverCommandMessages(t, side, cmd) + got := plainRender(t, side.renderFileViewFull(80)) + if !strings.Contains(got, "package after") || strings.Contains(got, "package before") { + t.Fatalf("%s left the side stale outside git: %s", tool, got) + } + }) + t.Run("side_"+tool+"_refreshes_parent", func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "parent_shown.go" + writeFile(t, dir, name, before) + m := nonGit(t, dir) + m = testOpenFile(m, name) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package before") { + t.Fatal("parent snapshot must show the pre-mutation content") + } + side, _ := m.handleBTWCommand("") + if !side.btw.active || side.btw.parent == nil { + t.Fatal("fork failed") + } + // Only the hidden parent keeps the file open. The inherited side load + // is revoked here and must not be delivered. + side = side.exitFileView() + side.activeRunID = side.btw.sideRunIDBase + side.runID = side.btw.sideRunIDBase + writeFile(t, dir, name, after) + updated, cmd := side.Update(agentRowMsg{ + runID: side.activeRunID, + row: transcriptRow{ + kind: rowToolResult, + tool: tool, + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + side = updated.(model) + if cmd == nil { + t.Fatalf("%s on the side must schedule a parent reload immediately", tool) + } + side = deliverCommandMessages(t, side, cmd) + if side.btw.parent == nil { + t.Fatal("hidden parent disappeared") + } + got := plainRender(t, side.btw.parent.renderFileViewFull(80)) + if !strings.Contains(got, "package after") || strings.Contains(got, "package before") { + t.Fatalf("%s left the parent stale outside git: %s", tool, got) + } + if side.btw.parent.fileView.loading { + t.Fatal("parent reload must have settled before returning from BTW") + } + }) + } + t.Run("unrelated_path_does_not_reload_side", func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + writeFile(t, dir, "keep.go", before) + m := nonGit(t, dir) + m.pending = true + m.runID = 4 + m.activeRunID = 4 + side, _ := m.handleBTWCommand("") + side = testOpenFile(side, "keep.go") + updated, cmd := side.Update(agentRowMsg{ + runID: 4, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{"other.go"}, + }, + }) + side = updated.(model) + if cmd != nil || side.fileView.loading || !side.fileView.snapshotReady { + t.Fatal("a path the side is not showing must not reload its snapshot") + } + got := plainRender(t, side.renderFileViewFull(80)) + if !strings.Contains(got, "package before") { + t.Fatalf("unrelated mutation disturbed the side snapshot: %s", got) + } + }) +} + +// TestFileViewCrossSurfaceMixedWriteStdinOutsideGit verifies that a mixed +// write_stdin result carrying both an exact changed path and an aggregated +// directory summary (e.g. dist/) refreshes both BTW surfaces outside a git +// workspace, whether the exact path is open on the parent or on the side. +func TestFileViewCrossSurfaceMixedWriteStdinOutsideGit(t *testing.T) { + const ( + exactBefore = "export const exact = 'before';\n" + exactAfter = "export const exact = 'after';\n" + distBefore = "console.log('dist before');\n" + distAfter = "console.log('dist after');\n" + ) + writeFile := func(t *testing.T, dir, rel, content string) { + t.Helper() + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + nonGit := func(t *testing.T, dir string) model { + t.Helper() + m := newBTWTestModel(t) + m.cwd = dir + m.width = 100 + m.height = 30 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + return m + } + mixedRow := func() transcriptRow { + return transcriptRow{ + kind: rowToolResult, + tool: "write_stdin", + status: tools.StatusOK, + changedFiles: []string{"src/exact.ts"}, + changeSummaries: []execution.Change{ + {Path: "dist/", Kind: execution.ChangeCreated, Aggregated: true}, + }, + } + } + + t.Run("parent_mixed_write_stdin_refreshes_visible_side_under_dist", func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + writeFile(t, dir, "src/exact.ts", exactBefore) + writeFile(t, dir, "dist/bundle.js", distBefore) + + m := nonGit(t, dir) + m.pending = true + m.runID = 4 + m.activeRunID = 4 + // Parent holds the exact path; side opens a generated file under dist/. + m = testOpenFile(m, "src/exact.ts") + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "before") { + t.Fatal("parent snapshot must show initial exact file") + } + + side, cmd := m.handleBTWCommand("") + side = deliverCommandMessages(t, side, cmd) + side = side.exitFileView() + side = testOpenFile(side, "dist/bundle.js") + if !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "dist before") { + t.Fatal("side snapshot must show initial dist/bundle.js") + } + + writeFile(t, dir, "src/exact.ts", exactAfter) + writeFile(t, dir, "dist/bundle.js", distAfter) + + updated, cmd := side.Update(agentRowMsg{runID: 4, row: mixedRow()}) + side = updated.(model) + if cmd == nil { + t.Fatal("mixed write_stdin on parent must schedule reloads for both surfaces") + } + side = deliverCommandMessages(t, side, cmd) + + gotSide := plainRender(t, side.renderFileViewFull(80)) + if !strings.Contains(gotSide, "dist after") || strings.Contains(gotSide, "dist before") { + t.Fatalf("visible side under dist/ stayed stale after parent mixed write_stdin: %s", gotSide) + } + parent, leaveCmd := side.leaveBTW() + parent = deliverCommandMessages(t, parent, leaveCmd) + gotParent := plainRender(t, parent.renderFileViewFull(80)) + if !strings.Contains(gotParent, "after") || strings.Contains(gotParent, "before") { + t.Fatalf("restored parent exact path stayed stale after parent mixed write_stdin: %s", gotParent) + } + }) + + t.Run("side_mixed_write_stdin_refreshes_hidden_parent_under_dist", func(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + writeFile(t, dir, "src/exact.ts", exactBefore) + writeFile(t, dir, "dist/bundle.js", distBefore) + + m := nonGit(t, dir) + // Parent holds the file under dist/; side holds the exact path. + m = testOpenFile(m, "dist/bundle.js") + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "dist before") { + t.Fatal("parent snapshot must show initial dist/bundle.js") + } + + side, cmd := m.handleBTWCommand("") + side = deliverCommandMessages(t, side, cmd) + side = side.exitFileView() + side = testOpenFile(side, "src/exact.ts") + if !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "before") { + t.Fatal("side snapshot must show initial src/exact.ts") + } + side.activeRunID = side.btw.sideRunIDBase + side.runID = side.btw.sideRunIDBase + + writeFile(t, dir, "src/exact.ts", exactAfter) + writeFile(t, dir, "dist/bundle.js", distAfter) + + updated, cmd := side.Update(agentRowMsg{runID: side.activeRunID, row: mixedRow()}) + side = updated.(model) + if cmd == nil { + t.Fatal("mixed write_stdin on side must schedule reloads for both surfaces") + } + side = deliverCommandMessages(t, side, cmd) + + gotSide := plainRender(t, side.renderFileViewFull(80)) + if !strings.Contains(gotSide, "after") || strings.Contains(gotSide, "before") { + t.Fatalf("side exact path stayed stale after side mixed write_stdin: %s", gotSide) + } + if side.btw.parent == nil { + t.Fatal("hidden parent disappeared") + } + if side.btw.parent.fileView.loading { + t.Fatal("hidden parent reload must settle before returning from BTW") + } + gotHiddenParent := plainRender(t, side.btw.parent.renderFileViewFull(80)) + if !strings.Contains(gotHiddenParent, "dist after") || strings.Contains(gotHiddenParent, "dist before") { + t.Fatalf("hidden parent under dist/ stayed stale after side mixed write_stdin: %s", gotHiddenParent) + } + + parent, leaveCmd := side.leaveBTW() + parent = deliverCommandMessages(t, parent, leaveCmd) + gotParent := plainRender(t, parent.renderFileViewFull(80)) + if !strings.Contains(gotParent, "dist after") || strings.Contains(gotParent, "dist before") { + t.Fatalf("restored parent under dist/ stayed stale on return from BTW: %s", gotParent) + } + }) +} diff --git a/internal/tui/file_view_unix_test.go b/internal/tui/file_view_unix_test.go new file mode 100644 index 000000000..3604a5fda --- /dev/null +++ b/internal/tui/file_view_unix_test.go @@ -0,0 +1,55 @@ +//go:build !windows + +package tui + +import ( + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func TestFileView_SpecialTargetNonRegularFileRejected(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + pipePath := filepath.Join(dir, "test.pipe") + + // Create a FIFO if supported on the platform + if err := syscall.Mkfifo(pipePath, 0o600); err != nil { + t.Skipf("mkfifo not supported on this environment: %v", err) + } + + res := readFileViewBounded(pipePath, 100, 1000, 10000) + if res.err == nil { + t.Fatal("expected error reading FIFO non-regular file, got nil") + } + if !strings.Contains(res.err.Error(), "non-regular") { + t.Fatalf("expected non-regular error message, got: %v", res.err) + } +} + +func TestFileView_FIFONeverBlocksOpen(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + pipePath := filepath.Join(dir, "hang_test.pipe") + + if err := syscall.Mkfifo(pipePath, 0o600); err != nil { + t.Skipf("mkfifo not supported on this environment: %v", err) + } + + done := make(chan fileViewReadResult, 1) + go func() { + res := readFileViewBounded(pipePath, 100, 1000, 10000) + done <- res + }() + + select { + case res := <-done: + if res.err == nil || !strings.Contains(res.err.Error(), "non-regular") { + t.Fatalf("expected non-regular error, got: %v", res.err) + } + case <-time.After(500 * time.Millisecond): + t.Fatal("readFileViewBounded blocked on open(2) for FIFO without writer (TOCTOU / missing O_NONBLOCK)") + } +} diff --git a/internal/tui/files_git_sweep_test.go b/internal/tui/files_git_sweep_test.go index cccdcb4d9..f2f2f6e2f 100644 --- a/internal/tui/files_git_sweep_test.go +++ b/internal/tui/files_git_sweep_test.go @@ -152,12 +152,12 @@ func TestTouchedFilesMergesGitSweep(t *testing.T) { func TestOpenFileViewGitOnlyFallsBackToFull(t *testing.T) { m := filesPanelTestModel() m.gitTouched = []gitSweepFile{{path: "kanban/board.tsx", created: true}} - m = m.openFileView("kanban/board.tsx") + 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") + m, _ = m.openFileView("web/app.js") if m.fileView.mode != fileViewDiff { t.Fatal("a file with edit cards still opens in diff mode") } diff --git a/internal/tui/files_panel.go b/internal/tui/files_panel.go index ea5485f5d..14da09ac1 100644 --- a/internal/tui/files_panel.go +++ b/internal/tui/files_panel.go @@ -15,6 +15,8 @@ import ( "sort" "strings" + tea "charm.land/bubbletea/v2" + "github.com/Twigpine/zero/internal/tools" ) @@ -357,8 +359,13 @@ func (m *model) setSelectedFile(path string) { // 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 { +// reading selectedFile (rowTouchesSelectedFile). A second activation of the +// same path drills into the file view and returns any load command. +func (m model) selectFile(path string) (model, tea.Cmd) { + if m.selectedFile == path { + m.runDetailsOpen = false + return m.openFileView(path) + } rowIndex := m.lastRowIndexForFile(path) m.setSelectedFile(path) if offset, ok := m.scrollOffsetForTranscriptRow(rowIndex); ok { @@ -367,7 +374,46 @@ func (m model) selectFile(path string) model { m.chatBodyLines = 0 } } - return m + return m, nil +} + +func (m model) runDetailsInnerWidth() int { + overlayWidth := minInt(72, maxInt(40, m.width-8)) + overlayWidth = minInt(overlayWidth, m.width) + return maxInt(12, overlayWidth-4) +} + +func (m model) runDetailsFileAtMouse(msg tea.MouseMsg) (string, bool) { + if !m.runDetailsVisible() { + return "", false + } + width := chatWidth(m.width) + overlay := m.runDetailsOverlay(width) + hit, ok := m.overlayMouseHit(msg, overlay, width) + if !ok { + return "", false + } + inner := m.runDetailsInnerWidth() + layout := m.runDetailsLayout(inner) + if layout.fileStart < 0 { + return "", false + } + // styledBlockFillTitle contributes a 1-line top border with title before the content rows. + const topBorderHeight = 1 + y := hit.y - topBorderHeight + var match fileHit + found := false + for _, h := range layout.fileHits { + if y == layout.fileStart+h.lineOffset { + match = h + found = true + break + } + } + if !found { + return "", false + } + return match.path, true } // scrollOffsetForTranscriptRow computes the chatScrollOffset that places the diff --git a/internal/tui/files_panel_test.go b/internal/tui/files_panel_test.go index b45ba9eda..a6e3ac3c4 100644 --- a/internal/tui/files_panel_test.go +++ b/internal/tui/files_panel_test.go @@ -6,6 +6,8 @@ import ( tea "charm.land/bubbletea/v2" + "github.com/Twigpine/zero/internal/config" + "github.com/Twigpine/zero/internal/dictation" "github.com/Twigpine/zero/internal/execution" "github.com/Twigpine/zero/internal/tools" ) @@ -296,6 +298,24 @@ func TestSidebarFileLinesOverflowExcludesLiveRow(t *testing.T) { // TestSidebarHasContentForLiveWrite: the sidebar counts an in-flight first // write as content, so the FILES pulse is visible before any result row lands. +func TestSidebarFileHitsDistinguishSuffixPaths(t *testing.T) { + m := sidebarTestModel() + m.transcript = append(m.transcript, + transcriptRow{kind: rowToolResult, tool: "edit_file", id: "a", status: tools.StatusOK, changedFiles: []string{"a.go"}}, + transcriptRow{kind: rowToolResult, tool: "edit_file", id: "b", status: tools.StatusOK, changedFiles: []string{"dir/a.go"}}, + ) + _, hits := m.sidebarFileLines(80) + if len(hits) < 2 { + t.Fatalf("want two fileHit rows, got %#v", hits) + } + if hits[0].path == hits[1].path { + t.Fatal("fileHit paths must stay distinct") + } + if hits[0].lineOffset == hits[1].lineOffset { + t.Fatal("fileHit rows must not share a line offset") + } +} + func TestSidebarHasContentForLiveWrite(t *testing.T) { m := sidebarTestModel() m.plan.steps = nil // drop the helper's seeded plan: no agents/plan/files now @@ -309,3 +329,175 @@ func TestSidebarHasContentForLiveWrite(t *testing.T) { t.Fatal("a live in-flight write must count as sidebar content") } } + +func TestRunDetailsFileMouseSelectionEndToEnd(t *testing.T) { + m := filesPanelTestModel() + m.width = 80 + m.height = 24 + m.altScreen = true + m.runDetailsOpen = true + + overlay := m.runDetailsOverlay(m.width) + if overlay == "" { + t.Fatal("expected non-empty runDetailsOverlay") + } + + inner := m.runDetailsInnerWidth() + layout := m.runDetailsLayout(inner) + if len(layout.fileHits) == 0 || layout.fileStart < 0 { + t.Fatalf("expected file hits in layout, got: %+v", layout) + } + + targetHit := layout.fileHits[0] // "internal/tui/sidebar.go" + overlayLines := viewLines(overlay) + left, trimmedLines, overlayWidth := normalizeOverlayBlock(overlayLines, m.width) + if overlayWidth <= 0 || len(trimmedLines) == 0 { + t.Fatalf("invalid overlay block normalization: left=%d, width=%d", left, overlayWidth) + } + + rect := m.overlayMouseRect(len(trimmedLines), m.width) + // Click on the target file row: top border is row 0 of the overlay, content starts at row 1 + clickY := rect.y + 1 + layout.fileStart + targetHit.lineOffset + clickX := left + 4 // inside the border and padding + + clickMsg := testMouseClick(tea.MouseLeft, clickX, clickY) + + path, ok := m.runDetailsFileAtMouse(clickMsg) + if !ok || path != targetHit.path { + t.Fatalf("runDetailsFileAtMouse failed: ok=%v, got=%q, want=%q", ok, path, targetHit.path) + } + + // Route the click through m.Update to verify end-to-end selection + updated, _ := m.Update(clickMsg) + m = updated.(model) + + if m.selectedFile != targetHit.path { + t.Fatalf("expected selectedFile %q after mouse click, got %q", targetHit.path, m.selectedFile) + } +} + +func TestRunDetailsFileMouseBlockedBehindHigherPriorityOverlays(t *testing.T) { + baseModel := func(t *testing.T) (model, tea.MouseClickMsg, string) { + t.Helper() + m := filesPanelTestModel() + m.width = 80 + m.height = 24 + m.altScreen = true + m.runDetailsOpen = true + + overlay := m.runDetailsOverlay(m.width) + if overlay == "" { + t.Fatal("expected non-empty runDetailsOverlay") + } + inner := m.runDetailsInnerWidth() + layout := m.runDetailsLayout(inner) + if len(layout.fileHits) == 0 || layout.fileStart < 0 { + t.Fatalf("expected file hits in layout, got: %+v", layout) + } + targetHit := layout.fileHits[0] + overlayLines := viewLines(overlay) + left, trimmedLines, overlayWidth := normalizeOverlayBlock(overlayLines, m.width) + if overlayWidth <= 0 || len(trimmedLines) == 0 { + t.Fatalf("invalid overlay block normalization: left=%d, width=%d", left, overlayWidth) + } + rect := m.overlayMouseRect(len(trimmedLines), m.width) + clickY := rect.y + 1 + layout.fileStart + targetHit.lineOffset + clickX := left + 4 + clickMsg := testMouseClick(tea.MouseLeft, clickX, clickY) + if path, ok := m.runDetailsFileAtMouse(clickMsg); !ok || path != targetHit.path { + t.Fatalf("precondition failed: visible Run details click should hit %q, got ok=%v path=%q", targetHit.path, ok, path) + } + return m, clickMsg, targetHit.path + } + + t.Run("speech_api_key_prompt_blocks_hidden_run_details_click", func(t *testing.T) { + m, clickMsg, wantPath := baseModel(t) + m.dictation.cfg = config.STTConfig{Provider: config.STTProviderGroq, Model: "whisper-large-v3-turbo"} + m.dictation.saveKey = func(string, string) error { return nil } + m.dictation.phase = dictTranscribing + + // Asynchronous dictation auth error opens the speech API-key prompt while Run details stays open. + updated, _ := m.Update(dictationTranscribedMsg{ + sessionID: m.dictation.sessionID, + err: &dictation.AuthError{Provider: "groq", Message: "auth error: your API key is missing or invalid"}, + }) + m = updated.(model) + if !m.runDetailsOpen || m.sttKeyPrompt == nil { + t.Fatalf("expected runDetailsOpen=true and sttKeyPrompt!=nil, got runDetailsOpen=%v sttKeyPrompt=%v", m.runDetailsOpen, m.sttKeyPrompt) + } + if m.runDetailsVisible() { + t.Fatal("Run details must not report visible while the speech API-key prompt owns the viewport") + } + if path, ok := m.runDetailsFileAtMouse(clickMsg); ok || path != "" { + t.Fatalf("runDetailsFileAtMouse must reject clicks behind speech prompt, got ok=%v path=%q", ok, path) + } + + // First and second clicks at the shared coordinate must neither select the file nor open fileView. + for i := 0; i < 2; i++ { + var cmd tea.Cmd + updated, cmd = m.Update(clickMsg) + m = updated.(model) + if cmd != nil { + t.Fatalf("click %d behind speech prompt should not return a command", i+1) + } + if m.selectedFile != "" || m.fileView.active { + t.Fatalf("click %d behind speech prompt leaked to Run details: selectedFile=%q fileView.active=%v", i+1, m.selectedFile, m.fileView.active) + } + } + + // Dismissing the speech prompt restores Run details to the foreground so the click works again. + updated, _ = m.Update(testKey(tea.KeyEsc)) + m = updated.(model) + if m.sttKeyPrompt != nil || !m.runDetailsVisible() { + t.Fatal("Run details should be visible again after dismissing the speech prompt") + } + updated, _ = m.Update(clickMsg) + m = updated.(model) + if m.selectedFile != wantPath { + t.Fatalf("expected selectedFile %q after speech prompt closed, got %q", wantPath, m.selectedFile) + } + }) + + t.Run("leader_help_overlay_blocks_hidden_run_details_click", func(t *testing.T) { + m, clickMsg, wantPath := baseModel(t) + + // Ctrl+X then ? opens the leader help overlay in front of Run details. + updated, _ := m.Update(testKeyCtrl('x')) + m = updated.(model) + updated, _ = m.Update(tea.KeyPressMsg{Code: '?', Text: "?"}) + m = updated.(model) + if !m.runDetailsOpen || !m.leaderHelpOverlay { + t.Fatalf("expected runDetailsOpen=true and leaderHelpOverlay=true, got runDetailsOpen=%v leaderHelpOverlay=%v", m.runDetailsOpen, m.leaderHelpOverlay) + } + if m.runDetailsVisible() { + t.Fatal("Run details must not report visible while leader help owns the viewport") + } + if path, ok := m.runDetailsFileAtMouse(clickMsg); ok || path != "" { + t.Fatalf("runDetailsFileAtMouse must reject clicks behind leader help, got ok=%v path=%q", ok, path) + } + + for i := 0; i < 2; i++ { + var cmd tea.Cmd + updated, cmd = m.Update(clickMsg) + m = updated.(model) + if cmd != nil { + t.Fatalf("click %d behind leader help should not return a command", i+1) + } + if m.selectedFile != "" || m.fileView.active { + t.Fatalf("click %d behind leader help leaked to Run details: selectedFile=%q fileView.active=%v", i+1, m.selectedFile, m.fileView.active) + } + } + + // Dismissing leader help with Esc restores Run details to the foreground. + updated, _ = m.Update(testKey(tea.KeyEsc)) + m = updated.(model) + if m.leaderHelpOverlay || !m.runDetailsVisible() { + t.Fatal("Run details should be visible again after dismissing leader help") + } + updated, _ = m.Update(clickMsg) + m = updated.(model) + if m.selectedFile != wantPath { + t.Fatalf("expected selectedFile %q after leader help closed, got %q", wantPath, m.selectedFile) + } + }) +} diff --git a/internal/tui/flush_test.go b/internal/tui/flush_test.go index 34b590ba0..d7ea742a0 100644 --- a/internal/tui/flush_test.go +++ b/internal/tui/flush_test.go @@ -111,7 +111,7 @@ func TestAltScreenSettledCacheInvalidatesWithFileSelection(t *testing.T) { t.Fatal("precondition: settled cache should be populated") } - m = m.selectFile("internal/tui/sidebar.go") + m, _ = m.selectFile("internal/tui/sidebar.go") if m.altScreenSettledWidth != 0 { t.Fatal("selecting a file touched by a settled row must invalidate the cache") } diff --git a/internal/tui/model.go b/internal/tui/model.go index 389acd46b..647a21dba 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -779,6 +779,13 @@ type specialistCompleteMsg struct { errorMsg string } +// unknownScopeMutationMsg carries the completion of a suppressed tool result +// (e.g. TaskOutput) that may have mutated workspace files without reporting the +// affected paths. It triggers the conservative file-view refresh. +type unknownScopeMutationMsg struct { + runID int +} + // swarmSessionsMsg carries swarm task_id -> member session_id pairs (from // swarm_collect's Meta) so the AGENTS sidebar rows can drill into a member's // session like a specialist card. @@ -1439,13 +1446,16 @@ func batchCommands(cmds ...tea.Cmd) tea.Cmd { } func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { + var resizeCmd tea.Cmd if size, ok := msg.(tea.WindowSizeMsg); ok { - m = m.resizeBTWParent(size) + m, resizeCmd = m.resizeBTWParent(size) } if next, cmd, routed := m.routeBTWParentMessage(msg); routed { - return next, cmd + return next, batchCommands(cmd, resizeCmd) } switch msg := msg.(type) { + case fileViewLoadedMsg: + return m.handleFileViewLoaded(msg) case uv.CellSizeEvent: if msg.Width > 0 && msg.Height > 0 { m.petCellPixelWidth = msg.Width @@ -1508,6 +1518,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.hasDarkBg = msg.IsDark() if m.themeMode != themeSystem { applyTheme(m.themeMode, m.hasDarkBg) + if m.fileView.active && m.fileView.mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } } return m, nil case tea.MouseMsg: @@ -1763,6 +1776,18 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { case m.runDetailsOpen: if keyIs(msg, tea.KeyEsc) || m.keyMatch(m.keyBindings.toggleSidebar, msg, func(tea.KeyMsg) bool { return keyCtrl(msg, 'b') }) { m.runDetailsOpen = false + return m, nil + } + if keyIs(msg, tea.KeyEnter) && m.selectedFile != "" { + overlayWidth := minInt(72, maxInt(40, m.width-8)) + inner := maxInt(12, overlayWidth-4) + layout := m.runDetailsLayout(inner) + for _, h := range layout.fileHits { + if h.path == m.selectedFile { + return m.selectFile(m.selectedFile) + } + } + return m, nil } return m, nil case m.keyMatch(m.keyBindings.toggleDetailed, msg, func(tea.KeyMsg) bool { return keyCtrl(msg, 'o') }): @@ -1772,9 +1797,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // (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 + return m.setFileViewMode(fileViewFull) } - return m.setFileViewMode(fileViewDiff), nil + return m.setFileViewMode(fileViewDiff) case m.keyMatch(m.keyBindings.toggleMouse, msg, func(tea.KeyMsg) bool { return keyCtrl(msg, 'e') }) && canFireComposerGatedToggle(m.keyBindings.toggleMouse, defaultToggleMouseChord, m.composerValue() == ""): // Release/recapture the mouse so the user can drag-select and copy text // natively (mouse capture otherwise intercepts terminal selection). The @@ -2561,7 +2586,12 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } // A resumed/idle session may already hold agents; keep their short lifecycle // fade alive. No-op when the loop is already running or nothing animates. - return m, m.ensureSpinnerTick() + if m.fileView.active && m.fileView.mode == fileViewFull { + var cmd tea.Cmd + m, cmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + return m, batchCommands(m.ensureSpinnerTick(), cmd, resizeCmd) + } + return m, batchCommands(m.ensureSpinnerTick(), resizeCmd) case permissionRequestMsg: // The agent goroutine that raised this request is BLOCKED waiting on the // decision callback, so every branch below must resolve it exactly once — @@ -2839,6 +2869,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // complete once the turn settles. var sweepCmd tea.Cmd m, sweepCmd = m.maybeGitSweep() + // The sweep is a no-op outside a git workspace, so an open full-file view + // still needs the conservative invalidation to pick up mutations whose + // paths were never reported (e.g. a hidden TaskOutput success). + var fileRefreshCmd tea.Cmd + m, fileRefreshCmd = m.invalidateFileViewForUnknownMutation() // If this run was a loop iteration, advance that loop (schedule its next wake // or stop it). Done before launchQueuedMessageIfReady so a user's queued prompt // still wins the immediate re-launch; the loop fires on the next idle tick. @@ -2870,7 +2905,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.goalAware && !hadQueuedMessage && peerCmd == nil && next.pendingPermission == nil && msg.specReview == nil { next, goalCmd = next.launchGoalContinuationIfReady() } - return next, tea.Batch(pendingClearCmd, titleCmd, recapCmd, sweepCmd, queuedCmd, peerCmd, peerApprovalCmd, loopTickCmd, goalCmd) + return next, tea.Batch(pendingClearCmd, titleCmd, recapCmd, sweepCmd, fileRefreshCmd, queuedCmd, peerCmd, peerApprovalCmd, loopTickCmd, goalCmd) case sessionTitleGeneratedMsg: return m.handleSessionTitleGenerated(msg) case recapIdleMsg: @@ -2898,6 +2933,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } m.lastCompactResult = &msg.result m = m.setCompactStatusRow(m.compactText(true)) + if msg.hasSessionSnapshot { + return m.refreshFileViewMarkers() + } return m, nil case planUpdateMsg: if msg.runID != m.activeRunID { @@ -2976,7 +3014,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } m.transcript = appendTranscriptRow(m.transcript, cardRow) } - return m, nil + // A specialist (Task) works in a separate workspace-visible process and + // its successful result is suppressed from the transcript, so this is the + // only boundary that sees the mutation. Conservatively invalidate the open + // snapshot: the child may have changed files without a path report. + var refresh tea.Cmd + m, refresh = m.invalidateFileViewForUnknownMutation() + return m, refresh case specialistProgressMsg: if msg.runID != m.activeRunID { return m, nil @@ -3031,10 +3075,69 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // 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 + if msg.row.kind == rowToolResult { + var loadCmds []tea.Cmd + invalidated := false + selfNeedsRefresh := false + for _, p := range msg.row.changedFiles { + invalidated = true + target := p + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + rev := defaultFileViewCache.invalidatePath(target) + if m.fileView.active && (p == m.fileView.path || target == m.fileView.path) { + if rev > m.fileView.requiredSourceRev { + m.fileView.requiredSourceRev = rev + } else { + m.fileView.requiredSourceRev++ + } + if m.fileView.mode == fileViewFull { + selfNeedsRefresh = true + } + } + } + // A tool that can mutate files outside exact ChangedFiles (command + // tools, terminal sessions, swarm status/collect, or results with + // aggregated directory summaries) must also run the conservative + // unknown-scope refresh even when exact paths were reported. + unknownScope := isPlanCommandTool(msg.row.tool) || + toolResultMayMutateUnknownScope(msg.row.tool) || + len(msg.row.changeSummaries) > 0 + if unknownScope { + var sweep tea.Cmd + m, sweep = m.maybeGitSweep() + if sweep != nil { + loadCmds = append(loadCmds, sweep) + } + var refresh tea.Cmd + m, refresh = m.invalidateFileViewForUnknownMutation() + if refresh != nil { + loadCmds = append(loadCmds, refresh) + } + selfNeedsRefresh = false + } + if selfNeedsRefresh { + var cmd tea.Cmd + m, cmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + if cmd != nil { + loadCmds = append(loadCmds, cmd) + } + } + // The handler above only reloads the surface that received the result. + // A side mutation must also wake the hidden parent when that parent is + // showing the same path; the reverse direction is recovered after the + // parent message is routed. Neither wait depends on a git sweep. + if invalidated && !unknownScope { + var peerCmd tea.Cmd + m, peerCmd = m.refreshHiddenParentFileView() + if peerCmd != nil { + loadCmds = append(loadCmds, peerCmd) + } + } + if len(loadCmds) > 0 { + return m, batchCommands(loadCmds...) + } } return m, nil case swarmSessionsMsg: @@ -3050,6 +3153,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } } return m, nil + case unknownScopeMutationMsg: + // A suppressed mutating result (TaskOutput) completed. Its success row is + // hidden, so trigger the conservative refresh on the owning surface. + if msg.runID != m.activeRunID { + return m, nil + } + return m.invalidateFileViewForUnknownMutation() case doctorCommandResultMsg: if msg.id == 0 || msg.id == m.doctorCommandSeq { m.doctorInFlight = false @@ -3067,7 +3177,22 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.prState = msg.state return m, nil case gitSweepMsg: - return m.handleGitSweepMsg(msg), nil + m = m.handleGitSweepMsg(msg) + if msg.ok && !msg.baseline { + for _, f := range msg.files { + p := f.path + if !filepath.IsAbs(p) { + p = filepath.Join(m.cwd, p) + } + defaultFileViewCache.invalidatePath(p) + } + } + if m.fileView.active && m.fileView.mode == fileViewFull { + var cmd tea.Cmd + m, cmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + return m, cmd + } + return m, nil case prWatcherStartedMsg: if msg.stop == nil { return m, nil @@ -3079,7 +3204,8 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case bashResultMsg: m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: msg.output}) - return m, nil + // invalidateFileViewForUnknownMutation schedules startFileViewRefreshCmd when fileView is active + return m.invalidateFileViewForUnknownMutation() case providerModelsDiscoveredMsg: return m.applyProviderModelsDiscovered(msg), nil case setupModelsDiscoveredMsg: @@ -3715,6 +3841,16 @@ func (m model) scrollChat(delta int) model { if !m.altScreen || delta == 0 { return m } + // A pending full-file reload renders a one-line placeholder whose geometry + // cannot represent the reader's position. Apply the delta to the pending + // intent against the last completed body instead, so a Page Down, arrow, or + // wheel during the reload is not discarded by the placeholder's zero height. + if m.fileView.active && m.fileView.mode == fileViewFull && m.fileView.loading { + maxOffset := maxInt(0, m.fileView.completedBodyLines-m.chatViewportHeight()) + m.fileView.preservedScrollOffset = clampInt(m.fileView.preservedScrollOffset+delta, 0, maxOffset) + m.chatScrollOffset = m.fileView.preservedScrollOffset + return m + } viewport, ok := m.chatTranscriptViewport() if !ok { return m @@ -3726,6 +3862,28 @@ func (m model) scrollChat(delta int) model { return m } +// chatViewportHeight returns the height of the transcript body viewport, or 0 +// when it cannot be measured. It lets pending-scroll math run against the last +// completed body even while the visible body is the loading placeholder. +func (m model) chatViewportHeight() int { + viewport, ok := m.chatTranscriptViewport() + if !ok { + return 0 + } + return viewport.height +} + +// resetChatScrollToBottom snaps the viewport to the tail for a real submission +// and, if a full-file reload is pending, records the same intent so completion +// cannot restore the reader's pre-submission position. +func (m model) resetChatScrollToBottom() model { + m.chatScrollOffset = 0 + if m.fileView.loading { + m.fileView.preservedScrollOffset = 0 + } + return m +} + func (m model) chatScrollMetrics() (int, int) { viewport, ok := m.chatTranscriptViewport() if !ok { @@ -3760,6 +3918,16 @@ func (m model) chatTranscriptViewport() (transcriptViewport, bool) { // so the absolute view holds; at the bottom (offset 0) it follows normally. Only the // scrolled-up path renders the body, so the common case stays cheap. func (m model) syncChatScroll() model { + // A pending full-file reload temporarily renders a one-line loading + // placeholder. Measuring it would clamp the reader's offset to 0 and lose + // their place, so hold the pending intent — which scroll actions and prompt + // submission keep up to date — until handleFileViewLoaded reconciles it + // against the real body. + if m.altScreen && m.fileView.active && m.fileView.mode == fileViewFull && + m.fileView.loading && m.fileView.preservedScrollOffset > 0 { + m.chatScrollOffset = m.fileView.preservedScrollOffset + return m + } if !m.altScreen || m.chatScrollOffset <= 0 { // At the bottom (or inline mode): follow the tail; reset the pin baseline. m.chatBodyLines = 0 @@ -4626,6 +4794,9 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) { if text != "" { m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) } + if text == "" { + m, cmd = m.refreshFileViewMarkers() + } case pickerSkill: // Fill the composer with "/name " so the user adds their request before // submitting (a bare second Enter runs it without one); names the slash @@ -4651,10 +4822,21 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) { // local preview and never changes the active palette. text := "" m, text = m.handleThemeCommand(item.Value) + var loadCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + } if validThemeMode(item.Value) && !strings.Contains(text, "could not save theme preference") { - return m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + next, noticeCmd := m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + if loadCmd != nil { + return next, tea.Batch(noticeCmd, loadCmd) + } + return next, noticeCmd } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + if loadCmd != nil { + return m, loadCmd + } case pickerNotify: // The picker item's Value is " "; reusing the text handler // keeps validation, persistence, and the user-facing message in one @@ -4730,9 +4912,10 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { m.clearSuggestions() // Snap the viewport back to the bottom for a real submission, but not for an // empty Enter (a no-op) — that would yank the user away from wherever they - // had scrolled without anything actually being submitted. + // had scrolled without anything actually being submitted. A pending file + // reload records the same intent so its completion cannot undo the reset. if command.kind != commandEmpty { - m.chatScrollOffset = 0 + m = m.resetChatScrollToBottom() } return m.dispatchCommand(command) @@ -4791,7 +4974,9 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { if m.loopActive() { m = m.appendLoopSystem(m.loopFooterSummary() + " still running — /loop stop all to end them.") } - return m, nil + var clearCmd tea.Cmd + m, clearCmd = m.refreshFileViewMarkers() + return m, clearCmd case commandNew: // A fresh session mid-run would strand the in-flight turn's events; make the // user cancel first. Idle, /new saves the current session (already on disk) @@ -4800,7 +4985,10 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: "A run is in progress. Press Esc to cancel it first, then /new."}) return m, nil } - return m.startNewSession(), nil + next := m.startNewSession() + var newCmd tea.Cmd + next, newCmd = next.refreshFileViewMarkers() + return next, newCmd case commandBTW: return m.handleBTWCommand(command.text) case commandLoop: @@ -4962,7 +5150,11 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { } else if text != "" { m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) } - return m, nil + var resumeCmd tea.Cmd + if text == "" { + m, resumeCmd = m.refreshFileViewMarkers() + } + return m, resumeCmd case commandRename: if title := strings.TrimSpace(command.text); title != "" { return m.renameActiveSession(title), nil @@ -4982,9 +5174,10 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { return m.toggleDetailedTranscript(), nil case commandRewind: text := "" - m, text = m.handleRewindCommand(command.text) + var rewindCmd tea.Cmd + m, text, rewindCmd = m.handleRewindCommand(command.text) m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) - return m, nil + return m, rewindCmd case commandEffort: if strings.TrimSpace(command.text) == "" { if m.pending { @@ -5072,10 +5265,21 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { } text := "" m, text = m.handleThemeCommand(command.text) + var loadCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + } if validThemeMode(command.text) && !strings.Contains(text, "could not save theme preference") { - return m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + next, noticeCmd := m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + if loadCmd != nil { + return next, tea.Batch(noticeCmd, loadCmd) + } + return next, noticeCmd } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + if loadCmd != nil { + return m, loadCmd + } return m, nil case commandNotify: // Bare `/notify` opens the popup picker so the user can pick mode + focus @@ -5212,7 +5416,7 @@ func (m model) executeSlash(input string) (tea.Model, tea.Cmd) { } m.rememberInput(input) m.clearSuggestions() - m.chatScrollOffset = 0 + m = m.resetChatScrollToBottom() return m.dispatchCommand(command) } @@ -6014,6 +6218,11 @@ func (m model) runAgentWithOptions(runID int, runCtx context.Context, prompt str if !toolResultCardSuppressedInTranscript(result.Name, result.Status) { rows = append(rows, row) m.sendAgentRow(runID, row) + } else if result.Name != "Task" && toolResultMayMutateUnknownScope(result.Name) && m.runtimeMessageSink != nil { + // A hidden successful mutating result (TaskOutput) still needs the + // conservative refresh; Task completion carries it via + // specialistCompleteMsg. + m.runtimeMessageSink(unknownScopeMutationMsg{runID: runID}) } // Keep the latest plan state in sync for run details and step drill-in. if result.Name == "update_plan" && m.registry != nil { diff --git a/internal/tui/mouse.go b/internal/tui/mouse.go index 00aa8f437..0a20636fc 100644 --- a/internal/tui/mouse.go +++ b/internal/tui/mouse.go @@ -154,6 +154,11 @@ func (m model) handleMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { if next, cmd, handled := m.handlePetMouse(msg); handled { return next, cmd } + if mouseLeftPress(msg) && m.runDetailsVisible() { + if path, ok := m.runDetailsFileAtMouse(msg); ok { + return m.selectFile(path) + } + } if mouseLeftPress(msg) { switch { case m.providerWizard != nil: diff --git a/internal/tui/run_details.go b/internal/tui/run_details.go index 26cf3ef8b..1ed2c64e3 100644 --- a/internal/tui/run_details.go +++ b/internal/tui/run_details.go @@ -19,6 +19,22 @@ func (m model) runDetailsAllowed() bool { m.mcpManager == nil && m.picker == nil && m.renamePrompt == nil && !m.suggestionsActive() } +// runDetailsVisible reports whether Run details is the overlay currently +// displayed in the viewport. Higher-priority overlays (terminal attach, speech +// API-key prompt, keybinding help, leader help) and the detailed transcript +// view take precedence over Run details in View/transcriptView. +func (m model) runDetailsVisible() bool { + if !m.runDetailsOpen || !m.runDetailsAllowed() || m.transcriptDetailed || + m.helpOverlay || m.leaderHelpOverlay || m.sttKeyPrompt != nil || m.terminalAttach != nil { + return false + } + width := chatWidth(m.width) + if m.terminalAttachOverlay(width) != "" || m.sttKeyPromptOverlay(width) != "" { + return false + } + return m.runDetailsOverlay(width) != "" +} + func (m model) runDetailsOverlay(width int) string { if !m.runDetailsOpen || width < runDetailsMinWidth || !m.runDetailsAllowed() { return "" @@ -26,11 +42,19 @@ func (m model) runDetailsOverlay(width int) string { overlayWidth := minInt(72, maxInt(40, width-8)) overlayWidth = minInt(overlayWidth, width) inner := maxInt(12, overlayWidth-4) - lines := m.runDetailsLines(inner) + lines := m.runDetailsLayout(inner).lines return centerRenderedBlock(styledBlockFillTitle(overlayWidth, "Run details", lines, zeroTheme.lineStrong, lipgloss.NewStyle()), width) } -func (m model) runDetailsLines(width int) []string { +type runDetailsLayout struct { + lines []string + fileHits []fileHit + fileStart int +} + +func (m model) runDetailsLayout(width int) runDetailsLayout { + var out runDetailsLayout + out.fileStart = -1 lines := make([]string, 0, 16) appendSection := func(header string, rows []string) { if len(rows) == 0 { @@ -48,8 +72,27 @@ func (m model) runDetailsLines(width int) []string { appendSection(m.sidebarAgentHeader(width), m.sidebarAgentLines(width)) appendSection(m.sidebarPlanHeader(width), m.sidebarPlanLines(width)) - fileLines, _ := m.sidebarFileLines(width) - appendSection(m.sidebarFilesHeader(width), fileLines) + fileLines, hits := m.sidebarFileLines(width) + if len(fileLines) > 0 { + if len(lines) > 0 { + lines = append(lines, "") + } + lines = append(lines, m.sidebarFilesHeader(width)) + out.fileStart = len(lines) + kept := fileLines + if len(kept) > runDetailsMaxItems { + kept = append(append([]string(nil), fileLines[:runDetailsMaxItems-1]...), " "+zeroTheme.faint.Render("… more in transcript")) + var filtered []fileHit + for _, h := range hits { + if h.lineOffset < runDetailsMaxItems-1 { + filtered = append(filtered, h) + } + } + hits = filtered + } + lines = append(lines, kept...) + out.fileHits = hits + } appendSection(sidebarHeader("ACTIVITY", width), m.sidebarActivityLines(width, runDetailsMaxItems)) if tokens := m.sidebarTokenText(); tokens != "" { if len(lines) > 0 { @@ -58,8 +101,10 @@ func (m model) runDetailsLines(width int) []string { lines = append(lines, zeroTheme.faint.Render(tokens)) } if len(lines) == 0 { - return []string{zeroTheme.faint.Render("No active run details yet.")} + out.lines = []string{zeroTheme.faint.Render("No active run details yet.")} + return out } lines = append(lines, "", zeroTheme.faint.Render("Esc or Ctrl+B closes")) - return lines + out.lines = lines + return out } diff --git a/internal/tui/run_details_test.go b/internal/tui/run_details_test.go index 4666d1109..1b9697f45 100644 --- a/internal/tui/run_details_test.go +++ b/internal/tui/run_details_test.go @@ -64,3 +64,31 @@ func TestRunDetailsOverlayHidesBehindPicker(t *testing.T) { t.Fatalf("run details must yield to a picker that owns the keyboard, got:\n%s", got) } } + +func TestRunDetailsHitsAlignWithSidebar(t *testing.T) { + m := filesPanelTestModel() + inner := 40 + fileLines, hits := m.sidebarFileLines(inner) + if len(hits) == 0 || len(fileLines) == 0 { + t.Fatal("sidebar hits") + } + layout := m.runDetailsLayout(inner) + if layout.fileStart < 0 { + t.Fatal("run details dropped FILES block identity") + } + if len(layout.fileHits) != len(hits) && len(fileLines) <= runDetailsMaxItems { + t.Fatalf("hit count %d vs sidebar %d", len(layout.fileHits), len(hits)) + } + for _, h := range layout.fileHits { + idx := layout.fileStart + h.lineOffset + if idx < 0 || idx >= len(layout.lines) { + t.Fatalf("hit %q offset %d outside details", h.path, h.lineOffset) + } + if h.lineOffset < 0 || h.lineOffset >= len(fileLines) { + t.Fatalf("hit %q offset %d outside sidebar lines", h.path, h.lineOffset) + } + if layout.lines[idx] != fileLines[h.lineOffset] { + t.Fatalf("hit map != rendered row for %q", h.path) + } + } +} diff --git a/internal/tui/session_controls.go b/internal/tui/session_controls.go index f3ccbb160..1e08e897c 100644 --- a/internal/tui/session_controls.go +++ b/internal/tui/session_controls.go @@ -740,27 +740,40 @@ func (m model) runCompact() tea.Cmd { // handleRewindCommand restores workspace files to a checkpoint and truncates the // session log. "/rewind" or "/rewind latest" undoes the most recent checkpoint; // "/rewind " rewinds to a specific event sequence. -func (m model) handleRewindCommand(args string) (model, string) { +func (m model) handleRewindCommand(args string) (model, string, tea.Cmd) { if m.sessionStore == nil || m.activeSession.SessionID == "" { - return m, "Rewind\nno active session to rewind." + return m, "Rewind\nno active session to rewind.", nil } if m.pending { - return m, "Rewind\ncannot rewind while a run is in progress." + return m, "Rewind\ncannot rewind while a run is in progress.", nil } // A cancelled run's late flush hasn't appended its checkpoint events yet: // ApplyRewind would prune those checkpoint blobs as unreferenced, then the // flush would re-append pre-rewind events after the rewind marker. if len(m.flushRunIDs) > 0 { - return m, "Rewind\ncannot rewind while a cancelled run is still flushing — retry in a moment." + return m, "Rewind\ncannot rewind while a cancelled run is still flushing — retry in a moment.", nil } arg := strings.TrimSpace(strings.ToLower(args)) target, err := m.resolveRewindTarget(arg) if err != nil { - return m, "Rewind\n" + err.Error() + return m, "Rewind\n" + err.Error(), nil } + + // Workspace files may be touched or restored starting with ApplyRewind: + // Invalidate file view cache immediately so any partial or full disk mutation + // cannot serve stale pre-rewind snapshots regardless of subsequent step failures. + defaultFileViewCache.invalidateUnknownScope() + if m.fileView.active { + m.fileView.requiredSourceRev++ + } + report, err := m.sessionStore.ApplyRewind(m.activeSession.SessionID, m.cwd, target) if err != nil { - return m, "Rewind\n" + err.Error() + var refreshCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, refreshCmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + } + return m, "Rewind\n" + err.Error(), refreshCmd } // ApplyRewind truncated the persisted event log, restored files, and appended a @@ -776,7 +789,11 @@ func (m model) handleRewindCommand(args string) (model, string) { events, readErr := m.sessionStore.ReadEvents(m.activeSession.SessionID) if readErr != nil { m.sessionEvents = nil // drop stale context so it can't reach the next prompt - return m, fmt.Sprintf("Rewind\nrewound to sequence %d, but reloading the session failed (in-memory context cleared): %s", target, readErr.Error()) + var refreshCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, refreshCmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + } + return m, fmt.Sprintf("Rewind\nrewound to sequence %d, but reloading the session failed (in-memory context cleared): %s", target, readErr.Error()), refreshCmd } m.sessionEvents = append([]sessions.Event{}, events...) rows := initialTranscript() @@ -786,12 +803,17 @@ func (m model) handleRewindCommand(args string) (model, string) { // pre-rewind scrollback above it stays, as scrollback cannot be un-printed. m.resetFlushFrontier("· rewound ·") + var refreshCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, refreshCmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) + } + summary := fmt.Sprintf("Rewound to sequence %d\n%d file(s) restored, %d deleted, %d skipped.", target, report.FilesRestored, report.FilesDeleted, len(report.Skipped)) if len(report.Skipped) > 0 { summary += "\nskipped (not recoverable): " + strings.Join(report.Skipped, ", ") } - return m, summary + return m, summary, refreshCmd } // resolveRewindTarget maps a /rewind argument to a keep-through event sequence. diff --git a/internal/tui/session_test.go b/internal/tui/session_test.go index 0867d167e..0cd3ca11d 100644 --- a/internal/tui/session_test.go +++ b/internal/tui/session_test.go @@ -845,7 +845,7 @@ func TestRewindRefreshesInMemorySessionState(t *testing.T) { } eventsBefore := len(m.sessionEvents) - m, out := m.handleRewindCommand("latest") + m, out, _ := m.handleRewindCommand("latest") if !strings.Contains(out, "Rewound") { t.Fatalf("expected a rewind summary, got %q", out) } diff --git a/internal/tui/syntax_highlight.go b/internal/tui/syntax_highlight.go index f923b1186..401ed1cf5 100644 --- a/internal/tui/syntax_highlight.go +++ b/internal/tui/syntax_highlight.go @@ -203,7 +203,15 @@ func highlightCodeAuto(code []string, lang string, measure int) ([]string, bool) } func highlightCodeForPath(code []string, path string, measure int, bg color.Color) ([]string, bool) { - return highlightCodeWithLexer(cachedLexerForPath(path), code, measure, bg) + return highlightCodeForPathWithTheme(code, path, measure, bg, zeroTheme) +} + +func highlightCodeForPathWithTheme(code []string, path string, measure int, bg color.Color, theme tuiTheme) ([]string, bool) { + backgrounds := make([]color.Color, len(code)) + for index := range backgrounds { + backgrounds[index] = bg + } + return highlightCodeWithLexerThemeAndLineBackgrounds(cachedLexerForPath(path), code, measure, theme, backgrounds, nil) } // highlightSpan overlays a background on a rune range in one source line while @@ -285,6 +293,10 @@ func highlightCodeWithLexerAndSpans(lexer chroma.Lexer, code []string, measure i } func highlightCodeWithLexerAndLineBackgrounds(lexer chroma.Lexer, code []string, measure int, backgrounds []color.Color, spans []highlightSpan) ([]string, bool) { + return highlightCodeWithLexerThemeAndLineBackgrounds(lexer, code, measure, zeroTheme, backgrounds, spans) +} + +func highlightCodeWithLexerThemeAndLineBackgrounds(lexer chroma.Lexer, code []string, measure int, theme tuiTheme, backgrounds []color.Color, spans []highlightSpan) ([]string, bool) { if measure < 4 { return nil, false } @@ -367,7 +379,7 @@ func highlightCodeWithLexerAndLineBackgrounds(lexer chroma.Lexer, code []string, line, column := 0, 0 for _, token := range iterator.Tokens() { - style := tokenStyle(token.Type) + style := tokenStyleForTheme(theme, token.Type) for index, part := range strings.Split(token.Value, "\n") { if index > 0 { flushLine() diff --git a/internal/tui/theme_select.go b/internal/tui/theme_select.go index 359df930d..ea4bb6222 100644 --- a/internal/tui/theme_select.go +++ b/internal/tui/theme_select.go @@ -92,6 +92,9 @@ func applyTheme(mode themeMode, terminalDark bool) themeMode { if defaultRenderCache != nil { defaultRenderCache.clear() // old-palette entries must not be reused } + if defaultFileViewCache != nil { + defaultFileViewCache.clear() + } return resolved } From 311290c01e60e9cd9cca0275497c160326110109 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 08:04:53 -0800 Subject: [PATCH 2/6] fix(tui): gate full-view read errors on snapshot validity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderFileViewFull returned renderedContent whenever hasError was set, before the snapshot-validity check that governs every other completed snapshot. A /btw fork of a full view that had settled on a read error invalidates the inherited snapshot and clears renderedContent, but the error latch survived, so the side pane rendered an empty body instead of the loading placeholder until its own load landed. Reloads of an errored view (resize, refresh, theme) likewise kept showing the stale error. Serve the error snapshot through the same validity predicate as content snapshots, so a pending or invalidated load falls through to "Loading…". Error displays still do not count as cache hits. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit a1887f4b0376e10c14f61806190efde47af532a6) --- internal/tui/btw_test.go | 43 +++++++++++++++++++++++++++++++++++++++ internal/tui/file_view.go | 10 +++++---- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/internal/tui/btw_test.go b/internal/tui/btw_test.go index f5f2d44b1..e58e65791 100644 --- a/internal/tui/btw_test.go +++ b/internal/tui/btw_test.go @@ -602,3 +602,46 @@ func TestBTWLeaveRevokesSideFileLoad(t *testing.T) { t.Fatal("restored parent must keep its snapshot") } } + +// TestBTWForkOfErroredFileViewShowsLoadingPlaceholder verifies that a side +// surface forked from a full view whose last snapshot was a read error does +// not render an empty body while its own load is pending. The inherited error +// snapshot is invalidated by the fork, so the side must show the loading +// placeholder until its load lands, then the side's own error snapshot. +func TestBTWForkOfErroredFileViewShowsLoadingPlaceholder(t *testing.T) { + resetFileViewCacheForTest() + m := newBTWTestModel(t) + m.cwd = t.TempDir() + + // A missing file opens straight in full mode and settles on a read error. + m, loadCmd := m.openFileView("missing.go") + if loadCmd == nil || m.fileView.mode != fileViewFull { + t.Fatal("expected a full-view load for a file without recorded edits") + } + m = deliverCommandMessages(t, m, loadCmd) + if !m.fileView.hasError { + t.Fatal("parent full view should settle on a read error") + } + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "Could not read file") { + t.Fatalf("parent should show the read error, got %q", got) + } + + side, sideLoadCmd := m.handleBTWCommand("") + if !side.btw.active || sideLoadCmd == nil { + t.Fatal("expected an active BTW surface with its own pending file load") + } + if got := plainRender(t, side.renderFileViewFull(80)); !strings.Contains(got, fileViewLoadingPlaceholder) { + t.Fatalf("pending side load must show the loading placeholder, got %q", got) + } + if got := plainRender(t, side.btw.parent.renderFileViewFull(80)); !strings.Contains(got, "Could not read file") { + t.Fatalf("hidden parent must keep its own error snapshot, got %q", got) + } + + side = deliverCommandMessages(t, side, sideLoadCmd) + if side.fileView.loading { + t.Fatal("side load should settle") + } + if got := plainRender(t, side.renderFileViewFull(80)); !strings.Contains(got, "Could not read file") { + t.Fatalf("settled side load should show its read error, got %q", got) + } +} diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 8074f1c24..639b1254d 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -1319,10 +1319,10 @@ func (m model) renderFileViewDiff(width int) string { // this session's diffs added (matched by exact text — an approximation that // tolerates later drift; a stale marker just doesn't highlight). // It is strictly non-blocking and performs O(1) lookup without invoking formatters. +// A read error is a completed snapshot like any other: it is shown only while +// that snapshot is still the authoritative one, so a pending reload or a fork +// that invalidated it falls through to the loading placeholder. func (m model) renderFileViewFull(width int) string { - if m.fileView.hasError { - return m.fileView.renderedContent - } target := m.fileView.path if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) @@ -1333,7 +1333,9 @@ func (m model) renderFileViewFull(width int) string { m.fileView.loadedRev >= m.fileView.requiredSourceRev && m.fileView.loadedGen == defaultFileViewCache.generation() && m.fileView.loadedToken == m.fileView.lifetimeToken { - defaultFileViewCache.recordHit() + if !m.fileView.hasError { + defaultFileViewCache.recordHit() + } return m.fileView.renderedContent } if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.desiredFingerprint, m.fileView.loadedSeq, m.fileView.desiredSeq, m.fileView.loadedRev, m.fileView.requiredSourceRev); ok { From 1ce4f96e832ffeb34f8b67b6559e7671c45b0320 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 09:01:29 -0800 Subject: [PATCH 3/6] fix(tui): keep file-view revisions monotonic and revalidate on git sweeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared path revisions reset to zero on every cache purge, while each surface kept its own required revision. Three sites could also raise that revision past the shared one by incrementing it locally. After a purge, an exact-path edit made on the other BTW surface could produce a shared revision no higher than this surface's own. recoverInvalidatedFileView then saw nothing new, and the view stayed stale. Revisions now come from one cache-wide clock that survives purges. A purge raises the epoch floor to that clock instead of resetting it, and each surface adopts the revision the cache returns. Every git sweep invalidated each swept path and forced a full refresh. The swept list is cumulative since the baseline, and each live sweep is launched beside the tool-result, turn-end or BTW-return handling that already reloads or rechecks the open view. The sweep that followed therefore reread and re-highlighted an unchanged file and flashed "Loading…". A sweep now revalidates the open view instead. It waits for an in-flight load to land, then rereads the bounded source in the background and compares it with the displayed snapshot's identity (hash and truncation flags). The view reloads only if the bytes changed. A mutation the swept list omits is still caught. Regression tests (all fail on 311290c0 and pass here): - TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface - TestFileViewGitSweepRevalidatesWithoutDuplicateReload - TestFileViewGitSweepDuringLoadDefersToThatLoad - TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot - TestFileViewCachePurgeResetsPathRevisions (now asserts monotonicity) Co-Authored-By: Claude Opus 5.5 --- internal/tui/btw.go | 7 + internal/tui/file_view.go | 216 +++++++++++++++++++------ internal/tui/file_view_test.go | 268 ++++++++++++++++++++++++++++++- internal/tui/model.go | 29 ++-- internal/tui/session_controls.go | 5 +- 5 files changed, 447 insertions(+), 78 deletions(-) diff --git a/internal/tui/btw.go b/internal/tui/btw.go index ca228a53a..e75eae7e8 100644 --- a/internal/tui/btw.go +++ b/internal/tui/btw.go @@ -314,6 +314,13 @@ func (m model) routeBTWParentMessage(msg tea.Msg) (model, tea.Cmd, bool) { } return m, nil, false } + if revalidated, ok := msg.(fileViewRevalidatedMsg); ok { + // A revalidation the parent started before /btw is routed the same way. + if m.btw.parent.fileView.active && m.btw.parent.fileView.lifetimeToken == revalidated.lifetimeToken { + return m.routeBTWMessageToParent(msg) + } + return m, nil, false + } runID, ok := btwMessageRunID(msg) if !ok || runID <= 0 || runID >= m.btw.sideRunIDBase { return m, nil, false diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 639b1254d..65bfd4d74 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -134,6 +134,19 @@ func (e *fileViewCachedEntry) byteSize() int { return n } +// fileViewSourceIdentity is what decides whether two bounded reads show the +// same file: the retained source hash plus both truncation flags. +type fileViewSourceIdentity struct { + hash [sha256.Size]byte + truncated bool + omittedLines bool +} + +// fileViewReadIdentity returns the identity of one bounded read. +func fileViewReadIdentity(res fileViewReadResult) fileViewSourceIdentity { + return fileViewSourceIdentity{hash: fileViewSourceHash(res.lines), truncated: res.truncated, omittedLines: res.omittedLines} +} + // fileViewSourceHash fingerprints the bounded source lines so a metadata-only // change (cp -p, rsync -t, tar -xp restoring mtime and size) cannot serve a // stale entry. Lines are joined with a separator the reader never emits inside @@ -215,7 +228,12 @@ type fileViewRenderCache struct { revOrder *list.List // LRU of revision keys: front = most recently invalidated path revElems map[string]*list.Element // targetPath -> *list.Element containing the path string revEpochFloor uint64 // minimum required revision for any path absent from pathRevisions - statsData fileViewCacheStats + // revClock is the last revision issued to any path. It never decreases, + // including across purges, so every new invalidation is newer than any + // revision a surface already holds and a path's required revision never + // moves backwards. + revClock uint64 + statsData fileViewCacheStats } var defaultFileViewCache = newFileViewRenderCache(defaultFileViewCacheMaxEntries, defaultFileViewCacheMaxBytes) @@ -232,14 +250,15 @@ func newFileViewRenderCache(maxEntries int, maxBytes int) *fileViewRenderCache { } } +// invalidatePath marks targetPath as mutated and returns its new required +// revision. Revisions come from one cache-wide clock, so the result is newer +// than every revision issued before it, including ones a BTW surface kept from +// before a purge. Surfaces adopt this value rather than counting on their own. func (c *fileViewRenderCache) invalidatePath(targetPath string) uint64 { c.mu.Lock() defer c.mu.Unlock() - rev := c.pathRevisions[targetPath] - if rev < c.revEpochFloor { - rev = c.revEpochFloor - } - rev++ + c.revClock++ + rev := c.revClock c.pathRevisions[targetPath] = rev c.touchRevisionLocked(targetPath) c.trimRevisionsLocked() @@ -379,6 +398,11 @@ func (c *fileViewRenderCache) evictPath(targetPath string) { c.mu.Unlock() } +// purgeLocked drops every entry and per-path revision key. The epoch floor +// moves up to the revision clock instead of resetting, so no path's required +// revision falls below a revision it already reported. Resetting it would let a +// surface hold a revision newer than the shared one, and that surface would +// miss a later exact-path edit made on the other surface. func (c *fileViewRenderCache) purgeLocked() { c.gen++ c.items = make(map[string]*list.Element) @@ -387,7 +411,7 @@ func (c *fileViewRenderCache) purgeLocked() { c.pathRevisions = make(map[string]uint64) c.revOrder = list.New() c.revElems = make(map[string]*list.Element) - c.revEpochFloor = 0 + c.revEpochFloor = c.revClock } func (c *fileViewRenderCache) clear() { @@ -722,28 +746,28 @@ func fileViewSuperseded(liveSeq *atomic.Uint64, seq uint64) bool { return liveSeq != nil && liveSeq.Load() != seq } -func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) { +func (c *fileViewRenderCache) loadAndRenderSource(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, fileViewSourceIdentity, error) { if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", fileViewSourceIdentity{}, errFileViewSuperseded } if fileViewInsideLoad != nil { fileViewInsideLoad() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", fileViewSourceIdentity{}, errFileViewSuperseded } stat, err := os.Stat(targetPath) if err != nil { c.evictPath(targetPath) rendered := theme.faint.Render("Could not read file: " + err.Error()) - return rendered, err + return rendered, fileViewSourceIdentity{}, err } if !stat.Mode().IsRegular() { c.evictPath(targetPath) readErr := fmt.Errorf("cannot display non-regular file %s (mode %s)", displayPath, stat.Mode().String()) rendered := theme.faint.Render("Could not read file: " + readErr.Error()) - return rendered, readErr + return rendered, fileViewSourceIdentity{}, readErr } modTime := stat.ModTime() @@ -753,7 +777,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin c.mu.Lock() if c.gen != reqGen { c.mu.Unlock() - return "", errors.New("request superseded by cache invalidation") + return "", fileViewSourceIdentity{}, errors.New("request superseded by cache invalidation") } c.statsData.DiskReads++ c.mu.Unlock() @@ -762,21 +786,22 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin fileViewBeforeDiskRead() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", fileViewSourceIdentity{}, errFileViewSuperseded } readRes := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq) if readRes.err != nil && len(readRes.lines) == 0 { c.evictPath(targetPath) rendered := theme.faint.Render("Could not read file: " + readRes.err.Error()) - return rendered, readRes.err + return rendered, fileViewSourceIdentity{}, readRes.err } - sourceHash := fileViewSourceHash(readRes.lines) + identity := fileViewReadIdentity(readRes) + sourceHash := identity.hash c.mu.Lock() if c.gen != reqGen { c.mu.Unlock() - return "", errors.New("request superseded by cache invalidation") + return "", identity, errors.New("request superseded by cache invalidation") } curRequiredRev := c.requiredRevisionLocked(targetPath) @@ -796,21 +821,21 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin entry.omittedLines == readRes.omittedLines { if fileViewSuperseded(liveSeq, seq) { c.mu.Unlock() - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } c.statsData.CacheHits++ c.lru.MoveToFront(elem) c.mu.Unlock() if rendered, ok := entry.getRender(renderKey); ok { - return rendered, nil + return rendered, identity, nil } if fileViewBeforeCacheHitFormat != nil { fileViewBeforeCacheHitFormat() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } // Re-format for the new width or changed markers using cached display and lines @@ -819,16 +844,16 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin fileViewBeforeCacheCommit() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } c.commitRenderPut(entry, renderKey, rendered) - return rendered, nil + return rendered, identity, nil } } if fileViewSuperseded(liveSeq, seq) { c.mu.Unlock() - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } c.statsData.CacheMisses++ @@ -843,7 +868,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin fileViewBeforeHighlight() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } display, ok := func() ([]string, bool) { @@ -864,7 +889,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin return highlightCodeForPathWithTheme(cleanLines, displayPath, 1<<20, nil, theme) }() if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } if !ok || len(display) != len(cleanLines) { display = cleanLines @@ -874,7 +899,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin fileViewBeforeFormat() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } rendered := formatFileViewLines(cleanLines, display, changed, readRes.truncated, readRes.omittedLines, width, theme) @@ -883,7 +908,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin fileViewBeforeCacheCommit() } if fileViewSuperseded(liveSeq, seq) { - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } sourceBytes := 0 @@ -913,24 +938,24 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin c.mu.Lock() if c.gen != reqGen { c.mu.Unlock() - return "", errors.New("request superseded by cache invalidation") + return "", identity, errors.New("request superseded by cache invalidation") } if fileViewSuperseded(liveSeq, seq) { c.mu.Unlock() - return "", errFileViewSuperseded + return "", identity, errFileViewSuperseded } if c.maxEntries <= 0 || c.maxBytes <= 0 || entry.byteSize() > c.maxBytes { c.statsData.SkippedOversized++ c.removeItemLocked(targetPath) c.mu.Unlock() - return rendered, nil + return rendered, identity, nil } if elem, ok := c.items[targetPath]; ok { existing := elem.Value.(*fileViewCachedEntry) if existing.modTime.After(readRes.modTime) && existing.sourceRev > reqSourceRev { c.mu.Unlock() - return rendered, nil + return rendered, identity, nil } c.removeItemLocked(targetPath) } @@ -939,7 +964,13 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin c.evictOverflowLocked() c.mu.Unlock() - return rendered, nil + return rendered, identity, nil +} + +// loadAndRender is loadAndRenderSource without the source identity. +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) { + rendered, _, err := c.loadAndRenderSource(targetPath, displayPath, width, changed, changedFingerprint, reqGen, theme, reqSourceRev, liveSeq, seq) + return rendered, err } // sanitizeRawFileLine strips or transforms raw control characters and terminal escape sequences @@ -979,7 +1010,9 @@ type fileViewLoadedMsg struct { width int fingerprint string rendered string - err error + // source identifies the bytes behind rendered; zero when err is set. + source fileViewSourceIdentity + err error } func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, seq uint64, gen int, reqRev uint64, theme tuiTheme, liveSeq *atomic.Uint64) tea.Cmd { @@ -987,7 +1020,7 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m if fileViewSuperseded(liveSeq, seq) { return fileViewLoadedMsg{lifetimeToken: token, seq: seq, requiredSourceRev: reqRev, generation: gen, targetPath: targetPath, displayPath: displayPath, width: width, fingerprint: fingerprint, err: errFileViewSuperseded} } - rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq) + rendered, source, err := defaultFileViewCache.loadAndRenderSource(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq) return fileViewLoadedMsg{ lifetimeToken: token, seq: seq, @@ -998,11 +1031,40 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m width: width, fingerprint: fingerprint, rendered: rendered, + source: source, err: err, } } } +// fileViewRevalidatedMsg reports whether the open full-file view's source still +// matches the snapshot it is showing. seq is the loadedSeq of that snapshot. +type fileViewRevalidatedMsg struct { + lifetimeToken [16]byte + displayPath string + seq uint64 + changed bool +} + +// revalidateFileViewCmd rereads the bounded source off the update goroutine and +// compares it with the displayed snapshot's identity. It neither highlights +// nor touches the cache. A read cancelled by a newer load reports no change, +// because that load reads the file itself. +func revalidateFileViewCmd(targetPath string, displayPath string, token [16]byte, shown fileViewSourceIdentity, liveSeq *atomic.Uint64, seq uint64) tea.Cmd { + return func() tea.Msg { + res := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq) + changed := false + switch { + case errors.Is(res.err, errFileViewSuperseded): + case res.err != nil && len(res.lines) == 0: + changed = true + default: + changed = fileViewReadIdentity(res) != shown + } + return fileViewRevalidatedMsg{lifetimeToken: token, displayPath: displayPath, seq: seq, changed: changed} + } +} + // 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 { @@ -1038,10 +1100,14 @@ type fileViewState struct { loadedToken [16]byte loadedSeq uint64 loadedRev uint64 - loading bool - hasError bool - snapshotReady bool - liveSeq *atomic.Uint64 + loadedSource fileViewSourceIdentity + // revalidateOnLoad defers a git-sweep revalidation until the in-flight load + // lands, so the sweep neither repeats nor cancels that load's work. + revalidateOnLoad bool + loading bool + hasError bool + snapshotReady bool + liveSeq *atomic.Uint64 } func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { @@ -1071,12 +1137,7 @@ func (m model) startFileViewLoad(width int, refreshSource bool) (model, tea.Cmd) target = filepath.Join(m.cwd, target) } if refreshSource { - rev := defaultFileViewCache.invalidatePath(target) - if rev > m.fileView.requiredSourceRev { - m.fileView.requiredSourceRev = rev - } else { - m.fileView.requiredSourceRev++ - } + m.fileView.requiredSourceRev = defaultFileViewCache.invalidatePath(target) } curReq := defaultFileViewCache.requiredRevision(target) if curReq > m.fileView.requiredSourceRev { @@ -1142,6 +1203,8 @@ func (m model) openFileView(path string) (model, tea.Cmd) { m.fileView.loadedToken = [16]byte{} m.fileView.loadedSeq = 0 m.fileView.loadedRev = 0 + m.fileView.loadedSource = fileViewSourceIdentity{} + m.fileView.revalidateOnLoad = false m.fileView.requiredSourceRev = defaultFileViewCache.requiredRevision(target) m.fileView.hasError = false // A file only the git sweep knows about (bash/subagent mutation) has no edit @@ -1225,6 +1288,7 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { m.fileView.loadedToken = msg.lifetimeToken m.fileView.loadedSeq = msg.seq m.fileView.loadedRev = msg.requiredSourceRev + m.fileView.loadedSource = msg.source m.fileView.hasError = (msg.err != nil) // Reconcile the held reading offset against the real body now that the // loading placeholder is gone. Apply the height delta exactly once to @@ -1239,9 +1303,66 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { } } m.fileView.completedBodyLines = current + if m.fileView.revalidateOnLoad { + m.fileView.revalidateOnLoad = false + return m.revalidateFileView() + } return m, nil } +// fileViewSnapshotCurrent reports whether the completed snapshot is still the +// authoritative body for the open view. +func (m model) fileViewSnapshotCurrent() bool { + return m.fileView.snapshotReady && + m.fileView.loadedPath == m.fileView.path && + m.fileView.loadedSeq == m.fileView.desiredSeq && + m.fileView.loadedRev >= m.fileView.requiredSourceRev && + m.fileView.loadedGen == defaultFileViewCache.generation() && + m.fileView.loadedToken == m.fileView.lifetimeToken +} + +// revalidateFileView handles a git sweep for the open full-file view. A sweep +// lists every path dirty since the session baseline, not what changed since +// this snapshot, and it lands after the tool-result, turn-end, or BTW-return +// handling it started beside. So it checks the file content: a current +// snapshot stays on screen while the source is reread and compared, and the +// view reloads only if the bytes differ. An in-flight load is left to finish +// and then checked. A snapshot that is not current, including a read error, +// takes an ordinary load. +func (m model) revalidateFileView() (model, tea.Cmd) { + if !m.fileView.active || m.fileView.mode != fileViewFull || m.fileView.path == "" { + return m, nil + } + if m.fileView.loading { + m.fileView.revalidateOnLoad = true + return m, nil + } + if !m.fileViewSnapshotCurrent() || m.fileView.hasError || m.fileView.liveSeq == nil { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + return m, revalidateFileViewCmd(target, m.fileView.path, m.fileView.lifetimeToken, m.fileView.loadedSource, m.fileView.liveSeq, m.fileView.loadedSeq) +} + +// handleFileViewRevalidated reloads the open view when its source changed after +// the displayed snapshot was read. The reload is a refresh, so the shared path +// revision advances and the other BTW surface picks up the change as well. If a +// newer load replaced the compared snapshot while the read ran, that load may +// already show the change, so the view is checked again rather than reloaded. +func (m model) handleFileViewRevalidated(msg fileViewRevalidatedMsg) (model, tea.Cmd) { + if !msg.changed || !m.fileView.active || m.fileView.mode != fileViewFull || + m.fileView.lifetimeToken != msg.lifetimeToken || m.fileView.path != msg.displayPath { + return m, nil + } + if msg.seq != m.fileView.loadedSeq || !m.fileViewSnapshotCurrent() { + return m.revalidateFileView() + } + return m.startFileViewRefreshCmd(m.chatColumnWidth()) +} + // 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 @@ -1327,12 +1448,7 @@ func (m model) renderFileViewFull(width int) string { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - if m.fileView.snapshotReady && - m.fileView.loadedPath == m.fileView.path && - m.fileView.loadedSeq == m.fileView.desiredSeq && - m.fileView.loadedRev >= m.fileView.requiredSourceRev && - m.fileView.loadedGen == defaultFileViewCache.generation() && - m.fileView.loadedToken == m.fileView.lifetimeToken { + if m.fileViewSnapshotCurrent() { if !m.fileView.hasError { defaultFileViewCache.recordHit() } diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index d70bf1d16..2ef178809 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -3053,13 +3053,16 @@ func TestFileViewCacheEvictionDropsPathRevision(t *testing.T) { } } -// TestFileViewCachePurgeResetsPathRevisions verifies clear() resets the revision +// TestFileViewCachePurgeResetsPathRevisions verifies clear() drops the revision // map instead of bumping dead keys forever, so /clear cannot leak one key per -// abandoned tool result. +// abandoned tool result, while every path's required revision stays monotonic: +// a purge never lowers it, and the next invalidation is newer than any +// revision issued before the purge. func TestFileViewCachePurgeResetsPathRevisions(t *testing.T) { cache := newFileViewRenderCache(8, defaultFileViewCacheMaxBytes) cache.invalidatePath("one.go") - cache.invalidatePath("two.go") + lastBefore := cache.invalidatePath("two.go") + oneBefore := cache.requiredRevision("one.go") cache.mu.Lock() before := len(cache.pathRevisions) cache.mu.Unlock() @@ -3074,8 +3077,16 @@ func TestFileViewCachePurgeResetsPathRevisions(t *testing.T) { if after != 0 { t.Fatalf("clear() must drop revision keys, got %d", after) } - if rev := cache.invalidatePath("one.go"); rev != 1 { - t.Fatalf("revision after purge = %d, want 1", rev) + for _, p := range []string{"one.go", "two.go", "never_seen.go"} { + if got := cache.requiredRevision(p); got < lastBefore { + t.Fatalf("required revision for %s fell to %d after purge, below %d", p, got, lastBefore) + } + } + if got := cache.requiredRevision("one.go"); got < oneBefore { + t.Fatalf("one.go required revision moved backwards across purge: %d -> %d", oneBefore, got) + } + if rev := cache.invalidatePath("one.go"); rev <= lastBefore { + t.Fatalf("revision after purge = %d, must exceed every pre-purge revision (%d)", rev, lastBefore) } } @@ -4223,3 +4234,250 @@ func TestFileViewCrossSurfaceMixedWriteStdinOutsideGit(t *testing.T) { } }) } + +// drainFileViewCommands runs cmd and every command the resulting updates return, +// so multi-hop flows (a sweep revalidation that then schedules a reload) settle. +func drainFileViewCommands(t *testing.T, m model, cmd tea.Cmd) model { + t.Helper() + pending := []tea.Cmd{cmd} + for steps := 0; len(pending) > 0; steps++ { + if steps > 64 { + t.Fatal("file-view command chain did not settle") + } + current := pending[0] + pending = pending[1:] + if current == nil { + continue + } + msg := current() + if msg == nil { + continue + } + if batch, ok := msg.(tea.BatchMsg); ok { + pending = append(pending, batch...) + continue + } + updated, next := m.Update(msg) + m = updated.(model) + pending = append(pending, next) + } + return m +} + +// TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface covers a purge +// followed by an exact-path edit on the other BTW surface. The purge used to +// reset shared path revisions to zero while the side kept its own (higher) +// required revision, so the parent's later edit produced a shared revision the +// side did not see as newer and the side kept rendering stale content. +func TestFileViewCrossSurfaceExactEditAfterPurgeReloadsOtherSurface(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "purge_then_edit.go" + write := func(content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + write("package before\n") + + m := newBTWTestModel(t) + m.cwd = dir + m.width = 100 + m.height = 30 + m.gitFileBaseline = nil + m.gitSweepUnavailable = true + m.pending = true + m.runID = 4 + m.activeRunID = 4 + side, _ := m.handleBTWCommand("") + if !side.btw.active || side.btw.parent == nil { + t.Fatal("fork failed") + } + side.activeRunID = side.btw.sideRunIDBase + side.runID = side.btw.sideRunIDBase + side = testOpenFile(side, name) + if side.fileView.mode != fileViewFull { + t.Fatal("side must open the untouched file in full mode") + } + + sideResult := func(tool string, changed []string) { + t.Helper() + updated, cmd := side.Update(agentRowMsg{ + runID: side.activeRunID, + row: transcriptRow{ + kind: rowToolResult, + tool: tool, + status: tools.StatusOK, + changedFiles: changed, + }, + }) + side = drainFileViewCommands(t, updated.(model), cmd) + } + // An exact side edit gives the path a revision, then an unknown-scope + // command purges the shared cache and reloads the side. + sideResult("edit_file", []string{name}) + sideResult("bash", nil) + if !strings.Contains(plainRender(t, side.renderFileViewFull(80)), "package before") { + t.Fatal("side must settle on the pre-mutation content after the purge reload") + } + + write("package after\n") + updated, cmd := side.Update(agentRowMsg{ + runID: 4, + row: transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + status: tools.StatusOK, + changedFiles: []string{name}, + }, + }) + side = drainFileViewCommands(t, updated.(model), cmd) + got := plainRender(t, side.renderFileViewFull(80)) + if !strings.Contains(got, "package after") || strings.Contains(got, "package before") { + t.Fatalf("parent edit after a purge left the side stale: %s", got) + } +} + +// TestFileViewGitSweepRevalidatesWithoutDuplicateReload covers a git sweep that +// lands after the open view already reloaded. An unchanged file must keep its +// snapshot on screen with no reread-and-highlight pass and no loading +// placeholder, while a mutation the sweep cannot attribute to the open path +// (the swept list omits it) must still reach the view. +func TestFileViewGitSweepRevalidatesWithoutDuplicateReload(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "sweep_revalidate.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + m = testOpenFile(m, name) + m = testSetMode(m, fileViewFull) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package v1") { + t.Fatal("expected v1") + } + + highlights := defaultFileViewCache.stats().HighlightCalls + updated, cmd := m.Update(gitSweepMsg{ok: true, files: []gitSweepFile{{path: name, adds: 1}}}) + m = updated.(model) + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v1") { + t.Fatalf("a sweep must not replace an unchanged snapshot with the loading placeholder: %q", got) + } + m = drainFileViewCommands(t, m, cmd) + if m.fileView.loading { + t.Fatal("an unchanged file must not leave the view loading after a sweep") + } + if after := defaultFileViewCache.stats().HighlightCalls; after != highlights { + t.Fatalf("a sweep over an unchanged file re-highlighted it: HighlightCalls %d -> %d", highlights, after) + } + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v1") { + t.Fatalf("unchanged snapshot lost after sweep: %q", got) + } + + // A later mutation that no tool reported, and that the swept list does not + // name, must still be picked up. + if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd = m.Update(gitSweepMsg{ok: true}) + m = drainFileViewCommands(t, updated.(model), cmd) + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") { + t.Fatalf("a mutation after the first refresh must reach the view: %q", got) + } +} + +// TestFileViewGitSweepDuringLoadDefersToThatLoad covers a sweep that lands while +// the tool-result refresh is still in flight. The sweep must not cancel and +// repeat that load. Once it lands, the view still checks for a mutation made +// after the load read the file. +func TestFileViewGitSweepDuringLoadDefersToThatLoad(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "sweep_inflight.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + m, loadCmd := m.openFileView(name) + if loadCmd == nil || m.fileView.mode != fileViewFull { + t.Fatal("expected an in-flight full-file load") + } + loaded := loadCmd() // reads v1 + if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil { + t.Fatal(err) + } + + updated, sweepCmd := m.Update(gitSweepMsg{ok: true, files: []gitSweepFile{{path: name}}}) + m = updated.(model) + if sweepCmd != nil { + t.Fatal("a sweep during a load must not start a competing load") + } + updated, next := m.Update(loaded) + m = updated.(model) + if next == nil { + t.Fatal("the landed load must run the deferred sweep revalidation") + } + m = drainFileViewCommands(t, m, next) + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") { + t.Fatalf("a mutation after the in-flight read must reach the view: %q", got) + } +} + +// TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot covers a revalidation +// that saw a change, but landed after a newer load already showed that change. +// It must recheck the newer snapshot instead of forcing another reload. +func TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "stale_revalidation.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package v1\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + m = testOpenFile(m, name) + if err := os.WriteFile(filePath, []byte("package v2\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, sweepCmd := m.Update(gitSweepMsg{ok: true}) + m = updated.(model) + if sweepCmd == nil { + t.Fatal("expected a revalidation for the settled snapshot") + } + stale, ok := sweepCmd().(fileViewRevalidatedMsg) + if !ok || !stale.changed { + t.Fatalf("revalidation must report the v2 change, got %#v", stale) + } + + updated, cmd := m.Update(agentRowMsg{ + runID: 1, + row: transcriptRow{kind: rowToolResult, tool: "edit_file", status: tools.StatusOK, changedFiles: []string{name}}, + }) + m = drainFileViewCommands(t, updated.(model), cmd) + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") { + t.Fatalf("tool-result refresh must show v2: %q", got) + } + + highlights := defaultFileViewCache.stats().HighlightCalls + updated, cmd = m.Update(stale) + m = updated.(model) + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package v2") { + t.Fatalf("a stale revalidation must not replace the current snapshot: %q", got) + } + m = drainFileViewCommands(t, m, cmd) + if m.fileView.loading { + t.Fatal("a stale revalidation must not leave the view loading") + } + if after := defaultFileViewCache.stats().HighlightCalls; after != highlights { + t.Fatalf("a stale revalidation re-highlighted an unchanged file: %d -> %d", highlights, after) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 647a21dba..474c238ce 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1456,6 +1456,8 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case fileViewLoadedMsg: return m.handleFileViewLoaded(msg) + case fileViewRevalidatedMsg: + return m.handleFileViewRevalidated(msg) case uv.CellSizeEvent: if msg.Width > 0 && msg.Height > 0 { m.petCellPixelWidth = msg.Width @@ -3087,11 +3089,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } rev := defaultFileViewCache.invalidatePath(target) if m.fileView.active && (p == m.fileView.path || target == m.fileView.path) { - if rev > m.fileView.requiredSourceRev { - m.fileView.requiredSourceRev = rev - } else { - m.fileView.requiredSourceRev++ - } + m.fileView.requiredSourceRev = rev if m.fileView.mode == fileViewFull { selfNeedsRefresh = true } @@ -3178,21 +3176,12 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case gitSweepMsg: m = m.handleGitSweepMsg(msg) - if msg.ok && !msg.baseline { - for _, f := range msg.files { - p := f.path - if !filepath.IsAbs(p) { - p = filepath.Join(m.cwd, p) - } - defaultFileViewCache.invalidatePath(p) - } - } - if m.fileView.active && m.fileView.mode == fileViewFull { - var cmd tea.Cmd - m, cmd = m.startFileViewRefreshCmd(m.chatColumnWidth()) - return m, cmd - } - return m, nil + // The swept list is cumulative since the baseline, so it says nothing + // about what changed since the open snapshot, and each live sweep starts + // beside tool-result, turn-end, or BTW-return handling that already + // reloads or rechecks the open view. Revalidate the open view's content + // rather than force a second reload. + return m.revalidateFileView() case prWatcherStartedMsg: if msg.stop == nil { return m, nil diff --git a/internal/tui/session_controls.go b/internal/tui/session_controls.go index 1e08e897c..1c3527b9b 100644 --- a/internal/tui/session_controls.go +++ b/internal/tui/session_controls.go @@ -762,10 +762,9 @@ func (m model) handleRewindCommand(args string) (model, string, tea.Cmd) { // Workspace files may be touched or restored starting with ApplyRewind: // Invalidate file view cache immediately so any partial or full disk mutation // cannot serve stale pre-rewind snapshots regardless of subsequent step failures. + // The purge advances the cache generation, which retires the open view's + // snapshot and any in-flight load without a view-local revision bump. defaultFileViewCache.invalidateUnknownScope() - if m.fileView.active { - m.fileView.requiredSourceRev++ - } report, err := m.sessionStore.ApplyRewind(m.activeSession.SessionID, m.cwd, target) if err != nil { From ca1cdaf3cef3d7647c48b4dc9a9ccdc477862104 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 09:05:23 -0800 Subject: [PATCH 4/6] refactor(tui): return source identity from loadAndRender directly 1ce4f96e added loadAndRenderSource and left loadAndRender as a two-value wrapper. After that change only tests called the wrapper, so make deadcode newly reported it. Fold the wrapper away: loadAndRender now returns the rendered text, the source identity, and the error, and its callers take the value they need. Behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/tui/file_view.go | 15 +++++---------- internal/tui/file_view_test.go | 34 +++++++++++++++++----------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 65bfd4d74..1548c3c67 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -727,7 +727,8 @@ func (c *fileViewRenderCache) peekRenderOnly(targetPath string, width int, chang // bytes, so the content hash decides freshness. On a matching metadata+hash pair // the highlight/format work is reused (re-formatting only for a new width // variant). It is intended to run from a tea.Cmd / background worker, off the -// View path. +// View path. It also returns the identity of the source it rendered, which a +// later git-sweep revalidation compares against. var errFileViewSuperseded = errors.New("file view request superseded") var ( @@ -746,7 +747,7 @@ func fileViewSuperseded(liveSeq *atomic.Uint64, seq uint64) bool { return liveSeq != nil && liveSeq.Load() != seq } -func (c *fileViewRenderCache) loadAndRenderSource(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, fileViewSourceIdentity, error) { +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, fileViewSourceIdentity, error) { if fileViewSuperseded(liveSeq, seq) { return "", fileViewSourceIdentity{}, errFileViewSuperseded } @@ -967,12 +968,6 @@ func (c *fileViewRenderCache) loadAndRenderSource(targetPath string, displayPath return rendered, identity, nil } -// loadAndRender is loadAndRenderSource without the source identity. -func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme, reqSourceRev uint64, liveSeq *atomic.Uint64, seq uint64) (string, error) { - rendered, _, err := c.loadAndRenderSource(targetPath, displayPath, width, changed, changedFingerprint, reqGen, theme, reqSourceRev, liveSeq, seq) - return rendered, err -} - // sanitizeRawFileLine strips or transforms raw control characters and terminal escape sequences // when syntax highlighting is bypassed or unavailable, preventing terminal screen corruption. func sanitizeRawFileLine(s string) string { @@ -995,7 +990,7 @@ func sanitizeRawFileLine(s string) string { func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { fingerprint := changedLinesFingerprint(changed) - rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, fingerprint, c.generation(), zeroTheme, 0, nil, 0) + rendered, _, _ := c.loadAndRender(targetPath, displayPath, width, changed, fingerprint, c.generation(), zeroTheme, 0, nil, 0) return rendered } @@ -1020,7 +1015,7 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m if fileViewSuperseded(liveSeq, seq) { return fileViewLoadedMsg{lifetimeToken: token, seq: seq, requiredSourceRev: reqRev, generation: gen, targetPath: targetPath, displayPath: displayPath, width: width, fingerprint: fingerprint, err: errFileViewSuperseded} } - rendered, source, err := defaultFileViewCache.loadAndRenderSource(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq) + rendered, source, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme, reqRev, liveSeq, seq) return fileViewLoadedMsg{ lifetimeToken: token, seq: seq, diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index 2ef178809..8a103be31 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -2072,7 +2072,7 @@ func TestFileViewCacheHitRespectsLiveSeq(t *testing.T) { defer func() { fileViewBeforeCacheCommit = nil }() target := filepath.Join(dir, name) gen := defaultFileViewCache.generation() - _, err := defaultFileViewCache.loadAndRender(target, name, m.fileView.loadedWidth+17, nil, m.fileView.loadedFingerprint, gen, zeroTheme, 0, live, seq) + _, _, err := defaultFileViewCache.loadAndRender(target, name, m.fileView.loadedWidth+17, nil, m.fileView.loadedFingerprint, gen, zeroTheme, 0, live, seq) if !errors.Is(err, errFileViewSuperseded) { t.Fatalf("cache-hit put after supersede: err=%v", err) } @@ -2737,7 +2737,7 @@ func TestFileView_SupersededInsideDiskReadAndHighlight(t *testing.T) { } defer func() { fileViewInsideDiskRead = nil }() - _, err := cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 1) + _, _, err := cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 1) if !errors.Is(err, errFileViewSuperseded) { t.Fatalf("expected errFileViewSuperseded on inside-disk-read supersession, got: %v", err) } @@ -2750,7 +2750,7 @@ func TestFileView_SupersededInsideDiskReadAndHighlight(t *testing.T) { } defer func() { fileViewBeforeHighlight = nil }() - _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 10) + _, _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 10) if !errors.Is(err, errFileViewSuperseded) { t.Fatalf("expected errFileViewSuperseded on before-highlight supersession, got: %v", err) } @@ -2763,7 +2763,7 @@ func TestFileView_SupersededInsideDiskReadAndHighlight(t *testing.T) { } defer func() { fileViewInsideHighlight = nil }() - _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 20) + _, _, err = cache.loadAndRender(filePath, "cancel_test.go", 80, nil, "", 0, theme, 0, &liveSeq, 20) if !errors.Is(err, errFileViewSuperseded) { t.Fatalf("expected errFileViewSuperseded on inside-highlight supersession, got: %v", err) } @@ -2964,7 +2964,7 @@ func TestFileViewCacheByteBudget(t *testing.T) { if err := os.WriteFile(path, []byte(payload), 0o644); err != nil { t.Fatal(err) } - if _, err := cache.loadAndRender(path, name, 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(path, name, 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { t.Fatalf("load %d: %v", i, err) } } @@ -2993,7 +2993,7 @@ func TestFileViewCacheRefusesOversizedEntry(t *testing.T) { t.Fatal(err) } cache := newFileViewRenderCache(64, 512) - rendered, err := cache.loadAndRender(path, "oversized.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + rendered, _, err := cache.loadAndRender(path, "oversized.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("load: %v", err) } @@ -3027,7 +3027,7 @@ func TestFileViewCacheEvictionDropsPathRevision(t *testing.T) { // Give a a revision, then load it so the entry and its revision coexist. cache.invalidatePath(a) - if _, err := cache.loadAndRender(a, "rev_a.txt", 80, nil, "", cache.generation(), zeroTheme, cache.requiredRevision(a), nil, 0); err != nil { + if _, _, err := cache.loadAndRender(a, "rev_a.txt", 80, nil, "", cache.generation(), zeroTheme, cache.requiredRevision(a), nil, 0); err != nil { t.Fatalf("load a: %v", err) } cache.mu.Lock() @@ -3038,7 +3038,7 @@ func TestFileViewCacheEvictionDropsPathRevision(t *testing.T) { } // Loading b with maxEntries=1 evicts a and must drop a's revision. - if _, err := cache.loadAndRender(b, "rev_b.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(b, "rev_b.txt", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { t.Fatalf("load b: %v", err) } cache.mu.Lock() @@ -3110,7 +3110,7 @@ func TestFileViewCacheFreshnessBySourceHash(t *testing.T) { } cache := newFileViewRenderCache(64, defaultFileViewCacheMaxBytes) - first, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + first, _, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("first load: %v", err) } @@ -3125,7 +3125,7 @@ func TestFileViewCacheFreshnessBySourceHash(t *testing.T) { t.Fatal(err) } - second, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) + second, _, err := cache.loadAndRender(path, "fresh.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("second load: %v", err) } @@ -3344,7 +3344,7 @@ func TestFileViewLifecycle_RevisionMetadataBounded(t *testing.T) { cache := newFileViewRenderCache(defaultFileViewCacheMaxEntries, defaultFileViewCacheMaxBytes) oldRev := cache.invalidatePath(warm) - if _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { t.Fatalf("warm load: %v", err) } @@ -3373,7 +3373,7 @@ func TestFileViewLifecycle_RevisionMetadataBounded(t *testing.T) { // A request captured before the retirement must not reuse the still-resident // entry; it has to re-render under the advanced authority. highlightBefore := cache.stats().HighlightCalls - if _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(warm, "revision_warm.go", 80, nil, "", cache.generation(), zeroTheme, oldRev, nil, 0); err != nil { t.Fatalf("stale load: %v", err) } if highlightAfter := cache.stats().HighlightCalls; highlightAfter != highlightBefore+1 { @@ -3401,7 +3401,7 @@ func TestFileViewLifecycle_TruncationStateEquivalence(t *testing.T) { if err := os.Chtimes(filePath, stamp, stamp); err != nil { t.Fatal(err) } - first, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + first, _, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("first load: %v", err) } @@ -3417,7 +3417,7 @@ func TestFileViewLifecycle_TruncationStateEquivalence(t *testing.T) { if err := os.Chtimes(filePath, stamp, stamp); err != nil { t.Fatal(err) } - second, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + second, _, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("second load: %v", err) } @@ -3432,7 +3432,7 @@ func TestFileViewLifecycle_TruncationStateEquivalence(t *testing.T) { if err := os.Chtimes(filePath, stamp, stamp); err != nil { t.Fatal(err) } - third, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) + third, _, err := defaultFileViewCache.loadAndRender(filePath, name, 80, nil, "", defaultFileViewCache.generation(), zeroTheme, 0, nil, 0) if err != nil { t.Fatalf("third load: %v", err) } @@ -3471,7 +3471,7 @@ func TestFileViewLifecycle_VariantAccountingConsistency(t *testing.T) { t.Fatal(err) } cache := newFileViewRenderCache(64, defaultFileViewCacheMaxBytes) - if _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { t.Fatalf("seed load: %v", err) } @@ -3512,7 +3512,7 @@ func TestFileViewLifecycle_VariantAccountingConsistency(t *testing.T) { } // A purge detaches every entry the same way. - if _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + if _, _, err := cache.loadAndRender(path, "variant_accounting.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { t.Fatalf("reload: %v", err) } cache.mu.Lock() From f2bb9ec9122b101040195c35ea698f9a0718a3e3 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 09:21:44 -0800 Subject: [PATCH 5/6] fix(tui): close three file-view staleness gaps from review - fileViewSourceHash joined lines with separators, so an empty file (no lines) and a file holding one newline (one empty line) hashed alike. Sweep revalidation compares identities without the file size, so adding or removing that blank line went undetected. Every line is now terminated, which keeps the encoding injective. - A load cancelled inside the bounded read returned errFileViewSuperseded with no lines. loadAndRender then treated it as a read error and evicted the path, discarding an entry a newer load had already committed. The sentinel now returns before the eviction branch. - A cancelled run keeps going until it finishes, and agentRowMsg ignores its late tool results. Its completion only flushed session events, so a write that finished during cancellation left an open full-file view stale. The flush completion now runs the unknown-scope refresh. Regression tests (each fails on ca1cdaf3 and passes here): - TestFileViewGitSweepDetectsBlankLineAddedToEmptyFile - TestFileViewSupersededReadKeepsNewerCacheEntry - TestFileViewCancelledRunCompletionRefreshesOpenView Co-Authored-By: Claude Opus 5.5 --- internal/tui/file_view.go | 16 ++++-- internal/tui/file_view_test.go | 100 +++++++++++++++++++++++++++++++++ internal/tui/model.go | 5 ++ 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 1548c3c67..441d940aa 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -149,15 +149,14 @@ func fileViewReadIdentity(res fileViewReadResult) fileViewSourceIdentity { // fileViewSourceHash fingerprints the bounded source lines so a metadata-only // change (cp -p, rsync -t, tar -xp restoring mtime and size) cannot serve a -// stale entry. Lines are joined with a separator the reader never emits inside -// a line, so shifted boundaries cannot collide. +// stale entry. Every line, including the last, is terminated with a separator +// the reader never emits inside a line. Shifted boundaries therefore cannot +// collide, and no lines (an empty file) stays distinct from one empty line. func fileViewSourceHash(lines []string) [sha256.Size]byte { h := sha256.New() - for i, line := range lines { - if i > 0 { - h.Write([]byte{'\n'}) - } + for _, line := range lines { h.Write([]byte(line)) + h.Write([]byte{'\n'}) } var out [sha256.Size]byte copy(out[:], h.Sum(nil)) @@ -791,6 +790,11 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin } readRes := readFileViewBoundedCancellable(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes, liveSeq, seq) + if errors.Is(readRes.err, errFileViewSuperseded) { + // A newer load owns this path now and may already have committed its + // entry; a cancelled read must not evict it. + return "", fileViewSourceIdentity{}, errFileViewSuperseded + } if readRes.err != nil && len(readRes.lines) == 0 { c.evictPath(targetPath) rendered := theme.faint.Render("Could not read file: " + readRes.err.Error()) diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index 8a103be31..5fef6bc1c 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -4481,3 +4481,103 @@ func TestFileViewStaleRevalidationDoesNotReloadNewerSnapshot(t *testing.T) { t.Fatalf("a stale revalidation re-highlighted an unchanged file: %d -> %d", highlights, after) } } + +// TestFileViewGitSweepDetectsBlankLineAddedToEmptyFile covers the one pair of +// sources the line-joined hash used to conflate: an empty file reads as no +// lines and a file holding a single newline reads as one empty line. Sweep +// revalidation compares identities without the file size, so it must still +// see that change and reload the line-numbered body. +func TestFileViewGitSweepDetectsBlankLineAddedToEmptyFile(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "empty_then_blank.txt" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, nil, 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m.activeRunID = 1 + m = testOpenFile(m, name) + if m.fileView.mode != fileViewFull || m.fileView.loading { + t.Fatal("expected a settled full-file view of the empty file") + } + emptyBody := plainRender(t, m.renderFileViewFull(80)) + + if err := os.WriteFile(filePath, []byte("\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(gitSweepMsg{ok: true}) + m = drainFileViewCommands(t, updated.(model), cmd) + if got := plainRender(t, m.renderFileViewFull(80)); got == emptyBody { + t.Fatalf("adding a blank line to an empty file was not detected; body still %q", got) + } +} + +// TestFileViewSupersededReadKeepsNewerCacheEntry covers a load that is +// superseded inside the bounded read after a newer load already committed its +// entry. The superseded load must not evict that entry. +func TestFileViewSupersededReadKeepsNewerCacheEntry(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "superseded_keep.go") + if err := os.WriteFile(filePath, []byte("package keep\n"), 0o644); err != nil { + t.Fatal(err) + } + cache := newFileViewRenderCache(10, defaultFileViewCacheMaxBytes) + if _, _, err := cache.loadAndRender(filePath, "superseded_keep.go", 80, nil, "", cache.generation(), zeroTheme, 0, nil, 0); err != nil { + t.Fatalf("newer load: %v", err) + } + + var liveSeq atomic.Uint64 + liveSeq.Store(1) + fileViewInsideDiskRead = func() { liveSeq.Store(2) } + defer func() { fileViewInsideDiskRead = nil }() + if _, _, err := cache.loadAndRender(filePath, "superseded_keep.go", 80, nil, "", cache.generation(), zeroTheme, 0, &liveSeq, 1); !errors.Is(err, errFileViewSuperseded) { + t.Fatalf("expected errFileViewSuperseded, got %v", err) + } + fileViewInsideDiskRead = nil + + cache.mu.Lock() + _, resident := cache.items[filePath] + cache.mu.Unlock() + if !resident { + t.Fatal("a superseded read evicted the newer resident cache entry") + } +} + +// TestFileViewCancelledRunCompletionRefreshesOpenView covers a write that +// finishes after its run was cancelled. Late tool results from a cancelled run +// are ignored, so its completion flush must still refresh an open full view. +func TestFileViewCancelledRunCompletionRefreshesOpenView(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + name := "cancelled_write.go" + filePath := filepath.Join(dir, name) + if err := os.WriteFile(filePath, []byte("package before\n"), 0o644); err != nil { + t.Fatal(err) + } + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, name) + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package before") { + t.Fatal("expected the pre-write snapshot") + } + + // The run was cancelled: it is no longer active, but its completion is + // still drained for session-event persistence. + const cancelledRunID = 7 + m.activeRunID = 0 + m.pending = false + m.flushRunIDs = map[int]string{cancelledRunID: m.activeSession.SessionID} + if err := os.WriteFile(filePath, []byte("package after\n"), 0o644); err != nil { + t.Fatal(err) + } + updated, cmd := m.Update(agentResponseMsg{runID: cancelledRunID}) + m = drainFileViewCommands(t, updated.(model), cmd) + if len(m.flushRunIDs) != 0 { + t.Fatal("the cancelled run's completion must still be drained") + } + if got := plainRender(t, m.renderFileViewFull(80)); !strings.Contains(got, "package after") { + t.Fatalf("a write that finished during cancellation left the view stale: %q", got) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 474c238ce..6cf739ccc 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -2723,6 +2723,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.exiting && len(m.flushRunIDs) == 0 { return m.quit() } + // The cancelled run kept going until it finished, and agentRowMsg + // ignored its late tool results because the run is no longer active. + // A write that completed in that window reported nothing, so give an + // open file view the conservative unknown-scope refresh. + return m.invalidateFileViewForUnknownMutation() } return m, nil } From cbbdec9e986236a452519eceadc9af06489b9f35 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 10:11:18 -0800 Subject: [PATCH 6/6] fix(tui): gate Run details Enter on the visible overlay The Run details Enter drill-in keyed off the raw runDetailsOpen flag, while mouse FILES picks use runDetailsVisible(). The flag outlives the overlay: clicking a specialist card beside Run details enters the subchat, which hides the overlay but leaves runDetailsOpen set, and Enter then opened the parent's file view from inside the subchat. Use runDetailsVisible() and the shared runDetailsInnerWidth() geometry on the keyboard path, matching the mouse path. The branch's existing Esc/Ctrl+B and key-swallow behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/tui/files_panel_test.go | 60 ++++++++++++++++++++++++++++++++ internal/tui/model.go | 9 ++--- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/internal/tui/files_panel_test.go b/internal/tui/files_panel_test.go index a6e3ac3c4..f16e8a0d5 100644 --- a/internal/tui/files_panel_test.go +++ b/internal/tui/files_panel_test.go @@ -501,3 +501,63 @@ func TestRunDetailsFileMouseBlockedBehindHigherPriorityOverlays(t *testing.T) { } }) } + +// TestRunDetailsEnterBlockedWhenRunDetailsHidden pins the keyboard twin of the +// mouse gate above: runDetailsOpen can outlive the visible overlay, and Enter +// must not drill into a FILES row the user can no longer see. +func TestRunDetailsEnterBlockedWhenRunDetailsHidden(t *testing.T) { + // setup returns a model with Run details visible and a FILES row selected + // that Enter would open (verified on a copy), so a later denial can only + // come from the visibility gate. + setup := func(t *testing.T, m model) (model, string) { + t.Helper() + m.transcript = append(m.transcript, transcriptRow{kind: rowToolResult, tool: "write_file", id: "f1", + status: tools.StatusOK, changedFiles: []string{"web/app.js"}}) + m.runDetailsOpen = true + layout := m.runDetailsLayout(m.runDetailsInnerWidth()) + if !m.runDetailsVisible() || len(layout.fileHits) == 0 { + t.Fatalf("precondition: Run details must be visible with FILES hits, visible=%v hits=%d", m.runDetailsVisible(), len(layout.fileHits)) + } + path := layout.fileHits[0].path + m, _ = m.selectFile(path) + if m.selectedFile != path || m.fileView.active { + t.Fatalf("precondition: first activation must only select %q, got selected=%q fileView=%v", path, m.selectedFile, m.fileView.active) + } + control, _ := m.Update(testKey(tea.KeyEnter)) + if fv := control.(model).fileView; !fv.active || fv.path != path { + t.Fatalf("precondition: Enter on visible Run details must open %q, got active=%v path=%q", path, fv.active, fv.path) + } + return m, path + } + assertEnterIgnored := func(t *testing.T, m model, path string) { + t.Helper() + if !m.runDetailsOpen || m.runDetailsVisible() { + t.Fatalf("expected runDetailsOpen=true with Run details hidden, got open=%v visible=%v", m.runDetailsOpen, m.runDetailsVisible()) + } + updated, cmd := m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if m.fileView.active || cmd != nil { + t.Fatalf("Enter behind hidden Run details opened %q: fileView.active=%v path=%q cmd=%v", path, m.fileView.active, m.fileView.path, cmd != nil) + } + } + + t.Run("specialist_card_click_enters_subchat", func(t *testing.T) { + m, path := setup(t, specialistClickTestModel(t, "child-sess-1")) + x, y, ok := specialistCardClickPoint(m, 2) + if !ok { + t.Fatal("expected a specialist card selectable line in the layout") + } + updated, _ := m.Update(testMouseClick(tea.MouseLeft, x, y)) + m = updated.(model) + if !m.subchat.active { + t.Fatal("clicking the specialist card beside Run details must enter the subchat") + } + assertEnterIgnored(t, m, path) + }) + + t.Run("detailed_transcript", func(t *testing.T) { + m, path := setup(t, filesPanelTestModel()) + m.transcriptDetailed = true + assertEnterIgnored(t, m, path) + }) +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 6cf739ccc..f876a517c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1780,10 +1780,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.runDetailsOpen = false return m, nil } - if keyIs(msg, tea.KeyEnter) && m.selectedFile != "" { - overlayWidth := minInt(72, maxInt(40, m.width-8)) - inner := maxInt(12, overlayWidth-4) - layout := m.runDetailsLayout(inner) + // Enter drills into the selected FILES row only while Run details is + // the overlay on screen, the same gate mouse picks use. The flag can + // outlive the visible overlay (e.g. a click entered a subchat). + if keyIs(msg, tea.KeyEnter) && m.selectedFile != "" && m.runDetailsVisible() { + layout := m.runDetailsLayout(m.runDetailsInnerWidth()) for _, h := range layout.fileHits { if h.path == m.selectedFile { return m.selectFile(m.selectedFile)