docs: plan 5 wrap-up — status, invariants, and re-numbered followups

This commit is contained in:
Claude
2026-08-30 11:52:30 +00:00
parent 80ac663989
commit 3e838ecf91
2 changed files with 35 additions and 3 deletions

View File

@@ -37,7 +37,20 @@ What shipped in addition to Plans 1 + 2 + 3:
- `AdminBootstrap.Seed` catches both `CryptographicException` and `FormatException` on the legacy plaintext `admin.password_hash_source` row and re-seeds cleanly.
- `deploy.json` (gitignored) holds non-secret Coolify config + secret env values; Coolify project uuid `c2miztqvd95mnw5vcii2ezco`, app uuid `zp0h5ao841myjb8k8z5y5h6s`, server uuid `d0c4owww4kwo0ow08wws4ss8`.
What's next: **initial launch scope complete.** Post-launch work goes to Plan 5 (CI + observability + DataProtection key persistence) — see `docs/superpowers/followups.md`.
**Plan 5 (Node Editor Dialog, N8N-style) shipped.** Head commit at plan close: `80ac663`. `dotnet test` 113/113 (2 new Executor coercion tests + prior growth), `pnpm --dir frontend test` 85/85 (across 12 files), `pnpm --dir frontend test:e2e` 1/1 (extended smoke covers double-click → edit label → save → reload).
What shipped in addition to Plans 1 + 2 + 3 + 4:
- **Double-click a node → 3-column modal dialog** (Input | Parameters | Output) — `frontend/src/editor/NodeEditorDialog.tsx`. Replaces the old right-side `NodeConfigPanel` (deleted).
- **Every parameter is an expression editor.** Monospace textarea with a mirrored highlight overlay coloring `{...}` tokens amber, plus a live preview line beneath (`→ evaluated string`). See `frontend/src/expr/ExpressionField.tsx`.
- **Client-side `TemplateEngine` port** (`frontend/src/expr/templateEngine.ts`) with 19 parity tests (17 mirrored from the backend + 2 for prototype-pollution and Int32-bound array indices).
- **`{input.foo}` click-to-insert.** Clicking a leaf in the Input tree splices `{input.<path>}` (or `{input["weird.key"]}` for non-identifier keys) into the last-focused parameter at its caret position.
- **Backend coercion of templated primitives.** `Executor.CoerceToSchema` runs after `InterpolateStrings`; templated string values on `integer`/`number`/`boolean` schema fields get parsed back to their native JSON type before the node reads `ctx.Config`. Parse failures surface as a normal per-node `failed` row with `config '<name>': expected <type>, got '<value>'`.
- **Client-side `coerceConfigForSave`** in `graphAdapter.ts` — literal number/boolean strings are saved as typed JSON (`"200"` → `200`); strings containing `{` stay as strings for the runtime coercion to handle. Regex-gated to match backend's `NumberStyles.Integer` / `NumberStyles.Float` (no hex, no scientific-on-integer).
- **`format: "password"`** fields keep the native `<input type="password">` — expressions don't apply to credentials.
- **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`.
## Config-field templates (`{input.foo}` interpolation)
@@ -80,6 +93,9 @@ What's next: **initial launch scope complete.** Post-launch work goes to Plan 5
- **`SERVICE_FQDN_APP_8080` (not `SERVICE_FQDN_APP`).** Coolify v4 requires the port suffix in the env-var name AND in the `docker_compose_domains[svc].domain` string (`https://core.tes.gd:8080`). Without either, Traefik has no idea which container port to route to and returns HTTP 500.
- **`deploy.json` is gitignored** — it carries secret app env values in `.env`. Never commit; if it ever gets committed, rotate every value in it.
- **Config templating is executor-side, not per-node.** `Executor.RunNodeAsync` walks the node's `ConfigJson` and runs every string leaf through `TemplateEngine.Render` against `byPort[0][0].Json` before building `ExecutionContext`. Nodes read `ctx.Config` and get interpolated strings for free. Do NOT push the substitution back into individual nodes ("hmm, this node needs templating too") — it already has it. If a node needs to opt OUT (e.g. a raw-code field that must not touch `{...}`), that's the exception that carries the burden, not the rule.
- **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)`.
## Working style

View File

@@ -2,7 +2,23 @@
Items surfaced during review of one plan that belong to a later plan. Prevents silent drops.
## Deferred to Plan 5 (CI + observability + hardening)
## Deferred to Plan 6 (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.
- **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 5: 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 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.
## Deferred from Plan 5 (Node Editor Dialog)
- **Webhook stand-by mode.** Trigger option where an active workflow pauses on the webhook node and only continues when a matching HTTP request arrives, instead of running through the first request. Needs pause/resume semantics in `Executor`; separate spec.
- **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.<id>.output.body}` and `{nodes.<id>.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.
- **`useMemo` around `normalizePorts` in `NodeEditorDialog`.** Runs every render, allocating fresh `Port[]` arrays; blocks downstream `React.memo` on `PortsColumn`. Cheap-per-run but wasteful.
- **Tab-wrap focus-trap test.** Implementation is in place; jsdom doesn't simulate real browser tab-order so it's not unit-tested. Add a Playwright smoke assertion if the behavior ever regresses in practice.
- **Preview numeric-precision drift.** `frontend/src/expr/templateEngine.ts` renders JS numbers via `String(v)`, which loses fidelity vs the backend's `GetRawText()` for over-precision or non-normalized JSON (e.g., `1.50`, `9007199254740993`). Documented in the header comment; would need JSON source-text preservation to fix.