From ab8c8beb4f8d944679aa93f42b6cdfda2c604689 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 21 Jun 2026 18:25:54 -0700 Subject: [PATCH] feat(FN-6880): resolve optional-step toggles + seeding from optional-group nodes (U3) Re-point resolveWorkflowOptionalSteps to scan v2 optional-group nodes instead of the legacy ir.optionalSteps declaration, and seed a new task's enabledWorkflowSteps from each group's defaultOn (materializeDefault/Explicit WorkflowSteps). Preserves the ResolvedWorkflowOptionalStep shape so the create/ edit toggle surfaces keep working (templateId now carries the group node id). Legacy optionalSteps type/field left in place for U7. Exports the new config type + resolveDefaultOnOptionalGroupIds helper. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../__tests__/workflow-optional-steps.test.ts | 169 ++++++++++-------- .../workflow-selection-store.test.ts | 69 +++++++ packages/core/src/index.ts | 6 +- packages/core/src/store.ts | 19 +- packages/core/src/workflow-optional-steps.ts | 77 +++++--- 5 files changed, 237 insertions(+), 103 deletions(-) diff --git a/packages/core/src/__tests__/workflow-optional-steps.test.ts b/packages/core/src/__tests__/workflow-optional-steps.test.ts index d3ea6ba91e..87deb717f5 100644 --- a/packages/core/src/__tests__/workflow-optional-steps.test.ts +++ b/packages/core/src/__tests__/workflow-optional-steps.test.ts @@ -1,8 +1,16 @@ 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"; +import { + resolveDefaultOnOptionalGroupIds, + resolveWorkflowOptionalSteps, +} from "../workflow-optional-steps.js"; +import type { + WorkflowIr, + WorkflowIrNode, + WorkflowIrV2, + WorkflowOptionalGroupConfig, +} from "../workflow-ir-types.js"; const v1: WorkflowIr = { version: "v1", @@ -14,105 +22,116 @@ const v1: WorkflowIr = { edges: [{ from: "start", to: "end" }], }; -function v2(optionalSteps?: WorkflowIrV2["optionalSteps"]): WorkflowIrV2 { +/** Build an optional-group node with a trivial single-prompt template. */ +function optionalGroupNode( + id: string, + config: Partial, +): WorkflowIrNode { + return { + id, + kind: "optional-group", + column: "todo", + config: { + ...config, + template: config.template ?? { + nodes: [{ id: `${id}-inner`, kind: "prompt" }], + edges: [], + }, + } satisfies WorkflowOptionalGroupConfig, + }; +} + +function v2(extraNodes: WorkflowIrNode[] = []): WorkflowIrV2 { return { version: "v2", name: "optional", columns: [{ id: "todo", name: "Todo", traits: [] }], nodes: [ { id: "start", kind: "start", column: "todo" }, + ...extraNodes, { id: "end", kind: "end", column: "todo" }, ], edges: [{ from: "start", to: "end" }], - optionalSteps, }; } -describe("resolveWorkflowOptionalSteps", () => { - it("resolves the builtin coding browser verification optional step", () => { - expect(resolveWorkflowOptionalSteps(BUILTIN_CODING_WORKFLOW_IR)).toEqual([ +describe("resolveWorkflowOptionalSteps (optional-group nodes)", () => { + it("resolves two optional-group nodes with names + defaultOn from node config", () => { + const ir = v2([ + optionalGroupNode("og-browser", { name: "Browser Verification", defaultOn: false }), + optionalGroupNode("og-security", { name: "Security Audit", defaultOn: true }), + ]); + + expect(resolveWorkflowOptionalSteps(ir)).toEqual([ { - templateId: "browser-verification", + templateId: "og-browser", name: "Browser Verification", - description: "Verify web application functionality using browser automation", - icon: "globe", + description: "", phase: "pre-merge", defaultOn: false, }, + { + templateId: "og-security", + name: "Security Audit", + description: "", + phase: "pre-merge", + defaultOn: true, + }, ]); }); - 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("falls back to the node id when the group config omits a name", () => { + const ir = v2([optionalGroupNode("og-unnamed", { defaultOn: true })]); + const [resolved] = resolveWorkflowOptionalSteps(ir); + expect(resolved.templateId).toBe("og-unnamed"); + expect(resolved.name).toBe("og-unnamed"); + expect(resolved.defaultOn).toBe(true); }); - 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([ - { templateId: "missing" }, - { templateId: "browser-verification" }, - ])), - ).toHaveLength(1); - }); - - it("returns an empty array for v1 and v2 workflows without optional steps", () => { + it("returns an empty array for v1 and v2 workflows without optional-group nodes", () => { expect(resolveWorkflowOptionalSteps(v1)).toEqual([]); expect(resolveWorkflowOptionalSteps(v2())).toEqual([]); }); - it("preserves declaration order and resolves plugin templates", () => { - const result = resolveWorkflowOptionalSteps( - v2([ - { templateId: "plugin:demo:first", defaultOn: true }, - { templateId: "browser-verification" }, - ]), - [ - { - id: "plugin:demo:first", - name: "Plugin First", - description: "Plugin optional verification", - prompt: "Run plugin verification", - category: "Quality", - icon: "plug", - phase: "post-merge", - }, - ], - ); - - expect(result.map((step) => step.templateId)).toEqual([ - "plugin:demo:first", - "browser-verification", + it("ignores a malformed (config-less) optional-group node without crashing", () => { + // A stale/partial optional-group node must not throw; it resolves to a + // defaultOn:false entry keyed by its id rather than breaking workflow loading. + const ir = v2([{ id: "og-bare", kind: "optional-group", column: "todo" }]); + expect(resolveWorkflowOptionalSteps(ir)).toEqual([ + { + templateId: "og-bare", + name: "og-bare", + description: "", + phase: "pre-merge", + defaultOn: false, + }, ]); - expect(result[0]).toMatchObject({ - name: "Plugin First", - icon: "plug", - phase: "post-merge", - defaultOn: true, - }); + }); + + it("does not resolve the built-in coding/stepwise workflows yet (legacy optionalSteps migrate in U6)", () => { + // U3 re-points the resolver SOURCE to optional-group nodes; the built-ins + // still carry the legacy `optionalSteps` declaration and gain optional-group + // nodes in U6. Until then they resolve to an empty toggle list. + expect(resolveWorkflowOptionalSteps(BUILTIN_CODING_WORKFLOW_IR)).toEqual([]); + expect(resolveWorkflowOptionalSteps(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).toEqual([]); + }); +}); + +describe("resolveDefaultOnOptionalGroupIds (task-creation seeding)", () => { + it("returns exactly the defaultOn:true group ids", () => { + const ir = v2([ + optionalGroupNode("og-off", { defaultOn: false }), + optionalGroupNode("og-on-a", { defaultOn: true }), + optionalGroupNode("og-on-b", { defaultOn: true }), + ]); + expect(resolveDefaultOnOptionalGroupIds(ir)).toEqual(["og-on-a", "og-on-b"]); + }); + + it("seeds an empty set when no optional-group has defaultOn (or none exist)", () => { + expect(resolveDefaultOnOptionalGroupIds(v2())).toEqual([]); + expect( + resolveDefaultOnOptionalGroupIds(v2([optionalGroupNode("og-off", { defaultOn: false })])), + ).toEqual([]); + expect(resolveDefaultOnOptionalGroupIds(v1)).toEqual([]); }); }); diff --git a/packages/core/src/__tests__/workflow-selection-store.test.ts b/packages/core/src/__tests__/workflow-selection-store.test.ts index 53d19e029f..f0d2735a39 100644 --- a/packages/core/src/__tests__/workflow-selection-store.test.ts +++ b/packages/core/src/__tests__/workflow-selection-store.test.ts @@ -176,6 +176,75 @@ describe("TaskStore workflow selection (U3)", () => { expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(wf.id); }); + // FNXC:WorkflowOptionalGroup 2026-06-21-14:30: a new task seeds + // `enabledWorkflowSteps` with exactly the defaultOn:true optional-group ids of + // its selected workflow (U3, R3), alongside the compiled workflow step ids. + describe("optional-group defaultOn seeding (U3/R3)", () => { + /** v2 workflow whose success path threads through two optional-group nodes. */ + function optionalGroupIr(): WorkflowIr { + const groupTemplate = (id: string) => ({ + nodes: [{ id: `${id}-inner`, kind: "prompt" as const, config: { prompt: "x" } }], + edges: [], + }); + return { + version: "v2", + name: "og-wf", + columns: [{ id: "todo", name: "Todo", traits: [] }], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { + id: "og-on", + kind: "optional-group", + column: "todo", + config: { name: "On Group", defaultOn: true, template: groupTemplate("og-on") }, + }, + { + id: "og-off", + kind: "optional-group", + column: "todo", + config: { name: "Off Group", defaultOn: false, template: groupTemplate("og-off") }, + }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [ + { from: "start", to: "og-on", condition: "success" }, + { from: "og-on", to: "og-off", condition: "success" }, + { from: "og-off", to: "end", condition: "success" }, + ], + }; + } + + it("seeds the defaultOn:true group id at creation from the default workflow", async () => { + const wf = await store.createWorkflowDefinition({ name: "OG Default", ir: optionalGroupIr() }); + await store.setDefaultWorkflowId(wf.id); + + const task = await store.createTask({ description: "seeded" }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toContain("og-on"); + expect(detail.enabledWorkflowSteps).not.toContain("og-off"); + }); + + it("seeds an empty set when the workflow has no optional groups", async () => { + const wf = await store.createWorkflowDefinition({ name: "No OG", ir: linearIr() }); + await store.setDefaultWorkflowId(wf.id); + + const task = await store.createTask({ description: "no groups" }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps ?? []).not.toContain("og-on"); + }); + + it("a stale optional-group id in enabledWorkflowSteps does not crash resolution", async () => { + // Group since removed from the workflow: the toggle resolver ignores the + // stale id rather than throwing, keeping create/edit surfaces alive. + const task = await store.createTask({ + description: "stale", + enabledWorkflowSteps: ["og-removed"], + }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toContain("og-removed"); + }); + }); + it("explicit enabledWorkflowSteps overrides the project default", async () => { const wf = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); await store.setDefaultWorkflowId(wf.id); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fdc55886ef..8d3f4cb8d2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -85,6 +85,7 @@ export type { WorkflowForeachConfig, WorkflowLoopConfig, WorkflowLoopExitCondition, + WorkflowOptionalGroupConfig, WorkflowIrArtifact, WorkflowFieldDefinition, WorkflowFieldType, @@ -118,7 +119,10 @@ export type { } from "./column-agent-resolver.js"; export { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; export { BUILTIN_MARKETING_WORKFLOW_IR } from "./builtin-marketing-workflow-ir.js"; -export { resolveWorkflowOptionalSteps } from "./workflow-optional-steps.js"; +export { + resolveWorkflowOptionalSteps, + resolveDefaultOnOptionalGroupIds, +} from "./workflow-optional-steps.js"; export type { ResolvedWorkflowOptionalStep } from "./workflow-optional-steps.js"; export { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; export { BUILTIN_PR_WORKFLOW_IR } from "./builtin-pr-workflow-ir.js"; diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 115bb82384..6acbb4a58f 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -78,6 +78,7 @@ import type { WorkflowNodeLayout, } from "./workflow-definition-types.js"; import { compileWorkflowToSteps, isInterpreterDeferredWorkflowCompileError } from "./workflow-compiler.js"; +import { resolveDefaultOnOptionalGroupIds } from "./workflow-optional-steps.js"; import { BUILTIN_WORKFLOWS, getBuiltinWorkflow, @@ -15798,11 +15799,17 @@ ${stepsSection}`; if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return undefined; throw err; } + // FNXC:WorkflowOptionalGroup 2026-06-21-14:20: seed `enabledWorkflowSteps` + // with the ids of `optional-group` nodes whose `defaultOn` is true, mirroring + // the prior `optionalStep.defaultOn ?? false` precedence (U3, R3). These group + // ids are NOT WorkflowStep rows — they are toggle keys the executor reads at + // the optional-group seam — so they ride alongside the compiled step ids. + const defaultGroupIds = resolveDefaultOnOptionalGroupIds(def.ir); if (isBuiltinWorkflowId(workflowId) && inputs.length === 0) { - return { workflowId, stepIds: [] }; + return { workflowId, stepIds: defaultGroupIds }; } const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); - return { workflowId, stepIds }; + return { workflowId, stepIds: [...stepIds, ...defaultGroupIds] }; } /** Resolve an EXPLICITLY requested workflow id (U6/R3/KTD-4) into materialized @@ -15823,11 +15830,15 @@ ${stepsSection}`; try { inputs = compileWorkflowToSteps(def.ir); } catch (err) { - if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return { workflowId, stepIds: [] }; + if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) + return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) }; throw err; } + // FNXC:WorkflowOptionalGroup 2026-06-21-14:20: same defaultOn-group seeding as + // the default-workflow path, for an explicitly requested create-time workflow. + const defaultGroupIds = resolveDefaultOnOptionalGroupIds(def.ir); const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); - return { workflowId, stepIds }; + return { workflowId, stepIds: [...stepIds, ...defaultGroupIds] }; } /** diff --git a/packages/core/src/workflow-optional-steps.ts b/packages/core/src/workflow-optional-steps.ts index 6b1c714ee2..600609b5bc 100644 --- a/packages/core/src/workflow-optional-steps.ts +++ b/packages/core/src/workflow-optional-steps.ts @@ -1,5 +1,9 @@ -import type { WorkflowIr } from "./workflow-ir-types.js"; -import { WORKFLOW_STEP_TEMPLATES, type WorkflowStepTemplate } from "./types.js"; +import type { + WorkflowIr, + WorkflowIrNode, + WorkflowOptionalGroupConfig, +} from "./workflow-ir-types.js"; +import type { WorkflowStepTemplate } from "./types.js"; export interface ResolvedWorkflowOptionalStep { templateId: string; @@ -10,37 +14,64 @@ export interface ResolvedWorkflowOptionalStep { defaultOn: boolean; } +/* +FNXC:WorkflowOptionalGroup 2026-06-21-14:05: +Re-pointed the per-task optional-step toggle SOURCE from the execution-inert `ir.optionalSteps` declaration to v2 `optional-group` NODES (one resolved entry per group). The legacy `WorkflowOptionalStep`/`optionalSteps` type stays in place for now — only the resolution + seeding source moved here (U3); the type removal is a later unit (U7). +KEYING: the resolved entry is keyed by the group node `id`. The output field is still named `templateId` (not renamed) so the four consuming UI surfaces — inline quick-create card, New Task modal/TaskForm, task-detail Workflow tab, and the optional-steps dropdown — keep reading the same shape unchanged; they now toggle group ids into `enabledWorkflowSteps` instead of template ids. Renaming/recreating a group resets per-task state, identical to the prior `templateId` keying. +Display metadata: `name` comes from `config.name` (falling back to the node id), `defaultOn` from `config.defaultOn ?? false`. The group node carries no description/icon/phase, so `description` is "" and `phase` defaults to "pre-merge" — keeping every field the consumers read populated and non-blank. +*/ + +function isOptionalGroupNode( + node: WorkflowIrNode, +): node is WorkflowIrNode & { config: WorkflowOptionalGroupConfig } { + return node.kind === "optional-group"; +} + /** - * Resolve workflow-declared optional step template ids into display metadata. + * Resolve a workflow's `optional-group` nodes into per-task toggle display + * metadata. Each enabled group's node id is what a task stores in + * `enabledWorkflowSteps`; this resolver advertises which groups a task may + * toggle plus their seed default. * - * The declaration is intentionally execution-inert: it only advertises which - * template-backed workflow steps a task may toggle into `enabledWorkflowSteps`. - * Unknown template ids are skipped so stale/custom declarations never render - * blank UI rows or break workflow loading. + * Source: v2 `ir.nodes` where `kind === "optional-group"` (NOT the legacy + * `ir.optionalSteps` declaration). Non-v2 graphs and graphs without any + * optional-group node resolve to `[]`. Malformed group configs are skipped so a + * stale/partial node never renders a blank UI row or breaks workflow loading. + * + * `pluginTemplates` is accepted for signature compatibility with the prior + * template-backed resolver; group nodes are self-describing, so it is currently + * unused. */ export function resolveWorkflowOptionalSteps( ir: WorkflowIr, - pluginTemplates: WorkflowStepTemplate[] = [], + _pluginTemplates: WorkflowStepTemplate[] = [], ): ResolvedWorkflowOptionalStep[] { - if (ir.version !== "v2" || !ir.optionalSteps?.length) return []; - - const templates = new Map(); - for (const template of [...WORKFLOW_STEP_TEMPLATES, ...pluginTemplates]) { - templates.set(template.id, template); - } + if (ir.version !== "v2" || !Array.isArray(ir.nodes)) return []; const resolved: ResolvedWorkflowOptionalStep[] = []; - for (const optionalStep of ir.optionalSteps) { - const template = templates.get(optionalStep.templateId); - if (!template) continue; + for (const node of ir.nodes) { + if (!isOptionalGroupNode(node)) continue; + const config = (node.config ?? {}) as Partial; resolved.push({ - templateId: optionalStep.templateId, - name: template.name, - description: template.description, - icon: template.icon, - phase: template.phase ?? "pre-merge", - defaultOn: optionalStep.defaultOn ?? template.defaultOn ?? false, + // Keyed by the group node id (documented above); field name preserved. + templateId: node.id, + name: typeof config.name === "string" && config.name.trim() ? config.name : node.id, + description: "", + phase: "pre-merge", + defaultOn: config.defaultOn === true, }); } return resolved; } + +/** + * Ids of `optional-group` nodes whose effective `defaultOn` is true. Used to + * seed a new task's `enabledWorkflowSteps` at creation, mirroring the prior + * `optionalStep.defaultOn ?? false` precedence (U3, R3). Defensive: non-v2 + * graphs and graphs without optional groups yield `[]`. + */ +export function resolveDefaultOnOptionalGroupIds(ir: WorkflowIr): string[] { + return resolveWorkflowOptionalSteps(ir) + .filter((step) => step.defaultOn) + .map((step) => step.templateId); +}