diff --git a/.changeset/graph-custom-workflows.md b/.changeset/graph-custom-workflows.md index 360628d5e2..5dc3007516 100644 --- a/.changeset/graph-custom-workflows.md +++ b/.changeset/graph-custom-workflows.md @@ -8,6 +8,12 @@ Prompt nodes carry an execution profile: run on a chosen model, as a named agent CLI nodes can run arbitrary commands (not just named scripts); the first run of an exact command pauses the task for explicit user approval. The task modal's input/approval banner is interactive — reply-and-resume for user-input nodes, approve-and-run for CLI commands. -Agents reach workflows too: new `fn_workflow_list` and `fn_workflow_select` task tools give agents the same list/select capability as the dashboard picker. Built-in workflows are now read-only in the editor (palette/inspector disabled, with a "Duplicate to edit" action), and a node's "Auto-approve requests" toggle now actually bypasses the CLI first-run approval pause. +Agents reach workflows too: the `fn_workflow_list`, `fn_workflow_get`, `fn_workflow_select`, `fn_workflow_create`, `fn_workflow_update`, and `fn_workflow_delete` tools (plus `fn_trait_list` for the column vocabulary) give agents the same author/list/select capability as the dashboard. These are exposed not only to the task executor but also to the chat and planning agents, so you can author and edit workflows directly in a chat or planning conversation; a guard test locks all six tool names to each lane to prevent silent exposure drift. Built-in workflows are now read-only in the editor (palette/inspector disabled, with a "Duplicate to edit" action), and a node's "Auto-approve requests" toggle now actually bypasses the CLI first-run approval pause. Also fixes a latent persistence bug where `pausedReason` was written to the in-memory task and read by queries but never stored by the task upsert or mapped back on read — so it was lost on every reload. This silently broke any pause/resume that depends on the reason (workflow CLI-approval and await-input nodes, token-budget pauses, worktrunk failures). The approve-CLI endpoint now derives the approved command solely from the task's pausedReason (ignoring any caller-supplied command), await-input nodes only resume when this node actually paused the task (not on a pre-existing steering comment), and write-capable custom nodes are refused until a task worktree exists so they never mutate the shared repo root. + +The editor itself got a major usability upgrade: card-style nodes with kind accents and live config summaries (model/agent/skill/command, gate mode, hold release, join mode); success/failure edge authoring on regular edges with distinct styling, parallel conditioned edges, and an author-time cycle guard; one-click auto-layout that respects column swimlanes; safe node/edge deletion with cascade semantics; proper dialogs (create/delete/discard) with inline rename, descriptions, and a dirty-state guard on every dismissal path; onboarding/empty states; and the Columns and Fields panels now live in the editor's left sidebar under the workflow list. + +The node editor is now the primary workflow surface: the header and mobile nav open it directly and the legacy Workflow Steps screen is retired. Existing flat steps migrate automatically (and idempotently) on first editor open — every step becomes an insertable template fragment in the new palette Templates section (alongside built-in and plugin step templates), and your default-on steps become a "Migrated steps" workflow that's set as the project default. Task creation now picks a workflow (applied atomically at create) instead of individual step checkboxes. + +Workflows and template fragments import/export as JSON files — with server-side validation, name-collision handling, and automatic stripping of approval-bypass flags from untrusted files. And you can ask AI to design a workflow: describe what you want in the create dialog (or redesign the active workflow from the toolbar) and a planning-lane model emits a validated graph, with interpreter-only branching flagged honestly. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index 7334449c25..0000000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,69 +0,0 @@ -name: CI - -# CI auto-trigger disabled per FN-1541 — workflow preserved for manual use via workflow_dispatch -on: - workflow_dispatch: - -# FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. -env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" - -jobs: - lint: - name: Lint - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Lint - run: pnpm lint - - test-shards: - name: Test shard ${{ matrix.shard }}/3 - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [1, 2, 3] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Test (deterministic shard) - run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 3 - - build: - name: Build - runs-on: ubuntu-latest - needs: [lint, test-shards] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Install Bun - uses: oven-sh/setup-bun@v2 - - - name: Build workspace - run: pnpm build - - - name: Run CLI slow lane (opt-in suites) - run: pnpm test:slow-cli - - - name: Build standalone binary - run: pnpm --filter @runfusion/fusion build:exe - - - name: Verify binary exists - run: test -f packages/cli/dist/fn diff --git a/.github/workflows/full-suite.yml b/.github/workflows/full-suite.yml new file mode 100644 index 0000000000..1859ef490e --- /dev/null +++ b/.github/workflows/full-suite.yml @@ -0,0 +1,176 @@ +name: Full Suite (non-blocking) + +# The demoted test tier (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md). +# Runs the full sharded suite, the engine slow tier, and the dashboard +# inventory guard on every push to main — post-merge signal only. These jobs +# are NON-BLOCKING by design: they never run on PRs and must never be added +# to branch-protection required checks. A red run here is information, not a +# merge stopper; see docs/testing.md for the quarantine ratchet that keeps +# this tier honest. + +on: + push: + branches: [main] + +# Key the concurrency group by SHA, not ref: on push to main the ref is +# always refs/heads/main, so a ref-keyed group with cancel-in-progress would +# let consecutive merges cancel each other's runs — silently skipping the +# only coverage for everything the gate dropped. Per-SHA groups never collide. +concurrency: + group: full-suite-${{ github.sha }} + cancel-in-progress: false + +# Least-privilege token: jobs only read the repo (checkout + cache) and upload +# workflow artifacts (timings), which needs no extra permission scope. +permissions: + contents: read + +# FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" + +jobs: + test-shards: + name: Test shard ${{ matrix.shard }}/4 + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shard: [1, 2, 3, 4] + + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + # Engine tests run real git operations (merge-base against main, + # case-variant ref checks) that require full history. Shallow + # clones silently break tests like worktree-acquisition's resume + # misbinding path. + fetch-depth: 0 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + # Dist-artifact cache (L1): ensureTestArtifacts otherwise rebuilds dist/ + # for 8 packages (~71s) on every shard because CI starts with no dist. + # Key on a stable, pre-build, git-based hash of ALL build packages' source + # inputs. Exact-match only — NO restore-keys: a partial/stale dist hit is + # the exact failure mode this repo has been bitten by (FN-4232/FN-4605), + # and ensureTestArtifacts still validates/rebuilds anything missing-or-stale + # after restore, so a miss is safe but a wrong-content hit would not be. + # NEVER add node_modules here (breaks Windows pnpm junctions elsewhere). + - name: Compute dist source hash + id: dist-hash + run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" + + - name: Cache built dist artifacts + id: dist-cache + uses: actions/cache@v4 + with: + path: | + packages/core/dist + packages/dashboard/dist + packages/engine/dist + packages/plugin-sdk/dist + plugins/fusion-plugin-dependency-graph/dist + plugins/fusion-plugin-hermes-runtime/dist + plugins/fusion-plugin-openclaw-runtime/dist + plugins/fusion-plugin-paperclip-runtime/dist + key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} + + # On a cache HIT, restored dist files carry their save-time mtimes while + # checkout rewrites src mtimes to "now" (src newer than dist), which would + # make ensureTestArtifacts' mtime fallback rebuild everything and defeat + # the cache. Seed the per-package content-hash cache so its content-hash + # short-circuit fires instead. ensureTestArtifacts still runs (inside + # test:ci:shard) and rebuilds anything genuinely missing/changed. + - name: Seed artifact hash-cache on cache hit + if: steps.dist-cache.outputs.cache-hit == 'true' + run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache + + - name: Test (deterministic shard) + run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4 + + # Each shard emits per-file vitest JSON timing reporter output under + # .timings/. Upload as an artifact so the timing snapshot can be + # refreshed locally/from the default branch via + # `node scripts/ci-test-shard.mjs --write-timings`. We do NOT commit the + # snapshot automatically — refresh is manual/scheduled only. + - name: Upload per-shard test timings + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-timings-shard-${{ matrix.shard }} + # Relative outputFile paths mean each package writes its own + # /.timings/ file — glob the whole tree, not just the root. + path: | + .timings/timings-*.json + packages/*/.timings/timings-*.json + plugins/*/.timings/timings-*.json + plugins/examples/*/.timings/timings-*.json + if-no-files-found: ignore + retention-days: 14 + + # The dashboard quality gate used to enumerate its test files by hand, so any + # unenumerated app/ or src/ test file ran in NO project. This guard fails when + # a dashboard test file is neither executed by a quality project (curated + + # backfill lanes) nor on the reviewed skip-list. Cheap: it only runs + # `vitest list`, not the tests. + test-inventory-guard: + name: Dashboard curated-gate guard + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + # Same dist-artifact cache as test-shards (L1): the curated-gate guard runs + # `vitest list`, whose config resolution can touch built dist, so it also + # pays the cold-dist rebuild. Exact-match key on the pre-build source hash; + # NO restore-keys (stale dist is the failure mode), NO node_modules. + - name: Compute dist source hash + id: dist-hash + run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" + + - name: Cache built dist artifacts + id: dist-cache + uses: actions/cache@v4 + with: + path: | + packages/core/dist + packages/dashboard/dist + packages/engine/dist + packages/plugin-sdk/dist + plugins/fusion-plugin-dependency-graph/dist + plugins/fusion-plugin-hermes-runtime/dist + plugins/fusion-plugin-openclaw-runtime/dist + plugins/fusion-plugin-paperclip-runtime/dist + key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} + + - name: Seed artifact hash-cache on cache hit + if: steps.dist-cache.outputs.cache-hit == 'true' + run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache + + - name: Assert every dashboard test file is gated or skip-listed + run: node scripts/check-test-inventory.mjs --dashboard-curated + + # The engine-slow tier (src/**/*.slow.test.ts) runs here with a non-empty + # execution assertion, so a glob/config drift that silently empties the tier + # fails this workflow instead of passing vacuously. Engine slow tests do real + # git operations, so a full clone (fetch-depth: 0) is required. + test-slow: + name: Engine slow tier + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + - name: Run engine-slow with non-empty-execution assertion + run: node scripts/assert-engine-slow-nonempty.mjs diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 4468206a2a..b8b75c6efd 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -1,17 +1,30 @@ name: PR Checks +# The thin trusted merge gate (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md). +# Blocking checks are exactly: Lint, Typecheck, Build, Gate. +# +# BRANCH-PROTECTION CUTOVER: required status checks are matched by job name. +# When this file changes job names, update the repo's branch-protection +# required checks to exactly [Lint, Typecheck, Build, Gate] — a stale required +# name (e.g. "Test shard 1/4") that no longer reports will block every PR +# with "Expected — waiting for status". Open PRs must rebase onto main after +# the cutover so they run this workflow shape. +# +# Everything that used to run here as shards / slow tier / inventory guard is +# non-blocking and lives in full-suite.yml (push to main). + on: pull_request: branches: [main] - # Also run on every push to main so post-merge regressions surface - # immediately instead of being discovered on the next PR. - push: - branches: [main] concurrency: group: pr-checks-${{ github.ref }} cancel-in-progress: true +# Least-privilege token: every job here only reads the repo (checkout + cache). +permissions: + contents: read + # FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" @@ -62,95 +75,18 @@ jobs: - name: Build run: pnpm build - test-shards: - name: Test shard ${{ matrix.shard }}/4 - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [1, 2, 3, 4] - - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - # Engine tests run real git operations (merge-base against main, - # case-variant ref checks) that require full history. Shallow - # clones silently break tests like worktree-acquisition's resume - # misbinding path. - fetch-depth: 0 - - - name: Setup Node.js and pnpm - uses: ./.github/actions/setup-node-pnpm - - # Dist-artifact cache (L1): ensureTestArtifacts otherwise rebuilds dist/ - # for 8 packages (~71s) on every shard because CI starts with no dist. - # Key on a stable, pre-build, git-based hash of ALL build packages' source - # inputs. Exact-match only — NO restore-keys: a partial/stale dist hit is - # the exact failure mode this repo has been bitten by (FN-4232/FN-4605), - # and ensureTestArtifacts still validates/rebuilds anything missing-or-stale - # after restore, so a miss is safe but a wrong-content hit would not be. - # NEVER add node_modules here (breaks Windows pnpm junctions elsewhere). - - name: Compute dist source hash - id: dist-hash - run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" - - - name: Cache built dist artifacts - id: dist-cache - uses: actions/cache@v4 - with: - path: | - packages/core/dist - packages/dashboard/dist - packages/engine/dist - packages/plugin-sdk/dist - plugins/fusion-plugin-dependency-graph/dist - plugins/fusion-plugin-hermes-runtime/dist - plugins/fusion-plugin-openclaw-runtime/dist - plugins/fusion-plugin-paperclip-runtime/dist - key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} - - # On a cache HIT, restored dist files carry their save-time mtimes while - # checkout rewrites src mtimes to "now" (src newer than dist), which would - # make ensureTestArtifacts' mtime fallback rebuild everything and defeat - # the cache. Seed the per-package content-hash cache so its content-hash - # short-circuit fires instead. ensureTestArtifacts still runs (inside - # test:ci:shard) and rebuilds anything genuinely missing/changed. - - name: Seed artifact hash-cache on cache hit - if: steps.dist-cache.outputs.cache-hit == 'true' - run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache - - - name: Test (deterministic shard) - run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4 - - # U1 (R4): each shard emits per-file vitest JSON timing reporter output - # under .timings/. Upload as an artifact so the timing snapshot can be - # refreshed locally/from the default branch via - # `node scripts/ci-test-shard.mjs --write-timings`. We do NOT commit the - # snapshot from PR branches — refresh is manual/scheduled only. - - name: Upload per-shard test timings - if: always() - uses: actions/upload-artifact@v4 - with: - name: test-timings-shard-${{ matrix.shard }} - # Relative outputFile paths mean each package writes its own - # /.timings/ file — glob the whole tree, not just the root. - path: | - .timings/timings-*.json - packages/*/.timings/timings-*.json - plugins/*/.timings/timings-*.json - plugins/examples/*/.timings/timings-*.json - if-no-files-found: ignore - retention-days: 14 - - # Plan U2 / R7: the dashboard quality gate used to enumerate its test files by - # hand, so any unenumerated app/ or src/ test file ran in NO project. This - # guard fails when a dashboard test file is neither executed by a quality - # project (curated + backfill lanes) nor on the reviewed skip-list. Cheap: - # it only runs `vitest list`, not the tests. - test-inventory-guard: - name: Dashboard curated-gate guard + # The only merge-blocking TEST signal (R3). Runs the boot smoke (the app + # starts and serves) plus the curated engine-core suite and the CI-shape + # test — see `test:gate` in the root package.json. Gate membership is the + # explicit allow-list in packages/engine/vitest.config.ts (engine-core + # project); a flaky gate test is evicted by removing it from that list. + gate: + name: Gate runs-on: ubuntu-latest + # The gate's value is speed; without a job timeout a hung build or + # deadlocked vitest worker blocks every PR for GitHub's default 6 hours. + # Expected runtime is ~3-5 min. + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@v4 @@ -158,10 +94,12 @@ jobs: - name: Setup Node.js and pnpm uses: ./.github/actions/setup-node-pnpm - # Same dist-artifact cache as test-shards (L1): the curated-gate guard runs - # `vitest list`, whose config resolution can touch built dist, so it also - # pays the cold-dist rebuild. Exact-match key on the pre-build source hash; - # NO restore-keys (stale dist is the failure mode), NO node_modules. + - name: Install Bun + uses: oven-sh/setup-bun@v2 + + # Dist-artifact cache (same contract as full-suite.yml): exact-match + # key only, NO restore-keys (stale dist is the known failure mode, + # FN-4232/FN-4605), NEVER node_modules (breaks Windows pnpm junctions). - name: Compute dist source hash id: dist-hash run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" @@ -185,25 +123,13 @@ jobs: if: steps.dist-cache.outputs.cache-hit == 'true' run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache - - name: Assert every dashboard test file is gated or skip-listed - run: node scripts/check-test-inventory.mjs --dashboard-curated + # Boot smoke needs the full built workspace (CLI dist is not in the + # cache list above); cached packages make this incremental-fast. + - name: Build + run: pnpm build - # Plan U2 / R8: the engine-slow tier (src/**/*.slow.test.ts) previously ran in - # NO automated gate — only via the local `test:full`. This job runs it and - # asserts a non-empty execution, so a glob/config drift that silently empties - # the tier fails CI instead of passing vacuously. Engine slow tests do real - # git operations, so a full clone (fetch-depth: 0) is required. - test-slow: - name: Engine slow tier - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - name: Boot smoke (app starts and serves) + run: node scripts/boot-smoke.mjs - - name: Setup Node.js and pnpm - uses: ./.github/actions/setup-node-pnpm - - - name: Run engine-slow with non-empty-execution assertion - run: node scripts/assert-engine-slow-nonempty.mjs + - name: Gate tests (curated engine-core + CI-shape) + run: pnpm test:gate diff --git a/AGENTS.md b/AGENTS.md index e876ae834d..52ba6b3e87 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,16 +66,27 @@ Rules: ### Testing commands -Tests are required. Typechecks/manual checks are not substitutes. +The merge gate is thin and trusted: CI blocks PRs on exactly Lint, Typecheck, Build, and Gate (boot smoke + `pnpm test:gate`). Everything else runs non-blocking in `full-suite.yml` on push to main. A red gate means a real problem; a red non-blocking run is information, not a merge stopper. Typechecks/manual checks are not substitutes for the gate. ```bash -pnpm test -pnpm test:full +pnpm test # gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite — explicit opt-in only pnpm lint pnpm build -pnpm verify:workspace +pnpm verify:workspace # deep opt-in verification (lint -> test:full -> build); NOT the merge gate ``` +### Standing Rule: Flaky Tests Are Quarantined on Sight (Deletion Ratchet) + +- A test observed failing without a corresponding real bug in the change is QUARANTINED ON SIGHT: add an entry to `scripts/lib/test-quarantine.json` (`file`, `reason` with a link to the failing run, `quarantinedAt`) AND a matching one-line `exclude` in that package's vitest config, in the same commit. +- **Agents must never appease a flaky test.** No widened timeouts, no added retries, no loosened or deleted assertions to make a flake pass. Quarantine it instead. Appeasement drains the test's signal and is how the suite rotted last time. +- A quarantined test is DELETED after 14 days (`quarantinedAt` + 2 weeks) unless rescued. Rescue requires evidence the test catches real regressions plus a root-cause fix — not stabilization passes. +- A flake INSIDE the merge gate is evicted, not skipped: remove its line from the `engine-core` allow-list in `packages/engine/vitest.config.ts` (the eviction PR does not need the flaky test to pass). +- A second quarantine in the same subsystem is a product-race smell — look at the product code before the deletion clock runs out (see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`: a flake "stabilized" three times was a real race). +- Gate admission requires evidence of value; tests never graduate into the gate by default. Mechanics: `docs/testing.md` → "Quarantine ledger and the deletion ratchet". + ### Standing Rule: Do Not Add Slow Tests (FN-5048) - Prefer narrow seams, in-memory fakes, shared harnesses, and targeted assertions. diff --git a/CONCEPTS.md b/CONCEPTS.md index 3b5065772f..ada33dde9e 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -13,6 +13,9 @@ User-level settings persisted server-side that apply across all Surfaces and all ### Three-Tier Setting The named persistence pattern for a user preference on the dashboard: a device-local cache for instant reads, a write-through to Global Settings so other Surfaces see it, and a hydrate-on-mount from the server when no local value exists. A local or in-flight user choice always wins over server hydration, and changes propagate to other open tabs. +### Translation Placeholder +An empty-string value for a catalog key in a non-English locale, marking "not yet translated." Placeholders are intentionally backfilled when keys are added; at runtime they are treated as missing (never rendered), falling back through the locale chain to English. A non-empty value — even an English one left in a non-en catalog — is rendered as-is. + ### Supported Locale A language tag in the closed set Fusion ships translations for. Any external tag (browser, environment, flag) is normalized into this set or rejected — never passed through raw. Chinese tags route by script and region so Traditional-script users are never silently served Simplified, and the two Chinese variants never collapse into a generic base tag. @@ -155,6 +158,14 @@ A first-class, workflow-defined unit of task state: an id, a display name, and a ### Trait Composable column configuration: declarative flags (e.g. `complete`, `archived`, `countsTowardWip`) plus optional lifecycle hooks (`guard`, `gate`, `onEnter`, `onExit`, `releaseCondition`). Built-in and plugin-contributed traits register through one registry. Sync `guard` hooks and the `complete`/`archived` flags are built-in-only; plugin traits get async hook points only. A column's effective flags are the merged flags of its traits; conflicting compositions are rejected at save (server-side and in the editor). +### Column agent +A permanent agent binding on a workflow-defined column — a registry agent plus a mode — staffing all session-running work attributable to that column (custom nodes, the execute seam's coding session, per-step sessions; foreach template nodes inherit the enclosing foreach's column unless they declare their own). `defer` makes the column agent the default, applying only when the work carries no own agent identity and no complete model pair; `override` supersedes node- and task-level agent/model settings wholesale. + +Requires both the workflow-columns and graph-executor flags; with either off, bindings are inert at execution time. A missing or deleted agent degrades to normal resolution without aborting a live session. Binding an agent whose permission policy is broader than the project default requires explicit confirmation at save time on every write surface. + +### Effective agent (execution principal) +The agent identity that actually runs a piece of work after column-agent precedence resolves — and the principal every identity-keyed subsystem must consult: permission gating, heartbeat serialization in both directions, resume re-dispatch, and mid-flight change detection. It may differ from the task's assigned agent under an override binding, and one task may have multiple effective agents across concurrent branch sessions. + ### Lane A horizontal row on the multi-lane board, one per workflow in use by visible cards. Each lane renders its own workflow's columns. Tasks with no workflow selection appear in the Default workflow's lane; every card appears in exactly one lane. Zero-card lanes are hidden; lanes are collapsible with persisted state. @@ -197,6 +208,20 @@ A server-owned PTY bound to a task or chat entity. It survives client disconnect ### Waiting-on-input The CLI Session state where the agent is blocked on the human (permission prompt, clarifying question), as distinct from idle-because-done. Entering it fires the notification configured on the workflow node; the task neither advances nor fails while in it. +## Testing + +### Merge Gate +The minimal set of merge-blocking PR checks: lint, typecheck, build, a Boot Smoke, and a small curated engine test suite. The gate is the only test signal that can block a PR; all other tests run non-blocking after merge. + +Gate membership is an explicit allow-list, never a glob: a test earns its slot with evidence of value and never graduates in by default. A flake inside the gate is *evicted* — its allow-list entry is removed — which deliberately requires no green run from the flaky test itself, so the gate can always be repaired while red. + +### Boot Smoke +The gate's "app starts and serves" proof: the CLI answers its help command and a real server boots on a throwaway port, answers its health endpoint, and shuts down cleanly on signal. A pass requires both that the shutdown signal was actually delivered and that the exit was clean — a crash after serving is a failed boot path, not a pass. + +### Deletion Ratchet +The standing policy for flaky tests: a test observed failing without a corresponding real bug is quarantined on sight — a dated ledger entry plus exclusion from all runs, not retried, not patched — then deleted 2 weeks later unless rescued with evidence it catches real regressions plus a root-cause fix. Appeasement (widened timeouts, added retries, loosened assertions) is prohibited, for agents especially. + +A second quarantine in the same subsystem is a product-race smell: the flake may be a real bug, so the product code gets a look before the deletion clock runs out. Gate flakes exit by Merge Gate eviction rather than quarantine, unless they should also leave the non-blocking tier. ## Flagged ambiguities diff --git a/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md b/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md new file mode 100644 index 0000000000..665d6379b5 --- /dev/null +++ b/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md @@ -0,0 +1,112 @@ +--- +date: 2026-06-04 +topic: fast-trusted-test-gate +--- + +# Fast, Trusted Test Gate + +## Summary + +Shrink the PR merge gate to a minimal trusted set — typecheck, build, boot smoke, and a small curated core-engine suite — demote everything else to non-blocking, and install a deletion ratchet: flaky tests are quarantined on sight and deleted after 2 weeks unless rescued with evidence. De-duplicate the overlapping shard runs across CI workflows. + +--- + +## Problem Frame + +PRs currently trigger multiple test shard jobs at 4–10 minutes each, and the suite appears to run in both `.github/workflows/ci.yml` (3 shards) and `.github/workflows/pr-checks.yml` (4 shards). Failures are mostly flaky/infra — timeouts, port conflicts, OOM kills — not real bugs. Local developers hit OOM running tests. + +The cost is severe: roughly 70% of shipping time goes into a loop of asking an agent to fix the failure and re-running. The agent-fix loop has a known degradation mode — agents appease flaky tests (widen timeouts, add retries, loosen assertions) rather than fix root causes, draining whatever signal the tests had. + +Against that cost, the suite's recalled value is zero: no PR test failure in recent memory caught a bug that would have actually broken users. ~3,880 test files are currently pure cost. Substantial speed infrastructure already exists on this branch (affected-only runner with content-hash caching in `scripts/test-changed.mjs`, CI sharding, a `.slow` split, isolation checks) — speed tuning alone has not fixed the problem because the core issue is trust, not throughput. + +--- + +## Key Decisions + +- **Thin trusted gate over broad coverage.** The merge gate guarantees only what the maintainer needs to merge without anxiety: typecheck, build, boot smoke, core-engine correctness. The evidence (zero recalled real catches across ~3,880 files) says breadth was not buying protection. +- **Delete over quarantine-forever.** Quarantine is a 2-week waiting room, not a retirement home. Without a deletion deadline, the demoted suite rots into a permanently red zombie that still consumes attention. +- **Policy, not machinery.** The ratchet is a written rule (in `AGENTS.md` and contributor docs), not new test infrastructure. This repo's history shows a pattern of answering test pain with more test machinery (sharding, isolation checks, lock runners, kill guards); this change deliberately breaks that pattern. +- **Agents are banned from "fixing" flaky tests.** When an agent encounters a flaky test, the correct action is quarantine, never appeasement. This stops the suite-weakening loop. +- **Reuse existing infrastructure.** `scripts/test-changed.mjs`, the `.slow` split, and shard timing artifacts stay. The change is what blocks merges, not how tests run. + +--- + +## Requirements + +**Merge gate** + +- R1. The PR merge gate consists of: typecheck, build, a boot smoke check (the app starts and serves), and a curated core-engine test suite. +- R2. The curated core-engine suite runs as a single CI job targeting under ~3 minutes, with zero known-flaky tests admitted. +- R3. The gate is the only merge-blocking test signal; no other test job can block a PR. +- R4. If no suitable boot smoke check exists, a single small one is created (see Dependencies). + +**CI de-duplication** + +- R5. The full suite runs at most once per PR event; the overlapping shard runs between `.github/workflows/ci.yml` and `.github/workflows/pr-checks.yml` are consolidated. +- R6. Tests outside the gate run as a non-blocking job (post-merge or scheduled); their failures never block a PR. + +**Deletion ratchet (policy)** + +- R7. Any test observed to fail without a corresponding real bug (flake) is quarantined on sight — removed from all blocking and non-blocking runs, not retried, not patched. +- R8. A quarantined test is deleted after 2 weeks unless someone rescues it with evidence that it catches real regressions; rescue requires fixing the flake at the root, not appeasing it. +- R9. The quarantine/delete rule and the agent prohibition on flaky-test appeasement (no widened timeouts, added retries, or loosened assertions to make a flake pass) are written into `AGENTS.md`. +- R10. Admission to the blocking gate requires evidence of value; tests do not graduate into the gate by default. + +**Local development** + +- R11. The default local test command (`pnpm test`) runs the gate suite, sized so it cannot OOM a typical dev machine. +- R12. Running anything larger locally is opt-in via explicitly named commands. + +--- + +## Acceptance Examples + +- AE1. **Covers R7, R8.** + - **Given:** a test fails on a PR, and the failure does not correspond to a bug in the change. + - **When:** a maintainer or agent triages it. + - **Then:** the test is quarantined (skipped everywhere) in that same PR or a follow-up, with a dated marker; 2 weeks later it is deleted unless rescued with evidence. +- AE2. **Covers R3, R6.** + - **Given:** a non-gate test fails in the non-blocking run. + - **When:** a PR is open. + - **Then:** the PR's mergeability is unaffected; the failure surfaces as information only. +- AE3. **Covers R9.** + - **Given:** an agent is asked to deal with a red flaky test. + - **When:** it consults `AGENTS.md`. + - **Then:** it quarantines the test rather than widening timeouts, adding retries, or weakening assertions. + +--- + +## Success Criteria + +- PR wall-clock for required checks drops from tens of minutes to under ~5 minutes. +- Maintainer time spent on the fix-and-rerun loop drops from ~70% to near zero; a red gate reliably indicates a real problem. +- No local OOMs from the default test command. +- Suite size shrinks over time via the ratchet rather than growing unboundedly. + +--- + +## Scope Boundaries + +- No new test machinery: no auto-quarantine infrastructure, no flake-scoring system, no test-value telemetry (the Approach C ratchet automation was considered and rejected as more of the machinery pattern). +- No root-cause fixing of existing flaky tests as part of this work — flakes exit via quarantine and deletion. +- Healthy non-gate tests survive indefinitely in the non-blocking run; this work does not mass-delete tests that aren't flaky. +- Coverage targets and coverage tooling are untouched. + +--- + +## Dependencies / Assumptions + +- **Unverified:** whether a usable boot smoke test already exists. If not, R4 creates one — kept deliberately small. +- **Unverified:** the exact trigger overlap between `ci.yml` and `pr-checks.yml` shard jobs on PR events; confirmed shard matrices exist in both, but trigger conditions need checking during planning. +- The existing `.slow` split and `scripts/test-changed.mjs` remain the mechanism for selecting and running tests; this work changes gating, not the runner. +- Branch protection / required-checks settings are adjustable to match the new gate. + +--- + +## Outstanding Questions + +**Deferred to planning** + +- Which engine tests make the curated gate suite (selection criteria: deterministic, fast, covering core orchestration logic the maintainer actually relies on). +- Quarantine mechanics: skip annotation vs. exclusion list vs. moving files — whichever is cheapest with the existing runner. +- Where the non-blocking run lives (post-merge on main vs. scheduled) and how its results surface without demanding attention. diff --git a/docs/contributing.md b/docs/contributing.md index 78c21adf59..f97cec31a2 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -63,11 +63,13 @@ FUSION_DEV_PREBUILD=full pnpm dev dashboard # production-like full workspace pr pnpm dev:ui # dashboard dev server only pnpm dev:hmr # dashboard API + Vite HMR UI, with no startup prebuild pnpm lint # lint all packages -pnpm test # changed-only workspace tests (falls back to full suite in safety contexts) -pnpm test:full # full workspace quality gate (clean-worktree compatible) +pnpm test # merge-gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite (explicit opt-in; clean-worktree compatible) pnpm build # workspace builds (excludes desktop/mobile) pnpm build:all # full workspace build (includes desktop/mobile) -pnpm verify:workspace # canonical lint -> test -> build verification gate +pnpm verify:workspace # deep opt-in verification: lint -> test:full -> build pnpm typecheck # workspace typechecks ``` @@ -80,20 +82,21 @@ Fusion codifies workspace verification as a deterministic contract: - Root test entrypoints (`pnpm test` via `scripts/test-changed.mjs` and `pnpm test:ci:shard` via `scripts/ci-test-shard.mjs`) call `scripts/ensure-test-artifacts.mjs`, which deterministically builds only missing/stale required workspace dist artifacts (`@fusion/core`, `@fusion/dashboard`, `@fusion/engine`, `@fusion/plugin-sdk`, and `@fusion-plugin-examples/{dependency-graph,hermes-runtime,openclaw-runtime,paperclip-runtime}`). - Package-scoped hooks now mirror this bootstrap for fresh worktrees where needed: `@fusion/dashboard` and `@fusion-plugin-examples/dependency-graph` run `pretest: node ../../scripts/ensure-test-artifacts.mjs`. - This includes clean states where those required dist directories are absent. -- `pnpm verify:workspace` is the canonical pre-merge gate and runs in strict order: +- `pnpm test:gate` is the merge gate: the curated `engine-core` suite plus the CI-shape test. CI blocks PRs on exactly Lint, Typecheck, Build, and Gate (boot smoke + `pnpm test:gate`) — see `.github/workflows/pr-checks.yml` and docs/testing.md. +- `pnpm verify:workspace` is the deep opt-in verification (not the merge gate) and runs in strict order: 1. `pnpm lint` 2. `pnpm test:full` 3. `pnpm build` -GitHub Actions now runs deterministic test sharding via `pnpm test:ci:shard --shard --total ` in both PR checks and manual CI, while keeping local semantics unchanged: +GitHub Actions runs deterministic test sharding via `pnpm test:ci:shard --shard --total ` in the non-blocking `full-suite.yml` workflow (push to main only — never a PR gate), while keeping local semantics unchanged: -- `pnpm test` remains changed-only local iteration. -- `pnpm test:full` remains the canonical workspace quality gate; dashboard exhaustive coverage is explicit via `pnpm --filter @fusion/dashboard test:deep`. -- `pnpm verify:workspace` remains the canonical local lint -> test -> build gate. +- `pnpm test` remains gate + changed-only local iteration. +- `pnpm test:full` remains the explicit full workspace suite; dashboard exhaustive coverage is explicit via `pnpm --filter @fusion/dashboard test:deep`. +- `pnpm verify:workspace` remains the deep opt-in lint -> test -> build verification. `test:ci:shard` is a CI-focused entrypoint (`scripts/ci-test-shard.mjs`) that deterministically balances workspace packages with `test` scripts by counting package-local `**/__tests__/**/*.test.{ts,tsx,mjs}` files, auto-splitting oversized packages into virtual shard entries (`{ name, shardIndex, shardCount }`), then assigning entries in descending weight order with best-fit placement for unsplit entries (closest under-budget fit, otherwise minimum overshoot) while keeping slices of the same package on different shards when possible. Whole entries run as grouped `pnpm --filter test` calls, and virtual entries run one-by-one via `pnpm --filter test -- --shard /`. This keeps coverage reproducible while improving shard balance. -`pnpm test` now uses a changed-only entrypoint (`scripts/test-changed.mjs`) for faster local iteration. It resolves the comparison base from `.changeset/config.json` (`baseBranch`) and runs only affected workspaces from `pnpm-workspace.yaml` (both `packages/*` and `plugins/**`) using safe package-first filtering (`pnpm --filter test`). It automatically falls back to the full suite when the run is forced (CI / `--full`), the git comparison base or diff cannot be resolved, no changes are detected, shared/root test infrastructure changes, or changed workspace paths cannot be resolved to a workspace package (fail-safe coverage behavior). +`pnpm test` now uses a changed-only entrypoint (`scripts/test-changed.mjs`) for faster local iteration. It resolves the comparison base from `.changeset/config.json` (`baseBranch`) and runs only affected workspaces from `pnpm-workspace.yaml` (both `packages/*` and `plugins/**`) using safe package-first filtering (`pnpm --filter test`). It runs the merge-gate suite first, then the affected set. The full suite runs only on explicit opt-in (`--full` / `pnpm test:full`); shared-infrastructure changes and unresolvable diffs widen the affected set but never escalate to an implicit full-suite run (the old escalation was the local OOM path). Root test entrypoints (`pnpm test`, `pnpm test:full`, and `pnpm test:ci:shard`) now use a shared CPU-aware default worker budget instead of fixed low values. By default, Fusion sets `FUSION_TEST_TOTAL_WORKERS` to `max(4, min(12, cpuCount - 1))` and `FUSION_TEST_CONCURRENCY` to `2` (clamped to the total budget), while still honoring explicit overrides from `VITEST_MAX_WORKERS`, `FUSION_TEST_TOTAL_WORKERS`, and `FUSION_TEST_CONCURRENCY`. @@ -113,7 +116,8 @@ If you add or change test entrypoints, keep this isolation guard path intact and Before submitting changes, verify: -- [ ] `pnpm verify:workspace` — canonical lint → test → build gate +- [ ] `pnpm test:gate` — the merge gate (curated engine-core suite + CI-shape test) +- [ ] `pnpm verify:workspace` — deep opt-in lint → test:full → build verification - [ ] `pnpm typecheck` — type checking passes ## Realtime/SSE change note @@ -160,7 +164,7 @@ pnpm build:exe:all # build multi-target executables Default workspace verification stays lean and deterministic: - `pnpm test` runs the standard suite and does **not** require Bun cross-build integration tests. -- `pnpm verify:workspace` remains the canonical `lint -> test -> build` gate. +- `pnpm verify:workspace` remains the deep opt-in `lint -> test -> build` verification. Slow/pre-release CLI coverage is explicit and opt-in: diff --git a/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md new file mode 100644 index 0000000000..f89647b905 --- /dev/null +++ b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md @@ -0,0 +1,211 @@ +--- + +title: "refactor: Shrink PR merge gate to a fast trusted set with a deletion ratchet" +type: refactor +status: completed +date: 2026-06-04 +origin: docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md + +--- + +# refactor: Shrink PR merge gate to a fast trusted set with a deletion ratchet + +## Summary + +Replace the current 9-runner PR gate (lint, typecheck, build, 4 test shards, dashboard curated-gate guard, engine slow tier) with a thin trusted gate — 4 blocking checks: lint, typecheck, build, and a gate job combining boot smoke + the curated `engine-core` suite, with the gate job's test run under ~1 minute. (Lint is preserved from the existing gate as status quo; origin R1 does not name it.) Everything else moves to a non-blocking workflow on push to main. A quarantine ledger plus 2-week deletion ratchet (written policy, minimal mechanics) keeps flaky tests from re-accumulating, and local `pnpm test` is re-defaulted so developers cannot OOM. + +--- + +## Problem Frame + +PR failures are mostly flaky/infra and consume ~70% of maintainer shipping time in an agent-fix-and-rerun loop; no recalled PR test failure caught a real user-facing bug (see origin: docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md). Research corrected one origin assumption: there is **no double shard run** — `.github/workflows/ci.yml` is trigger-disabled (`workflow_dispatch` only, per FN-1541). The live gate is `.github/workflows/pr-checks.yml` alone. The local OOM path is also pinned: `shouldForceFullSuite` in `scripts/test-changed.mjs` (~line 431) escalates almost any shared-file change to a full-suite run at `--workspace-concurrency=2`, with dashboard lanes requesting 6GB heaps. + +--- + +## Key Technical Decisions + +- **The gate gets a dedicated command (`pnpm test:gate`); CI never invokes `pnpm test`.** `scripts/test-changed.mjs` hard-forces the full suite when `CI === "true"` (~line 1064), so any gate job calling `pnpm test` silently expands to everything. A dedicated command is the only way the ~1 min target is structurally guaranteed. +- **Gate membership is an explicit allow-list, not a glob.** A new `engine-core` vitest project in `packages/engine/vitest.config.ts` with an enumerated include list (precedent: the existing `engine-default`/`engine-reliability`/`engine-slow` project split). Recorded membership makes R10 (tests earn their way in) enforceable, and lets a flaky gate test be evicted by editing the list — no need for the flaky test itself to pass (resolves the chicken-and-egg eviction problem). +- **The non-blocking tier is a separate workflow file, not jobs inside `pr-checks.yml`.** A red full-suite run must not paint the gate's workflow red, or "red means real" dies on day one. New `full-suite.yml` triggered on push to main carries the 4 shards, engine slow tier, and inventory guard. +- **`ci.yml` is deleted and `packages/cli/src/__tests__/ci-workflow.test.ts` is rewritten in the same unit.** That test hard-asserts the current CI shape (ci.yml exists, 3-shard matrix, pr-checks 4-shard matrix, `docs/contributing.md` gate wording). Deleting the workflow without rewriting the test makes the change self-inconsistent. The rewritten test guards the *new* gate shape and is admitted to the gate suite — a test that guards the gate's own shape earns blocking status. +- **Quarantine ledger is a dated JSON file modeled on `scripts/lib/dashboard-curated-skiplist.json`, with `check-test-inventory.mjs --diff` left unwired.** The dashboard skiplist (entries with mandatory `reason`, shared between a guard script and vitest excludes) is the proven template; the ledger adds `quarantinedAt` so the 2-week clock is computable. `--diff` would fail CI on any test deletion — the exact opposite of the ratchet — so it stays unwired, documented as a deliberate exemption. +- **Quarantine stays on-sight (origin decision), with a product-race escalation note in the policy.** Institutional learning (`docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`) documents a flake "stabilized" three times that was a real product race. The policy keeps on-sight quarantine but states: a second quarantine in the same subsystem is a product-race smell worth a look before the deletion clock runs out. No triage gate, no machinery. +- **Local `pnpm test` = gate suite + affected-package tests, with the force-full fallback removed.** Keeps the value of changed-code coverage (the affected-package expansion in `test-changed.mjs` already works) while deleting the OOM path: `shouldForceFullSuite` no longer escalates to a full recursive run locally; shared-infra changes run the gate suite plus a bounded affected set instead. `pnpm test:full` remains the explicit opt-in full suite. + +--- + +## High-Level Technical Design + +CI topology before → after: + +```mermaid +flowchart TB + subgraph before [Before: pr-checks.yml on PR] + L1[lint] ~~~ T1[typecheck] ~~~ B1[build] + S1[test shard 1/4] ~~~ S2[shard 2/4] ~~~ S3[shard 3/4] ~~~ S4[shard 4/4] + G1[dashboard curated guard] ~~~ E1[engine slow tier] + end + subgraph after_gate [After: pr-checks.yml on PR — blocking] + L2[lint] ~~~ T2[typecheck] ~~~ B2[build] + GATE[gate: boot smoke + curated engine-core suite + CI-shape test] + end + subgraph after_full [After: full-suite.yml on push to main — non-blocking] + S5[test shards 1..4] ~~~ E2[engine slow tier] ~~~ G2[inventory guard] + end + before -->|this plan| after_gate + before -->|this plan| after_full +``` + +Test lifecycle state machine (where each state is recorded): + +```mermaid +stateDiagram-v2 + [*] --> NonBlocking: new test (default) + NonBlocking --> Gate: evidence of value, added to engine-core allow-list + Gate --> NonBlocking: evicted (flaked in gate, removed from allow-list) + NonBlocking --> Quarantined: flaked, ledger entry with reason + quarantinedAt + Gate --> Quarantined: flake confirmed not gate-worthy + Quarantined --> NonBlocking: rescued with evidence + root-cause fix + Quarantined --> Deleted: 2 weeks elapsed, no rescue + Deleted --> [*] +``` + +State recording: **Gate** = presence in the `engine-core` include list (or gate job steps); **Quarantined** = entry in `scripts/lib/test-quarantine.json`; **NonBlocking** = default for everything else; **Deleted** = git history. + +--- + +## Requirements + +Carried from origin (R-IDs below are the origin's; see origin doc for full text). + +**Merge gate** + +- R1–R3. Gate = typecheck, build, boot smoke, curated `engine-core` suite; gate job's test run ~1 min; only merge-blocking test signal. (Lint stays as a separate pre-existing blocking check — status-quo preservation, not new scope from R1.) +- R4. Boot smoke is greenfield — verified that nothing real exists (`packages/cli/src/commands/__tests__/serve.test.ts` mocks `listen`; the only artifact smoke lives in the disabled `ci.yml`). + +**CI de-duplication (revised by research)** + +- R5 (revised). No double run exists; the work is deleting dead `ci.yml` and consolidating the gate in `pr-checks.yml`. +- R6. Non-gate tests run non-blocking in `full-suite.yml` on push to main; failures never block a PR. + +**Deletion ratchet** + +- R7–R10. Quarantine on sight; delete after 2 weeks unless rescued; policy in `AGENTS.md` including the agent appeasement ban; gate admission requires evidence. + +**Local development** + +- R11–R12. `pnpm test` cannot OOM (gate + bounded affected set, no force-full); bigger runs are explicit opt-ins. + +--- + +## Implementation Units + +### U1. Curated `engine-core` gate suite and `test:gate` command + +- **Goal:** A single fast command that runs the gate's test content, locally and in CI. +- **Requirements:** R1, R2, R10. +- **Dependencies:** none. +- **Files:** `packages/engine/vitest.config.ts`, `packages/engine/package.json`, `package.json` (root), `scripts/test-timings.json` (read-only input for selection). +- **Approach:** Add an `engine-core` vitest project with an explicit include list. Selection criteria: deterministic (no module-level shared-state bleed, no real timers/network), fast (use `scripts/test-timings.json` per-file data; total budget ~60s single-threaded, leaving headroom under 1 min with boot smoke), and covering invariants the KB marks regression-prone: merge-queue trigger-gate eligibility, shared-branch-group landing/promotion idempotency, fork-point/files-changed attribution, executor/scheduler core paths. Exclude `reliability-interactions/**` and `*.slow.test.ts` outright (heaviest, single-threaded). Root script `test:gate` runs the `engine-core` project plus the rewritten CI-shape test from U3. Exact file list is execution-time discovery against timing data — the criteria above are binding, the list is not pre-enumerable here. +- **Patterns to follow:** the existing three-project split in `packages/engine/vitest.config.ts` (lines ~50–95). +- **Test scenarios:** Test expectation: none — config/selection unit; its verification is the gate run itself (below). +- **Verification:** `pnpm test:gate` passes locally in under ~1 min cold; running it 5× consecutively produces 5 green runs (flake screen); it executes only the allow-listed files (verify via vitest `list`). + +### U2. Boot smoke check + +- **Goal:** A real "the app starts and serves" check — greenfield (R4). +- **Requirements:** R1, R4. +- **Dependencies:** none. +- **Files:** `scripts/boot-smoke.mjs` (new), `package.json` (root, `test:gate` integration or separate `smoke:boot` script). +- **Approach:** Script builds nothing itself (gate job already runs after `pnpm build` artifacts exist or reuses the build job's dist cache — mirror the dist-artifact cache steps in `pr-checks.yml` lines ~86–121). It starts the dashboard server on an ephemeral port (respect `FUSION_RESERVED_PORTS`; never touch port 4040 — see the kill-guard conventions in `scripts/check-no-kill-4040.mjs`), polls an HTTP endpoint until 200 or a hard timeout (~60s), asserts the CLI binary answers `--help`, then shuts down cleanly. Exit code is the verdict. +- **Patterns to follow:** the packaged `--help` smoke in `scripts/release.mjs`; server-start handling in `packages/cli/src/commands/serve.ts` (the real one, not the mocked test). +- **Test scenarios:** + - Happy path: server starts → 200 within timeout → clean shutdown → exit 0. + - Error path: server fails to bind / crashes → nonzero exit with captured stderr. + - Error path: port already in use → picks another ephemeral port rather than failing or killing anything. +- **Verification:** `node scripts/boot-smoke.mjs` exits 0 on a built workspace and nonzero when the dashboard entry point is deliberately broken. + +### U3. Delete `ci.yml`, rewrite the CI-shape test, update CONTRIBUTING wording + +- **Goal:** Remove the dead workflow without leaving the repo self-inconsistent. +- **Requirements:** R5 (revised). +- **Dependencies:** U4 (the new pr-checks shape must be settled so the test asserts it; land in the same PR). +- **Files:** `.github/workflows/ci.yml` (delete), `packages/cli/src/__tests__/ci-workflow.test.ts` (rewrite), `docs/contributing.md`, `README.md` (command block the test pins, if affected). +- **Approach:** The test's first describe block loads `ci.yml` in `beforeAll` — deleting the workflow without removing that block crashes the entire suite at setup, not just one assertion. Delete the whole CI-workflow describe block; rewrite the PR-checks describe block (drop the 4-shard-matrix and no-pre-test-build assertions, add the new invariants); **preserve** the unrelated version.yml/release.yml/test-release.yml/code-signing describe blocks sharing the file. New invariants: `pr-checks.yml` contains the gate jobs (lint, typecheck, build, gate) and no shard matrices; `full-suite.yml` exists, triggers only on push to main, and contains the demoted jobs; `docs/contributing.md` names `pnpm test:gate` as the merge gate (the old "canonical pre-merge gate" string lives at `docs/contributing.md:83`). This test joins the gate suite (via U1's `test:gate`). +- **Patterns to follow:** the existing assertion style in `ci-workflow.test.ts` (YAML load + structural expectations). +- **Test scenarios:** (this unit IS a test) + - Asserts gate job set and absence of shard matrices in `pr-checks.yml`. + - Asserts `full-suite.yml` trigger is push-to-main only (no `pull_request`). + - Asserts `docs/contributing.md` gate wording matches the new commands. +- **Verification:** rewritten test passes against the new workflows and fails if a shard matrix is reintroduced into `pr-checks.yml`. + +### U4. Rework `pr-checks.yml` and create `full-suite.yml` + +- **Goal:** The blocking gate becomes lint + typecheck + build + gate; demoted jobs move to a separate non-blocking workflow. +- **Requirements:** R1, R2, R3, R6. +- **Dependencies:** U1, U2. +- **Files:** `.github/workflows/pr-checks.yml`, `.github/workflows/full-suite.yml` (new). +- **Approach:** `pr-checks.yml` keeps `lint`, `typecheck`, `build`, and gains a `gate` job (boot smoke + `pnpm test:gate`) reusing the dist-artifact cache; it loses `test-shards`, `test-slow`, and `test-inventory-guard`, and its `push: main` trigger (post-merge signal moves to full-suite.yml). `full-suite.yml` runs on `push: branches: [main]` with the 4-shard matrix, engine slow tier, and inventory guard moved verbatim, keeping the per-shard timing artifact upload. Keep the `pretest` guards (`check-no-nohup`, `check-no-kill-4040`) on any path that runs tests. Cutover (manual admin step, do immediately after merge): update branch protection required checks to exactly `Lint`, `Typecheck`, `Build`, `Gate` — stale names like `Test shard 1/4` left required will block every PR forever ("Expected — waiting for status"). Open PRs must rebase onto post-change main before merging. +- **Test scenarios:** covered by U3's rewritten CI-shape test (Covers AE2: a red `full-suite.yml` run does not affect PR mergeability — verify once live by observing a PR merge during a red main run). +- **Verification:** a test PR shows only the 4 gate checks, total wall-clock under ~5 min; a deliberate failure in a demoted test does not block that PR. + +### U5. Quarantine ledger + +- **Goal:** A single recorded place for quarantined tests, feeding both vitest excludes and the 2-week clock. +- **Requirements:** R7, R8. +- **Dependencies:** none. +- **Files:** `scripts/lib/test-quarantine.json` (new), vitest configs of packages that gain quarantined entries (exclude entries maintained by hand), `scripts/check-test-inventory.mjs` (doc comment only — `--diff` exemption note). +- **Approach:** Schema per entry: `{ "file": "", "reason": "", "quarantinedAt": "YYYY-MM-DD" }` — modeled on `scripts/lib/dashboard-curated-skiplist.json` but with the date the ratchet needs. **No loader module, no CLI flag** (a shared module wired into vitest configs would itself be new test machinery — the failure mode the origin rejected). Quarantining a test = add the ledger entry AND add a matching one-line `exclude` entry to that package's vitest config, by hand, in the same commit; the ledger is the dated record, the config exclude is the mechanism. The 2-week sweep is performed by whoever (human or agent) touches the suite, per policy in U7 — an entry is expired when `quarantinedAt` is older than 14 days. Document that `check-test-inventory.mjs --diff` stays unwired because it would fail on ratchet deletions. +- **Patterns to follow:** `scripts/lib/dashboard-curated-skiplist.json` (data file with mandatory `reason`, mirrored by config excludes). +- **Test scenarios:** Test expectation: none — a data file plus hand-maintained config excludes; no executable surface to test. (Covers AE1: a quarantined file listed in the ledger with its config exclude no longer appears in the package's vitest run — verify via vitest `list`.) +- **Verification:** adding a real test file to the ledger plus its config exclude removes it from `pnpm test:gate` and shard discovery without editing the test file itself. + +### U6. Local `pnpm test` re-default + +- **Goal:** Developers cannot OOM from the default command (R11), and changed-code coverage is preserved. +- **Requirements:** R11, R12. +- **Dependencies:** U1. +- **Files:** `scripts/test-changed.mjs`, `package.json` (root). +- **Approach:** `pnpm test` becomes: run `test:gate`, then affected-package tests via the existing changed-file → package → reverse-dependents expansion. Remove the local full-suite escalation: `shouldForceFullSuite` (~line 431) no longer triggers a recursive full run — shared-infra changes now run gate + a bounded affected set, with a printed note naming `pnpm test:full` for the full sweep. The `CI === "true"` force-full branch (~line 1064) is removed (CI no longer calls this script). `test:full`, `test:serial`, `test:fast`, `verify:workspace` keep their current full-suite semantics as explicit opt-ins; docs reposition `verify:workspace` as the deep pre-release check, not the pre-merge gate. +- **Execution note:** characterization-first — `scripts/__tests__/` has existing coverage of test-changed behavior; capture the current selection behavior you're keeping before removing the escalation paths. +- **Test scenarios:** (extend `scripts/__tests__/`) + - Changed file in one package → that package + reverse-dependents selected (unchanged behavior). + - Changed shared-infra file (e.g. `.github/workflows/x.yml`) → no full-suite escalation; gate + affected set only, hint printed. + - `--full` flag still runs the full suite (opt-in preserved). +- **Verification:** `pnpm test` after touching a workflow file completes without spawning the recursive full run; memory stays bounded (no 6GB dashboard lanes invoked). + +### U7. Policy docs: ratchet, appeasement ban, gate semantics + +- **Goal:** The policy is written where humans and agents actually look (R9). +- **Requirements:** R7, R8, R9, R10. +- **Dependencies:** U1–U6 (documents the shipped reality). +- **Files:** `AGENTS.md`, `docs/testing.md`. +- **Approach:** `AGENTS.md`: rewrite line ~69 ("Tests are required. Typechecks/manual checks are not substitutes.") to describe the gate-vs-non-blocking split; add the ratchet as a standing rule adjacent to FN-5048 (~lines 79–85): quarantine on sight via ledger entry; delete after 2 weeks unless rescued with evidence and a root-cause fix; **agents must never appease a flaky test** (no widened timeouts, added retries, loosened assertions — quarantine instead); a flake *inside the gate* is evicted from the allow-list, not skipped; a second quarantine in the same subsystem is a product-race smell — look before the clock runs out (see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). `docs/testing.md`: new section with ledger schema, rescue procedure, gate admission criteria (evidence of value), and the known blind spot stated honestly: the gate does not run the union suite a merge creates — logic regressions outside the curated set land non-blocking by design. Mind the AGENTS.md add/add pointer-line merge convention (`docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.md`). +- **Test scenarios:** Test expectation: none — documentation unit. (AE3 — agent consults AGENTS.md and quarantines instead of appeasing — is enforced by the policy text this unit writes; U3's test pins the CONTRIBUTING wording.) +- **Verification:** AGENTS.md and docs/testing.md describe the shipped gate accurately; no remaining references to the 4-shard PR gate or `verify:workspace` as "the canonical pre-merge gate". + +--- + +## Scope Boundaries + +- No new test machinery: no auto-quarantine, no flake-scoring, no test-value telemetry, no quarantine loader module (see origin). The ledger JSON + hand-maintained vitest config excludes are the entire mechanical surface. +- No root-cause fixing of existing flaky tests; flakes exit via quarantine and deletion. +- Healthy non-gate tests survive indefinitely in `full-suite.yml`; no mass deletion. +- Release pipelines untouched: `version.yml`/`release.yml` already run zero tests (verified), so nothing weakens; the consequence — regressions can reach main and ship behind build+typecheck+smoke — is the accepted thesis of this change. +- Coverage tooling untouched. + +### Deferred to Follow-Up Work + +- Capturing the vitest auto-kill incident, port-4040 kill guards, and `.slow` split conventions into `docs/solutions/` (learnings researcher flagged these exist only in auto-memory/AGENTS.md). +- A scheduled `--write-timings` refresh if shard balance in `full-suite.yml` degrades once PR-driven timing uploads stop (accept staleness initially). +- Extending the `.slow`-style project split to non-engine packages if their non-blocking lanes ever need tiering. + +--- + +## Risks & Dependencies + +- **Branch-protection cutover is the sharpest edge.** Required checks are matched by job name; leaving a removed name required blocks all PRs indefinitely. Mitigation: U4 names the exact new check set; do the admin update immediately after merge; audit open PRs and require rebase. +- **Gate blind spot (deliberate).** Typecheck + build + smoke + curated suite does not test the union a merge creates; documented honestly in U7. +- **Curated suite quality risk.** If the allow-list admits a latent flake, the gate loses trust fast. Mitigation: 5×-consecutive-green screen in U1 verification; eviction rule in U7. +- **Timing snapshot staleness** once shards leave PRs (deferred above) — affects only non-blocking shard balance. +- **Branch protection settings are server-side** — unverifiable from the repo; the actual required-check list at cutover time must be read from GitHub settings, not assumed. \ No newline at end of file diff --git a/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md b/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md new file mode 100644 index 0000000000..cd854dec04 --- /dev/null +++ b/docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md @@ -0,0 +1,267 @@ +--- +title: "feat: Node editor visual redesign + success/failure edge authoring" +type: feat +status: completed +date: 2026-06-04 +depth: standard +origin: none (solo planning bootstrap) +--- + +# feat: Node editor visual redesign + success/failure edge authoring + +## Summary + +Upgrade the workflow node editor's authoring experience: redesign graph nodes from small icon+label pills into larger card-style nodes with kind accent colors and config summaries; generalize edge-condition authoring so success/failure is selectable on regular edges (today only step-review edges are editable) with distinct visual styling; and round out editor power/polish — safe node/edge deletion, proper dialogs replacing `window.prompt`/`window.confirm`, inline rename/description, dirty-state guard, auto-layout, and a real empty/onboarding state. UI/authoring layer only — no engine, IR-schema, or compiler-semantics changes. + +--- + +## Problem Frame + +The editor (`packages/dashboard/app/components/WorkflowNodeEditor.tsx`, built on `@xyflow/react`) has grown to 13 editor node kinds with swimlane columns and an edge inspector, but the authoring surface lags the capability underneath: + +- **Nodes are unreadable at a glance.** `NodeShell` (`packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx`) renders icon + label + tiny badges. A prompt node configured with a model, an agent, or a CLI command looks identical to an unconfigured one; users must click every node to see what it does. +- **Failure edges exist everywhere except the editor.** The IR accepts any `edge.condition` (`parseWorkflowIr` never validates condition values), and the graph executor natively traverses `failure` edges (`shouldTraverseEdge`, `packages/engine/src/workflow-graph-executor.ts:385-392`). But `onConnect` hardcodes every new edge to `success`, and the edge inspector only offers condition controls when the source node is `step-review`. There is no way to author the branching the engine already supports. +- **Authoring chrome is crude.** `window.prompt` for workflow names, `window.confirm` for deletes, no keyboard deletion, no dirty tracking (switching workflows silently discards edits), no auto-layout, and a bare "Select or create a workflow" empty state. + +--- + +## Scope Boundaries + +### In scope +- Card-style node redesign with config summaries and kind accent colors. +- Success/failure edge-condition authoring on regular edges, with distinct edge styling and an honest "interpreter-only" presentation when branching makes the graph non-compilable to the linear step engine. +- Deletion UX (keyboard + buttons) with explicit cascade semantics. +- Dialogs, inline rename/description, dirty-state guard, auto-layout, empty/onboarding state. + +### Deferred to Follow-Up Work +- Undo/redo history for the canvas. +- Workflow import/export, versioning, templates gallery. +- Localizing edge condition labels (kept as canonical IR tokens — see KTD-8). +- Auto-layout inside `foreach` template groups beyond the existing seeded row. + +### Outside this product's identity +- Changing edge/branching **execution** semantics. The graph interpreter, `parseWorkflowIr` graph validation, and the linear-step compiler keep their current behavior; this plan only lets users author what they already support and presents their limits honestly. + +--- + +## Requirements + +**Visual** +- R1 — Graph nodes render as card-style nodes: kind accent color, icon, label, and a config-summary line (model/agent/skill/CLI for prompt nodes; script name; gate mode; hold release; join mode; parser; review type), with a defined header-overflow priority and truncation; existing badges and error badges preserved. +- R2 — Success, failure, and rework edges are distinguishable by at least two independent visual channels: the condition label is always rendered, and failure edges use a distinct dash pattern from success edges; color (token-only, both themes) is a third channel, never the only one. + +**Edge authoring** +- R3 — A user can set a regular edge's condition to `success` or `failure` from the edge inspector via a native `` gated per KTD-2 inside the existing disabled fieldset; compile-banner suffix match + info tone (KTD-4); `interactionWidth` on edges for a forgiving hit target (touch + pointer). +- `packages/dashboard/app/components/workflow-flow-mapping.ts` (modify) — edge `className` for failure edges in `irEdgeToFlow`; always-rendered condition labels; dash styling hooks; ancestor-reachability helper for the cycle guard. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify) — `.wf-edge-failure` (distinct dash pattern + `--ws-error`-derived stroke), success default styling, info-tone banner. +- `packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts` (extend) — mapping-level edge tests. +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend) — inspector gating tests. +**Approach:** Edge-level behavior is tested at the mapping layer (React Flow doesn't render edges under jsdom). The inspector reuses `updateSelectedEdge` unchanged — only the rendering gate widens, and the condition control is a native ``; persistent inline error region for validation failures; toast only for network errors; input resets after any attempt; notice when approval flags were stripped). +- `packages/dashboard/app/api/legacy.ts` (modify) — client fns. +- `packages/dashboard/src/routes/__tests__/workflow-import-export.test.ts` (new), `__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Approach:** Envelope per HTD. Server is the sole validator. Export reads the persisted definition (dirty-guard makes stale export impossible). +**Test scenarios:** +- Covers R9: export → import reproduces ir/layout/description semantically; fragment kind preserved; export blocked while dirty (button disabled). +- Covers R10: name collision → suffixed; builtin export → fresh non-builtin editable ID; missing envelope marker → 400; malformed IR → 422 + parser message, zero writes; unknown trait → 422 naming the trait; `schemaVersion` older → accepted; newer → 409 version message; CLI node with `cliSkipApproval: true` → persisted node lacks the field and the response flags the strip; script node with unknown scriptName → 200 + warning field. +- Editor: validation failure renders the inline error (not a toast) and the list is unchanged; success refreshes + activates; strip notice shown when flagged. + +### U6. Workflow-centric TaskForm + create-time workflowId + +**Goal:** Tasks pick a workflow, applied atomically at creation. +**Requirements:** R3. +**Dependencies:** U1 (fragment exclusion). (U2's migrated workflow appears automatically once present — runtime ordering, not a build dependency.) +**Files:** +- `packages/core/src/types.ts` + `packages/core/src/store.ts` (modify) — `workflowId?: string` on the task-create input; materialization inside the creation transaction mirroring the default-workflow block (`materializeDefaultWorkflowSteps`/`pendingWorkflowSelection`/`writeTaskWorkflowSelection`, store.ts ~3974-4035); explicit `workflowId` overrides the project default; fragment IDs rejected. +- `packages/dashboard/src/routes` task-create route (modify) — accept + pass `workflowId`. +- `packages/dashboard/app/components/TaskForm.tsx` (modify) — replace the per-step checkbox section (~280, ~330-339, ~1331-1337) with the workflow dropdown (states per R3); remove `fetchWorkflowSteps` usage; empty-workflow-list CTA into the editor. +- `packages/dashboard/app/components/__tests__/` TaskForm tests (extend), `packages/core/src/__tests__/` task-create tests (extend). +**Approach:** The engine path is untouched — materialization writes `enabledWorkflowSteps` exactly as the default-workflow path does, in the same transaction, so no executor-pickup race exists. `selectTaskWorkflow` remains the post-create path only. +**Test scenarios:** +- Covers R3: create with `workflowId` → task's `enabledWorkflowSteps` populated within the creation write (no intermediate empty state observable); explicit pick overrides project default; "No workflow" → no custom steps; fragment ID → rejected. +- Dropdown: loading placeholder; "(default)" badge on the project default; "No workflow" listed first; fragments absent; built-ins present. +- Empty project → CTA opens the editor. +- Regression: per-step checkboxes gone; no `fetchWorkflowSteps` call remains in TaskForm. + +### U7. AI design route (server) + +**Goal:** Prompt → validated, stripped, rate-limited IR. +**Requirements:** R11 (server half). +**Dependencies:** U1. +**Files:** +- `packages/dashboard/src/routes/register-workflow-routes.ts` (modify) — `POST /api/workflows/design` `{prompt, workflowId?}` per KTD-6: module-level `__setCreateFnAgentForDesign` DI seam co-located with the route; planning-lane model; tool-less; JSON-from-text extraction via the existing helper (planning/agent-generation precedent); `parseWorkflowIr` + compile triage (`interpreterOnly` flag) + approval-flag stripping; `workflowId` read from the store (client never posts IR); rate limit 10/hour mirroring `/ai/refine-text`; bounded prompt length. +- `packages/dashboard/src/routes/__tests__/workflow-design-route.test.ts` (new). +**Execution note:** route tests use the DI seam with a fake agent — no real model calls. +**Test scenarios:** +- Covers R11: fake agent returns valid linear IR → 200 `{ir, interpreterOnly:false}`; branching IR → 200 `{interpreterOnly:true}`; fenced/prose-wrapped JSON → still extracted and 200; invalid JSON / IR failing `parseWorkflowIr` → 422 + message, nothing persisted; IR containing `cliSkipApproval` → returned IR lacks it + strip flag set. +- `workflowId` flow: route reads the persisted IR; unknown ID → 404. +- Rate limit: 11th call within the window → 429. +- Over-length prompt → 400. + +### U8. Fragment insertion + graph-copy helpers (mapping layer) + +**Goal:** Pure, tested primitives for inserting fragments and copying graphs. +**Requirements:** R8 (helper half), R7 (copy helpers). +**Dependencies:** U1. +**Files:** +- `packages/dashboard/app/components/workflow-flow-mapping.ts` (modify) — `insertFragment(nodes, edges, fragmentIr, position)` (strips start/end, remaps all node IDs to fresh `newNodeId`s, rewires internal edges), `fragmentSeamConflicts(fragmentIr, nodes)`, `copyIrWithFreshIds(ir, layout)`. +- `packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts` (extend). +**Approach:** Pure-helper-first so jsdom limits don't bite; consumed by U4 (copies) and U9 (palette insertion). +**Test scenarios:** +- Covers R8: `insertFragment` remaps every node ID (no collisions), strips start/end, preserves internal edges/config; double-insert → disjoint ID sets. +- Fragment containing a `merge` seam vs a graph that has one → `fragmentSeamConflicts` flags it. +- `copyIrWithFreshIds` → same structure, all-new IDs, layout keys remapped consistently. + +### U9. Palette Templates section (editor) + +**Goal:** The template library is insertable from the palette. +**Requirements:** R8. +**Dependencies:** U1, U8. (U2's fragments appear once migrated — runtime ordering.) +**Files:** +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` (modify) — Templates palette section: Fragments / Built-in steps / Plugin steps subsections (alphabetical; filter input when combined > 8; plugin owner badges); entries keyboard-activatable (Enter/Space) with descriptive aria-labels; fragment insertion via U8 with the persistent inline conflict error in the section; preset step nodes via the converter field mapping; section collapsed state persisted. +- `packages/dashboard/app/api/legacy.ts` (modify) — fragments fetch (kind param); reuse existing `fetchWorkflowStepTemplates`/`fetchPluginWorkflowStepTemplates`. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify). +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Test scenarios:** +- Covers R8: three subsections render with their sources; plugin entry carries owner badge; inserting a step template adds a node with prefilled config; inserting a fragment with a seam conflict → inline error, no insertion; filter input appears above 8 combined entries and filters across groups. +- Empty fragment library → Fragments subsection hidden. +- Builtin active → insertion disabled (read-only gating). +- Keyboard activation inserts (a11y). + +### U10. Design-with-AI editor affordances + +**Goal:** Prompt-to-workflow UX in both entry points. +**Requirements:** R11 (client half). +**Dependencies:** U7. +**Files:** +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` (modify) — create dialog: an optional "Describe it instead" textarea (placeholder with an example prompt) before template selection — submitting designs a new workflow from the result; toolbar: "Design with AI" opens a popover panel (textarea + submit) targeting the active workflow via `workflowId`; proposed replacement applies only through the dirty-guard confirm. In-flight: control disabled + spinner + `aria-busy`, client-side cancel (abort the fetch); failure → server message inline, canvas untouched; `interpreterOnly` → existing info banner on the seeded graph; strip notice when flagged. +- `packages/dashboard/app/components/WorkflowNodeEditor.css` (modify). +- `packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx` (extend). +**Test scenarios:** +- Covers R11: mocked design success in create dialog → new workflow seeded from returned IR; toolbar flow over a dirty canvas → discard confirm first, cancel keeps edits. +- Mocked 422 → inline error, canvas untouched. +- In-flight state: control disabled, aria-busy set; cancel aborts and re-enables. +- interpreterOnly result → info banner visible. + +### U11. Agent-lane exposure (engine) + +**Goal:** Chat and planning agents can author workflows; drift-guarded. +**Requirements:** R12. +**Dependencies:** none (independent; lands first safely). +**Files:** +- `packages/dashboard/src/chat.ts` (modify, ~1288/1612) — pass `fn_workflow_*` factories via `createFnAgent`'s `customTools` (chat passes none today — introduce the array) with the scoped store. +- `packages/dashboard/src/planning.ts` (modify, ~842) — append the workflow tool factories to the existing `customTools: [...createPlanningBoardTools(store)]`. +- `packages/engine/src/__tests__/agent-workflow-tools-exposure.test.ts` (new) — asserts all six names (`fn_workflow_create/update/delete/list/get/select`) per lane: executor, chat, planning. +- Touched tool handlers (verify) — defensive arg parsing (string-JSON accepted). +**Approach:** Grep every `fn_workflow_` registration surface first and mirror all hits (drift learning). +**Test scenarios:** +- Covers R12: exposure test enumerates executor + chat + planning toolsets and asserts `fn_workflow_create/update/delete/list/get/select` membership in each; fails when any lane loses one. +- Chat lane: customTools array introduced without disturbing existing chat tool behavior (existing chat tests stay green). +- A workflow tool invoked with stringified-JSON args still parses. + +--- + +## Risks & Dependencies + +- **Migration writes to user DBs.** Additive only (2 columns, new rows, marker stamps, one project-default settings write); `transactionImmediate`; idempotent by stored marker; nothing deleted or rewritten. +- **defaultOn policy is a behavior interpretation.** Mapping defaultOn → combined-workflow-as-project-default preserves "new tasks run these" but collapses per-task uncheckability into workflow choice + fragments. Named in release notes; the migration test suite pins the policy. +- **Round-trip fidelity gaps.** Parity covers compiler-visible fields only — by design; `enabled`/`defaultOn`/`templateId` are policy-handled. Extend parity when `nodeToStepInput` gains fields (comments at both sites). +- **Trust boundary.** `cliSkipApproval`/`autoApprove` bypass the CLI approval gate; import and design strip them (R10/R11). Systemic schema-level rejection is an explicit follow-up. Exported files contain full prompt/command text — disclosure note on the export affordance. +- **Removal blast radius.** U3's Surface Enumeration is the sweep list; U3 is gated on U6 to avoid the no-surface window. +- **Import strictness vs. portability.** Unknown traits block (422 naming trait + owning plugin); unknown scriptNames warn without blocking — scripts are project-settings content the user can add after import. +- **AI output variance.** JSON extraction + server validation bound the failure mode to a clean 422; retry/repair deferred. Synchronous route bounded by rate limit + prompt-length cap; detached-turn upgrade documented. +- **Registration drift** (agent tools, palette template sources): grep-and-mirror; U11 guard test. +- **Mid-migration TaskForm state.** A user can open TaskForm before ever opening the editor — they see built-ins (+ any existing workflows) until migration runs on first editor open; acceptable, noted here so it isn't mistaken for a bug. +- **Changeset:** user-facing feature in the bundled CLI → `@runfusion/fusion` minor changeset. + +--- + +## System-Wide Impact + +- **Schema:** SCHEMA_VERSION 108→109 (two additive columns). +- **Engine:** no execution-semantics change; chat/planning lanes gain workflow tools (additive). +- **Existing users:** flat steps keep executing on existing tasks; the step-authoring UI is replaced by migrated workflows/fragments; defaultOn behavior is preserved via the migrated project default; TaskForm visibly changes (workflow picker) — release-notes worthy, plus the in-editor one-time migration notice. +- **Plugins:** contributed step templates move to the editor palette; plugin API unchanged. + +--- + +## Sources & Research + +- `packages/dashboard/app/components/WorkflowStepManager.tsx` (surface inventory: form fields ~718-963, templates tab + plugin templates ~185-200, refine ~830-844, onOpenGraphEditor ~430). +- `packages/dashboard/app/components/Header.tsx` (~1601, ~1947), `MobileNavBar.tsx` (~592), `useModalManager.ts` (~180, ~199, ~348-351), `AppModals.tsx` (~377-400), `TaskForm.tsx` (~242, ~280, ~330-339, ~1331-1337). +- `packages/core/src/workflow-compiler.ts` (`compileWorkflowToSteps` ~200, `nodeToStepInput` ~162-189 — emits neither `enabled` nor `defaultOn`), `builtin-workflows.ts` (`linear()` ~25-44), `store.ts` (`createWorkflowDefinition` ~12238 — fixed INSERT, no name uniqueness; `listWorkflowDefinitions` ~12289 — single unconditional cache; `selectTaskWorkflow` ~13266 — requires task id; default-workflow materialization ~3974-4035; `materializeWorkflowSteps` ~13230; `transactionImmediate` precedent), `db.ts` (`SCHEMA_VERSION = 108` ~152, `addColumnIfMissing` ~3795), `types.ts` (`WorkflowStep` ~510-548 — `enabled` required, `defaultOn` optional; `WORKFLOW_STEP_TEMPLATES` ~772; task-create input carries only `enabledWorkflowSteps`). +- `packages/dashboard/src/routes.ts` (refine route + `__setCreateFnAgentForRefine` ~370, ~3019-3092 — free-text accumulation, no JSON extraction; rate limits on `/ai/refine-text` ~1717), `register-workflow-routes.ts`; JSON-from-text extraction precedent in `planning.ts`/agent-generation. +- `packages/engine/src/agent-tools.ts` (`fn_workflow_*` ~1007-1365), `executor.ts` (~5687-5694 toolset; `cliSkipApproval`/`autoApprove` gate ~4576-4581), `packages/dashboard/src/chat.ts` (`tools: "coding"`, no customTools ~1288/1612), `planning.ts` (`customTools` ~842). +- Import/export precedent: `SettingsModal.tsx` (~1625-1671, ~7662), `register-agent-import-export-generation-routes.ts`, `AgentImportModal.tsx` (~246-257, ~495-507). +- Learnings: `docs/solutions/integration-issues/bundled-plugin-registration-drift.md`, `docs/solutions/integration-issues/branch-group-single-pr-synthetic-id-dead-wiring.md`, `docs/solutions/architecture-patterns/observable-long-running-agent-turns-through-blocking-plugin-route-seam.md`, `docs/solutions/architecture-patterns/mass-migration-agent-fleet-orchestration.md`. +- Prior plans: `docs/plans/2026-06-03-001-feat-executable-custom-workflows-node-editor-plan.md`, `docs/plans/2026-06-04-002-feat-node-editor-visual-edge-upgrade-plan.md`. diff --git a/docs/solutions/architecture-patterns/per-entity-execution-principal-override-blast-radius.md b/docs/solutions/architecture-patterns/per-entity-execution-principal-override-blast-radius.md new file mode 100644 index 0000000000..068147d392 --- /dev/null +++ b/docs/solutions/architecture-patterns/per-entity-execution-principal-override-blast-radius.md @@ -0,0 +1,56 @@ +--- +category: architecture-patterns +module: engine +date: 2026-06-05 +problem_type: architecture_pattern +component: tooling +severity: high +applies_when: + - "Adding a per-entity override that substitutes WHO/WHAT executes work (agent identity, model, principal)" + - "Wiring a new binding that supersedes task- or node-level settings (e.g. column agents, defer/override precedence)" + - "Reviewing a feature whose rollback story is 'disable the experimental flag'" +tags: + - column-agent + - execution-principal + - override-precedence + - kill-switch + - heartbeat + - workflow-columns +related: + - docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md + - docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md +--- + +# Per-entity execution-principal override: the full blast-radius checklist + +## Context + +The column-agent feature (PR #1432) lets a workflow column bind a registry agent that supersedes task/node agent settings (`defer`/`override`). The auto-merge-override lesson already taught "consult the override at every trigger gate, not just the action site." This feature showed the *execution-principal* variant has an even wider blast radius: plan review, code review, and two rounds of PR bots each found another subsystem still keyed on the old identity (`task.assignedAgentId`) — found one at a time, at increasing cost. + +## Guidance + +When work can run as an identity different from the one stored on the entity, enumerate and re-key ALL of these up front (Fusion's catalog; analogous sets exist elsewhere): + +1. **Session identity** — model resolution (`resolveExecutorSessionModel` runtimeConfig arg), persona, memory tools, attribution id. +2. **Permission gating** — `buildActionGateContext`/`buildPermanentAgentGatingContext` must receive the agent *actually running* (security boundary, not UX). +3. **Serialization, BOTH directions** — the deferral gate (`shouldDeferForHeartbeat`) AND the reverse guards keyed on `agent.taskId` in `agent-heartbeat.ts` (an agent may be effectively executing work it isn't assigned to → `isAgentEffectivelyExecuting` callback, wired at every scheduler construction site). +4. **Wake-up/resume queries** — `resumeTaskForAgent`'s task-SELECTION filter, not just its gate input; a second pass matching the *effective* identity. Watch for nodes that live only in nested structures (foreach templates are not in `ir.nodes` — walk subgraphs). +5. **Change detection / hot-swap** — the restart watcher diffs *task fields*; an override sourced from a workflow definition or agent config needs its own invalidation path, including the **release** branch (binding removed, or defer re-resolving to own settings) which must also clear the tracked principal and reverse-guard map. +6. **Kill-switch parity** — if the rollback story is "disable flag X," every execution-path entry point must actually read flag X. Gate the single choke point (resolver installation) AND any path that resolves independently (resume pass 2 resolved the IR directly and needed its own guard). +7. **Write-surface parity for safety gates** — a confirmation gate (policy escalation) added to the HTTP route is bypassed by agent tools writing through the store; share one validator (`validateColumnAgentBindings` in core) across ALL write surfaces. + +Precedence itself: one shared core resolver with explicit named branches (no `??` collapse), discriminated result for audit, and all-or-nothing own-settings semantics matched to the existing model resolver's both-present rule. + +## Why This Matters + +Each missed subsystem is a distinct production failure: gates computed for the wrong principal (privilege error), tasks stranded in-progress after heartbeats (resume miss), serialization contract violated (reverse guard), stale sessions after edits (watcher), and a rollback flag that doesn't actually roll back. None are caught by the feature's own happy-path tests; all were found by adversarial review or bots after implementation. The checklist converts five rounds of discovery into one design pass. + +## When to Apply + +Any feature where resolution of "who runs this" gains a new input: column/lane staffing, per-project agent defaults, delegation, impersonation, or model-override layers. Also when reviewing: grep every reader of the old identity field (`assignedAgentId`-style) and demand each is either re-keyed or argued irrelevant. + +## Examples + +- Single-flight interaction: when one memoized implementation pass serves many callers (foreach instances), a per-call mutable slot races — the pass-*initiating* caller must own the slot for the pass's lifetime (`runGraphTaskStep` stamps `graphSeamGoverningNodeId` only when it creates the memo, clears on settle). +- Surface-matrix tests (FN-5893): mode (defer/override) × surface (custom node, execute seam, step-execute, heartbeat, missing-agent fallback) × own-settings, plus characterization tests pinning the no-binding path byte-identical (parity oracle) and a kill-switch inertness test. +- Ambiguous composite ids: `#:` is unparseable under any single split when ids contain the delimiters — iterate candidates and validate against the graph (`parseInstanceNodeIdCandidates`), including that the *template node* exists, not just the container. diff --git a/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md b/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md new file mode 100644 index 0000000000..967384ce78 --- /dev/null +++ b/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md @@ -0,0 +1,99 @@ +--- +title: "Thin trusted merge gate with a flaky-test deletion ratchet" +date: 2026-06-05 +category: architecture-patterns +module: ci-test-gate +problem_type: architecture_pattern +component: testing_framework +severity: high +applies_when: + - "PR test gate failures are flake-dominated — most red runs do not correspond to a real regression in the PR" + - "No one can recall the last time a gate failure caught a real user-facing bug" + - "Agents or engineers are appeasing flaky tests (widened timeouts, added retries, loosened assertions)" + - "Fix-and-rerun cycles dominate PR shipping time over actual coding" + - "Local default test command escalates to a full-suite run that OOMs dev machines" +tags: [ci, merge-gate, flaky-tests, test-quarantine, deletion-ratchet, vitest, monorepo, developer-experience] +--- + +# Thin trusted merge gate with a flaky-test deletion ratchet + +## Context + +The PR test gate had undergone trust collapse: 9 required CI checks (4 duration-balanced test shards at 4–10 min each, an engine slow tier, a dashboard inventory guard) failed mostly for flaky/infra reasons. ~70% of maintainer shipping time went to an agent-fix-and-rerun loop, and no recalled PR test failure had ever caught a real user-facing bug — the gate was pure cost. Worse, agents "stabilized" flakes by widening timeouts and adding retries, draining each test's remaining signal (the suite rotted from the inside). A separate incident compounded the distrust: the fn TUI was SIGKILLing all vitest processes on a broken memory metric, producing silent exit-137 deaths misread as flakiness/OOM (auto memory [claude]). Locally, `pnpm test` escalated to an implicit full recursive run on any shared-infra change — the dev-machine OOM path. + +Shipped in Runfusion/Fusion#1453 (origin: `docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md`, plan: `docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md`). + +## Guidance + +The pattern: **thin trusted gate on PRs + demoted non-blocking tier on main + deletion ratchet as written policy**. + +**1. Block PRs on a minimal trusted set, and pin that set with a test.** PRs block on exactly Lint, Typecheck, Build, Gate (`.github/workflows/pr-checks.yml`). The CI-shape test (`packages/cli/src/__tests__/ci-workflow.test.ts`) — itself part of the gate — makes drift fail loudly: + +```typescript +it("blocks PRs on exactly lint, typecheck, build, and gate", () => { + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(["build", "gate", "lint", "typecheck"]); +}); +``` + +**2. The Gate job = boot smoke + curated allow-list suite.** The boot smoke (`scripts/boot-smoke.mjs`) proves the app actually starts: CLI answers `--help`, a real `fn serve` returns `/api/health` 200 on an ephemeral port (isolated `$HOME`), and shutdown is SIGTERM-verified. The test half is `pnpm test:gate`: a curated `engine-core` vitest project (`packages/engine/vitest.config.ts`) whose membership is an **explicit file-by-file allow-list, not a glob** — 20 files, ~4s wall, ~1,025 tests, selected from committed per-file timing data (`scripts/test-timings.json`) with a determinism criterion (no real-git contention suites, no `*.slow.test.ts`, no shared-state bleed). + +**3. Demote everything else to a separate non-blocking workflow.** `full-suite.yml` runs the shards/slow tier/inventory guard on push to main only — a red run there is information, never a merge stopper. A separate workflow file (not `continue-on-error` jobs) keeps a red full-suite run from painting the gate's workflow red. Key the concurrency group by SHA: + +```yaml +concurrency: + group: full-suite-${{ github.sha }} # ref-keyed + cancel-in-progress would let + cancel-in-progress: false # consecutive merges cancel each other's coverage +``` + +**4. The deletion ratchet is policy with minimal mechanics.** A test that fails without a corresponding real bug is quarantined on sight: a dated entry in `scripts/lib/test-quarantine.json` (`file`, `reason` + failing-run link, `quarantinedAt`) plus a hand-maintained `exclude` line in that package's vitest config, same commit. The entry expires in 14 days — then the test is deleted unless rescued with evidence it catches real regressions plus a root-cause fix. There is deliberately no loader module and no automation; `check-test-inventory.mjs --diff` stays unwired because a snapshot diff guard would fail on exactly the deletions the ratchet performs. Appeasement (timeouts/retries/loosened assertions) is banned outright in `AGENTS.md` — for agents especially. + +**5. Remove implicit full-suite escalation from the local default.** `scripts/test-changed.mjs` routes every ambiguous condition to gate mode instead of an implicit full run: + +```javascript +if (!comparisonBase) return { mode: "gate", reason: "missing-comparison-base" }; +if (!changedFiles) return { mode: "gate", reason: "diff-failed" }; +if (changedFiles.length === 0) return { mode: "gate", reason: "no-changes" }; +if (isSharedInfraChange(changedFiles)) return { mode: "gate", reason: "shared-infra-changed" }; +``` + +The full suite runs only on explicit opt-in (`--full` / `pnpm test:full`). In changed mode the gate suite runs first, under the isolation guard. + +## Why This Matters + +- **Red means real.** A gate with a high false-positive rate teaches everyone that red is noise. A gate small enough to curate and cheap enough to evict from restores the only property a merge gate needs. +- **The allow-list solves the eviction chicken-and-egg.** Under a glob, removing a flaky gate test needs a PR that passes the flaky gate. Under an allow-list, eviction is deleting one line — the eviction PR never waits on the flaky test. +- **Policy beats machinery.** This repo's history answered test pain with more test machinery (sharding, isolation checks, lock runners, kill guards). The ratchet is a written rule plus a dated JSON record; automation would let entries accumulate silently. +- **Appeasement is how suites rot.** The ratchet leaves exactly two exits for a flake: deletion or a root-cause rescue. There is no "stabilize it" option — the prior suite was the proof of where that leads. +- **The blind spot is documented, not hidden.** The gate does not run the union suite a merge creates; logic regressions outside the curated set land non-blocking by design (`docs/testing.md`). Stating this honestly beats the illusion of coverage that 9 untrusted jobs provided. + +## When to Apply + +- Trust collapse indicators (see `applies_when`): flake-dominated reds, zero recalled catches, appeasement loops, fix-and-rerun dominating shipping time, reflexive dismissal of red CI. +- When eviction must be cheap: an explicit allow-list is the right gate shape even for small suites — one flaky gate test poisons the gate for everyone. +- **Counter-case:** a repeatedly "stabilized" flake can be a real product race — see `../ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md` (a flake "fixed" three times that was a genuine race). The complete triage tree: root cause known → fix the invariant; root cause unknown and no real bug → quarantine → deletion clock; second quarantine in the same subsystem → product-race smell, look before deleting. + +## Examples + +Before/after CI topology: + +``` +Before: PR → lint/typecheck/build + 4 test shards (4–10 min, flaky) + slow tier + inventory guard +After: PR → Lint, Typecheck, Build, Gate (boot smoke + ~4s curated suite) [blocking] + main push → full-suite.yml: shards + slow tier + guard [non-blocking] +``` + +Operational gotchas discovered while shipping this: + +- **A PR in `CONFLICTING` mergeable state runs zero GitHub Actions.** No `pull_request` workflows fire because GitHub cannot build the merge ref — it looks exactly like "CI never started" (no pending checks at all). Fix: merge the base branch; the resulting push triggers normally. +- **Deleting a workflow file crashes tests that `readFileSync` it.** The asserting test fails at `beforeAll`, taking its whole file with it. Rewrite the test in the same PR and convert the absence into an invariant: `expect(() => loadWorkflow("ci.yml")).toThrow()`. +- **Node does not fire `'exit'` on SIGTERM/SIGINT.** A smoke script registering only `process.on("exit", cleanup)` orphans its server child when the CI job is cancelled. Register explicit signal handlers that call cleanup and re-exit with 143/130. +- **A new "always run" mode must be excluded from cache fast paths.** Gate mode initially fell into `test-changed.mjs`'s cache-fresh short-circuit and silently no-opped; the fix treats gate mode as always having work. +- **Smoke-test shutdown verdicts need both halves:** SIGTERM actually delivered AND a clean exit (`code 0`/`SIGTERM`) — otherwise a server that crashes after the health check still prints PASS. + +## Related + +- `docs/testing.md` ("The merge gate", "Quarantine ledger and the deletion ratchet") and the `AGENTS.md` standing rule — the operative policy text +- `../ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md` — the complementary triage path: when a flake is a real product race, fix the invariant instead of quarantining +- `../architecture-patterns/i18n-foundation-vite-ink-monorepo-code-split-catalogs.md` — references "CI shard 1/2" as PR-blocking surfaces; post-#1453 those run non-blocking in `full-suite.yml` +- Runfusion/Fusion#1453 (the change), follow-up gaps Runfusion/Fusion#1447–#1452 (untested gate-mode invariants, dist-cache composite action, workflow-loader dedup) +- Open flaky-test issues Runfusion/Fusion#1430, Runfusion/Fusion#1355 — first candidates for the quarantine ledger under the new policy diff --git a/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md b/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md new file mode 100644 index 0000000000..ecd436d16e --- /dev/null +++ b/docs/solutions/ui-bugs/i18n-empty-locale-placeholders-render-blank.md @@ -0,0 +1,65 @@ +--- +title: Empty-string locale placeholders render blank UI (i18next returnEmptyString default) +date: 2026-06-05 +category: ui-bugs +module: i18n +problem_type: ui_bug +component: frontend +symptoms: + - "Buttons, labels, and dialog copy render completely blank in non-English locales" + - "Inline English defaults passed to t(\"key\", \"Default\") are ignored — blank wins" + - "No console errors; en locale looks perfect; only translated locales affected" +root_cause: wrong_api +resolution_type: config_fix +severity: high +related_components: + - dashboard + - tooling +tags: [i18next, returnEmptyString, locale-placeholders, translation-fallback, i18n-extract, catalog-pruning] +--- + +# Empty-string locale placeholders render blank UI (i18next returnEmptyString default) + +## Problem + +The repo's translator workflow backfills `""` placeholders into non-en catalogs for untranslated keys, on the assumption that empty values fall back to English at runtime. They don't: i18next's default `returnEmptyString: true` treats `""` as a *found* value, so es/fr/ko/zh users saw blank buttons, nav labels, and dialog copy for every new key — even though components pass inline English defaults (`t("key", "Default")`). + +## Symptoms + +- New UI strings render blank in any non-English locale while en looks correct. +- The inline second-argument default to `t()` does not rescue it — `""` short-circuits the fallback chain entirely. +- Verified empirically (i18next 26.x): `t("empty", "InlineDefault")` returns `""` when the active locale defines the key as `""`. + +## What Didn't Work + +- Assuming the standing convention was safe because hundreds of `""` placeholders pre-existed — the convention had been silently rendering blanks all along for any key reached in a non-en locale. +- Relying on inline `t()` defaults as a safety net — they only apply when the key is *missing*, not empty. + +## Solution + +One config line in the shared i18next init (`packages/i18n/src/config.ts`, `baseInitOptions()`): + +```ts +returnEmptyString: false, +``` + +With this set, `""` values are treated as missing and fall through the fallback chain (`fallbackLng` → en, or the inline default). Locale files stay untouched, the `""`-placeholder translator convention keeps working, and every existing empty placeholder is fixed at once. + +Empirical check that settles the question in seconds (run against `node_modules` i18next, initialized like the app): + +```js +// lng: "fr", resources: { fr: { empty: "" }, en: { empty: "EnglishValue" } } +t("empty", "InlineDefault") +// returnEmptyString true (default) → "" ← blank UI +// returnEmptyString false → "EnglishValue" +``` + +## Why This Works + +i18next resolution asks "does the key exist with a usable value?" — `returnEmptyString` defines whether `""` is usable. The default (`true`) is meant for apps where empty is a legitimate translation; in a placeholder-backfill workflow it's exactly wrong, because every placeholder is an intentional "not translated yet" marker. + +## Prevention + +- When adopting any `""`-placeholder catalog convention, set `returnEmptyString: false` in the same commit — the two are a package deal. +- Don't trust the inline-`t()`-default mental model; prove fallback behavior with a 5-line init script before relying on it. +- **Related catalog trap (hit twice in the same PR):** `pnpm i18n:extract` prunes keys whose usages it cannot see (CLI/TUI surfaces, dynamic keys) — it deleted live keys like `taskFields.*` and `common.cancel` from `en/app.json`. After running extract, semantically diff catalogs against the base ref (flatten both JSONs, assert zero removed/changed keys vs upstream, only intended additions) before committing. The content sanity test `packages/i18n/src/__tests__/config.test.ts` ("has real en content") exists because of this; prefer hand-adding keys + `i18n:sync`/`i18n:types` over trusting `i18n:extract` output wholesale. diff --git a/docs/testing.md b/docs/testing.md index 3817e9a446..e64608225f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -4,18 +4,26 @@ This guide consolidates the detailed testing guidance moved from `AGENTS.md`. -## Required workspace gates +## The merge gate -Tests are required. Typechecks and manual verification are not substitutes for assertions. +CI blocks PRs on exactly four checks (`.github/workflows/pr-checks.yml`): **Lint, Typecheck, Build, Gate**. The Gate job runs the boot smoke (`scripts/boot-smoke.mjs`: CLI `--help` + a real `fn serve` answering `GET /api/health`) and `pnpm test:gate` (the curated `engine-core` vitest project + the CI-shape test). Everything else — the 4-way shards, the engine slow tier, the dashboard inventory guard — runs NON-BLOCKING in `.github/workflows/full-suite.yml` on push to main. + +Gate membership is the explicit allow-list in `packages/engine/vitest.config.ts` (`engine-core` project). Admission requires evidence of value (the test catches real regressions); tests never graduate in by default. A flaky gate test is evicted by deleting its allow-list line — the eviction PR does not need the flaky test to pass. The whole `engine-core` project must stay under ~60s wall-clock. + +**The gate's blind spot, stated honestly:** typecheck + build + boot smoke + curated suite does not run the union suite a merge creates. Logic regressions outside the curated set land non-blocking by design — that is the accepted trade: the old broad gate caught no recalled real bugs while consuming ~70% of shipping time in flake triage. + +## Required workspace gates Use the narrowest command that exercises the behavior you changed, then broaden before reporting completion. ```bash -pnpm test # changed-only workspace tests; falls back to full gate in safety contexts -pnpm test:full # full workspace quality gate +pnpm test # gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite — explicit opt-in only pnpm lint # lint all packages pnpm build # build workspace packages (excludes desktop/mobile) -pnpm verify:workspace # canonical pre-merge gate: lint -> test:full -> build +pnpm verify:workspace # deep opt-in verification: lint -> test:full -> build (NOT the merge gate) ``` `pnpm test:full` runs each package's default test script with capped worker fanout (`FUSION_TEST_TOTAL_WORKERS=4 FUSION_TEST_CONCURRENCY=2 pnpm -r --workspace-concurrency=2 test`). Do not casually raise worker counts; dashboard/jsdom and integration-heavy packages destabilize when oversubscribed. Use `VITEST_MAX_WORKERS=` only for targeted package-level investigation. @@ -50,7 +58,7 @@ list only when you want it in a specific fast lane rather than the backfill catc The dashboard quality gate is a chain of curated lanes plus two backfill lanes. Together they must execute **every** `*.test.{ts,tsx}` under `packages/dashboard/app` and `packages/dashboard/src`, or the file must be on the reviewed skip-list. This is -enforced by a guard (CI job `Dashboard curated-gate guard` in `pr-checks.yml`): +enforced by a guard (CI job `Dashboard curated-gate guard` in `full-suite.yml`, non-blocking): ```bash node scripts/check-test-inventory.mjs --dashboard-curated @@ -88,19 +96,39 @@ The capture spec (which packages/projects to enumerate) lives in a renamed file shows up as a remove (old path) + add (new path), so the rename is reviewable. New test ids never fail the diff. -## Engine slow tier (CI gate) +## Engine slow tier (non-blocking CI) The `engine-slow` vitest project (`packages/engine/src/**/*.slow.test.ts`) holds the long real-git suites. It runs locally via `pnpm --filter @fusion/engine test:slow` and -in CI via the `Engine slow tier` job in `pr-checks.yml`, which uses +in CI via the `Engine slow tier` job in `full-suite.yml` (non-blocking, push to main), which uses `scripts/assert-engine-slow-nonempty.mjs` to **fail if zero tests executed** (so a glob -or config drift that silently empties the tier breaks CI instead of passing vacuously). +or config drift that silently empties the tier breaks the run instead of passing vacuously). The CI job uses `fetch-depth: 0` because these tests run real git operations. +## Quarantine ledger and the deletion ratchet + +Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. This is written policy with minimal mechanics — deliberately no loader module, no automation (see the AGENTS.md standing rule "Flaky Tests Are Quarantined on Sight"). + +**To quarantine a test** (a test that failed without a corresponding real bug in the change), in one commit: + +1. Add an entry to `scripts/lib/test-quarantine.json`: + `{ "file": "", "reason": "", "quarantinedAt": "YYYY-MM-DD" }` +2. Add a matching one-line `exclude` entry to that package's vitest config. + +**The clock:** an entry expires 14 days after `quarantinedAt`. Whoever touches the suite and finds an expired entry deletes the test file, its ledger entry, and its config exclude (git history is the archive). `scripts/check-test-inventory.mjs --diff` stays deliberately unwired in CI because it would fail on exactly these deletions. + +**Rescue** (before the clock runs out) requires both: evidence the test catches real regressions, and a root-cause fix for the flake. Stabilization passes — widened timeouts, retries, loosened assertions — are appeasement, not rescue, and are banned (for agents especially). + +**Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier). + +**Gate admission:** the mirror operation — add the test's path to the `engine-core` `include` array in `packages/engine/vitest.config.ts`, citing the evidence of value (a real regression it caught) in the PR. Keep the project under its ~60s wall-clock budget. + +**Product-race escalation:** a second quarantine in the same subsystem is a smell that the flake is a real product race, not test noise — look at the product code before deleting (a dashboard flake was "stabilized" three times before being found to be a real race; see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). + ## CI shard balancing (duration-weighted) `scripts/ci-test-shard.mjs` packs the 4 CI shards (`pnpm test:ci:shard --shard N --total 4`, -called from `pr-checks.yml`) by **measured duration**, not test-file count, using the +called from `full-suite.yml`, non-blocking) by **measured duration**, not test-file count, using the committed `scripts/test-timings.json` snapshot (U1/R4). A package's weight is the sum of its files' recorded durations; files (or whole packages) absent from the snapshot fall back to the snapshot's **median per-file duration** so untimed packages weigh diff --git a/package.json b/package.json index 34a91cf147..24269b677d 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,8 @@ "scripts": { "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", + "test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", + "smoke:boot": "node scripts/boot-smoke.mjs", "local": "node scripts/start-local.mjs", "dev": "node scripts/dev-with-memory.mjs", "dev:ui": "pnpm --filter @fusion/dashboard dev", diff --git a/packages/cli/package.json b/packages/cli/package.json index 6e56adcb2a..3a3c9d188d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -50,6 +50,7 @@ "build:exe:all": "bun run build.ts --all", "typecheck": "tsc --noEmit", "test": "vitest run --silent=passed-only --reporter=dot", + "test:ci-shape": "vitest run src/__tests__/ci-workflow.test.ts --silent=passed-only --reporter=dot", "test:slow-cli": "cross-env FUSION_TEST_SLOW_CLI=1 vitest run src/commands/__tests__/agent-export.test.ts --silent=passed-only --reporter=dot", "test:extension-integration": "cross-env FUSION_TEST_EXTENSION_INTEGRATION=1 vitest run src/__tests__/extension-integration.test.ts --silent=passed-only --reporter=dot", "test:build-exe": "cross-env FUSION_TEST_BUILD_EXE=1 vitest run --config vitest.build-exe.config.ts --silent=passed-only --reporter=dot", diff --git a/packages/cli/skill/fusion/references/engine-tools.md b/packages/cli/skill/fusion/references/engine-tools.md index 79e3dae523..07e40b23cd 100644 --- a/packages/cli/skill/fusion/references/engine-tools.md +++ b/packages/cli/skill/fusion/references/engine-tools.md @@ -15,14 +15,14 @@ These tools are **not** part of the user-invokable extension surface. They are i | `fn_task_log` | executor, heartbeat | Write significant task log entries | `message` (string), `outcome?` (string) | | `fn_task_document_write` | triage, executor, heartbeat | Save/update a named task document revision | `key` (string), `content` (string), `author?` (string) | | `fn_task_document_read` | triage, executor, heartbeat | Read one task document or list all | `key?` (string) | -| `fn_workflow_list` | executor | List the project's custom workflows (read-only built-ins plus user definitions) | none | -| `fn_workflow_get` | executor | Fetch one workflow definition by id — name, description, builtin flag, and the full IR (nodes/edges/columns/artifacts/fields) as JSON | `workflow_id` (string) | -| `fn_workflow_select` | executor | Assign a custom workflow to a task (defaults to the current task) | `workflow_id` (string), `task_id?` (string) | -| `fn_workflow_create` | executor | Create a custom workflow definition from a graph IR (validated server-side). v2 IR supports step-inversion constructs: `parse-steps`, `foreach` (mode/isolation/concurrency/maxReworkCycles), `step-execute`, `step-review`, `code` nodes, `rework` edges, plus `artifacts` and custom `fields` declarations | `name` (string), `description?` (string), `ir` (object), `layout?` (object) | -| `fn_workflow_update` | executor | Update a custom workflow definition's name/description/ir/layout (built-ins cannot be edited; same step-inversion IR constructs as create; editing `fields` orphans rather than destroys existing task values) | `workflow_id` (string), `name?` (string), `description?` (string), `ir?` (object), `layout?` (object), `rehome_to?` (string) | -| `fn_workflow_delete` | executor | Delete a custom workflow definition (built-ins cannot be deleted); selecting tasks are re-homed to the default workflow's entry column | `workflow_id` (string) | +| `fn_workflow_list` | executor, chat, planning | List the project's custom workflows (read-only built-ins plus user definitions) | none | +| `fn_workflow_get` | executor, chat, planning | Fetch one workflow definition by id — name, description, builtin flag, and the full IR (nodes/edges/columns/artifacts/fields) as JSON | `workflow_id` (string) | +| `fn_workflow_select` | executor, chat, planning | Assign a custom workflow to a task (defaults to the current task) | `workflow_id` (string), `task_id?` (string) | +| `fn_workflow_create` | executor, chat, planning | Create a custom workflow definition from a graph IR (validated server-side). v2 IR supports step-inversion constructs: `parse-steps`, `foreach` (mode/isolation/concurrency/maxReworkCycles), `step-execute`, `step-review`, `code` nodes, `rework` edges, plus `artifacts` and custom `fields` declarations | `name` (string), `description?` (string), `ir` (object), `layout?` (object) | +| `fn_workflow_update` | executor, chat, planning | Update a custom workflow definition's name/description/ir/layout (built-ins cannot be edited; same step-inversion IR constructs as create; editing `fields` orphans rather than destroys existing task values) | `workflow_id` (string), `name?` (string), `description?` (string), `ir?` (object), `layout?` (object), `rehome_to?` (string) | +| `fn_workflow_delete` | executor, chat, planning | Delete a custom workflow definition (built-ins cannot be deleted); selecting tasks are re-homed to the default workflow's entry column | `workflow_id` (string) | | `fn_task_promote` | executor | Promote a held task out of a manual-release hold column (defaults to the current task) | `task_id?` (string) | -| `fn_trait_list` | executor | List the registered column trait catalog (built-in and plugin traits) | none | +| `fn_trait_list` | executor, chat, planning | List the registered column trait catalog (built-in and plugin traits) | none | | `fn_memory_search` | triage, executor, heartbeat | Search project memory plus per-agent layered memory snippets | `query` (string), `limit?` (number) | | `fn_memory_get` | triage, executor, heartbeat | Read a bounded memory file window (including bounded per-agent layered paths) | `path` (string), `startLine?` (number), `lineCount?` (number) | | `fn_memory_append` | executor, heartbeat (when writable backend enabled) | Append memory notes with explicit scope: `scope="agent"` for private operating context, `scope="project"` for workspace-wide durable knowledge | `scope?` (`project` \| `agent`), `layer` (`long-term` \| `daily`), `content` (string) | diff --git a/packages/cli/src/__tests__/ci-workflow.test.ts b/packages/cli/src/__tests__/ci-workflow.test.ts index 236b6fa9e5..3fa71994b9 100644 --- a/packages/cli/src/__tests__/ci-workflow.test.ts +++ b/packages/cli/src/__tests__/ci-workflow.test.ts @@ -27,12 +27,10 @@ function findCompositeSetupStep(steps: any[]) { return steps.find((step) => step.uses === "./.github/actions/setup-node-pnpm"); } -describe("CI workflow (.github/workflows/ci.yml)", () => { +describe("Merge gate (.github/workflows/pr-checks.yml)", () => { let workflow: any; let content: string; let compositeAction: any; - let buildSteps: any[]; - let testShardJob: any; let contributingContent: string; let readmeContent: string; let cliPackageJsonContent: string; @@ -41,12 +39,10 @@ describe("CI workflow (.github/workflows/ci.yml)", () => { let buildExeSuiteContent: string; beforeAll(() => { - const result = loadWorkflow("ci.yml"); + const result = loadWorkflow("pr-checks.yml"); workflow = result.parsed; content = result.content; compositeAction = loadYamlFile(".github", "actions", "setup-node-pnpm", "action.yml").parsed; - buildSteps = workflow.jobs?.build?.steps ?? []; - testShardJob = workflow.jobs?.["test-shards"]; contributingContent = readFileSync(join(workspaceRoot, "docs", "contributing.md"), "utf-8"); readmeContent = readFileSync(join(workspaceRoot, "README.md"), "utf-8"); cliPackageJsonContent = readFileSync(join(workspaceRoot, "packages", "cli", "package.json"), "utf-8"); @@ -64,134 +60,56 @@ describe("CI workflow (.github/workflows/ci.yml)", () => { ); }); - const findBuildStepByRun = (runSnippet: string) => - buildSteps.find((step) => typeof step.run === "string" && step.run.includes(runSnippet)); - it("is valid YAML", () => { expect(workflow).toBeDefined(); expect(typeof workflow).toBe("object"); }); - it("uses workflow_dispatch trigger (auto CI disabled)", () => { - expect(workflow.on).toHaveProperty("workflow_dispatch"); + it("runs on pull requests targeting main and ONLY there", () => { + expect(workflow.on?.pull_request?.branches).toContain("main"); + // Post-merge signal lives in full-suite.yml; the gate workflow must not + // double-run on push (that conflates blocking and non-blocking surfaces). + expect(workflow.on?.push).toBeUndefined(); }); - it("does not auto-trigger on push/pull_request", () => { - expect(workflow.on.push).toBeUndefined(); - expect(workflow.on.pull_request).toBeUndefined(); + it("blocks PRs on exactly lint, typecheck, build, and gate", () => { + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(["build", "gate", "lint", "typecheck"]); }); - it("pins dependency bootstrap to frozen lockfile", () => { - const jobs = [workflow.jobs?.lint, workflow.jobs?.["test-shards"], workflow.jobs?.build]; - for (const job of jobs) { - expect(findCompositeSetupStep(job?.steps ?? [])).toBeDefined(); + it("contains no shard matrix or full-suite invocation (demoted to full-suite.yml)", () => { + expect(workflow.jobs?.["test-shards"]).toBeUndefined(); + expect(workflow.jobs?.["test-slow"]).toBeUndefined(); + expect(workflow.jobs?.["test-inventory-guard"]).toBeUndefined(); + expect(content).not.toContain("test:ci:shard"); + expect(content).not.toContain("run: pnpm test\n"); + expect(content).not.toContain("pnpm verify:workspace"); + }); + + it("gate job runs boot smoke and the dedicated test:gate command", () => { + const gateSteps = workflow.jobs?.gate?.steps ?? []; + expect( + gateSteps.some( + (step: any) => typeof step.run === "string" && step.run.includes("node scripts/boot-smoke.mjs"), + ), + ).toBe(true); + // The gate must use the dedicated command — `pnpm test` routes through + // scripts/test-changed.mjs whose selection semantics are for local runs. + expect( + gateSteps.some( + (step: any) => typeof step.run === "string" && step.run.includes("pnpm test:gate"), + ), + ).toBe(true); + }); + + it("pins dependency bootstrap to frozen lockfile in every job", () => { + for (const jobName of ["lint", "typecheck", "build", "gate"]) { + expect(findCompositeSetupStep(workflow.jobs?.[jobName]?.steps ?? [])).toBeDefined(); } expect(content).not.toContain("run: pnpm install\n"); expect(content).not.toContain("--no-frozen-lockfile"); expect(compositeAction.inputs?.["install-args"]?.default).toBe("--frozen-lockfile"); }); - it("uses deterministic test sharding and keeps lint/build as explicit jobs", () => { - expect(workflow.jobs?.lint).toBeDefined(); - expect(testShardJob).toBeDefined(); - expect(workflow.jobs?.build).toBeDefined(); - - expect(testShardJob.strategy?.matrix?.shard).toEqual([1, 2, 3]); - expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 3"); - expect(content).not.toContain("pnpm verify:workspace"); - }); - - it("runs build job after lint and sharded tests, then executes slow lane and binary packaging", () => { - expect(workflow.jobs?.build?.needs).toEqual(["lint", "test-shards"]); - expect(findBuildStepByRun("pnpm build")).toBeDefined(); - expect(findBuildStepByRun("pnpm test:slow-cli")).toBeDefined(); - expect(findBuildStepByRun("build:exe")).toBeDefined(); - }); - - it("keeps contributing docs aligned with verification and slow-lane contracts", () => { - expect(contributingContent).toContain("pnpm test:full` must be runnable in a clean worktree without requiring a prior `pnpm build`."); - expect(contributingContent).toContain("`pnpm verify:workspace` is the canonical pre-merge gate"); - expect(contributingContent).toContain("1. `pnpm lint`"); - expect(contributingContent).toContain("2. `pnpm test:full`"); - expect(contributingContent).toContain("3. `pnpm build`"); - expect(contributingContent).toContain("`pnpm test` now uses a changed-only entrypoint"); - - expect(contributingContent).toContain("pnpm test:slow-cli"); - expect(contributingContent).toContain("test:pre-release"); - expect(contributingContent).toContain("test:extension-integration"); - }); - - it("keeps docs aligned with default and explicit build commands", () => { - expect(readmeContent).toContain("pnpm build # Build default workspace packages (excludes desktop/mobile)"); - expect(readmeContent).toContain("pnpm build:all # Build all packages (including desktop/mobile)"); - - expect(contributingContent).toContain("pnpm build # default build (excludes desktop/mobile)"); - expect(contributingContent).toContain("pnpm build:all # full recursive build including desktop/mobile"); - }); - - it("includes binary build step", () => { - expect(content).toContain("build:exe"); - }); - - it("keeps explicit gating for audited CLI integration suites", () => { - expect(cliPackageJsonContent).toContain('"test:slow-cli"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_SLOW_CLI=1"); - expect(cliPackageJsonContent).toContain('"test:extension-integration"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION=1"); - expect(cliPackageJsonContent).toContain("extension-integration.test.ts"); - expect(cliPackageJsonContent).toContain('"test:build-exe"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_BUILD_EXE=1"); - - expect(extensionSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_EXTENSION_INTEGRATION)"); - expect(extensionSuiteContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION"); - expect(extensionSuiteContent).toContain("dist/extension.js"); - - expect(agentExportSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_SLOW_CLI)"); - expect(agentExportSuiteContent).toContain("FUSION_TEST_SLOW_CLI"); - - expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "1"'); - expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "true"'); - expect(buildExeSuiteContent).not.toContain("Boolean(process.env.FUSION_TEST_BUILD_EXE)"); - }); - - it("includes Bun setup", () => { - expect(content).toContain("oven-sh/setup-bun"); - }); - - it("verifies binary exists after build", () => { - expect(content).toContain("test -f packages/cli/dist/fn"); - }); -}); - -describe("PR checks workflow (.github/workflows/pr-checks.yml)", () => { - let workflow: any; - let content: string; - - beforeAll(() => { - const result = loadWorkflow("pr-checks.yml"); - workflow = result.parsed; - content = result.content; - }); - - it("is valid YAML", () => { - expect(workflow).toBeDefined(); - expect(typeof workflow).toBe("object"); - }); - - it("runs on pull requests targeting main", () => { - expect(workflow.on?.pull_request?.branches).toContain("main"); - }); - - it("uses the same deterministic test sharding command as manual CI", () => { - expect(workflow.jobs?.lint).toBeDefined(); - expect(workflow.jobs?.typecheck).toBeDefined(); - expect(workflow.jobs?.build).toBeDefined(); - expect(workflow.jobs?.["test-shards"]).toBeDefined(); - expect(workflow.jobs?.["test-shards"]?.strategy?.matrix?.shard).toEqual([1, 2, 3, 4]); - expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4"); - expect(content).not.toContain("run: pnpm test\n"); - }); - it("keeps lint as install + lint only, without Bun/setup build coupling", () => { const lintSteps = workflow.jobs?.lint?.steps ?? []; expect( @@ -229,7 +147,99 @@ describe("PR checks workflow (.github/workflows/pr-checks.yml)", () => { ).toBe(true); }); - it("does not spend PR action minutes on a pre-test workspace build", () => { + it("keeps contributing docs aligned with the gate contract", () => { + expect(contributingContent).toContain("pnpm test:full` must be runnable in a clean worktree without requiring a prior `pnpm build`."); + expect(contributingContent).toContain("`pnpm test:gate` is the merge gate"); + expect(contributingContent).toContain("`pnpm verify:workspace` is the deep opt-in verification (not the merge gate)"); + expect(contributingContent).toContain("1. `pnpm lint`"); + expect(contributingContent).toContain("2. `pnpm test:full`"); + expect(contributingContent).toContain("3. `pnpm build`"); + expect(contributingContent).toContain("`pnpm test` now uses a changed-only entrypoint"); + + expect(contributingContent).toContain("pnpm test:slow-cli"); + expect(contributingContent).toContain("test:pre-release"); + expect(contributingContent).toContain("test:extension-integration"); + }); + + it("keeps docs aligned with default and explicit build commands", () => { + expect(readmeContent).toContain("pnpm build # Build default workspace packages (excludes desktop/mobile)"); + expect(readmeContent).toContain("pnpm build:all # Build all packages (including desktop/mobile)"); + + expect(contributingContent).toContain("pnpm build # default build (excludes desktop/mobile)"); + expect(contributingContent).toContain("pnpm build:all # full recursive build including desktop/mobile"); + }); + + it("keeps explicit gating for audited CLI integration suites", () => { + expect(cliPackageJsonContent).toContain('"test:slow-cli"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_SLOW_CLI=1"); + expect(cliPackageJsonContent).toContain('"test:extension-integration"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION=1"); + expect(cliPackageJsonContent).toContain("extension-integration.test.ts"); + expect(cliPackageJsonContent).toContain('"test:build-exe"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_BUILD_EXE=1"); + + expect(extensionSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_EXTENSION_INTEGRATION)"); + expect(extensionSuiteContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION"); + expect(extensionSuiteContent).toContain("dist/extension.js"); + + expect(agentExportSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_SLOW_CLI)"); + expect(agentExportSuiteContent).toContain("FUSION_TEST_SLOW_CLI"); + + expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "1"'); + expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "true"'); + expect(buildExeSuiteContent).not.toContain("Boolean(process.env.FUSION_TEST_BUILD_EXE)"); + }); + + it("the deleted manual CI workflow stays deleted", () => { + // ci.yml was the trigger-disabled (FN-1541) 3-shard manual workflow; the + // merge-gate redesign removed it. Reintroducing it would resurrect a + // second, drift-prone definition of the test pipeline. + expect(() => loadWorkflow("ci.yml")).toThrow(); + }); +}); + +describe("Full suite workflow (.github/workflows/full-suite.yml)", () => { + let workflow: any; + let content: string; + + beforeAll(() => { + const result = loadWorkflow("full-suite.yml"); + workflow = result.parsed; + content = result.content; + }); + + it("is valid YAML", () => { + expect(workflow).toBeDefined(); + expect(typeof workflow).toBe("object"); + }); + + it("runs ONLY on push to main — never as a PR gate", () => { + expect(workflow.on?.push?.branches).toEqual(["main"]); + expect(workflow.on?.pull_request).toBeUndefined(); + }); + + it("carries the demoted tier: 4-way shards, engine slow, inventory guard", () => { + expect(workflow.jobs?.["test-shards"]?.strategy?.matrix?.shard).toEqual([1, 2, 3, 4]); + expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4"); + expect(workflow.jobs?.["test-slow"]).toBeDefined(); + expect(workflow.jobs?.["test-inventory-guard"]).toBeDefined(); + }); + + it("keeps full clones where real-git tests need history", () => { + const shardSteps = workflow.jobs?.["test-shards"]?.steps ?? []; + const slowSteps = workflow.jobs?.["test-slow"]?.steps ?? []; + for (const steps of [shardSteps, slowSteps]) { + expect( + steps.some((step: any) => step.uses?.includes("actions/checkout") && step.with?.["fetch-depth"] === 0), + ).toBe(true); + } + }); + + it("still uploads per-shard timing artifacts for snapshot refresh", () => { + expect(content).toContain("test-timings-shard-${{ matrix.shard }}"); + }); + + it("does not spend action minutes on a pre-test workspace build", () => { const testSteps = workflow.jobs?.["test-shards"]?.steps ?? []; expect( testSteps.some( diff --git a/packages/cli/src/__tests__/package-config.test.ts b/packages/cli/src/__tests__/package-config.test.ts index 9c6ed4b801..3d34449432 100644 --- a/packages/cli/src/__tests__/package-config.test.ts +++ b/packages/cli/src/__tests__/package-config.test.ts @@ -299,10 +299,16 @@ describe("Workspace bootstrap script contract", () => { }); describe("Workflow YAML validity", () => { - it("ci.yml is valid YAML", () => { - const parsed = loadWorkflowYaml("ci.yml"); + it("pr-checks.yml is valid YAML", () => { + const parsed = loadWorkflowYaml("pr-checks.yml"); expect(parsed).toBeDefined(); - expect(parsed.name).toBe("CI"); + expect(parsed.name).toBe("PR Checks"); + }); + + it("full-suite.yml is valid YAML", () => { + const parsed = loadWorkflowYaml("full-suite.yml"); + expect(parsed).toBeDefined(); + expect(parsed.name).toBe("Full Suite (non-blocking)"); }); it("version.yml is valid YAML", () => { diff --git a/packages/cli/src/__tests__/root-test-command.test.ts b/packages/cli/src/__tests__/root-test-command.test.ts index d91c649a48..80b594b1a5 100644 --- a/packages/cli/src/__tests__/root-test-command.test.ts +++ b/packages/cli/src/__tests__/root-test-command.test.ts @@ -3,7 +3,7 @@ import { decideExecutionPlan, normalizeForwardedArgs, resolveAffectedPackages, - shouldForceFullSuite, + isSharedInfraChange, } from "../../../../scripts/test-changed.mjs"; import { computeSplitPlan, parseShardArgs, planShardAssignments, selectShardPackages } from "../../../../scripts/ci-test-shard.mjs"; @@ -24,7 +24,7 @@ describe("root test command changed-only planning", () => { expect(plan).toEqual({ mode: "changed", packages: ["@fusion/core", "@fusion/engine"] }); }); - it("falls back to full suite when shared test infra changes", () => { + it("routes to gate mode when shared test infra changes (no implicit full suite)", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", @@ -32,10 +32,10 @@ describe("root test command changed-only planning", () => { packageNameByDir: new Map([["packages/core", "@fusion/core"]]), }); - expect(plan).toEqual({ mode: "full", reason: "shared-infra-changed" }); + expect(plan).toEqual({ mode: "gate", reason: "shared-infra-changed" }); }); - it("falls back to full suite when comparison base cannot be resolved", () => { + it("routes to gate mode when comparison base cannot be resolved", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: null, @@ -43,18 +43,18 @@ describe("root test command changed-only planning", () => { packageNameByDir: new Map(), }); - expect(plan).toEqual({ mode: "full", reason: "missing-comparison-base" }); + expect(plan).toEqual({ mode: "gate", reason: "missing-comparison-base" }); }); - it("treats unknown package directories as full-suite fallback", () => { + it("treats unknown package directories as gate-mode fallback (resolver returns null)", () => { const resolved = resolveAffectedPackages(["packages/unknown/src/index.ts"], new Map()); expect(resolved).toBeNull(); }); - it("marks root workflow/config changes as full-suite triggers", () => { - expect(shouldForceFullSuite([".github/workflows/ci.yml"])).toBe(true); - expect(shouldForceFullSuite(["package.json"])).toBe(true); - expect(shouldForceFullSuite(["packages/core/src/store.ts"])).toBe(false); + it("marks root workflow/config changes as shared-infra (gate-mode) triggers", () => { + expect(isSharedInfraChange([".github/workflows/pr-checks.yml"])).toBe(true); + expect(isSharedInfraChange(["package.json"])).toBe(true); + expect(isSharedInfraChange(["packages/core/src/store.ts"])).toBe(false); }); it("strips forwarded silent flags so package vitest scripts do not receive duplicates", () => { diff --git a/packages/core/src/__tests__/db-migrate.test.ts b/packages/core/src/__tests__/db-migrate.test.ts index 2adac04a10..3621167bde 100644 --- a/packages/core/src/__tests__/db-migrate.test.ts +++ b/packages/core/src/__tests__/db-migrate.test.ts @@ -715,7 +715,7 @@ describe("schema migration", () => { const row = db.prepare("SELECT deletedAt FROM tasks WHERE id = 'FN-legacy'").get() as { deletedAt: string | null }; expect(row.deletedAt).toBeNull(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -748,7 +748,7 @@ describe("schema migration", () => { { id: "WS-001", mode: "prompt", gateMode: "advisory" }, { id: "WS-002", mode: "script", gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -798,7 +798,7 @@ describe("schema migration", () => { reviewerContextRetryCount: 0, reviewerFallbackRetryCount: 0, }); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -827,7 +827,7 @@ describe("schema migration", () => { const columns = db.prepare("PRAGMA table_info(milestones)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("acceptanceCriteria"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -868,7 +868,7 @@ describe("schema migration", () => { const missionColumns = db.prepare("PRAGMA table_info(missions)").all() as Array<{ name: string }>; expect(missionColumns.map((column) => column.name)).toContain("autoMerge"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -902,7 +902,7 @@ describe("schema migration", () => { { id: "WS-002", mode: "script", enabled: 1, gateMode: "advisory" }, { id: "WS-003", mode: "prompt", enabled: 0, gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -939,7 +939,7 @@ describe("schema migration", () => { const indexes = db.prepare("PRAGMA index_list(mission_goals)").all() as Array<{ name: string }>; expect(indexes.some((index) => index.name === "idxMissionGoalsGoalId")).toBe(true); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -1000,7 +1000,7 @@ describe("schema migration", () => { expect(customFieldsColumn).toBeDefined(); expect(customFieldsColumn?.dflt_value).toBe("'{}'"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -1053,7 +1053,7 @@ describe("schema migration", () => { expect(indexNames).toContain("idx_cli_sessions_chatSessionId"); expect(indexNames).toContain("idx_cli_sessions_project_state"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -1085,7 +1085,7 @@ describe("schema migration", () => { .all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("cliExecutorAdapterId"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -1095,7 +1095,81 @@ describe("schema migration", () => { const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all() as Array<{ name: string }>; expect(tables.map((row) => row.name)).toContain("cli_sessions"); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); + + it("adds workflows.kind + workflow_steps.migrated_fragment_id when migrating from schema version 108", () => { + const db = new Database(fusionDir); + db.exec("CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT)"); + db.exec("INSERT INTO __meta (key, value) VALUES ('schemaVersion', '108')"); + db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')"); + db.exec(` + CREATE TABLE IF NOT EXISTS workflows ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + ir TEXT NOT NULL, + layout TEXT NOT NULL DEFAULT '{}', + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + db.exec(` + CREATE TABLE IF NOT EXISTS workflow_steps ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + description TEXT NOT NULL, + mode TEXT NOT NULL DEFAULT 'prompt', + phase TEXT NOT NULL DEFAULT 'pre-merge', + prompt TEXT NOT NULL DEFAULT '', + enabled INTEGER NOT NULL DEFAULT 1, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + db.exec( + `INSERT INTO workflows (id, name, ir, createdAt, updatedAt) VALUES ('WF-legacy', 'Legacy', '{"version":"v1","name":"x","nodes":[],"edges":[]}', '2025-01-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z')`, + ); + db.exec( + "INSERT INTO workflow_steps (id, name, description, createdAt, updatedAt) VALUES ('WS-legacy', 'Legacy', 'desc', '2025-01-01T00:00:00.000Z', '2025-01-01T00:00:00.000Z')", + ); + + db.init(); + + const workflowColumns = db.prepare("PRAGMA table_info(workflows)").all() as Array<{ + name: string; + }>; + expect(workflowColumns.map((c) => c.name)).toContain("kind"); + // Existing rows default to 'workflow'. + const wfRow = db.prepare("SELECT kind FROM workflows WHERE id = 'WF-legacy'").get() as { kind: string }; + expect(wfRow.kind).toBe("workflow"); + + const stepColumns = db.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; + expect(stepColumns.map((c) => c.name)).toContain("migrated_fragment_id"); + const stepRow = db + .prepare("SELECT migrated_fragment_id FROM workflow_steps WHERE id = 'WS-legacy'") + .get() as { migrated_fragment_id: string | null }; + expect(stepRow.migrated_fragment_id).toBeNull(); + + expect(db.getSchemaVersion()).toBe(111); + db.close(); + }); + + it("migration 109 is idempotent on re-init", () => { + const db = new Database(fusionDir); + db.init(); + expect(db.getSchemaVersion()).toBe(111); + db.close(); + + // Re-open the same on-disk DB: already at 109, the 109 block must be a no-op. + const reopened = new Database(fusionDir); + reopened.init(); + expect(reopened.getSchemaVersion()).toBe(111); + const workflowColumns = reopened.prepare("PRAGMA table_info(workflows)").all() as Array<{ name: string }>; + expect(workflowColumns.filter((c) => c.name === "kind")).toHaveLength(1); + const stepColumns = reopened.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; + expect(stepColumns.filter((c) => c.name === "migrated_fragment_id")).toHaveLength(1); + reopened.close(); + }); }); diff --git a/packages/core/src/__tests__/db.test.ts b/packages/core/src/__tests__/db.test.ts index 974e0be1c2..e5ba441541 100644 --- a/packages/core/src/__tests__/db.test.ts +++ b/packages/core/src/__tests__/db.test.ts @@ -334,7 +334,7 @@ describe("Database", () => { }); it("seeds schema version", () => { - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); it("includes tokenUsageCacheWriteTokens on freshly initialized tasks table", () => { @@ -393,7 +393,7 @@ describe("Database", () => { it("is idempotent - calling init() twice does not fail", () => { expect(() => db.init()).not.toThrow(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); it("does not overwrite existing config on re-init", () => { // Update the config @@ -1463,7 +1463,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 (includes v1→v2 through v26→v29) - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -1488,11 +1488,11 @@ describe("schema migrations", () => { const db = new Database(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); // Re-init should not fail db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); db.close(); }); @@ -1527,7 +1527,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("priority"); @@ -1568,7 +1568,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1640,7 +1640,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1880,7 +1880,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("attachments"); @@ -1954,7 +1954,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "agentRatings" }]); @@ -1978,7 +1978,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "mission_events" }]); @@ -2082,7 +2082,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -2301,7 +2301,7 @@ describe("schema migrations", () => { localDb.init(); - expect(localDb.getSchemaVersion()).toBe(110); + expect(localDb.getSchemaVersion()).toBe(111); const columns = localDb.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("tokenUsageCacheWriteTokens"); @@ -2612,7 +2612,7 @@ describe("createDatabase factory", () => { const db = createDatabase(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); expect(db.getLastModified()).toBeGreaterThan(0); db.close(); @@ -2766,7 +2766,7 @@ describe("migration v77 task token budget columns", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(110); + expect(migrated.getSchemaVersion()).toBe(111); const rows = migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const names = new Set(rows.map((row) => row.name)); expect(names.has("tokenBudgetSoftAlertedAt")).toBe(true); @@ -2797,7 +2797,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(110); + expect(fresh.getSchemaVersion()).toBe(111); const names = new Set( (fresh.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2825,7 +2825,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(110); + expect(migrated.getSchemaVersion()).toBe(111); const names = new Set( (migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2851,7 +2851,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(110); + expect(fresh.getSchemaVersion()).toBe(111); const table = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2885,7 +2885,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(110); + expect(migrated.getSchemaVersion()).toBe(111); const table = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2926,7 +2926,7 @@ describe("migration v67 drops orphan project auth tables", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(110); + expect(migrated.getSchemaVersion()).toBe(111); const tables = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; @@ -2953,7 +2953,7 @@ describe("migration v67 drops orphan project auth tables", () => { try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(110); + expect(fresh.getSchemaVersion()).toBe(111); const tables = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; diff --git a/packages/core/src/__tests__/goals-schema.test.ts b/packages/core/src/__tests__/goals-schema.test.ts index 73d1e70d50..0f75a1d8b1 100644 --- a/packages/core/src/__tests__/goals-schema.test.ts +++ b/packages/core/src/__tests__/goals-schema.test.ts @@ -91,6 +91,6 @@ describe("goals schema", () => { }); it("reports schema version 101", () => { - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); }); diff --git a/packages/core/src/__tests__/insight-store.test.ts b/packages/core/src/__tests__/insight-store.test.ts index e6d5b4bcb1..34095d580a 100644 --- a/packages/core/src/__tests__/insight-store.test.ts +++ b/packages/core/src/__tests__/insight-store.test.ts @@ -1000,7 +1000,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh database at v33 (runs all migrations up to 33) const db1 = createDatabase(legacyDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(110); + expect(db1.getSchemaVersion()).toBe(111); db1.close(); // Step 2: Manually downgrade to version 32 and drop insight tables @@ -1035,7 +1035,7 @@ describe("Migration: pre-33 DB upgrade", () => { expect(tableNamesBefore).not.toContain("project_insight_runs"); // Now run init — this triggers the v32→v33 migration db3.init(); - expect(db3.getSchemaVersion()).toBe(110); + expect(db3.getSchemaVersion()).toBe(111); // Step 4: Verify insight tables exist after migration const tablesAfter = db3.prepare( @@ -1066,12 +1066,12 @@ describe("Migration: pre-33 DB upgrade", () => { try { const db1 = createDatabase(testDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(110); + expect(db1.getSchemaVersion()).toBe(111); db1.close(); const db2 = createDatabase(testDir); expect(() => db2.init()).not.toThrow(); - expect(db2.getSchemaVersion()).toBe(110); + expect(db2.getSchemaVersion()).toBe(111); db2.close(); } finally { rmSync(testDir, { recursive: true, force: true }); @@ -1085,7 +1085,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh DB and run migrations const db1 = createDatabase(compatDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(110); + expect(db1.getSchemaVersion()).toBe(111); // Step 2: Strip lifecycle and cancelledAt columns by recreating the // table without them. This simulates a DB that was created before the diff --git a/packages/core/src/__tests__/merge-request-record.test.ts b/packages/core/src/__tests__/merge-request-record.test.ts index 193e76bf7e..1f542406fd 100644 --- a/packages/core/src/__tests__/merge-request-record.test.ts +++ b/packages/core/src/__tests__/merge-request-record.test.ts @@ -38,7 +38,7 @@ describe("TaskStore merge request record + completion handoff marker", () => { .all() as Array<{ name: string }>; expect(tableRows).toEqual([{ name: "completion_handoff_markers" }, { name: "merge_requests" }]); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); it("upserts merge request records", async () => { diff --git a/packages/core/src/__tests__/mission-store.test.ts b/packages/core/src/__tests__/mission-store.test.ts index 18830ed7c9..db19c82852 100644 --- a/packages/core/src/__tests__/mission-store.test.ts +++ b/packages/core/src/__tests__/mission-store.test.ts @@ -3746,7 +3746,7 @@ describe("MissionStore", () => { describe("Loop State & Validator Run Schema (v31)", () => { it("schema version is 101 after migration", () => { - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); it("mission_features table has loop state columns", () => { diff --git a/packages/core/src/__tests__/run-audit.test.ts b/packages/core/src/__tests__/run-audit.test.ts index 0b4a7864d3..26632a8cff 100644 --- a/packages/core/src/__tests__/run-audit.test.ts +++ b/packages/core/src/__tests__/run-audit.test.ts @@ -584,7 +584,7 @@ describe("Run Audit", () => { }); it("schema version is bumped to 40", () => { - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); }); }); }); diff --git a/packages/core/src/__tests__/store-merge-queue.test.ts b/packages/core/src/__tests__/store-merge-queue.test.ts index ae8bdd7c66..cc93ed8ad8 100644 --- a/packages/core/src/__tests__/store-merge-queue.test.ts +++ b/packages/core/src/__tests__/store-merge-queue.test.ts @@ -60,7 +60,7 @@ describe("TaskStore merge queue", () => { expect.arrayContaining(["idx_mergeQueue_lease_ready", "idx_mergeQueue_leaseExpiresAt"]), ); - expect(store.getDatabase().getSchemaVersion()).toBe(110); + expect(store.getDatabase().getSchemaVersion()).toBe(111); }); it("migrates a legacy v88 database and preserves task rows", async () => { diff --git a/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts b/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts new file mode 100644 index 0000000000..60de85e497 --- /dev/null +++ b/packages/core/src/__tests__/strip-approval-bypass-flags.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect } from "vitest"; +import { stripApprovalBypassFlags } from "../workflow-ir.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; + +/** + * P0 security helper: removes the CLI-approval-bypass flags + * (`cliSkipApproval`/`autoApprove`) from every node config, recursing into + * foreach `config.template.nodes` at any nesting depth. + */ +describe("stripApprovalBypassFlags", () => { + it("removes both flags from a top-level node config and reports stripped:true", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [{ id: "n1", kind: "prompt", config: { cliSkipApproval: true, autoApprove: true, name: "x" } }], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const cfg = (out as any).nodes[0].config; + expect(cfg.cliSkipApproval).toBeUndefined(); + expect(cfg.autoApprove).toBeUndefined(); + expect(cfg.name).toBe("x"); // unrelated config preserved + }); + + it("strips nested foreach-in-foreach template nodes (arbitrary depth)", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [ + { + id: "outer", + kind: "foreach", + config: { + template: { + nodes: [ + { + id: "inner-foreach", + kind: "foreach", + config: { + template: { + nodes: [ + { id: "deep", kind: "step-execute", config: { autoApprove: true } }, + ], + edges: [], + }, + }, + }, + ], + edges: [], + }, + }, + }, + ], + edges: [], + } as unknown as WorkflowIr; + const { stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const deep = (ir as any).nodes[0].config.template.nodes[0].config.template.nodes[0]; + expect(deep.config.autoApprove).toBeUndefined(); + }); + + it("returns stripped:false when no flags present", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [{ id: "n1", kind: "prompt", config: { name: "x" } }], + edges: [], + } as unknown as WorkflowIr; + expect(stripApprovalBypassFlags(ir).stripped).toBe(false); + }); + + it("tolerates a non-array nodes field", () => { + const ir = { version: "v1", name: "wf" } as unknown as WorkflowIr; + expect(stripApprovalBypassFlags(ir).stripped).toBe(false); + }); + + it("tolerates non-object entries in nodes (untrusted input)", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [null, "bogus", 42, { id: "n1", kind: "prompt", config: { cliSkipApproval: true } }], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + expect((out as any).nodes[3].config.cliSkipApproval).toBeUndefined(); + }); + + it("tolerates non-object entries in nested template.nodes", () => { + const ir = { + version: "v1", + name: "wf", + nodes: [ + { + id: "fe", + kind: "foreach", + config: { + template: { nodes: [null, 0, "x", { id: "inner", kind: "prompt", config: { autoApprove: true } }] }, + }, + }, + ], + edges: [], + } as unknown as WorkflowIr; + const { ir: out, stripped } = stripApprovalBypassFlags(ir); + expect(stripped).toBe(true); + expect((out as any).nodes[0].config.template.nodes[3].config.autoApprove).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/task-documents.test.ts b/packages/core/src/__tests__/task-documents.test.ts index 33417c08a4..4f0f9fbbb3 100644 --- a/packages/core/src/__tests__/task-documents.test.ts +++ b/packages/core/src/__tests__/task-documents.test.ts @@ -51,7 +51,7 @@ describe("TaskStore task documents", () => { expect(tableNames.has("task_documents")).toBe(true); expect(tableNames.has("task_document_revisions")).toBe(true); - expect(db.getSchemaVersion()).toBe(110); + expect(db.getSchemaVersion()).toBe(111); const index = db .prepare( diff --git a/packages/core/src/__tests__/workflow-definition-store.test.ts b/packages/core/src/__tests__/workflow-definition-store.test.ts index 7d78accce5..64860b46c2 100644 --- a/packages/core/src/__tests__/workflow-definition-store.test.ts +++ b/packages/core/src/__tests__/workflow-definition-store.test.ts @@ -180,4 +180,132 @@ describe("TaskStore workflow definitions (U1)", () => { const c = await store.createWorkflowDefinition({ name: "C", ir: makeIr() }); expect(c.id).toBe("WF-003"); }); + + // ── kind discriminator (U1, R6/KTD-1) ──────────────────────────────── + + // A pure-v1 start→node→end fragment IR. + function fragmentIr(): WorkflowIr { + return { + version: "v1", + name: "frag", + nodes: [ + { id: "start", kind: "start" }, + { id: "step-1", kind: "prompt", config: { name: "Doc", gateMode: "advisory", prompt: "doc it" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "step-1", condition: "success" }, + { from: "step-1", to: "end", condition: "success" }, + ], + }; + } + + it("defaults a created workflow to kind 'workflow'", async () => { + const created = await store.createWorkflowDefinition({ name: "W", ir: makeIr() }); + expect(created.kind).toBe("workflow"); + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("workflow"); + }); + + it("persists and round-trips kind 'fragment' (INSERT includes kind)", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + expect(created.kind).toBe("fragment"); + // Raw column persisted. + const raw = (store as any).db.prepare("SELECT kind FROM workflows WHERE id = ?").get(created.id) as { kind: string }; + expect(raw.kind).toBe("fragment"); + // Reload. + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("fragment"); + }); + + it("preserves kind across updateWorkflowDefinition", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const updated = await store.updateWorkflowDefinition(created.id, { description: "edited" }); + expect(updated.kind).toBe("fragment"); + expect((await store.getWorkflowDefinition(created.id))?.kind).toBe("fragment"); + }); + + it("listWorkflowDefinitions({kind:'fragment'}) returns only fragments", async () => { + await store.createWorkflowDefinition({ name: "W1", ir: makeIr() }); + const frag = await store.createWorkflowDefinition({ name: "F1", ir: fragmentIr(), kind: "fragment" }); + const fragments = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(fragments.map((w) => w.id)).toEqual([frag.id]); + expect(fragments.every((w) => w.kind === "fragment")).toBe(true); + }); + + it("built-in list entries are kind 'workflow'", async () => { + const all = await store.listWorkflowDefinitions(); + const builtins = all.filter((w) => isBuiltinWorkflowId(w.id)); + expect(builtins.length).toBeGreaterThan(0); + expect(builtins.every((w) => w.kind === "workflow")).toBe(true); + // The workflow filter includes built-ins; the fragment filter excludes them. + expect((await store.listWorkflowDefinitions({ kind: "workflow" })).some((w) => isBuiltinWorkflowId(w.id))).toBe(true); + expect((await store.listWorkflowDefinitions({ kind: "fragment" })).some((w) => isBuiltinWorkflowId(w.id))).toBe(false); + }); + + it("cache regression: filtered then unfiltered (and reverse) are both correct", async () => { + await store.createWorkflowDefinition({ name: "W1", ir: makeIr() }); + const frag = await store.createWorkflowDefinition({ name: "F1", ir: fragmentIr(), kind: "fragment" }); + + // filtered → unfiltered + const f1 = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(f1.map((w) => w.id)).toEqual([frag.id]); + const allAfterFiltered = await store.listWorkflowDefinitions(); + expect(allAfterFiltered.filter((w) => !isBuiltinWorkflowId(w.id)).map((w) => w.kind).sort()).toEqual([ + "fragment", + "workflow", + ]); + + // unfiltered → filtered (cache already populated by the unfiltered call) + const f2 = await store.listWorkflowDefinitions({ kind: "fragment" }); + expect(f2.map((w) => w.id)).toEqual([frag.id]); + const w2 = await store.listWorkflowDefinitions({ kind: "workflow" }); + expect(w2.filter((w) => !isBuiltinWorkflowId(w.id)).every((w) => w.kind === "workflow")).toBe(true); + }); + + it("a fragment IR survives downgradeIrToV1IfPure unchanged (persists as v1)", async () => { + const created = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const raw = (store as any).db.prepare("SELECT ir FROM workflows WHERE id = ?").get(created.id) as { ir: string }; + expect(JSON.parse(raw.ir).version).toBe("v1"); + }); + + it("selectTaskWorkflow rejects a fragment id with a clear error", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + // Create a task to select against. + const task = await store.createTask({ description: "t" }); + await expect(store.selectTaskWorkflow(task.id, frag.id)).rejects.toThrow(/fragment/i); + }); + + it("setDefaultWorkflowId rejects a fragment id at the write boundary", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + await expect(store.setDefaultWorkflowId(frag.id)).rejects.toThrow(/fragment/i); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("setDefaultWorkflowId accepts a real workflow and clears with null", async () => { + const wf = await store.createWorkflowDefinition({ name: "W", ir: makeIr() }); + await store.setDefaultWorkflowId(wf.id); + expect(await store.getDefaultWorkflowId()).toBe(wf.id); + await store.setDefaultWorkflowId(null); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("createTaskWithReservedId honors an explicit workflowId (precedence over default)", async () => { + const def = await store.createWorkflowDefinition({ name: "Explicit", ir: makeIr() }); + const task = await store.createTaskWithReservedId( + { description: "t", workflowId: def.id }, + { taskId: "task-explicit-wf" }, + ); + const sel = store.getTaskWorkflowSelection(task.id); + expect(sel?.workflowId).toBe(def.id); + }); + + it("createTaskWithReservedId treats workflowId:null as explicit opt-out", async () => { + const def = await store.createWorkflowDefinition({ name: "Def", ir: makeIr() }); + await store.setDefaultWorkflowId(def.id); + const task = await store.createTaskWithReservedId( + { description: "t", workflowId: null }, + { taskId: "task-optout-wf" }, + ); + const sel = store.getTaskWorkflowSelection(task.id); + expect(sel?.workflowId ?? undefined).toBeUndefined(); + }); }); diff --git a/packages/core/src/__tests__/workflow-selection-store.test.ts b/packages/core/src/__tests__/workflow-selection-store.test.ts index cfc4b37157..840715be59 100644 --- a/packages/core/src/__tests__/workflow-selection-store.test.ts +++ b/packages/core/src/__tests__/workflow-selection-store.test.ts @@ -23,6 +23,23 @@ function linearIr(): WorkflowIr { }; } +/** A single-node fragment IR (start → one node → end). */ +function fragmentIr(): WorkflowIr { + return { + version: "v1", + name: "frag", + nodes: [ + { id: "start", kind: "start" }, + { id: "step-1", kind: "prompt", config: { name: "Doc", prompt: "doc it" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "step-1", condition: "success" }, + { from: "step-1", to: "end", condition: "success" }, + ], + }; +} + function branchingIr(): WorkflowIr { return { version: "v1", @@ -172,4 +189,71 @@ describe("TaskStore workflow selection (U3)", () => { await store.setDefaultWorkflowId(null); expect(await store.getDefaultWorkflowId()).toBeUndefined(); }); + + // U6/R3/KTD-4: create-time `workflowId` materializes the selection atomically. + describe("create-time workflowId (U6/R3)", () => { + it("materializes enabledWorkflowSteps atomically when workflowId is given", async () => { + const wf = await store.createWorkflowDefinition({ name: "Pick", ir: linearIr() }); + + const task = await store.createTask({ description: "with workflow", workflowId: wf.id }); + // Reading the task right after create observes the populated steps — no + // intermediate empty state visible to the executor. + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(wf.id); + expect(store.getTaskWorkflowSelection(task.id)?.stepIds).toEqual(detail.enabledWorkflowSteps); + }); + + it("explicit workflowId overrides the project default", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + const chosen = await store.createWorkflowDefinition({ name: "Chosen", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "override default", workflowId: chosen.id }); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(chosen.id); + }); + + it("workflowId: null skips default materialization (explicit No workflow)", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "no workflow", workflowId: null }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps ?? []).toHaveLength(0); + expect(store.getTaskWorkflowSelection(task.id)).toBeUndefined(); + }); + + it("undefined workflowId still inherits the project default (unchanged)", async () => { + const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + await store.setDefaultWorkflowId(def.id); + + const task = await store.createTask({ description: "inherit" }); + const detail = await store.getTask(task.id); + expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(def.id); + }); + + it("rejects a fragment id before creating the task row", async () => { + const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" }); + const before = (await store.listTasks({ includeArchived: true })).length; + + await expect( + store.createTask({ description: "frag pick", workflowId: frag.id }), + ).rejects.toThrow(/fragment/i); + + const after = (await store.listTasks({ includeArchived: true })).length; + expect(after).toBe(before); + }); + + it("rejects an unknown workflow id before creating the task row", async () => { + const before = (await store.listTasks({ includeArchived: true })).length; + + await expect( + store.createTask({ description: "bad pick", workflowId: "WF-404" }), + ).rejects.toThrow(/not found/i); + + const after = (await store.listTasks({ includeArchived: true })).length; + expect(after).toBe(before); + }); + }); }); diff --git a/packages/core/src/__tests__/workflow-step-migration.test.ts b/packages/core/src/__tests__/workflow-step-migration.test.ts new file mode 100644 index 0000000000..62d47bf2d3 --- /dev/null +++ b/packages/core/src/__tests__/workflow-step-migration.test.ts @@ -0,0 +1,194 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { isBuiltinWorkflowId } from "../builtin-workflows.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/** + * U2 / R5 / KTD-3 — lazy idempotent migration of legacy user-authored workflow + * steps into the dual fragment + combined-workflow representation. + */ +describe("TaskStore.migrateLegacyWorkflowSteps (U2/R5)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + + afterEach(async () => { + await harness.afterEach(); + }); + + /** User-owned (non-builtin) workflow definitions only. */ + async function userDefs() { + return (await store.listWorkflowDefinitions()).filter((w) => !isBuiltinWorkflowId(w.id)); + } + + it("converts defaultOn + optional + disabled user steps to fragments, builds the combined workflow from defaultOn only, sets the project default, and leaves the compiled row untouched", async () => { + // defaultOn (ran automatically on new tasks) → fragment + joins combined workflow. + const on = await store.createWorkflowStep({ + name: "Default On", + description: "ran by default", + prompt: "do the default thing", + defaultOn: true, + enabled: true, + }); + // enabled-but-optional → fragment only (NOT in combined workflow). + const optional = await store.createWorkflowStep({ + name: "Optional", + description: "opt-in", + prompt: "optional work", + defaultOn: false, + enabled: true, + }); + // disabled → still gets a fragment (every user step does). + const disabled = await store.createWorkflowStep({ + name: "Disabled", + description: "off", + prompt: "disabled work", + defaultOn: false, + enabled: false, + }); + // compiled-materialized row (execution detail) → must be ignored entirely. + const compiled = await store.createWorkflowStep({ + name: "Compiled", + description: "materialized", + templateId: "workflow:WF-999", + defaultOn: true, + enabled: true, + }); + + const result = await store.migrateLegacyWorkflowSteps(); + + // 3 user steps converted; nothing previously migrated. + expect(result.migrated).toBe(3); + expect(result.skipped).toBe(0); + expect(result.combinedWorkflowId).toBeTruthy(); + + const defs = await userDefs(); + const fragments = defs.filter((d) => d.kind === "fragment"); + const workflows = defs.filter((d) => d.kind === "workflow"); + + // Exactly 3 fragments (one per user step), exactly 1 combined workflow. + expect(fragments).toHaveLength(3); + expect(workflows).toHaveLength(1); + expect(fragments.map((f) => f.name).sort()).toEqual(["Default On", "Disabled", "Optional"]); + + // Combined workflow: named "Migrated steps", carries the system description, + // and contains ONLY the defaultOn step's user node (plus start/end + seams). + const combined = workflows[0]; + expect(combined.id).toBe(result.combinedWorkflowId); + expect(combined.name).toBe("Migrated steps"); + expect(combined.description).toBe("Converted from your legacy workflow steps"); + const userNodes = combined.ir.nodes.filter( + (n) => n.kind !== "start" && n.kind !== "end" && typeof n.config?.seam !== "string", + ); + expect(userNodes).toHaveLength(1); + expect(userNodes[0].config?.name).toBe("Default On"); + + // Project default points at the combined workflow. + expect(await store.getDefaultWorkflowId()).toBe(combined.id); + + // All 3 user source rows are stamped; the compiled row is untouched. + expect((await store.getWorkflowStep(on.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(optional.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(disabled.id))?.migratedFragmentId).toBeTruthy(); + expect((await store.getWorkflowStep(compiled.id))?.migratedFragmentId).toBeUndefined(); + + // No source records were deleted. + const steps = await store.listWorkflowSteps(); + expect(steps.map((s) => s.id)).toEqual(expect.arrayContaining([on.id, optional.id, disabled.id])); + }); + + it("creates fragments but NO combined workflow and leaves the default unchanged when no step is defaultOn", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: false }); + await store.createWorkflowStep({ name: "B", description: "b", prompt: "b", enabled: false }); + + const result = await store.migrateLegacyWorkflowSteps(); + + expect(result.migrated).toBe(2); + expect(result.combinedWorkflowId).toBeUndefined(); + + const defs = await userDefs(); + expect(defs.filter((d) => d.kind === "fragment")).toHaveLength(2); + expect(defs.filter((d) => d.kind === "workflow")).toHaveLength(0); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); + + it("is idempotent: a second run converts nothing and creates no new definitions", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + + const first = await store.migrateLegacyWorkflowSteps(); + expect(first.migrated).toBe(1); + const afterFirst = (await userDefs()).length; + + const second = await store.migrateLegacyWorkflowSteps(); + expect(second.migrated).toBe(0); + expect(second.skipped).toBe(1); + expect(second.combinedWorkflowId).toBeUndefined(); + expect((await userDefs()).length).toBe(afterFirst); + }); + + it("does not clobber a pre-existing project default", async () => { + // A user-chosen default workflow exists before migration. + const existing = await store.createWorkflowDefinition({ + name: "My choice", + ir: { + version: "v1", + name: "My choice", + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + }, + kind: "workflow", + }); + await store.setDefaultWorkflowId(existing.id); + + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + const result = await store.migrateLegacyWorkflowSteps(); + + // The combined workflow is still created, but the explicit default is kept. + expect(result.combinedWorkflowId).toBeTruthy(); + expect(await store.getDefaultWorkflowId()).toBe(existing.id); + }); + + it("compare-and-set: re-reads the default after the transaction and skips when a concurrent writer set one", async () => { + await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); + + const concurrent = await store.createWorkflowDefinition({ + name: "Concurrent", + ir: { + version: "v1", + name: "Concurrent", + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + }, + kind: "workflow", + }); + + // A project default exists when migration's post-transaction compare-and-set + // re-reads it. Because the set is gated on the re-read (not a pre-transaction + // snapshot), an existing default is observed and never clobbered. + await store.setDefaultWorkflowId(concurrent.id); + + const result = await store.migrateLegacyWorkflowSteps(); + + expect(result.combinedWorkflowId).toBeTruthy(); + expect(result.combinedWorkflowId).not.toBe(concurrent.id); + // The compare-and-set re-read observed the existing default and did NOT clobber it. + expect(await store.getDefaultWorkflowId()).toBe(concurrent.id); + }); + + it("is a no-op with zero user steps", async () => { + const result = await store.migrateLegacyWorkflowSteps(); + expect(result).toEqual({ migrated: 0, skipped: 0, combinedWorkflowId: undefined }); + expect(await userDefs()).toHaveLength(0); + expect(await store.getDefaultWorkflowId()).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/workflow-steps-to-ir.test.ts b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts new file mode 100644 index 0000000000..de041be2d3 --- /dev/null +++ b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts @@ -0,0 +1,226 @@ +import { describe, it, expect } from "vitest"; + +import { stepsToWorkflowIr, stepToFragmentIr, layoutForIr } from "../workflow-steps-to-ir.js"; +import { compileWorkflowToSteps } from "../workflow-compiler.js"; +import { parseWorkflowIr } from "../workflow-ir.js"; +import type { WorkflowStep, WorkflowStepInput } from "../types.js"; + +/** Build a fully-specified WorkflowStep fixture. */ +function step(overrides: Partial): WorkflowStep { + return { + id: overrides.id ?? "WS-000", + name: overrides.name ?? "Step", + description: overrides.description ?? "", + mode: overrides.mode ?? "prompt", + phase: overrides.phase, + gateMode: overrides.gateMode ?? "advisory", + prompt: overrides.prompt ?? "", + toolMode: overrides.toolMode, + scriptName: overrides.scriptName, + enabled: overrides.enabled ?? true, + defaultOn: overrides.defaultOn, + modelProvider: overrides.modelProvider, + modelId: overrides.modelId, + migratedFragmentId: overrides.migratedFragmentId, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; +} + +/** Project a compiled step input down to exactly the compiler-visible fields the + * round-trip contract pins (KTD-2). Normalizes optional fields for comparison. */ +function visible(input: WorkflowStepInput) { + return { + name: input.name, + mode: input.mode, + phase: input.phase, + gateMode: input.gateMode, + prompt: input.mode === "script" ? undefined : (input.prompt ?? ""), + scriptName: input.scriptName, + toolMode: input.mode === "script" ? undefined : input.toolMode, + modelProvider: input.modelProvider, + modelId: input.modelId, + }; +} + +function visibleStep(s: WorkflowStep) { + return { + name: s.name, + mode: s.mode, + phase: s.phase ?? "pre-merge", + gateMode: s.gateMode, + prompt: s.mode === "script" ? undefined : (s.prompt ?? ""), + scriptName: s.mode === "script" ? s.scriptName : undefined, + toolMode: s.mode === "script" ? undefined : (s.toolMode ?? "readonly"), + modelProvider: s.mode === "prompt" ? s.modelProvider : undefined, + modelId: s.mode === "prompt" ? s.modelId : undefined, + }; +} + +describe("stepsToWorkflowIr — round-trip parity (R4/KTD-2)", () => { + it("reproduces every compiler-visible field for a mixed step set", () => { + const steps: WorkflowStep[] = [ + step({ + id: "WS-1", + name: "Implement", + description: "do the work", + mode: "prompt", + gateMode: "advisory", + prompt: "Implement the change", + toolMode: "coding", + phase: "pre-merge", + }), + step({ + id: "WS-2", + name: "Lint", + mode: "script", + gateMode: "gate", + scriptName: "lint", + phase: "pre-merge", + }), + step({ + id: "WS-3", + name: "Security gate", + mode: "prompt", + gateMode: "gate", + prompt: "Block on exploitable findings", + toolMode: "readonly", + modelProvider: "anthropic", + modelId: "claude-sonnet-4-5", + phase: "pre-merge", + }), + step({ + id: "WS-4", + name: "Document", + mode: "prompt", + gateMode: "advisory", + prompt: "Write docs", + phase: "post-merge", + }), + step({ + id: "WS-5", + name: "Deploy script", + mode: "script", + gateMode: "advisory", + scriptName: "deploy", + phase: "post-merge", + }), + ]; + + const ir = stepsToWorkflowIr(steps, "Migrated"); + const compiled = compileWorkflowToSteps(ir); + + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); + + it("undefined phase maps to pre-merge and round-trips", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" }), + step({ id: "WS-2", name: "B", mode: "prompt", gateMode: "advisory", prompt: "b" }), + ]; + const ir = stepsToWorkflowIr(steps, "AllUndefined"); + // parseable + expect(() => parseWorkflowIr(ir)).not.toThrow(); + const compiled = compileWorkflowToSteps(ir); + expect(compiled.map((c) => c.phase)).toEqual(["pre-merge", "pre-merge"]); + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); + + it("empty step list yields a minimal valid IR that compiles to []", () => { + const ir = stepsToWorkflowIr([], "Empty"); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + expect(compileWorkflowToSteps(ir)).toEqual([]); + // start + 3 seams + end. + expect(ir.nodes.map((n) => n.id)).toEqual(["start", "execute", "review", "merge", "end"]); + }); + + it("post-merge-only set places nodes after the merge seam", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "After", mode: "prompt", gateMode: "advisory", prompt: "x", phase: "post-merge" }), + ]; + const ir = stepsToWorkflowIr(steps, "PostOnly"); + const ids = ir.nodes.map((n) => n.id); + expect(ids.indexOf("merge")).toBeLessThan(ids.indexOf("step-1")); + const compiled = compileWorkflowToSteps(ir); + expect(compiled).toHaveLength(1); + expect(compiled[0].phase).toBe("post-merge"); + }); + + it("produced IR passes parseWorkflowIr and encodes seams exactly per linear()", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" }), + ]; + const ir = stepsToWorkflowIr(steps, "Seams"); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + + // Each seam appears exactly once, in execute → review → merge order. + const seamNodes = ir.nodes.filter((n) => typeof n.config?.seam === "string"); + expect(seamNodes.map((n) => n.config!.seam)).toEqual(["execute", "review", "merge"]); + + // Each seam has a failure → end edge. + for (const seam of ["execute", "review", "merge"]) { + const failEdge = ir.edges.find((e) => e.from === seam && e.condition === "failure"); + expect(failEdge?.to).toBe("end"); + } + // No duplicate failure edges per seam. + const failureEdges = ir.edges.filter((e) => e.condition === "failure"); + expect(failureEdges).toHaveLength(3); + }); + + it("gate vs advisory both round-trip for prompt and script modes", () => { + const steps: WorkflowStep[] = [ + step({ id: "WS-1", name: "PG", mode: "prompt", gateMode: "gate", prompt: "p" }), + step({ id: "WS-2", name: "PA", mode: "prompt", gateMode: "advisory", prompt: "p" }), + step({ id: "WS-3", name: "SG", mode: "script", gateMode: "gate", scriptName: "s" }), + step({ id: "WS-4", name: "SA", mode: "script", gateMode: "advisory", scriptName: "s" }), + ]; + const compiled = compileWorkflowToSteps(stepsToWorkflowIr(steps, "Gates")); + expect(compiled.map((c) => c.gateMode)).toEqual(["gate", "advisory", "gate", "advisory"]); + expect(compiled.map(visible)).toEqual(steps.map(visibleStep)); + }); +}); + +describe("stepToFragmentIr (R6/KTD-1)", () => { + it("produces a parseable start → node → end fragment mirroring the step", () => { + const s = step({ + id: "WS-1", + name: "Doc", + description: "doc it", + mode: "prompt", + gateMode: "advisory", + prompt: "Document the change", + toolMode: "readonly", + }); + const ir = stepToFragmentIr(s); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + expect(ir.nodes.map((n) => n.id)).toEqual(["start", "step-1", "end"]); + expect(ir.nodes.map((n) => n.kind)).toEqual(["start", "prompt", "end"]); + + // The single node compiles back to a step mirroring the source. + const compiled = compileWorkflowToSteps(ir); + expect(compiled).toHaveLength(1); + expect(visible(compiled[0])).toEqual(visibleStep(s)); + }); + + it("fragment IR is pure v1 (no v2-only features)", () => { + const ir = stepToFragmentIr(step({ id: "WS-1", name: "S", mode: "script", gateMode: "gate", scriptName: "lint" })); + // parseWorkflowIr upgrades to v2 in-memory; the SOURCE we built is v1-shaped. + const compiled = compileWorkflowToSteps(ir); + expect(compiled[0].mode).toBe("script"); + expect(compiled[0].scriptName).toBe("lint"); + }); +}); + +describe("layoutForIr", () => { + it("produces x-spaced positions for every node", () => { + const ir = stepsToWorkflowIr( + [step({ id: "WS-1", name: "A", mode: "prompt", gateMode: "advisory", prompt: "a" })], + "L", + ); + const layout = layoutForIr(ir); + expect(Object.keys(layout).sort()).toEqual(ir.nodes.map((n) => n.id).sort()); + expect(layout.start).toEqual({ x: 60, y: 160 }); + // Second node is one column over. + expect(layout[ir.nodes[1].id].x).toBe(60 + 170); + }); +}); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 62e734cf44..270fda980d 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -47,6 +47,8 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { id: spec.id, name: spec.name, description: spec.description, + // Built-ins are always selectable workflows, never fragments (KTD-1). + kind: "workflow", ir, layout, createdAt: BUILTIN_TS, @@ -152,6 +154,7 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ name: "Stepwise coding (built-in)", description: "Per-step plan, execute, and review modeled as graph structure: each planned step runs and is reviewed (approve / revise / rethink) before the next, with bounded rework. Requires the workflow graph executor.", + kind: "workflow", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, layout: { start: { x: 60, y: 160 }, diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index c4c824f90a..6e3265fc6e 100644 --- a/packages/core/src/db.ts +++ b/packages/core/src/db.ts @@ -149,7 +149,7 @@ export function probeFts5(db: DatabaseSync): boolean { // ── Schema Definition ──────────────────────────────────────────────── -const SCHEMA_VERSION = 110; +const SCHEMA_VERSION = 111; export { SCHEMA_VERSION }; @@ -385,6 +385,10 @@ CREATE TABLE IF NOT EXISTS workflow_steps ( defaultOn INTEGER DEFAULT 0, modelProvider TEXT, modelId TEXT, + -- (workflow-editor-consolidation U1/U2) when this step has been migrated into a + -- fragment WorkflowDefinition, the fragment's id is stamped here so re-runs of + -- the lazy migration skip already-migrated rows (marker idempotency). + migrated_fragment_id TEXT, createdAt TEXT NOT NULL, updatedAt TEXT NOT NULL ); @@ -398,6 +402,11 @@ CREATE TABLE IF NOT EXISTS workflows ( description TEXT NOT NULL DEFAULT '', ir TEXT NOT NULL, layout TEXT NOT NULL DEFAULT '{}', + -- (workflow-editor-consolidation U1, KTD-1) discriminates reusable single-node + -- "fragment" templates from full "workflow" definitions. Fragments never appear + -- in task workflow pickers, default-workflow selection, or compile/selection + -- paths. Legacy rows default to 'workflow'. + kind TEXT NOT NULL DEFAULT 'workflow', createdAt TEXT NOT NULL, updatedAt TEXT NOT NULL ); @@ -4308,19 +4317,24 @@ export class Database { }); } - // Migration 109: Durable CLI agent session records (CLI Agent Executor U1). - // Adds cli_sessions — one row per long-lived CLI agent session (task - // execution, planning, validator, ce, or chat) — so a crashed/restarted - // Fusion instance can reason about, resume, or reap sessions from their - // persisted state (agentState + terminationReason + resumeAttempts + - // nativeSessionId). taskId/chatSessionId are the nullable owning-entity - // references; autonomyPosture is JSON. Additive-only, idempotent - // (table-exists guard); no backfill. - // agentState ∈ starting|ready|busy|waitingOnInput|done|dead|needsAttention. - // terminationReason ∈ completed|userExited|killed|crashed|authFailed|engineDeath. - // purpose ∈ execute|planning|validator|ce|chat. + // Migration 109: Workflow editor consolidation. Adds workflows.kind + // (fragment vs workflow discriminator; existing rows default 'workflow') + // and workflow_steps.migrated_fragment_id (idempotent lazy step migration). + // Additive-only, idempotent (addColumnIfMissing guards); no backfill. if (version < 109) { this.applyMigration(109, () => { + this.addColumnIfMissing("workflows", "kind", "TEXT NOT NULL DEFAULT 'workflow'"); + this.addColumnIfMissing("workflow_steps", "migrated_fragment_id", "TEXT"); + }); + } + + // Migration 110: Durable CLI agent session records (CLI Agent Executor U1). + // cli_sessions — one row per long-lived CLI agent session. agentState ∈ + // starting|ready|busy|waitingOnInput|done|dead|needsAttention; terminationReason + // ∈ completed|userExited|killed|crashed|authFailed|engineDeath; purpose ∈ + // execute|planning|validator|ce|chat. Additive-only, idempotent. + if (version < 110) { + this.applyMigration(110, () => { this.db.exec(` CREATE TABLE IF NOT EXISTS cli_sessions ( id TEXT PRIMARY KEY, @@ -4345,12 +4359,9 @@ export class Database { }); } - // CLI Agent Executor (U12): per-chat-session selection of a cli-agent - // adapter. When set, the chat is CLI-backed — composer sends route through - // the inject path and adapter transcript events map to chat_messages rows. - // Null/empty means the chat uses the standard provider path. - if (version < 110) { - this.applyMigration(110, () => { + // Migration 111: per-chat-session cli-agent adapter selection (U12). + if (version < 111) { + this.applyMigration(111, () => { if (this.hasTable("chat_sessions")) { this.addColumnIfMissing("chat_sessions", "cliExecutorAdapterId", "TEXT"); } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d00b2d8f86..c2c177db06 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -49,6 +49,7 @@ export { export { parseWorkflowIr, serializeWorkflowIr, + stripApprovalBypassFlags, WorkflowIrError, DEFAULT_WORKFLOW_COLUMN_IDS, } from "./workflow-ir.js"; @@ -236,6 +237,7 @@ export type { WorkflowDefinition, WorkflowDefinitionInput, WorkflowDefinitionUpdate, + WorkflowDefinitionKind, WorkflowNodeLayout, } from "./workflow-definition-types.js"; export { @@ -243,6 +245,11 @@ export { validateLinearity, WorkflowCompileError, } from "./workflow-compiler.js"; +export { + stepsToWorkflowIr, + stepToFragmentIr, + layoutForIr, +} from "./workflow-steps-to-ir.js"; export { BUILTIN_WORKFLOWS, BUILTIN_WORKFLOW_ID_PREFIX, @@ -468,6 +475,7 @@ export { toJson, toJsonNullable, fromJson, + SCHEMA_VERSION, } from "./db.js"; export { ProjectIdentityConflictError, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index c451f6c708..02476f2a23 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -8,6 +8,7 @@ import { createActivityLogSnapshot, createRunAuditSnapshot, createTaskMetadataSn import { VALID_TRANSITIONS, COLUMNS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js"; import { DEFAULT_PROJECT_SETTINGS } from "./settings-schema.js"; import { parseWorkflowIr, serializeWorkflowIr, downgradeIrToV1IfPure } from "./workflow-ir.js"; +import { stepsToWorkflowIr, stepToFragmentIr, layoutForIr } from "./workflow-steps-to-ir.js"; import { isWorkflowColumnsEnabled } from "./workflow-columns-settings.js"; import { resolveAllowedColumns, workflowHasColumn } from "./workflow-transitions.js"; import { @@ -3738,6 +3739,7 @@ export class TaskStore extends EventEmitter { defaultOn: number | null; modelProvider: string | null; modelId: string | null; + migrated_fragment_id?: string | null; createdAt: string; updatedAt: string; }): import("./types.js").WorkflowStep { @@ -3758,6 +3760,7 @@ export class TaskStore extends EventEmitter { defaultOn: row.defaultOn === null || row.defaultOn === undefined ? undefined : Boolean(row.defaultOn), modelProvider: row.modelProvider ?? undefined, modelId: row.modelId ?? undefined, + migratedFragmentId: row.migrated_fragment_id ?? undefined, createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -3972,7 +3975,24 @@ export class TaskStore extends EventEmitter { // When a project default workflow is configured, new tasks inherit it // (compiled to steps) ahead of the legacy default-on step behavior. let pendingWorkflowSelection: { workflowId: string; stepIds: string[] } | undefined; - if (input.enabledWorkflowSteps === undefined) { + // U6/R3/KTD-4: an explicit create-time workflowId beats the project default. + // `null` is an explicit opt-out (no workflow), `string` materializes that + // workflow, `undefined` falls through to the default-workflow behavior below. + // Explicit enabledWorkflowSteps still wins over workflowId for trusted callers. + const explicitWorkflowId = + input.enabledWorkflowSteps === undefined ? input.workflowId : undefined; + if (explicitWorkflowId !== undefined) { + if (explicitWorkflowId === null) { + // Explicit "No workflow": skip default materialization entirely. + resolvedWorkflowSteps = undefined; + } else { + // Compile + materialize up front so unknown/fragment ids throw BEFORE + // the task row is created (no orphaned steps, no half-created task). + const selected = await this.materializeExplicitWorkflowSteps(explicitWorkflowId); + resolvedWorkflowSteps = selected.stepIds; + pendingWorkflowSelection = selected; + } + } else if (input.enabledWorkflowSteps === undefined) { try { const inherited = await this.materializeDefaultWorkflowSteps(); if (inherited) { @@ -4141,7 +4161,24 @@ export class TaskStore extends EventEmitter { : undefined; let pendingWorkflowSelection: { workflowId: string; stepIds: string[] } | undefined; - if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) { + // U6/R3/KTD-4: an explicit create-time workflowId beats the project default, + // mirroring createTask(). `null` is an explicit opt-out, `string` materializes + // that workflow, `undefined` falls through to the default-workflow behavior. + // Explicit enabledWorkflowSteps still wins over workflowId for trusted callers. + const explicitWorkflowId = + input.enabledWorkflowSteps === undefined ? input.workflowId : undefined; + if (explicitWorkflowId !== undefined) { + if (explicitWorkflowId === null) { + // Explicit "No workflow": skip default materialization entirely. + resolvedWorkflowSteps = undefined; + } else { + // Compile + materialize up front so unknown/fragment ids throw BEFORE + // the task row is created (no orphaned steps, no half-created task). + const selected = await this.materializeExplicitWorkflowSteps(explicitWorkflowId); + resolvedWorkflowSteps = selected.stepIds; + pendingWorkflowSelection = selected; + } + } else if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) { // Mirror createTask: a configured project default workflow takes // precedence over legacy default-on steps on this creation path too. try { @@ -11827,6 +11864,7 @@ ${stepsSection}`; defaultOn: input.defaultOn !== undefined ? input.defaultOn : undefined, modelProvider: mode === "prompt" ? input.modelProvider : undefined, modelId: mode === "prompt" ? input.modelId : undefined, + migratedFragmentId: input.migratedFragmentId, createdAt: now, updatedAt: now, }; @@ -11847,9 +11885,10 @@ ${stepsSection}`; defaultOn, modelProvider, modelId, + migrated_fragment_id, createdAt, updatedAt - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ).run( step.id, step.templateId ?? null, @@ -11865,6 +11904,7 @@ ${stepsSection}`; step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, step.modelProvider ?? null, step.modelId ?? null, + step.migratedFragmentId ?? null, step.createdAt, step.updatedAt, ); @@ -12087,6 +12127,7 @@ ${stepsSection}`; if ("modelProvider" in updates) step.modelProvider = updates.modelProvider; if ("modelId" in updates) step.modelId = updates.modelId; } + if ("migratedFragmentId" in updates) step.migratedFragmentId = updates.migratedFragmentId; step.updatedAt = new Date().toISOString(); this.db.prepare( @@ -12104,6 +12145,7 @@ ${stepsSection}`; defaultOn = ?, modelProvider = ?, modelId = ?, + migrated_fragment_id = ?, updatedAt = ? WHERE id = ?`, ).run( @@ -12120,6 +12162,7 @@ ${stepsSection}`; step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, step.modelProvider ?? null, step.modelId ?? null, + step.migratedFragmentId ?? null, step.updatedAt, step.id, ); @@ -12195,6 +12238,7 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; }): WorkflowDefinition { @@ -12202,6 +12246,8 @@ ${stepsSection}`; id: row.id, name: row.name, description: row.description, + // Legacy rows (pre-migration-109) have no kind column; default to "workflow". + kind: row.kind === "fragment" ? "fragment" : "workflow", ir: parseWorkflowIr(row.ir), layout: this.parseWorkflowLayout(row.layout), createdAt: row.createdAt, @@ -12256,6 +12302,9 @@ ${stepsSection}`; id, name, description: input.description ?? "", + // KTD-1: fragments are pure-v1 IRs and pass through downgradeIrToV1IfPure + // unchanged; default to "workflow" when the caller omits the kind. + kind: input.kind === "fragment" ? "fragment" : "workflow", ir, layout, createdAt: now, @@ -12264,8 +12313,8 @@ ${stepsSection}`; this.db .prepare( - `INSERT INTO workflows (id, name, description, ir, layout, createdAt, updatedAt) - VALUES (?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO workflows (id, name, description, ir, layout, kind, createdAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, ) .run( definition.id, @@ -12275,6 +12324,7 @@ ${stepsSection}`; flagOnForCreate ? definition.ir : downgradeIrToV1IfPure(definition.ir), ), JSON.stringify(definition.layout), + definition.kind, definition.createdAt, definition.updatedAt, ); @@ -12285,8 +12335,26 @@ ${stepsSection}`; }); } - /** List all workflow definitions, oldest first. Cached until a mutation. */ - async listWorkflowDefinitions(): Promise { + /** List workflow definitions, oldest first. The `kind` filter (KTD-1) selects + * only workflows or only fragments; omit it to get the full merged set. + * + * Cache invariant: `workflowDefinitionsCache` ALWAYS holds the full merged set + * (built-ins + every row of every kind). The `kind` filter is applied to a + * slice taken AFTER the cache read — a filtered result is never cached, so a + * filtered call can never poison an unfiltered consumer (or vice versa). + */ + async listWorkflowDefinitions( + options?: { kind?: WorkflowDefinition["kind"] }, + ): Promise { + const all = await this.readAllWorkflowDefinitions(); + if (options?.kind) return all.filter((wf) => wf.kind === options.kind); + return all; + } + + /** Read (and cache) the full merged workflow-definition set, oldest first. + * Built-in templates lead the list and cannot be edited/deleted; built-ins + * are always kind "workflow". */ + private async readAllWorkflowDefinitions(): Promise { if (this.workflowDefinitionsCache) return this.workflowDefinitionsCache; const rows = this.db.prepare("SELECT * FROM workflows ORDER BY createdAt ASC").all() as Array<{ id: string; @@ -12294,10 +12362,10 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; }>; - // Built-in templates lead the list and cannot be edited/deleted. this.workflowDefinitionsCache = [...BUILTIN_WORKFLOWS, ...rows.map((row) => this.toWorkflowDefinition(row))]; return this.workflowDefinitionsCache; } @@ -12315,6 +12383,7 @@ ${stepsSection}`; description: string; ir: string; layout: string; + kind?: string | null; createdAt: string; updatedAt: string; } @@ -12958,11 +13027,189 @@ ${stepsSection}`; if (workflowId) { const exists = await this.getWorkflowDefinition(workflowId); if (!exists) throw new Error(`Workflow '${workflowId}' not found`); + // KTD-1/R6: a fragment is a reusable palette piece, not a selectable + // workflow. Reject it at the write boundary so a fragment can never be + // persisted as the project default (the read-side skip in + // materializeDefaultWorkflowSteps remains as defense in depth). + if (exists.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be set as the project default`); + } } // null is updateSettings' explicit-delete sentinel for project keys. await this.updateSettings({ defaultWorkflowId: workflowId } as unknown as Partial); } + /** + * Synchronous workflow-definition insert used by migration (U2/KTD-3). Mirrors + * the persistence side of `createWorkflowDefinition` (validation + flag-aware + * downgrade + INSERT + cache bust) but stays synchronous so it can run inside + * `transactionImmediate`. The flag value is resolved by the async caller and + * passed in, since reading it is async. + */ + private insertWorkflowDefinitionSync( + input: WorkflowDefinitionInput, + flagOn: boolean, + ): WorkflowDefinition { + const name = input.name?.trim(); + if (!name) throw new Error("Workflow name is required"); + const ir = parseWorkflowIr(input.ir); + this.assertWorkflowIrTraitsValid(ir); + const layout = input.layout ?? {}; + const now = new Date().toISOString(); + const id = this.nextWorkflowDefinitionId(); + const definition: WorkflowDefinition = { + id, + name, + description: input.description ?? "", + kind: input.kind === "fragment" ? "fragment" : "workflow", + ir, + layout, + createdAt: now, + updatedAt: now, + }; + this.db + .prepare( + `INSERT INTO workflows (id, name, description, ir, layout, kind, createdAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + definition.id, + definition.name, + definition.description, + serializeWorkflowIr(flagOn ? definition.ir : downgradeIrToV1IfPure(definition.ir)), + JSON.stringify(definition.layout), + definition.kind, + definition.createdAt, + definition.updatedAt, + ); + this.workflowDefinitionsCache = null; + return definition; + } + + /** + * Lazy, idempotent migration of legacy user-authored workflow steps into the + * dual workflow-definition representation (U2 / R5 / KTD-3). Runs on first + * editor open per project via `POST /api/workflows/migrate-legacy-steps`. + * + * Policy: + * - Every unmigrated user step (enabled or not, excluding compiled-materialized + * rows) becomes a `kind: "fragment"` definition — the reusable palette piece. + * - The `defaultOn` subset additionally becomes ONE combined `kind: "workflow"` + * definition named "Migrated steps" (these were the steps that ran + * automatically on new tasks); when non-empty and no project default is + * already set, it becomes the project default so new-task behavior is + * preserved. An explicit existing default is never clobbered. + * - Each source row is stamped with `migratedFragmentId` (idempotency marker). + * Source rows are never deleted. + * + * Idempotency: the unmigrated-rows SELECT and the marker stamping happen inside + * a single `transactionImmediate` (write lock acquired BEFORE the SELECT, + * matching `selectTaskWorkflow`'s ordering rationale), so concurrent opens / + * re-runs converge to a single set of definitions. A second run sees zero + * unmigrated rows and returns `{ migrated: 0, skipped: n }`. + */ + async migrateLegacyWorkflowSteps(): Promise<{ + migrated: number; + skipped: number; + combinedWorkflowId?: string; + }> { + // Resolve async prerequisites BEFORE the synchronous transaction: the + // workflow-columns flag (for flag-aware persistence). The project default is + // re-read AFTER the transaction (compare-and-set) so a concurrently-set + // default is never clobbered. + const flagOn = await this.workflowColumnsFlagOn(); + + const result = this.db.transactionImmediate(() => { + // Write lock is now held. Read the raw step rows directly (the cached, + // plugin-merged listWorkflowSteps() is not transaction-scoped). Mirror + // listWorkflowSteps()'s compiled-materialized filter and toStoredWorkflowStep + // mapping so policy decisions match the user-facing step listing. + const rows = this.db + .prepare("SELECT * FROM workflow_steps ORDER BY createdAt ASC") + .all() as Array[0]>; + + const userSteps = rows + .map((row) => this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(row))) + // Compiled-materialized rows are an execution detail, not user-authored. + .filter((step) => !step.templateId?.startsWith(WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX)); + + const alreadyMigrated = userSteps.filter((s) => s.migratedFragmentId); + const unmigrated = userSteps.filter((s) => !s.migratedFragmentId); + + if (unmigrated.length === 0) { + return { migrated: 0, skipped: alreadyMigrated.length, combinedWorkflowId: undefined as string | undefined }; + } + + // Every unmigrated user step → a single-node fragment; stamp the source row. + for (const step of unmigrated) { + // parseWorkflowIr runs inside both insertWorkflowDefinitionSync and + // layoutForIr, so compute the fragment IR once and reuse it. + const fragmentIr = stepToFragmentIr(step); + const fragment = this.insertWorkflowDefinitionSync( + { + name: step.name, + description: step.description, + kind: "fragment", + ir: fragmentIr, + layout: layoutForIr(fragmentIr), + }, + flagOn, + ); + this.db + .prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?") + .run(fragment.id, new Date().toISOString(), step.id); + } + this.workflowStepsCache = null; + this.db.bumpLastModified(); + + // The defaultOn subset → one combined "Migrated steps" workflow. + const defaultOnSteps = unmigrated.filter((s) => s.defaultOn === true); + let combinedWorkflowId: string | undefined; + if (defaultOnSteps.length > 0) { + const ir = stepsToWorkflowIr(defaultOnSteps, "Migrated steps"); + const combined = this.insertWorkflowDefinitionSync( + { + name: "Migrated steps", + description: "Converted from your legacy workflow steps", + kind: "workflow", + ir, + layout: layoutForIr(ir), + }, + flagOn, + ); + combinedWorkflowId = combined.id; + } + + return { migrated: unmigrated.length, skipped: alreadyMigrated.length, combinedWorkflowId }; + }); + + // Set the combined workflow as the project default — only when one was + // created AND no explicit default is already set (don't clobber a user + // choice). Done outside the transaction via the async setter so the project + // default-workflow hooks run. Compare-and-set against the CURRENT default + // (re-read immediately before writing, not the pre-transaction snapshot) so + // a default set concurrently by another writer is never overwritten. If the + // set fails, swallow the error: a missing migrated default is recoverable + // (the user can set one), but throwing here would surface the whole + // migration as failed even though the definitions were written. + if (result.combinedWorkflowId) { + const currentDefaultId = await this.getDefaultWorkflowId(); + if (!currentDefaultId) { + try { + await this.setDefaultWorkflowId(result.combinedWorkflowId); + } catch (err) { + storeLog.warn("Failed to set migrated combined workflow as project default", { + phase: "migrateLegacyWorkflowSteps:set-default", + combinedWorkflowId: result.combinedWorkflowId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } + + return result; + } + /** Whether a raw workflow CLI command has been approved (trust-on-first-use). * Comparison is on the exact trimmed command string. */ async isWorkflowCliCommandApproved(command: string): Promise { @@ -13294,6 +13541,9 @@ ${stepsSection}`; if (!workflowId) return undefined; const def = await this.getWorkflowDefinition(workflowId); if (!def) return undefined; + // KTD-1/R6: a fragment must never act as a project default (it is not a + // selectable workflow); fall back to no default rather than materializing it. + if (def.kind === "fragment") return undefined; // Compile (and validate) before creating any rows so a non-compilable // default falls back cleanly with nothing written. const inputs = compileWorkflowToSteps(def.ir); @@ -13301,6 +13551,25 @@ ${stepsSection}`; return { workflowId, stepIds }; } + /** Resolve an EXPLICITLY requested workflow id (U6/R3/KTD-4) into materialized + * step ids for the create-time `workflowId` parameter. Unlike + * `materializeDefaultWorkflowSteps`, unknown ids and fragments are hard errors + * (thrown BEFORE any task row is created) rather than silent fallbacks, since + * the caller asked for a specific workflow. Compilation happens up front so a + * non-compilable workflow aborts before any rows are written. */ + private async materializeExplicitWorkflowSteps( + workflowId: string, + ): Promise<{ workflowId: string; stepIds: string[] }> { + const def = await this.getWorkflowDefinition(workflowId); + if (!def) throw new Error(`Workflow '${workflowId}' not found`); + if (def.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); + } + const inputs = compileWorkflowToSteps(def.ir); + const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); + return { workflowId, stepIds }; + } + /** * Select a workflow for a task: compile it, materialize its steps, and write * their ids into the task's enabledWorkflowSteps. Replaces any prior selection @@ -13315,6 +13584,12 @@ ${stepsSection}`; return this.withTaskLock(taskId, async () => { const def = await this.getWorkflowDefinition(workflowId); if (!def) throw new Error(`Workflow '${workflowId}' not found`); + // KTD-1/R6: fragments are reusable single-node palette templates, not + // selectable workflows. Reject them from task selection with a clear error + // rather than materializing a degenerate single-step task. + if (def.kind === "fragment") { + throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); + } // Compile once up front: a non-linear graph aborts before any mutation. const inputs = compileWorkflowToSteps(def.ir); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index d5a31992a7..f62f3105b8 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -541,6 +541,11 @@ export interface WorkflowStep { * Must be set together with `modelProvider`. When both model fields are undefined, * the executor uses global settings defaults. Only used when mode is "prompt". */ modelId?: string; + /** (workflow-editor-consolidation U1/U2, KTD-1/KTD-3) when this legacy step has + * been migrated into a fragment WorkflowDefinition, the fragment's id is stamped + * here so the lazy step migration is idempotent (already-stamped rows are + * skipped). Stored in the `migrated_fragment_id` column. */ + migratedFragmentId?: string; /** ISO-8601 timestamp of creation */ createdAt: string; /** ISO-8601 timestamp of last update */ @@ -651,6 +656,9 @@ export interface WorkflowStepInput { modelProvider?: string; /** AI model ID override. Must be set together with modelProvider. Only used when mode is "prompt". */ modelId?: string; + /** (workflow-editor-consolidation U2, KTD-3) fragment id stamped when this step + * was migrated into a fragment WorkflowDefinition. Set by the migration only. */ + migratedFragmentId?: string; } /** Result of a workflow step execution on a task. */ @@ -2283,6 +2291,23 @@ export interface TaskCreateInput { noCommitsExpected?: boolean; /** IDs of workflow steps to enable for this task */ enabledWorkflowSteps?: string[]; + /** + * Workflow selection applied atomically at task creation (U6/R3/KTD-4). + * + * Semantics: + * - `undefined` → inherit the project default workflow (today's behavior: + * `materializeDefaultWorkflowSteps` runs, falling back to default-on steps). + * - `null` → explicitly NO workflow: skip default materialization entirely; + * the task is created with no custom workflow steps. + * - `string` → that workflow's compiled steps are materialized and selected + * inside the creation flow, overriding any project default. Fragment IDs + * and unknown IDs are rejected with a clear error BEFORE the task row is + * created. + * + * Mutually exclusive with `enabledWorkflowSteps`: when `enabledWorkflowSteps` + * is provided, it takes precedence and `workflowId` materialization is skipped. + */ + workflowId?: string | null; /** Model preset selected during task creation. Presets resolve to concrete model overrides at creation time. */ modelPresetId?: string; /** AI model provider override for the executor agent (e.g., "anthropic"). diff --git a/packages/core/src/workflow-compiler.ts b/packages/core/src/workflow-compiler.ts index dfbd3f1bcd..dcf7ce0363 100644 --- a/packages/core/src/workflow-compiler.ts +++ b/packages/core/src/workflow-compiler.ts @@ -95,6 +95,10 @@ export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { return new WorkflowCompileError(`node '${node.id}' has no outgoing edge`); } if (outs.length > 1) { + // NOTE: the `require the workflow interpreter (deferred)` suffix is matched + // by the dashboard editor (WorkflowNodeEditor handleSave, KTD-4) to render + // an info-tone "interpreter-only" banner instead of an error. Keep both + // interpreter-deferred messages carrying this exact suffix in sync. return new WorkflowCompileError( `node '${node.id}' branches into ${outs.length} edges — graphs with branches require the workflow interpreter (deferred)`, ); @@ -155,6 +159,18 @@ function defaultGateMode(node: WorkflowIrNode, mode: "prompt" | "script"): Workf return mode === "script" ? "gate" : "advisory"; } +/** + * Map a single user IR node onto a WorkflowStepInput. This is the forward half + * of the steps↔IR round-trip contract (workflow-editor-consolidation R4/KTD-2); + * its exact inverse is `stepInputToNode` in `workflow-steps-to-ir.ts`. Parity is + * pinned by `__tests__/workflow-steps-to-ir.test.ts` over exactly the + * compiler-visible fields: name / mode / phase / gateMode / prompt / scriptName / + * toolMode / modelProvider / modelId. `enabled` / `defaultOn` / `templateId` are + * NOT compiler-visible and are handled by migration policy, not the converter. + * + * INVERSION CONTRACT: when you add a field here, extend `stepInputToNode` (and + * the parity test) in `workflow-steps-to-ir.ts` to keep the round-trip exact. + */ function nodeToStepInput(node: WorkflowIrNode, phase: "pre-merge" | "post-merge"): WorkflowStepInput { const scriptName = configString(node, "scriptName"); const mode: "prompt" | "script" = node.kind === "script" || (node.kind === "gate" && scriptName) ? "script" : "prompt"; diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 412bfca6c0..e5f14087d1 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -7,6 +7,12 @@ export interface WorkflowNodeLayout { y: number; } +/** Discriminates a full, selectable workflow from a reusable single-node + * "fragment" template (workflow-editor-consolidation U1, KTD-1). Fragments are + * excluded from task workflow pickers, default-workflow selection, and the + * compile/selection paths; both kinds are stored as parseable full IRs. */ +export type WorkflowDefinitionKind = "workflow" | "fragment"; + /** A named, persisted workflow authored as a WorkflowIr graph plus editor layout. */ export interface WorkflowDefinition { /** Unique identifier (e.g., "WF-001"). */ @@ -15,6 +21,8 @@ export interface WorkflowDefinition { name: string; /** Short description for UI display. */ description: string; + /** Discriminates full workflows from reusable fragment templates (KTD-1). */ + kind: WorkflowDefinitionKind; /** The validated workflow graph (v1 IR contract). */ ir: WorkflowIr; /** Editor node positions keyed by IR node id. May be empty (auto-layout). */ @@ -32,6 +40,9 @@ export interface WorkflowDefinitionInput { /** Workflow graph; validated via parseWorkflowIr on write. */ ir: WorkflowIr; layout?: Record; + /** Discriminates full workflows from reusable fragment templates (KTD-1). + * Defaults to "workflow" when omitted. */ + kind?: WorkflowDefinitionKind; } /** Partial update for an existing workflow definition. */ diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index 793c4e30ae..373eb7d955 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -943,3 +943,42 @@ export function downgradeIrToV1IfPure(ir: WorkflowIr): WorkflowIr { export function serializeWorkflowIr(ir: WorkflowIr): string { return JSON.stringify(ir, null, 2); } + +/** + * Strip the trust-escalating `cliSkipApproval`/`autoApprove` flags from every + * node config in an IR, recursing into foreach `config.template.nodes` at any + * nesting depth (foreach-in-foreach). Mutates the passed IR in place and returns + * it alongside a `stripped` flag indicating whether anything was removed. + * + * These flags bypass the CLI first-run approval gate (see executor.ts). They are + * legitimate only for workflows authored through the trusted dashboard editor / + * executor lane; on prompt-injectable surfaces (chat/planning authoring tools, + * import, AI design) they must be removed at the write boundary. + */ +export function stripApprovalBypassFlags(ir: WorkflowIr): { ir: WorkflowIr; stripped: boolean } { + const nodes = (ir as { nodes?: WorkflowIrNode[] }).nodes; + if (!Array.isArray(nodes)) return { ir, stripped: false }; + let stripped = false; + const stripNode = (node: WorkflowIrNode): void => { + // Untrusted input may contain non-object entries (null, strings, numbers) + // in `nodes` / `template.nodes`; skip them rather than dereferencing. + if (!node || typeof node !== "object") return; + const cfg = node.config as Record | undefined; + if (cfg && typeof cfg === "object") { + if ("cliSkipApproval" in cfg) { + delete cfg.cliSkipApproval; + stripped = true; + } + if ("autoApprove" in cfg) { + delete cfg.autoApprove; + stripped = true; + } + const template = (cfg as { template?: { nodes?: unknown } }).template; + if (template && Array.isArray(template.nodes)) { + for (const inner of template.nodes as WorkflowIrNode[]) stripNode(inner); + } + } + }; + for (const node of nodes) stripNode(node); + return { ir, stripped }; +} diff --git a/packages/core/src/workflow-steps-to-ir.ts b/packages/core/src/workflow-steps-to-ir.ts new file mode 100644 index 0000000000..5d4f3cbe96 --- /dev/null +++ b/packages/core/src/workflow-steps-to-ir.ts @@ -0,0 +1,162 @@ +import type { WorkflowStep } from "./types.js"; +import type { WorkflowIr, WorkflowIrNode, WorkflowIrEdge } from "./workflow-ir-types.js"; +import type { WorkflowNodeLayout } from "./workflow-definition-types.js"; +import { parseWorkflowIr } from "./workflow-ir.js"; + +/** + * Steps → IR converter (workflow-editor-consolidation U1, R4/KTD-2). + * + * This module is the exact INVERSE of the compiler's `nodeToStepInput` + * (`workflow-compiler.ts`). The round-trip contract is: + * + * compileWorkflowToSteps(stepsToWorkflowIr(steps, name)) ≡ steps + * + * over exactly the compiler-visible fields: name / mode / phase / gateMode / + * prompt / scriptName / toolMode / modelProvider / modelId. `enabled` / + * `defaultOn` / `templateId` / `migratedFragmentId` are NOT compiler-visible and + * are handled by migration policy (KTD-3), not by this converter. Parity is + * pinned by `__tests__/workflow-steps-to-ir.test.ts`. + * + * INVERSION CONTRACT: when a compiler-visible field is added to `nodeToStepInput` + * (see the contract comment there), extend `stepInputToNode` below and the parity + * test to keep the round-trip exact. + * + * Seam encoding mirrors `linear()` in `builtin-workflows.ts` exactly: the fixed + * execute → review → merge pipeline is emitted as prompt-kind nodes carrying + * `config.seam`, chained by `success` edges, with each seam also wired + * `failure → end`. + */ + +/** The fixed seam pipeline, in canonical order. The `merge` seam is the + * pre-/post-merge boundary and is always emitted (R4). */ +const SEAM_ORDER = ["execute", "review", "merge"] as const; + +/** Horizontal spacing used by `linear()`; reused so migrated graphs lay out the + * same way built-ins do. */ +const LAYOUT_X0 = 60; +const LAYOUT_DX = 170; +const LAYOUT_Y = 160; + +/** + * Inverse of `nodeToStepInput` (workflow-compiler.ts). Produces a single user IR + * node whose forward compilation reproduces every compiler-visible field of the + * given step. + * + * kind ↔ mode/gateMode mapping (the heart of the contract): + * - mode "script" → kind "script", `config.scriptName` set. The compiler reads + * mode from `kind === "script"`, so this round-trips to mode "script". + * - mode "prompt" → kind "prompt", `config.prompt`/`toolMode`/model overrides. + * - gateMode is ALWAYS written to `config.gateMode` (both "gate" and "advisory"). + * The compiler's `defaultGateMode` returns an explicit `config.gateMode` for + * non-gate-kind nodes verbatim, so this round-trips for both modes without + * needing the `gate` node kind (which the compiler only emits via scriptName + * heuristics — using explicit `config.gateMode` keeps the inverse total). + */ +function stepInputToNode(step: WorkflowStep, id: string): WorkflowIrNode { + const config: Record = { + name: step.name, + // Always carry gateMode so the compiler reproduces it exactly for both modes. + gateMode: step.gateMode, + }; + if (step.description) config.description = step.description; + + if (step.mode === "script") { + if (step.scriptName) config.scriptName = step.scriptName; + return { id, kind: "script", config }; + } + + // prompt mode + config.prompt = step.prompt ?? ""; + config.toolMode = step.toolMode === "coding" ? "coding" : "readonly"; + // Model overrides only round-trip when BOTH are present (compiler requirement). + if (step.modelProvider && step.modelId) { + config.modelProvider = step.modelProvider; + config.modelId = step.modelId; + } + return { id, kind: "prompt", config }; +} + +/** Build a seam node exactly as `linear()` does: a prompt-kind node tagged with + * `config.seam`. */ +function seamNode(seam: (typeof SEAM_ORDER)[number]): WorkflowIrNode { + return { id: seam, kind: "prompt", config: { seam } }; +} + +/** + * Convert an ordered `WorkflowStep[]` into a valid v1 WorkflowIr: + * + * start → [pre-merge user nodes] → execute → review → merge + * → [post-merge user nodes] → end + * + * Steps with `phase` undefined map to pre-merge (R4). Seam nodes get an extra + * `failure → end` edge, mirroring `linear()`. The result always passes + * `parseWorkflowIr`. An empty step list yields the minimal seam-only pipeline + * (which compiles back to `[]`). + */ +export function stepsToWorkflowIr(steps: WorkflowStep[], name: string): WorkflowIr { + const preMerge = steps.filter((s) => (s.phase ?? "pre-merge") === "pre-merge"); + const postMerge = steps.filter((s) => s.phase === "post-merge"); + + const nodes: WorkflowIrNode[] = [{ id: "start", kind: "start" }]; + const userNodeIds = new Set(); + + // Deterministic ids that cannot collide with the reserved start/end/seam ids. + const userNode = (step: WorkflowStep, index: number): WorkflowIrNode => { + let id = `step-${index + 1}`; + while (userNodeIds.has(id)) id = `${id}-x`; + userNodeIds.add(id); + return stepInputToNode(step, id); + }; + + preMerge.forEach((step, i) => nodes.push(userNode(step, i))); + // Fixed execute → review → merge seam pipeline; merge is the boundary (R4). + for (const seam of SEAM_ORDER) nodes.push(seamNode(seam)); + postMerge.forEach((step, i) => nodes.push(userNode(step, preMerge.length + i))); + nodes.push({ id: "end", kind: "end" }); + + const edges: WorkflowIrEdge[] = []; + for (let i = 0; i < nodes.length - 1; i += 1) { + edges.push({ from: nodes[i].id, to: nodes[i + 1].id, condition: "success" }); + } + // Seam nodes also fail straight to end (mirrors `linear()` / the legacy pipeline). + for (const node of nodes) { + if (typeof node.config?.seam === "string") { + edges.push({ from: node.id, to: "end", condition: "failure" }); + } + } + + return parseWorkflowIr({ version: "v1", name, nodes, edges }); +} + +/** + * Convert a single `WorkflowStep` into a minimal fragment IR (R6/KTD-1): + * + * start → node → end + * + * No seams. The node mirrors the step via `stepInputToNode`. The result passes + * `parseWorkflowIr` and is a pure-v1 graph (survives `downgradeIrToV1IfPure`). + */ +export function stepToFragmentIr(step: WorkflowStep): WorkflowIr { + const node = stepInputToNode(step, "step-1"); + return parseWorkflowIr({ + version: "v1", + name: step.name, + nodes: [{ id: "start", kind: "start" }, node, { id: "end", kind: "end" }], + edges: [ + { from: "start", to: node.id, condition: "success" }, + { from: node.id, to: "end", condition: "success" }, + ], + }); +} + +/** + * Deterministic x-spaced layout for an IR, matching `linear()`'s geometry. Keyed + * by node id; supply alongside the IR when persisting a `WorkflowDefinitionInput`. + */ +export function layoutForIr(ir: WorkflowIr): Record { + const layout: Record = {}; + ir.nodes.forEach((node, i) => { + layout[node.id] = { x: LAYOUT_X0 + i * LAYOUT_DX, y: LAYOUT_Y }; + }); + return layout; +} diff --git a/packages/dashboard/app/App.tsx b/packages/dashboard/app/App.tsx index ec73184f45..34504f4f09 100644 --- a/packages/dashboard/app/App.tsx +++ b/packages/dashboard/app/App.tsx @@ -1227,9 +1227,9 @@ function AppInner() { pushNav({ type: "modal", close: modalManager.closeScripts }); }, [modalManager, pushNav]); - const openWorkflowStepsWithNav = useCallback(() => { - modalManager.openWorkflowSteps(); - pushNav({ type: "modal", close: modalManager.closeWorkflowSteps }); + const openWorkflowEditorWithNav = useCallback(() => { + modalManager.openWorkflowEditor(); + pushNav({ type: "modal", close: modalManager.closeWorkflowEditor }); }, [modalManager, pushNav]); const openUsageWithNav = useCallback((anchorRect?: DOMRect | null) => { @@ -1803,7 +1803,7 @@ function AppInner() { onOpenGitManager={openGitManagerWithNav} onOpenNodes={handleOpenNodesWithNav} showNodesButton={nodesEnabled} - onOpenWorkflowSteps={openWorkflowStepsWithNav} + onOpenWorkflowEditor={openWorkflowEditorWithNav} onOpenScripts={openScriptsWithNav} onRunScript={runScriptWithNav} onToggleTerminal={toggleTerminalWithNav} @@ -2007,7 +2007,7 @@ function AppInner() { chatHasUnreadResponse={chatHasUnreadResponse} stashOrphanCount={stashOrphanCount} onOpenGitManager={openGitManagerWithNav} - onOpenWorkflowSteps={openWorkflowStepsWithNav} + onOpenWorkflowEditor={openWorkflowEditorWithNav} onOpenSchedules={openSchedulesWithNav} onOpenScripts={openScriptsWithNav} onToggleTerminal={toggleTerminalWithNav} diff --git a/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx b/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx index b6688642a8..6a56028d42 100644 --- a/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx +++ b/packages/dashboard/app/__tests__/mobile-feature-access-regression.test.tsx @@ -49,7 +49,7 @@ const createDefaultMobileNavProps = () => ({ onOpenNodes: vi.fn(), mailboxUnreadCount: 0, onOpenGitManager: vi.fn(), - onOpenWorkflowSteps: vi.fn(), + onOpenWorkflowEditor: vi.fn(), onOpenSchedules: vi.fn(), onOpenScripts: vi.fn(), onToggleTerminal: vi.fn(), diff --git a/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx b/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx index bcb2e9fb32..eca5a3e11f 100644 --- a/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx +++ b/packages/dashboard/app/__tests__/tablet-header-controls.test.tsx @@ -178,9 +178,9 @@ describe("tablet header controls", () => { expect(screen.queryByTitle("Git Manager")).toBeNull(); }); - it("does not render workflow steps button inline on tablet", () => { - renderTabletHeader({ onOpenWorkflowSteps: noop }); - expect(screen.queryByTitle("Workflow Steps")).toBeNull(); + it("does not render workflows button inline on tablet", () => { + renderTabletHeader({ onOpenWorkflowEditor: noop }); + expect(screen.queryByTitle("Workflows")).toBeNull(); }); // ── Overflow menu on tablet ──────────────────────────────────── @@ -254,8 +254,8 @@ describe("tablet header controls", () => { expect(screen.getByTestId("overflow-git-btn")).toBeDefined(); }); - it("overflow menu contains workflow steps on tablet when provided", () => { - renderTabletHeader({ onOpenWorkflowSteps: noop }); + it("overflow menu contains workflows on tablet when provided", () => { + renderTabletHeader({ onOpenWorkflowEditor: noop }); fireEvent.click(screen.getByTitle("More header actions")); expect(screen.getByTestId("overflow-workflow-steps-btn")).toBeDefined(); }); @@ -538,7 +538,7 @@ describe("tablet header controls", () => { const { container } = renderTabletHeader({ onOpenUsage: noop, onOpenActivityLog: noop, - onOpenWorkflowSteps: noop, + onOpenWorkflowEditor: noop, onOpenFiles: noop, onOpenGitManager: noop, }); diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index c674af47bf..30ef1390c6 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -371,6 +371,7 @@ export async function createTask( dependencies, breakIntoSubtasks, enabledWorkflowSteps, + workflowId, assignedAgentId, modelPresetId, modelProvider, @@ -407,6 +408,7 @@ export async function createTask( dependencies, breakIntoSubtasks, enabledWorkflowSteps, + workflowId, assignedAgentId, modelPresetId, modelProvider, @@ -5108,6 +5110,106 @@ export function compileWorkflow(id: string, projectId?: string): Promise<{ steps }); } +/** A workflow export envelope (U5/R9/KTD-5). `schemaVersion` is the SERVER's + * schema version at export time — the import route version-gates against it + * (the app build aliases @fusion/core to types-only, so the value can only come + * from the server, never an app-side core import). */ +export interface WorkflowExportEnvelope { + fusionWorkflowExport: 1; + schemaVersion: number; + kind: import("@fusion/core").WorkflowDefinition["kind"]; + name: string; + description: string; + ir: import("@fusion/core").WorkflowIr; + layout: import("@fusion/core").WorkflowDefinition["layout"]; +} + +/** Fetch a workflow's export envelope and trigger a browser download as + * `.workflow.json` (U5/R9). Built-ins are exportable too. Mirrors the + * SettingsModal export pattern (Blob + createObjectURL + a.download). */ +export async function exportWorkflow(id: string, projectId?: string): Promise { + const envelope = await api( + withProjectId(`/workflows/${encodeURIComponent(id)}/export`, projectId), + ); + const safeName = (envelope.name || "workflow").replace(/[^\w.-]+/g, "-").replace(/^-+|-+$/g, "") || "workflow"; + const blob = new Blob([JSON.stringify(envelope, null, 2)], { type: "application/json" }); + const url = URL.createObjectURL(blob); + const link = document.createElement("a"); + link.href = url; + link.download = `${safeName}.workflow.json`; + document.body.appendChild(link); + link.click(); + document.body.removeChild(link); + URL.revokeObjectURL(url); + return envelope; +} + +/** Result of POST /api/workflows/import (U5/R10). `strippedApprovalFlags` is set + * when `cliSkipApproval`/`autoApprove` were removed from any node config at the + * trust boundary; `warnings` lists non-blocking issues (e.g. unknown scriptName). */ +export interface ImportWorkflowResult { + workflow: import("@fusion/core").WorkflowDefinition; + strippedApprovalFlags: boolean; + warnings: string[]; +} + +/** Import a workflow export envelope (U5/R10). The server is the sole validator; + * validation failures reject with an ApiError carrying the server message. */ +export function importWorkflow( + envelope: unknown, + projectId?: string, +): Promise { + return api(withProjectId("/workflows/import", projectId), { + method: "POST", + body: JSON.stringify(envelope), + }); +} + +/** Result of the lazy legacy-step migration (U2/R5). `migrated` is the number of + * newly converted user steps; `skipped` the count already migrated; when the + * defaultOn subset was non-empty a combined "Migrated steps" workflow id is set. */ +export interface MigrateLegacyStepsResult { + migrated: number; + skipped: number; + combinedWorkflowId?: string; +} + +/** Run the lazy, idempotent migration of legacy user-authored workflow steps into + * fragments + a combined workflow (U2/R5). Safe to call repeatedly. */ +export function migrateLegacyWorkflowSteps(projectId?: string): Promise { + return api(withProjectId("/workflows/migrate-legacy-steps", projectId), { + method: "POST", + }); +} + +/** Result of POST /api/workflows/design (U10/R11). The server validates the + * AI-produced IR (parseWorkflowIr), triages compilability (`interpreterOnly`), + * and strips trust-escalating flags (`strippedApprovalFlags`). Persists nothing + * — the client decides what to do with the returned graph. */ +export interface DesignWorkflowResult { + ir: import("@fusion/core").WorkflowIr; + layout: import("@fusion/core").WorkflowDefinition["layout"]; + interpreterOnly: boolean; + strippedApprovalFlags: boolean; +} + +/** Design a workflow from a natural-language prompt (U10/R11). When `workflowId` + * is supplied the route reads that workflow's persisted IR server-side and folds + * it into the prompt as the base graph (the client never posts IR). An optional + * AbortSignal cancels the in-flight request. Validation failures reject with an + * ApiError carrying the server message; 429 on rate limit. */ +export function designWorkflow( + input: { prompt: string; workflowId?: string }, + projectId?: string, + signal?: AbortSignal, +): Promise { + return api(withProjectId("/workflows/design", projectId), { + method: "POST", + body: JSON.stringify(input), + signal, + }); +} + /** Read the workflow currently selected for a task. */ export function fetchTaskWorkflow(taskId: string, projectId?: string): Promise<{ workflowId: string | null }> { return api<{ workflowId: string | null }>( diff --git a/packages/dashboard/app/components/AppModals.tsx b/packages/dashboard/app/components/AppModals.tsx index 9b543557c1..3b266568f4 100644 --- a/packages/dashboard/app/components/AppModals.tsx +++ b/packages/dashboard/app/components/AppModals.tsx @@ -20,7 +20,6 @@ import { NewTaskModal } from "./NewTaskModal"; import { SystemStatsModal } from "./SystemStatsModal"; import { ActivityLogModal } from "./ActivityLogModal"; import { GitManagerModal } from "./GitManagerModal"; -import { WorkflowStepManager } from "./WorkflowStepManager"; import { AgentListModal } from "./AgentListModal"; import { ModelOnboardingModal } from "./ModelOnboardingModal"; import { ToastContainer } from "./ToastContainer"; @@ -373,19 +372,6 @@ export function AppModals({ /> - - { - modalManager.closeWorkflowSteps(); - modalManager.openWorkflowEditor(); - }} - /> - - {modalManager.workflowEditorOpen && ( diff --git a/packages/dashboard/app/components/Header.tsx b/packages/dashboard/app/components/Header.tsx index 17e3143235..034afa89a4 100644 --- a/packages/dashboard/app/components/Header.tsx +++ b/packages/dashboard/app/components/Header.tsx @@ -197,7 +197,7 @@ export interface HeaderProps { onOpenNodes?: () => void; /** When false, hides the Nodes management button. Defaults to true for backward compat. */ showNodesButton?: boolean; - onOpenWorkflowSteps?: () => void; + onOpenWorkflowEditor?: () => void; onOpenScripts?: () => void; onRunScript?: (name: string, command: string) => void; onToggleTerminal?: () => void; @@ -266,7 +266,7 @@ export function Header({ onOpenGitManager, onOpenNodes, showNodesButton, - onOpenWorkflowSteps, + onOpenWorkflowEditor, onOpenScripts, onRunScript, onToggleTerminal, @@ -1593,12 +1593,12 @@ export function Header({ )} - {/* Workflow Steps - desktop only (moved to overflow on mobile/tablet) */} - {!isCompact && onOpenWorkflowSteps && ( + {/* Workflows - desktop only (moved to overflow on mobile/tablet) */} + {!isCompact && onOpenWorkflowEditor && ( )} - {/* Workflow Steps - in overflow on mobile */} - {onOpenWorkflowSteps && ( + {/* Workflows - in overflow on mobile */} + {onOpenWorkflowEditor && ( )} {/* Settings - always last in overflow menu */} diff --git a/packages/dashboard/app/components/MobileNavBar.tsx b/packages/dashboard/app/components/MobileNavBar.tsx index ac421e502d..6ae76d6df0 100644 --- a/packages/dashboard/app/components/MobileNavBar.tsx +++ b/packages/dashboard/app/components/MobileNavBar.tsx @@ -60,7 +60,7 @@ export interface MobileNavBarProps { chatHasUnreadResponse?: boolean; stashOrphanCount?: number; onOpenGitManager?: () => void; - onOpenWorkflowSteps?: () => void; + onOpenWorkflowEditor?: () => void; onOpenSchedules?: () => void; onOpenScripts?: () => void; onToggleTerminal?: () => void; @@ -127,7 +127,7 @@ export function MobileNavBar({ chatHasUnreadResponse = false, stashOrphanCount = 0, onOpenGitManager, - onOpenWorkflowSteps, + onOpenWorkflowEditor, onOpenSchedules, onOpenScripts, onToggleTerminal, @@ -590,10 +590,10 @@ export function MobileNavBar({ type="button" className="mobile-more-item" data-testid="mobile-more-item-workflow" - onClick={() => handleMoreAction(onOpenWorkflowSteps)} + onClick={() => handleMoreAction(onOpenWorkflowEditor)} > - {t("nav.workflowSteps", "Workflow Steps")} + {t("nav.workflows", "Workflows")} - - - - - ); - })} - - )} - + )} {(onGithubTrackingEnabledChange || onGithubRepoOverrideChange) && (
diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index b25d2f9c90..de7943da73 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -35,6 +35,37 @@ cursor: pointer; } +/* U2/R5: one-time legacy-step migration notice banner. */ +.wf-migration-notice { + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-sm); + padding: var(--space-sm) var(--space-md); + background: var(--accent-subtle, rgba(59, 130, 246, 0.12)); + border-bottom: 1px solid var(--border); + color: var(--text); + font-size: 0.85rem; +} + +.wf-migration-notice-text { + flex: 1; +} + +.wf-migration-notice-dismiss { + display: inline-flex; + align-items: center; + background: transparent; + border: none; + color: var(--text-muted); + cursor: pointer; + flex-shrink: 0; +} + +.wf-migration-notice-dismiss:hover { + color: var(--text); +} + .wf-editor-close:hover { color: var(--text); } @@ -49,12 +80,76 @@ display: flex; flex-direction: column; gap: var(--space-xs); - width: 220px; + width: 300px; padding: var(--space-sm); border-right: 1px solid var(--border); overflow-y: auto; } +/* U12: columns + fields authoring sections moved into the left sidebar, below + the workflow list. Each is a collapsible disclosure whose toggle button is the + section header; the panels' own internal

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

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

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