diff --git a/.changeset/graph-custom-workflows.md b/.changeset/graph-custom-workflows.md index 360628d5e2..5dc3007516 100644 --- a/.changeset/graph-custom-workflows.md +++ b/.changeset/graph-custom-workflows.md @@ -8,6 +8,12 @@ Prompt nodes carry an execution profile: run on a chosen model, as a named agent CLI nodes can run arbitrary commands (not just named scripts); the first run of an exact command pauses the task for explicit user approval. The task modal's input/approval banner is interactive — reply-and-resume for user-input nodes, approve-and-run for CLI commands. -Agents reach workflows too: new `fn_workflow_list` and `fn_workflow_select` task tools give agents the same list/select capability as the dashboard picker. Built-in workflows are now read-only in the editor (palette/inspector disabled, with a "Duplicate to edit" action), and a node's "Auto-approve requests" toggle now actually bypasses the CLI first-run approval pause. +Agents reach workflows too: the `fn_workflow_list`, `fn_workflow_get`, `fn_workflow_select`, `fn_workflow_create`, `fn_workflow_update`, and `fn_workflow_delete` tools (plus `fn_trait_list` for the column vocabulary) give agents the same author/list/select capability as the dashboard. These are exposed not only to the task executor but also to the chat and planning agents, so you can author and edit workflows directly in a chat or planning conversation; a guard test locks all six tool names to each lane to prevent silent exposure drift. Built-in workflows are now read-only in the editor (palette/inspector disabled, with a "Duplicate to edit" action), and a node's "Auto-approve requests" toggle now actually bypasses the CLI first-run approval pause. Also fixes a latent persistence bug where `pausedReason` was written to the in-memory task and read by queries but never stored by the task upsert or mapped back on read — so it was lost on every reload. This silently broke any pause/resume that depends on the reason (workflow CLI-approval and await-input nodes, token-budget pauses, worktrunk failures). The approve-CLI endpoint now derives the approved command solely from the task's pausedReason (ignoring any caller-supplied command), await-input nodes only resume when this node actually paused the task (not on a pre-existing steering comment), and write-capable custom nodes are refused until a task worktree exists so they never mutate the shared repo root. + +The editor itself got a major usability upgrade: card-style nodes with kind accents and live config summaries (model/agent/skill/command, gate mode, hold release, join mode); success/failure edge authoring on regular edges with distinct styling, parallel conditioned edges, and an author-time cycle guard; one-click auto-layout that respects column swimlanes; safe node/edge deletion with cascade semantics; proper dialogs (create/delete/discard) with inline rename, descriptions, and a dirty-state guard on every dismissal path; onboarding/empty states; and the Columns and Fields panels now live in the editor's left sidebar under the workflow list. + +The node editor is now the primary workflow surface: the header and mobile nav open it directly and the legacy Workflow Steps screen is retired. Existing flat steps migrate automatically (and idempotently) on first editor open — every step becomes an insertable template fragment in the new palette Templates section (alongside built-in and plugin step templates), and your default-on steps become a "Migrated steps" workflow that's set as the project default. Task creation now picks a workflow (applied atomically at create) instead of individual step checkboxes. + +Workflows and template fragments import/export as JSON files — with server-side validation, name-collision handling, and automatic stripping of approval-bypass flags from untrusted files. And you can ask AI to design a workflow: describe what you want in the create dialog (or redesign the active workflow from the toolbar) and a planning-lane model emits a validated graph, with interpreter-only branching flagged honestly. diff --git a/CONCEPTS.md b/CONCEPTS.md index 528e6a62de..7b1e22c730 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -13,6 +13,9 @@ User-level settings persisted server-side that apply across all Surfaces and all ### Three-Tier Setting The named persistence pattern for a user preference on the dashboard: a device-local cache for instant reads, a write-through to Global Settings so other Surfaces see it, and a hydrate-on-mount from the server when no local value exists. A local or in-flight user choice always wins over server hydration, and changes propagate to other open tabs. +### Translation Placeholder +An empty-string value for a catalog key in a non-English locale, marking "not yet translated." Placeholders are intentionally backfilled when keys are added; at runtime they are treated as missing (never rendered), falling back through the locale chain to English. A non-empty value — even an English one left in a non-en catalog — is rendered as-is. + ### Supported Locale A language tag in the closed set Fusion ships translations for. Any external tag (browser, environment, flag) is normalized into this set or rejected — never passed through raw. Chinese tags route by script and region so Traditional-script users are never silently served Simplified, and the two Chinese variants never collapse into a generic base tag. diff --git a/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md b/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md new file mode 100644 index 0000000000..cd854dec04 --- /dev/null +++ b/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md @@ -0,0 +1,267 @@ +--- +title: "feat: Node editor visual redesign + success/failure edge authoring" +type: feat +status: completed +date: 2026-06-04 +depth: standard +origin: none (solo planning bootstrap) +--- + +# feat: Node editor visual redesign + success/failure edge authoring + +## Summary + +Upgrade the workflow node editor's authoring experience: redesign graph nodes from small icon+label pills into larger card-style nodes with kind accent colors and config summaries; generalize edge-condition authoring so success/failure is selectable on regular edges (today only step-review edges are editable) with distinct visual styling; and round out editor power/polish — safe node/edge deletion, proper dialogs replacing `window.prompt`/`window.confirm`, inline rename/description, dirty-state guard, auto-layout, and a real empty/onboarding state. UI/authoring layer only — no engine, IR-schema, or compiler-semantics changes. + +--- + +## Problem Frame + +The editor (`packages/dashboard/app/components/WorkflowNodeEditor.tsx`, built on `@xyflow/react`) has grown to 13 editor node kinds with swimlane columns and an edge inspector, but the authoring surface lags the capability underneath: + +- **Nodes are unreadable at a glance.** `NodeShell` (`packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx`) renders icon + label + tiny badges. A prompt node configured with a model, an agent, or a CLI command looks identical to an unconfigured one; users must click every node to see what it does. +- **Failure edges exist everywhere except the editor.** The IR accepts any `edge.condition` (`parseWorkflowIr` never validates condition values), and the graph executor natively traverses `failure` edges (`shouldTraverseEdge`, `packages/engine/src/workflow-graph-executor.ts:385-392`). But `onConnect` hardcodes every new edge to `success`, and the edge inspector only offers condition controls when the source node is `step-review`. There is no way to author the branching the engine already supports. +- **Authoring chrome is crude.** `window.prompt` for workflow names, `window.confirm` for deletes, no keyboard deletion, no dirty tracking (switching workflows silently discards edits), no auto-layout, and a bare "Select or create a workflow" empty state. + +--- + +## Scope Boundaries + +### In scope +- Card-style node redesign with config summaries and kind accent colors. +- Success/failure edge-condition authoring on regular edges, with distinct edge styling and an honest "interpreter-only" presentation when branching makes the graph non-compilable to the linear step engine. +- Deletion UX (keyboard + buttons) with explicit cascade semantics. +- Dialogs, inline rename/description, dirty-state guard, auto-layout, empty/onboarding state. + +### Deferred to Follow-Up Work +- Undo/redo history for the canvas. +- Workflow import/export, versioning, templates gallery. +- Localizing edge condition labels (kept as canonical IR tokens — see KTD-8). +- Auto-layout inside `foreach` template groups beyond the existing seeded row. + +### Outside this product's identity +- Changing edge/branching **execution** semantics. The graph interpreter, `parseWorkflowIr` graph validation, and the linear-step compiler keep their current behavior; this plan only lets users author what they already support and presents their limits honestly. + +--- + +## Requirements + +**Visual** +- R1 — Graph nodes render as card-style nodes: kind accent color, icon, label, and a config-summary line (model/agent/skill/CLI for prompt nodes; script name; gate mode; hold release; join mode; parser; review type), with a defined header-overflow priority and truncation; existing badges and error badges preserved. +- R2 — Success, failure, and rework edges are distinguishable by at least two independent visual channels: the condition label is always rendered, and failure edges use a distinct dash pattern from success edges; color (token-only, both themes) is a third channel, never the only one. + +**Edge authoring** +- R3 — A user can set a regular edge's condition to `success` or `failure` from the edge inspector via a native `` gated per KTD-2 inside the existing disabled fieldset; compile-banner suffix match + info tone (KTD-4); `interactionWidth` on edges for a forgiving hit target (touch + pointer). +- `packages/dashboard/app/components/workflow-flow-mapping.ts` (modify) — edge `className` for failure edges in `irEdgeToFlow`; always-rendered condition labels; dash styling hooks; ancestor-reachability helper for the cycle guard. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify) — `.wf-edge-failure` (distinct dash pattern + `--ws-error`-derived stroke), success default styling, info-tone banner. +- `packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts` (extend) — mapping-level edge tests. +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend) — inspector gating tests. +**Approach:** Edge-level behavior is tested at the mapping layer (React Flow doesn't render edges under jsdom). The inspector reuses `updateSelectedEdge` unchanged — only the rendering gate widens, and the condition control is a native ``; persistent inline error region for validation failures; toast only for network errors; input resets after any attempt; notice when approval flags were stripped). +- `packages/dashboard/app/api/legacy.ts` (modify) — client fns. +- `packages/dashboard/src/routes/__tests__/workflow-import-export.test.ts` (new), `__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Approach:** Envelope per HTD. Server is the sole validator. Export reads the persisted definition (dirty-guard makes stale export impossible). +**Test scenarios:** +- Covers R9: export → import reproduces ir/layout/description semantically; fragment kind preserved; export blocked while dirty (button disabled). +- Covers R10: name collision → suffixed; builtin export → fresh non-builtin editable ID; missing envelope marker → 400; malformed IR → 422 + parser message, zero writes; unknown trait → 422 naming the trait; `schemaVersion` older → accepted; newer → 409 version message; CLI node with `cliSkipApproval: true` → persisted node lacks the field and the response flags the strip; script node with unknown scriptName → 200 + warning field. +- Editor: validation failure renders the inline error (not a toast) and the list is unchanged; success refreshes + activates; strip notice shown when flagged. + +### U6. Workflow-centric TaskForm + create-time workflowId + +**Goal:** Tasks pick a workflow, applied atomically at creation. +**Requirements:** R3. +**Dependencies:** U1 (fragment exclusion). (U2's migrated workflow appears automatically once present — runtime ordering, not a build dependency.) +**Files:** +- `packages/core/src/types.ts` + `packages/core/src/store.ts` (modify) — `workflowId?: string` on the task-create input; materialization inside the creation transaction mirroring the default-workflow block (`materializeDefaultWorkflowSteps`/`pendingWorkflowSelection`/`writeTaskWorkflowSelection`, store.ts ~3974-4035); explicit `workflowId` overrides the project default; fragment IDs rejected. +- `packages/dashboard/src/routes` task-create route (modify) — accept + pass `workflowId`. +- `packages/dashboard/app/components/TaskForm.tsx` (modify) — replace the per-step checkbox section (~280, ~330-339, ~1331-1337) with the workflow dropdown (states per R3); remove `fetchWorkflowSteps` usage; empty-workflow-list CTA into the editor. +- `packages/dashboard/app/components/__tests__/` TaskForm tests (extend), `packages/core/src/__tests__/` task-create tests (extend). +**Approach:** The engine path is untouched — materialization writes `enabledWorkflowSteps` exactly as the default-workflow path does, in the same transaction, so no executor-pickup race exists. `selectTaskWorkflow` remains the post-create path only. +**Test scenarios:** +- Covers R3: create with `workflowId` → task's `enabledWorkflowSteps` populated within the creation write (no intermediate empty state observable); explicit pick overrides project default; "No workflow" → no custom steps; fragment ID → rejected. +- Dropdown: loading placeholder; "(default)" badge on the project default; "No workflow" listed first; fragments absent; built-ins present. +- Empty project → CTA opens the editor. +- Regression: per-step checkboxes gone; no `fetchWorkflowSteps` call remains in TaskForm. + +### U7. AI design route (server) + +**Goal:** Prompt → validated, stripped, rate-limited IR. +**Requirements:** R11 (server half). +**Dependencies:** U1. +**Files:** +- `packages/dashboard/src/routes/register-workflow-routes.ts` (modify) — `POST /api/workflows/design` `{prompt, workflowId?}` per KTD-6: module-level `__setCreateFnAgentForDesign` DI seam co-located with the route; planning-lane model; tool-less; JSON-from-text extraction via the existing helper (planning/agent-generation precedent); `parseWorkflowIr` + compile triage (`interpreterOnly` flag) + approval-flag stripping; `workflowId` read from the store (client never posts IR); rate limit 10/hour mirroring `/ai/refine-text`; bounded prompt length. +- `packages/dashboard/src/routes/__tests__/workflow-design-route.test.ts` (new). +**Execution note:** route tests use the DI seam with a fake agent — no real model calls. +**Test scenarios:** +- Covers R11: fake agent returns valid linear IR → 200 `{ir, interpreterOnly:false}`; branching IR → 200 `{interpreterOnly:true}`; fenced/prose-wrapped JSON → still extracted and 200; invalid JSON / IR failing `parseWorkflowIr` → 422 + message, nothing persisted; IR containing `cliSkipApproval` → returned IR lacks it + strip flag set. +- `workflowId` flow: route reads the persisted IR; unknown ID → 404. +- Rate limit: 11th call within the window → 429. +- Over-length prompt → 400. + +### U8. Fragment insertion + graph-copy helpers (mapping layer) + +**Goal:** Pure, tested primitives for inserting fragments and copying graphs. +**Requirements:** R8 (helper half), R7 (copy helpers). +**Dependencies:** U1. +**Files:** +- `packages/dashboard/app/components/workflow-flow-mapping.ts` (modify) — `insertFragment(nodes, edges, fragmentIr, position)` (strips start/end, remaps all node IDs to fresh `newNodeId`s, rewires internal edges), `fragmentSeamConflicts(fragmentIr, nodes)`, `copyIrWithFreshIds(ir, layout)`. +- `packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts` (extend). +**Approach:** Pure-helper-first so jsdom limits don't bite; consumed by U4 (copies) and U9 (palette insertion). +**Test scenarios:** +- Covers R8: `insertFragment` remaps every node ID (no collisions), strips start/end, preserves internal edges/config; double-insert → disjoint ID sets. +- Fragment containing a `merge` seam vs a graph that has one → `fragmentSeamConflicts` flags it. +- `copyIrWithFreshIds` → same structure, all-new IDs, layout keys remapped consistently. + +### U9. Palette Templates section (editor) + +**Goal:** The template library is insertable from the palette. +**Requirements:** R8. +**Dependencies:** U1, U8. (U2's fragments appear once migrated — runtime ordering.) +**Files:** +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` (modify) — Templates palette section: Fragments / Built-in steps / Plugin steps subsections (alphabetical; filter input when combined > 8; plugin owner badges); entries keyboard-activatable (Enter/Space) with descriptive aria-labels; fragment insertion via U8 with the persistent inline conflict error in the section; preset step nodes via the converter field mapping; section collapsed state persisted. +- `packages/dashboard/app/api/legacy.ts` (modify) — fragments fetch (kind param); reuse existing `fetchWorkflowStepTemplates`/`fetchPluginWorkflowStepTemplates`. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify). +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Test scenarios:** +- Covers R8: three subsections render with their sources; plugin entry carries owner badge; inserting a step template adds a node with prefilled config; inserting a fragment with a seam conflict → inline error, no insertion; filter input appears above 8 combined entries and filters across groups. +- Empty fragment library → Fragments subsection hidden. +- Builtin active → insertion disabled (read-only gating). +- Keyboard activation inserts (a11y). + +### U10. Design-with-AI editor affordances + +**Goal:** Prompt-to-workflow UX in both entry points. +**Requirements:** R11 (client half). +**Dependencies:** U7. +**Files:** +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` (modify) — create dialog: an optional "Describe it instead" textarea (placeholder with an example prompt) before template selection — submitting designs a new workflow from the result; toolbar: "Design with AI" opens a popover panel (textarea + submit) targeting the active workflow via `workflowId`; proposed replacement applies only through the dirty-guard confirm. In-flight: control disabled + spinner + `aria-busy`, client-side cancel (abort the fetch); failure → server message inline, canvas untouched; `interpreterOnly` → existing info banner on the seeded graph; strip notice when flagged. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify). +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Test scenarios:** +- Covers R11: mocked design success in create dialog → new workflow seeded from returned IR; toolbar flow over a dirty canvas → discard confirm first, cancel keeps edits. +- Mocked 422 → inline error, canvas untouched. +- In-flight state: control disabled, aria-busy set; cancel aborts and re-enables. +- interpreterOnly result → info banner visible. + +### U11. Agent-lane exposure (engine) + +**Goal:** Chat and planning agents can author workflows; drift-guarded. +**Requirements:** R12. +**Dependencies:** none (independent; lands first safely). +**Files:** +- `packages/dashboard/src/chat.ts` (modify, ~1288/1612) — pass `fn_workflow_*` factories via `createFnAgent`'s `customTools` (chat passes none today — introduce the array) with the scoped store. +- `packages/dashboard/src/planning.ts` (modify, ~842) — append the workflow tool factories to the existing `customTools: [...createPlanningBoardTools(store)]`. +- `packages/engine/src/__tests__/agent-workflow-tools-exposure.test.ts` (new) — asserts all six names (`fn_workflow_create/update/delete/list/get/select`) per lane: executor, chat, planning. +- Touched tool handlers (verify) — defensive arg parsing (string-JSON accepted). +**Approach:** Grep every `fn_workflow_` registration surface first and mirror all hits (drift learning). +**Test scenarios:** +- Covers R12: exposure test enumerates executor + chat + planning toolsets and asserts `fn_workflow_create/update/delete/list/get/select` membership in each; fails when any lane loses one. +- Chat lane: customTools array introduced without disturbing existing chat tool behavior (existing chat tests stay green). +- A workflow tool invoked with stringified-JSON args still parses. + +--- + +## Risks & Dependencies + +- **Migration writes to user DBs.** Additive only (2 columns, new rows, marker stamps, one project-default settings write); `transactionImmediate`; idempotent by stored marker; nothing deleted or rewritten. +- **defaultOn policy is a behavior interpretation.** Mapping defaultOn → combined-workflow-as-project-default preserves "new tasks run these" but collapses per-task uncheckability into workflow choice + fragments. Named in release notes; the migration test suite pins the policy. +- **Round-trip fidelity gaps.** Parity covers compiler-visible fields only — by design; `enabled`/`defaultOn`/`templateId` are policy-handled. Extend parity when `nodeToStepInput` gains fields (comments at both sites). +- **Trust boundary.** `cliSkipApproval`/`autoApprove` bypass the CLI approval gate; import and design strip them (R10/R11). Systemic schema-level rejection is an explicit follow-up. Exported files contain full prompt/command text — disclosure note on the export affordance. +- **Removal blast radius.** U3's Surface Enumeration is the sweep list; U3 is gated on U6 to avoid the no-surface window. +- **Import strictness vs. portability.** Unknown traits block (422 naming trait + owning plugin); unknown scriptNames warn without blocking — scripts are project-settings content the user can add after import. +- **AI output variance.** JSON extraction + server validation bound the failure mode to a clean 422; retry/repair deferred. Synchronous route bounded by rate limit + prompt-length cap; detached-turn upgrade documented. +- **Registration drift** (agent tools, palette template sources): grep-and-mirror; U11 guard test. +- **Mid-migration TaskForm state.** A user can open TaskForm before ever opening the editor — they see built-ins (+ any existing workflows) until migration runs on first editor open; acceptable, noted here so it isn't mistaken for a bug. +- **Changeset:** user-facing feature in the bundled CLI → `@runfusion/fusion` minor changeset. + +--- + +## System-Wide Impact + +- **Schema:** SCHEMA_VERSION 108→109 (two additive columns). +- **Engine:** no execution-semantics change; chat/planning lanes gain workflow tools (additive). +- **Existing users:** flat steps keep executing on existing tasks; the step-authoring UI is replaced by migrated workflows/fragments; defaultOn behavior is preserved via the migrated project default; TaskForm visibly changes (workflow picker) — release-notes worthy, plus the in-editor one-time migration notice. +- **Plugins:** contributed step templates move to the editor palette; plugin API unchanged. + +--- + +## Sources & Research + +- `packages/dashboard/app/components/WorkflowStepManager.tsx` (surface inventory: form fields ~718-963, templates tab + plugin templates ~185-200, refine ~830-844, onOpenGraphEditor ~430). +- `packages/dashboard/app/components/Header.tsx` (~1601, ~1947), `MobileNavBar.tsx` (~592), `useModalManager.ts` (~180, ~199, ~348-351), `AppModals.tsx` (~377-400), `TaskForm.tsx` (~242, ~280, ~330-339, ~1331-1337). +- `packages/core/src/workflow-compiler.ts` (`compileWorkflowToSteps` ~200, `nodeToStepInput` ~162-189 — emits neither `enabled` nor `defaultOn`), `builtin-workflows.ts` (`linear()` ~25-44), `store.ts` (`createWorkflowDefinition` ~12238 — fixed INSERT, no name uniqueness; `listWorkflowDefinitions` ~12289 — single unconditional cache; `selectTaskWorkflow` ~13266 — requires task id; default-workflow materialization ~3974-4035; `materializeWorkflowSteps` ~13230; `transactionImmediate` precedent), `db.ts` (`SCHEMA_VERSION = 108` ~152, `addColumnIfMissing` ~3795), `types.ts` (`WorkflowStep` ~510-548 — `enabled` required, `defaultOn` optional; `WORKFLOW_STEP_TEMPLATES` ~772; task-create input carries only `enabledWorkflowSteps`). +- `packages/dashboard/src/routes.ts` (refine route + `__setCreateFnAgentForRefine` ~370, ~3019-3092 — free-text accumulation, no JSON extraction; rate limits on `/ai/refine-text` ~1717), `register-workflow-routes.ts`; JSON-from-text extraction precedent in `planning.ts`/agent-generation. +- `packages/engine/src/agent-tools.ts` (`fn_workflow_*` ~1007-1365), `executor.ts` (~5687-5694 toolset; `cliSkipApproval`/`autoApprove` gate ~4576-4581), `packages/dashboard/src/chat.ts` (`tools: "coding"`, no customTools ~1288/1612), `planning.ts` (`customTools` ~842). +- Import/export precedent: `SettingsModal.tsx` (~1625-1671, ~7662), `register-agent-import-export-generation-routes.ts`, `AgentImportModal.tsx` (~246-257, ~495-507). +- Learnings: `docs/solutions/integration-issues/bundled-plugin-registration-drift.md`, `docs/solutions/integration-issues/branch-group-single-pr-synthetic-id-dead-wiring.md`, `docs/solutions/architecture-patterns/observable-long-running-agent-turns-through-blocking-plugin-route-seam.md`, `docs/solutions/architecture-patterns/mass-migration-agent-fleet-orchestration.md`. +- Prior plans: `docs/plans/2026-06-03-001-feat-executable-custom-workflows-node-editor-plan.md`, `docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md`. diff --git a/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md b/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md new file mode 100644 index 0000000000..ecd436d16e --- /dev/null +++ b/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md @@ -0,0 +1,65 @@ +--- +title: Empty-string locale placeholders render blank UI (i18next returnEmptyString default) +date: 2026-06-05 +category: ui-bugs +module: i18n +problem_type: ui_bug +component: frontend +symptoms: + - "Buttons, labels, and dialog copy render completely blank in non-English locales" + - "Inline English defaults passed to t(\"key\", \"Default\") are ignored — blank wins" + - "No console errors; en locale looks perfect; only translated locales affected" +root_cause: wrong_api +resolution_type: config_fix +severity: high +related_components: + - dashboard + - tooling +tags: [i18next, returnEmptyString, locale-placeholders, translation-fallback, i18n-extract, catalog-pruning] +--- + +# Empty-string locale placeholders render blank UI (i18next returnEmptyString default) + +## Problem + +The repo's translator workflow backfills `""` placeholders into non-en catalogs for untranslated keys, on the assumption that empty values fall back to English at runtime. They don't: i18next's default `returnEmptyString: true` treats `""` as a *found* value, so es/fr/ko/zh users saw blank buttons, nav labels, and dialog copy for every new key — even though components pass inline English defaults (`t("key", "Default")`). + +## Symptoms + +- New UI strings render blank in any non-English locale while en looks correct. +- The inline second-argument default to `t()` does not rescue it — `""` short-circuits the fallback chain entirely. +- Verified empirically (i18next 26.x): `t("empty", "InlineDefault")` returns `""` when the active locale defines the key as `""`. + +## What Didn't Work + +- Assuming the standing convention was safe because hundreds of `""` placeholders pre-existed — the convention had been silently rendering blanks all along for any key reached in a non-en locale. +- Relying on inline `t()` defaults as a safety net — they only apply when the key is *missing*, not empty. + +## Solution + +One config line in the shared i18next init (`packages/i18n/src/config.ts`, `baseInitOptions()`): + +```ts +returnEmptyString: false, +``` + +With this set, `""` values are treated as missing and fall through the fallback chain (`fallbackLng` → en, or the inline default). Locale files stay untouched, the `""`-placeholder translator convention keeps working, and every existing empty placeholder is fixed at once. + +Empirical check that settles the question in seconds (run against `node_modules` i18next, initialized like the app): + +```js +// lng: "fr", resources: { fr: { empty: "" }, en: { empty: "EnglishValue" } } +t("empty", "InlineDefault") +// returnEmptyString true (default) → "" ← blank UI +// returnEmptyString false → "EnglishValue" +``` + +## Why This Works + +i18next resolution asks "does the key exist with a usable value?" — `returnEmptyString` defines whether `""` is usable. The default (`true`) is meant for apps where empty is a legitimate translation; in a placeholder-backfill workflow it's exactly wrong, because every placeholder is an intentional "not translated yet" marker. + +## Prevention + +- When adopting any `""`-placeholder catalog convention, set `returnEmptyString: false` in the same commit — the two are a package deal. +- Don't trust the inline-`t()`-default mental model; prove fallback behavior with a 5-line init script before relying on it. +- **Related catalog trap (hit twice in the same PR):** `pnpm i18n:extract` prunes keys whose usages it cannot see (CLI/TUI surfaces, dynamic keys) — it deleted live keys like `taskFields.*` and `common.cancel` from `en/app.json`. After running extract, semantically diff catalogs against the base ref (flatten both JSONs, assert zero removed/changed keys vs upstream, only intended additions) before committing. The content sanity test `packages/i18n/src/__tests__/config.test.ts` ("has real en content") exists because of this; prefer hand-adding keys + `i18n:sync`/`i18n:types` over trusting `i18n:extract` output wholesale. diff --git a/packages/cli/skill/fusion/references/engine-tools.md b/packages/cli/skill/fusion/references/engine-tools.md index 79e3dae523..07e40b23cd 100644 --- a/packages/cli/skill/fusion/references/engine-tools.md +++ b/packages/cli/skill/fusion/references/engine-tools.md @@ -15,14 +15,14 @@ These tools are **not** part of the user-invokable extension surface. They are i | `fn_task_log` | executor, heartbeat | Write significant task log entries | `message` (string), `outcome?` (string) | | `fn_task_document_write` | triage, executor, heartbeat | Save/update a named task document revision | `key` (string), `content` (string), `author?` (string) | | `fn_task_document_read` | triage, executor, heartbeat | Read one task document or list all | `key?` (string) | -| `fn_workflow_list` | executor | List the project's custom workflows (read-only built-ins plus user definitions) | none | -| `fn_workflow_get` | executor | Fetch one workflow definition by id — name, description, builtin flag, and the full IR (nodes/edges/columns/artifacts/fields) as JSON | `workflow_id` (string) | -| `fn_workflow_select` | executor | Assign a custom workflow to a task (defaults to the current task) | `workflow_id` (string), `task_id?` (string) | -| `fn_workflow_create` | executor | Create a custom workflow definition from a graph IR (validated server-side). v2 IR supports step-inversion constructs: `parse-steps`, `foreach` (mode/isolation/concurrency/maxReworkCycles), `step-execute`, `step-review`, `code` nodes, `rework` edges, plus `artifacts` and custom `fields` declarations | `name` (string), `description?` (string), `ir` (object), `layout?` (object) | -| `fn_workflow_update` | executor | Update a custom workflow definition's name/description/ir/layout (built-ins cannot be edited; same step-inversion IR constructs as create; editing `fields` orphans rather than destroys existing task values) | `workflow_id` (string), `name?` (string), `description?` (string), `ir?` (object), `layout?` (object), `rehome_to?` (string) | -| `fn_workflow_delete` | executor | Delete a custom workflow definition (built-ins cannot be deleted); selecting tasks are re-homed to the default workflow's entry column | `workflow_id` (string) | +| `fn_workflow_list` | executor, chat, planning | List the project's custom workflows (read-only built-ins plus user definitions) | none | +| `fn_workflow_get` | executor, chat, planning | Fetch one workflow definition by id — name, description, builtin flag, and the full IR (nodes/edges/columns/artifacts/fields) as JSON | `workflow_id` (string) | +| `fn_workflow_select` | executor, chat, planning | Assign a custom workflow to a task (defaults to the current task) | `workflow_id` (string), `task_id?` (string) | +| `fn_workflow_create` | executor, chat, planning | Create a custom workflow definition from a graph IR (validated server-side). v2 IR supports step-inversion constructs: `parse-steps`, `foreach` (mode/isolation/concurrency/maxReworkCycles), `step-execute`, `step-review`, `code` nodes, `rework` edges, plus `artifacts` and custom `fields` declarations | `name` (string), `description?` (string), `ir` (object), `layout?` (object) | +| `fn_workflow_update` | executor, chat, planning | Update a custom workflow definition's name/description/ir/layout (built-ins cannot be edited; same step-inversion IR constructs as create; editing `fields` orphans rather than destroys existing task values) | `workflow_id` (string), `name?` (string), `description?` (string), `ir?` (object), `layout?` (object), `rehome_to?` (string) | +| `fn_workflow_delete` | executor, chat, planning | Delete a custom workflow definition (built-ins cannot be deleted); selecting tasks are re-homed to the default workflow's entry column | `workflow_id` (string) | | `fn_task_promote` | executor | Promote a held task out of a manual-release hold column (defaults to the current task) | `task_id?` (string) | -| `fn_trait_list` | executor | List the registered column trait catalog (built-in and plugin traits) | none | +| `fn_trait_list` | executor, chat, planning | List the registered column trait catalog (built-in and plugin traits) | none | | `fn_memory_search` | triage, executor, heartbeat | Search project memory plus per-agent layered memory snippets | `query` (string), `limit?` (number) | | `fn_memory_get` | triage, executor, heartbeat | Read a bounded memory file window (including bounded per-agent layered paths) | `path` (string), `startLine?` (number), `lineCount?` (number) | | `fn_memory_append` | executor, heartbeat (when writable backend enabled) | Append memory notes with explicit scope: `scope="agent"` for private operating context, `scope="project"` for workspace-wide durable knowledge | `scope?` (`project` \| `agent`), `layer` (`long-term` \| `daily`), `content` (string) | diff --git a/packages/core/src/__tests__/db-migrate.test.ts b/packages/core/src/__tests__/db-migrate.test.ts index b36c04e128..d38e55c9eb 100644 --- a/packages/core/src/__tests__/db-migrate.test.ts +++ b/packages/core/src/__tests__/db-migrate.test.ts @@ -715,7 +715,7 @@ describe("schema migration", () => { const row = db.prepare("SELECT deletedAt FROM tasks WHERE id = 'FN-legacy'").get() as { deletedAt: string | null }; expect(row.deletedAt).toBeNull(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -748,7 +748,7 @@ describe("schema migration", () => { { id: "WS-001", mode: "prompt", gateMode: "advisory" }, { id: "WS-002", mode: "script", gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -798,7 +798,7 @@ describe("schema migration", () => { reviewerContextRetryCount: 0, reviewerFallbackRetryCount: 0, }); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -827,7 +827,7 @@ describe("schema migration", () => { const columns = db.prepare("PRAGMA table_info(milestones)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("acceptanceCriteria"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -868,7 +868,7 @@ describe("schema migration", () => { const missionColumns = db.prepare("PRAGMA table_info(missions)").all() as Array<{ name: string }>; expect(missionColumns.map((column) => column.name)).toContain("autoMerge"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -902,7 +902,7 @@ describe("schema migration", () => { { id: "WS-002", mode: "script", enabled: 1, gateMode: "advisory" }, { id: "WS-003", mode: "prompt", enabled: 0, gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -939,7 +939,7 @@ describe("schema migration", () => { const indexes = db.prepare("PRAGMA index_list(mission_goals)").all() as Array<{ name: string }>; expect(indexes.some((index) => index.name === "idxMissionGoalsGoalId")).toBe(true); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -1000,7 +1000,81 @@ describe("schema migration", () => { expect(customFieldsColumn).toBeDefined(); expect(customFieldsColumn?.dflt_value).toBe("'{}'"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); + + it("adds workflows.kind + workflow_steps.migrated_fragment_id when migrating from schema version 108", () => { + const db = new Database(fusionDir); + db.exec("CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT)"); + db.exec("INSERT INTO __meta (key, value) VALUES ('schemaVersion', '108')"); + db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')"); + db.exec(` + CREATE TABLE IF NOT EXISTS workflows ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + ir TEXT NOT NULL, + layout TEXT NOT NULL DEFAULT '{}', + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + db.exec(` + CREATE TABLE IF NOT EXISTS workflow_steps ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + description TEXT NOT NULL, + mode TEXT NOT NULL DEFAULT 'prompt', + phase TEXT NOT NULL DEFAULT 'pre-merge', + prompt TEXT NOT NULL DEFAULT '', + enabled INTEGER NOT NULL DEFAULT 1, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + db.exec( + `INSERT INTO workflows (id, name, ir, createdAt, updatedAt) VALUES ('WF-legacy', 'Legacy', '{"version":"v1","name":"x","nodes":[],"edges":[]}', '2025-01-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z')`, + ); + db.exec( + "INSERT INTO workflow_steps (id, name, description, createdAt, updatedAt) VALUES ('WS-legacy', 'Legacy', 'desc', '2025-01-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z')", + ); + + db.init(); + + const workflowColumns = db.prepare("PRAGMA table_info(workflows)").all() as Array<{ + name: string; + }>; + expect(workflowColumns.map((c) => c.name)).toContain("kind"); + // Existing rows default to 'workflow'. + const wfRow = db.prepare("SELECT kind FROM workflows WHERE id = 'WF-legacy'").get() as { kind: string }; + expect(wfRow.kind).toBe("workflow"); + + const stepColumns = db.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; + expect(stepColumns.map((c) => c.name)).toContain("migrated_fragment_id"); + const stepRow = db + .prepare("SELECT migrated_fragment_id FROM workflow_steps WHERE id = 'WS-legacy'") + .get() as { migrated_fragment_id: string | null }; + expect(stepRow.migrated_fragment_id).toBeNull(); + + expect(db.getSchemaVersion()).toBe(109); + db.close(); + }); + + it("migration 109 is idempotent on re-init", () => { + const db = new Database(fusionDir); + db.init(); + expect(db.getSchemaVersion()).toBe(109); + db.close(); + + // Re-open the same on-disk DB: already at 109, the 109 block must be a no-op. + const reopened = new Database(fusionDir); + reopened.init(); + expect(reopened.getSchemaVersion()).toBe(109); + const workflowColumns = reopened.prepare("PRAGMA table_info(workflows)").all() as Array<{ name: string }>; + expect(workflowColumns.filter((c) => c.name === "kind")).toHaveLength(1); + const stepColumns = reopened.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; + expect(stepColumns.filter((c) => c.name === "migrated_fragment_id")).toHaveLength(1); + reopened.close(); + }); }); diff --git a/packages/core/src/__tests__/db.test.ts b/packages/core/src/__tests__/db.test.ts index 8a618bd583..191b67591a 100644 --- a/packages/core/src/__tests__/db.test.ts +++ b/packages/core/src/__tests__/db.test.ts @@ -334,7 +334,7 @@ describe("Database", () => { }); it("seeds schema version", () => { - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("includes tokenUsageCacheWriteTokens on freshly initialized tasks table", () => { @@ -393,7 +393,7 @@ describe("Database", () => { it("is idempotent - calling init() twice does not fail", () => { expect(() => db.init()).not.toThrow(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("does not overwrite existing config on re-init", () => { // Update the config @@ -1463,7 +1463,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 (includes v1→v2 through v26→v29) - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -1488,11 +1488,11 @@ describe("schema migrations", () => { const db = new Database(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Re-init should not fail db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -1527,7 +1527,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("priority"); @@ -1568,7 +1568,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1640,7 +1640,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1880,7 +1880,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("attachments"); @@ -1954,7 +1954,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "agentRatings" }]); @@ -1978,7 +1978,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "mission_events" }]); @@ -2082,7 +2082,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -2301,7 +2301,7 @@ describe("schema migrations", () => { localDb.init(); - expect(localDb.getSchemaVersion()).toBe(108); + expect(localDb.getSchemaVersion()).toBe(109); const columns = localDb.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("tokenUsageCacheWriteTokens"); @@ -2612,7 +2612,7 @@ describe("createDatabase factory", () => { const db = createDatabase(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); expect(db.getLastModified()).toBeGreaterThan(0); db.close(); @@ -2766,7 +2766,7 @@ describe("migration v77 task token budget columns", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const rows = migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const names = new Set(rows.map((row) => row.name)); expect(names.has("tokenBudgetSoftAlertedAt")).toBe(true); @@ -2797,7 +2797,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const names = new Set( (fresh.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2825,7 +2825,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const names = new Set( (migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2851,7 +2851,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const table = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2885,7 +2885,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const table = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2926,7 +2926,7 @@ describe("migration v67 drops orphan project auth tables", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const tables = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; @@ -2953,7 +2953,7 @@ describe("migration v67 drops orphan project auth tables", () => { try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const tables = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; diff --git a/packages/core/src/__tests__/goals-schema.test.ts b/packages/core/src/__tests__/goals-schema.test.ts index 333a68e551..1bbcf5c447 100644 --- a/packages/core/src/__tests__/goals-schema.test.ts +++ b/packages/core/src/__tests__/goals-schema.test.ts @@ -90,7 +90,7 @@ describe("goals schema", () => { expect(table?.name).toBe("goals"); }); - it("reports schema version 101", () => { - expect(db.getSchemaVersion()).toBe(108); + it("reports schema version 109", () => { + expect(db.getSchemaVersion()).toBe(109); }); }); diff --git a/packages/core/src/__tests__/insight-store.test.ts b/packages/core/src/__tests__/insight-store.test.ts index 7aa0e611ad..5d8ac9b47a 100644 --- a/packages/core/src/__tests__/insight-store.test.ts +++ b/packages/core/src/__tests__/insight-store.test.ts @@ -1000,7 +1000,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh database at v33 (runs all migrations up to 33) const db1 = createDatabase(legacyDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); db1.close(); // Step 2: Manually downgrade to version 32 and drop insight tables @@ -1035,7 +1035,7 @@ describe("Migration: pre-33 DB upgrade", () => { expect(tableNamesBefore).not.toContain("project_insight_runs"); // Now run init — this triggers the v32→v33 migration db3.init(); - expect(db3.getSchemaVersion()).toBe(108); + expect(db3.getSchemaVersion()).toBe(109); // Step 4: Verify insight tables exist after migration const tablesAfter = db3.prepare( @@ -1066,12 +1066,12 @@ describe("Migration: pre-33 DB upgrade", () => { try { const db1 = createDatabase(testDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); db1.close(); const db2 = createDatabase(testDir); expect(() => db2.init()).not.toThrow(); - expect(db2.getSchemaVersion()).toBe(108); + expect(db2.getSchemaVersion()).toBe(109); db2.close(); } finally { rmSync(testDir, { recursive: true, force: true }); @@ -1085,7 +1085,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh DB and run migrations const db1 = createDatabase(compatDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); // Step 2: Strip lifecycle and cancelledAt columns by recreating the // table without them. This simulates a DB that was created before the diff --git a/packages/core/src/__tests__/merge-request-record.test.ts b/packages/core/src/__tests__/merge-request-record.test.ts index ef2a10d136..64dc82817b 100644 --- a/packages/core/src/__tests__/merge-request-record.test.ts +++ b/packages/core/src/__tests__/merge-request-record.test.ts @@ -38,7 +38,7 @@ describe("TaskStore merge request record + completion handoff marker", () => { .all() as Array<{ name: string }>; expect(tableRows).toEqual([{ name: "completion_handoff_markers" }, { name: "merge_requests" }]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("upserts merge request records", async () => { diff --git a/packages/core/src/__tests__/mission-store.test.ts b/packages/core/src/__tests__/mission-store.test.ts index 7410346c67..10de8917a2 100644 --- a/packages/core/src/__tests__/mission-store.test.ts +++ b/packages/core/src/__tests__/mission-store.test.ts @@ -3745,8 +3745,8 @@ describe("MissionStore", () => { // ── Loop State & Validator Run Schema Tests ─────────────────────────── describe("Loop State & Validator Run Schema (v31)", () => { - it("schema version is 101 after migration", () => { - expect(db.getSchemaVersion()).toBe(108); + it("schema version is 109 after migration", () => { + expect(db.getSchemaVersion()).toBe(109); }); it("mission_features table has loop state columns", () => { diff --git a/packages/core/src/__tests__/run-audit.test.ts b/packages/core/src/__tests__/run-audit.test.ts index 3be684ea9a..7c818fa605 100644 --- a/packages/core/src/__tests__/run-audit.test.ts +++ b/packages/core/src/__tests__/run-audit.test.ts @@ -583,8 +583,8 @@ describe("Run Audit", () => { expect(indexNames).toContain("idxRunAuditEventsTimestamp"); }); - it("schema version is bumped to 40", () => { - expect(db.getSchemaVersion()).toBe(108); + it("schema version is bumped to 109", () => { + expect(db.getSchemaVersion()).toBe(109); }); }); }); diff --git a/packages/core/src/__tests__/store-merge-queue.test.ts b/packages/core/src/__tests__/store-merge-queue.test.ts index 6c09641156..96a9ea905b 100644 --- a/packages/core/src/__tests__/store-merge-queue.test.ts +++ b/packages/core/src/__tests__/store-merge-queue.test.ts @@ -60,7 +60,7 @@ describe("TaskStore merge queue", () => { expect.arrayContaining(["idx_mergeQueue_lease_ready", "idx_mergeQueue_leaseExpiresAt"]), ); - expect(store.getDatabase().getSchemaVersion()).toBe(108); + expect(store.getDatabase().getSchemaVersion()).toBe(109); }); it("migrates a legacy v88 database and preserves task rows", async () => { diff --git a/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts b/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts new file mode 100644 index 0000000000..60de85e497 --- /dev/null +++ b/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect } from "vitest"; +import { stripApprovalBypassFlags } from "../workflow-ir.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; + +/** + * P0 security helper: removes the CLI-approval-bypass flags + * (`cliSkipApproval`/`autoApprove`) from every node config, recursing into + * foreach `config.template.nodes` at any nesting depth. + */ +describe("stripApprovalBypassFlags", () => { + it("removes both flags from a top-level node config and reports stripped:true", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [{ id: "n1", kind: "prompt", config: { cliSkipApproval: true, autoApprove: true, name: "x" } }], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const cfg = (out as any).nodes[0].config; + expect(cfg.cliSkipApproval).toBeUndefined(); + expect(cfg.autoApprove).toBeUndefined(); + expect(cfg.name).toBe("x"); // unrelated config preserved + }); + + it("strips nested foreach-in-foreach template nodes (arbitrary depth)", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [ + { + id: "outer", + kind: "foreach", + config: { + template: { + nodes: [ + { + id: "inner-foreach", + kind: "foreach", + config: { + template: { + nodes: [ + { id: "deep", kind: "step-execute", config: { autoApprove: true } }, + ], + edges: [], + }, + }, + }, + ], + edges: [], + }, + }, + }, + ], + edges: [], + } as unknown as WorkflowIr; + const { stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const deep = (ir as any).nodes[0].config.template.nodes[0].config.template.nodes[0]; + expect(deep.config.autoApprove).toBeUndefined(); + }); + + it("returns stripped:false when no flags present", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [{ id: "n1", kind: "prompt", config: { name: "x" } }], + edges: [], + } as unknown as WorkflowIr; + expect(stripApprovalBypassFlags(ir).stripped).toBe(false); + }); + + it("tolerates a non-array nodes field", () => { + const ir = { version: "v1", name: "wf" } as unknown as WorkflowIr; + expect(stripApprovalBypassFlags(ir).stripped).toBe(false); + }); + + it("tolerates non-object entries in nodes (untrusted input)", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [null, "bogus", 42, { id: "n1", kind: "prompt", config: { cliSkipApproval: true } }], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + expect((out as any).nodes[3].config.cliSkipApproval).toBeUndefined(); + }); + + it("tolerates non-object entries in nested template.nodes", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [ + { + id: "fe", + kind: "foreach", + config: { + template: { nodes: [null, 0, "x", { id: "inner", kind: "prompt", config: { autoApprove: true } }] }, + }, + }, + ], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + expect((out as any).nodes[0].config.template.nodes[3].config.autoApprove).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/task-documents.test.ts b/packages/core/src/__tests__/task-documents.test.ts index 7352ec08bf..63e5eebfad 100644 --- a/packages/core/src/__tests__/task-documents.test.ts +++ b/packages/core/src/__tests__/task-documents.test.ts @@ -51,7 +51,7 @@ describe("TaskStore task documents", () => { expect(tableNames.has("task_documents")).toBe(true); expect(tableNames.has("task_document_revisions")).toBe(true); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const index = db .prepare( diff --git a/packages/core/src/__tests__/workflow-definition-store.test.ts b/packages/core/src/__tests__/workflow-definition-store.test.ts index 7d78accce5..64860b46c2 100644 --- a/packages/core/src/__tests__/workflow-definition-store.test.ts +++ b/packages/core/src/__tests__/workflow-definition-store.test.ts @@ -180,4 +180,132 @@ describe("TaskStore workflow definitions (U1)", () => { const c = await store.createWorkflowDefinition({ name: "C", ir: makeIr() }); expect(c.id).toBe("WF-003"); }); + + // ── kind discriminator (U1, R6/KTD-1) ──────────────────────────────── + + // A pure-v1 start→node→end fragment IR. + function fragmentIr(): WorkflowIr { + return { + version: "v1", + name: "frag", + nodes: [ + { id: "start", kind: "start" }, + { id: "step-1", kind: "prompt", config: { name: "Doc", gateMode: "advisory", prompt: "doc it" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "step-1", condition: "success" }, + { from: "step-1", to: "end", condition: "success" }, + ], + }; + } + + it("defaults a created workflow to kind 'workflow'", async () => { + const created = await store.createWorkflowDefinition({ name: "W", ir: makeIr() }); + expect(created.kind).toBe("workflow"); + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("workflow"); + }); + + it("persists and round-trips kind 'fragment' (INSERT includes kind)", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + expect(created.kind).toBe("fragment"); + // Raw column persisted. + const raw = (store as any).db.prepare("SELECT kind FROM workflows WHERE id = ?").get(created.id) as { kind: string }; + expect(raw.kind).toBe("fragment"); + // Reload. + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("fragment"); + }); + + it("preserves kind across updateWorkflowDefinition", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const updated = await store.updateWorkflowDefinition(created.id, { description: "edited" }); + expect(updated.kind).toBe("fragment"); + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("fragment"); + }); + + it("listWorkflowDefinitions({kind:'fragment'}) returns only fragments", async () => { + await store.createWorkflowDefinition({ name: "W1", ir: makeIr() }); + const frag = await store.createWorkflowDefinition({ name: "F1", ir: fragmentIr(), kind: "fragment" }); + const fragments = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(fragments.map((w) => w.id)).toEqual([frag.id]); + expect(fragments.every((w) => w.kind === "fragment")).toBe(true); + }); + + it("built-in list entries are kind 'workflow'", async () => { + const all = await store.listWorkflowDefinitions(); + const builtins = all.filter((w) => isBuiltinWorkflowId(w.id)); + expect(builtins.length).toBeGreaterThan(0); + expect(builtins.every((w) => w.kind === "workflow")).toBe(true); + // The workflow filter includes built-ins; the fragment filter excludes them. + expect((await store.listWorkflowDefinitions({ kind: "workflow" })).some((w) => isBuiltinWorkflowId(w.id))).toBe(true); + expect((await store.listWorkflowDefinitions({ kind: "fragment" })).some((w) => isBuiltinWorkflowId(w.id))).toBe(false); + }); + + it("cache regression: filtered then unfiltered (and reverse) are both correct", async () => { + await store.createWorkflowDefinition({ name: "W1", ir: makeIr() }); + const frag = await store.createWorkflowDefinition({ name: "F1", ir: fragmentIr(), kind: "fragment" }); + + // filtered → unfiltered + const f1 = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(f1.map((w) => w.id)).toEqual([frag.id]); + const allAfterFiltered = await store.listWorkflowDefinitions(); + expect(allAfterFiltered.filter((w) => !isBuiltinWorkflowId(w.id)).map((w) => w.kind).sort()).toEqual([ + "fragment", + "workflow", + ]); + + // unfiltered → filtered (cache already populated by the unfiltered call) + const f2 = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(f2.map((w) => w.id)).toEqual([frag.id]); + const w2 = await store.listWorkflowDefinitions({ kind: "workflow" }); + expect(w2.filter((w) => !isBuiltinWorkflowId(w.id)).every((w) => w.kind === "workflow")).toBe(true); + }); + + it("a fragment IR survives downgradeIrToV1IfPure unchanged (persists as v1)", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const raw = (store as any).db.prepare("SELECT ir FROM workflows WHERE id = ?").get(created.id) as { ir: string }; + expect(JSON.parse(raw.ir).version).toBe("v1"); + }); + + it("selectTaskWorkflow rejects a fragment id with a clear error", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + // Create a task to select against. + const task = await store.createTask({ description: "t" }); + await expect(store.selectTaskWorkflow(task.id, frag.id)).rejects.toThrow(/fragment/i); + }); + + it("setDefaultWorkflowId rejects a fragment id at the write boundary", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + await expect(store.setDefaultWorkflowId(frag.id)).rejects.toThrow(/fragment/i); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("setDefaultWorkflowId accepts a real workflow and clears with null", async () => { + const wf = await store.createWorkflowDefinition({ name: "W", ir: makeIr() }); + await store.setDefaultWorkflowId(wf.id); + expect(await store.getDefaultWorkflowId()).toBe(wf.id); + await store.setDefaultWorkflowId(null); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("createTaskWithReservedId honors an explicit workflowId (precedence over default)", async () => { + const def = await store.createWorkflowDefinition({ name: "Explicit", ir: makeIr() }); + const task = await store.createTaskWithReservedId( + { description: "t", workflowId: def.id }, + { taskId: "task-explicit-wf" }, + ); + const sel = store.getTaskWorkflowSelection(task.id); + expect(sel?.workflowId).toBe(def.id); + }); + + it("createTaskWithReservedId treats workflowId:null as explicit opt-out", async () => { + const def = await store.createWorkflowDefinition({ name: "Def", ir: makeIr() }); + await store.setDefaultWorkflowId(def.id); + const task = await store.createTaskWithReservedId( + { description: "t", workflowId: null }, + { taskId: "task-optout-wf" }, + ); + const sel = store.getTaskWorkflowSelection(task.id); + expect(sel?.workflowId ?? undefined).toBeUndefined(); + }); }); diff --git a/packages/core/src/__tests__/workflow-selection-store.test.ts b/packages/core/src/__tests__/workflow-selection-store.test.ts index cfc4b37157..840715be59 100644 --- a/packages/core/src/__tests__/workflow-selection-store.test.ts +++ b/packages/core/src/__tests__/workflow-selection-store.test.ts @@ -23,6 +23,23 @@ function linearIr(): WorkflowIr { }; } +/** A single-node fragment IR (start → one node → end). */ +function fragmentIr(): WorkflowIr { + return { + version: "v1", + name: "frag", + nodes: [ + { id: "start", kind: "start" }, + { id: "step-1", kind: "prompt", config: { name: "Doc", prompt: "doc it" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "step-1", condition: "success" }, + { from: "step-1", to: "end", condition: "success" }, + ], + }; +} + function branchingIr(): WorkflowIr { return { version: "v1", @@ -172,4 +189,71 @@ describe("TaskStore workflow selection (U3)", () => { await store.setDefaultWorkflowId(null); expect(await store.getDefaultWorkflowId()).toBeUndefined(); }); + + // U6/R3/KTD-4: create-time `workflowId` materializes the selection atomically. + describe("create-time workflowId (U6/R3)", () => { + it("materializes enabledWorkflowSteps atomically when workflowId is given", async () => { + const wf = await store.createWorkflowDefinition({ name: "Pick", ir: linearIr() }); + + const task = await store.createTask({ description: "with workflow", workflowId: wf.id }); + // Reading the task right after create observes the populated steps — no + // intermediate empty state visible to the executor. + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(wf.id); + expect(store.getTaskWorkflowSelection(task.id)?.stepIds).toEqual(detail.enabledWorkflowSteps); + }); + + it("explicit workflowId overrides the project default", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + const chosen = await store.createWorkflowDefinition({ name: "Chosen", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "override default", workflowId: chosen.id }); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(chosen.id); + }); + + it("workflowId: null skips default materialization (explicit No workflow)", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "no workflow", workflowId: null }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps ?? []).toHaveLength(0); + expect(store.getTaskWorkflowSelection(task.id)).toBeUndefined(); + }); + + it("undefined workflowId still inherits the project default (unchanged)", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "inherit" }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(def.id); + }); + + it("rejects a fragment id before creating the task row", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const before = (await store.listTasks({ includeArchived: true })).length; + + await expect( + store.createTask({ description: "frag pick", workflowId: frag.id }), + ).rejects.toThrow(/fragment/i); + + const after = (await store.listTasks({ includeArchived: true })).length; + expect(after).toBe(before); + }); + + it("rejects an unknown workflow id before creating the task row", async () => { + const before = (await store.listTasks({ includeArchived: true })).length; + + await expect( + store.createTask({ description: "bad pick", workflowId: "WF-404" }), + ).rejects.toThrow(/not found/i); + + const after = (await store.listTasks({ includeArchived: true })).length; + expect(after).toBe(before); + }); + }); }); diff --git a/packages/core/src/__tests__/workflow-step-migration.test.ts b/packages/core/src/__tests__/workflow-step-migration.test.ts new file mode 100644 index 0000000000..62d47bf2d3 --- /dev/null +++ b/packages/core/src/__tests__/workflow-step-migration.test.ts @@ -0,0 +1,194 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { isBuiltinWorkflowId } from "../builtin-workflows.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/** + * U2 / R5 / KTD-3 — lazy idempotent migration of legacy user-authored workflow + * steps into the dual fragment + combined-workflow representation. + */ +describe("TaskStore.migrateLegacyWorkflowSteps (U2/R5)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + + afterEach(async () => { + await harness.afterEach(); + }); + + /** User-owned (non-builtin) workflow definitions only. */ + async function userDefs() { + return (await store.listWorkflowDefinitions()).filter((w) => !isBuiltinWorkflowId(w.id)); + } + + it("converts defaultOn + optional + disabled user steps to fragments, builds the combined workflow from defaultOn only, sets the project default, and leaves the compiled row untouched", async () => { + // defaultOn (ran automatically on new tasks) → fragment + joins combined workflow. + const on = await store.createWorkflowStep({ + name: "Default On", + description: "ran by default", + prompt: "do the default thing", + defaultOn: true, + enabled: true, + }); + // enabled-but-optional → fragment only (NOT in combined workflow). + const optional = await store.createWorkflowStep({ + name: "Optional", + description: "opt-in", + prompt: "optional work", + defaultOn: false, + enabled: true, + }); + // disabled → still gets a fragment (every user step does). + const disabled = await store.createWorkflowStep({ + name: "Disabled", + description: "off", + prompt: "disabled work", + defaultOn: false, + enabled: false, + }); + // compiled-materialized row (execution detail) → must be ignored entirely. + const compiled = await store.createWorkflowStep({ + name: "Compiled", + description: "materialized", + templateId: "workflow:WF-999", + defaultOn: true, + enabled: true, + }); + + const result = await store.migrateLegacyWorkflowSteps(); + + // 3 user steps converted; nothing previously migrated. + expect(result.migrated).toBe(3); + expect(result.skipped).toBe(0); + expect(result.combinedWorkflowId).toBeTruthy(); + + const defs = await userDefs(); + const fragments = defs.filter((d) => d.kind === "fragment"); + const workflows = defs.filter((d) => d.kind === "workflow"); + + // Exactly 3 fragments (one per user step), exactly 1 combined workflow. + expect(fragments).toHaveLength(3); + expect(workflows).toHaveLength(1); + expect(fragments.map((f) => f.name).sort()).toEqual(["Default On", "Disabled", "Optional"]); + + // Combined workflow: named "Migrated steps", carries the system description, + // and contains ONLY the defaultOn step's user node (plus start/end + seams). + const combined = workflows[0]; + expect(combined.id).toBe(result.combinedWorkflowId); + expect(combined.name).toBe("Migrated steps"); + expect(combined.description).toBe("Converted from your legacy workflow steps"); + const userNodes = combined.ir.nodes.filter( + (n) => n.kind !== "start" && n.kind !== "end" && typeof n.config?.seam !== "string", + ); + expect(userNodes).toHaveLength(1); + expect(userNodes[0].config?.name).toBe("Default On"); + + // Project default points at the combined workflow. + expect(await store.getDefaultWorkflowId()).toBe(combined.id); + + // All 3 user source rows are stamped; the compiled row is untouched. + expect((await store.getWorkflowStep(on.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(optional.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(disabled.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(compiled.id))?.migratedFragmentId).toBeUndefined(); + + // No source records were deleted. + const steps = await store.listWorkflowSteps(); + expect(steps.map((s) => s.id)).toEqual(expect.arrayContaining([on.id, optional.id, disabled.id])); + }); + + it("creates fragments but NO combined workflow and leaves the default unchanged when no step is defaultOn", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: false }); + await store.createWorkflowStep({ name: "B", description: "b", prompt: "b", enabled: false }); + + const result = await store.migrateLegacyWorkflowSteps(); + + expect(result.migrated).toBe(2); + expect(result.combinedWorkflowId).toBeUndefined(); + + const defs = await userDefs(); + expect(defs.filter((d) => d.kind === "fragment")).toHaveLength(2); + expect(defs.filter((d) => d.kind === "workflow")).toHaveLength(0); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("is idempotent: a second run converts nothing and creates no new definitions", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + + const first = await store.migrateLegacyWorkflowSteps(); + expect(first.migrated).toBe(1); + const afterFirst = (await userDefs()).length; + + const second = await store.migrateLegacyWorkflowSteps(); + expect(second.migrated).toBe(0); + expect(second.skipped).toBe(1); + expect(second.combinedWorkflowId).toBeUndefined(); + expect((await userDefs()).length).toBe(afterFirst); + }); + + it("does not clobber a pre-existing project default", async () => { + // A user-chosen default workflow exists before migration. + const existing = await store.createWorkflowDefinition({ + name: "My choice", + ir: { + version: "v1", + name: "My choice", + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + }, + kind: "workflow", + }); + await store.setDefaultWorkflowId(existing.id); + + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + const result = await store.migrateLegacyWorkflowSteps(); + + // The combined workflow is still created, but the explicit default is kept. + expect(result.combinedWorkflowId).toBeTruthy(); + expect(await store.getDefaultWorkflowId()).toBe(existing.id); + }); + + it("compare-and-set: re-reads the default after the transaction and skips when a concurrent writer set one", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + + const concurrent = await store.createWorkflowDefinition({ + name: "Concurrent", + ir: { + version: "v1", + name: "Concurrent", + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + }, + kind: "workflow", + }); + + // A project default exists when migration's post-transaction compare-and-set + // re-reads it. Because the set is gated on the re-read (not a pre-transaction + // snapshot), an existing default is observed and never clobbered. + await store.setDefaultWorkflowId(concurrent.id); + + const result = await store.migrateLegacyWorkflowSteps(); + + expect(result.combinedWorkflowId).toBeTruthy(); + expect(result.combinedWorkflowId).not.toBe(concurrent.id); + // The compare-and-set re-read observed the existing default and did NOT clobber it. + expect(await store.getDefaultWorkflowId()).toBe(concurrent.id); + }); + + it("is a no-op with zero user steps", async () => { + const result = await store.migrateLegacyWorkflowSteps(); + expect(result).toEqual({ migrated: 0, skipped: 0, combinedWorkflowId: undefined }); + expect(await userDefs()).toHaveLength(0); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/workflow-steps-to-ir.test.ts b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts new file mode 100644 index 0000000000..de041be2d3 --- /dev/null +++ b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts @@ -0,0 +1,226 @@ +import { describe, it, expect } from "vitest"; + +import { stepsToWorkflowIr, stepToFragmentIr, layoutForIr } from "../workflow-steps-to-ir.js"; +import { compileWorkflowToSteps } from "../workflow-compiler.js"; +import { parseWorkflowIr } from "../workflow-ir.js"; +import type { WorkflowStep, WorkflowStepInput } from "../types.js"; + +/** Build a fully-specified WorkflowStep fixture. */ +function step(overrides: Partial): WorkflowStep { + return { + id: overrides.id ?? "WS-000", + name: overrides.name ?? "Step", + description: overrides.description ?? "", + mode: overrides.mode ?? "prompt", + phase: overrides.phase, + gateMode: overrides.gateMode ?? "advisory", + prompt: overrides.prompt ?? "", + toolMode: overrides.toolMode, + scriptName: overrides.scriptName, + enabled: overrides.enabled ?? true, + defaultOn: overrides.defaultOn, + modelProvider: overrides.modelProvider, + modelId: overrides.modelId, + migratedFragmentId: overrides.migratedFragmentId, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; +} + +/** Project a compiled step input down to exactly the compiler-visible fields the + * round-trip contract pins (KTD-2). Normalizes optional fields for comparison. */ +function visible(input: WorkflowStepInput) { + return { + name: input.name, + mode: input.mode, + phase: input.phase, + gateMode: input.gateMode, + prompt: input.mode === "script" ? undefined : (input.prompt ?? ""), + scriptName: input.scriptName, + toolMode: input.mode === "script" ? undefined : input.toolMode, + modelProvider: input.modelProvider, + modelId: input.modelId, + }; +} + +function visibleStep(s: WorkflowStep) { + return { + name: s.name, + mode: s.mode, + phase: s.phase ?? "pre-merge", + gateMode: s.gateMode, + prompt: s.mode === "script" ? undefined : (s.prompt ?? ""), + scriptName: s.mode === "script" ? s.scriptName : undefined, + toolMode: s.mode === "script" ? undefined : (s.toolMode ?? "readonly"), + modelProvider: s.mode === "prompt" ? s.modelProvider : undefined, + modelId: s.mode === "prompt" ? s.modelId : undefined, + }; +} + +describe("stepsToWorkflowIr — round-trip parity (R4/KTD-2)", () => { + it("reproduces every compiler-visible field for a mixed step set", () => { + const steps: WorkflowStep[] = [ + step({ + id: "WS-1", + name: "Implement", + description: "do the work", + mode: "prompt", + gateMode: "advisory", + prompt: "Implement the change", + toolMode: "coding", + phase: "pre-merge", + }), + step({ + id: "WS-2", + name: "Lint", + mode: "script", + gateMode: "gate", + scriptName: "lint", + phase: "pre-merge", + }), + step({ + id: "WS-3", + name: "Security gate", + mode: "prompt", + gateMode: "gate", + prompt: "Block on exploitable findings", + toolMode: "readonly", + modelProvider: "anthropic", + modelId: "claude-sonnet-4-5", + phase: "pre-merge", + }), + step({ + id: "WS-4", + name: "Document", + mode: "prompt", + gateMode: "advisory", + prompt: "Write docs", + phase: "post-merge", + }), + step({ + id: "WS-5", + name: "Deploy script", + mode: "script", + gateMode: "advisory", + scriptName: "deploy", + phase: "post-merge", + }), + ]; + + const ir = stepsToWorkflowIr(steps, "Migrated"); + const compiled = compileWorkflowToSteps(ir); + + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); + + it("undefined phase maps to pre-merge and round-trips", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" }), + step({ id: "WS-2", name: "B", mode: "prompt", gateMode: "advisory", prompt: "b" }), + ]; + const ir = stepsToWorkflowIr(steps, "AllUndefined"); + // parseable + expect(() => parseWorkflowIr(ir)).not.toThrow(); + const compiled = compileWorkflowToSteps(ir); + expect(compiled.map((c) => c.phase)).toEqual(["pre-merge", "pre-merge"]); + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); + + it("empty step list yields a minimal valid IR that compiles to []", () => { + const ir = stepsToWorkflowIr([], "Empty"); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + expect(compileWorkflowToSteps(ir)).toEqual([]); + // start + 3 seams + end. + expect(ir.nodes.map((n) => n.id)).toEqual(["start", "execute", "review", "merge", "end"]); + }); + + it("post-merge-only set places nodes after the merge seam", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "After", mode: "prompt", gateMode: "advisory", prompt: "x", phase: "post-merge" }), + ]; + const ir = stepsToWorkflowIr(steps, "PostOnly"); + const ids = ir.nodes.map((n) => n.id); + expect(ids.indexOf("merge")).toBeLessThan(ids.indexOf("step-1")); + const compiled = compileWorkflowToSteps(ir); + expect(compiled).toHaveLength(1); + expect(compiled[0].phase).toBe("post-merge"); + }); + + it("produced IR passes parseWorkflowIr and encodes seams exactly per linear()", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" }), + ]; + const ir = stepsToWorkflowIr(steps, "Seams"); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + + // Each seam appears exactly once, in execute → review → merge order. + const seamNodes = ir.nodes.filter((n) => typeof n.config?.seam === "string"); + expect(seamNodes.map((n) => n.config!.seam)).toEqual(["execute", "review", "merge"]); + + // Each seam has a failure → end edge. + for (const seam of ["execute", "review", "merge"]) { + const failEdge = ir.edges.find((e) => e.from === seam && e.condition === "failure"); + expect(failEdge?.to).toBe("end"); + } + // No duplicate failure edges per seam. + const failureEdges = ir.edges.filter((e) => e.condition === "failure"); + expect(failureEdges).toHaveLength(3); + }); + + it("gate vs advisory both round-trip for prompt and script modes", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "PG", mode: "prompt", gateMode: "gate", prompt: "p" }), + step({ id: "WS-2", name: "PA", mode: "prompt", gateMode: "advisory", prompt: "p" }), + step({ id: "WS-3", name: "SG", mode: "script", gateMode: "gate", scriptName: "s" }), + step({ id: "WS-4", name: "SA", mode: "script", gateMode: "advisory", scriptName: "s" }), + ]; + const compiled = compileWorkflowToSteps(stepsToWorkflowIr(steps, "Gates")); + expect(compiled.map((c) => c.gateMode)).toEqual(["gate", "advisory", "gate", "advisory"]); + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); +}); + +describe("stepToFragmentIr (R6/KTD-1)", () => { + it("produces a parseable start → node → end fragment mirroring the step", () => { + const s = step({ + id: "WS-1", + name: "Doc", + description: "doc it", + mode: "prompt", + gateMode: "advisory", + prompt: "Document the change", + toolMode: "readonly", + }); + const ir = stepToFragmentIr(s); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + expect(ir.nodes.map((n) => n.id)).toEqual(["start", "step-1", "end"]); + expect(ir.nodes.map((n) => n.kind)).toEqual(["start", "prompt", "end"]); + + // The single node compiles back to a step mirroring the source. + const compiled = compileWorkflowToSteps(ir); + expect(compiled).toHaveLength(1); + expect(visible(compiled[0])).toEqual(visibleStep(s)); + }); + + it("fragment IR is pure v1 (no v2-only features)", () => { + const ir = stepToFragmentIr(step({ id: "WS-1", name: "S", mode: "script", gateMode: "gate", scriptName: "lint" })); + // parseWorkflowIr upgrades to v2 in-memory; the SOURCE we built is v1-shaped. + const compiled = compileWorkflowToSteps(ir); + expect(compiled[0].mode).toBe("script"); + expect(compiled[0].scriptName).toBe("lint"); + }); +}); + +describe("layoutForIr", () => { + it("produces x-spaced positions for every node", () => { + const ir = stepsToWorkflowIr( + [step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" })], + "L", + ); + const layout = layoutForIr(ir); + expect(Object.keys(layout).sort()).toEqual(ir.nodes.map((n) => n.id).sort()); + expect(layout.start).toEqual({ x: 60, y: 160 }); + // Second node is one column over. + expect(layout[ir.nodes[1].id].x).toBe(60 + 170); + }); +}); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 62e734cf44..270fda980d 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -47,6 +47,8 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { id: spec.id, name: spec.name, description: spec.description, + // Built-ins are always selectable workflows, never fragments (KTD-1). + kind: "workflow", ir, layout, createdAt: BUILTIN_TS, @@ -152,6 +154,7 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ name: "Stepwise coding (built-in)", description: "Per-step plan, execute, and review modeled as graph structure: each planned step runs and is reviewed (approve / revise / rethink) before the next, with bounded rework. Requires the workflow graph executor.", + kind: "workflow", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, layout: { start: { x: 60, y: 160 }, diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index 6403a186f1..886b80a64b 100644 --- a/packages/core/src/db.ts +++ b/packages/core/src/db.ts @@ -149,7 +149,7 @@ export function probeFts5(db: DatabaseSync): boolean { // ── Schema Definition ──────────────────────────────────────────────── -const SCHEMA_VERSION = 108; +const SCHEMA_VERSION = 109; export { SCHEMA_VERSION }; @@ -385,6 +385,10 @@ CREATE TABLE IF NOT EXISTS workflow_steps ( defaultOn INTEGER DEFAULT 0, modelProvider TEXT, modelId TEXT, + -- (workflow-editor-consolidation U1/U2) when this step has been migrated into a + -- fragment WorkflowDefinition, the fragment's id is stamped here so re-runs of + -- the lazy migration skip already-migrated rows (marker idempotency). + migrated_fragment_id TEXT, createdAt TEXT NOT NULL, updatedAt TEXT NOT NULL ); @@ -398,6 +402,11 @@ CREATE TABLE IF NOT EXISTS workflows ( description TEXT NOT NULL DEFAULT '', ir TEXT NOT NULL, layout TEXT NOT NULL DEFAULT '{}', + -- (workflow-editor-consolidation U1, KTD-1) discriminates reusable single-node + -- "fragment" templates from full "workflow" definitions. Fragments never appear + -- in task workflow pickers, default-workflow selection, or compile/selection + -- paths. Legacy rows default to 'workflow'. + kind TEXT NOT NULL DEFAULT 'workflow', createdAt TEXT NOT NULL, updatedAt TEXT NOT NULL ); @@ -4291,6 +4300,19 @@ export class Database { }); } + // Migration 109: Workflow editor consolidation (workflow-editor-consolidation + // U1, KTD-1). Adds workflows.kind (fragment vs workflow discriminator; + // existing rows default to 'workflow') and workflow_steps.migrated_fragment_id + // (nullable marker stamping a step that has been migrated into a fragment, so + // the lazy step migration is idempotent). Additive-only, idempotent + // (addColumnIfMissing guards); no backfill. + if (version < 109) { + this.applyMigration(109, () => { + this.addColumnIfMissing("workflows", "kind", "TEXT NOT NULL DEFAULT 'workflow'"); + this.addColumnIfMissing("workflow_steps", "migrated_fragment_id", "TEXT"); + }); + } + } /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 33edfa9488..4ba50726c1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -48,6 +48,7 @@ export { export { parseWorkflowIr, serializeWorkflowIr, + stripApprovalBypassFlags, WorkflowIrError, DEFAULT_WORKFLOW_COLUMN_IDS, } from "./workflow-ir.js"; @@ -232,6 +233,7 @@ export type { WorkflowDefinition, WorkflowDefinitionInput, WorkflowDefinitionUpdate, + WorkflowDefinitionKind, WorkflowNodeLayout, } from "./workflow-definition-types.js"; export { @@ -239,6 +241,11 @@ export { validateLinearity, WorkflowCompileError, } from "./workflow-compiler.js"; +export { + stepsToWorkflowIr, + stepToFragmentIr, + layoutForIr, +} from "./workflow-steps-to-ir.js"; export { BUILTIN_WORKFLOWS, BUILTIN_WORKFLOW_ID_PREFIX, @@ -464,6 +471,7 @@ export { toJson, toJsonNullable, fromJson, + SCHEMA_VERSION, } from "./db.js"; export { ProjectIdentityConflictError, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 352b281004..4dafc0fc5f 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -8,6 +8,7 @@ import { createActivityLogSnapshot, createRunAuditSnapshot, createTaskMetadataSn import { VALID_TRANSITIONS, COLUMNS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js"; import { DEFAULT_PROJECT_SETTINGS } from "./settings-schema.js"; import { parseWorkflowIr, serializeWorkflowIr, downgradeIrToV1IfPure } from "./workflow-ir.js"; +import { stepsToWorkflowIr, stepToFragmentIr, layoutForIr } from "./workflow-steps-to-ir.js"; import { isWorkflowColumnsEnabled } from "./workflow-columns-settings.js"; import { resolveAllowedColumns, workflowHasColumn } from "./workflow-transitions.js"; import { @@ -3738,6 +3739,7 @@ export class TaskStore extends EventEmitter { defaultOn: number | null; modelProvider: string | null; modelId: string | null; + migrated_fragment_id?: string | null; createdAt: string; updatedAt: string; }): import("./types.js").WorkflowStep { @@ -3758,6 +3760,7 @@ export class TaskStore extends EventEmitter { defaultOn: row.defaultOn === null || row.defaultOn === undefined ? undefined : Boolean(row.defaultOn), modelProvider: row.modelProvider ?? undefined, modelId: row.modelId ?? undefined, + migratedFragmentId: row.migrated_fragment_id ?? undefined, createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -3972,7 +3975,24 @@ export class TaskStore extends EventEmitter { // When a project default workflow is configured, new tasks inherit it // (compiled to steps) ahead of the legacy default-on step behavior. let pendingWorkflowSelection: { workflowId: string; stepIds: string[] } | undefined; - if (input.enabledWorkflowSteps === undefined) { + // U6/R3/KTD-4: an explicit create-time workflowId beats the project default. + // `null` is an explicit opt-out (no workflow), `string` materializes that + // workflow, `undefined` falls through to the default-workflow behavior below. + // Explicit enabledWorkflowSteps still wins over workflowId for trusted callers. + const explicitWorkflowId = + input.enabledWorkflowSteps === undefined ? input.workflowId : undefined; + if (explicitWorkflowId !== undefined) { + if (explicitWorkflowId === null) { + // Explicit "No workflow": skip default materialization entirely. + resolvedWorkflowSteps = undefined; + } else { + // Compile + materialize up front so unknown/fragment ids throw BEFORE + // the task row is created (no orphaned steps, no half-created task). + const selected = await this.materializeExplicitWorkflowSteps(explicitWorkflowId); + resolvedWorkflowSteps = selected.stepIds; + pendingWorkflowSelection = selected; + } + } else if (input.enabledWorkflowSteps === undefined) { try { const inherited = await this.materializeDefaultWorkflowSteps(); if (inherited) { @@ -4141,7 +4161,24 @@ export class TaskStore extends EventEmitter { : undefined; let pendingWorkflowSelection: { workflowId: string; stepIds: string[] } | undefined; - if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) { + // U6/R3/KTD-4: an explicit create-time workflowId beats the project default, + // mirroring createTask(). `null` is an explicit opt-out, `string` materializes + // that workflow, `undefined` falls through to the default-workflow behavior. + // Explicit enabledWorkflowSteps still wins over workflowId for trusted callers. + const explicitWorkflowId = + input.enabledWorkflowSteps === undefined ? input.workflowId : undefined; + if (explicitWorkflowId !== undefined) { + if (explicitWorkflowId === null) { + // Explicit "No workflow": skip default materialization entirely. + resolvedWorkflowSteps = undefined; + } else { + // Compile + materialize up front so unknown/fragment ids throw BEFORE + // the task row is created (no orphaned steps, no half-created task). + const selected = await this.materializeExplicitWorkflowSteps(explicitWorkflowId); + resolvedWorkflowSteps = selected.stepIds; + pendingWorkflowSelection = selected; + } + } else if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) { // Mirror createTask: a configured project default workflow takes // precedence over legacy default-on steps on this creation path too. try { @@ -11827,6 +11864,7 @@ ${stepsSection}`; defaultOn: input.defaultOn !== undefined ? input.defaultOn : undefined, modelProvider: mode === "prompt" ? input.modelProvider : undefined, modelId: mode === "prompt" ? input.modelId : undefined, + migratedFragmentId: input.migratedFragmentId, createdAt: now, updatedAt: now, }; @@ -11847,9 +11885,10 @@ ${stepsSection}`; defaultOn, modelProvider, modelId, + migrated_fragment_id, createdAt, updatedAt - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ).run( step.id, step.templateId ?? null, @@ -11865,6 +11904,7 @@ ${stepsSection}`; step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, step.modelProvider ?? null, step.modelId ?? null, + step.migratedFragmentId ?? null, step.createdAt, step.updatedAt, ); @@ -12087,6 +12127,7 @@ ${stepsSection}`; if ("modelProvider" in updates) step.modelProvider = updates.modelProvider; if ("modelId" in updates) step.modelId = updates.modelId; } + if ("migratedFragmentId" in updates) step.migratedFragmentId = updates.migratedFragmentId; step.updatedAt = new Date().toISOString(); this.db.prepare( @@ -12104,6 +12145,7 @@ ${stepsSection}`; defaultOn = ?, modelProvider = ?, modelId = ?, + migrated_fragment_id = ?, updatedAt = ? WHERE id = ?`, ).run( @@ -12120,6 +12162,7 @@ ${stepsSection}`; step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, step.modelProvider ?? null, step.modelId ?? null, + step.migratedFragmentId ?? null, step.updatedAt, step.id, ); @@ -12195,6 +12238,7 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; }): WorkflowDefinition { @@ -12202,6 +12246,8 @@ ${stepsSection}`; id: row.id, name: row.name, description: row.description, + // Legacy rows (pre-migration-109) have no kind column; default to "workflow". + kind: row.kind === "fragment" ? "fragment" : "workflow", ir: parseWorkflowIr(row.ir), layout: this.parseWorkflowLayout(row.layout), createdAt: row.createdAt, @@ -12256,6 +12302,9 @@ ${stepsSection}`; id, name, description: input.description ?? "", + // KTD-1: fragments are pure-v1 IRs and pass through downgradeIrToV1IfPure + // unchanged; default to "workflow" when the caller omits the kind. + kind: input.kind === "fragment" ? "fragment" : "workflow", ir, layout, createdAt: now, @@ -12264,8 +12313,8 @@ ${stepsSection}`; this.db .prepare( - `INSERT INTO workflows (id, name, description, ir, layout, createdAt, updatedAt) - VALUES (?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO workflows (id, name, description, ir, layout, kind, createdAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, ) .run( definition.id, @@ -12275,6 +12324,7 @@ ${stepsSection}`; flagOnForCreate ? definition.ir : downgradeIrToV1IfPure(definition.ir), ), JSON.stringify(definition.layout), + definition.kind, definition.createdAt, definition.updatedAt, ); @@ -12285,8 +12335,26 @@ ${stepsSection}`; }); } - /** List all workflow definitions, oldest first. Cached until a mutation. */ - async listWorkflowDefinitions(): Promise { + /** List workflow definitions, oldest first. The `kind` filter (KTD-1) selects + * only workflows or only fragments; omit it to get the full merged set. + * + * Cache invariant: `workflowDefinitionsCache` ALWAYS holds the full merged set + * (built-ins + every row of every kind). The `kind` filter is applied to a + * slice taken AFTER the cache read — a filtered result is never cached, so a + * filtered call can never poison an unfiltered consumer (or vice versa). + */ + async listWorkflowDefinitions( + options?: { kind?: WorkflowDefinition["kind"] }, + ): Promise { + const all = await this.readAllWorkflowDefinitions(); + if (options?.kind) return all.filter((wf) => wf.kind === options.kind); + return all; + } + + /** Read (and cache) the full merged workflow-definition set, oldest first. + * Built-in templates lead the list and cannot be edited/deleted; built-ins + * are always kind "workflow". */ + private async readAllWorkflowDefinitions(): Promise { if (this.workflowDefinitionsCache) return this.workflowDefinitionsCache; const rows = this.db.prepare("SELECT * FROM workflows ORDER BY createdAt ASC").all() as Array<{ id: string; @@ -12294,10 +12362,10 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; }>; - // Built-in templates lead the list and cannot be edited/deleted. this.workflowDefinitionsCache = [...BUILTIN_WORKFLOWS, ...rows.map((row) => this.toWorkflowDefinition(row))]; return this.workflowDefinitionsCache; } @@ -12315,6 +12383,7 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; } @@ -12958,11 +13027,189 @@ ${stepsSection}`; if (workflowId) { const exists = await this.getWorkflowDefinition(workflowId); if (!exists) throw new Error(`Workflow '${workflowId}' not found`); + // KTD-1/R6: a fragment is a reusable palette piece, not a selectable + // workflow. Reject it at the write boundary so a fragment can never be + // persisted as the project default (the read-side skip in + // materializeDefaultWorkflowSteps remains as defense in depth). + if (exists.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be set as the project default`); + } } // null is updateSettings' explicit-delete sentinel for project keys. await this.updateSettings({ defaultWorkflowId: workflowId } as unknown as Partial); } + /** + * Synchronous workflow-definition insert used by migration (U2/KTD-3). Mirrors + * the persistence side of `createWorkflowDefinition` (validation + flag-aware + * downgrade + INSERT + cache bust) but stays synchronous so it can run inside + * `transactionImmediate`. The flag value is resolved by the async caller and + * passed in, since reading it is async. + */ + private insertWorkflowDefinitionSync( + input: WorkflowDefinitionInput, + flagOn: boolean, + ): WorkflowDefinition { + const name = input.name?.trim(); + if (!name) throw new Error("Workflow name is required"); + const ir = parseWorkflowIr(input.ir); + this.assertWorkflowIrTraitsValid(ir); + const layout = input.layout ?? {}; + const now = new Date().toISOString(); + const id = this.nextWorkflowDefinitionId(); + const definition: WorkflowDefinition = { + id, + name, + description: input.description ?? "", + kind: input.kind === "fragment" ? "fragment" : "workflow", + ir, + layout, + createdAt: now, + updatedAt: now, + }; + this.db + .prepare( + `INSERT INTO workflows (id, name, description, ir, layout, kind, createdAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + definition.id, + definition.name, + definition.description, + serializeWorkflowIr(flagOn ? definition.ir : downgradeIrToV1IfPure(definition.ir)), + JSON.stringify(definition.layout), + definition.kind, + definition.createdAt, + definition.updatedAt, + ); + this.workflowDefinitionsCache = null; + return definition; + } + + /** + * Lazy, idempotent migration of legacy user-authored workflow steps into the + * dual workflow-definition representation (U2 / R5 / KTD-3). Runs on first + * editor open per project via `POST /api/workflows/migrate-legacy-steps`. + * + * Policy: + * - Every unmigrated user step (enabled or not, excluding compiled-materialized + * rows) becomes a `kind: "fragment"` definition — the reusable palette piece. + * - The `defaultOn` subset additionally becomes ONE combined `kind: "workflow"` + * definition named "Migrated steps" (these were the steps that ran + * automatically on new tasks); when non-empty and no project default is + * already set, it becomes the project default so new-task behavior is + * preserved. An explicit existing default is never clobbered. + * - Each source row is stamped with `migratedFragmentId` (idempotency marker). + * Source rows are never deleted. + * + * Idempotency: the unmigrated-rows SELECT and the marker stamping happen inside + * a single `transactionImmediate` (write lock acquired BEFORE the SELECT, + * matching `selectTaskWorkflow`'s ordering rationale), so concurrent opens / + * re-runs converge to a single set of definitions. A second run sees zero + * unmigrated rows and returns `{ migrated: 0, skipped: n }`. + */ + async migrateLegacyWorkflowSteps(): Promise<{ + migrated: number; + skipped: number; + combinedWorkflowId?: string; + }> { + // Resolve async prerequisites BEFORE the synchronous transaction: the + // workflow-columns flag (for flag-aware persistence). The project default is + // re-read AFTER the transaction (compare-and-set) so a concurrently-set + // default is never clobbered. + const flagOn = await this.workflowColumnsFlagOn(); + + const result = this.db.transactionImmediate(() => { + // Write lock is now held. Read the raw step rows directly (the cached, + // plugin-merged listWorkflowSteps() is not transaction-scoped). Mirror + // listWorkflowSteps()'s compiled-materialized filter and toStoredWorkflowStep + // mapping so policy decisions match the user-facing step listing. + const rows = this.db + .prepare("SELECT * FROM workflow_steps ORDER BY createdAt ASC") + .all() as Array[0]>; + + const userSteps = rows + .map((row) => this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(row))) + // Compiled-materialized rows are an execution detail, not user-authored. + .filter((step) => !step.templateId?.startsWith(WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX)); + + const alreadyMigrated = userSteps.filter((s) => s.migratedFragmentId); + const unmigrated = userSteps.filter((s) => !s.migratedFragmentId); + + if (unmigrated.length === 0) { + return { migrated: 0, skipped: alreadyMigrated.length, combinedWorkflowId: undefined as string | undefined }; + } + + // Every unmigrated user step → a single-node fragment; stamp the source row. + for (const step of unmigrated) { + // parseWorkflowIr runs inside both insertWorkflowDefinitionSync and + // layoutForIr, so compute the fragment IR once and reuse it. + const fragmentIr = stepToFragmentIr(step); + const fragment = this.insertWorkflowDefinitionSync( + { + name: step.name, + description: step.description, + kind: "fragment", + ir: fragmentIr, + layout: layoutForIr(fragmentIr), + }, + flagOn, + ); + this.db + .prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?") + .run(fragment.id, new Date().toISOString(), step.id); + } + this.workflowStepsCache = null; + this.db.bumpLastModified(); + + // The defaultOn subset → one combined "Migrated steps" workflow. + const defaultOnSteps = unmigrated.filter((s) => s.defaultOn === true); + let combinedWorkflowId: string | undefined; + if (defaultOnSteps.length > 0) { + const ir = stepsToWorkflowIr(defaultOnSteps, "Migrated steps"); + const combined = this.insertWorkflowDefinitionSync( + { + name: "Migrated steps", + description: "Converted from your legacy workflow steps", + kind: "workflow", + ir, + layout: layoutForIr(ir), + }, + flagOn, + ); + combinedWorkflowId = combined.id; + } + + return { migrated: unmigrated.length, skipped: alreadyMigrated.length, combinedWorkflowId }; + }); + + // Set the combined workflow as the project default — only when one was + // created AND no explicit default is already set (don't clobber a user + // choice). Done outside the transaction via the async setter so the project + // default-workflow hooks run. Compare-and-set against the CURRENT default + // (re-read immediately before writing, not the pre-transaction snapshot) so + // a default set concurrently by another writer is never overwritten. If the + // set fails, swallow the error: a missing migrated default is recoverable + // (the user can set one), but throwing here would surface the whole + // migration as failed even though the definitions were written. + if (result.combinedWorkflowId) { + const currentDefaultId = await this.getDefaultWorkflowId(); + if (!currentDefaultId) { + try { + await this.setDefaultWorkflowId(result.combinedWorkflowId); + } catch (err) { + storeLog.warn("Failed to set migrated combined workflow as project default", { + phase: "migrateLegacyWorkflowSteps:set-default", + combinedWorkflowId: result.combinedWorkflowId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } + + return result; + } + /** Whether a raw workflow CLI command has been approved (trust-on-first-use). * Comparison is on the exact trimmed command string. */ async isWorkflowCliCommandApproved(command: string): Promise { @@ -13250,6 +13497,9 @@ ${stepsSection}`; if (!workflowId) return undefined; const def = await this.getWorkflowDefinition(workflowId); if (!def) return undefined; + // KTD-1/R6: a fragment must never act as a project default (it is not a + // selectable workflow); fall back to no default rather than materializing it. + if (def.kind === "fragment") return undefined; // Compile (and validate) before creating any rows so a non-compilable // default falls back cleanly with nothing written. const inputs = compileWorkflowToSteps(def.ir); @@ -13257,6 +13507,25 @@ ${stepsSection}`; return { workflowId, stepIds }; } + /** Resolve an EXPLICITLY requested workflow id (U6/R3/KTD-4) into materialized + * step ids for the create-time `workflowId` parameter. Unlike + * `materializeDefaultWorkflowSteps`, unknown ids and fragments are hard errors + * (thrown BEFORE any task row is created) rather than silent fallbacks, since + * the caller asked for a specific workflow. Compilation happens up front so a + * non-compilable workflow aborts before any rows are written. */ + private async materializeExplicitWorkflowSteps( + workflowId: string, + ): Promise<{ workflowId: string; stepIds: string[] }> { + const def = await this.getWorkflowDefinition(workflowId); + if (!def) throw new Error(`Workflow '${workflowId}' not found`); + if (def.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); + } + const inputs = compileWorkflowToSteps(def.ir); + const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); + return { workflowId, stepIds }; + } + /** * Select a workflow for a task: compile it, materialize its steps, and write * their ids into the task's enabledWorkflowSteps. Replaces any prior selection @@ -13271,6 +13540,12 @@ ${stepsSection}`; return this.withTaskLock(taskId, async () => { const def = await this.getWorkflowDefinition(workflowId); if (!def) throw new Error(`Workflow '${workflowId}' not found`); + // KTD-1/R6: fragments are reusable single-node palette templates, not + // selectable workflows. Reject them from task selection with a clear error + // rather than materializing a degenerate single-step task. + if (def.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); + } // Compile once up front: a non-linear graph aborts before any mutation. const inputs = compileWorkflowToSteps(def.ir); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index a856b620fb..9ca818d3e6 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -541,6 +541,11 @@ export interface WorkflowStep { * Must be set together with `modelProvider`. When both model fields are undefined, * the executor uses global settings defaults. Only used when mode is "prompt". */ modelId?: string; + /** (workflow-editor-consolidation U1/U2, KTD-1/KTD-3) when this legacy step has + * been migrated into a fragment WorkflowDefinition, the fragment's id is stamped + * here so the lazy step migration is idempotent (already-stamped rows are + * skipped). Stored in the `migrated_fragment_id` column. */ + migratedFragmentId?: string; /** ISO-8601 timestamp of creation */ createdAt: string; /** ISO-8601 timestamp of last update */ @@ -651,6 +656,9 @@ export interface WorkflowStepInput { modelProvider?: string; /** AI model ID override. Must be set together with modelProvider. Only used when mode is "prompt". */ modelId?: string; + /** (workflow-editor-consolidation U2, KTD-3) fragment id stamped when this step + * was migrated into a fragment WorkflowDefinition. Set by the migration only. */ + migratedFragmentId?: string; } /** Result of a workflow step execution on a task. */ @@ -2283,6 +2291,23 @@ export interface TaskCreateInput { noCommitsExpected?: boolean; /** IDs of workflow steps to enable for this task */ enabledWorkflowSteps?: string[]; + /** + * Workflow selection applied atomically at task creation (U6/R3/KTD-4). + * + * Semantics: + * - `undefined` → inherit the project default workflow (today's behavior: + * `materializeDefaultWorkflowSteps` runs, falling back to default-on steps). + * - `null` → explicitly NO workflow: skip default materialization entirely; + * the task is created with no custom workflow steps. + * - `string` → that workflow's compiled steps are materialized and selected + * inside the creation flow, overriding any project default. Fragment IDs + * and unknown IDs are rejected with a clear error BEFORE the task row is + * created. + * + * Mutually exclusive with `enabledWorkflowSteps`: when `enabledWorkflowSteps` + * is provided, it takes precedence and `workflowId` materialization is skipped. + */ + workflowId?: string | null; /** Model preset selected during task creation. Presets resolve to concrete model overrides at creation time. */ modelPresetId?: string; /** AI model provider override for the executor agent (e.g., "anthropic"). diff --git a/packages/core/src/workflow-compiler.ts b/packages/core/src/workflow-compiler.ts index dfbd3f1bcd..dcf7ce0363 100644 --- a/packages/core/src/workflow-compiler.ts +++ b/packages/core/src/workflow-compiler.ts @@ -95,6 +95,10 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { return new WorkflowCompileError(`node '${node.id}' has no outgoing edge`); } if (outs.length > 1) { + // NOTE: the `require the workflow interpreter (deferred)` suffix is matched + // by the dashboard editor (WorkflowNodeEditor handleSave, KTD-4) to render + // an info-tone "interpreter-only" banner instead of an error. Keep both + // interpreter-deferred messages carrying this exact suffix in sync. return new WorkflowCompileError( `node '${node.id}' branches into ${outs.length} edges — graphs with branches require the workflow interpreter (deferred)`, ); @@ -155,6 +159,18 @@ function defaultGateMode(node: WorkflowIrNode, mode: "prompt" | "script"): Workf return mode === "script" ? "gate" : "advisory"; } +/** + * Map a single user IR node onto a WorkflowStepInput. This is the forward half + * of the steps↔IR round-trip contract (workflow-editor-consolidation R4/KTD-2); + * its exact inverse is `stepInputToNode` in `workflow-steps-to-ir.ts`. Parity is + * pinned by `__tests__/workflow-steps-to-ir.test.ts` over exactly the + * compiler-visible fields: name / mode / phase / gateMode / prompt / scriptName / + * toolMode / modelProvider / modelId. `enabled` / `defaultOn` / `templateId` are + * NOT compiler-visible and are handled by migration policy, not the converter. + * + * INVERSION CONTRACT: when you add a field here, extend `stepInputToNode` (and + * the parity test) in `workflow-steps-to-ir.ts` to keep the round-trip exact. + */ function nodeToStepInput(node: WorkflowIrNode, phase: "pre-merge" | "post-merge"): WorkflowStepInput { const scriptName = configString(node, "scriptName"); const mode: "prompt" | "script" = node.kind === "script" || (node.kind === "gate" && scriptName) ? "script" : "prompt"; diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 412bfca6c0..e5f14087d1 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -7,6 +7,12 @@ export interface WorkflowNodeLayout { y: number; } +/** Discriminates a full, selectable workflow from a reusable single-node + * "fragment" template (workflow-editor-consolidation U1, KTD-1). Fragments are + * excluded from task workflow pickers, default-workflow selection, and the + * compile/selection paths; both kinds are stored as parseable full IRs. */ +export type WorkflowDefinitionKind = "workflow" | "fragment"; + /** A named, persisted workflow authored as a WorkflowIr graph plus editor layout. */ export interface WorkflowDefinition { /** Unique identifier (e.g., "WF-001"). */ @@ -15,6 +21,8 @@ export interface WorkflowDefinition { name: string; /** Short description for UI display. */ description: string; + /** Discriminates full workflows from reusable fragment templates (KTD-1). */ + kind: WorkflowDefinitionKind; /** The validated workflow graph (v1 IR contract). */ ir: WorkflowIr; /** Editor node positions keyed by IR node id. May be empty (auto-layout). */ @@ -32,6 +40,9 @@ export interface WorkflowDefinitionInput { /** Workflow graph; validated via parseWorkflowIr on write. */ ir: WorkflowIr; layout?: Record; + /** Discriminates full workflows from reusable fragment templates (KTD-1). + * Defaults to "workflow" when omitted. */ + kind?: WorkflowDefinitionKind; } /** Partial update for an existing workflow definition. */ diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index 793c4e30ae..373eb7d955 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -943,3 +943,42 @@ export function downgradeIrToV1IfPure(ir: WorkflowIr): WorkflowIr { export function serializeWorkflowIr(ir: WorkflowIr): string { return JSON.stringify(ir, null, 2); } + +/** + * Strip the trust-escalating `cliSkipApproval`/`autoApprove` flags from every + * node config in an IR, recursing into foreach `config.template.nodes` at any + * nesting depth (foreach-in-foreach). Mutates the passed IR in place and returns + * it alongside a `stripped` flag indicating whether anything was removed. + * + * These flags bypass the CLI first-run approval gate (see executor.ts). They are + * legitimate only for workflows authored through the trusted dashboard editor / + * executor lane; on prompt-injectable surfaces (chat/planning authoring tools, + * import, AI design) they must be removed at the write boundary. + */ +export function stripApprovalBypassFlags(ir: WorkflowIr): { ir: WorkflowIr; stripped: boolean } { + const nodes = (ir as { nodes?: WorkflowIrNode[] }).nodes; + if (!Array.isArray(nodes)) return { ir, stripped: false }; + let stripped = false; + const stripNode = (node: WorkflowIrNode): void => { + // Untrusted input may contain non-object entries (null, strings, numbers) + // in `nodes` / `template.nodes`; skip them rather than dereferencing. + if (!node || typeof node !== "object") return; + const cfg = node.config as Record | undefined; + if (cfg && typeof cfg === "object") { + if ("cliSkipApproval" in cfg) { + delete cfg.cliSkipApproval; + stripped = true; + } + if ("autoApprove" in cfg) { + delete cfg.autoApprove; + stripped = true; + } + const template = (cfg as { template?: { nodes?: unknown } }).template; + if (template && Array.isArray(template.nodes)) { + for (const inner of template.nodes as WorkflowIrNode[]) stripNode(inner); + } + } + }; + for (const node of nodes) stripNode(node); + return { ir, stripped }; +} diff --git a/packages/core/src/workflow-steps-to-ir.ts b/packages/core/src/workflow-steps-to-ir.ts new file mode 100644 index 0000000000..5d4f3cbe96 --- /dev/null +++ b/packages/core/src/workflow-steps-to-ir.ts @@ -0,0 +1,162 @@ +import type { WorkflowStep } from "./types.js"; +import type { WorkflowIr, WorkflowIrNode, WorkflowIrEdge } from "./workflow-ir-types.js"; +import type { WorkflowNodeLayout } from "./workflow-definition-types.js"; +import { parseWorkflowIr } from "./workflow-ir.js"; + +/** + * Steps → IR converter (workflow-editor-consolidation U1, R4/KTD-2). + * + * This module is the exact INVERSE of the compiler's `nodeToStepInput` + * (`workflow-compiler.ts`). The round-trip contract is: + * + * compileWorkflowToSteps(stepsToWorkflowIr(steps, name)) ≡ steps + * + * over exactly the compiler-visible fields: name / mode / phase / gateMode / + * prompt / scriptName / toolMode / modelProvider / modelId. `enabled` / + * `defaultOn` / `templateId` / `migratedFragmentId` are NOT compiler-visible and + * are handled by migration policy (KTD-3), not by this converter. Parity is + * pinned by `__tests__/workflow-steps-to-ir.test.ts`. + * + * INVERSION CONTRACT: when a compiler-visible field is added to `nodeToStepInput` + * (see the contract comment there), extend `stepInputToNode` below and the parity + * test to keep the round-trip exact. + * + * Seam encoding mirrors `linear()` in `builtin-workflows.ts` exactly: the fixed + * execute → review → merge pipeline is emitted as prompt-kind nodes carrying + * `config.seam`, chained by `success` edges, with each seam also wired + * `failure → end`. + */ + +/** The fixed seam pipeline, in canonical order. The `merge` seam is the + * pre-/post-merge boundary and is always emitted (R4). */ +const SEAM_ORDER = ["execute", "review", "merge"] as const; + +/** Horizontal spacing used by `linear()`; reused so migrated graphs lay out the + * same way built-ins do. */ +const LAYOUT_X0 = 60; +const LAYOUT_DX = 170; +const LAYOUT_Y = 160; + +/** + * Inverse of `nodeToStepInput` (workflow-compiler.ts). Produces a single user IR + * node whose forward compilation reproduces every compiler-visible field of the + * given step. + * + * kind ↔ mode/gateMode mapping (the heart of the contract): + * - mode "script" → kind "script", `config.scriptName` set. The compiler reads + * mode from `kind === "script"`, so this round-trips to mode "script". + * - mode "prompt" → kind "prompt", `config.prompt`/`toolMode`/model overrides. + * - gateMode is ALWAYS written to `config.gateMode` (both "gate" and "advisory"). + * The compiler's `defaultGateMode` returns an explicit `config.gateMode` for + * non-gate-kind nodes verbatim, so this round-trips for both modes without + * needing the `gate` node kind (which the compiler only emits via scriptName + * heuristics — using explicit `config.gateMode` keeps the inverse total). + */ +function stepInputToNode(step: WorkflowStep, id: string): WorkflowIrNode { + const config: Record = { + name: step.name, + // Always carry gateMode so the compiler reproduces it exactly for both modes. + gateMode: step.gateMode, + }; + if (step.description) config.description = step.description; + + if (step.mode === "script") { + if (step.scriptName) config.scriptName = step.scriptName; + return { id, kind: "script", config }; + } + + // prompt mode + config.prompt = step.prompt ?? ""; + config.toolMode = step.toolMode === "coding" ? "coding" : "readonly"; + // Model overrides only round-trip when BOTH are present (compiler requirement). + if (step.modelProvider && step.modelId) { + config.modelProvider = step.modelProvider; + config.modelId = step.modelId; + } + return { id, kind: "prompt", config }; +} + +/** Build a seam node exactly as `linear()` does: a prompt-kind node tagged with + * `config.seam`. */ +function seamNode(seam: (typeof SEAM_ORDER)[number]): WorkflowIrNode { + return { id: seam, kind: "prompt", config: { seam } }; +} + +/** + * Convert an ordered `WorkflowStep[]` into a valid v1 WorkflowIr: + * + * start → [pre-merge user nodes] → execute → review → merge + * → [post-merge user nodes] → end + * + * Steps with `phase` undefined map to pre-merge (R4). Seam nodes get an extra + * `failure → end` edge, mirroring `linear()`. The result always passes + * `parseWorkflowIr`. An empty step list yields the minimal seam-only pipeline + * (which compiles back to `[]`). + */ +export function stepsToWorkflowIr(steps: WorkflowStep[], name: string): WorkflowIr { + const preMerge = steps.filter((s) => (s.phase ?? "pre-merge") === "pre-merge"); + const postMerge = steps.filter((s) => s.phase === "post-merge"); + + const nodes: WorkflowIrNode[] = [{ id: "start", kind: "start" }]; + const userNodeIds = new Set(); + + // Deterministic ids that cannot collide with the reserved start/end/seam ids. + const userNode = (step: WorkflowStep, index: number): WorkflowIrNode => { + let id = `step-${index + 1}`; + while (userNodeIds.has(id)) id = `${id}-x`; + userNodeIds.add(id); + return stepInputToNode(step, id); + }; + + preMerge.forEach((step, i) => nodes.push(userNode(step, i))); + // Fixed execute → review → merge seam pipeline; merge is the boundary (R4). + for (const seam of SEAM_ORDER) nodes.push(seamNode(seam)); + postMerge.forEach((step, i) => nodes.push(userNode(step, preMerge.length + i))); + nodes.push({ id: "end", kind: "end" }); + + const edges: WorkflowIrEdge[] = []; + for (let i = 0; i < nodes.length - 1; i += 1) { + edges.push({ from: nodes[i].id, to: nodes[i + 1].id, condition: "success" }); + } + // Seam nodes also fail straight to end (mirrors `linear()` / the legacy pipeline). + for (const node of nodes) { + if (typeof node.config?.seam === "string") { + edges.push({ from: node.id, to: "end", condition: "failure" }); + } + } + + return parseWorkflowIr({ version: "v1", name, nodes, edges }); +} + +/** + * Convert a single `WorkflowStep` into a minimal fragment IR (R6/KTD-1): + * + * start → node → end + * + * No seams. The node mirrors the step via `stepInputToNode`. The result passes + * `parseWorkflowIr` and is a pure-v1 graph (survives `downgradeIrToV1IfPure`). + */ +export function stepToFragmentIr(step: WorkflowStep): WorkflowIr { + const node = stepInputToNode(step, "step-1"); + return parseWorkflowIr({ + version: "v1", + name: step.name, + nodes: [{ id: "start", kind: "start" }, node, { id: "end", kind: "end" }], + edges: [ + { from: "start", to: node.id, condition: "success" }, + { from: node.id, to: "end", condition: "success" }, + ], + }); +} + +/** + * Deterministic x-spaced layout for an IR, matching `linear()`'s geometry. Keyed + * by node id; supply alongside the IR when persisting a `WorkflowDefinitionInput`. + */ +export function layoutForIr(ir: WorkflowIr): Record { + const layout: Record = {}; + ir.nodes.forEach((node, i) => { + layout[node.id] = { x: LAYOUT_X0 + i * LAYOUT_DX, y: LAYOUT_Y }; + }); + return layout; +} diff --git a/packages/dashboard/app/App.tsx b/packages/dashboard/app/App.tsx index ec73184f45..34504f4f09 100644 --- a/packages/dashboard/app/App.tsx +++ b/packages/dashboard/app/App.tsx @@ -1227,9 +1227,9 @@ function AppInner() { pushNav({ type: "modal", close: modalManager.closeScripts }); }, [modalManager, pushNav]); - const openWorkflowStepsWithNav = useCallback(() => { - modalManager.openWorkflowSteps(); - pushNav({ type: "modal", close: modalManager.closeWorkflowSteps }); + const openWorkflowEditorWithNav = useCallback(() => { + modalManager.openWorkflowEditor(); + pushNav({ type: "modal", close: modalManager.closeWorkflowEditor }); }, [modalManager, pushNav]); const openUsageWithNav = useCallback((anchorRect?: DOMRect | null) => { @@ -1803,7 +1803,7 @@ function AppInner() { onOpenGitManager={openGitManagerWithNav} onOpenNodes={handleOpenNodesWithNav} showNodesButton={nodesEnabled} - onOpenWorkflowSteps={openWorkflowStepsWithNav} + onOpenWorkflowEditor={openWorkflowEditorWithNav} onOpenScripts={openScriptsWithNav} onRunScript={runScriptWithNav} onToggleTerminal={toggleTerminalWithNav} @@ -2007,7 +2007,7 @@ function AppInner() { chatHasUnreadResponse={chatHasUnreadResponse} stashOrphanCount={stashOrphanCount} onOpenGitManager={openGitManagerWithNav} - onOpenWorkflowSteps={openWorkflowStepsWithNav} + onOpenWorkflowEditor={openWorkflowEditorWithNav} onOpenSchedules={openSchedulesWithNav} onOpenScripts={openScriptsWithNav} onToggleTerminal={toggleTerminalWithNav} diff --git a/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx b/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx index b6688642a8..6a56028d42 100644 --- a/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx +++ b/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx @@ -49,7 +49,7 @@ const createDefaultMobileNavProps = () => ({ onOpenNodes: vi.fn(), mailboxUnreadCount: 0, onOpenGitManager: vi.fn(), - onOpenWorkflowSteps: vi.fn(), + onOpenWorkflowEditor: vi.fn(), onOpenSchedules: vi.fn(), onOpenScripts: vi.fn(), onToggleTerminal: vi.fn(), diff --git a/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx b/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx index bcb2e9fb32..eca5a3e11f 100644 --- a/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx +++ b/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx @@ -178,9 +178,9 @@ describe("tablet header controls", () => { expect(screen.queryByTitle("Git Manager")).toBeNull(); }); - it("does not render workflow steps button inline on tablet", () => { - renderTabletHeader({ onOpenWorkflowSteps: noop }); - expect(screen.queryByTitle("Workflow Steps")).toBeNull(); + it("does not render workflows button inline on tablet", () => { + renderTabletHeader({ onOpenWorkflowEditor: noop }); + expect(screen.queryByTitle("Workflows")).toBeNull(); }); // ── Overflow menu on tablet ──────────────────────────────────── @@ -254,8 +254,8 @@ describe("tablet header controls", () => { expect(screen.getByTestId("overflow-git-btn")).toBeDefined(); }); - it("overflow menu contains workflow steps on tablet when provided", () => { - renderTabletHeader({ onOpenWorkflowSteps: noop }); + it("overflow menu contains workflows on tablet when provided", () => { + renderTabletHeader({ onOpenWorkflowEditor: noop }); fireEvent.click(screen.getByTitle("More header actions")); expect(screen.getByTestId("overflow-workflow-steps-btn")).toBeDefined(); }); @@ -538,7 +538,7 @@ describe("tablet header controls", () => { const { container } = renderTabletHeader({ onOpenUsage: noop, onOpenActivityLog: noop, - onOpenWorkflowSteps: noop, + onOpenWorkflowEditor: noop, onOpenFiles: noop, onOpenGitManager: noop, }); diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index 8c4b40e8a6..fbad170578 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -371,6 +371,7 @@ export async function createTask( dependencies, breakIntoSubtasks, enabledWorkflowSteps, + workflowId, assignedAgentId, modelPresetId, modelProvider, @@ -407,6 +408,7 @@ export async function createTask( dependencies, breakIntoSubtasks, enabledWorkflowSteps, + workflowId, assignedAgentId, modelPresetId, modelProvider, @@ -5108,6 +5110,106 @@ export function compileWorkflow(id: string, projectId?: string): Promise<{ steps }); } +/** A workflow export envelope (U5/R9/KTD-5). `schemaVersion` is the SERVER's + * schema version at export time — the import route version-gates against it + * (the app build aliases @fusion/core to types-only, so the value can only come + * from the server, never an app-side core import). */ +export interface WorkflowExportEnvelope { + fusionWorkflowExport: 1; + schemaVersion: number; + kind: import("@fusion/core").WorkflowDefinition["kind"]; + name: string; + description: string; + ir: import("@fusion/core").WorkflowIr; + layout: import("@fusion/core").WorkflowDefinition["layout"]; +} + +/** Fetch a workflow's export envelope and trigger a browser download as + * `.workflow.json` (U5/R9). Built-ins are exportable too. Mirrors the + * SettingsModal export pattern (Blob + createObjectURL + a.download). */ +export async function exportWorkflow(id: string, projectId?: string): Promise { + const envelope = await api( + withProjectId(`/workflows/${encodeURIComponent(id)}/export`, projectId), + ); + const safeName = (envelope.name || "workflow").replace(/[^\w.-]+/g, "-").replace(/^-+|-+$/g, "") || "workflow"; + const blob = new Blob([JSON.stringify(envelope, null, 2)], { type: "application/json" }); + const url = URL.createObjectURL(blob); + const link = document.createElement("a"); + link.href = url; + link.download = `${safeName}.workflow.json`; + document.body.appendChild(link); + link.click(); + document.body.removeChild(link); + URL.revokeObjectURL(url); + return envelope; +} + +/** Result of POST /api/workflows/import (U5/R10). `strippedApprovalFlags` is set + * when `cliSkipApproval`/`autoApprove` were removed from any node config at the + * trust boundary; `warnings` lists non-blocking issues (e.g. unknown scriptName). */ +export interface ImportWorkflowResult { + workflow: import("@fusion/core").WorkflowDefinition; + strippedApprovalFlags: boolean; + warnings: string[]; +} + +/** Import a workflow export envelope (U5/R10). The server is the sole validator; + * validation failures reject with an ApiError carrying the server message. */ +export function importWorkflow( + envelope: unknown, + projectId?: string, +): Promise { + return api(withProjectId("/workflows/import", projectId), { + method: "POST", + body: JSON.stringify(envelope), + }); +} + +/** Result of the lazy legacy-step migration (U2/R5). `migrated` is the number of + * newly converted user steps; `skipped` the count already migrated; when the + * defaultOn subset was non-empty a combined "Migrated steps" workflow id is set. */ +export interface MigrateLegacyStepsResult { + migrated: number; + skipped: number; + combinedWorkflowId?: string; +} + +/** Run the lazy, idempotent migration of legacy user-authored workflow steps into + * fragments + a combined workflow (U2/R5). Safe to call repeatedly. */ +export function migrateLegacyWorkflowSteps(projectId?: string): Promise { + return api(withProjectId("/workflows/migrate-legacy-steps", projectId), { + method: "POST", + }); +} + +/** Result of POST /api/workflows/design (U10/R11). The server validates the + * AI-produced IR (parseWorkflowIr), triages compilability (`interpreterOnly`), + * and strips trust-escalating flags (`strippedApprovalFlags`). Persists nothing + * — the client decides what to do with the returned graph. */ +export interface DesignWorkflowResult { + ir: import("@fusion/core").WorkflowIr; + layout: import("@fusion/core").WorkflowDefinition["layout"]; + interpreterOnly: boolean; + strippedApprovalFlags: boolean; +} + +/** Design a workflow from a natural-language prompt (U10/R11). When `workflowId` + * is supplied the route reads that workflow's persisted IR server-side and folds + * it into the prompt as the base graph (the client never posts IR). An optional + * AbortSignal cancels the in-flight request. Validation failures reject with an + * ApiError carrying the server message; 429 on rate limit. */ +export function designWorkflow( + input: { prompt: string; workflowId?: string }, + projectId?: string, + signal?: AbortSignal, +): Promise { + return api(withProjectId("/workflows/design", projectId), { + method: "POST", + body: JSON.stringify(input), + signal, + }); +} + /** Read the workflow currently selected for a task. */ export function fetchTaskWorkflow(taskId: string, projectId?: string): Promise<{ workflowId: string | null }> { return api<{ workflowId: string | null }>( diff --git a/packages/dashboard/app/components/AppModals.tsx b/packages/dashboard/app/components/AppModals.tsx index 9b543557c1..3b266568f4 100644 --- a/packages/dashboard/app/components/AppModals.tsx +++ b/packages/dashboard/app/components/AppModals.tsx @@ -20,7 +20,6 @@ import { NewTaskModal } from "./NewTaskModal"; import { SystemStatsModal } from "./SystemStatsModal"; import { ActivityLogModal } from "./ActivityLogModal"; import { GitManagerModal } from "./GitManagerModal"; -import { WorkflowStepManager } from "./WorkflowStepManager"; import { AgentListModal } from "./AgentListModal"; import { ModelOnboardingModal } from "./ModelOnboardingModal"; import { ToastContainer } from "./ToastContainer"; @@ -373,19 +372,6 @@ export function AppModals({ /> - - { - modalManager.closeWorkflowSteps(); - modalManager.openWorkflowEditor(); - }} - /> - - {modalManager.workflowEditorOpen && ( diff --git a/packages/dashboard/app/components/Header.tsx b/packages/dashboard/app/components/Header.tsx index 17e3143235..034afa89a4 100644 --- a/packages/dashboard/app/components/Header.tsx +++ b/packages/dashboard/app/components/Header.tsx @@ -197,7 +197,7 @@ export interface HeaderProps { onOpenNodes?: () => void; /** When false, hides the Nodes management button. Defaults to true for backward compat. */ showNodesButton?: boolean; - onOpenWorkflowSteps?: () => void; + onOpenWorkflowEditor?: () => void; onOpenScripts?: () => void; onRunScript?: (name: string, command: string) => void; onToggleTerminal?: () => void; @@ -266,7 +266,7 @@ export function Header({ onOpenGitManager, onOpenNodes, showNodesButton, - onOpenWorkflowSteps, + onOpenWorkflowEditor, onOpenScripts, onRunScript, onToggleTerminal, @@ -1593,12 +1593,12 @@ export function Header({ )} - {/* Workflow Steps - desktop only (moved to overflow on mobile/tablet) */} - {!isCompact && onOpenWorkflowSteps && ( + {/* Workflows - desktop only (moved to overflow on mobile/tablet) */} + {!isCompact && onOpenWorkflowEditor && ( )} - {/* Workflow Steps - in overflow on mobile */} - {onOpenWorkflowSteps && ( + {/* Workflows - in overflow on mobile */} + {onOpenWorkflowEditor && ( )} {/* Settings - always last in overflow menu */} diff --git a/packages/dashboard/app/components/MobileNavBar.tsx b/packages/dashboard/app/components/MobileNavBar.tsx index ac421e502d..6ae76d6df0 100644 --- a/packages/dashboard/app/components/MobileNavBar.tsx +++ b/packages/dashboard/app/components/MobileNavBar.tsx @@ -60,7 +60,7 @@ export interface MobileNavBarProps { chatHasUnreadResponse?: boolean; stashOrphanCount?: number; onOpenGitManager?: () => void; - onOpenWorkflowSteps?: () => void; + onOpenWorkflowEditor?: () => void; onOpenSchedules?: () => void; onOpenScripts?: () => void; onToggleTerminal?: () => void; @@ -127,7 +127,7 @@ export function MobileNavBar({ chatHasUnreadResponse = false, stashOrphanCount = 0, onOpenGitManager, - onOpenWorkflowSteps, + onOpenWorkflowEditor, onOpenSchedules, onOpenScripts, onToggleTerminal, @@ -590,10 +590,10 @@ export function MobileNavBar({ type="button" className="mobile-more-item" data-testid="mobile-more-item-workflow" - onClick={() => handleMoreAction(onOpenWorkflowSteps)} + onClick={() => handleMoreAction(onOpenWorkflowEditor)} > - {t("nav.workflowSteps", "Workflow Steps")} + {t("nav.workflows", "Workflows")} - - - - - ); - })} - - )} - + )} {(onGithubTrackingEnabledChange || onGithubRepoOverrideChange) && (
diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index b25d2f9c90..de7943da73 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -35,6 +35,37 @@ cursor: pointer; } +/* U2/R5: one-time legacy-step migration notice banner. */ +.wf-migration-notice { + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-sm); + padding: var(--space-sm) var(--space-md); + background: var(--accent-subtle, rgba(59, 130, 246, 0.12)); + border-bottom: 1px solid var(--border); + color: var(--text); + font-size: 0.85rem; +} + +.wf-migration-notice-text { + flex: 1; +} + +.wf-migration-notice-dismiss { + display: inline-flex; + align-items: center; + background: transparent; + border: none; + color: var(--text-muted); + cursor: pointer; + flex-shrink: 0; +} + +.wf-migration-notice-dismiss:hover { + color: var(--text); +} + .wf-editor-close:hover { color: var(--text); } @@ -49,12 +80,76 @@ display: flex; flex-direction: column; gap: var(--space-xs); - width: 220px; + width: 300px; padding: var(--space-sm); border-right: 1px solid var(--border); overflow-y: auto; } +/* U12: columns + fields authoring sections moved into the left sidebar, below + the workflow list. Each is a collapsible disclosure whose toggle button is the + section header; the panels' own internal

is suppressed to avoid a double + header. */ +.wf-sidebar-panels { + display: flex; + flex-direction: column; + gap: var(--space-xs); + margin-top: var(--space-sm); + padding-top: var(--space-sm); + border-top: 1px solid var(--border); +} + +.wf-sidebar-section { + display: flex; + flex-direction: column; + gap: var(--space-xs); +} + +.wf-sidebar-section-toggle { + display: flex; + align-items: center; + gap: var(--space-xs); + width: 100%; + padding: var(--space-xs) var(--space-sm); + background: transparent; + border: none; + border-radius: var(--radius-sm); + color: var(--text); + font-size: 0.7rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.04em; + cursor: pointer; +} + +.wf-sidebar-section-toggle:hover { + background: var(--bg-secondary); +} + +/* When nested in the sidebar disclosure, the panels are stacked full-width + blocks rather than side columns: drop the dedicated width/border and let them + flow inside the sidebar's own scroll. */ +.wf-sidebar-section .wf-column-panel, +.wf-sidebar-section .wf-fields-panel { + width: auto; + min-width: 0; + padding: 0 var(--space-xs) var(--space-xs); + border-left: none; + overflow-y: visible; +} + +/* The disclosure toggle is the visible section header; hide the panels' own + title heading to avoid a duplicate. The Add button (also in the header) stays. */ +.wf-sidebar-section .wf-column-panel-header h3, +.wf-sidebar-section .wf-fields-panel-header h3 { + display: none; +} + +.wf-sidebar-section .wf-column-panel-header, +.wf-sidebar-section .wf-fields-panel-header { + justify-content: flex-end; +} + .wf-editor-new { display: inline-flex; align-items: center; @@ -71,6 +166,54 @@ background: var(--bg-tertiary); } +/* U5/R10: sidebar import affordance + persistent inline error/warning regions. */ +.wf-editor-import { + display: inline-flex; + align-items: center; + gap: var(--space-xs); + padding: var(--space-xs) var(--space-sm); + background: var(--bg-secondary); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + color: var(--text); + cursor: pointer; +} + +.wf-editor-import:hover { + background: var(--bg-tertiary); +} + +.wf-editor-import:disabled { + opacity: 0.6; + cursor: default; +} + +.wf-editor-import-error { + padding: var(--space-xs) var(--space-sm); + background: color-mix(in srgb, var(--ws-error) 8%, var(--bg-secondary)); + border: 1px solid var(--ws-error); + border-radius: var(--radius-sm); + color: var(--ws-error); + font-size: 0.8rem; +} + +.wf-editor-import-warnings { + padding: var(--space-xs) var(--space-sm); + background: color-mix(in srgb, var(--ws-warning) 8%, var(--bg-secondary)); + border: 1px solid var(--ws-warning); + border-radius: var(--radius-sm); + color: var(--ws-warning); + font-size: 0.8rem; +} + +.wf-editor-import-warning { + margin: 0; +} + +.wf-editor-import-warning + .wf-editor-import-warning { + margin-top: var(--space-xs); +} + .wf-editor-list { list-style: none; margin: 0; @@ -121,6 +264,52 @@ justify-content: center; } +/* No-workflow onboarding panel (R9): icon + heading + explanation + create CTA. */ +.wf-editor-onboard { + flex-direction: column; + text-align: center; + gap: var(--space-sm); + padding: var(--space-lg); +} + +.wf-editor-onboard-icon { + color: var(--text-muted); +} + +.wf-editor-onboard-title { + margin: 0; + font-size: 1rem; + color: var(--text); +} + +.wf-editor-onboard-text { + margin: 0; + max-width: 36ch; + color: var(--text-muted); +} + +.wf-editor-onboard-cta { + margin-top: var(--space-xs); +} + +/* Trivial-graph palette hint (R9): non-blocking banner over the canvas. */ +.wf-trivial-hint { + position: absolute; + top: var(--space-sm); + left: 50%; + transform: translateX(-50%); + z-index: 5; + pointer-events: none; + max-width: min(90%, 42ch); + padding: var(--space-xs) var(--space-sm); + background: color-mix(in srgb, var(--ws-info) 8%, var(--bg-secondary)); + border: 1px solid var(--ws-info); + border-radius: var(--radius-sm); + color: var(--ws-info); + font-size: 0.8rem; + text-align: center; +} + .wf-editor-toolbar { display: flex; align-items: center; @@ -138,6 +327,7 @@ } .wf-palette-btn, +.wf-editor-action, .wf-editor-delete, .wf-editor-save { display: inline-flex; @@ -153,10 +343,16 @@ } .wf-palette-btn:hover, +.wf-editor-action:hover, .wf-editor-delete:hover { background: var(--bg-tertiary); } +.wf-editor-action:disabled { + opacity: 0.6; + cursor: default; +} + .wf-editor-actions { display: flex; align-items: center; @@ -170,6 +366,116 @@ letter-spacing: 0.04em; } +/* U9/R8: palette Templates section — collapsible, grouped, filterable. */ +.wf-templates { + display: flex; + flex-direction: column; + gap: var(--space-xs); + padding: var(--space-sm); + border-bottom: 1px solid var(--border); +} + +.wf-templates-header { + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-sm); +} + +.wf-templates-toggle { + display: inline-flex; + align-items: center; + gap: var(--space-xs); + padding: var(--space-xs) var(--space-sm); + background: transparent; + border: none; + border-radius: var(--radius-sm); + color: var(--text); + font-weight: 600; + cursor: pointer; +} + +.wf-templates-toggle:hover { + background: var(--bg-tertiary); +} + +.wf-templates-filter { + flex: 0 1 220px; + padding: var(--space-xs) var(--space-sm); + background: var(--bg-secondary); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + color: var(--text); + font-size: 0.8rem; +} + +.wf-templates-body { + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.wf-templates-conflict { + padding: var(--space-xs) var(--space-sm); + background: color-mix(in srgb, var(--ws-error) 8%, var(--bg-secondary)); + border: 1px solid var(--ws-error); + border-radius: var(--radius-sm); + color: var(--ws-error); + font-size: 0.8rem; +} + +.wf-templates-group { + display: flex; + flex-direction: column; + gap: var(--space-xs); +} + +.wf-templates-group-title { + margin: 0; + font-size: 0.7rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.04em; + color: var(--text-tertiary); +} + +.wf-templates-entries { + display: flex; + flex-wrap: wrap; + gap: var(--space-xs); +} + +.wf-templates-entry { + display: inline-flex; + align-items: center; + gap: var(--space-xs); + padding: var(--space-xs) var(--space-sm); + background: var(--bg-secondary); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + color: var(--text); + font-size: 0.8rem; + cursor: pointer; + transition: background var(--transition-fast); +} + +.wf-templates-entry:hover { + background: var(--bg-tertiary); +} + +.wf-templates-entry:disabled { + opacity: 0.6; + cursor: default; +} + +.wf-templates-badge { + padding: 0 var(--space-xs); + background: var(--accent-subtle, rgba(59, 130, 246, 0.12)); + border-radius: var(--radius-sm); + color: var(--text-muted); + font-size: 0.7rem; +} + /* Neutralize the fieldset wrapper so it only gates interactivity, not layout. */ .wf-inspector-fields { display: contents; @@ -194,6 +500,12 @@ color: var(--ws-error); } +/* Inspector delete buttons (U3): sit below the field group, sized to the panel. */ +.wf-inspector-delete { + margin-top: var(--space-sm); + justify-content: center; +} + .wf-editor-banner { padding: var(--space-sm) var(--space-md); background: var(--bg-secondary); @@ -202,6 +514,14 @@ font-size: 0.85rem; } +/* Info-tone banner (KTD-4): branching graph runs on the interpreter only — not a + * failure, so it uses the info token rather than the warning treatment. */ +.wf-editor-banner--info { + border-bottom-color: var(--ws-info); + color: var(--ws-info); + background: color-mix(in srgb, var(--ws-info) 6%, var(--bg-secondary)); +} + .wf-editor-canvas { flex: 1; min-height: 0; @@ -278,51 +598,102 @@ } /* Canvas nodes */ +/* ── U1: card-style nodes ── + * Cards have a header row (icon + label + badges + error badge) and an + * independent config-summary row. Sizing mirrors WF_CARD_WIDTH / + * WF_CARD_MAX_WIDTH in workflow-flow-mapping.ts; the max-width ceiling forces + * long labels/summaries to truncate rather than grow the canvas. Per-kind + * accent uses a left border + a faint header tint over a semantic token. */ .wf-node { - display: inline-flex; - align-items: center; - gap: var(--space-xs); + display: flex; + flex-direction: column; + gap: 2px; + box-sizing: border-box; + width: 200px; + max-width: 240px; padding: var(--space-xs) var(--space-sm); background: var(--bg-secondary); border: 1px solid var(--border); + border-left: 3px solid var(--border); border-radius: var(--radius-sm); color: var(--text); font-size: 0.8rem; } +.wf-node-header { + display: flex; + align-items: center; + gap: var(--space-xs); + min-width: 0; +} + +/* Summary row: single line, truncates independently of the header. */ +.wf-node-summary { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 0.68rem; + color: var(--text-muted); +} + +/* Per-kind accent: left border color + a faint header tint. color-mix over + * semantic tokens keeps both themes legible without raw colors. */ .wf-node-start { - border-color: var(--ws-success); + border-left-color: var(--ws-success); } .wf-node-end { - border-color: var(--text-muted); + border-left-color: var(--text-muted); +} + +.wf-node-prompt { + border-left-color: var(--accent, var(--todo)); + background: color-mix(in srgb, var(--accent, var(--todo)) 5%, var(--bg-secondary)); +} + +.wf-node-script { + border-left-color: var(--text-muted); + font-family: var(--font-mono, monospace); } .wf-node-gate { - border-color: var(--ws-warning); + border-left-color: var(--ws-warning); + background: color-mix(in srgb, var(--ws-warning) 6%, var(--bg-secondary)); +} + +.wf-node-hold { + border-left-color: var(--ws-warning); +} + +.wf-node-split, +.wf-node-join { + border-left-color: var(--accent, var(--ws-info)); } .wf-node-merge { - border-color: var(--ws-info); + border-left-color: var(--ws-info); border-style: dashed; } /* ── Step-inversion nodes (KTD-3/4/12/15, U8) ── */ .wf-node-step-execute { - border-color: var(--accent, var(--ws-info)); + border-left-color: var(--accent, var(--ws-info)); + background: color-mix(in srgb, var(--accent, var(--ws-info)) 5%, var(--bg-secondary)); } .wf-node-step-review { - border-color: var(--ws-info); + border-left-color: var(--ws-info); + background: color-mix(in srgb, var(--ws-info) 6%, var(--bg-secondary)); } .wf-node-parse-steps { - border-color: var(--ws-info); + border-left-color: var(--ws-info); } .wf-node-code { - border-color: var(--text-muted); + border-left-color: var(--text-muted); font-family: var(--font-mono, monospace); } @@ -367,6 +738,16 @@ stroke-width: 2; } +/* Failure edges (R2): a distinct dash pattern from rework plus an error-token + * stroke. Two-channel rule — the condition label is always rendered (third + * channel is color) so failure edges stay distinguishable in low-contrast + * themes. */ +.react-flow__edge.wf-edge-failure .react-flow__edge-path { + stroke: var(--ws-error); + stroke-dasharray: 2 4; + stroke-width: 2; +} + .wf-code-source { font-family: var(--font-mono, monospace); font-size: 0.72rem; @@ -374,12 +755,24 @@ overflow-x: auto; } +/* Header overflow priority (R1): icon fixed-width; label flex-shrinks first + * with ellipsis; badges + error badge hold their width flush right. */ .wf-node-icon { display: inline-flex; + flex-shrink: 0; color: var(--text-muted); } +.wf-node-label { + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + .wf-node-badge { + flex-shrink: 0; font-size: 0.65rem; text-transform: uppercase; padding: 1px var(--space-xs); @@ -412,12 +805,14 @@ .wf-node--error { border-color: var(--ws-error); + border-left-color: var(--ws-error); } .wf-node-error-badge { display: inline-flex; align-items: center; gap: var(--space-xs); + flex-shrink: 0; font-size: 0.65rem; padding: 1px var(--space-xs); border-radius: var(--radius-sm); @@ -444,6 +839,149 @@ color: var(--bg); } +/* Inline name + description strip (KTD-10). */ +.wf-name-strip { + display: flex; + align-items: baseline; + gap: var(--space-sm); + padding: var(--space-xs) var(--space-sm); + border-bottom: 1px solid var(--border); + flex-wrap: wrap; +} + +.wf-workflow-name, +.wf-workflow-name--readonly { + font-size: 0.95rem; + font-weight: 600; + color: var(--text); + background: none; + border: 1px solid transparent; + border-radius: var(--radius-sm); + padding: 2px var(--space-xs); + cursor: pointer; + text-align: left; +} + +.wf-workflow-name:hover { + background: var(--bg-tertiary); +} + +.wf-workflow-name--readonly { + cursor: default; +} + +.wf-workflow-name-input { + font-size: 0.95rem; + font-weight: 600; + color: var(--text); + background: var(--bg-secondary); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + padding: 2px var(--space-xs); +} + +.wf-workflow-description, +.wf-workflow-description--readonly { + font-size: 0.78rem; + color: var(--text-tertiary); + background: none; + border: 1px solid transparent; + border-radius: var(--radius-sm); + padding: 2px var(--space-xs); + cursor: pointer; + text-align: left; +} + +.wf-workflow-description:hover { + background: var(--bg-tertiary); +} + +.wf-workflow-description--readonly { + cursor: default; +} + +.wf-workflow-description-input { + font-size: 0.78rem; + color: var(--text); + background: var(--bg-secondary); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + padding: 2px var(--space-xs); + min-width: 220px; +} + +/* Create-workflow dialog (KTD-7). */ +/* Template picker (U4/R7): radiogroup of Blank + built-ins + user workflows. */ +.wf-template-list { + display: flex; + flex-direction: column; + gap: var(--space-xs); + max-height: 220px; + overflow-y: auto; + padding: var(--space-xs); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + background: var(--bg-secondary); +} + +.wf-template-section { + margin: var(--space-xs) 0 var(--space-xs); + font-size: 0.7rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.04em; + color: var(--text-tertiary); +} + +.wf-template-option { + display: flex; + flex-direction: column; + gap: var(--space-xs); + padding: var(--space-xs) var(--space-sm); + border: 1px solid transparent; + border-radius: var(--radius-sm); + cursor: pointer; + color: var(--text); +} + +.wf-template-option:hover { + background: var(--bg-hover); +} + +.wf-template-option.selected { + border-color: var(--accent); + background: var(--bg-active); +} + +.wf-template-option:focus-visible { + outline: none; + box-shadow: var(--focus-ring); +} + +.wf-template-option-name { + font-size: 0.85rem; + font-weight: 600; +} + +.wf-template-option-desc { + font-size: 0.78rem; + color: var(--text-muted); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.wf-template-option-count { + font-size: 0.72rem; + color: var(--text-tertiary); +} + +.wf-create-error { + margin: var(--space-xs) 0 0; + font-size: 0.8rem; + color: var(--ws-error); +} + .wf-column-panel { display: flex; flex-direction: column; @@ -527,3 +1065,83 @@ text-transform: uppercase; color: var(--text-tertiary); } + +/* ── U10/R11: Design-with-AI affordances ─────────────────────────────────── */ + +/* Create-dialog disclosure (above the template picker). */ +.wf-ai-create { + display: flex; + flex-direction: column; + gap: var(--space-xs); + padding: var(--space-sm); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + background: var(--bg-secondary); +} + +.wf-ai-toggle { + display: inline-flex; + align-items: center; + gap: var(--space-xs); + align-self: flex-start; + padding: var(--space-xs) var(--space-xs); + background: transparent; + border: none; + border-radius: var(--radius-sm); + color: var(--accent); + font-size: 0.82rem; + font-weight: 600; + cursor: pointer; +} + +.wf-ai-toggle:hover { + background: var(--bg-hover); +} + +.wf-ai-create-body { + display: flex; + flex-direction: column; + gap: var(--space-xs); +} + +.wf-ai-prompt { + width: 100%; + resize: vertical; + padding: var(--space-xs) var(--space-sm); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + background: var(--bg); + color: var(--text); + font-size: 0.85rem; +} + +.wf-ai-prompt:focus-visible { + outline: none; + box-shadow: var(--focus-ring); +} + +.wf-ai-actions { + display: flex; + gap: var(--space-xs); +} + +/* Toolbar popover panel anchored under the "Design with AI" button. */ +.wf-ai-edit-wrap { + position: relative; +} + +.wf-ai-panel { + position: absolute; + top: calc(100% + var(--space-xs)); + right: 0; + z-index: 20; + width: 320px; + display: flex; + flex-direction: column; + gap: var(--space-xs); + padding: var(--space-sm); + border: 1px solid var(--border); + border-radius: var(--radius-md); + background: var(--bg); + box-shadow: var(--shadow-md); +} diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index d40be781f1..56ae6244fd 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -7,7 +7,6 @@ import { Background, Controls, MiniMap, - addEdge, useNodesState, useEdgesState, type Connection, @@ -15,8 +14,8 @@ import { type Edge as FlowEdge, } from "@xyflow/react"; import { useTranslation } from "react-i18next"; -import { X, Plus, Trash2, Save, MessageSquare, Terminal, Shield, GitMerge, Loader2, HelpCircle, PauseCircle, Split, Merge, Repeat, ClipboardCheck, ListChecks, Code2 } from "lucide-react"; -import type { WorkflowDefinition, WorkflowIrColumn, TraitViolation } from "@fusion/core"; +import { X, Plus, Trash2, Save, MessageSquare, Terminal, Shield, GitMerge, Loader2, HelpCircle, PauseCircle, Split, Merge, Repeat, ClipboardCheck, ListChecks, Code2, LayoutGrid, Workflow, Download, Upload, ChevronDown, ChevronRight, Library, Sparkles } from "lucide-react"; +import type { WorkflowDefinition, WorkflowIrColumn, TraitViolation, WorkflowStepTemplate } from "@fusion/core"; import { getErrorMessage } from "@fusion/core"; import { fetchWorkflows, @@ -24,23 +23,36 @@ import { updateWorkflow, deleteWorkflow, compileWorkflow, + exportWorkflow, + importWorkflow, + designWorkflow, + ApiRequestError, + migrateLegacyWorkflowSteps, fetchModels, fetchAgents, fetchDiscoveredSkills, + fetchWorkflowStepTemplates, + fetchPluginWorkflowStepTemplates, type ModelInfo, } from "../api"; import type { Agent } from "../api"; import type { DiscoveredSkill } from "../api"; import type { ToastType } from "../hooks/useToast"; import { useOverlayDismiss } from "../hooks/useOverlayDismiss"; +import { useConfirm } from "../hooks/useConfirm"; import { useModalResizePersist } from "../hooks/useModalResizePersist"; import { useAppSettings } from "../hooks/useAppSettings"; import { workflowNodeTypes, type WorkflowFlowNodeData, type WorkflowEditorNodeKind } from "./nodes/WorkflowNodeTypes"; +import { WorkflowEditorCatalogContext } from "./nodes/WorkflowEditorCatalogContext"; +import type { NodeSummaryCatalogs } from "./nodes/node-summary"; import { irToFlow, flowToIr, emptyWorkflowIr, emptyWorkflowLayout, + copyIrWithFreshIds, + insertFragment, + fragmentSeamConflicts, columnsOf, fieldsOf, columnsToBandNodes, @@ -50,11 +62,17 @@ import { isColumnBandNode, foreachChildFlowId, shortConditionLabel, + edgeClassName, + edgeConditionEditability, + buildConnectionEdge, + cascadeDelete, + WF_EDGE_INTERACTION_WIDTH, FOREACH_GROUP_WIDTH, FOREACH_GROUP_HEIGHT, FOREACH_CHILD_X, FOREACH_CHILD_Y, } from "./workflow-flow-mapping"; +import { autoLayout, applyAutoLayout } from "./workflow-auto-layout"; import { fetchTraits, fetchStepParsers, type TraitCatalogEntry } from "../api"; import { WorkflowColumnPanel } from "./WorkflowColumnPanel"; import { WorkflowFieldsPanel } from "./WorkflowFieldsPanel"; @@ -81,6 +99,29 @@ function parseModelDropdownValue(value: string): { provider: string; modelId: st return { provider: value.slice(0, slashIndex), modelId: value.slice(slashIndex + 1) }; } +/** Normalized serialization of the editor's authoring state for dirty tracking + * (U4). Serializes nodes/edges through flowToIr (so mapping-layer defaults are + * materialized identically on the loaded and live sides) plus the editor-owned + * name/description and the resulting layout (auto-layout/drag position changes + * count as dirty). Returns a stable JSON string for cheap equality. */ +function serializeGraph( + name: string, + description: string, + nodes: FlowNode[], + edges: FlowEdge[], + columns: WorkflowIrColumn[], + fields: WorkflowFieldDefinition[], +): string { + const { ir, layout } = flowToIr( + name, + nodes, + edges, + columns.length ? columns : undefined, + fields.length ? fields : undefined, + ); + return JSON.stringify({ name, description, ir, layout }); +} + interface WorkflowNodeEditorProps { isOpen: boolean; onClose: () => void; @@ -119,6 +160,449 @@ const PALETTE: Array<{ kind: WorkflowEditorNodeKind; label: string; icon: typeof { kind: "code", label: "Code", icon: Code2, presetConfig: { source: "" } }, ]; +/** Map a step template to a single pre-configured editor node (kind + config), + * mirroring the U1 `stepInputToNode` converter's field mapping (mode → kind; + * prompt/scriptName/toolMode/gateMode/model overrides → config). Inserting one + * template thus produces the same node the steps→IR migration would. */ +function stepTemplateToNode(tpl: WorkflowStepTemplate): { + kind: WorkflowEditorNodeKind; + label: string; + config: Record; +} { + const config: Record = { + name: tpl.name, + // Always carry gateMode so a materialized node round-trips both modes. + gateMode: tpl.gateMode ?? "advisory", + }; + if (tpl.description) config.description = tpl.description; + + if (tpl.mode === "script") { + if (tpl.scriptName) config.scriptName = tpl.scriptName; + return { kind: "script", label: tpl.name, config }; + } + + // prompt mode (default) + config.prompt = tpl.prompt ?? ""; + config.toolMode = tpl.toolMode === "coding" ? "coding" : "readonly"; + // Model overrides only round-trip when BOTH are present (compiler requirement). + if (tpl.modelProvider && tpl.modelId) { + config.modelProvider = tpl.modelProvider; + config.modelId = tpl.modelId; + } + return { kind: "prompt", label: tpl.name, config }; +} + +// Node kinds a user authors from the palette. Structural/derived nodes +// (start/end and column bands — which map to data.kind "start") are excluded, so +// a fresh start→end graph counts as trivial. Used by the palette-hint (R9). +const USER_NODE_KINDS: ReadonlySet = new Set([ + "prompt", + "script", + "gate", + "code", + "hold", + "split", + "join", + "foreach", + "step-review", + "parse-steps", + "merge", +]); + +/** A pickable creation template: "Blank" (id null) or a copyable source + * workflow (built-in or user kind="workflow"). U4/R7. */ +interface WorkflowCreateTemplate { + /** null = blank; otherwise the source definition's id. */ + id: string | null; + name: string; + description: string; + /** Node count of the source IR (0 for blank). */ + nodeCount: number; + /** Source definition for seeding via copyIrWithFreshIds (absent for blank). */ + source?: WorkflowDefinition; + /** True for built-in sources (grouped separately). */ + builtin: boolean; +} + +/** Local create-workflow dialog (KTD-7). Built on the shared `.modal` primitives + * (precedent: NewTaskModal). Owns its own template/name/description/error state; + * the parent supplies the candidate `workflows` (fragments filtered out here) + * and an async `onCreate` that performs the createWorkflow call and throws on + * failure so the dialog can surface server rejections inline without losing the + * typed input. Escape/overlay close (no dirty state of its own). + * + * U4/R7: a template step precedes the name/description fields — a + * radiogroup-semantics option list (Blank default-selected + built-ins + user + * workflows) navigable by ArrowUp/Down; selecting a template prefills the name + * (" copy") while untouched and inherits the source description. */ +function CreateWorkflowDialog({ + workflows, + onCreate, + onDesign, + onClose, +}: { + workflows: WorkflowDefinition[]; + onCreate: (name: string, description: string, template: WorkflowCreateTemplate) => Promise; + /** U10/R11: design a brand-new workflow from a prompt. Resolves on success + * (the parent seeds + activates the workflow and closes the dialog); throws on + * failure so the dialog surfaces the server message inline without closing. + * `signal` aborts the in-flight design request. */ + onDesign: (prompt: string, name: string, signal: AbortSignal) => Promise; + onClose: () => void; +}) { + const { t } = useTranslation("app"); + const [name, setName] = useState(""); + const [description, setDescription] = useState(""); + const [error, setError] = useState(null); + const [submitting, setSubmitting] = useState(false); + // U10/R11: AI-design disclosure state. `aiOpen` reveals the prompt textarea; + // `aiPrompt` holds the request; `aiBusy` flags the in-flight design call (the + // submit disables + a spinner + Cancel show); `aiError` is the inline failure. + const [aiOpen, setAiOpen] = useState(false); + const [aiPrompt, setAiPrompt] = useState(""); + const [aiBusy, setAiBusy] = useState(false); + const [aiError, setAiError] = useState(null); + const aiAbortRef = useRef(null); + // Tracks whether the user has edited the name; once true, selecting a template + // no longer overwrites it (R7: prefill only when untouched). + const [nameTouched, setNameTouched] = useState(false); + const nameRef = useRef(null); + const optionRefs = useRef>([]); + + // Build the option list: Blank first (default), then built-in workflows, then + // the user's own kind="workflow" definitions. Fragments are excluded entirely. + const templates = useMemo(() => { + const blank: WorkflowCreateTemplate = { + id: null, + name: t("workflows.templateBlank", "Blank"), + description: t("workflows.templateBlankDescription", "Start from an empty start → end graph."), + nodeCount: 0, + builtin: false, + }; + const usable = workflows.filter((w) => w.kind !== "fragment"); + const toTemplate = (w: WorkflowDefinition): WorkflowCreateTemplate => ({ + id: w.id, + name: w.name, + description: w.description ?? "", + nodeCount: w.ir.nodes.length, + source: w, + builtin: isBuiltinWorkflowId(w.id), + }); + const builtins = usable.filter((w) => isBuiltinWorkflowId(w.id)).map(toTemplate); + const yours = usable.filter((w) => !isBuiltinWorkflowId(w.id)).map(toTemplate); + return [blank, ...builtins, ...yours]; + }, [workflows, t]); + + const [selectedIndex, setSelectedIndex] = useState(0); + const selected = templates[selectedIndex] ?? templates[0]; + + useEffect(() => { + nameRef.current?.focus(); + }, []); + + // Apply a template selection: move the radio focus state and (R7) prefill the + // name (" copy") + description from the source, but only while the user + // has not edited the name. + const selectTemplate = useCallback( + (index: number) => { + const tmpl = templates[index]; + if (!tmpl) return; + setSelectedIndex(index); + if (!nameTouched) { + if (tmpl.id === null) { + setName(""); + setDescription(""); + } else { + setName(t("workflows.templateCopyName", "{{name}} copy", { name: tmpl.name })); + setDescription(tmpl.description); + } + } + if (error) setError(null); + }, + [templates, nameTouched, error, t], + ); + + // ArrowUp/Down move the radio selection; Enter confirms and shifts focus to + // the name input. Other keys (incl. Escape) bubble to the dialog handler. + const handleOptionKeyDown = useCallback( + (e: React.KeyboardEvent) => { + if (e.key === "ArrowDown" || e.key === "ArrowRight") { + e.preventDefault(); + const next = Math.min(selectedIndex + 1, templates.length - 1); + selectTemplate(next); + optionRefs.current[next]?.focus(); + } else if (e.key === "ArrowUp" || e.key === "ArrowLeft") { + e.preventDefault(); + const prev = Math.max(selectedIndex - 1, 0); + selectTemplate(prev); + optionRefs.current[prev]?.focus(); + } else if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + selectTemplate(selectedIndex); + nameRef.current?.focus(); + } + }, + [selectedIndex, templates.length, selectTemplate], + ); + + const overlayProps = useOverlayDismiss(onClose); + + const handleSubmit = useCallback( + async (e: React.FormEvent) => { + e.preventDefault(); + const trimmed = name.trim(); + if (!trimmed) { + setError(t("workflows.createNameRequired", "Enter a workflow name")); + return; + } + setSubmitting(true); + setError(null); + try { + await onCreate(trimmed, description.trim(), selected); + // Success path closes the dialog from the parent. + } catch (err) { + setError(getErrorMessage(err) || t("workflows.createFailed", "Failed to create workflow")); + setSubmitting(false); + } + }, + [name, description, selected, onCreate, t], + ); + + // U10/R11: submit the AI design request. On success the parent seeds the + // workflow and closes the dialog; on failure the server message renders inline + // (role="alert") and the dialog stays open. The fetch is cancelable via the + // Cancel button (AbortController); an abort re-enables the controls silently. + const handleAiSubmit = useCallback(async () => { + const trimmed = aiPrompt.trim(); + if (!trimmed) { + setAiError(t("workflows.aiPromptRequired", "Describe the workflow you want")); + return; + } + const controller = new AbortController(); + aiAbortRef.current = controller; + setAiBusy(true); + setAiError(null); + try { + await onDesign(trimmed, name.trim(), controller.signal); + // Success closes the dialog from the parent. + } catch (err) { + if (controller.signal.aborted) { + // User-initiated cancel: re-enable silently (no error message). + return; + } + setAiError(getErrorMessage(err) || t("workflows.aiFailed", "Failed to design workflow")); + } finally { + if (aiAbortRef.current === controller) aiAbortRef.current = null; + setAiBusy(false); + } + }, [aiPrompt, name, onDesign, t]); + + const handleAiCancel = useCallback(() => { + aiAbortRef.current?.abort(); + setAiBusy(false); + }, []); + + // Section boundaries for group headers (built-ins / your workflows). Blank is + // always index 0; built-ins follow, then user workflows. + const firstBuiltinIndex = templates.findIndex((tmpl) => tmpl.id !== null && tmpl.builtin); + const firstYoursIndex = templates.findIndex((tmpl) => tmpl.id !== null && !tmpl.builtin); + + return ( +
+
e.stopPropagation()} + onKeyDown={(e) => { + if (e.key === "Escape") { + e.stopPropagation(); + onClose(); + } + }} + > +
+

{t("workflows.createTitle", "New workflow")}

+ +
+
+
+ {/* U10/R11: AI-design disclosure. Toggling reveals a prompt textarea + + "Design with AI" submit; submitting designs a brand-new workflow + from the result (the parent seeds + activates it). In-flight: the + submit disables + spins, aria-busy is set on the section, and a + Cancel aborts the fetch. Failure renders inline (role="alert"). */} +
+ + {aiOpen && ( +
+