From 6ffb157d7e9052d7372910f2f1e56be2bcd80c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 23 Apr 2026 09:54:06 +0100 Subject: [PATCH] fix: user workflow deadlock (#677) --- apps/backend/internal/github/service_test.go | 1 - .../orchestrator/event_handlers_agent.go | 15 +- .../event_handlers_github_issue_test.go | 3 +- .../event_handlers_github_test.go | 1 - .../orchestrator/event_handlers_workflow.go | 84 ++++- .../internal/orchestrator/task_operations.go | 109 ++++++ .../orchestrator/workflow_e2e_test.go | 37 +- .../task/handlers/task_http_handlers.go | 9 +- .../task/handlers/task_ws_handlers.go | 9 +- .../task/use-passthrough-terminal.ts | 54 +-- apps/web/e2e/fixtures/test-base.ts | 10 +- apps/web/e2e/helpers/git-helper.ts | 15 +- .../tests/layout/changes-panel-focus.spec.ts | 19 +- .../e2e/tests/pr/vcs-button-context.spec.ts | 28 +- .../workflow/workflow-agent-switch.spec.ts | 328 +++++++++++++++++- 15 files changed, 607 insertions(+), 115 deletions(-) diff --git a/apps/backend/internal/github/service_test.go b/apps/backend/internal/github/service_test.go index 667fab3657..bed5916df9 100644 --- a/apps/backend/internal/github/service_test.go +++ b/apps/backend/internal/github/service_test.go @@ -776,4 +776,3 @@ func TestTimeEqual(t *testing.T) { }) } } - diff --git a/apps/backend/internal/orchestrator/event_handlers_agent.go b/apps/backend/internal/orchestrator/event_handlers_agent.go index 76b980790e..c025410555 100644 --- a/apps/backend/internal/orchestrator/event_handlers_agent.go +++ b/apps/backend/internal/orchestrator/event_handlers_agent.go @@ -149,9 +149,20 @@ func (s *Service) handleAgentReady(ctx context.Context, data watcher.AgentEventD // Check for workflow transition based on session's current step. // Uses the engine when available; falls back to legacy evaluation. // The ViaEngine method handles setSessionWaitingForInput internally when no transition occurs. - s.processOnTurnCompleteViaEngine(ctx, data.TaskID, session) + transitioned := s.processOnTurnCompleteViaEngine(ctx, data.TaskID, session) - // ALWAYS check for queued messages after agent becomes ready, regardless of workflow transition + // When a workflow transition occurred (e.g. Work → Review), the new step's + // on_enter actions handle the next prompt (auto_start_agent launches a goroutine). + // Skip the queued-message check to avoid racing with that auto-start goroutine — + // both would try to call PromptTask and the loser's queued message would be lost. + if transitioned { + s.logger.Debug("workflow transition occurred, skipping queued message check", + zap.String("task_id", data.TaskID), + zap.String("session_id", data.SessionID)) + return + } + + // Check for queued messages when no workflow transition occurred. queueStatus := s.messageQueue.GetStatus(ctx, data.SessionID) s.logger.Info("checking for queued messages", zap.String("session_id", data.SessionID), diff --git a/apps/backend/internal/orchestrator/event_handlers_github_issue_test.go b/apps/backend/internal/orchestrator/event_handlers_github_issue_test.go index f935b0bf40..466a3b5f6e 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_issue_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_issue_test.go @@ -117,7 +117,6 @@ func setupIssueTaskTest(t *testing.T) (*Service, *mockStepGetter) { return createTestService(repo, stepGetter, newMockTaskRepo()), stepGetter } - func TestCreateIssueTask_SkipsWhenAlreadyReserved(t *testing.T) { svc, _ := setupIssueTaskTest(t) ghSvc := &mockGitHubService{issueReserveReturn: false} @@ -185,4 +184,4 @@ func TestCreateIssueTask_ReleasesOnTaskCreationFailure(t *testing.T) { if ghSvc.issueAssignCalls != 0 { t.Errorf("expected no Assign call after failure, got %d", ghSvc.issueAssignCalls) } -} \ No newline at end of file +} diff --git a/apps/backend/internal/orchestrator/event_handlers_github_test.go b/apps/backend/internal/orchestrator/event_handlers_github_test.go index 3fbfa4d531..563c99a36e 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_test.go @@ -674,4 +674,3 @@ func TestListTasksNeedingPRWatch(t *testing.T) { } }) } - diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index afe0028fbc..c2763ddb19 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -511,7 +511,9 @@ func (s *Service) switchSessionForStep(ctx context.Context, taskID string, curre } // Promote the new session to primary so it's loaded when navigating back to this task. - if err := s.repo.SetSessionPrimary(ctx, newSession.ID); err != nil { + // Use SetPrimarySession (not repo.SetSessionPrimary) to broadcast a task.updated WS + // event — the frontend reads primarySessionId from the task to render the star icon. + if err := s.SetPrimarySession(ctx, newSession.ID); err != nil { s.logger.Warn("failed to set new session as primary", zap.String("session_id", newSession.ID), zap.Error(err)) } @@ -526,8 +528,14 @@ func (s *Service) switchSessionForStep(ctx context.Context, taskID string, curre } // Mark the current session as completed. - currentSession.State = models.TaskSessionStateCompleted + // Use updateTaskSessionState to publish a session.state_changed WS event so the + // frontend learns the old session is terminal and can adopt the new one. + s.updateTaskSessionState(ctx, taskID, currentSession.ID, models.TaskSessionStateCompleted, "", false) + // Also set CompletedAt timestamp for bookkeeping (updateTaskSessionState only sets state). + // Clear IsPrimary to avoid overwriting the SetPrimarySession call above when writing back. now := time.Now().UTC() + currentSession.State = models.TaskSessionStateCompleted + currentSession.IsPrimary = false currentSession.CompletedAt = &now currentSession.UpdatedAt = now if err := s.repo.UpdateTaskSession(ctx, currentSession); err != nil { @@ -542,6 +550,9 @@ func (s *Service) switchSessionForStep(ctx context.Context, taskID string, curre // maybySwitchSessionForProfile checks whether the step requires a different agent profile // and switches the session if so. Passthrough sessions are returned unchanged. +// When the step has no agent_profile override, falls back to the task's original +// agent profile (from task metadata) so that moving to a "plain" step reverts +// to the default agent instead of keeping the previous step's override. // Returns the effective session (new or original) and whether processing should continue. // A false return means the switch failed; the caller should return immediately. func (s *Service) maybySwitchSessionForProfile( @@ -551,6 +562,18 @@ func (s *Service) maybySwitchSessionForProfile( return session, true } effectiveProfile := s.resolveStepAgentProfile(ctx, step) + + // When the step has no override, fall back to the task's original agent profile + // so that moving to a step without an agent_profile reverts to the default. + if effectiveProfile == "" { + task, err := s.repo.GetTask(ctx, taskID) + if err == nil && task.Metadata != nil { + if pid, ok := task.Metadata[models.MetaKeyAgentProfileID].(string); ok && pid != "" { + effectiveProfile = pid + } + } + } + if effectiveProfile == "" || effectiveProfile == session.AgentProfileID { return session, true } @@ -629,18 +652,18 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo case hasAutoStart: // ACP path: build prompt from step configuration. - // Run auto-start inline so queue state is visible before handleAgentReady - // checks for queued messages. + // When called from applyEngineTransition (on_turn_complete), processOnEnter + // runs in a goroutine and the session is already WAITING_FOR_INPUT, so + // autoStartStepPrompt sends the prompt directly via PromptTask. effectivePrompt := s.buildWorkflowPrompt(taskDescription, step, taskID, sessionID) - planMode := hasPlanMode - err := s.autoStartStepPrompt(ctx, taskID, session, step.Name, effectivePrompt, planMode, true) - if err != nil { + if err := s.autoStartStepPrompt(ctx, taskID, session, step.Name, effectivePrompt, hasPlanMode, true); err != nil { s.logger.Error("failed to auto-start agent for step", zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.String("step_name", step.Name), zap.Error(err)) s.setSessionWaitingForInput(ctx, taskID, sessionID, session) + s.publishSessionWaitingEvent(ctx, taskID, sessionID, step.ID, session) } default: @@ -650,19 +673,27 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo if sessionSwitched && step.Prompt != "" { effectivePrompt := s.buildWorkflowPrompt(taskDescription, step, taskID, sessionID) planMode := hasPlanMode + stepName := step.Name + stepID := step.ID s.logger.Info("auto-launching agent after profile switch (no explicit auto_start)", zap.String("task_id", taskID), zap.String("session_id", sessionID), - zap.String("step_name", step.Name)) - err := s.autoStartStepPrompt(ctx, taskID, session, step.Name, effectivePrompt, planMode, true) - if err != nil { - s.logger.Error("failed to launch agent after profile switch", - zap.String("task_id", taskID), - zap.String("session_id", sessionID), - zap.Error(err)) - s.setSessionWaitingForInput(ctx, taskID, sessionID, session) - s.publishSessionWaitingEvent(ctx, taskID, sessionID, step.ID, session) - } + zap.String("step_name", stepName)) + // Launch asynchronously because processOnEnter may also be called + // synchronously from finalizeStepEnter (manual task move). In that path, + // autoStartStepPrompt would block the caller's goroutine. + go func() { + asyncCtx := context.WithoutCancel(ctx) + err := s.autoStartStepPrompt(asyncCtx, taskID, session, stepName, effectivePrompt, planMode, true) + if err != nil { + s.logger.Error("failed to launch agent after profile switch", + zap.String("task_id", taskID), + zap.String("session_id", sessionID), + zap.Error(err)) + s.setSessionWaitingForInput(asyncCtx, taskID, sessionID, session) + s.publishSessionWaitingEvent(asyncCtx, taskID, sessionID, stepID, session) + } + }() return } s.setSessionWaitingForInput(ctx, taskID, sessionID, session) @@ -1232,7 +1263,24 @@ func (s *Service) applyEngineTransition( return true } - s.processOnEnter(ctx, taskID, session, targetStep, taskDescription) + // When triggered from on_turn_complete, the agent has finished its turn but + // handleAgentReady returns early without setting WAITING_FOR_INPUT (because the + // transition already occurred). The session is still RUNNING in the DB. + // Flip to WAITING_FOR_INPUT so that autoStartStepPrompt in processOnEnter sends + // the prompt directly instead of queueing it — the queue would never be drained + // because handleAgentReady already returned. + if session.State == models.TaskSessionStateRunning || session.State == models.TaskSessionStateStarting { + s.updateTaskSessionState(ctx, taskID, session.ID, models.TaskSessionStateWaitingForInput, "", false, session) + session.State = models.TaskSessionStateWaitingForInput + } + + // Launch processOnEnter asynchronously to avoid blocking the stream reader goroutine. + // When triggered from on_turn_complete, the entire call chain runs in the WebSocket + // stream reader goroutine (G_reader). processOnEnter may call resetAgentContext → + // ResetAgentContext → sendStreamRequest, which blocks G_reader waiting for a response + // that can only be delivered by G_reader reading from the same WebSocket — a deadlock. + // The DB transition is already persisted above, so it's safe to process on_enter async. + go s.processOnEnter(context.WithoutCancel(ctx), taskID, session, targetStep, taskDescription) return true } diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index ddb12ddd94..c99c2eb521 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -273,6 +273,41 @@ func (s *Service) StartCreatedSession(ctx context.Context, taskID, sessionID, ag return nil, fmt.Errorf("agent_profile_id is required") } + // Override with workflow step's agent profile if one is configured. + effectiveProfileID = s.resolveEffectiveAgentProfile(ctx, taskID, "", effectiveProfileID) + + // If the workflow step overrode the profile, update the session record in DB + // so the frontend tab displays the correct agent (it reads session.agent_profile_id). + if effectiveProfileID != session.AgentProfileID { + s.logger.Info("updating session agent profile for workflow step override", + zap.String("session_id", sessionID), + zap.String("old_profile", session.AgentProfileID), + zap.String("new_profile", effectiveProfileID)) + session.AgentProfileID = effectiveProfileID + // Re-resolve the agent profile snapshot so the tab shows the correct agent logo/name. + // Set a minimal snapshot first so stale data is never persisted if resolution fails. + session.AgentProfileSnapshot = map[string]interface{}{"id": effectiveProfileID} + if profileInfo, err := s.agentManager.ResolveAgentProfile(ctx, effectiveProfileID); err != nil { + s.logger.Warn("failed to resolve agent profile snapshot for workflow step override", + zap.String("session_id", sessionID), + zap.String("profile_id", effectiveProfileID), + zap.Error(err)) + } else if profileInfo != nil { + session.AgentProfileSnapshot = map[string]interface{}{ + "id": profileInfo.ProfileID, + "name": profileInfo.ProfileName, + "agent_id": profileInfo.AgentID, + "agent_name": profileInfo.AgentName, + "model": profileInfo.Model, + } + } + if err := s.repo.UpdateTaskSession(ctx, session); err != nil { + s.logger.Warn("failed to update session agent profile", + zap.String("session_id", sessionID), + zap.Error(err)) + } + } + // Transition task state: CREATED → SCHEDULING → (IN_PROGRESS via executor) if err := s.taskRepo.UpdateTaskState(ctx, taskID, v1.TaskStateScheduling); err != nil { s.logger.Warn("failed to update task state to SCHEDULING", @@ -391,6 +426,11 @@ func (s *Service) StartTask(ctx context.Context, taskID string, agentProfileID s s.moveTaskToWorkflowStep(ctx, taskID, workflowStepID) + // Resolve the workflow step's agent profile override. + // The frontend may pass the workspace default profile, but the step may + // require a different agent (e.g., Codex on "In Progress", Auggie on "Review"). + agentProfileID = s.resolveEffectiveAgentProfile(ctx, taskID, workflowStepID, agentProfileID) + // Fetch the task from the repository to get complete task info task, err := s.scheduler.GetTask(ctx, taskID) if err != nil { @@ -471,6 +511,75 @@ func (s *Service) moveTaskToWorkflowStep(ctx context.Context, taskID, workflowSt } } +// resolveEffectiveAgentProfile checks whether the task's workflow step overrides +// the agent profile. If the step (or workflow default) specifies a different +// profile, that profile is returned instead of the caller-provided one. +// This ensures the initial task start uses the step's agent — not just the +// workspace default the frontend sends. +func (s *Service) resolveEffectiveAgentProfile(ctx context.Context, taskID, workflowStepID, callerProfileID string) string { + if s.workflowStepGetter == nil { + s.logger.Debug("resolveEffectiveAgentProfile: no workflowStepGetter, using caller profile", + zap.String("task_id", taskID), + zap.String("caller_profile", callerProfileID)) + return callerProfileID + } + + // Determine the effective step ID: explicit param > task's current step. + effectiveStepID := workflowStepID + if effectiveStepID == "" { + dbTask, err := s.repo.GetTask(ctx, taskID) + if err != nil { + s.logger.Debug("resolveEffectiveAgentProfile: failed to load task from DB", + zap.String("task_id", taskID), + zap.Error(err)) + return callerProfileID + } + s.logger.Debug("resolveEffectiveAgentProfile: loaded task from DB", + zap.String("task_id", taskID), + zap.String("db_workflow_step_id", dbTask.WorkflowStepID)) + if dbTask.WorkflowStepID == "" { + s.logger.Debug("resolveEffectiveAgentProfile: task has no workflow step, using caller profile", + zap.String("task_id", taskID)) + return callerProfileID + } + effectiveStepID = dbTask.WorkflowStepID + } + + step, err := s.workflowStepGetter.GetStep(ctx, effectiveStepID) + if err != nil || step == nil { + s.logger.Debug("resolveEffectiveAgentProfile: failed to load step", + zap.String("task_id", taskID), + zap.String("step_id", effectiveStepID), + zap.Error(err)) + return callerProfileID + } + + s.logger.Debug("resolveEffectiveAgentProfile: loaded step", + zap.String("task_id", taskID), + zap.String("step_id", effectiveStepID), + zap.String("step_name", step.Name), + zap.String("step_agent_profile_id", step.AgentProfileID), + zap.String("step_workflow_id", step.WorkflowID)) + + stepProfile := s.resolveStepAgentProfile(ctx, step) + s.logger.Debug("resolveEffectiveAgentProfile: resolved step profile", + zap.String("task_id", taskID), + zap.String("step_profile", stepProfile), + zap.String("caller_profile", callerProfileID)) + + if stepProfile == "" || stepProfile == callerProfileID { + return callerProfileID + } + + s.logger.Info("overriding agent profile with workflow step profile", + zap.String("task_id", taskID), + zap.String("step_id", effectiveStepID), + zap.String("step_name", step.Name), + zap.String("caller_profile", callerProfileID), + zap.String("step_profile", stepProfile)) + return stepProfile +} + // postLaunchStart records the initial message and sets plan mode after a successful launch. func (s *Service) postLaunchStart(ctx context.Context, taskID string, execution *executor.TaskExecution, prompt string, recordPlanMode, setPlanMode bool, attachments []v1.MessageAttachment) { if execution.SessionID != "" { diff --git a/apps/backend/internal/orchestrator/workflow_e2e_test.go b/apps/backend/internal/orchestrator/workflow_e2e_test.go index e97a235b23..ce8af1b69d 100644 --- a/apps/backend/internal/orchestrator/workflow_e2e_test.go +++ b/apps/backend/internal/orchestrator/workflow_e2e_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "testing" + "time" "github.com/kandev/kandev/internal/orchestrator/executor" "github.com/kandev/kandev/internal/orchestrator/messagequeue" @@ -111,9 +112,13 @@ var workflowTestCases = []workflowTestCase{ WorkflowJSON: developmentWorkflowJSON, StartStep: "Backlog", Events: []testEvent{ - // Agent finishes at Backlog → In Progress (auto_start queues) + // Agent finishes at Backlog → In Progress (auto_start_agent on_enter). + // applyEngineTransition flips RUNNING → WAITING_FOR_INPUT before the + // async processOnEnter goroutine, so autoStartStepPrompt attempts a + // direct send instead of queueing. In this mock environment the send + // fails (no executor), so the message is NOT queued. {Trigger: engine.TriggerOnTurnComplete, ExpectStep: "In Progress", - ExpectTransitioned: true, ExpectQueued: true, ExpectResets: 0}, + ExpectTransitioned: true, ExpectQueued: false, ExpectResets: 0}, // Agent finishes at In Progress → New Context (reset + auto_start). // After reset_agent_context, processOnEnter flips session state to // WAITING_FOR_INPUT so auto_start sends directly instead of queueing @@ -244,6 +249,12 @@ func runSingleEvent( t.Fatalf("unsupported trigger: %s", ev.Trigger) } + // processOnEnter runs asynchronously (goroutine) after engine transitions. + // Give it time to complete before checking side effects. + if transitioned { + time.Sleep(100 * time.Millisecond) + } + if transitioned != ev.ExpectTransitioned { t.Errorf("transitioned = %v, want %v", transitioned, ev.ExpectTransitioned) } @@ -364,13 +375,23 @@ func assertQueueState(t *testing.T, ctx context.Context, svc *Service, sessionID } // assertSessionState verifies the session's current state. +// Polls briefly because processOnEnter runs asynchronously (goroutine) and +// the state change may not be visible immediately after the trigger returns. func assertSessionState(t *testing.T, ctx context.Context, repo sessionExecutorStore, sessionID string, expect models.TaskSessionState) { t.Helper() - session, err := repo.GetTaskSession(ctx, sessionID) - if err != nil { - t.Fatalf("failed to load session: %v", err) - } - if session.State != expect { - t.Errorf("session state = %q, want %q", session.State, expect) + deadline := time.Now().Add(500 * time.Millisecond) + for { + session, err := repo.GetTaskSession(ctx, sessionID) + if err != nil { + t.Fatalf("failed to load session: %v", err) + } + if session.State == expect { + return + } + if time.Now().After(deadline) { + t.Errorf("session state = %q, want %q (after polling)", session.State, expect) + return + } + time.Sleep(10 * time.Millisecond) } } diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index d5df41501d..c3870a56f1 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -408,10 +408,11 @@ func (h *TaskHandlers) httpCreateTask(c *gin.Context) { return } - // When not starting the agent immediately, persist profile IDs in task metadata - // so handleTaskMovedNoSession can retrieve them when the task is later dragged - // to a step with auto_start_agent. - if !body.StartAgent && !body.PrepareSession && body.AgentProfileID != "" { + // Always persist profile IDs in task metadata so they can be used as the + // task's "default" agent profile. This is needed both for deferred agent start + // (handleTaskMovedNoSession) and for reverting to the default agent when a + // workflow step has no agent_profile override. + if body.AgentProfileID != "" { if body.Metadata == nil { body.Metadata = make(map[string]interface{}) } diff --git a/apps/backend/internal/task/handlers/task_ws_handlers.go b/apps/backend/internal/task/handlers/task_ws_handlers.go index 5bf1550161..b87b02ee42 100644 --- a/apps/backend/internal/task/handlers/task_ws_handlers.go +++ b/apps/backend/internal/task/handlers/task_ws_handlers.go @@ -128,10 +128,11 @@ func (h *TaskHandlers) wsCreateTask(ctx context.Context, msg *ws.Message) (*ws.M }) } - // When not starting the agent immediately, persist profile IDs in task metadata - // so handleTaskMovedNoSession can retrieve them when the task is later dragged - // to a step with auto_start_agent. - if !req.StartAgent && req.AgentProfileID != "" { + // Always persist profile IDs in task metadata so they can be used as the + // task's "default" agent profile. This is needed both for deferred agent start + // (handleTaskMovedNoSession) and for reverting to the default agent when a + // workflow step has no agent_profile override. + if req.AgentProfileID != "" { if req.Metadata == nil { req.Metadata = make(map[string]interface{}) } diff --git a/apps/web/components/task/use-passthrough-terminal.ts b/apps/web/components/task/use-passthrough-terminal.ts index e685cf0db8..223805d166 100644 --- a/apps/web/components/task/use-passthrough-terminal.ts +++ b/apps/web/components/task/use-passthrough-terminal.ts @@ -71,6 +71,39 @@ function createKeyEventHandler(options: TerminalKeyHandlerOptions) { }; } +/** + * Defer WebGL addon loading to the next animation frame so the initial + * synchronous work (Terminal + FitAddon + open) stays within the browser's + * frame budget. + * + * Skips WebGL on Firefox: its canvas fingerprinting protection silently + * poisons readback data, corrupting the glyph texture atlas. + */ +function deferWebGLAddon(refs: Pick) { + const isFirefox = typeof navigator !== "undefined" && /firefox/i.test(navigator.userAgent); + if (isFirefox) { + log("Skipping WebGL addon on Firefox (canvas fingerprinting protection)"); + return; + } + requestAnimationFrame(() => { + const term = refs.xtermRef.current; + if (!term || refs.webglAddonRef.current) return; + try { + const webglAddon = new WebglAddon(); + webglAddon.onContextLoss(() => { + log("WebGL context lost"); + webglAddon.dispose(); + refs.webglAddonRef.current = null; + }); + term.loadAddon(webglAddon); + refs.webglAddonRef.current = webglAddon; + log("WebGL addon loaded"); + } catch (e) { + log("WebGL failed, using canvas:", e); + } + }); +} + function initTerminalInstance( termContainer: HTMLDivElement, refs: Pick< @@ -121,26 +154,7 @@ function initTerminalInstance( } refs.xtermRef.current = terminal; refs.fitAddonRef.current = fitAddon; - // Defer WebGL addon loading to the next animation frame so the initial - // synchronous work (Terminal + FitAddon + open) stays within the browser's - // frame budget and avoids "Violation: 'setTimeout' handler took Xms" warnings. - requestAnimationFrame(() => { - const term = refs.xtermRef.current; - if (!term || refs.webglAddonRef.current) return; - try { - const webglAddon = new WebglAddon(); - webglAddon.onContextLoss(() => { - log("WebGL context lost"); - webglAddon.dispose(); - refs.webglAddonRef.current = null; - }); - term.loadAddon(webglAddon); - refs.webglAddonRef.current = webglAddon; - log("WebGL addon loaded"); - } catch (e) { - log("WebGL failed, using canvas:", e); - } - }); + deferWebGLAddon(refs); exposeBufferReader(termContainer, terminal); const handleResize = () => { const rect = termContainer.getBoundingClientRect(); diff --git a/apps/web/e2e/fixtures/test-base.ts b/apps/web/e2e/fixtures/test-base.ts index 241848017c..bee12b7023 100644 --- a/apps/web/e2e/fixtures/test-base.ts +++ b/apps/web/e2e/fixtures/test-base.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { backendFixture, type BackendContext } from "./backend"; import { ApiClient } from "../helpers/api-client"; import { PrAssetCapture } from "../helpers/pr-asset-capture"; +import { makeGitEnv } from "../helpers/git-helper"; import type { WorkflowStep } from "../../lib/types/http"; export type SeedData = { @@ -48,14 +49,7 @@ export const test = backendFixture.extend< // This ensures discoveryRoots() allows the path for branch listing. const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); fs.mkdirSync(repoDir, { recursive: true }); - const gitEnv = { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }; + const gitEnv = makeGitEnv(backend.tmpDir); execSync("git init -b main", { cwd: repoDir, env: gitEnv }); execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv }); const repo = await apiClient.createRepository(workspace.id, repoDir); diff --git a/apps/web/e2e/helpers/git-helper.ts b/apps/web/e2e/helpers/git-helper.ts index 9430d3eac5..74bc45d6d8 100644 --- a/apps/web/e2e/helpers/git-helper.ts +++ b/apps/web/e2e/helpers/git-helper.ts @@ -62,9 +62,22 @@ export class GitHelper { } } +/** + * Strip GIT_CONFIG_* environment variables that can inject global git hooks + * into fresh git repos, breaking E2E test setup. + */ +function stripGitConfigOverrides(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const clean: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(env)) { + if (/^GIT_CONFIG_(KEY|VALUE|COUNT|PARAMETERS|GLOBAL|SYSTEM)/i.test(key)) continue; + clean[key] = value; + } + return clean; +} + export function makeGitEnv(tmpDir: string): NodeJS.ProcessEnv { return { - ...process.env, + ...stripGitConfigOverrides(process.env), HOME: tmpDir, GIT_AUTHOR_NAME: "E2E Test", GIT_AUTHOR_EMAIL: "e2e@test.local", diff --git a/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts b/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts index c4a4245d37..9e7a4f843c 100644 --- a/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts +++ b/apps/web/e2e/tests/layout/changes-panel-focus.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; +import { makeGitEnv } from "../../helpers/git-helper"; import fs from "node:fs"; import path from "node:path"; import { execSync } from "node:child_process"; @@ -47,14 +48,7 @@ test.describe("Changes panel focus behavior", () => { test.setTimeout(90_000); const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); - const gitEnv = { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }; + const gitEnv = makeGitEnv(backend.tmpDir); const git = new GitHelper(repoDir, gitEnv); // Create a task and wait for the agent to be ready @@ -118,14 +112,7 @@ test.describe("Changes panel focus behavior", () => { test.setTimeout(90_000); const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); - const gitEnv = { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }; + const gitEnv = makeGitEnv(backend.tmpDir); const git = new GitHelper(repoDir, gitEnv); const task = await apiClient.createTaskWithAgent( diff --git a/apps/web/e2e/tests/pr/vcs-button-context.spec.ts b/apps/web/e2e/tests/pr/vcs-button-context.spec.ts index e281bd3a21..c8a05a52fa 100644 --- a/apps/web/e2e/tests/pr/vcs-button-context.spec.ts +++ b/apps/web/e2e/tests/pr/vcs-button-context.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; +import { makeGitEnv } from "../../helpers/git-helper"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import type { Page } from "@playwright/test"; @@ -114,14 +115,7 @@ test.describe("VCS split button context", () => { // Set up git with a remote so we can get "ahead" status const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); - const gitEnv = { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }; + const gitEnv = makeGitEnv(backend.tmpDir); const git = new GitHelper(repoDir, gitEnv); git.setupRemote(backend.tmpDir); @@ -179,14 +173,7 @@ test.describe("VCS split button context", () => { // Set up git with a remote const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); - const gitEnv = { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }; + const gitEnv = makeGitEnv(backend.tmpDir); const git = new GitHelper(repoDir, gitEnv); // Only set up remote if not already configured @@ -251,14 +238,7 @@ test.describe("VCS split button context", () => { // Create an uncommitted file const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); - const git = new GitHelper(repoDir, { - ...process.env, - HOME: backend.tmpDir, - GIT_AUTHOR_NAME: "E2E Test", - GIT_AUTHOR_EMAIL: "e2e@test.local", - GIT_COMMITTER_NAME: "E2E Test", - GIT_COMMITTER_EMAIL: "e2e@test.local", - }); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); git.createFile("uncommitted.txt", "uncommitted content"); // Even with open PR, uncommitted files → "Commit" takes priority diff --git a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts index 4e1fc384e9..4a835f8922 100644 --- a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts @@ -292,12 +292,19 @@ test.describe("Workflow agent profile switching", () => { // Move to Step2 — should auto-launch agent despite no auto_start_agent await apiClient.moveTask(task.id, workflow.id, step2.id); - // Poll for a session with profileB that has completed at least one turn - const finalSessions = await pollSessions(apiClient, task.id, 2, 30_000); - const step2Session = finalSessions.find((s) => s.agent_profile_id === profileB.id); - expect(step2Session).toBeDefined(); - // The session should have progressed past CREATED (agent was launched) - expect(step2Session!.state).not.toBe("CREATED"); + // Poll until a session with profileB progresses past CREATED (agent launched). + // The agent launch is async (goroutine), so we need to wait for state progression. + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + const step2Session = sessions.find((s) => s.agent_profile_id === profileB.id); + if (!step2Session || step2Session.state === "CREATED") return "CREATED"; + return step2Session.state; + }, + { timeout: 30_000, message: "Waiting for step2 agent to launch past CREATED" }, + ) + .not.toBe("CREATED"); }); test("new session inherits task_environment_id from old session", async ({ @@ -357,6 +364,315 @@ test.describe("Workflow agent profile switching", () => { } }); + /** + * Verifies the StartCreatedSession fix: when a task is created in a step with an + * agent profile override, the session tab shows the step's agent profile name + * (not the workspace default). + */ + test("initial task creation shows step's agent profile in session tab", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const { profileA } = await createProfiles(apiClient); + + // Create workflow with Step1 (profileA, auto_start, is_start) + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Initial Profile Tab"); + const step1 = await apiClient.createWorkflowStep(workflow.id, "Step1", 0, { + is_start_step: true, + }); + await apiClient.createWorkflowStep(workflow.id, "Done", 1); + + await apiClient.updateWorkflowStep(step1.id, { + agent_profile_id: profileA.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + + // Create task — should use Step1's profileA, not the workspace default + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Initial Profile Task", + profileA.id, + { + workflow_id: workflow.id, + workflow_step_id: step1.id, + repository_ids: [seedData.repositoryId], + }, + ); + + // Navigate to task + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await expect(session.chat).toBeVisible({ timeout: 15_000 }); + + // The session tab should show "Profile A" (the step's agent profile name) + const sessionTab = testPage.locator('[data-testid^="session-tab-"]').first(); + await expect(sessionTab).toBeVisible({ timeout: 30_000 }); + await expect(sessionTab).toContainText("Profile A", { timeout: 10_000 }); + }); + + /** + * Verifies the switchSessionForStep WS event fix: when a task is manually moved + * to a step with a different agent profile, the new session tab becomes the + * active dockview tab. + */ + test("manual step move switches active session tab to new agent", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const { profileA, profileB } = await createProfiles(apiClient); + + // Step1 (profileA, auto_start, is_start) → Step2 (profileB, auto_start) + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Active Tab Switch"); + const step1 = await apiClient.createWorkflowStep(workflow.id, "Step1", 0, { + is_start_step: true, + }); + const step2 = await apiClient.createWorkflowStep(workflow.id, "Step2", 1); + await apiClient.createWorkflowStep(workflow.id, "Done", 2); + + await apiClient.updateWorkflowStep(step1.id, { + agent_profile_id: profileA.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + await apiClient.updateWorkflowStep(step2.id, { + agent_profile_id: profileB.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Tab Switch Task", + profileA.id, + { + workflow_id: workflow.id, + workflow_step_id: step1.id, + repository_ids: [seedData.repositoryId], + }, + ); + + // Navigate and wait for first session + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await expect(session.chat).toBeVisible({ timeout: 15_000 }); + + // Wait for Profile A tab to appear and be active + const profileATab = session.sessionTabByText("Profile A"); + await expect(profileATab).toBeVisible({ timeout: 30_000 }); + + // Wait for agent to be ready (WAITING_FOR_INPUT) + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 30_000, message: "Waiting for agent to be ready" }, + ) + .toBe(true); + + // Move to Step2 — triggers new session with profileB + await apiClient.moveTask(task.id, workflow.id, step2.id); + + // The new "Profile B" tab should appear and become the active dockview tab + const activeProfileBTab = testPage.locator( + '.dv-tab.dv-active-tab [data-testid^="session-tab-"]', + ); + await expect(activeProfileBTab).toContainText("Profile B", { timeout: 30_000 }); + }); + + /** + * Verifies that an on_turn_complete cascade that switches agent profiles + * also switches the active dockview session tab. + */ + test("on_turn_complete cascade switches active session tab to new agent", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const { profileA, profileB } = await createProfiles(apiClient); + + // Step1 (profileA, auto_start, move_to_next) → Step2 (profileB, auto_start) + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Cascade Tab Switch"); + const inbox = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0); + const step1 = await apiClient.createWorkflowStep(workflow.id, "Step1", 1); + const step2 = await apiClient.createWorkflowStep(workflow.id, "Step2", 2); + await apiClient.createWorkflowStep(workflow.id, "Done", 3); + + await apiClient.updateWorkflowStep(step1.id, { + agent_profile_id: profileA.id, + prompt: 'e2e:delay(1000)\ne2e:message("step1 done")', + events: { + on_enter: [{ type: "auto_start_agent" }], + on_turn_complete: [{ type: "move_to_next" }], + }, + }); + await apiClient.updateWorkflowStep(step2.id, { + agent_profile_id: profileB.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + + const task = await apiClient.createTask(seedData.workspaceId, "Cascade Tab Task", { + workflow_id: workflow.id, + workflow_step_id: inbox.id, + agent_profile_id: profileA.id, + repository_ids: [seedData.repositoryId], + }); + + // Navigate to task page first so WS is connected + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await expect(session.chat).toBeVisible({ timeout: 15_000 }); + + // Move to Step1 → auto_start with profileA → on_turn_complete → Step2 (profileB) + await apiClient.moveTask(task.id, workflow.id, step1.id); + + // After the cascade, the active tab should be "Profile B" (the final step's agent) + const activeProfileBTab = testPage.locator( + '.dv-tab.dv-active-tab [data-testid^="session-tab-"]', + ); + await expect(activeProfileBTab).toContainText("Profile B", { timeout: 45_000 }); + }); + + /** + * Verifies that after a manual step move, the new session tab shows + * the primary star icon (★) without needing a page reload. + * Covers the fix: switchSessionForStep uses SetPrimarySession (with WS + * broadcast) instead of the raw repo.SetSessionPrimary. + */ + test("primary star icon appears on new session tab after step move", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const { profileA, profileB } = await createProfiles(apiClient); + + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Primary Star Test"); + const step1 = await apiClient.createWorkflowStep(workflow.id, "Step1", 0, { + is_start_step: true, + }); + const step2 = await apiClient.createWorkflowStep(workflow.id, "Step2", 1); + await apiClient.createWorkflowStep(workflow.id, "Done", 2); + + await apiClient.updateWorkflowStep(step1.id, { + agent_profile_id: profileA.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + await apiClient.updateWorkflowStep(step2.id, { + agent_profile_id: profileB.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Primary Star Task", + profileA.id, + { + workflow_id: workflow.id, + workflow_step_id: step1.id, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await expect(session.chat).toBeVisible({ timeout: 15_000 }); + + // Wait for Profile A session to be ready + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 30_000, message: "Waiting for agent to be ready" }, + ) + .toBe(true); + + // Move to Step2 + await apiClient.moveTask(task.id, workflow.id, step2.id); + + // The Profile B tab should become active + const activeTab = testPage.locator('.dv-tab.dv-active-tab [data-testid^="session-tab-"]'); + await expect(activeTab).toContainText("Profile B", { timeout: 30_000 }); + + // The star icon should be visible inside the active tab (no reload needed) + const starIcon = activeTab.locator("svg.tabler-icon-star"); + await expect(starIcon).toBeVisible({ timeout: 5_000 }); + }); + + /** + * Verifies that moving to a step with NO agent_profile override reverts + * to the task's original (default) agent profile from task metadata. + * Covers the fix: maybySwitchSessionForProfile falls back to + * task.Metadata["agent_profile_id"] when the step has no override. + */ + test("moving to step without agent override reverts to task default agent", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const { profileA, profileB } = await createProfiles(apiClient); + + // Step1 (profileB, auto_start) → Step2 (NO override, auto_start) + // Task created with profileA → Step1 overrides to profileB → Step2 should revert to profileA + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Revert Agent Test"); + const step1 = await apiClient.createWorkflowStep(workflow.id, "Step1", 0, { + is_start_step: true, + }); + const step2 = await apiClient.createWorkflowStep(workflow.id, "Step2", 1); + await apiClient.createWorkflowStep(workflow.id, "Done", 2); + + await apiClient.updateWorkflowStep(step1.id, { + agent_profile_id: profileB.id, + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + // Step2 has NO agent_profile_id — should revert to task default (profileA) + await apiClient.updateWorkflowStep(step2.id, { + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Revert Agent Task", + profileA.id, + { + workflow_id: workflow.id, + workflow_step_id: step1.id, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await expect(session.chat).toBeVisible({ timeout: 15_000 }); + + // Wait for Profile B session (Step1 override) to be ready + const profileBTab = session.sessionTabByText("Profile B"); + await expect(profileBTab).toBeVisible({ timeout: 30_000 }); + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 30_000, message: "Waiting for agent to be ready" }, + ) + .toBe(true); + + // Move to Step2 (no override) — should revert to profileA + await apiClient.moveTask(task.id, workflow.id, step2.id); + + // The active tab should show Profile A (the task's default agent) + const activeTab = testPage.locator('.dv-tab.dv-active-tab [data-testid^="session-tab-"]'); + await expect(activeTab).toContainText("Profile A", { timeout: 30_000 }); + }); + test("reset context checkbox is disabled when step has agent profile override", async ({ testPage, apiClient,