diff --git a/.changeset/workflow-optional-steps.md b/.changeset/workflow-optional-steps.md new file mode 100644 index 0000000000..140a3347ab --- /dev/null +++ b/.changeset/workflow-optional-steps.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Add workflow optional steps: workflows can declare optional step templates that tasks toggle on/off per task, with a workflow-level default. The built-in coding and stepwise-coding workflows expose agent browser verification as an optional step (the stepwise workflow gains a pre-merge workflow-step seam so enabled steps actually run). Optional steps are authorable in the node editor, preserved across node-editor saves, and selectable from a steps dropdown in both the quick-add card and the full New Task modal. diff --git a/docs/plans/2026-06-20-001-feat-workflow-optional-steps-node-editor-modal-plan.md b/docs/plans/2026-06-20-001-feat-workflow-optional-steps-node-editor-modal-plan.md new file mode 100644 index 0000000000..07ef59d0ac --- /dev/null +++ b/docs/plans/2026-06-20-001-feat-workflow-optional-steps-node-editor-modal-plan.md @@ -0,0 +1,556 @@ +--- +title: "feat: Workflow optional steps — node-editor authoring, full-modal parity, stepwise declaration" +status: active +date: 2026-06-20 +type: feat +plan_id: 2026-06-20-001-feat-workflow-optional-steps-node-editor-modal +--- + +# feat: Workflow optional steps — node-editor authoring, full-modal parity, stepwise declaration + +## Summary + +Workflows can declare **optional steps** — workflow-step templates (e.g. Browser Verification) that a +task may toggle on/off per task, with a workflow-level `defaultOn` seeding the initial state. Most of +the plumbing for this already exists on the `gsxdsm/workflow-optional-steps` branch: the IR type +(`WorkflowOptionalStep` + `WorkflowIrV2.optionalSteps`), parse-time validation, the +`resolveWorkflowOptionalSteps` resolver, the `GET /workflows/:id/optional-steps` route, the +`fetchWorkflowOptionalSteps` API client, the executor's `enabledWorkflowSteps` execution path, the +**inline quick-create card** toggles, and the **task-detail Workflow tab** edit toggles. The built-in +**coding** workflow already declares `browser-verification` as its one optional step. + +This plan closes the five remaining gaps: + +1. The **built-in stepwise-coding workflow** does not yet support `browser-verification` as an optional + step — and, unlike the coding workflow, its graph has **no `workflow-step` seam node**, so even + declaring the optional step would create a dead toggle that never executes. Both the seam node and the + declaration are needed. +2. The **node editor's save path (`flowToIr`) silently drops `optionalSteps`** — editing any workflow in + the visual editor and saving destroys its optional-step declaration (a real data-loss bug). +3. The **node editor has no UI** to view or author optional steps (the user's explicit ask: "the node + editor view also needs to show optional steps properly"). +4. The **full New Task modal** (`NewTaskModal` / `TaskForm`) lacks the optional-step toggles that the + inline quick-create card already has, so the two creation surfaces are inconsistent. +5. The **quick-add area** exposes optional steps only as inline chip toggle buttons; it should instead + offer a dedicated **steps dropdown** for selecting/deselecting optional steps. + +**Plan depth:** Standard. This is focused, additive work building on substantial existing scaffolding — +no new persistence model, no executor changes. + +--- + +## Problem Frame + +The optional-steps feature is ~70% built but has a correctness hole and two missing surfaces: + +- **Data loss:** `flowToIr` (`packages/dashboard/app/components/workflow-flow-mapping.ts:435`) + reconstructs the v2 IR from canvas nodes and re-attaches `fields` and `settings` from threaded + arguments, but **never carries `optionalSteps`**. Any round-trip through the node editor (open custom + workflow → save) strips the declaration. The built-in coding workflow is safe only because it is never + re-serialized through the editor, but any user-authored or edited workflow loses optional steps on + first save. +- **Missing authoring surface:** `WorkflowNodeEditor.tsx` has zero optional-step awareness. There is no + way to declare which step templates are optional, or set their `defaultOn`, from the visual editor — + the only way today is hand-editing IR. +- **Inconsistent creation surfaces:** `InlineCreateCard.tsx` fully implements optional-step toggles + (fetches via `fetchWorkflowOptionalSteps`, seeds enabled set from `defaultOn`, sends + `enabledWorkflowSteps` on submit). The richer `NewTaskModal` / `TaskForm` create path exposes only a + whole-workflow dropdown. +- **Incomplete built-in parity + missing seam:** the original request asks for browser verification to be + optional in **both** the coding and stepwise-coding built-ins; only coding has it. Critically, the + coding IR routes through a `workflow-step` seam node (`builtin-coding-workflow-ir.ts:69`, + `execute → workflow-step → review`) which is the only thing that makes the graph call `runWorkflowSteps` + (`workflow-node-handlers.ts:264`). The stepwise IR has no such node, so its `enabledWorkflowSteps` are + never executed on the main graph path — making this an executor-graph change, not just a declaration. +- **Quick-add chip UX (user-requested):** the quick-add card renders optional steps as inline chip + toggles. The user explicitly asked for a dedicated **steps dropdown** in the quick-add area; R7/U5 honor + that request (this is the stated goal behind R7, not an inferred redesign of a working surface). + +The scope is bounded to authoring + display + creation-surface parity, **plus the stepwise seam node** +needed to make the stepwise optional step actually run. The executor's `enabledWorkflowSteps` runtime path +is wired and verified **for workflows that contain a `workflow-step` seam node** (coding); the resolver, +route, and edit-after-creation Workflow tab are already wired and covered by existing tests. This plan does +not change the executor itself beyond adding the stepwise seam node and a stepwise execution test. + +--- + +## Requirements + +- **R1** — The built-in **stepwise-coding** workflow declares `browser-verification` as an optional step, + default OFF, matching the coding workflow, **and runs it when enabled** — which requires adding a + `workflow-step` seam node to the stepwise graph so `enabledWorkflowSteps` actually execute. +- **R2** — Saving a workflow through the node editor **preserves** any declared `optionalSteps` (no + round-trip data loss). +- **R3** — The node editor provides an **Optional Steps** authoring panel where a workflow author can add + or remove optional step declarations and set each one's `defaultOn` (on/off). +- **R4** — The full **New Task modal** exposes the same optional-step toggles as the inline quick-create + card: it loads the selected workflow's optional steps, seeds enabled state from `defaultOn`, and submits + the chosen `enabledWorkflowSteps`. +- **R5** — Unknown/stale optional-step template ids never crash any surface (resolver already drops them; + the new UI must follow the same defensive posture). +- **R6** — Legacy v1 workflows and workflows with no optional steps continue to serialize and render + byte-identically (additive-only; `optionalSteps` omitted entirely when empty). +- **R7** — The quick-add area presents optional steps as a multi-select **steps dropdown** (select / + deselect), replacing the current inline chip toggles, while preserving the same `defaultOn` seeding and + `enabledWorkflowSteps` submit behavior. + +--- + +## High-Level Technical Design + +The optional-steps declaration flows from the workflow IR through three independent consumer surfaces. +The two new/fixed edges are the node-editor round-trip (U2/U3) and the full-modal create path (U4). + +```mermaid +flowchart TD + IR["WorkflowIrV2.optionalSteps\n[{ templateId, defaultOn? }]"] + IR -->|resolveWorkflowOptionalSteps| RES["ResolvedWorkflowOptionalStep[]\n(name, description, icon, phase, defaultOn)"] + RES -->|GET /workflows/:id/optional-steps| API["fetchWorkflowOptionalSteps()"] + + subgraph Authoring [Node editor — U2 + U3] + NE["WorkflowNodeEditor state\n(optionalStepsOf)"] -->|flowToIr(... optionalSteps)| SAVE["saved IR\n(preserves optionalSteps)"] + SAVE -.-> IR + end + + subgraph Create [Task creation] + API --> INLINE["InlineCreateCard\n(U5 — chips → steps dropdown)"] + API --> MODAL["NewTaskModal / TaskForm\n(U4 — new)"] + INLINE --> ENABLED["enabledWorkflowSteps[] on createTask"] + MODAL --> ENABLED + end + + subgraph Edit [Post-create] + API --> WRT["WorkflowResultsTab\n(DONE)"] + end + + ENABLED --> SEAM["workflow-step seam node\n(coding: present · stepwise: ADDED in U1)"] + SEAM --> EXEC["runWorkflowSteps()\n(runs enabledWorkflowSteps)"] +``` + +Key boundary: the `optionalSteps` **declaration** is execution-inert — it only advertises which templates +are toggleable and seeds per-task `enabledWorkflowSteps`. But execution of those steps is **not** free: +`runWorkflowSteps` fires only when the graph reaches a `workflow-step` seam node +(`workflow-node-handlers.ts:264`). The coding IR has one; the stepwise IR does not, so U1 adds it. With +that one exception, the executor is untouched — the rest of the work is confined to declaration (core), +serialization (dashboard mapping), and UI (dashboard components). + +--- + +## Key Technical Decisions + +- **KTD-1 — Thread `optionalSteps` through `flowToIr` as an explicit parameter**, mirroring how `fields` + and `settings` are threaded today (`workflow-flow-mapping.ts:435`). Optional steps are not graph nodes, + so they cannot be reconstructed from the canvas; they must be passed alongside `columns`/`fields`/ + `settings` and re-attached to the v2 IR. Add an `optionalStepsOf(def)` reader mirroring `fieldsOf` / + `settingsOf` so `WorkflowNodeEditor` can hydrate editor state from a loaded workflow. +- **KTD-2 — Treat a non-empty `optionalSteps` as a v2 signal**, like `fields`/`settings`. A workflow with + optional steps but no custom columns still serializes as v2 with synthesized default columns. Empty/ + absent `optionalSteps` must be omitted entirely from the serialized IR (never `optionalSteps: []` or + `null`) to preserve R6 byte-identity for legacy graphs. +- **KTD-3 — Author optional steps from a catalog of step templates, not free text.** The Optional Steps + panel picks from `WORKFLOW_STEP_TEMPLATES` (plus plugin templates where available), storing only + `{ templateId, defaultOn }`. Display metadata (name/description/icon/phase) is always resolved from the + template catalog at render time — never duplicated into the IR — so the resolver stays the single source + of truth and stale ids degrade gracefully (R5). +- **KTD-4 — Reuse the inline card's create semantics in the full modal.** `TaskForm`/`NewTaskModal` should + load optional steps for the selected workflow, seed the enabled set from `defaultOn`, re-seed when the + workflow selection changes, and pass `enabledWorkflowSteps` only when non-empty (matching + `InlineCreateCard.tsx:478`). This keeps both creation surfaces behaviorally identical. +- **KTD-5 — `defaultOn` toggle is a per-declaration boolean stored on the IR**, distinct from the + template's own `defaultOn`. The resolver already prefers the workflow declaration's `defaultOn` over the + template default (`optionalStep.defaultOn ?? template.defaultOn ?? false`); the authoring UI writes the + declaration-level value. + +--- + +## Implementation Units + +### U1. Add the `workflow-step` seam node + optional-step declaration to the stepwise-coding workflow + +**Goal:** Make `browser-verification` a real, runnable optional step on the stepwise-coding workflow — +both declared and actually executed when enabled (R1). This is more than a one-line declaration: the +stepwise graph currently has no `workflow-step` seam node, so without one the toggle would be dead. + +**Requirements:** R1, R6 + +**Dependencies:** none + +**Files:** +- `packages/core/src/builtin-stepwise-coding-workflow-ir.ts` (modify — add `workflow-step` seam node + + rewire edges + `optionalSteps` declaration) +- `packages/core/src/__tests__/workflow-optional-steps.test.ts` (modify — add stepwise resolver case) +- `packages/engine/src/__tests__/stepwise-workflow-parity.test.ts` or a sibling engine test (modify/create + — **execution-level** test that an enabled step actually runs) +- Any stepwise IR snapshot/parity oracle fixture (update if a byte-identity snapshot exists — see R-5) + +**Approach:** +- Add a `workflow-step` seam node to the stepwise IR mirroring `builtin-coding-workflow-ir.ts:69` + (`{ id: "workflow-step", kind: "prompt", column: ..., config: builtinPromptConfig("workflow-step", + "Pre-merge workflow steps") }`), and rewire the success path so it sits between the foreach `steps` + region and `review` (i.e. `steps → workflow-step → review`, plus the `workflow-step → end` + outcome/failure edges that the coding IR carries at `:99-116`). Confirm placement is **after** the + foreach completes (the seam reads task-level `enabledWorkflowSteps` once, not per step-instance — see + R-5 note). +- Add `optionalSteps: [{ templateId: "browser-verification" }]` to the stepwise IR (mirrors + `builtin-coding-workflow-ir.ts:123`). No explicit `defaultOn` → resolves OFF. +- Confirm the IR still passes `parseWorkflowIr`. + +**Patterns to follow:** `builtin-coding-workflow-ir.ts` `workflow-step` node (`:69`) and its edges +(`:97-116`); `optionalSteps` declaration (`:123`). + +**Test scenarios:** +- `resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)` returns a single + `browser-verification` entry (`name: "Browser Verification"`, `phase: "pre-merge"`, `defaultOn: false`). +- The stepwise IR parses without error and contains exactly one `workflow-step` seam node on the success + path between `steps` and `review`. +- **Execution divergence (the critical test):** a stepwise-coding task with + `enabledWorkflowSteps: ["browser-verification"]` actually invokes `runWorkflowSteps` and records a + `workflowStepResults` entry for `browser-verification`; a sibling task with it OFF records none. This is + the two-task divergence shape from the per-task-auto-merge learning — it guards against the dead-toggle + failure that a resolver-only test would miss. +- Foreach interaction: the `workflow-step` seam runs **once** post-foreach, not per step-instance (R-5). + +**Verification:** `pnpm --filter @fusion/core test workflow-optional-steps` plus the engine execution +test pass; the byte-identity parity oracle still passes (or its fixture is updated, R-5). + +--- + +### U2. Preserve `optionalSteps` across the node-editor round-trip (`flowToIr`) + +**Goal:** Fix the data-loss bug so saving a workflow through the node editor never drops its optional-step +declaration (R2, R6). This is the correctness prerequisite for U3. + +**Requirements:** R2, R6 + +**Dependencies:** none (independent of U1; pairs with U3) + +**Files:** +- `packages/dashboard/app/components/workflow-flow-mapping.ts` (modify — `flowToIr` signature + v2 + detection + re-attach; add `optionalStepsOf` reader) +- `packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts` (modify/create — round-trip + test) + +**Approach:** +- Extend `flowToIr(name, nodes, edges, columns?, fields?, settings?, optionalSteps?)` with a trailing + optional `optionalSteps?: WorkflowOptionalStep[]` parameter (keep it trailing so existing call sites + compile). +- Include a non-empty `optionalSteps` in the v2 signal alongside `hasFields`/`hasSettings` + (`workflow-flow-mapping.ts:454-456`), and re-attach it to the constructed v2 IR exactly where `fields` + and `settings` are attached (`:544-561`). Omit entirely when empty/absent (R6). +- Add `optionalStepsOf(def: WorkflowDefinition): WorkflowOptionalStep[]` mirroring `fieldsOf`/`settingsOf` + (`:946`, `:960`) — returns a deep-ish copy for v2 IRs with `optionalSteps`, `[]` otherwise. Used by U3 to + hydrate editor state. +- **Also thread `optionalSteps` through `serializeGraph`** (`WorkflowNodeEditor.tsx:160-177`), the + indirection that `flowToIr` is called through for dirty-tracking. `serializeGraph` is invoked by the + dirty-check effect (`:1012`) and the load-baseline calls (`:1187`, `:1818`). If only the save path is + threaded but `serializeGraph` is not, the loaded baseline and live snapshots both omit `optionalSteps`, + so **editing optional steps never marks the editor dirty and the Save button never enables** — the user + cannot persist the change. Give `serializeGraph` an `optionalSteps` parameter and pass the editor state + at the dirty-check call and `optionalStepsOf(activeWorkflow)` at the baseline calls. (This belongs to + U3's wiring but is called out here because it rides the same `flowToIr` seam.) + +**Patterns to follow:** the `fields`/`settings` threading and `fieldsOf`/`settingsOf` readers in the same +file are the exact template to clone. + +**Technical design (directional, not spec):** +```text +flowToIr(..., optionalSteps?) { + const hasOptional = Array.isArray(optionalSteps) && optionalSteps.length > 0 + const v2 = columns?.length || hasFields || hasSettings || hasOptional + ... + if (v2) { + if (hasFields) ir.fields = ... + if (hasSettings) ir.settings = ... + if (hasOptional) ir.optionalSteps = optionalSteps.map(o => ({ ...o })) // NEW + } +} +``` + +**Test scenarios:** +- Round-trip: an IR with `optionalSteps: [{ templateId: "browser-verification", defaultOn: true }]` → + `irToFlow` → `flowToIr(..., optionalStepsOf(def))` yields an IR whose `optionalSteps` equals the input. +- A workflow with optional steps but no custom columns/fields/settings serializes as **v2** (not v1). +- A workflow with no optional steps serializes **without** an `optionalSteps` key (no `[]`, no `null`) — + legacy byte-identity preserved (R6). +- `optionalStepsOf` returns `[]` for a v1 IR and for a v2 IR with no `optionalSteps`. +- `optionalStepsOf` returns a copy (mutating the result does not mutate the source IR). + +**Verification:** round-trip test green; existing `workflow-flow-mapping` tests unaffected. + +--- + +### U3. Optional Steps authoring panel in the node editor + +**Goal:** Let a workflow author add/remove optional-step declarations and set each one's `defaultOn` from +the visual editor, and persist them via the U2 round-trip (R3, R5). + +**Requirements:** R3, R5, R6 + +**Dependencies:** U2 (needs `flowToIr` to accept `optionalSteps` + `optionalStepsOf` to hydrate) + +**Files:** +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` (modify — add `optionalSteps` state, + hydrate via `optionalStepsOf` on **both** load paths, render the panel inline alongside Fields/Settings, + thread into every `flowToIr`/`serializeGraph` call site) +- `packages/dashboard/app/components/workflow-phase-badge.tsx` (create — extract `phaseBadge`; see below) +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (modify — round-trip + dirty + + add/remove/toggle) + +**Approach:** +- Add `optionalSteps` editor state to `WorkflowNodeEditor`. **Hydrate via `optionalStepsOf(activeWorkflow)` + on both load paths:** the primary load effect AND the fragment/AI-generate load path at `:1496-1497`. + Note: that fragment path today calls only `columnsOf`/`fieldsOf` — it omits even `settingsOf` — so + `optionalSteps` (and settings) must be added explicitly there, not copied from the existing two-call + pattern. Missing this re-introduces the U2 data loss on the fragment path (load empty → save strips). +- **Render the panel inline** in `WorkflowNodeEditor` as a third sibling alongside the existing Fields and + Settings panels (same collapsible/expandable behavior and sidebar position, immediately below Settings). + Do not extract a separate single-consumer `WorkflowOptionalStepsEditor` component/CSS file — co-locate + the panel JSX with the Fields/Settings panels it mirrors. The panel is a list of declared optional steps, + each showing the template's resolved name + phase chip + an on/off `defaultOn` control, an "Add optional + step" picker sourced from `WORKFLOW_STEP_TEMPLATES` (filtering out already-declared ids), and a remove + control per row. Store only `{ templateId, defaultOn }`; resolve display metadata from the catalog at + render time (KTD-3). +- Thread the editor's `optionalSteps` into **every** `flowToIr(...)` AND `serializeGraph(...)` call site + (save `:1802`/`:1818`, dirty-check `:1012`, baselines `:1187`, fragment/preview) so no save or + dirty-tracking route bypasses the param (see U2 / R-1). +- **Unknown/stale template ids:** render a muted "Unknown step (`templateId`)" row with a remove control — + do **not** silently skip them. Skipping would hide a declaration the user then can't remove without + hand-editing IR; the muted row satisfies R5's "remain removable." +- **Accessibility:** the `defaultOn` control is a labeled checkbox/switch with + `aria-label="Default on for "` and a visible focus ring; the "Add optional step" picker + inherits keyboard behavior from the editor's existing picker pattern. + +**phaseBadge extraction (F3):** `phaseBadge` is currently a non-exported module-local in +`WorkflowResultsTab.tsx:137` (takes a `t` arg). Extract it into a shared `workflow-phase-badge.tsx` helper +and re-import it in `WorkflowResultsTab`, the node-editor panel, and the U5 dropdown — rather than +duplicating the chip three times. + +**Patterns to follow:** the existing Fields and Settings editor panels in the node editor (inline, not +separate components) and `TaskFieldsSection.tsx` for list-of-typed-declarations UI. + +**Test scenarios:** +- Adding an optional step from the picker appends `{ templateId, defaultOn: false }`, removes it from the + picker's available list, **and marks the editor dirty** (Save enables — guards the `serializeGraph` gap). +- Toggling a row's `defaultOn` flips the stored boolean and marks dirty. +- Removing a row deletes the declaration and returns the template to the picker. +- Editing a workflow that already declares `browser-verification`, making no change, and saving yields an + IR whose `optionalSteps` is unchanged (round-trip through the live editor, not just `flowToIr`). +- A workflow loaded via the **fragment/generate path** preserves its `optionalSteps` on save (guards F2). +- A loaded declaration with an unknown `templateId` renders a muted removable row without crashing (R5). +- A workflow with zero optional steps saves without an `optionalSteps` key (R6). + +**Verification:** new component tests green; node-editor save round-trip preserves declarations; manual +real-browser check per the worktree dashboard recipe (see Risks). + +--- + +### U4. Optional-step toggles in the full New Task modal + +**Goal:** Bring `NewTaskModal` / `TaskForm` to parity with the inline quick-create card's optional-step +toggles (R4, R5). + +**Requirements:** R4, R5 + +**Dependencies:** **U5** — U4 consumes the shared `WorkflowOptionalStepsDropdown` built in U5 (see the +ordering decision below). U1 only enriches which workflows expose steps. + +**Ordering decision (resolves the U4/U5 contradiction):** U5 builds **one** shared +`WorkflowOptionalStepsDropdown` component and lands first; U4 consumes it. U4 does **not** ship inline chip +markup that would later need migration. Both creation surfaces (quick-add card, full modal) therefore +present optional steps with the same dropdown interaction from day one — no divergent pickers. + +**Files:** +- `packages/dashboard/app/components/TaskForm.tsx` (modify — load optional steps for selected workflow, + render the shared dropdown, expose enabled-set state) +- `packages/dashboard/app/components/NewTaskModal.tsx` (modify — own the enabled-set state, include + `enabledWorkflowSteps` in the create payload at `:243`) +- `packages/dashboard/app/components/__tests__/NewTaskModal.test.tsx` (modify/create) +- `packages/dashboard/app/components/__tests__/TaskForm.test.tsx` (modify/create) + +**Approach:** +- In `TaskForm`, when a workflow is selected (dropdown gated by `onWorkflowIdChange`, `:1290`), call + `fetchWorkflowOptionalSteps(effectiveWorkflowId, projectId)` and render the **`WorkflowOptionalStepsDropdown` + (from U5)** directly below the workflow dropdown. +- Seed the enabled set from steps where `defaultOn` is true; re-seed whenever the selected workflow changes + (clear + refetch), matching `InlineCreateCard.tsx:272-284`. +- Lift the enabled-set state to `NewTaskModal` (or expose via a `TaskForm` callback prop, consistent with + the existing `onWorkflowIdChange` pattern) so the create handler includes + `enabledWorkflowSteps: enabled.length ? enabled : undefined` in the `onCreateTask` payload (`:243`). +- **Loading/empty states:** while the optional-steps fetch is in flight, render a loading affordance + consistent with whatever `InlineCreateCard` shows (a skeleton/disabled trigger, not a layout jump); + submit stays enabled. "No workflow", undefined selection, or a workflow with no optional steps → render + nothing (render-nothing is the committed empty-state behavior shared with U5), no `enabledWorkflowSteps` + sent. Fetch failure → usable form, no dropdown (defensive, R5), mirroring the inline card's `.catch`. + +**Patterns to follow:** `InlineCreateCard.tsx` for the state/fetch/seed/submit wiring; +`WorkflowOptionalStepsDropdown` (U5) for the presentation. Keep `data-testid` conventions. + +**Test scenarios:** +- Selecting a workflow with an optional `browser-verification` step renders the dropdown with the step + OFF by default (`defaultOn` false). +- A workflow whose optional step has `defaultOn: true` shows the step pre-selected and includes it in the + create payload if left on. +- Selecting a step and submitting sends `enabledWorkflowSteps: ["browser-verification"]` to `onCreateTask`. +- Changing the workflow selection clears and refetches (no stale selection from the prior workflow). +- While the fetch is pending, a loading affordance shows and submit remains enabled. +- "No workflow" or a workflow with no optional steps renders no dropdown and omits `enabledWorkflowSteps`. +- Fetch failure leaves the form usable with no dropdown (R5). + +**Verification:** modal/form tests green; manual real-browser create with browser-verification selected +produces a task whose detail Workflow tab shows the step enabled. Verify in a real mobile viewport (R-3). + +--- + +### U5. Steps dropdown in the quick-add area + +**Goal:** Build the shared multi-select **steps dropdown** and adopt it in the quick-add card, replacing +the inline chip toggles (R7). This component is the single optional-step picker consumed by both the +quick-add card (here) and the full modal (U4). + +**Requirements:** R7, R5 + +**Dependencies:** none. **Lands before U4** (U4 consumes this component). Build the shared component with +both consumers in mind from the start — there is no interim chip version. + +**Files:** +- `packages/dashboard/app/components/WorkflowOptionalStepsDropdown.tsx` (create — reusable multi-select + dropdown over `ResolvedWorkflowOptionalStep[]`) +- `packages/dashboard/app/components/WorkflowOptionalStepsDropdown.css` (create) +- `packages/dashboard/app/components/InlineCreateCard.tsx` (modify — swap the chip-toggle block at + `:1080-1101` for the dropdown; keep existing state/seeding/submit wiring) +- `packages/dashboard/app/components/InlineCreateCard.css` (modify — remove now-unused chip styles) +- `packages/dashboard/app/components/__tests__/WorkflowOptionalStepsDropdown.test.tsx` (create) +- `packages/dashboard/app/components/__tests__/InlineCreateCard.test.tsx` (modify — assert dropdown + behavior replaces chip behavior) + +**Approach:** +- Build `WorkflowOptionalStepsDropdown` as a controlled multi-select: props are the resolved optional + steps, the enabled `templateId` set, and an `onToggle(templateId)` callback. The open panel lists each + step with a checkbox, name, phase chip (shared `phaseBadge` from U3's extraction), and description. It + owns only open/close UI state — selection stays lifted in the parent (`InlineCreateCard`'s existing + `enabledOptionalStepIds`). +- **Trigger label matrix (committed):** no steps available → render nothing; steps available, zero + selected → `"Steps: none"`; N selected → `"Steps: N selected"`. Use the same strings in both consumers. +- **Empty state (committed):** render **nothing** when the selected workflow has no optional steps + (matches `InlineCreateCard`'s current no-chip-block behavior and U4's empty-state choice — both surfaces + identical). No disabled-placeholder variant. +- **Accessibility / keyboard (committed):** trigger is a button with `aria-haspopup` + `aria-expanded`; + the panel is `role="listbox"` labeled via `aria-labelledby` pointing at the trigger; each option is + `role="option"` with `aria-checked`. Escape closes and returns focus to the trigger; Enter/Space on the + trigger toggles open; arrow keys move focus between options; outside-click closes. Inherit this behavior + from `CustomModelDropdown.tsx` rather than re-implementing. +- **Positioning:** render the panel via a portal (appended to `document.body`) so it is not clipped by the + modal's `overflow` boundary when reused in U4. Verify it anchors to the trigger in both the card and the + modal. +- Swap the inline chip block in `InlineCreateCard` (`:1080-1101`) for the dropdown, passing the existing + `optionalSteps`, `enabledOptionalStepIds`, and `toggleOptionalStep` (`:292`). Leave the fetch/seed/submit + wiring (`:272-284`, `:478`) untouched — only the presentation changes. Unknown/stale ids are already + filtered by the resolver (R5). + +**Patterns to follow:** `CustomModelDropdown.tsx` for trigger+panel, portal positioning, keyboard nav, and +outside-click-close; the shared `phaseBadge` (U3) for the phase chip; keep `InlineCreateCard`'s existing +`data-testid` conventions so current tests migrate cleanly. + +**Test scenarios:** +- Trigger label reflects state: `"Steps: none"` with steps available and none selected; `"Steps: 1 + selected"` with one selected; renders nothing when no optional steps exist. +- Opening the panel and checking a step calls `onToggle` and adds it; unchecking removes it. +- A step with `defaultOn: true` shows pre-checked on first open (seeded by the parent). +- Submitting the quick-add card with a step selected sends `enabledWorkflowSteps: ["browser-verification"]` + (existing submit path unchanged). +- Keyboard: Escape closes and refocuses the trigger; arrow keys move option focus; outside-click closes + without losing selection. +- The panel is not clipped when rendered inside an `overflow:hidden` container (portal positioning). + +**Verification:** dropdown + inline-card tests green; manual real-browser check that the quick-add dropdown +selects/deselects, keyboard nav works, and the created task reflects the chosen steps. Verify the open +panel in a real mobile viewport (R-3). + +--- + +## Scope Boundaries + +**In scope:** +- Stepwise-coding built-in optional-step declaration (U1). +- Node-editor round-trip preservation + authoring panel (U2, U3). +- Full New Task modal optional-step parity (U4). +- Quick-add steps dropdown replacing inline chips (U5). + +**Already built (verify-only, no changes):** +- IR types + parse validation (`workflow-ir-types.ts`, `workflow-ir.ts`). +- `resolveWorkflowOptionalSteps` (`workflow-optional-steps.ts`) and its route/API client. +- Inline quick-create card fetch/seed/submit wiring (`InlineCreateCard.tsx`) — presentation changes in U5, + but the optional-steps load, `defaultOn` seeding, and submit payload are reused as-is. +- Task-detail Workflow tab edit toggles (`WorkflowResultsTab.tsx`). +- Executor `enabledWorkflowSteps` execution path — **but only for workflows that contain a `workflow-step` + seam node** (coding). The stepwise workflow lacks one; U1 adds it. Do not treat the executor as fully + "done" for stepwise until the U1 execution test passes. +- POST/PATCH `/tasks` acceptance of `enabledWorkflowSteps`. + +### Deferred to Follow-Up Work +- Generalizing optional-step authoring to **plugin-contributed** step templates in the picker (use the + built-in `WORKFLOW_STEP_TEMPLATES` catalog for now; plugin template merging can follow once the picker + needs it). +- A reusable "per-task workflow facet override" abstraction unifying optional steps, auto-merge, and + column-agent overrides — strong `/ce-compound` candidate after this lands, not part of this change. + +**Out of scope:** +- Any executor/runtime behavior change. Optional steps remain execution-inert; the runtime already honors + `enabledWorkflowSteps`. +- New persistence/migrations. Optional steps ride the existing IR (`optionalSteps`) and the existing + `tasks.enabledWorkflowSteps` column — no schema bump. + +--- + +## Risks & Dependencies + +- **R-1 — Missed `flowToIr` / `serializeGraph` call site (data loss OR dead Save button).** `flowToIr` is + reached both directly (save `:1802`/`:1818`) and through `serializeGraph` (`:160-177`), which the + dirty-check effect (`:1012`) and load baselines (`:1187`) call. A missed **save** site drops the + declaration; a missed **serializeGraph** site means optional-step edits never mark the editor dirty, so + Save never enables and the change can't be persisted. *Mitigation:* grep every `flowToIr(` AND + `serializeGraph(` call in `WorkflowNodeEditor` and thread the param at each (save, dirty-check, + baselines, fragment/preview); the U3 dirty + round-trip-through-editor tests guard both failure modes. +- **R-2 — Stale dashboard bundle masks UI changes.** `fn dashboard` serves `packages/cli/dist/client`, + not the dashboard worktree dist, so new editor/modal UI can appear "missing" when it's a stale bundle. + *Mitigation:* verify with the worktree recipe — build all four packages, point `FUSION_CLIENT_DIR` at + the dashboard worktree dist, run on a non-4040 port with `--dev` (never `fn daemon`/`serve`). Respect + the port-4040 kill guards. + (`docs/solutions/developer-experience/browser-testing-dashboard-from-worktree-safely.md`) +- **R-3 — Mobile toggle layout regression.** Per-task toggle switches on the board have a history of + real-browser-only mobile failures (document horizontal scroll → blank dashboard) invisible to jsdom. + *Mitigation:* the new toggles live in the create modal/inline card and the node editor, not on board + cards, but verify the modal toggles in a real mobile viewport. + (`docs/solutions/ui-bugs/mobile-auto-merge-toggle-document-scroll-blank.md`) +- **R-4 — v2-detection / byte-identity drift.** Incorrectly treating empty `optionalSteps` as a v2 signal + would upgrade legacy v1 workflows and break R6 byte-identity. *Mitigation:* gate the v2 signal on + **non-empty** `optionalSteps` and omit the key entirely when empty; cover with the U2 "no optional + steps serializes without the key" test. +- **R-5 — Stepwise IR is a byte-identity parity oracle; adding the seam node may shift snapshots.** The + stepwise IR is documented as the `execute`-seam byte-identity parity oracle. Adding a `workflow-step` + seam node (U1) changes the graph, so any snapshot/parity fixture asserting the stepwise node/edge set + will need updating, and the foreach interaction must be confirmed (the seam must run **once** after the + foreach completes, not per step-instance — the learnings flag subgraph-walking as the #1 pitfall). + *Mitigation:* place the seam node after the foreach `steps` region on the success path; update the parity + fixture deliberately; add the U1 foreach-interaction test. + +--- + +## Sources & Research + +- Backend/core state map: `WorkflowOptionalStep` + `WorkflowIrV2.optionalSteps` + (`packages/core/src/workflow-ir-types.ts:314-339`), resolver + (`packages/core/src/workflow-optional-steps.ts`), coding declaration + (`packages/core/src/builtin-coding-workflow-ir.ts:123`), executor path + (`packages/engine/src/executor.ts` `executeWorkflowSteps`). +- UI state map: inline card toggles (`packages/dashboard/app/components/InlineCreateCard.tsx:113-1101`), + edit toggles (`packages/dashboard/app/components/WorkflowResultsTab.tsx:332-488`), workflow picker + (`packages/dashboard/app/components/TaskForm.tsx:1290`), modal create payload + (`packages/dashboard/app/components/NewTaskModal.tsx:243`), route + (`packages/dashboard/src/routes/register-workflow-routes.ts:317-329`), serialization gap + (`packages/dashboard/app/components/workflow-flow-mapping.ts:435-562`, `:946-963`). +- Institutional learnings: per-entity override blast-radius and per-task auto-merge override precedent + (`docs/solutions/architecture-patterns/per-entity-execution-principal-override-blast-radius.md`, + `docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md`) — informed the + decision to keep optional steps execution-inert rather than adding executor branches; worktree browser + testing and mobile toggle gotchas as above. diff --git a/docs/solutions/build-errors/eslint-exhaustive-deps-rule-not-registered-fails-ci-lint.md b/docs/solutions/build-errors/eslint-exhaustive-deps-rule-not-registered-fails-ci-lint.md new file mode 100644 index 0000000000..685969bd6d --- /dev/null +++ b/docs/solutions/build-errors/eslint-exhaustive-deps-rule-not-registered-fails-ci-lint.md @@ -0,0 +1,59 @@ +--- +title: eslint-disable for react-hooks/exhaustive-deps fails CI lint (rule not registered) +date: 2026-06-21 +category: docs/solutions/build-errors +module: dashboard +problem_type: build_error +component: tooling +symptoms: + - "CI Lint job fails: Definition for rule 'react-hooks/exhaustive-deps' was not found react-hooks/exhaustive-deps" + - "Local `pnpm test` (vitest) passes green while the PR's Lint check goes red" +root_cause: config_error +resolution_type: code_fix +severity: medium +tags: [eslint, react-hooks, exhaustive-deps, ci-lint, lint, flat-config] +--- + +# eslint-disable for react-hooks/exhaustive-deps fails CI lint (rule not registered) + +## Problem +This repo's flat `eslint.config.mjs` does not register the `react-hooks/exhaustive-deps` rule. A `// eslint-disable-next-line react-hooks/exhaustive-deps` directive — the usual way to silence a deliberately-incomplete `useEffect` dependency array — therefore references a rule ESLint doesn't know, and `eslint .` treats the unknown rule name in a disable directive as a hard error. The PR's CI `Lint` job fails. + +## Symptoms +- CI `Lint` job (`pnpm lint` → `eslint .`) fails with: `Definition for rule 'react-hooks/exhaustive-deps' was not found react-hooks/exhaustive-deps`. +- The failure does **not** reproduce under `pnpm test` / vitest — those never invoke ESLint, so the whole feature's test suite is green locally while the Lint check is red on the PR. +- Often the only file flagged is the one that added the directive (e.g. a single `useEffect` with an intentionally trimmed dep array). + +## What Didn't Work +- Assuming a green local `pnpm test` meant the branch was CI-clean. Vitest runs through `tsx` and does not lint, so an eslint-only failure is invisible until CI (or an explicit `pnpm lint`) runs. +- Treating it as a missing-dependency warning to satisfy. The error is not exhaustive-deps complaining about the dep array — it's ESLint rejecting the *disable directive* because the named rule isn't registered. Adding the "missing" dep would not silence it; only the directive itself is the problem. + +## Solution +Remove the `eslint-disable` directive. Because `react-hooks/exhaustive-deps` is not enforced in this config, the intentionally-incomplete dep array needs no suppression at all. Keep a plain explanatory comment for the human reader. + +```tsx +// Before — fails CI lint: + // onEnabledWorkflowStepsChange intentionally omitted: a new identity each render + // must not re-trigger the fetch/re-seed (would clobber user toggles). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [onWorkflowIdChange, effectiveOptionalWorkflowId, projectId]); + +// After — passes: + // onEnabledWorkflowStepsChange intentionally omitted from deps: a new identity + // each render must not re-trigger the fetch/re-seed (would clobber user toggles). + // Callers must pass a stable callback (NewTaskModal passes a useState setter). + }, [onWorkflowIdChange, effectiveOptionalWorkflowId, projectId]); +``` + +## Why This Works +ESLint validates rule names referenced in inline disable directives against the rules actually registered by the active config. The flat `eslint.config.mjs` here does not load the `react-hooks` plugin's `exhaustive-deps` rule, so the name is unknown and the directive errors out. Removing the directive removes the dangling reference. There is no behavioral regression: with the rule unregistered, the incomplete dep array was never going to be flagged in the first place — the suppression was protecting against a check that doesn't run. + +## Prevention +- **Don't add `eslint-disable` for `react-hooks/exhaustive-deps` in this repo.** When deliberately omitting a dependency, document the intent in a plain comment and rely on the dep array as written. (If exhaustive-deps enforcement is ever wanted repo-wide, register the rule in `eslint.config.mjs` first — then the directive becomes valid.) +- **Run `pnpm lint` (or `npx eslint `) before pushing.** A passing `pnpm test` does not cover lint — vitest and ESLint are independent gates, and the CI `Lint` job (`.github/workflows/pr-checks.yml`) is the first place an eslint-only error surfaces otherwise. +- Before suppressing any rule with an inline directive, confirm the rule is actually registered in the active flat config; an unknown rule name in a disable directive is itself a lint error under `eslint .`. + +## Related Issues +- Surfaced and fixed during the workflow-optional-steps work (PR #1703), commit `e16d48910` "fix(ci): drop unknown react-hooks/exhaustive-deps disable directive in TaskForm". +- Related to the broader "lint/typecheck gates are separate from vitest" gotcha: TS editor diagnostics and vitest both miss eslint-only failures in `packages/dashboard`. +- `docs/solutions/architecture-patterns/thin-trusted-merge-gate.md` — establishes `Lint` as one of the four merge-blocking CI checks (Lint, Typecheck, Build, Gate). That CI topology is why this failure surfaces only on the PR: `pnpm test`/vitest is not one of those gates and never invokes ESLint. diff --git a/packages/core/src/__tests__/workflow-optional-steps.test.ts b/packages/core/src/__tests__/workflow-optional-steps.test.ts index 93bb6661bf..d3ea6ba91e 100644 --- a/packages/core/src/__tests__/workflow-optional-steps.test.ts +++ b/packages/core/src/__tests__/workflow-optional-steps.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "../builtin-stepwise-coding-workflow-ir.js"; import { resolveWorkflowOptionalSteps } from "../workflow-optional-steps.js"; import type { WorkflowIr, WorkflowIrV2 } from "../workflow-ir-types.js"; @@ -41,6 +42,35 @@ describe("resolveWorkflowOptionalSteps", () => { ]); }); + it("resolves the builtin stepwise-coding browser verification optional step", () => { + expect(resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual([ + { + templateId: "browser-verification", + name: "Browser Verification", + description: "Verify web application functionality using browser automation", + icon: "globe", + phase: "pre-merge", + defaultOn: false, + }, + ]); + }); + + it("places a single workflow-step seam node between steps and review in stepwise", () => { + const ir = BUILTIN_STEPWISE_CODING_WORKFLOW_IR; + if (ir.version !== "v2") throw new Error("expected v2"); + const seamNodes = ir.nodes.filter( + (n) => n.kind === "prompt" && n.config?.seam === "workflow-step", + ); + expect(seamNodes).toHaveLength(1); + // success path: steps -> workflow-step -> review + expect(ir.edges).toEqual( + expect.arrayContaining([ + expect.objectContaining({ from: "steps", to: "workflow-step", condition: "success" }), + expect.objectContaining({ from: "workflow-step", to: "review", condition: "success" }), + ]), + ); + }); + it("skips unknown template ids", () => { expect( resolveWorkflowOptionalSteps(v2([ diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 21e105870b..348125fd5a 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -123,6 +123,16 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { }, // KTD-5: rework exhaustion escalates to a manual hold (a human releases it). { id: "rework-hold", kind: "hold", column: "in-progress", config: { release: "manual" } }, + // FNXC:WorkflowOptionalSteps 2026-06-21-00:00: + // The stepwise workflow must actually run a task's enabled optional steps (e.g. + // browser verification), so it needs the same pre-merge workflow-step seam the + // coding workflow has — declaring the optional step without this node would be a + // dead toggle. Pre-merge workflow-step seam (parity with builtin-coding-workflow-ir): + // the ONLY node that makes the graph invoke `runWorkflowSteps`, so a per-task + // `enabledWorkflowSteps` (e.g. the optional browser-verification step declared + // below) actually executes. Runs ONCE after the foreach completes, between + // implementation and review — not per step-instance. + { id: "workflow-step", kind: "prompt", column: "in-progress", config: builtinPromptConfig("workflow-step", "Pre-merge workflow steps") }, { id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") }, { id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } }, { id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } }, @@ -153,10 +163,17 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "parse", to: "steps", condition: "outcome:no-steps" }, { from: "parse", to: "end", condition: "failure" }, { from: "parse", to: "end", condition: "outcome:parse-error" }, - { from: "steps", to: "review", condition: "success" }, - // KTD-5: bounded rework exhaustion → manual hold; release re-enters review. + // Implementation complete → pre-merge workflow-step seam → review. Both the + // normal foreach-success path and the rework-exhausted manual-release path flow + // through the seam so enabled workflow steps run regardless of route. + { from: "steps", to: "workflow-step", condition: "success" }, + // KTD-5: bounded rework exhaustion → manual hold; release re-enters the seam. { from: "steps", to: "rework-hold", condition: "outcome:rework-exhausted" }, - { from: "rework-hold", to: "review", condition: "success" }, + { from: "rework-hold", to: "workflow-step", condition: "success" }, + { from: "workflow-step", to: "review", condition: "success" }, + { from: "workflow-step", to: "end", condition: "outcome:remediation-scheduled" }, + { from: "workflow-step", to: "end", condition: "outcome:deferred-paused" }, + { from: "workflow-step", to: "end", condition: "failure" }, { from: "steps", to: "end", condition: "failure" }, { from: "review", to: "merge-gate", condition: "success" }, { from: "review", to: "end", condition: "failure" }, @@ -176,6 +193,9 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { ], // Workflow-settings (U1, R4): same moved-key catalog as the default builtin. settings: BUILTIN_WORKFLOW_SETTINGS, + // Optional browser-verification step, parity with builtin-coding-workflow-ir. + // Default OFF; runnable because the workflow-step seam node above is present. + optionalSteps: [{ templateId: "browser-verification" }], }; export const BUILTIN_STEPWISE_CODING_WORKFLOW_IR = parseWorkflowIr( diff --git a/packages/dashboard/app/components/InlineCreateCard.css b/packages/dashboard/app/components/InlineCreateCard.css index 6d361c59f0..c739f8aa67 100644 --- a/packages/dashboard/app/components/InlineCreateCard.css +++ b/packages/dashboard/app/components/InlineCreateCard.css @@ -160,11 +160,6 @@ gap: var(--space-xs); } -.inline-create-optional-step[aria-pressed="true"] { - border-color: var(--accent); - color: var(--accent); -} - .inline-create-hint { font-size: 11px; color: var(--text-dim); @@ -318,10 +313,6 @@ width: 100%; } - .inline-create-optional-step { - flex: 1 1 auto; - } - .inline-create-priority-select { min-height: 36px; } diff --git a/packages/dashboard/app/components/InlineCreateCard.tsx b/packages/dashboard/app/components/InlineCreateCard.tsx index bc9b1cf1c4..0dd6e64951 100644 --- a/packages/dashboard/app/components/InlineCreateCard.tsx +++ b/packages/dashboard/app/components/InlineCreateCard.tsx @@ -16,6 +16,7 @@ import { LoadingSpinner } from "./LoadingSpinner"; import { applyPresetToSelection } from "../utils/modelPresets"; import { getScopedItem, removeScopedItem, setScopedItem } from "../utils/projectStorage"; import { WorkflowSelector } from "./WorkflowSelector"; +import { WorkflowOptionalStepsDropdown } from "./WorkflowOptionalStepsDropdown"; const ALLOWED_IMAGE_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"]; const STORAGE_KEY = "kb-inline-create-text"; @@ -1083,27 +1084,16 @@ export function InlineCreateCard({ className="inline-create-optional-steps" aria-label={t("inline.optionalWorkflowSteps", "Optional workflow steps")} > - {optionalSteps.map((step) => { - const enabled = enabledOptionalStepIds.includes(step.templateId); - const testId = step.templateId === "browser-verification" - ? "inline-create-browser-verification-toggle" - : `inline-create-optional-step-${step.templateId}`; - return ( - - ); - })} + {/* FNXC:TaskCreation 2026-06-21-00:00: + Inline quick-add uses the shared optional-steps dropdown so it matches + the modal/quick-add keyboard + a11y behavior and toggles the same + enabledWorkflowSteps set submitted on create. */} + )} diff --git a/packages/dashboard/app/components/NewTaskModal.tsx b/packages/dashboard/app/components/NewTaskModal.tsx index 2961e020df..4d245f1693 100644 --- a/packages/dashboard/app/components/NewTaskModal.tsx +++ b/packages/dashboard/app/components/NewTaskModal.tsx @@ -62,6 +62,9 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, // `null` = explicit "No workflow", `string` = a specific workflow. Materialized // atomically at create time via the `workflowId` create parameter. const [selectedWorkflowId, setSelectedWorkflowId] = useState(undefined); + // Optional workflow steps the user opted into; TaskForm fetches + seeds these + // from the selected workflow's defaultOn and lifts the enabled set up here. + const [enabledWorkflowSteps, setEnabledWorkflowSteps] = useState([]); const [reviewLevel, setReviewLevel] = useState(undefined); const [autoMerge, setAutoMerge] = useState(undefined); const [priority, setPriority] = useState(DEFAULT_TASK_PRIORITY); @@ -162,6 +165,10 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, dependencies.length > 0 || pendingImages.length > 0 || selectedWorkflowId !== undefined || + // Optional workflow steps the user toggled count as unsaved work. (Workflows + // whose steps are defaultOn:false — today's only shipped step — seed an empty + // set, so this stays false until the user actually opts a step in.) + enabledWorkflowSteps.length > 0 || executorModel !== "" || validatorModel !== "" || planningModel !== "" || @@ -177,7 +184,7 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, githubTrackingEnabled || githubRepoOverrideTrimmed !== ""; setHasDirtyState(isDirty); - }, [description, dependencies, pendingImages, selectedWorkflowId, executorModel, validatorModel, planningModel, thinkingLevel, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed]); + }, [description, dependencies, pendingImages, selectedWorkflowId, enabledWorkflowSteps, executorModel, validatorModel, planningModel, thinkingLevel, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed]); const handleClose = useCallback(async () => { if (hasDirtyState) { @@ -201,6 +208,7 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, setSelectedPresetId(""); setPresetMode("default"); setSelectedWorkflowId(undefined); + setEnabledWorkflowSteps([]); setSelectedAgentId(null); setShowAgentPicker(false); setReviewLevel(undefined); @@ -242,6 +250,9 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, // - null → explicit "No workflow" (store skips default materialization) // - string → that workflow, materialized atomically at create time. ...(selectedWorkflowId !== undefined ? { workflowId: selectedWorkflowId } : {}), + // Optional steps the user toggled on (omit when none so the store keeps its + // default materialization behavior). + ...(enabledWorkflowSteps.length ? { enabledWorkflowSteps } : {}), ...(selectedAgentId ? { assignedAgentId: selectedAgentId } : {}), modelPresetId: presetMode === "preset" ? selectedPresetId || undefined : undefined, modelProvider: executorModel && executorSlashIdx !== -1 ? executorModel.slice(0, executorSlashIdx) : undefined, @@ -302,6 +313,7 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, setSelectedPresetId(""); setPresetMode("default"); setSelectedWorkflowId(undefined); + setEnabledWorkflowSteps([]); setSelectedAgentId(null); setShowAgentPicker(false); setReviewLevel(undefined); @@ -319,7 +331,7 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, } finally { setIsSubmitting(false); } - }, [description, dependencies, pendingImages, executorModel, validatorModel, planningModel, thinkingLevel, isSubmitting, githubRepoOverrideInvalid, hasInvalidBranchSelection, onCreateTask, addToast, onClose, projectId, presetMode, selectedPresetId, selectedWorkflowId, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, isBranchNameRequired, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed, t]); + }, [description, dependencies, pendingImages, executorModel, validatorModel, planningModel, thinkingLevel, isSubmitting, githubRepoOverrideInvalid, hasInvalidBranchSelection, onCreateTask, addToast, onClose, projectId, presetMode, selectedPresetId, selectedWorkflowId, enabledWorkflowSteps, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, isBranchNameRequired, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed, t]); // Handle keyboard shortcuts const handleKeyDown = useCallback((e: React.KeyboardEvent) => { @@ -507,6 +519,8 @@ export function NewTaskModal({ isOpen, onClose, projectId, tasks, onCreateTask, onSelectedPresetIdChange={setSelectedPresetId} selectedWorkflowId={selectedWorkflowId} onWorkflowIdChange={setSelectedWorkflowId} + enabledWorkflowSteps={enabledWorkflowSteps} + onEnabledWorkflowStepsChange={setEnabledWorkflowSteps} pendingImages={pendingImages} onImagesChange={setPendingImages} tasks={tasks} diff --git a/packages/dashboard/app/components/TaskForm.tsx b/packages/dashboard/app/components/TaskForm.tsx index 4112bfca78..370a42cd8f 100644 --- a/packages/dashboard/app/components/TaskForm.tsx +++ b/packages/dashboard/app/components/TaskForm.tsx @@ -1,8 +1,9 @@ import { useState, useCallback, useEffect, useRef } from "react"; import { useTranslation } from "react-i18next"; -import { DEFAULT_TASK_PRIORITY, TASK_PRIORITIES, type GlobalSettings, type Task, type TaskPriority, type Settings, type WorkflowDefinition } from "@fusion/core"; +import { DEFAULT_TASK_PRIORITY, TASK_PRIORITIES, type GlobalSettings, type Task, type TaskPriority, type Settings, type WorkflowDefinition, type ResolvedWorkflowOptionalStep } from "@fusion/core"; import type { ToastType } from "../hooks/useToast"; -import { fetchModels, fetchSettings, fetchWorkflows, refineText, getRefineErrorMessage, updateGlobalSettings, fetchGlobalSettings, fetchGitBranches, type RefinementType, type ModelInfo, type NodeInfo } from "../api"; +import { fetchModels, fetchSettings, fetchWorkflows, fetchWorkflowOptionalSteps, refineText, getRefineErrorMessage, updateGlobalSettings, fetchGlobalSettings, fetchGitBranches, type RefinementType, type ModelInfo, type NodeInfo } from "../api"; +import { WorkflowOptionalStepsDropdown } from "./WorkflowOptionalStepsDropdown"; import { applyPresetToSelection, getRecommendedPresetForSize } from "../utils/modelPresets"; import { CustomModelDropdown } from "./CustomModelDropdown"; import { NodeHealthDot } from "./NodeHealthDot"; @@ -100,6 +101,11 @@ export interface TaskFormProps { // edit-mode workflow management lives in the task detail Workflow tab. selectedWorkflowId?: string | null; onWorkflowIdChange?: (workflowId: string | null) => void; + // Optional workflow steps the task can opt into. TaskForm fetches + seeds these + // from the selected workflow's `defaultOn` and lifts the enabled set to the + // parent (which puts it in the create payload). Only active in create mode. + enabledWorkflowSteps?: string[]; + onEnabledWorkflowStepsChange?: (ids: string[]) => void; // Attachments pendingImages: PendingImage[]; @@ -177,6 +183,8 @@ export function TaskForm({ onSelectedPresetIdChange, selectedWorkflowId, onWorkflowIdChange, + enabledWorkflowSteps, + onEnabledWorkflowStepsChange, pendingImages, onImagesChange, tasks, @@ -236,6 +244,8 @@ export function TaskForm({ // U6/R3: full workflow definitions for the picker (fragments excluded below). const [workflows, setWorkflows] = useState([]); const [workflowsLoading, setWorkflowsLoading] = useState(false); + const [optionalSteps, setOptionalSteps] = useState([]); + const [optionalStepsLoading, setOptionalStepsLoading] = useState(false); const [autoSaveStatus, setAutoSaveStatus] = useState<"idle" | "saving" | "saved">("idle"); const [baseBranchOptions, setBaseBranchOptions] = useState([]); const [baseBranchCustomMode, setBaseBranchCustomMode] = useState(false); @@ -286,6 +296,63 @@ export function TaskForm({ .catch(() => setGlobalSettings(null)); }, [isActive, projectId, onWorkflowIdChange]); + // FNXC:WorkflowOptionalSteps 2026-06-21-00:00: + // Creating a task should let the user opt into the selected workflow's optional + // steps, seeded from each step's defaultOn. TaskForm fetches + seeds these in + // create mode and lifts the enabled set to NewTaskModal for the create payload. + // Optional workflow steps for the currently-selected workflow (create mode only). + // `null` selection ("No workflow") → no steps; `undefined` → project default. + const effectiveOptionalWorkflowId = + selectedWorkflowId === null + ? null + : (selectedWorkflowId ?? settings?.defaultWorkflowId ?? null); + useEffect(() => { + if (!onWorkflowIdChange) return; // edit mode: optional steps are managed in the Workflow tab. + let cancelled = false; + setOptionalSteps([]); + if (!effectiveOptionalWorkflowId) { + // Clear any in-flight loading state (a prior fetch may have been cancelled + // mid-flight when switching to "No workflow"), so the loading row never sticks. + setOptionalStepsLoading(false); + onEnabledWorkflowStepsChange?.([]); + return; + } + setOptionalStepsLoading(true); + fetchWorkflowOptionalSteps(effectiveOptionalWorkflowId, projectId) + .then((steps) => { + if (cancelled) return; + setOptionalSteps(steps); + // Re-seed the enabled set from each step's defaultOn on every workflow change. + onEnabledWorkflowStepsChange?.(steps.filter((s) => s.defaultOn).map((s) => s.templateId)); + }) + .catch(() => { + if (cancelled) return; + setOptionalSteps([]); + onEnabledWorkflowStepsChange?.([]); + }) + .finally(() => { + if (!cancelled) setOptionalStepsLoading(false); + }); + return () => { + cancelled = true; + }; + // onEnabledWorkflowStepsChange intentionally omitted from deps: a new identity + // each render must not re-trigger the fetch/re-seed (would clobber user toggles). + // Callers must pass a stable callback (NewTaskModal passes a useState setter). + }, [onWorkflowIdChange, effectiveOptionalWorkflowId, projectId]); + + const enabledOptionalStepIds = enabledWorkflowSteps ?? []; + const toggleOptionalStep = useCallback( + (templateId: string) => { + const current = enabledWorkflowSteps ?? []; + const next = current.includes(templateId) + ? current.filter((id) => id !== templateId) + : [...current, templateId]; + onEnabledWorkflowStepsChange?.(next); + }, + [enabledWorkflowSteps, onEnabledWorkflowStepsChange], + ); + const availablePresets = settings?.modelPresets || []; const selectedPreset = availablePresets.find((preset) => preset.id === selectedPresetId); const effectiveGithubRepoDefault = resolveEffectiveGithubRepoDefault(settings, globalSettings); @@ -1341,6 +1408,23 @@ export function TaskForm({ {t("taskForm.workflowHelp", "The selected workflow's steps run automatically around this task's execution.")} + {optionalStepsLoading ? ( + + {t("taskForm.optionalStepsLoading", "Loading optional steps…")} + + ) : ( + optionalSteps.length > 0 && ( +
+ +
+ ) + )} )} diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index c2d49126b4..69c5017161 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -17,7 +17,7 @@ import { import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; import { X, Plus, Trash2, Save, MessageSquare, Terminal, Shield, GitMerge, Loader2, HelpCircle, PauseCircle, Split, Merge, Repeat, ClipboardCheck, ListChecks, Code2, Bell, LayoutGrid, Workflow, Download, Upload, ChevronDown, ChevronRight, ChevronLeft, Library, Sparkles, Maximize2, Minimize2 } from "lucide-react"; -import type { WorkflowDefinition, WorkflowIrColumn, TraitViolation, WorkflowStepTemplate } from "@fusion/core"; +import type { WorkflowDefinition, WorkflowIrColumn, TraitViolation, WorkflowStepTemplate, WorkflowOptionalStep } from "@fusion/core"; import { getErrorMessage } from "@fusion/core"; import { fetchWorkflows, @@ -64,6 +64,7 @@ import { columnsOf, fieldsOf, settingsOf, + optionalStepsOf, columnsToBandNodes, reconcileNodeColumns, strictColumnForY, @@ -87,6 +88,7 @@ import { fetchTraits, fetchStepParsers, type TraitCatalogEntry } from "../api"; import { WorkflowColumnPanel } from "./WorkflowColumnPanel"; import { WorkflowFieldsPanel } from "./WorkflowFieldsPanel"; import { WorkflowSettingsPanel } from "./WorkflowSettingsPanel"; +import { WorkflowOptionalStepsPanel } from "./WorkflowOptionalStepsPanel"; import type { WorkflowFieldDefinition, WorkflowSettingDefinition } from "../api"; import { CustomModelDropdown } from "./CustomModelDropdown"; import { MobileWorkflowGraphView } from "./MobileWorkflowGraphView"; @@ -98,7 +100,7 @@ import { } from "./workflow-mobile-graph"; type ExecutorKind = "model" | "agent" | "skill" | "cli" | "cli-agent"; -type MobileWorkflowPanel = "graph" | "add" | "settings" | "fields" | "columns" | "actions"; +type MobileWorkflowPanel = "graph" | "add" | "settings" | "fields" | "optional-steps" | "columns" | "actions"; function builtinSeamPrompt(config: Record | undefined): string { const seam = typeof config?.seam === "string" ? config.seam : ""; @@ -165,6 +167,7 @@ function serializeGraph( columns: WorkflowIrColumn[], fields: WorkflowFieldDefinition[], settings: WorkflowSettingDefinition[], + optionalSteps: WorkflowOptionalStep[], ): string { const { ir, layout } = flowToIr( name, @@ -173,6 +176,7 @@ function serializeGraph( columns.length ? columns : undefined, fields.length ? fields : undefined, settings.length ? settings : undefined, + optionalSteps.length ? optionalSteps : undefined, ); return JSON.stringify({ name, description, ir, layout }); } @@ -740,6 +744,7 @@ function InnerEditor({ // VALUES live per-project in the workflow_settings table (KTD-2) and are // managed by the panel's Values tab, not this declaration array. const [settings, setSettings] = useState([]); + const [optionalSteps, setOptionalSteps] = useState([]); // Ref to the settings panel so a `?panel=settings` deep link can scroll it // into view on mount (U6/U9 redirect stubs). const settingsPanelRef = useRef(null); @@ -778,6 +783,7 @@ function InnerEditor({ const columnsCollapsedStorageKey = "fusion:wf-sidebar-columns-collapsed"; const fieldsCollapsedStorageKey = "fusion:wf-sidebar-fields-collapsed"; const settingsCollapsedStorageKey = "fusion:wf-sidebar-settings-collapsed"; + const optionalStepsCollapsedStorageKey = "fusion:wf-sidebar-optional-steps-collapsed"; const [columnsCollapsed, setColumnsCollapsed] = useState(() => { try { return localStorage.getItem(columnsCollapsedStorageKey) === "1"; @@ -799,6 +805,13 @@ function InnerEditor({ return false; } }); + const [optionalStepsCollapsed, setOptionalStepsCollapsed] = useState(() => { + try { + return localStorage.getItem(optionalStepsCollapsedStorageKey) === "1"; + } catch { + return false; + } + }); useEffect(() => { try { localStorage.setItem(columnsCollapsedStorageKey, columnsCollapsed ? "1" : "0"); @@ -820,6 +833,13 @@ function InnerEditor({ // localStorage unavailable (private mode / SSR): non-fatal. } }, [settingsCollapsed]); + useEffect(() => { + try { + localStorage.setItem(optionalStepsCollapsedStorageKey, optionalStepsCollapsed ? "1" : "0"); + } catch { + // localStorage unavailable (private mode / SSR): non-fatal. + } + }, [optionalStepsCollapsed]); // React Flow instance for programmatic viewport control (auto-layout on load). const { setViewport } = useReactFlow(); // Wrapper around so keyboard deletion can return focus to the @@ -1009,10 +1029,10 @@ function InnerEditor({ if (isBuiltin) return false; if (!activeWorkflow || loadedSnapshotRef.current === null) return false; return ( - serializeGraph(name, description, nodes, edges, columns, fields, settings) !== + serializeGraph(name, description, nodes, edges, columns, fields, settings, optionalSteps) !== loadedSnapshotRef.current ); - }, [isBuiltin, activeWorkflow, name, description, nodes, edges, columns, fields, settings]); + }, [isBuiltin, activeWorkflow, name, description, nodes, edges, columns, fields, settings, optionalSteps]); const loadWorkflows = useCallback(async () => { setLoading(true); @@ -1160,6 +1180,7 @@ function InnerEditor({ setColumns([]); setFields([]); setSettings([]); + setOptionalSteps([]); setName(""); setDescription(""); loadedSnapshotRef.current = null; @@ -1169,6 +1190,7 @@ function InnerEditor({ const loadedColumns = columnsOf(activeWorkflow); const loadedFields = fieldsOf(activeWorkflow); const loadedSettings = settingsOf(activeWorkflow); + const loadedOptionalSteps = optionalStepsOf(activeWorkflow); // Auto-layout on load: compute tidy positions and apply them before the // first render so nodes are visible in the top-left viewport. const layoutPositions = autoLayout(flow.nodes, flow.edges, loadedColumns); @@ -1178,6 +1200,7 @@ function InnerEditor({ setColumns(loadedColumns); setFields(loadedFields); setSettings(loadedSettings); + setOptionalSteps(loadedOptionalSteps); setName(activeWorkflow.name); setDescription(activeWorkflow.description ?? ""); setEditingName(false); @@ -1192,6 +1215,7 @@ function InnerEditor({ loadedColumns, loadedFields, loadedSettings, + loadedOptionalSteps, ); setSelectedNodeId(null); setSelectedEdgeId(null); @@ -1495,6 +1519,11 @@ function InnerEditor({ setEdges(flow.edges); setColumns(columnsOf({ ...targetWorkflow, ir: result.ir })); setFields(fieldsOf({ ...targetWorkflow, ir: result.ir })); + // Hydrate settings + optionalSteps on the fragment/generate path too — it + // previously dropped both, which silently lost the declarations on the next + // save (the round-trip data loss U2 fixes for the primary load path). + setSettings(settingsOf({ ...targetWorkflow, ir: result.ir })); + setOptionalSteps(optionalStepsOf({ ...targetWorkflow, ir: result.ir })); setSelectedNodeId(null); setSelectedEdgeId(null); setValidationError(null); @@ -1806,6 +1835,7 @@ function InnerEditor({ columns.length ? columns : undefined, fields.length ? fields : undefined, settings.length ? settings : undefined, + optionalSteps.length ? optionalSteps : undefined, ); // Include name/description in the PATCH only when they changed from the // loaded workflow (KTD-10 inline rename/description persist here). @@ -1823,6 +1853,7 @@ function InnerEditor({ columns, fields, settings, + optionalSteps, ); setName(updated.name); setDescription(updated.description ?? ""); @@ -1892,7 +1923,7 @@ function InnerEditor({ } finally { setSaving(false); } - }, [activeWorkflow, name, description, nodes, edges, columns, fields, settings, unplaced, blockingViolationCount, projectId, addToast, t]); + }, [activeWorkflow, name, description, nodes, edges, columns, fields, settings, optionalSteps, unplaced, blockingViolationCount, projectId, addToast, t]); // Stamp the shared error-state badge onto offending nodes: unplaced step // nodes and any node the server flagged (seam-in-branch). One component @@ -2454,6 +2485,27 @@ function InnerEditor({ )} + +
+ + {!optionalStepsCollapsed && ( + p.template)} + /> + )} +
)} @@ -2576,6 +2628,7 @@ function InnerEditor({ ["add", t("workflowNodes.mobileAdd", "Add")], ["settings", t("workflowSettings.title", "Settings")], ["fields", t("workflowFields.title", "Fields")], + ["optional-steps", t("workflowOptionalSteps.title", "Optional steps")], ["columns", t("workflowColumns.title", "Columns")], ["actions", t("workflowNodes.mobileActions", "Actions")], ] as Array<[MobileWorkflowPanel, string]>).map(([panel, label]) => ( @@ -2754,6 +2807,17 @@ function InnerEditor({ )} + {mobilePanel === "optional-steps" && ( +
+ p.template)} + /> +
+ )} + {mobilePanel === "columns" && (
void; + disabled?: boolean; + /** Test/styling hook applied to the trigger. */ + triggerTestId?: string; +} + +interface PanelPosition { + top: number; + left: number; + width: number; +} + +export function WorkflowOptionalStepsDropdown({ + steps, + enabledIds, + onToggle, + disabled = false, + triggerTestId = "wf-optional-steps-dropdown-trigger", +}: WorkflowOptionalStepsDropdownProps) { + const { t } = useTranslation("app"); + const [isOpen, setIsOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(0); + const [position, setPosition] = useState(null); + const triggerRef = useRef(null); + const panelRef = useRef(null); + const optionRefs = useRef<(HTMLDivElement | null)[]>([]); + const labelId = useId(); + + const reposition = useCallback(() => { + const trigger = triggerRef.current; + if (!trigger) return; + const rect = trigger.getBoundingClientRect(); + setPosition({ top: rect.bottom + 4, left: rect.left, width: rect.width }); + }, []); + + // Reposition on open and keep anchored during scroll/resize. + useEffect(() => { + if (!isOpen) return; + reposition(); + const handle = () => reposition(); + window.addEventListener("resize", handle); + window.addEventListener("scroll", handle, true); + return () => { + window.removeEventListener("resize", handle); + window.removeEventListener("scroll", handle, true); + }; + }, [isOpen, reposition]); + + // Outside-click closes (capture so it fires before the trigger's own handler). + useEffect(() => { + if (!isOpen) return; + const onDocMouseDown = (e: MouseEvent) => { + const target = e.target as Node; + if (triggerRef.current?.contains(target) || panelRef.current?.contains(target)) return; + setIsOpen(false); + }; + document.addEventListener("mousedown", onDocMouseDown); + return () => document.removeEventListener("mousedown", onDocMouseDown); + }, [isOpen]); + + const close = useCallback(() => { + setIsOpen(false); + triggerRef.current?.focus(); + }, []); + + // Focus the active option when the panel opens or the active index changes — + // driven by an effect (not an inline ref callback) so a re-render from toggling + // a step does not steal focus back to the active option on every commit. + useEffect(() => { + if (isOpen && position) optionRefs.current[activeIndex]?.focus(); + }, [isOpen, position, activeIndex]); + + // Empty state: render nothing (committed behavior, shared with the modal). + if (steps.length === 0) return null; + + const selectedCount = steps.filter((s) => enabledIds.includes(s.templateId)).length; + const triggerLabel = + selectedCount === 0 + ? t("workflowOptionalSteps.triggerNone", "Steps: none") + : t("workflowOptionalSteps.triggerCount", "Steps: {{count}} selected", { count: selectedCount }); + + const onTriggerKeyDown = (e: React.KeyboardEvent) => { + // ArrowUp also opens (ARIA listbox authoring guidance), landing on the last option. + if (e.key === "ArrowDown" || e.key === "ArrowUp" || e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setIsOpen(true); + setActiveIndex(e.key === "ArrowUp" ? steps.length - 1 : 0); + } + }; + + const onPanelKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "Escape") { + e.preventDefault(); + close(); + } else if (e.key === "ArrowDown") { + e.preventDefault(); + setActiveIndex((i) => Math.min(i + 1, steps.length - 1)); + } else if (e.key === "ArrowUp") { + e.preventDefault(); + setActiveIndex((i) => Math.max(i - 1, 0)); + } else if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + const step = steps[activeIndex]; + if (step) onToggle(step.templateId); + } + }; + + return ( +
+ + + {isOpen && + position && + createPortal( +
+ {steps.map((step, i) => { + const checked = enabledIds.includes(step.templateId); + return ( +
{ + optionRefs.current[i] = el; + }} + className={`wf-optional-steps-dropdown-option${i === activeIndex ? " is-active" : ""}`} + data-testid={`wf-optional-steps-dropdown-option-${step.templateId}`} + onClick={() => onToggle(step.templateId)} + > + +
+ + {step.name} + {phaseBadge(step.phase, step.templateId, "wf-optional-steps-dropdown-phase", t)} + + {step.description && ( + {step.description} + )} +
+
+ ); + })} +
, + document.body, + )} +
+ ); +} + +export default WorkflowOptionalStepsDropdown; diff --git a/packages/dashboard/app/components/WorkflowOptionalStepsPanel.css b/packages/dashboard/app/components/WorkflowOptionalStepsPanel.css new file mode 100644 index 0000000000..3b2a8503d6 --- /dev/null +++ b/packages/dashboard/app/components/WorkflowOptionalStepsPanel.css @@ -0,0 +1,107 @@ +/* WorkflowOptionalStepsPanel — sibling of WorkflowFieldsPanel; mirrors its layout + * so the optional-steps panel reads consistently alongside Fields/Settings. */ + +.wf-optional-steps-panel { + display: flex; + flex-direction: column; + gap: var(--space-sm); + padding: var(--space-md); +} + +.wf-optional-steps-header h3 { + margin: 0; +} + +.wf-optional-steps-hint, +.wf-optional-steps-empty { + font-size: 0.75rem; + color: var(--text-muted); + margin: 0; +} + +.wf-optional-steps-list { + list-style: none; + margin: 0; + padding: 0; + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.wf-optional-step-item { + display: flex; + flex-direction: column; + gap: 4px; + padding: var(--space-sm); + border: 1px solid var(--border); + border-radius: var(--radius-sm, 6px); +} + +.wf-optional-step-item.is-unknown { + opacity: 0.6; +} + +.wf-optional-step-head { + display: flex; + align-items: center; + justify-content: space-between; + gap: 8px; +} + +.wf-optional-step-title { + display: inline-flex; + align-items: center; + gap: 6px; + min-width: 0; +} + +.wf-optional-step-name { + font-weight: 600; + font-size: 0.8rem; +} + +.wf-optional-step-name--unknown { + font-style: italic; + font-weight: 400; +} + +.wf-optional-step-description { + font-size: 0.72rem; + color: var(--text-muted); + margin: 0; +} + +.wf-optional-step-default { + display: inline-flex; + align-items: center; + gap: 6px; + font-size: 0.75rem; +} + +.wf-optional-step-remove { + display: inline-flex; + align-items: center; + justify-content: center; + background: transparent; + border: none; + color: var(--text-muted); + cursor: pointer; +} + +.wf-optional-step-remove:hover:not(:disabled) { + color: var(--danger, #ef4444); +} + +.wf-optional-steps-add { + display: flex; + flex-direction: column; + gap: 4px; +} + +.wf-optional-steps-add-label { + display: inline-flex; + align-items: center; + gap: 4px; + font-size: 0.75rem; + color: var(--text-muted); +} diff --git a/packages/dashboard/app/components/WorkflowOptionalStepsPanel.tsx b/packages/dashboard/app/components/WorkflowOptionalStepsPanel.tsx new file mode 100644 index 0000000000..40bdc94a0f --- /dev/null +++ b/packages/dashboard/app/components/WorkflowOptionalStepsPanel.tsx @@ -0,0 +1,177 @@ +/** + * FNXC:WorkflowOptionalSteps 2026-06-21-00:00: + * Workflow authors need to declare which step templates are optional and set each + * one's defaultOn from the visual editor (persisted on the IR's `optionalSteps` + * array) so optional steps are authorable without hand-editing IR. + * + * WorkflowOptionalStepsPanel — the workflow editor's optional-step authoring + * surface. Sibling to {@link WorkflowFieldsPanel} / WorkflowSettingsPanel: lives + * alongside the canvas in {@link WorkflowNodeEditor} and mutates the IR's + * `optionalSteps` array through the same state/save flow (preserved across the + * round-trip by `flowToIr`). + * + * A declaration is just `{ templateId, defaultOn? }`. Display metadata + * (name/description/phase) is resolved from the built-in step-template catalog at + * render time — never duplicated into the IR — so the resolver stays the single + * source of truth. Unknown/stale template ids render a muted, still-removable row + * rather than being silently dropped. + */ +import { useCallback, useMemo } from "react"; +import { useTranslation } from "react-i18next"; +import { Plus, Trash2 } from "lucide-react"; +import { WORKFLOW_STEP_TEMPLATES, type WorkflowOptionalStep, type WorkflowStepTemplate } from "@fusion/core"; +import { phaseBadge } from "./workflow-phase-badge"; +import "./WorkflowOptionalStepsPanel.css"; + +interface WorkflowOptionalStepsPanelProps { + optionalSteps: WorkflowOptionalStep[]; + onChange: (next: WorkflowOptionalStep[]) => void; + readOnly: boolean; + /** Plugin-contributed templates, merged into the catalog when available. */ + pluginTemplates?: WorkflowStepTemplate[]; +} + +export function WorkflowOptionalStepsPanel({ + optionalSteps, + onChange, + readOnly, + pluginTemplates = [], +}: WorkflowOptionalStepsPanelProps) { + const { t } = useTranslation("app"); + + const templatesById = useMemo(() => { + const map = new Map(); + for (const tpl of [...WORKFLOW_STEP_TEMPLATES, ...pluginTemplates]) map.set(tpl.id, tpl); + return map; + }, [pluginTemplates]); + + const declaredIds = useMemo(() => new Set(optionalSteps.map((s) => s.templateId)), [optionalSteps]); + + // Catalog entries not already declared — the "Add optional step" picker source. + const available = useMemo( + () => [...templatesById.values()].filter((tpl) => !declaredIds.has(tpl.id)), + [templatesById, declaredIds], + ); + + const addStep = useCallback( + (templateId: string) => { + if (!templateId || declaredIds.has(templateId)) return; + onChange([...optionalSteps, { templateId, defaultOn: false }]); + }, + [optionalSteps, onChange, declaredIds], + ); + + const removeStep = useCallback( + (templateId: string) => onChange(optionalSteps.filter((s) => s.templateId !== templateId)), + [optionalSteps, onChange], + ); + + const toggleDefaultOn = useCallback( + (templateId: string, defaultOn: boolean) => + onChange(optionalSteps.map((s) => (s.templateId === templateId ? { ...s, defaultOn } : s))), + [optionalSteps, onChange], + ); + + return ( + + ); +} + +export default WorkflowOptionalStepsPanel; diff --git a/packages/dashboard/app/components/WorkflowResultsTab.tsx b/packages/dashboard/app/components/WorkflowResultsTab.tsx index dbd22cff8b..a87efbc6dc 100644 --- a/packages/dashboard/app/components/WorkflowResultsTab.tsx +++ b/packages/dashboard/app/components/WorkflowResultsTab.tsx @@ -15,6 +15,7 @@ import type { AgentLogEntry, Settings, Task, TaskDetail, WorkflowDefinition, Wor import { getErrorMessage, resolveTaskExecutionModel, resolveTaskPlanningModel, resolveTaskValidatorModel } from "@fusion/core"; import { approveTaskWorkflowCli, fetchWorkflow, fetchWorkflows, fetchWorkflowSteps, fetchTaskWorkflow, fetchWorkflowOptionalSteps, selectTaskWorkflow, submitTaskWorkflowInput } from "../api"; import { WorkflowSelector } from "./WorkflowSelector"; +import { phaseBadge } from "./workflow-phase-badge"; import { useAgentLogs } from "../hooks/useAgentLogs"; import { ProviderIcon } from "./ProviderIcon"; import { irToFlow } from "./workflow-flow-mapping"; @@ -134,17 +135,8 @@ function getOutputPreview(output: string): string { return `${lines.length} lines`; } -function phaseBadge(phase: "pre-merge" | "post-merge", id: string, prefix: string, t: ReturnType["t"]): ReactNode { - const phaseClass = phase === "post-merge" ? "phase-badge--post-merge" : "phase-badge--pre-merge"; - return ( - - {phase === "post-merge" ? t("app:workflow.postMerge", "Post-merge") : t("app:workflow.preMerge", "Pre-merge")} - - ); -} +// phaseBadge moved to ./workflow-phase-badge (shared with the optional-steps panel +// and the optional-steps dropdown). Imported above. function getWorkflowName( selectedWorkflowId: string | null, diff --git a/packages/dashboard/app/components/__tests__/InlineCreateCard.test.tsx b/packages/dashboard/app/components/__tests__/InlineCreateCard.test.tsx index d2d823997a..2696f3136f 100644 --- a/packages/dashboard/app/components/__tests__/InlineCreateCard.test.tsx +++ b/packages/dashboard/app/components/__tests__/InlineCreateCard.test.tsx @@ -1079,12 +1079,14 @@ describe("InlineCreateCard button visibility when collapsed", () => { target: { value: "Verify login flow in browser" }, }); - const toggle = await screen.findByTestId("inline-create-browser-verification-toggle"); - expect(toggle).toHaveTextContent("Browser Verification"); - expect(toggle).toHaveAttribute("aria-pressed", "false"); - - fireEvent.click(toggle); - expect(toggle).toHaveAttribute("aria-pressed", "true"); + // Open the optional-steps dropdown and select browser verification. + const trigger = await screen.findByTestId("inline-create-optional-steps-trigger"); + expect(trigger).toHaveTextContent("Steps: none"); + fireEvent.click(trigger); + const option = await screen.findByTestId("wf-optional-steps-dropdown-option-browser-verification"); + expect(option).toHaveAttribute("aria-checked", "false"); + fireEvent.click(option); + expect(trigger).toHaveTextContent("Steps: 1 selected"); fireEvent.click(screen.getByTestId("save-button")); await waitFor(() => { diff --git a/packages/dashboard/app/components/__tests__/NewTaskModal.test.tsx b/packages/dashboard/app/components/__tests__/NewTaskModal.test.tsx index 9622b2cc40..79104ae68f 100644 --- a/packages/dashboard/app/components/__tests__/NewTaskModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/NewTaskModal.test.tsx @@ -31,6 +31,7 @@ vi.mock("../../api", () => ({ // U6/R3: TaskForm's picker fetches whole workflows; the per-step // fetchWorkflowSteps + post-create selectTaskWorkflow flow is gone. fetchWorkflows: vi.fn().mockResolvedValue([]), + fetchWorkflowOptionalSteps: vi.fn().mockResolvedValue([]), fetchGlobalSettings: vi.fn().mockResolvedValue({}), fetchGitBranches: vi.fn().mockResolvedValue([]), fetchAgents: vi.fn().mockResolvedValue([]), @@ -239,6 +240,89 @@ describe("NewTaskModal", () => { }); }); + describe("optional workflow steps (U4)", () => { + const WF = { + id: "wf-x", + name: "Custom", + kind: "workflow" as const, + description: "", + ir: { version: "v1" as const, name: "Custom", nodes: [], edges: [] }, + layout: {}, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }; + const STEP = { + templateId: "browser-verification", + name: "Browser Verification", + description: "Verify web application functionality using browser automation", + icon: "globe", + phase: "pre-merge" as const, + defaultOn: false, + }; + + it("includes a toggled-on optional step in the create payload", async () => { + const { fetchWorkflows, fetchWorkflowOptionalSteps } = await import("../../api"); + vi.mocked(fetchWorkflows).mockResolvedValue([WF]); + vi.mocked(fetchWorkflowOptionalSteps).mockResolvedValue([STEP]); + + const { props } = renderNewTaskModal(); + fireEvent.change(screen.getByPlaceholderText("What needs to be done?"), { + target: { value: "Verify the login page" }, + }); + fireEvent.change(await screen.findByTestId("task-workflow-select"), { target: { value: "wf-x" } }); + + const trigger = await screen.findByTestId("task-optional-steps-trigger"); + expect(trigger).toHaveTextContent("Steps: none"); + fireEvent.click(trigger); + fireEvent.click(await screen.findByTestId("wf-optional-steps-dropdown-option-browser-verification")); + + fireEvent.click(screen.getByRole("button", { name: "Create Task" })); + await waitFor(() => { + expect(props.onCreateTask).toHaveBeenCalledWith( + expect.objectContaining({ enabledWorkflowSteps: ["browser-verification"] }), + ); + }); + }); + + it("seeds defaultOn steps as pre-enabled and submits them without toggling", async () => { + const { fetchWorkflows, fetchWorkflowOptionalSteps } = await import("../../api"); + vi.mocked(fetchWorkflows).mockResolvedValue([WF]); + vi.mocked(fetchWorkflowOptionalSteps).mockResolvedValue([{ ...STEP, defaultOn: true }]); + + const { props } = renderNewTaskModal(); + fireEvent.change(screen.getByPlaceholderText("What needs to be done?"), { target: { value: "task" } }); + fireEvent.change(await screen.findByTestId("task-workflow-select"), { target: { value: "wf-x" } }); + + const trigger = await screen.findByTestId("task-optional-steps-trigger"); + await waitFor(() => expect(trigger).toHaveTextContent("Steps: 1 selected")); + + fireEvent.click(screen.getByRole("button", { name: "Create Task" })); + await waitFor(() => { + expect(props.onCreateTask).toHaveBeenCalledWith( + expect.objectContaining({ enabledWorkflowSteps: ["browser-verification"] }), + ); + }); + }); + + it("renders no dropdown and omits enabledWorkflowSteps for 'No workflow'", async () => { + const { fetchWorkflows, fetchWorkflowOptionalSteps } = await import("../../api"); + vi.mocked(fetchWorkflows).mockResolvedValue([WF]); + vi.mocked(fetchWorkflowOptionalSteps).mockResolvedValue([STEP]); + + const { props } = renderNewTaskModal(); + fireEvent.change(screen.getByPlaceholderText("What needs to be done?"), { target: { value: "task" } }); + // "No workflow" → null selection → no optional-steps fetch, no dropdown. + fireEvent.change(await screen.findByTestId("task-workflow-select"), { target: { value: "__none__" } }); + + expect(screen.queryByTestId("task-optional-steps-trigger")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Create Task" })); + await waitFor(() => { + const call = vi.mocked(props.onCreateTask).mock.calls.at(-1)?.[0]; + expect(call).not.toHaveProperty("enabledWorkflowSteps"); + }); + }); + }); + it("submits project-default branch selection by default", async () => { const { props } = renderNewTaskModal(); diff --git a/packages/dashboard/app/components/__tests__/TaskForm.test.tsx b/packages/dashboard/app/components/__tests__/TaskForm.test.tsx index ecf3eba463..bc2ee2c918 100644 --- a/packages/dashboard/app/components/__tests__/TaskForm.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskForm.test.tsx @@ -27,6 +27,7 @@ vi.mock("../../api", () => ({ }), // U6/R3: TaskForm now fetches whole workflows (not steps) for the picker. fetchWorkflows: vi.fn().mockResolvedValue([]), + fetchWorkflowOptionalSteps: vi.fn().mockResolvedValue([]), fetchGlobalSettings: vi.fn().mockResolvedValue({}), refineText: vi.fn().mockResolvedValue("Refined text"), getRefineErrorMessage: vi.fn((err) => err?.message || "Failed to refine text. Please try again."), diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index b8ae11847d..a7e54ef6b4 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -164,6 +164,14 @@ function v2Def(): WorkflowDefinition { }; } +function v2DefWithOptional(): WorkflowDefinition { + const base = v2Def(); + return { + ...base, + ir: { ...(base.ir as object), optionalSteps: [{ templateId: "browser-verification" }] } as WorkflowDefinition["ir"], + }; +} + function builtinDef(): WorkflowDefinition { return { id: "builtin:coding", @@ -742,6 +750,34 @@ describe("WorkflowNodeEditor", () => { expect(start?.column).toBe("done"); }); + it("hydrates declared optional steps and preserves them through a dirty save (round-trip)", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([v2DefWithOptional()]); + vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ + ...v2DefWithOptional(), + ...(updates as object), + })); + vi.mocked(compileWorkflow).mockResolvedValue({ steps: [] }); + + render( {}} addToast={() => {}} />); + + await screen.findByText("Save"); + // The declared optional step is hydrated into the panel (optionalStepsOf). + const row = await screen.findByTestId("wf-optional-step-browser-verification"); + expect(within(row).getByText("Browser Verification")).toBeTruthy(); + + // Toggling defaultOn must mark the editor dirty (serializeGraph threading) so + // the Save button enables and persists the change. + fireEvent.click(within(row).getByRole("checkbox")); + fireEvent.click(screen.getByText("Save").closest("button")!); + + await waitFor(() => expect(updateWorkflow).toHaveBeenCalled()); + const [, updates] = vi.mocked(updateWorkflow).mock.calls[0]; + const ir = (updates as { ir: WorkflowDefinition["ir"] }).ir as { + optionalSteps?: { templateId: string; defaultOn?: boolean }[]; + }; + expect(ir.optionalSteps).toEqual([{ templateId: "browser-verification", defaultOn: true }]); + }); + it("renders the start inspector without the entry-column select for v1 workflows", async () => { vi.mocked(fetchWorkflows).mockResolvedValue([def()]); diff --git a/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsDropdown.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsDropdown.test.tsx new file mode 100644 index 0000000000..4d458345b2 --- /dev/null +++ b/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsDropdown.test.tsx @@ -0,0 +1,107 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup, within } from "@testing-library/react"; +import { useState } from "react"; +import type { ResolvedWorkflowOptionalStep } from "@fusion/core"; +import { WorkflowOptionalStepsDropdown } from "../WorkflowOptionalStepsDropdown"; + +const STEP: ResolvedWorkflowOptionalStep = { + templateId: "browser-verification", + name: "Browser Verification", + description: "Verify web application functionality using browser automation", + icon: "globe", + phase: "pre-merge", + defaultOn: false, +}; + +// Controlled host: parent owns the enabled set, mirroring the create surfaces. +function Host({ steps, initial = [] }: { steps: ResolvedWorkflowOptionalStep[]; initial?: string[] }) { + const [enabled, setEnabled] = useState(initial); + return ( + + setEnabled((prev) => (prev.includes(id) ? prev.filter((x) => x !== id) : [...prev, id])) + } + /> + ); +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe("WorkflowOptionalStepsDropdown", () => { + it("renders nothing when there are no optional steps", () => { + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); + + it("reflects the selected count in the trigger label", () => { + render(); + const trigger = screen.getByTestId("wf-optional-steps-dropdown-trigger"); + expect(trigger).toHaveTextContent("Steps: none"); + }); + + it("opens, toggles a step, and updates the trigger count", () => { + render(); + const trigger = screen.getByTestId("wf-optional-steps-dropdown-trigger"); + expect(trigger).toHaveAttribute("aria-expanded", "false"); + fireEvent.click(trigger); + expect(trigger).toHaveAttribute("aria-expanded", "true"); + + const option = screen.getByTestId("wf-optional-steps-dropdown-option-browser-verification"); + expect(option).toHaveAttribute("role", "option"); + expect(option).toHaveAttribute("aria-checked", "false"); + fireEvent.click(option); + expect(screen.getByTestId("wf-optional-steps-dropdown-option-browser-verification")).toHaveAttribute( + "aria-checked", + "true", + ); + expect(trigger).toHaveTextContent("Steps: 1 selected"); + }); + + it("pre-checks a step seeded as enabled by the parent (defaultOn)", () => { + render(); + fireEvent.click(screen.getByTestId("wf-optional-steps-dropdown-trigger")); + expect(screen.getByTestId("wf-optional-steps-dropdown-option-browser-verification")).toHaveAttribute( + "aria-checked", + "true", + ); + }); + + it("exposes the panel as an accessible listbox labelled by the trigger", () => { + render(); + fireEvent.click(screen.getByTestId("wf-optional-steps-dropdown-trigger")); + const panel = screen.getByTestId("wf-optional-steps-dropdown-panel"); + expect(panel).toHaveAttribute("role", "listbox"); + expect(within(panel).getByText("Browser Verification")).toBeTruthy(); + }); + + it("closes on Escape", () => { + render(); + const trigger = screen.getByTestId("wf-optional-steps-dropdown-trigger"); + fireEvent.click(trigger); + const panel = screen.getByTestId("wf-optional-steps-dropdown-panel"); + fireEvent.keyDown(panel, { key: "Escape" }); + expect(screen.queryByTestId("wf-optional-steps-dropdown-panel")).toBeNull(); + expect(trigger).toHaveAttribute("aria-expanded", "false"); + }); + + it("closes on outside click without losing selection", () => { + render( +
+ + +
, + ); + const trigger = screen.getByTestId("wf-optional-steps-dropdown-trigger"); + fireEvent.click(trigger); + expect(screen.getByTestId("wf-optional-steps-dropdown-panel")).toBeTruthy(); + fireEvent.mouseDown(screen.getByTestId("outside")); + expect(screen.queryByTestId("wf-optional-steps-dropdown-panel")).toBeNull(); + // Selection preserved. + expect(trigger).toHaveTextContent("Steps: 1 selected"); + }); +}); diff --git a/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsPanel.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsPanel.test.tsx new file mode 100644 index 0000000000..93398b01f9 --- /dev/null +++ b/packages/dashboard/app/components/__tests__/WorkflowOptionalStepsPanel.test.tsx @@ -0,0 +1,92 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup, within } from "@testing-library/react"; +import { useState } from "react"; +import type { WorkflowOptionalStep } from "@fusion/core"; +import { WorkflowOptionalStepsPanel } from "../WorkflowOptionalStepsPanel"; + +// Controlled host mirroring how WorkflowNodeEditor drives the panel. +function Host({ + initial, + readOnly = false, + onState, +}: { + initial: WorkflowOptionalStep[]; + readOnly?: boolean; + onState?: (s: WorkflowOptionalStep[]) => void; +}) { + const [optionalSteps, setOptionalSteps] = useState(initial); + return ( + { + setOptionalSteps(next); + onState?.(next); + }} + /> + ); +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe("WorkflowOptionalStepsPanel", () => { + it("renders the empty state and an add picker when no steps are declared", () => { + render(); + expect(screen.getByText(/No optional steps/i)).toBeTruthy(); + const select = screen.getByTestId("wf-optional-steps-add-select") as HTMLSelectElement; + // browser-verification is in the catalog and not yet declared → available. + expect(within(select).getByRole("option", { name: "Browser Verification" })).toBeTruthy(); + }); + + it("adds a step from the picker (defaultOn false) and removes it from the picker", () => { + const onState = vi.fn(); + render(); + fireEvent.change(screen.getByTestId("wf-optional-steps-add-select"), { + target: { value: "browser-verification" }, + }); + expect(onState).toHaveBeenCalledWith([{ templateId: "browser-verification", defaultOn: false }]); + // The declared row is shown with the resolved template name… + const row = screen.getByTestId("wf-optional-step-browser-verification"); + expect(within(row).getByText("Browser Verification")).toBeTruthy(); + // …and the picker no longer offers it. + const select = screen.getByTestId("wf-optional-steps-add-select") as HTMLSelectElement; + expect(within(select).queryByRole("option", { name: "Browser Verification" })).toBeNull(); + }); + + it("toggles defaultOn for a declared step", () => { + const onState = vi.fn(); + render(); + const row = screen.getByTestId("wf-optional-step-browser-verification"); + fireEvent.click(within(row).getByRole("checkbox")); + expect(onState).toHaveBeenCalledWith([{ templateId: "browser-verification", defaultOn: true }]); + }); + + it("removes a declared step and returns it to the picker", () => { + render(); + const row = screen.getByTestId("wf-optional-step-browser-verification"); + fireEvent.click(within(row).getByRole("button", { name: /Remove optional step/i })); + expect(screen.queryByTestId("wf-optional-step-browser-verification")).toBeNull(); + const select = screen.getByTestId("wf-optional-steps-add-select") as HTMLSelectElement; + expect(within(select).getByRole("option", { name: "Browser Verification" })).toBeTruthy(); + }); + + it("renders an unknown/stale templateId as a muted, still-removable row", () => { + const onState = vi.fn(); + render(); + const row = screen.getByTestId("wf-optional-step-does-not-exist"); + expect(row.className).toContain("is-unknown"); + expect(within(row).getByText(/Unknown step/i)).toBeTruthy(); + fireEvent.click(within(row).getByRole("button", { name: /Remove optional step/i })); + expect(onState).toHaveBeenCalledWith([]); + }); + + it("disables editing when readOnly", () => { + render(); + const row = screen.getByTestId("wf-optional-step-browser-verification"); + expect((within(row).getByRole("checkbox") as HTMLInputElement).disabled).toBe(true); + expect((within(row).getByRole("button", { name: /Remove optional step/i }) as HTMLButtonElement).disabled).toBe(true); + }); +}); diff --git a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts index 18dd5b39b3..19d46d461c 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -9,6 +9,7 @@ import { fragmentSeamConflicts, copyIrWithFreshIds, columnsOf, + optionalStepsOf, columnForY, bandTop, columnsToBandNodes, @@ -1503,3 +1504,84 @@ describe("copyIrWithFreshIds", () => { expect(t1NewId).toEqual({ x: 10, y: 20 }); }); }); + +describe("optionalSteps round-trip (U2)", () => { + const v2WithOptional = (optionalSteps?: { templateId: string; defaultOn?: boolean }[]) => + makeDef( + parseWorkflowIr({ + version: "v2", + name: "wf-opt", + columns: [ + { id: "triage", name: "Triage", traits: [] }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "triage" }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [{ from: "start", to: "end" }], + ...(optionalSteps ? { optionalSteps } : {}), + }), + ); + + it("optionalStepsOf reads declarations from a v2 IR and returns a copy", () => { + const def = v2WithOptional([{ templateId: "browser-verification", defaultOn: true }]); + const read = optionalStepsOf(def); + expect(read).toEqual([{ templateId: "browser-verification", defaultOn: true }]); + // mutating the result does not mutate the source IR + read[0].defaultOn = false; + expect(optionalStepsOf(def)).toEqual([{ templateId: "browser-verification", defaultOn: true }]); + }); + + it("optionalStepsOf returns [] for v1 and for v2 without optionalSteps", () => { + const v1 = makeDef({ + version: "v1", + name: "legacy", + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end" }], + }); + expect(optionalStepsOf(v1)).toEqual([]); + expect(optionalStepsOf(v2WithOptional())).toEqual([]); + }); + + it("flowToIr preserves optionalSteps across a full irToFlow round-trip", () => { + const def = v2WithOptional([{ templateId: "browser-verification", defaultOn: true }]); + const { nodes, edges } = irToFlow(def); + const { ir: out } = flowToIr("wf-opt", nodes, edges, columnsOf(def), [], [], optionalStepsOf(def)); + expect((out as { optionalSteps?: unknown }).optionalSteps).toEqual([ + { templateId: "browser-verification", defaultOn: true }, + ]); + }); + + it("serializes as v2 when optionalSteps present but no custom columns/fields/settings", () => { + const { ir: out } = flowToIr( + "opt-only", + [ + { id: "start", type: "workflowNode", position: { x: 0, y: 0 }, data: { kind: "start" } }, + { id: "end", type: "workflowNode", position: { x: 0, y: 200 }, data: { kind: "end" } }, + ] as unknown as FlowNode[], + [{ id: "e1", source: "start", target: "end" }], + [], + [], + [], + [{ templateId: "browser-verification" }], + ); + expect(out.version).toBe("v2"); + expect((out as { optionalSteps?: unknown }).optionalSteps).toEqual([ + { templateId: "browser-verification" }, + ]); + }); + + it("omits the optionalSteps key entirely when empty (R6 byte-identity)", () => { + const def = v2WithOptional(); + const { nodes, edges } = irToFlow(def); + const { ir: out } = flowToIr("wf-opt", nodes, edges, columnsOf(def), [], [], []); + expect("optionalSteps" in out).toBe(false); + // and with the arg omitted entirely + const { ir: out2 } = flowToIr("wf-opt", nodes, edges, columnsOf(def)); + expect("optionalSteps" in out2).toBe(false); + }); +}); diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 4cf2f9ee10..cea334ada6 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -9,6 +9,7 @@ import type { WorkflowDefinition, WorkflowFieldDefinition, WorkflowSettingDefinition, + WorkflowOptionalStep, } from "@fusion/core"; import type { WorkflowFlowNodeData, WorkflowEditorNodeKind } from "./nodes/WorkflowNodeTypes"; @@ -434,6 +435,7 @@ export function flowToIr( columns?: WorkflowIrColumn[], fields?: WorkflowFieldDefinition[], settings?: WorkflowSettingDefinition[], + optionalSteps?: WorkflowOptionalStep[], ): { ir: WorkflowIr; layout: Record } { const realNodes = nodes.filter((n) => !isColumnBandNode(n.id)); // Partition by parentId: foreach group children reassemble into that group's @@ -453,9 +455,15 @@ export function flowToIr( ); const hasFields = Array.isArray(fields) && fields.length > 0; const hasSettings = Array.isArray(settings) && settings.length > 0; - // Fields and settings are v2-only declarations: a workflow with either but no - // custom columns still serializes as v2 (with the synthesized default columns). - const v2 = (Array.isArray(columns) && columns.length > 0) || hasFields || hasSettings; + const hasOptionalSteps = Array.isArray(optionalSteps) && optionalSteps.length > 0; + // FNXC:WorkflowOptionalSteps 2026-06-21-00:00: + // Optional steps must round-trip through the node editor without data loss, yet + // must never upgrade a legacy v1 graph. Fields, settings, and optional steps are + // v2-only declarations: a workflow with any of them but no custom columns still + // serializes as v2 (with the synthesized default columns). Empty/absent → not a + // v2 signal, and the key is omitted entirely (R6 byte-identity for legacy graphs). + const v2 = + (Array.isArray(columns) && columns.length > 0) || hasFields || hasSettings || hasOptionalSteps; const layout: Record = {}; /** Project one flow node (top-level or template child) into an IR node. */ @@ -556,6 +564,12 @@ export function flowToIr( render: s.render ? { ...s.render } : undefined, })); } + if (hasOptionalSteps) { + // Optional-step DECLARATIONS round-trip through the editor opaquely (they are + // not graph nodes; the resolver + server validator are the source of truth). + // Omitted entirely when empty so legacy graphs stay byte-identical (R6). + (ir as { optionalSteps?: unknown }).optionalSteps = optionalSteps!.map((o) => ({ ...o })); + } return { ir, layout }; } @@ -967,6 +981,16 @@ export function settingsOf(def: WorkflowDefinition): WorkflowSettingDefinition[] })); } +/** Extract the editor's working optional-step declaration list from a definition. + * v2 with `optionalSteps` → a shallow copy; v1 or none → empty. Display metadata + * (name/icon/phase) is NOT carried here — it is resolved from the step-template + * catalog at render time so the resolver stays the single source of truth. */ +export function optionalStepsOf(def: WorkflowDefinition): WorkflowOptionalStep[] { + const ir = def.ir as { optionalSteps?: WorkflowOptionalStep[] }; + if (!isV2(def.ir) || !Array.isArray(ir.optionalSteps)) return []; + return ir.optionalSteps.map((o) => ({ ...o })); +} + /** Seed graph for a brand-new workflow: start → end with room to insert steps. */ export function emptyWorkflowIr(name: string): WorkflowIr { return { diff --git a/packages/dashboard/app/components/workflow-phase-badge.tsx b/packages/dashboard/app/components/workflow-phase-badge.tsx new file mode 100644 index 0000000000..fe5a171a97 --- /dev/null +++ b/packages/dashboard/app/components/workflow-phase-badge.tsx @@ -0,0 +1,27 @@ +/** + * FNXC:WorkflowOptionalSteps 2026-06-21-00:00: + * One phase chip (pre-merge / post-merge) shared by every workflow-step surface so + * the badge looks identical across the results tab, authoring panel, and dropdown. + * + * Shared phase chip for workflow steps (pre-merge / post-merge). Extracted from + * WorkflowResultsTab so the node-editor optional-steps panel and the optional-step + * dropdown render an identical badge without duplicating markup. + */ +import type { ReactNode } from "react"; +import type { useTranslation } from "react-i18next"; + +export function phaseBadge( + phase: "pre-merge" | "post-merge", + id: string, + prefix: string, + t: ReturnType["t"], +): ReactNode { + const phaseClass = phase === "post-merge" ? "phase-badge--post-merge" : "phase-badge--pre-merge"; + return ( + + {phase === "post-merge" + ? t("app:workflow.postMerge", "Post-merge") + : t("app:workflow.preMerge", "Pre-merge")} + + ); +} diff --git a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts index 379604d840..3d435e0c0a 100644 --- a/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts +++ b/packages/engine/src/__tests__/stepwise-workflow-parity.test.ts @@ -156,6 +156,7 @@ async function runStepwiseGraph( signal?: AbortSignal; onReset?: (active: ForeachActiveContext) => void; captureResetResult?: (ok: boolean, reason?: string) => void; + workflowStep?: WorkflowLegacySeams["workflowStep"]; } = {}, ): Promise<{ trajectory: TrajectoryEntry[]; outcome: string; result: Awaited> }> { const task = taskWithSteps(stepCount); @@ -168,6 +169,7 @@ async function runStepwiseGraph( merge: async () => ({ outcome: "success" }), schedule: async () => ({ outcome: "success" }), execute: async () => ({ outcome: "success" }), + ...(opts.workflowStep ? { workflowStep: opts.workflowStep } : {}), stepExecute: async (_t, ctx) => { const active = ctx[FOREACH_ACTIVE_CONTEXT_KEY] as ForeachActiveContext; const result = await runTaskStep( @@ -575,4 +577,45 @@ describe("stepwise workflow parity (U7 / KTD-9)", () => { // Merge ran (the task merges with no step work). expect(result.visitedNodeIds).toContain("merge"); }); + + // ── Pre-merge workflow-step seam (optional-step execution, R1) ───────────── + + it("runs the pre-merge workflow-step seam exactly once after the foreach (enabled steps execute)", async () => { + // This is the dead-toggle guard: without a workflow-step seam node on the + // success path, a stepwise task's enabledWorkflowSteps (e.g. browser + // verification) would never run. Wire a workflowStep spy and assert the graph + // invokes it once, between the foreach and review. + let workflowStepCalls = 0; + const { outcome, result } = await runStepwiseGraph( + 3, + [["APPROVE"], ["APPROVE"], ["APPROVE"]], + { + workflowStep: async () => { + workflowStepCalls++; + return { outcome: "success" }; + }, + }, + ); + + expect(outcome).toBe("success"); + // The seam ran ONCE post-foreach — not per step-instance (3 steps here). + expect(workflowStepCalls).toBe(1); + expect(result.visitedNodeIds).toContain("workflow-step"); + // Ordering: all step instances complete before the workflow-step seam, which + // precedes review. + const seamIdx = result.visitedNodeIds.indexOf("workflow-step"); + const reviewIdx = result.visitedNodeIds.indexOf("review"); + const lastStepIdx = result.visitedNodeIds.map((id) => id.startsWith("steps#")).lastIndexOf(true); + expect(lastStepIdx).toBeLessThan(seamIdx); + expect(seamIdx).toBeLessThan(reviewIdx); + }); + + it("treats the workflow-step seam as a no-op pass-through when no steps are enabled", async () => { + // No workflowStep seam wired → the handler skips to success and routes to + // review, leaving the trajectory identical to the pre-seam behavior. + const { outcome, result } = await runStepwiseGraph(2, [["APPROVE"], ["APPROVE"]]); + expect(outcome).toBe("success"); + expect(result.visitedNodeIds).toContain("workflow-step"); + expect(result.visitedNodeIds).toContain("review"); + }); });