fix(session): clear shutdown-saved marker so killed sessions stay dead (#2319) (#2320)

After Cmd+Q quit + reopen, sessions the user killed reappeared as alive.
The session_worktrees "shutdown-saved" restore marker was write-only:
RestoreAll auto-relaunches any terminated session that still has a marker,
but Kill never deleted it and RestoreAll never consumed it. A session that
survived one reopen cycle (gaining a marker via reconcileLive) and was then
killed still carried the marker, so the next boot resurrected it.

Two-layer fix:
- Kill now deletes the marker (best-effort) after MarkTerminated: a user
  kill is explicit terminal intent and must not leave a resurrection marker.
- RestoreAll deletes the marker after consuming preserveRef and attempting
  relaunch, making it one-shot so it never outlives a single restart. A
  still-live session re-acquires a fresh marker at the next quit.

Manual Restore stays marker-independent, so killed sessions remain
restorable on demand, just not auto-resurrected on boot.

Adds DeleteSessionWorktrees to the Manager Store interface (the concrete
store already implements it) and tests covering all three behaviors.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

backend/internal/session_manager/manager.go

@@ -96,6 +96,8 @@ type Store interface {
 	// presence of any row is the marker; preserved_ref may be empty for clean
 	// worktrees.
 	ListSessionWorktrees(ctx context.Context, id domain.SessionID) ([]domain.SessionWorktreeRecord, error)
+	// DeleteSessionWorktrees clears the "shutdown-saved" restore marker.
+	DeleteSessionWorktrees(ctx context.Context, id domain.SessionID) error
 }
 
 // Manager coordinates internal session spawn, restore, kill, and cleanup over
@@ -448,6 +450,12 @@ func (m *Manager) Kill(ctx context.Context, id domain.SessionID) (bool, error) {
 		return false, fmt.Errorf("kill %s: %w", id, err)
 	}
 
+	// Clear the restore marker so the next boot's RestoreAll cannot resurrect a
+	// killed session (#2319). Best-effort: teardown below still matters.
+	if err := m.store.DeleteSessionWorktrees(ctx, id); err != nil {
+		m.logger.Warn("kill: delete restore marker failed", "sessionID", id, "error", err)
+	}
+
 	// Only tear down what exists. A session may have lost its handle after a
 	// crash or never acquired one if spawn failed partway.
 	if handle.ID != "" {
@@ -841,6 +849,12 @@ func (m *Manager) RestoreAll(ctx context.Context) error {
 				m.logger.Error("restore-all: relaunch failed", "sessionID", rec.ID, "error", err)
 			}
 		}
+
+		// One-shot: drop the consumed marker so it never outlives one restart
+		// (#2319). A still-live session re-acquires it at the next quit.
+		if err := m.store.DeleteSessionWorktrees(ctx, rec.ID); err != nil {
+			m.logger.Warn("restore-all: delete restore marker failed", "sessionID", rec.ID, "error", err)
+		}
 	}
 	return nil
 }

backend/internal/session_manager/manager_test.go

