Add TUI control socket, --no-quit flag, and runtime metadata (#488)

## Summary

- Add a newline-delimited JSON protocol over a Unix domain socket
(`tui.{PID}.sock`) that allows external tools to programmatically query
and mutate running TUI instances
- Support 13 commands: 4 queries (`get-state`, `get-filter`, `get-jobs`,
`get-selected`) and 9 mutations (`set-filter`, `clear-filter`,
`set-hide-closed`, `select-job`, `set-view`, `close-review`,
`cancel-job`, `rerun-job`, `quit`)
- Write runtime metadata to `tui.{PID}.json` (PID, socket path, server
address) for discoverability; refresh on daemon reconnect when server
address changes
- Add `--control-socket` flag for custom socket paths and `--no-quit`
flag that suppresses keyboard quit (`q`) in queue/tasks views for
managed TUI instances
- `quit` control command terminates the TUI regardless of `--no-quit`,
allowing external controllers to manage lifecycle programmatically
- `set-filter` resolves display names (e.g. "msgvault") to root paths
via `/api/repos`, matching the interactive filter modal's resolution;
`reposMsg` carries an explicit `branchFiltered` flag so branch-scoped
loads don't clobber the display-name map
- Help text conditionally omits `q`/quit references when `--no-quit` is
active
- Socket security: stale sockets from dead processes cleaned via
platform-specific PID checks (signal-0 on Unix, tasklist on Windows);
socket created with `0600` permissions via `os.Chmod` (not process-wide
umask); managed socket directory tightened to `0700`; custom
`--control-socket` paths skip directory chmod to avoid clobbering shared
directories like `/tmp`
- Startup safety: control listener deferred until Bubble Tea event loop
is running (`ready` channel); `controlSocketReadyMsg` prevents
advertising a socket path before the listener binds; `cleanupDone`
channel ensures socket removal completes before `Run()` returns;
`programDone` channel prevents deadlock if program exits before first
`Update`
- Cleanup ordering: socket file removed before listener close to prevent
a successor process's socket from being unlinked by a dying predecessor
- Filter mutations (`set-filter`, `clear-filter`) validate all lock
constraints before mutating any state to prevent partial application
- `rerun-job` clears `Closed`/`Verdict` fields so rerun jobs remain
visible under `hideClosed`; both the control socket and keyboard rerun
paths use a shared `rerunSnapshot` for rollback on failure
- `select-job` rejects jobs hidden by active filters;
`close-review`/`cancel-job` only reflow selection when the mutated job
is the currently selected row

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: wesm

AGENTS.md

@@ -138,6 +138,8 @@ CLI (roborev) -> HTTP API -> Daemon -> Worker Pool -> Agent adapters
 
 ## Testing
 
+Use `testify` (`github.com/stretchr/testify`) for all test assertions. Use `require.*` for fatal preconditions and `assert.*` for non-fatal checks. Do not use raw `if`/`t.Errorf`/`t.Fatalf` patterns.
+
 - After any Go code changes, run `go fmt ./...` and `go vet ./...` before committing.
 - Fast test pass: `go test ./...`
 - Integration tests: `go test -tags=integration ./...`

CLAUDE.md

@@ -341,6 +341,9 @@ roborev tui                          # Terminal UI
 - `test` agent is always available (no PATH lookup)
 - Worker pool test hooks enable deterministic synchronization
 - Table-driven tests are preferred
+- Use `testify` (`assert`/`require`) for all test assertions -- do not use raw `if`/`t.Errorf`/`t.Fatalf` patterns
+- `require.*` for fatal preconditions (test cannot continue if this fails), `assert.*` for non-fatal checks
+- In tests with more than three assertions, prefer `assert := assert.New(t)` shorthand
 
 ## Conventions
 

cmd/roborev/tui/action_test.go

@@ -512,7 +512,7 @@ func TestTUICancelJobSuccess(t *testing.T) {
 	}
 	_, m := mockServerModel(t, expectJSONPost(t, "/api/job/cancel", cancelRequest{JobID: 42}, map[string]any{"success": true}))
 	oldFinishedAt := time.Now().Add(-1 * time.Hour)
-	cmd := m.cancelJob(42, storage.JobStatusRunning, &oldFinishedAt)
+	cmd := m.cancelJob(42, storage.JobStatusRunning, &oldFinishedAt, false)
 	msg := cmd()
 
 	result := assertMsgType[cancelResultMsg](t, msg)
@@ -530,7 +530,7 @@ func TestTUICancelJobNotFound(t *testing.T) {
 		w.WriteHeader(http.StatusNotFound)
 		json.NewEncoder(w).Encode(map[string]string{"error": "not found"})
 	})
