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.
This commit is contained in:
@@ -11,6 +11,9 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
|||||||
to remove a shared project scratchpad.
|
to remove a shared project scratchpad.
|
||||||
|
|
||||||
### Fixed
|
### 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
|
- Sidebar timer indicators now repaint as their visible countdown
|
||||||
value changes, so labels progress from minutes to seconds without
|
value changes, so labels progress from minutes to seconds without
|
||||||
waiting for unrelated terminal output or focus changes.
|
waiting for unrelated terminal output or focus changes.
|
||||||
|
|||||||
1
TODO.md
1
TODO.md
@@ -1,7 +1,6 @@
|
|||||||
- [ ] We should deduplicate /r/n newlines or /n newlines to save tokens on mcp responses for terminal reads.
|
- [ ] 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]
|
- [ ] Codex idle detection seems to trigger too soon, see below [CODEX IDLE]
|
||||||
- [ ] Issue with mcp timing out [MCP TIMEOUT]
|
- [ ] 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.
|
- [ ] 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?
|
- 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: when it fails, is a Codex startup popup visible (trust/workspace, auth/model selection, permissions), or is the normal composer focused?
|
||||||
|
|||||||
@@ -2345,11 +2345,9 @@ func (st *uiState) handleChildRename(childID, newName string) {
|
|||||||
st.drawStatusLine()
|
st.drawStatusLine()
|
||||||
}
|
}
|
||||||
|
|
||||||
// handleChildClose removes a child entry entirely. For agents this is
|
// handleChildClose removes a child entry entirely for process deletes.
|
||||||
// equivalent to a SIGTERM kill (the entry is ephemeral and disappears
|
// For agent Close, it terminates the PTY with escalation but preserves
|
||||||
// from the session once the PTY exits). For command processes it's
|
// the exited pane so the user can still read the corpse.
|
||||||
// equivalent to the MCP close_process tool: SIGKILL if alive, then
|
|
||||||
// drop the entry so it stops appearing in the switch/restart lists.
|
|
||||||
func (st *uiState) handleChildClose(childID string, kill bool) {
|
func (st *uiState) handleChildClose(childID string, kill bool) {
|
||||||
if childID == "" {
|
if childID == "" {
|
||||||
st.repaintFocused()
|
st.repaintFocused()
|
||||||
@@ -2364,7 +2362,11 @@ func (st *uiState) handleChildClose(childID string, kill bool) {
|
|||||||
if kill {
|
if kill {
|
||||||
_ = st.sess.Close(childID, syscall.SIGKILL)
|
_ = st.sess.Close(childID, syscall.SIGKILL)
|
||||||
} else {
|
} 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.repaintFocused()
|
||||||
st.drawTabBar()
|
st.drawTabBar()
|
||||||
|
|||||||
@@ -267,7 +267,7 @@ func (p *paletteState) buildItems(macro string) []paletteItem {
|
|||||||
out = append(out,
|
out = append(out,
|
||||||
paletteItem{label: "Rename", hint: "rename agent · " + name,
|
paletteItem{label: "Rename", hint: "rename agent · " + name,
|
||||||
action: paletteAction{kind: "agent-rename-form", childID: c.ID}, group: groupFocused},
|
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},
|
action: paletteAction{kind: "agent-close", childID: c.ID}, group: groupFocused},
|
||||||
)
|
)
|
||||||
case KindTerminal:
|
case KindTerminal:
|
||||||
|
|||||||
@@ -395,6 +395,20 @@ func (s *Session) Close(id string, sig syscall.Signal) error {
|
|||||||
return nil
|
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
|
// mintUniqueIDLocked mints an opaque process_id (SPEC §7) and retries
|
||||||
// if it collides with an existing entry. Caller holds s.mu.
|
// if it collides with an existing entry. Caller holds s.mu.
|
||||||
func (s *Session) mintUniqueIDLocked() string {
|
func (s *Session) mintUniqueIDLocked() string {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package app
|
package app
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"strings"
|
||||||
"syscall"
|
"syscall"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"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) {
|
func waitUntilLive(t *testing.T, c *Child) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
deadline := time.Now().Add(5 * time.Second)
|
deadline := time.Now().Add(5 * time.Second)
|
||||||
|
|||||||
Reference in New Issue
Block a user