diff --git a/CLAUDE.md b/CLAUDE.md index eeddbccb1..64cf4cd17 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -349,6 +349,10 @@ roborev tui # Terminal UI - **Test agent**: Use `agent = "test"` for testing without calling real AI - **Isolated tests**: All tests use `t.TempDir()` for temp directories +## Terminal Safety + +Sanitize untrusted strings before TUI display: `stripControlChars()` for table cells, `sanitizeForDisplay()` for multi-line content, `sanitizeEscapes()` for streaming lines. + ## Design Constraints - **Daemon tasks must not modify the git working tree.** Background jobs (reviews, CI polling, synthesis) are read-only with respect to the user's repo checkout. They read source files and write results to the database only. CLI commands like `roborev fix` run synchronously in the foreground and may modify files. Background `fix` jobs run agents in isolated git worktrees (via `internal/worktree`) and store resulting patches in the database — patches are only applied to the working tree when the user explicitly confirms in the TUI. diff --git a/cmd/roborev/tui/queue_test.go b/cmd/roborev/tui/queue_test.go index f672a299c..27beb4525 100644 --- a/cmd/roborev/tui/queue_test.go +++ b/cmd/roborev/tui/queue_test.go @@ -2169,6 +2169,12 @@ func TestMigrateColumnConfig(t *testing.T) { wantDirty: false, wantColOrder: []string{"ref", "branch", "repo", "agent", "queued", "elapsed", "status", "pf", "closed"}, }, + { + name: "existing hidden_columns preserved without backfill", + hiddenCols: []string{"branch"}, + wantDirty: false, + wantHidden: []string{"branch"}, + }, } for _, tt := range tests { diff --git a/cmd/roborev/tui/render_queue.go b/cmd/roborev/tui/render_queue.go index 8eab741e4..9c2cdfda2 100644 --- a/cmd/roborev/tui/render_queue.go +++ b/cmd/roborev/tui/render_queue.go @@ -97,18 +97,19 @@ func (m model) getVisibleSelectedIdx() int { // Queue table column indices. const ( - colSel = iota // "> " selection indicator - colJobID // Job ID - colRef // Git ref (short SHA or range) - colBranch // Branch name - colRepo // Repository display name - colAgent // Agent name - colQueued // Enqueue timestamp - colElapsed // Elapsed time - colStatus // Job status - colPF // Pass/Fail verdict - colHandled // Done status - colCount // total number of columns + colSel = iota // "> " selection indicator + colJobID // Job ID + colRef // Git ref (short SHA or range) + colBranch // Branch name + colRepo // Repository display name + colAgent // Agent name + colQueued // Enqueue timestamp + colElapsed // Elapsed time + colStatus // Job status + colPF // Pass/Fail verdict + colHandled // Done status + colSessionID // Session ID + colCount // total number of columns ) func (m model) renderQueueView() string { @@ -257,7 +258,7 @@ func (m model) renderQueueView() string { visCols := m.visibleColumns() // Compute per-column max content widths, using cache when data hasn't changed. - allHeaders := [colCount]string{"", "JobID", "Ref", "Branch", "Repo", "Agent", "Queued", "Elapsed", "Status", "P/F", "Closed"} + allHeaders := [colCount]string{"", "JobID", "Ref", "Branch", "Repo", "Agent", "Queued", "Elapsed", "Status", "P/F", "Closed", "Session"} allFullRows := make([][]string, len(visibleJobList)) for i, job := range visibleJobList { cells := m.jobCells(job) @@ -305,14 +306,15 @@ func (m model) renderQueueView() string { // Fixed-width columns: exact sizes (content + padding, not counting inter-column spacing) fixedWidth := map[int]int{ - colSel: 2, - colJobID: idWidth, - colStatus: max(contentWidth[colStatus], 6), // "Status" header = 6, auto-sizes to content - colQueued: 12, - colElapsed: 8, - colPF: 3, // "P/F" header = 3 - colHandled: max(contentWidth[colHandled], 6), // "Closed" header = 6 - colAgent: min(max(contentWidth[colAgent], 5), 12), // "Agent" header = 5, cap at 12 + colSel: 2, + colJobID: idWidth, + colStatus: max(contentWidth[colStatus], 6), // "Status" header = 6, auto-sizes to content + colQueued: 12, + colElapsed: 8, + colPF: 3, // "P/F" header = 3 + colHandled: max(contentWidth[colHandled], 6), // "Closed" header = 6 + colAgent: min(max(contentWidth[colAgent], 5), 12), // "Agent" header = 5, cap at 12 + colSessionID: min(max(contentWidth[colSessionID], 7), 12), // "Session" header = 7, cap at 12 } // Flexible columns absorb excess space @@ -635,7 +637,12 @@ func (m model) jobCells(job storage.ReviewJob) []string { } } - return []string{ref, branch, repo, agentName, enqueued, elapsed, status, verdict, handled} + sessionID := stripControlChars(job.SessionID) + if runes := []rune(sessionID); len(runes) > 12 { + sessionID = string(runes[:12]) + } + + return []string{ref, branch, repo, agentName, enqueued, elapsed, status, verdict, handled, sessionID} } // statusLabel returns a capitalized display label for the job status. @@ -760,32 +767,34 @@ func migrateColumnConfig(cfg *config.Config) bool { // toggleableColumns is the ordered list of columns the user can show/hide. // colSel and colJobID are always visible and not included here. -var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colPF, colHandled} +var toggleableColumns = []int{colRef, colBranch, colRepo, colAgent, colQueued, colElapsed, colStatus, colPF, colHandled, colSessionID} // columnNames maps column constants to display names. var columnNames = map[int]string{ - colRef: "Ref", - colBranch: "Branch", - colRepo: "Repo", - colAgent: "Agent", - colStatus: "Status", - colQueued: "Queued", - colElapsed: "Elapsed", - colPF: "P/F", - colHandled: "Closed", + colRef: "Ref", + colBranch: "Branch", + colRepo: "Repo", + colAgent: "Agent", + colStatus: "Status", + colQueued: "Queued", + colElapsed: "Elapsed", + colPF: "P/F", + colHandled: "Closed", + colSessionID: "Session", } // columnConfigNames maps column constants to config file names (lowercase). var columnConfigNames = map[int]string{ - colRef: "ref", - colBranch: "branch", - colRepo: "repo", - colAgent: "agent", - colStatus: "status", - colQueued: "queued", - colElapsed: "elapsed", - colPF: "pf", - colHandled: "closed", + colRef: "ref", + colBranch: "branch", + colRepo: "repo", + colAgent: "agent", + colStatus: "status", + colQueued: "queued", + colElapsed: "elapsed", + colPF: "pf", + colHandled: "closed", + colSessionID: "session_id", } // drainFlexOverflow reduces flex column widths to absorb overflow, @@ -823,9 +832,25 @@ func columnDisplayName(col int) string { return lookupDisplayName(col, columnNames) } +// defaultHiddenColumns lists columns that are hidden by default. +// Users can enable them via the column options modal. +var defaultHiddenColumns = map[int]bool{ + colSessionID: true, +} + // parseHiddenColumns converts config hidden_columns strings to column ID set. +// When names is empty (no user config), defaultHiddenColumns are applied. +// When names contains only the sentinel "_", the user explicitly has nothing hidden. +// Otherwise, only the listed columns are hidden. func parseHiddenColumns(names []string) map[int]bool { result := map[int]bool{} + if len(names) == 0 { + maps.Copy(result, defaultHiddenColumns) + return result + } + if len(names) == 1 && names[0] == config.HiddenColumnsNoneSentinel { + return result + } lookup := map[string]int{} for id, name := range columnConfigNames { lookup[name] = id @@ -839,6 +864,8 @@ func parseHiddenColumns(names []string) map[int]bool { } // hiddenColumnsToNames converts a hidden column ID set to config names. +// When nothing is hidden, returns the sentinel ["_"] to distinguish +// from an unconfigured (nil) slice. func hiddenColumnsToNames(hidden map[int]bool) []string { var names []string // Maintain stable order @@ -847,6 +874,9 @@ func hiddenColumnsToNames(hidden map[int]bool) []string { names = append(names, columnConfigNames[col]) } } + if len(names) == 0 { + return []string{config.HiddenColumnsNoneSentinel} + } return names } diff --git a/internal/config/config.go b/internal/config/config.go index 83503f4af..d60a7e6e1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -702,6 +702,11 @@ func LoadGlobalFrom(path string) (*Config, error) { return cfg, nil } +// HiddenColumnsNoneSentinel is saved to hidden_columns when the +// user explicitly wants all columns visible. This distinguishes +// "hide nothing" from "never configured" (nil/empty slice). +const HiddenColumnsNoneSentinel = "_" + // migrateDeprecated promotes deprecated config keys to their // replacements so the rest of the codebase only reads the new names. // Uses TOML metadata to avoid overriding explicitly-set new keys. @@ -713,19 +718,28 @@ func (c *Config) migrateDeprecated(md toml.MetaData) { } c.HideAddressedByDefault = false - // hidden_columns: "handled"/"done" → "closed", remove defunct "pf" + // Preserve explicit hidden_columns = [] as "hide nothing" before + // the rename filter runs — otherwise a stale list that becomes + // empty after filtering would be misinterpreted as "hide nothing" + // instead of falling through to defaults. + explicitlyEmpty := md.IsDefined("hidden_columns") && + len(c.HiddenColumns) == 0 + + // hidden_columns: "handled"/"done" → "closed" filtered := c.HiddenColumns[:0] for _, name := range c.HiddenColumns { switch name { case "handled", "done": filtered = append(filtered, "closed") - case "pf": - // column removed; drop from config default: filtered = append(filtered, name) } } c.HiddenColumns = filtered + + if explicitlyEmpty { + c.HiddenColumns = []string{HiddenColumnsNoneSentinel} + } } // LoadRepoConfig loads per-repo config from .roborev.toml diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 55a0b83fd..d1b093e8c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2505,6 +2505,35 @@ func TestHideAddressedDoesNotOverrideExplicitNewKey(t *testing.T) { } +func TestHiddenColumnsExplicitEmptyBecomesSentinel(t *testing.T) { + testenv.SetDataDir(t) + + cfgPath := GlobalConfigPath() + require.NoError(t, os.MkdirAll(filepath.Dir(cfgPath), 0755)) + require.NoError(t, os.WriteFile(cfgPath, + []byte("hidden_columns = []\n"), 0644)) + + loaded, err := LoadGlobal() + require.NoError(t, err) + assert.Equal(t, + []string{HiddenColumnsNoneSentinel}, loaded.HiddenColumns, + "explicit hidden_columns = [] should become sentinel") +} + +func TestHiddenColumnsRenamedOnlyDoesNotBecomeSentinel(t *testing.T) { + testenv.SetDataDir(t) + + cfgPath := GlobalConfigPath() + require.NoError(t, os.MkdirAll(filepath.Dir(cfgPath), 0755)) + require.NoError(t, os.WriteFile(cfgPath, + []byte("hidden_columns = [\"handled\"]\n"), 0644)) + + loaded, err := LoadGlobal() + require.NoError(t, err) + assert.Equal(t, []string{"closed"}, loaded.HiddenColumns, + "hidden_columns with renamed entries should migrate, not become sentinel") +} + func TestIsDefaultReviewType(t *testing.T) { defaults := []string{"", "default", "general", "review"} for _, rt := range defaults {