From 49cf75c616831399fcc630cebc8e0b96ede79c81 Mon Sep 17 00:00:00 2001 From: Yuval Hayke Date: Sun, 2 Aug 2026 15:03:23 +0300 Subject: [PATCH 1/2] feat(cli): add `fleet worktree` and adopt externally-created sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #227. Creating a worktree meant opening the TUI and pressing `w`; nothing in the CLI surface touched worktrees, so it couldn't be scripted or fired from wherever you already were. `fleet worktree ` (alias `wt`) mirrors the `w` key: create the worktree, copy `.claude/settings.local.json` + `copy_files` entries, start a session. `--base` / `--path` / `--agent` steer it; `--no-session` prints only the path so `cd "$(fleet worktree foo --no-session)"` composes. Two details worth calling out. Flags parse on either side of the branch — a single `flag.Parse` stops at the first positional and would reject `fleet worktree foo --no-session`, the order most people type. And `--agent` is validated explicitly, because `agent.Parse` falls back to Claude for anything unrecognized, so a typo would silently launch the wrong agent. The command resolves to the main worktree before creating, so running it from inside `repo-foo` yields `repo-bar` rather than `repo-foo-bar` — same as pressing `w` on an origin header. It also calls `debuglog.Init()` first: the fallback logger writes to stderr and the provider logs at Info, so without it the command spews slog lines over the terminal (`fleet add` still does). The TUI read sessions from SQLite exactly once, at startup, which would have made a CLI-created session invisible until restart — the new command feeling broken for exactly the user most likely to run it. A self-throttled sweep in the worker's heavy pass now diffs the table and adopts unknown rows. Adoption only: rows deleted elsewhere are never dropped. Unlike a session the user just created, an adopted row arrives on a timer, so the cursor is captured by row identity and restored after the rebuild rather than left to drift. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Q1yXo3oP17DQwVbnagnWDY --- CLAUDE.md | 3 + .../unreleased/adopt-external-sessions.md | 5 + changelog/unreleased/worktree-from-cli.md | 5 + cmd/fleet/main.go | 4 + cmd/fleet/worktree.go | 284 ++++++++++++++++++ cmd/fleet/worktree_test.go | 120 ++++++++ internal/ui/adopt_test.go | 150 +++++++++ internal/ui/app.go | 134 +++++++++ 8 files changed, 705 insertions(+) create mode 100644 changelog/unreleased/adopt-external-sessions.md create mode 100644 changelog/unreleased/worktree-from-cli.md create mode 100644 cmd/fleet/worktree.go create mode 100644 cmd/fleet/worktree_test.go create mode 100644 internal/ui/adopt_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 7e3f827e..36909f04 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,6 +44,7 @@ Use [conventional commits](https://www.conventionalcommits.org/). Version is aut ## Package Structure ```text cmd/fleet/main.go # CLI entry point +cmd/fleet/worktree.go # `fleet worktree ` — worktree + session from the shell internal/tmux/tmux.go # Tmux abstraction (create, kill, capture) internal/tmux/pty.go # PTY-based attach with Ctrl+Q detach internal/session/session.go # Session model, status detection, claude --resume @@ -86,6 +87,8 @@ chrome-extension/ # Chrome MV3 extension (service worker, manifes - Tmux session prefix: `fleet_` (agent sessions); drawer shells use a distinct `fleetsh_` prefix — intentionally not a prefix of `fleet_`, so shells never leak into agent-session enumeration (`tmux.ListSessions`) - Session ID format: `<8hex>-` - SQLite DB: `~/.config/fleet/state.db` +- `fleet worktree ` (alias `wt`, `cmd/fleet/worktree.go`): the TUI's `w` key as a CLI command — create the worktree, copy `.claude/settings.local.json` + `copy_files` entries, start a session. Flags: `--base` (defaults to `git.GetDefaultBranch`), `--path` (defaults to cwd), `--agent` (defaults to `default_agent`; **validated explicitly** — `agent.Parse` falls back to Claude for anything unrecognized, so a typo would silently launch the wrong agent), `--no-session` (print only the worktree path, so `cd "$(fleet worktree foo --no-session)"` composes; conflicts with `--agent`). Flags parse on **either side** of the branch (a loop peeling one positional per `fs.Parse` — a single Parse stops at the first positional and would reject `fleet worktree foo --no-session`). It resolves to the **main** worktree (`git.GetMainWorktreePath`) before creating, so running it from inside `repo-foo` yields `repo-bar`, not `repo-foo-bar` — same as pressing `w` on an origin header. It calls `debuglog.Init()` first: `debuglog.Logger`'s fallback writes to **stderr**, and the provider/`Session.Start` log at Info, so without it the command spews slog lines over the user's terminal (`fleet add` still does). +- External-session adoption: the TUI reads sessions from SQLite exactly **once**, at startup (`loadSessions`), so a session created by another fleet process would be invisible until restart. `maybeAdoptExternalSessions` (heavy worker pass, self-throttled to `adoptSweepInterval`=5s) diffs the table against the cycle's snapshot via the pure `unknownSessions` and sends `adoptSessionsMsg`; `handleAdoptSessions` re-checks `sessionByID` (the snapshot is taken at cycle start, so the Update goroutine is the only authoritative dedupe point) then mirrors `handleSessionCreateResult` minus `SaveSession`/analytics. **Adoption only** — rows deleted elsewhere are never dropped. Unlike a session the user just created, an adopted row arrives on a timer, so the cursor must not move: it's captured with `targetForCursor()` and restored by identity via `target.find` after the rebuild. No resurrection risk — `deferDelete` deletes the SQLite row immediately, ahead of the 5s undo window. - Sessions grouped by git repo root in sidebar with tree lines (├─/└─) - Status: Running, Waiting, Finished, Idle, Error, Starting, Suspended - Status icons: ● (running/finished), ◐ (waiting), ○ (idle/starting), ✕ (error), · dim (suspended — same dot as idle; dim style + "suspended" label distinguish it) diff --git a/changelog/unreleased/adopt-external-sessions.md b/changelog/unreleased/adopt-external-sessions.md new file mode 100644 index 00000000..0dc8d650 --- /dev/null +++ b/changelog/unreleased/adopt-external-sessions.md @@ -0,0 +1,5 @@ +--- +type: improved +--- + +**No restart to see new sessions.** A session created from another terminal — `fleet worktree`, `fleet add` — now shows up in a running fleet within ~5 seconds, instead of waiting until the next launch. diff --git a/changelog/unreleased/worktree-from-cli.md b/changelog/unreleased/worktree-from-cli.md new file mode 100644 index 00000000..74a5b834 --- /dev/null +++ b/changelog/unreleased/worktree-from-cli.md @@ -0,0 +1,5 @@ +--- +type: added +--- + +**Worktrees from your shell.** `fleet worktree ` creates the worktree and starts a session in it without opening the TUI — `--base`, `--path` and `--agent` to steer it, `--no-session` to print just the path for `cd "$(fleet worktree foo --no-session)"`. diff --git a/cmd/fleet/main.go b/cmd/fleet/main.go index 619d15eb..1b844071 100644 --- a/cmd/fleet/main.go +++ b/cmd/fleet/main.go @@ -63,6 +63,8 @@ func main() { os.Exit(1) } runRemove(args[1]) + case "worktree", "wt": + runWorktree(args[1:]) case "hook-handler": handleHookHandler() case "chrome-host": @@ -348,6 +350,8 @@ Usage: fleet add Add a new session fleet list List all sessions fleet remove Remove a session + fleet worktree Create a git worktree and start a session in it + (--base, --path, --agent, --no-session) fleet hooks Manage Claude Code hooks fleet update Update to latest version fleet version Show version diff --git a/cmd/fleet/worktree.go b/cmd/fleet/worktree.go new file mode 100644 index 00000000..4538361a --- /dev/null +++ b/cmd/fleet/worktree.go @@ -0,0 +1,284 @@ +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/brizzai/fleet/internal/agent" + "github.com/brizzai/fleet/internal/config" + "github.com/brizzai/fleet/internal/debuglog" + "github.com/brizzai/fleet/internal/git" + "github.com/brizzai/fleet/internal/hooks" + "github.com/brizzai/fleet/internal/session" + "github.com/brizzai/fleet/internal/tmux" + "github.com/brizzai/fleet/internal/workspace" +) + +const worktreeUsage = "Usage: fleet worktree [flags]" + +// errMissingBranch is returned when no branch was given. runWorktree prints the +// usage line alongside it, so the message itself stays a plain error string. +var errMissingBranch = errors.New("missing branch name") + +// worktreeOpts holds the parsed `fleet worktree` invocation. Base and agent are +// left empty when unset; their defaults depend on the repo (default branch) and +// the user's config (default agent), which parsing can't see. +type worktreeOpts struct { + branch string + base string + repoPath string + agentName string + noSession bool +} + +// parseWorktreeArgs parses and validates the `fleet worktree` flags. Kept pure +// (no git, no config, no filesystem) so the argument rules are testable. +func parseWorktreeArgs(args []string) (worktreeOpts, error) { + fs := flag.NewFlagSet("fleet worktree", flag.ContinueOnError) + fs.Usage = func() { + fmt.Fprintln(fs.Output(), worktreeUsage) + fs.PrintDefaults() + } + var o worktreeOpts + fs.StringVar(&o.base, "base", "", "base branch to branch from (default: the repo's default branch)") + fs.StringVar(&o.repoPath, "path", "", "repo to create the worktree in (default: current directory)") + fs.StringVar(&o.agentName, "agent", "", "agent to run: claude, codex, or opencode (default: default_agent config)") + fs.BoolVar(&o.noSession, "no-session", false, "create the worktree only, print its path, and start no session") + // Parse in a loop, peeling off one positional at a time: Go's flag package + // stops at the first non-flag argument, so a single Parse would reject + // `fleet worktree my-branch --no-session` — the order most people type. + var positional []string + for rest := args; ; { + if err := fs.Parse(rest); err != nil { + return o, err + } + remaining := fs.Args() + if len(remaining) == 0 { + break + } + positional = append(positional, remaining[0]) + rest = remaining[1:] + } + + if len(positional) == 0 { + return o, errMissingBranch + } + if len(positional) > 1 { + return o, fmt.Errorf("unexpected argument %q — expected a single branch name", positional[1]) + } + o.branch = strings.TrimSpace(positional[0]) + + if msg := workspace.ValidateBranchName(o.branch); msg != "" { + return o, fmt.Errorf("%s", msg) + } + // agent.Parse falls back to Claude for anything it doesn't recognize, so a + // typo would silently launch the wrong agent. Reject it here instead. + if o.agentName != "" { + switch agent.Type(o.agentName) { + case agent.Claude, agent.Codex, agent.OpenCode: + default: + return o, fmt.Errorf("unknown agent %q — expected claude, codex, or opencode", o.agentName) + } + if o.noSession { + return o, fmt.Errorf("--agent has no effect with --no-session (no session is started)") + } + } + return o, nil +} + +func runWorktree(args []string) { + opts, err := parseWorktreeArgs(args) + if err != nil { + if errors.Is(err, flag.ErrHelp) { + return // flag package already printed usage + } + fmt.Fprintln(os.Stderr, err) + if errors.Is(err, errMissingBranch) { + fmt.Fprintln(os.Stderr, worktreeUsage) + } + os.Exit(1) + } + + // Both the workspace provider and Session.Start log at Info level, and + // debuglog's fallback logger writes to stderr — without Init those lines + // land on the user's terminal instead of ~/.config/fleet/debug.log. + debuglog.Init() + defer debuglog.Close() + + if err := tmux.IsTmuxAvailable(); err != nil && !opts.noSession { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + repoPath, err := resolveWorktreeRepo(opts.repoPath) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + base := opts.base + if base == "" { + base = git.GetDefaultBranch(repoPath) + } + + cfg := config.Load() + ag := agent.Parse(cfg.GetDefaultAgent()) + if opts.agentName != "" { + ag = agent.Parse(opts.agentName) + } + if !opts.noSession { + if _, err := exec.LookPath(ag.Binary()); err != nil { + fmt.Fprintf(os.Stderr, "%s CLI not found — install %s to create sessions\n", ag.Binary(), ag.DisplayName()) + os.Exit(1) + } + } + + name := workspace.SanitizeBranchName(opts.branch) + provider := workspace.ResolveProvider(repoPath) + if !provider.CanCreate() { + fmt.Fprintf(os.Stderr, "This repo's workspace provider can't create worktrees (no create command in .fleet.json)\n") + os.Exit(1) + } + + info, err := provider.Create(repoPath, name, opts.branch, base) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to create worktree: %v\n", err) + os.Exit(1) + } + if info == nil || info.Path == "" { + fmt.Fprintln(os.Stderr, "Worktree provider returned no path") + os.Exit(1) + } + + if cfg.IsCopyClaudeSettingsEnabled() && !provider.IsCustom() { + copyClaudeSettings(repoPath, info.Path) + } + workspace.CopyConfiguredFiles(repoPath, info.Path) + + // --no-session prints the path and nothing else, so the command composes: + // cd "$(fleet worktree my-branch --no-session)" + if opts.noSession { + fmt.Println(info.Path) + return + } + + // A CLI-created session needs the agent's hooks installed for status + // detection, which normally only happens on TUI launch. Only the chosen + // agent's hooks are touched — never create a config dir for an agent that + // isn't being launched. + installAgentHooks(ag, info.Path) + + storage, err := session.Open(session.DefaultDBPath()) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to open database: %v\n", err) + os.Exit(1) + } + defer storage.Close() + + s := session.NewSession(name, info.Path) + s.Agent = ag + s.WorkspaceName = name + if err := s.Start(); err != nil { + fmt.Fprintf(os.Stderr, "Created worktree %s, but failed to start session: %v\n", info.Path, err) + os.Exit(1) + } + if err := storage.SaveSession(s.ToRow()); err != nil { + fmt.Fprintf(os.Stderr, "Failed to save session: %v\n", err) + os.Exit(1) + } + // Pin the new checkout so it shows in the sidebar even before it has a + // running session, mirroring what the TUI does on session create. + if err := storage.PinRepo(session.GetRepoRoot(info.Path)); err != nil { + debuglog.Logger.Error("failed to pin repo", "repo", info.Path, "err", err) + } + + fmt.Printf("Created worktree %s (branch %s)\n", info.Path, opts.branch) + fmt.Printf("Started %s session '%s' (%s)\n", ag.DisplayName(), name, s.ID) +} + +// resolveWorktreeRepo resolves the repo to create the worktree in: the given +// path (or the current directory), walked up to its git root and then to the +// main worktree. Basing on the main clone keeps derived worktree paths siblings +// of it — running this from inside worktree "repo-foo" otherwise yields +// "repo-foo-bar" — and matches pressing `w` on an origin header in the TUI. +func resolveWorktreeRepo(path string) (string, error) { + if path == "" { + cwd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("cannot determine current directory: %w", err) + } + path = cwd + } + path = expandPath(path) + if info, err := os.Stat(path); err != nil || !info.IsDir() { + return "", fmt.Errorf("invalid directory: %s", path) + } + root := repoRootOf(path) + if root == "" { + return "", fmt.Errorf("not a git repository: %s", path) + } + return git.GetMainWorktreePath(root), nil +} + +// repoRootOf returns path's git repo root, or "" when path isn't inside a repo. +// session.GetRepoRoot can't be used here: it returns the input unchanged when +// git fails, which is indistinguishable from "path is already the root" — so a +// non-repo would silently proceed into `git worktree add`. +func repoRootOf(path string) string { + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second) + defer cancel() + out, err := exec.CommandContext(ctx, "git", "-C", path, "rev-parse", "--show-toplevel").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// installAgentHooks installs the status hooks for the agent about to be +// launched. Failures are logged, not fatal: a session without hooks still runs, +// it just falls back to pane-based status detection. +func installAgentHooks(ag agent.Type, projectPath string) { + switch ag { + case agent.Codex: + if _, err := hooks.InjectCodexHooks(hooks.GetCodexConfigDir()); err != nil { + debuglog.Logger.Error("codex hook inject failed", "err", err) + } + // Codex prompts to trust a new directory on first launch; pre-seed trust + // so the session opens straight to the prompt. + if err := hooks.EnsureCodexDirTrust(hooks.GetCodexConfigDir(), projectPath); err != nil { + debuglog.Logger.Error("codex dir trust seeding failed", "path", projectPath, "err", err) + } + case agent.OpenCode: + if _, err := hooks.InjectOpenCodePlugin(hooks.GetOpenCodeConfigDir()); err != nil { + debuglog.Logger.Error("opencode plugin inject failed", "err", err) + } + default: + if _, err := hooks.InjectClaudeHooks(hooks.GetClaudeConfigDir()); err != nil { + debuglog.Logger.Error("claude hook inject failed", "err", err) + } + } +} + +// copyClaudeSettings copies .claude/settings.local.json from srcRepo to dstRepo. +// Mirrors the TUI's copyClaudeSettingsFile, which lives in package ui. +func copyClaudeSettings(srcRepo, dstRepo string) { + data, err := os.ReadFile(filepath.Join(srcRepo, ".claude", "settings.local.json")) + if err != nil { + return // source doesn't exist, nothing to copy + } + dstDir := filepath.Join(dstRepo, ".claude") + if err := os.MkdirAll(dstDir, 0755); err != nil { + debuglog.Logger.Error("copyClaudeSettings: failed to create .claude dir", "dst", dstDir, "err", err) + return + } + if err := os.WriteFile(filepath.Join(dstDir, "settings.local.json"), data, 0600); err != nil { + debuglog.Logger.Error("copyClaudeSettings: failed to write settings file", "dst", dstRepo, "err", err) + } +} diff --git a/cmd/fleet/worktree_test.go b/cmd/fleet/worktree_test.go new file mode 100644 index 00000000..8f06b636 --- /dev/null +++ b/cmd/fleet/worktree_test.go @@ -0,0 +1,120 @@ +package main + +import ( + "errors" + "strings" + "testing" +) + +func TestParseWorktreeArgs(t *testing.T) { + t.Run("branch only", func(t *testing.T) { + o, err := parseWorktreeArgs([]string{"fix-login"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if o.branch != "fix-login" { + t.Errorf("branch = %q, want fix-login", o.branch) + } + // Base and agent stay empty so the caller can fill in the repo's default + // branch and the configured default agent — parsing can't see either. + if o.base != "" || o.agentName != "" || o.repoPath != "" { + t.Errorf("expected unset defaults, got base=%q agent=%q path=%q", o.base, o.agentName, o.repoPath) + } + if o.noSession { + t.Error("noSession should default to false") + } + }) + + t.Run("all flags", func(t *testing.T) { + o, err := parseWorktreeArgs([]string{"--base", "develop", "--path", "/tmp/repo", "--agent", "codex", "feature/x"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if o.branch != "feature/x" || o.base != "develop" || o.repoPath != "/tmp/repo" || o.agentName != "codex" { + t.Errorf("got %+v", o) + } + }) + + // `fleet worktree my-branch --no-session` is the order most people type, and + // a plain flag.Parse would reject it — it stops at the first positional. + t.Run("flags after branch", func(t *testing.T) { + o, err := parseWorktreeArgs([]string{"fix-login", "--base", "develop", "--no-session"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if o.branch != "fix-login" || o.base != "develop" || !o.noSession { + t.Errorf("got %+v", o) + } + }) + + t.Run("flags on both sides of branch", func(t *testing.T) { + o, err := parseWorktreeArgs([]string{"--base", "develop", "fix-login", "--agent", "codex"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if o.branch != "fix-login" || o.base != "develop" || o.agentName != "codex" { + t.Errorf("got %+v", o) + } + }) + + t.Run("two positionals rejected", func(t *testing.T) { + _, err := parseWorktreeArgs([]string{"fix-login", "extra"}) + if err == nil { + t.Fatal("expected an error for a second positional argument") + } + }) + + t.Run("missing branch", func(t *testing.T) { + _, err := parseWorktreeArgs(nil) + if !errors.Is(err, errMissingBranch) { + t.Fatalf("err = %v, want errMissingBranch", err) + } + }) + + t.Run("invalid branch name", func(t *testing.T) { + // A leading '-' is unreachable here — the flag package claims it before + // ValidateBranchName ever sees it — so these are the cases that matter. + for _, branch := range []string{"has..dots", "trailing/", "@", "with@{brace"} { + if _, err := parseWorktreeArgs([]string{branch}); err == nil { + t.Errorf("branch %q: expected a validation error", branch) + } + } + }) + + t.Run("unknown agent is rejected", func(t *testing.T) { + // agent.Parse falls back to Claude for anything it doesn't recognize, so + // without this check a typo would silently launch the wrong agent. + _, err := parseWorktreeArgs([]string{"--agent", "codek", "fix-login"}) + if err == nil { + t.Fatal("expected an error for an unknown agent") + } + if !strings.Contains(err.Error(), "codek") { + t.Errorf("error should name the bad value, got %q", err) + } + }) + + t.Run("known agents accepted", func(t *testing.T) { + for _, name := range []string{"claude", "codex", "opencode"} { + if _, err := parseWorktreeArgs([]string{"--agent", name, "fix-login"}); err != nil { + t.Errorf("agent %q: unexpected error %v", name, err) + } + } + }) + + t.Run("agent conflicts with no-session", func(t *testing.T) { + _, err := parseWorktreeArgs([]string{"--agent", "codex", "--no-session", "fix-login"}) + if err == nil { + t.Fatal("expected --agent + --no-session to be rejected") + } + }) + + t.Run("no-session alone", func(t *testing.T) { + o, err := parseWorktreeArgs([]string{"--no-session", "fix-login"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !o.noSession { + t.Error("noSession should be true") + } + }) +} diff --git a/internal/ui/adopt_test.go b/internal/ui/adopt_test.go new file mode 100644 index 00000000..95df9a4c --- /dev/null +++ b/internal/ui/adopt_test.go @@ -0,0 +1,150 @@ +package ui + +import ( + "path/filepath" + "testing" + + "github.com/brizzai/fleet/internal/analytics" + "github.com/brizzai/fleet/internal/config" + "github.com/brizzai/fleet/internal/session" +) + +func adoptTestHome(t *testing.T) *Home { + t.Helper() + storage, err := session.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { storage.Close() }) + h := NewHome(storage, &config.Config{TickIntervalSec: 2}, "test", analytics.Identity{}) + h.width, h.height = 120, 40 + return h +} + +// storeSession writes a session row straight to SQLite without telling the +// running model about it — what `fleet worktree` does from another process. +func storeSession(t *testing.T, h *Home, title, path string) *session.SessionRow { + t.Helper() + s := session.NewSession(title, path) + row := s.ToRow() + if err := h.storage.SaveSession(row); err != nil { + t.Fatalf("save session: %v", err) + } + return row +} + +func TestUnknownSessionsFindsOnlyNewRows(t *testing.T) { + rowA := session.NewSession("a", "/tmp/a").ToRow() + rowB := session.NewSession("b", "/tmp/b").ToRow() + known := []*session.Session{session.FromRow(rowA)} + + got := unknownSessions([]*session.SessionRow{rowA, rowB}, known, "") + if len(got) != 1 || got[0].ID != rowB.ID { + t.Fatalf("got %d sessions, want just %s", len(got), rowB.ID) + } + + // Nothing new once both are known. + known = append(known, session.FromRow(rowB)) + if got := unknownSessions([]*session.SessionRow{rowA, rowB}, known, ""); len(got) != 0 { + t.Fatalf("expected no unknown sessions, got %d", len(got)) + } +} + +// The demo filter has to match loadSessions', or a demo-mode TUI adopts back +// the sessions it was launched to hide. +func TestUnknownSessionsRespectsDemoPrefix(t *testing.T) { + inside := session.NewSession("inside", "/demo/repo").ToRow() + outside := session.NewSession("outside", "/elsewhere/repo").ToRow() + + got := unknownSessions([]*session.SessionRow{inside, outside}, nil, "/demo") + if len(got) != 1 || got[0].ID != inside.ID { + t.Fatalf("got %d sessions, want just the /demo one", len(got)) + } +} + +func TestHandleAdoptSessionsAddsAndPins(t *testing.T) { + h := adoptTestHome(t) + repo := t.TempDir() + row := storeSession(t, h, "from-cli", repo) + + h.handleAdoptSessions(adoptSessionsMsg{sessions: []*session.Session{session.FromRow(row)}}) + + if _, ok := h.sessionByID[row.ID]; !ok { + t.Fatal("adopted session missing from sessionByID") + } + if len(h.sessions) != 1 { + t.Fatalf("h.sessions has %d entries, want 1", len(h.sessions)) + } + repoRoot := session.GetRepoRoot(repo) + if !h.pinnedRepos[repoRoot] { + t.Error("adopted session's repo was not pinned") + } + pinned, err := h.storage.LoadPinnedRepos() + if err != nil { + t.Fatalf("load pinned repos: %v", err) + } + var found bool + for _, p := range pinned { + if p == repoRoot { + found = true + } + } + if !found { + t.Error("pin was not persisted to storage") + } +} + +// The worker diffs against a snapshot taken at cycle start, so it can offer a +// session the Update loop created since. The handler is the only place that +// check is authoritative. +func TestHandleAdoptSessionsSkipsAlreadyKnown(t *testing.T) { + h := adoptTestHome(t) + row := storeSession(t, h, "from-cli", t.TempDir()) + + msg := adoptSessionsMsg{sessions: []*session.Session{session.FromRow(row)}} + h.handleAdoptSessions(msg) + h.handleAdoptSessions(msg) // same row again + + if len(h.sessions) != 1 { + t.Fatalf("h.sessions has %d entries after a duplicate adopt, want 1", len(h.sessions)) + } +} + +// An adopted row arrives on a timer, not because the user asked for it, so it +// must never slide the selection out from under someone mid-keystroke. +func TestHandleAdoptSessionsKeepsCursorOnItsRow(t *testing.T) { + h := adoptTestHome(t) + // t.TempDir() hands out .../001, .../002 in call order, and checkouts sort + // by path — so a session in the first dir renders above one in the second. + above, below := t.TempDir(), t.TempDir() + + parkedRow := storeSession(t, h, "parked", below) + h.handleAdoptSessions(adoptSessionsMsg{sessions: []*session.Session{session.FromRow(parkedRow)}}) + + idxBefore := indexOfSession(h, parkedRow.ID) + if idxBefore < 0 { + t.Fatal("parked session not in the sidebar") + } + h.cursor = idxBefore + + // Adopt a session that renders above the parked one, pushing its index down. + newRow := storeSession(t, h, "adopted", above) + h.handleAdoptSessions(adoptSessionsMsg{sessions: []*session.Session{session.FromRow(newRow)}}) + + idxAfter := indexOfSession(h, parkedRow.ID) + if idxAfter == idxBefore { + t.Fatalf("test is vacuous: the adopted row did not shift the parked row (still at %d)", idxBefore) + } + if h.cursor != idxAfter { + t.Fatalf("cursor moved off its row: parked session is at %d, cursor is at %d", idxAfter, h.cursor) + } +} + +func indexOfSession(h *Home, id string) int { + for i, item := range h.flatItems { + if item.Session != nil && item.Session.ID == id { + return i + } + } + return -1 +} diff --git a/internal/ui/app.go b/internal/ui/app.go index 03d6bf0f..8d209862 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -139,6 +139,11 @@ type ( prCache map[string]*session.PRCacheRow err error } + // adoptSessionsMsg carries session rows found in SQLite that this TUI + // doesn't know about — created by another fleet process while it ran. + adoptSessionsMsg struct { + sessions []*session.Session + } openEditorMsg struct{ err error } openPRMsg struct{ err error } quickApproveMsg struct{ err error } @@ -283,6 +288,9 @@ type Home struct { // lastSuspendSweepAt throttles the memory-pressure idle-suspend sweep so it // runs on its own slow cadence inside the ~2s heavy pass. lastSuspendSweepAt time.Time + // lastAdoptSweepAt throttles the externally-created-session sweep, mirroring + // lastSuspendSweepAt. Worker-goroutine-only, so it needs no lock. + lastAdoptSweepAt time.Time // groupSnooze maps a sidebar group key ("origin:" or a checkout path) // to its umbrella snooze deadline. Read only while building the sidebar @@ -848,6 +856,9 @@ func (h *Home) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case sessionCreateResultMsg: return h.handleSessionCreateResult(msg) + case adoptSessionsMsg: + return h.handleAdoptSessions(msg) + case tccProbeResultMsg: if !msg.determined { // Inconclusive (no server yet / tmux error) — unlatch so a later create @@ -3003,6 +3014,59 @@ func (h *Home) handleSessionCreateResult(msg sessionCreateResultMsg) (tea.Model, return h, tea.Batch(h.probeTCCCmd(s.ProjectPath), h.fetchPreviewForSelected()) } +// handleAdoptSessions folds sessions created by another fleet process into the +// running TUI. It mirrors handleSessionCreateResult's bookkeeping minus the +// parts that don't apply: the DB row already exists (no SaveSession), and this +// isn't a session *this* user just asked for (no analytics, no auto-select). +func (h *Home) handleAdoptSessions(msg adoptSessionsMsg) (tea.Model, tea.Cmd) { + // The worker diffed against a snapshot taken at cycle start, so it can offer + // a session the Update loop created in the meantime. This is the only place + // the check is authoritative. + fresh := make([]*session.Session, 0, len(msg.sessions)) + for _, s := range msg.sessions { + if _, exists := h.sessionByID[s.ID]; !exists { + fresh = append(fresh, s) + } + } + if len(fresh) == 0 { + return h, nil + } + + // Unlike a session the user just created, an adopted row arrives on a timer. + // Inserting it above the cursor would silently move the selection out from + // under whoever is mid-keystroke, so re-find the row by identity afterwards. + target := h.targetForCursor() + + h.workerMu.Lock() + h.sessions = append(h.sessions, fresh...) + h.rebuildSessionMap() + h.workerMu.Unlock() + + cmds := make([]tea.Cmd, 0, len(fresh)) + for _, s := range fresh { + repo := session.GetRepoRoot(s.ProjectPath) + h.setExpanded(repo, true) + if !h.pinnedRepos[repo] { + h.pinnedRepos[repo] = true + if err := h.storage.PinRepo(repo); err != nil { + debuglog.Logger.Error("failed to pin repo", "repo", repo, "err", err) + } + } + // The creating process already spun up the tmux server, so the probe + // runs against a live one. Self-guards: nil unless the path sits under + // an unprobed macOS-protected folder. + if cmd := h.probeTCCCmd(s.ProjectPath); cmd != nil { + cmds = append(cmds, cmd) + } + } + h.rebuildFlatItems() + if idx := target.find(h.flatItems); idx >= 0 { + h.cursor = idx + h.syncViewport() + } + return h, tea.Batch(cmds...) +} + // deleteAtCursor runs the delete whose scope follows the cursor: an origin // header forgets the whole group, any other header acts on the container // (§ confirmDeleteHeader), and a session row deletes just that session. Shared by @@ -3714,6 +3778,71 @@ func (h *Home) maybeSuspendIdleSessions(sessions []*session.Session) { } } +// adoptSweepInterval throttles the externally-created-session sweep to its own +// cadence inside the ~2s heavy worker pass. Re-reading the sessions table every +// heavy cycle would buy nothing: the payoff is a CLI-created session showing up +// without a restart, and a few seconds is a fine latency for that. +const adoptSweepInterval = 5 * time.Second + +// maybeAdoptExternalSessions picks up session rows written by another fleet +// process — `fleet worktree`, `fleet add` — while this TUI is running. Sessions +// are otherwise read from SQLite exactly once, at startup (loadSessions), so +// without this a session created from the shell stays invisible until restart. +// +// Adoption only: rows deleted by another process are NOT dropped from the +// in-memory list. Runs on the worker goroutine (it reads SQLite); the actual +// mutation happens on the Update goroutine via adoptSessionsMsg. +func (h *Home) maybeAdoptExternalSessions(known []*session.Session) { + if !h.lastAdoptSweepAt.IsZero() && time.Since(h.lastAdoptSweepAt) < adoptSweepInterval { + return + } + h.lastAdoptSweepAt = time.Now() + + rows, err := h.storage.LoadSessions() + if err != nil { + debuglog.Logger.Error("adopt sweep: failed to load sessions", "err", err) + return + } + adopted := unknownSessions(rows, known, os.Getenv("FLEET_DEMO_PREFIX")) + if len(adopted) == 0 { + return + } + debuglog.Logger.Info("adopting externally-created sessions", "count", len(adopted)) + // Same rendezvous hazard as the suspend sweep's send: program.Send blocks on + // Tea's unbuffered channel, and tea.Exec suspends the loop that drains it. + // Skipping costs nothing — the next sweep re-finds these rows. + if !h.isAttaching.Load() { + h.send(adoptSessionsMsg{sessions: adopted}) + } +} + +// unknownSessions returns the stored rows that aren't in known, hydrated into +// sessions. demoPrefix mirrors the FLEET_DEMO_PREFIX filter loadSessions +// applies, so a demo-mode TUI doesn't adopt back the sessions it was launched +// to hide; "" disables it. +// +// This is a cheap pre-filter only: the caller's snapshot is taken at worker +// cycle start, so it can miss a session the Update loop created since. The +// adoptSessionsMsg handler re-checks sessionByID, which is where dedupe is +// authoritative. +func unknownSessions(rows []*session.SessionRow, known []*session.Session, demoPrefix string) []*session.Session { + seen := make(map[string]bool, len(known)) + for _, s := range known { + seen[s.ID] = true + } + var out []*session.Session + for _, row := range rows { + if seen[row.ID] { + continue + } + if demoPrefix != "" && !strings.HasPrefix(row.ProjectPath, demoPrefix) { + continue + } + out = append(out, session.FromRow(row)) + } + return out +} + // suspendIdleThreshold returns the minimum idle duration a StatusIdle session must // exceed to be auto-suspended in the given mode at the given memory-pressure level, // and whether any suspension should happen at all right now. @@ -5400,6 +5529,11 @@ drainPriority: // which is why it lives here on the worker, not the Update loop. h.maybeSuspendIdleSessions(sessions) + // 5c. Adopt sessions another fleet process created (e.g. `fleet worktree` + // from a shell) since the last sweep. Self-throttled; reads SQLite, which is + // why it lives here rather than on the Update loop. + h.maybeAdoptExternalSessions(sessions) + // 5. Git+PR refresh used to run here, inline. It now lives on its own // goroutine (gitWorker) — see the comment there for why sharing this // one was a bug. From c01cb2891b55e6b2c817f7f13c4fb73561e26102 Mon Sep 17 00:00:00 2001 From: Yuval Hayke Date: Sun, 2 Aug 2026 15:37:41 +0300 Subject: [PATCH 2/2] fix(cli): address PR #230 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten findings, all verified against the code before fixing; none were false positives. The serious one: the adoption sweep never ran on an empty fleet. statusWorkerCycle returns early when there are no sessions and no shells, and the sweep sat below that guard — so a fresh install, or "I just deleted my last session", would never pick up a CLI-created session at all. That is precisely the case this feature exists for, and my manual verification missed it because the test TUI had one session, which put the cycle past the guard. The sweep now runs above the early return, with a regression test that drives a zero-session cycle (confirmed failing without the fix). Also on the adoption path: session.GetRepoRoot shelled out to git inside Update(), and for an adopted session the cache miss is guaranteed — the path was created by another process — so the roots are now resolved on the worker and carried in the message. Cursor preservation missed "Creating…" phantom rows, which is the row handleWorkspaceCreate auto-selects, so the cursor could drift during exactly the window this PR's command lands in; contextMenuTarget grows a pendingID case. Auto-expanding the group is gone: setExpanded persists, so a 5s timer would overwrite a collapse the user chose — sharpest against snooze, where the collapse is the visual half of the snooze. On the CLI side, flag parse errors printed twice (flag.ContinueOnError doesn't suppress output), a SaveSession failure left a live tmux session no DB row pointed at — invisible to the very adoption sweep this PR adds — and --base was silently dropped when the branch already existed, while the success line still implied a fresh branch. The command also missed migration.Run(), so a legacy brizz-code config dir could be stranded permanently by creating state.db before the migration ever looked; scoped to runWorktree rather than main() so hook-handler's hot path is untouched. Telemetry now fires at adoption with source: "cli", and the existing TUI call is tagged "tui" so the property isn't null on half the events. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Q1yXo3oP17DQwVbnagnWDY --- .../unreleased/adopt-external-sessions.md | 2 +- cmd/fleet/worktree.go | 119 +++++++++++++++--- internal/ui/adopt_test.go | 89 +++++++++++++ internal/ui/app.go | 96 +++++++++++++- 4 files changed, 280 insertions(+), 26 deletions(-) diff --git a/changelog/unreleased/adopt-external-sessions.md b/changelog/unreleased/adopt-external-sessions.md index 0dc8d650..613b6933 100644 --- a/changelog/unreleased/adopt-external-sessions.md +++ b/changelog/unreleased/adopt-external-sessions.md @@ -2,4 +2,4 @@ type: improved --- -**No restart to see new sessions.** A session created from another terminal — `fleet worktree`, `fleet add` — now shows up in a running fleet within ~5 seconds, instead of waiting until the next launch. +**No restart needed.** A session created from another terminal — `fleet worktree`, `fleet add` — now shows up in a running fleet within ~5 seconds, instead of waiting until the next launch. diff --git a/cmd/fleet/worktree.go b/cmd/fleet/worktree.go index 4538361a..dc47b73b 100644 --- a/cmd/fleet/worktree.go +++ b/cmd/fleet/worktree.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -16,6 +17,7 @@ import ( "github.com/brizzai/fleet/internal/debuglog" "github.com/brizzai/fleet/internal/git" "github.com/brizzai/fleet/internal/hooks" + "github.com/brizzai/fleet/internal/migration" "github.com/brizzai/fleet/internal/session" "github.com/brizzai/fleet/internal/tmux" "github.com/brizzai/fleet/internal/workspace" @@ -38,19 +40,38 @@ type worktreeOpts struct { noSession bool } -// parseWorktreeArgs parses and validates the `fleet worktree` flags. Kept pure -// (no git, no config, no filesystem) so the argument rules are testable. -func parseWorktreeArgs(args []string) (worktreeOpts, error) { +// worktreeFlagSet builds the `fleet worktree` flag set, binding into o. +// +// The flag package is kept silent — `flag.ContinueOnError` does NOT suppress +// output: Parse calls failf, which writes the error to fs.Output() and then +// runs Usage. Left alone, a bad flag prints the error, the usage block, and +// then the error again from the caller. printWorktreeUsage owns the usage text +// and runWorktree owns the errors, so each is emitted exactly once, in order. +func worktreeFlagSet(o *worktreeOpts) *flag.FlagSet { fs := flag.NewFlagSet("fleet worktree", flag.ContinueOnError) - fs.Usage = func() { - fmt.Fprintln(fs.Output(), worktreeUsage) - fs.PrintDefaults() - } - var o worktreeOpts + fs.SetOutput(io.Discard) + fs.Usage = func() {} fs.StringVar(&o.base, "base", "", "base branch to branch from (default: the repo's default branch)") fs.StringVar(&o.repoPath, "path", "", "repo to create the worktree in (default: current directory)") fs.StringVar(&o.agentName, "agent", "", "agent to run: claude, codex, or opencode (default: default_agent config)") fs.BoolVar(&o.noSession, "no-session", false, "create the worktree only, print its path, and start no session") + return fs +} + +// printWorktreeUsage writes the usage line and flag defaults to w. +func printWorktreeUsage(w io.Writer) { + fmt.Fprintln(w, worktreeUsage) + var discard worktreeOpts + fs := worktreeFlagSet(&discard) + fs.SetOutput(w) + fs.PrintDefaults() +} + +// parseWorktreeArgs parses and validates the `fleet worktree` flags. Kept pure +// (no git, no config, no filesystem) so the argument rules are testable. +func parseWorktreeArgs(args []string) (worktreeOpts, error) { + var o worktreeOpts + fs := worktreeFlagSet(&o) // Parse in a loop, peeling off one positional at a time: Go's flag package // stops at the first non-flag argument, so a single Parse would reject // `fleet worktree my-branch --no-session` — the order most people type. @@ -97,15 +118,27 @@ func runWorktree(args []string) { opts, err := parseWorktreeArgs(args) if err != nil { if errors.Is(err, flag.ErrHelp) { - return // flag package already printed usage + printWorktreeUsage(os.Stdout) // `-h` is a request, not a failure + return } fmt.Fprintln(os.Stderr, err) - if errors.Is(err, errMissingBranch) { - fmt.Fprintln(os.Stderr, worktreeUsage) + // A bad flag or a missing branch is a usage error — show the flags. A + // bad *value* (branch name, agent) isn't: the message already says what + // to fix, and the flag list would bury it. + if errors.Is(err, errMissingBranch) || strings.HasPrefix(err.Error(), "flag ") { + printWorktreeUsage(os.Stderr) } os.Exit(1) } + // Migrate a legacy `brizz-code` config dir before anything below creates + // `~/.config/fleet/` — debuglog.Init does, and session.Open creates + // state.db. Once state.db exists, migrateConfigDir permanently bails out + // ("both dirs have state.db") and still writes its marker, silently + // stranding the user's sessions, pins and slot bindings. runTUI does this + // for the same reason and in the same order. + migration.Run() + // Both the workspace provider and Session.Start log at Info level, and // debuglog's fallback logger writes to stderr — without Init those lines // land on the user's terminal instead of ~/.config/fleet/debug.log. @@ -147,6 +180,32 @@ func runWorktree(args []string) { os.Exit(1) } + // GitWorktreeProvider.Create retries without `-b` when the branch already + // exists, which silently drops --base too. In the TUI you'd have seen the + // branch in the existing-worktrees list first; from a shell there's no + // signal at all, and "Created … (branch X)" reads as "made X off your base". + // Say it up front instead. Git provider only — a ShellProvider defines its + // own branch semantics. + reusedBranch := !provider.IsCustom() && branchExists(repoPath, opts.branch) + if reusedBranch { + fmt.Fprintf(os.Stderr, "Branch %q already exists — reusing it.\n", opts.branch) + if opts.base != "" { + fmt.Fprintf(os.Stderr, "--base %s ignored: the branch already has a base.\n", opts.base) + } + } + + // Open the DB before anything touches the disk: a database that can't be + // opened at all should fail before a worktree exists, not after. + var storage *session.StateDB + if !opts.noSession { + storage, err = session.Open(session.DefaultDBPath()) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to open database: %v\n", err) + os.Exit(1) + } + defer storage.Close() + } + info, err := provider.Create(repoPath, name, opts.branch, base) if err != nil { fmt.Fprintf(os.Stderr, "Failed to create worktree: %v\n", err) @@ -175,13 +234,6 @@ func runWorktree(args []string) { // isn't being launched. installAgentHooks(ag, info.Path) - storage, err := session.Open(session.DefaultDBPath()) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to open database: %v\n", err) - os.Exit(1) - } - defer storage.Close() - s := session.NewSession(name, info.Path) s.Agent = ag s.WorkspaceName = name @@ -190,7 +242,16 @@ func runWorktree(args []string) { os.Exit(1) } if err := storage.SaveSession(s.ToRow()); err != nil { + // The tmux session is already live but nothing will ever point at it — + // no DB row means the TUI can't list it, adopt it, or offer to delete + // it. Tear it down so the failure is clean; if that also fails, name it + // so the user can find it with `tmux ls`. fmt.Fprintf(os.Stderr, "Failed to save session: %v\n", err) + if killErr := s.GetTmuxSession().Kill(); killErr != nil { + fmt.Fprintf(os.Stderr, "Also failed to stop its tmux session %q: %v\n", s.TmuxSessionName, killErr) + } else { + fmt.Fprintf(os.Stderr, "Stopped the orphaned tmux session. The worktree %s was kept.\n", info.Path) + } os.Exit(1) } // Pin the new checkout so it shows in the sidebar even before it has a @@ -199,7 +260,7 @@ func runWorktree(args []string) { debuglog.Logger.Error("failed to pin repo", "repo", info.Path, "err", err) } - fmt.Printf("Created worktree %s (branch %s)\n", info.Path, opts.branch) + fmt.Printf("Created worktree %s (%s)\n", info.Path, branchNote(opts.branch, reusedBranch)) fmt.Printf("Started %s session '%s' (%s)\n", ag.DisplayName(), name, s.ID) } @@ -241,6 +302,26 @@ func repoRootOf(path string) string { return strings.TrimSpace(string(out)) } +// branchNote describes what happened to the branch, so the success line can't +// imply a fresh branch off --base when an existing one was checked out. +func branchNote(branch string, reused bool) string { + if reused { + return fmt.Sprintf("existing branch %s", branch) + } + return fmt.Sprintf("branch %s", branch) +} + +// branchExists reports whether repoPath already has a local branch named +// branch. Used to warn that the worktree reuses it and that --base is moot, +// since GitWorktreeProvider.Create silently falls back to the existing branch. +func branchExists(repoPath, branch string) bool { + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second) + defer cancel() + err := exec.CommandContext(ctx, "git", "-C", repoPath, + "show-ref", "--verify", "--quiet", "refs/heads/"+branch).Run() + return err == nil +} + // installAgentHooks installs the status hooks for the agent about to be // launched. Failures are logged, not fatal: a session without hooks still runs, // it just falls back to pane-based status detection. diff --git a/internal/ui/adopt_test.go b/internal/ui/adopt_test.go index 95df9a4c..50a72e3d 100644 --- a/internal/ui/adopt_test.go +++ b/internal/ui/adopt_test.go @@ -3,6 +3,7 @@ package ui import ( "path/filepath" "testing" + "time" "github.com/brizzai/fleet/internal/analytics" "github.com/brizzai/fleet/internal/config" @@ -140,6 +141,94 @@ func TestHandleAdoptSessionsKeepsCursorOnItsRow(t *testing.T) { } } +// statusWorkerCycle returns early when there are no sessions and no shells. +// The adoption sweep has to run anyway: an empty sidebar is the likeliest place +// to be driving fleet from the shell, and it's the one case where a session +// appearing is the whole point. Regression guard for the bug where the sweep +// sat below that guard and was unreachable. +func TestStatusWorkerCycleAdoptsOnEmptyFleet(t *testing.T) { + h := adoptTestHome(t) + storeSession(t, h, "from-cli", t.TempDir()) + + if len(h.sessions) != 0 || len(h.shells) != 0 { + t.Fatalf("precondition: fleet must be empty, got %d sessions / %d shells", len(h.sessions), len(h.shells)) + } + + h.statusWorkerCycle() + + // h.send is a no-op without a running program, so the adopted session can't + // land in h.sessions here — what's under test is that the sweep was reached + // at all past the empty-fleet guard. + if h.lastAdoptSweepAt.IsZero() { + t.Fatal("adoption sweep never ran on an empty fleet") + } +} + +// A "Creating…" phantom is the row handleWorkspaceCreate auto-selects, so it's +// exactly where the cursor sits while a TUI-side worktree create is in flight — +// the window a shell `fleet worktree` lands in. targetForCursor has to know it, +// or the cursor silently drifts when an adopted row is inserted above. +func TestHandleAdoptSessionsKeepsCursorOnPendingRow(t *testing.T) { + h := adoptTestHome(t) + above, pendingRepo := t.TempDir(), t.TempDir() + + h.pinnedRepos[pendingRepo] = true + pw := &PendingWorkspace{ID: "pending-test", Name: "creating", RepoPath: pendingRepo} + h.pendingWorkspaces = append(h.pendingWorkspaces, pw) + h.rebuildFlatItems() + + idxBefore := indexOfPending(h, pw.ID) + if idxBefore < 0 { + t.Fatal("phantom row not in the sidebar") + } + h.cursor = idxBefore + + newRow := storeSession(t, h, "adopted", above) + h.handleAdoptSessions(adoptSessionsMsg{ + sessions: []*session.Session{session.FromRow(newRow)}, + repoRoots: map[string]string{newRow.ID: above}, + }) + + idxAfter := indexOfPending(h, pw.ID) + if idxAfter == idxBefore { + t.Fatalf("test is vacuous: the adopted row did not shift the phantom (still at %d)", idxBefore) + } + if h.cursor != idxAfter { + t.Fatalf("cursor drifted off the phantom: phantom is at %d, cursor is at %d", idxAfter, h.cursor) + } +} + +// setExpanded persists, so auto-expanding on a 5s timer would overwrite a +// collapse the user chose. Snooze is the sharp case: it collapses its group on +// purpose, and re-opening leaves an expanded group still showing a ☾ countdown. +func TestCanAutoExpandRespectsCollapseAndSnooze(t *testing.T) { + h := adoptTestHome(t) + + if !h.canAutoExpand("/repo/untouched") { + t.Error("a group with no explicit state should be expandable") + } + + h.repoExpanded["/repo/collapsed"] = false + if h.canAutoExpand("/repo/collapsed") { + t.Error("a deliberately collapsed group must not be auto-expanded") + } + + h.repoExpanded["/repo/snoozed"] = false + h.groupSnooze["/repo/snoozed"] = time.Now().Add(time.Hour) + if h.canAutoExpand("/repo/snoozed") { + t.Error("a snoozed group must not be auto-expanded") + } +} + +func indexOfPending(h *Home, id string) int { + for i, item := range h.flatItems { + if item.Pending != nil && item.Pending.ID == id { + return i + } + } + return -1 +} + func indexOfSession(h *Home, id string) int { for i, item := range h.flatItems { if item.Session != nil && item.Session.ID == id { diff --git a/internal/ui/app.go b/internal/ui/app.go index 8d209862..94b3310c 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -143,6 +143,12 @@ type ( // doesn't know about — created by another fleet process while it ran. adoptSessionsMsg struct { sessions []*session.Session + // repoRoots maps session ID -> git repo root, resolved on the worker. + // session.GetRepoRoot shells out to git on a cache miss, and an adopted + // session's path was created by another process, so the miss is + // guaranteed — resolving it in Update() would put a git subprocess with + // an 8s timeout on the UI loop. + repoRoots map[string]string } openEditorMsg struct{ err error } openPRMsg struct{ err error } @@ -2971,7 +2977,13 @@ func (h *Home) handleSessionCreateResult(msg sessionCreateResultMsg) (tea.Model, return h, nil } - analytics.Track(analytics.EventSessionCreated, map[string]interface{}{"agent": string(msg.session.Agent)}) + // "source" is also set on the adopted-session path, so it must be set here + // too — otherwise it reads as null on every TUI-created session and the + // property can't be split on. + analytics.Track(analytics.EventSessionCreated, map[string]interface{}{ + "agent": string(msg.session.Agent), + "source": "tui", + }) if analytics.MarkOnboardingMilestone(analytics.MilestoneFirstSession) { analytics.Track(analytics.EventOnboardingFirstSessionCreated, map[string]interface{}{ "seconds_since_install": int(analytics.SecondsSinceInstall()), @@ -3044,14 +3056,36 @@ func (h *Home) handleAdoptSessions(msg adoptSessionsMsg) (tea.Model, tea.Cmd) { cmds := make([]tea.Cmd, 0, len(fresh)) for _, s := range fresh { - repo := session.GetRepoRoot(s.ProjectPath) - h.setExpanded(repo, true) + // Resolved on the worker — see adoptSessionsMsg.repoRoots. + repo := msg.repoRoots[s.ID] + if repo == "" { + repo = s.ProjectPath + } + // Deliberately NOT setExpanded(repo, true), unlike + // handleSessionCreateResult: that runs because the user just pressed a + // key, this runs off a timer. setExpanded also persists, so auto-opening + // would overwrite a collapse the user chose. The snooze case is the + // sharp one — snoozing a group collapses it, and that collapse is the + // visual half of the snooze, so re-expanding leaves an open group still + // carrying a ☾ countdown. + if h.canAutoExpand(repo) { + h.setExpanded(repo, true) + } if !h.pinnedRepos[repo] { h.pinnedRepos[repo] = true if err := h.storage.PinRepo(repo); err != nil { debuglog.Logger.Error("failed to pin repo", "repo", repo, "err", err) } } + analytics.Track(analytics.EventSessionCreated, map[string]interface{}{ + "agent": string(s.Agent), + "source": "cli", + }) + if analytics.MarkOnboardingMilestone(analytics.MilestoneFirstSession) { + analytics.Track(analytics.EventOnboardingFirstSessionCreated, map[string]interface{}{ + "seconds_since_install": int(analytics.SecondsSinceInstall()), + }) + } // The creating process already spun up the tmux server, so the probe // runs against a live one. Self-guards: nil unless the path sits under // an unprobed macOS-protected folder. @@ -3062,11 +3096,33 @@ func (h *Home) handleAdoptSessions(msg adoptSessionsMsg) (tea.Model, tea.Cmd) { h.rebuildFlatItems() if idx := target.find(h.flatItems); idx >= 0 { h.cursor = idx - h.syncViewport() } + // Outside the branch: adoption only ever adds rows, so even when the target + // is gone the cursor index is still in range — but the rows under it moved, + // so the viewport has to be re-synced either way. + h.syncViewport() return h, tea.Batch(cmds...) } +// canAutoExpand reports whether a group may be opened without the user asking. +// False when they collapsed it themselves, and false while it's snoozed — a +// snooze collapses its group on purpose, so re-opening it would leave the two +// halves of the snooze contradicting each other. +func (h *Home) canAutoExpand(repo string) bool { + if expanded, set := h.repoExpanded[repo]; set && !expanded { + return false + } + if _, snoozed := h.groupSnooze[repo]; snoozed { + return false + } + if origin := OriginExpandKey(h.originOf(repo)); origin != "" { + if _, snoozed := h.groupSnooze[origin]; snoozed { + return false + } + } + return true +} + // deleteAtCursor runs the delete whose scope follows the cursor: an origin // header forgets the whole group, any other header acts on the container // (§ confirmDeleteHeader), and a session row deletes just that session. Shared by @@ -3808,11 +3864,19 @@ func (h *Home) maybeAdoptExternalSessions(known []*session.Session) { return } debuglog.Logger.Info("adopting externally-created sessions", "count", len(adopted)) + // Resolve repo roots here, on the worker: these paths were created by + // another process, so session.GetRepoRoot's cache always misses and it + // shells out to git. Doing it in the Update handler would violate the + // no-blocking-I/O-in-Update() rule once per adopted session. + roots := make(map[string]string, len(adopted)) + for _, s := range adopted { + roots[s.ID] = session.GetRepoRoot(s.ProjectPath) + } // Same rendezvous hazard as the suspend sweep's send: program.Send blocks on // Tea's unbuffered channel, and tea.Exec suspends the loop that drains it. // Skipping costs nothing — the next sweep re-finds these rows. if !h.isAttaching.Load() { - h.send(adoptSessionsMsg{sessions: adopted}) + h.send(adoptSessionsMsg{sessions: adopted, repoRoots: roots}) } } @@ -5328,6 +5392,13 @@ func (h *Home) statusWorkerCycle() { h.workerMu.Unlock() if len(sessions) == 0 && len(shells) == 0 { + // Adoption is the one thing that must still run on an empty fleet: it is + // how a session created by another process first appears, and an empty + // sidebar is the likeliest place to be driving from the shell. Every + // other pass below has nothing to work on. + if heavy { + h.maybeAdoptExternalSessions(sessions) + } return } @@ -5531,7 +5602,8 @@ drainPriority: // 5c. Adopt sessions another fleet process created (e.g. `fleet worktree` // from a shell) since the last sweep. Self-throttled; reads SQLite, which is - // why it lives here rather than on the Update loop. + // why it lives here rather than on the Update loop. Also called on the + // empty-fleet early-return path above — the throttle makes that safe. h.maybeAdoptExternalSessions(sessions) // 5. Git+PR refresh used to run here, inline. It now lives on its own @@ -6894,6 +6966,12 @@ type contextMenuTarget struct { sessionID string // session rows repoPath string // checkout headers originKey string // origin headers + // pendingID identifies a "Creating…" phantom row. The context menu never + // sets it — a phantom has no actions — but cursor preservation needs it: + // handleWorkspaceCreate auto-selects the phantom, so without this case the + // cursor sits on an unfindable row for the whole of a worktree create and + // silently drifts if anything is inserted above it. + pendingID string } // find returns the index of the target's row in items, or -1 if it's gone. @@ -6912,6 +6990,10 @@ func (t contextMenuTarget) find(items []SidebarItem) int { if item.IsOriginHeader && item.OriginKey == t.originKey { return i } + case t.pendingID != "": + if item.Pending != nil && item.Pending.ID == t.pendingID { + return i + } } } return -1 @@ -6930,6 +7012,8 @@ func (h *Home) targetForCursor() contextMenuTarget { return contextMenuTarget{repoPath: item.RepoPath} case item.Session != nil: return contextMenuTarget{sessionID: item.Session.ID} + case item.Pending != nil: + return contextMenuTarget{pendingID: item.Pending.ID} } return contextMenuTarget{} }