Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **A restarted session no longer reads as busy forever.** A session whose
agent died mid-turn, or that was closed mid-turn, came back under the same
card still listed as busy to other sessions: a task sent to it waited for a
turn that would never end. It now starts with no state, like a new one.
- **Codex, Cursor CLI, Antigravity and Crush stop asking where you already
answered.** A sandboxed session of any of them opens straight at its
prompt. Cursor CLI and Antigravity no longer ask again in a worktree of a
Expand Down
19 changes: 19 additions & 0 deletions internal/relay/observe.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,25 @@ func (s *Service) recordState(sessionID, state string) {

}

// SessionExited drops what the session's last process reported, once lich
// has reaped it: a process that dies mid-turn never sends the SessionEnd
// (idle) that would, so its busy outlived it, and the same id respawned read
// as busy to every delivery and in every roster until its own process ended a
// turn. The terminal calls it for a process that ended on its own
// (terminal.SetSessionExited); a close goes through SessionClosed.
func (s *Service) SessionExited(sessionID string) {
s.mu.Lock()
s.forgetStateLocked(sessionID)
s.mu.Unlock()
}

// forgetStateLocked removes a session from the turn map and the roster, as an
// idle report does. Called with s.mu held.
func (s *Service) forgetStateLocked(sessionID string) {
delete(s.state, sessionID)
delete(s.reported, sessionID)
}

// endedErrands takes every ticket the report ends off the table and closes its
// stall channel, for Observe to announce outside the lock. Called with s.mu
// held.
Expand Down
30 changes: 30 additions & 0 deletions internal/relay/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3060,3 +3060,33 @@ func TestSelfRefusesACallerWithoutASession(t *testing.T) {
}
}
}

