From cc4bf9e9045a7a3c485e5b08459011a6808aaa4b Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Thu, 14 May 2026 20:51:37 +0100 Subject: [PATCH] Simplify session lifecycle and MCP cleanup --- CHANGELOG.md | 25 +++++ internal/app/app.go | 62 +++++++----- internal/app/child.go | 66 +++++++++---- internal/app/host.go | 93 +++++++++++------- internal/app/launch.go | 64 ++++++------ internal/app/palette.go | 4 +- internal/app/session.go | 155 +++++++++++++++--------------- internal/app/sidebar.go | 4 +- internal/app/tabbar.go | 2 +- internal/harness/env.go | 12 ++- internal/harness/recorder.go | 13 ++- internal/harness/session.go | 42 +++++--- internal/mcp/protocol.go | 8 +- internal/mcp/protocol_test.go | 16 +++ internal/mcp/tools.go | 118 ++++++++++++++--------- internal/scratchpad/scratchpad.go | 10 ++ 16 files changed, 439 insertions(+), 255 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d380835..92b82ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,16 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). available macros. ### Changed +- Focus, lifecycle, and repaint paths now capture terminal layout before + taking UI state locks, reducing resize-time deadlock risk without + changing visible behavior. +- Focused PTY output no longer rebuilds the scratchpad sidebar on every + chunk. The sidebar still repaints on focus/lifecycle/resize changes + and when child output scrolls over the chrome, but normal output avoids + repeated scratchpad disk reads. +- Harness scenario tests now reuse one built patterm binary per test run + and write failure artifacts under a repo-rooted, collision-proof + directory. - Palette ordering: open agents/processes (`Switch to …`) now appear above the option to spawn new ones, with kill entries pushed down toward the end of the list. @@ -45,6 +55,21 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). extra row of viewport. ### Fixed +- Agent MCP injection no longer writes unused config files for inline + injection modes (`cli_override` / `config_env`). File-backed injection + modes track their generated paths and clean them up when the child is + closed, exits, or patterm shuts down. +- MCP `tools/list` descriptions now match the runtime argument values + for process output and pattern waiting, and typed invalid-argument + errors map to JSON-RPC invalid params instead of generic internal + errors. +- Scratchpad writes and appends are serialized inside a patterm process + so `expected_revision` checks cannot race another local scratchpad + mutation. +- The sidebar scratchpad list now refreshes after MCP + `scratchpad_write` and `scratchpad_append` calls. +- UI chrome now reads renamed child display names through the + `DisplayName` accessor, avoiding races with `rename_process`. - Child processes spawned by an orchestrator are now killed when the orchestrator dies, recursively through the tree. Applies whether the parent was closed via MCP, Ctrl-C'd by the user, or exited on its diff --git a/internal/app/app.go b/internal/app/app.go index eb27e18..b3f4d9f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -105,6 +105,7 @@ func Run(ctx context.Context, opts Options) error { host.attention = st host.focus = st host.prompter = st + host.scratch = st st.lastExit.Store(-1) sess.Subscribe(st) @@ -241,10 +242,10 @@ type uiState struct { // usually doesn't change between calls — caching the rendered // output and skipping a write when it matches eliminates the // flicker (especially in the sidebar's session tree). - chromeCacheMu sync.Mutex - tabBarCache string - sidebarCache string - statusLineCache string + chromeCacheMu sync.Mutex + tabBarCache string + sidebarCache string + statusLineCache string lastExit atomic.Int32 } @@ -278,10 +279,11 @@ func (st *uiState) focusProcess(processID string) { if c == nil { return } + layout := st.layoutSnapshot() st.mu.Lock() st.focusedID = c.ID st.focusedName = c.DisplayName() - st.renderer = newViewportRenderer(st.layoutSnapshot()) + st.renderer = newViewportRenderer(layout) st.mu.Unlock() st.repaintFocused() st.drawTabBar() @@ -297,7 +299,7 @@ func (st *uiState) notifyAttention(childID, reason string) { c := st.sess.FindChild(childID) name := childID if c != nil { - name = c.Name + name = c.DisplayName() } st.mu.Lock() st.attentionText = fmt.Sprintf("attention: %s — %s", name, reason) @@ -306,12 +308,20 @@ func (st *uiState) notifyAttention(childID, reason string) { st.drawStatusLine() } +func (st *uiState) scratchpadsChanged() { + st.chromeCacheMu.Lock() + st.sidebarCache = "" + st.chromeCacheMu.Unlock() + st.drawSidebar() +} + // OnChildSpawned auto-focuses the new child. func (st *uiState) OnChildSpawned(c *Child) { + layout := st.layoutSnapshot() st.mu.Lock() st.focusedID = c.ID - st.focusedName = c.Name - renderer := newViewportRenderer(st.layoutSnapshot()) + st.focusedName = c.DisplayName() + renderer := newViewportRenderer(layout) st.renderer = renderer palOpen := st.palette != nil if palOpen { @@ -343,17 +353,19 @@ func (st *uiState) OnChildSpawned(c *Child) { // focused child. func (st *uiState) OnChildExited(c *Child) { st.lastExit.Store(int32(c.ExitCode())) + layout := st.layoutSnapshot() + renderEmpty := false st.mu.Lock() if c.ID == st.focusedID { next := firstRunningTopLevel(st.sess.Children()) if next == nil { st.focusedID = "" st.focusedName = "" - st.renderEmptyStateLocked() + renderEmpty = true } else { st.focusedID = next.ID - st.focusedName = next.Name - st.renderer = newViewportRenderer(st.layoutSnapshot()) + st.focusedName = next.DisplayName() + st.renderer = newViewportRenderer(layout) } } if st.palette != nil { @@ -362,8 +374,12 @@ func (st *uiState) OnChildExited(c *Child) { st.palette.rebuild() st.renderPaletteLocked() } + repaint := st.focusedID != "" st.mu.Unlock() - if st.focusedID != "" { + if renderEmpty { + st.renderEmptyState() + } + if repaint { st.repaintFocused() } st.drawTabBar() @@ -417,13 +433,16 @@ func (st *uiState) OnPTYOut(childID string, chunk []byte) { // contained one of those escapes; when set, drop the sidebar cache // so the next drawSidebar repaints over the clobber instead of // hitting the cache and leaving the gap visible. - if renderer.TookScrollAction() { + scrolled := renderer.TookScrollAction() + if scrolled { st.chromeCacheMu.Lock() st.sidebarCache = "" st.chromeCacheMu.Unlock() } st.drawTabBar() - st.drawSidebar() + if scrolled { + st.drawSidebar() + } st.drawStatusLine() } @@ -559,15 +578,9 @@ func (st *uiState) drawStatusLine() { // renderEmptyState is the SPEC §4 blank-canvas hint. Drawn whenever no // child is focused. func (st *uiState) renderEmptyState() { - st.mu.Lock() - defer st.mu.Unlock() - st.renderEmptyStateLocked() -} - -func (st *uiState) renderEmptyStateLocked() { + layout := st.layoutSnapshot() st.outMu.Lock() defer st.outMu.Unlock() - layout := st.layoutSnapshot() line := "Press Ctrl-K to spawn an agent or process" row := int(layout.mainTop) + (int(layout.childRows()) / 2) col := int(layout.mainLeft) + ((int(layout.childCols()) - len(line)) / 2) @@ -897,10 +910,11 @@ func (st *uiState) closePalette(action paletteAction) { st.repaintFocused() return } + layout := st.layoutSnapshot() st.mu.Lock() st.focusedID = action.childID - st.focusedName = c.Name - st.renderer = newViewportRenderer(st.layoutSnapshot()) + st.focusedName = c.DisplayName() + st.renderer = newViewportRenderer(layout) st.mu.Unlock() st.repaintFocused() st.drawTabBar() @@ -953,10 +967,10 @@ func (st *uiState) flashTransient(msg string) { // emulator grid; the padded snapshot is the source of truth for visible // cells. func (st *uiState) repaintFocused() { + layout := st.layoutSnapshot() st.mu.Lock() id := st.focusedID renderer := st.renderer - layout := st.layoutLocked() st.mu.Unlock() if id == "" { st.renderEmptyState() diff --git a/internal/app/child.go b/internal/app/child.go index 8689c5e..d1f5a32 100644 --- a/internal/app/child.go +++ b/internal/app/child.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "errors" "fmt" + "os" "os/exec" "regexp" "strconv" @@ -88,6 +89,7 @@ type Child struct { ptyMu sync.RWMutex pty *pkgpty.PTY em *vt.GhosttyEmulator + runID uint64 status atomic.Pointer[ChildStatus] exitCode atomic.Int32 @@ -115,6 +117,10 @@ type Child struct { // portsMu guards ports. Best-effort port detection: regex on stream. portsMu sync.Mutex ports []PortSighting + + cleanupMu sync.Mutex + cleanupPaths []string + restarting atomic.Bool } // PortSighting is one entry returned by get_process_ports. @@ -126,10 +132,7 @@ type PortSighting struct { const ringCap = 1 << 20 // 1 MiB per SPEC §5 -// newChildEntry builds the in-memory Child record but does NOT start a -// PTY. Used so command entries can exist in the `stopped` state from the -// moment they're created. Agents and terminals call newChild() which -// chains newChildEntry + startPTY for the initial run. +// newChildEntry builds the in-memory Child record but does NOT start a PTY. func newChildEntry(id, name string, kind ChildKind, argv, env []string, parentID, workDir, presetRef string) *Child { c := &Child{ ID: id, @@ -156,25 +159,14 @@ func newChildEntry(id, name string, kind ChildKind, argv, env []string, parentID return c } -func newChild(id, name string, kind ChildKind, argv, env []string, cols, rows uint16, parentID, workDir, presetRef string) (*Child, error) { - if len(argv) == 0 { - return nil, errors.New("child: empty argv") - } - c := newChildEntry(id, name, kind, argv, env, parentID, workDir, presetRef) - if err := c.startPTY(cols, rows); err != nil { - return nil, err - } - return c, nil -} - // startPTY (re)builds the emulator + PTY for this entry. Called by // newChild on initial spawn and by Restart on subsequent runs. The // status transitions stopped/exited → starting → running. On error the // entry returns to errored. -func (c *Child) startPTY(cols, rows uint16) error { +func (c *Child) startPTY(cols, rows uint16) (uint64, error) { em, err := vt.NewGhosttyEmulator(cols, rows) if err != nil { - return fmt.Errorf("child %s emulator: %w", c.ID, err) + return 0, fmt.Errorf("child %s emulator: %w", c.ID, err) } starting := StatusStarting c.status.Store(&starting) @@ -183,12 +175,14 @@ func (c *Child) startPTY(cols, rows uint16) error { em.Close() errored := StatusErrored c.status.Store(&errored) - return fmt.Errorf("child %s pty: %w", c.ID, err) + return 0, fmt.Errorf("child %s pty: %w", c.ID, err) } em.OnWritePTY(func(b []byte) { _, _ = p.Write(b) }) c.ptyMu.Lock() + c.runID++ + runID := c.runID c.pty = p c.em = em c.ptyMu.Unlock() @@ -196,7 +190,7 @@ func (c *Child) startPTY(cols, rows uint16) error { c.status.Store(&running) c.exitCode.Store(-1) c.lastWriteNS.Store(0) - return nil + return runID, nil } // IsLive reports whether the PTY is currently attached and running. @@ -222,6 +216,21 @@ func (c *Child) Emulator() *vt.GhosttyEmulator { return c.em } +func (c *Child) ptyForRun(runID uint64) *pkgpty.PTY { + c.ptyMu.RLock() + defer c.ptyMu.RUnlock() + if c.runID != runID { + return nil + } + return c.pty +} + +func (c *Child) isCurrentRun(runID uint64) bool { + c.ptyMu.RLock() + defer c.ptyMu.RUnlock() + return c.runID == runID +} + // DisplayName is the rename_process-aware accessor for Name. Callers // that read Name directly skip the lock; the field is still safe to // read because Go strings are immutable, but DisplayName signals intent. @@ -425,6 +434,25 @@ func (c *Child) teardownPTY() { } } +func (c *Child) AddCleanupPath(path string) { + if path == "" { + return + } + c.cleanupMu.Lock() + c.cleanupPaths = append(c.cleanupPaths, path) + c.cleanupMu.Unlock() +} + +func (c *Child) cleanupOwnedPaths() { + c.cleanupMu.Lock() + paths := c.cleanupPaths + c.cleanupPaths = nil + c.cleanupMu.Unlock() + for _, p := range paths { + _ = os.RemoveAll(p) + } +} + // InjectAsUser is the path the human takes when typing in the focused // pane. SPEC §6: the user's first keystroke flips ownership. func (c *Child) InjectAsUser(b []byte) error { diff --git a/internal/app/host.go b/internal/app/host.go index 653bb99..c8e81fd 100644 --- a/internal/app/host.go +++ b/internal/app/host.go @@ -36,6 +36,10 @@ type trustPrompter interface { promptTrust(processID, presetName, reason string) } +type scratchpadSink interface { + scratchpadsChanged() +} + // toolHost adapts the running session + scratchpad store + trust store // to the MCP ToolHost interface. SPEC §7 tools route through here. type toolHost struct { @@ -55,6 +59,7 @@ type toolHost struct { attention attentionSink focus focusSink prompter trustPrompter + scratch scratchpadSink timersMu sync.Mutex nextTimer int @@ -129,7 +134,7 @@ func (h *toolHost) SpawnAgent(callerID string, args mcp.SpawnAgentArgs) (mcp.Pro } } if p == nil { - return mcp.ProcessInfo{}, mcp.Errorf("unknown_agent", "unknown agent preset %q", args.Agent) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindUnknownAgent, "unknown agent preset %q", args.Agent) } display := args.Name if display == "" { @@ -148,7 +153,7 @@ func (h *toolHost) SpawnProcess(callerID string, args mcp.SpawnProcessArgs) (mcp args.Kind = "command" } if args.Kind != "command" && args.Kind != "terminal" { - return mcp.ProcessInfo{}, mcp.Errorf("invalid_kind", "spawn_process: kind must be 'command' or 'terminal'") + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindInvalidKind, "spawn_process: kind must be 'command' or 'terminal'") } env := h.mergeEnv(args.Env) if args.Kind == "terminal" { @@ -163,11 +168,11 @@ func (h *toolHost) SpawnProcess(callerID string, args mcp.SpawnProcessArgs) (mcp if args.Preset != "" { if !h.trust.IsTrusted(args.Preset) { h.askForTrust(callerID, args.Preset, "spawn_process") - return mcp.ProcessInfo{}, mcp.Errorf("needs_trust", "command preset %q is not trusted in this project — patterm has surfaced a confirmation; retry after the user accepts", args.Preset) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNeedsTrust, "command preset %q is not trusted in this project — patterm has surfaced a confirmation; retry after the user accepts", args.Preset) } ps := h.commandPresetByName(args.Preset) if ps == nil { - return mcp.ProcessInfo{}, mcp.Errorf("not_found", "command preset %q not found", args.Preset) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNotFound, "command preset %q not found", args.Preset) } display := args.Name if display == "" { @@ -181,7 +186,7 @@ func (h *toolHost) SpawnProcess(callerID string, args mcp.SpawnProcessArgs) (mcp return h.processInfoOf(c), nil } if len(args.Argv) == 0 { - return mcp.ProcessInfo{}, mcp.Errorf("invalid_args", "spawn_process: either preset or argv required") + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindInvalidArgs, "spawn_process: either preset or argv required") } display := args.Name if display == "" { @@ -198,17 +203,17 @@ func (h *toolHost) SpawnProcess(callerID string, args mcp.SpawnProcessArgs) (mcp func (h *toolHost) StartProcess(callerID, processID string) (mcp.ProcessInfo, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.ProcessInfo{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } if c.Kind != KindCommand { - return mcp.ProcessInfo{}, mcp.Errorf("wrong_kind", "start_process: only command entries can be started post-creation (this is %s)", c.Kind) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindWrongKind, "start_process: only command entries can be started post-creation (this is %s)", c.Kind) } if c.IsLive() { return h.processInfoOf(c), nil } if c.PresetRef != "" && !h.trust.IsTrusted(c.PresetRef) { h.askForTrust(callerID, c.PresetRef, "start_process") - return mcp.ProcessInfo{}, mcp.Errorf("needs_trust", "command preset %q is not trusted in this project", c.PresetRef) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNeedsTrust, "command preset %q is not trusted in this project", c.PresetRef) } cols, rows := h.size() if err := h.sess.Start(processID, cols, rows); err != nil { @@ -221,14 +226,14 @@ func (h *toolHost) StartProcess(callerID, processID string) (mcp.ProcessInfo, er func (h *toolHost) RestartProcess(callerID, processID string, sig syscall.Signal) (mcp.ProcessInfo, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.ProcessInfo{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } if c.Kind != KindCommand && !c.IsLive() { - return mcp.ProcessInfo{}, mcp.Errorf("wrong_kind", "restart_process: %s entries can only be restarted while live", c.Kind) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindWrongKind, "restart_process: %s entries can only be restarted while live", c.Kind) } if c.Kind == KindCommand && c.PresetRef != "" && !h.trust.IsTrusted(c.PresetRef) { h.askForTrust(callerID, c.PresetRef, "restart_process") - return mcp.ProcessInfo{}, mcp.Errorf("needs_trust", "command preset %q is not trusted in this project", c.PresetRef) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNeedsTrust, "command preset %q is not trusted in this project", c.PresetRef) } cols, rows := h.size() if err := h.sess.Restart(processID, sig, cols, rows); err != nil { @@ -241,7 +246,7 @@ func (h *toolHost) RestartProcess(callerID, processID string, sig syscall.Signal func (h *toolHost) StopProcess(callerID, processID string, sig syscall.Signal) (mcp.ProcessInfo, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.ProcessInfo{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.ProcessInfo{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } if err := h.sess.Kill(processID, sig); err != nil { return mcp.ProcessInfo{}, err @@ -252,7 +257,7 @@ func (h *toolHost) StopProcess(callerID, processID string, sig syscall.Signal) ( func (h *toolHost) CloseProcess(callerID, processID string) error { c := h.sess.FindChild(processID) if c == nil { - return mcp.Errorf("not_found", "no such process %q", processID) + return mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } _ = c // close removes by id; the lookup just validates existence. return h.sess.Close(processID, syscall.SIGTERM) @@ -261,10 +266,10 @@ func (h *toolHost) CloseProcess(callerID, processID string) error { func (h *toolHost) RenameProcess(callerID, processID, name string) error { c := h.sess.FindChild(processID) if c == nil { - return mcp.Errorf("not_found", "no such process %q", processID) + return mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } if name == "" { - return mcp.Errorf("invalid_args", "rename_process: name required") + return mcp.Errorf(mcp.ErrorKindInvalidArgs, "rename_process: name required") } c.SetName(name) return nil @@ -272,7 +277,7 @@ func (h *toolHost) RenameProcess(callerID, processID, name string) error { func (h *toolHost) SelectProcess(callerID, processID string) error { if h.sess.FindChild(processID) == nil { - return mcp.Errorf("not_found", "no such process %q", processID) + return mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } if h.focus != nil { h.focus.focusProcess(processID) @@ -299,7 +304,7 @@ func (h *toolHost) ListProcesses(callerID, kindFilter string) []mcp.ProcessInfo func (h *toolHost) GetProcessStatus(callerID, processID string) (mcp.ProcessStatus, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.ProcessStatus{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.ProcessStatus{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } info := h.processInfoOf(c) st := mcp.ProcessStatus{ProcessInfo: info} @@ -337,7 +342,7 @@ func (h *toolHost) GetProjectStatus(callerID string) (mcp.ProjectStatus, error) func (h *toolHost) GetProcessOutput(callerID, processID, mode string, sinceOffset int64) (mcp.ProcessOutput, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.ProcessOutput{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.ProcessOutput{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } out := mcp.ProcessOutput{ Mode: mode, @@ -376,14 +381,14 @@ func (h *toolHost) GetProcessOutput(callerID, processID, mode string, sinceOffse out.NewOffset = end return out, nil default: - return mcp.ProcessOutput{}, mcp.Errorf("invalid_args", "unknown mode %q (want grid|stream)", mode) + return mcp.ProcessOutput{}, mcp.Errorf(mcp.ErrorKindInvalidArgs, "unknown mode %q (want grid|stream)", mode) } } func (h *toolHost) GetProcessRawOutput(callerID, processID string, sinceOffset int64) (mcp.RawOutput, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.RawOutput{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.RawOutput{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } b, end := c.StreamRead(sinceOffset) return mcp.RawOutput{ @@ -396,11 +401,11 @@ func (h *toolHost) GetProcessRawOutput(callerID, processID string, sinceOffset i func (h *toolHost) SearchOutput(callerID, processID, pattern, kind string, limit int) (mcp.SearchResult, error) { c := h.sess.FindChild(processID) if c == nil { - return mcp.SearchResult{}, mcp.Errorf("not_found", "no such process %q", processID) + return mcp.SearchResult{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } re, err := regexp.Compile(pattern) if err != nil { - return mcp.SearchResult{}, mcp.Errorf("invalid_args", "regex: %v", err) + return mcp.SearchResult{}, mcp.Errorf(mcp.ErrorKindInvalidArgs, "regex: %v", err) } b, _ := c.StreamRead(0) text := string(b) @@ -425,11 +430,11 @@ func (h *toolHost) SearchOutput(callerID, processID, pattern, kind string, limit func (h *toolHost) WaitForPattern(callerID, processID, pattern string, timeoutSeconds float64, scope string) (bool, string, error) { c := h.sess.FindChild(processID) if c == nil { - return false, "", mcp.Errorf("not_found", "no such process %q", processID) + return false, "", mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } re, err := regexp.Compile(pattern) if err != nil { - return false, "", mcp.Errorf("invalid_args", "regex: %v", err) + return false, "", mcp.Errorf(mcp.ErrorKindInvalidArgs, "regex: %v", err) } if scope == "" { scope = "grid" @@ -450,7 +455,7 @@ func (h *toolHost) WaitForPattern(callerID, processID, pattern string, timeoutSe b, _ := c.StreamRead(0) text = stripANSI(string(b)) default: - return false, "", mcp.Errorf("invalid_args", "unknown scope %q (want grid|scrollback)", scope) + return false, "", mcp.Errorf(mcp.ErrorKindInvalidArgs, "unknown scope %q (want grid|scrollback)", scope) } if m := re.FindString(text); m != "" { return true, m, nil @@ -468,7 +473,7 @@ func (h *toolHost) WaitForPattern(callerID, processID, pattern string, timeoutSe func (h *toolHost) GetProcessPorts(callerID, processID string) ([]mcp.PortSighting, error) { c := h.sess.FindChild(processID) if c == nil { - return nil, mcp.Errorf("not_found", "no such process %q", processID) + return nil, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", processID) } src := c.Ports() out := make([]mcp.PortSighting, 0, len(src)) @@ -485,7 +490,7 @@ func (h *toolHost) GetProcessPorts(callerID, processID string) ([]mcp.PortSighti func (h *toolHost) SendInput(callerID string, args mcp.SendInputArgs) (mcp.SendInputResult, error) { c := h.sess.FindChild(args.ProcessID) if c == nil { - return mcp.SendInputResult{}, mcp.Errorf("not_found", "no such process %q", args.ProcessID) + return mcp.SendInputResult{}, mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", args.ProcessID) } if !c.IsLive() { return mcp.SendInputResult{}, fmt.Errorf("process %q is %s", args.ProcessID, c.Status()) @@ -539,7 +544,7 @@ func encodeInput(args mcp.SendInputArgs) ([]byte, error) { case "key": return encodeKey(args.Key) } - return nil, mcp.Errorf("invalid_args", "send_input: unknown kind %q", args.Kind) + return nil, mcp.Errorf(mcp.ErrorKindInvalidArgs, "send_input: unknown kind %q", args.Kind) } // encodeKey maps a SPEC §7 named key to bytes. We use legacy xterm @@ -601,7 +606,7 @@ func encodeKey(key string) ([]byte, error) { case "f12": return []byte("\x1b[24~"), nil } - return nil, mcp.Errorf("invalid_args", "unknown key %q", key) + return nil, mcp.Errorf(mcp.ErrorKindInvalidArgs, "unknown key %q", key) } // ─────────────────────────────────────────────────────────────────── @@ -616,7 +621,7 @@ func encodeKey(key string) ([]byte, error) { func (h *toolHost) SendMessage(callerID, targetID, message string) error { target := h.sess.FindChild(targetID) if target == nil { - return mcp.Errorf("not_found", "no such process %q", targetID) + return mcp.Errorf(mcp.ErrorKindNotFound, "no such process %q", targetID) } caller := h.sess.FindChild(callerID) line, err := classifySendMessage(caller, target, callerID, message) @@ -637,7 +642,7 @@ func (h *toolHost) SendMessage(callerID, targetID, message string) error { // top-level process. func classifySendMessage(caller, target *Child, callerID, message string) (string, error) { if target.ID == callerID { - return "", mcp.Errorf("not_related", "send_message: cannot send to self") + return "", mcp.Errorf(mcp.ErrorKindNotRelated, "send_message: cannot send to self") } if caller != nil && target.ParentID == caller.ID { return "[orchestrator] " + message + "\r", nil @@ -648,7 +653,7 @@ func classifySendMessage(caller, target *Child, callerID, message string) (strin if caller == nil && target.ParentID == "" { return "[orchestrator] " + message + "\r", nil } - return "", mcp.Errorf("not_related", "send_message: %q is neither parent nor child of caller (siblings must route through the parent in v1)", target.ID) + return "", mcp.Errorf(mcp.ErrorKindNotRelated, "send_message: %q is neither parent nor child of caller (siblings must route through the parent in v1)", target.ID) } func (h *toolHost) RequestHumanAttention(callerID, processID, reason string) error { @@ -661,7 +666,7 @@ func (h *toolHost) RequestHumanAttention(callerID, processID, reason string) err func (h *toolHost) TimerWait(callerID string, seconds float64, label string) (string, error) { caller := h.sess.FindChild(callerID) if caller == nil { - return "", mcp.Errorf("not_found", "caller %q not known to patterm", callerID) + return "", mcp.Errorf(mcp.ErrorKindNotFound, "caller %q not known to patterm", callerID) } h.timersMu.Lock() h.nextTimer++ @@ -685,7 +690,27 @@ func (h *toolHost) TimerWait(callerID string, seconds float64, label string) (st // Scratchpads / Meta // ─────────────────────────────────────────────────────────────────── -func (h *toolHost) Scratchpads() *scratchpad.Store { return h.pads } +func (h *toolHost) ScratchpadList() ([]scratchpad.Entry, error) { return h.pads.List() } + +func (h *toolHost) ScratchpadRead(name string) (string, string, error) { + return h.pads.Read(name) +} + +func (h *toolHost) ScratchpadWrite(name, content, expectedRevision string) (string, error) { + rev, err := h.pads.Write(name, content, expectedRevision) + if err == nil && h.scratch != nil { + h.scratch.scratchpadsChanged() + } + return rev, err +} + +func (h *toolHost) ScratchpadAppend(name, content string) error { + err := h.pads.Append(name, content) + if err == nil && h.scratch != nil { + h.scratch.scratchpadsChanged() + } + return err +} func (h *toolHost) WhoAmI(callerID string) mcp.WhoAmI { w := mcp.WhoAmI{ diff --git a/internal/app/launch.go b/internal/app/launch.go index a5ab239..d11aee3 100644 --- a/internal/app/launch.go +++ b/internal/app/launch.go @@ -56,14 +56,12 @@ func (l *Launcher) LaunchAgent(p *preset.Preset, displayName, initialPrompt, par env = append(env, k+"="+v) } - // Mint a per-spawn MCP config file pointing at the mcp-stdio proxy - // with the new child's identity. We don't know the identity until - // we've created the child, but the child needs the env/argv at - // creation time — so we reserve the identity by pre-creating the - // MCP config with a placeholder, then patching it post-spawn. - identity, mcpConfigPath, err := l.writeMCPConfig() - if err != nil { - return nil, err + identity := mintIdentity() + var cleanupPaths []string + cleanup := func() { + for _, path := range cleanupPaths { + _ = os.RemoveAll(path) + } } if p.MCPInjection != nil { @@ -72,24 +70,33 @@ func (l *Launcher) LaunchAgent(p *preset.Preset, displayName, initialPrompt, par if p.MCPInjection.Flag == "" { return nil, fmt.Errorf("preset %s: mcp_injection.flag required for kind=flag", p.Name) } + mcpConfigPath, err := l.writeMCPConfig(identity) + if err != nil { + return nil, err + } + cleanupPaths = append(cleanupPaths, mcpConfigPath) argv = append(argv, p.MCPInjection.Flag, mcpConfigPath) case "env_var": if p.MCPInjection.Var == "" { return nil, fmt.Errorf("preset %s: mcp_injection.var required for kind=env_var", p.Name) } + mcpConfigPath, err := l.writeMCPConfig(identity) + if err != nil { + return nil, err + } + cleanupPaths = append(cleanupPaths, mcpConfigPath) env = append(env, p.MCPInjection.Var+"="+mcpConfigPath) case "config_file": // Merge patterm's MCP entry into a vendored copy of the // user's existing config file, then point the child at the // vendored copy via the preset's home_var. The real config // file is never modified. - envAssign, _, mErr := mcpConfigMerge(p, p.MCPInjection, identity, l.bin, l.mcpSocket) + envAssign, homeDir, mErr := mcpConfigMerge(p, p.MCPInjection, identity, l.bin, l.mcpSocket) if mErr != nil { - _ = os.Remove(mcpConfigPath) return nil, mErr } + cleanupPaths = append(cleanupPaths, homeDir) env = append(env, envAssign) - env = append(env, "PATTERM_MCP_CONFIG="+mcpConfigPath) case "cli_override": // Inline -c key=value overrides for agents that accept // them (codex's `-c mcp_servers.patterm.command=...`). No @@ -97,7 +104,6 @@ func (l *Launcher) LaunchAgent(p *preset.Preset, displayName, initialPrompt, par // are untouched. extra, err := mcpCLIOverrideArgs(p, p.MCPInjection, identity, l.bin, l.mcpSocket) if err != nil { - _ = os.Remove(mcpConfigPath) return nil, err } argv = append(argv, extra...) @@ -108,11 +114,11 @@ func (l *Launcher) LaunchAgent(p *preset.Preset, displayName, initialPrompt, par // XDG_CONFIG_HOME stays as the user set it. assignment, err := mcpConfigEnv(p, p.MCPInjection, identity, l.bin, l.mcpSocket) if err != nil { - _ = os.Remove(mcpConfigPath) return nil, err } env = append(env, assignment) default: + cleanup() return nil, fmt.Errorf("preset %s: unknown mcp_injection.kind %q", p.Name, p.MCPInjection.Kind) } } @@ -120,16 +126,17 @@ func (l *Launcher) LaunchAgent(p *preset.Preset, displayName, initialPrompt, par // Spawn with the chosen identity. cols, rows := l.size() c, err := l.sess.Spawn(SpawnSpec{ - Kind: KindAgent, - Argv: argv, - Env: env, - Name: displayName, - ParentID: parentID, - PresetRef: p.Name, - Identity: identity, + Kind: KindAgent, + Argv: argv, + Env: env, + Name: displayName, + ParentID: parentID, + PresetRef: p.Name, + Identity: identity, + CleanupPaths: cleanupPaths, }, cols, rows) if err != nil { - _ = os.Remove(mcpConfigPath) + cleanup() return nil, err } @@ -219,17 +226,16 @@ func (l *Launcher) LaunchTerminal(argv []string, displayName, parentID, workDir }, cols, rows) } -func (l *Launcher) writeMCPConfig() (identity, path string, err error) { - identity = mintIdentity() +func (l *Launcher) writeMCPConfig(identity string) (string, error) { dir, err := preset.ConfigDir() if err != nil { - return "", "", err + return "", err } dir = filepath.Join(dir, "mcp") if err := os.MkdirAll(dir, 0o700); err != nil { - return "", "", err + return "", err } - path = filepath.Join(dir, identity+".json") + path := filepath.Join(dir, identity+".json") cfg := map[string]any{ "mcpServers": map[string]any{ "patterm": map[string]any{ @@ -240,13 +246,13 @@ func (l *Launcher) writeMCPConfig() (identity, path string, err error) { } body, err := json.MarshalIndent(cfg, "", " ") if err != nil { - return "", "", err + return "", err } body = append(body, '\n') if err := os.WriteFile(path, body, 0o600); err != nil { - return "", "", err + return "", err } - return identity, path, nil + return path, nil } // waitForIdle polls the child's IdleMS until it exceeds idle, or until diff --git a/internal/app/palette.go b/internal/app/palette.go index 62c8afd..f257885 100644 --- a/internal/app/palette.go +++ b/internal/app/palette.go @@ -115,7 +115,7 @@ func (p *paletteState) allItems() []paletteItem { if c.Kind == KindAgent && c.Status() != StatusRunning { continue } - label := "Switch to " + c.Name + label := "Switch to " + c.DisplayName() hint := strings.Join(c.Argv, " ") if c.ID == p.focused { label = "• " + label + " (current)" @@ -153,7 +153,7 @@ func (p *paletteState) allItems() []paletteItem { continue } out = append(out, paletteItem{ - label: "Kill " + c.Name, + label: "Kill " + c.DisplayName(), hint: "SIGTERM " + strings.Join(c.Argv, " "), action: paletteAction{kind: "kill", childID: c.ID}, }) diff --git a/internal/app/session.go b/internal/app/session.go index acb5e5c..08bcd7e 100644 --- a/internal/app/session.go +++ b/internal/app/session.go @@ -18,6 +18,8 @@ import ( "github.com/hjbdev/patterm/internal/vt" ) +const childStopTimeout = 2 * time.Second + // Session is the in-memory state for the running patterm process. // In SPEC §4 terms each top-level tab is a session; v1 ships with a // single implicit session and reserves room to grow. @@ -117,6 +119,10 @@ type SpawnSpec struct { ParentID string PresetRef string Identity string // pre-minted; otherwise the constructor mints one for agents + // CleanupPaths are owned runtime files/dirs removed when the child exits + // or is closed. They must be attached before the PTY starts so a + // fast-exiting child cannot outrun cleanup registration. + CleanupPaths []string } // Spawn creates a new entry and starts its PTY. For Kind = command the @@ -144,7 +150,12 @@ func (s *Session) Spawn(spec SpawnSpec, cols, rows uint16) (*Child, error) { if spec.Identity != "" { c.Identity = spec.Identity } - if err := c.startPTY(cols, rows); err != nil { + for _, path := range spec.CleanupPaths { + c.AddCleanupPath(path) + } + runID, err := c.startPTY(cols, rows) + if err != nil { + c.cleanupOwnedPaths() return nil, err } @@ -154,33 +165,11 @@ func (s *Session) Spawn(spec SpawnSpec, cols, rows uint16) (*Child, error) { s.mu.Unlock() s.emitSpawn(c) - go s.pumpChild(c) - go s.reapChild(c) + go s.pumpChild(c, runID) + go s.reapChild(c, runID) return c, nil } -// AddCommandEntry registers a command entry without starting it. Used -// by spawn_process(kind: command) when SPEC §7 needs the entry to exist -// in `stopped` state first (we always start it after; the indirection -// is here so future versions can support deferred starts). -func (s *Session) AddCommandEntry(spec SpawnSpec) *Child { - s.mu.Lock() - id := s.mintUniqueIDLocked() - s.nameSeq[spec.Kind]++ - if spec.Name == "" { - spec.Name = fmt.Sprintf("%s-%d", spec.Kind, s.nameSeq[spec.Kind]) - } - if spec.Env == nil { - spec.Env = s.ChildEnv() - } - c := newChildEntry(id, spec.Name, spec.Kind, spec.Argv, spec.Env, spec.ParentID, spec.WorkDir, spec.PresetRef) - s.children[id] = c - s.order = append(s.order, id) - s.mu.Unlock() - s.emitSpawn(c) - return c -} - // Start (re)attaches a PTY to an entry that is currently stopped or // exited. Errors if the entry is already live. func (s *Session) Start(id string, cols, rows uint16) error { @@ -191,11 +180,12 @@ func (s *Session) Start(id string, cols, rows uint16) error { if c.IsLive() { return nil // SPEC §7 start_process is a no-op on a running entry } - if err := c.startPTY(cols, rows); err != nil { + runID, err := c.startPTY(cols, rows) + if err != nil { return err } - go s.pumpChild(c) - go s.reapChild(c) + go s.pumpChild(c, runID) + go s.reapChild(c, runID) return nil } @@ -210,32 +200,20 @@ func (s *Session) Restart(id string, sig syscall.Signal, cols, rows uint16) erro if c.Kind != KindCommand && !c.IsLive() { return fmt.Errorf("restart: %s entries can only be restarted while live", c.Kind) } + // Only live entries can own runtime MCP config paths today. Keep the + // reaper from cleaning those paths while restart swaps the PTY. + c.restarting.Store(true) + defer c.restarting.Store(false) if c.IsLive() { - if sig == 0 { - sig = syscall.SIGTERM - } - _ = c.signal(sig) - // Wait briefly for the reaper to mark exited. We don't need - // strict synchronization — the reaper will run regardless; we - // just want startPTY to land after teardown. - deadline := time.Now().Add(2 * time.Second) - for c.IsLive() && time.Now().Before(deadline) { - time.Sleep(20 * time.Millisecond) - } - if c.IsLive() { - // Force. - _ = c.signal(syscall.SIGKILL) - for c.IsLive() { - time.Sleep(20 * time.Millisecond) - } - } + terminateAndWait(c, sig, childStopTimeout) } c.teardownPTY() - if err := c.startPTY(cols, rows); err != nil { + runID, err := c.startPTY(cols, rows) + if err != nil { return err } - go s.pumpChild(c) - go s.reapChild(c) + go s.pumpChild(c, runID) + go s.reapChild(c, runID) return nil } @@ -247,22 +225,10 @@ func (s *Session) Close(id string, sig syscall.Signal) error { return fmt.Errorf("no such process %q", id) } if c.IsLive() { - if sig == 0 { - sig = syscall.SIGTERM - } - _ = c.signal(sig) - deadline := time.Now().Add(2 * time.Second) - for c.IsLive() && time.Now().Before(deadline) { - time.Sleep(20 * time.Millisecond) - } - if c.IsLive() { - _ = c.signal(syscall.SIGKILL) - for c.IsLive() { - time.Sleep(20 * time.Millisecond) - } - } + terminateAndWait(c, sig, childStopTimeout) } c.teardownPTY() + c.cleanupOwnedPaths() s.mu.Lock() delete(s.children, id) for i, oid := range s.order { @@ -286,15 +252,18 @@ func (s *Session) mintUniqueIDLocked() string { } } -func (s *Session) pumpChild(c *Child) { +func (s *Session) pumpChild(c *Child, runID uint64) { + pty := c.ptyForRun(runID) + if pty == nil { + return + } buf := make([]byte, 64*1024) for { - pty := c.PTY() - if pty == nil { - return - } n, err := pty.Read(buf) if n > 0 { + if !c.isCurrentRun(runID) { + return + } chunk := make([]byte, n) copy(chunk, buf[:n]) if em := c.Emulator(); em != nil { @@ -314,16 +283,22 @@ func (s *Session) pumpChild(c *Child) { } } -func (s *Session) reapChild(c *Child) { - pty := c.PTY() +func (s *Session) reapChild(c *Child, runID uint64) { + pty := c.ptyForRun(runID) if pty == nil { return } err := pty.Wait() + if !c.isCurrentRun(runID) || c.restarting.Load() { + return + } c.markExited(err) logf("child %s exited (err=%v)", c.ID, err) s.emitExit(c) s.killDescendantsOf(c.ID) + if !c.restarting.Load() { + c.cleanupOwnedPaths() + } } // killDescendantsOf terminates every still-live direct child of @@ -352,24 +327,49 @@ func (s *Session) killDescendantsOf(parentID string) { for _, c := range live { _ = c.signal(syscall.SIGTERM) } - deadline := time.Now().Add(2 * time.Second) + waitForAllStopped(live, childStopTimeout) + for _, c := range live { + if c.IsLive() { + _ = c.signal(syscall.SIGKILL) + } + } + waitForAllStopped(live, childStopTimeout) +} + +func waitForAllStopped(children []*Child, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { anyLive := false - for _, c := range live { + for _, c := range children { if c.IsLive() { anyLive = true break } } if !anyLive { - return + return true } time.Sleep(20 * time.Millisecond) } - for _, c := range live { - if c.IsLive() { - _ = c.signal(syscall.SIGKILL) - } + return false +} + +func terminateAndWait(c *Child, sig syscall.Signal, timeout time.Duration) { + if sig == 0 { + sig = syscall.SIGTERM + } + _ = c.signal(sig) + deadline := time.Now().Add(timeout) + for c.IsLive() && time.Now().Before(deadline) { + time.Sleep(20 * time.Millisecond) + } + if !c.IsLive() { + return + } + _ = c.signal(syscall.SIGKILL) + deadline = time.Now().Add(timeout) + for c.IsLive() && time.Now().Before(deadline) { + time.Sleep(20 * time.Millisecond) } } @@ -524,6 +524,7 @@ func (s *Session) Shutdown() { // emitExit as Wait() returns. for _, c := range cs { c.teardownPTY() + c.cleanupOwnedPaths() } } diff --git a/internal/app/sidebar.go b/internal/app/sidebar.go index a976ad1..e7113fe 100644 --- a/internal/app/sidebar.go +++ b/internal/app/sidebar.go @@ -88,9 +88,9 @@ func (st *uiState) drawSidebar() { var line string if focused { line = " " + styleAccent + "▎" + styleReset + " " + indent + glyph + " " + - styleBold + c.Name + styleReset + styleBold + c.DisplayName() + styleReset } else { - line = " " + indent + glyph + " " + styleHint + c.Name + styleReset + line = " " + indent + glyph + " " + styleHint + c.DisplayName() + styleReset } write(line) } diff --git a/internal/app/tabbar.go b/internal/app/tabbar.go index 80c0149..f68acf4 100644 --- a/internal/app/tabbar.go +++ b/internal/app/tabbar.go @@ -99,7 +99,7 @@ func (st *uiState) drawTabBar() { if i < extra { w++ } - label := c.Name + label := c.DisplayName() labelW := utf8.RuneCountInString(label) maxLabelW := w - 2 // one pad on each side if maxLabelW < 1 { diff --git a/internal/harness/env.go b/internal/harness/env.go index 37a88f6..bb4b1a6 100644 --- a/internal/harness/env.go +++ b/internal/harness/env.go @@ -8,6 +8,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "github.com/hjbdev/patterm/internal/projectkey" "github.com/hjbdev/patterm/internal/trust" @@ -179,9 +180,18 @@ func defaultPattermBin() (string, error) { if p := os.Getenv("PATTERM_BIN"); p != "" { return p, nil } - return buildPattermBinary() + defaultBinOnce.Do(func() { + defaultBinPath, defaultBinErr = buildPattermBinary() + }) + return defaultBinPath, defaultBinErr } +var ( + defaultBinOnce sync.Once + defaultBinPath string + defaultBinErr error +) + func buildPattermBinary() (string, error) { root, err := repoRoot() if err != nil { diff --git a/internal/harness/recorder.go b/internal/harness/recorder.go index 095778f..ccbd4d4 100644 --- a/internal/harness/recorder.go +++ b/internal/harness/recorder.go @@ -21,9 +21,16 @@ func (s *Session) DumpArtifacts(sc *Scenario, failingStep int, cause error) (*Ar if name == "" { name = "scenario" } - dir := filepath.Join("internal", "harness", ".artifacts", fmt.Sprintf("%s-%d", name, time.Now().Unix())) - abs, _ := filepath.Abs(dir) - if err := os.MkdirAll(abs, 0o700); err != nil { + root, err := repoRoot() + if err != nil { + return nil, err + } + base := filepath.Join(root, "internal", "harness", ".artifacts") + if err := os.MkdirAll(base, 0o700); err != nil { + return nil, err + } + abs, err := os.MkdirTemp(base, fmt.Sprintf("%s-%d-*", name, time.Now().UnixNano())) + if err != nil { return nil, err } screen, _ := s.em.ScreenText() diff --git a/internal/harness/session.go b/internal/harness/session.go index 4acb83a..7928353 100644 --- a/internal/harness/session.go +++ b/internal/harness/session.go @@ -210,19 +210,19 @@ func (s *Session) WaitForStable(timeout time.Duration) error { } func (s *Session) WaitForText(text string, timeout time.Duration) error { - deadline := time.Now().Add(timeout) - for time.Now().Before(deadline) { + return pollUntil(timeout, 25*time.Millisecond, func() (bool, error) { screen, err := s.Screen() if err != nil { - return err + return false, err } if strings.Contains(screen, text) { - return nil + return true, nil } - time.Sleep(25 * time.Millisecond) - } - screen, _ := s.Screen() - return fmt.Errorf("text %q not found before timeout; screen:\n%s", text, screen) + return false, nil + }, func() error { + screen, _ := s.Screen() + return fmt.Errorf("text %q not found before timeout; screen:\n%s", text, screen) + }) } func (s *Session) WaitForRegex(pattern string, timeout time.Duration) error { @@ -230,19 +230,31 @@ func (s *Session) WaitForRegex(pattern string, timeout time.Duration) error { if err != nil { return err } - deadline := time.Now().Add(timeout) - for time.Now().Before(deadline) { + return pollUntil(timeout, 25*time.Millisecond, func() (bool, error) { screen, err := s.Screen() if err != nil { - return err + return false, err } if re.MatchString(screen) { - return nil + return true, nil } - time.Sleep(25 * time.Millisecond) + return false, nil + }, func() error { + screen, _ := s.Screen() + return fmt.Errorf("regex %q not found before timeout; screen:\n%s", pattern, screen) + }) +} + +func pollUntil(timeout, interval time.Duration, check func() (bool, error), timeoutErr func() error) error { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + ok, err := check() + if err != nil || ok { + return err + } + time.Sleep(interval) } - screen, _ := s.Screen() - return fmt.Errorf("regex %q not found before timeout; screen:\n%s", pattern, screen) + return timeoutErr() } func (s *Session) MCPCall(method string, params json.RawMessage) (json.RawMessage, error) { diff --git a/internal/mcp/protocol.go b/internal/mcp/protocol.go index bad525a..0266eee 100644 --- a/internal/mcp/protocol.go +++ b/internal/mcp/protocol.go @@ -166,10 +166,10 @@ func toolCatalog() []toolDescriptor { }, { Name: "get_process_output", - Description: "Read rendered grid (\"grid\") or scrollback (\"scrollback\") output, with screen-version watermark.", + Description: "Read rendered grid (\"grid\") or ANSI-stripped stream (\"stream\") output, with screen-version watermark.", InputSchema: objectSchema(map[string]any{ "process_id": stringProp("Target process id."), - "mode": stringProp("\"grid\" (default) or \"scrollback\"."), + "mode": stringProp("\"grid\" (default) or \"stream\"."), "since_offset": integerProp("Watermark offset from a previous call."), }, []string{"process_id"}), }, @@ -198,7 +198,7 @@ func toolCatalog() []toolDescriptor { "process_id": stringProp("Target process id."), "pattern": stringProp("Regex pattern."), "timeout_seconds": numberProp("Max time to wait (seconds)."), - "scope": stringProp("\"new\" (default) or \"all\"."), + "scope": stringProp("\"grid\" (default) or \"scrollback\"."), }, []string{"process_id", "pattern"}), }, { @@ -215,7 +215,7 @@ func toolCatalog() []toolDescriptor { "process_id": stringProp("Target process id."), "kind": stringProp("\"text\", \"paste\", or \"key\"."), "text": stringProp("Text payload for kind=text/paste."), - "key": stringProp("Named key for kind=key (e.g. \"enter\", \"esc\")."), + "key": stringProp("Named key for kind=key (e.g. \"enter\", \"escape\")."), "submit": booleanProp("Whether to append a submit keystroke."), "wait_ms": integerProp("After sending, wait this many ms before tailing."), "tail_mode": stringProp("\"none\" (default), \"stream\", or \"grid\"."), diff --git a/internal/mcp/protocol_test.go b/internal/mcp/protocol_test.go index e9276ab..0d958c4 100644 --- a/internal/mcp/protocol_test.go +++ b/internal/mcp/protocol_test.go @@ -126,3 +126,19 @@ func TestPingReturnsEmptyObject(t *testing.T) { t.Fatal("ping result missing") } } + +func TestTypedInvalidArgsMapToInvalidParams(t *testing.T) { + for _, errKind := range []string{ErrorKindInvalidArgs, ErrorKindInvalidKind} { + _, code, msg, data := mapToolError(Errorf(errKind, "bad args")) + if code != codeInvalidParams { + t.Fatalf("%s code = %d, want %d", errKind, code, codeInvalidParams) + } + if msg != "bad args" { + t.Fatalf("%s message = %q", errKind, msg) + } + kind, ok := data.(map[string]string) + if !ok || kind["kind"] != errKind { + t.Fatalf("%s data = %#v", errKind, data) + } + } +} diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index d0bc073..ccf4765 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -14,6 +14,15 @@ import ( // names live in the -32000 range with a structured `data.kind` so the // caller can branch on the error type rather than parsing strings. const ( + ErrorKindInvalidArgs = "invalid_args" + ErrorKindInvalidKind = "invalid_kind" + ErrorKindNeedsTrust = "needs_trust" + ErrorKindRoleForbidden = "role_forbidden" + ErrorKindNotRelated = "not_related" + ErrorKindNotFound = "not_found" + ErrorKindWrongKind = "wrong_kind" + ErrorKindUnknownAgent = "unknown_agent" + codeParseError = -32700 codeInvalidRequest = -32600 codeMethodNotFound = -32601 @@ -81,7 +90,10 @@ type ToolHost interface { TimerWait(callerID string, seconds float64, label string) (string, error) // Scratchpads. - Scratchpads() *scratchpad.Store + ScratchpadList() ([]scratchpad.Entry, error) + ScratchpadRead(name string) (content string, revision string, err error) + ScratchpadWrite(name, content, expectedRevision string) (revision string, err error) + ScratchpadAppend(name, content string) error // Meta. WhoAmI(callerID string) WhoAmI @@ -105,14 +117,14 @@ type ProcessInfo struct { // ProcessInfo: includes pane geometry, cursor, and active screen. type ProcessStatus struct { ProcessInfo - WorkingDir string `json:"working_dir,omitempty"` + WorkingDir string `json:"working_dir,omitempty"` Argv []string `json:"argv,omitempty"` - StartedAt string `json:"started_at,omitempty"` - ActiveScreen string `json:"active_screen,omitempty"` - Rows int `json:"rows,omitempty"` - Cols int `json:"cols,omitempty"` - Cursor Cursor `json:"cursor"` - ScreenVersion int64 `json:"screen_version,omitempty"` + StartedAt string `json:"started_at,omitempty"` + ActiveScreen string `json:"active_screen,omitempty"` + Rows int `json:"rows,omitempty"` + Cols int `json:"cols,omitempty"` + Cursor Cursor `json:"cursor"` + ScreenVersion int64 `json:"screen_version,omitempty"` } // Cursor matches SPEC §7's `{x, y}` payload. @@ -124,10 +136,10 @@ type Cursor struct { // ProjectStatus is what get_project_status returns — everything an // agent needs to orient itself in one call. type ProjectStatus struct { - Project ProjectMeta `json:"project"` - Caller WhoAmI `json:"caller"` - Processes []ProcessInfo `json:"processes"` - Scratchpads []scratchpad.Entry `json:"scratchpads"` + Project ProjectMeta `json:"project"` + Caller WhoAmI `json:"caller"` + Processes []ProcessInfo `json:"processes"` + Scratchpads []scratchpad.Entry `json:"scratchpads"` } // ProjectMeta is the project root info echoed in many payloads. @@ -178,20 +190,20 @@ type PortSighting struct { // SpawnAgentArgs is the input shape for spawn_agent. type SpawnAgentArgs struct { - Agent string `json:"agent"` - AgentInstructions string `json:"agent_instructions"` - Name string `json:"name"` + Agent string `json:"agent"` + AgentInstructions string `json:"agent_instructions"` + Name string `json:"name"` } // SpawnProcessArgs is the input shape for spawn_process. type SpawnProcessArgs struct { - Kind string `json:"kind"` // "terminal" | "command" - Preset string `json:"preset"` - Argv []string `json:"argv"` - Name string `json:"name"` - WorkingDir string `json:"working_dir"` + Kind string `json:"kind"` // "terminal" | "command" + Preset string `json:"preset"` + Argv []string `json:"argv"` + Name string `json:"name"` + WorkingDir string `json:"working_dir"` Env map[string]string `json:"env"` - Shell bool `json:"shell"` + Shell bool `json:"shell"` } // SendInputArgs is the input shape for send_input — covers text / @@ -214,12 +226,12 @@ type SendInputResult struct { // WhoAmI is the whoami return shape. type WhoAmI struct { - ProcessID string `json:"process_id"` - Name string `json:"name"` - Role CallerRole `json:"role"` - ParentProcessID string `json:"parent_process_id,omitempty"` + ProcessID string `json:"process_id"` + Name string `json:"name"` + Role CallerRole `json:"role"` + ParentProcessID string `json:"parent_process_id,omitempty"` Project ProjectMeta `json:"project"` - AvailableTools []string `json:"available_tools"` + AvailableTools []string `json:"available_tools"` } // HelpResponse is the help return shape. @@ -332,7 +344,9 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return mapToolResult(info, err) case "start_process": - var p struct{ ProcessID string `json:"process_id"` } + var p struct { + ProcessID string `json:"process_id"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } @@ -364,7 +378,9 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return mapToolResult(info, err) case "close_process": - var p struct{ ProcessID string `json:"process_id"` } + var p struct { + ProcessID string `json:"process_id"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } @@ -387,7 +403,9 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return "ok", 0, "", nil case "select_process": - var p struct{ ProcessID string `json:"process_id"` } + var p struct { + ProcessID string `json:"process_id"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } @@ -397,12 +415,16 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return "ok", 0, "", nil case "list_processes": - var p struct{ Kind string `json:"kind"` } + var p struct { + Kind string `json:"kind"` + } _ = unmarshalParamsOptional(params, &p) return h.ListProcesses(callerID, p.Kind), 0, "", nil case "get_process_status": - var p struct{ ProcessID string `json:"process_id"` } + var p struct { + ProcessID string `json:"process_id"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } @@ -490,7 +512,9 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return map[string]any{"matched": matched, "snippet": snippet}, 0, "", nil case "get_process_ports": - var p struct{ ProcessID string `json:"process_id"` } + var p struct { + ProcessID string `json:"process_id"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } @@ -552,18 +576,20 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return map[string]string{"timer_id": id}, 0, "", nil case "scratchpad_list": - entries, err := h.Scratchpads().List() + entries, err := h.ScratchpadList() if err != nil { return nil, codeInternal, err.Error(), nil } return entries, 0, "", nil case "scratchpad_read": - var p struct{ Name string `json:"name"` } + var p struct { + Name string `json:"name"` + } if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } - content, rev, err := h.Scratchpads().Read(p.Name) + content, rev, err := h.ScratchpadRead(p.Name) if err != nil { return nil, codeInternal, err.Error(), nil } @@ -578,7 +604,7 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } - rev, err := h.Scratchpads().Write(p.Name, p.Content, p.ExpectedRevision) + rev, err := h.ScratchpadWrite(p.Name, p.Content, p.ExpectedRevision) if err != nil { // Optimistic-concurrency miss returns ok:false + current_revision // rather than a JSON-RPC error so callers can re-read + merge. @@ -598,7 +624,7 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, if err := unmarshalParams(params, &p); err != nil { return nil, codeInvalidParams, err.Error(), nil } - if err := h.Scratchpads().Append(p.Name, p.Content); err != nil { + if err := h.ScratchpadAppend(p.Name, p.Content); err != nil { return nil, codeInternal, err.Error(), nil } return map[string]any{"ok": true}, 0, "", nil @@ -607,7 +633,9 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, return h.WhoAmI(callerID), 0, "", nil case "help": - var p struct{ Topic string `json:"topic"` } + var p struct { + Topic string `json:"topic"` + } _ = unmarshalParamsOptional(params, &p) return h.Help(callerID, p.Topic), 0, "", nil } @@ -632,17 +660,19 @@ func mapToolError(err error) (any, int, string, any) { if errors.As(err, &te) { code := codeInternal switch te.Kind { - case "needs_trust": + case ErrorKindInvalidArgs, ErrorKindInvalidKind: + code = codeInvalidParams + case ErrorKindNeedsTrust: code = codeNeedsTrust - case "role_forbidden": + case ErrorKindRoleForbidden: code = codeRoleForbidden - case "not_related": + case ErrorKindNotRelated: code = codeNotRelated - case "not_found": + case ErrorKindNotFound: code = codeNotFound - case "wrong_kind": + case ErrorKindWrongKind: code = codeWrongKind - case "unknown_agent": + case ErrorKindUnknownAgent: code = codeUnknownAgent } return nil, code, te.Message, structuredKind(te.Kind) diff --git a/internal/scratchpad/scratchpad.go b/internal/scratchpad/scratchpad.go index 93a3911..77f5564 100644 --- a/internal/scratchpad/scratchpad.go +++ b/internal/scratchpad/scratchpad.go @@ -13,10 +13,12 @@ import ( "path/filepath" "sort" "strings" + "sync" ) // Store is the per-project scratchpad directory. type Store struct { + mu sync.Mutex dir string } @@ -55,6 +57,8 @@ type Entry struct { func (s *Store) Dir() string { return s.dir } func (s *Store) List() ([]Entry, error) { + s.mu.Lock() + defer s.mu.Unlock() entries, err := os.ReadDir(s.dir) if err != nil { return nil, err @@ -79,6 +83,8 @@ func (s *Store) List() ([]Entry, error) { } func (s *Store) Read(name string) (content string, revision string, err error) { + s.mu.Lock() + defer s.mu.Unlock() p, err := s.safePath(name) if err != nil { return "", "", err @@ -106,6 +112,8 @@ func (e *RevisionMismatchError) Error() string { // must match the current revision or the write is rejected with a // *RevisionMismatchError (SPEC §14 last-write-wins-with-token). func (s *Store) Write(name, content, expectedRevision string) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() p, err := s.safePath(name) if err != nil { return "", err @@ -125,6 +133,8 @@ func (s *Store) Write(name, content, expectedRevision string) (string, error) { } func (s *Store) Append(name, content string) error { + s.mu.Lock() + defer s.mu.Unlock() p, err := s.safePath(name) if err != nil { return err