From 12a3a035242dcd23c62d04bd3e472838df7ec5d0 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 21:52:36 -0700 Subject: [PATCH 01/20] docs(plan): workflow settings mechanism, settings hard-move, Settings UI redesign plan --- CONCEPTS.md | 3 + ...2-feat-workflow-settings-mechanism-plan.md | 339 ++++++++++++++++++ 2 files changed, 342 insertions(+) create mode 100644 docs/plans/2026-06-04-002-feat-workflow-settings-mechanism-plan.md diff --git a/CONCEPTS.md b/CONCEPTS.md index 4602719bf1..930f075379 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -10,6 +10,9 @@ One of Fusion's user-facing frontends — the browser dashboard and the terminal ### Global Settings User-level settings persisted server-side that apply across all Surfaces and all projects, as opposed to per-project settings. Values are validated at the write boundary — an invalid value is dropped rather than persisted — so every reader can trust what it loads. +### Workflow Setting +A typed setting declared by a workflow in its IR (id, type, default, options), mirroring the custom-task-field shape. Declarations describe the schema; *values* persist per workflow + project through a single validating store authority, so built-in workflows can carry values without their IR being editable. The engine consumes **effective settings** — stored value falling back to declaration default, with values that no longer validate against the current declaration dropped (never fed to execution). + ### Three-Tier Setting The named persistence pattern for a user preference on the dashboard: a device-local cache for instant reads, a write-through to Global Settings so other Surfaces see it, and a hydrate-on-mount from the server when no local value exists. A local or in-flight user choice always wins over server hydration, and changes propagate to other open tabs. diff --git a/docs/plans/2026-06-04-002-feat-workflow-settings-mechanism-plan.md b/docs/plans/2026-06-04-002-feat-workflow-settings-mechanism-plan.md new file mode 100644 index 0000000000..55b3bc86e1 --- /dev/null +++ b/docs/plans/2026-06-04-002-feat-workflow-settings-mechanism-plan.md @@ -0,0 +1,339 @@ +--- +title: "feat: Workflow settings mechanism, settings hard-move, and Settings UI redesign" +type: feat +status: active +date: 2026-06-04 +depth: deep +origin: none (solo planning bootstrap; builds on docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md and docs/plans/2026-06-04-001-feat-step-inversion-workflow-modelable-steps-plan.md) +--- + +# feat: Workflow settings mechanism, settings hard-move, and Settings UI redesign + +## Summary + +Give workflows a first-class **typed settings mechanism**: workflows declare settings in their IR (mirroring the shipped custom-task-fields pattern), setting *values* persist per `(workflowId, projectId)` through a single validating store authority, and the engine resolves **effective settings per task** at executor entry. Then **hard-move** the global/project settings that are actually workflow policy — step execution, review/approval, per-phase model lanes — onto this mechanism via a one-time, idempotent, marker-gated migration that removes the keys from the settings schema entirely. Finally, **redesign the Settings modal**: replace ~7,900 lines of ad-hoc inline controls with shared schema-driven field primitives, per-section components with co-located CSS, consistent grouping/naming, and redirect stubs pointing users to the workflow editor for moved settings. + +Keys already destined for column **trait** config under the columns/traits track (merge strategy cluster, `maxConcurrent` → WIP trait) go there, not here — this plan draws that boundary explicitly (KTD-4). + +--- + +## Problem Frame + +The columns/traits and step-inversion tracks made workflows the home for board and step *policy* — columns, traits, custom task fields, step modeling. But the policy *knobs* that parameterize that behavior still live as ambient project/global settings in `packages/core/src/settings-schema.ts`: `workflowStepTimeoutMs`, `runStepsInNewSessions`, `requirePrApproval`, `reviewHandoffPolicy`, per-phase model lanes, and dozens more. This is now incoherent: + +- A workflow models *how* tasks execute, but the timeouts, review gates, and model lanes that govern that execution are configured somewhere else entirely, with no relationship to the workflow. +- The columns plan's identity posture (KTD-6) says user-lowerable enforcement floors belong *inside an explicitly authored workflow, never as ambient settings* — the current settings catalog violates this. +- The Settings modal has grown to ~7,900 lines of bespoke inline controls across ~25 sections with no shared field components, making every settings change expensive and the UI inconsistent. + +There is no mechanism for a workflow to declare a setting at all — that's the gap this plan fills first, then exploits. + +--- + +## Scope Boundaries + +### In scope + +- `settings` declarations on WorkflowIr v2 (additive): `WorkflowSettingDefinition[]` with typed values, defaults, enum options, descriptions, render hints; `validateSettings` in `parseWorkflowIr`. +- A per-`(workflowId, projectId)` setting-**value** store (new table, schema bump) with a single validating write authority; values writable for built-in workflows even though built-in IR is non-editable. +- Per-task effective-settings resolution in core, consumed by the engine at executor entry, preserving the flat `Partial` read shape. +- Built-in workflow declarations for every moved key, with defaults byte-equal to today's `DEFAULT_PROJECT_SETTINGS` values. +- `WorkflowSettingsPanel` in the workflow node editor (declarations + defaults; per-project values in project context); agent-tool parity (`fn_workflow_create/update` declarations; a value read/write path). +- One-time hard-move migration (per-project marker, idempotent) of the moved-key catalog (see U4) out of `DEFAULT_PROJECT_SETTINGS`, with tombstone allowlist, explicit value nulling, and surface sweep: settings export v2, cross-node sync guard, SettingsModal save-split, CLI, consistency test. +- Full SettingsModal redesign: shared schema-driven field primitives, per-section components + co-located CSS files, regrouped navigation, redirect stubs for moved settings, i18n throughout. + +### Deferred to Follow-Up Work + +- Removing the redirect stubs (one release after this ships). +- A `workflowSettings` channel in cross-node settings *sync* (this plan excludes moved keys from sync and reports the exclusion; full sync of the value table is follow-up — see KTD-8). +- Per-task setting value overrides (values are per workflow+project only this round). +- Plugin-contributed setting types or plugin-declared settings. +- Moving capacity/scheduler ops knobs (`backlogPressure*`, `stale*`, `pollIntervalMs`, etc.) — these are engine/scheduler operations policy, not per-workflow process policy; reconsider only after the mechanism proves out. +- Migrating merge-cluster keys — owned by the columns plan's merge-trait track (U7 there), not this plan. + +### Outside this plan's identity + +- No global-default-plus-workflow-override layering: the user decision is a hard move. A moved key has exactly one home. +- Integrity guarantees (lost-work trio, crash recovery, audit) stay non-configurable — they never become workflow settings. +- Device-local three-tier prefs (theme, language, font scale) stay exactly as they are. + +--- + +## Requirements + +**Mechanism** + +- R1. Workflows can declare typed settings in IR (`id`, `name`, `type`, `default`, `options`, `description`, render hints); declarations are validated at save by `parseWorkflowIr` with the same rigor as `validateFields` (unique ids, type whitelist, options iff enum-kind). +- R2. Setting values persist per `(workflowId, projectId)` through a single store write authority that validates each value against the named workflow's declaration schema and rejects invalid values with typed errors — invalid values are never persisted. +- R3. The engine resolves effective settings **per task** (stored value → declaration default), as a flat `Partial`-shaped object, via a never-throw resolver; all moved-key engine read sites receive values from this resolution. +- R4. Built-in workflows (`builtin:coding`, stepwise) declare every moved setting with defaults equal to today's `DEFAULT_PROJECT_SETTINGS` values; setting *values* for built-ins are writable even though built-in IR is not editable. +- R5. The workflow node editor has a settings panel for declarations/defaults and per-project values; `fn_workflow_create/update` accept `settings` declarations and agents can read/write values with the same typed-rejection contract. + +**Migration** + +- R6. A one-time, idempotent, per-project migration (gated by a persisted migration marker) snapshots each project's effective moved-key values, writes them to **every workflow the project's tasks can resolve** — the distinct `task_workflow_selection` workflowIds in use, unioned with the resolved project default, where an unset/empty `defaultWorkflowId` normalizes to `builtin:coding` (matching the resolver's falsy-id degradation) — removes the keys from `DEFAULT_PROJECT_SETTINGS`/`GLOBAL` schema objects, and explicitly nulls persisted raw values. +- R7. Every settings surface stays consistent with the move: keys lists/predicates, validation, export/import (v2), cross-node sync, SettingsModal save-split, `useAppSettings`, CLI settings commands — guarded by a consistency test so the lists cannot silently drift. +- R8. Pre-migration payloads cannot resurrect moved keys: importing a v1 export upgrades moved keys into workflow setting values; sync of moved keys is suppressed via the tombstone allowlist. + +**Settings UI** + +- R9. SettingsModal is rebuilt from shared schema-driven field primitives (toggle/number/select/text/textarea rows) and per-section components with co-located CSS files following the dashboard CSS conventions. +- R10. Each moved setting's former location shows a redirect stub ("moved to the workflow editor" with a link) for one release. +- R11. Behavior of remaining settings is preserved: save-splitting by scope predicates, null-as-delete clears, changed-only project writes, three-tier device prefs untouched, all strings `t()`-wrapped. + +--- + +## High-Level Technical Design + +Effective-settings resolution (the load-bearing data flow — flat shape preserved so ~20 engine read sites don't change): + +```mermaid +flowchart TB + subgraph Declarations + IR["WorkflowIr v2
settings: WorkflowSettingDefinition[]"] + BI["Built-in workflow IRs
(declare all moved keys,
defaults = legacy defaults)"] + end + subgraph Values + VT["workflow_settings table
(workflowId, projectId, values JSON)"] + WA["Store write authority
validate → typed rejection
(invalid never persisted)"] + WA --> VT + end + T["Task"] --> RES + IR --> RES + BI --> RES + VT --> RES + RES["resolveEffectiveSettings(task)
value ?? declaration.default
drop-on-orphan, never-throw"] + RES --> ENG["Engine executor entry:
flat Partial<Settings> shape
settings.workflowStepTimeoutMs etc."] + PS["Project settings
(remaining keys only)"] --> ENG +``` + +One-time migration sequence (per project, idempotent, marker-gated): + +```mermaid +flowchart TB + S["Store open / migration runner"] --> M{"project has
settingsMigrationVersion ≥ 1?"} + M -->|yes| DONE["no-op"] + M -->|no| SNAP["Snapshot effective values of
moved keys (typed read,
pre-removal schema)"] + SNAP --> WV["Write values to every in-use
(workflowId, projectId): distinct task
selections ∪ resolved project default
(unset default → builtin:coding)"] + WV --> NULLS["Explicitly null moved keys
in raw project + global stores"] + NULLS --> MARK["Set marker"] + MARK --> DONE2["Engine + UI read only
new home from now on"] + TOMB["Tombstone allowlist
(moved-key names)"] -.->|"shields: sync diff,
v1 import, stale writers"| NULLS +``` + +The schema-object key removal (from `DEFAULT_PROJECT_SETTINGS`) ships in the same commit as the migration — the two are inseparable, because `GlobalSettingsStore`/project `updateSettings` re-inject `DEFAULT_*` values after deletion (`packages/core/src/global-settings.ts:181-211`): a key left in the DEFAULT object re-materializes on the next unrelated save and silently overrides the migrated value. + +--- + +## Key Technical Decisions + +- KTD-1 — **Mirror the fields pattern exactly.** `WorkflowSettingDefinition` clones the shape of `WorkflowFieldDefinition` (`packages/core/src/workflow-ir-types.ts:90-98`); `validateSettings` clones `validateFields` (`packages/core/src/workflow-ir.ts:659-727`); the editor panel clones `WorkflowFieldsPanel.tsx`. This is the established, shipped pattern for "workflow-declared typed schema" — inventing a second idiom would be gratuitous divergence. Settings get their **own** render-hint type (widget only — no `card`/`detail` placement, which is task-card-specific). + +- KTD-2 — **Values live per `(workflowId, projectId)` in a new table, not in IR.** Built-in workflows are non-editable (`isBuiltinWorkflowId` guard in store CRUD), so values cannot be written into built-in IR; and per-project tuning of the same workflow must survive the migration (two projects using `builtin:coding` with different step timeouts). Declarations describe the schema; the value table carries the data — exactly the workflow-fields ↔ `tasks.customFields` split, one level up. Value writes are validated against the *named* workflow's schema (not the project's current default workflow), and built-in workflow values are writable while built-in declarations are not — two distinct error paths in the write authority. + +- KTD-3 — **Per-task effective-settings resolution, flat shape.** The engine reads moved keys as flat fields on `Partial` at ~20 sites (`packages/engine/src/executor.ts:9974, :2149, :5154`, `packages/engine/src/step-session-executor.ts:671`, reviewer/merger). A `resolveEffectiveSettings(task)` sibling of `resolveWorkflowIrForTask` (`packages/core/src/workflow-ir-resolver.ts`) builds the same flat shape from value-table + declaration defaults at executor entry, so read sites keep their exact expressions. Same never-throw degradation contract as the IR resolver. Each read site's hardcoded `?? ` fallback must be audited to match the built-in declaration default — otherwise resolution returning `undefined` silently overrides migrated values. + +- KTD-4 — **Trait-config boundary.** Column-scoped policy belongs to column *traits* (merge strategy/squash/fileScope → merge trait; `maxConcurrent` → WIP trait, per columns plan KTD-6/U6/U7). Workflow *settings* carry workflow-scoped policy not tied to a single column: step execution knobs, review/approval policy, per-phase model lanes. A key gets exactly one home; the moved-key catalog (U4) records the home for every candidate so the same policy never has two sources of truth. + +- KTD-5 — **Hard-move = schema removal + tombstones + explicit nulls + per-project marker; no experimental flag.** A behavior flag would require moved keys to exist in both homes simultaneously (flag-OFF reads old, flag-ON reads new), which contradicts a hard move and re-creates the dual-writer hazard. Instead, the migrated/not-migrated state is per project, gated by a persisted `settingsMigrationVersion` marker, and transitions exactly once. A `MOVED_SETTINGS_KEYS` tombstone allowlist (the only remaining record of the old names) shields the surfaces that can encounter old payloads: sync diff, v1 import, stale CLI writers. Safety comes from characterization tests proving effective-value equivalence across the migration boundary, not from a flag. + +- KTD-6 — **Drop-on-orphan for setting values (deliberate divergence from fields).** `reconcileFieldsOnWorkflowChange` retains orphaned task-field values and surfaces them in a disclosure — fine for display data, dangerous for policy the engine consumes (a retyped enum→number setting with a stale string value would feed garbage into execution). Effective resolution drops values that no longer validate against the current declaration and falls to the declaration default. The editor surfaces dropped values; the engine never sees them. + +- KTD-7 — **Model-lane resolution chain.** Per-phase project lanes (`executionProvider/ModelId`, `planningProvider/ModelId`, `validatorProvider/ModelId`, fallbacks, title summarizer) move to workflow settings. The documented chain (`packages/engine/src/executor.ts:5755-5770`, the `resolveExecutorSessionModel` lane-hierarchy site) becomes: workflow-setting lane → global lane (`executionGlobalProvider` etc., which stay global) → project default override → global default. An empty workflow lane falls through; characterization tests pin the chain before and after. + +- KTD-8 — **Export v2; sync excludes moved keys this round.** `settings-export.ts` bumps to `version: 2` with a `workflowSettings` section (declarations are in workflows; export carries values). Importing v1 upgrades moved keys into workflow setting values using the same write-target rule as the migration (in-use workflows ∪ resolved default, unset default normalized to `builtin:coding`) instead of dead-writing them into project settings. Cross-node settings sync (`packages/dashboard/src/routes/register-settings-sync-routes.ts:15-33`) filters moved keys out of diffs/push/pull via the tombstone list and surfaces "workflow settings are not synced yet" in the sync UI; a full sync channel is deferred (Scope Boundaries). + +- KTD-9 — **Cascade-delete values on workflow deletion.** Deleting a custom workflow deletes its value rows; tasks pinned to a deleted workflow already degrade to `builtin:coding` via the resolver and therefore read built-in declarations + built-in values. No unreachable orphan rows. + +- KTD-10 — **Schema-driven Settings UI primitives.** The redesign introduces shared field-row primitives (toggle/number/select/text/textarea + section scaffolding) rendered from a per-section descriptor, the same render-by-type idiom as `WorkflowFieldsPanel` widgets. SettingsModal becomes a shell (nav + save-split + scope handling) composing per-section components, each with a co-located CSS file. This is what makes the modal cheap to change and is also the convergence point: `WorkflowSettingsPanel` value editing reuses the same primitives. + +--- + +## Implementation Units + +### U1. Workflow IR settings declarations + validation + built-in declarations + +- **Goal:** Workflows can declare typed settings; built-ins declare the full moved-key catalog. +- **Requirements:** R1, R4 +- **Dependencies:** none +- **Files:** `packages/core/src/workflow-ir-types.ts`, `packages/core/src/workflow-ir.ts`, `packages/core/src/builtin-coding-workflow-ir.ts`, `packages/core/src/builtin-stepwise-coding-workflow-ir.ts`, `packages/core/src/__tests__/workflow-ir.test.ts` +- **Approach:** Add `settings?: WorkflowSettingDefinition[]` to `WorkflowIrV2` (additive; v1 stays frozen). Definition shape: `{ id, name, type, default?, options?, description?, render? }` with type whitelist `string | text | number | boolean | enum | multi-enum`; settings-specific render hint (`widget` only). `validateSettings` in `validateV2` mirrors `validateFields`: non-empty unique ids, type whitelist, options iff enum-kind, unique option values, default validates against its own type/options. Built-in IRs declare every moved key with defaults byte-equal to current `DEFAULT_PROJECT_SETTINGS` literals. Note: presence of `settings` keeps IR v2 under `downgradeIrToV1IfPure`. +- **Patterns to follow:** `validateFields` (`workflow-ir.ts:659-727`), `WorkflowFieldDefinition` types, `WorkflowIrError` surfacing. +- **Test scenarios:** + - Valid declaration of each type parses and round-trips through `parseWorkflowIr`. + - Duplicate setting ids → `WorkflowIrError`; empty id → error; unknown type → error. + - `enum` without options → error; options on non-enum type → error; duplicate option values → error. + - Default value violating its own type (`type: number`, `default: "x"`) or enum options → error. + - Built-in coding workflow declares every key in `MOVED_SETTINGS_KEYS` and each declaration default strictly equals the legacy `DEFAULT_PROJECT_SETTINGS` literal (consistency assertion — this is the parity anchor for the migration). + - IR with `settings` present is not downgraded to v1. +- **Verification:** `pnpm test` for core IR suites green; consistency assertion ties built-in defaults to legacy literals. + +### U2. Setting-value store: table, write authority, validation core + +- **Goal:** Persist values per `(workflowId, projectId)` behind a single validating authority. +- **Requirements:** R2, R4 +- **Dependencies:** U1 +- **Files:** `packages/core/src/db.ts`, `packages/core/src/store.ts`, `packages/core/src/workflow-settings.ts` (new), `packages/core/src/__tests__/workflow-settings.test.ts`, `packages/core/src/__tests__/db-migrate.test.ts` +- **Approach:** New table `workflow_settings (workflowId, projectId, values JSON, updatedAt)` with composite PK; `SCHEMA_VERSION` bump (additive, forward-only, idempotent migration step). `workflow-settings.ts` is the side-effect-free validation core mirroring `task-fields.ts`: `validateSettingValuePatch(declarations, patch)` → typed rejections (`unknown-setting`, `type-mismatch`, `enum-violation`, `no-settings-defined`), plus `resolveEffectiveSettingValues(declarations, stored)` implementing drop-on-orphan (KTD-6) with an explicit comment marking the deliberate divergence from the field reconciler. Store authority `updateWorkflowSettingValues(workflowId, projectId, patch)`: validates against the **named** workflow's declarations; built-in workflow ids accepted for value writes (declaration edits stay rejected); null-as-delete per key. Cascade-delete rows in workflow deletion (KTD-9). +- **Execution note:** Bumping `SCHEMA_VERSION` requires the broad literal sweep — `grep -rn 'toBe()' packages/` hits ~40+ sites across ≥8 test files (`db.test.ts` ~26, `db-migrate.test.ts`, `goals-schema.test.ts`, `task-documents.test.ts`, plus insight-store/run-audit/store-merge-queue/merge-request-record/mission-store suites); update all of them atomically in this unit's commit. The U4 settings migration is a separate commit with no DB schema change. +- **Patterns to follow:** `updateTaskCustomFields` / `validateCustomFieldPatch` (`store.ts:6947`, `task-fields.ts`), `addColumnIfMissing` migration discipline, db-migrate forward-path tests. +- **Test scenarios:** + - Write a valid value for a custom workflow → persisted; read back typed. + - Write value for `(builtin:coding, project)` → accepted (R4); attempt to edit built-in declarations via workflow update → still rejected. + - Type-mismatch / unknown-setting / enum-violation patches → typed rejection, nothing persisted (write boundary contract). + - Null value in patch deletes the key; subsequent effective resolution falls to declaration default. + - Retype a declared setting (enum→number) with a stale string value stored → effective resolution drops it and returns declaration default; stored row untouched until next write (drop-on-orphan). + - Delete custom workflow → its value rows are gone; task pinned to deleted workflow resolves builtin values. + - db-migrate forward-path test for the new version; schema-version literal sweep complete. +- **Verification:** Core suites green; no row rewrites in migration; corruption-resilience posture unchanged. + +### U3. Effective-settings resolution + engine integration + fallback audit + +- **Goal:** Engine reads moved keys from per-task resolution; behavior is characterization-identical for untouched defaults. +- **Requirements:** R3 +- **Dependencies:** U1, U2 +- **Files:** `packages/core/src/workflow-ir-resolver.ts` (or sibling `workflow-settings-resolver.ts`), `packages/engine/src/executor.ts`, `packages/engine/src/step-session-executor.ts`, `packages/engine/src/reviewer.ts`, `packages/engine/src/merger.ts`, `packages/core/src/__tests__/workflow-settings-resolver.test.ts`, `packages/engine/src/__tests__/executor-settings.test.ts` +- **Approach:** `resolveEffectiveSettings(task | workflowId+projectId)` composes `resolveWorkflowIrForTask` + value table + drop-on-orphan into a flat `Partial`-shaped object (never-throw, degrade like the IR resolver). Engine builds it once at executor entry and merges over the remaining project/global settings object so the ~20 read sites keep their exact `settings.` expressions. Audit every moved-key read site's hardcoded `?? ` fallback (e.g. `executor.ts:9974` `?? 360_000`, `step-session-executor.ts:671`) and align each with the built-in declaration default — assert alignment in a test rather than by eye. Model-lane chain rewired per KTD-7. +- **Execution note:** Characterization-first — capture current effective values consumed by a scripted run (default settings, and a customized-project fixture) before wiring resolution; then prove the post-wiring run consumes identical values. +- **Patterns to follow:** `resolveWorkflowIrForTask` never-throw contract; `workflow-parity.ts` observation machinery for characterization. +- **Test scenarios:** + - Task on `builtin:coding`, no stored values → effective values equal legacy defaults for every moved key (parity anchor). + - Stored value for `(workflow, project)` → engine read site receives it (spot-check `workflowStepTimeoutMs`, `runStepsInNewSessions`, `requirePrApproval`). + - Two tasks in one project resolving different workflows → each gets its own workflow's effective values (per-task resolution, not per-project). + - Workflow lacking a declaration for a moved key (custom workflow with empty settings) → falls to the declaration-absent path → read-site fallback; test asserts the fallback equals the legacy default (I2 guard). + - New custom workflow created post-migration with empty settings → effective values are declaration/read-site defaults, **not** the project's prior customized values — asserted explicitly as expected behavior (and documented in U10's user docs: switching a project to a new workflow starts from that workflow's defaults). + - Model lanes: workflow lane set → wins; empty → global lane; both empty → global default (chain pinned, KTD-7). + - Corrupt/missing workflow → resolver degrades, never throws, run proceeds on builtin declarations. + - Fallback-alignment assertion: for every moved key, read-site literal fallback === built-in declaration default. +- **Verification:** Engine suites green; characterization fixtures prove value-equivalence pre/post. + +### U4. One-time hard-move migration + tombstones + schema removal + +- **Goal:** Each project's effective moved-key values land in the value table; moved keys leave the settings schema for good. +- **Requirements:** R6, R8 +- **Dependencies:** U1, U2, U3 +- **Files:** `packages/core/src/settings-schema.ts`, `packages/core/src/settings-validation.ts`, `packages/core/src/global-settings.ts`, `packages/core/src/store.ts`, `packages/core/src/moved-settings.ts` (new: `MOVED_SETTINGS_KEYS` tombstone list + marker helpers), `packages/core/src/__tests__/settings-migration.test.ts` +- **Approach:** Single commit containing: (a) `MOVED_SETTINGS_KEYS` tombstone allowlist with the definitive moved-key catalog — step execution (`workflowStepTimeoutMs`, `workflowStepScopeEnforcement`, `planOnlyScopeLeakEnforcement`, `workflowRevisionForkOnScopeMismatch`, `strictScopeEnforcement`, `runStepsInNewSessions`, `maxParallelSteps`, `buildRetryCount`, `buildTimeoutMs`, `verificationFixRetries`, `maxPostReviewFixes`), review/approval (`requirePrApproval`, `requirePlanApproval`, `reviewHandoffPolicy`, `maxReviewerContextRetries`, `maxReviewerFallbackRetries`, the `reflection*` trio — verify during the U3 audit that `reflectionAfterTask`/`reflectionIntervalMs` actually have engine read sites; any key without a per-task reader stays in project settings per the catalog-shrink rule), and per-phase model lanes (`executionProvider/ModelId`, `planningProvider/ModelId` + fallback, `validatorProvider/ModelId` + fallback, `titleSummarizerProvider/ModelId` + fallback); each entry records its new home and built-in default. `completionDocumentationMode` stays in project settings — `triage.ts:1082` reads it via `store.getSettings()` outside per-task execution scope, so it fails the per-task-reader rule. Merge-cluster and `maxConcurrent` keys are explicitly annotated as trait-owned (KTD-4) and not in this list. (b) Migration runner at store open, per project: skip if `settingsMigrationVersion ≥ 1`; snapshot effective values via the **pre-removal typed read**; write the snapshot to **every in-use `(workflowId, projectId)`** — the distinct workflowIds across the project's `task_workflow_selection` rows, unioned with the resolved project default, normalizing an unset/empty `defaultWorkflowId` to `builtin:coding` (the id every selection-less task resolves to) — then explicitly null raw persisted keys in both stores; set marker. The value-writes and the project-store nulling share one SQLite transaction (same DB); the global-store null is defensive only (all moved keys are project-scoped) and may follow outside the transaction. (c) Removal of moved keys from `DEFAULT_PROJECT_SETTINGS`/`DEFAULT_GLOBAL_SETTINGS` and their validators — inseparable from (b) because the stores re-inject DEFAULT values after deletion (`global-settings.ts:181-211`). The `SCHEMA_VERSION` bump itself lands in U2 (the new table); this commit contains no DB schema change — only the settings-schema key removal, tombstones, and the runner. +- **Execution note:** Characterization-first: a fixture project with customized moved keys must produce identical engine-effective values before and after the migration runs. +- **Test scenarios:** + - Fresh project post-migration: effective values equal declaration defaults; no moved key present in `PROJECT_SETTINGS_KEYS`. + - Project with customized `workflowStepTimeoutMs`/`requirePrApproval`/`executionProvider` → values appear under every in-use `(workflowId, projectId)`; raw settings file no longer contains the keys; engine-effective values identical pre/post (characterization). + - Mixed-pinning fixture: one task on `builtin:coding` (no selection row) and one pinned to a custom workflow, project `defaultWorkflowId` unset → both tasks read identical customized effective values post-migration (the in-use-union write target plus the `builtin:coding` normalization). + - Project with `defaultWorkflowId` unset and no task selections → snapshot lands on `(builtin:coding, projectId)`; a default-workflow task reads it identically pre/post. + - Migration runs twice → second run is a no-op (idempotency via marker). + - Crash between value-write and nulling → re-run converges to the same end state (write-then-null is re-runnable; values overwrite identically). + - Post-migration save of an unrelated setting does **not** re-materialize any moved key (the C1 default re-injection trap — the load-bearing regression test). + - Project whose `defaultWorkflowId` points at a deleted/missing workflow → values land on `builtin:coding` (resolver degradation path). + - Stale writer sends a moved key through `updateSettings` post-migration → key is filtered/ignored via tombstone, not persisted. +- **Verification:** Migration suite green; the default re-injection regression test is the gate; characterization equivalence proven. + +### U5. Surface sweep: export v2, sync guard, CLI, consistency test + +- **Goal:** Every settings surface agrees about which keys exist where; old payloads can't resurrect moved keys. +- **Requirements:** R7, R8 +- **Dependencies:** U4 +- **Files:** `packages/core/src/settings-export.ts`, `packages/dashboard/src/routes/register-settings-sync-routes.ts`, `packages/dashboard/src/routes/register-settings-sync-helpers.ts`, `packages/dashboard/app/hooks/useNodeSettingsSync.ts`, `packages/cli/src/commands/settings.ts`, `packages/cli/src/commands/settings-export.ts`, `packages/cli/src/commands/settings-import.ts`, `packages/core/src/__tests__/settings-export.test.ts`, `packages/core/src/__tests__/settings-consistency.test.ts` (new) +- **Approach:** Export bumps to `version: 2` with a `workflowSettings` value section; importing v1 upgrades moved keys into the project-default workflow's values (KTD-8); merge-mode semantics documented for the new section. Sync: `computeSettingsDiff` filters `MOVED_SETTINGS_KEYS` from field lists; inbound `applyRemoteSettings` drops them; the sync UI renders an inline, non-dismissible informational note at the bottom of the sync diff section ("Workflow settings are not synced across nodes yet.", `--text-muted`, no action affordance this release). CLI settings commands stop listing moved keys and print a pointer to the workflow editor. New consistency test (registration-drift lesson): asserts that schema key lists, tombstone list, built-in declarations, and the SettingsModal section descriptors are mutually consistent — a key may appear in exactly one regime. +- **Test scenarios:** + - Export post-migration → v2 payload carries workflow setting values; no moved key under `project`. + - Import v1 payload containing `workflowStepTimeoutMs` → value lands in the default workflow's values, not project settings; remaining v1 keys import normally. + - Import v2 payload round-trips values. + - Sync diff between migrated and unmigrated nodes → moved keys never appear in diff/push/pull; inbound push containing a moved key is dropped (multi-node race guard). + - Consistency test fails if a key is simultaneously in `DEFAULT_PROJECT_SETTINGS` and `MOVED_SETTINGS_KEYS`, or in tombstones but missing from built-in declarations. + - CLI `settings` listing excludes moved keys and shows the redirect hint. +- **Verification:** Export/sync/CLI suites green; consistency test in place as the permanent drift guard. + +### U6. WorkflowSettingsPanel in the node editor + routes + +- **Goal:** Users author setting declarations and edit values where the rest of workflow config lives. +- **Requirements:** R5 +- **Dependencies:** U1, U2 +- **Files:** `packages/dashboard/app/components/WorkflowSettingsPanel.tsx` (new), `packages/dashboard/app/components/WorkflowSettingsPanel.css` (new), `packages/dashboard/app/components/WorkflowNodeEditor.tsx`, `packages/dashboard/app/utils/workflow-flow-mapping.ts`, dashboard server routes for value read/write, `packages/dashboard/app/__tests__/WorkflowSettingsPanel.test.tsx` +- **Approach:** Clone the `WorkflowFieldsPanel` conventions: sibling panel in the editor, kebab-case id slugify, immutable id (edit = remove+add with warning), client mirrors the server type whitelist, validation server-side at save surfaced through the shared error band, i18n via `useTranslation`. The panel has an internal **tab pair** — "Definitions" (declarations + defaults; custom workflows only, read-only declaration view for built-ins) and "Values" (per-project values, editable for any workflow including built-ins) — so the editor gains one sibling panel, not two. Declaration edits ride the editor's existing IR save flow; **value edits batch in panel state and commit through a dedicated Save action in the Values tab** (one patch to the store authority route — never per-field writes, never fused with the IR Save; the two write authorities stay separate). Rejections render the typed error per field. The Values tab **binds to the projectId active when the panel opened**; if the active project changes while the editor is open, show a stale-context notice ("Values shown are for project X — reopen to edit the current project") instead of silently rebinding writes; when no project is active, the Values tab states that a project context is required. Orphaned stored values (KTD-6) render in a collapsible "Orphaned values" section below the live list: each row shows the key id, the stored raw value, and a delete affordance (null-patch through the store authority) — no edit affordance, with a note explaining the definition changed or was removed. +- **Patterns to follow:** `WorkflowFieldsPanel.tsx` + `.css`, editor save flow in `WorkflowNodeEditor.tsx`, CSS token conventions (`--duration-*`, `--text-muted`). +- **Test scenarios:** + - Declare a setting of each type via the panel → IR save round-trips; invalid declaration (dup id) surfaces the server error band. + - Built-in workflow: declarations render read-only; values editable for the active project. + - Value edits batch: editing three fields then Save emits exactly one patch; a rejected field keeps the other two applied per the authority's typed-rejection semantics and renders the per-field error. + - Value edit with type mismatch → typed rejection rendered, value unchanged. + - No active project → Values tab shows the requires-project state, no write path. + - Active project changes while editor open → stale-context notice shown; pending edits do not write to the new project. + - Orphaned values render in the collapsible disclosure with delete affordance; delete removes the stored row via null-patch. +- **Verification:** Dashboard suites green; manual editor walkthrough (declare → set value → engine pick-up) in a worktree dashboard instance. + +### U7. Agent-tool and SDK parity + +- **Goal:** Agents can do everything the editor can: declare settings, read/write values. +- **Requirements:** R5 +- **Dependencies:** U1, U2 +- **Files:** `packages/cli/src/extension.ts`, `packages/core/src/agent-prompts.ts`, `packages/cli/skill/fusion/references/engine-tools.md`, `packages/plugin-sdk` type surface, `packages/cli/src/__tests__/extension-workflow-settings.test.ts` +- **Approach:** `fn_workflow_create/update` accept `settings` declaration arrays (validated by the same `parseWorkflowIr` path; built-in declaration edits rejected with the existing built-in error). New value read/write tool (or extension of an existing workflow tool) with the typed-rejection contract from U2; reads return effective values (post drop-on-orphan) plus raw stored values so agents see both. Document in engine-tools reference; mirror types in plugin-sdk. +- **Test scenarios:** + - Agent creates a workflow with settings declarations → persisted and validated identically to editor saves. + - Agent writes a valid value for `(builtin:coding, project)` → accepted; declaration edit on builtin → rejected with the distinct error (I8 two-path contract). + - Agent write with enum violation → typed rejection surfaced through the tool result. + - Tool read returns effective values matching `resolveEffectiveSettings`. +- **Verification:** CLI extension suites green; engine-tools doc updated. + +### U8. Settings UI primitives + section scaffolding + +- **Goal:** The shared, schema-driven building blocks the redesigned modal and the workflow settings panel both compose. +- **Requirements:** R9 +- **Dependencies:** none (parallel with U1-U5) +- **Files:** `packages/dashboard/app/components/settings/` (new directory: `SettingsFieldRow.tsx`, `SettingsToggleRow.tsx`, `SettingsNumberRow.tsx`, `SettingsSelectRow.tsx`, `SettingsTextRow.tsx`, `SettingsSection.tsx`, plus co-located `.css` per component), `packages/dashboard/app/__tests__/settings-primitives.test.tsx` +- **Approach:** Primitives render from a field descriptor (`{ key, labelKey, type, options?, scope, help? }`) — the same render-by-type idiom as `WorkflowFieldsPanel` widgets — with uniform layout, label/help/error placement, and scope badge (global/project). Co-located CSS per component following the extraction conventions: `--duration-*` tokens for any animation (never `--transition-*` as a duration), canonical `--text-muted` (FN-4286 guard), no additions to monolith stylesheets; the existing `animation-duration-tokens.css.test.ts` sweep must stay green. +- **Test scenarios:** + - Each primitive renders label/value/help and propagates change events with the right type. + - Null-clear interaction emits the null-as-delete signal (preserving the modal's clear semantics). + - CSS sweep test stays green over the new files; no banned tokens. +- **Test expectation note:** visual polish is verified in U9's browser pass; unit scope here is behavior + tokens. +- **Verification:** Component tests green; lint (including i18n and CSS guards) green. + +### U9. SettingsModal redesign: section-by-section rebuild + +- **Goal:** SettingsModal becomes a thin shell over per-section components built from U8 primitives; moved settings disappear behind redirect stubs; everything else behaves identically. +- **Requirements:** R9, R10, R11 +- **Dependencies:** U4, U5, U8 +- **Files:** `packages/dashboard/app/components/SettingsModal.tsx`, `packages/dashboard/app/components/SettingsModal.css`, `packages/dashboard/app/components/settings/sections/` (new per-section components + CSS), `packages/dashboard/app/hooks/useAppSettings.ts`, `packages/dashboard/app/__tests__/SettingsModal.test.tsx` +- **Approach:** Keep the proven shell mechanics — `SETTINGS_SECTIONS` nav model with group headers, `visibleSections` gating, save-splitting via `isGlobalSettingsKey`/`isProjectSettingsKey`, null-as-delete, changed-only project writes — but extract each section into a descriptor-driven component under `settings/sections/`. Remove moved settings from their sections; where a section's content moved wholesale (per-phase model lanes, step-execution and review knobs), render a redirect stub row that opens the workflow editor with the Settings panel pre-selected via a query/hash param (e.g. `?panel=settings`, read by `WorkflowNodeEditor` on mount — deterministic and testable), targeting the project's default workflow (one release, per KTD-5). Target IA for the regroup (group headers → sections): **Account** (Authentication); **Global** — General, Appearance, Models & Providers (merging global-models + openrouter + onboarding), Notifications (ntfy/webhook/failure), Research, Remote Access & Node Sync (merging remote + node-sync), Experimental; **Runtimes** unchanged; **Project** — General, Commands & Scripts, Git & Worktrees, Scheduling & Capacity, GitHub Integration, Agents & Permissions, Memory & Backups, Research, Secrets, Plugins. Former project-models and review/step sections collapse into redirect stubs under Project. Section renames keep stable section `id`s where a section survives so deep links and `DEFAULT_SETTINGS_SECTION` stay valid. Device-local three-tier prefs (theme/language/font scale) keep their hooks untouched. The 7,900-line file shrinks to shell + imports. +- **Execution note:** Land section-by-section in reviewable slices rather than one mega-commit — this is the branch most exposed to the extraction-vs-semantics merge hazard; if `main` changes a setting's behavior mid-flight, port the semantic change to the section's new home and run the union suite. +- **Test scenarios:** + - Save-split regression: editing one global + one project setting in the same session produces the same `updateGlobalSettings`/`updateSettings` patches as before the redesign (characterization of the split function). + - Clearing a project override emits null-as-delete; untouched inherited values are not written (changed-only gate preserved). + - Moved-setting sections render redirect stubs with a working link to the workflow editor; no moved key is renderable or savable anywhere in the modal. + - Section visibility gating (remote/research/evals) unchanged. + - i18n: all new strings `t()`-wrapped (lint-enforced); language switch re-renders section labels. + - Three-tier prefs still hydrate/write-through (theme toggle round-trip). +- **Verification:** Dashboard suites + lint green; browser walkthrough of every section (fresh bundle, free port — never 4040) confirming layout, save, clear, and stub navigation. + +### U10. End-to-end characterization, docs, and parity closure + +- **Goal:** Prove the whole move is behavior-preserving and leave the documentation trail. +- **Requirements:** R3, R6, R7 +- **Dependencies:** U1-U9 +- **Files:** `packages/core/src/__tests__/workflow-settings-e2e.test.ts` (new), `docs/` user-facing settings/workflow docs, `CONCEPTS.md` +- **Approach:** One end-to-end suite that runs the canonical journey: pre-migration project with customized moved keys → migration → engine run consuming identical effective values → value edited via panel/tool → engine run consuming the new value → export v2 → wipe → import → same effective values. Update user docs for "where did my setting go" and the workflow-settings authoring story; CONCEPTS.md gains the Workflow Setting / Effective Settings vocabulary. +- **Test scenarios:** + - The full journey above as a single deterministic test (in-memory store, fake timers, no real polling). + - Surface enumeration check (FN-5893 discipline): engine, dashboard modal, workflow editor, CLI, agent tools, export/import, sync — each surface has at least one assertion touching workflow settings. +- **Verification:** Full relevant suites green via `pnpm test` (scoped packages); docs reviewed. + +--- + +## Risks & Dependencies + +- **Default re-injection trap (highest severity).** If schema removal and migration ever separate, saved defaults silently overwrite migrated values. Mitigated by single-commit rule (U4) and the dedicated regression test. +- **Concurrent tracks.** The step-inversion plan (2026-06-04-001) also touches built-in IRs, `SCHEMA_VERSION`, and the workflow editor. Coordinate schema-version numbering and built-in IR edits; whichever lands second rebases the version bump and re-runs the literal sweep. +- **Long-lived branch vs `main` settings changes.** The SettingsModal rebuild collides with any concurrent settings semantics change. Mitigation: section-by-section slices (U9 execution note), union test suite on conflict. +- **Multi-node fleets mid-migration.** Nodes migrate independently; the tombstone sync filter prevents cross-contamination, but workflow setting values diverge across nodes until the sync follow-up ships. Surfaced in the sync UI (KTD-8); accepted for this round. +- **Engine `vi.mock("@fusion/core")` drift.** New core exports (settings types/resolver) break hand-written core mocks in CI shards; sweep mocks when adding exports. +- **Moved-key catalog disputes.** If implementation reveals a key with readers outside per-task execution (e.g. a scheduler reading `maxParallelSteps` outside task scope), the key stays put and the catalog shrinks — the tombstone list is the single place to amend, and the consistency test enforces coherence. + +--- + +## Sources & Research + +- Fields pattern (the template): `packages/core/src/workflow-ir-types.ts:65-98`, `packages/core/src/workflow-ir.ts:659-727`, `packages/core/src/task-fields.ts`, `packages/dashboard/app/components/WorkflowFieldsPanel.tsx`. +- Settings stack: `packages/core/src/settings-schema.ts` (DEFAULT objects + derived key lists), `packages/core/src/global-settings.ts:140-211` (schema protection + default re-injection), `packages/core/src/settings-export.ts`, `packages/dashboard/src/routes/register-settings-sync-routes.ts:15-33`. +- Engine read sites: `packages/engine/src/executor.ts:2149, 5154, 9974` (model-lane hierarchy at `executor.ts:5755-5770`, `resolveExecutorSessionModel`), `packages/engine/src/step-session-executor.ts:671`. +- Upstream plans: `docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md` (trait boundary, identity posture KTD-6), `docs/plans/2026-06-04-001-feat-step-inversion-workflow-modelable-steps-plan.md` (built-in parity-oracle posture, schema-sweep convention). +- Institutional learnings: `docs/solutions/integration-issues/bundled-plugin-registration-drift.md` (consistency test), `docs/solutions/ui-bugs/css-animation-frozen-by-transition-token-shape-mismatch.md` (token shape contract), `docs/solutions/architecture-patterns/i18n-foundation-vite-ink-monorepo-code-split-catalogs.md` (three-tier pattern, core-mock drift), `docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.md` (long-branch hazard), `docs/solutions/logic-errors/queued-chat-message-flush-trusts-stale-isgenerating.md` (fresh-read gating for the migration). From 04d5e66d24fa328d1772e80f5325aa6256be66f3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 22:00:04 -0700 Subject: [PATCH 02/20] feat(core): workflow IR typed settings declarations with built-in moved-key catalog --- .../__tests__/workflow-ir-settings.test.ts | 229 +++++++++++++++ .../core/src/builtin-coding-workflow-ir.ts | 4 + .../builtin-stepwise-coding-workflow-ir.ts | 3 + .../core/src/builtin-workflow-settings.ts | 266 ++++++++++++++++++ packages/core/src/index.ts | 8 + packages/core/src/workflow-ir-types.ts | 45 +++ packages/core/src/workflow-ir.ts | 166 ++++++++++- 7 files changed, 719 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/__tests__/workflow-ir-settings.test.ts create mode 100644 packages/core/src/builtin-workflow-settings.ts diff --git a/packages/core/src/__tests__/workflow-ir-settings.test.ts b/packages/core/src/__tests__/workflow-ir-settings.test.ts new file mode 100644 index 0000000000..a3ce1c3692 --- /dev/null +++ b/packages/core/src/__tests__/workflow-ir-settings.test.ts @@ -0,0 +1,229 @@ +import { describe, expect, it } from "vitest"; +import { + parseWorkflowIr, + serializeWorkflowIr, + downgradeIrToV1IfPure, + WorkflowIrError, +} from "../workflow-ir.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import { BUILTIN_WORKFLOW_SETTINGS } from "../builtin-workflow-settings.js"; +import { DEFAULT_PROJECT_SETTINGS } from "../types.js"; +import type { + WorkflowIrV2, + WorkflowIrNode, + WorkflowSettingDefinition, +} from "../workflow-ir-types.js"; + +const startEnd: WorkflowIrNode[] = [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, +]; + +function withSettings(settings: WorkflowSettingDefinition[]): WorkflowIrV2 { + return { + version: "v2", + name: "test", + columns: [], + nodes: startEnd, + edges: [{ from: "start", to: "end" }], + settings, + }; +} + +describe("parseWorkflowIr — workflow settings declarations (U1)", () => { + it("parses and round-trips a valid declaration of each type", () => { + const settings: WorkflowSettingDefinition[] = [ + { id: "s-string", name: "S", type: "string", default: "x" }, + { id: "s-text", name: "T", type: "text", default: "long" }, + { id: "s-number", name: "N", type: "number", default: 42 }, + { id: "s-boolean", name: "B", type: "boolean", default: true }, + { + id: "s-enum", + name: "E", + type: "enum", + default: "a", + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + }, + { + id: "s-multi", + name: "M", + type: "multi-enum", + default: ["a"], + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + render: { widget: "chips" }, + }, + ]; + const parsed = parseWorkflowIr(withSettings(settings)) as WorkflowIrV2; + expect(parsed.settings).toEqual(settings); + const reparsed = parseWorkflowIr(serializeWorkflowIr(parsed)); + expect(reparsed).toEqual(parsed); + }); + + it("allows a declaration with no default and a description", () => { + const parsed = parseWorkflowIr( + withSettings([ + { id: "lane", name: "Lane", type: "string", description: "a model lane" }, + ]), + ) as WorkflowIrV2; + expect(parsed.settings?.[0].default).toBeUndefined(); + }); + + it("rejects duplicate setting ids", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { id: "dup", name: "A", type: "string" }, + { id: "dup", name: "B", type: "string" }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects an empty id", () => { + expect(() => + parseWorkflowIr(withSettings([{ id: "", name: "A", type: "string" }])), + ).toThrow(WorkflowIrError); + }); + + it("rejects an unknown type", () => { + expect(() => + parseWorkflowIr( + withSettings([{ id: "x", name: "A", type: "date" as never }]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects an enum without options", () => { + expect(() => + parseWorkflowIr(withSettings([{ id: "x", name: "A", type: "enum" }])), + ).toThrow(WorkflowIrError); + }); + + it("rejects options on a non-enum type", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { id: "x", name: "A", type: "number", options: [{ value: "a", label: "A" }] }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects duplicate option values", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { + id: "x", + name: "A", + type: "enum", + options: [ + { value: "a", label: "A" }, + { value: "a", label: "A2" }, + ], + }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects a disallowed render widget", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { id: "x", name: "A", type: "string", render: { widget: "slider" as never } }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects a default violating its own type (number with string)", () => { + expect(() => + parseWorkflowIr( + withSettings([{ id: "x", name: "A", type: "number", default: "x" }]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects a default violating boolean type", () => { + expect(() => + parseWorkflowIr( + withSettings([{ id: "x", name: "A", type: "boolean", default: "true" }]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects an enum default not among options", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { + id: "x", + name: "A", + type: "enum", + default: "c", + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("rejects a multi-enum default containing an unknown option", () => { + expect(() => + parseWorkflowIr( + withSettings([ + { + id: "x", + name: "A", + type: "multi-enum", + default: ["a", "c"], + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + }, + ]), + ), + ).toThrow(WorkflowIrError); + }); + + it("does not downgrade an IR with settings present to v1", () => { + const parsed = parseWorkflowIr( + withSettings([{ id: "x", name: "A", type: "string", default: "v" }]), + ); + const down = downgradeIrToV1IfPure(parsed); + expect(down.version).toBe("v2"); + }); +}); + +describe("built-in workflow settings parity anchor (U1, R4)", () => { + it("the built-in coding workflow declares the full moved-key catalog", () => { + const builtin = BUILTIN_CODING_WORKFLOW_IR as WorkflowIrV2; + const declaredIds = new Set((builtin.settings ?? []).map((s) => s.id)); + for (const setting of BUILTIN_WORKFLOW_SETTINGS) { + expect(declaredIds.has(setting.id)).toBe(true); + } + expect(builtin.settings).toEqual(BUILTIN_WORKFLOW_SETTINGS); + }); + + it("each declaration default strictly equals the legacy DEFAULT_PROJECT_SETTINGS literal", () => { + const legacy = DEFAULT_PROJECT_SETTINGS as Record; + for (const setting of BUILTIN_WORKFLOW_SETTINGS) { + // Catalog keys must exist as a known project-settings key. + expect(Object.prototype.hasOwnProperty.call(legacy, setting.id)).toBe(true); + // A declared default must byte-equal the legacy literal; an omitted + // default corresponds to a legacy `undefined` literal. + expect(setting.default).toStrictEqual(legacy[setting.id]); + } + }); +}); diff --git a/packages/core/src/builtin-coding-workflow-ir.ts b/packages/core/src/builtin-coding-workflow-ir.ts index 381d78156e..3ae675ed8d 100644 --- a/packages/core/src/builtin-coding-workflow-ir.ts +++ b/packages/core/src/builtin-coding-workflow-ir.ts @@ -1,5 +1,6 @@ import type { WorkflowIr } from "./workflow-ir-types.js"; import { parseWorkflowIr } from "./workflow-ir.js"; +import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; /** * The built-in default workflow as a v2 IR. Its six columns have ids that are @@ -59,6 +60,9 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = { { from: "review", to: "end", condition: "failure" }, { from: "merge", to: "end", condition: "failure" }, ], + // Workflow-settings (U1, R4): declare the full moved-key catalog with defaults + // byte-equal to today's DEFAULT_PROJECT_SETTINGS literals. Inert until U3. + settings: BUILTIN_WORKFLOW_SETTINGS, }; export const BUILTIN_CODING_WORKFLOW_IR = parseWorkflowIr(RAW_BUILTIN_CODING_WORKFLOW_IR); diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 150e47f96e..179ae134f5 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -1,5 +1,6 @@ import type { WorkflowIr } from "./workflow-ir-types.js"; import { parseWorkflowIr } from "./workflow-ir.js"; +import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; /** * The built-in **stepwise** coding workflow (KTD-9) — the demonstration of step @@ -144,6 +145,8 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "merge", to: "end", condition: "success" }, { from: "merge", to: "end", condition: "failure" }, ], + // Workflow-settings (U1, R4): same moved-key catalog as the default builtin. + settings: BUILTIN_WORKFLOW_SETTINGS, }; export const BUILTIN_STEPWISE_CODING_WORKFLOW_IR = parseWorkflowIr( diff --git a/packages/core/src/builtin-workflow-settings.ts b/packages/core/src/builtin-workflow-settings.ts new file mode 100644 index 0000000000..7f3cce9676 --- /dev/null +++ b/packages/core/src/builtin-workflow-settings.ts @@ -0,0 +1,266 @@ +import type { WorkflowSettingDefinition } from "./workflow-ir-types.js"; + +/** + * The moved-key catalog declared as workflow settings (U1, R4). + * + * Single source of truth, imported by both built-in workflow IR files + * (`builtin-coding-workflow-ir.ts`, `builtin-stepwise-coding-workflow-ir.ts`) so + * the catalog has exactly one definition. + * + * Each `default` here MUST be byte-equal to the corresponding literal in + * `DEFAULT_PROJECT_SETTINGS` (`settings-schema.ts`) — this is the parity anchor + * for the U4 hard-move migration. The U1 test + * (`workflow-ir-settings.test.ts`) asserts strict equality against the legacy + * literals. Keys with `undefined` legacy defaults (the per-phase model lanes) + * omit `default` entirely, which round-trips to the same effective value. + * + * NOTE: these declarations are inert in U1 — nothing reads them until the + * effective-settings resolver and engine integration land (U3). Adding them does + * not change any built-in workflow's behavior. + * + * Keys deliberately NOT in this catalog (per KTD-4 / the catalog-shrink rule): + * - `completionDocumentationMode` — read outside per-task scope (triage), stays + * in project settings. + * - merge-cluster keys + `maxConcurrent` — owned by the columns/traits track. + */ +export const BUILTIN_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [ + // ── Step execution ───────────────────────────────────────────────────── + { + id: "workflowStepTimeoutMs", + name: "Step timeout (ms)", + type: "number", + default: 360_000, + description: "Maximum time a single workflow step may run before it is timed out.", + }, + { + id: "workflowStepScopeEnforcement", + name: "Step scope enforcement", + type: "enum", + default: "block", + options: [ + { value: "block", label: "Block" }, + { value: "warn", label: "Warn" }, + { value: "off", label: "Off" }, + ], + description: "How to handle a step that writes outside its declared file scope.", + }, + { + id: "planOnlyScopeLeakEnforcement", + name: "Plan-only scope leak enforcement", + type: "enum", + default: "warn", + options: [ + { value: "off", label: "Off" }, + { value: "warn", label: "Warn" }, + { value: "block", label: "Block" }, + ], + description: "How to handle code changes during a plan-only step.", + }, + { + id: "workflowRevisionForkOnScopeMismatch", + name: "Fork workflow revision on scope mismatch", + type: "boolean", + default: true, + description: "Fork a new workflow revision when a step's actual scope diverges from its plan.", + }, + { + id: "strictScopeEnforcement", + name: "Strict scope enforcement", + type: "boolean", + default: false, + description: "Enforce declared step scope strictly, rejecting any out-of-scope change.", + }, + { + id: "runStepsInNewSessions", + name: "Run steps in new sessions", + type: "boolean", + default: false, + description: "Run each workflow step in its own agent session instead of a shared one.", + }, + { + id: "maxParallelSteps", + name: "Max parallel steps", + type: "number", + default: 2, + description: "Maximum number of steps to run in parallel when running steps in new sessions.", + }, + { + id: "buildRetryCount", + name: "Build retry count", + type: "number", + default: 0, + description: "Number of times to retry a failing build before giving up.", + }, + { + id: "buildTimeoutMs", + name: "Build timeout (ms)", + type: "number", + default: 300_000, + description: "Maximum time a build command may run before it is timed out.", + }, + { + id: "verificationFixRetries", + name: "Verification fix retries", + type: "number", + default: 3, + description: "Number of automatic fix attempts after a failed verification.", + }, + { + id: "maxPostReviewFixes", + name: "Max post-review fixes", + type: "number", + default: 1, + description: "Maximum number of automatic fix passes after review feedback.", + }, + + // ── Review / approval ────────────────────────────────────────────────── + { + id: "requirePrApproval", + name: "Require PR approval", + type: "boolean", + default: false, + description: "Require explicit approval before a pull request can be merged.", + }, + { + id: "requirePlanApproval", + name: "Require plan approval", + type: "boolean", + default: false, + description: "Require explicit approval of the plan before execution begins.", + }, + { + id: "reviewHandoffPolicy", + name: "Review handoff policy", + type: "enum", + default: "disabled", + options: [ + { value: "disabled", label: "Disabled" }, + { value: "comment-triggered", label: "Comment-triggered" }, + { value: "always", label: "Always" }, + ], + description: "When to hand off a task to a human reviewer.", + }, + { + id: "maxReviewerContextRetries", + name: "Max reviewer context retries", + type: "number", + default: 2, + description: "Maximum reviewer retries due to insufficient context before falling back.", + }, + { + id: "maxReviewerFallbackRetries", + name: "Max reviewer fallback retries", + type: "number", + default: 2, + description: "Maximum reviewer retries on the fallback model before failing.", + }, + { + id: "reflectionEnabled", + name: "Reflection enabled", + type: "boolean", + default: false, + description: "Enable periodic reflection passes over completed work.", + }, + { + id: "reflectionIntervalMs", + name: "Reflection interval (ms)", + type: "number", + default: 3_600_000, + description: "How often to run a reflection pass when reflection is enabled.", + }, + { + id: "reflectionAfterTask", + name: "Reflect after each task", + type: "boolean", + default: true, + description: "Run a reflection pass after each task completes.", + }, + + // ── Per-phase model lanes ────────────────────────────────────────────── + // Legacy defaults are all `undefined`; `default` is omitted so resolution + // falls through to the global lane / project default (KTD-7). + { + id: "executionProvider", + name: "Execution provider", + type: "string", + description: "Provider for the execution phase. Empty falls through to the global lane.", + }, + { + id: "executionModelId", + name: "Execution model", + type: "string", + description: "Model id for the execution phase. Empty falls through to the global lane.", + }, + { + id: "planningProvider", + name: "Planning provider", + type: "string", + description: "Provider for the planning phase. Empty falls through to the global lane.", + }, + { + id: "planningModelId", + name: "Planning model", + type: "string", + description: "Model id for the planning phase. Empty falls through to the global lane.", + }, + { + id: "planningFallbackProvider", + name: "Planning fallback provider", + type: "string", + description: "Fallback provider for the planning phase.", + }, + { + id: "planningFallbackModelId", + name: "Planning fallback model", + type: "string", + description: "Fallback model id for the planning phase.", + }, + { + id: "validatorProvider", + name: "Validator provider", + type: "string", + description: "Provider for the validation phase. Empty falls through to the global lane.", + }, + { + id: "validatorModelId", + name: "Validator model", + type: "string", + description: "Model id for the validation phase. Empty falls through to the global lane.", + }, + { + id: "validatorFallbackProvider", + name: "Validator fallback provider", + type: "string", + description: "Fallback provider for the validation phase.", + }, + { + id: "validatorFallbackModelId", + name: "Validator fallback model", + type: "string", + description: "Fallback model id for the validation phase.", + }, + { + id: "titleSummarizerProvider", + name: "Title summarizer provider", + type: "string", + description: "Provider for summarizing task titles.", + }, + { + id: "titleSummarizerModelId", + name: "Title summarizer model", + type: "string", + description: "Model id for summarizing task titles.", + }, + { + id: "titleSummarizerFallbackProvider", + name: "Title summarizer fallback provider", + type: "string", + description: "Fallback provider for summarizing task titles.", + }, + { + id: "titleSummarizerFallbackModelId", + name: "Title summarizer fallback model", + type: "string", + description: "Fallback model id for summarizing task titles.", + }, +]; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 999eb211cb..4d99fe1da2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -50,6 +50,8 @@ export { serializeWorkflowIr, WorkflowIrError, DEFAULT_WORKFLOW_COLUMN_IDS, + WORKFLOW_SETTING_TYPES, + SETTING_RENDER_WIDGETS, } from "./workflow-ir.js"; export type { WorkflowIr, @@ -70,9 +72,15 @@ export type { WorkflowFieldType, WorkflowFieldOption, WorkflowFieldRender, + // Workflow-settings (U1): typed setting declaration IR types. + WorkflowSettingDefinition, + WorkflowSettingType, + WorkflowSettingOption, + WorkflowSettingRender, } 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"; +export { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; // ── Trait model (U2) ───────────────────────────────────────────────── export type { diff --git a/packages/core/src/workflow-ir-types.ts b/packages/core/src/workflow-ir-types.ts index b96e538fb5..91b78e0743 100644 --- a/packages/core/src/workflow-ir-types.ts +++ b/packages/core/src/workflow-ir-types.ts @@ -97,6 +97,48 @@ export interface WorkflowFieldDefinition { render?: WorkflowFieldRender; } +/** Workflow-settings (U1): the supported setting value types. A whitelist + * mirroring the scalar/enum subset of `WorkflowFieldType` — settings carry + * workflow-scoped policy (step timeouts, review gates, model lanes), so the + * date/url field types do not apply. */ +export type WorkflowSettingType = + | "string" + | "text" + | "number" + | "boolean" + | "enum" + | "multi-enum"; + +/** A single enum/multi-enum option for a workflow setting (mirrors + * `WorkflowFieldOption`). */ +export interface WorkflowSettingOption { + value: string; + label: string; + color?: string; +} + +/** Rendering instructions for a workflow setting (U1, KTD-1). Settings get their + * OWN render-hint type: a widget only — NO `card`/`detail` placement, which is + * task-card-specific. The widget whitelist mirrors the field render widgets. */ +export interface WorkflowSettingRender { + widget?: "select" | "radio" | "chips" | "input" | "textarea" | "toggle"; +} + +/** Workflow-settings (U1, R1, KTD-1): a workflow-declared typed setting. Clones + * the shape of `WorkflowFieldDefinition` (one level up) — declarations describe + * the schema; the per-`(workflowId, projectId)` value table (U2) carries data. + * `default` is consumed by the engine's effective-settings resolver (U3), so it + * is validated against its own type/options at parse time. */ +export interface WorkflowSettingDefinition { + id: string; + name: string; + type: WorkflowSettingType; + default?: unknown; + options?: WorkflowSettingOption[]; + description?: string; + render?: WorkflowSettingRender; +} + /** A single trait configuration applied to a column. The `trait` is an opaque * registry id (resolved by the trait registry shipped in U2); `config` carries * trait-specific options validated by that trait's schema. */ @@ -145,6 +187,9 @@ export interface WorkflowIrV2 { edges: WorkflowIrEdge[]; artifacts?: WorkflowIrArtifact[]; fields?: WorkflowFieldDefinition[]; + /** Workflow-settings (U1, R1): typed setting declarations. Additive; absent on + * legacy graphs. Values persist per-`(workflowId, projectId)` (U2), not here. */ + settings?: WorkflowSettingDefinition[]; } /** 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 6a3cc5eff8..265f22c3e7 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -10,6 +10,8 @@ import type { WorkflowForeachConfig, WorkflowFieldDefinition, WorkflowFieldType, + WorkflowSettingDefinition, + WorkflowSettingType, } from "./workflow-ir-types.js"; export class WorkflowIrError extends Error { @@ -64,6 +66,27 @@ const FIELD_RENDER_WIDGETS: ReadonlySet = new Set([ "toggle", ]); +/** Workflow-settings (U1) value-type whitelist (mirrors WORKFLOW_FIELD_TYPES). */ +export const WORKFLOW_SETTING_TYPES: ReadonlySet = new Set([ + "string", + "text", + "number", + "boolean", + "enum", + "multi-enum", +]); + +/** Workflow-settings render-widget whitelist (mirrors FIELD_RENDER_WIDGETS; + * no placement — settings have no card/detail placement). */ +export const SETTING_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; @@ -726,6 +749,139 @@ function validateFields(fields: WorkflowFieldDefinition[] | undefined): void { } } +/** Validate that a setting's `default` conforms to its own type/options (U1). + * Unlike `validateFields`, settings validate defaults because the engine's + * effective-settings resolver (U3) consumes the default directly — a malformed + * default would feed garbage into execution. */ +function validateSettingDefault(setting: WorkflowSettingDefinition): void { + const value = setting.default; + if (value === undefined) return; + const id = setting.id; + switch (setting.type) { + case "string": + case "text": + if (typeof value !== "string") { + throw new WorkflowIrError( + `Workflow setting '${id}' default must be a string for type '${setting.type}'`, + ); + } + break; + case "number": + if (typeof value !== "number" || !Number.isFinite(value)) { + throw new WorkflowIrError( + `Workflow setting '${id}' default must be a finite number`, + ); + } + break; + case "boolean": + if (typeof value !== "boolean") { + throw new WorkflowIrError(`Workflow setting '${id}' default must be a boolean`); + } + break; + case "enum": { + const allowed = new Set((setting.options ?? []).map((o) => o.value)); + if (typeof value !== "string" || !allowed.has(value)) { + throw new WorkflowIrError( + `Workflow setting '${id}' default '${String(value)}' is not one of its enum options`, + ); + } + break; + } + case "multi-enum": { + const allowed = new Set((setting.options ?? []).map((o) => o.value)); + if (!Array.isArray(value)) { + throw new WorkflowIrError( + `Workflow setting '${id}' default must be an array for type 'multi-enum'`, + ); + } + for (const entry of value) { + if (typeof entry !== "string" || !allowed.has(entry)) { + throw new WorkflowIrError( + `Workflow setting '${id}' default '${String(entry)}' is not one of its enum options`, + ); + } + } + break; + } + } +} + +/** Validate `settings` declarations (U1, R1). Mirrors `validateFields`: non-empty + * unique ids, type whitelist, options iff enum-kind, unique option values, render + * widget whitelist — plus default validation (settings need it; see + * `validateSettingDefault`). */ +function validateSettings(settings: WorkflowSettingDefinition[] | undefined): void { + if (settings === undefined) return; + if (!Array.isArray(settings)) { + throw new WorkflowIrError("Workflow IR settings must be an array"); + } + const seen = new Set(); + for (const setting of settings) { + if (!setting || typeof setting.id !== "string" || setting.id === "") { + throw new WorkflowIrError("Workflow setting must have a non-empty id"); + } + if (seen.has(setting.id)) { + throw new WorkflowIrError(`Workflow IR has duplicate setting id '${setting.id}'`); + } + seen.add(setting.id); + if (typeof setting.name !== "string" || setting.name === "") { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' must have a non-empty name`, + ); + } + if (!WORKFLOW_SETTING_TYPES.has(setting.type)) { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' has unknown type '${String(setting.type)}'`, + ); + } + const isEnum = setting.type === "enum" || setting.type === "multi-enum"; + if (isEnum) { + if (!Array.isArray(setting.options) || setting.options.length === 0) { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' of type '${setting.type}' must declare non-empty options`, + ); + } + const optSeen = new Set(); + for (const opt of setting.options) { + if (!opt || typeof opt.value !== "string" || opt.value === "") { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' option must have a non-empty value`, + ); + } + if (typeof opt.label !== "string" || opt.label === "") { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' option '${opt.value}' must have a non-empty label`, + ); + } + if (optSeen.has(opt.value)) { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' has duplicate option value '${opt.value}'`, + ); + } + optSeen.add(opt.value); + } + } else if (setting.options !== undefined) { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' of type '${setting.type}' must not declare options`, + ); + } + if (setting.description !== undefined && typeof setting.description !== "string") { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' description must be a string`, + ); + } + if (setting.render !== undefined) { + const r = setting.render; + if (r.widget !== undefined && !SETTING_RENDER_WIDGETS.has(r.widget)) { + throw new WorkflowIrError( + `Workflow setting '${setting.id}' render.widget '${String(r.widget)}' is not allowed`, + ); + } + } + validateSettingDefault(setting); + } +} + function validateColumns(ir: WorkflowIrV2): void { if (!Array.isArray(ir.columns)) { throw new WorkflowIrError("Workflow IR v2 columns must be an array"); @@ -781,6 +937,7 @@ function validateV2(ir: WorkflowIrV2): void { validateParseStepsNodes(ir); validateCodeNodes(ir.nodes); validateFields(ir.fields); + validateSettings(ir.settings); // Rework edges are legal only intra-template; any rework edge at the top level // is rejected (template rework edges are validated inside validateForeach and @@ -869,8 +1026,13 @@ 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)) { + // Step-inversion declarations (artifacts/fields) and workflow settings (U1) + // are v2-only features. + if ( + (ir.artifacts && ir.artifacts.length > 0) || + (ir.fields && ir.fields.length > 0) || + (ir.settings && ir.settings.length > 0) + ) { return ir; } From 5e52f6a3f61ce29b2074cbc9b768f06affc5e3ff Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 22:00:04 -0700 Subject: [PATCH 03/20] feat(dashboard): shared schema-driven settings UI primitives --- .../__tests__/settings-primitives.test.tsx | 249 ++++++++++++++++++ .../components/settings/SettingsFieldRow.css | 94 +++++++ .../components/settings/SettingsFieldRow.tsx | 95 +++++++ .../components/settings/SettingsNumberRow.css | 5 + .../components/settings/SettingsNumberRow.tsx | 63 +++++ .../components/settings/SettingsSection.css | 38 +++ .../components/settings/SettingsSection.tsx | 30 +++ .../components/settings/SettingsSelectRow.css | 5 + .../components/settings/SettingsSelectRow.tsx | 55 ++++ .../components/settings/SettingsTextRow.css | 5 + .../components/settings/SettingsTextRow.tsx | 51 ++++ .../settings/SettingsTextareaRow.css | 7 + .../settings/SettingsTextareaRow.tsx | 51 ++++ .../components/settings/SettingsToggleRow.css | 14 + .../components/settings/SettingsToggleRow.tsx | 51 ++++ .../app/components/settings/index.ts | 25 ++ .../app/components/settings/types.ts | 45 ++++ 17 files changed, 883 insertions(+) create mode 100644 packages/dashboard/app/__tests__/settings-primitives.test.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsFieldRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsFieldRow.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsNumberRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsNumberRow.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsSection.css create mode 100644 packages/dashboard/app/components/settings/SettingsSection.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsSelectRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsSelectRow.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsTextRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsTextRow.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsTextareaRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsTextareaRow.tsx create mode 100644 packages/dashboard/app/components/settings/SettingsToggleRow.css create mode 100644 packages/dashboard/app/components/settings/SettingsToggleRow.tsx create mode 100644 packages/dashboard/app/components/settings/index.ts create mode 100644 packages/dashboard/app/components/settings/types.ts diff --git a/packages/dashboard/app/__tests__/settings-primitives.test.tsx b/packages/dashboard/app/__tests__/settings-primitives.test.tsx new file mode 100644 index 0000000000..48c12dc0b6 --- /dev/null +++ b/packages/dashboard/app/__tests__/settings-primitives.test.tsx @@ -0,0 +1,249 @@ +// @vitest-environment jsdom +/** + * Settings UI primitives (U8 / KTD-10) — behavior + typing contract. + * + * Scope here is behavior and value typing (visual polish is verified in U9's + * browser pass): each primitive renders label/help/error, the scope badge + * renders, change events propagate with correctly-typed values (numbers not + * strings, booleans, the selected option value), and the clearable affordance + * emits the null-as-delete signal that preserves the modal's clear semantics. + */ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import * as jestDomMatchers from "@testing-library/jest-dom/matchers"; + +import { + SettingsFieldRow, + SettingsToggleRow, + SettingsNumberRow, + SettingsSelectRow, + SettingsTextRow, + SettingsTextareaRow, + SettingsSection, +} from "../components/settings"; + +expect.extend(jestDomMatchers); + +afterEach(() => cleanup()); + +describe("SettingsFieldRow", () => { + it("renders label, help, and error", () => { + render( + + + , + ); + expect(screen.getByText("Theme")).toBeInTheDocument(); + expect(screen.getByText("Pick a theme")).toBeInTheDocument(); + expect(screen.getByRole("alert")).toHaveTextContent("Required"); + }); + + it("renders a scope badge when scope is set", () => { + render( + + + , + ); + const badge = screen.getByTestId("settings-field-row-scope"); + expect(badge).toHaveTextContent("global"); + expect(badge).toHaveClass("settings-field-row-scope--global"); + }); + + it("renders no scope badge by default", () => { + render( + + + , + ); + expect(screen.queryByTestId("settings-field-row-scope")).not.toBeInTheDocument(); + }); + + it("renders the clear affordance and fires onClear when clearable", () => { + const onClear = vi.fn(); + render( + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onClear).toHaveBeenCalledTimes(1); + }); + + it("hides the clear affordance when not clearable", () => { + render( + + + , + ); + expect(screen.queryByRole("button", { name: "Reset to default" })).not.toBeInTheDocument(); + }); +}); + +describe("SettingsToggleRow", () => { + const descriptor = { key: "notify", label: "Notifications", help: "Toggle alerts" }; + + it("renders label and help and reflects value", () => { + render( {}} />); + expect(screen.getByText("Notifications")).toBeInTheDocument(); + expect(screen.getByText("Toggle alerts")).toBeInTheDocument(); + expect(screen.getByRole("checkbox")).toBeChecked(); + }); + + it("emits a boolean on change", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("checkbox")); + expect(onChange).toHaveBeenCalledWith(true); + expect(typeof onChange.mock.calls[0][0]).toBe("boolean"); + }); + + it("emits null when cleared", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onChange).toHaveBeenCalledWith(null); + }); +}); + +describe("SettingsNumberRow", () => { + const descriptor = { key: "max", label: "Max parallel", min: 1, max: 10, step: 1 }; + + it("renders label and reflects value", () => { + render( {}} />); + expect(screen.getByText("Max parallel")).toBeInTheDocument(); + expect(screen.getByRole("spinbutton")).toHaveValue(4); + }); + + it("emits a number, not a string", () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole("spinbutton"), { target: { value: "7" } }); + expect(onChange).toHaveBeenCalledWith(7); + expect(typeof onChange.mock.calls[0][0]).toBe("number"); + }); + + it("emits null when emptied", () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole("spinbutton"), { target: { value: "" } }); + expect(onChange).toHaveBeenCalledWith(null); + }); + + it("emits null when cleared", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onChange).toHaveBeenCalledWith(null); + }); + + it("shows an empty field for a null value", () => { + render( {}} />); + expect(screen.getByRole("spinbutton")).toHaveValue(null); + }); +}); + +describe("SettingsSelectRow", () => { + const descriptor = { + key: "theme", + label: "Theme", + options: [ + { value: "light", label: "Light" }, + { value: "dark", label: "Dark" }, + ], + }; + + it("renders all options", () => { + render( {}} />); + expect(screen.getByRole("option", { name: "Light" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "Dark" })).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toHaveValue("light"); + }); + + it("emits the selected value", () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole("combobox"), { target: { value: "dark" } }); + expect(onChange).toHaveBeenCalledWith("dark"); + }); + + it("emits null when cleared", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onChange).toHaveBeenCalledWith(null); + }); +}); + +describe("SettingsTextRow", () => { + const descriptor = { key: "name", label: "Display name", placeholder: "e.g. Ada" }; + + it("renders label and placeholder and reflects value", () => { + render( {}} />); + expect(screen.getByText("Display name")).toBeInTheDocument(); + const input = screen.getByRole("textbox"); + expect(input).toHaveValue("Ada"); + expect(input).toHaveAttribute("placeholder", "e.g. Ada"); + }); + + it("emits the string value", () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Grace" } }); + expect(onChange).toHaveBeenCalledWith("Grace"); + expect(typeof onChange.mock.calls[0][0]).toBe("string"); + }); + + it("emits null when cleared", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onChange).toHaveBeenCalledWith(null); + }); +}); + +describe("SettingsTextareaRow", () => { + const descriptor = { key: "notes", label: "Notes", placeholder: "Anything..." }; + + it("renders label and reflects value", () => { + render( {}} />); + expect(screen.getByText("Notes")).toBeInTheDocument(); + expect(screen.getByRole("textbox")).toHaveValue("hello"); + }); + + it("emits the string value", () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole("textbox"), { target: { value: "line1\nline2" } }); + expect(onChange).toHaveBeenCalledWith("line1\nline2"); + expect(typeof onChange.mock.calls[0][0]).toBe("string"); + }); + + it("emits null when cleared", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + expect(onChange).toHaveBeenCalledWith(null); + }); +}); + +describe("SettingsSection", () => { + it("renders title, description, and children", () => { + render( + +
content
+ , + ); + expect(screen.getByRole("heading", { name: "General" })).toBeInTheDocument(); + expect(screen.getByText("Top-level options")).toBeInTheDocument(); + expect(screen.getByTestId("child")).toBeInTheDocument(); + }); + + it("renders without a description", () => { + render( + +
content
+ , + ); + expect(screen.getByRole("heading", { name: "General" })).toBeInTheDocument(); + }); +}); diff --git a/packages/dashboard/app/components/settings/SettingsFieldRow.css b/packages/dashboard/app/components/settings/SettingsFieldRow.css new file mode 100644 index 0000000000..4ace13c67c --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsFieldRow.css @@ -0,0 +1,94 @@ +/* SettingsFieldRow (U8 / KTD-10) — base layout for a single settings control: + * label + optional scope badge on the leading line, the control slot beside or + * below it, then help text and an error band. Shared by every typed row so the + * redesigned SettingsModal and the WorkflowSettingsPanel read identically. + * Mirrors the token/class conventions of WorkflowFieldsPanel.css. */ + +.settings-field-row { + display: flex; + flex-direction: column; + gap: var(--space-xs); + padding: var(--space-sm) 0; +} + +.settings-field-row.is-disabled { + opacity: 0.6; +} + +.settings-field-row-head { + display: flex; + align-items: center; + gap: var(--space-xs); + flex-wrap: wrap; +} + +.settings-field-row-label { + font-size: 0.8rem; + font-weight: 500; + color: var(--text); +} + +.settings-field-row-scope { + font-size: 0.6rem; + text-transform: uppercase; + letter-spacing: 0.04em; + color: var(--text-muted); + background: var(--surface-muted, rgba(255, 255, 255, 0.04)); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + padding: 1px 6px; +} + +.settings-field-row-scope--global { + color: var(--accent, #7c5cbf); + border-color: var(--accent, #7c5cbf); +} + +.settings-field-row-control { + display: flex; + align-items: center; + gap: var(--space-xs); +} + +.settings-field-row-control > input:not([type="checkbox"]), +.settings-field-row-control > select, +.settings-field-row-control > textarea { + flex: 1; + min-width: 0; +} + +.settings-field-row-clear { + display: inline-flex; + align-items: center; + justify-content: center; + flex: 0 0 auto; + background: none; + border: 1px solid var(--border); + border-radius: var(--radius-sm); + color: var(--text-muted); + cursor: pointer; + padding: 2px; + transition: color var(--duration-fast) ease, border-color var(--duration-fast) ease; +} + +.settings-field-row-clear:hover:not(:disabled) { + color: var(--text); + border-color: var(--text-muted); +} + +.settings-field-row-clear:disabled { + cursor: default; + opacity: 0.5; +} + +.settings-field-row-help { + margin: 0; + font-size: 0.7rem; + color: var(--text-muted); +} + +.settings-field-row-error { + margin: 0; + font-size: 0.7rem; + color: var(--color-error, #f85149); +} diff --git a/packages/dashboard/app/components/settings/SettingsFieldRow.tsx b/packages/dashboard/app/components/settings/SettingsFieldRow.tsx new file mode 100644 index 0000000000..8b2c08e08e --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsFieldRow.tsx @@ -0,0 +1,95 @@ +/** + * SettingsFieldRow — the base layout primitive every typed settings row composes + * (U8 / KTD-10). It owns nothing about the control itself: callers pass the + * control as `children` and this row handles the surrounding chrome — label, + * scope badge (global/project), help text, error band, and an optional + * "reset to default" clear affordance. + * + * Strings are pre-translated by callers (the descriptor carries label/help), so + * this primitive hardcodes no user-facing copy. The only intrinsic string is the + * clear button's aria-label, sourced via useTranslation like neighboring + * components (e.g. WorkflowFieldsPanel). + */ +import type { ReactNode } from "react"; +import { useTranslation } from "react-i18next"; +import { RotateCcw } from "lucide-react"; +import "./SettingsFieldRow.css"; + +/** Which authority level a setting is being edited at. `undefined` renders no + * badge (the common case for a plain app/global setting). */ +export type SettingsScope = "global" | "project"; + +export interface SettingsFieldRowProps { + /** Stable id, used to associate the label with the control. */ + htmlFor?: string; + /** Pre-translated label text. */ + label: string; + /** Pre-translated help/description text rendered under the control. */ + help?: string; + /** Pre-translated validation message; renders the error band when set. */ + error?: string; + /** Scope badge to display next to the label. */ + scope?: SettingsScope; + /** Disables the clear affordance and dims the row. */ + disabled?: boolean; + /** When set, renders a clear/reset-to-default button that calls onClear. */ + clearable?: boolean; + /** Invoked when the user presses the clear affordance. */ + onClear?: () => void; + /** The control element (input/select/textarea/toggle). */ + children: ReactNode; +} + +export function SettingsFieldRow({ + htmlFor, + label, + help, + error, + scope, + disabled, + clearable, + onClear, + children, +}: SettingsFieldRowProps) { + const { t } = useTranslation("app"); + return ( +
+
+ + {scope && ( + + {scope} + + )} +
+
+ {children} + {clearable && ( + + )} +
+ {help &&

{help}

} + {error && ( +

+ {error} +

+ )} +
+ ); +} + +export default SettingsFieldRow; diff --git a/packages/dashboard/app/components/settings/SettingsNumberRow.css b/packages/dashboard/app/components/settings/SettingsNumberRow.css new file mode 100644 index 0000000000..bbb191470c --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsNumberRow.css @@ -0,0 +1,5 @@ +/* SettingsNumberRow (U8 / KTD-10) — numeric input control slot. */ + +.settings-number { + width: 100%; +} diff --git a/packages/dashboard/app/components/settings/SettingsNumberRow.tsx b/packages/dashboard/app/components/settings/SettingsNumberRow.tsx new file mode 100644 index 0000000000..e31726bb6b --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsNumberRow.tsx @@ -0,0 +1,63 @@ +/** + * SettingsNumberRow — numeric control composing SettingsFieldRow (U8 / KTD-10). + * Emits numbers (never strings) through onChange. An empty input emits null — + * the modal's null-as-delete signal — which is also what the clear affordance + * emits when `clearable` is set. + */ +import { SettingsFieldRow } from "./SettingsFieldRow"; +import type { SettingsNumberDescriptor } from "./types"; +import "./SettingsNumberRow.css"; + +export interface SettingsNumberRowProps { + descriptor: SettingsNumberDescriptor; + value: number | null; + onChange: (value: number | null) => void; + error?: string; + /** Renders a reset-to-default affordance that emits onChange(null). */ + clearable?: boolean; +} + +export function SettingsNumberRow({ + descriptor, + value, + onChange, + error, + clearable, +}: SettingsNumberRowProps) { + const { key, label, help, scope, disabled, min, max, step, placeholder } = descriptor; + return ( + onChange(null)} + > + { + const raw = e.target.value; + // Empty → null (delete). Otherwise coerce to a real number, never a + // string; ignore unparseable intermediate input. + if (raw === "") return onChange(null); + const n = Number(raw); + if (Number.isNaN(n)) return; + onChange(n); + }} + /> + + ); +} + +export default SettingsNumberRow; diff --git a/packages/dashboard/app/components/settings/SettingsSection.css b/packages/dashboard/app/components/settings/SettingsSection.css new file mode 100644 index 0000000000..d2d6ec7c17 --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsSection.css @@ -0,0 +1,38 @@ +/* SettingsSection (U8 / KTD-10) — titled grouping for settings rows. */ + +.settings-section { + display: flex; + flex-direction: column; + gap: var(--space-sm); + padding: var(--space-md) 0; + border-bottom: 1px solid var(--border); +} + +.settings-section:last-child { + border-bottom: none; +} + +.settings-section-head { + display: flex; + flex-direction: column; + gap: var(--space-xs); +} + +.settings-section-title { + margin: 0; + font-size: 0.9rem; + font-weight: 600; + color: var(--text); +} + +.settings-section-desc { + margin: 0; + font-size: 0.75rem; + color: var(--text-muted); +} + +.settings-section-body { + display: flex; + flex-direction: column; + gap: var(--space-xs); +} diff --git a/packages/dashboard/app/components/settings/SettingsSection.tsx b/packages/dashboard/app/components/settings/SettingsSection.tsx new file mode 100644 index 0000000000..c1d52b30c8 --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsSection.tsx @@ -0,0 +1,30 @@ +/** + * SettingsSection — section scaffolding for grouped settings rows (U8 / KTD-10). + * Renders a titled block with optional description and consistent spacing; the + * redesigned SettingsModal and the WorkflowSettingsPanel both group their rows + * inside one. Title/description are pre-translated by the caller. + */ +import type { ReactNode } from "react"; +import "./SettingsSection.css"; + +export interface SettingsSectionProps { + /** Pre-translated section title. */ + title: string; + /** Pre-translated section description, rendered under the title. */ + description?: string; + children: ReactNode; +} + +export function SettingsSection({ title, description, children }: SettingsSectionProps) { + return ( +
+
+

{title}

+ {description &&

{description}

} +
+
{children}
+
+ ); +} + +export default SettingsSection; diff --git a/packages/dashboard/app/components/settings/SettingsSelectRow.css b/packages/dashboard/app/components/settings/SettingsSelectRow.css new file mode 100644 index 0000000000..b7bd8513ff --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsSelectRow.css @@ -0,0 +1,5 @@ +/* SettingsSelectRow (U8 / KTD-10) — select control slot. */ + +.settings-select { + width: 100%; +} diff --git a/packages/dashboard/app/components/settings/SettingsSelectRow.tsx b/packages/dashboard/app/components/settings/SettingsSelectRow.tsx new file mode 100644 index 0000000000..05ec201861 --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsSelectRow.tsx @@ -0,0 +1,55 @@ +/** + * SettingsSelectRow — single-select control composing SettingsFieldRow + * (U8 / KTD-10). Emits the selected option's string value, or null when cleared + * (the modal's null-as-delete signal) if `clearable` is set. + */ +import { SettingsFieldRow } from "./SettingsFieldRow"; +import type { SettingsSelectDescriptor } from "./types"; +import "./SettingsSelectRow.css"; + +export interface SettingsSelectRowProps { + descriptor: SettingsSelectDescriptor; + value: string | null; + onChange: (value: string | null) => void; + error?: string; + /** Renders a reset-to-default affordance that emits onChange(null). */ + clearable?: boolean; +} + +export function SettingsSelectRow({ + descriptor, + value, + onChange, + error, + clearable, +}: SettingsSelectRowProps) { + const { key, label, help, scope, disabled, options } = descriptor; + return ( + onChange(null)} + > + + + ); +} + +export default SettingsSelectRow; diff --git a/packages/dashboard/app/components/settings/SettingsTextRow.css b/packages/dashboard/app/components/settings/SettingsTextRow.css new file mode 100644 index 0000000000..c66dba888a --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsTextRow.css @@ -0,0 +1,5 @@ +/* SettingsTextRow (U8 / KTD-10) — single-line text input control slot. */ + +.settings-text { + width: 100%; +} diff --git a/packages/dashboard/app/components/settings/SettingsTextRow.tsx b/packages/dashboard/app/components/settings/SettingsTextRow.tsx new file mode 100644 index 0000000000..c18109fda5 --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsTextRow.tsx @@ -0,0 +1,51 @@ +/** + * SettingsTextRow — single-line text control composing SettingsFieldRow + * (U8 / KTD-10). Emits the string value, or null when cleared (the modal's + * null-as-delete signal) if `clearable` is set. + */ +import { SettingsFieldRow } from "./SettingsFieldRow"; +import type { SettingsTextDescriptor } from "./types"; +import "./SettingsTextRow.css"; + +export interface SettingsTextRowProps { + descriptor: SettingsTextDescriptor; + value: string | null; + onChange: (value: string | null) => void; + error?: string; + /** Renders a reset-to-default affordance that emits onChange(null). */ + clearable?: boolean; +} + +export function SettingsTextRow({ + descriptor, + value, + onChange, + error, + clearable, +}: SettingsTextRowProps) { + const { key, label, help, scope, disabled, placeholder } = descriptor; + return ( + onChange(null)} + > + onChange(e.target.value)} + /> + + ); +} + +export default SettingsTextRow; diff --git a/packages/dashboard/app/components/settings/SettingsTextareaRow.css b/packages/dashboard/app/components/settings/SettingsTextareaRow.css new file mode 100644 index 0000000000..e27d024e9f --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsTextareaRow.css @@ -0,0 +1,7 @@ +/* SettingsTextareaRow (U8 / KTD-10) — multi-line text input control slot. */ + +.settings-textarea { + width: 100%; + resize: vertical; + font-family: inherit; +} diff --git a/packages/dashboard/app/components/settings/SettingsTextareaRow.tsx b/packages/dashboard/app/components/settings/SettingsTextareaRow.tsx new file mode 100644 index 0000000000..a90362cade --- /dev/null +++ b/packages/dashboard/app/components/settings/SettingsTextareaRow.tsx @@ -0,0 +1,51 @@ +/** + * SettingsTextareaRow — multi-line text control composing SettingsFieldRow + * (U8 / KTD-10). Emits the string value, or null when cleared (the modal's + * null-as-delete signal) if `clearable` is set. + */ +import { SettingsFieldRow } from "./SettingsFieldRow"; +import type { SettingsTextDescriptor } from "./types"; +import "./SettingsTextareaRow.css"; + +export interface SettingsTextareaRowProps { + descriptor: SettingsTextDescriptor; + value: string | null; + onChange: (value: string | null) => void; + error?: string; + /** Renders a reset-to-default affordance that emits onChange(null). */ + clearable?: boolean; +} + +export function SettingsTextareaRow({ + descriptor, + value, + onChange, + error, + clearable, +}: SettingsTextareaRowProps) { + const { key, label, help, scope, disabled, placeholder } = descriptor; + return ( + onChange(null)} + > +