-	cmd := m.cancelJob(99, storage.JobStatusQueued, nil)
+	cmd := m.cancelJob(99, storage.JobStatusQueued, nil, false)
 	msg := cmd()
 
 	result := assertMsgType[cancelResultMsg](t, msg)

cmd/roborev/tui/actions.go

@@ -144,22 +144,60 @@ func (m model) markParentClosed(parentJobID int64) tea.Cmd {
 	}
 }
 
-// cancelJob sends a cancel request to the server
-func (m model) cancelJob(jobID int64, oldStatus storage.JobStatus, oldFinishedAt *time.Time) tea.Cmd {
+// cancelJob sends a cancel request to the server.
+// restoreSelection tells the result handler to reselect the job
+// if the request fails and the optimistic update is rolled back.
+func (m model) cancelJob(
+	jobID int64, oldStatus storage.JobStatus,
+	oldFinishedAt *time.Time, restoreSelection bool,
+) tea.Cmd {
 	return func() tea.Msg {
-		err := m.postJSON("/api/job/cancel", map[string]any{"job_id": jobID}, nil)
-		return cancelResultMsg{jobID: jobID, oldState: oldStatus, oldFinishedAt: oldFinishedAt, err: err}
+		err := m.postJSON(
+			"/api/job/cancel",
+			map[string]any{"job_id": jobID}, nil,
+		)
+		return cancelResultMsg{
+			jobID:            jobID,
+			oldState:         oldStatus,
+			oldFinishedAt:    oldFinishedAt,
+			restoreSelection: restoreSelection,
+			err:              err,
+		}
 	}
 }
 
-// rerunJob sends a rerun request to the server for failed/canceled jobs
-func (m model) rerunJob(jobID int64, oldStatus storage.JobStatus, oldStartedAt, oldFinishedAt *time.Time, oldError string) tea.Cmd {
+// rerunJob sends a rerun request to the server for failed/canceled jobs.
+func (m model) rerunJob(snap rerunSnapshot) tea.Cmd {
 	return func() tea.Msg {
-		err := m.postJSON("/api/job/rerun", map[string]any{"job_id": jobID}, nil)
-		return rerunResultMsg{jobID: jobID, oldState: oldStatus, oldStartedAt: oldStartedAt, oldFinishedAt: oldFinishedAt, oldError: oldError, err: err}
+		err := m.postJSON(
+			"/api/job/rerun",
+			map[string]any{"job_id": snap.jobID}, nil,
+		)
+		return rerunResultMsg{
+			jobID:         snap.jobID,
+			oldState:      snap.oldStatus,
+			oldStartedAt:  snap.oldStartedAt,
+			oldFinishedAt: snap.oldFinishedAt,
+			oldError:      snap.oldError,
+			oldClosed:     snap.oldClosed,
+			oldVerdict:    snap.oldVerdict,
+			err:           err,
+		}
 	}
 }
 
+// rerunSnapshot captures job state before an optimistic rerun
+// update so it can be rolled back if the server request fails.
+type rerunSnapshot struct {
+	jobID         int64
+	oldStatus     storage.JobStatus
+	oldStartedAt  *time.Time
+	oldFinishedAt *time.Time
+	oldError      string
+	oldClosed     *bool
+	oldVerdict    *string
+}
+
 func (m model) submitComment(jobID int64, text string) tea.Cmd {
 	return func() tea.Msg {
 		commenter := os.Getenv("USER")

cmd/roborev/tui/control.go

@@ -0,0 +1,251 @@
+package tui
+
+import (
+	"bufio"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"log"
+	"net"
+	"os"
+	"path/filepath"
+	"syscall"
+	"time"
+
+	tea "github.com/charmbracelet/bubbletea"
+)
+
+const controlResponseTimeout = 3 * time.Second
+
+// removeStaleSocket checks whether socketPath is a leftover Unix
+// socket from a previous crash and removes it. Returns an error if
+// the path exists but is not a socket (protecting regular files from
+// accidental deletion via a mistyped --control-socket).
+func removeStaleSocket(socketPath string) error {
+	fi, err := os.Lstat(socketPath)
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("stat socket path: %w", err)
+	}
+	if fi.Mode().Type()&os.ModeSocket == 0 {
+		return fmt.Errorf(
+			"%s already exists and is not a socket", socketPath,
+		)
+	}
+	// It's a socket — try to connect to see if it's still live.
+	conn, dialErr := net.DialTimeout("unix", socketPath, 500*time.Millisecond)
+	if dialErr == nil {
+		conn.Close()
+		return fmt.Errorf(
+			"%s is already in use by another listener", socketPath,
+		)
+	}
+	// Only treat ECONNREFUSED as proof that nothing is listening.
+	// Other dial errors (wrong socket type, permission denied, etc.)
+	// are ambiguous and could indicate a live non-stream socket.
+	if !isConnRefused(dialErr) {
+		return fmt.Errorf(
+			"%s: cannot determine socket state: %w",
+			socketPath, dialErr,
+		)
+	}
+	// ECONNREFUSED — nothing is listening. Safe to remove.
+	if err := os.Remove(socketPath); err != nil {
+		return fmt.Errorf("remove stale socket: %w", err)
+	}
+	return nil
+}
+
+// isConnRefused returns true when the error chain contains
+// ECONNREFUSED, which means a socket exists but nothing is listening.
+func isConnRefused(err error) bool {
+	return errors.Is(err, syscall.ECONNREFUSED)
+}
+
+// startControlListener creates a Unix domain socket and starts
+// accepting connections. Each connection receives one JSON command,
+// dispatches it through the tea.Program, and returns a JSON response.
+// Returns a cleanup function that closes the listener and removes
+// the socket file.
+// ensureSocketDir creates the socket parent directory with
+// owner-only permissions. MkdirAll is a no-op on existing
+// directories, so we always chmod explicitly to tighten
+// pre-existing dirs (e.g. ~/.roborev created with 0755).
+// This must only be called for managed directories (the
+// default data dir), never for arbitrary user-supplied paths.
+func ensureSocketDir(dir string) error {
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		return fmt.Errorf("create socket directory: %w", err)
+	}
+	if err := os.Chmod(dir, 0700); err != nil {
+		return fmt.Errorf("chmod socket directory: %w", err)
+	}
+	return nil
+}
+
+func startControlListener(
+	socketPath string, p *tea.Program,
+) (func(), error) {
+	// Ensure the parent directory exists for custom socket paths.
+	// Permission tightening is handled separately by ensureSocketDir
+	// for the default managed directory only.
+	if err := os.MkdirAll(filepath.Dir(socketPath), 0755); err != nil {
+		return nil, fmt.Errorf("create socket directory: %w", err)
+	}
+
+	// Only remove an existing path if it is a stale Unix socket.
+	// Refusing to remove regular files prevents data loss from
+	// a mistyped --control-socket path.
+	if err := removeStaleSocket(socketPath); err != nil {
+		return nil, err
+	}
+
+	ln, err := net.Listen("unix", socketPath)
+	if err != nil {
+		return nil, fmt.Errorf("listen on %s: %w", socketPath, err)
+	}
+
+	// Restrict socket permissions to owner-only.
+	if err := os.Chmod(socketPath, 0600); err != nil {
+		ln.Close()
+		return nil, fmt.Errorf("chmod socket: %w", err)
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	go acceptLoop(ctx, ln, p)
+
+	// Disable automatic unlinking so ln.Close() does not remove
+	// whatever is at socketPath — a successor process may have
+	// already bound a new socket there.
+	ln.(*net.UnixListener).SetUnlinkOnClose(false)
+
+	cleanup := func() {
+		cancel()
+		os.Remove(socketPath)
+		ln.Close()
+	}
+	return cleanup, nil
+}
+
+func acceptLoop(ctx context.Context, ln net.Listener, p *tea.Program) {
+	for {
+		conn, err := ln.Accept()
+		if err != nil {
+			// Check if listener was closed (normal shutdown)
+			select {
+			case <-ctx.Done():
+				return
+			default:
+			}
+			log.Printf("control: accept error: %v", err)
+			// Back off on transient errors (e.g. EMFILE) to
+			// avoid a tight CPU-pegging loop.
+			time.Sleep(100 * time.Millisecond)
+			continue
+		}
+		go handleControlConn(ctx, conn, p)
+	}
+}
+
+func handleControlConn(
+	ctx context.Context, conn net.Conn, p *tea.Program,
+) {
+	defer conn.Close()
+
+	// Set read deadline to prevent hung connections
+	if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		return
+	}
+
+	scanner := bufio.NewScanner(conn)
+	scanner.Buffer(make([]byte, 64*1024), 64*1024)
+	if !scanner.Scan() {
+		writeError(conn, "empty request")
+		return
+	}
+
+	var req controlRequest
+	if err := json.Unmarshal(scanner.Bytes(), &req); err != nil {
+		writeError(conn, "invalid JSON: "+err.Error())
+		return
+	}
+
+	resp := dispatchCommand(ctx, req, p)
+	writeResponse(conn, resp)
+}
+
+func dispatchCommand(
+	ctx context.Context, req controlRequest, p *tea.Program,
+) controlResponse {
+	isQuery, isMutation := isControlCommand(req.Command)
+
+	switch {
+	case isQuery:
+		return queryViaProgram(ctx, p, req)
+	case isMutation:
+		return mutateViaProgram(ctx, p, req)
+	default:
+		return controlResponse{
+			Error: fmt.Sprintf("unknown command: %s", req.Command),
+		}
+	}
+}
+
+// queryViaProgram sends a controlQueryMsg through the program and
+// waits for the Update handler to write the response. The control
+// listener is only started after the event loop is running (via the
+// ready channel in Run), so p.Send will not block here.
+func queryViaProgram(
+	ctx context.Context, p *tea.Program, req controlRequest,
+) controlResponse {
+	respCh := make(chan controlResponse, 1)
+	p.Send(controlQueryMsg{req: req, respCh: respCh})
+
+	select {
+	case resp := <-respCh:
+		return resp
+	case <-ctx.Done():
+		return controlResponse{Error: "TUI is shutting down"}
+	case <-time.After(controlResponseTimeout):
+		return controlResponse{Error: "response timeout"}
+	}
+}
+
+// mutateViaProgram sends a controlMutationMsg through the program
+// and waits for the Update handler to write the response.
+func mutateViaProgram(
+	ctx context.Context, p *tea.Program, req controlRequest,
+) controlResponse {
+	respCh := make(chan controlResponse, 1)
+	p.Send(controlMutationMsg{req: req, respCh: respCh})
+
+	select {
+	case resp := <-respCh:
+		return resp
+	case <-ctx.Done():
+		return controlResponse{Error: "TUI is shutting down"}
+	case <-time.After(controlResponseTimeout):
+		return controlResponse{Error: "response timeout"}
+	}
+}
+
+func writeResponse(conn net.Conn, resp controlResponse) {
+	data, err := json.Marshal(resp)
+	if err != nil {
+		writeError(conn, "marshal error: "+err.Error())
+		return
+	}
+	data = append(data, '\n')
+	_, _ = conn.Write(data)
+}
+
+func writeError(conn net.Conn, msg string) {
+	resp := controlResponse{Error: msg}
+	data, _ := json.Marshal(resp)
+	data = append(data, '\n')
+	_, _ = conn.Write(data)
+}