diff --git a/.changeset/step-inversion-workflow-modelable-steps.md b/.changeset/step-inversion-workflow-modelable-steps.md new file mode 100644 index 0000000000..177dfb813a --- /dev/null +++ b/.changeset/step-inversion-workflow-modelable-steps.md @@ -0,0 +1,13 @@ +--- +"@runfusion/fusion": minor +--- + +Make task steps workflow-modelable, behind the `experimentalFeatures.workflowGraphExecutor` flag (off by default). + +Step policy — how a task breaks into steps, how each step is reviewed, and what happens on revision/rethink — was previously fixed engine law. Workflows can now model it as graph structure: a `foreach` node instantiates a per-step template subgraph once per planned step; a `step-review` node surfaces APPROVE/REVISE/RETHINK/UNAVAILABLE verdicts as outcome edges; `rework` edges (the only legal graph cycles, bounded per instance) route revisions back to a `step-execute` seam, with RETHINK triggering a substrate reset-to-baseline (git reset + session rewind). Steps additionally gain parallel execution: with `mode: parallel` + per-instance worktrees, dependency-satisfied steps (declared via `### Step N (depends: 1,2):` annotations) run concurrently off a common base, with an ordered integration stage that lands branches in step order and routes rebase conflicts to a budget-counted rework outcome. + +Step parsing itself becomes a graph node: `parse-steps(artifact, parser)` reads a workflow-declared task artifact and runs a registry parser (built-in `step-headings`/`json-steps`, or plugin-contributed parsers under `plugin::`) to write the step list, with routable `no-steps`/`parse-error` outcomes. A `code` node runs sandboxed TypeScript (esbuild + child process, clamped timeout, no store handle) for arbitrary computed routing/field logic. Workflows also declare typed custom task fields (string/text/number/boolean/enum/multi-enum/date/url, with enum options and render hints); values are validated through a single store authority and the task UI renders the field schema dynamically (detail form widgets, card badges, and a workflow-editor Fields panel). `fn_task_update` accepts a `custom_fields` patch; `fn_workflow_create/update` accept the new IR constructs. + +The default coding workflow is untouched and byte-identical (the parity oracle); a new built-in stepwise coding workflow demonstrates the full modeling. With the flag off, step execution, review, and the board are exactly as before. + +**ROLLBACK:** This is flag-gated by `experimentalFeatures.workflowGraphExecutor` and additive on disk. Schema migration v108 only ADDS the `workflow_run_step_instances` table and the `tasks.customFields` column (default `'{}'`) — it rewrites no existing rows. The flag is read once and pinned per run, so a mid-flight toggle never switches a task between the legacy and graph step paths; flag-off rollback mid-task converges via the existing fell-back + git-reconcile recovery, because `Task.steps[]` remains the always-git-reconcilable projection sink. Instance rows are per-run prunable and are never the authority over git history. IR using the new node kinds (`foreach`/`step-review`/`parse-steps`/`code`) is v2-only, and `downgradeIrToV1IfPure` already refuses non-v1 node kinds, so the v2 rollback contract from the columns track is preserved automatically. To downgrade to a pre-v108 binary, turn the flag off and let in-flight stepwise tasks settle (or reconcile from git) first; custom-field values on the dropped column are lost on downgrade, so export any needed field values beforehand. diff --git a/CONCEPTS.md b/CONCEPTS.md index 7d892fe025..4602719bf1 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -166,6 +166,19 @@ The built-in workflow (`builtin:coding`) that reproduces the legacy pipeline ver ### transitionPending A persisted crash-safe marker (`tasks.transitionPending`) written in the same transaction as a column change, recording the post-commit hooks (`hooksRemaining`) that still owe idempotent execution. Cleared once they complete. Recovery reads it exclusively from SQLite (the authoritative store); a crash mid-transition re-runs the idempotent hooks. A throwing or missing hook degrades (audit) and clears its entry — it never strands the card or wedges the task lock. +## Step inversion + +*Behind the `experimentalFeatures.workflowGraphExecutor` flag (orthogonal to `workflowColumns`). With the flag off, and for the Default workflow always, step policy is the legacy engine-owned path (PROMPT.md parsing, in-session review verdicts, RETHINK reset) — unchanged.* + +### Step instance +One runtime expansion of a `foreach` template subgraph, bound to a single planned step (`Task.steps[i]`). Identity is deterministic — `#:` — so resume reconstructs the full instance set from the pinned step count without persisting the expansion itself. Each instance carries its own run-state (current node, rework count, baseline/checkpoint, and in worktree mode its branch and integration status) in `workflow_run_step_instances` (schema v108). The step count is pinned at expansion; a later disagreement with the live step list is a `pin-mismatch` failure, never a silent re-expansion. An instance's lifecycle writes flow through `store.updateStep` so `Task.steps[]` stays the physical projection sink for every existing consumer. + +### parse-steps +A workflow graph node that reads a declared Artifact and runs a registry parser to write the canonical step list (`Task.steps[]`) — the only graph-side writer of steps. Built-in parsers are `step-headings` (the `### Step N:` convention, extracted byte-identically from the legacy regex, including the `(depends: N,M)` annotation) and `json-steps`; plugins contribute parsers under `plugin::`. Parsing failures fail closed to a routable `outcome:parse-error` rather than crashing. A parse-steps node must dominate (precede on all paths) any `foreach(source:"task-steps")`, and running one after a foreach has already expanded trips pin protection (an audited failure) so re-plan loops cannot desynchronize an expanded region. + +### Custom task field +A workflow-declared, typed task field (`string | text | number | boolean | enum | multi-enum | date | url`, with enum options and render hints) whose values live in `tasks.customFields`, keyed by field id. The task model is thereby recast as core fields (title, description) + standard metadata + these workflow-defined fields. Writes pass through a single store authority (`updateTaskCustomFields`) that validates each value against the resolving workflow's schema and returns typed rejections (offending `fieldId` + `code`); agents write them via `fn_task_update`'s `custom_fields` patch. Editing a workflow's fields or switching a task's workflow orphans (never destroys) values for removed or type-incompatible ids — orphans are retained and surfaced under a detail disclosure, excluded from cards. Same id means the same field within a project; there is no cross-workflow shared field namespace. + ## Flagged ambiguities - "Merging" a shared-branch-group Task had been used for both member integration and group promotion — these are distinct steps with independent gating and must not be conflated. diff --git a/docs/architecture.md b/docs/architecture.md index b23c6e640a..7d7e8d8949 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1247,6 +1247,25 @@ Tune sensitivity by adjusting the exported constants in `stalled-review-detector **Graduation.** The flag default-flip is gated by `computeWorkflowColumnsGraduationReport()` (`workflow-parity.ts`; store method `TaskStore.computeWorkflowColumnsGraduationReport`), aggregating: five-invariant dual-observe parity, default-workflow transition parity vs `VALID_TRANSITIONS` (`checkTransitionParity`), and the U6 dual-accept marker/column disagreement count. `ready` is true only when all gates pass over a non-empty observation window. The report is the gate; it does not flip the flag. +### Step inversion: steps as workflow-modelable nodes (`experimentalFeatures.workflowGraphExecutor`) + +The columns/traits track moved *board* policy (transitions, capacity, hold, merge orchestration) onto the substrate/policy line. The **step-inversion** track extends the same inversion to *task steps* and to the *task shape itself*, riding the existing `workflowGraphExecutor` flag (orthogonal to `workflowColumns`). With the flag off — and for the default coding workflow always — step policy stays exactly as it is today (the monolithic `execute` seam, PROMPT.md `### Step N:` parsing, in-session `fn_review_step` verdicts, RETHINK git-reset/session-rewind). The default workflow is the byte-identical parity oracle; inversion is opt-in via custom workflows and a built-in stepwise coding workflow. + +**One new substrate seam pair.** The substrate gains exactly one new capability, expressed as two methods: `runTaskStep(task, stepIndex)` (run exactly one step inside the task's session and observe its `complete Step N` commit) and `resetStepToBaseline(task, stepIndex, baselineSha, checkpointId?)` (the RETHINK mechanics — git reset + session rewind + `updateStep(...,"pending")`). Both delegate to existing code (extracted from `StepSessionExecutor` and the legacy RETHINK block); neither reimplements step physics or authors commits. The substrate owns *how* a step runs and resets; the graph owns *when*. Baseline/checkpoint state, previously fragile in-memory Maps lost on restart, moves into persisted instance run-state (`workflow_run_step_instances`, schema v108). + +**Everything else becomes authored graph structure (policy).** Step granularity, per-step plan/code review, the verdict→action mapping, rework/escalation routing, parallelism, and even the existence of PROMPT.md stop being engine law: + +- A `parse-steps` node reads a workflow-declared **artifact** (PROMPT.md is just the default workflow's declared `step-source` artifact) and runs a registry **parser** (`step-headings`, `json-steps`, or a plugin-contributed parser) to write `Task.steps[]`. It is the only graph-side step-list writer and must dominate any `foreach`. Parsers fail closed to a routable `outcome:parse-error`. +- A `foreach(source:"task-steps")` node instantiates an inline template subgraph once per planned step, with `mode` (sequential/parallel) and `isolation` (shared/worktree) as explicit axes and per-instance run-state pinned + persisted for crash-safe resume. +- A `step-review` node surfaces reviewer verdicts (APPROVE/REVISE/RETHINK/UNAVAILABLE) as outcome edges; `rework` edges (the only legal graph cycles, bounded per instance) route REVISE/RETHINK back to `step-execute`, with RETHINK traversal triggering the reset seam. +- A `code` node runs sandboxed TypeScript (esbuild + child process, clamped timeout, no store handle) for arbitrary computed routing/field logic — the same trust tier as project-local script steps. + +**`Task.steps[]` stays the physical projection sink.** Instance lifecycle transitions write *through* `store.updateStep` with explicit indices (projection-first ordering closes the merge-blocker race), so every existing consumer — the merge-blocker, dashboard/TUI step display, `reconcileStepsFromGitHistory`, lost-work reset — keeps working unchanged. Git reconcile remains authoritative over the instance rows (rows are corrected to match git, never the reverse). + +**Task shape recast.** The task model reduces to core fields (title, description) + standard metadata + **workflow-defined custom fields** (typed, enum options, render hints; values in `tasks.customFields`, validated through one store authority with typed rejections). Field-schema edits orphan rather than destroy values. This round ships the field *system*; recasting existing built-in fields (priority, labels) onto it is a deferred, additive follow-up. + +**Invariant bar.** The five lifecycle invariants (FN-5147 terminal-until-merged, hard-cancel, in-review stall, file-scope, squash) plus the lost-work guard trio remain the non-configurable correctness bar on the stepwise path. The v108 migration is additive; instance rows are prunable; flag-off rollback mid-task converges via the existing fell-back + git-reconcile recovery (the projection is always git-reconcilable). + ## 10) Agent System Fusion has two complementary agent models: diff --git a/docs/residual-review-findings/gsxdsm-step-inversion.md b/docs/residual-review-findings/gsxdsm-step-inversion.md new file mode 100644 index 0000000000..61e0cf2fb6 --- /dev/null +++ b/docs/residual-review-findings/gsxdsm-step-inversion.md @@ -0,0 +1,17 @@ +# Residual Review Findings — `gsxdsm/step-inversion` + +Source: `ce-code-review mode:autofix` run `20260604-132117-b1269bcd` (12 reviewers) against merge-base `d0b5dcbf`, plan `docs/plans/2026-06-04-001-feat-step-inversion-workflow-modelable-steps-plan.md`. 17 findings were fixed and committed (`3ebaa321f`); the items below remain as tracked residual work. + +## Residual Review Findings + +- [P1] `packages/engine/src/executor.ts:3945` — **Per-instance worktree isolation is commit-cosmetic**: the memoized single implementation pass runs in the MAIN worktree; instance branches receive no per-step commits, so parallel-mode integration rebases empty branches. Requires a per-step `StepSessionExecutor` scoped to `active.worktreePath`. Bookkeeping (rows/worktrees/budgets/ordering) is correct and tested; write-isolation is not yet real. Flag-gated experimental path only. +- [P2] `packages/engine/src/self-healing.ts` — Stale step-instance self-healing sweep (the `recoverStaleTransitionPending` analogue) not implemented; in-progress rows on never-re-dispatched tasks orphan forever (per-run resume seeding exists). +- [P2] `packages/engine/src/plugin-parser-adapter.ts:105` — Plugin parser timeout is post-call, not pre-emptive; a runaway synchronous parser blocks the event loop. +- [P2] `packages/engine/src/workflow-graph-foreach.ts:605` — Integration-conflict retries and reviewer rework share one `maxReworkCycles` budget; repeated conflicts can exhaust it before any REVISE runs. +- [P2] `packages/engine/src/__tests__/stepwise-workflow-parity.test.ts` — Parity oracle is a hand-written legacy simulator, not the real `StepSessionExecutor`; fidelity should be cross-checked against the step-session characterization suite. +- [P2] Test gaps: pin-mismatch grow/shrink on resume; plugin-parser timeout path; explicit `outcome:integration-conflict` edge override; `step-review type:"plan"` via the graph handler; code-node child env-restriction regression; TUI field-chip render. +- [P3] `packages/engine/src/code-node-runner.ts:316` — Temp-dir sweep for parent-crash leftovers (`fusion-code-node-*`). +- [P3] `packages/engine/src/code-node-runner.ts:245` — Compile cache keyed on source hash (N× esbuild spawns per foreach). +- [P3] `packages/engine/src/executor.ts` — Store-capability `as unknown as` casts → optional-capability interface. +- [P3] `packages/core/src/index.ts:127` — `__resetStepParserRegistryForTests` exported on the public barrel; consider a test-support entry point. +- [P3] Agent/dashboard read surface for step-instance state (rework counts, verdicts) — parity debt for when the dashboard surfaces it. diff --git a/docs/solutions/developer-experience/browser-testing-dashboard-from-worktree-safely.md b/docs/solutions/developer-experience/browser-testing-dashboard-from-worktree-safely.md new file mode 100644 index 0000000000..271b5441e5 --- /dev/null +++ b/docs/solutions/developer-experience/browser-testing-dashboard-from-worktree-safely.md @@ -0,0 +1,56 @@ +--- +module: dashboard-testing +date: "2026-06-04" +problem_type: developer_experience +title: "Browser-testing the Fusion dashboard from a worktree safely (no engine, fresh bundle, free port)" +applies_when: "Running browser/E2E verification of dashboard changes from a linked worktree during a pipeline (ce-test-browser or manual agent-browser sessions)" +tags: + - "browser-testing" + - "fn-dashboard" + - "worktree" + - "stale-dist" + - "port-4040" + - "fusion-daemon" +--- + +# Browser-testing the Fusion dashboard from a worktree safely + +## Context + +During the step-inversion pipeline (PR #1424), browser verification of dashboard changes from a linked worktree hit three traps in sequence, two of them dangerous: + +1. **`fn daemon --paused` still executed real tasks.** The daemon shares the user's central DB; within ~60s it spun up executor sessions on live tasks (racing the user's main instance — a dual-engine hazard) despite `--paused`. The `--paused` flag does not prevent engine dispatch in this path. +2. **`fn dashboard --dev` (no `--port`) bound the reserved port 4040** — the default in `packages/cli/src/bin.ts` is 4040, which is reserved for the user's own dashboard (see `dev-server-port-detect.ts: RESERVED_DASHBOARD_PORT`). +3. **The served UI bundle was stale.** `fn dashboard` serves `packages/cli/dist/client` (the CLI's own copy of the dashboard build), NOT `packages/dashboard/dist/client`. Rebuilding `@fusion/dashboard` does not update the CLI copy — newly added React Flow node types rendered as `react-flow__node-default` and foreach template children were missing from the DOM entirely, which looked exactly like a source bug (it wasn't). + +## Guidance + +The safe recipe for worktree browser testing: + +```bash +pnpm --filter @fusion/core --filter @fusion/engine --filter @fusion/dashboard \ + --filter "@fusion-plugin-examples/*" build + +FUSION_ALLOW_NESTED_PROJECT=1 FUSION_SKIP_ONBOARDING=1 \ +FUSION_CLIENT_DIR=$PWD/packages/dashboard/dist/client \ +node packages/cli/bin.mjs dashboard --dev --port 4101 --token cetest123 & +# open: http://localhost:4101/?token=cetest123 +``` + +- **`fn dashboard --dev`** = web UI only, AI engine disabled. Never use `fn daemon`/`fn serve` for UI verification — they run the engine against the shared central DB. +- **Always pass `--port `** (anything but 4040). The dashboard subcommand's default is the reserved 4040. +- **`FUSION_CLIENT_DIR` pointed at the fresh `packages/dashboard/dist/client`** beats the CLI's stale `packages/cli/dist/client` copy. Without it, UI changes silently don't appear. +- Killing your own spawned test server is fine; the no-kill rule protects the user's live instance on 4040. +- If the canvas "renders nothing": check the served bundle hash before debugging source — `agent-browser eval` on `document.querySelectorAll('.react-flow__node')` distinguishes "nodes absent" from "nodes mis-typed" (`react-flow__node-default` = nodeType not registered in the served bundle). + +## Why This Matters + +The daemon trap is a data hazard, not just wasted time: two engines on one SQLite central DB race task leases and can strand tasks in limbo. The stale-bundle trap costs hours because it perfectly mimics a source-level rendering bug — the jsdom tests pass (they test source) while the browser shows old behavior (it serves dist). + +## When to Apply + +Any time a pipeline or agent verifies dashboard behavior in a real browser from a worktree: ce-test-browser runs, manual agent-browser sessions, screenshot verification of editor/board changes. + +## Examples + +Diagnosing the stale bundle (PR #1424): source `WorkflowNodeTypes.tsx` registered `foreach`, jsdom tests green, but live DOM showed `react-flow__node-default` for the foreach node and no `steps::*` children. `grep nodeTypes` in the served `WorkflowNodeEditor-*.js` chunk showed the registry ending at `join` — a pre-U8 bundle from `packages/cli/dist/client`. diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index 4818e04dd3..b093b0723f 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -61,6 +61,54 @@ FN-5769 evaluated whether those conventions required a `1.1.0` schema bump and r The `workflowColumns` track introduces **IR v2** (`version: "v2"`), where a workflow additionally defines its own **columns** (`{ id, name, traits: [{ trait, config }] }`), places nodes in columns (`node.column`), and gains `hold`, `split`, and `join` node kinds. Columns become first-class, workflow-defined task state carrying composable **traits** (declarative flags + lifecycle hooks); this generalizes the fixed pipeline + the `gateMode` semantics documented below into per-column trait configuration. v1 graphs still parse and upgrade by synthesizing default-workflow columns. The column/trait model — the trait vocabulary, the substrate/policy line, the transition authority, and the graduation gate — is documented in **`docs/architecture.md` § 9 "Workflow-defined columns & traits"** and the **Concepts** glossary (column, trait, lane, hold node, split/join, default workflow, `transitionPending`). The whole v2 model is gated behind `experimentalFeatures.workflowColumns`; with the flag off, the v1 IR and the quality-gate `WorkflowStep` model below are unchanged. +### Workflow IR v2 — step inversion (foreach, step-review, parse-steps, code) + +The **step-inversion** track makes task *steps* themselves workflow-modelable. Today the engine owns step policy end-to-end (PROMPT.md parsing, per-step review verdicts, RETHINK/REVISE control flow, merge blocking). Step inversion extracts exactly one new substrate capability — *run one step inside a task's session, and reset one step to its baseline* — and exposes everything else as authored graph structure. It is additive to IR v2 and gated by `experimentalFeatures.workflowGraphExecutor`. The default coding workflow is untouched and byte-identical (it keeps its monolithic `execute` seam and is the parity oracle); inversion is opt-in via custom workflows and a new built-in **stepwise coding workflow**. + +#### `parse-steps` node — step list as graph structure + +`parse-steps` reads a declared **artifact** and runs a named **parser** to write the canonical step list (`Task.steps[]`). Config: `{ artifact: , parser: "step-headings" | "json-steps" | "plugin::" }`. + +- Built-in parsers: `step-headings` (the `### Step N:` convention, extracted byte-identically from the legacy regex) and `json-steps` (a `[{ name, depends? }]` JSON document). Plugins register additional parsers under `plugin::`. +- Outcomes: `success`, `outcome:no-steps` (parsed cleanly, zero steps — routable, defaults to success), `outcome:parse-error` (malformed artifact or a throwing/unavailable plugin parser — fail-closed, routable, defaults to failure). A plugin parser never crashes the run. +- It is the **only** graph-side writer of the step list, and **must dominate** (precede on all paths) any `foreach(source:"task-steps")` — a validator rule that prevents merging a task that reached the foreach before steps were parsed. + +#### `foreach` node — a per-step template region + +`foreach` instantiates an inline template subgraph once per planned step. Config: + +``` +{ source: "task-steps", template: { nodes, edges }, + mode?: "sequential" | "parallel", // default sequential + isolation?: "shared" | "worktree", // default: shared (sequential), worktree (parallel) + concurrency?: number, // parallel only, 1..8, default 2 + maxReworkCycles?: number } // default 3, cap 10 +``` + +- The template has exactly one entry and one exit. A `step-execute` seam node is legal **only** inside a foreach template; `step-execute` may not appear in `split` branches. +- Expansion happens when the walk reaches the node; the step count is **pinned** at expansion and persisted (PROMPT.md edits afterward do not re-expand — a `pin-mismatch` failure surfaces if the live step list later disagrees on resume). +- Zero steps → the foreach traverses its `success` edge immediately (no merge blocker, matching today). + +#### Parallel mode & the `(depends:)` annotation + +`mode` and `isolation` are independent axes. `parallel + shared` is rejected (concurrent writers in one worktree are unguardable). Under `worktree` isolation each instance runs in its own worktree/branch off a common base, with an **ordered integration stage** that lands step branches in step order (done iff integrated); a rebase conflict routes `outcome:integration-conflict` (default: rework on the updated base, budget-counted). + +Parallelism is opt-in *per step by the planner*, not asserted by the workflow author. A step depends on the previous step unless its PROMPT.md heading carries a `(depends: N,M)` annotation listing the 1-indexed steps it actually depends on — e.g. `### Step 3 (depends: 1): Title`. An unannotated plan is fully sequential regardless of `mode`. Annotate **conservatively**: only mark a step independent when it genuinely does not read or modify the prior step's output, or heavily-overlapping "independent" steps will loop integrate→conflict→rework until the budget exhausts. + +#### `step-review` node & rework edges + +`step-review` (`{ type: "plan" | "code", model? }`, legal only inside a foreach template) runs the reviewer against the current instance's step and maps the verdict to outcome edges: `outcome:approve` (marks the step done), `outcome:revise` (typically a rework edge — revise in place, no reset), `outcome:rethink` (a rework edge whose traversal first triggers reset-to-baseline: git reset + session rewind + step→pending), `outcome:unavailable` (bounded retry then route). The validator requires `approve` and `revise` routed; `rethink` defaults to the revise target with reset semantics. Verdict authority is single-writer — review nodes inside `split` branches are advisory-only. + +`rework` edges (`edge.kind: "rework"`) are the **only legal cycles**: a loop-back within one foreach instance, bounded by `maxReworkCycles`. Exhaustion emits `outcome:rework-exhausted` (validator requires it routed — escalation, hold, or failure; defaults to failure). Non-rework cycles still throw. + +#### `code` node — sandboxed TypeScript + +`code` (`{ source, timeoutMs? }`, default 30s, cap 300s) runs inline TypeScript (compiled with esbuild, executed in a timeout-bounded child process with cwd = the task worktree) for logic no built-in node covers. The script default-exports `async (ctx) => result` where `ctx = { task, steps, customFields, context, artifacts: { read(key) }, instance? }` (`instance` present inside a foreach template). The returned `{ outcome?, value?, contextPatch?, customFields? }` routes `outcome:` edges, merges `contextPatch` into walk context, and writes `customFields` through the validated field authority. It gets **no store handle**, cannot write the step list, and a throw/timeout/non-zero exit becomes an audited `failure`. Source compile errors are rejected at save time (a dashboard 400 listing the failing node ids). It runs at the same trust tier as existing project-local script steps. + +#### Workflow-defined custom task fields + +Workflows declare typed task fields via IR `fields: [{ id, name, type, required?, default?, options?, render? }]` (`type ∈ string | text | number | boolean | enum | multi-enum | date | url`; `options` for enum kinds; `render.placement ∈ card | detail | detail-section`, `render.widget`, `render.badge`). Values live in `tasks.customFields` and are validated through a single store authority (`updateTaskCustomFields`) with typed rejections (offending `fieldId` + `code`). Editing or switching a workflow **orphans** (never destroys) values for removed/incompatible fields — orphans are retained and shown under a detail disclosure. The task UI renders the schema dynamically (detail-form widgets by type, up to 3 card badges by placement). Agents read/write fields via `fn_task_update`'s `custom_fields` patch; authors set them via `fn_workflow_create/update`. Field values are surfaced in task/session context. + ## What They Are A workflow step is a reusable check (AI prompt or script) that can be enabled on tasks. diff --git a/packages/cli/skill/fusion/references/engine-tools.md b/packages/cli/skill/fusion/references/engine-tools.md index bf1ce2eddc..79e3dae523 100644 --- a/packages/cli/skill/fusion/references/engine-tools.md +++ b/packages/cli/skill/fusion/references/engine-tools.md @@ -16,9 +16,10 @@ These tools are **not** part of the user-invokable extension surface. They are i | `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) | `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) | `workflow_id` (string), `name?` (string), `description?` (string), `ir?` (object), `layout?` (object), `rehome_to?` (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_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 | @@ -58,7 +59,7 @@ Note: step-session execution (`step-session-executor.ts`) reuses executor coordi | Tool | Purpose | Parameters | |---|---|---| -| `fn_task_update` | Update a spec step status (`pending`/`in-progress`/`done`/`skipped`) | `step` (number), `status` (enum) | +| `fn_task_update` | Update a spec step status (`pending`/`in-progress`/`done`/`skipped`), task dependencies, and/or workflow-defined custom field values | `step?` (number, 1-indexed), `status?` (enum), `dependencies?` (string[]), `custom_fields?` (object keyed by field id; validated against the workflow field schema, `null` clears a field) | | `fn_task_add_dep` | Add a dependency to current task (confirmation-gated) | `task_id` (string), `confirm?` (boolean) | | `fn_task_done` | Mark task complete and optionally store summary | `summary?` (string) | | `fn_review_step` | Spawn step plan/code reviewer | `step` (number), `type` (`plan` \| `code`), `step_name` (string), `baseline?` (string) | diff --git a/packages/cli/src/commands/dashboard-tui/app.tsx b/packages/cli/src/commands/dashboard-tui/app.tsx index 913e77b9a9..a8441ea749 100644 --- a/packages/cli/src/commands/dashboard-tui/app.tsx +++ b/packages/cli/src/commands/dashboard-tui/app.tsx @@ -1613,6 +1613,15 @@ function TaskDetailScreen({ )} + {/* Card-placed custom fields (U13/KTD-14): read-only bracketed labels. */} + {detail.customFields && detail.customFields.length > 0 && ( + + {detail.customFields.map((f) => ( + [{f.label}: {f.value}] + ))} + + )} + {/* Steps section */} diff --git a/packages/cli/src/commands/dashboard-tui/state.ts b/packages/cli/src/commands/dashboard-tui/state.ts index b5bd86f976..8a144a2ac5 100644 --- a/packages/cli/src/commands/dashboard-tui/state.ts +++ b/packages/cli/src/commands/dashboard-tui/state.ts @@ -230,6 +230,10 @@ export interface TaskDetailData { currentStepIndex?: number; steps: TaskStep[]; recentLogs: TaskLogEntry[]; // last ~200 entries on initial load + /** Card-placed custom field values, pre-rendered as read-only bracketed + * labels for the task detail view (U13/KTD-14). Absent/empty when the + * workflow declares no card fields or none have values. */ + customFields?: Array<{ label: string; value: string }>; } export type TaskEvent = diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 8df4be7de2..0618905751 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -19,6 +19,7 @@ import { isWorkflowColumnsEnabled, resolveColumnFlags, BUILTIN_CODING_WORKFLOW_IR, + parseWorkflowIr, type WorkflowIrColumn, type TraitFlags, } from "@fusion/core"; @@ -2742,6 +2743,48 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: text: entry.outcome ? `${entry.action} → ${entry.outcome}` : entry.action, source: entry.runContext?.agentId ? "agent" : "executor", })); + // Card-placed custom fields → read-only bracketed labels + // (U13/KTD-14). Resolve the task's workflow IR, filter + // card-placed field defs, and render any present values. + // Best-effort: any resolution failure simply omits the chips. + let customFields: Array<{ label: string; value: string }> | undefined; + try { + const values = (t as { customFields?: Record }).customFields; + if (values && Object.keys(values).length > 0) { + const selection = projectStore.getTaskWorkflowSelection(t.id); + const def = selection?.workflowId + ? await projectStore.getWorkflowDefinition(selection.workflowId) + : undefined; + const ir = def + ? (typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir) + : BUILTIN_CODING_WORKFLOW_IR; + const fields = ir.version === "v2" ? (ir.fields ?? []) : []; + const chips: Array<{ label: string; value: string }> = []; + for (const field of fields) { + if (field.render?.placement !== "card") continue; + const raw = values[field.id]; + if (raw === undefined || raw === null || raw === "") continue; + const optLabel = (v: string): string => + field.options?.find((o) => o.value === v)?.label ?? v; + let display: string; + if (field.type === "boolean") { + if (raw !== true) continue; + display = field.name; + } else if (field.type === "multi-enum" && Array.isArray(raw)) { + if (raw.length === 0) continue; + display = raw.map((v) => optLabel(String(v))).join(", "); + } else if (field.type === "enum") { + display = optLabel(String(raw)); + } else { + display = String(raw); + } + chips.push({ label: field.name, value: display }); + } + if (chips.length > 0) customFields = chips; + } + } catch { + customFields = undefined; + } return { id: t.id, title: t.title, @@ -2753,6 +2796,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: currentStepIndex: t.currentStep, steps, recentLogs, + ...(customFields ? { customFields } : {}), }; } catch { // Task not found (deleted/archived between selection and fetch). diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index e605ee50f0..78afed3ed2 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -7,15 +7,38 @@ import { DEFAULT_WORKFLOW_COLUMN_IDS, parseWorkflowIr } from "../workflow-ir.js" import { createTaskStoreTestHarness } from "./store-test-helpers.js"; describe("built-in workflows", () => { - it("every built-in has a valid IR and compiles without error", () => { + // Graph-only built-ins (step inversion, KTD-9) model branching/foreach/rework + // structure the linear compiler cannot lower to a step list — they run only + // under the workflow graph executor. They still must parse as valid IR. + const GRAPH_ONLY_BUILTIN_IDS = new Set(["builtin:stepwise-coding"]); + + it("every built-in has a valid IR; linear built-ins compile without error", () => { expect(BUILTIN_WORKFLOWS.length).toBeGreaterThanOrEqual(4); for (const wf of BUILTIN_WORKFLOWS) { expect(isBuiltinWorkflowId(wf.id)).toBe(true); expect(() => parseWorkflowIr(wf.ir)).not.toThrow(); - expect(() => compileWorkflowToSteps(wf.ir)).not.toThrow(); + if (!GRAPH_ONLY_BUILTIN_IDS.has(wf.id)) { + expect(() => compileWorkflowToSteps(wf.ir)).not.toThrow(); + } } }); + it("includes the stepwise coding built-in modeling step inversion (KTD-9)", () => { + const stepwise = getBuiltinWorkflow("builtin:stepwise-coding"); + expect(stepwise).toBeDefined(); + const ir = parseWorkflowIr(stepwise!.ir); + if (ir.version !== "v2") throw new Error("expected v2"); + // The chain: a parse-steps node dominating a foreach with a step-review template. + expect(ir.nodes.some((n) => n.kind === "parse-steps")).toBe(true); + const foreach = ir.nodes.find((n) => n.kind === "foreach"); + expect(foreach).toBeDefined(); + const template = ( + foreach!.config as { template: { nodes: Array<{ kind: string; config?: { seam?: string } }> } } + ).template; + expect(template.nodes.some((n) => n.kind === "step-review")).toBe(true); + expect(template.nodes.some((n) => n.config?.seam === "step-execute")).toBe(true); + }); + it("default workflow column ids equal the legacy enum values, in legacy order (KTD-1)", () => { expect(BUILTIN_CODING_WORKFLOW_IR.version).toBe("v2"); if (BUILTIN_CODING_WORKFLOW_IR.version !== "v2") throw new Error("expected v2"); diff --git a/packages/core/src/__tests__/db-migrate.test.ts b/packages/core/src/__tests__/db-migrate.test.ts index 8a989400d8..b36c04e128 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); db.close(); }); @@ -798,7 +798,7 @@ describe("schema migration", () => { reviewerContextRetryCount: 0, reviewerFallbackRetryCount: 0, }); - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); db.close(); }); @@ -939,8 +939,68 @@ 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(107); + expect(db.getSchemaVersion()).toBe(108); db.close(); }); + + it("adds workflow_run_step_instances table + tasks.customFields when migrating from schema version 107", () => { + 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', '107')"); + db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')"); + db.exec(` + CREATE TABLE IF NOT EXISTS tasks ( + id TEXT PRIMARY KEY, + description TEXT NOT NULL, + "column" TEXT NOT NULL, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + + db.init(); + + // The new per-step-instance run-state table exists with its index. + const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all() as Array<{ name: string }>; + expect(tables.map((row) => row.name)).toContain("workflow_run_step_instances"); + + const stepInstanceColumns = db + .prepare("PRAGMA table_info(workflow_run_step_instances)") + .all() as Array<{ name: string }>; + expect(stepInstanceColumns.map((column) => column.name)).toEqual([ + "taskId", + "runId", + "foreachNodeId", + "stepIndex", + "pinnedStepCount", + "currentNodeId", + "status", + "baselineSha", + "checkpointId", + "reworkCount", + "branchName", + "integratedAt", + "updatedAt", + ]); + + const stepInstanceIndexes = db + .prepare("PRAGMA index_list(workflow_run_step_instances)") + .all() as Array<{ name: string }>; + expect( + stepInstanceIndexes.some((index) => index.name === "idx_workflow_run_step_instances_task_run"), + ).toBe(true); + + // tasks.customFields column is added with a default-'{}' definition. + const taskColumns = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ + name: string; + dflt_value: string | null; + }>; + const customFieldsColumn = taskColumns.find((column) => column.name === "customFields"); + expect(customFieldsColumn).toBeDefined(); + expect(customFieldsColumn?.dflt_value).toBe("'{}'"); + + expect(db.getSchemaVersion()).toBe(108); + db.close(); + }); }); diff --git a/packages/core/src/__tests__/db.test.ts b/packages/core/src/__tests__/db.test.ts index 7ef8601b31..8a618bd583 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(107); + expect(db.getSchemaVersion()).toBe(108); }); 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(107); + expect(db.getSchemaVersion()).toBe(108); }); 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(107); + expect(db.getSchemaVersion()).toBe(108); // 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(107); + expect(db.getSchemaVersion()).toBe(108); // Re-init should not fail db.init(); - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); db.close(); }); @@ -1527,7 +1527,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); // 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(107); + expect(localDb.getSchemaVersion()).toBe(108); 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(107); + expect(db.getSchemaVersion()).toBe(108); 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(107); + expect(migrated.getSchemaVersion()).toBe(108); 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(107); + expect(fresh.getSchemaVersion()).toBe(108); 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(107); + expect(migrated.getSchemaVersion()).toBe(108); 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(107); + expect(fresh.getSchemaVersion()).toBe(108); 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(107); + expect(migrated.getSchemaVersion()).toBe(108); 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(107); + expect(migrated.getSchemaVersion()).toBe(108); 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(107); + expect(fresh.getSchemaVersion()).toBe(108); 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 1e449be32b..333a68e551 100644 --- a/packages/core/src/__tests__/goals-schema.test.ts +++ b/packages/core/src/__tests__/goals-schema.test.ts @@ -91,6 +91,6 @@ describe("goals schema", () => { }); it("reports schema version 101", () => { - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); }); }); diff --git a/packages/core/src/__tests__/insight-store.test.ts b/packages/core/src/__tests__/insight-store.test.ts index 447411ad52..7aa0e611ad 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(107); + expect(db1.getSchemaVersion()).toBe(108); 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(107); + expect(db3.getSchemaVersion()).toBe(108); // 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(107); + expect(db1.getSchemaVersion()).toBe(108); db1.close(); const db2 = createDatabase(testDir); expect(() => db2.init()).not.toThrow(); - expect(db2.getSchemaVersion()).toBe(107); + expect(db2.getSchemaVersion()).toBe(108); 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(107); + expect(db1.getSchemaVersion()).toBe(108); // 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 fd508a69be..ef2a10d136 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(107); + expect(db.getSchemaVersion()).toBe(108); }); 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 fbff57d141..7410346c67 100644 --- a/packages/core/src/__tests__/mission-store.test.ts +++ b/packages/core/src/__tests__/mission-store.test.ts @@ -3746,7 +3746,7 @@ describe("MissionStore", () => { describe("Loop State & Validator Run Schema (v31)", () => { it("schema version is 101 after migration", () => { - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); }); 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 c64b9a50f3..3be684ea9a 100644 --- a/packages/core/src/__tests__/run-audit.test.ts +++ b/packages/core/src/__tests__/run-audit.test.ts @@ -584,7 +584,7 @@ describe("Run Audit", () => { }); it("schema version is bumped to 40", () => { - expect(db.getSchemaVersion()).toBe(107); + expect(db.getSchemaVersion()).toBe(108); }); }); }); diff --git a/packages/core/src/__tests__/step-parsers.test.ts b/packages/core/src/__tests__/step-parsers.test.ts new file mode 100644 index 0000000000..44d0f1d738 --- /dev/null +++ b/packages/core/src/__tests__/step-parsers.test.ts @@ -0,0 +1,317 @@ +import { describe, it, expect, afterEach, beforeEach } from "vitest"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; +import { + StepParserRegistry, + StepParserRegistrationError, + getStepParser, + listStepParsers, + registerStepParser, + unregisterStepParser, + parseStepHeadings, + parseJsonSteps, + __resetStepParserRegistryForTests, + type StepParser, +} from "../step-parsers.js"; + +describe("step-parsers registry (U12, KTD-12)", () => { + afterEach(() => { + __resetStepParserRegistryForTests(); + }); + + describe("step-headings built-in (byte-identical to legacy)", () => { + const headings = () => getStepParser("step-headings")!; + + it("is registered as a built-in", () => { + expect(getStepParser("step-headings")).toBeDefined(); + expect(listStepParsers().map((p) => p.id)).toContain("step-headings"); + }); + + it("parses unannotated headings byte-identically to the legacy regex", () => { + const content = `## Steps + +### Step 0: Preflight + +- [ ] x + +### Step 1: Implementation + +### Step 2: Testing +`; + expect(headings().parse(content).steps).toEqual([ + { name: "Preflight" }, + { name: "Implementation" }, + { name: "Testing" }, + ]); + }); + + it("matches the legacy regex output exactly for varied unannotated headings", () => { + const content = [ + "### Step 0: A", + "### Step 12: Multi word title", + "### Step 3 — dash but no annotation: Real Name", + "### Step 4: trailing spaces here ", + "### Step 5 no colon at all", + "not a step heading: ignored", + ].join("\n"); + const legacy: { name: string }[] = []; + const re = /^###\s+Step\s+\d+[^:]*:\s*(.+)$/gm; + let m: RegExpExecArray | null; + while ((m = re.exec(content)) !== null) { + legacy.push({ name: m[1].trim() }); + } + expect(headings().parse(content).steps).toEqual(legacy); + }); + + it("parses (depends: 1,2) into 0-indexed dependsOn", () => { + expect(headings().parse("### Step 3 (depends: 1,2): Title").steps).toEqual([ + { name: "Title", dependsOn: [0, 1] }, + ]); + }); + + it("dedupes and sorts depends values", () => { + expect(headings().parse("### Step 5 (depends: 3,1,3,2): T").steps).toEqual([ + { name: "T", dependsOn: [0, 1, 2] }, + ]); + }); + + it("empty depends list yields no dependsOn", () => { + expect(headings().parse("### Step 2 (depends: ): T").steps).toEqual([ + { name: "T" }, + ]); + }); + + it("falls back deterministically on a malformed depends annotation", () => { + expect(headings().parse("### Step 1 (depends: bad): Real Title").steps).toEqual([ + { name: "Real Title" }, + ]); + }); + + it("falls back deterministically when the annotation has no closing paren", () => { + expect(headings().parse("### Step 1 (depends: 1,2 oops: Title").steps).toEqual([ + { name: "1,2 oops: Title" }, + ]); + }); + + it("the extracted parseStepHeadings still yields TaskStep[] with status", () => { + // The store-facing function keeps the `status: "pending"` field. + expect(parseStepHeadings("### Step 0: Preflight")).toEqual([ + { name: "Preflight", status: "pending" }, + ]); + }); + }); + + describe("json-steps built-in", () => { + const json = () => getStepParser("json-steps")!; + + it("is registered as a built-in", () => { + expect(getStepParser("json-steps")).toBeDefined(); + }); + + it("parses a happy-path array of {name, depends}", () => { + const content = JSON.stringify([ + { name: "Plan" }, + { name: "Implement", depends: [1] }, + { name: "Test", depends: [1, 2] }, + ]); + expect(json().parse(content).steps).toEqual([ + { name: "Plan" }, + { name: "Implement", dependsOn: [0] }, + { name: "Test", dependsOn: [0, 1] }, + ]); + }); + + it("converts 1-indexed depends to 0-indexed dependsOn, deduped and sorted", () => { + const content = JSON.stringify([{ name: "X", depends: [3, 1, 3, 2] }]); + expect(json().parse(content).steps).toEqual([ + { name: "X", dependsOn: [0, 1, 2] }, + ]); + }); + + it("trims names and omits dependsOn when depends is empty", () => { + const content = JSON.stringify([{ name: " Spaced ", depends: [] }]); + expect(json().parse(content).steps).toEqual([{ name: "Spaced" }]); + }); + + it("parseJsonSteps is exported directly and matches the registry parser", () => { + const content = JSON.stringify([{ name: "A" }]); + expect(parseJsonSteps(content)).toEqual(json().parse(content)); + }); + + it("throws a descriptive error on non-JSON input", () => { + expect(() => json().parse("not json {")).toThrow(/not valid JSON/); + }); + + it("throws when the document is not an array", () => { + expect(() => json().parse(JSON.stringify({ name: "X" }))).toThrow( + /must be a JSON array/, + ); + }); + + it("throws when a step is missing its name", () => { + expect(() => json().parse(JSON.stringify([{ foo: "bar" }]))).toThrow( + /index 0 must have a non-empty string 'name'/, + ); + }); + + it("throws when a step name is blank", () => { + expect(() => json().parse(JSON.stringify([{ name: " " }]))).toThrow( + /non-empty string 'name'/, + ); + }); + + it("throws when depends is not an array", () => { + expect(() => + json().parse(JSON.stringify([{ name: "X", depends: 1 }])), + ).toThrow(/'depends' must be an array/); + }); + + it("throws when depends contains a non-positive-integer", () => { + expect(() => + json().parse(JSON.stringify([{ name: "X", depends: [0] }])), + ).toThrow(/positive integers/); + expect(() => + json().parse(JSON.stringify([{ name: "X", depends: ["1"] }])), + ).toThrow(/positive integers/); + }); + + it("throws when an entry is not an object", () => { + expect(() => json().parse(JSON.stringify(["just a string"]))).toThrow( + /index 0 must be an object/, + ); + }); + }); + + describe("registry semantics", () => { + it("rejects overwriting a built-in with a non-builtin id", () => { + const reg = new StepParserRegistry(); + reg.register({ id: "step-headings", parse: () => ({ steps: [] }) }, { builtin: true }); + expect(() => + reg.register({ id: "step-headings", parse: () => ({ steps: [] }) }), + ).toThrowError(StepParserRegistrationError); + try { + reg.register({ id: "step-headings", parse: () => ({ steps: [] }) }); + } catch (e) { + expect((e as StepParserRegistrationError).reason).toBe( + "builtin-namespace-protected", + ); + } + }); + + it("rejects a duplicate registration", () => { + const reg = new StepParserRegistry(); + const parser: StepParser = { + id: "plugin:acme:custom", + parse: () => ({ steps: [] }), + }; + reg.register(parser); + expect(() => reg.register(parser)).toThrowError(StepParserRegistrationError); + }); + + it("enforces the plugin id shape for non-builtins", () => { + const reg = new StepParserRegistry(); + const bad = ["custom", "plugin:acme", "plugin::custom", "plugin:Acme:Custom", "other:acme:custom"]; + for (const id of bad) { + expect(() => reg.register({ id, parse: () => ({ steps: [] }) })).toThrowError( + StepParserRegistrationError, + ); + } + // A well-formed namespaced id is accepted. + expect(() => + reg.register({ id: "plugin:acme:custom", parse: () => ({ steps: [] }) }), + ).not.toThrow(); + }); + + it("allows a built-in to use a non-namespaced id", () => { + const reg = new StepParserRegistry(); + expect(() => + reg.register({ id: "step-headings", parse: () => ({ steps: [] }) }, { builtin: true }), + ).not.toThrow(); + }); + + it("rejects an invalid definition (no id / no parse)", () => { + const reg = new StepParserRegistry(); + expect(() => reg.register({ id: "", parse: () => ({ steps: [] }) })).toThrowError( + StepParserRegistrationError, + ); + expect(() => + reg.register({ id: "plugin:acme:x" } as unknown as StepParser), + ).toThrowError(StepParserRegistrationError); + }); + + it("round-trips register/unregister for a plugin parser via the shared API", () => { + const id = "plugin:acme:json2"; + expect(getStepParser(id)).toBeUndefined(); + registerStepParser({ id, parse: () => ({ steps: [{ name: "ok" }] }) }); + expect(getStepParser(id)?.parse("").steps).toEqual([{ name: "ok" }]); + expect(unregisterStepParser(id)).toBe(true); + expect(getStepParser(id)).toBeUndefined(); + // Unregistering again (or a missing id) is a no-op false. + expect(unregisterStepParser(id)).toBe(false); + }); + + it("never unregisters a built-in", () => { + const reg = new StepParserRegistry(); + reg.register({ id: "step-headings", parse: () => ({ steps: [] }) }, { builtin: true }); + expect(reg.unregister("step-headings")).toBe(false); + expect(reg.has("step-headings")).toBe(true); + }); + + it("getStepParser returns undefined for an unknown id", () => { + expect(getStepParser("nope")).toBeUndefined(); + expect(getStepParser("plugin:acme:absent")).toBeUndefined(); + }); + }); + + describe("parseStepsFromPrompt-through-registry parity (KTD-12)", () => { + const harness = createTaskStoreTestHarness(); + + beforeEach(async () => { + await harness.beforeEach(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + const FIXTURES = [ + `## Steps + +### Step 0: Preflight + +### Step 1: Implementation + +### Step 2: Testing +`, + `# Task + +## Steps + +### Step 1: First + +### Step 2 (depends: 1): Second + +### Step 3 (depends: 1,2): Third +`, + `### Step 1 (depends: bad): Real Title`, + ]; + + it("store path equals the direct step-headings parser on the same content", async () => { + const store = harness.store(); + const rootDir = harness.rootDir(); + for (const content of FIXTURES) { + const task = await store.createTask({ description: "parity" }); + const dir = join(rootDir, ".fusion", "tasks", task.id); + await writeFile(join(dir, "PROMPT.md"), content); + + const viaStore = await store.parseStepsFromPrompt(task.id); + // Direct parser yields { name, dependsOn? }; the store path re-applies + // the `pending` status. Reconstruct the expected store shape from the + // direct parse to assert identical behavior through both paths. + const direct = parseStepHeadings(content); + expect(viaStore).toEqual(direct); + } + }); + }); +}); diff --git a/packages/core/src/__tests__/store-merge-queue.test.ts b/packages/core/src/__tests__/store-merge-queue.test.ts index 6d2b141a97..6c09641156 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(107); + expect(store.getDatabase().getSchemaVersion()).toBe(108); }); it("migrates a legacy v88 database and preserves task rows", async () => { diff --git a/packages/core/src/__tests__/store-parsing.test.ts b/packages/core/src/__tests__/store-parsing.test.ts index cea876e75b..6f407c9793 100644 --- a/packages/core/src/__tests__/store-parsing.test.ts +++ b/packages/core/src/__tests__/store-parsing.test.ts @@ -6,7 +6,7 @@ import { existsSync } from "node:fs"; import * as projectMemory from "../project-memory.js"; import { AgentStore } from "../agent-store.js"; import { CentralDatabase } from "../central-db.js"; -import { InvalidFileScopeError, isValidFileScopeEntry, TaskStore, TaskHasDependentsError } from "../store.js"; +import { InvalidFileScopeError, isValidFileScopeEntry, parseStepHeadings, TaskStore, TaskHasDependentsError } from "../store.js"; import { buildResearchDocumentKey, type Task } from "../types.js"; import { createTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.js"; @@ -41,6 +41,104 @@ describe("TaskStore", () => { const steps = await store.parseStepsFromPrompt(task.id); expect(steps).toEqual([]); }); + + it("parses depends annotations from PROMPT.md (1-indexed → 0-indexed)", async () => { + const task = await store.createTask({ description: "Task with depends" }); + const dir = join(rootDir, ".fusion", "tasks", task.id); + await writeFile( + join(dir, "PROMPT.md"), + `# ${task.id}: Task + +## Steps + +### Step 1: First + +### Step 2 (depends: 1): Second + +### Step 3 (depends: 1,2): Third +`, + ); + const steps = await store.parseStepsFromPrompt(task.id); + expect(steps).toEqual([ + { name: "First", status: "pending" }, + { name: "Second", status: "pending", dependsOn: [0] }, + { name: "Third", status: "pending", dependsOn: [0, 1] }, + ]); + }); + }); + + describe("parseStepHeadings (step-inversion U1)", () => { + it("parses unannotated headings byte-identically to the legacy regex", () => { + const content = `## Steps + +### Step 0: Preflight + +- [ ] x + +### Step 1: Implementation + +### Step 2: Testing +`; + // The legacy behavior: name = text after the first colon, trimmed; no dependsOn. + expect(parseStepHeadings(content)).toEqual([ + { name: "Preflight", status: "pending" }, + { name: "Implementation", status: "pending" }, + { name: "Testing", status: "pending" }, + ]); + }); + + it("matches the legacy regex output exactly for varied unannotated headings", () => { + const content = [ + "### Step 0: A", + "### Step 12: Multi word title", + "### Step 3 — dash but no annotation: Real Name", + "### Step 4: trailing spaces here ", + "### Step 5 no colon at all", + "not a step heading: ignored", + ].join("\n"); + // Reference: the original regex. + const legacy: { name: string; status: "pending" }[] = []; + const re = /^###\s+Step\s+\d+[^:]*:\s*(.+)$/gm; + let m: RegExpExecArray | null; + while ((m = re.exec(content)) !== null) { + legacy.push({ name: m[1].trim(), status: "pending" }); + } + expect(parseStepHeadings(content)).toEqual(legacy); + }); + + it("parses (depends: 1,2) into 0-indexed dependsOn", () => { + expect(parseStepHeadings("### Step 3 (depends: 1,2): Title")).toEqual([ + { name: "Title", status: "pending", dependsOn: [0, 1] }, + ]); + }); + + it("dedupes and sorts depends values", () => { + expect(parseStepHeadings("### Step 5 (depends: 3,1,3,2): T")).toEqual([ + { name: "T", status: "pending", dependsOn: [0, 1, 2] }, + ]); + }); + + it("empty depends list yields no dependsOn", () => { + expect(parseStepHeadings("### Step 2 (depends: ): T")).toEqual([ + { name: "T", status: "pending" }, + ]); + }); + + it("falls back deterministically on a malformed depends annotation (name after colon following the paren)", () => { + // 'bad' is not a positive integer → fallback: name starts after the colon + // following the closing paren. + expect(parseStepHeadings("### Step 1 (depends: bad): Real Title")).toEqual([ + { name: "Real Title", status: "pending" }, + ]); + }); + + it("falls back deterministically when the annotation has no closing paren", () => { + // No closing paren → name starts after the FIRST colon (inside `depends:`), + // per the documented deterministic fallback. + expect(parseStepHeadings("### Step 1 (depends: 1,2 oops: Title")).toEqual([ + { name: "1,2 oops: Title", status: "pending" }, + ]); + }); }); diff --git a/packages/core/src/__tests__/store-update-step-order.test.ts b/packages/core/src/__tests__/store-update-step-order.test.ts index adde221985..42117abdd8 100644 --- a/packages/core/src/__tests__/store-update-step-order.test.ts +++ b/packages/core/src/__tests__/store-update-step-order.test.ts @@ -54,4 +54,81 @@ describe("TaskStore.updateStep step-order guard", () => { expect(updated.steps[0].status).toBe("done"); expect(updated.log.some((entry) => entry.action.includes("Ignored done→in-progress regression"))).toBe(true); }); + + // ── U6: graph-source projection discipline (KTD-7/KTD-11) ────────────────── + + it("graph source: done is legal in dependency order even when an earlier step is pending", async () => { + // Step 2 depends only on the previous step (1) by default. With step 1 done, + // step 2 may go done under graph source even though step 0 is still pending — + // the legacy strict-index-order guard relaxes to dependency order. + const store = harness.store(); + const task = await harness.createTaskWithSteps(); + // Prime the step list, then give step 2 an explicit dependency on step 0 only + // (skipping step 1), so step 2 may go done with step 1 still pending. + await store.updateStep(task.id, 0, "in-progress"); + const primed = await store.getTask(task.id); + const steps = primed.steps.map((s, i) => (i === 2 ? { ...s, dependsOn: [0] } : { ...s })); + await store.updateTask(task.id, { steps }); + + await store.updateStep(task.id, 0, "done", { source: "graph" }); + const updated = await store.updateStep(task.id, 2, "done", { source: "graph" }); + + expect(updated.steps[2].status).toBe("done"); + // Step 1 was never touched and remains pending — strict index order would have + // suppressed the step-2 done write. + expect(updated.steps[1].status).toBe("pending"); + }); + + it("graph source: out-of-order done (unmet dependency) is suppressed AND audited loudly", async () => { + // Step 1's default dependency is step 0, which is still pending → suppressed. + const store = harness.store(); + const task = await harness.createTaskWithSteps(); + // Prime the step list (graph source bypasses PROMPT.md auto-init). + await store.updateStep(task.id, 1, "in-progress"); + + const updated = await store.updateStep(task.id, 1, "done", { source: "graph" }); + + // Suppressed: step 1's default dependency (step 0) is still pending, so the + // done write is rejected and step 1 keeps its prior (non-done) status. + expect(updated.steps[1].status).not.toBe("done"); + expect( + updated.log.some((e) => e.action.includes("Ignored dependency-order done for step 1")), + ).toBe(true); + // Graph suppression is surfaced loudly (not the legacy silent ignore). + expect( + updated.log.some((e) => e.action.includes("[integrity-warning] graph-source updateStep suppressed")), + ).toBe(true); + }); + + it("legacy source: silent out-of-order ignore behavior is unchanged (no integrity-warning)", async () => { + const store = harness.store(); + const task = await harness.createTaskWithSteps(); + + await store.updateStep(task.id, 0, "done"); + const updated = await store.updateStep(task.id, 2, "done"); // legacy, no source + + expect(updated.steps[2].status).toBe("pending"); + expect(updated.log.some((e) => e.action.includes("Ignored out-of-order done for step 2"))).toBe(true); + // Legacy stays silent — no integrity-warning emitted. + expect(updated.log.some((e) => e.action.includes("[integrity-warning]"))).toBe(false); + }); + + it("graph source: auto-reinit from PROMPT.md is bypassed (explicit indices only)", async () => { + // A fresh task with no JSON steps would, under legacy semantics, parse steps + // from PROMPT.md on the first updateStep. Graph source bypasses that — so an + // index into an unparsed (empty) step list is out of range and rejects. + const store = harness.store(); + const task = await store.createTask({ description: "graph reinit bypass" }); + // No PROMPT.md steps are written; task.steps starts empty. + + await expect(store.updateStep(task.id, 0, "in-progress", { source: "graph" })).rejects.toThrow( + /out of range/, + ); + + // Legacy path on the same empty task would attempt the PROMPT.md reinit + // instead of bypassing — proving the divergence is graph-source-only. (Here + // there is no PROMPT.md either, so legacy also has zero steps and rejects, + // but via the auto-init path rather than the bypass.) + await expect(store.updateStep(task.id, 0, "in-progress")).rejects.toThrow(/out of range/); + }); }); diff --git a/packages/core/src/__tests__/task-documents.test.ts b/packages/core/src/__tests__/task-documents.test.ts index ccde60a72e..7352ec08bf 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(107); + expect(db.getSchemaVersion()).toBe(108); const index = db .prepare( diff --git a/packages/core/src/__tests__/task-fields.test.ts b/packages/core/src/__tests__/task-fields.test.ts new file mode 100644 index 0000000000..5306ad9547 --- /dev/null +++ b/packages/core/src/__tests__/task-fields.test.ts @@ -0,0 +1,530 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, +} from "../task-fields.js"; +import type { WorkflowFieldDefinition, WorkflowIr } from "../workflow-ir-types.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/** + * U11 / KTD-13 — custom task fields: validation authority, defaults, + * reconciliation, and the store-level write authority. + * + * The pure functions in task-fields.ts are the single validation core; the + * store delegates to them for updateTask/updateTaskCustomFields and for + * workflow-switch / definition-edit reconciliation. These tests cover both. + */ + +// ── Field-definition fixtures ──────────────────────────────────────────────── + +const F = (over: Partial & { id: string; type: WorkflowFieldDefinition["type"] }): WorkflowFieldDefinition => ({ + name: over.id, + ...over, +}); + +const enumOpts = [ + { value: "high", label: "High" }, + { value: "low", label: "Low" }, +]; + +const ALL_TYPES: WorkflowFieldDefinition[] = [ + F({ id: "s", type: "string" }), + F({ id: "tx", type: "text" }), + F({ id: "n", type: "number" }), + F({ id: "b", type: "boolean" }), + F({ id: "e", type: "enum", options: enumOpts }), + F({ id: "m", type: "multi-enum", options: enumOpts }), + F({ id: "d", type: "date" }), + F({ id: "u", type: "url" }), +]; + +// ── Pure validation: every type ────────────────────────────────────────────── + +describe("validateCustomFieldPatch — per-type validate/reject", () => { + it("string/text accept strings, reject non-strings", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { s: "hi", tx: "yo" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { s: 5 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); + + it("number accepts finite numbers, rejects NaN/Infinity/non-number", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { n: 3 }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { n: 0 }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { n: Number.NaN }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { n: Number.POSITIVE_INFINITY }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { n: "3" }).ok).toBe(false); + }); + + it("boolean accepts booleans only", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { b: true }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { b: "true" }).ok).toBe(false); + }); + + it("date accepts parseable ISO strings, rejects garbage", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { d: "2026-06-04" }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { d: "2026-06-04T12:00:00Z" }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { d: "not-a-date" }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { d: 20260604 }).ok).toBe(false); + }); + + it("url accepts URL-parseable strings, rejects bad", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { u: "https://example.com/x" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { u: "not a url" }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — enum membership", () => { + it("accepts a declared option, rejects a non-member with enum-violation", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { e: "high" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { e: "medium" }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.rejection.code).toBe("enum-violation"); + expect(r.rejection.fieldId).toBe("e"); + } + }); + it("rejects a non-string enum value with type-mismatch", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { e: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — multi-enum subsets + dupes", () => { + it("accepts a subset of options", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high"] }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.normalized.m).toEqual(["high"]); + }); + it("accepts the empty array", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { m: [] }).ok).toBe(true); + }); + it("rejects a non-member with enum-violation", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high", "medium"] }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("enum-violation"); + }); + it("rejects duplicate members", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high", "high"] }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("enum-violation"); + }); + it("rejects a non-array", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: "high" }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — unknown field & no-fields", () => { + it("rejects a patch key naming no declared field", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { nope: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.rejection.code).toBe("unknown-field"); + expect(r.rejection.fieldId).toBe("nope"); + } + }); + it("rejects any non-empty patch when no fields are defined (no-fields-defined)", () => { + const r = validateCustomFieldPatch(undefined, { anything: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("no-fields-defined"); + const r2 = validateCustomFieldPatch([], { x: 1 }); + expect(r2.ok).toBe(false); + if (!r2.ok) expect(r2.rejection.code).toBe("no-fields-defined"); + }); + it("accepts an EMPTY patch even with no fields defined", () => { + expect(validateCustomFieldPatch(undefined, {}).ok).toBe(true); + expect(validateCustomFieldPatch([], {}).ok).toBe(true); + }); + it("treats null/undefined patch values as delete sentinels (normalized to null)", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { s: null, n: undefined }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.normalized).toEqual({ s: null, n: null }); + }); +}); + +// ── Defaults ────────────────────────────────────────────────────────────── + +describe("applyFieldDefaults", () => { + const fields: WorkflowFieldDefinition[] = [ + F({ id: "req", type: "string", required: true, default: "x" }), + F({ id: "reqNoDefault", type: "string", required: true }), + F({ id: "optDefault", type: "number", default: 7 }), + ]; + it("fills required field defaults absent from current", () => { + expect(applyFieldDefaults(fields, {})).toEqual({ req: "x" }); + }); + it("does not override an existing value", () => { + expect(applyFieldDefaults(fields, { req: "kept" })).toEqual({ req: "kept" }); + }); + it("ignores non-required defaults and required-without-default", () => { + const out = applyFieldDefaults(fields, {}); + expect(out).not.toHaveProperty("optDefault"); + expect(out).not.toHaveProperty("reqNoDefault"); + }); +}); + +// ── Reconciliation ────────────────────────────────────────────────────────── + +describe("reconcileFieldsOnWorkflowChange", () => { + it("keeps same-id type-compatible values, orphans removed ids", () => { + const oldF = [F({ id: "a", type: "string" }), F({ id: "gone", type: "number" })]; + const newF = [F({ id: "a", type: "string" })]; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldF, newF, { a: "v", gone: 1 }); + expect(kept).toEqual({ a: "v" }); + expect(orphaned).toEqual({ gone: 1 }); + }); + + it("orphans a value when the new type is incompatible", () => { + const oldF = [F({ id: "a", type: "string" })]; + const newF = [F({ id: "a", type: "number" })]; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldF, newF, { a: "still-a-string" }); + expect(kept).toEqual({}); + expect(orphaned).toEqual({ a: "still-a-string" }); + }); + + it("keeps an enum value still in the new options, orphans one no longer present", () => { + const oldF = [F({ id: "e", type: "enum", options: enumOpts })]; + const newF = [F({ id: "e", type: "enum", options: [{ value: "high", label: "H" }] })]; + expect(reconcileFieldsOnWorkflowChange(oldF, newF, { e: "high" }).kept).toEqual({ e: "high" }); + expect(reconcileFieldsOnWorkflowChange(oldF, newF, { e: "low" }).orphaned).toEqual({ e: "low" }); + }); +}); + +// ── Store authority integration ────────────────────────────────────────────── + +describe("store: updateTaskCustomFields + updateTask integration (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name = "wf"): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + async function taskWithFields(fields: WorkflowFieldDefinition[]) { + const def = await (store as any).createWorkflowDefinition({ name: "WF", ir: irWith(fields) }); + const t = await store.createTask({ description: "field task" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + return { task: t, workflowId: def.id as string }; + } + + it("happy path: validates, merges, persists, returns ok", async () => { + const { task } = await taskWithFields([ + F({ id: "sev", type: "enum", options: enumOpts }), + F({ id: "pts", type: "number" }), + ]); + const r = await (store as any).updateTaskCustomFields(task.id, { sev: "high", pts: 5 }); + expect(r.ok).toBe(true); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({ sev: "high", pts: 5 }); + }); + + it("reject path: returns a typed rejection, does not mutate", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + const r = await (store as any).updateTaskCustomFields(task.id, { pts: "not-a-number" }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("type-mismatch"); + expect(r.rejection.fieldId).toBe("pts"); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({}); + }); + + it("unknown-field rejection on an undeclared key", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + const r = await (store as any).updateTaskCustomFields(task.id, { nope: 1 }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("unknown-field"); + }); + + it("default workflow (zero fields) rejects cleanly with no-fields-defined", async () => { + const t = await store.createTask({ description: "default wf" }); + const r = await (store as any).updateTaskCustomFields(t.id, { anything: 1 }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("no-fields-defined"); + }); + + it("emits task:updated on a successful write", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + let emitted = 0; + (store as any).on("task:updated", () => { + emitted += 1; + }); + const r = await (store as any).updateTaskCustomFields(task.id, { pts: 1 }); + expect(r.ok).toBe(true); + expect(emitted).toBeGreaterThanOrEqual(1); + }); + + it("null patch value deletes the stored value", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" }), F({ id: "x", type: "number" })]); + await (store as any).updateTaskCustomFields(task.id, { pts: 1, x: 2 }); + await (store as any).updateTaskCustomFields(task.id, { pts: null }); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({ x: 2 }); + }); + + it("updateTask with an invalid customFields patch throws CustomFieldRejectionError", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + await expect(store.updateTask(task.id, { customFields: { pts: "bad" } })).rejects.toThrow(/pts/); + }); + + it("applies required+default fields at workflow selection", async () => { + const def = await (store as any).createWorkflowDefinition({ + name: "Defaults", + ir: irWith([F({ id: "tier", type: "string", required: true, default: "bronze" })]), + }); + const t = await store.createTask({ description: "defaults" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ tier: "bronze" }); + }); +}); + +describe("store: workflow switch reconciliation (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name: string): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + it("keeps same-id compatible values and orphans the rest (orphan-not-delete)", async () => { + const wfA = await (store as any).createWorkflowDefinition({ + name: "A", + ir: irWith([F({ id: "shared", type: "string" }), F({ id: "onlyA", type: "number" })], "A"), + }); + const wfB = await (store as any).createWorkflowDefinition({ + name: "B", + ir: irWith([F({ id: "shared", type: "string" }), F({ id: "onlyB", type: "boolean" })], "B"), + }); + const t = await store.createTask({ description: "switch" }); + await (store as any).selectTaskWorkflow(t.id, wfA.id); + await (store as any).updateTaskCustomFields(t.id, { shared: "v", onlyA: 3 }); + + await (store as any).selectTaskWorkflow(t.id, wfB.id); + const got = await store.getTask(t.id); + // shared kept; onlyA orphaned but RETAINED in storage (never destroyed). + expect(got?.customFields).toEqual({ shared: "v", onlyA: 3 }); + }); +}); + +describe("store: updateWorkflowDefinition field-type change coercion (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name = "WF"): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + async function fieldedTaskAndWf(fields: WorkflowFieldDefinition[]) { + const def = await (store as any).createWorkflowDefinition({ name: "WF", ir: irWith(fields) }); + const t = await store.createTask({ description: "edit" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + return { workflowId: def.id as string, taskId: t.id as string }; + } + + it("rejects an incompatible type change with occupants and no coerce", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await expect( + store.updateWorkflowDefinition(workflowId, { ir: irWith([F({ id: "x", type: "number" })]) }), + ).rejects.toThrow(/IncompatibleFieldChange|incompatibl/i); + }); + + it("coerce:keep-orphaned retains the now-incompatible value", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await store.updateWorkflowDefinition(workflowId, { + ir: irWith([F({ id: "x", type: "number" })]), + coerce: "keep-orphaned", + }); + const got = await store.getTask(taskId); + expect(got?.customFields).toEqual({ x: "hello" }); + }); + + it("coerce:drop discards the now-incompatible value", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await store.updateWorkflowDefinition(workflowId, { + ir: irWith([F({ id: "x", type: "number" })]), + coerce: "drop", + }); + const got = await store.getTask(taskId); + expect(got?.customFields).toEqual({}); + }); + + it("removing a field outright orphans (never blocks, value retained)", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([ + F({ id: "x", type: "string" }), + F({ id: "y", type: "string" }), + ]); + await (store as any).updateTaskCustomFields(taskId, { x: "a", y: "b" }); + await store.updateWorkflowDefinition(workflowId, { ir: irWith([F({ id: "x", type: "string" })]) }); + const got = await store.getTask(taskId); + // y orphaned but retained. + expect(got?.customFields).toEqual({ x: "a", y: "b" }); + }); + + // T1 (store.ts:12410): a field-schema edit that adds a new required+default + // field must backfill the default onto EVERY occupant, including occupants + // that currently hold no custom field values — not only ones already populated. + it("backfills a new required+default field onto occupants with no existing values", async () => { + const { taskId, workflowId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + // Occupant deliberately has NO custom field values stored. + const before = await store.getTask(taskId); + expect(before?.customFields ?? {}).toEqual({}); + + await store.updateWorkflowDefinition(workflowId, { + ir: irWith([ + F({ id: "x", type: "string" }), + F({ id: "tier", type: "string", required: true, default: "bronze" }), + ]), + }); + + const got = await store.getTask(taskId); + expect(got?.customFields).toEqual({ tier: "bronze" }); + }); +}); + +// ── Archive → unarchive customFields round-trip ────────────────────────────── + +describe("store: archive → unarchive preserves customFields (T0)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name = "WF"): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + it("restores customFields after an archive → unarchive round-trip", async () => { + const def = await (store as any).createWorkflowDefinition({ + name: "WF", + ir: irWith([F({ id: "sev", type: "enum", options: enumOpts }), F({ id: "pts", type: "number" })]), + }); + const t = await store.createTask({ description: "round-trip" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + await (store as any).updateTaskCustomFields(t.id, { sev: "high", pts: 5 }); + + // Move through the legacy transition chain to reach 'done', then archive. + await store.moveTask(t.id, "todo"); + await store.moveTask(t.id, "in-progress"); + await store.moveTask(t.id, "in-review"); + await store.moveTask(t.id, "done"); + const archived = await store.archiveTask(t.id); + expect(archived.column).toBe("archived"); + + const restored = await store.unarchiveTask(t.id); + expect(restored.customFields).toEqual({ sev: "high", pts: 5 }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ sev: "high", pts: 5 }); + }); +}); + +// ── JSON round-trip stability ──────────────────────────────────────────────── + +describe("custom-field values JSON round-trip", () => { + it("normalized values survive a JSON round-trip unchanged", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { + s: "x", + n: 1.5, + b: false, + e: "low", + m: ["high", "low"], + d: "2026-06-04", + u: "https://x.test/", + }); + expect(r.ok).toBe(true); + if (r.ok) { + expect(JSON.parse(JSON.stringify(r.normalized))).toEqual(r.normalized); + } + }); +}); diff --git a/packages/core/src/__tests__/workflow-ir-foreach.test.ts b/packages/core/src/__tests__/workflow-ir-foreach.test.ts new file mode 100644 index 0000000000..2bb114a9f6 --- /dev/null +++ b/packages/core/src/__tests__/workflow-ir-foreach.test.ts @@ -0,0 +1,551 @@ +import { describe, expect, it } from "vitest"; +import { + parseWorkflowIr, + serializeWorkflowIr, + downgradeIrToV1IfPure, + WorkflowIrError, +} from "../workflow-ir.js"; +import type { + WorkflowIrEdge, + WorkflowIrNode, + WorkflowIrV2, +} from "../workflow-ir-types.js"; + +// Step-inversion (U1) — foreach / step-review / parse-steps / code / rework / +// fields validation. + +const defaultColumns: WorkflowIrV2["columns"] = [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, +]; + +function v2( + nodes: WorkflowIrNode[], + edges: WorkflowIrEdge[], + extra: Partial = {}, +): WorkflowIrV2 { + return { version: "v2", name: "test", columns: defaultColumns, nodes, edges, ...extra }; +} + +/** A minimal valid foreach template: step-execute → step-review(approve→exit). */ +function stepTemplate(): { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] } { + return { + nodes: [ + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "rev", kind: "step-review", config: { type: "code" } }, + { id: "exit", kind: "prompt" }, + ], + edges: [ + { from: "se", to: "rev" }, + { from: "rev", to: "exit", condition: "outcome:approve" }, + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + ], + }; +} + +/** A graph: start → parse-steps → foreach → end. */ +function graphWithForeach( + foreachConfig: Record, + extra: Partial = {}, +): WorkflowIrV2 { + return v2( + [ + { id: "start", kind: "start" }, + { id: "ps", kind: "parse-steps", config: { artifact: "PROMPT.md", parser: "step-headings" } }, + { id: "fe", kind: "foreach", config: { source: "task-steps", template: stepTemplate(), ...foreachConfig } }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "ps" }, + { from: "ps", to: "fe" }, + { from: "fe", to: "end" }, + ], + extra, + ); +} + +describe("foreach validation", () => { + it("parses a valid foreach dominated by parse-steps", () => { + const ir = parseWorkflowIr(graphWithForeach({})) as WorkflowIrV2; + expect(ir.version).toBe("v2"); + const fe = ir.nodes.find((n) => n.id === "fe")!; + expect(fe.kind).toBe("foreach"); + }); + + it("rejects foreach with empty template", () => { + const ir = graphWithForeach({ template: { nodes: [], edges: [] } }); + expect(() => parseWorkflowIr(ir)).toThrow(/non-empty/); + }); + + it("rejects template with two entry nodes", () => { + const tmpl = { + nodes: [ + { id: "a", kind: "prompt", config: { seam: "step-execute" } }, + { id: "b", kind: "prompt" }, + ] as WorkflowIrNode[], + edges: [] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /exactly one entry/, + ); + }); + + it("rejects template with two exit nodes", () => { + const tmpl = { + nodes: [ + { id: "a", kind: "prompt", config: { seam: "step-execute" } }, + { id: "b", kind: "prompt" }, + { id: "c", kind: "prompt" }, + ] as WorkflowIrNode[], + edges: [ + { from: "a", to: "b" }, + { from: "a", to: "c" }, + ] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /exactly one (entry|exit)/, + ); + }); + + it("rejects nested foreach in a template", () => { + const tmpl = { + nodes: [ + { id: "inner", kind: "foreach", config: { source: "task-steps", template: stepTemplate() } }, + ] as WorkflowIrNode[], + edges: [] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /nested foreach/, + ); + }); + + it("rejects step-execute at the top level", () => { + const ir = v2( + [ + { id: "start", kind: "start" }, + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "se" }, + { from: "se", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/only legal inside a foreach template/); + }); + + it("rejects step-execute inside a split branch (extends SEAM_FORBIDDEN_IN_BRANCH)", () => { + const tmpl = { + nodes: [ + { id: "split", kind: "split" }, + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "other", kind: "prompt" }, + { id: "join", kind: "join" }, + ] as WorkflowIrNode[], + edges: [ + { from: "split", to: "se" }, + { from: "split", to: "other" }, + { from: "se", to: "join" }, + { from: "other", to: "join" }, + ] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /step-execute.*forbidden inside a parallel branch/, + ); + }); + + it("rejects a rework edge crossing the template boundary", () => { + const tmpl = stepTemplate(); + // Point the rework edge at a node outside the template. + tmpl.edges = tmpl.edges.map((e) => + e.kind === "rework" ? { ...e, to: "end" } : e, + ); + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /both endpoints inside the same template/, + ); + }); + + it("rejects a top-level rework edge", () => { + const ir = v2( + [ + { id: "start", kind: "start" }, + { id: "a", kind: "prompt" }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "a" }, + { from: "a", to: "end" }, + { from: "end", to: "a", kind: "rework" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/only legal inside a foreach template/); + }); + + it("rejects foreach not dominated by a parse-steps node", () => { + const ir = v2( + [ + { id: "start", kind: "start" }, + { id: "fe", kind: "foreach", config: { source: "task-steps", template: stepTemplate() } }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "fe" }, + { from: "fe", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/must be dominated by a parse-steps node/); + }); + + it("rejects foreach when parse-steps is only on one branch (not all paths)", () => { + // start → split into (ps→join) and (direct→join), join → fe. + const ir = v2( + [ + { id: "start", kind: "start" }, + { id: "split", kind: "split" }, + { id: "ps", kind: "parse-steps", config: { artifact: "PROMPT.md", parser: "step-headings" } }, + { id: "direct", kind: "prompt" }, + { id: "join", kind: "join" }, + { id: "fe", kind: "foreach", config: { source: "task-steps", template: stepTemplate() } }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "split" }, + { from: "split", to: "ps" }, + { from: "split", to: "direct" }, + { from: "ps", to: "join" }, + { from: "direct", to: "join" }, + { from: "join", to: "fe" }, + { from: "fe", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/must be dominated by a parse-steps node/); + }); +}); + +describe("foreach mode / isolation / concurrency", () => { + it("rejects parallel + shared", () => { + expect(() => + parseWorkflowIr(graphWithForeach({ mode: "parallel", isolation: "shared" })), + ).toThrow(/cannot combine mode 'parallel' with isolation 'shared'/); + }); + + it("accepts parallel + worktree", () => { + expect(() => + parseWorkflowIr(graphWithForeach({ mode: "parallel", isolation: "worktree", concurrency: 4 })), + ).not.toThrow(); + }); + + it("rejects concurrency on sequential mode", () => { + expect(() => + parseWorkflowIr(graphWithForeach({ mode: "sequential", concurrency: 2 })), + ).toThrow(/concurrency is only valid in 'parallel' mode/); + }); + + it("rejects concurrency out of range", () => { + expect(() => + parseWorkflowIr(graphWithForeach({ mode: "parallel", isolation: "worktree", concurrency: 9 })), + ).toThrow(/concurrency must be an integer in 1\.\.8/); + expect(() => + parseWorkflowIr(graphWithForeach({ mode: "parallel", isolation: "worktree", concurrency: 0 })), + ).toThrow(/concurrency must be an integer in 1\.\.8/); + }); +}); + +describe("foreach maxReworkCycles clamp", () => { + it("rejects maxReworkCycles < 1", () => { + expect(() => parseWorkflowIr(graphWithForeach({ maxReworkCycles: 0 }))).toThrow( + /maxReworkCycles must be an integer >= 1/, + ); + }); + + it("clamps maxReworkCycles > 10 to 10", () => { + const ir = parseWorkflowIr(graphWithForeach({ maxReworkCycles: 99 })) as WorkflowIrV2; + const fe = ir.nodes.find((n) => n.id === "fe")!; + expect((fe.config as { maxReworkCycles: number }).maxReworkCycles).toBe(10); + }); + + it("keeps maxReworkCycles <= 10 unchanged", () => { + const ir = parseWorkflowIr(graphWithForeach({ maxReworkCycles: 5 })) as WorkflowIrV2; + const fe = ir.nodes.find((n) => n.id === "fe")!; + expect((fe.config as { maxReworkCycles: number }).maxReworkCycles).toBe(5); + }); +}); + +describe("step-review verdict routing", () => { + function templateWithReview(reviewEdges: WorkflowIrEdge[]): WorkflowIrV2 { + const tmpl = { + nodes: [ + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "rev", kind: "step-review", config: { type: "plan" } }, + { id: "exit", kind: "prompt" }, + ] as WorkflowIrNode[], + edges: [{ from: "se", to: "rev" }, ...reviewEdges], + }; + return graphWithForeach({ template: tmpl }); + } + + it("rejects step-review missing approve routing", () => { + expect(() => + parseWorkflowIr( + templateWithReview([ + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + { from: "rev", to: "exit", condition: "outcome:other" }, + ]), + ), + ).toThrow(/must route outcome:approve/); + }); + + it("rejects step-review missing revise routing", () => { + expect(() => + parseWorkflowIr( + templateWithReview([{ from: "rev", to: "exit", condition: "outcome:approve" }]), + ), + ).toThrow(/must route outcome:revise/); + }); + + it("accepts approve+revise routing (rethink optional)", () => { + expect(() => + parseWorkflowIr( + templateWithReview([ + { from: "rev", to: "exit", condition: "outcome:approve" }, + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + ]), + ), + ).not.toThrow(); + }); + + it("rejects a verdict-authoring step-review inside a split branch (advisory-only)", () => { + const tmpl = { + nodes: [ + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "split", kind: "split" }, + { id: "advrev", kind: "step-review", config: { type: "code" } }, + { id: "other", kind: "prompt" }, + { id: "join", kind: "join" }, + { id: "exit", kind: "prompt" }, + ] as WorkflowIrNode[], + edges: [ + { from: "se", to: "split" }, + { from: "split", to: "advrev" }, + { from: "split", to: "other" }, + // advisory review illegally carries approve routing + { from: "advrev", to: "join", condition: "outcome:approve" }, + { from: "other", to: "join" }, + { from: "join", to: "exit" }, + ] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).toThrow( + /advisory-only/, + ); + }); + + it("accepts an advisory step-review inside a split branch without verdict routing", () => { + const tmpl = { + nodes: [ + { id: "se", kind: "prompt", config: { seam: "step-execute" } }, + { id: "split", kind: "split" }, + { id: "advrev", kind: "step-review", config: { type: "code" } }, + { id: "other", kind: "prompt" }, + { id: "join", kind: "join" }, + { id: "rev", kind: "step-review", config: { type: "code" } }, + { id: "exit", kind: "prompt" }, + ] as WorkflowIrNode[], + edges: [ + { from: "se", to: "split" }, + { from: "split", to: "advrev" }, + { from: "split", to: "other" }, + { from: "advrev", to: "join" }, + { from: "other", to: "join" }, + { from: "join", to: "rev" }, + { from: "rev", to: "exit", condition: "outcome:approve" }, + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + ] as WorkflowIrEdge[], + }; + expect(() => parseWorkflowIr(graphWithForeach({ template: tmpl }))).not.toThrow(); + }); +}); + +describe("parse-steps validation", () => { + it("rejects parse-steps with empty parser", () => { + const ir = graphWithForeach({}); + (ir.nodes.find((n) => n.id === "ps")!.config as Record).parser = ""; + expect(() => parseWorkflowIr(ir)).toThrow(/non-empty parser/); + }); + + it("rejects parse-steps referencing an undeclared artifact", () => { + const ir = graphWithForeach({}, { artifacts: [{ key: "OTHER.md" }] }); + expect(() => parseWorkflowIr(ir)).toThrow(/undeclared artifact 'PROMPT.md'/); + }); + + it("accepts parse-steps referencing a declared artifact", () => { + const ir = graphWithForeach({}, { artifacts: [{ key: "PROMPT.md", role: "step-source" }] }); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + }); + + it("allows only PROMPT.md when no artifacts are declared", () => { + const ir = graphWithForeach({}); + (ir.nodes.find((n) => n.id === "ps")!.config as Record).artifact = "SPEC.md"; + expect(() => parseWorkflowIr(ir)).toThrow(/only 'PROMPT.md' is allowed/); + }); +}); + +describe("code node validation", () => { + function graphWithCode(config: Record): WorkflowIrV2 { + return v2( + [ + { id: "start", kind: "start" }, + { id: "c", kind: "code", config }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "c" }, + { from: "c", to: "end" }, + ], + ); + } + + it("rejects empty source", () => { + expect(() => parseWorkflowIr(graphWithCode({ source: "" }))).toThrow(/non-empty source/); + }); + + it("rejects source over 64KB", () => { + expect(() => parseWorkflowIr(graphWithCode({ source: "x".repeat(65537) }))).toThrow( + /exceeds 65536/, + ); + }); + + it("accepts valid source and timeout", () => { + expect(() => + parseWorkflowIr(graphWithCode({ source: "export default async () => ({})", timeoutMs: 30000 })), + ).not.toThrow(); + }); + + it("rejects timeoutMs out of range", () => { + expect(() => parseWorkflowIr(graphWithCode({ source: "x", timeoutMs: 999 }))).toThrow( + /timeoutMs must be an integer in 1000\.\.300000/, + ); + expect(() => parseWorkflowIr(graphWithCode({ source: "x", timeoutMs: 300001 }))).toThrow( + /timeoutMs must be an integer in 1000\.\.300000/, + ); + }); +}); + +describe("fields validation", () => { + function graphWithFields(fields: unknown): WorkflowIrV2 { + return v2( + [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + [{ from: "start", to: "end" }], + { fields: fields as WorkflowIrV2["fields"] }, + ); + } + + it("accepts well-formed fields", () => { + expect(() => + parseWorkflowIr( + graphWithFields([ + { id: "sev", name: "Severity", type: "enum", options: [{ value: "lo", label: "Low" }] }, + { id: "note", name: "Note", type: "text", render: { placement: "detail", widget: "textarea" } }, + ]), + ), + ).not.toThrow(); + }); + + it("rejects duplicate field ids", () => { + expect(() => + parseWorkflowIr( + graphWithFields([ + { id: "a", name: "A", type: "string" }, + { id: "a", name: "A2", type: "number" }, + ]), + ), + ).toThrow(/duplicate field id 'a'/); + }); + + it("rejects unknown field type", () => { + expect(() => parseWorkflowIr(graphWithFields([{ id: "a", name: "A", type: "color" }]))).toThrow( + /unknown type 'color'/, + ); + }); + + it("requires options on enum/multi-enum", () => { + expect(() => parseWorkflowIr(graphWithFields([{ id: "a", name: "A", type: "enum" }]))).toThrow( + /must declare non-empty options/, + ); + expect(() => + parseWorkflowIr(graphWithFields([{ id: "a", name: "A", type: "multi-enum", options: [] }])), + ).toThrow(/must declare non-empty options/); + }); + + it("rejects options on non-enum types", () => { + expect(() => + parseWorkflowIr( + graphWithFields([{ id: "a", name: "A", type: "string", options: [{ value: "x", label: "X" }] }]), + ), + ).toThrow(/must not declare options/); + }); + + it("rejects bad render placement / widget", () => { + expect(() => + parseWorkflowIr(graphWithFields([{ id: "a", name: "A", type: "string", render: { placement: "footer" } }])), + ).toThrow(/render.placement 'footer' is not allowed/); + expect(() => + parseWorkflowIr(graphWithFields([{ id: "a", name: "A", type: "string", render: { widget: "slider" } }])), + ).toThrow(/render.widget 'slider' is not allowed/); + }); +}); + +describe("downgradeIrToV1IfPure refuses step-inversion features", () => { + it("returns v2 unchanged for a graph with a foreach", () => { + const ir = parseWorkflowIr(graphWithForeach({})) as WorkflowIrV2; + expect(downgradeIrToV1IfPure(ir).version).toBe("v2"); + }); + + it("returns v2 unchanged when fields/artifacts are declared even with pure-v1 nodes", () => { + const ir = v2( + [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + [{ from: "start", to: "end" }], + { fields: [{ id: "a", name: "A", type: "string" }] }, + ); + expect(downgradeIrToV1IfPure(ir).version).toBe("v2"); + }); +}); + +describe("JSON round-trip stability", () => { + it("re-parses a serialized foreach graph identically", () => { + const ir = parseWorkflowIr(graphWithForeach({ maxReworkCycles: 3 })) as WorkflowIrV2; + const serialized = serializeWorkflowIr(ir); + const reparsed = parseWorkflowIr(serialized) as WorkflowIrV2; + expect(serializeWorkflowIr(reparsed)).toBe(serialized); + }); +}); + +describe("illegal cycle detection (rework exemption)", () => { + it("still rejects a non-rework cycle at the top level", () => { + const ir = v2( + [ + { id: "start", kind: "start" }, + { id: "a", kind: "prompt" }, + { id: "b", kind: "prompt" }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "a" }, + { from: "a", to: "b" }, + { from: "b", to: "a" }, + { from: "a", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/illegal cycle/); + }); + + it("does not complain about the rework cycle inside a foreach template", () => { + // graphWithForeach's template has a rework edge rev → se; should parse fine. + expect(() => parseWorkflowIr(graphWithForeach({}))).not.toThrow(); + }); +}); diff --git a/packages/core/src/__tests__/workflow-step-instances.test.ts b/packages/core/src/__tests__/workflow-step-instances.test.ts new file mode 100644 index 0000000000..8968f58452 --- /dev/null +++ b/packages/core/src/__tests__/workflow-step-instances.test.ts @@ -0,0 +1,290 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import type { WorkflowRunStepInstance } from "../types.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/** + * Step-inversion U4 (KTD-6/KTD-13): persistence groundwork for the foreach + * step-instance region. Covers the workflow_run_step_instances CRUD trio + * (save/load/clear) — upsert-on-conflict, per-run pruning, load ordering — plus + * the raw tasks.customFields JSON round-trip through create/update/get. + * + * The CRUD trio mirrors workflow_run_branches: a `save` is an idempotent UPSERT + * keyed by (taskId, runId, foreachNodeId, stepIndex); `load` returns the run's + * rows ordered by stepIndex; `clear` prunes either everything-but-a-kept-run + * (per-run prune) or, with no runId, every row for the task. + */ + +describe("workflow_run_step_instances CRUD (U4, KTD-6)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + type StepInstanceStore = { + saveWorkflowRunStepInstance(state: WorkflowRunStepInstance): void; + loadWorkflowRunStepInstances(taskId: string, runId: string): WorkflowRunStepInstance[]; + clearWorkflowRunStepInstances(taskId: string, keepRunId?: string): void; + }; + const sis = (): StepInstanceStore => store as unknown as StepInstanceStore; + + function rawCount(taskId: string): number { + const db = (store as unknown as { db: { prepare: (s: string) => { get: (...a: unknown[]) => unknown } } }).db; + const row = db + .prepare("SELECT COUNT(*) AS c FROM workflow_run_step_instances WHERE taskId = ?") + .get(taskId) as { c: number }; + return row.c; + } + + function makeInstance(overrides: Partial = {}): WorkflowRunStepInstance { + return { + taskId: "T-1", + runId: "r1", + foreachNodeId: "fe", + stepIndex: 0, + pinnedStepCount: 3, + currentNodeId: "n1", + status: "in-progress", + baselineSha: "abc123", + checkpointId: "ckpt-1", + reworkCount: 0, + branchName: null, + integratedAt: null, + updatedAt: "2026-06-04T00:00:00.000Z", + ...overrides, + }; + } + + it("round-trips a full instance row through save → load", async () => { + const t = await store.createTask({ description: "stepped" }); + const inst = makeInstance({ + taskId: t.id, + branchName: "step/0", + integratedAt: "2026-06-04T01:00:00.000Z", + status: "completed", + reworkCount: 2, + }); + sis().saveWorkflowRunStepInstance(inst); + + const [loaded] = sis().loadWorkflowRunStepInstances(t.id, "r1"); + expect(loaded.taskId).toBe(t.id); + expect(loaded.runId).toBe("r1"); + expect(loaded.foreachNodeId).toBe("fe"); + expect(loaded.stepIndex).toBe(0); + expect(loaded.pinnedStepCount).toBe(3); + expect(loaded.currentNodeId).toBe("n1"); + expect(loaded.status).toBe("completed"); + expect(loaded.baselineSha).toBe("abc123"); + expect(loaded.checkpointId).toBe("ckpt-1"); + expect(loaded.reworkCount).toBe(2); + expect(loaded.branchName).toBe("step/0"); + expect(loaded.integratedAt).toBe("2026-06-04T01:00:00.000Z"); + expect(typeof loaded.updatedAt).toBe("string"); + }); + + it("save UPSERTS on (taskId, runId, foreachNodeId, stepIndex) conflict", async () => { + const t = await store.createTask({ description: "upsert" }); + sis().saveWorkflowRunStepInstance( + makeInstance({ taskId: t.id, stepIndex: 0, currentNodeId: "n1", status: "in-progress", reworkCount: 0 }), + ); + // Same PK — overwrites in place, not a second row. + sis().saveWorkflowRunStepInstance( + makeInstance({ taskId: t.id, stepIndex: 0, currentNodeId: "n5", status: "completed", reworkCount: 1 }), + ); + // Different stepIndex — a new row. + sis().saveWorkflowRunStepInstance( + makeInstance({ taskId: t.id, stepIndex: 1, currentNodeId: "n2", status: "pending" }), + ); + + expect(rawCount(t.id)).toBe(2); + const loaded = sis().loadWorkflowRunStepInstances(t.id, "r1"); + const step0 = loaded.find((row) => row.stepIndex === 0); + expect(step0?.currentNodeId).toBe("n5"); + expect(step0?.status).toBe("completed"); + expect(step0?.reworkCount).toBe(1); + }); + + it("persists nullable anchors as null and reads them back as null", async () => { + const t = await store.createTask({ description: "nulls" }); + sis().saveWorkflowRunStepInstance( + makeInstance({ + taskId: t.id, + currentNodeId: null, + baselineSha: null, + checkpointId: null, + branchName: null, + integratedAt: null, + status: "pending", + }), + ); + const [loaded] = sis().loadWorkflowRunStepInstances(t.id, "r1"); + expect(loaded.currentNodeId).toBeNull(); + expect(loaded.baselineSha).toBeNull(); + expect(loaded.checkpointId).toBeNull(); + expect(loaded.branchName).toBeNull(); + expect(loaded.integratedAt).toBeNull(); + }); + + it("loadWorkflowRunStepInstances returns the run ordered by stepIndex", async () => { + const t = await store.createTask({ description: "ordered" }); + // Insert out of order. + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, stepIndex: 2, currentNodeId: "n2" })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, stepIndex: 0, currentNodeId: "n0" })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, stepIndex: 1, currentNodeId: "n1" })); + + const loaded = sis().loadWorkflowRunStepInstances(t.id, "r1"); + expect(loaded.map((row) => row.stepIndex)).toEqual([0, 1, 2]); + }); + + it("loadWorkflowRunStepInstances scopes to the requested run only", async () => { + const t = await store.createTask({ description: "scoped" }); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "r1", stepIndex: 0 })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "r2", stepIndex: 0 })); + expect(sis().loadWorkflowRunStepInstances(t.id, "r1").length).toBe(1); + expect(sis().loadWorkflowRunStepInstances(t.id, "r2").length).toBe(1); + }); + + it("clear with keepRunId prunes every other run, keeps the kept run", async () => { + const t = await store.createTask({ description: "prune" }); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "old", stepIndex: 0 })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "old", stepIndex: 1 })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "cur", stepIndex: 0 })); + + sis().clearWorkflowRunStepInstances(t.id, "cur"); + + expect(rawCount(t.id)).toBe(1); + expect(sis().loadWorkflowRunStepInstances(t.id, "old").length).toBe(0); + expect(sis().loadWorkflowRunStepInstances(t.id, "cur").length).toBe(1); + }); + + it("clear with no keepRunId prunes all rows for the task", async () => { + const t = await store.createTask({ description: "wipe" }); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "r1", stepIndex: 0 })); + sis().saveWorkflowRunStepInstance(makeInstance({ taskId: t.id, runId: "r2", stepIndex: 0 })); + + sis().clearWorkflowRunStepInstances(t.id); + + expect(rawCount(t.id)).toBe(0); + }); +}); + +describe("tasks.customFields JSON round-trip under a fielded workflow (U11/KTD-13)", () => { + // U11 behavior change vs. U4: customFields is no longer an opaque whole-object + // round-trip — every write is now validated against the task's workflow field + // schema through the single store authority (task-fields.ts). The default + // workflow declares no fields, so the original U4 tests (which wrote arbitrary + // keys onto a default-workflow task) would now be rejected with + // `no-fields-defined`. They are reworked here to attach a workflow that + // declares the fields under test, and `updateTask` is now a MERGE-with-delete + // patch (not whole-object replacement). The zero-fields rejection path is + // covered in task-fields.test.ts. + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + // A v2 workflow declaring the fields exercised below. + const fieldedIr = (): WorkflowIr => + ({ + version: "v2", + name: "fielded", + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields: [ + { + id: "severity", + name: "Severity", + type: "enum", + options: [ + { value: "high", label: "High" }, + { value: "low", label: "Low" }, + ], + }, + { id: "points", name: "Points", type: "number" }, + { id: "flagged", name: "Flagged", type: "boolean" }, + { + id: "tags", + name: "Tags", + type: "multi-enum", + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + }, + { id: "keep", name: "Keep", type: "string" }, + { id: "a", name: "A", type: "number" }, + { id: "b", name: "B", type: "number" }, + ], + }) as unknown as WorkflowIr; + + let workflowId: string; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + const def = await (store as any).createWorkflowDefinition({ name: "Fielded", ir: fieldedIr() }); + workflowId = def.id; + }); + afterEach(async () => { + await harness.afterEach(); + }); + + async function fieldedTask(description: string) { + const t = await store.createTask({ description }); + await (store as any).selectTaskWorkflow(t.id, workflowId); + return t; + } + + it("a freshly created task has no customFields (legacy-shape default)", async () => { + const t = await store.createTask({ description: "no fields" }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({}); + }); + + it("round-trips a validated customFields object through updateTask → getTask", async () => { + const t = await fieldedTask("fielded"); + await store.updateTask(t.id, { + customFields: { severity: "high", points: 3, flagged: true, tags: ["a", "b"] }, + }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ severity: "high", points: 3, flagged: true, tags: ["a", "b"] }); + }); + + it("updateTask MERGES the customFields patch (U11 change from U4's whole-object replace)", async () => { + const t = await fieldedTask("merge"); + await store.updateTask(t.id, { customFields: { a: 1, b: 2 } }); + await store.updateTask(t.id, { customFields: { a: 9 } }); + const got = await store.getTask(t.id); + // U11 merge semantics: `b` survives, `a` is overwritten. (U4 replaced wholesale.) + expect(got?.customFields).toEqual({ a: 9, b: 2 }); + }); + + it("null in the patch deletes that field's value", async () => { + const t = await fieldedTask("delete"); + await store.updateTask(t.id, { customFields: { a: 1, b: 2 } }); + await store.updateTask(t.id, { customFields: { a: null } }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ b: 2 }); + }); + + it("leaves customFields untouched when an unrelated field is updated", async () => { + const t = await fieldedTask("untouched"); + await store.updateTask(t.id, { customFields: { keep: "me" } }); + await store.updateTask(t.id, { summary: "an unrelated change" }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ keep: "me" }); + }); +}); diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts new file mode 100644 index 0000000000..150e47f96e --- /dev/null +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -0,0 +1,151 @@ +import type { WorkflowIr } from "./workflow-ir-types.js"; +import { parseWorkflowIr } from "./workflow-ir.js"; + +/** + * The built-in **stepwise** coding workflow (KTD-9) — the demonstration of step + * inversion and the parity-comparison subject for the engine's + * `stepwise-workflow-parity.test.ts`. + * + * Unlike the default `builtin-coding-workflow-ir` (which keeps a single monolithic + * `execute` seam and is the byte-identity parity oracle, KTD-1), this workflow + * models per-step policy explicitly as graph structure: + * + * plan seam + * → parse-steps(PROMPT.md, step-headings) (KTD-12: graph-native parse) + * → foreach(task-steps, sequential, shared) { (KTD-3: runtime expansion) + * step-execute (KTD-2: run one step) + * → step-review(code): (KTD-4: verdicts as edges) + * approve → step-done (template exit) (APPROVE auto-completes) + * revise → rework back to step-execute (revise in place, no reset) + * rethink → rework back to step-execute (reset semantics handler-side) + * unavailable → (advisory) routes onward + * } + * rework-exhausted → hold(manual) (KTD-5: bounded escalation) + * → review seam + * → merge seam + * + * The columns/traits are identical to the default builtin so the full lifecycle + * (merge-blocker, capacity, hold, complete, archived) behaves exactly as it does + * for the default workflow — only the in-progress step modeling differs. + * + * It declares its step-source artifact (KTD-12): PROMPT.md produced by the + * planning seam. The IR is v2-only (foreach/step-review/parse-steps are v2 node + * kinds), so `downgradeIrToV1IfPure` refuses it and the flag-OFF rollback contract + * (KTD-8) is preserved automatically. + */ +const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { + version: "v2", + name: "builtin-stepwise-coding", + columns: [ + { id: "triage", name: "Triage", traits: [{ trait: "intake" }] }, + { + id: "todo", + name: "Todo", + traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }], + }, + { + id: "in-progress", + name: "In progress", + traits: [{ trait: "wip" }, { trait: "abort-on-exit" }, { trait: "timing" }], + }, + { + id: "in-review", + name: "In review", + traits: [{ trait: "merge-blocker" }, { trait: "stall-detection" }, { trait: "merge" }], + }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + { id: "archived", name: "Archived", traits: [{ trait: "archived" }] }, + ], + // KTD-12: PROMPT.md is the planning-produced step-source artifact this workflow + // parses into task steps. + artifacts: [{ key: "PROMPT.md", title: "Plan", producedBy: "planning", role: "step-source" }], + nodes: [ + { id: "start", kind: "start", column: "triage" }, + // Planning seam: produces PROMPT.md (the declared step-source artifact). + { id: "plan", kind: "prompt", column: "in-progress", config: { seam: "planning" } }, + // KTD-12: parse the planned PROMPT.md into the task step list. This node must + // dominate the foreach (validator-enforced). + { + id: "parse", + kind: "parse-steps", + column: "in-progress", + config: { artifact: "PROMPT.md", parser: "step-headings" }, + }, + // KTD-3: runtime-expanding per-step region. Sequential + shared isolation is + // the default baseline physics (one step at a time in the task's worktree). + { + id: "steps", + kind: "foreach", + column: "in-progress", + config: { + source: "task-steps", + mode: "sequential", + isolation: "shared", + maxReworkCycles: 3, + template: { + nodes: [ + // KTD-2: run exactly this step inside the task's session/worktree. + { id: "step-execute", kind: "prompt", config: { seam: "step-execute" } }, + // KTD-4: per-step code review; verdicts become outcome edges. + { id: "step-review", kind: "step-review", config: { type: "code" } }, + // Template exit (the single sink the validator requires): a config-less + // gate is a pure pass-through (createGateHandler → success), so APPROVE + // routes here and the instance exits. The step is already marked done by + // the step-review APPROVE verdict (projection authority, KTD-4/KTD-7). + { id: "step-done", kind: "gate", config: {} }, + ], + edges: [ + { from: "step-execute", to: "step-review", condition: "success" }, + // APPROVE → template exit (step-done). The step-review verdict already + // marked the step done through the projection. + { from: "step-review", to: "step-done", condition: "outcome:approve" }, + // REVISE → rework back to step-execute, revise in place (no reset). + { + from: "step-review", + to: "step-execute", + condition: "outcome:revise", + kind: "rework", + }, + // RETHINK → rework back to step-execute; the traversal triggers + // resetStepToBaseline (reset semantics are handler-side, KTD-4/U5). + { + from: "step-review", + to: "step-execute", + condition: "outcome:rethink", + kind: "rework", + }, + ], + }, + }, + }, + // KTD-5: rework exhaustion escalates to a manual hold (a human releases it). + { id: "rework-hold", kind: "hold", column: "in-progress", config: { release: "manual" } }, + { id: "review", kind: "prompt", column: "in-review", config: { seam: "review" } }, + { id: "merge", kind: "prompt", column: "in-review", config: { seam: "merge" } }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "plan" }, + { from: "plan", to: "parse", condition: "success" }, + { from: "plan", to: "end", condition: "failure" }, + { from: "parse", to: "steps", condition: "success" }, + // parse-steps no-steps defaults to success; route it explicitly to the foreach + // (zero steps → foreach no-ops through its success edge, KTD-8/R8). + { from: "parse", to: "steps", condition: "outcome:no-steps" }, + { from: "parse", to: "end", condition: "failure" }, + { from: "parse", to: "end", condition: "outcome:parse-error" }, + { from: "steps", to: "review", condition: "success" }, + // KTD-5: bounded rework exhaustion → manual hold; release re-enters review. + { from: "steps", to: "rework-hold", condition: "outcome:rework-exhausted" }, + { from: "rework-hold", to: "review", condition: "success" }, + { from: "steps", to: "end", condition: "failure" }, + { from: "review", to: "merge", condition: "success" }, + { from: "review", to: "end", condition: "failure" }, + { from: "merge", to: "end", condition: "success" }, + { from: "merge", to: "end", condition: "failure" }, + ], +}; + +export const BUILTIN_STEPWISE_CODING_WORKFLOW_IR = parseWorkflowIr( + RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR, +); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index b8ecafc400..62e734cf44 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -1,3 +1,4 @@ +import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; import type { WorkflowDefinition } from "./workflow-definition-types.js"; import type { WorkflowIr } from "./workflow-ir-types.js"; import { parseWorkflowIr } from "./workflow-ir.js"; @@ -139,6 +140,32 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ }, ], }), + // The stepwise coding workflow (KTD-9) — step inversion as authored graph + // structure (parse-steps → foreach{ step-execute → step-review } → review → + // merge). Authored directly as a v2 IR (the `linear` helper only builds simple + // pipelines); it is read-only like every built-in. Requires the + // `workflowGraphExecutor` flag at run time (foreach/step-review/parse-steps are + // interpreter-only node kinds, KTD-8); under the flag-off compile path its + // step-inversion nodes are skipped, the same posture as the other seam nodes. + { + id: "builtin:stepwise-coding", + 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.", + ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, + layout: { + start: { x: 60, y: 160 }, + plan: { x: 230, y: 160 }, + parse: { x: 400, y: 160 }, + steps: { x: 570, y: 160 }, + "rework-hold": { x: 570, y: 320 }, + review: { x: 740, y: 160 }, + merge: { x: 910, y: 160 }, + end: { x: 1080, y: 160 }, + }, + createdAt: BUILTIN_TS, + updatedAt: BUILTIN_TS, + }, ]; const BUILTIN_BY_ID = new Map(BUILTIN_WORKFLOWS.map((wf) => [wf.id, wf])); diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index 0b9d5a0dba..6403a186f1 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 = 107; +const SCHEMA_VERSION = 108; export { SCHEMA_VERSION }; @@ -323,7 +323,8 @@ CREATE TABLE IF NOT EXISTS tasks ( checkoutLeaseEpoch INTEGER DEFAULT 0, deletedAt TEXT, allowResurrection INTEGER DEFAULT 0, - transitionPending TEXT + transitionPending TEXT, + customFields TEXT DEFAULT '{}' ); -- Config table (single row with project settings) @@ -589,6 +590,32 @@ CREATE TABLE IF NOT EXISTS workflow_run_branches ( ); CREATE INDEX IF NOT EXISTS idx_workflow_run_branches_task_run ON workflow_run_branches(taskId, runId); +-- Per-step-instance run state for the step-inversion foreach region (step-inversion +-- U4, KTD-6). One row per expanded step instance inside a foreach region; resume +-- reconstructs the instance set from pinnedStepCount + persisted currentNodeId/ +-- reworkCount without re-running completed instances. baselineSha/checkpointId +-- persist the RETHINK reset anchors (previously in-memory, lost on restart). +-- branchName/integratedAt and the "awaiting-integration" status serve parallel +-- mode (KTD-11) and are null/unused at concurrency 1. Additive-only, reconstructible. +-- status ∈ "pending" | "in-progress" | "awaiting-integration" | "completed" | "failed". +CREATE TABLE IF NOT EXISTS workflow_run_step_instances ( + taskId TEXT NOT NULL REFERENCES tasks(id) ON DELETE CASCADE, + runId TEXT NOT NULL, + foreachNodeId TEXT NOT NULL, + stepIndex INTEGER NOT NULL, + pinnedStepCount INTEGER NOT NULL, + currentNodeId TEXT, + status TEXT NOT NULL, + baselineSha TEXT, + checkpointId TEXT, + reworkCount INTEGER NOT NULL DEFAULT 0, + branchName TEXT, + integratedAt TEXT, + updatedAt TEXT NOT NULL, + PRIMARY KEY (taskId, runId, foreachNodeId, stepIndex) +); +CREATE INDEX IF NOT EXISTS idx_workflow_run_step_instances_task_run ON workflow_run_step_instances(taskId, runId); + -- Task documents (key-value store per task with revision tracking) CREATE TABLE IF NOT EXISTS task_documents ( id TEXT PRIMARY KEY, @@ -4229,6 +4256,41 @@ export class Database { }); } + // Migration 108: Step-inversion persistence (step-inversion U4, KTD-6/KTD-13). + // Adds workflow_run_step_instances — one row per expanded step instance inside a + // foreach region — so a crashed/restarted run reconstructs the instance set from + // pinnedStepCount + persisted currentNodeId/reworkCount, and the RETHINK reset + // anchors (baselineSha/checkpointId) survive restart (previously in-memory Maps). + // branchName/integratedAt + "awaiting-integration" status serve parallel mode + // (KTD-11; null/unused at concurrency 1). Also adds tasks.customFields (KTD-13), + // the JSON store for workflow-defined custom task field values. Additive-only, + // idempotent (table-exists / addColumnIfMissing guards); no backfill. + // status ∈ "pending" | "in-progress" | "awaiting-integration" | "completed" | "failed". + if (version < 108) { + this.applyMigration(108, () => { + this.db.exec(` + CREATE TABLE IF NOT EXISTS workflow_run_step_instances ( + taskId TEXT NOT NULL REFERENCES tasks(id) ON DELETE CASCADE, + runId TEXT NOT NULL, + foreachNodeId TEXT NOT NULL, + stepIndex INTEGER NOT NULL, + pinnedStepCount INTEGER NOT NULL, + currentNodeId TEXT, + status TEXT NOT NULL, + baselineSha TEXT, + checkpointId TEXT, + reworkCount INTEGER NOT NULL DEFAULT 0, + branchName TEXT, + integratedAt TEXT, + updatedAt TEXT NOT NULL, + PRIMARY KEY (taskId, runId, foreachNodeId, stepIndex) + ); + CREATE INDEX IF NOT EXISTS idx_workflow_run_step_instances_task_run ON workflow_run_step_instances(taskId, runId); + `); + this.addColumnIfMissing("tasks", "customFields", "TEXT DEFAULT '{}'"); + }); + } + } /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 67272ebed3..999eb211cb 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -63,8 +63,16 @@ export type { WorkflowHoldRelease, WorkflowJoinMode, WorkflowJoinBranchFailure, + // Step-inversion (KTD-3/12/13): foreach / artifacts / custom-field IR types. + WorkflowForeachConfig, + WorkflowIrArtifact, + WorkflowFieldDefinition, + WorkflowFieldType, + WorkflowFieldOption, + WorkflowFieldRender, } from "./workflow-ir-types.js"; export { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; +export { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; // ── Trait model (U2) ───────────────────────────────────────────────── export type { @@ -104,6 +112,26 @@ export { registerBuiltinTraits, } from "./builtin-traits.js"; export type { BuiltinTraitId } from "./builtin-traits.js"; +// Step-inversion U12 (KTD-12): step-parser registry + built-ins. +export { + StepParserRegistry, + StepParserRegistrationError, + getStepParserRegistry, + registerStepParser, + getStepParser, + listStepParsers, + unregisterStepParser, + registerBuiltinStepParsers, + parseStepHeadings, + parseJsonSteps, + __resetStepParserRegistryForTests, +} from "./step-parsers.js"; +export type { + StepParser, + StepParseResult, + ParsedStep, + StepParserRegistrationReason, +} from "./step-parsers.js"; export { registerDefaultWorkflowHooks, __resetDefaultWorkflowHooksForTests, @@ -151,9 +179,11 @@ export type { ColumnCapacity } from "./workflow-capacity.js"; export { OccupiedColumnsError, InvalidRehomeTargetError, + IncompatibleFieldChangeError, resolveEntryColumnId, resolveSwitchReconciliation, computeRemovedOccupiedColumns, + computeIncompatibleFieldChanges, assertRehomeTargetValid, setReconciliationAbort, runReconciliationAbort, @@ -162,9 +192,24 @@ export { export type { SwitchReconciliation, ColumnOccupancy, + IncompatibleFieldChange, ReconciliationAbort, ReconciliationAbortContext, } from "./workflow-reconciliation.js"; +export { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, + makeCustomFieldRejection, + CustomFieldRejectionError, + CUSTOM_FIELD_REJECTION_CODES, +} from "./task-fields.js"; +export type { + CustomFieldRejection, + CustomFieldRejectionCode, + CustomFieldPatchResult, + FieldReconciliation, +} from "./task-fields.js"; export { readTransitionPending, writeTransitionPending, diff --git a/packages/core/src/step-parsers.ts b/packages/core/src/step-parsers.ts new file mode 100644 index 0000000000..4962c3b63d --- /dev/null +++ b/packages/core/src/step-parsers.ts @@ -0,0 +1,372 @@ +/** + * Step-parser registry (U12, KTD-12). + * + * Step parsing becomes a graph-native node (`parse-steps`): a registry resolves + * a parser id to an implementation that reads an artifact's content and yields a + * canonical step list. Built-ins: + * - `step-headings` — the extracted `parseStepsFromPrompt` logic (the + * `### Step N:` regex + `(depends: …)` annotation from U1); legacy callers + * in `store.ts` delegate to this exact function (byte-identical parity). + * - `json-steps` — a structured `[{ name, depends? }]` JSON document for + * workflows that plan in JSON. + * + * The registry mirrors the trait-registry posture: built-ins are protected from + * override, and plugins register under namespaced ids + * (`plugin::`). This module is engine-free and must NOT + * import `store.ts` (store imports the extracted parser from here). + * + * Parsers may throw on malformed input; callers (the engine's parse-steps + * handler) map a throw to a routable `outcome:parse-error`. + */ + +import type { TaskStep } from "./types.js"; + +// ── Parser contract ────────────────────────────────────────────────────────── + +/** A parsed step as produced by a parser. `dependsOn` is 0-indexed (same + * convention as the headings `(depends: …)` annotation). */ +export interface ParsedStep { + name: string; + dependsOn?: number[]; +} + +/** The result of running a step parser over an artifact's content. */ +export interface StepParseResult { + steps: ParsedStep[]; +} + +/** A step parser. `parse` may throw on malformed input; the caller maps a throw + * to a routable parse-error outcome. */ +export interface StepParser { + id: string; + parse(content: string): StepParseResult; +} + +// ── Registration error ────────────────────────────────────────────────────── + +/** Named reason codes for a rejected step-parser registration. */ +export type StepParserRegistrationReason = + | "duplicate-id" + | "builtin-namespace-protected" + | "invalid-id" + | "invalid-definition"; + +export class StepParserRegistrationError extends Error { + readonly reason: StepParserRegistrationReason; + readonly parserId: string; + constructor(reason: StepParserRegistrationReason, parserId: string, message: string) { + super(message); + this.name = "StepParserRegistrationError"; + this.reason = reason; + this.parserId = parserId; + } +} + +// ── The registry ──────────────────────────────────────────────────────────── + +interface RegisteredParser { + parser: StepParser; + builtin: boolean; +} + +/** Validate a plugin-namespaced parser id: `plugin::` with + * each segment a non-empty `[a-z0-9-]+` token. */ +function isValidPluginParserId(id: string): boolean { + const parts = id.split(":"); + if (parts.length !== 3) return false; + if (parts[0] !== "plugin") return false; + const seg = /^[a-z0-9-]+$/; + return seg.test(parts[1]) && seg.test(parts[2]); +} + +export class StepParserRegistry { + private readonly parsers = new Map(); + + /** Register a parser. Built-in ids cannot be overridden by non-builtins; a + * non-builtin must use a `plugin::` id. */ + register(parser: StepParser, opts?: { builtin?: boolean }): void { + const builtin = opts?.builtin ?? false; + if (!parser || typeof parser.id !== "string" || parser.id === "") { + throw new StepParserRegistrationError( + "invalid-definition", + String(parser?.id), + "Step parser must have a non-empty string id", + ); + } + if (typeof parser.parse !== "function") { + throw new StepParserRegistrationError( + "invalid-definition", + parser.id, + `Step parser '${parser.id}' must have a parse() function`, + ); + } + + // Existing-id checks first (built-in protection, then duplicate) so a + // non-builtin trying to overwrite a built-in surfaces the protection reason + // rather than the id-shape reason. + const existing = this.parsers.get(parser.id); + if (existing) { + if (!builtin && existing.builtin) { + throw new StepParserRegistrationError( + "builtin-namespace-protected", + parser.id, + `Step parser id '${parser.id}' is a built-in parser and cannot be overridden by a non-builtin registration`, + ); + } + throw new StepParserRegistrationError( + "duplicate-id", + parser.id, + `Step parser id '${parser.id}' is already registered`, + ); + } + + if (!builtin && !isValidPluginParserId(parser.id)) { + throw new StepParserRegistrationError( + "invalid-id", + parser.id, + `Non-builtin step parser '${parser.id}' must use a namespaced id of the form 'plugin::'`, + ); + } + + this.parsers.set(parser.id, { parser, builtin }); + } + + getParser(id: string): StepParser | undefined { + return this.parsers.get(id)?.parser; + } + + has(id: string): boolean { + return this.parsers.has(id); + } + + listParsers(): StepParser[] { + return [...this.parsers.values()].map((r) => r.parser); + } + + /** Remove a parser. Built-ins are never removed (callers should only pass + * plugin-namespaced ids — e.g. for plugin teardown). Returns true if a + * non-builtin parser was present and removed. */ + unregister(id: string): boolean { + const existing = this.parsers.get(id); + if (!existing || existing.builtin) return false; + return this.parsers.delete(id); + } +} + +// ── Built-in: step-headings ─────────────────────────────────────────────────── + +/** + * Parse `### Step N:` headings into the task step list (step-inversion U1). + * + * Backward compatibility is exact: an UNannotated heading parses byte-identically + * to the legacy regex `^###\s+Step\s+\d+[^:]*:\s*(.+)$` (name = text after the + * first colon, trimmed). + * + * The annotation `### Step N (depends: 1,2): Title` is parsed explicitly (the + * legacy regex breaks on the colon inside `depends:`): depends values are + * 1-indexed step numbers in the document and are stored as 0-indexed indices on + * `dependsOn` (deduped, sorted, dropping values <= 0). + * + * Malformed `(depends: …)` annotations fall back deterministically: the heading + * is treated as `### Step N:` with the name starting after the FIRST colon + * following the closing paren (if present), else after the first colon — and no + * `dependsOn` is recorded. + */ +export function parseStepHeadings(content: string): TaskStep[] { + const steps: TaskStep[] = []; + // Legacy matcher — UNCHANGED from the original implementation, so unannotated + // headings (and every legacy edge case, including `[^:]*` spanning newlines) + // parse byte-identically. The full match (`m[0]`) is re-inspected only to layer + // the `(depends: …)` annotation on top. + const stepRegex = /^###\s+Step\s+\d+[^:]*:\s*(.+)$/gm; + // Well-formed annotation form: `### Step N (depends: …): name`. + const annotatedRegex = /^###\s+Step\s+\d+\s*\(depends:\s*([^)]*)\)\s*:\s*([^\n]+)$/; + + let match: RegExpExecArray | null; + while ((match = stepRegex.exec(content)) !== null) { + const full = match[0]; + + // No annotation present → byte-identical legacy behavior. + if (!full.includes("(depends:")) { + steps.push({ name: match[1].trim(), status: "pending" }); + continue; + } + + // 1) Well-formed depends annotation. + const annotated = annotatedRegex.exec(full); + if (annotated) { + const parsed = parseDependsList(annotated[1]); + const name = annotated[2].trim(); + if (parsed !== null) { + if (parsed.length > 0) steps.push({ name, status: "pending", dependsOn: parsed }); + else steps.push({ name, status: "pending" }); + continue; + } + } + + // 2) Annotation present but unparseable (bad values or no closing paren): + // deterministic fallback — name starts after the FIRST colon following the + // closing paren if present, else after the first colon. Operate on the + // first line of the match only (the heading line itself). + const line = full.split("\n")[0]; + const parenIdx = line.indexOf(")"); + const colonAfterParen = parenIdx >= 0 ? line.indexOf(":", parenIdx) : -1; + const colonIdx = colonAfterParen >= 0 ? colonAfterParen : line.indexOf(":"); + if (colonIdx >= 0) { + const fallbackName = line.slice(colonIdx + 1).trim(); + if (fallbackName) steps.push({ name: fallbackName, status: "pending" }); + } + } + return steps; +} + +/** Parse a `depends:` value list (1-indexed step numbers) into 0-indexed, + * deduped, sorted indices. Returns null if any token is not a positive integer. */ +function parseDependsList(raw: string): number[] | null { + const trimmed = raw.trim(); + if (trimmed === "") return []; + const tokens = trimmed.split(",").map((t) => t.trim()); + const out = new Set(); + for (const token of tokens) { + if (!/^\d+$/.test(token)) return null; + const n = Number(token); + if (!Number.isInteger(n) || n < 1) return null; + out.add(n - 1); + } + return [...out].sort((a, b) => a - b); +} + +// ── Built-in: json-steps ────────────────────────────────────────────────────── + +/** + * Parse a JSON document: an array of `{ name: string, depends?: number[] }`. + * `depends` values are 1-indexed step numbers in the document (same convention + * as the headings annotation), converted to 0-indexed `dependsOn` (deduped, + * sorted). Throws a descriptive error on any malformed input (not JSON, not an + * array, missing/blank name, bad depends). + */ +export function parseJsonSteps(content: string): StepParseResult { + let doc: unknown; + try { + doc = JSON.parse(content); + } catch (err) { + throw new Error( + `json-steps: content is not valid JSON: ${(err as Error).message}`, + ); + } + + if (!Array.isArray(doc)) { + throw new Error("json-steps: document must be a JSON array of step objects"); + } + + const steps: ParsedStep[] = []; + doc.forEach((entry, i) => { + if (typeof entry !== "object" || entry === null || Array.isArray(entry)) { + throw new Error(`json-steps: step at index ${i} must be an object`); + } + const obj = entry as Record; + const name = obj.name; + if (typeof name !== "string" || name.trim() === "") { + throw new Error( + `json-steps: step at index ${i} must have a non-empty string 'name'`, + ); + } + + const step: ParsedStep = { name: name.trim() }; + + if (obj.depends !== undefined) { + if (!Array.isArray(obj.depends)) { + throw new Error( + `json-steps: step at index ${i} 'depends' must be an array of positive integers`, + ); + } + const out = new Set(); + for (const raw of obj.depends) { + if (typeof raw !== "number" || !Number.isInteger(raw) || raw < 1) { + throw new Error( + `json-steps: step at index ${i} 'depends' must contain only positive integers (1-indexed step numbers); got ${JSON.stringify(raw)}`, + ); + } + out.add(raw - 1); + } + const dependsOn = [...out].sort((a, b) => a - b); + if (dependsOn.length > 0) step.dependsOn = dependsOn; + } + + steps.push(step); + }); + + return { steps }; +} + +// ── Built-in parser definitions ─────────────────────────────────────────────── + +const BUILTIN_STEP_PARSERS: StepParser[] = [ + { + id: "step-headings", + parse(content: string): StepParseResult { + // The headings parser yields TaskStep[]; map to the parser contract + // (dropping the `status` field, which the caller re-applies). + const steps = parseStepHeadings(content).map((s) => { + const out: ParsedStep = { name: s.name }; + if (s.dependsOn) out.dependsOn = s.dependsOn; + return out; + }); + return { steps }; + }, + }, + { + id: "json-steps", + parse: parseJsonSteps, + }, +]; + +/** Register the built-in step parsers into the given registry (defaults to the + * shared registry). Idempotent via `has`. */ +export function registerBuiltinStepParsers( + registry: StepParserRegistry = getStepParserRegistry(), +): void { + for (const parser of BUILTIN_STEP_PARSERS) { + if (registry.has(parser.id)) continue; + registry.register(parser, { builtin: true }); + } +} + +// ── Module-level default registry ─────────────────────────────────────────── + +let defaultRegistry: StepParserRegistry | undefined; + +export function getStepParserRegistry(): StepParserRegistry { + if (!defaultRegistry) { + defaultRegistry = new StepParserRegistry(); + registerBuiltinStepParsers(defaultRegistry); + } + return defaultRegistry; +} + +/** Test-only: reset the shared registry (so built-in registration can be + * re-exercised in isolation). */ +export function __resetStepParserRegistryForTests(): void { + defaultRegistry = undefined; +} + +// ── Convenience pass-throughs to the default registry ──────────────────────── + +export function registerStepParser(parser: StepParser, opts?: { builtin?: boolean }): void { + getStepParserRegistry().register(parser, opts); +} + +export function getStepParser(id: string): StepParser | undefined { + return getStepParserRegistry().getParser(id); +} + +export function listStepParsers(): StepParser[] { + return getStepParserRegistry().listParsers(); +} + +export function unregisterStepParser(id: string): boolean { + return getStepParserRegistry().unregister(id); +} + +// Register built-ins into the shared registry on import (idempotent via `has`). +registerBuiltinStepParsers(); diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 3a436d8903..352b281004 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -21,6 +21,8 @@ import { OccupiedColumnsError, assertRehomeTargetValid, computeRemovedOccupiedColumns, + computeIncompatibleFieldChanges, + IncompatibleFieldChangeError, resolveEntryColumnId, resolveSwitchReconciliation, runReconciliationAbort, @@ -43,10 +45,21 @@ import { reconcileHooksRemaining, } from "./transition-pending.js"; import { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; -import type { WorkflowIr, WorkflowIrColumn } from "./workflow-ir-types.js"; +import type { WorkflowIr, WorkflowIrColumn, WorkflowFieldDefinition } from "./workflow-ir-types.js"; +import { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, + CustomFieldRejectionError, + type CustomFieldRejection, +} from "./task-fields.js"; // Side-effect import: registers the 14 built-in trait DEFINITIONS into the // shared trait registry on load (the flag-ON path resolves traits by id). import "./builtin-traits.js"; +// Step-inversion U12 (KTD-12): the legacy `parseStepsFromPrompt` path resolves +// the `step-headings` parser through the registry (proving the registry path), +// staying byte-identical with the direct extracted function. +import { getStepParser } from "./step-parsers.js"; import type { WorkflowDefinition, WorkflowDefinitionInput, @@ -207,6 +220,7 @@ interface TaskRow { executionCompletedAt: string | null; dependencies: string | null; steps: string | null; + customFields: string | null; log: string | null; attachments: string | null; steeringComments: string | null; @@ -798,6 +812,12 @@ const KNOWN_FILE_SCOPE_ROOT_FILES = new Set([ "agents.md", ]); +// `parseStepHeadings` (the `### Step N:` parser, step-inversion U1) was extracted +// into `step-parsers.ts` as the `step-headings` built-in parser (U12, KTD-12). +// It is re-exported here for back-compat with callers/tests that import it from +// `store.ts`. `parseStepsFromPrompt` below delegates through the registry. +export { parseStepHeadings } from "./step-parsers.js"; + export function isValidFileScopeEntry(token: string): boolean { const trimmed = token.trim(); if (!trimmed) return false; @@ -1697,6 +1717,7 @@ export class TaskStore extends EventEmitter { executionCompletedAt: row.executionCompletedAt || undefined, dependencies: fromJson(row.dependencies) || [], steps: fromJson(row.steps) || [], + customFields: fromJson>(row.customFields) ?? undefined, log: fromJson(row.log) || [], tokenBudgetSoftAlertedAt: row.tokenBudgetSoftAlertedAt || undefined, tokenBudgetHardAlertedAt: row.tokenBudgetHardAlertedAt || undefined, @@ -1844,6 +1865,7 @@ export class TaskStore extends EventEmitter { dependencies: entry.dependencies ?? [], steps: entry.steps ?? [], currentStep: entry.currentStep ?? 0, + customFields: entry.customFields ?? undefined, size: entry.size, reviewLevel: entry.reviewLevel, prInfo: slim ? undefined : entry.prInfo, @@ -1976,6 +1998,7 @@ export class TaskStore extends EventEmitter { dependencies: task.dependencies, steps: task.steps, currentStep: task.currentStep, + customFields: task.customFields, size: task.size, reviewLevel: task.reviewLevel, prInfo: task.prInfo, @@ -2184,7 +2207,7 @@ export class TaskStore extends EventEmitter { "error", "summary", "thinkingLevel", "executionMode", "tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageCacheWriteTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt", "tokenBudgetSoftAlertedAt", "tokenBudgetHardAlertedAt", "tokenBudgetOverride", "createdAt", "updatedAt", "columnMovedAt", "firstExecutionAt", "cumulativeActiveMs", "executionStartedAt", "executionCompletedAt", - "dependencies", "steps", "comments", "review", "reviewState", "workflowStepResults", "steeringComments", + "dependencies", "steps", "customFields", "comments", "review", "reviewState", "workflowStepResults", "steeringComments", "attachments", "prInfo", "prInfos", "issueInfo", "githubTracking", "sourceIssueProvider", "sourceIssueRepository", "sourceIssueExternalIssueId", "sourceIssueNumber", "sourceIssueUrl", "mergeDetails", "breakIntoSubtasks", "noCommitsExpected", "enabledWorkflowSteps", "modifiedFiles", "missionId", "sliceId", "scopeOverride", "scopeOverrideReason", "scopeAutoWiden", "assignedAgentId", "pausedByAgentId", "assigneeUserId", "nodeId", "effectiveNodeId", "effectiveNodeSource", @@ -2233,7 +2256,7 @@ export class TaskStore extends EventEmitter { "error", "summary", "thinkingLevel", "executionMode", "tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageCacheWriteTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt", "tokenBudgetSoftAlertedAt", "tokenBudgetHardAlertedAt", "tokenBudgetOverride", "createdAt", "updatedAt", "columnMovedAt", "firstExecutionAt", "cumulativeActiveMs", "executionStartedAt", "executionCompletedAt", - "dependencies", "steps", "attachments", "steeringComments", + "dependencies", "steps", "customFields", "attachments", "steeringComments", "comments", "review", "reviewState", "workflowStepResults", "prInfo", "prInfos", "issueInfo", "githubTracking", "sourceIssueProvider", "sourceIssueRepository", "sourceIssueExternalIssueId", "sourceIssueNumber", "sourceIssueUrl", "mergeDetails", "breakIntoSubtasks", "noCommitsExpected", "enabledWorkflowSteps", "modifiedFiles", "missionId", "sliceId", "scopeOverride", "scopeOverrideReason", "scopeAutoWiden", "assignedAgentId", "pausedByAgentId", "assigneeUserId", "nodeId", "effectiveNodeId", "effectiveNodeSource", @@ -2335,6 +2358,7 @@ export class TaskStore extends EventEmitter { task.executionCompletedAt ?? null, toJson(task.dependencies || []), toJson(task.steps || []), + toJson(task.customFields ?? {}), toJson(task.log || []), toJson(task.attachments || []), toJson(task.steeringComments || []), @@ -2402,7 +2426,7 @@ export class TaskStore extends EventEmitter { summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens, tokenUsageCacheWriteTokens, tokenUsageTotalTokens, tokenUsageFirstUsedAt, tokenUsageLastUsedAt, tokenBudgetSoftAlertedAt, tokenBudgetHardAlertedAt, tokenBudgetOverride, createdAt, updatedAt, columnMovedAt, firstExecutionAt, cumulativeActiveMs, executionStartedAt, executionCompletedAt, - dependencies, steps, log, attachments, steeringComments, + dependencies, steps, customFields, log, attachments, steeringComments, comments, review, reviewState, workflowStepResults, prInfo, prInfos, issueInfo, githubTracking, sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl, mergeDetails, breakIntoSubtasks, noCommitsExpected, autoMerge, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, scopeOverride, scopeOverrideReason, scopeAutoWiden, assignedAgentId, pausedByAgentId, assigneeUserId, nodeId, effectiveNodeId, effectiveNodeSource, sourceType, sourceAgentId, sourceRunId, sourceSessionId, sourceMessageId, sourceParentTaskId, sourceMetadata, checkedOutBy, checkedOutAt, checkoutNodeId, checkoutRunId, checkoutLeaseRenewedAt, checkoutLeaseEpoch, deletedAt, allowResurrection @@ -2429,7 +2453,7 @@ export class TaskStore extends EventEmitter { summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens, tokenUsageCacheWriteTokens, tokenUsageTotalTokens, tokenUsageFirstUsedAt, tokenUsageLastUsedAt, tokenBudgetSoftAlertedAt, tokenBudgetHardAlertedAt, tokenBudgetOverride, createdAt, updatedAt, columnMovedAt, firstExecutionAt, cumulativeActiveMs, executionStartedAt, executionCompletedAt, - dependencies, steps, log, attachments, steeringComments, + dependencies, steps, customFields, log, attachments, steeringComments, comments, review, reviewState, workflowStepResults, prInfo, prInfos, issueInfo, githubTracking, sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl, mergeDetails, breakIntoSubtasks, noCommitsExpected, autoMerge, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, scopeOverride, scopeOverrideReason, scopeAutoWiden, assignedAgentId, pausedByAgentId, assigneeUserId, nodeId, effectiveNodeId, effectiveNodeSource, sourceType, sourceAgentId, sourceRunId, sourceSessionId, sourceMessageId, sourceParentTaskId, sourceMetadata, checkedOutBy, checkedOutAt, checkoutNodeId, checkoutRunId, checkoutLeaseRenewedAt, checkoutLeaseEpoch, deletedAt, allowResurrection @@ -2504,6 +2528,7 @@ export class TaskStore extends EventEmitter { executionCompletedAt = excluded.executionCompletedAt, dependencies = excluded.dependencies, steps = excluded.steps, + customFields = excluded.customFields, log = excluded.log, attachments = excluded.attachments, steeringComments = excluded.steeringComments, @@ -5214,6 +5239,103 @@ export class TaskStore extends EventEmitter { } } + /** + * Persist (idempotent upsert) one step instance's run-state inside a foreach + * region (step-inversion U4, KTD-6). Keyed by (taskId, runId, foreachNodeId, + * stepIndex) — the table PK — so re-writing the same instance overwrites its + * single row with the latest currentNodeId/status/anchors. `updatedAt` is + * stamped server-side. Mirrors `saveWorkflowRunBranch`: additive, silently + * no-ops on a legacy/missing table. + */ + saveWorkflowRunStepInstance( + state: import("./types.js").WorkflowRunStepInstance, + ): void { + try { + this.db + .prepare( + `INSERT INTO workflow_run_step_instances + (taskId, runId, foreachNodeId, stepIndex, pinnedStepCount, currentNodeId, status, baselineSha, checkpointId, reworkCount, branchName, integratedAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(taskId, runId, foreachNodeId, stepIndex) DO UPDATE SET + pinnedStepCount = excluded.pinnedStepCount, + currentNodeId = excluded.currentNodeId, + status = excluded.status, + baselineSha = excluded.baselineSha, + checkpointId = excluded.checkpointId, + reworkCount = excluded.reworkCount, + branchName = excluded.branchName, + integratedAt = excluded.integratedAt, + updatedAt = excluded.updatedAt`, + ) + .run( + state.taskId, + state.runId, + state.foreachNodeId, + state.stepIndex, + state.pinnedStepCount, + state.currentNodeId ?? null, + state.status, + state.baselineSha ?? null, + state.checkpointId ?? null, + state.reworkCount ?? 0, + state.branchName ?? null, + state.integratedAt ?? null, + new Date().toISOString(), + ); + } catch { + // Legacy/missing table — persistence is additive, so degrade silently. + } + } + + /** + * Load persisted step-instance run-state for a run (crash-resume; KTD-6). + * Ordered by stepIndex so the executor can reconstruct the instance set in + * step order. Additive: returns [] on a legacy/missing table. + */ + loadWorkflowRunStepInstances( + taskId: string, + runId: string, + ): import("./types.js").WorkflowRunStepInstance[] { + try { + const rows = this.db + .prepare( + `SELECT taskId, runId, foreachNodeId, stepIndex, pinnedStepCount, currentNodeId, status, baselineSha, checkpointId, reworkCount, branchName, integratedAt, updatedAt + FROM workflow_run_step_instances + WHERE taskId = ? AND runId = ? + ORDER BY stepIndex ASC`, + ) + .all(taskId, runId) as import("./types.js").WorkflowRunStepInstance[]; + return rows; + } catch { + return []; + } + } + + /** + * Prune step-instance rows for a task (KTD-6, #1412 pattern). When `runId` is + * provided, deletes every row for `taskId` whose runId differs (bounding growth + * across a long-lived task's repeated runs — call on run start/completion). + * When `runId` is omitted, deletes all rows for the task (e.g. on archive). + * Additive: silently no-ops on a legacy/missing table. + */ + clearWorkflowRunStepInstances(taskId: string, keepRunId?: string): void { + try { + if (keepRunId === undefined) { + this.db + .prepare(`DELETE FROM workflow_run_step_instances WHERE taskId = ?`) + .run(taskId); + } else { + this.db + .prepare( + `DELETE FROM workflow_run_step_instances WHERE taskId = ? AND runId != ?`, + ) + .run(taskId, keepRunId); + } + } catch { + // Legacy/missing table — pruning is additive, so degrade silently. + } + } + async listTasksForGithubTrackingReconcile(options?: { offset?: number; limit?: number }): Promise<{ tasks: Task[]; hasMore: boolean }> { const reconcileScanLimit = 200; const offset = Math.max(0, options?.offset ?? 0); @@ -6784,12 +6906,64 @@ export class TaskStore extends EventEmitter { async updateTask( id: string, - updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; currentStep?: number; blockedBy?: string | null; overlapBlockedBy?: string | null; assignedAgentId?: string | null; pausedByAgentId?: string | null; pausedReason?: string | null; tokenBudgetSoftAlertedAt?: string | null; worktrunkFallbackAlertedAt?: string | null; worktrunkFailure?: import("./types.js").Task["worktrunkFailure"] | null; tokenBudgetHardAlertedAt?: string | null; tokenBudgetOverride?: import("./types.js").TaskTokenBudgetOverride | null; dispatchStormCount?: number | null; lastDispatchAt?: string | null; assigneeUserId?: string | null; scopeOverride?: boolean | null; scopeOverrideReason?: string | null; scopeAutoWiden?: string[] | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; checkoutNodeId?: string | null; checkoutRunId?: string | null; checkoutLeaseRenewedAt?: string | null; checkoutLeaseEpoch?: number | null; paused?: boolean; baseBranch?: string | null; autoMerge?: boolean | null; branch?: string | null; executionStartBranch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; resumeLimboCount?: number | null; resumeLimboTipSha?: string | null; resumeLimboStepSignature?: string | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; worktreeSessionRetryCount?: number | null; completionHandoffLimboRecoveryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; mergeAuditBounceCount?: number | null; mergeTransientRetryCount?: number | null; branchConflictRecoveryCount?: number | null; reviewerContextRetryCount?: number | null; reviewerFallbackRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; noCommitsExpected?: boolean | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; firstExecutionAt?: string | null; cumulativeActiveMs?: number | null; executionStartedAt?: string | null; executionCompletedAt?: string | null; review?: import("./types.js").TaskReview | null; reviewState?: import("./types.js").TaskReviewState | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; sourceMetadataPatch?: Record | null; githubTracking?: import("./types.js").TaskGithubTracking | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null }, + updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; customFields?: Record; currentStep?: number; blockedBy?: string | null; overlapBlockedBy?: string | null; assignedAgentId?: string | null; pausedByAgentId?: string | null; pausedReason?: string | null; tokenBudgetSoftAlertedAt?: string | null; worktrunkFallbackAlertedAt?: string | null; worktrunkFailure?: import("./types.js").Task["worktrunkFailure"] | null; tokenBudgetHardAlertedAt?: string | null; tokenBudgetOverride?: import("./types.js").TaskTokenBudgetOverride | null; dispatchStormCount?: number | null; lastDispatchAt?: string | null; assigneeUserId?: string | null; scopeOverride?: boolean | null; scopeOverrideReason?: string | null; scopeAutoWiden?: string[] | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; checkoutNodeId?: string | null; checkoutRunId?: string | null; checkoutLeaseRenewedAt?: string | null; checkoutLeaseEpoch?: number | null; paused?: boolean; baseBranch?: string | null; autoMerge?: boolean | null; branch?: string | null; executionStartBranch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; resumeLimboCount?: number | null; resumeLimboTipSha?: string | null; resumeLimboStepSignature?: string | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; worktreeSessionRetryCount?: number | null; completionHandoffLimboRecoveryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; mergeAuditBounceCount?: number | null; mergeTransientRetryCount?: number | null; branchConflictRecoveryCount?: number | null; reviewerContextRetryCount?: number | null; reviewerFallbackRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; noCommitsExpected?: boolean | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; firstExecutionAt?: string | null; cumulativeActiveMs?: number | null; executionStartedAt?: string | null; executionCompletedAt?: string | null; review?: import("./types.js").TaskReview | null; reviewState?: import("./types.js").TaskReviewState | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; sourceMetadataPatch?: Record | null; githubTracking?: import("./types.js").TaskGithubTracking | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null }, runContext?: RunMutationContext, ): Promise { return this.withTaskLock(id, () => this.updateTaskUnlocked(id, updates, runContext)); } + /** + * Merge a validated/normalized custom-field patch into the existing values. + * `null` in the patch deletes that field's value (the delete sentinel from + * {@link validateCustomFieldPatch}); any other value overwrites. Returns a new + * object (never mutates the input) so the caller assigns it onto the task. + */ + private mergeCustomFieldPatch( + current: Record | undefined, + patch: Record, + ): Record { + const next: Record = { ...(current ?? {}) }; + for (const [key, value] of Object.entries(patch)) { + if (value === null) { + delete next[key]; + } else { + next[key] = value; + } + } + return next; + } + + /** + * Single write authority for custom task fields (U11 / KTD-13). + * + * Resolves the task's workflow field definitions, validates `patch` against + * them via {@link validateCustomFieldPatch}, merges the normalized result into + * `Task.customFields` (delete-on-null), persists through the standard update + * path, and emits `task:updated` like every other task mutation. A workflow + * with no fields (e.g. the default) rejects any non-empty patch with + * `no-fields-defined`. Returns a typed result rather than throwing so callers + * (agent tools, HTTP routes) can surface the field path/code directly. + */ + async updateTaskCustomFields( + taskId: string, + patch: Record, + runContext?: RunMutationContext, + ): Promise<{ ok: true; task: Task } | { ok: false; rejection: CustomFieldRejection }> { + return this.withTaskLock(taskId, async () => { + const defs = this.resolveTaskCustomFieldDefsSync(taskId); + const result = validateCustomFieldPatch(defs, patch); + if (!result.ok) { + return { ok: false as const, rejection: result.rejection }; + } + // Pass the validated PATCH through (with null delete-sentinels) — the + // merge-with-delete happens once, inside updateTaskUnlocked, against the + // freshly-read task. Pre-merging here would lose the delete semantics on + // the second merge. + const task = await this.updateTaskUnlocked(taskId, { customFields: result.normalized }, runContext); + return { ok: true as const, task }; + }); + } + /** * The body of {@link updateTask} WITHOUT acquiring the per-task lock. Callers * that already hold `withTaskLock(id)` — e.g. workflow-selection mutations @@ -6898,6 +7072,19 @@ export class TaskStore extends EventEmitter { } } if (updates.steps !== undefined) task.steps = updates.steps; + // U11/KTD-13: customFields writes are validated against the task's workflow + // field schema through the single authority (task-fields.ts). The patch is + // merged into the existing values (delete-on-null), mirroring + // updateTaskCustomFields. Backward-compat note: U4 round-tripped the object + // opaquely; the field system now enforces type/enum/unknown-id rules, so a + // write against a workflow with no fields (the default) is rejected with a + // typed CustomFieldRejectionError rather than silently persisted. + if (updates.customFields !== undefined) { + const defs = this.resolveTaskCustomFieldDefsSync(id); + const result = validateCustomFieldPatch(defs, updates.customFields); + if (!result.ok) throw new CustomFieldRejectionError(result.rejection); + task.customFields = this.mergeCustomFieldPatch(task.customFields, result.normalized); + } if (updates.currentStep !== undefined) task.currentStep = updates.currentStep; if (updates.status === null) { task.status = undefined; @@ -7547,13 +7734,27 @@ export class TaskStore extends EventEmitter { id: string, stepIndex: number, status: import("./types.js").StepStatus, + options?: { source?: "graph" }, ): Promise { + // Step-inversion projection discipline (U6/KTD-7). A `source: "graph"` write + // is the workflow-graph executor projecting a foreach instance's lifecycle + // (in-progress / done / pending) onto Task.steps[] with EXPLICIT indices. Three + // behaviors diverge from the legacy (default) write: + // (a) the out-of-order-done guard relaxes from strict index order to + // DEPENDENCY order (a done write is legal when every dependsOn step — + // default: the immediately-preceding step — is done/skipped, KTD-11); + // (b) a guard that DOES suppress a graph write logs an audit warning loudly + // (legacy stays silent — a graph suppression is a projection bug); + // (c) the auto-reinit-from-PROMPT.md path is bypassed (the graph pinned the + // step count at foreach expansion; re-parsing here would desync, KTD-3). + const graphSource = options?.source === "graph"; return this.withTaskLock(id, async () => { const dir = this.taskDir(id); const task = await this.readTaskJson(dir); - // Auto-initialize steps from PROMPT.md if empty - if (task.steps.length === 0) { + // Auto-initialize steps from PROMPT.md if empty. Bypassed for graph-source + // writes (U6/KTD-3): the graph owns explicit indices pinned at expansion. + if (task.steps.length === 0 && !graphSource) { task.steps = await this.parseStepsFromPrompt(id); } @@ -7590,22 +7791,63 @@ export class TaskStore extends EventEmitter { } if (status === "done") { - for (let i = 0; i < stepIndex; i++) { - const priorStatus = task.steps[i].status; - if (priorStatus === "pending" || priorStatus === "in-progress") { - const ts = new Date().toISOString(); - task.updatedAt = ts; + // The set of predecessor steps that must be done/skipped before this step + // may go done. Legacy: strict index order (every earlier step). Graph: the + // step's dependsOn list (default = the immediately-preceding step when the + // annotation is absent — preserving sequential behavior, KTD-11). + let blockingIndex = -1; + let blockingStatus: import("./types.js").StepStatus | undefined; + if (graphSource) { + const deps = task.steps[stepIndex]?.dependsOn; + const depIndices = + Array.isArray(deps) && deps.length > 0 + ? deps + : stepIndex > 0 + ? [stepIndex - 1] + : []; + for (const i of depIndices) { + const priorStatus = task.steps[i]?.status; + if (priorStatus === "pending" || priorStatus === "in-progress") { + blockingIndex = i; + blockingStatus = priorStatus; + break; + } + } + } else { + for (let i = 0; i < stepIndex; i++) { + const priorStatus = task.steps[i].status; + if (priorStatus === "pending" || priorStatus === "in-progress") { + blockingIndex = i; + blockingStatus = priorStatus; + break; + } + } + } + if (blockingIndex !== -1) { + const ts = new Date().toISOString(); + task.updatedAt = ts; + const kind = graphSource ? "dependency-order" : "out-of-order"; + task.log.push({ + timestamp: ts, + action: + `Ignored ${kind} ${status} for step ${stepIndex} (${task.steps[stepIndex].name}) — ` + + `${graphSource ? "dependency" : "earlier"} step ${blockingIndex} (${task.steps[blockingIndex].name}) is still ${blockingStatus}`, + }); + // Graph-source suppression is a projection bug — surface it loudly in + // the activity log (U6) rather than the legacy silent ignore. + if (graphSource) { task.log.push({ timestamp: ts, action: - `Ignored out-of-order ${status} for step ${stepIndex} (${task.steps[stepIndex].name}) — ` + - `earlier step ${i} (${task.steps[i].name}) is still ${priorStatus}`, + `[integrity-warning] graph-source updateStep suppressed: step ${stepIndex} ` + + `(${task.steps[stepIndex].name}) → done blocked by unmet dependency ` + + `step ${blockingIndex} (${blockingStatus})`, }); - await this.atomicWriteTaskJson(dir, task); - if (this.isWatching) this.taskCache.set(id, { ...task }); - this.emit("task:updated", task); - return task; } + await this.atomicWriteTaskJson(dir, task); + if (this.isWatching) this.taskCache.set(id, { ...task }); + this.emit("task:updated", task); + return task; } } @@ -8537,13 +8779,19 @@ export class TaskStore extends EventEmitter { if (!existsSync(promptPath)) return []; const content = await readFile(promptPath, "utf-8"); - const steps: import("./types.js").TaskStep[] = []; - const stepRegex = /^###\s+Step\s+\d+[^:]*:\s*(.+)$/gm; - let match; - while ((match = stepRegex.exec(content)) !== null) { - steps.push({ name: match[1].trim(), status: "pending" }); + // Step-inversion U12 (KTD-12): delegate to the registry's `step-headings` + // parser (resolved by id, not a direct import) so the registry path is + // proven and stays byte-identical to the extracted function. The parser + // yields `{ name, dependsOn? }`; re-apply the `pending` status here. + const parser = getStepParser("step-headings"); + if (!parser) { + throw new Error("Step parser 'step-headings' is not registered"); } - return steps; + return parser.parse(content).steps.map((s) => + s.dependsOn + ? { name: s.name, status: "pending" as const, dependsOn: s.dependsOn } + : { name: s.name, status: "pending" as const }, + ); } /** @@ -11455,6 +11703,7 @@ export class TaskStore extends EventEmitter { dependencies: entry.dependencies, steps: entry.steps, currentStep: entry.currentStep, + customFields: entry.customFields ?? undefined, size: entry.size, reviewLevel: entry.reviewLevel, prInfo: entry.prInfo, @@ -12111,6 +12360,60 @@ ${stepsSection}`; pendingRehome = { rehomeTo: updates.rehomeTo, occupantTaskIds }; } } + + // U11/KTD-13: when the IR changes custom field types incompatibly for tasks + // that already hold values, block with a typed IncompatibleFieldChangeError + // unless `coerce` is supplied. Removed/added fields never block (removal + // orphans). Flag-independent: fields are orthogonal to the columns flag. + // Reconciliation runs per occupant task AFTER the IR save commits. + let pendingFieldReconcile: + | { oldFields: WorkflowFieldDefinition[]; newFields: WorkflowFieldDefinition[]; occupantTaskIds: string[]; coerce?: "drop" | "keep-orphaned" } + | undefined; + if (updates.ir !== undefined) { + const existingForFields = await this.getWorkflowDefinition(id); + if (!existingForFields) throw new Error(`Workflow '${id}' not found`); + const nextIrForFields = parseWorkflowIr(updates.ir); + const oldFields: WorkflowFieldDefinition[] = + existingForFields.ir.version === "v2" ? (existingForFields.ir.fields ?? []) : []; + const newFields: WorkflowFieldDefinition[] = + nextIrForFields.version === "v2" ? (nextIrForFields.fields ?? []) : []; + const fieldsChanged = + JSON.stringify(oldFields) !== JSON.stringify(newFields); + if (fieldsChanged) { + const occupantTaskIds = this.listWorkflowOccupantTaskIds(id, false); + const occupantsByField = new Map(); + for (const taskId of occupantTaskIds) { + const row = this.db.prepare("SELECT customFields FROM tasks WHERE id = ?").get(taskId) as + | { customFields: string | null } + | undefined; + const values = row?.customFields + ? (fromJson>(row.customFields) ?? {}) + : {}; + // Incompatible-change detection only blocks on occupants that already + // HOLD a value for a field, so count only those. Reconciliation itself + // must still touch every occupant so new required+default fields get + // backfilled onto tasks that currently have no custom field values. + if (Object.keys(values).length === 0) continue; + for (const key of Object.keys(values)) { + occupantsByField.set(key, (occupantsByField.get(key) ?? 0) + 1); + } + } + const incompatible = computeIncompatibleFieldChanges( + existingForFields.ir, + nextIrForFields, + occupantsByField, + ); + if (incompatible.length > 0 && updates.coerce === undefined) { + throw new IncompatibleFieldChangeError(id, incompatible); + } + pendingFieldReconcile = { + oldFields, + newFields, + occupantTaskIds, + coerce: updates.coerce, + }; + } + } const saved = await this.withConfigLock(async () => { const existing = await this.getWorkflowDefinition(id); if (!existing) throw new Error(`Workflow '${id}' not found`); @@ -12159,6 +12462,23 @@ ${stepsSection}`; }); } } + + // U11/KTD-13: now that the new field schema is committed, reconcile each + // occupant task's stored values against it (orphan-not-delete by default; + // coerce:"drop" discards orphans). Each runs under its own task lock. + if (pendingFieldReconcile) { + const dropOrphans = pendingFieldReconcile.coerce === "drop"; + for (const taskId of pendingFieldReconcile.occupantTaskIds) { + await this.withTaskLock(taskId, () => + this.reconcileTaskCustomFieldsForSchema( + taskId, + pendingFieldReconcile!.oldFields, + pendingFieldReconcile!.newFields, + dropOrphans, + ), + ); + } + } return saved; } @@ -12716,6 +13036,16 @@ ${stepsSection}`; return list; } + /** + * Resolve the custom-field definitions (KTD-13) governing a task, via its + * workflow selection. v1 IR and the default workflow declare none → `[]`. + * Pure DB read, safe inside transactions. + */ + private resolveTaskCustomFieldDefsSync(taskId: string): WorkflowFieldDefinition[] { + const ir = this.resolveTaskWorkflowIrSync(taskId); + return ir.version === "v2" ? (ir.fields ?? []) : []; + } + private resolveTaskWorkflowIrSync(taskId: string): WorkflowIr { const selection = this.getTaskWorkflowSelection(taskId); const workflowId = selection?.workflowId; @@ -12948,6 +13278,12 @@ ${stepsSection}`; // prior selection's rows, so a mid-flight failure never leaves the task // referencing already-deleted step ids. const priorSelection = this.getTaskWorkflowSelection(taskId); + // U11/KTD-13: capture the OLD field schema (from the prior selection's IR) + // before the selection row flips, so we can reconcile existing field values + // against the NEW workflow's schema below. + const oldFieldDefs = this.resolveTaskCustomFieldDefsSync(taskId); + const newFieldDefs: WorkflowFieldDefinition[] = + def.ir.version === "v2" ? (def.ir.fields ?? []) : []; const ids = await this.materializeWorkflowSteps(workflowId, inputs); try { await this.updateTaskUnlocked(taskId, { enabledWorkflowSteps: ids }); @@ -12973,10 +13309,56 @@ ${stepsSection}`; } this.workflowStepsCache = null; } + + // U11/KTD-13: reconcile custom field values against the NEW workflow's + // schema. Same-id, type-compatible values are kept; incompatible/removed + // ids are orphaned — but RETAINED in storage (orphan-not-delete) so a later + // switch back, or the orphaned-fields disclosure, can still surface them. + // Then fill defaults for the new workflow's required+default fields that + // are absent. The merged object is written DIRECTLY (bypassing the + // validating patch path) because orphaned ids are by definition unknown to + // the new schema and would otherwise be rejected. + await this.reconcileTaskCustomFieldsForSchema(taskId, oldFieldDefs, newFieldDefs); + return ids; }); } + /** + * U11/KTD-13: reconcile a task's stored custom field values when its governing + * field schema changes (workflow switch or definition edit). Values are + * partitioned by {@link reconcileFieldsOnWorkflowChange}; orphans are retained + * (never destroyed). Required+default fields absent from the result are filled. + * Writes the merged values directly onto task.json — orphaned ids are unknown + * to the new schema, so this deliberately bypasses the validating patch path. + * Assumes the caller already holds the per-task lock. + */ + private async reconcileTaskCustomFieldsForSchema( + taskId: string, + oldFieldDefs: WorkflowFieldDefinition[], + newFieldDefs: WorkflowFieldDefinition[], + dropOrphans = false, + ): Promise { + const dir = this.taskDir(taskId); + const task = await this.readTaskJson(dir); + const current = task.customFields ?? {}; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldFieldDefs, newFieldDefs, current); + // Default (keep-orphaned): storage keeps everything (kept ∪ orphaned). + // coerce:"drop" discards the orphaned values entirely. + const base = dropOrphans ? { ...kept } : { ...kept, ...orphaned }; + const reconciled = applyFieldDefaults(newFieldDefs, base); + // Skip the write when nothing changed (no defaults added, same keys/values). + const unchanged = + Object.keys(reconciled).length === Object.keys(current).length && + Object.entries(reconciled).every(([k, v]) => current[k] === v); + if (unchanged) return; + task.customFields = reconciled; + task.updatedAt = new Date().toISOString(); + await this.atomicWriteTaskJson(dir, task); + if (this.isWatching) this.taskCache.set(taskId, { ...task }); + this.emitTaskLifecycleEventSafely("task:updated", [task]); + } + /** * U5 (R20) workflow switch: select a workflow for a task and, when the * `workflowColumns` flag is ON, reconcile the card's board column against the diff --git a/packages/core/src/task-fields.ts b/packages/core/src/task-fields.ts new file mode 100644 index 0000000000..93fc8a0a9b --- /dev/null +++ b/packages/core/src/task-fields.ts @@ -0,0 +1,355 @@ +/** + * Custom task field validation & reconciliation authority (U11 / KTD-13). + * + * Workflows declare typed custom task fields ({@link WorkflowFieldDefinition}); + * task values live in `tasks.customFields` (a JSON object keyed by field id). + * This module is the single, side-effect-free validation core that the store + * write authority (`updateTaskCustomFields` / `updateTask`) delegates to. It + * mirrors the `TransitionRejection` style: a flat, JSON-safe typed rejection + * with a machine-stable `code`, the offending `fieldId`, and a non-localized + * `detail` string for audit/logs. + * + * Three operations: + * - {@link validateCustomFieldPatch} — validate a `Record` + * patch against a field schema, normalizing accepted values. `null`/`undefined` + * in the patch is a delete sentinel for that field (always accepted). + * - {@link applyFieldDefaults} — fill `default` for required fields absent from + * the current values (task create / workflow selection). + * - {@link reconcileFieldsOnWorkflowChange} — partition existing values into + * `kept` (same id, type-compatible) and `orphaned` (everything else) when a + * workflow's fields change or the task switches workflows. Orphans are + * RETAINED in storage — this only computes the partition so the UI can render + * the orphaned-fields disclosure. + */ + +import type { + WorkflowFieldDefinition, +} from "./workflow-ir-types.js"; + +// --------------------------------------------------------------------------- +// Typed rejection (TransitionRejection-style: flat, JSON-safe, no class) +// --------------------------------------------------------------------------- + +/** + * Reason codes for a rejected custom-field write. Stable string literals — they + * cross the agent-tool / HTTP boundary and are matched by surfaces for copy, so + * they must not change without migrating consumers. + */ +export type CustomFieldRejectionCode = + | "no-fields-defined" + | "unknown-field" + | "type-mismatch" + | "enum-violation"; + +/** The full, immutable set of custom-field rejection codes. */ +export const CUSTOM_FIELD_REJECTION_CODES: readonly CustomFieldRejectionCode[] = [ + "no-fields-defined", + "unknown-field", + "type-mismatch", + "enum-violation", +] as const; + +/** + * A typed custom-field rejection. Flat and JSON-safe by construction — mirrors + * {@link import("./transition-types.js").TransitionRejection}. + * + * - `code` — machine-stable {@link CustomFieldRejectionCode}. + * - `fieldId` — the offending field id (the patch key that failed). + * - `detail` — non-localized diagnostic context for audit/logs. + */ +export interface CustomFieldRejection { + code: CustomFieldRejectionCode; + fieldId: string; + detail: string; +} + +/** Result of validating a custom-field patch. Discriminated on `ok`. */ +export type CustomFieldPatchResult = + | { ok: true; normalized: Record } + | { ok: false; rejection: CustomFieldRejection }; + +/** Construct a {@link CustomFieldRejection}. */ +export function makeCustomFieldRejection( + code: CustomFieldRejectionCode, + fieldId: string, + detail: string, +): CustomFieldRejection { + return { code, fieldId, detail }; +} + +/** + * Thrown by the throw-based write paths (`updateTask` with a `customFields` + * patch) when validation rejects. `updateTaskCustomFields` returns the typed + * rejection instead; this wrapper exists for the legacy throw contract so a bad + * `updateTask` write fails loudly rather than silently round-tripping an invalid + * value (the U4 opaque behavior). Carries the structured rejection so HTTP/agent + * surfaces can recover the field path and code. + */ +export class CustomFieldRejectionError extends Error { + readonly rejection: CustomFieldRejection; + constructor(rejection: CustomFieldRejection) { + super(`custom field '${rejection.fieldId}' rejected (${rejection.code}): ${rejection.detail}`); + this.name = "CustomFieldRejectionError"; + this.rejection = rejection; + } +} + +// --------------------------------------------------------------------------- +// Per-type value validation +// --------------------------------------------------------------------------- + +/** True iff `value` is a non-empty option-value member of `field.options`. */ +function isEnumMember(field: WorkflowFieldDefinition, value: string): boolean { + return (field.options ?? []).some((o) => o.value === value); +} + +/** + * Validate (and normalize) a single non-null value against a field's type. + * Returns the normalized value on success, or a rejection. The caller has + * already resolved the field definition. + */ +function validateValue( + field: WorkflowFieldDefinition, + value: unknown, +): { ok: true; value: unknown } | { ok: false; rejection: CustomFieldRejection } { + const reject = ( + code: CustomFieldRejectionCode, + detail: string, + ): { ok: false; rejection: CustomFieldRejection } => ({ + ok: false, + rejection: makeCustomFieldRejection(code, field.id, detail), + }); + + switch (field.type) { + case "string": + case "text": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' expects a string, got ${typeof value}`); + } + return { ok: true, value }; + } + case "number": { + if (typeof value !== "number" || !Number.isFinite(value)) { + return reject( + "type-mismatch", + `field '${field.id}' expects a finite number, got ${typeof value === "number" ? String(value) : typeof value}`, + ); + } + return { ok: true, value }; + } + case "boolean": { + if (typeof value !== "boolean") { + return reject("type-mismatch", `field '${field.id}' expects a boolean, got ${typeof value}`); + } + return { ok: true, value }; + } + case "enum": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (enum) expects a string option value, got ${typeof value}`); + } + if (!isEnumMember(field, value)) { + return reject("enum-violation", `field '${field.id}' value '${value}' is not a declared option`); + } + return { ok: true, value }; + } + case "multi-enum": { + if (!Array.isArray(value)) { + return reject("type-mismatch", `field '${field.id}' (multi-enum) expects an array, got ${typeof value}`); + } + const seen = new Set(); + for (const item of value) { + if (typeof item !== "string") { + return reject("type-mismatch", `field '${field.id}' (multi-enum) members must be strings`); + } + if (!isEnumMember(field, item)) { + return reject("enum-violation", `field '${field.id}' member '${item}' is not a declared option`); + } + if (seen.has(item)) { + return reject("enum-violation", `field '${field.id}' has duplicate member '${item}'`); + } + seen.add(item); + } + return { ok: true, value: [...value] as string[] }; + } + case "date": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (date) expects an ISO date string, got ${typeof value}`); + } + const ms = Date.parse(value); + if (Number.isNaN(ms)) { + return reject("type-mismatch", `field '${field.id}' value '${value}' is not a parseable date`); + } + return { ok: true, value }; + } + case "url": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (url) expects a string, got ${typeof value}`); + } + try { + new URL(value); + } catch { + return reject("type-mismatch", `field '${field.id}' value '${value}' is not a valid URL`); + } + return { ok: true, value }; + } + default: { + // Exhaustiveness guard — an unknown type cannot validate. + const _exhaustive: never = field.type; + return reject("type-mismatch", `field '${field.id}' has unsupported type '${String(_exhaustive)}'`); + } + } +} + +// --------------------------------------------------------------------------- +// Patch validation authority +// --------------------------------------------------------------------------- + +/** + * Validate a custom-field `patch` against a workflow's field `fields`. + * + * - A `null`/`undefined` patch value is a DELETE sentinel: the field's stored + * value should be removed. It is always accepted (even for required fields — + * required is not a write-time gate this round, KTD-13) and surfaces in + * `normalized` as `null` so the caller can apply the delete uniformly. + * - A non-null value is validated/normalized per the field's type. + * - A patch key that names no declared field → `unknown-field`. + * - When `fields` is undefined/empty and the patch carries any key → the whole + * patch is rejected `no-fields-defined` (the default workflow declares no + * fields; nothing can be written). An empty patch against no fields is `ok`. + * + * Validation is fail-fast: the first offending key produces the rejection. + */ +export function validateCustomFieldPatch( + fields: WorkflowFieldDefinition[] | undefined, + patch: Record, +): CustomFieldPatchResult { + const keys = Object.keys(patch); + const byId = new Map((fields ?? []).map((f) => [f.id, f])); + + if (byId.size === 0) { + if (keys.length === 0) return { ok: true, normalized: {} }; + return { + ok: false, + rejection: makeCustomFieldRejection( + "no-fields-defined", + keys[0]!, + "the resolved workflow declares no custom fields; no values may be written", + ), + }; + } + + const normalized: Record = {}; + for (const key of keys) { + const value = patch[key]; + const field = byId.get(key); + if (!field) { + return { + ok: false, + rejection: makeCustomFieldRejection( + "unknown-field", + key, + `field '${key}' is not declared by the task's workflow`, + ), + }; + } + // null/undefined = delete this field's value. + if (value === null || value === undefined) { + normalized[key] = null; + continue; + } + const res = validateValue(field, value); + if (!res.ok) return res; + normalized[key] = res.value; + } + return { ok: true, normalized }; +} + +// --------------------------------------------------------------------------- +// Defaults at create / workflow selection +// --------------------------------------------------------------------------- + +/** + * Fill `default` values for REQUIRED fields that are absent from `current`. + * Returns a NEW merged object (does not mutate `current`); existing values win. + * Non-required fields and fields without a declared `default` are left absent. + * + * Used at task create / workflow selection so a workflow with required+default + * fields lands sensible initial values. Defaults are taken on trust from the + * (already-validated-at-save) field schema. + */ +export function applyFieldDefaults( + fields: WorkflowFieldDefinition[] | undefined, + current: Record | undefined, +): Record { + const out: Record = { ...(current ?? {}) }; + for (const field of fields ?? []) { + if (!field.required) continue; + if (field.default === undefined) continue; + if (Object.prototype.hasOwnProperty.call(out, field.id) && out[field.id] !== undefined) { + continue; + } + out[field.id] = field.default; + } + return out; +} + +// --------------------------------------------------------------------------- +// Reconciliation on workflow edit / switch +// --------------------------------------------------------------------------- + +/** + * A stored value for `field` is type-compatible with a new field definition iff + * the new value re-validates cleanly. For enum-kind fields, compatibility also + * requires the value still be a member of the new options (handled by + * re-validation). This is the same gate {@link validateValue} applies on write, + * so "kept" values are guaranteed re-writable under the new schema. + */ +function valueCompatible(newField: WorkflowFieldDefinition, value: unknown): boolean { + if (value === null || value === undefined) return true; + return validateValue(newField, value).ok; +} + +/** Partition of existing values produced by {@link reconcileFieldsOnWorkflowChange}. */ +export interface FieldReconciliation { + /** Values whose id survives in the new schema AND remain type-compatible. */ + kept: Record; + /** + * Values that no longer fit: id removed from the new schema, or the type + * changed incompatibly (including an enum value no longer in the new options). + * RETAINED in storage — listed here only so the UI can render them under the + * orphaned-fields disclosure. + */ + orphaned: Record; +} + +/** + * Reconcile stored `values` when a workflow's field schema changes (edit) or a + * task switches workflows. Same-id values are KEPT when the new field is + * type-compatible (same type, or both enum-kind with the value still a member — + * enforced by re-validation); everything else is ORPHANED. + * + * Storage keeps EVERYTHING — this function only computes the partition. Callers + * persist `{...kept, ...orphaned}` (i.e. the original values, unchanged) and use + * `orphaned` purely for UI disclosure. `oldFields` is accepted for symmetry and + * future heuristics; the decision is driven entirely by `newFields` + the value. + */ +export function reconcileFieldsOnWorkflowChange( + oldFields: WorkflowFieldDefinition[] | undefined, + newFields: WorkflowFieldDefinition[] | undefined, + values: Record | undefined, +): FieldReconciliation { + void oldFields; // reserved for future migration heuristics; intentionally unused + const newById = new Map((newFields ?? []).map((f) => [f.id, f])); + const kept: Record = {}; + const orphaned: Record = {}; + + for (const [id, value] of Object.entries(values ?? {})) { + const newField = newById.get(id); + if (newField && valueCompatible(newField, value)) { + kept[id] = value; + } else { + orphaned[id] = value; + } + } + return { kept, orphaned }; +} diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 41ea3f2de9..a856b620fb 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -681,6 +681,59 @@ export interface WorkflowStepResult { completedAt?: string; } +/** + * Lifecycle status of one persisted step instance (step-inversion U4, KTD-6). + * - `pending` — expanded but not yet started. + * - `in-progress` — actively executing inside its foreach sub-walk. + * - `awaiting-integration` — work complete on a parallel-mode branch, waiting + * for the ordered integration stage (KTD-11; unused at concurrency 1). + * - `completed` — terminal success (integrated in parallel mode). + * - `failed` — terminal failure. + */ +export type WorkflowRunStepInstanceStatus = + | "pending" + | "in-progress" + | "awaiting-integration" + | "completed" + | "failed"; + +/** + * Persisted run-state for one expanded step instance inside a foreach region + * (step-inversion U4, KTD-6). One row per `(taskId, runId, foreachNodeId, + * stepIndex)`; mirrors the `workflow_run_branches` posture. Resume reconstructs + * the instance set from `pinnedStepCount` + per-instance `currentNodeId` / + * `reworkCount`. `baselineSha` / `checkpointId` are the RETHINK reset anchors + * (previously in-memory, lost on restart). `branchName` / `integratedAt` and the + * `awaiting-integration` status serve parallel mode (KTD-11); null/unused at + * concurrency 1. This is the core row shape; the engine-side instance model is + * separate and engine-owned. + */ +export interface WorkflowRunStepInstance { + taskId: string; + runId: string; + /** Node id of the foreach region that expanded this instance. */ + foreachNodeId: string; + /** Zero-based index of the step this instance runs. */ + stepIndex: number; + /** Step count pinned at expansion; resume fails on mismatch with live steps[]. */ + pinnedStepCount: number; + /** Current sub-walk node id for the in-flight instance; null when not started. */ + currentNodeId?: string | null; + status: WorkflowRunStepInstanceStatus; + /** Git sha the RETHINK reset rewinds to; null when no baseline captured. */ + baselineSha?: string | null; + /** Session checkpoint to rewind to on RETHINK; null when none captured. */ + checkpointId?: string | null; + /** Number of rework cycles consumed against the rework budget. */ + reworkCount: number; + /** Per-instance branch name in worktree-isolation mode (KTD-11); null otherwise. */ + branchName?: string | null; + /** ISO-8601 timestamp the instance branch was integrated (KTD-11); null otherwise. */ + integratedAt?: string | null; + /** ISO-8601 timestamp of the last write to this row. */ + updatedAt: string; +} + /** A built-in workflow step template for one-click creation. */ export interface WorkflowStepTemplate { /** Unique template identifier (e.g., "documentation-review") */ @@ -1061,6 +1114,11 @@ export type StepStatus = "pending" | "in-progress" | "done" | "skipped"; export interface TaskStep { name: string; status: StepStatus; + /** Step-inversion (KTD-11): 0-indexed indices of steps this step depends on, + * parsed from the PROMPT.md `### Step N (depends: 1,2): Title` annotation + * (1-indexed step numbers in the doc → 0-indexed indices here). Absent for + * unannotated steps. */ + dependsOn?: number[]; } /** Correlation metadata linking a task mutation to the agent run that caused it. */ @@ -1820,6 +1878,14 @@ export interface Task { worktree?: string; steps: TaskStep[]; currentStep: number; + /** + * Workflow-defined custom task field values (KTD-13), keyed by field id. + * Persisted as the `tasks.customFields` JSON column. Treated as opaque by + * the core row⇄Task mapping and `updateTask`; the validation/write authority + * (type/enum/render checks against the workflow's field schema) lands in a + * later unit. Absent on legacy tasks. + */ + customFields?: Record; status?: string; /** ID of the in-progress task whose file scope overlaps with this task, * causing the scheduler to defer it. Set when the scheduler queues @@ -4032,6 +4098,8 @@ export interface ArchivedTaskEntry { dependencies: string[]; steps: TaskStep[]; currentStep: number; + /** Workflow-defined custom task field values (KTD-13) frozen at archive time. */ + customFields?: Record; size?: "S" | "M" | "L"; reviewLevel?: number; /** Execution mode for task implementation at time of archival. diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 026f544684..60aee809e4 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -48,4 +48,14 @@ export interface WorkflowDefinitionUpdate { * the `workflowColumns` flag is ON. */ rehomeTo?: string; + /** + * U11/KTD-13: when an IR update changes a custom field's type incompatibly for + * tasks that already hold a value under that field, the update is blocked with + * a typed {@link import("./workflow-reconciliation.js").IncompatibleFieldChangeError} + * unless `coerce` is supplied. `"drop"` discards the now-incompatible stored + * values; `"keep-orphaned"` retains them as orphans (rendered under the + * orphaned-fields disclosure). Removing a field outright always orphans (never + * blocks). Mirrors the `rehomeTo` conflict-resolution posture for columns. + */ + coerce?: "drop" | "keep-orphaned"; } diff --git a/packages/core/src/workflow-ir-types.ts b/packages/core/src/workflow-ir-types.ts index d636618f8d..b96e538fb5 100644 --- a/packages/core/src/workflow-ir-types.ts +++ b/packages/core/src/workflow-ir-types.ts @@ -1,5 +1,9 @@ /** Node kinds. v1 kinds (start/prompt/script/gate/end) plus the v2 additions: - * `hold` (passive dwell column states), and `split`/`join` (parallel fan-out). */ + * `hold` (passive dwell column states), `split`/`join` (parallel fan-out), and + * the step-inversion additions (FN step-inversion, KTD-3/4/12/15): + * `foreach` (runtime-expanding per-step template region), `step-review` + * (per-step review verdicts as outcome edges), `parse-steps` (graph-native + * step-list parsing), and `code` (sandboxed TypeScript). */ export type WorkflowIrNodeKind = | "start" | "prompt" @@ -8,7 +12,11 @@ export type WorkflowIrNodeKind = | "end" | "hold" | "split" - | "join"; + | "join" + | "foreach" + | "step-review" + | "parse-steps" + | "code"; export interface WorkflowIrNode { id: string; @@ -22,6 +30,71 @@ export interface WorkflowIrEdge { from: string; to: string; condition?: string; + /** Step-inversion (KTD-5): `rework` edges are the only legal cycles, scoped to + * one foreach template instance and bounded by the foreach `maxReworkCycles`. + * They are exempt from cycle/parallelism complaints. */ + kind?: "rework"; +} + +/** Step-inversion (KTD-3): config for a `foreach` node — a runtime-expanding + * template region instantiated once per planned step. + * Defaults: `mode` sequential; `isolation` shared for sequential / worktree for + * parallel; `concurrency` parallel-only. */ +export interface WorkflowForeachConfig { + source: "task-steps"; + maxReworkCycles?: number; + mode?: "sequential" | "parallel"; + concurrency?: number; + isolation?: "shared" | "worktree"; + template: { + nodes: WorkflowIrNode[]; + edges: WorkflowIrEdge[]; + }; +} + +/** Step-inversion (KTD-12): a workflow-declared task document. Artifacts ride the + * existing task-documents machinery; `step-source` artifacts feed `parse-steps`. */ +export interface WorkflowIrArtifact { + key: string; + title?: string; + producedBy?: "planning" | "manual"; + role?: "step-source" | "context"; +} + +/** Step-inversion (KTD-13): the supported custom-field value types. */ +export type WorkflowFieldType = + | "string" + | "text" + | "number" + | "boolean" + | "enum" + | "multi-enum" + | "date" + | "url"; + +/** A single enum/multi-enum option (KTD-13). */ +export interface WorkflowFieldOption { + value: string; + label: string; + color?: string; +} + +/** Rendering instructions for a custom field (KTD-14). */ +export interface WorkflowFieldRender { + placement?: "card" | "detail" | "detail-section"; + widget?: "select" | "radio" | "chips" | "input" | "textarea" | "toggle"; + badge?: boolean; +} + +/** Step-inversion (KTD-13): a workflow-defined custom task field. */ +export interface WorkflowFieldDefinition { + id: string; + name: string; + type: WorkflowFieldType; + required?: boolean; + default?: unknown; + options?: WorkflowFieldOption[]; + render?: WorkflowFieldRender; } /** A single trait configuration applied to a column. The `trait` is an opaque @@ -61,13 +134,17 @@ export interface WorkflowIrV1 { edges: WorkflowIrEdge[]; } -/** A v2 workflow IR graph: v1 plus workflow-defined columns and node placement. */ +/** A v2 workflow IR graph: v1 plus workflow-defined columns and node placement. + * Step-inversion adds optional `artifacts` (KTD-12) and `fields` (KTD-13) + * declarations — both additive; absent on legacy graphs. */ export interface WorkflowIrV2 { version: "v2"; name: string; columns: WorkflowIrColumn[]; nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[]; + artifacts?: WorkflowIrArtifact[]; + fields?: WorkflowFieldDefinition[]; } /** Either IR version. v1 graphs upgrade to v2 on parse (see parseWorkflowIr). */ diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index 95ed2ac100..6a3cc5eff8 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -7,6 +7,9 @@ import type { WorkflowIrV1, WorkflowIrV2, WorkflowHoldRelease, + WorkflowForeachConfig, + WorkflowFieldDefinition, + WorkflowFieldType, } from "./workflow-ir-types.js"; export class WorkflowIrError extends Error { @@ -25,8 +28,56 @@ const HOLD_RELEASE_KINDS: ReadonlySet = new Set([ ]); /** Seam config values that may not appear inside a parallel branch (KTD-11): - * one worktree/session per task and exclusive merge are physical constraints. */ -const SEAM_FORBIDDEN_IN_BRANCH: ReadonlySet = new Set(["execute", "merge"]); + * one worktree/session per task and exclusive merge are physical constraints. + * Step-inversion (KTD-4) extends this posture: `step-execute` seam prompt nodes + * may never appear in a split branch either. */ +const SEAM_FORBIDDEN_IN_BRANCH: ReadonlySet = new Set([ + "execute", + "merge", + "step-execute", +]); + +/** Step-inversion field-type whitelist (KTD-13). */ +const WORKFLOW_FIELD_TYPES: ReadonlySet = new Set([ + "string", + "text", + "number", + "boolean", + "enum", + "multi-enum", + "date", + "url", +]); + +const FIELD_RENDER_PLACEMENTS: ReadonlySet = new Set([ + "card", + "detail", + "detail-section", +]); + +const FIELD_RENDER_WIDGETS: ReadonlySet = new Set([ + "select", + "radio", + "chips", + "input", + "textarea", + "toggle", +]); + +/** Hard cap on a foreach `maxReworkCycles` (KTD-5: default 3, clamp >10 to 10, + * reject <1). */ +const MAX_REWORK_CYCLES_CAP = 10; + +/** Parallel concurrency bounds (KTD-3): range 1..8. */ +const MAX_FOREACH_CONCURRENCY = 8; + +/** The implicit step-source artifact allowed when no artifacts are declared. */ +const IMPLICIT_DEFAULT_ARTIFACT = "PROMPT.md"; + +/** True when a prompt node carries the `step-execute` seam (KTD-2/KTD-4). */ +function isStepExecuteNode(node: WorkflowIrNode): boolean { + return node.kind === "prompt" && node.config?.seam === "step-execute"; +} /** Default-workflow column ids in legacy enum order (KTD-1). */ export const DEFAULT_WORKFLOW_COLUMN_IDS = [ @@ -180,6 +231,501 @@ function innerJoinNext(joinId: string, outgoing: Map): return (outgoing.get(joinId) ?? []).find((e) => e.condition !== "failure")?.to; } +// --------------------------------------------------------------------------- +// Step-inversion validation (FN step-inversion, U1) +// --------------------------------------------------------------------------- + +/** True for a `rework`-kind edge (KTD-5). */ +function isReworkEdge(edge: WorkflowIrEdge): boolean { + return edge.kind === "rework"; +} + +/** Collect the set of node ids reachable from `start` following non-rework edges + * (rework edges are intra-template back-edges; the top-level reachability / + * dominance analysis ignores them). */ +function reachableFrom( + start: string, + outgoing: Map, +): Set { + const seen = new Set(); + const queue = [start]; + while (queue.length) { + const id = queue.shift()!; + if (seen.has(id)) continue; + seen.add(id); + for (const edge of outgoing.get(id) ?? []) { + if (isReworkEdge(edge)) continue; + if (!seen.has(edge.to)) queue.push(edge.to); + } + } + return seen; +} + +/** + * Validate a foreach `template` subgraph recursively (KTD-3): + * - non-empty; + * - exactly one entry (no incoming template edges) and one exit (no outgoing); + * - NO nested foreach; + * - `step-execute` seam nodes are legal here but never inside a split branch + * (SEAM_FORBIDDEN_IN_BRANCH already enforces this via validateParallelism); + * - rework edges legal only when both endpoints are inside this template; + * - step-review verdict routing rules (KTD-4). + */ +function validateForeach(node: WorkflowIrNode, topLevelNodeIds: Set): void { + const cfg = node.config as Partial | undefined; + if (!cfg || cfg.source !== "task-steps") { + throw new WorkflowIrError( + `foreach node '${node.id}' must declare source 'task-steps'`, + ); + } + const template = cfg.template; + if ( + !template || + !Array.isArray(template.nodes) || + !Array.isArray(template.edges) + ) { + throw new WorkflowIrError( + `foreach node '${node.id}' must declare a template with nodes and edges arrays`, + ); + } + if (template.nodes.length === 0) { + throw new WorkflowIrError(`foreach node '${node.id}' template must be non-empty`); + } + + // mode / isolation / concurrency (KTD-3). + const mode = cfg.mode ?? "sequential"; + if (mode !== "sequential" && mode !== "parallel") { + throw new WorkflowIrError( + `foreach node '${node.id}' mode must be 'sequential' or 'parallel'`, + ); + } + const isolation = cfg.isolation ?? (mode === "parallel" ? "worktree" : "shared"); + if (isolation !== "shared" && isolation !== "worktree") { + throw new WorkflowIrError( + `foreach node '${node.id}' isolation must be 'shared' or 'worktree'`, + ); + } + if (mode === "parallel" && isolation === "shared") { + throw new WorkflowIrError( + `foreach node '${node.id}' cannot combine mode 'parallel' with isolation 'shared' (concurrent writes in one worktree are unguardable races)`, + ); + } + if (cfg.concurrency !== undefined) { + if (mode !== "parallel") { + throw new WorkflowIrError( + `foreach node '${node.id}' concurrency is only valid in 'parallel' mode`, + ); + } + const c = cfg.concurrency; + if (typeof c !== "number" || !Number.isInteger(c) || c < 1 || c > MAX_FOREACH_CONCURRENCY) { + throw new WorkflowIrError( + `foreach node '${node.id}' concurrency must be an integer in 1..${MAX_FOREACH_CONCURRENCY}`, + ); + } + } + if (cfg.maxReworkCycles !== undefined) { + const m = cfg.maxReworkCycles; + if (typeof m !== "number" || !Number.isInteger(m) || m < 1) { + throw new WorkflowIrError( + `foreach node '${node.id}' maxReworkCycles must be an integer >= 1`, + ); + } + // >10 is clamped at parse time (clampForeachConfig); validation only rejects <1. + } + + const templateNodes = template.nodes; + const templateIds = new Set(templateNodes.map((n) => n.id)); + if (templateIds.size !== templateNodes.length) { + throw new WorkflowIrError( + `foreach node '${node.id}' template has duplicate node ids`, + ); + } + + // No nested foreach. + for (const inner of templateNodes) { + if (inner.kind === "foreach") { + throw new WorkflowIrError( + `foreach node '${node.id}' template may not contain a nested foreach ('${inner.id}')`, + ); + } + } + + // Edge endpoints must reference template nodes; rework edges must stay intra-template. + for (const edge of template.edges) { + const fromInside = templateIds.has(edge.from); + const toInside = templateIds.has(edge.to); + if (!fromInside || !toInside) { + if (isReworkEdge(edge)) { + throw new WorkflowIrError( + `rework edge '${edge.from}' -> '${edge.to}' in foreach '${node.id}' must have both endpoints inside the same template`, + ); + } + throw new WorkflowIrError( + `foreach node '${node.id}' template edge '${edge.from}' -> '${edge.to}' references a node outside the template`, + ); + } + } + + // Single entry / single exit (ignoring rework back-edges, which intentionally + // create incoming edges to earlier template nodes). + const incoming = new Map(); + const outgoingCount = new Map(); + for (const edge of template.edges) { + if (isReworkEdge(edge)) continue; + incoming.set(edge.to, (incoming.get(edge.to) ?? 0) + 1); + outgoingCount.set(edge.from, (outgoingCount.get(edge.from) ?? 0) + 1); + } + const entries = templateNodes.filter((n) => (incoming.get(n.id) ?? 0) === 0); + const exits = templateNodes.filter((n) => (outgoingCount.get(n.id) ?? 0) === 0); + if (entries.length !== 1) { + throw new WorkflowIrError( + `foreach node '${node.id}' template must have exactly one entry node (found ${entries.length})`, + ); + } + if (exits.length !== 1) { + throw new WorkflowIrError( + `foreach node '${node.id}' template must have exactly one exit node (found ${exits.length})`, + ); + } + + // Recurse: validate the template as its own region for parallelism + verdict + // routing. step-execute nodes legal here (they are not validated as forbidden + // at top level — that check lives in validateStepExecutePlacement). + const templateById = new Map(templateNodes.map((n) => [n.id, n])); + const templateOutgoing = buildOutgoing(template.edges); + validateParallelism(templateNodes, templateOutgoing, templateById); + validateStepReviewRouting(templateNodes, templateOutgoing, templateById, true); + + // Defensive: top-level node ids and template node ids should not collide + // (instance identity is `#:`, but a raw collision + // is still confusing). + for (const id of templateIds) { + if (topLevelNodeIds.has(id)) { + throw new WorkflowIrError( + `foreach node '${node.id}' template node id '${id}' collides with a top-level node id`, + ); + } + } +} + +/** step-execute seam nodes are legal ONLY inside a foreach template (KTD-4): + * reject any at the top level. (Inside-split-branch rejection is handled by + * SEAM_FORBIDDEN_IN_BRANCH within validateParallelism.) */ +function validateStepExecutePlacement(topLevelNodes: WorkflowIrNode[]): void { + for (const node of topLevelNodes) { + if (isStepExecuteNode(node)) { + throw new WorkflowIrError( + `step-execute seam node '${node.id}' is only legal inside a foreach template`, + ); + } + } +} + +/** + * step-review verdict routing (KTD-4). For each step-review node: + * - it must have outgoing edges covering `outcome:approve` and `outcome:revise`; + * - `outcome:rethink` optional (defaults to the revise target with reset semantics); + * - `outcome:unavailable` optional; + * - a step-review node inside a split branch is advisory-only: it must NOT carry + * rework or `outcome:approve` routing. + */ +function validateStepReviewRouting( + nodes: WorkflowIrNode[], + outgoing: Map, + nodesById: Map, + insideForeachTemplate: boolean, +): void { + // Determine which nodes sit inside a split branch (advisory-only zone). + const inBranch = nodesInSplitBranches(nodes, outgoing, nodesById); + + for (const node of nodes) { + if (node.kind !== "step-review") continue; + if (node.config?.type !== "plan" && node.config?.type !== "code") { + throw new WorkflowIrError( + `step-review node '${node.id}' must declare type 'plan' or 'code'`, + ); + } + if (node.config.model !== undefined && typeof node.config.model !== "string") { + throw new WorkflowIrError( + `step-review node '${node.id}' model must be a string when present`, + ); + } + + const edges = outgoing.get(node.id) ?? []; + const conditions = new Set(edges.map((e) => e.condition)); + const hasRework = edges.some(isReworkEdge); + + if (inBranch.has(node.id)) { + // Advisory-only inside a split branch: no rework, no approve routing. + if (hasRework) { + throw new WorkflowIrError( + `step-review node '${node.id}' inside a split branch is advisory-only and may not have rework edges`, + ); + } + if (conditions.has("outcome:approve")) { + throw new WorkflowIrError( + `step-review node '${node.id}' inside a split branch is advisory-only and may not carry outcome:approve routing`, + ); + } + continue; + } + + // Main-path step-review: must route approve and revise. + if (!conditions.has("outcome:approve")) { + throw new WorkflowIrError( + `step-review node '${node.id}' must route outcome:approve`, + ); + } + if (!conditions.has("outcome:revise")) { + throw new WorkflowIrError( + `step-review node '${node.id}' must route outcome:revise`, + ); + } + void insideForeachTemplate; + } +} + +/** Compute the set of node ids that lie strictly inside some split..join branch + * region. Walks each split's branches forward to the join. Lightweight; used + * for the step-review advisory-only rule. */ +function nodesInSplitBranches( + nodes: WorkflowIrNode[], + outgoing: Map, + nodesById: Map, +): Set { + const inBranch = new Set(); + const splits = nodes.filter((n) => n.kind === "split"); + for (const split of splits) { + for (const edge of outgoing.get(split.id) ?? []) { + let cursor: string | undefined = edge.to; + const visited = new Set(); + while (cursor && !visited.has(cursor)) { + const id: string = cursor; + visited.add(id); + const n = nodesById.get(id); + if (!n || n.kind === "join") break; + inBranch.add(id); + const next: WorkflowIrEdge | undefined = (outgoing.get(id) ?? []).find( + (e) => !isReworkEdge(e) && e.condition !== "failure", + ); + cursor = next?.to; + } + } + } + return inBranch; +} + +/** + * Cycle detection across the top-level graph that EXEMPTS rework edges (KTD-5). + * Any non-rework cycle is rejected; rework edges (intra-template back-edges) are + * skipped. Run over the top-level graph; template internals are validated + * separately. + */ +function validateNoIllegalCycles( + nodes: WorkflowIrNode[], + outgoing: Map, +): void { + const WHITE = 0; + const GRAY = 1; + const BLACK = 2; + const color = new Map(); + for (const n of nodes) color.set(n.id, WHITE); + + const visit = (id: string): void => { + color.set(id, GRAY); + for (const edge of outgoing.get(id) ?? []) { + if (isReworkEdge(edge)) continue; + const c = color.get(edge.to); + if (c === GRAY) { + throw new WorkflowIrError( + `Workflow IR has an illegal cycle (edge '${edge.from}' -> '${edge.to}'); only rework edges may form cycles`, + ); + } + if (c === WHITE) visit(edge.to); + } + color.set(id, BLACK); + }; + + for (const n of nodes) { + if (color.get(n.id) === WHITE) visit(n.id); + } +} + +/** + * Dominance check (KTD-3): every `foreach(source:"task-steps")` must be dominated + * by a `parse-steps` node — a parse-steps node lies on EVERY path from start to + * the foreach. Implemented via the classic "removal disconnects start from + * target" definition, which is correct for DAGs: for each parse-steps node, + * check whether the foreach is still reachable from start with that node removed. + * The foreach is dominated iff some parse-steps node's removal disconnects it. + */ +function validateForeachDominance( + nodes: WorkflowIrNode[], + edges: WorkflowIrEdge[], + outgoing: Map, +): void { + const startNode = nodes.find((n) => n.kind === "start"); + if (!startNode) return; // parse-time guarantees exactly one start. + const foreaches = nodes.filter( + (n) => n.kind === "foreach" && (n.config as { source?: unknown } | undefined)?.source === "task-steps", + ); + if (foreaches.length === 0) return; + const parseStepsNodes = nodes.filter((n) => n.kind === "parse-steps"); + + for (const fe of foreaches) { + // Reachable from start at all? + if (!reachableFrom(startNode.id, outgoing).has(fe.id)) { + throw new WorkflowIrError( + `foreach node '${fe.id}' is not reachable from the start node`, + ); + } + const dominated = parseStepsNodes.some((ps) => { + if (ps.id === fe.id) return false; + // Build outgoing with ps removed (as both source and target). + const trimmed = buildOutgoing( + edges.filter((e) => e.from !== ps.id && e.to !== ps.id), + ); + return !reachableFrom(startNode.id, trimmed).has(fe.id); + }); + if (!dominated) { + throw new WorkflowIrError( + `foreach node '${fe.id}' (source:'task-steps') must be dominated by a parse-steps node on every path from start`, + ); + } + } +} + +/** Validate `parse-steps` node config (KTD-12). */ +function validateParseStepsNodes(ir: WorkflowIrV2): void { + const declaredArtifacts = new Set((ir.artifacts ?? []).map((a) => a.key)); + const hasDeclaredArtifacts = (ir.artifacts ?? []).length > 0; + + for (const node of ir.nodes) { + if (node.kind !== "parse-steps") continue; + const cfg = node.config as { artifact?: unknown; parser?: unknown } | undefined; + const artifact = cfg?.artifact; + const parser = cfg?.parser; + if (typeof parser !== "string" || parser.trim() === "") { + throw new WorkflowIrError( + `parse-steps node '${node.id}' must declare a non-empty parser`, + ); + } + if (typeof artifact !== "string" || artifact.trim() === "") { + throw new WorkflowIrError( + `parse-steps node '${node.id}' must declare a non-empty artifact`, + ); + } + if (hasDeclaredArtifacts) { + if (!declaredArtifacts.has(artifact)) { + throw new WorkflowIrError( + `parse-steps node '${node.id}' references undeclared artifact '${artifact}'`, + ); + } + } else if (artifact !== IMPLICIT_DEFAULT_ARTIFACT) { + throw new WorkflowIrError( + `parse-steps node '${node.id}' references artifact '${artifact}', but only '${IMPLICIT_DEFAULT_ARTIFACT}' is allowed when no artifacts are declared`, + ); + } + } +} + +/** Validate `code` node config (KTD-15). TS is NOT compiled in core (esbuild + * check is engine/editor side). */ +function validateCodeNodes(nodes: WorkflowIrNode[]): void { + const MAX_SOURCE = 65536; + for (const node of nodes) { + if (node.kind !== "code") continue; + const cfg = node.config as { source?: unknown; timeoutMs?: unknown } | undefined; + const source = cfg?.source; + if (typeof source !== "string" || source.length === 0) { + throw new WorkflowIrError(`code node '${node.id}' must declare a non-empty source`); + } + if (source.length > MAX_SOURCE) { + throw new WorkflowIrError( + `code node '${node.id}' source exceeds ${MAX_SOURCE} characters`, + ); + } + if (cfg?.timeoutMs !== undefined) { + const t = cfg.timeoutMs; + if (typeof t !== "number" || !Number.isInteger(t) || t < 1000 || t > 300000) { + throw new WorkflowIrError( + `code node '${node.id}' timeoutMs must be an integer in 1000..300000`, + ); + } + } + } +} + +/** Validate `fields` declarations (KTD-13). */ +function validateFields(fields: WorkflowFieldDefinition[] | undefined): void { + if (fields === undefined) return; + if (!Array.isArray(fields)) { + throw new WorkflowIrError("Workflow IR fields must be an array"); + } + const seen = new Set(); + for (const field of fields) { + if (!field || typeof field.id !== "string" || field.id === "") { + throw new WorkflowIrError("Workflow field must have a non-empty id"); + } + if (seen.has(field.id)) { + throw new WorkflowIrError(`Workflow IR has duplicate field id '${field.id}'`); + } + seen.add(field.id); + if (typeof field.name !== "string" || field.name === "") { + throw new WorkflowIrError(`Workflow field '${field.id}' must have a non-empty name`); + } + if (!WORKFLOW_FIELD_TYPES.has(field.type)) { + throw new WorkflowIrError( + `Workflow field '${field.id}' has unknown type '${String(field.type)}'`, + ); + } + const isEnum = field.type === "enum" || field.type === "multi-enum"; + if (isEnum) { + if (!Array.isArray(field.options) || field.options.length === 0) { + throw new WorkflowIrError( + `Workflow field '${field.id}' of type '${field.type}' must declare non-empty options`, + ); + } + const optSeen = new Set(); + for (const opt of field.options) { + if (!opt || typeof opt.value !== "string" || opt.value === "") { + throw new WorkflowIrError( + `Workflow field '${field.id}' option must have a non-empty value`, + ); + } + if (typeof opt.label !== "string" || opt.label === "") { + throw new WorkflowIrError( + `Workflow field '${field.id}' option '${opt.value}' must have a non-empty label`, + ); + } + if (optSeen.has(opt.value)) { + throw new WorkflowIrError( + `Workflow field '${field.id}' has duplicate option value '${opt.value}'`, + ); + } + optSeen.add(opt.value); + } + } else if (field.options !== undefined) { + throw new WorkflowIrError( + `Workflow field '${field.id}' of type '${field.type}' must not declare options`, + ); + } + if (field.render !== undefined) { + const r = field.render; + if (r.placement !== undefined && !FIELD_RENDER_PLACEMENTS.has(r.placement)) { + throw new WorkflowIrError( + `Workflow field '${field.id}' render.placement '${String(r.placement)}' is not allowed`, + ); + } + if (r.widget !== undefined && !FIELD_RENDER_WIDGETS.has(r.widget)) { + throw new WorkflowIrError( + `Workflow field '${field.id}' render.widget '${String(r.widget)}' is not allowed`, + ); + } + } + } +} + function validateColumns(ir: WorkflowIrV2): void { if (!Array.isArray(ir.columns)) { throw new WorkflowIrError("Workflow IR v2 columns must be an array"); @@ -223,6 +769,48 @@ function validateV2(ir: WorkflowIrV2): void { const outgoing = buildOutgoing(ir.edges); validateParallelism(ir.nodes, outgoing, nodesById); + + // Step-inversion (U1) — additive validation. Order matters: validate node + // configs first, then structural rules. + const topLevelIds = new Set(ir.nodes.map((n) => n.id)); + validateStepExecutePlacement(ir.nodes); + for (const node of ir.nodes) { + if (node.kind === "foreach") validateForeach(node, topLevelIds); + } + validateStepReviewRouting(ir.nodes, outgoing, nodesById, false); + validateParseStepsNodes(ir); + validateCodeNodes(ir.nodes); + validateFields(ir.fields); + + // Rework edges are legal only intra-template; any rework edge at the top level + // is rejected (template rework edges are validated inside validateForeach and + // never appear in ir.edges). + for (const edge of ir.edges) { + if (isReworkEdge(edge)) { + throw new WorkflowIrError( + `rework edge '${edge.from}' -> '${edge.to}' is only legal inside a foreach template`, + ); + } + } + + validateNoIllegalCycles(ir.nodes, outgoing); + validateForeachDominance(ir.nodes, ir.edges, outgoing); +} + +/** Clamp foreach `maxReworkCycles` > cap down to the cap, in place, mirroring the + * maxRetries clamp posture (KTD-5). Reject-of-<1 happens in validation. */ +function clampForeachConfigs(ir: WorkflowIrV2): void { + for (const node of ir.nodes) { + if (node.kind !== "foreach") continue; + const cfg = node.config as Partial | undefined; + if ( + cfg && + typeof cfg.maxReworkCycles === "number" && + cfg.maxReworkCycles > MAX_REWORK_CYCLES_CAP + ) { + cfg.maxReworkCycles = MAX_REWORK_CYCLES_CAP; + } + } } export function parseWorkflowIr(input: string | WorkflowIr): WorkflowIr { @@ -249,6 +837,7 @@ export function parseWorkflowIr(input: string | WorkflowIr): WorkflowIr { return upgradeV1ToV2(ir); } + clampForeachConfigs(ir); validateV2(ir); return ir; } @@ -280,6 +869,11 @@ export function downgradeIrToV1IfPure(ir: WorkflowIr): WorkflowIr { if (!V1_NODE_KINDS.has(node.kind)) return ir; } + // Step-inversion declarations (artifacts/fields) are v2-only features. + if ((ir.artifacts && ir.artifacts.length > 0) || (ir.fields && ir.fields.length > 0)) { + return ir; + } + // Columns must be exactly the synthesized default set, same ids, same order, // with the minimal (placement-only) empty trait set. Any custom column, rename, // reorder, or applied trait forces v2. diff --git a/packages/core/src/workflow-reconciliation.ts b/packages/core/src/workflow-reconciliation.ts index 382d3645fa..e417c79a9e 100644 --- a/packages/core/src/workflow-reconciliation.ts +++ b/packages/core/src/workflow-reconciliation.ts @@ -32,7 +32,12 @@ * is independently testable and reused identically across switch/edit/delete. */ -import type { WorkflowIr, WorkflowIrV2, WorkflowIrColumn } from "./workflow-ir-types.js"; +import type { + WorkflowIr, + WorkflowIrV2, + WorkflowIrColumn, + WorkflowFieldDefinition, +} from "./workflow-ir-types.js"; import { resolveColumnFlags } from "./trait-registry.js"; import { workflowHasColumn } from "./workflow-transitions.js"; @@ -181,6 +186,90 @@ export function assertRehomeTargetValid(nextIr: WorkflowIr, rehomeTo: string): v } } +// ── Custom-field schema-evolution reconciliation (U11/KTD-13) ──────────────── + +/** A field whose type changed incompatibly while tasks hold values under it. */ +export interface IncompatibleFieldChange { + fieldId: string; + fromType: string; + toType: string; + /** Number of tasks (under this workflow) currently holding a value for it. */ + occupantCount: number; +} + +/** + * Thrown by the workflow update path when an IR edit changes one or more custom + * fields' types incompatibly for tasks that already hold a value, and no + * `coerce` option was supplied. Mirrors {@link OccupiedColumnsError}: a typed, + * conflict-signaling error the surface maps to a 409 prompting for a coercion + * choice (`drop` | `keep-orphaned`). + */ +export class IncompatibleFieldChangeError extends Error { + readonly workflowId: string; + readonly changes: IncompatibleFieldChange[]; + constructor(workflowId: string, changes: IncompatibleFieldChange[]) { + const summary = changes + .map((c) => `${c.fieldId} (${c.fromType}→${c.toType}, ${c.occupantCount})`) + .join(", "); + super( + `Workflow '${workflowId}' edit changes field type(s) incompatibly: ${summary}. ` + + `Supply coerce ("drop" | "keep-orphaned") to proceed.`, + ); + this.name = "IncompatibleFieldChangeError"; + this.workflowId = workflowId; + this.changes = changes; + } +} + +/** The v2 fields of an IR, or `[]` when absent (v1 or undeclared). */ +function fieldsOf(ir: WorkflowIr): WorkflowFieldDefinition[] { + const v2 = ir as WorkflowIrV2; + return Array.isArray(v2.fields) ? v2.fields : []; +} + +/** Enum-kind sibling check (enum / multi-enum). */ +function sameEnumKind(a: string, b: string): boolean { + const enumKind = (t: string) => t === "enum" || t === "multi-enum"; + return enumKind(a) && enumKind(b); +} + +/** + * Compute which custom fields change type INCOMPATIBLY between `existingIr` and + * `nextIr` AND still have occupant tasks holding a value. A type is compatible + * with itself; enum↔multi-enum is treated as compatible-shape (values are + * re-validated against the new options at reconcile time — a value dropped by + * the new options orphans individually, not via a hard block). A field removed + * outright is NOT a conflict (removal always orphans, never blocks). Returns one + * entry per blocking change in the existing IR's field order. + * + * `occupantsByField` maps a field id to the count of tasks (under this workflow) + * currently holding a value for it. + */ +export function computeIncompatibleFieldChanges( + existingIr: WorkflowIr, + nextIr: WorkflowIr, + occupantsByField: Map, +): IncompatibleFieldChange[] { + const nextById = new Map(fieldsOf(nextIr).map((f) => [f.id, f])); + const changes: IncompatibleFieldChange[] = []; + for (const oldField of fieldsOf(existingIr)) { + const next = nextById.get(oldField.id); + if (!next) continue; // removed → orphan, not a block + if (next.type === oldField.type) continue; // identical type → fine + if (sameEnumKind(oldField.type, next.type)) continue; // enum↔multi-enum → soft + const occupantCount = occupantsByField.get(oldField.id) ?? 0; + if (occupantCount > 0) { + changes.push({ + fieldId: oldField.id, + fromType: oldField.type, + toType: next.type, + occupantCount, + }); + } + } + return changes; +} + // ── Abort-on-switch DI seam (core stays engine-free) ───────────────────────── // // A workflow switch must abort the card's in-flight processing BEFORE the move diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index 719bfaa214..8c4b40e8a6 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -79,6 +79,10 @@ import type { TaskIdIntegrityReport, BranchGroup, BranchGroupPrState, + WorkflowFieldDefinition, + WorkflowFieldType, + WorkflowFieldOption, + WorkflowFieldRender, } from "@fusion/core"; import type { PlanningQuestion, PlanningSummary } from "@fusion/core"; import type { GithubIssueAction, ScheduledTask, ScheduledTaskCreateInput, ScheduledTaskUpdateInput, AutomationRunResult, Routine, RoutineCreateInput, RoutineUpdateInput, RoutineExecutionResult } from "@fusion/core"; @@ -552,10 +556,17 @@ export interface BoardWorkflowColumn { flags: BoardWorkflowColumnFlags; } +// WorkflowFieldDefinition, WorkflowFieldType, WorkflowFieldOption, WorkflowFieldRender +// are re-exported from @fusion/core above (KTD-13/14). +export type { WorkflowFieldDefinition, WorkflowFieldType, WorkflowFieldOption, WorkflowFieldRender }; + export interface BoardWorkflowDefinition { id: string; name: string; columns: BoardWorkflowColumn[]; + /** Custom field definitions declared by this workflow (U13/KTD-14). Absent on + * workflows with no fields, or from older servers. */ + fields?: WorkflowFieldDefinition[]; } export interface BoardWorkflowsPayload { @@ -565,6 +576,31 @@ export interface BoardWorkflowsPayload { taskWorkflowIds: Record; } +/** A typed custom-field rejection surfaced by the PATCH endpoint (KTD-13). */ +export interface CustomFieldRejection { + code: "no-fields-defined" | "unknown-field" | "type-mismatch" | "enum-violation"; + fieldId: string; + detail: string; +} + +/** + * Patch a task's custom field values (U13/KTD-14). The server validates the + * patch against the task's workflow field schema and returns the updated task; + * a validation failure surfaces as a 400 carrying `{ fieldId, code, detail }`. + * A `null` value for a field deletes it. + */ +export function updateTaskCustomFields( + id: string, + customFields: Record, + projectId?: string, +): Promise { + return api(withProjectId(`/tasks/${id}/custom-fields`, projectId), { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ customFields }), + }); +} + /** Fetch the multi-lane board metadata (U9). When the flag is OFF the server * returns `{ flagEnabled: false }` and the board renders its legacy form. */ export function fetchBoardWorkflows(projectId?: string): Promise { @@ -5022,6 +5058,16 @@ export function fetchTraits(projectId?: string): Promise { ); } +/** Fetch the step-parser id catalog (built-ins + registered plugin parsers) for + * the parse-steps node inspector (KTD-12). Registry-backed, read-only, + * session-scoped. Mirrors fetchTraits. */ +export function fetchStepParsers(projectId?: string): Promise { + const path = withProjectId("/step-parsers", projectId); + return dedupe(path, () => + api<{ parsers: Array<{ id: string }> }>(path).then((res) => res.parsers.map((p) => p.id)), + ); +} + /** Fetch a single workflow definition. */ export function fetchWorkflow(id: string, projectId?: string): Promise { return api(withProjectId(`/workflows/${encodeURIComponent(id)}`, projectId)); diff --git a/packages/dashboard/app/components/Board.tsx b/packages/dashboard/app/components/Board.tsx index 59e68579bd..9b7ed08a90 100644 --- a/packages/dashboard/app/components/Board.tsx +++ b/packages/dashboard/app/components/Board.tsx @@ -379,6 +379,33 @@ export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onPauseTask return result; }, [boardWorkflows, flagOn, tasks]); + // Card-placed field defs grouped by workflow id (U13/KTD-14). Only recomputes + // when the board-workflows payload changes, not on every SSE task tick. + const cardDefsByWorkflow = useMemo(() => { + const map = new Map(); + if (!boardWorkflows) return map; + for (const wf of boardWorkflows.workflows) { + const cardDefs = (wf.fields ?? []).filter((f) => f.render?.placement === "card"); + if (cardDefs.length > 0) map.set(wf.id, cardDefs); + } + return map; + }, [boardWorkflows]); + + // Per-task card field defs (U13/KTD-14). Recomputes on task list changes but + // reuses the stable cardDefsByWorkflow map so the inner loop is cheap. + const taskCardFieldDefs = useMemo(() => { + const map = new Map(); + if (cardDefsByWorkflow.size === 0) return map; + if (!boardWorkflows) return map; + const { taskWorkflowIds, defaultWorkflowId } = boardWorkflows; + for (const task of tasks) { + const workflowId = taskWorkflowIds[task.id] ?? defaultWorkflowId; + const defs = cardDefsByWorkflow.get(workflowId); + if (defs) map.set(task.id, defs); + } + return map; + }, [cardDefsByWorkflow, tasks, boardWorkflows]); + // Drag pre-check (R17): adjacency + capacity from the lane's column metadata. // Cross-lane drag → workflow-mismatch. Deterministic rejections return a // messageKey (no-move); null = allowed. @@ -467,6 +494,7 @@ export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onPauseTask onOpenMission={onOpenMission} lastFetchTimeMs={lastFetchTimeMs} workflowStepNameLookup={workflowStepNameLookup} + taskCardFieldDefs={taskCardFieldDefs} blockerFanoutMap={blockerFanoutMap} prAuthAvailable={prAuthAvailable} /> @@ -508,6 +536,7 @@ export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onPauseTask onOpenMission={onOpenMission} lastFetchTimeMs={lastFetchTimeMs} workflowStepNameLookup={workflowStepNameLookup} + taskCardFieldDefs={taskCardFieldDefs} blockerFanoutMap={blockerFanoutMap} prAuthAvailable={prAuthAvailable} autoMerge={autoMerge} diff --git a/packages/dashboard/app/components/Column.tsx b/packages/dashboard/app/components/Column.tsx index b04835e8e4..309f251064 100644 --- a/packages/dashboard/app/components/Column.tsx +++ b/packages/dashboard/app/components/Column.tsx @@ -140,6 +140,8 @@ interface ColumnProps { lastFetchTimeMs?: number; /** Lookup of workflow step IDs to display names, fetched once at board level. */ workflowStepNameLookup?: ReadonlyMap; + /** Per-task card-placed custom field definitions (U13/KTD-14). */ + taskCardFieldDefs?: ReadonlyMap; /** Precomputed blocker fanout keyed by blocker task ID. */ blockerFanoutMap?: ReadonlyMap; /** Whether GitHub CLI auth is available for creating PRs from task cards. */ @@ -168,7 +170,7 @@ interface ColumnProps { getDraggingTaskId?: () => string | null; } -function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onPauseTask, onOpenDetail, onOpenGroupModal, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onRetryTask, onArchiveTask, onUnarchiveTask, onDeleteTask, onArchiveAllDone, collapsed, onToggleCollapse, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs, onOpenMission, lastFetchTimeMs, workflowStepNameLookup, blockerFanoutMap, prAuthAvailable, workflowMode, columnDisplayName, columnFlags, onPromote, canDropTask, getDraggingTaskId }: ColumnProps) { +function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onPauseTask, onOpenDetail, onOpenGroupModal, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onRetryTask, onArchiveTask, onUnarchiveTask, onDeleteTask, onArchiveAllDone, collapsed, onToggleCollapse, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs, onOpenMission, lastFetchTimeMs, workflowStepNameLookup, taskCardFieldDefs, blockerFanoutMap, prAuthAvailable, workflowMode, columnDisplayName, columnFlags, onPromote, canDropTask, getDraggingTaskId }: ColumnProps) { const { t } = useTranslation("app"); // Anchor the board.rejection.* catalog keys for the i18next extractor (it // scopes `t` to the useTranslation binding, so the shared translateRejection @@ -695,6 +697,7 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onOpenMission={onOpenMission} lastFetchTimeMs={lastFetchTimeMs} workflowStepNameLookup={workflowStepNameLookup} + taskCardFieldDefs={taskCardFieldDefs} blockerFanoutMap={blockerFanoutMap} prAuthAvailable={prAuthAvailable} autoMergeEnabled={Boolean(autoMerge)} @@ -725,6 +728,7 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onMoveTask={onMoveTask} lastFetchTimeMs={lastFetchTimeMs} workflowStepNameLookup={workflowStepNameLookup} + cardFieldDefs={taskCardFieldDefs?.get(task.id)} fanout={blockerFanoutMap?.get(task.id)} prAuthAvailable={prAuthAvailable} autoMergeEnabled={Boolean(autoMerge)} diff --git a/packages/dashboard/app/components/Lane.tsx b/packages/dashboard/app/components/Lane.tsx index da791276a0..c772b95582 100644 --- a/packages/dashboard/app/components/Lane.tsx +++ b/packages/dashboard/app/components/Lane.tsx @@ -68,6 +68,8 @@ export interface LaneProps { onOpenMission?: (missionId: string) => void; lastFetchTimeMs?: number; workflowStepNameLookup?: ReadonlyMap; + /** Per-task card-placed custom field definitions (U13/KTD-14). */ + taskCardFieldDefs?: ReadonlyMap; blockerFanoutMap?: ReadonlyMap; prAuthAvailable?: boolean; } @@ -191,6 +193,7 @@ function LaneComponent(props: LaneProps) { onOpenMission={props.onOpenMission} lastFetchTimeMs={props.lastFetchTimeMs} workflowStepNameLookup={props.workflowStepNameLookup} + taskCardFieldDefs={props.taskCardFieldDefs} blockerFanoutMap={props.blockerFanoutMap} prAuthAvailable={props.prAuthAvailable} autoMerge={props.autoMerge} diff --git a/packages/dashboard/app/components/TaskCard.css b/packages/dashboard/app/components/TaskCard.css index 42edbbfb4d..7be8c7f0cc 100644 --- a/packages/dashboard/app/components/TaskCard.css +++ b/packages/dashboard/app/components/TaskCard.css @@ -1447,3 +1447,53 @@ flex-wrap: wrap; } } + +/* Card-placed custom field badges (U13 / KTD-14). */ +.card-field-badges { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 4px; + margin: 4px 0 2px; +} + +.card-field-badge { + display: inline-flex; + align-items: center; + gap: 3px; + padding: 1px 7px; + border: 1px solid var(--border-color, #2a2d34); + border-radius: 999px; + background: var(--chip-bg, #1c1f26); + color: var(--text-muted, #b4b8c0); + font-size: 11px; + line-height: 1.5; + max-width: 16ch; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.card-field-badge--boolean { + background: var(--accent, #4f7cff); + border-color: var(--accent, #4f7cff); + color: #fff; +} + +.card-field-badge--multi { + gap: 3px; + max-width: none; +} + +.card-field-badge-token { + display: inline-flex; + align-items: center; + padding: 0 5px; + border-radius: 999px; + border: 1px solid var(--border-color, #2a2d34); + background: var(--chip-bg, #1c1f26); +} + +.card-field-badge--overflow { + font-weight: 600; +} diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index 896d611b7c..56037e61ce 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -1,7 +1,7 @@ import "./TaskCard.css"; import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; -import { memo, useCallback, useState, useRef, useEffect, useMemo } from "react"; +import { memo, useCallback, useState, useRef, useEffect, useMemo, type ReactElement } from "react"; import { Link, Clock, Layers, Pencil, ChevronDown, Folder, Target, Bot, Trash2, RotateCw, Zap, GitBranch, GitPullRequest } from "lucide-react"; import type { Task, TaskDetail, Column, ColumnId, PrInfo, IssueInfo, TaskPriority, GithubIssueAction } from "@fusion/core"; import { @@ -11,7 +11,7 @@ import { VALID_TRANSITIONS, getErrorMessage, } from "@fusion/core"; -import { fetchTaskDetail, uploadAttachment, fetchMission, fetchAgent } from "../api"; +import { fetchTaskDetail, uploadAttachment, fetchMission, fetchAgent, type WorkflowFieldDefinition } from "../api"; import { GitHubBadge } from "./GitHubBadge"; import { PrCreateModal } from "./PrCreateModal"; import { ProviderIcon } from "./ProviderIcon"; @@ -299,6 +299,72 @@ export function formatElapsedDurationDone(elapsedMs: number): string { } +/** Max number of card-placed custom fields rendered before an overflow chip + * (KTD-14: "max 3 card fields rendered with a +N overflow indicator"). */ +const MAX_CARD_FIELDS = 3; + +/** Render a single card-placed custom field value as a badge/chip (U13/KTD-14). + * Returns null for empty/unset values so absent fields take no card space. */ +function renderCardFieldBadge( + field: WorkflowFieldDefinition, + value: unknown, +): ReactElement | null { + const colorOf = (v: string): string | undefined => field.options?.find((o) => o.value === v)?.color; + const labelOf = (v: string): string => field.options?.find((o) => o.value === v)?.label ?? v; + + if (field.type === "boolean") { + // Boolean true → labeled chip; false/unset → nothing. + if (value !== true) return null; + return ( + + {field.name} + + ); + } + if (field.type === "enum") { + if (typeof value !== "string" || value === "") return null; + const color = colorOf(value); + return ( + + {labelOf(value)} + + ); + } + if (field.type === "multi-enum") { + const arr = Array.isArray(value) ? (value as string[]) : []; + if (arr.length === 0) return null; + return ( + + {arr.map((v) => { + const color = colorOf(v); + return ( + + {labelOf(v)} + + ); + })} + + ); + } + // string / text / number / date / url → simple labeled chip. + if (value === undefined || value === null || value === "") return null; + const display = field.type === "date" && typeof value === "string" ? value.slice(0, 10) : String(value); + return ( + + {display} + + ); +} + interface TaskCardProps { task: Task; projectId?: string; @@ -338,6 +404,9 @@ interface TaskCardProps { prAuthAvailable?: boolean; /** Whether project-level auto-merge is enabled (hides manual Create PR quick action when true). */ autoMergeEnabled?: boolean; + /** Card-placed custom field definitions for this task's workflow (U13/KTD-14). + * Empty/undefined → no field badges render (card byte-identical to today). */ + cardFieldDefs?: WorkflowFieldDefinition[]; } function getTaskPrimaryPrInfo(task: Pick): PrInfo | undefined { @@ -471,6 +540,10 @@ function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): bo previous.taskStuckTimeoutMs === next.taskStuckTimeoutMs && previous.prAuthAvailable === next.prAuthAvailable && previous.autoMergeEnabled === next.autoMergeEnabled && + previous.cardFieldDefs === next.cardFieldDefs && + (previous.cardFieldDefs == null && next.cardFieldDefs == null + ? true + : JSON.stringify(previousTask.customFields ?? null) === JSON.stringify(nextTask.customFields ?? null)) && previous.onOpenDetail === next.onOpenDetail && previous.onOpenGroupModal === next.onOpenGroupModal && previous.addToast === next.addToast && @@ -584,6 +657,7 @@ function TaskCardComponent({ fanout, prAuthAvailable, autoMergeEnabled = false, + cardFieldDefs, }: TaskCardProps) { const { t } = useTranslation("app"); const columnLabel = useColumnLabel(); @@ -1947,6 +2021,30 @@ function TaskCardComponent({
{truncate(task.title, MAX_TITLE_LENGTH) || truncate(task.description, MAX_TITLE_LENGTH) || task.id}
+ {(() => { + // Card-placed custom field badges (U13/KTD-14). Bounded to MAX_CARD_FIELDS + // with a "+N" overflow chip. Nothing renders when no card fields are + // defined or all values are empty — card stays byte-identical to today. + const cardDefs = (cardFieldDefs ?? []).filter((f) => f.render?.placement === "card"); + if (cardDefs.length === 0) return null; + const values = task.customFields ?? {}; + const badges = cardDefs + .map((f) => renderCardFieldBadge(f, values[f.id])) + .filter((b): b is ReactElement => b !== null); + if (badges.length === 0) return null; + const shown = badges.slice(0, MAX_CARD_FIELDS); + const overflow = badges.length - shown.length; + return ( +
+ {shown} + {overflow > 0 ? ( + + +{overflow} + + ) : null} +
+ ); + })()} {hasBranchMetadata && (
{branchMetadata.branch && ( diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index c6549955b9..0d32ed7567 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -21,8 +21,10 @@ import { resolveTaskPlanningModel, resolveTaskValidatorModel, } from "@fusion/core"; -import { uploadAttachment, deleteAttachment, updateTask, pauseTask, unpauseTask, fetchTaskDetail, fetchSettings, fetchGlobalSettings, requestSpecRevision, rebuildTaskSpec, approvePlan, rejectPlan, refineTask, fetchWorkflowResults, assignTask, fetchAgents, fetchAgent, recoverBranchBinding, refreshPrStatus } from "../api"; -import type { RecoverBranchBindingOutcome } from "../api"; +import { uploadAttachment, deleteAttachment, updateTask, pauseTask, unpauseTask, fetchTaskDetail, fetchSettings, fetchGlobalSettings, requestSpecRevision, rebuildTaskSpec, approvePlan, rejectPlan, refineTask, fetchWorkflowResults, assignTask, fetchAgents, fetchAgent, recoverBranchBinding, refreshPrStatus, fetchBoardWorkflows, updateTaskCustomFields } from "../api"; +import type { RecoverBranchBindingOutcome, WorkflowFieldDefinition, CustomFieldRejection } from "../api"; +import { ApiRequestError } from "../api"; +import { TaskFieldsSection } from "./TaskFieldsSection"; import type { ToastType } from "../hooks/useToast"; import { useAgentLogs } from "../hooks/useAgentLogs"; import { useConfirm } from "../hooks/useConfirm"; @@ -306,6 +308,11 @@ export interface TaskDetailModalProps { initialTab?: TabId; /** Mobile-only header affordance mode. */ mobileHeaderMode?: "close" | "back"; + /** Pre-resolved workflow field defs for this task's workflow (U13/KTD-14). + * When provided (e.g. threaded from a Board that already holds the payload) + * the modal skips its own board-workflows fetch entirely. Falls back to the + * self-fetch when absent (e.g. modal opened from non-board contexts). */ + workflowFieldDefs?: WorkflowFieldDefinition[] | null; } export type TaskDetailContentProps = Omit & { @@ -481,6 +488,7 @@ export function TaskDetailContent({ mobileHeaderMode = "close", embedded = false, onRequestClose, + workflowFieldDefs: workflowFieldDefsProp, }: TaskDetailContentProps) { const { t } = useTranslation("app"); const columnLabel = useColumnLabel(); @@ -605,6 +613,69 @@ export function TaskDetailContent({ const [showRefineModal, setShowRefineModal] = useState(false); const [prCreateOpen, setPrCreateOpen] = useState(false); + // Custom field definitions (U13/KTD-14). Resolved for this task's workflow + // from the board-workflows payload; absent when the workflow declares none, + // in which case the fields section renders nothing (today's UI byte-identical). + // When `workflowFieldDefsProp` is provided by the caller (e.g. the Board + // already holds the payload) we skip the self-fetch entirely. + const [customFieldDefs, setCustomFieldDefs] = useState( + workflowFieldDefsProp !== undefined ? (workflowFieldDefsProp ?? null) : null, + ); + const [customFieldValues, setCustomFieldValues] = useState>(task.customFields ?? {}); + const [customFieldError, setCustomFieldError] = useState(null); + + // Keep local field values in sync when the task prop changes (SSE refresh). + useEffect(() => { + setCustomFieldValues(task.customFields ?? {}); + }, [task.id, task.customFields]); + + // Resolve this task's workflow field definitions once per task. Skipped when + // the caller supplies `workflowFieldDefs` directly (Board context). Best-effort: + // a failed fetch (or flag-OFF empty payload) leaves defs null → no section. + useEffect(() => { + if (workflowFieldDefsProp !== undefined) { + // Prop-driven path: keep in sync if the prop changes (task switch etc.). + setCustomFieldDefs(workflowFieldDefsProp ?? null); + return; + } + let cancelled = false; + void fetchBoardWorkflows(projectId) + .then((payload) => { + if (cancelled) return; + const workflowId = payload.taskWorkflowIds[task.id] ?? payload.defaultWorkflowId; + const workflow = payload.workflows.find((w) => w.id === workflowId); + setCustomFieldDefs(workflow?.fields ?? null); + }) + .catch(() => { + if (!cancelled) setCustomFieldDefs(null); + }); + return () => { + cancelled = true; + }; + }, [task.id, projectId, workflowFieldDefsProp]); + + const handleSaveCustomFields = useCallback( + async (patch: Record) => { + setCustomFieldError(null); + try { + const updated = await updateTaskCustomFields(task.id, patch, projectId); + setCustomFieldValues(updated.customFields ?? {}); + onTaskUpdated?.(updated); + } catch (err) { + if (err instanceof ApiRequestError && err.details && typeof err.details.fieldId === "string") { + setCustomFieldError({ + code: (err.details.code as CustomFieldRejection["code"]) ?? "type-mismatch", + fieldId: err.details.fieldId, + detail: typeof err.details.detail === "string" ? err.details.detail : err.message, + }); + return; + } + addToast(getErrorMessage(err) || t("taskFields.saveFailed", "Failed to save field"), "error"); + } + }, + [task.id, projectId, onTaskUpdated, addToast, t], + ); + useEffect(() => { if (activeTab !== "logs" || logSubview !== "activity") { setHighlightStallCode(null); @@ -2485,6 +2556,15 @@ export function TaskDetailContent({ ); })()} + {customFieldDefs && customFieldDefs.length > 0 ? ( + + ) : null} {showNearDuplicateWarning && (
diff --git a/packages/dashboard/app/components/TaskFieldsSection.css b/packages/dashboard/app/components/TaskFieldsSection.css new file mode 100644 index 0000000000..6e0d69f31e --- /dev/null +++ b/packages/dashboard/app/components/TaskFieldsSection.css @@ -0,0 +1,214 @@ +/* Schema-driven custom-field form section (U13 / KTD-14). */ + +.task-fields-section { + display: flex; + flex-direction: column; + gap: 12px; + margin: 12px 0; +} + +.task-field-row { + display: flex; + flex-direction: column; + gap: 4px; +} + +.task-field-label { + font-size: 12px; + font-weight: 600; + color: var(--text-muted, #8a8f98); + text-transform: uppercase; + letter-spacing: 0.02em; +} + +.task-field-required { + color: var(--accent-danger, #e5484d); +} + +.task-field-control { + display: flex; + align-items: center; + flex-wrap: wrap; + gap: 6px; +} + +.task-field-input, +.task-field-textarea, +.task-field-select { + width: 100%; + box-sizing: border-box; + padding: 6px 8px; + border: 1px solid var(--border-color, #2a2d34); + border-radius: 6px; + background: var(--input-bg, #16181d); + color: var(--text-primary, #e6e6e6); + font-size: 13px; + font-family: inherit; +} + +.task-field-textarea { + resize: vertical; + min-height: 56px; +} + +.task-field-input:disabled, +.task-field-textarea:disabled, +.task-field-select:disabled { + opacity: 0.6; + cursor: not-allowed; +} + +/* Chips (enum single + multi-enum) */ +.task-field-chips { + display: flex; + flex-wrap: wrap; + gap: 6px; +} + +.task-field-chip { + padding: 3px 10px; + border: 1px solid var(--border-color, #2a2d34); + border-radius: 999px; + background: var(--chip-bg, #1c1f26); + color: var(--text-muted, #b4b8c0); + font-size: 12px; + cursor: pointer; + transition: background 0.12s ease, border-color 0.12s ease, color 0.12s ease; +} + +.task-field-chip:hover:not(:disabled) { + border-color: var(--accent, #4f7cff); +} + +.task-field-chip.is-active { + background: var(--accent, #4f7cff); + border-color: var(--accent, #4f7cff); + color: #fff; +} + +.task-field-chip:disabled { + opacity: 0.6; + cursor: not-allowed; +} + +/* Radio group */ +.task-field-radio-group { + display: flex; + flex-direction: column; + gap: 4px; +} + +.task-field-radio { + display: flex; + align-items: center; + gap: 6px; + font-size: 13px; + color: var(--text-primary, #e6e6e6); + cursor: pointer; +} + +/* Boolean toggle */ +.task-field-toggle { + display: inline-flex; + align-items: center; + cursor: pointer; +} + +.task-field-toggle input { + position: absolute; + opacity: 0; + width: 0; + height: 0; +} + +.task-field-toggle-track { + display: inline-block; + width: 34px; + height: 18px; + border-radius: 999px; + background: var(--border-color, #2a2d34); + position: relative; + transition: background 0.15s ease; +} + +.task-field-toggle-track::after { + content: ""; + position: absolute; + top: 2px; + left: 2px; + width: 14px; + height: 14px; + border-radius: 50%; + background: #fff; + transition: transform 0.15s ease; +} + +.task-field-toggle input:checked + .task-field-toggle-track { + background: var(--accent, #4f7cff); +} + +.task-field-toggle input:checked + .task-field-toggle-track::after { + transform: translateX(16px); +} + +.task-field-toggle input:disabled + .task-field-toggle-track { + opacity: 0.6; +} + +/* Inline validation error */ +.task-field-error { + font-size: 12px; + color: var(--accent-danger, #e5484d); +} + +.task-field-row.has-error .task-field-input, +.task-field-row.has-error .task-field-textarea, +.task-field-row.has-error .task-field-select { + border-color: var(--accent-danger, #e5484d); +} + +/* Collapsible detail-section group */ +.task-fields-group, +.task-fields-orphaned { + border-top: 1px solid var(--border-color, #2a2d34); + padding-top: 8px; +} + +.task-fields-group-header, +.task-fields-orphaned-header { + display: flex; + align-items: center; + gap: 6px; + width: 100%; + padding: 4px 0; + background: none; + border: none; + color: var(--text-muted, #8a8f98); + font-size: 12px; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.02em; + cursor: pointer; +} + +.task-fields-group-body, +.task-fields-orphaned-body { + display: flex; + flex-direction: column; + gap: 12px; + margin-top: 8px; +} + +.task-fields-orphaned-count { + margin-left: auto; + background: var(--chip-bg, #1c1f26); + border-radius: 999px; + padding: 0 8px; + font-size: 11px; +} + +.task-field-orphaned-value { + font-size: 13px; + color: var(--text-muted, #b4b8c0); + word-break: break-word; +} diff --git a/packages/dashboard/app/components/TaskFieldsSection.tsx b/packages/dashboard/app/components/TaskFieldsSection.tsx new file mode 100644 index 0000000000..db5eaa730c --- /dev/null +++ b/packages/dashboard/app/components/TaskFieldsSection.tsx @@ -0,0 +1,436 @@ +/** + * Schema-driven custom-field form section (U13 / KTD-14). + * + * Renders a task's workflow-defined custom fields ({@link WorkflowFieldDefinition}) + * as editable widgets, grouped by `render.placement`: + * - `detail` (and the default when unset) → inline, near the description. + * - `detail-section` → inside a collapsible group. + * Card-placed fields (`placement: "card"`) are intentionally NOT rendered here — + * those surface as badges on {@link TaskCard}. + * + * Widget selection (per `type` + optional `render.widget`): + * - enum → select (default) | radio | chips (single-select) + * - multi-enum → chips (multi-select) + * - boolean → toggle + * - date → date input + * - url/number → validated + * - string → text input + * - text → textarea + * + * Editing is per-field, save-on-commit (blur for inputs, change for + * toggles/selects/chips/radio). Each save calls `onSave({ [fieldId]: value })`; + * on a 400 the caller surfaces the typed rejection through `error`, which this + * component renders inline beneath the offending field. + * + * Orphaned values — keys in `customFields` with no matching definition — render + * read-only under a collapsed "Orphaned fields" disclosure (never destroyed, + * KTD-13). + * + * Zero field definitions AND zero orphaned values → the component renders + * nothing (null), so a task on a field-less workflow is byte-identical to + * today's UI (snapshot-guarded by the test suite). + */ +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { ChevronRight, ChevronDown } from "lucide-react"; +import type { + WorkflowFieldDefinition, + WorkflowFieldOption, + CustomFieldRejection, +} from "../api"; +import "./TaskFieldsSection.css"; + +export interface TaskFieldsSectionProps { + /** The task's workflow field definitions (from board-workflows payload). */ + fieldDefs: WorkflowFieldDefinition[]; + /** Current custom field values, keyed by field id. */ + customFields: Record; + /** + * Persist a single-field patch. Resolves on success; the caller is expected + * to throw / reject with the server's typed rejection so it can flow into + * `error`. May be omitted to render read-only (e.g. archived tasks). + */ + onSave?: (patch: Record) => Promise; + /** + * The most recent typed rejection from a failed save (400), surfaced inline + * beneath the matching field. Cleared by the caller on a successful save. + */ + error?: CustomFieldRejection | null; + /** When true, fields render read-only (no edit affordances). */ + readOnly?: boolean; +} + +/** Resolve the effective widget for a field, applying the per-type default. */ +function resolveWidget(field: WorkflowFieldDefinition): NonNullable["widget"] { + const explicit = field.render?.widget; + if (explicit) return explicit; + switch (field.type) { + case "enum": + return "select"; + case "multi-enum": + return "chips"; + case "boolean": + return "toggle"; + case "text": + return "textarea"; + default: + return "input"; + } +} + +interface FieldRowProps { + field: WorkflowFieldDefinition; + value: unknown; + onSave?: (patch: Record) => Promise; + error?: CustomFieldRejection | null; + readOnly: boolean; +} + +function FieldRow({ field, value, onSave, error, readOnly }: FieldRowProps) { + const { t } = useTranslation("app"); + const widget = resolveWidget(field); + const fieldError = error && error.fieldId === field.id ? error : null; + const disabled = readOnly || !onSave; + + // Serialize per-field saves: rapid chip/toggle/blur edits to the same field + // would otherwise fire overlapping PATCHes whose responses can resolve out of + // order, letting an older request clobber a newer selection. We chain each + // save onto the previous one for this field so they apply in click order. + const saveTailRef = useRef>(Promise.resolve()); + const commit = useCallback( + (next: unknown) => { + if (!onSave) return; + const run = () => onSave({ [field.id]: next }); + // Run after any in-flight save for this field, regardless of its outcome, + // so a rejected save doesn't permanently break the chain. The tail is kept + // settled-always (.catch) so its own rejection never floats unhandled and + // never blocks the next queued save — the caller surfaces failures via + // `error`, so we intentionally swallow here for ordering purposes only. + const prev = saveTailRef.current; + saveTailRef.current = prev.then(run, run).catch(() => {}); + }, + [onSave, field.id], + ); + + const labelId = `task-field-label-${field.id}`; + const controlId = `task-field-${field.id}`; + + // Prop-derived string value for the uncontrolled-style inputs (date / text / + // string / number / url). These were previously rendered with `defaultValue`, + // which only seeds on mount — so an external refresh of `customFields` (SSE or + // a save round-trip) left the DOM showing a stale value, and a later blur would + // commit that stale value back over the refreshed one. We make them controlled + // and re-sync to the latest prop whenever it changes. + const propTextValue = + field.type === "date" + ? typeof value === "string" + ? value.slice(0, 10) + : "" + : field.type === "number" + ? typeof value === "number" + ? String(value) + : "" + : typeof value === "string" + ? value + : ""; + const [localValue, setLocalValue] = useState(propTextValue); + useEffect(() => { + setLocalValue(propTextValue); + }, [propTextValue]); + + const renderControl = () => { + // enum → select / radio / chips (single) + if (field.type === "enum") { + const current = typeof value === "string" ? value : ""; + if (widget === "radio") { + return ( +
+ {(field.options ?? []).map((opt: WorkflowFieldOption) => ( + + ))} +
+ ); + } + if (widget === "chips") { + return ( +
+ {(field.options ?? []).map((opt) => { + const active = current === opt.value; + return ( + + ); + })} +
+ ); + } + // default: select + return ( + + ); + } + + // multi-enum → chips (multi-select) + if (field.type === "multi-enum") { + const current = Array.isArray(value) ? (value as string[]) : []; + return ( +
+ {(field.options ?? []).map((opt) => { + const active = current.includes(opt.value); + return ( + + ); + })} +
+ ); + } + + // boolean → toggle + if (field.type === "boolean") { + const checked = value === true; + return ( + + ); + } + + // date → date input + if (field.type === "date") { + return ( + setLocalValue(e.target.value)} + onBlur={(e) => { + const next = e.target.value; + if (next === propTextValue) return; + commit(next === "" ? null : next); + }} + /> + ); + } + + // text → textarea + if (field.type === "text") { + return ( +