refactor: cleanup pass + internal/tui strict eviction seam

AGENTS.md

@@ -426,6 +426,20 @@ removed on the next `config.Save`. No manual edit is required.
 - All paths in `workspace.toml` are relative to the workspace root.
 - Scripts and reconciler logic must be idempotent — safe to re-run.
 - No secrets in this repo.
+- **No comments in production Go code.** Things explained by a comment
+  should become code (named constants, asserts, typed wrappers, explicit
+  state enums). Permitted: build constraints (`//go:build`), package doc
+  comments (one line, required by `go doc`), `// DECISION:` blocks for
+  non-obvious WHY anchored to a specific line, `// TODO:` / `// FIXME:`
+  / `// HACK:` markers. Test files (`*_test.go`) are exempt — explanatory
+  comments in tests are fine.
+- **TUI consumers import `internal/tui` only.** Direct imports of
+  `github.com/charmbracelet/{bubbletea,lipgloss,bubbles/*}` outside
+  `internal/tui/` are a regression. The seam exists so a future PR can
+  swap the bubbletea implementation behind `internal/tui` without
+  touching consumers. Quick check:
+  `grep -rln "charmbracelet" --include='*.go' | grep -v internal/tui`
+  should return zero.
 - `workspace.toml` is the only file that changes during normal operation
   (plus `.gitattributes` once, on the reconciler's first run).
 - The daemon **never** runs `merge`, `rebase`, `reset`, `force`, **or

internal/add/add.go

@@ -21,37 +21,16 @@ import (
 	"strings"
 	"time"
 
-	tea "github.com/charmbracelet/bubbletea"
 	"github.com/kuchmenko/workspace/internal/clipboard"
 	"github.com/kuchmenko/workspace/internal/config"
 	"github.com/kuchmenko/workspace/internal/github"
+	"github.com/kuchmenko/workspace/internal/tui"
 )
 
-// ErrEmbedNotSupported is returned when a caller asks for ModeEmbedded.
-// Reserved for a future `ws agent` add screen that will host AddModel
-// inside its own bubbletea program; the interface is frozen now so
-// the embedding contract is locked.
 var ErrEmbedNotSupported = errors.New("embedded mode not yet supported")
 
-// ErrNoURLs is returned by headless runs with Options.URLs empty.
-// The TUI is the usual answer to "I didn't bring URLs"; headless by
-// definition cannot consult the user.
 var ErrNoURLs = errors.New("no URLs provided; pass one or more git remote URLs")
 
-// Run is the single entry point for every `ws add` style operation.
-// It owns the sidecar lifecycle and dispatches on Mode:
-//
-//	ModeAuto     with URLs    → headless
-//	ModeAuto     without URLs → TUI
-//	ModeHeadless with URLs    → headless
-//	ModeHeadless without URLs → ErrNoURLs
-//	ModeTUI                   → TUI
-//	ModeEmbedded              → ErrEmbedNotSupported (reserved)
-//
-// Per-URL errors accumulate in Result.Errors and do NOT abort the
-// loop; Run returns a non-nil error only when the whole operation
-// fails to start (nil required fields, sidecar conflict, ctx cancel
-// before the first register).
 func Run(ctx context.Context, opts Options) (*Result, error) {
 	if err := ctx.Err(); err != nil {
 		return nil, err
@@ -73,7 +52,7 @@ func Run(ctx context.Context, opts Options) (*Result, error) {
 		if len(opts.URLs) == 0 {
 			useTUI = true
 		}
-		// ModeAuto with URLs → headless (useTUI stays false).
+
 	case ModeHeadless:
 		if len(opts.URLs) == 0 {
 			return nil, ErrNoURLs
@@ -82,10 +61,6 @@ func Run(ctx context.Context, opts Options) (*Result, error) {
 		return nil, fmt.Errorf("add.Run: unknown mode %d", opts.Mode)
 	}
 
-	// Sidecar acquire (blocks concurrent `ws add`, pauses the daemon).
-	// We don't read the returned sidecar struct — the file's mere
-	// presence with our pid is the contract; `defer releaseSidecar`
-	// removes it on every exit.
 	if _, err := acquireSidecar(opts.WsRoot, opts.Mode, opts.URLs); err != nil {
 		return nil, err
 	}
@@ -97,23 +72,9 @@ func Run(ctx context.Context, opts Options) (*Result, error) {
 	return runHeadless(ctx, opts)
 }
 
-// runTUI launches AddModel as a standalone tea.Program and returns
-// the Result accumulated by the model when it reaches its done state.
-//
-// The sources used by the gather pass come from buildSources unless
-// the caller pre-populated opts.GhProvider (which only overrides the
-// GitHub source — disk and clipboard sources are always built from
-// opts.WsRoot/opts.Workspace). This keeps standalone `ws add` working
-// out of the box while letting tests inject any subset.
 func runTUI(ctx context.Context, opts Options) (*Result, error) {
 	sources := buildSources(opts)
 
-	// 10s per-source budget covers gh CLI paginate at ~300 repos
-	// (observed: 7.5s for 294 repos via gh --paginate). Disk and
-	// clipboard sources always finish well under this; the cap only
-	// matters for github. Increase further if real users at 1k+
-	// repos hit it — the TUI keeps spinning until ctx.Done either
-	// way.
 	model := NewAddModel(AddModelOptions{
 		WsRoot:        opts.WsRoot,
 		Workspace:     opts.Workspace,
@@ -123,12 +84,10 @@ func runTUI(ctx context.Context, opts Options) (*Result, error) {
 		Standalone:    true,
 	})
 
-	// Use AltScreen + signal handler so Ctrl+C surfaces as a clean
-	// AddDoneMsg and the terminal restores correctly on quit.
-	prog := tea.NewProgram(
+	prog := tui.NewProgram(
 		model,
-		tea.WithAltScreen(),
-		tea.WithContext(ctx),
+		tui.WithAltScreen(),
+		tui.WithContext(ctx),
 	)
 
 	finalModel, err := prog.Run()
@@ -147,17 +106,6 @@ func runTUI(ctx context.Context, opts Options) (*Result, error) {
 	}, nil
 }
 
-// buildSources assembles the default suggestion sources for a TUI run.
-// Honors opts.GhProvider override but constructs disk and clipboard
-// sources from the current environment (workspace, default reader).
-//
-// The GitHub source receives a workspace-derived `KnownRemotes` map so
-// it can mark suggestions whose URL matches an already-registered
-// project. The TUI uses that mark to render the "already cloned"
-// highlight on the affected rows.
-//
-// Tests that need to swap sources construct their own AddModel directly
-// (see tui_test.go); buildSources is the production wiring.
 func buildSources(opts Options) []Source {
 	gh := opts.GhProvider
 	if gh == nil {
@@ -174,18 +122,6 @@ func buildSources(opts Options) []Source {
 	}
 }
 
-// knownRemotesFromWorkspace builds a "owner/repo" → project-path map
-// from the workspace's registered projects. Used to flag GitHub
-// suggestions whose remote already exists locally — the TUI then
-// renders those rows with a "● cloned at <path>" suffix and a dimmed
-// style so the user can see at a glance which suggestions would
-// produce duplicates.
-//
-// Lower-cased keys so case differences in URLs (Foo/Bar vs foo/bar)
-// still collide. Lossy on the rare case of two registered projects
-// with the same upstream URL — last write wins, which is fine because
-// the "already cloned" highlight is a hint, not a data integrity
-// guarantee.
 func knownRemotesFromWorkspace(ws *config.Workspace) map[string]string {
 	if ws == nil || len(ws.Projects) == 0 {
 		return nil
@@ -204,43 +140,33 @@ func knownRemotesFromWorkspace(ws *config.Workspace) map[string]string {
 	return out
 }
 
-// ownerRepoFromRemote extracts a lowercased "owner/repo" from a git
-// remote URL. Handles SSH shorthand (`git@host:owner/repo.git`) and
-// scheme://host/owner/repo[.git] forms. Returns empty string when the
-// shape doesn't match — the caller treats that as "no match".
 func ownerRepoFromRemote(remote string) string {
 	s := strings.TrimSpace(remote)
 	s = strings.TrimSuffix(s, ".git")
 	s = strings.TrimSuffix(s, "/")
 
-	// SSH shorthand: git@host:owner/repo
 	if at := strings.Index(s, "@"); at >= 0 && !strings.Contains(s, "://") {
 		rest := s[at+1:]
 		if colon := strings.Index(rest, ":"); colon >= 0 {
-			s = rest[colon+1:] // owner/repo
+			s = rest[colon+1:]
 			return strings.ToLower(s)
 		}
 	}
 
-	// scheme://host/owner/repo
 	if i := strings.Index(s, "://"); i >= 0 {
 		s = s[i+3:]
 		if slash := strings.Index(s, "/"); slash >= 0 {
 			s = s[slash+1:]
 		}
 	}
 
-	// Strip any trailing path segments beyond owner/repo.
 	parts := strings.Split(s, "/")
 	if len(parts) >= 2 {
 		return strings.ToLower(parts[0] + "/" + parts[1])
 	}
 	return ""
 }
 
-// resolveSaveFn returns opts.Save when set, else falls back to the
-// production config.Save call. AddModel needs a non-nil saver because
-// Register asserts on the field at the per-job level.
 func resolveSaveFn(opts Options) func(*config.Workspace) error {
 	if opts.Save != nil {
 		return opts.Save
@@ -250,20 +176,14 @@ func resolveSaveFn(opts Options) func(*config.Workspace) error {
 	}
 }
 
-// runHeadless walks Options.URLs, calling Register for each. It does
-// NOT short-circuit on the first failure — failed URLs accumulate in
-// Result.Errors so the user sees the whole story after one invocation.
-// Between URLs, runHeadless re-checks ctx so a Ctrl+C mid-batch is
-// honored promptly.
 func runHeadless(ctx context.Context, opts Options) (*Result, error) {
 	res := &Result{}
 	for _, url := range opts.URLs {
 		if err := ctx.Err(); err != nil {
 			res.Errors = append(res.Errors, err)
 			return res, nil
 		}
-		// Reset per-URL options: Name is URL-specific, mustn't leak
-		// across loop iterations even if the caller set it once.
+
 		perURL := opts
 		regRes, err := Register(perURL, url)
 		if err != nil {