From 55c5c349ec5f1f1b95af5a1b2eeb246b27698b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Manso?= Date: Fri, 7 Aug 2026 16:26:11 +0100 Subject: [PATCH] fix(lifecycle): reject terminal sessions before creating an execution (#2325) --- .../runtime/lifecycle/manager_execution.go | 13 ++++ .../lifecycle/manager_execution_test.go | 73 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go index 745eea5cb9..b39f821577 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go @@ -419,6 +419,10 @@ func (m *Manager) resumeExistingExecution(ctx context.Context, sessionID string, // createExecutionFromSessionInfo creates a new execution for a passthrough session // when no execution exists (e.g., backend restarted and execution store was cleared). +// +// Terminal sessions are rejected by the guard at the top of createExecution, the +// same one every other creation path goes through, so this recovery path never +// spawns a runtime for a session that ended before the restart. func (m *Manager) createExecutionFromSessionInfo(ctx context.Context, sessionID string) (*AgentExecution, error) { if m.workspaceInfoProvider == nil { return nil, fmt.Errorf("cannot restore session %s: workspace info provider not configured", sessionID) @@ -510,6 +514,15 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work if info == nil { return nil, fmt.Errorf("workspace info is required") } + // A terminal session can never gain an execution, so reject it before + // reconciling the workspace, taking an activity lease, or creating a + // runtime instance. User-facing panels (terminal, git, files) reconnect on + // a timer; without this every retry paid for a full instance creation that + // the post-creation check below tore straight back down. That check stays — + // it guards the session that terminalizes *during* creation. + if err := m.ensureLaunchSessionStillActive(ctx, info.SessionID); err != nil { + return nil, err + } owner := ownedDirectoryLinkOwner(taskID, info.TaskDirName) if err := reconcileWorkspaceSources(ctx, info.WorkspacePath, info.WorkspaceFolders, owner); err != nil { return nil, err diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go index 7d0a351aae..d2653cc754 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go @@ -358,6 +358,79 @@ func TestCreateExecutionRollsBackWhenRegistrationCannotPersist(t *testing.T) { } } +const ( + terminalSessionID = "session-terminal-shell" + terminalEnvironmentID = "env-terminal-shell" + terminalTaskID = "task-terminal-shell" +) + +func newTerminalSessionManager(t *testing.T, state models.TaskSessionState) (*Manager, *createInstanceExecutor) { + t.Helper() + mgr, backend := newEnvironmentExecutionTestManager(t, &mockWorkspaceInfoProvider{ + infos: map[string]*WorkspaceInfo{ + terminalSessionID: { + TaskID: terminalTaskID, SessionID: terminalSessionID, + TaskEnvironmentID: terminalEnvironmentID, WorkspacePath: "/workspace/task", + AgentID: "auggie", + }, + }, + }) + mgr.SetExecutorProfileReader(&fakeExecutorProfileReader{session: &models.TaskSession{ + ID: terminalSessionID, TaskID: terminalTaskID, State: state, + }}) + return mgr, backend +} + +// A shell terminal left open on a terminal session reconnects on a timer, and +// the file, git, LSP and port panels poll their own session-keyed path. Every +// entry point must be rejected from the session state alone: creating the +// runtime instance first and rolling it back turned an idle panel into a +// spawn/teardown loop for as long as the tab stayed open. +func TestEnsureExecutionRejectsTerminalSessionWithoutCreatingInstance(t *testing.T) { + entryPoints := []struct { + name string + call func(*Manager) error + }{ + {"GetOrEnsureExecutionForEnvironment", func(m *Manager) error { + _, err := m.GetOrEnsureExecutionForEnvironment(context.Background(), terminalEnvironmentID) + return err + }}, + {"GetOrEnsureExecution", func(m *Manager) error { + _, err := m.GetOrEnsureExecution(context.Background(), terminalSessionID) + return err + }}, + {"EnsureWorkspaceExecutionForSession", func(m *Manager) error { + _, err := m.EnsureWorkspaceExecutionForSession(context.Background(), terminalTaskID, terminalSessionID) + return err + }}, + } + states := []models.TaskSessionState{ + models.TaskSessionStateFailed, + models.TaskSessionStateCancelled, + models.TaskSessionStateCompleted, + } + for _, entryPoint := range entryPoints { + for _, state := range states { + t.Run(entryPoint.name+"/"+string(state), func(t *testing.T) { + mgr, backend := newTerminalSessionManager(t, state) + + if err := entryPoint.call(mgr); !errors.Is(err, ErrSessionTerminal) { + t.Fatalf("%s error = %v, want ErrSessionTerminal", entryPoint.name, err) + } + if got := backend.createCount.Load(); got != 0 { + t.Fatalf("CreateInstance calls = %d, want 0", got) + } + if got := backend.stopCount.Load(); got != 0 { + t.Fatalf("StopInstance calls = %d, want 0", got) + } + if _, exists := mgr.executionStore.GetBySessionID(terminalSessionID); exists { + t.Fatal("terminal session must not register an execution") + } + }) + } + } +} + func TestResolveTaskEnvironmentID(t *testing.T) { t.Run("returns TaskEnvironmentID when execution carries it", func(t *testing.T) { store := NewExecutionStore()