From 50fd7be70dcd63e3cddba036aefffb813f44b010 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Mon, 25 May 2026 12:30:13 +0100 Subject: [PATCH] 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)