fix(terminal): prevent zellij session resurrection

This commit is contained in:
yyovil 2026-06-11 16:38:19 +05:30 committed by GitHub
parent 785f060b71
commit 0ee86a6314
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 200 additions and 42 deletions

View File

@ -50,8 +50,14 @@ func listSessionsArgs() []string {
return []string{"list-sessions", "--no-formatting"}
}
func killSessionArgs(id string) []string {
return []string{"kill-session", id}
// deleteSessionArgs builds the teardown command. `delete-session --force`
// kills a running session AND removes its serialized resurrection state in one
// step. Plain `kill-session` is not enough: zellij can keep the session in its
// global resurrection cache as "(EXITED - attach to resurrect)", and any later
// `zellij attach <id>` (e.g. the terminal mux re-opening a pane) would resurrect
// it — re-running the agent command for a session the daemon already destroyed.
func deleteSessionArgs(id string) []string {
return []string{"delete-session", "--force", id}
}
func attachArgs(id string) []string {

View File

@ -152,14 +152,15 @@ func (r *Runtime) Create(ctx context.Context, cfg ports.RuntimeConfig) (ports.Ru
return ports.RuntimeHandle{ID: handleIDValue(id, paneID)}, nil
}
// Destroy kills the handle's zellij session. An already-gone session is treated
// as success.
// Destroy kills the handle's zellij session and deletes its serialized state,
// so the session can never be resurrected by a later `zellij attach`. An
// already-gone session is treated as success.
func (r *Runtime) Destroy(ctx context.Context, handle ports.RuntimeHandle) error {
id, _, err := handleID(handle)
if err != nil {
return err
}
if _, err := r.run(ctx, killSessionArgs(id)...); err != nil {
if _, err := r.run(ctx, deleteSessionArgs(id)...); err != nil {
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
return nil
@ -204,7 +205,10 @@ func (r *Runtime) GetOutput(ctx context.Context, handle ports.RuntimeHandle, lin
}
// IsAlive reports whether the handle's session still appears in `zellij
// list-sessions`.
// list-sessions`. Only the documented "no sessions exist" failure counts as a
// definitive "not alive"; any other list-sessions failure is reported as a
// probe error so callers (the reaper feeding the LCM) treat it as a failed
// probe, never as proof of death.
func (r *Runtime) IsAlive(ctx context.Context, handle ports.RuntimeHandle) (bool, error) {
id, _, err := handleID(handle)
if err != nil {
@ -213,7 +217,7 @@ func (r *Runtime) IsAlive(ctx context.Context, handle ports.RuntimeHandle) (bool
out, err := r.run(ctx, listSessionsArgs()...)
if err != nil {
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
if errors.As(err, &exitErr) && noActiveSessionsOutput(string(out)) {
return false, nil
}
return false, fmt.Errorf("zellij runtime: probe session %s: %w", id, err)
@ -221,6 +225,15 @@ func (r *Runtime) IsAlive(ctx context.Context, handle ports.RuntimeHandle) (bool
return sessionListedAlive(string(out), id), nil
}
// noActiveSessionsOutput reports whether a non-zero `zellij list-sessions`
// failed because no sessions exist at all — the one exit-error case that is a
// definitive "dead" rather than a probe failure. zellij 0.44 emits either
// "No active zellij sessions found." or "There is no active session!".
func noActiveSessionsOutput(out string) bool {
s := strings.ToLower(out)
return strings.Contains(s, "no active") && strings.Contains(s, "session")
}
// AttachCommand returns the argv a human runs to attach their terminal to the
// session.
func (r *Runtime) AttachCommand(handle ports.RuntimeHandle) ([]string, error) {

View File

@ -42,6 +42,12 @@ func TestCommandBuilders(t *testing.T) {
if got, want := dumpScreenArgs("sess-1", "terminal_0"), []string{"--session", "sess-1", "action", "dump-screen", "--pane-id", "terminal_0", "--full"}; !reflect.DeepEqual(got, want) {
t.Fatalf("dumpScreenArgs = %#v, want %#v", got, want)
}
// delete-session --force (not kill-session): teardown must also purge the
// serialized resurrection state, or a later `zellij attach` re-creates the
// session — and re-runs its agent — after the daemon destroyed it.
if got, want := deleteSessionArgs("sess-1"), []string{"delete-session", "--force", "sess-1"}; !reflect.DeepEqual(got, want) {
t.Fatalf("deleteSessionArgs = %#v, want %#v", got, want)
}
}
func TestZellijSessionNameSanitizesIssueRefs(t *testing.T) {
@ -375,8 +381,11 @@ func TestIsAliveParsesNoFormattingOutput(t *testing.T) {
}
}
func TestIsAliveTreatsExitStatusAsNotAlive(t *testing.T) {
fr := &fakeRunner{err: &exec.ExitError{}}
// IsAlive may treat a non-zero list-sessions ONLY as "not alive" when zellij
// says no sessions exist at all. Any other exit failure is a probe error: the
// reaper reports it as a failed probe and the LCM must never read it as death.
func TestIsAliveTreatsNoSessionsExitAsNotAlive(t *testing.T) {
fr := &fakeRunner{outputs: [][]byte{[]byte("No active zellij sessions found.")}, err: &exec.ExitError{}}
r := New(Options{Timeout: time.Second})
r.runner = fr
@ -389,6 +398,20 @@ func TestIsAliveTreatsExitStatusAsNotAlive(t *testing.T) {
}
}
func TestIsAliveReportsOtherExitFailuresAsProbeErrors(t *testing.T) {
fr := &fakeRunner{outputs: [][]byte{[]byte("thread 'main' panicked")}, err: &exec.ExitError{}}
r := New(Options{Timeout: time.Second})
r.runner = fr
alive, err := r.IsAlive(context.Background(), ports.RuntimeHandle{ID: "sess-1/terminal_0"})
if err == nil {
t.Fatal("IsAlive: got nil error, want probe failure — a failed probe must not read as dead")
}
if alive {
t.Fatal("alive = true on probe failure")
}
}
func TestDestroyIsIdempotentWhenSessionMissing(t *testing.T) {
fr := &fakeRunner{err: &exec.ExitError{}}
r := New(Options{Timeout: time.Second})
@ -397,8 +420,27 @@ func TestDestroyIsIdempotentWhenSessionMissing(t *testing.T) {
if err := r.Destroy(context.Background(), ports.RuntimeHandle{ID: "sess-1/terminal_0"}); err != nil {
t.Fatalf("Destroy: %v", err)
}
if len(fr.calls) != 1 || fr.calls[0].args[0] != "kill-session" {
t.Fatalf("calls = %#v, want only kill-session", fr.calls)
if len(fr.calls) != 1 || fr.calls[0].args[0] != "delete-session" {
t.Fatalf("calls = %#v, want only delete-session", fr.calls)
}
}
// Destroy must delete the session's serialized state, not merely kill it: a
// killed-but-cached session is resurrected (agent re-run included) by any later
// `zellij attach`, bringing a terminated session's runtime back to life.
func TestDestroyForceDeletesSerializedSession(t *testing.T) {
fr := &fakeRunner{}
r := New(Options{Timeout: time.Second})
r.runner = fr
if err := r.Destroy(context.Background(), ports.RuntimeHandle{ID: "sess-1/terminal_0"}); err != nil {
t.Fatalf("Destroy: %v", err)
}
if len(fr.calls) != 1 {
t.Fatalf("calls = %d, want 1", len(fr.calls))
}
if got, want := fr.calls[0].args, []string{"delete-session", "--force", "sess-1"}; !reflect.DeepEqual(got, want) {
t.Fatalf("destroy args = %#v, want %#v", got, want)
}
}

View File

@ -6,6 +6,7 @@ import (
"net/http/httptest"
"runtime"
"strings"
"sync/atomic"
"testing"
"time"
@ -19,13 +20,21 @@ import (
// stubSource attaches a throwaway shell command instead of a real Zellij pane, so
// the /mux path exercises the genuine upgrade + wsjson + Serve + creack/pty flow
// without needing Zellij. IsAlive=false means the pane is treated as gone once the
// command exits (no re-attach).
type stubSource struct{ argv []string }
// without needing Zellij. The pane reports alive until the first attach happens
// (the mux refuses to attach to a dead pane), then dead, so the command's exit is
// treated as the pane being gone (no re-attach).
type stubSource struct {
argv []string
attached atomic.Bool
}
func (s stubSource) AttachCommand(ports.RuntimeHandle) ([]string, error) { return s.argv, nil }
func (stubSource) IsAlive(context.Context, ports.RuntimeHandle) (bool, error) {
return false, nil
func (s *stubSource) AttachCommand(ports.RuntimeHandle) ([]string, error) {
s.attached.Store(true)
return s.argv, nil
}
func (s *stubSource) IsAlive(context.Context, ports.RuntimeHandle) (bool, error) {
return !s.attached.Load(), nil
}
type terminalMuxFrame struct {
@ -72,7 +81,7 @@ func TestMuxUpgradeStreamsTerminal(t *testing.T) {
t.Skip("PTY spawning not supported on Windows")
}
mgr := terminal.NewManager(
stubSource{argv: []string{"/bin/sh", "-c", "printf MUXOK; exit 0"}},
&stubSource{argv: []string{"/bin/sh", "-c", "printf MUXOK; exit 0"}},
nil, discardLogger(),
)
defer mgr.Close()
@ -101,7 +110,7 @@ func TestMuxSystemPingPong(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("PTY spawning not supported on Windows")
}
mgr := terminal.NewManager(stubSource{argv: []string{"/bin/sh"}}, nil, discardLogger())
mgr := terminal.NewManager(&stubSource{argv: []string{"/bin/sh"}}, nil, discardLogger())
defer mgr.Close()
c, done := dialMux(t, mgr)

View File

@ -41,6 +41,13 @@ func (f *fakeSource) setAlive(v bool) {
f.mu.Unlock()
}
func (f *fakeSource) setAliveResult(v bool, err error) {
f.mu.Lock()
f.alive = v
f.aliveErr = err
f.mu.Unlock()
}
// fakePTY is a scripted ptyProcess: Read drains the out channel, Write records,
// Resize records, and Close unblocks reads.
type fakePTY struct {

View File

@ -69,7 +69,7 @@ func recv(t *testing.T, c *fakeConn, ch, typ string, d time.Duration) serverMsg
}
func TestServeOpenStreamsAndWritesTerminal(t *testing.T) {
src := &fakeSource{}
src := &fakeSource{alive: true}
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
mgr := NewManager(src, nil, testLogger(), WithSpawn(sp.spawn), WithHeartbeat(0))
@ -152,8 +152,7 @@ func TestServeOpenAlreadyExitedSessionDoesNotBlockReopen(t *testing.T) {
// exit, so a later open for the same id is served rather than dropped by the
// already-open guard.
func TestServeExitAfterOpenClearsEntryAllowingReopen(t *testing.T) {
src := &fakeSource{}
src.setAlive(false) // a dropped pty must not re-attach -> session exits
src := &fakeSource{alive: true} // alive for the first attach
p := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{p}}
mgr := NewManager(src, nil, testLogger(), WithSpawn(sp.spawn), WithHeartbeat(0))
@ -167,7 +166,8 @@ func TestServeExitAfterOpenClearsEntryAllowingReopen(t *testing.T) {
conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen}
recv(t, conn, chTerminal, msgOpened, time.Second)
p.Close() // drop the pty; IsAlive false => session exits, no re-attach
src.setAlive(false) // a dropped pty must not re-attach -> session exits
p.Close() // drop the pty; IsAlive false => session exits, no re-attach
recv(t, conn, chTerminal, msgExited, time.Second)
conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen}

View File

@ -102,6 +102,30 @@ func (s *session) run(ctx context.Context) {
return
}
// Gate EVERY attach (including the first) on the runtime actually
// being alive. `zellij attach` resurrects EXITED sessions — re-running
// the serialized agent command — so attaching to a dead handle would
// re-create a runtime the daemon already destroyed, outside lifecycle
// control. A definitive "not alive" is a clean exit. A probe ERROR is
// not proof of death: it retries with backoff up to the same
// consecutive-failure cap as attach failures.
alive, err := s.src.IsAlive(ctx, s.handle)
if err != nil {
failures++
if failures > s.maxReattach {
s.fail("liveness probe: " + err.Error())
return
}
if !s.backoff(ctx, failures) {
return
}
continue
}
if !alive {
s.markExited()
return
}
argv, err := s.src.AttachCommand(s.handle)
if err != nil {
s.fail("attach command: " + err.Error())
@ -110,10 +134,13 @@ func (s *session) run(ctx context.Context) {
p, err := s.spawn(ctx, argv)
if err != nil {
failures++
if !s.shouldReattach(ctx, failures) {
if failures > s.maxReattach {
s.fail("spawn pty: " + err.Error())
return
}
if !s.backoff(ctx, failures) {
return
}
continue
}
@ -127,10 +154,13 @@ func (s *session) run(ctx context.Context) {
}
failures++
if !s.shouldReattach(ctx, failures) {
if failures > s.maxReattach {
s.markExited()
return
}
if !s.backoff(ctx, failures) {
return
}
s.log.Debug("terminal re-attaching", "id", s.id, "failures", failures)
}
}
@ -152,17 +182,11 @@ func (s *session) copyOut(p ptyProcess) {
}
}
// shouldReattach decides whether a dropped/failed PTY warrants another attempt:
// only while not closed/cancelled, the Zellij session still exists, and we are
// under the consecutive-failure cap. A backoff sleep separates attempts.
func (s *session) shouldReattach(ctx context.Context, failures int) bool {
if s.isClosed() || ctx.Err() != nil || failures > s.maxReattach {
return false
}
alive, err := s.src.IsAlive(ctx, s.handle)
if err != nil || !alive {
return false
}
// backoff sleeps between attach attempts; false means ctx was cancelled.
// Whether another attempt is warranted at all (liveness, failure cap) is
// decided at the top of the run loop, so a re-attach and a first attach share
// one gate.
func (s *session) backoff(ctx context.Context, failures int) bool {
select {
case <-ctx.Done():
return false

View File

@ -17,7 +17,7 @@ func newTestSession(src PTYSource, spawn spawnFunc) *session {
}
func TestSessionFansOutLiveOutputToSubscribers(t *testing.T) {
src := &fakeSource{}
src := &fakeSource{alive: true}
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
s := newTestSession(src, sp.spawn)
@ -35,7 +35,7 @@ func TestSessionFansOutLiveOutputToSubscribers(t *testing.T) {
}
func TestSessionReplaysRingBufferOnSubscribe(t *testing.T) {
src := &fakeSource{}
src := &fakeSource{alive: true}
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
s := newTestSession(src, sp.spawn)
@ -59,7 +59,7 @@ func ringLen(s *session) int {
}
func TestSessionWriteAndResizeReachPTY(t *testing.T) {
src := &fakeSource{}
src := &fakeSource{alive: true}
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
s := newTestSession(src, sp.spawn)
@ -81,7 +81,7 @@ func TestSessionWriteAndResizeReachPTY(t *testing.T) {
}
func TestSessionSkipsReattachOnCleanExit(t *testing.T) {
src := &fakeSource{alive: false} // Zellij session gone -> no re-attach
src := &fakeSource{alive: true} // alive for the first attach
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
s := newTestSession(src, sp.spawn)
@ -93,7 +93,9 @@ func TestSessionSkipsReattachOnCleanExit(t *testing.T) {
go s.run(ctx)
s.subscribe(func([]byte) {}, func() { close(exited) })
pty.Close() // pane ends
eventually(t, time.Second, func() bool { return sp.calls() == 1 })
src.setAlive(false) // Zellij session gone -> no re-attach
pty.Close() // pane ends
select {
case <-exited:
case <-time.After(time.Second):
@ -104,6 +106,61 @@ func TestSessionSkipsReattachOnCleanExit(t *testing.T) {
}
}
// TestSessionNeverAttachesToDeadRuntime covers the resurrection bug: `zellij
// attach` on a killed-but-cached session resurrects it, re-running the agent
// command. A session whose runtime probes definitively dead must therefore
// report exited WITHOUT ever spawning an attach PTY — even on the very first
// open (the original code only checked liveness on re-attach).
func TestSessionNeverAttachesToDeadRuntime(t *testing.T) {
src := &fakeSource{alive: false}
sp := &fakeSpawner{}
s := newTestSession(src, sp.spawn)
exited := make(chan struct{})
s.subscribe(func([]byte) {}, func() { close(exited) })
go s.run(context.Background())
select {
case <-exited:
case <-time.After(time.Second):
t.Fatal("expected exit when runtime is dead before first attach")
}
if got := sp.calls(); got != 0 {
t.Fatalf("attach must never run against a dead runtime, got %d spawns", got)
}
}
// TestSessionRetriesProbeErrorsBeforeAttaching pins the hard rule that a
// failed liveness probe is NOT proof of death: a transient probe error must
// not flip the terminal to exited, and the attach proceeds once the probe
// recovers.
func TestSessionRetriesProbeErrorsBeforeAttaching(t *testing.T) {
src := &fakeSource{aliveErr: io.ErrUnexpectedEOF}
pty := newFakePTY()
sp := &fakeSpawner{ptys: []*fakePTY{pty}}
s := newTestSession(src, sp.spawn)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go s.run(ctx)
// While probes error the session must neither exit nor attach.
time.Sleep(50 * time.Millisecond)
if s.isExited() {
t.Fatal("probe error must not be treated as runtime death")
}
if got := sp.calls(); got != 0 {
t.Fatalf("attach must wait for a successful probe, got %d spawns", got)
}
// Probe recovers -> the attach goes through.
src.setAliveResult(true, nil)
eventually(t, 2*time.Second, func() bool { return sp.calls() == 1 })
if s.isExited() {
t.Fatal("session exited despite a live runtime")
}
}
func TestSessionReattachesWhileSessionAlive(t *testing.T) {
src := &fakeSource{alive: true} // session still alive -> re-attach on drop
p1, p2 := newFakePTY(), newFakePTY()
@ -125,7 +182,7 @@ func TestSessionReattachesWhileSessionAlive(t *testing.T) {
}
func TestSessionFailsWhenAttachCommandErrors(t *testing.T) {
src := &fakeSource{attachErr: io.ErrUnexpectedEOF}
src := &fakeSource{alive: true, attachErr: io.ErrUnexpectedEOF}
sp := &fakeSpawner{}
s := newTestSession(src, sp.spawn)