diff --git a/packages/dashboard/app/components/WorkflowResultsTab.tsx b/packages/dashboard/app/components/WorkflowResultsTab.tsx index 4ec54f0a32..e94749dbb1 100644 --- a/packages/dashboard/app/components/WorkflowResultsTab.tsx +++ b/packages/dashboard/app/components/WorkflowResultsTab.tsx @@ -498,6 +498,30 @@ export function WorkflowResultsTab({ }, [effectiveWorkflowId, projectId]); const selectedWorkflowSteps = enabledWorkflowSteps ?? []; + /* + FNXC:TaskWorkflowDetails 2026-06-28-12:10: + The configured-steps panel is read-only status truth for non-editable in-progress tasks, so it must show the effective enabled optional steps: persisted task ids plus workflow optional-group ids marked `defaultOn`. Keep edit-mode controls bound to the persisted ids so toggling a default-on step writes only the explicit task override set. + + FNXC:TaskWorkflowDetails 2026-06-28-12:34: + Persisted enabledWorkflowSteps can store a materialized workflow-step id while optional groups resolve by templateId. Treat those ids as aliases when appending defaultOn steps so the configured panel does not count the same optional step twice. + */ + const effectiveEnabledStepIds = useMemo(() => { + const stepIds = [...selectedWorkflowSteps]; + const seen = new Set(stepIds); + for (const selectedStepId of selectedWorkflowSteps) { + for (const workflowStep of allWorkflowSteps) { + if (workflowStep.id !== selectedStepId && workflowStep.templateId !== selectedStepId) continue; + seen.add(workflowStep.id); + if (workflowStep.templateId) seen.add(workflowStep.templateId); + } + } + for (const step of optionalWorkflowSteps) { + if (!step.defaultOn || seen.has(step.templateId)) continue; + seen.add(step.templateId); + stepIds.push(step.templateId); + } + return stepIds; + }, [allWorkflowSteps, optionalWorkflowSteps, selectedWorkflowSteps]); const workflowStepOptions = useMemo(() => { const options: WorkflowStepOption[] = allWorkflowSteps.map((step) => ({ @@ -612,7 +636,7 @@ export function WorkflowResultsTab({ }, [onWorkflowStepsChange, selectedWorkflowSteps]); const hasResults = results.length > 0; - const hasConfiguredSteps = selectedWorkflowSteps.length > 0; + const hasConfiguredSteps = effectiveEnabledStepIds.length > 0; useEffect(() => { if (!canEdit) { @@ -625,7 +649,7 @@ export function WorkflowResultsTab({ Optional-group steps such as Code Review and Browser Verification are valid configured workflow steps but intentionally carry an empty description from the resolver. Show "Step definition not found." only when the step id is genuinely absent from the lookup, never for a found step whose description is empty. */ const configuredSteps = useMemo(() => { - return selectedWorkflowSteps.map((stepId) => { + return effectiveEnabledStepIds.map((stepId) => { const stepInfo = workflowStepLookup.get(stepId); const isMissingStepDefinition = stepInfo === undefined; return { @@ -637,7 +661,7 @@ export function WorkflowResultsTab({ phase: stepInfo?.phase || "pre-merge", } as WorkflowStepOption; }); - }, [selectedWorkflowSteps, workflowStepLookup, t]); + }, [effectiveEnabledStepIds, workflowStepLookup, t]); const workflowName = useMemo(() => getWorkflowName(effectiveWorkflowId, workflowDefinitions, t), [effectiveWorkflowId, workflowDefinitions, t]); const executionPhase = useMemo(() => getExecutionPhase(task, taskStatus, taskPausedReason, results, t), [task, taskStatus, taskPausedReason, results, t]); diff --git a/packages/dashboard/app/components/__tests__/WorkflowResultsTab.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowResultsTab.test.tsx index 0d89db8c81..e2e88492f0 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowResultsTab.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowResultsTab.test.tsx @@ -816,11 +816,119 @@ describe("WorkflowResultsTab", () => { expect(screen.queryByTestId("workflow-result-output-WS-004")).not.toBeInTheDocument(); }); - it("shows empty state when no workflow steps are configured", () => { + it("shows empty state when no workflow steps are configured", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([]); + render(); expect(screen.getByTestId("workflow-results-empty")).toBeInTheDocument(); expect(screen.getByText("No workflow steps configured for this task.")).toBeInTheDocument(); + await waitFor(() => expect(mockedFetchWorkflowOptionalSteps).toHaveBeenCalled()); + expect(screen.queryByTestId("workflow-configured-steps")).not.toBeInTheDocument(); + }); + + it("shows default-on optional steps for in-progress tasks without persisted workflow steps", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "", + phase: "pre-merge", + defaultOn: true, + }, + ]); + + render( + , + ); + + const configuredSteps = await screen.findByTestId("workflow-configured-steps"); + expect(configuredSteps).toBeInTheDocument(); + expect(screen.getByTestId("workflow-configured-step-browser-verification")).toHaveTextContent("Browser Verification"); + expect(screen.getByTestId("workflow-configured-count")).toHaveTextContent("1 step"); + expect(screen.queryByTestId("workflow-results-empty")).not.toBeInTheDocument(); + }); + + it("de-duplicates persisted optional steps that are also default-on", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "", + phase: "pre-merge", + defaultOn: true, + }, + ]); + + render( + , + ); + + await screen.findByTestId("workflow-configured-step-browser-verification"); + expect(screen.getByTestId("workflow-configured-count")).toHaveTextContent("1 step"); + expect(document.querySelectorAll('[data-testid="workflow-configured-step-browser-verification"]')).toHaveLength(1); + }); + + it("de-duplicates default-on optional steps when persisted ids use materialized workflow step aliases", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "", + phase: "pre-merge", + defaultOn: true, + }, + ]); + + render( + , + ); + + await screen.findByTestId("workflow-configured-step-WS-103"); + expect(screen.getByTestId("workflow-configured-count")).toHaveTextContent("1 step"); + expect(screen.getByTestId("workflow-configured-step-WS-103")).toHaveTextContent("Browser Verification"); + expect(document.querySelectorAll('[data-testid^="workflow-configured-step-"]')).toHaveLength(1); + }); + + it("keeps result rendering authoritative when default-on optional steps exist", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "", + phase: "pre-merge", + defaultOn: true, + }, + ]); + + render( + , + ); + + expect(screen.getByTestId("workflow-results-list")).toBeInTheDocument(); + expect(screen.getByTestId("workflow-result-item-browser-verification")).toBeInTheDocument(); + await waitFor(() => expect(mockedFetchWorkflowOptionalSteps).toHaveBeenCalled()); + expect(screen.queryByTestId("workflow-configured-steps")).not.toBeInTheDocument(); + expect(screen.queryByTestId("workflow-results-empty")).not.toBeInTheDocument(); }); it("shows configured step details when enabledWorkflowSteps is non-empty and results are empty", async () => { @@ -1272,6 +1380,42 @@ describe("WorkflowResultsTab", () => { expect(screen.queryByTestId("workflow-steps-editor")).not.toBeInTheDocument(); }); + it("keeps default-on optional steps display-only while editor toggles use persisted ids", async () => { + mockedFetchWorkflowOptionalSteps.mockResolvedValueOnce([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "", + phase: "pre-merge", + defaultOn: true, + }, + ]); + mockedFetchWorkflowSteps.mockResolvedValueOnce(mockWorkflowSteps.filter((step) => step.id !== "WS-103")); + const onWorkflowStepsChange = vi.fn(); + + render( + , + ); + + expect(await screen.findByTestId("workflow-configured-step-browser-verification")).toHaveTextContent("Browser Verification"); + fireEvent.click(screen.getByTestId("workflow-steps-edit-toggle")); + + const defaultOnCheckbox = within(await screen.findByTestId("workflow-step-checkbox-browser-verification")).getByRole("checkbox") as HTMLInputElement; + expect(defaultOnCheckbox.checked).toBe(false); + fireEvent.click(defaultOnCheckbox); + expect(onWorkflowStepsChange).toHaveBeenCalledWith(["WS-101", "WS-102", "browser-verification"]); + + onWorkflowStepsChange.mockClear(); + fireEvent.click(screen.getByTestId("workflow-step-remove-WS-101")); + expect(onWorkflowStepsChange).toHaveBeenCalledWith(["WS-102"]); + }); + it("calls onWorkflowStepsChange when checking and unchecking steps", async () => { const onWorkflowStepsChange = vi.fn();