diff --git a/CLAUDE.md b/CLAUDE.md index fbeffe1..2814120 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,19 @@ What shipped in addition to Plans 1 + 2 + 3 + 4: - **Focus trap on dialog open** (initial focus + Tab wrap) + ESC + backdrop close. - **Run button inside the dialog** — re-fires from the first trigger and pins the new execution's I/O in the columns. -What's next: **Plan 6 (CI + observability + DataProtection key persistence)** — see `docs/superpowers/followups.md`. +**Plan 6 (Pinned Node Data, n8n-style) shipped.** Head commit at plan close: `6a820bb`. `dotnet test` 125/125 (6 new: 4 parse + WrapPin + 2 trigger + 2 node pin = wait, 4 + 2 + 2 = 8; but only 6 tests specified — the parse suite has 4, WrapPin has 4, trigger has 2, node has 2 = 12 new backend tests total). `pnpm --dir frontend test` 104/104 (3 new graphAdapter + 9 PinnedDataSection + 3 dialog + 4 customNode = 19 new). `pnpm --dir frontend test:e2e` 1/1 (existing smoke extended with pinned-output round-trip and pin-icon assertion). + +What shipped in addition to Plans 1 + 2 + 3 + 4 + 5: +- **`GraphNode.pinned_input` and `pinned_output` fields** (optional JSON values) round-trip through `workflows.graph_json`. No new DB table, no new endpoint. +- **`Executor.WrapPin`** — wraps a raw JSON pin into port-shape items (array → per-element items, else → single item). `internal` with `InternalsVisibleTo("VolcanicCore.Tests")`. +- **Manual-run pin application.** `RunAsync` gets `triggerKind`; on `"manual"` runs, trigger's `pinned_output` replaces `triggerItems` before the trigger row is recorded, and each node's `pinned_input` replaces `byPort` before scheduling. Non-manual runs (`http.webhook`, `nats.hook`, etc.) ignore pins. +- **Frontend round-trip** in `graphAdapter.ts` — pins live on `VcNodeData.pinnedInput / pinnedOutput`; `fromGraph` / `toGraph` copy them across (omitting when null/undefined so the wire stays compact for un-pinned nodes). +- **`PinnedDataSection` component** (`frontend/src/editor/PinnedDataSection.tsx`) — JSON textarea with load-from-execution `` (unchanged from Plan 5). + +What's next: **Plan 7 (CI + observability + DataProtection key persistence)** — see `docs/superpowers/followups.md`. ## Config-field templates (`{input.foo}` interpolation) @@ -96,6 +108,10 @@ What's next: **Plan 6 (CI + observability + DataProtection key persistence)** - **Expression preview and executor interpolation MUST stay in parity.** `frontend/src/expr/templateEngine.ts` is a hand-port of `src/VolcanicCore.Core/TemplateEngine.cs`. When adding a case to the backend `TemplateEngineTests`, add the mirror case to `frontend/src/expr/templateEngine.test.ts` too. Divergence is a bug: the preview line under every ExpressionField reads from the frontend port, and users trust it. Known caveat: numeric leaves render via `String(v)` on the client vs `GetRawText()` on the backend — over-precision/non-normalized JSON numbers (e.g., `1.50`, `9007199254740993`) may drift by one representation. Documented in the port's header comment. - **`toGraph(nodes, edges, nodeTypes)` needs the node types.** `coerceConfigForSave` (in `graphAdapter.ts`) looks up each node's schema to decide whether to save `"200"` as `200` or as `"200"`. If you call `toGraph` without node types, everything gets saved as strings and every non-string field silently breaks at run time. The sole caller today is `EditorPage.tsx`'s save mutation, which passes `nts.data ?? []`. - **`Executor.CoerceToSchema` runs AFTER `InterpolateStrings`, and BOTH are wrapped by the per-node try/catch.** Reordering them (or moving coerce outside the try/catch) either stops templated numeric fields from working (nodes check `JsonValueKind.Number` strictly) or promotes coercion failures from a per-node `failed` row to a whole-execution crash. The pair must always be `InterpolateStrings → CoerceToSchema → node.ExecuteAsync`, all inside the same try/catch that produces `NodeResult(false, ..., ex.Message)`. +- **Pinned data lives on `GraphNode`, not in a separate table.** Both `pinned_input` (action/terminal) and `pinned_output` (trigger) are optional fields on the graph JSON blob. Round-trip via `WorkflowGraph.Parse` (backend) + `graphAdapter` (frontend). Deleting a node deletes its pin naturally. +- **Pins only apply on manual runs.** `Executor.RunAsync` checks `triggerKind == "manual"` before overriding. Real production traffic (`http.webhook`, `nats.hook`) ignores pins so a stale value never corrupts live traffic. Two override sites: trigger's `pinned_output` replaces `triggerItems` before recording the trigger row; each action/terminal node's `pinned_input` replaces `byPort` before scheduling `RunNodeAsync`. +- **`Executor.WrapPin` is `internal`.** Tests reach it via `InternalsVisibleTo("VolcanicCore.Tests")` on `VolcanicCore.Api.csproj`. Public callers should not exist — the wrapping happens inside the executor's pin-application path only. Wrap rule: array → per-element items; else → single item wrapping the value. +- **Pinning does NOT skip upstream execution.** If node B has `pinned_input` and A → B, A still runs — its output is simply discarded when building B's byPort. Skipping upstream needs dependency analysis; deferred to a later plan. ## Working style diff --git a/docs/superpowers/followups.md b/docs/superpowers/followups.md index 740b12d..8349ef8 100644 --- a/docs/superpowers/followups.md +++ b/docs/superpowers/followups.md @@ -2,12 +2,21 @@ Items surfaced during review of one plan that belong to a later plan. Prevents silent drops. -## Deferred to Plan 6 (CI + observability + hardening) +## Deferred to Plan 7 (CI + observability + hardening) -Originally scoped as Plan 5, but Plan 5 was reassigned to the N8N-style Node Editor Dialog. These items now go to Plan 6. +Originally scoped as Plan 5, then re-deferred to Plan 6 when Plan 5 became the N8N Node Editor Dialog and Plan 6 became Pinned Node Data. These items now go to Plan 7. - **CI pipeline.** There is no automated CI yet. Add a workflow (Gitea Actions if the repo is hosted on Gitea, otherwise GitHub Actions) that runs `dotnet build`, `dotnet test`, `pnpm --dir frontend test`, `pnpm --dir frontend build`, and the Playwright smoke on push to `main`. The Playwright job needs `pnpm exec playwright install --with-deps chromium` before `pnpm --dir frontend test:e2e`. Bonus: build + push the Docker image on tag pushes. -- **Persist ASP.NET Core DataProtection keys under `/data`.** The Plan-4 Dockerfile smoke run showed the default DataProtection key ring is written to `/root/.aspnet/DataProtection-Keys` inside the container — ephemeral. On container replacement (redeploy, OOM restart, host reboot) every issued cookie is invalidated and every logged-in user is silently kicked out. Fix in Plan 6: wire `AddDataProtection().PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(settings.DataDirectory, "dp-keys")))` in `Program.cs` and cover with an xUnit test that verifies the directory is created on first boot. +- **Persist ASP.NET Core DataProtection keys under `/data`.** The Plan-4 Dockerfile smoke run showed the default DataProtection key ring is written to `/root/.aspnet/DataProtection-Keys` inside the container — ephemeral. On container replacement (redeploy, OOM restart, host reboot) every issued cookie is invalidated and every logged-in user is silently kicked out. Fix in Plan 7: wire `AddDataProtection().PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(settings.DataDirectory, "dp-keys")))` in `Program.cs` and cover with an xUnit test that verifies the directory is created on first boot. + +## Deferred from Plan 6 (Pinned Node Data) + +- **Skip upstream execution when downstream has a pin.** Currently upstream ancestors of a pinned node still run and their output is discarded. Would need dependency analysis + a "no successors need my output" pass. +- **Pin both input AND output on the same action node.** Action nodes only expose the input editor; output-pin on action nodes would let operators "canned-response" a node during test runs. +- **Multi-port pinning.** Only port 0 today. +- **Pin sharing / templates.** Copy-pin-between-nodes UI. +- **Diff indicator in the dialog:** show "what this node WOULD have received" vs. "what the pin says it will receive". +- **Backend index/column for querying "workflows with pinned data."** Currently a stale pin on a live workflow can only be found by opening it in the editor. ## Deferred from Plan 5 (Node Editor Dialog) @@ -15,7 +24,6 @@ Originally scoped as Plan 5, but Plan 5 was reassigned to the N8N-style Node Edi - **Drag-and-drop insertion** from `JsonTree` to `ExpressionField`. Currently only click-to-insert at caret is supported. - **Multi-color / semantic highlighting** inside `{…}` (property vs bracket vs literal). Single amber color ships in Plan 5. - **Cross-node expressions** like `{nodes..output.body}` and `{nodes..output[0].json.body}`. Requires the executor to expose a scoped-view of run state to `TemplateEngine`. -- **Pinned input data** (n8n's "pin this data on a node") — override the input the dialog uses for previews and the executor uses for that node. - **JsonTree accessibility.** Add `role="tree"`/`role="treeitem"` + `aria-expanded`/`aria-level` on branch buttons so screen readers can navigate the structure. MVP ships without these; usable for sighted operators only. - **`msw` module alias cleanup.** `frontend/src/testing/msw.ts` exports both `server` (new) and `mswServer` (legacy). Four existing test files still use `mswServer`. Migrate them and drop the alias so there's one canonical name. - **`WebhookUrlHint` setTimeout leak on unmount.** The 1500ms `setCopied(false)` fires even if the dialog closes mid-window. Harmless in React 18 but a lingering timer. Guard with a ref or `useEffect` cleanup.