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)