diff --git a/internal/sessioncmd/session.go b/internal/sessioncmd/session.go index 3291d481..5e68b904 100644 --- a/internal/sessioncmd/session.go +++ b/internal/sessioncmd/session.go @@ -214,6 +214,11 @@ func (r *runtime) sessionInfo(sess store.Session, running, self bool) Session { dir = current } } + return r.sessionInfoIn(sess, dir, running, self) +} + +// sessionInfoIn is sessionInfo with the directory already read. +func (r *runtime) sessionInfoIn(sess store.Session, dir string, running, self bool) Session { return Session{ ID: sess.ID, Name: sess.Name, @@ -390,7 +395,7 @@ func (r *runtime) list(callerID string, opts ListOptions) (SessionList, error) { if err != nil { return SessionList{}, err } - panes, err := r.driver.Panes() + scan, err := r.driver.ScanPanesWithPaths() if err != nil { return SessionList{}, err } @@ -419,8 +424,15 @@ func (r *runtime) list(callerID string, opts ListOptions) (SessionList, error) { more = true continue } - _, running := panes[sess.ID] - sessions = append(sessions, r.sessionInfo(sess, running, callerID != "" && sess.ID == callerID)) + self := callerID != "" && sess.ID == callerID + _, running := scan.PIDs[sess.ID] + // The listing that proved the pane alive also read where it sits, + // so a page of rows costs one tmux call, not one per running row. + if path, ok := scan.Paths[sess.ID]; ok && running { + sessions = append(sessions, r.sessionInfoIn(sess, path, true, self)) + } else { + sessions = append(sessions, r.sessionInfo(sess, running, self)) + } last = keys[i] } granted, err := r.store.ActiveGrants() diff --git a/internal/sessioncmd/tmuxcalls_test.go b/internal/sessioncmd/tmuxcalls_test.go new file mode 100644 index 00000000..739ff6fd --- /dev/null +++ b/internal/sessioncmd/tmuxcalls_test.go @@ -0,0 +1,104 @@ +package sessioncmd + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/usestring/gate-inbox/internal/git" + "github.com/usestring/gate-inbox/internal/tmux" +) + +// countingSessions is h.sessions with every tmux call it makes logged, so a +// test can count the processes a board read forks rather than assert about +// its shape. The shim execs whichever tmux was first on PATH, so the test +// server keeps the isolation tmuxtest put in front of it. +func countingSessions(t *testing.T, h *sessionHarness) (*Sessions, func() []string) { + t.Helper() + real, err := exec.LookPath("tmux") + if err != nil { + t.Skip("tmux not installed") + } + dir := t.TempDir() + log := filepath.Join(dir, "calls.log") + script := "#!/bin/sh\n{ printf '%s\\n' \"$*\"; } >> " + log + "\nexec " + real + " \"$@\"\n" + if err := os.WriteFile(filepath.Join(dir, "tmux"), []byte(script), 0o700); err != nil { + t.Fatalf("shim: %v", err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + driver, err := tmux.NewWithSocket(h.driver.SocketName()) + if err != nil { + t.Fatalf("tmux driver: %v", err) + } + sessions := newSessions(h.sessions.configDir, MCPVocabulary(), func(string) (*tmux.Driver, error) { return driver, nil }, git.New) + return sessions, func() []string { + raw, err := os.ReadFile(log) + if err != nil { + return nil + } + var calls []string + for _, line := range strings.Split(strings.TrimRight(string(raw), "\n"), "\n") { + if line != "" { + calls = append(calls, line) + } + } + return calls + } +} + +// A board-wide listing is what an extension runs on every poll pass, so its +// cost per running row is the cost of the whole board every two seconds. The +// pane directory each row reports comes off the one listing that proves the +// pane alive: one tmux call for the page, however many rows are running. +func TestBoardListReadsEveryDirectoryInOneTmuxCall(t *testing.T) { + h := newSessionHarness(t) + const children = 6 + moved := t.TempDir() + want := map[string]string{h.caller.ID: h.caller.Cwd} + for i := range children { + id := fmt.Sprintf("dirs%03d", i) + command := "cat %s; sleep 60" + if i == 0 { + // A pane that moved since launch reports where it is now, + // not the directory its row was filed with. + command = "cd " + moved + " && cat %s; sleep 60" + } + child := childRunning(t, h, h.caller.ID, id, id, "ready\n", command) + want[child.ID] = child.Cwd + if i == 0 { + want[child.ID] = moved + } + } + sessions, calls := countingSessions(t, h) + list, err := sessions.BoardList(ListOptions{}) + if err != nil { + t.Fatalf("BoardList: %v", err) + } + got := calls() + if len(list.Sessions) != children+1 { + t.Fatalf("listed %d sessions, want %d", len(list.Sessions), children+1) + } + for _, sess := range list.Sessions { + if !sess.Running { + t.Errorf("%s listed as not running", sess.ID) + } + if resolved(t, sess.Directory) != resolved(t, want[sess.ID]) { + t.Errorf("%s directory = %q, want %q", sess.ID, sess.Directory, want[sess.ID]) + } + } + if len(got) != 1 { + t.Fatalf("BoardList over %d running sessions forked tmux %d times, want 1:\n%s", children+1, len(got), strings.Join(got, "\n")) + } +} + +func resolved(t *testing.T, path string) string { + t.Helper() + real, err := filepath.EvalSymlinks(path) + if err != nil { + return path + } + return real +} diff --git a/internal/tmux/panescan_test.go b/internal/tmux/panescan_test.go new file mode 100644 index 00000000..a7c9ab5c --- /dev/null +++ b/internal/tmux/panescan_test.go @@ -0,0 +1,58 @@ +package tmux + +import "testing" + +// A directory name is its occupant's to choose, line breaks included. What +// follows one must not read as a pane line: here it claims a session that +// has no pane, and a pid for one that does. +func TestPaneScanIgnoresALineBreakInADirectory(t *testing.T) { + const mark = "0123abcd " + out := mark + "gi_aaaa 100 /work/one\n" + + mark + "gi_bbbb 200 /evil\ngi_ghost 300 /x\ngi_aaaa 400 /y\n" + + mark + "gi_cccc 500 /work/ends in a space \n" + + mark + "someone-else 600 /home\n" + scan := parsePaneScan(out, mark) + want := map[string]int{"aaaa": 100, "bbbb": 200, "cccc": 500} + if len(scan.PIDs) != len(want) { + t.Fatalf("PIDs = %v, want %v", scan.PIDs, want) + } + for id, pid := range want { + if scan.PIDs[id] != pid { + t.Errorf("PIDs[%s] = %d, want %d", id, scan.PIDs[id], pid) + } + } + if path, ok := scan.Paths["bbbb"]; ok { + t.Errorf("Paths[bbbb] = %q; a directory cut at its line break is no answer", path) + } + if got := scan.Paths["aaaa"]; got != "/work/one" { + t.Errorf("Paths[aaaa] = %q, want /work/one", got) + } + if got := scan.Paths["cccc"]; got != "/work/ends in a space " { + t.Errorf("Paths[cccc] = %q, want the trailing space kept", got) + } +} + +// The board's poll pass scans every two seconds and shows no directory, so +// its scan must not ask tmux for one; the listing that reports sessions does. +func TestOnlyThePathScanReadsDirectories(t *testing.T) { + driver := requireTmux(t) + id := uniqueID("paths") + if err := driver.Create(id, "/tmp", "cat", nil, 80, 24); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { driver.Kill(id) }) + scan, err := driver.ScanPanes() + if err != nil { + t.Fatalf("ScanPanes: %v", err) + } + if _, ok := scan.PIDs[id]; !ok || scan.Paths != nil { + t.Errorf("ScanPanes = pids %v, paths %v; want the pane live and no directories read", scan.PIDs, scan.Paths) + } + scan, err = driver.ScanPanesWithPaths() + if err != nil { + t.Fatalf("ScanPanesWithPaths: %v", err) + } + if _, ok := scan.PIDs[id]; !ok || scan.Paths[id] != "/tmp" { + t.Errorf("ScanPanesWithPaths = pids %v, paths %v; want the pane live in /tmp", scan.PIDs, scan.Paths) + } +} diff --git a/internal/tmux/tmux.go b/internal/tmux/tmux.go index d24c9d6a..835e9bce 100644 --- a/internal/tmux/tmux.go +++ b/internal/tmux/tmux.go @@ -4,6 +4,8 @@ package tmux import ( "context" + "crypto/rand" + "encoding/hex" "errors" "fmt" "hash/fnv" @@ -1897,6 +1899,13 @@ func noServer(out string) bool { // live on a server this pass could not read; Gone holds only the first kind. type PaneScan struct { PIDs map[string]int + // Paths is where each managed session's pane sits now, as + // PaneCurrentPath reads it, from the same listing as PIDs. A caller + // describing every session reads it here rather than asking tmux once + // per session. Adopted sessions are not in it, and only + // ScanPanesWithPaths fills it: the board's poll pass scans every two + // seconds and never reads a directory, so it does not pay for them. + Paths map[string]string // Gone is the adopted sessions whose pane is proven no longer there: // their server produced a pane listing, and the pane was not on it. A // server that could not be listed at all fails the whole scan rather @@ -1939,11 +1948,24 @@ const ScanTimeout = 4 * time.Second func (d *Driver) ScanPanes() (PaneScan, error) { ctx, cancel := context.WithTimeout(context.Background(), ScanTimeout) defer cancel() - return d.scanPanes(ctx) + return d.scanPanes(ctx, false) } -func (d *Driver) scanPanes(ctx context.Context) (PaneScan, error) { - out, err := d.combinedWithin(ctx, d.args("list-panes", "-a", "-F", "#{session_name} #{pane_pid}")) +// ScanPanesWithPaths is ScanPanes with each managed pane's directory read +// in the same listing, for a caller about to report every session's. +func (d *Driver) ScanPanesWithPaths() (PaneScan, error) { + ctx, cancel := context.WithTimeout(context.Background(), ScanTimeout) + defer cancel() + return d.scanPanes(ctx, true) +} + +func (d *Driver) scanPanes(ctx context.Context, withPaths bool) (PaneScan, error) { + format, mark := "#{session_name} #{pane_pid}", "" + if withPaths { + mark = scanMark() + format = mark + format + " #{pane_current_path}" + } + out, err := d.combinedWithin(ctx, d.args("list-panes", "-a", "-F", format)) // A deadline that fired is not an answer about this server, so it is // checked ahead of the no-server reading below: that one treats silence // as "no sessions here", which is the reading a timeout must never get. @@ -1959,8 +1981,22 @@ func (d *Driver) scanPanes(ctx context.Context) (PaneScan, error) { if err != nil { out = nil } + var scan PaneScan + if withPaths { + scan = parsePaneScan(string(out), mark) + } else { + scan = parsePaneLiveness(string(out)) + } + if err := d.adoptedPanes(ctx, scan); err != nil { + return PaneScan{}, err + } + return scan, nil +} + +// parsePaneLiveness reads a listing of session names and pane pids. +func parsePaneLiveness(out string) PaneScan { scan := PaneScan{PIDs: map[string]int{}, Gone: map[string]bool{}} - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + for _, line := range strings.Split(strings.TrimSpace(out), "\n") { name, pidText, ok := strings.Cut(line, " ") if !ok || !managedName(name) { continue @@ -1973,10 +2009,56 @@ func (d *Driver) scanPanes(ctx context.Context) (PaneScan, error) { scan.PIDs[id] = pid } } - if err := d.adoptedPanes(ctx, scan); err != nil { - return PaneScan{}, err + return scan +} + +// scanMark is a fresh prefix for every line of one pane listing. A pane's +// directory is whatever its occupant named it, line breaks included, and +// the text after one would otherwise read as a pane line of tmux's own: +// any session's liveness and pid, written by whoever controls a directory +// name. The occupant cannot know the mark before the listing runs. +func scanMark() string { + var b [8]byte + _, _ = rand.Read(b[:]) + return hex.EncodeToString(b[:]) + " " +} + +// parsePaneScan reads a managed listing whose every line tmux began with +// mark. A line without it is the rest of the directory above, so that +// directory is dropped, cut short as it is; PaneCurrentPath still answers +// for the session the way it always has. +func parsePaneScan(out, mark string) PaneScan { + scan := PaneScan{PIDs: map[string]int{}, Paths: map[string]string{}, Gone: map[string]bool{}} + last := "" + // Only line breaks are trimmed: a trailing space on the last pane's + // path is part of the directory name. + for _, line := range strings.Split(strings.TrimRight(out, "\r\n"), "\n") { + line, marked := strings.CutPrefix(line, mark) + if !marked { + delete(scan.Paths, last) + continue + } + last = "" + name, rest, ok := strings.Cut(line, " ") + if !ok || !managedName(name) { + continue + } + id := strings.TrimPrefix(name, prefix) + if _, taken := scan.PIDs[id]; taken { + continue + } + pidText, path, _ := strings.Cut(rest, " ") + pid, err := strconv.Atoi(pidText) + if err != nil { + continue + } + scan.PIDs[id] = pid + if path = strings.TrimSuffix(path, "\r"); path != "" { + scan.Paths[id] = path + last = id + } } - return scan, nil + return scan } // adoptedPanes adds the adopted sessions to the liveness map. They carry no