// A session's process can die mid-turn, and a dead process never reports the
// SessionEnd (idle) that would clear its busy. Once lich has reaped it, or the
// card was closed, the same id respawned is a new process that has reported
// nothing: it must not read busy to deliveries or in the roster.
func TestAReapedOrClosedSessionForgetsItsState(t *testing.T) {
svc := newRelay(workspace(), newFakeTerminal("s1", "s2", "s3"), nil)
svc.Observe("s2", "busy")
svc.Observe("s3", "busy")

svc.SessionExited("s2")
svc.SessionClosed("s3", "")

peers, err := svc.Peers(PeersOptions{From: "s1"})
if err != nil {
t.Fatalf("Peers: %v", err)
}
for _, p := range peers {
if p.State != "" {
t.Errorf("%s state = %q after its process went, want none", p.Name, p.State)
}
}
svc.mu.Lock()
defer svc.mu.Unlock()
for _, id := range []string{"s2", "s3"} {
if turn, ok := svc.state[id]; ok {
t.Errorf("turn state of %s = %q after its process went, want none", id, turn)
}
}
}
4 changes: 3 additions & 1 deletion internal/relay/workeranswer.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,14 @@ func (s *Service) errandOpenAtLocked(sessionID string) bool {
// (WorkerAnswered). The terminal calls this before the process dies
// (terminal.SetSessionClosed), so the SessionEnd its CLI reports on the way out
// finds nothing to call unanswered. Errands of any other kind end as they
// always did.
// always did. The session's reported state goes too: the same id reopened is
// a new process that has reported nothing yet.
func (s *Service) SessionClosed(sessionID, closerID string) {
var stopped []*ticket
var told []string
s.mu.Lock()
delete(s.reportedWorkers, sessionID)
s.forgetStateLocked(sessionID)
for id, t := range s.tickets {
if t.targetID != sessionID || !t.subagent {
continue
Expand Down
50 changes: 50 additions & 0 deletions internal/terminal/exit_watch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Drives a real PTY, so Unix-only like the rest of the service's spawn tests
// (terminal_test.go).
//go:build !windows

package terminal

import (
"slices"
"sync"
"testing"
)

// A process that ends on its own is reported to the exit watcher once it is
// reaped, which is how the relay learns to drop the state that process left
// (relay.SessionExited). A close is not: it reaches the relay through the
// close watcher instead.
func TestAReapedSessionIsReportedToTheExitWatcher(t *testing.T) {
hub, _ := newProbeHub(t)
svc := New(stubBins{bin: exitBin(t, 0)}, nil, hub)
var mu sync.Mutex
var exited []string
svc.SetSessionExited(func(id string) {
mu.Lock()
defer mu.Unlock()
exited = append(exited, id)
})
t.Cleanup(func() { _ = svc.Close("s1") })

if err := svc.Start("s1", "p1", t.TempDir(), "claude", "", "", false, false, 80, 24); err != nil {
t.Fatalf("start: %v", err)
}
waitFor(t, func() bool {
mu.Lock()
defer mu.Unlock()
return slices.Contains(exited, "s1")
}, "the exit watcher to hear s1")

svc2 := New(stubBins{bin: echoBin(t)}, nil, hub)
closed := false
svc2.SetSessionExited(func(string) { closed = true })
if err := svc2.Start("s2", "p1", t.TempDir(), "claude", "", "", false, false, 80, 24); err != nil {
t.Fatalf("start: %v", err)
}
if err := svc2.Close("s2"); err != nil {
t.Fatalf("close: %v", err)
}
if closed {
t.Error("a closed session was reported to the exit watcher")
}
}
6 changes: 6 additions & 0 deletions internal/terminal/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (s *Service) stream(id string, sess *session) {

s.mu.Lock()
reaped := false
exited := s.onExited
if current, ok := s.sessions[id]; ok && current.pty == p {
delete(s.sessions, id)
close(current.done)
Expand Down Expand Up @@ -79,6 +80,11 @@ func (s *Service) stream(id string, sess *session) {
// Emit blocks on a stalled /events client, and holding s.mu across it would
// freeze every session's I/O.
if reaped {
// Outside mu: the relay takes its own lock, and it calls into this
// service under it.
if exited != nil {
exited(id)
}
s.hub.Emit(exitEventPrefix+id, exitPayload(code))
}
}
Expand Down
13 changes: 13 additions & 0 deletions internal/terminal/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,10 @@ type Service struct {
// errand there, ahead of the SessionEnd the dying CLI reports. Guarded by mu
// like onState.
onClosed func(id, closerID string)
// onExited, when set, is told about every session whose process ended on
// its own and was reaped: the relay drops the state that process reported.
// Guarded by mu like onState.
onExited func(id string)
// onLimit, when set, is told about every turn a usage limit ended that names
// its reset (limit.go). Guarded by mu like onState.
onLimit func(id string, resetsAt int64)
Expand Down Expand Up @@ -738,6 +742,15 @@ func (s *Service) SetSessionClosed(fn func(id, closerID string)) {
s.onClosed = fn
}

// SetSessionExited wires fn to every session whose process ended on its own,
// called once lich has reaped it. Only one watcher, the relay
// (relay.SessionExited).
func (s *Service) SetSessionExited(fn func(id string)) {
s.mu.Lock()
defer s.mu.Unlock()
s.onExited = fn
}

// stateWatcher reads the watcher under the lock, so a report arriving while it
// is being wired sees one function or none, never a torn read.
func (s *Service) stateWatcher() func(id, state string) {
Expand Down
9 changes: 7 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ func registerServices(db *store.Service, term *terminal.Service, hub *events.Hub
term.SetWorkerAnswer(rl.WorkerAnswered)
term.SetWorkerUnanswered(rl.WorkerUnanswered)
term.SetSessionClosed(rl.SessionClosed)
term.SetSessionExited(rl.SessionExited)
term.SetErrandStatus(rl.Status)
// Both questions the relay asks about a provider — whether its sessions
// report at all, and whether they can answer with a tool — are about what
Expand Down Expand Up @@ -516,7 +517,9 @@ func registerServices(db *store.Service, term *terminal.Service, hub *events.Hub
// relay.WorkerUnanswered arrives the same way and ends one unanswered.
// - relay.SessionClosed is what the terminal tells the relay as it closes a
// session: called here it ends a running worker's errand, in silence when
// the closer named is the worker's caller.
// the closer named is the worker's caller. relay.SessionExited is what it
// tells the relay as it reaps a process that ended on its own: called here
// it erases a live session's reported state.
// - terminal.CloseBy is a close on behalf of a session, which `lich close`
// reaches through spawn.Close: called here with the worker's caller as the
// closer, it ends that caller's errand without telling it. The window
Expand Down Expand Up @@ -547,7 +550,7 @@ func registerServices(db *store.Service, term *terminal.Service, hub *events.Hub
// store.SetBranchOf, store.SetTranscriptOf, store.SetConversationsOf,
// store.SetCheckoutsOf, terminal.SetDropDir,
// terminal.SetRateLimitReports, terminal.SetUsageLimit, terminal.SetWorkerAnswer,
// terminal.SetWorkerUnanswered, terminal.SetStartReports, terminal.SetPromptLanguage and terminal.SetSessionClosed are startup wiring. Called with [null] they silently
// terminal.SetWorkerUnanswered, terminal.SetStartReports, terminal.SetPromptLanguage, terminal.SetSessionClosed and terminal.SetSessionExited are startup wiring. Called with [null] they silently
// nil what they wired (encoding/json leaves a func or pointer alone on
// null), and the write races the readers already serving — nilling
// SetProjects also disarms the guard that keeps two projects off the same
Expand Down Expand Up @@ -600,6 +603,7 @@ func denyInternal(d *rpc.Handler) {
"relay.WorkerAnswered",
"relay.WorkerUnanswered",
"relay.SessionClosed",
"relay.SessionExited",
"terminal.CloseBy",
"relay.RunSchedules",
"relay.ParkResume",
Expand All @@ -622,6 +626,7 @@ func denyInternal(d *rpc.Handler) {
"terminal.SetWorkerAnswer",
"terminal.SetWorkerUnanswered",
"terminal.SetSessionClosed",
"terminal.SetSessionExited",
"terminal.SetStartReports",
"terminal.SetPromptLanguage",
"terminal.EnqueueModCommand",
Expand Down
Loading