From 96f7c66d5febb48112dbe5087d579edd1f98ced3 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:23:58 +0100 Subject: [PATCH 1/8] Add scratchpad_delete MCP tool Mirrors the existing scratchpad_* tools end-to-end: catalog schema, dispatch, ToolHost.ScratchpadDelete, and a host method that delegates to scratchpad.Store.Delete and fires scratchpadsChanged() on success so the sidebar refreshes. Missing-pad errors surface rather than being masked. Resolves the [MCP SCRATCHPAD DELETE] TODO item. --- CHANGELOG.md | 4 ++ TODO.md | 69 ++++++++++++++++++++++++++ internal/app/host.go | 14 ++++-- internal/app/scratchpad_delete_test.go | 40 +++++++++++++++ internal/mcp/protocol.go | 7 +++ internal/mcp/tools.go | 17 ++++++- 6 files changed, 146 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e6c497..1f449da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- MCP clients can now call `scratchpad_delete` with a scratchpad name + to remove a shared project scratchpad. + ### Fixed - Sidebar timer indicators now repaint as their visible countdown value changes, so labels progress from minutes to seconds without diff --git a/TODO.md b/TODO.md index 628b485..fc892e3 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,74 @@ +- [ ] We should deduplicate /r/n newlines or /n newlines to save tokens on mcp responses for terminal reads. +- [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE] +- [ ] Issue with mcp timing out [MCP TIMEOUT] +- [ ] I have to run the close action twice on opencode for it to actually close the tab. - [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. - This appears to be inconsistent. Sometimes it works, sometimes it doesn't. Might be because of popups on codex sub agents? - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? - Question: if the message is sitting in the composer, does pressing Enter once manually submit it, or does something else need to be dismissed first? - Question: does this happen with short one-line prompts as well as long/multiline sub-agent instructions? + + + +# [CODEX IDLE] +Crunched for 1m 57s │ + │ Agent Tree +❯ codex-implement is idle. Read your own pane for the [sub-agent:codex-implement] reply. │ ───────────────────────── + │ ▎ ○ claude ⏱ 59m + Called patterm, searched memories (ctrl+o to expand) │ ● codex-implement-2 + │ +● Bash(git -C /home/harry/Dev/popdog/.worktrees/harvester-issue-136-kick-segment-repair log -3 --oneline && git -C /home/harry/Dev/popdog/.worktrees/harvester-issue-13…) │ Issue #136 plan drafted + ⎿  9604013 fix(youtube): finalize segments on ghost-cleanup path (#120) │ in scratchpad; + 6da9125 fix(kick): bump KickCategoryAuditor MaxPerCycle to 500 (#135) │ verification sub-agent + db149e6 fix(twitch): enqueue stream segment repairs (#134) │ + ⎿  Allowed by auto mode classifier │ Scratchpads + │ ───────────────────────── +● No commits in the worktree. Let me check the codex-implement pane. │ 0027-review + │ 569-state + Called patterm (ctrl+o to expand) │ db-infra-options + │ db-perf-batching-plan +● Codex exited mid-update prompting "Please restart Codex." Let me spawn a fresh one — the update is now installed. │ e2e-canonicalisation-ru… + │ issue-136-kick-segment-… + Called patterm 3 times (ctrl+o to expand) │ pg-stat-optimization-de… + │ pg-stat-statements-find… +● Fresh codex-implement-2 spawned. Waiting. │ pr-569-stream-category-… + │ stream-category-cutover… +✻ Sautéed for 55s │ + │ +❯ codex-implement-2 is idle. Read your own pane for the [sub-agent:codex-implement-2] reply. │ + │ +● Bash(git log -3 --oneline && echo "---STATUS---" && git status -s && echo "---BRANCH---" && git branch -vv && echo "---REMOTE---" && git ls-remote origin fix/kick-se…) │ + ⎿  9604013 fix(youtube): finalize segments on ghost-cleanup path (#120) │ + 6da9125 fix(kick): bump KickCategoryAuditor MaxPerCycle to 500 (#135) │ + db149e6 fix(twitch): enqueue stream segment repairs (#134) │ + … +44 lines (ctrl+o to expand) │ + ⎿  Allowed by auto mode classifier │ + ⎿  Interrupted · What should Claude do instead? │ + │ +❯ it's still running. │ + │ +● Got it — re-scheduling the wait. │ + │ + Called patterm (ctrl+o to expand) │ + │ +● Waiting for codex-implement-2 to finish. │ + │ +✻ Sautéed for 12s │ + │ + 7 tasks (5 done, 1 in progress, 1 open) │ + ◼ Spawn codex sub-agent to implement plan │ + ◻ Open PR for issue 136 │ + ✔ Read GitHub issue 136 and gather reference code │ + ✔ Set up worktree for issue 136 implementation │ + ✔ Draft implementation plan │ + … +2 completed │ + + +# [MCP TIMEOUT] +⚙ patterm_send_input [key=enter, kind=key, process_id=p_a6726d, submit=false, tail_mode=stream, text=, wait_ms=1000] │ + │ +⚙ patterm_wait_for_pattern [pattern=Findings|No findings|No issues|Residual risk, process_id=p_a6726d, scope=scrollback, timeout_seconds=300] │ +MCP error -32001: Request timed out │ + │ +⚙ patterm_get_process_status [process_id=p_a6726d] │ +MCP error -32001: Request timed out │ diff --git a/internal/app/host.go b/internal/app/host.go index 651ca91..370ca31 100644 --- a/internal/app/host.go +++ b/internal/app/host.go @@ -832,6 +832,14 @@ func (h *toolHost) ScratchpadAppend(name, content string) error { return err } +func (h *toolHost) ScratchpadDelete(name string) error { + err := h.pads.Delete(name) + if err == nil && h.scratch != nil { + h.scratch.scratchpadsChanged() + } + return err +} + func (h *toolHost) WhoAmI(callerID string) mcp.WhoAmI { w := mcp.WhoAmI{ ProcessID: callerID, @@ -1091,7 +1099,7 @@ func availableToolsForRole(role mcp.CallerRole) []string { "send_input", "send_message", "request_human_attention", "timer_wait", "timer_set", "timer_fire_when_idle_any", "timer_fire_when_idle_all", "timer_cancel", "timer_pause", "timer_resume", "timer_list", - "scratchpad_list", "scratchpad_read", "scratchpad_write", "scratchpad_append", + "scratchpad_list", "scratchpad_read", "scratchpad_write", "scratchpad_append", "scratchpad_delete", "whoami", "help", } if role == mcp.RoleOrchestrator { @@ -1146,8 +1154,8 @@ func helpFor(topic string) mcp.HelpResponse { case "scratchpads": return mcp.HelpResponse{ Topic: "scratchpads", - Content: "Project-scoped markdown files. Read returns content + revision; pass that back as expected_revision on write to get last-write-wins-with-detection. Append is unconditional.", - RelatedTools: []string{"scratchpad_list", "scratchpad_read", "scratchpad_write", "scratchpad_append"}, + Content: "Project-scoped markdown files. Read returns content + revision; pass that back as expected_revision on write to get last-write-wins-with-detection. Append is unconditional; delete removes a pad by name.", + RelatedTools: []string{"scratchpad_list", "scratchpad_read", "scratchpad_write", "scratchpad_append", "scratchpad_delete"}, } case "timers": return mcp.HelpResponse{ diff --git a/internal/app/scratchpad_delete_test.go b/internal/app/scratchpad_delete_test.go index 50d0f64..6074098 100644 --- a/internal/app/scratchpad_delete_test.go +++ b/internal/app/scratchpad_delete_test.go @@ -1,10 +1,12 @@ package app import ( + "errors" "io" "os" "testing" + "github.com/hjbdev/patterm/internal/preset" "github.com/hjbdev/patterm/internal/scratchpad" ) @@ -95,3 +97,41 @@ func TestDeletingLastFocusedScratchpadFocusesRunningChild(t *testing.T) { t.Fatalf("focusedID = %q, want pid", st.focusedID) } } + +type scratchpadChangeRecorder struct { + count int +} + +func (r *scratchpadChangeRecorder) scratchpadsChanged() { + r.count++ +} + +func TestToolHostScratchpadDeleteRemovesPadAndRefreshes(t *testing.T) { + t.Setenv("XDG_DATA_HOME", t.TempDir()) + pads, err := scratchpad.Open("scratchpad-delete-host-test") + if err != nil { + t.Fatalf("scratchpad.Open: %v", err) + } + if _, err := pads.Write("doomed.md", "content", ""); err != nil { + t.Fatalf("write doomed.md: %v", err) + } + recorder := &scratchpadChangeRecorder{} + host := newToolHost(nil, pads, nil, preset.Set{}, nil, 120, 40) + host.scratch = recorder + + if err := host.ScratchpadDelete("doomed.md"); err != nil { + t.Fatalf("ScratchpadDelete: %v", err) + } + if recorder.count != 1 { + t.Fatalf("scratchpadsChanged calls = %d, want 1", recorder.count) + } + if _, _, err := pads.Read("doomed.md"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("read deleted pad error = %v, want os.ErrNotExist", err) + } + if err := host.ScratchpadDelete("doomed.md"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("delete missing error = %v, want os.ErrNotExist", err) + } + if recorder.count != 1 { + t.Fatalf("scratchpadsChanged calls after failed delete = %d, want 1", recorder.count) + } +} diff --git a/internal/mcp/protocol.go b/internal/mcp/protocol.go index 73cb65b..73e32f0 100644 --- a/internal/mcp/protocol.go +++ b/internal/mcp/protocol.go @@ -358,6 +358,13 @@ func toolCatalog() []toolDescriptor { "content": stringProp("Text to append."), }, []string{"name", "content"}), }, + { + Name: "scratchpad_delete", + Description: "Delete a scratchpad entry.", + InputSchema: objectSchema(map[string]any{ + "name": stringProp("Scratchpad name."), + }, []string{"name"}), + }, { Name: "whoami", Description: "Return the caller's identity, role, parent, project metadata, and available tools.", diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index cffd2cc..f005026 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -101,6 +101,7 @@ type ToolHost interface { ScratchpadRead(name string) (content string, revision string, err error) ScratchpadWrite(name, content, expectedRevision string) (revision string, err error) ScratchpadAppend(name, content string) error + ScratchpadDelete(name string) error // Meta. WhoAmI(callerID string) WhoAmI @@ -244,8 +245,8 @@ type TimerInfo struct { ID string `json:"timer_id"` Label string `json:"label,omitempty"` Body string `json:"body,omitempty"` - Kind string `json:"kind"` // "delay" | "idle_any" | "idle_all" - Status string `json:"status"` // "pending" | "paused" + Kind string `json:"kind"` // "delay" | "idle_any" | "idle_all" + Status string `json:"status"` // "pending" | "paused" OwnerID string `json:"owner_process_id"` WatchedIDs []string `json:"watched,omitempty"` FiresAtUnixMS int64 `json:"fires_at_unix_ms,omitempty"` @@ -776,6 +777,18 @@ func callTool(h ToolHost, callerID, method string, params json.RawMessage) (any, } return map[string]any{"ok": true}, 0, "", nil + case "scratchpad_delete": + var p struct { + Name string `json:"name"` + } + if err := unmarshalParams(params, &p); err != nil { + return nil, codeInvalidParams, err.Error(), nil + } + if err := h.ScratchpadDelete(p.Name); err != nil { + return nil, codeInternal, err.Error(), nil + } + return map[string]any{"ok": true}, 0, "", nil + case "whoami": return h.WhoAmI(callerID), 0, "", nil From 50fd7be70dcd63e3cddba036aefffb813f44b010 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:30:13 +0100 Subject: [PATCH 2/8] Escalate agent Close to SIGKILL so it terminates in one action Agent 'Close' (agent-close) sent a single SIGTERM via Session.Kill and never escalated, so an agent that traps/ignores SIGTERM (e.g. opencode) stayed in the running tab bar until the user closed it again. Add Session.Terminate, which reuses terminateAndWait (SIGTERM, wait, then SIGKILL) but preserves the session entry so the exited pane stays readable, and route handleChildClose's agent path through it in a goroutine to keep the UI responsive during the stop timeout. Resolves the opencode double-close TODO item. --- CHANGELOG.md | 3 +++ TODO.md | 1 - internal/app/app.go | 14 ++++++----- internal/app/palette.go | 2 +- internal/app/session.go | 14 +++++++++++ internal/app/session_test.go | 45 ++++++++++++++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f449da..e4940a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). to remove a shared project scratchpad. ### Fixed +- Closing an agent now escalates from SIGTERM to SIGKILL when needed, + so agents that ignore SIGTERM disappear from the running tab bar + after one Close action while keeping their exited pane readable. - Sidebar timer indicators now repaint as their visible countdown value changes, so labels progress from minutes to seconds without waiting for unrelated terminal output or focus changes. diff --git a/TODO.md b/TODO.md index fc892e3..761f3b5 100644 --- a/TODO.md +++ b/TODO.md @@ -1,7 +1,6 @@ - [ ] We should deduplicate /r/n newlines or /n newlines to save tokens on mcp responses for terminal reads. - [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE] - [ ] Issue with mcp timing out [MCP TIMEOUT] -- [ ] I have to run the close action twice on opencode for it to actually close the tab. - [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. - This appears to be inconsistent. Sometimes it works, sometimes it doesn't. Might be because of popups on codex sub agents? - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? diff --git a/internal/app/app.go b/internal/app/app.go index 92ce784..fcdaf28 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -2345,11 +2345,9 @@ func (st *uiState) handleChildRename(childID, newName string) { st.drawStatusLine() } -// handleChildClose removes a child entry entirely. For agents this is -// equivalent to a SIGTERM kill (the entry is ephemeral and disappears -// from the session once the PTY exits). For command processes it's -// equivalent to the MCP close_process tool: SIGKILL if alive, then -// drop the entry so it stops appearing in the switch/restart lists. +// handleChildClose removes a child entry entirely for process deletes. +// For agent Close, it terminates the PTY with escalation but preserves +// the exited pane so the user can still read the corpse. func (st *uiState) handleChildClose(childID string, kill bool) { if childID == "" { st.repaintFocused() @@ -2364,7 +2362,11 @@ func (st *uiState) handleChildClose(childID string, kill bool) { if kill { _ = st.sess.Close(childID, syscall.SIGKILL) } else { - _ = st.sess.Kill(childID, syscall.SIGTERM) + go func() { + if err := st.sess.Terminate(childID, syscall.SIGTERM); err != nil { + logf("terminate child %s: %v", childID, err) + } + }() } st.repaintFocused() st.drawTabBar() diff --git a/internal/app/palette.go b/internal/app/palette.go index 6cb96d8..76bb8ce 100644 --- a/internal/app/palette.go +++ b/internal/app/palette.go @@ -267,7 +267,7 @@ func (p *paletteState) buildItems(macro string) []paletteItem { out = append(out, paletteItem{label: "Rename", hint: "rename agent · " + name, action: paletteAction{kind: "agent-rename-form", childID: c.ID}, group: groupFocused}, - paletteItem{label: "Close", hint: "close agent · " + name + " (SIGTERM)", + paletteItem{label: "Close", hint: "close agent · " + name + " (SIGTERM, escalates)", action: paletteAction{kind: "agent-close", childID: c.ID}, group: groupFocused}, ) case KindTerminal: diff --git a/internal/app/session.go b/internal/app/session.go index d4d4b3d..b7851f3 100644 --- a/internal/app/session.go +++ b/internal/app/session.go @@ -395,6 +395,20 @@ func (s *Session) Close(id string, sig syscall.Signal) error { return nil } +// Terminate stops a live child with SIGTERM/SIGKILL escalation but +// leaves its session entry intact so callers can keep showing the +// exited pane. +func (s *Session) Terminate(id string, sig syscall.Signal) error { + c := s.FindChild(id) + if c == nil { + return fmt.Errorf("no such process %q", id) + } + if c.IsLive() { + terminateAndWait(c, sig, childStopTimeout) + } + return nil +} + // mintUniqueIDLocked mints an opaque process_id (SPEC §7) and retries // if it collides with an existing entry. Caller holds s.mu. func (s *Session) mintUniqueIDLocked() string { diff --git a/internal/app/session_test.go b/internal/app/session_test.go index 7e253a8..2f90d96 100644 --- a/internal/app/session_test.go +++ b/internal/app/session_test.go @@ -1,6 +1,7 @@ package app import ( + "strings" "syscall" "testing" "time" @@ -101,6 +102,50 @@ func TestSpawnInstallsIdleDetectionBeforePublish(t *testing.T) { } } +func TestTerminateEscalatesWithoutRemovingEntry(t *testing.T) { + sess := NewSession(t.TempDir(), "test") + c, err := sess.Spawn(SpawnSpec{ + Kind: KindAgent, + Argv: []string{"sh", "-c", "trap '' TERM; echo ready; while :; do sleep 1; done"}, + }, 80, 24) + if err != nil { + t.Fatalf("spawn: %v", err) + } + t.Cleanup(func() { + if c.IsLive() { + _ = c.signal(syscall.SIGKILL) + } + }) + waitUntilLive(t, c) + waitForStreamText(t, c, "ready") + + start := time.Now() + if err := sess.Terminate(c.ID, syscall.SIGTERM); err != nil { + t.Fatalf("Terminate: %v", err) + } + if elapsed := time.Since(start); elapsed < childStopTimeout { + t.Fatalf("Terminate returned before SIGKILL fallback: elapsed=%s timeout=%s", elapsed, childStopTimeout) + } + waitUntilNotLive(t, c) + + if got := sess.FindChild(c.ID); got == nil { + t.Fatalf("Terminate removed child entry %s", c.ID) + } +} + +func waitForStreamText(t *testing.T, c *Child, want string) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + b, _ := c.StreamRead(0) + if strings.Contains(string(b), want) { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("child %s never wrote %q", c.ID, want) +} + func waitUntilLive(t *testing.T, c *Child) { t.Helper() deadline := time.Now().Add(5 * time.Second) From 53f06b604f2b7a6528e9439f1363010c317ae9a9 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:33:59 +0100 Subject: [PATCH 3/8] Normalize whitespace in grid get_process_output to save tokens Grid snapshots pad every row to the full terminal width and leave the bottom of the screen blank, so MCP grid reads carried a lot of dead whitespace. Add normalizeGridText (CRLF/lone-CR to LF, right-trim each line, collapse blank runs to a single blank, drop leading/trailing blanks) and apply it to the grid branch of GetProcessOutput only. Stream output, raw output, and WaitForPattern matching are untouched. Resolves the terminal-read newline/token-waste TODO item. --- CHANGELOG.md | 5 +++++ TODO.md | 1 - internal/app/host.go | 27 +++++++++++++++++++++++++- internal/app/ring_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4940a5..8fdb4c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - MCP clients can now call `scratchpad_delete` with a scratchpad name to remove a shared project scratchpad. +### Changed +- Grid-mode `get_process_output` now returns whitespace-normalized + text to avoid sending padded terminal rows and repeated blank lines + over MCP. + ### Fixed - Closing an agent now escalates from SIGTERM to SIGKILL when needed, so agents that ignore SIGTERM disappear from the running tab bar diff --git a/TODO.md b/TODO.md index 761f3b5..e832166 100644 --- a/TODO.md +++ b/TODO.md @@ -1,4 +1,3 @@ -- [ ] We should deduplicate /r/n newlines or /n newlines to save tokens on mcp responses for terminal reads. - [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE] - [ ] Issue with mcp timing out [MCP TIMEOUT] - [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. diff --git a/internal/app/host.go b/internal/app/host.go index 370ca31..4f5ad49 100644 --- a/internal/app/host.go +++ b/internal/app/host.go @@ -7,6 +7,7 @@ import ( "sync" "syscall" "time" + "unicode" "github.com/hjbdev/patterm/internal/mcp" "github.com/hjbdev/patterm/internal/preset" @@ -398,7 +399,7 @@ func (h *toolHost) GetProcessOutput(callerID, processID, mode string, sinceOffse if c.Kind == KindAgent { txt = applyChromeTrim(txt, h.chromeHintsFor(c.PresetRef)) } - out.Content = txt + out.Content = normalizeGridText(txt) return out, nil case "stream": b, end := c.StreamRead(sinceOffset) @@ -1018,6 +1019,30 @@ func stripANSI(s string) string { return ansiRegexp.ReplaceAllString(s, "") } +func normalizeGridText(s string) string { + s = strings.ReplaceAll(s, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + + lines := strings.Split(s, "\n") + out := make([]string, 0, len(lines)) + pendingBlank := false + for _, line := range lines { + line = strings.TrimRightFunc(line, unicode.IsSpace) + if line == "" { + if len(out) > 0 { + pendingBlank = true + } + continue + } + if pendingBlank { + out = append(out, "") + pendingBlank = false + } + out = append(out, line) + } + return strings.Join(out, "\n") +} + // stripANSIBytes is the byte-slice form of stripANSI. Skips the // string conversion and the regex DFA — useful when the caller will // itself walk the result line-by-line (SearchOutput) or feed it to a diff --git a/internal/app/ring_test.go b/internal/app/ring_test.go index ec2424a..f166f1c 100644 --- a/internal/app/ring_test.go +++ b/internal/app/ring_test.go @@ -104,3 +104,44 @@ func TestStripANSIBytesEquivalence(t *testing.T) { } } } + +func TestNormalizeGridText(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + { + name: "line endings", + in: "one\r\ntwo\rthree", + want: "one\ntwo\nthree", + }, + { + name: "trailing whitespace", + in: "one \ntwo\t\t\nthree", + want: "one\ntwo\nthree", + }, + { + name: "collapse blank runs", + in: "one\n\n\n two\n \n\t\nthree", + want: "one\n\n two\n\nthree", + }, + { + name: "trim leading and trailing blanks", + in: "\n \n\t\none\n\n", + want: "one", + }, + { + name: "already clean", + in: "one\n\ntwo\nthree", + want: "one\n\ntwo\nthree", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := normalizeGridText(tc.in); got != tc.want { + t.Fatalf("normalizeGridText(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} From 7b5a22618f8454d0b97a232c95ebd5915f8b4606 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:39:31 +0100 Subject: [PATCH 4/8] Dispatch MCP requests concurrently per connection handleConn processed requests serially, so a slow tool (e.g. wait_for_pattern with a 300s timeout) monopolized the single per-agent MCP connection and every queued call timed out behind it. Handle each request in its own goroutine, serialize responses through a per-conn write mutex (full response written atomically, partial writes handled), copy the request line before handing it off (bufio reuses its buffer), and wait on a WaitGroup before closing the conn so in-flight handlers finish cleanly. Greeting stays sequential; notifications still get no response. Resolves the [MCP TIMEOUT] TODO item. --- CHANGELOG.md | 2 + TODO.md | 11 --- internal/mcp/mcp.go | 47 +++++++--- internal/mcp/mcp_test.go | 190 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 226 insertions(+), 24 deletions(-) create mode 100644 internal/mcp/mcp_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fdb4c2..13a1658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). over MCP. ### Fixed +- Slow MCP tool calls such as `wait_for_pattern` no longer block later + tool calls on the same MCP connection. - Closing an agent now escalates from SIGTERM to SIGKILL when needed, so agents that ignore SIGTERM disappear from the running tab bar after one Close action while keeping their exited pane readable. diff --git a/TODO.md b/TODO.md index e832166..7f9b34a 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,4 @@ - [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE] -- [ ] Issue with mcp timing out [MCP TIMEOUT] - [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. - This appears to be inconsistent. Sometimes it works, sometimes it doesn't. Might be because of popups on codex sub agents? - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? @@ -60,13 +59,3 @@ Crunched for 1m 57s ✔ Set up worktree for issue 136 implementation │ ✔ Draft implementation plan │ … +2 completed │ - - -# [MCP TIMEOUT] -⚙ patterm_send_input [key=enter, kind=key, process_id=p_a6726d, submit=false, tail_mode=stream, text=, wait_ms=1000] │ - │ -⚙ patterm_wait_for_pattern [pattern=Findings|No findings|No issues|Residual risk, process_id=p_a6726d, scope=scrollback, timeout_seconds=300] │ -MCP error -32001: Request timed out │ - │ -⚙ patterm_get_process_status [process_id=p_a6726d] │ -MCP error -32001: Request timed out │ diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index c548ec3..c1df603 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -96,10 +96,34 @@ func (s *Server) acceptLoop() { // identity token (SPEC §10); we resolve it to a child id and stash that // as the caller for every subsequent tool call. func (s *Server) handleConn(conn net.Conn) { - defer conn.Close() + var writeMu sync.Mutex + var wg sync.WaitGroup + defer func() { + wg.Wait() + _ = conn.Close() + }() r := bufio.NewReader(conn) var callerID string + writeResp := func(resp []byte) bool { + if resp == nil { + return true + } + resp = append(resp, '\n') + writeMu.Lock() + defer writeMu.Unlock() + for len(resp) > 0 { + n, err := conn.Write(resp) + if err != nil { + return false + } + if n == 0 { + return false + } + resp = resp[n:] + } + return true + } greeting, err := r.ReadBytes('\n') if err != nil { @@ -115,24 +139,21 @@ func (s *Server) handleConn(conn net.Conn) { } else { // Treat as a real request from an unknown caller. resp := s.dispatch("", greeting) - if resp != nil { - resp = append(resp, '\n') - if _, werr := conn.Write(resp); werr != nil { - return - } + if !writeResp(resp) { + return } } for { line, err := r.ReadBytes('\n') if len(line) > 0 { - resp := s.dispatch(callerID, line) - if resp != nil { - resp = append(resp, '\n') - if _, werr := conn.Write(resp); werr != nil { - return - } - } + req := append([]byte(nil), line...) + wg.Add(1) + go func() { + defer wg.Done() + resp := s.dispatch(callerID, req) + _ = writeResp(resp) + }() } if err != nil { return diff --git a/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go new file mode 100644 index 0000000..066f080 --- /dev/null +++ b/internal/mcp/mcp_test.go @@ -0,0 +1,190 @@ +package mcp + +import ( + "bufio" + "encoding/json" + "fmt" + "net" + "sync" + "syscall" + "testing" + "time" + + "github.com/hjbdev/patterm/internal/scratchpad" +) + +func TestHandleConnDispatchesRequestsConcurrently(t *testing.T) { + serverConn, clientConn := net.Pipe() + t.Cleanup(func() { _ = clientConn.Close() }) + + host := &blockingToolHost{ + waitEntered: make(chan struct{}), + waitRelease: make(chan struct{}), + } + s := &Server{} + s.SetHost(host) + done := make(chan struct{}) + go func() { + s.handleConn(serverConn) + close(done) + }() + + reader := bufio.NewReader(clientConn) + writeLine(t, clientConn, `{"patterm_identity":"ident"}`) + writeLine(t, clientConn, `{"jsonrpc":"2.0","id":1,"method":"wait_for_pattern","params":{"process_id":"p_slow","pattern":"never","timeout_seconds":300}}`) + select { + case <-host.waitEntered: + case <-time.After(time.Second): + t.Fatal("wait_for_pattern did not enter fake host") + } + + writeLine(t, clientConn, `{"jsonrpc":"2.0","id":2,"method":"get_process_status","params":{"process_id":"p_fast"}}`) + fast := readJSONRPCResponse(t, clientConn, reader, time.Second) + if got := string(fast.ID); got != "2" { + t.Fatalf("first response id = %s, want 2; response=%s", got, fast.Raw) + } + if fast.Error != nil { + t.Fatalf("fast response returned error: %+v", fast.Error) + } + + _ = clientConn.SetReadDeadline(time.Now().Add(50 * time.Millisecond)) + if line, err := reader.ReadBytes('\n'); err == nil { + t.Fatalf("slow response arrived before release: %s", line) + } + + close(host.waitRelease) + slow := readJSONRPCResponse(t, clientConn, reader, time.Second) + if got := string(slow.ID); got != "1" { + t.Fatalf("second response id = %s, want 1; response=%s", got, slow.Raw) + } + if slow.Error != nil { + t.Fatalf("slow response returned error: %+v", slow.Error) + } + + _ = clientConn.Close() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("handleConn did not exit after client close") + } +} + +type jsonRPCResponse struct { + Raw string + ID json.RawMessage `json:"id"` + Result map[string]any `json:"result"` + Error *jsonRPCErrorShape `json:"error"` +} + +type jsonRPCErrorShape struct { + Code int `json:"code"` + Message string `json:"message"` +} + +func writeLine(t *testing.T, conn net.Conn, line string) { + t.Helper() + _ = conn.SetWriteDeadline(time.Now().Add(time.Second)) + if _, err := fmt.Fprintln(conn, line); err != nil { + t.Fatalf("write %s: %v", line, err) + } +} + +func readJSONRPCResponse(t *testing.T, conn net.Conn, reader *bufio.Reader, timeout time.Duration) jsonRPCResponse { + t.Helper() + _ = conn.SetReadDeadline(time.Now().Add(timeout)) + line, err := reader.ReadBytes('\n') + if err != nil { + t.Fatalf("read response: %v", err) + } + var resp jsonRPCResponse + resp.Raw = string(line) + if err := json.Unmarshal(line, &resp); err != nil { + t.Fatalf("parse response %s: %v", line, err) + } + return resp +} + +type blockingToolHost struct { + waitEntered chan struct{} + waitRelease chan struct{} + waitOnce sync.Once +} + +func (h *blockingToolHost) ResolveCallerIdentity(identity string) string { return "caller-" + identity } +func (h *blockingToolHost) CallerRole(string) CallerRole { return RoleOrchestrator } +func (h *blockingToolHost) SpawnAgent(string, SpawnAgentArgs) (ProcessInfo, error) { + return ProcessInfo{}, nil +} +func (h *blockingToolHost) SpawnProcess(string, SpawnProcessArgs) (ProcessInfo, error) { + return ProcessInfo{}, nil +} +func (h *blockingToolHost) StartProcess(string, string) (ProcessInfo, error) { + return ProcessInfo{}, nil +} +func (h *blockingToolHost) RestartProcess(string, string, syscall.Signal) (ProcessInfo, error) { + return ProcessInfo{}, nil +} +func (h *blockingToolHost) StopProcess(string, string, syscall.Signal) (ProcessInfo, error) { + return ProcessInfo{}, nil +} +func (h *blockingToolHost) CloseProcess(string, string) error { return nil } +func (h *blockingToolHost) RenameProcess(string, string, string) error { return nil } +func (h *blockingToolHost) SelectProcess(string, string) error { return nil } +func (h *blockingToolHost) ListProcesses(string, string) []ProcessInfo { return nil } +func (h *blockingToolHost) GetProcessStatus(string, string) (ProcessStatus, error) { + return ProcessStatus{ProcessInfo: ProcessInfo{ID: "p_fast", Status: "running"}}, nil +} +func (h *blockingToolHost) GetProjectStatus(string) (ProjectStatus, error) { + return ProjectStatus{}, nil +} +func (h *blockingToolHost) GetProcessOutput(string, string, string, int64) (ProcessOutput, error) { + return ProcessOutput{}, nil +} +func (h *blockingToolHost) GetProcessRawOutput(string, string, int64) (RawOutput, error) { + return RawOutput{}, nil +} +func (h *blockingToolHost) SearchOutput(string, string, string, string, int) (SearchResult, error) { + return SearchResult{}, nil +} +func (h *blockingToolHost) WaitForPattern(string, string, string, float64, string) (bool, string, error) { + h.waitOnce.Do(func() { close(h.waitEntered) }) + <-h.waitRelease + return true, "matched", nil +} +func (h *blockingToolHost) GetProcessPorts(string, string) ([]PortSighting, error) { + return nil, nil +} +func (h *blockingToolHost) SendInput(string, SendInputArgs) (SendInputResult, error) { + return SendInputResult{}, nil +} +func (h *blockingToolHost) SendMessage(string, string, string) error { return nil } +func (h *blockingToolHost) RequestHumanAttention(string, string, string) error { return nil } +func (h *blockingToolHost) TimerWait(string, float64, string) (string, error) { + return "", nil +} +func (h *blockingToolHost) TimerSet(string, TimerSetArgs) (TimerHandle, error) { + return TimerHandle{}, nil +} +func (h *blockingToolHost) TimerFireWhenIdleAny(string, TimerFireWhenIdleArgs) (TimerFireWhenIdleResponse, error) { + return TimerFireWhenIdleResponse{}, nil +} +func (h *blockingToolHost) TimerFireWhenIdleAll(string, TimerFireWhenIdleArgs) (TimerFireWhenIdleResponse, error) { + return TimerFireWhenIdleResponse{}, nil +} +func (h *blockingToolHost) TimerCancel(string, string) error { return nil } +func (h *blockingToolHost) TimerPause(string, string) error { return nil } +func (h *blockingToolHost) TimerResume(string, string) error { return nil } +func (h *blockingToolHost) TimerList(string) ([]TimerInfo, error) { + return nil, nil +} +func (h *blockingToolHost) ScratchpadList() ([]scratchpad.Entry, error) { return nil, nil } +func (h *blockingToolHost) ScratchpadRead(string) (string, string, error) { + return "", "", nil +} +func (h *blockingToolHost) ScratchpadWrite(string, string, string) (string, error) { + return "", nil +} +func (h *blockingToolHost) ScratchpadAppend(string, string) error { return nil } +func (h *blockingToolHost) ScratchpadDelete(string) error { return nil } +func (h *blockingToolHost) WhoAmI(string) WhoAmI { return WhoAmI{} } +func (h *blockingToolHost) Help(string, string) HelpResponse { return HelpResponse{} } From 3022e4adeb751f4d9098e4768691bc34569e172e Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:40:23 +0100 Subject: [PATCH 5/8] Track per-tab summary visibility TODO --- TODO.md | 1 + 1 file changed, 1 insertion(+) diff --git a/TODO.md b/TODO.md index 7f9b34a..9a1ee9c 100644 --- a/TODO.md +++ b/TODO.md @@ -4,6 +4,7 @@ - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? - Question: if the message is sitting in the composer, does pressing Enter once manually submit it, or does something else need to be dismissed first? - Question: does this happen with short one-line prompts as well as long/multiline sub-agent instructions? +- [ ] The per-tab agent summary text should display below the tab always, not just when the tab is focused. From 0725375755471569b29ec8d6333ee164bb3d6e19 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:43:56 +0100 Subject: [PATCH 6/8] Hold codex in thinking while a turn is running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex uses the osc_title_stability idle strategy, but it draws its progress in the pane body ('Working … esc to interrupt'), not the OSC title. The title goes stable mid-turn, so ~2s later the classifier declared codex idle while it was still working. Add a thinking-promoter pattern ((?i)esc to interrupt) to the codex built-in preset; classify() checks promoter regexes against the rendered screen before the title-stability verdict, so codex stays in thinking until the turn's in-progress footer actually disappears. Resolves the [CODEX IDLE] TODO item. --- CHANGELOG.md | 2 ++ TODO.md | 56 ---------------------------------- internal/app/idle_test.go | 15 +++++++++ internal/preset/preset.go | 5 ++- internal/preset/preset_test.go | 7 +++++ 5 files changed, 28 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13a1658..ae66ac0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). over MCP. ### Fixed +- Codex agents are no longer reported idle while a turn is still + running. - Slow MCP tool calls such as `wait_for_pattern` no longer block later tool calls on the same MCP connection. - Closing an agent now escalates from SIGTERM to SIGKILL when needed, diff --git a/TODO.md b/TODO.md index 9a1ee9c..e409838 100644 --- a/TODO.md +++ b/TODO.md @@ -1,62 +1,6 @@ -- [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE] - [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. - This appears to be inconsistent. Sometimes it works, sometimes it doesn't. Might be because of popups on codex sub agents? - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? - Question: if the message is sitting in the composer, does pressing Enter once manually submit it, or does something else need to be dismissed first? - Question: does this happen with short one-line prompts as well as long/multiline sub-agent instructions? - [ ] The per-tab agent summary text should display below the tab always, not just when the tab is focused. - - - -# [CODEX IDLE] -Crunched for 1m 57s │ - │ Agent Tree -❯ codex-implement is idle. Read your own pane for the [sub-agent:codex-implement] reply. │ ───────────────────────── - │ ▎ ○ claude ⏱ 59m - Called patterm, searched memories (ctrl+o to expand) │ ● codex-implement-2 - │ -● Bash(git -C /home/harry/Dev/popdog/.worktrees/harvester-issue-136-kick-segment-repair log -3 --oneline && git -C /home/harry/Dev/popdog/.worktrees/harvester-issue-13…) │ Issue #136 plan drafted - ⎿  9604013 fix(youtube): finalize segments on ghost-cleanup path (#120) │ in scratchpad; - 6da9125 fix(kick): bump KickCategoryAuditor MaxPerCycle to 500 (#135) │ verification sub-agent - db149e6 fix(twitch): enqueue stream segment repairs (#134) │ - ⎿  Allowed by auto mode classifier │ Scratchpads - │ ───────────────────────── -● No commits in the worktree. Let me check the codex-implement pane. │ 0027-review - │ 569-state - Called patterm (ctrl+o to expand) │ db-infra-options - │ db-perf-batching-plan -● Codex exited mid-update prompting "Please restart Codex." Let me spawn a fresh one — the update is now installed. │ e2e-canonicalisation-ru… - │ issue-136-kick-segment-… - Called patterm 3 times (ctrl+o to expand) │ pg-stat-optimization-de… - │ pg-stat-statements-find… -● Fresh codex-implement-2 spawned. Waiting. │ pr-569-stream-category-… - │ stream-category-cutover… -✻ Sautéed for 55s │ - │ -❯ codex-implement-2 is idle. Read your own pane for the [sub-agent:codex-implement-2] reply. │ - │ -● Bash(git log -3 --oneline && echo "---STATUS---" && git status -s && echo "---BRANCH---" && git branch -vv && echo "---REMOTE---" && git ls-remote origin fix/kick-se…) │ - ⎿  9604013 fix(youtube): finalize segments on ghost-cleanup path (#120) │ - 6da9125 fix(kick): bump KickCategoryAuditor MaxPerCycle to 500 (#135) │ - db149e6 fix(twitch): enqueue stream segment repairs (#134) │ - … +44 lines (ctrl+o to expand) │ - ⎿  Allowed by auto mode classifier │ - ⎿  Interrupted · What should Claude do instead? │ - │ -❯ it's still running. │ - │ -● Got it — re-scheduling the wait. │ - │ - Called patterm (ctrl+o to expand) │ - │ -● Waiting for codex-implement-2 to finish. │ - │ -✻ Sautéed for 12s │ - │ - 7 tasks (5 done, 1 in progress, 1 open) │ - ◼ Spawn codex sub-agent to implement plan │ - ◻ Open PR for issue 136 │ - ✔ Read GitHub issue 136 and gather reference code │ - ✔ Set up worktree for issue 136 implementation │ - ✔ Draft implementation plan │ - … +2 completed │ diff --git a/internal/app/idle_test.go b/internal/app/idle_test.go index 784adf4..37dfaf5 100644 --- a/internal/app/idle_test.go +++ b/internal/app/idle_test.go @@ -57,6 +57,21 @@ func TestClassifyTitleStability(t *testing.T) { } } +func TestClassifyTitleStabilityThinkingPatternOverridesIdle(t *testing.T) { + cfg := &resolvedIdleDetection{ + strategy: StrategyOSCTitleStability, + idleThresholdMS: 2000, + thinkingRegexes: []*regexp.Regexp{mustCompile(t, `(?i)esc to interrupt`)}, + } + screen := []byte("• Working (5s • esc to interrupt)") + if got, _ := classify(cfg, false, false, 9999, 5000, "codex", nil, screen); got != StateThinking { + t.Fatalf("thinking screen marker: got %q want %q", got, StateThinking) + } + if got, _ := classify(cfg, false, false, 9999, 5000, "codex", nil, []byte(">_")); got != StateIdle { + t.Fatalf("stable title without marker: got %q want %q", got, StateIdle) + } +} + func TestClassifyTitleStatus(t *testing.T) { cfg := &resolvedIdleDetection{ strategy: StrategyOSCTitleStatus, diff --git a/internal/preset/preset.go b/internal/preset/preset.go index 84b5be1..1a04785 100644 --- a/internal/preset/preset.go +++ b/internal/preset/preset.go @@ -352,7 +352,10 @@ func defaultAgentPresets() []*Preset { "ready_signal": { "idle_ms": 1000 }, "idle_detection": { "strategy": "osc_title_stability", - "idle_threshold_ms": 2000 + "idle_threshold_ms": 2000, + "thinking_patterns": [ + "(?i)esc to interrupt" + ] }, "chrome_trim_hints": [ "^OpenAI Codex", diff --git a/internal/preset/preset_test.go b/internal/preset/preset_test.go index 835b837..0ef4e2c 100644 --- a/internal/preset/preset_test.go +++ b/internal/preset/preset_test.go @@ -27,6 +27,13 @@ func TestLoadUsesBuiltInDefaultsWithoutWritingConfig(t *testing.T) { if claude.IdleDetection == nil || len(claude.IdleDetection.PermissionPatterns) == 0 { t.Fatalf("built-in claude missing permission patterns: %+v", claude.IdleDetection) } + codex := presetByName(set.Agents, "codex") + if codex == nil { + t.Fatal("missing built-in codex preset") + } + if codex.IdleDetection == nil || len(codex.IdleDetection.ThinkingPatterns) == 0 { + t.Fatalf("built-in codex missing thinking patterns: %+v", codex.IdleDetection) + } } func TestLoadMergesUserOverlayIntoBuiltInPreset(t *testing.T) { From 178b4437b116e84be2065992d7698e7d541adce3 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 13:00:54 +0100 Subject: [PATCH 7/8] Give injected agent submit Enter a longer settle delay The trailing CR that submits orchestrator-injected input was written only 15ms after the body, inside TUI agents' paste-coalescing window, so codex (and other paste-detecting agents) intermittently swallowed it as a newline and left the message composed but unsent. Centralize the per-piece timing in a pure pieceWriteDelay helper: keep 15ms between body lines but give the final lone Enter a 100ms settle gap so the agent closes the preceding burst and registers the CR as submit. Covers send_input, send_message, timers, and the spawn initial prompt (all go through writeInput). Resolves the codex composer-submit TODO item. --- CHANGELOG.md | 3 ++ TODO.md | 5 --- internal/app/child.go | 23 ++++++++++-- internal/app/child_input_test.go | 61 ++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae66ac0..711034c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). over MCP. ### Fixed +- Injected agent input now sends the submit Enter as a separated, + settled keystroke so messages reliably submit instead of sometimes + sitting unsent in the composer. - Codex agents are no longer reported idle while a turn is still running. - Slow MCP tool calls such as `wait_for_pattern` no longer block later diff --git a/TODO.md b/TODO.md index e409838..b5dae71 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1 @@ -- [ ] When opening a codex sub agent, the message gets input to the field, but the message is never submitted. - - This appears to be inconsistent. Sometimes it works, sometimes it doesn't. Might be because of popups on codex sub agents? - - Question: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused? - - Question: if the message is sitting in the composer, does pressing Enter once manually submit it, or does something else need to be dismissed first? - - Question: does this happen with short one-line prompts as well as long/multiline sub-agent instructions? - [ ] The per-tab agent summary text should display below the tab always, not just when the tab is focused. diff --git a/internal/app/child.go b/internal/app/child.go index 92df21b..91f96f4 100644 --- a/internal/app/child.go +++ b/internal/app/child.go @@ -26,6 +26,11 @@ import ( // false positives (timestamps, exit codes, etc.). var portRegex = regexp.MustCompile(`https?://[^\s:/]+:(\d{2,5})(?:/[^\s]*)?`) +const ( + agentInterPieceDelay = 15 * time.Millisecond + agentSubmitSettleDelay = 100 * time.Millisecond +) + type ChildStatus string const ( @@ -642,8 +647,8 @@ func (c *Child) writeInput(b []byte) error { return err } for i, piece := range pieces { - if i > 0 { - time.Sleep(15 * time.Millisecond) + if delay := pieceWriteDelay(i, len(pieces), piece); delay > 0 { + time.Sleep(delay) } if _, err := pty.Write(piece); err != nil { return err @@ -659,6 +664,20 @@ func inputWritePieces(kind ChildKind, b []byte) [][]byte { return splitOnEnter(b) } +func pieceWriteDelay(index, total int, piece []byte) time.Duration { + if index == 0 { + return 0 + } + if index == total-1 && isLoneEnter(piece) { + return agentSubmitSettleDelay + } + return agentInterPieceDelay +} + +func isLoneEnter(piece []byte) bool { + return len(piece) == 1 && (piece[0] == '\r' || piece[0] == '\n') +} + func mintIdentity() string { var buf [12]byte _, _ = rand.Read(buf[:]) diff --git a/internal/app/child_input_test.go b/internal/app/child_input_test.go index f834457..1e472ca 100644 --- a/internal/app/child_input_test.go +++ b/internal/app/child_input_test.go @@ -3,6 +3,7 @@ package app import ( "bytes" "testing" + "time" ) func TestInputWritePiecesOnlySplitAgentEnters(t *testing.T) { @@ -27,3 +28,63 @@ func TestInputWritePiecesOnlySplitAgentEnters(t *testing.T) { } } } + +func TestPieceWriteDelay(t *testing.T) { + cases := []struct { + name string + index int + total int + piece []byte + want time.Duration + }{ + { + name: "first piece", + index: 0, + total: 3, + piece: []byte("body"), + want: 0, + }, + { + name: "middle body piece", + index: 1, + total: 3, + piece: []byte("body"), + want: agentInterPieceDelay, + }, + { + name: "final carriage return submit", + index: 1, + total: 2, + piece: []byte("\r"), + want: agentSubmitSettleDelay, + }, + { + name: "final newline submit", + index: 1, + total: 2, + piece: []byte("\n"), + want: agentSubmitSettleDelay, + }, + { + name: "final non-enter piece", + index: 2, + total: 3, + piece: []byte("tail"), + want: agentInterPieceDelay, + }, + { + name: "standalone enter fast path", + index: 0, + total: 1, + piece: []byte("\r"), + want: 0, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := pieceWriteDelay(tc.index, tc.total, tc.piece); got != tc.want { + t.Fatalf("pieceWriteDelay(%d, %d, %q) = %s, want %s", tc.index, tc.total, tc.piece, got, tc.want) + } + }) + } +} From d2342f99cf4922b67c0ca50ed27647416fa5102b Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 13:06:53 +0100 Subject: [PATCH 8/8] Show every agent tab's summary, not just the focused one The tab bar's row-2 summary was painted only for the active tab. Add a per-child summaryTextFor/summaryRawFor helper (active variants now delegate to it), carry each tab's childID on its tabRect, and loop over all visible tabs so each renders its own summary under its column. Layout is unchanged (still 3 rows); narrow tabs clip as before. Resolves the per-tab summary TODO item. --- CHANGELOG.md | 2 ++ TODO.md | 1 - internal/app/app.go | 26 +++++++++++++++--------- internal/app/summarizer_test.go | 35 +++++++++++++++++++++++++++++++++ internal/app/tabbar.go | 12 ++++------- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 711034c..0dc40a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). to remove a shared project scratchpad. ### Changed +- The tab bar now shows each visible agent tab's own summary instead + of only rendering the focused tab's summary. - Grid-mode `get_process_output` now returns whitespace-normalized text to avoid sending padded terminal rows and repeated blank lines over MCP. diff --git a/TODO.md b/TODO.md index b5dae71..e69de29 100644 --- a/TODO.md +++ b/TODO.md @@ -1 +0,0 @@ -- [ ] The per-tab agent summary text should display below the tab always, not just when the tab is focused. diff --git a/internal/app/app.go b/internal/app/app.go index fcdaf28..a4a9339 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -514,7 +514,14 @@ func (st *uiState) dbgf(format string, args ...any) { } func (st *uiState) activeSummaryText(width int) string { - text := st.activeSummaryRaw() + st.mu.Lock() + active := st.activeAgentID + st.mu.Unlock() + return st.summaryTextFor(active, width) +} + +func (st *uiState) summaryTextFor(childID string, width int) string { + text := st.summaryRawFor(childID) if text == "" || width <= 0 { return "" } @@ -525,7 +532,14 @@ func (st *uiState) activeSummaryText(width int) string { } func (st *uiState) activeSummaryRaw() string { - if st.summaries == nil { + st.mu.Lock() + active := st.activeAgentID + st.mu.Unlock() + return st.summaryRawFor(active) +} + +func (st *uiState) summaryRawFor(childID string) string { + if st.summaries == nil || childID == "" { return "" } st.settingsMu.Lock() @@ -534,13 +548,7 @@ func (st *uiState) activeSummaryRaw() string { if !enabled { return "" } - st.mu.Lock() - active := st.activeAgentID - st.mu.Unlock() - if active == "" { - return "" - } - sum := st.summaries.Summary(active) + sum := st.summaries.Summary(childID) text := strings.TrimSpace(sum.Text) if text == "" { return "" diff --git a/internal/app/summarizer_test.go b/internal/app/summarizer_test.go index a38fc7f..bbaab62 100644 --- a/internal/app/summarizer_test.go +++ b/internal/app/summarizer_test.go @@ -52,6 +52,41 @@ func TestWrapSidebarSummaryKeepsWordBoundaries(t *testing.T) { } } +func TestSummaryTextForSelectsChildAndClips(t *testing.T) { + sess := NewSession(t.TempDir(), "test") + cfg := defaultSettings() + st := &uiState{ + sess: sess, + settings: cfg, + summaries: newSummaryManager(sess, t.TempDir(), preset.Set{}, func() autoSummarySettings { + return cfg.AutoSummary.clone() + }, nil, nil), + } + st.summaries.mu.Lock() + st.summaries.entries["a1"] = &summaryEntry{state: summaryState{Text: " alpha summary "}} + st.summaries.entries["a2"] = &summaryEntry{state: summaryState{Text: "beta summary"}} + st.summaries.entries["empty"] = &summaryEntry{state: summaryState{Text: " "}} + st.summaries.entries["long"] = &summaryEntry{state: summaryState{Text: "abcdefghijklmnopqrstuvwxyz"}} + st.summaries.mu.Unlock() + + if got := st.summaryTextFor("a2", 20); got != "beta summary" { + t.Fatalf("summaryTextFor(a2) = %q, want beta summary", got) + } + if got := st.summaryTextFor("empty", 20); got != "" { + t.Fatalf("summaryTextFor(empty) = %q, want empty", got) + } + if got := st.summaryTextFor("long", 8); got != "abcdefg…" { + t.Fatalf("summaryTextFor(long) = %q, want abcdefg…", got) + } + + st.settingsMu.Lock() + st.settings.AutoSummary.Enabled = false + st.settingsMu.Unlock() + if got := st.summaryTextFor("a1", 20); got != "" { + t.Fatalf("summaryTextFor disabled = %q, want empty", got) + } +} + func TestSummaryManagerArmsOnlyTrackedTopLevelAgents(t *testing.T) { sess := NewSession(t.TempDir(), "test") c := newChildEntry("a1", "agent", KindAgent, []string{"fake"}, nil, "", "", "") diff --git a/internal/app/tabbar.go b/internal/app/tabbar.go index 55341f4..6672187 100644 --- a/internal/app/tabbar.go +++ b/internal/app/tabbar.go @@ -59,6 +59,7 @@ func (st *uiState) drawTabBar() { newHintW := utf8.RuneCountInString(newHint) + 2 // " + new " framing type tabRect struct { + childID string startCol int width int label string @@ -66,8 +67,6 @@ func (st *uiState) drawTabBar() { glyphStyle string active bool } - activeTab := -1 - // Reserve space at the right edge for "+ new". If there are too // many tabs to fit even at minTabWidth, drop tabs from the right // until they do. The current focus stays visible. @@ -139,6 +138,7 @@ func (st *uiState) drawTabBar() { labelW = utf8.RuneCountInString(label) } tabs = append(tabs, tabRect{ + childID: c.ID, startCol: col, width: w, label: label, @@ -146,9 +146,6 @@ func (st *uiState) drawTabBar() { glyphStyle: glyphStyle, active: active, }) - if tabs[len(tabs)-1].active { - activeTab = len(tabs) - 1 - } col += w } } @@ -224,10 +221,9 @@ func (st *uiState) drawTabBar() { hintCol, styleBorder, strings.Repeat("─", newHintW), styleReset) } - if activeTab >= 0 { - tab := tabs[activeTab] + for _, tab := range tabs { summaryWidth := tab.width - 2 - if summary := st.activeSummaryText(summaryWidth); summary != "" { + if summary := st.summaryTextFor(tab.childID, summaryWidth); summary != "" { fmt.Fprintf(&b, "\x1b[2;%dH %s%s%s", tab.startCol, styleDim, summary, styleReset) } }