diff --git a/CHANGELOG.md b/CHANGELOG.md index e0a0cd5..c5129e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JSON export now reports a clean "no space left to write <path>" message on a full disk instead of leaking the internal temporary spool file path into the error shown to the user. +- Opening a file in the interactive viewer that cannot be read (for example a + file that was removed by a later layer, or a daemon that became unreachable + mid-session) now shows a readable message in the status bar instead of a raw + internal error string. +- Saving an extracted file to a full disk now reports "not enough disk space" + and names only the file you chose, instead of leaking the internal temporary + file path and a raw system error into the status bar. +- A failure while fetching image size during loading is now surfaced as a brief + status message instead of being silently dropped, so a size lookup that fails + on its own no longer leaves the loading screen blank with no explanation. +- Every failure shown in the status bar now renders in the error colour. + Previously the colour was chosen by checking whether the message began with the + word "Error:", so the friendlier messages above — which no longer start with + that word — would have appeared in the success colour. ## [v1.6.0] - 2026-07-28 diff --git a/tui/model.go b/tui/model.go index c2650f5..28adb1e 100644 --- a/tui/model.go +++ b/tui/model.go @@ -11,8 +11,8 @@ import ( "syscall" "time" - tea "charm.land/bubbletea/v2" "charm.land/bubbles/v2/key" + tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" @@ -132,9 +132,22 @@ type clearCopyMsg struct{} type clearStatusMsg struct{ gen uint64 } // setStatus assigns msg to the status bar and bumps statusGen so any -// previously-scheduled clearStatusMsg ticks become stale and no-ops. +// previously-scheduled clearStatusMsg ticks become stale and no-ops. The +// message is styled as informational (non-error); use setErrorStatus for +// failures that should render in the error colour. func (m *model) setStatus(msg string) { m.statusMsg = msg + m.statusIsError = false + m.statusGen++ +} + +// setErrorStatus is setStatus for failure messages: it renders in the error +// colour. Kept separate from string-prefix sniffing so the status bar's +// colour is driven by intent, not by whether a message happens to start with +// a particular word. +func (m *model) setErrorStatus(msg string) { + m.statusMsg = msg + m.statusIsError = true m.statusGen++ } @@ -202,34 +215,35 @@ type treeCache struct { } type model struct { - width int - height int - focus focus - state appState - imageRef string - platform string - analysis *image.Analysis - layerCursor int - layerOffset int - treeCursor int - treeOffset int - errMsg string - quitting bool - resolver image.Resolver - spinnerFrame int - imageSize int64 - loadPhase image.ProgressPhase - pullLayers int - pullTotal int - pullBytes int64 - pullBytesMax int64 - progressCh chan image.ProgressEvent - copyConfirm bool - statusMsg string - statusGen uint64 - showHelp bool - filterActive bool - filterQuery string + width int + height int + focus focus + state appState + imageRef string + platform string + analysis *image.Analysis + layerCursor int + layerOffset int + treeCursor int + treeOffset int + errMsg string + quitting bool + resolver image.Resolver + spinnerFrame int + imageSize int64 + loadPhase image.ProgressPhase + pullLayers int + pullTotal int + pullBytes int64 + pullBytesMax int64 + progressCh chan image.ProgressEvent + copyConfirm bool + statusMsg string + statusIsError bool + statusGen uint64 + showHelp bool + filterActive bool + filterQuery string diffOnly bool aggregated bool sortMode sortMode @@ -240,45 +254,45 @@ type model struct { // and collapse separately — the value of the split view is being able to // inspect "what just changed" and "the full carry-forward state" without // losing one's place in either. - aggCursor int - aggOffset int - aggCollapsed map[string]bool - viewState viewState - viewContent *image.FileContent + aggCursor int + aggOffset int + aggCollapsed map[string]bool + viewState viewState + viewContent *image.FileContent viewHighlightedLines []string // viewLines is the plain-text split of viewContent.Data, computed once when // the file opens. The viewer's hot paths (scroll clamp, cursor-column bound, // search indexing, render) read this instead of re-running splitFileLines — // which copies the whole body and allocates per line — on every keystroke // and every frame. nil when no file is open; parallels viewHighlightedLines. - viewLines []string - viewOffset int - viewHOffset int - viewCursorCol int - viewOriginLayer int - viewOriginCmd string - viewSearchActive bool - viewSearchQuery string + viewLines []string + viewOffset int + viewHOffset int + viewCursorCol int + viewOriginLayer int + viewOriginCmd string + viewSearchActive bool + viewSearchQuery string viewSearchMatches [][2]int - viewSearchCursor int - viewRequestID uint64 - viewerCancel context.CancelFunc - saveRequestID uint64 - saveCancel context.CancelFunc - extractor image.Extractor - efficiency *image.EfficiencyResult - writeFile func(string, []byte, os.FileMode) error - statFile func(string) (os.FileInfo, error) - keys keyMap - showWaste bool - wasteCursor int - wasteOffset int - wasteExpanded bool - wasteRows []wasteRow - sizeMode sizeColMode - noCache bool - theme Theme - transparentBg bool + viewSearchCursor int + viewRequestID uint64 + viewerCancel context.CancelFunc + saveRequestID uint64 + saveCancel context.CancelFunc + extractor image.Extractor + efficiency *image.EfficiencyResult + writeFile func(string, []byte, os.FileMode) error + statFile func(string) (os.FileInfo, error) + keys keyMap + showWaste bool + wasteCursor int + wasteOffset int + wasteExpanded bool + wasteRows []wasteRow + sizeMode sizeColMode + noCache bool + theme Theme + transparentBg bool // renderedImageRef is the gradient-coloured image ref for the header, // precomputed once in NewModel. Both inputs (imageRef, theme gradient // stops) are immutable for the session, so renderHeader must not recompute @@ -336,22 +350,22 @@ func NewModel(cfg Config) model { ctx, cancel := context.WithCancel(context.Background()) theme := themeFor(cfg.Theme) return model{ - state: stateLoading, - imageRef: cfg.ImageRef, - platform: cfg.Platform, - resolver: cfg.Resolver, - progressCh: ch, - writeFile: atomicWriteFile, - statFile: os.Lstat, - keys: defaultKeys(), - noCache: cfg.NoCache, - theme: theme, - transparentBg: cfg.TransparentBg, + state: stateLoading, + imageRef: cfg.ImageRef, + platform: cfg.Platform, + resolver: cfg.Resolver, + progressCh: ch, + writeFile: atomicWriteFile, + statFile: os.Lstat, + keys: defaultKeys(), + noCache: cfg.NoCache, + theme: theme, + transparentBg: cfg.TransparentBg, treeCache: &treeCache{}, styles: newThemeStyles(theme), renderedImageRef: renderGradient(cfg.ImageRef, theme.GradientStart, theme.GradientEnd), - fetchCtx: ctx, - fetchCancel: cancel, + fetchCtx: ctx, + fetchCancel: cancel, } } @@ -427,6 +441,13 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case inspectMsg: if msg.err == nil && msg.meta != nil { m.imageSize = msg.meta.Size + } else if msg.err != nil && !errors.Is(msg.err, context.Canceled) && m.state == stateLoading { + // Inspect runs concurrently with the analysis fetch. If it fails + // fast (e.g. connection refused) while analysis is still blocked + // on a slow pull, this is the only diagnostic the user gets until + // analysisMsg arrives. No scheduleStatusClear: analysisMsg will + // overwrite it, or the error state will replace the whole screen. + m.setErrorStatus("Inspect failed: " + friendlyError(msg.err)) } return m, nil @@ -471,6 +492,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case clearStatusMsg: if msg.gen == m.statusGen { m.statusMsg = "" + m.statusIsError = false } return m, nil @@ -509,7 +531,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if msg.err != nil { m.viewState = viewNone - m.setStatus("Error: " + msg.err.Error()) + m.setErrorStatus(friendlyError(msg.err)) return m, m.scheduleStatusClear(3 * time.Second) } m.viewState = viewReady @@ -549,7 +571,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.saveCancel = nil } if msg.err != nil { - m.setStatus("Error: " + msg.err.Error()) + m.setErrorStatus(friendlyError(msg.err)) return m, m.scheduleStatusClear(3 * time.Second) } // Run stat + write off-thread so a slow disk (network mount, encrypted @@ -565,7 +587,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if msg.err != nil { - m.setStatus("Error: " + msg.err.Error()) + m.setErrorStatus(friendlySaveError(msg.err, msg.original)) return m, m.scheduleStatusClear(3 * time.Second) } if msg.target != msg.original { @@ -928,15 +950,15 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } f := files[cur] if f.IsDir { - m.setStatus("Error: cannot extract directory") + m.setErrorStatus("Cannot extract a directory") return m, m.scheduleStatusClear(2 * time.Second) } if f.DiffType == image.Removed { - m.setStatus("Error: file removed in this layer") + m.setErrorStatus("File was removed in this layer") return m, m.scheduleStatusClear(2 * time.Second) } if m.extractor == nil { - m.setStatus("Error: extractor unavailable") + m.setErrorStatus("Extractor unavailable") return m, m.scheduleStatusClear(2 * time.Second) } m.setStatus("Extracting...") @@ -1154,11 +1176,11 @@ func (m model) tryOpenSelectedFile() (tea.Model, tea.Cmd) { return m, m.scheduleStatusClear(2 * time.Second) } if f.DiffType == image.Removed { - m.setStatus("Error: file removed in this layer") + m.setErrorStatus("File was removed in this layer") return m, m.scheduleStatusClear(2 * time.Second) } if m.extractor == nil { - m.setStatus("Error: extractor unavailable") + m.setErrorStatus("Extractor unavailable") return m, m.scheduleStatusClear(2 * time.Second) } m.viewState = viewLoading @@ -1880,22 +1902,22 @@ func (m model) viewReady() tea.View { if m.viewState != viewNone { viewer := renderFileView(viewerParams{ - content: m.viewContent, - lines: m.viewLines, - offset: m.viewOffset, - hOffset: m.viewHOffset, - cursorCol: m.viewCursorCol, - width: m.width, - height: panelHeight, - loading: m.viewState == viewLoading, - spinnerFrame: m.spinnerFrame, - originLayer: m.viewOriginLayer, - originCmd: m.viewOriginCmd, - currentLayer: m.layerCursor, - searchQuery: m.viewSearchQuery, - searchMatches: m.viewSearchMatches, - searchCursor: m.viewSearchCursor, - searchActive: m.viewSearchActive, + content: m.viewContent, + lines: m.viewLines, + offset: m.viewOffset, + hOffset: m.viewHOffset, + cursorCol: m.viewCursorCol, + width: m.width, + height: panelHeight, + loading: m.viewState == viewLoading, + spinnerFrame: m.spinnerFrame, + originLayer: m.viewOriginLayer, + originCmd: m.viewOriginCmd, + currentLayer: m.layerCursor, + searchQuery: m.viewSearchQuery, + searchMatches: m.viewSearchMatches, + searchCursor: m.viewSearchCursor, + searchActive: m.viewSearchActive, highlightedLines: m.viewHighlightedLines, theme: m.theme, styles: m.styles, @@ -2036,7 +2058,7 @@ func (m model) renderStatusBar(treeFiles []*image.FileNode) string { var right string if m.statusMsg != "" { msgStyle := m.styles.addedBg - if strings.HasPrefix(m.statusMsg, "Error:") { + if m.statusIsError { msgStyle = m.styles.removedStatusBg } right = msgStyle.Render(m.statusMsg) + " " @@ -2302,7 +2324,6 @@ func atomicWriteFile(name string, data []byte, perm os.FileMode) error { return nil } - // viewLineCount returns the viewer's rendered line count from the cached split // in m.viewLines, preserving fileViewLineCount's contract: non-empty text whose // only content is a trailing newline counts as one line. Reads the cache so @@ -2459,6 +2480,30 @@ func friendlyError(err error) string { return err.Error() } +// friendlySaveError renders a file-save write failure for the status bar. +// The write path spools through a temp file, so the raw os error names the +// internal spool path ("write /out/.layerx-save-123: no space left on +// device"). ENOSPC gets a dedicated recovery hint (matching the CLI and the +// ErrArchiveInfra path in friendlyError); every other write error has its +// *os.PathError / *os.LinkError wrapper unwrapped so only the syscall reason +// survives — the temp spool path never reaches the user. name is the path +// the user asked to save to; only its base name is shown. +func friendlySaveError(err error, name string) string { + base := filepath.Base(name) + if errors.Is(err, syscall.ENOSPC) { + return fmt.Sprintf("Could not save %s: not enough disk space. Free space or choose another directory.", base) + } + // Strip the leaking spool path: *os.PathError.Error() is "op path: reason", + // and path here is the internal temp file. Reduce to the bare reason. + if pathErr, ok := errors.AsType[*os.PathError](err); ok { + return fmt.Sprintf("Could not save %s: %v", base, pathErr.Err) + } + if linkErr, ok := errors.AsType[*os.LinkError](err); ok { + return fmt.Sprintf("Could not save %s: %v", base, linkErr.Err) + } + return fmt.Sprintf("Could not save %s: %v", base, err) +} + // Run starts the TUI program with the given configuration. func Run(cfg Config) error { m := NewModel(cfg) diff --git a/tui/model_test.go b/tui/model_test.go index e29bafd..03312fa 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -413,7 +413,7 @@ func TestViewLoadingPullProgressFitsInBox(t *testing.T) { m.loadPhase = image.PhasePulling m.pullLayers = 1 m.pullTotal = 3 - m.pullBytes = 254 * 1024 * 1024 // "254.0 MB" + m.pullBytes = 254 * 1024 * 1024 // "254.0 MB" m.pullBytesMax = 4 * 1024 * 1024 * 1024 // "4.0 GB" content := viewContent(m.View()) @@ -1373,7 +1373,7 @@ func TestEnterOnRemovedFileShowsStatusMsg(t *testing.T) { updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEnter}) um := updated.(model) assert.Equal(t, viewNone, um.viewState) - assert.Equal(t, "Error: file removed in this layer", um.statusMsg) + assert.Equal(t, "File was removed in this layer", um.statusMsg) } func TestEscClosesFileViewer(t *testing.T) { @@ -1607,7 +1607,7 @@ func TestExtractKeyOnDirectoryShowsStatus(t *testing.T) { updated, _ := m.Update(keyPress('x')) um := updated.(model) - assert.Equal(t, "Error: cannot extract directory", um.statusMsg) + assert.Equal(t, "Cannot extract a directory", um.statusMsg) } func TestExtractKeyOnFileTriggersExtraction(t *testing.T) { @@ -1663,7 +1663,7 @@ func TestExtractKeyOnRemovedFile(t *testing.T) { updated, _ := m.Update(keyPress('x')) um := updated.(model) - assert.Equal(t, "Error: file removed in this layer", um.statusMsg) + assert.Equal(t, "File was removed in this layer", um.statusMsg) } func TestFileSaveMsgSuccess(t *testing.T) { @@ -1698,7 +1698,7 @@ func TestFileSaveMsgExtractError(t *testing.T) { updated, _ := m.Update(fileSaveMsg{requestID: 1, filename: "test.txt", err: errors.New("connection refused")}) um := updated.(model) - assert.Equal(t, "Error: connection refused", um.statusMsg) + assert.Equal(t, "connection refused", um.statusMsg) } func TestFileSaveMsgWriteError(t *testing.T) { @@ -1715,7 +1715,7 @@ func TestFileSaveMsgWriteError(t *testing.T) { require.Error(t, saved.err) updated2, _ := updated.(model).Update(saved) - assert.Equal(t, "Error: permission denied", updated2.(model).statusMsg) + assert.Equal(t, "Could not save test.txt: permission denied", updated2.(model).statusMsg) } func TestFileSaveMsgExistingFileAutoRenames(t *testing.T) { @@ -2792,3 +2792,83 @@ func TestDisplayTreeCacheReflectsReanalysis(t *testing.T) { assert.True(t, found, "a replaced analysis must invalidate the warm tree cache") } +// --- inspectMsg error surfacing (ERR-5) -------------------------------------- + +func TestInspectMsgErrorSurfacesDuringLoading(t *testing.T) { + m := NewModel(Config{ImageRef: "test:latest"}) + require.Equal(t, stateLoading, m.state) + m = send(m, inspectMsg{err: &image.ErrDaemonNotRunning{Engine: "docker", Cause: errors.New("connection refused")}}) + assert.Contains(t, m.statusMsg, "Inspect failed:") + assert.Contains(t, m.statusMsg, "Docker is not running") +} + +func TestInspectMsgCanceledDoesNotSurface(t *testing.T) { + m := NewModel(Config{ImageRef: "test:latest"}) + require.Equal(t, stateLoading, m.state) + m = send(m, inspectMsg{err: context.Canceled}) + assert.Empty(t, m.statusMsg, "a canceled inspect (user quit) must not raise a status warning") +} + +func TestInspectMsgErrorIgnoredWhenReady(t *testing.T) { + m := setupModel() // stateReady + m = send(m, inspectMsg{err: errors.New("late failure")}) + assert.Empty(t, m.statusMsg, "a late inspect error after the tree is ready is noise, not a diagnostic") +} + +func TestInspectMsgSuccessSetsImageSize(t *testing.T) { + m := NewModel(Config{ImageRef: "test:latest"}) + m = send(m, inspectMsg{meta: &image.ImageMeta{Size: 12345}}) + assert.Equal(t, int64(12345), m.imageSize) + assert.Empty(t, m.statusMsg) +} + +// --- friendlySaveError (ERR-2 write phase) ----------------------------------- + +func TestFriendlySaveErrorENOSPC(t *testing.T) { + err := fmt.Errorf("write /home/user/out/.tmp-123: %w", syscall.ENOSPC) + got := friendlySaveError(err, "/home/user/out/server.bin") + assert.Contains(t, got, "server.bin") + assert.Contains(t, got, "disk space") + assert.NotContains(t, got, ".tmp-123", "the internal temp path must not leak into the message") + assert.NotContains(t, got, "/home/user/out/server.bin", "only the base name should appear, not the full path") +} + +func TestFriendlySaveErrorGeneric(t *testing.T) { + err := errors.New("permission denied") + got := friendlySaveError(err, "/some/dir/file.txt") + assert.Contains(t, got, "file.txt") + assert.Contains(t, got, "permission denied") + assert.NotContains(t, got, "/some/dir", "only the base name should appear, not the directory") +} + +func TestFriendlySaveErrorStripsTempPathFromPathError(t *testing.T) { + // A non-ENOSPC write failure returns a *os.PathError whose path is the + // internal spool file. friendlySaveError must strip it so the user never + // sees ".layerx-save-*". + err := &os.PathError{Op: "write", Path: "/home/user/out/.layerx-save-9931", Err: syscall.EIO} + got := friendlySaveError(err, "/home/user/out/server.bin") + assert.Contains(t, got, "server.bin") + assert.NotContains(t, got, ".layerx-save-9931", "the internal spool path must not leak into the message") + assert.NotContains(t, got, "/home/user/out", "no directory path should appear") +} + +// --- status-bar error colouring ---------------------------------------------- + +func TestSetErrorStatusMarksError(t *testing.T) { + m := setupModel() + m.setErrorStatus("Could not save x: disk full") + assert.True(t, m.statusIsError, "setErrorStatus must flag the message for error colouring") + // A subsequent informational status must clear the error flag so it does + // not render in the error colour. + m.setStatus("Saved: x") + assert.False(t, m.statusIsError, "setStatus must clear the error flag") +} + +func TestClearStatusResetsErrorFlag(t *testing.T) { + m := setupModel() + m.setErrorStatus("boom") + gen := m.statusGen + m = send(m, clearStatusMsg{gen: gen}) + assert.Empty(t, m.statusMsg) + assert.False(t, m.statusIsError, "clearing the status must also reset the error flag") +}