fix: resolve dynamic workflow replacement profiles (#3202) (#3217)

2026-08-31 20:42:25 +00:00
parent 71de0df83e
commit b8952c865c
4 changed files with 445 additions and 6 deletions
@@ -2204,6 +2204,11 @@ func (s *Service) createNewSessionForStep(ctx context.Context, taskID string, cu
 	if err != nil {
 		return nil, fmt.Errorf("failed to get db task for session switch: %w", err)
 	}
+	if s.profileExecutionResolver != nil {
+		if err := s.profileExecutionResolver.ValidateProfile(ctx, newAgentProfileID); err != nil {
+			return nil, fmt.Errorf("failed to validate workflow replacement profile: %w", err)
+		}
+	}

 	// Create a new session with the new agent profile.
 	// Reuse the same executor profile from the current session.
@@ -2211,8 +2216,29 @@ func (s *Service) createNewSessionForStep(ctx context.Context, taskID string, cu
 	if err != nil {
 		return nil, fmt.Errorf("failed to prepare new session: %w", err)
 	}
+	newSession, err := s.repo.GetTaskSession(ctx, sessionID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get new session: %w", err)
+	}
+	launchProfileID, err := s.resolveDynamicLaunchExecution(ctx, newSession, newAgentProfileID, true)
+	if err != nil {
+		resolutionErr := fmt.Errorf("failed to resolve workflow replacement profile: %w", err)
+		if deleteErr := s.repo.DeleteTaskSession(ctx, sessionID); deleteErr != nil {
+			s.logger.Warn("failed to delete workflow replacement after profile resolution failure",
+				zap.String("task_id", taskID),
+				zap.String("session_id", sessionID),
+				zap.Error(deleteErr))
+			if terminalErr := s.repo.UpdateTaskSessionState(ctx, sessionID, models.TaskSessionStateFailed, resolutionErr.Error()); terminalErr != nil {
+				s.logger.Warn("failed to terminalize workflow replacement after delete failure",
+					zap.String("task_id", taskID),
+					zap.String("session_id", sessionID),
+					zap.Error(terminalErr))
+			}
+		}
+		return nil, resolutionErr
+	}
 	if _, err := s.executor.LaunchPreparedSession(ctx, task, sessionID, executor.LaunchOptions{
-		AgentProfileID: newAgentProfileID,
+		AgentProfileID: launchProfileID,
 		ExecutorID:     currentSession.ExecutorID,
 		WorkflowStepID: dbTask.WorkflowStepID,
 		StartAgent:     false,
@@ -2220,11 +2246,6 @@ func (s *Service) createNewSessionForStep(ctx context.Context, taskID string, cu
 		return nil, fmt.Errorf("failed to attach workflow replacement workspace: %w", err)
 	}

-	newSession, err := s.repo.GetTaskSession(ctx, sessionID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get new session: %w", err)
-	}
-
 	// Tag the session as workflow-spawned for provenance: its agent profile
 	// was selected by the workflow step override rather than direct user choice.
 	s.tagSessionAsWorkflowSwitched(ctx, newSession.ID)
@@ -0,0 +1,205 @@
+package orchestrator
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/jmoiron/sqlx"
+	_ "github.com/mattn/go-sqlite3"
+
+	agentruntime "github.com/kandev/kandev/internal/agent/runtime"
+	dynamicruntime "github.com/kandev/kandev/internal/agent/runtime/dynamic"
+	"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
+	agentsettingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
+	agentsettingsstore "github.com/kandev/kandev/internal/agent/settings/store"
+	"github.com/kandev/kandev/internal/orchestrator/executor"
+	"github.com/kandev/kandev/internal/task/models"
+	sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
+	wfmodels "github.com/kandev/kandev/internal/workflow/models"
+	v1 "github.com/kandev/kandev/pkg/api/v1"
+)
+
+// @covers AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.1
+// @covers AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.3
+func TestCreateNewSessionForStep_ResolvesDynamicProfileBeforeWorkspaceAttach(t *testing.T) {
+	ctx := context.Background()
+	taskRepo, current := seedDynamicWorkflowSwitch(t)
+	const dynamicProfileID = "profile-dynamic"
+	const concreteProfileID = "profile-concrete"
+
+	resolver := newWorkflowDynamicProfileResolver(t, dynamicProfileID, concreteProfileID)
+	agentManager := &mockAgentManager{
+		repoForExecutionLookup: taskRepo,
+		launchAgentFunc: func(_ context.Context, req *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
+			if req.AgentProfileID != concreteProfileID {
+				return nil, fmt.Errorf("failed to resolve agent profile: profile %s: %w", req.AgentProfileID, lifecycle.ErrVirtualProfile)
+			}
+			return &executor.LaunchAgentResponse{AgentExecutionID: "replacement-execution"}, nil
+		},
+	}
+	schedulerRepo := newMockTaskRepo()
+	schedulerRepo.tasks[current.TaskID] = &v1.Task{ID: current.TaskID, WorkspaceID: "ws1", Title: "Test Task"}
+	stepGetter := newMockStepGetter()
+	stepGetter.steps["step-work"] = &wfmodels.WorkflowStep{ID: "step-work", WorkflowID: "wf1", Name: "Work"}
+	svc := createTestServiceWithScheduler(taskRepo, stepGetter, schedulerRepo, agentManager)
+	svc.SetProfileExecutionResolver(resolver)
+
+	created, err := svc.createNewSessionForStep(ctx, current.TaskID, current, dynamicProfileID)
+	if err != nil {
+		t.Fatalf("createNewSessionForStep: %v", err)
+	}
+	initialGeneration := created.RouteGeneration
+	if _, err := svc.StartCreatedSession(ctx, current.TaskID, created.ID, dynamicProfileID, "start", true, false, true, nil, nil); err != nil {
+		t.Fatalf("StartCreatedSession: %v", err)
+	}
+	persisted, err := taskRepo.GetTaskSession(ctx, created.ID)
+	if err != nil {
+		t.Fatalf("re-read session: %v", err)
+	}
+	resolvedProfileID, err := svc.resolveDynamicLaunchExecution(ctx, persisted, persisted.AgentProfileID, true)
+	if err != nil {
+		t.Fatalf("resolve persisted route: %v", err)
+	}
+	if persisted.AgentProfileID != dynamicProfileID ||
+		persisted.ExecutionProfileID != concreteProfileID ||
+		resolvedProfileID != concreteProfileID ||
+		persisted.RouteGeneration != initialGeneration || initialGeneration == 0 {
+		t.Fatalf("resolved session = logical %q concrete %q returned %q generation %d→%d",
+			persisted.AgentProfileID, persisted.ExecutionProfileID, resolvedProfileID,
+			initialGeneration, persisted.RouteGeneration)
+	}
+}
+
+// @covers AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.1
+func TestCreateNewSessionForStep_RemovesPreparedSessionWhenDynamicResolutionFails(t *testing.T) {
+	ctx := context.Background()
+	taskRepo, current := seedDynamicWorkflowSwitch(t)
+	const dynamicProfileID = "profile-dynamic"
+	const concreteProfileID = "profile-concrete"
+
+	resolver := newWorkflowNoCandidateProfileResolver(t, dynamicProfileID, concreteProfileID)
+	launchCalled := false
+	agentManager := &mockAgentManager{
+		repoForExecutionLookup: taskRepo,
+		launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
+			launchCalled = true
+			return &executor.LaunchAgentResponse{AgentExecutionID: "unexpected-launch"}, nil
+		},
+	}
+	schedulerRepo := newMockTaskRepo()
+	schedulerRepo.tasks[current.TaskID] = &v1.Task{ID: current.TaskID, WorkspaceID: "ws1", Title: "Test Task"}
+	svc := createTestServiceWithScheduler(taskRepo, newMockStepGetter(), schedulerRepo, agentManager)
+	svc.SetProfileExecutionResolver(resolver)
+
+	if _, err := svc.createNewSessionForStep(ctx, current.TaskID, current, dynamicProfileID); err == nil {
+		t.Fatal("createNewSessionForStep succeeded without an eligible dynamic candidate")
+	}
+	if launchCalled {
+		t.Fatal("lifecycle launch ran after dynamic profile resolution failed")
+	}
+	sessions, err := taskRepo.ListTaskSessions(ctx, current.TaskID)
+	if err != nil {
+		t.Fatalf("list task sessions: %v", err)
+	}
+	if len(sessions) != 1 || sessions[0].ID != current.ID {
+		t.Fatalf("sessions after failed resolution = %+v, want only %q", sessions, current.ID)
+	}
+	active, err := taskRepo.GetActiveTaskSessionByTaskID(ctx, current.TaskID)
+	if err != nil {
+		t.Fatalf("get active session: %v", err)
+	}
+	if active == nil || active.ID != current.ID || active.State != models.TaskSessionStateRunning {
+		t.Fatalf("active session after failed resolution = %+v, want running %q", active, current.ID)
+	}
+	if reusable, err := svc.findReusableSessionForProfile(ctx, current.TaskID, dynamicProfileID, current.ID); err != nil {
+		t.Fatalf("find reusable session: %v", err)
+	} else if reusable != nil {
+		t.Fatalf("stale replacement session remained reusable: %+v", reusable)
+	}
+}
+
+func seedDynamicWorkflowSwitch(t *testing.T) (*sqliterepo.Repository, *models.TaskSession) {
+	t.Helper()
+	ctx := context.Background()
+	repo := setupTestRepo(t)
+	seedSession(t, repo, "task-dynamic-switch", "session-current", "step-work")
+	current, err := repo.GetTaskSession(ctx, "session-current")
+	if err != nil {
+		t.Fatal(err)
+	}
+	current.State = models.TaskSessionStateRunning
+	current.AgentProfileID = "profile-old"
+	current.ExecutorID = models.ExecutorIDWorktree
+	current.TaskEnvironmentID = "environment-dynamic-switch"
+	if err := repo.UpdateTaskSession(ctx, current); err != nil {
+		t.Fatal(err)
+	}
+	if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
+		ID: current.TaskEnvironmentID, TaskID: current.TaskID,
+		ExecutorType: string(models.ExecutorTypeLocal), Status: models.TaskEnvironmentStatusReady,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	return repo, current
+}
+
+func newWorkflowDynamicProfileResolver(
+	t *testing.T,
+	dynamicProfileID, concreteProfileID string,
+) *agentruntime.ProfileExecutionResolver {
+	return newWorkflowDynamicProfileResolverWithCandidate(t, dynamicProfileID, concreteProfileID, true)
+}
+
+func newWorkflowNoCandidateProfileResolver(
+	t *testing.T,
+	dynamicProfileID, concreteProfileID string,
+) *agentruntime.ProfileExecutionResolver {
+	return newWorkflowDynamicProfileResolverWithCandidate(t, dynamicProfileID, concreteProfileID, false)
+}
+
+func newWorkflowDynamicProfileResolverWithCandidate(
+	t *testing.T,
+	dynamicProfileID, concreteProfileID string,
+	candidateEnabled bool,
+) *agentruntime.ProfileExecutionResolver {
+	t.Helper()
+	db, err := sqlx.Open("sqlite3", ":memory:")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = db.Close() })
+	repo, cleanup, err := agentsettingsstore.Provide(db, db, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = cleanup() })
+	ctx := context.Background()
+	for _, agent := range []*agentsettingsmodels.Agent{
+		{ID: "dynamic", Name: "dynamic"},
+		{ID: "concrete-agent", Name: "concrete-agent"},
+	} {
+		if err := repo.CreateAgent(ctx, agent); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, profile := range []*agentsettingsmodels.AgentProfile{
+		{ID: dynamicProfileID, AgentID: "dynamic", Name: "Cascade", Enabled: true},
+		{ID: concreteProfileID, AgentID: "concrete-agent", Name: "Concrete", Enabled: true},
+	} {
+		if err := repo.CreateAgentProfile(ctx, profile); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := repo.CreateDynamicAgentProfile(ctx,
+		&agentsettingsmodels.DynamicAgentProfile{ProfileID: dynamicProfileID, Version: 1},
+		[]agentsettingsmodels.DynamicAgentRoute{{
+			DynamicProfileID:   dynamicProfileID,
+			ExecutionProfileID: concreteProfileID,
+			Enabled:            candidateEnabled,
+		}},
+	); err != nil {
+		t.Fatal(err)
+	}
+	return agentruntime.NewProfileExecutionResolver(repo, dynamicruntime.NewEngine(), true)
+}
@@ -0,0 +1,98 @@
+---
+created: 2026-08-31
+status: done
+requirements:
+  - REQ-AGENTS-DYNAMIC-AGENT-ROUTING-001
+system_design:
+  - ../../specs/agents/system-design/dynamic-agent-routing-01.md
+  - ../../specs/agents/system-design/dynamic-agent-routing-02.md
+legacy_specs: []
+---
+
+# Implementation Plan: Workflow Dynamic Profile Auto-Start
+
+## Overview
+
+Repair issue #3202 at the workflow replacement boundary. Resolve the logical
+dynamic profile before the executor attaches the retained task environment.
+
+One TDD work order adds a regression test and uses the existing shared
+resolver. The session keeps its logical identity. Lifecycle receives the
+selected concrete execution profile.
+
+## Scope
+
+### In scope
+
+- Cover workflow entry that switches an existing task session to a dynamic
+  profile before `auto_start_agent` runs.
+- Persist the selected `execution_profile_id` and route generation before the
+  replacement workspace is attached.
+- Preserve the existing concrete-profile workflow-switch behavior and the
+  original session until replacement preparation succeeds.
+
+### Out of scope
+
+- Changes to dynamic candidate selection, fallback policy, or route recovery.
+- Changes to workflow-step validation or the profile picker.
+- Changes to manual session creation, initial task start, Office routing, or
+  utility-agent routing.
+- Browser UI changes and new public documentation.
+
+## Technical approach
+
+In `Service.createNewSessionForStep`, validate the destination before creating
+the replacement session, then load the new session before workspace attachment.
+This function is in `apps/backend/internal/orchestrator/event_handlers_workflow.go`.
+
+Resolve the logical profile with `resolveDynamicLaunchExecution`. Pass the
+returned concrete profile ID to `Executor.LaunchPreparedSession`.
+
+Keep `TaskSession.AgentProfileID` unchanged. The resolver persists
+`ExecutionProfileID`, route generation, route reason, and the concrete profile
+snapshot before lifecycle receives the launch request.
+
+If route selection fails after session preparation, delete the replacement row.
+If deletion fails, mark the replacement terminal so it cannot be reused.
+
+The later `StartCreatedSession` call must reuse the persisted route through
+`ResolveExisting`. It must not claim a second generation or select another
+candidate. Concrete profiles keep the same effective profile ID.
+
+## Tests
+
+- `AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.1` and
+  `AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.3`: add
+  `TestCreateNewSessionForStep_ResolvesDynamicProfileBeforeWorkspaceAttach` in
+  `apps/backend/internal/orchestrator/event_handlers_workflow_dynamic_profile_test.go`.
+  The test uses real SQLite stores and the real dynamic resolver. It rejects a
+  lifecycle launch that receives the virtual profile, starts the created
+  session through `StartCreatedSession`, and reloads persisted attribution. A
+  second test verifies that route-resolution failure removes the prepared row
+  and keeps the current session reusable.
+- Existing concrete workflow-switch tests in
+  `apps/backend/internal/orchestrator/event_handlers_workflow_profile_test.go`
+  remain the control for unchanged concrete behavior.
+
+## E2E tests
+
+No browser E2E test is added. The error is below the UI. The focused Go test
+drives the service, resolver, executor, and real SQLite persistence. The UI and
+API contracts do not change.
+
+## Work orders
+
+- [x] [Task 01: Resolve dynamic workflow replacement profiles](task-01-resolve-dynamic-workflow-replacement-profiles.md) (`done`)
+
+## Verification results
+
+The focused orchestrator test command passed all seven tests. It covers the
+dynamic-profile regression, resolution-failure cleanup, three terminal-session
+states, and the existing workspace-attachment failure behavior.
+
+## Risks
+
+- Resolving during workspace attachment must not advance the route again when
+  `auto_start_agent` starts the created session.
+- A resolution or persistence failure must leave the current session active and
+  must not promote the replacement session.
@@ -0,0 +1,115 @@
+---
+id: "01-resolve-dynamic-workflow-replacement-profiles"
+title: "Resolve dynamic workflow replacement profiles"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+requirements:
+  - REQ-AGENTS-DYNAMIC-AGENT-ROUTING-001
+acceptance_criteria:
+  - AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.1
+  - AC-AGENTS-DYNAMIC-AGENT-ROUTING-001.3
+system_design:
+  - ../../specs/agents/system-design/dynamic-agent-routing-01.md
+  - ../../specs/agents/system-design/dynamic-agent-routing-02.md
+---
+
+# Task 01: Resolve Dynamic Workflow Replacement Profiles
+
+## Summary
+
+Resolve a dynamic destination before workflow profile switching attaches the
+retained task environment. Add a regression test for the lifecycle profile.
+The Kandev session must keep its logical profile and route attribution.
+
+## In scope
+
+- Add a focused orchestrator regression test using real profile routing and
+  task-session persistence.
+- Resolve the newly prepared workflow replacement session through
+  `resolveDynamicLaunchExecution` before `LaunchPreparedSession`.
+- Pass the concrete execution profile to workspace attachment without changing
+  the logical session profile.
+- Remove or terminalize the prepared replacement when route resolution fails.
+- Verify the selected route remains sticky for the later created-session start.
+
+## Out of scope
+
+- Dynamic routing policy or candidate-order changes.
+- Workflow validation, frontend, API, schema, migration, or documentation
+  changes.
+- Refactoring other direct `LaunchPreparedSession` callers.
+
+## Acceptance
+
+- A workflow switch does not pass the virtual profile to lifecycle during
+  replacement workspace attachment.
+- The replacement session stores the dynamic logical profile and selected
+  concrete execution profile before it becomes primary.
+- A resolution or attachment error leaves the prior session active and does not
+  leave a reusable partial replacement.
+- Concrete-profile switching remains unchanged.
+
+## Verification
+
+```bash
+cd apps/backend && go test -tags fts5 ./internal/orchestrator -run 'TestCreateNewSessionForStep(_(ResolvesDynamicProfileBeforeWorkspaceAttach|RemovesPreparedSessionWhenDynamicResolutionFails|TerminalPrimaryReusesCanonicalEnvironment)|KeepsCurrentSessionWhenWorkspaceAttachFails)' -count=1
+```
+
+## Files likely touched
+
+- `apps/backend/internal/orchestrator/event_handlers_workflow.go`
+- `apps/backend/internal/orchestrator/event_handlers_workflow_dynamic_profile_test.go`
+
+## Dependencies
+
+None.
+
+## Risks
+
+- A second route claim during `StartCreatedSession` could select a different
+  candidate or increment the generation. The test must assert sticky persisted
+  attribution across preparation and start boundaries.
+
+## Parallelism
+
+`sequential`
+
+## Inputs
+
+- `REQ-AGENTS-DYNAMIC-AGENT-ROUTING-001`, especially the transparent profile
+  execution and workflow-use sections.
+- `docs/specs/agents/system-design/dynamic-agent-routing-01.md` profile
+  execution and route-selection boundaries.
+- `docs/specs/agents/system-design/dynamic-agent-routing-02.md` persistence and
+  failure guarantees.
+- `docs/decisions/2026-08-13-dynamic-agent-profile-routing.md`.
+- `Service.createNewSessionForStep`, `Service.resolveDynamicLaunchExecution`,
+  and the existing workflow profile-switch tests.
+
+## Results
+
+RED: the dynamic-profile regression failed because lifecycle received
+`profile-dynamic`. Lifecycle rejected that virtual profile before it attached
+the retained task environment. The resolution-failure cleanup test also found
+a prepared replacement row left behind.
+
+The workflow replacement path now resolves the prepared session before the
+workspace attachment. It stores the concrete execution profile and route
+generation, keeps the logical dynamic profile, and passes the concrete profile
+to lifecycle. A second resolution reuses the stored route.
+
+The path validates the destination before preparation and removes a prepared
+replacement when route selection fails. If removal fails, it terminalizes the
+row so later workflow switches cannot reuse it. The regression now starts the
+created session through `StartCreatedSession` and reloads the persisted row.
+
+Verification:
+
+- `cd apps/backend && go test -tags fts5 ./internal/orchestrator -run 'TestCreateNewSessionForStep(_(ResolvesDynamicProfileBeforeWorkspaceAttach|RemovesPreparedSessionWhenDynamicResolutionFails|TerminalPrimaryReusesCanonicalEnvironment)|KeepsCurrentSessionWhenWorkspaceAttachFails)' -count=1`: seven tests passed.
+
+PR fixup: addressed two exact-head review findings. The test now proves
+persisted sticky attribution across the replacement and created-session start
+boundaries. Resolution failures no longer leave a partial replacement that a
+later workflow switch could promote.