@@ -113,6 +113,10 @@ func (f *fakeStore) UpsertSessionWorktree(_ context.Context, row domain.SessionW
 func (f *fakeStore) ListSessionWorktrees(_ context.Context, id domain.SessionID) ([]domain.SessionWorktreeRecord, error) {
 	return f.worktrees[id], nil
 }
+func (f *fakeStore) DeleteSessionWorktrees(_ context.Context, id domain.SessionID) error {
+	delete(f.worktrees, id)
+	return nil
+}
 
 type fakeLCM struct {
 	store     *fakeStore
@@ -570,6 +574,29 @@ func TestKill_OtherWorkspaceErrorStillFails(t *testing.T) {
 		t.Fatalf("kill err = %v, want workspace error surfaced", err)
 	}
 }
+
+// TestKill_DeletesRestoreMarker covers issue #2319 (a): a user kill is explicit
+// terminal intent and must delete the session_worktrees "shutdown-saved" marker.
+// A session that carried a marker (e.g. it survived a prior reopen cycle) and is
+// then killed must not keep that marker, or the next boot's RestoreAll would
+// resurrect it.
+func TestKill_DeletesRestoreMarker(t *testing.T) {
+	m, st, _, _ := newManager()
+	st.sessions["mer-1"] = mkLive("mer-1")
+	// The session carries a leftover shutdown-saved marker from a prior cycle.
+	st.worktrees["mer-1"] = []domain.SessionWorktreeRecord{{SessionID: "mer-1", RepoName: "__root__"}}
+
+	if _, err := m.Kill(ctx, "mer-1"); err != nil {
+		t.Fatalf("kill err = %v", err)
+	}
+	rows, err := st.ListSessionWorktrees(ctx, "mer-1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(rows) != 0 {
+		t.Fatalf("kill must delete the restore marker, got %d rows", len(rows))
+	}
+}
 func TestRestore_ReopensTerminal(t *testing.T) {
 	m, st, rt, _ := newManager()
 	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"})
@@ -1578,6 +1605,81 @@ func TestRestoreAll_SkipsSessionsKilledBeforeShutdown(t *testing.T) {
 	}
 }
 
+// TestRestoreAll_DeletesMarkerAfterRelaunch covers issue #2319 (b): the
+// shutdown-saved marker is one-shot. After RestoreAll relaunches a session, its
+// session_worktrees marker is deleted, so a second RestoreAll (with no fresh
+// marker) does NOT relaunch it again.
+func TestRestoreAll_DeletesMarkerAfterRelaunch(t *testing.T) {
+	m, st, rt, _ := newLifecycleManager()
+
+	st.sessions["mer-1"] = domain.SessionRecord{
+		ID:           "mer-1",
+		ProjectID:    "mer",
+		Kind:         domain.KindWorker,
+		Harness:      domain.HarnessClaudeCode,
+		IsTerminated: true,
+		Metadata:     domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "ao/mer-1/root", AgentSessionID: "agent-w"},
+		Activity:     domain.Activity{State: domain.ActivityExited},
+	}
+	st.worktrees["mer-1"] = []domain.SessionWorktreeRecord{{SessionID: "mer-1", RepoName: "__root__"}}
+
+	if err := m.RestoreAll(ctx); err != nil {
+		t.Fatalf("RestoreAll err = %v", err)
+	}
+	if rt.created != 1 {
+		t.Fatalf("first RestoreAll must relaunch once, runtime.Create called %d times", rt.created)
+	}
+	rows, err := st.ListSessionWorktrees(ctx, "mer-1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(rows) != 0 {
+		t.Fatalf("RestoreAll must delete the one-shot marker, got %d rows", len(rows))
+	}
+}
+
+// TestRestoreAll_KilledSessionNotResurrectedOnSecondBoot covers issue #2319 (c),
+// the killed-session-resurrection scenario. A terminated session WITH a marker
+// is relaunched exactly once; on a second RestoreAll (no new marker) it stays
+// terminated and is not relaunched again.
+func TestRestoreAll_KilledSessionNotResurrectedOnSecondBoot(t *testing.T) {
+	m, st, rt, _ := newLifecycleManager()
+
+	st.sessions["mer-1"] = domain.SessionRecord{
+		ID:           "mer-1",
+		ProjectID:    "mer",
+		Kind:         domain.KindWorker,
+		Harness:      domain.HarnessClaudeCode,
+		IsTerminated: true,
+		Metadata:     domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "ao/mer-1/root", AgentSessionID: "agent-w"},
+		Activity:     domain.Activity{State: domain.ActivityExited},
+	}
+	st.worktrees["mer-1"] = []domain.SessionWorktreeRecord{{SessionID: "mer-1", RepoName: "__root__"}}
+
+	// First boot: marker present, session relaunches once.
+	if err := m.RestoreAll(ctx); err != nil {
+		t.Fatalf("first RestoreAll err = %v", err)
+	}
+	if rt.created != 1 {
+		t.Fatalf("first RestoreAll must relaunch once, runtime.Create called %d times", rt.created)
+	}
+
+	// Simulate the user killing the relaunched session before the next quit, so
+	// it has no fresh marker, then a second boot.
+	if _, err := m.Kill(ctx, "mer-1"); err != nil {
+		t.Fatalf("kill err = %v", err)
+	}
+	if err := m.RestoreAll(ctx); err != nil {
+		t.Fatalf("second RestoreAll err = %v", err)
+	}
+	if rt.created != 1 {
+		t.Fatalf("killed session must NOT be resurrected on second boot, runtime.Create total = %d, want 1", rt.created)
+	}
+	if !st.sessions["mer-1"].IsTerminated {
+		t.Error("killed session must remain terminated after second RestoreAll")
+	}
+}
+
 // TestRestoreAll_AppliesPreservedRef: when the session_worktrees row has a
 // non-empty preserved_ref, RestoreAll calls ApplyPreserved after workspace
 // restore but before relaunching.