From c120342709df14a9be3a2e1a8f0ac2536a318364 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Fri, 15 May 2026 12:41:47 +0100 Subject: [PATCH] Clear TODO backlog: --debug/--profile, codex selection, MCP orientation, perf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add --debug[=DIR] / --profile[=DIR] flags that write run artefacts (patterm.log, events.jsonl, per-child raw PTY captures, CPU + heap + goroutine pprof) to a dir without polluting stdout/stderr. - Strengthen vendor-TUI orientation in three places (MCP initialize.instructions, the spawn_agent tool description, and help('spawning')) to head off codex's habits of poking the Unix socket via perl and shelling out to launch peers — both bypass caller identity and produce orphaned top-level tabs. - Fix click-and-drag text selection from alt-screen TUIs. Host SGR mouse reporting now follows the focused child's screen side instead of being permanently armed; alt-screen TUIs that need mouse re-enable it themselves and the toggle is forwarded. - Move drawSidebar() off the per-PTY-chunk hot path. Long claude session resume was paying a full sidebar rebuild for every scrolled chunk; the chrome ticker now drains a dirty flag at 60 Hz. - Gate the per-chunk Title() CGO poll on a containsOSC scan so codex/ratatui's many SGR-only chunks no longer pay a CGO call each. --- CHANGELOG.md | 52 +++++++++ TODO.md | 12 -- cmd/patterm/main.go | 97 ++++++++++++++++ internal/app/app.go | 110 ++++++++++++++++-- internal/app/debug.go | 155 +++++++++++++++++++++++++ internal/app/host.go | 2 +- internal/app/session.go | 33 +++++- internal/app/viewport_renderer.go | 51 +++++++- internal/app/viewport_renderer_test.go | 30 ++++- internal/mcp/protocol.go | 23 +++- internal/mcp/protocol_test.go | 7 ++ 11 files changed, 536 insertions(+), 36 deletions(-) create mode 100644 internal/app/debug.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f308dcc..aef03f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,59 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Long claude session resume (and codex steady-state rendering) is + noticeably faster. Two costs that scaled per-PTY-chunk are now + deferred or short-circuited: (1) `drawSidebar()` used to run + synchronously for every chunk that scrolled — on a session + resume where every chunk scrolls, this rebuilt the full sidebar + string hundreds of times for a frame that was almost always + cache-equal. The sidebar now signals dirty and the chrome ticker + (60 Hz) handles the repaint. (2) `pumpChild` polled the + emulator's OSC title after every PTY chunk via CGO, even for + chunks (the common case under codex/ratatui) that carry no OSC + bytes at all. The poll is now gated on a containsOSC scan over + the chunk. +- Click-and-drag text selection from alt-screen TUIs (codex in + particular) now works. Patterm used to keep host SGR mouse + reporting armed continuously, which forced the host terminal to + forward every click as an escape sequence and prevented native + selection. The host's mouse mode now follows the focused child's + screen side: primary-screen children keep mouse armed (so wheel + scrollback works), alt-screen children get host mouse disabled by + default. Alt-screen TUIs that need mouse events (vim, less, etc.) + re-enable mouse-mode themselves; the viewport renderer forwards + those toggles to the host while the child is on alt. Leaving alt + re-arms host mouse reporting so wheel scrollback resumes. + ### Added +- MCP `initialize.instructions`, the `spawn_agent` tool description + (visible to LLMs via `tools/list`), and the `help('spawning')` + topic now spell out — in the three places vendor TUIs actually + consult — that the connected `patterm` MCP server is the only + correct way to drive the host. Anti-patterns called out by name: + (a) trying to launch `patterm` / `patterm mcp-stdio` themselves, + (b) piping JSON-RPC into the per-PID Unix socket via `perl` / + `nc` / `socat` / `curl`, and (c) shelling out to `claude` / + `codex` / `opencode` to start a peer. Each of those bypasses + caller identity, so a sub-agent spawned that way reads back as + a stray top-level tab instead of a child under the spawning + agent. Codex was hitting (b) and (c) in practice — this is the + fix. +- `--debug[=DIR]` flag captures detailed run artefacts for offline + analysis: a verbose `patterm.log` (the existing `PATTERM_DEBUG_LOG` + stream), an `events.jsonl` lifecycle log (spawn / exit / idle-state + changes with timestamps), and per-child `.raw` files containing + the raw PTY byte stream. With no argument, the dated subdir + `$XDG_STATE_HOME/patterm/debug/YYYYMMDD-HHMMSS` is used; pass an + explicit path to override. All output goes to files — stdout/stderr + are untouched. +- `--profile[=DIR]` flag captures pprof data for performance work: + `cpu.pprof` (running for the lifetime of the session), plus + `heap.pprof` and `goroutine.pprof` snapshots written on shutdown. + Defaults to `$XDG_STATE_HOME/patterm/profile/YYYYMMDD-HHMMSS`. + All diagnostics (startup, errors) are written to `profile.log` + inside the dir, never to stdout/stderr. - "New Terminal" entry in the command palette spawns a bare interactive `$SHELL` pane (kind `terminal`). Unlike "Run process: …" presets, which are session-persistent and reachable via `restart_process`, diff --git a/TODO.md b/TODO.md index e09c8b8..62018d3 100644 --- a/TODO.md +++ b/TODO.md @@ -1,15 +1,3 @@ -- [ ] Codex seemed to think that it needed to launch patterm itself to get the mcp working -- [ ] I cant click and drag to select text from codex -- [ ] codex uses perl to interact with the socket rather than calling mcp tools - - when it _did_ open a sub claude it opened it as a separate tab rather than a sub-agent. -- [ ] codex rendering is VERY slow - - maybe we need to use diffing rather than rendering the entire viewport for performance -- We should add a --debug and --profile flag, so we can get detailed performance data and full logs of the agent output to be debugged later on. - - I don't mind what format this is in, ideally easy for LLMs to understand -- [ ] Resuming a long claude session takes a couple of seconds for the entire buffer to load in, it looks like it's scrolling down for a couple seconds. - - In raw alacritty this is instant, so there's some sort of performance issue with patterm's terminal emulation. - - # On Hold - [ ] There's a unicode being displayed in opencode [ON HOLD] - Investigated 2026-05-14: patterm passes ghostty grapheme codepoints diff --git a/cmd/patterm/main.go b/cmd/patterm/main.go index 61c009c..cad1935 100644 --- a/cmd/patterm/main.go +++ b/cmd/patterm/main.go @@ -16,7 +16,10 @@ import ( "context" "fmt" "os" + "path/filepath" + "runtime" "runtime/debug" + "runtime/pprof" "time" flag "github.com/spf13/pflag" @@ -49,7 +52,13 @@ func main() { var ( projectDir = flag.String("project", "", "project directory (default $PWD)") showVersion = flag.Bool("version", false, "print version and exit") + debugDir = flag.String("debug", "", "write debug logs + per-child raw PTY output to DIR (auto-picks a dated subdir under $XDG_STATE_HOME/patterm/debug when DIR is omitted)") + profileDir = flag.String("profile", "", "write CPU+heap+goroutine pprof files to DIR (auto-picks a dated subdir under $XDG_STATE_HOME/patterm/profile when DIR is omitted)") ) + // Allow bare `--debug` / `--profile` with no value — pflag treats + // them as boolean-shaped strings, picking a sensible default dir. + flag.Lookup("debug").NoOptDefVal = "auto" + flag.Lookup("profile").NoOptDefVal = "auto" flag.Parse() if *showVersion { @@ -73,15 +82,103 @@ func main() { die("chdir %s: %v", cwd, err) } + resolvedDebug, err := resolveDiagDir(*debugDir, "debug") + if err != nil { + die("debug: %v", err) + } + resolvedProfile, err := resolveDiagDir(*profileDir, "profile") + if err != nil { + die("profile: %v", err) + } + + stopProfile := startProfile(resolvedProfile) + defer stopProfile() + ctx := context.Background() if err := app.Run(ctx, app.Options{ ProjectDir: cwd, ProjectKey: key, + DebugDir: resolvedDebug, }); err != nil { die("%v", err) } } +// resolveDiagDir turns the raw flag value into an absolute directory +// path. Empty string disables the feature. The sentinel "auto" (set by +// NoOptDefVal on bare flags) picks $XDG_STATE_HOME/patterm//. +// Any other value is treated as a literal path. +func resolveDiagDir(raw, kind string) (string, error) { + if raw == "" { + return "", nil + } + if raw == "auto" { + base := os.Getenv("XDG_STATE_HOME") + if base == "" { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + base = filepath.Join(home, ".local", "state") + } + ts := time.Now().Format("20060102-150405") + return filepath.Join(base, "patterm", kind, ts), nil + } + return raw, nil +} + +// startProfile begins a CPU profile under dir and returns a stop func +// that writes heap + goroutine snapshots before flushing the CPU file. +// Returns a no-op stop func when dir is empty. All diagnostics are +// written to /profile.log — never to stdout/stderr — so the TUI +// stays uncluttered. +func startProfile(dir string) func() { + if dir == "" { + return func() {} + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return func() {} + } + logPath := filepath.Join(dir, "profile.log") + plog := func(format string, args ...any) { + f, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) + if err != nil { + return + } + defer f.Close() + fmt.Fprintf(f, format+"\n", args...) + } + cpuPath := filepath.Join(dir, "cpu.pprof") + f, err := os.Create(cpuPath) + if err != nil { + plog("cpu open: %v", err) + return func() {} + } + if err := pprof.StartCPUProfile(f); err != nil { + plog("cpu start: %v", err) + _ = f.Close() + return func() {} + } + plog("profiling started at %s", time.Now().Format(time.RFC3339Nano)) + return func() { + pprof.StopCPUProfile() + _ = f.Close() + // Heap and goroutine snapshots at exit. Heap captures + // steady-state allocation; goroutine catches stragglers + // that didn't get cleaned up. + runtime.GC() + if hf, err := os.Create(filepath.Join(dir, "heap.pprof")); err == nil { + _ = pprof.Lookup("heap").WriteTo(hf, 0) + _ = hf.Close() + } + if gf, err := os.Create(filepath.Join(dir, "goroutine.pprof")); err == nil { + _ = pprof.Lookup("goroutine").WriteTo(gf, 0) + _ = gf.Close() + } + plog("profiling stopped at %s", time.Now().Format(time.RFC3339Nano)) + } +} + func runMCPProxy() { var ( socket = flag.String("socket", "", "path to the running patterm process's MCP socket") diff --git a/internal/app/app.go b/internal/app/app.go index 3d82f03..a4f1034 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -29,6 +29,11 @@ import ( type Options struct { ProjectDir string ProjectKey string + // DebugDir, when non-empty, enables verbose debug logging to + // /patterm.log and per-child raw PTY output capture to + // /.raw. The dir is created if missing. Events + // (spawn / exit / state change) land in /events.jsonl. + DebugDir string } const keyCtrlK byte = 0x0b @@ -77,6 +82,22 @@ func Run(ctx context.Context, opts Options) error { sess := NewSession(opts.ProjectDir, opts.ProjectKey) defer sess.Shutdown() + + // Debug capture: when --debug= is set, write a verbose log + // (patterm.log), per-child raw PTY output (.raw), and a + // JSONL event stream (events.jsonl). Installed before the TUI + // listener so the very first OnChildSpawned / OnPTYOut event + // is captured. + if opts.DebugDir != "" { + dc, err := openDebugCapture(opts.DebugDir) + if err != nil { + return fmt.Errorf("app: debug capture: %w", err) + } + os.Setenv("PATTERM_DEBUG_LOG", dc.LogPath()) + sess.Subscribe(dc) + defer dc.Close() + logf("debug capture enabled at %s", opts.DebugDir) + } // Snapshot persisted processes BEFORE attaching the store: Spawn // mints fresh ids, so the old records would otherwise linger // alongside the new ones. Drop them up front; the restore loop @@ -248,11 +269,18 @@ func Run(ctx context.Context, opts Options) error { case <-st.chromeWake: case <-ticker.C: } - if !st.chromeDirty.Swap(false) { + chromeChanged := st.chromeDirty.Swap(false) + sidebarChanged := st.sidebarDirty.Swap(false) + if !chromeChanged && !sidebarChanged { continue } - st.drawTabBar() - st.drawStatusLine() + if chromeChanged { + st.drawTabBar() + st.drawStatusLine() + } + if sidebarChanged { + st.drawSidebar() + } } }() @@ -372,7 +400,14 @@ type uiState struct { // sensitive paths (owner flip, attention, trust, focus change) // continue to call drawStatusLine / drawTabBar synchronously. chromeDirty atomic.Bool - chromeWake chan struct{} + // sidebarDirty defers sidebar repaints off the per-chunk hot path + // in the same way. A long claude session resume — where every PTY + // chunk scrolls the viewport — used to call drawSidebar() + // synchronously per chunk, which dominated the resume's wall time + // (hundreds of full-sidebar rebuilds for a frame that was almost + // always cache-equal). + sidebarDirty atomic.Bool + chromeWake chan struct{} // padsCacheMu guards the cached scratchpad listing. The sidebar // and palette/sidebar nav helpers read it on every chunk-driven @@ -415,14 +450,18 @@ func (st *uiState) focusProcess(processID string) { return } layout := st.layoutSnapshot() + onAlt := childIsOnAlt(c) st.mu.Lock() leavingPad := st.focusedPad != "" st.focusedPad = "" st.focusedID = c.ID st.focusedName = c.DisplayName() st.updateActiveAgentLocked(c) - st.renderer = newViewportRenderer(layout) + r := newViewportRenderer(layout) + r.SetChildOnAlt(onAlt) + st.renderer = r st.mu.Unlock() + st.syncHostMouseForChild(onAlt) // Wipe whatever the previous focus (PTY child or pad view) left in // the viewport before painting the new child's snapshot. if leavingPad { @@ -434,6 +473,41 @@ func (st *uiState) focusProcess(processID string) { st.drawStatusLine() } +// childIsOnAlt reports whether the child's emulator is currently on +// its alternate screen. Returns false if the emulator is gone or the +// query fails. +func childIsOnAlt(c *Child) bool { + if c == nil { + return false + } + em := c.Emulator() + if em == nil { + return false + } + sc, err := em.ActiveScreen() + if err != nil { + return false + } + return sc == vt.ScreenAlternate +} + +// syncHostMouseForChild emits the host mouse-reporting toggle that +// matches a newly-focused child's screen side. Primary-screen children +// want host mouse armed so the wheel drives inline scrollback; alt- +// screen children get host mouse disabled by default so click-and-drag +// selection works. Alt-screen TUIs that need mouse (vim, ranger, etc.) +// re-enable it themselves, and the viewport renderer forwards those +// toggles back to the host. +func (st *uiState) syncHostMouseForChild(onAlt bool) { + st.outMu.Lock() + defer st.outMu.Unlock() + if onAlt { + _, _ = os.Stdout.WriteString("\x1b[?1000l\x1b[?1006l") + } else { + _, _ = os.Stdout.WriteString("\x1b[?1000h\x1b[?1006h") + } +} + // focusScratchpad shifts focus to a scratchpad. The main viewport // renders the pad's text instead of any child PTY; PTY output for the // previously focused child is dropped until focus moves back to a @@ -572,12 +646,14 @@ func (st *uiState) scratchpadsChanged() { // OnChildSpawned auto-focuses the new child. func (st *uiState) OnChildSpawned(c *Child) { layout := st.layoutSnapshot() + onAlt := childIsOnAlt(c) st.mu.Lock() st.focusedPad = "" st.focusedID = c.ID st.focusedName = c.DisplayName() st.updateActiveAgentLocked(c) renderer := newViewportRenderer(layout) + renderer.SetChildOnAlt(onAlt) st.renderer = renderer palOpen := st.palette != nil if palOpen { @@ -611,6 +687,7 @@ func (st *uiState) OnChildSpawned(c *Child) { st.outMu.Unlock() } + st.syncHostMouseForChild(onAlt) st.moveToViewportOrigin() st.drawTabBar() st.drawSidebar() @@ -760,9 +837,14 @@ func (st *uiState) OnPTYOut(childID string, chunk []byte) { st.chromeCacheMu.Lock() st.sidebarCache = "" st.chromeCacheMu.Unlock() - // Scrolled chunks can clobber the sidebar columns; repaint - // synchronously so the gap fills before the next chunk lands. - st.drawSidebar() + // Defer the sidebar repaint to the chrome ticker. On a long + // session resume every PTY chunk scrolls, and a synchronous + // drawSidebar() per chunk dominates wall time even when the + // frame ends up cache-equal — the rebuild work is unconditional. + // The chrome ticker drains the dirty flag at ~60 Hz, so the + // visible gap a scrolled chunk can leave in the sidebar columns + // is bounded by one frame. + st.markSidebarDirty() } // Defer the tab bar + status line repaint to the chrome ticker. // The cached frame already short-circuits the wire write, but @@ -866,6 +948,18 @@ func (st *uiState) markChromeDirty() { } } +// markSidebarDirty schedules a sidebar repaint on the next ticker +// frame. Hot path — every scrolled PTY chunk lands here. Synchronous +// repaints from latency-sensitive sites (spawn, exit, focus, state +// change, trust) keep calling drawSidebar directly. +func (st *uiState) markSidebarDirty() { + st.sidebarDirty.Store(true) + select { + case st.chromeWake <- struct{}{}: + default: + } +} + func (st *uiState) invalidateChromeCache() { st.chromeCacheMu.Lock() st.tabBarCache = "" diff --git a/internal/app/debug.go b/internal/app/debug.go new file mode 100644 index 0000000..fad4b0b --- /dev/null +++ b/internal/app/debug.go @@ -0,0 +1,155 @@ +package app + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" + "time" +) + +// debugCapture implements ChildEventListener and writes structured +// debug artefacts under a single directory: +// +// - patterm.log — the existing logf() stream +// - events.jsonl — one JSON object per lifecycle event +// - .raw — raw PTY bytes for each child, by id+name +// +// The capture is installed only when --debug= is set, so default +// runs pay nothing. +type debugCapture struct { + dir string + logPath string + + mu sync.Mutex + events *os.File + rawByID map[string]*os.File +} + +func openDebugCapture(dir string) (*debugCapture, error) { + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, err + } + logPath := filepath.Join(dir, "patterm.log") + // Truncate-style fresh log per run is friendlier for grep'ing one + // session. The existing logf opens O_APPEND though, so concurrent + // runs against the same dir would interleave — that's on the user. + if f, err := os.Create(logPath); err != nil { + return nil, err + } else { + _ = f.Close() + } + ev, err := os.Create(filepath.Join(dir, "events.jsonl")) + if err != nil { + return nil, err + } + dc := &debugCapture{ + dir: dir, + logPath: logPath, + events: ev, + rawByID: make(map[string]*os.File), + } + dc.writeEvent("session_start", map[string]any{ + "time": time.Now().Format(time.RFC3339Nano), + "pid": os.Getpid(), + }) + return dc, nil +} + +func (d *debugCapture) LogPath() string { return d.logPath } + +func (d *debugCapture) Close() error { + d.mu.Lock() + defer d.mu.Unlock() + d.writeEventLocked("session_end", map[string]any{ + "time": time.Now().Format(time.RFC3339Nano), + }) + for _, f := range d.rawByID { + _ = f.Close() + } + d.rawByID = nil + if d.events != nil { + _ = d.events.Close() + d.events = nil + } + return nil +} + +func (d *debugCapture) OnChildSpawned(c *Child) { + d.writeEvent("child_spawned", map[string]any{ + "time": time.Now().Format(time.RFC3339Nano), + "id": c.ID, + "name": c.Name, + "kind": string(c.Kind), + "parent_id": c.ParentID, + "preset": c.PresetRef, + "argv": c.Argv, + }) +} + +func (d *debugCapture) OnChildExited(c *Child) { + d.writeEvent("child_exited", map[string]any{ + "time": time.Now().Format(time.RFC3339Nano), + "id": c.ID, + "name": c.Name, + "exit_code": c.ExitCode(), + }) + d.mu.Lock() + defer d.mu.Unlock() + if f, ok := d.rawByID[c.ID]; ok { + _ = f.Close() + delete(d.rawByID, c.ID) + } +} + +func (d *debugCapture) OnChildStateChanged(id string, state IdleState) { + d.writeEvent("child_state", map[string]any{ + "time": time.Now().Format(time.RFC3339Nano), + "id": id, + "state": string(state), + }) +} + +func (d *debugCapture) OnPTYOut(childID string, chunk []byte) { + if len(chunk) == 0 { + return + } + d.mu.Lock() + defer d.mu.Unlock() + f, ok := d.rawByID[childID] + if !ok { + path := filepath.Join(d.dir, childID+".raw") + nf, err := os.Create(path) + if err != nil { + return + } + f = nf + d.rawByID[childID] = nf + } + // Listener contract: don't retain chunk past return. Writing now + // is fine; the slice's backing buffer is reused for the next read + // only after this listener chain completes. + _, _ = f.Write(chunk) +} + +func (d *debugCapture) writeEvent(kind string, fields map[string]any) { + d.mu.Lock() + defer d.mu.Unlock() + d.writeEventLocked(kind, fields) +} + +func (d *debugCapture) writeEventLocked(kind string, fields map[string]any) { + if d.events == nil { + return + } + if fields == nil { + fields = map[string]any{} + } + fields["event"] = kind + enc, err := json.Marshal(fields) + if err != nil { + return + } + _, _ = fmt.Fprintln(d.events, string(enc)) +} diff --git a/internal/app/host.go b/internal/app/host.go index 3917431..9c77990 100644 --- a/internal/app/host.go +++ b/internal/app/host.go @@ -1111,7 +1111,7 @@ func helpFor(topic string) mcp.HelpResponse { case "spawning": return mcp.HelpResponse{ Topic: "spawning", - Content: "spawn_agent launches another vendor LLM CLI as a sub-agent (orchestrator only). spawn_process(kind: command, preset: …) starts a stored command; spawn_process(kind: terminal) opens a shell. Command presets need trust the first time — you'll get needs_trust until the human accepts. Whatever you spawn is yours to clean up — see help('lifecycle').", + Content: "spawn_agent launches another vendor LLM CLI as a sub-agent (orchestrator only). spawn_process(kind: command, preset: …) starts a stored command; spawn_process(kind: terminal) opens a shell. Command presets need trust the first time — you'll get needs_trust until the human accepts. ANTI-PATTERNS: do not shell out to `claude` / `codex` / `opencode` (or any other agent CLI) yourself, and do not pipe JSON-RPC into patterm's Unix socket via perl / nc / socat / curl. Either path bypasses caller-identity and the new agent reads back as a stray top-level tab instead of your child — call spawn_agent through the MCP transport you were initialised on. Whatever you spawn is yours to clean up — see help('lifecycle').", RelatedTools: []string{"spawn_agent", "spawn_process", "start_process", "restart_process", "close_process"}, } case "lifecycle": diff --git a/internal/app/session.go b/internal/app/session.go index dc90ea1..6e1699c 100644 --- a/internal/app/session.go +++ b/internal/app/session.go @@ -397,12 +397,15 @@ func (s *Session) pumpChild(c *Child, runID uint64) { } // OSC 0/2 title updates ride on the same byte stream as // the rest of the output. Polling the emulator after each - // Write is cheap (one cgo call returning a borrowed - // string) and lets the classifier treat title changes as - // an activity signal — even when the title isn't visible - // in the rendered grid. - if t, terr := em.Title(); terr == nil { - c.recordTitle(t) + // chunk is cheap on its own (one CGO call) but codex/ + // ratatui sends so many small chunks that the per-chunk + // CGO cost becomes measurable. Skip the Title poll when + // the chunk doesn't carry an OSC start byte at all; the + // title can only change on chunks that include one. + if containsOSC(chunk) { + if t, terr := em.Title(); terr == nil { + c.recordTitle(t) + } } } c.recordWrite(chunk) @@ -679,6 +682,24 @@ func (s *Session) Shutdown() { } } +// containsOSC reports whether chunk holds a sequence that could begin +// an OSC. OSC starts as ESC ] (0x1b 0x5d) or the bare C1 ] (0x9d), +// so a chunk without either cannot have changed the emulator's OSC +// title state. Used to short-circuit the per-chunk Title() poll from +// pumpChild, which otherwise pays a CGO call for every chunk even +// when codex/ratatui is just emitting SGR-styled output. +func containsOSC(chunk []byte) bool { + for i, b := range chunk { + if b == 0x9d { + return true + } + if b == 0x1b && i+1 < len(chunk) && chunk[i+1] == ']' { + return true + } + } + return false +} + func logf(format string, args ...any) { if os.Getenv("PATTERM_DEBUG_LOG") == "" { return diff --git a/internal/app/viewport_renderer.go b/internal/app/viewport_renderer.go index f85a0ef..6625ffc 100644 --- a/internal/app/viewport_renderer.go +++ b/internal/app/viewport_renderer.go @@ -33,6 +33,14 @@ type viewportRenderer struct { // cache so the next drawSidebar repaints over the clobber. scrolled bool + // childOnAlt tracks whether the focused child has entered its + // alternate screen (via ?47 / ?1047 / ?1049). Used to gate mouse- + // tracking-mode forwarding to the host: filter on primary so + // patterm's wheel-scrollback stays armed, forward on alt so codex + // (which disables mouse) lets the user select text and vim (which + // enables it) still gets mouse events. + childOnAlt bool + // skipUTF8 is set when the current multi-byte UTF-8 character started // past the viewport's right edge. The starter byte was dropped, so // the remaining continuation bytes must be dropped too instead of @@ -65,6 +73,16 @@ func newViewportRenderer(l terminalLayout) *viewportRenderer { return vr } +// SetChildOnAlt seeds the renderer's view of the focused child's screen +// side. Used when a new renderer is constructed for an already-running +// child whose alt-screen transition we missed, so subsequent mouse-mode +// toggles are filtered/forwarded according to the right side. +func (vr *viewportRenderer) SetChildOnAlt(onAlt bool) { + vr.mu.Lock() + defer vr.mu.Unlock() + vr.childOnAlt = onAlt +} + func (vr *viewportRenderer) SetLayout(l terminalLayout) { vr.mu.Lock() defer vr.mu.Unlock() @@ -236,15 +254,36 @@ func (vr *viewportRenderer) emitCSI() { return } if isAltScreenMode(params) { + // Track the child's screen side so we know whether to filter + // or forward subsequent mouse-mode toggles. Entering alt + // disables host mouse reporting by default so codex (and + // any other alt-screen TUI that doesn't request mouse) + // allows the user to click-drag to select text. Alt-screen + // TUIs that want mouse (vim, less with -X) re-enable it + // via ?1000h after switching to alt — the forwarder below + // passes that through. Leaving alt re-arms host mouse for + // primary-screen wheel-scrollback. + wasAlt := vr.childOnAlt + vr.childOnAlt = final == 'h' + if !wasAlt && vr.childOnAlt { + vr.pending.WriteString("\x1b[?1000l\x1b[?1006l") + } + if wasAlt && !vr.childOnAlt { + vr.pending.WriteString("\x1b[?1000h\x1b[?1006h") + } return } if isMouseTrackingMode(params) { - // Patterm owns mouse reporting on the host so wheel events keep - // flowing for scroll-viewport. The child's own emulator still - // observes the mode set/reset (it processes the same bytes we - // hand to ghostty_terminal_vt_write), so we know whether the - // child wants mouse input — we just don't let it disarm our - // host listener. + // On the child's primary screen patterm owns mouse reporting so + // wheel events keep flowing for in-pane scrollback — drop the + // child's toggle. On the alt screen the child should be free + // to enable mouse (vim, less) or disable it (codex); we forward + // the toggle to the host so click-and-drag selection works for + // alt-screen TUIs that don't want mouse, and mouse-aware ones + // still see the events they need. + if vr.childOnAlt { + vr.pending.Write(vr.buf) + } return } } diff --git a/internal/app/viewport_renderer_test.go b/internal/app/viewport_renderer_test.go index 0ca2081..d285f34 100644 --- a/internal/app/viewport_renderer_test.go +++ b/internal/app/viewport_renderer_test.go @@ -24,8 +24,36 @@ func TestViewportRendererShiftsCursor(t *testing.T) { func TestViewportRendererSwallowsAltScreenToggles(t *testing.T) { vr := newViewportRenderer(newTerminalLayout(120, 40)) got := string(vr.Render([]byte("a\x1b[?1049hb\x1b[?1049lc"))) + // The ?1049h/l toggles themselves must not reach the host (patterm + // owns its own alt screen). On the transition we re-sync host mouse + // reporting so codex (which doesn't request mouse) lets the user + // drag-select; leaving alt re-arms it for primary-screen wheel + // scrollback. + want := "a\x1b[?1000l\x1b[?1006lb\x1b[?1000h\x1b[?1006hc" + if got != want { + t.Fatalf("alt-screen toggles: got %q want %q", got, want) + } +} + +func TestViewportRendererMouseTrackingFilteredOnPrimary(t *testing.T) { + vr := newViewportRenderer(newTerminalLayout(120, 40)) + got := string(vr.Render([]byte("a\x1b[?1000lb\x1b[?1000hc"))) if got != "abc" { - t.Fatalf("alt-screen toggles: got %q", got) + t.Fatalf("mouse mode on primary should be filtered: got %q", got) + } +} + +func TestViewportRendererMouseTrackingForwardedOnAlt(t *testing.T) { + vr := newViewportRenderer(newTerminalLayout(120, 40)) + // Enter alt; subsequent mouse-mode toggles should reach the host so + // alt-screen TUIs (vim, less) can run with mouse on, and selection- + // using ones (codex) stay with mouse off. + got := string(vr.Render([]byte("\x1b[?1049h\x1b[?1000lx\x1b[?1000hy"))) + if !strings.Contains(got, "\x1b[?1000l") { + t.Fatalf("alt-screen mouse disable should reach host: %q", got) + } + if !strings.Contains(got, "\x1b[?1000h") { + t.Fatalf("alt-screen mouse enable should reach host: %q", got) } } diff --git a/internal/mcp/protocol.go b/internal/mcp/protocol.go index 77367b1..07e33aa 100644 --- a/internal/mcp/protocol.go +++ b/internal/mcp/protocol.go @@ -27,6 +27,24 @@ var serverInfo = map[string]any{ "version": "0.1.0", } +// serverInstructions is returned in the MCP `initialize` response. MCP +// clients show this to the underlying LLM as context for how to use +// the server. Failure modes we've seen and want to head off: +// - The agent assumes patterm is something it has to launch (running +// `patterm` or `patterm mcp-stdio` from its own shell). It's +// already attached — it just calls the tools. +// - The agent reaches for shell tools (perl / nc / socat / curl) to +// poke patterm's Unix socket directly. That socket connection +// carries no caller identity, so any sub-agent the agent spawns +// that way ends up as a stray top-level tab instead of a child +// under the spawning agent. Always go through the MCP tools. +// - The agent shells out to `claude` / `codex` / `opencode` to start +// a peer instead of calling `spawn_agent`. Those peers won't show +// up as sub-agents and won't be tied into the patterm lifecycle. +// +// Keep this short — clients vary in how much they surface to the LLM. +const serverInstructions = "You are already running INSIDE patterm; the `patterm` MCP server is connected over the same stdio MCP transport you use for any other MCP server. Use the MCP tools you see in tools/list — do NOT (a) try to launch `patterm` or `patterm mcp-stdio` yourself, (b) poke the Unix socket through perl / nc / socat / curl, or (c) shell out to `claude` / `codex` / `opencode` to start a peer. Any of those bypasses caller-identity and the new agent will land as a stray top-level tab instead of a child under you. Start with `whoami` for your role and the full tool list, then `help('topics')` for orientation. `spawn_agent` is the only correct way to start a sub-agent; `spawn_process` is for non-LLM commands; `list_processes` / `get_process_output` inspect them; `send_input` / `send_message` drive them. Whatever you spawn is yours to `close_process` when done." + // toolDescriptor is the shape returned by `tools/list`. inputSchema is // a JSON Schema object — we provide a minimal `{type: "object"}` schema // for each tool, which lets MCP clients accept arbitrary arguments and @@ -88,7 +106,7 @@ func toolCatalog() []toolDescriptor { return []toolDescriptor{ { Name: "spawn_agent", - Description: "Spawn a sub-agent from an agent preset and optionally seed it with initial instructions. Caller owns lifecycle: when the sub-agent's work is done (it reports back via send_message, or you no longer need it), call close_process on its process_id to free the pane and tear down the PTY. See help('lifecycle').", + Description: "Spawn a sub-agent from an agent preset and optionally seed it with initial instructions. This is the ONLY correct way to start a sub-agent under you — do not shell out to `claude` / `codex` / `opencode` and do not poke patterm's Unix socket via perl / nc / socat. Either bypasses caller identity and the new agent lands as a stray top-level tab instead of your child. Caller owns lifecycle: when the sub-agent's work is done (it reports back via send_message, or you no longer need it), call close_process on its process_id to free the pane and tear down the PTY. See help('spawning') and help('lifecycle').", InputSchema: objectSchema(map[string]any{ "agent": stringProp("Preset name (e.g. \"claude\", \"codex\")."), "agent_instructions": stringProp("Initial prompt typed into the agent after it's ready."), @@ -377,7 +395,8 @@ func (s *Server) handleProtocolMethod(callerID, method string, params json.RawMe "capabilities": map[string]any{ "tools": map[string]any{"listChanged": false}, }, - "serverInfo": serverInfo, + "serverInfo": serverInfo, + "instructions": serverInstructions, } return result, true, 0, "", nil diff --git a/internal/mcp/protocol_test.go b/internal/mcp/protocol_test.go index 0d958c4..c025bf2 100644 --- a/internal/mcp/protocol_test.go +++ b/internal/mcp/protocol_test.go @@ -36,6 +36,13 @@ func TestInitializeReturnsCapabilities(t *testing.T) { if caps["tools"] == nil { t.Fatalf("tools capability missing: %+v", caps) } + // patterm-specific orientation: clients show this to the underlying + // LLM, so it's our primary hook for steering vendor TUIs (codex in + // particular) toward the MCP tool surface instead of shell-ing out. + instructions, ok := parsed.Result["instructions"].(string) + if !ok || instructions == "" { + t.Fatalf("instructions missing or wrong type: %+v", parsed.Result) + } } func TestInitializedNotificationSuppressesResponse(t *testing.T) {