diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 8595ef0ff6..0d51423b0f 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -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) diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow_dynamic_profile_test.go b/apps/backend/internal/orchestrator/event_handlers_workflow_dynamic_profile_test.go new file mode 100644 index 0000000000..57f487463b --- /dev/null +++ b/apps/backend/internal/orchestrator/event_handlers_workflow_dynamic_profile_test.go @@ -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) +} diff --git a/docs/plans/workflow-dynamic-profile-auto-start/plan.md b/docs/plans/workflow-dynamic-profile-auto-start/plan.md new file mode 100644 index 0000000000..cf3f7d65b4 --- /dev/null +++ b/docs/plans/workflow-dynamic-profile-auto-start/plan.md @@ -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. diff --git a/docs/plans/workflow-dynamic-profile-auto-start/task-01-resolve-dynamic-workflow-replacement-profiles.md b/docs/plans/workflow-dynamic-profile-auto-start/task-01-resolve-dynamic-workflow-replacement-profiles.md new file mode 100644 index 0000000000..16b04575cb --- /dev/null +++ b/docs/plans/workflow-dynamic-profile-auto-start/task-01-resolve-dynamic-workflow-replacement-profiles.md @@ -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.