From 56fd461fb31efa4a8cdcd37ae957bc615f577487 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Thu, 14 May 2026 21:17:03 +0100 Subject: [PATCH] Teach parent agents to clean up the processes they spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `lifecycle` help topic spelling out that the caller owns the processes it spawns and should `close_process` when a sub-agent or spawned child is no longer needed. The `spawn_agent` and `spawn_process` descriptions advertised via `tools/list` now restate the same duty inline (with a pointer to `help('lifecycle')`), so vendor TUIs see the expectation at the moment they reach for the tool. The `spawning` topic and `topics` index cross-reference the new content. Bundles two already-staged improvements that fall in the same area: - OnChildSpawned primes the snapshot-replay budget for new panes so diff-based vendor TUIs come up clean without a manual Ctrl+W/Ctrl+S refresh. - TODO drops the three items now actioned (prompt-injection preface, agent cleanup duty, opencode→claude view corruption) and keeps the unicode `` entry with the investigation notes. --- CHANGELOG.md | 21 +++++++++++ TODO.md | 9 +++-- internal/app/app.go | 12 +++++++ internal/app/host.go | 34 +++++++++++++++--- internal/app/host_test.go | 76 +++++++++++++++++++++++++++++++++++++++ internal/mcp/protocol.go | 4 +-- 6 files changed, 147 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b82ed..e2c3c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,28 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Sub-agent panes spawned while another diff-based TUI (claude/codex/ + opencode) held focus could come up corrupted because the new child's + first incremental updates targeted cells the host viewport hadn't + populated yet. `OnChildSpawned` now primes the snapshot-replay budget + for the new child, so its first PTY chunks render from the full + styled emulator grid — the same path that fixed the symptom when the + user manually cycled focus with Ctrl+W / Ctrl+S. + ### Added +- New `lifecycle` help topic spelling out that the caller owns the + processes it spawns and should call `close_process` when a sub-agent + or spawned process is no longer needed. The `spawn_agent` and + `spawn_process` tool descriptions advertised via `tools/list` now + call out the same cleanup duty (with a pointer to `help('lifecycle')`), + and the `spawning` help topic and `topics` index cross-reference it. +- `spawn_agent` now prepends a single-line `[system: …]` orientation + block to the sub-agent's first prompt so vendor TUIs (claude, codex, + opencode) know they're a sub-agent, are reminded to `send_message` + their parent with a summary when done, and to clean up processes / + scratchpads they created. Skipped for top-level launches and empty + instructions. - Ctrl+A / Ctrl+D step focus between top-level tabs; Ctrl+W / Ctrl+S step through processes (root + sub-agents) inside the current tab. Recognised in legacy, kitty CSI u, and xterm modifyOtherKeys diff --git a/TODO.md b/TODO.md index a780be1..90fd808 100644 --- a/TODO.md +++ b/TODO.md @@ -1,4 +1,7 @@ - [ ] There's a unicode being displayed in opencode -- [ ] Opencode opening a sub claude agent fucks up the view bigtime. Hitting ctrl + W and ctrl + S to switch back to opencode then back to claude again fixes it -- [ ] Sub agents need to have context injected into their prompt to let them know they're a sub agent or they don't report back -- [ ] Agents should also be informed they should clean up after themselves where possible + - Investigated 2026-05-14: patterm passes ghostty grapheme codepoints + through unchanged (vt/ghostty.go:452-462), so the `` glyph is + most likely the *host* terminal's font fallback for opencode's + Nerd Font private-use codepoints, not a patterm substitution. + Need a concrete reproduction (which codepoint, which host + terminal/font) before changing rendering. diff --git a/internal/app/app.go b/internal/app/app.go index b3f4d9f..6807452 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -330,6 +330,18 @@ func (st *uiState) OnChildSpawned(c *Child) { st.palette.rebuild() st.renderPaletteLocked() } + // Prime the snapshot-replay budget for the new child. Diff-based + // vendor TUIs (claude/codex/opencode) emit incremental updates that + // assume the host display already matches their internal "last + // frame" model. On a fresh spawn the host viewport was just cleared, + // so incremental ops target cells that aren't populated yet — + // leaving the corrupted pane the user works around by toggling + // focus (which routes through repaintFocused). Setting the budget + // here makes the next ~8 PTY chunks render from the full styled + // emulator grid, so the host display tracks the emulator state + // without needing a manual focus cycle. + st.repaintNextPTY = c.ID + st.repaintNextPTYBudget = 8 st.mu.Unlock() // Wipe the viewport area so the previous focused child's PTY diff --git a/internal/app/host.go b/internal/app/host.go index c8e81fd..cf50fbb 100644 --- a/internal/app/host.go +++ b/internal/app/host.go @@ -140,7 +140,8 @@ func (h *toolHost) SpawnAgent(callerID string, args mcp.SpawnAgentArgs) (mcp.Pro if display == "" { display = args.Agent } - c, err := h.launcher.LaunchAgent(p, display, args.AgentInstructions, callerID) + prompt := wrapSubAgentPrompt(args.AgentInstructions, h.sess.FindChild(callerID) != nil) + c, err := h.launcher.LaunchAgent(p, display, prompt, callerID) if err != nil { return mcp.ProcessInfo{}, err } @@ -815,6 +816,25 @@ func (h *toolHost) askForTrust(callerID, presetName, reason string) { h.prompter.promptTrust(callerID, presetName, reason) } +// wrapSubAgentPrompt prepends a one-line orientation block to the +// caller-supplied agent_instructions. patterm injects nothing on its +// own (SPEC §7), but vendor TUIs that learn their role purely from +// their first turn need to be told they're a sub-agent — otherwise +// they finish without reporting back to the parent or cleaning up +// processes/scratchpads they spawned. The block is single-line on +// purpose: writeInput splits on CR/LF, so any embedded newline would +// submit prematurely. +func wrapSubAgentPrompt(instructions string, hasParent bool) string { + if !hasParent { + return instructions + } + if instructions == "" { + return "" + } + const preface = "[system: you are a patterm sub-agent. When your work is done, call send_message to your parent (use whoami to get parent_process_id) with a summary, and close_process / scratchpad cleanup anything you created. See help('conventions').] " + return preface + instructions +} + // applyChromeTrim deletes lines matching any of the given regexes. // SPEC §10 chrome_trim_hints. func applyChromeTrim(txt string, hints []string) string { @@ -894,7 +914,7 @@ func helpFor(topic string) mcp.HelpResponse { case "", "topics": return mcp.HelpResponse{ Topic: "topics", - Content: "Available topics: spawning, inspection, io, coordination, " + + Content: "Available topics: spawning, lifecycle, inspection, io, coordination, " + "scratchpads, timers, readiness, permissions, conventions, topics. " + "Call help(topic) for guidance. Call whoami for your role and the " + "complete tool list available to you.", @@ -902,8 +922,14 @@ 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.", - RelatedTools: []string{"spawn_agent", "spawn_process", "start_process", "restart_process"}, + 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').", + RelatedTools: []string{"spawn_agent", "spawn_process", "start_process", "restart_process", "close_process"}, + } + case "lifecycle": + return mcp.HelpResponse{ + Topic: "lifecycle", + Content: "You own the processes you spawn. When a sub-agent has finished its task (it reports back via send_message, or you've collected what you need from it) call close_process on its process_id to remove the entry and tear down the PTY. Same goes for spawn_process children: command/terminal panes you started are not auto-reclaimed when their work completes. close_process is the normal cleanup path; stop_process(signal) is for sending a signal without removing the entry; start_process re-attaches an exited command preset. Leaving idle sub-agents around wastes vendor tokens and clutters the host — close them as soon as you're done. Sub-agents themselves are reminded (via the [system: …] preface on their first prompt) to clean up anything they created before reporting done.", + RelatedTools: []string{"close_process", "stop_process", "start_process", "list_processes", "get_process_status"}, } case "inspection": return mcp.HelpResponse{ diff --git a/internal/app/host_test.go b/internal/app/host_test.go index 0ceacee..4fb1c84 100644 --- a/internal/app/host_test.go +++ b/internal/app/host_test.go @@ -97,3 +97,79 @@ func TestClassifySendMessageNilCallerRejectsNonTopLevelTarget(t *testing.T) { } } +func TestWrapSubAgentPromptPrependsSystemBlockWhenParented(t *testing.T) { + out := wrapSubAgentPrompt("ship feature X", true) + if !strings.HasPrefix(out, "[system:") { + t.Fatalf("expected prepended [system: …] block, got %q", out) + } + if !strings.Contains(out, "send_message") { + t.Fatalf("wrapper should mention send_message reporting, got %q", out) + } + if !strings.Contains(out, "close_process") || !strings.Contains(out, "scratchpad") { + t.Fatalf("wrapper should mention cleanup (close_process / scratchpad), got %q", out) + } + if !strings.HasSuffix(out, "ship feature X") { + t.Fatalf("wrapper should keep original instructions at the tail, got %q", out) + } + if strings.Contains(out, "\n") || strings.Contains(out, "\r") { + t.Fatalf("wrapper must be single-line (writeInput splits CR/LF), got %q", out) + } +} + +func TestWrapSubAgentPromptPassthroughWhenNoParent(t *testing.T) { + out := wrapSubAgentPrompt("hello", false) + if out != "hello" { + t.Fatalf("expected passthrough for top-level spawn, got %q", out) + } +} + +func TestWrapSubAgentPromptEmptyStaysEmpty(t *testing.T) { + // Empty instructions mean "no inject" upstream; we must not synthesize + // content here or LaunchAgent would type the system block into an + // otherwise-idle agent. + if out := wrapSubAgentPrompt("", true); out != "" { + t.Fatalf("empty instructions should stay empty, got %q", out) + } +} + +func TestHelpLifecycleTopicCoversCleanup(t *testing.T) { + resp := helpFor("lifecycle") + if resp.Topic != "lifecycle" { + t.Fatalf("expected topic=lifecycle, got %q", resp.Topic) + } + for _, kw := range []string{"close_process", "spawn", "sub-agent"} { + if !strings.Contains(resp.Content, kw) { + t.Fatalf("lifecycle help should mention %q, got %q", kw, resp.Content) + } + } + if !containsString(resp.RelatedTools, "close_process") { + t.Fatalf("lifecycle help should list close_process in related tools, got %v", resp.RelatedTools) + } +} + +func TestHelpTopicsIndexListsLifecycle(t *testing.T) { + resp := helpFor("topics") + if !strings.Contains(resp.Content, "lifecycle") { + t.Fatalf("topics index should advertise the lifecycle topic, got %q", resp.Content) + } +} + +func TestHelpSpawningPointsAtLifecycle(t *testing.T) { + resp := helpFor("spawning") + if !strings.Contains(resp.Content, "lifecycle") { + t.Fatalf("spawning help should reference the lifecycle topic, got %q", resp.Content) + } + if !containsString(resp.RelatedTools, "close_process") { + t.Fatalf("spawning help should include close_process in related tools, got %v", resp.RelatedTools) + } +} + +func containsString(haystack []string, needle string) bool { + for _, s := range haystack { + if s == needle { + return true + } + } + return false +} + diff --git a/internal/mcp/protocol.go b/internal/mcp/protocol.go index 0266eee..7616b49 100644 --- a/internal/mcp/protocol.go +++ b/internal/mcp/protocol.go @@ -80,7 +80,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.", + 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').", 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."), @@ -89,7 +89,7 @@ func toolCatalog() []toolDescriptor { }, { Name: "spawn_process", - Description: "Spawn a process: a terminal, a process preset, or a freeform argv command.", + Description: "Spawn a process: a terminal, a process preset, or a freeform argv command. Caller owns lifecycle: when the process is no longer needed, call close_process to remove its entry (live children are SIGKILL'd first). See help('lifecycle').", InputSchema: objectSchema(map[string]any{ "kind": stringProp("\"terminal\" or \"command\"."), "preset": stringProp("Process preset name (mutually exclusive with argv)."),