From 30373ea40ce6875d2d7fae5e97c678f0b9b11c84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 14 Jul 2026 23:14:37 +0100 Subject: [PATCH] fix: promote commandless workspace executions (#1692) --- .../runtime/lifecycle/manager_execution.go | 10 +++ apps/backend/internal/backendapp/adapters.go | 4 + .../simulated_agent_manager_test.go | 2 + .../orchestrator/event_handlers_test.go | 1 + .../orchestrator/executor/executor.go | 5 ++ .../orchestrator/executor/executor_execute.go | 16 +++- .../executor/executor_mocks_test.go | 8 ++ .../orchestrator/executor/executor_test.go | 81 +++++++++++++++++++ .../orchestrator/scheduler/scheduler_test.go | 2 + 9 files changed, 126 insertions(+), 3 deletions(-) diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go index c0356259f8..abee46d8e9 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go @@ -234,6 +234,16 @@ func (m *Manager) GetExecutionIDForSession(_ context.Context, sessionID string) return "", fmt.Errorf("%w: %s", ErrNoExecutionForSession, sessionID) } +// IsAgentCommandConfigured reports whether an execution has been promoted from +// workspace-only infrastructure to an agent execution ready to start. +func (m *Manager) IsAgentCommandConfigured(executionID string) bool { + configured := false + _ = m.executionStore.WithRLock(executionID, func(execution *AgentExecution) { + configured = execution.AgentCommand != "" + }) + return configured +} + // EnsurePassthroughExecution ensures an execution exists for a passthrough session // and starts the passthrough process if needed. This is called when the terminal // handler receives a connection for a session that might need recovery after backend restart. diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index cd221812de..fd7f67c72c 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -274,6 +274,10 @@ func (a *lifecycleAdapter) StartAgentProcess(ctx context.Context, agentInstanceI return a.mgr.StartAgentProcess(ctx, agentInstanceID) } +func (a *lifecycleAdapter) IsAgentCommandConfigured(agentInstanceID string) bool { + return a.mgr.IsAgentCommandConfigured(agentInstanceID) +} + // StopAgent stops a running agent func (a *lifecycleAdapter) StopAgent(ctx context.Context, agentInstanceID string, force bool) error { return a.mgr.StopAgent(ctx, agentInstanceID, force) diff --git a/apps/backend/internal/integration/simulated_agent_manager_test.go b/apps/backend/internal/integration/simulated_agent_manager_test.go index cd2745c54b..1125ade129 100644 --- a/apps/backend/internal/integration/simulated_agent_manager_test.go +++ b/apps/backend/internal/integration/simulated_agent_manager_test.go @@ -147,6 +147,8 @@ func (s *SimulatedAgentManagerClient) StartAgentProcess(ctx context.Context, age return nil } +func (s *SimulatedAgentManagerClient) IsAgentCommandConfigured(_ string) bool { return true } + // runAgentSimulation simulates the agent execution lifecycle func (s *SimulatedAgentManagerClient) runAgentSimulation(instance *simulatedInstance, req *executor.LaunchAgentRequest) { // Wait for launch delay diff --git a/apps/backend/internal/orchestrator/event_handlers_test.go b/apps/backend/internal/orchestrator/event_handlers_test.go index 49fa5a668a..a42490b63f 100644 --- a/apps/backend/internal/orchestrator/event_handlers_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_test.go @@ -281,6 +281,7 @@ func (m *mockAgentManager) LaunchAgent(ctx context.Context, req *executor.Launch return nil, nil } func (m *mockAgentManager) StartAgentProcess(_ context.Context, _ string) error { return nil } +func (m *mockAgentManager) IsAgentCommandConfigured(_ string) bool { return true } func (m *mockAgentManager) StopAgent(_ context.Context, agentExecutionID string, force bool) error { m.mu.Lock() defer m.mu.Unlock() diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 1fec1fa4bd..bffe9261f4 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -76,6 +76,7 @@ var ( ErrNoCloneURL = errors.New("repository has no clone URL: provider owner and name are required") ErrTaskArchived = errors.New("task is archived") ErrStaleExecution = errors.New("stale execution: no live execution in memory") + ErrAgentCommandMissing = errors.New("existing execution has no agent command configured") ) // PromptResult contains the result of a prompt operation @@ -94,6 +95,10 @@ type AgentManagerClient interface { // The command is built internally based on the execution's agent profile. StartAgentProcess(ctx context.Context, agentExecutionID string) error + // IsAgentCommandConfigured reports whether an execution is ready for + // StartAgentProcess or still needs workspace-only promotion via LaunchAgent. + IsAgentCommandConfigured(agentExecutionID string) bool + // StopAgent stops a running agent StopAgent(ctx context.Context, agentExecutionID string, force bool) error StopAgentWithReason(ctx context.Context, agentExecutionID string, reason string, force bool) error diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index dd8bdcc23c..87a257dfb3 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -535,12 +535,13 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses hasRunning, _ := e.repo.HasExecutorRunningRow(ctx, sessionID) if hasRunning { result, err := e.startAgentOnExistingWorkspace(ctx, task, session, prompt, startAgent, opts.McpMode, opts.Env) - if !errors.Is(err, ErrStaleExecution) { + if !errors.Is(err, ErrStaleExecution) && !errors.Is(err, ErrAgentCommandMissing) { return result, err } - e.logger.Info("falling through to full LaunchAgent after stale execution", + e.logger.Info("falling through to full LaunchAgent for existing workspace", zap.String("task_id", task.ID), - zap.String("session_id", sessionID)) + zap.String("session_id", sessionID), + zap.Error(err)) } allRepos, err := e.resolveAllRepoInfo(ctx, task.ID) @@ -1131,6 +1132,15 @@ func (e *Executor) startAgentOnExistingWorkspace(ctx context.Context, task *v1.T } } + // Lazy workspace restoration creates an execution without an agent command. + // Preserve the request's description, environment, and MCP mode above, then + // route it through LaunchAgent so lifecycle.Launch can promote the execution + // with the effective profile, model, route override, and CLI flags before the + // subprocess is started. + if !e.agentManager.IsAgentCommandConfigured(executionID) { + return nil, ErrAgentCommandMissing + } + // Transition session to STARTING now := time.Now().UTC() session.State = models.TaskSessionStateStarting diff --git a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go index 357e3eb8cd..5e76d0d7da 100644 --- a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go @@ -23,6 +23,7 @@ type mockAgentManager struct { resolveAgentProfileFunc func(ctx context.Context, profileID string) (*AgentProfileInfo, error) setExecutionDescriptionFunc func(ctx context.Context, agentExecutionID string, description string) error getExecutionIDForSessionFunc func(ctx context.Context, sessionID string) (string, error) + isAgentCommandConfiguredFunc func(agentExecutionID string) bool isAgentRunningForSessionFunc func(ctx context.Context, sessionID string) bool cleanupStaleExecutionFunc func(ctx context.Context, sessionID string) error promptAgentFunc func(ctx context.Context, agentExecutionID, prompt string, attachments []v1.MessageAttachment, dispatchOnly bool) (*PromptResult, error) @@ -78,6 +79,13 @@ func (m *mockAgentManager) StartAgentProcess(ctx context.Context, agentExecution return nil } +func (m *mockAgentManager) IsAgentCommandConfigured(agentExecutionID string) bool { + if m.isAgentCommandConfiguredFunc != nil { + return m.isAgentCommandConfiguredFunc(agentExecutionID) + } + return true +} + func (m *mockAgentManager) StopAgent(ctx context.Context, agentExecutionID string, force bool) error { if m.stopAgentFunc != nil { return m.stopAgentFunc(ctx, agentExecutionID, force) diff --git a/apps/backend/internal/orchestrator/executor/executor_test.go b/apps/backend/internal/orchestrator/executor/executor_test.go index 67b6b79a97..4078b36332 100644 --- a/apps/backend/internal/orchestrator/executor/executor_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_test.go @@ -709,6 +709,87 @@ func TestLaunchPreparedSession_ExistingWorkspace_StartAgent(t *testing.T) { } } +// Regression: a workspace-only execution can be created by lazy workspace +// restoration before workflow auto-start reaches LaunchPreparedSession. That +// execution has no agent command, so it must go through LaunchAgent's promotion +// path before StartAgentProcess runs. +func TestLaunchPreparedSession_CommandlessExistingWorkspace_PromotesBeforeStart(t *testing.T) { + repo := newMockRepository() + session := &models.TaskSession{ + ID: "session-workspace-only", + TaskID: "task-123", + AgentProfileID: "profile-override", + State: models.TaskSessionStateCreated, + StartedAt: time.Now(), + UpdatedAt: time.Now(), + } + repo.sessions[session.ID] = session + repo.executorsRunning[session.ID] = &models.ExecutorRunning{ + ID: session.ID, + SessionID: session.ID, + TaskID: session.TaskID, + AgentExecutionID: "exec-workspace-only", + Status: models.ExecutorRunningStatusPrepared, + } + + var commandConfigured atomic.Bool + var executionDescription string + startDone := make(chan error, 1) + agentManager := &mockAgentManager{ + getExecutionIDForSessionFunc: func(_ context.Context, _ string) (string, error) { + return "exec-workspace-only", nil + }, + isAgentCommandConfiguredFunc: func(_ string) bool { + return commandConfigured.Load() + }, + setExecutionDescriptionFunc: func(_ context.Context, _ string, description string) error { + executionDescription = description + return nil + }, + launchAgentFunc: func(_ context.Context, _ *LaunchAgentRequest) (*LaunchAgentResponse, error) { + if executionDescription != "implement the approved plan" { + return nil, fmt.Errorf("execution description was not updated before promotion: %q", executionDescription) + } + commandConfigured.Store(true) + return &LaunchAgentResponse{ + AgentExecutionID: "exec-workspace-only", + Status: v1.AgentStatusStarting, + }, nil + }, + startAgentProcessFunc: func(_ context.Context, _ string) error { + if !commandConfigured.Load() { + err := errors.New(`execution "exec-workspace-only" has no agent command configured`) + startDone <- err + return err + } + startDone <- nil + return nil + }, + } + executor := newTestExecutor(t, agentManager, repo) + task := &v1.Task{ID: session.TaskID, WorkspaceID: "workspace-123", Title: "Test Task"} + + _, err := executor.LaunchPreparedSession(context.Background(), task, session.ID, LaunchOptions{ + AgentProfileID: session.AgentProfileID, + Prompt: "implement the approved plan", + StartAgent: true, + }) + if err != nil { + t.Fatalf("LaunchPreparedSession: %v", err) + } + if agentManager.launchAgentCallCount != 1 { + t.Fatalf("LaunchAgent call count = %d, want 1 command-promotion launch", agentManager.launchAgentCallCount) + } + select { + case err := <-startDone: + if err != nil { + t.Fatalf("StartAgentProcess: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for StartAgentProcess") + } +} + // TestLaunchPreparedSession_StaleExecutionID_CorrectedFromLiveStore was removed: // the "DB has stale ID, in-memory has live ID, correct DB to match" code path no // longer exists. With executors_running as the single source of truth and lifecycle- diff --git a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go index 11f406c18b..6c58a0781e 100644 --- a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go +++ b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go @@ -66,6 +66,8 @@ func (m *mockAgentManager) StartAgentProcess(ctx context.Context, agentExecution return nil } +func (m *mockAgentManager) IsAgentCommandConfigured(_ string) bool { return true } + func (m *mockAgentManager) StopAgent(ctx context.Context, agentExecutionID string, force bool) error { return nil }