tui: add session-id column to queue view

[mariusvniekerk]

Summary

• Add a "Session" column to the TUI queue view showing the agent's session ID, hidden by default
• Users can enable it via the column options modal (o key)
• Sanitize session IDs for terminal-safe display (strip control chars, truncate by rune count, cap at 12 chars)
• Use a sentinel value (_) in hidden_columns config to distinguish "show all columns" from "unconfigured", so parseHiddenColumns only applies defaults for unconfigured users
• Preserve explicit hidden_columns = [] in TOML as the sentinel during config migration
• Drop the startup backfill that re-added default-hidden columns on every launch — unconfigured users get defaults via parseHiddenColumns; users with custom hidden_columns see the new column appear and can hide it in one click

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (2bdae8a)

Summary Verdict: The PR introduces a new SessionID column to the TUI, but contains medium-severity issues regarding configuration migration and potential terminal injection
.

Findings

Medium Severity

1. Terminal injection via unsanitized session_id display

• File: cmd/roborev/tui/render_queue.go:640
• Description: The new queue column renders job.SessionID directly
  after only truncating it to 12 bytes. session_id is captured from streamed agent JSON, which can originate from external agent/API responses, and this path does not sanitize or validate the value before writing it to the terminal. A malicious or compromised upstream could embed ANSI/control sequences in the session
  ID and, once the hidden Session column is enabled, manipulate terminal output (screen clearing, spoofing, OSC actions depending on terminal support).
• Suggested Fix: Validate captured session IDs against the existing allowlist (agent.IsValidResumeSessionID) before persisting or displaying them. As defense in depth, also pass
  the rendered value through the existing terminal-sanitization helper (stripControlChars or equivalent) before adding it to the table.

2. Configuration migration flaw for empty hidden_columns

• File: cmd/roborev/tui/render_queue.go (around
  @@ -744, @@ -860, @@ -883)
• Description: The sentinel fix only distinguishes ["_"] from “unset”. A valid explicit config of hidden_columns = [] is still treated as “unconfigured”, so the new default
  -hidden session_id column will be re-hidden on load. That means users with a manual empty list, and possibly users with older saved configs if they serialized empty slices that way, still lose the “show all columns” preference after upgrade.
• Suggested Fix: Preserve TOML metadata for
  hidden_columns so an explicitly-defined empty array can be migrated to the sentinel, or treat an explicit empty list as “nothing hidden” during config load.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5ce3734)

Verdict: Adds a session_id column to the TUI queue view, hidden by default, with configuration parsing and display sanitization.

High

• cmd/roborev/tui/render_queue.go:752-7
  58
  The migration logic unconditionally appends default-hidden columns to the config if the user has manually hidden any other column. This makes it impossible for a user to persistently show
  the session_id column while keeping another column hidden, as it will be forcibly re-hidden on every app restart.
  Suggested fix: Remove this migration block. If a user has explicitly customized HiddenColumns, new columns should default to visible for them, or a config version integer must be used to ensure the
  migration only runs once.

Medium

• cmd/roborev/tui/render_queue.go:856
  parseHiddenColumns () now treats any zero-length hidden_columns value as "unconfigured" and applies defaultHiddenColumns. That breaks the natural config meaning of hidden_columns = []: users who explicitly want all columns visible will still get session_id hidden, and older configs that represented "show everything
  " as an empty list won’t round-trip correctly after upgrade. migrateColumnConfig() only recognizes the new "_" sentinel, so it does not repair this case.
  Suggested fix: Only apply defaultHiddenColumns when the field is truly unset, not merely empty. In practice that
  means distinguishing nil from an explicit empty slice at load/migration time and normalizing explicit-empty configs to the sentinel.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (3004899)

Verdict: The PR successfully adds a sanitized, default-hidden Session column to the TUI,
but introduces a medium-severity persistence bug in the column configuration migration logic.

Medium

• [cmd/roborev/tui/render_queue.go](/home/roborev/repos/roborev/cmd/roborev/tui/render_queue.go#L
  749)
  migrateColumnConfig() re-adds session_id to any non-empty hidden_columns list that omits it. That breaks persistence for users who want Session visible while still hiding some other column. Example: user hides branch, enables Session,
  and saves; config becomes ["branch"]. On next startup, migration rewrites it to ["branch", "session_id"], so Session is hidden again.
  Suggested fix: Only backfill session_id for clearly legacy configs, or persist explicit opt-in state for
  default-hidden columns so omission is not treated as “legacy” forever.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (a9a0048)

Verdict: The PR successfully adds a new Session ID column but introduces a configuration migration bug that unhides the P/F column for users who had it hidden.

Medium

• internal/ config/config.go (lines 728-740)
  migrateDeprecated now treats "pf" in hidden_columns as defunct and drops it, but this
  same range still keeps P/F as a real toggleable queue column (e.g., around cmd/roborev/tui/render_queue.go:770-797). That means users who had explicitly hidden the P/F column will see it come back as
  soon as config is loaded. Suggested fix: Stop filtering out "pf" here unless the column is actually removed from the UI.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (e248a82)

Verdict: All agents agree the code is clean, with no medium, high, or critical issues found.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)