Skip to content

Commit 7807015

Browse files
alex-strukclaude
andcommitted
docs(workflow-builder): walk Parts 2 and 14; fix the 14.1 example
Parts 2 and 14 had never been walked in a browser. Part 14 is where the /dynamic-nodes admin breakage lived, so it went first. 19 of 19 runnable checks now walked. Three pass outright and needed nothing; the rest surfaced five findings, one already fixed in 64d86d7: D-9 the plan's OWN 14.1 example cannot publish — `dynamicNode(ctx, params)` is untyped and `deno check` runs under noImplicitAny, so the first command in Part 14 400s and everything downstream cascades. Replaced with a typed script matching the editor boilerplate. (fixed here) D-10 publishing from the palette modal never dropped the node (fixed, 64d86d7) D-11 Try/Run stay enabled on a graph whose dyn lineage is deleted, which the validator already reports as an error. NEEDS A RULING: does any error disable Try, or only structurally-unrunnable ones? D-12 a succeeded non-deterministic dyn node previews "cache evicted, re-run to repopulate" — nothing was evicted and re-running can never repopulate it, because such nodes are deliberately never cached D-13 the script editor fetches Monaco from cdn.jsdelivr.net at runtime. TLS interception (what a government network does) leaves it on "Loading…" forever with no error and Publish still enabled. monaco-editor is already in node_modules at the exact version served Security tier is genuinely solid: net egress, remote import and env access are all denied by the Deno sandbox, allowlisting a host lifts the denial (proving the allowlist is the gate), and the full typed-error matrix is 14/14 green against the live runner. Also records a method note — my first reading of D-11 was wrong because I sampled while the activity catalog was still loading, and dyn.* types are deliberately given the benefit of the doubt until it resolves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 64d86d7 commit 7807015

2 files changed

Lines changed: 169 additions & 17 deletions

File tree

docs-md/workflows/MANUAL_TEST_PLAN.md

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,11 @@ Each test below is one of: **✅ E2E** (a Playwright spec guards it), **🔬 uni
173173

174174
## Part 2 — Smoke Test (bring-up verification)
175175

176-
- [ ] **2.1** `curl -H "x-api-key: <KEY>" http://localhost:3002/api/workflows` returns `200` with a JSON array.
177-
- [ ] **2.2** `curl http://localhost:9099/health` returns `{"ok":true,"denoVersion":"2.1.4"}`. ⚙️
176+
- [x] **2.1** `curl -H "x-api-key: <KEY>" http://localhost:3002/api/workflows` returns `200` with a JSON array.
177+
- [x] **2.2** `curl http://localhost:9099/health` returns `{"ok":true,"denoVersion":"2.1.4"}`. ⚙️
178178
- [ ] **2.3** Browse to http://localhost:3000, complete IDIR login, land on the app shell. 🔑
179-
- [ ] **2.4** Temporal UI at http://localhost:8088 loads and shows the `default` namespace.
180-
- [ ] **2.5** Navigate to `/workflows` → list page renders with the kind filter (`Workflows / Libraries / All`).
179+
- [x] **2.4** Temporal UI at http://localhost:8088 loads and shows the `default` namespace.
180+
- [x] **2.5** Navigate to `/workflows` → list page renders with the kind filter (`Workflows / Libraries / All`).
181181

182182
---
183183

@@ -583,36 +583,40 @@ One `source.api` and one `source.upload` max per workflow.
583583
`DYNAMIC_NODE_ALLOW_NET` must be set **identically on both the backend and the Temporal worker** (read at startup — restart both to change). Unset = only the API base host is auto-granted.
584584

585585
### Publish / manage (API)
586-
- [ ] **14.1 Publish (create).**
586+
- [x] **14.1 Publish (create).**
587587
```bash
588+
# NOTE: `deno check` runs under `noImplicitAny` — an untyped
589+
# `dynamicNode(ctx, params)` fails with `400 stage:"ts-check"` before the
590+
# lineage is ever created. Type the parameters, as the editor boilerplate and
591+
# `seed-feature-demos.mjs` both do.
588592
curl -X POST http://localhost:3002/api/dynamic-nodes -H "x-api-key: <KEY>" \
589593
-H "content-type: application/json" \
590-
-d '{"script":"/**\n * @workflow-node\n * @name uppercase-url\n * @description Uppercases the document URL.\n * @inputs { document: { kind: \"Document\", required: true } }\n * @outputs { uppercased: { kind: \"Artifact\" } }\n */\nexport default async function dynamicNode(ctx, params){ return { uppercased: { url: ctx.document.url.toUpperCase() } }; }"}'
594+
-d '{"script":"import type { Document } from \"@ai-di/graph-workflow/kinds\";\n\n/**\n * @workflow-node\n * @name uppercase-url\n * @description Uppercases the document URL.\n * @inputs { document: { kind: \"Document\", required: true } }\n * @outputs { uppercased: { kind: \"Artifact\" } }\n */\nexport default async function dynamicNode(\n ctx: { document: Document },\n _params: Record<string, unknown>,\n): Promise<{ uppercased: { url: string } }> {\n const url = String((ctx.document as { url?: string }).url ?? \"\");\n return { uppercased: { url: url.toUpperCase() } };\n}"}'
591595
```
592596
**Pass:** `201 {slug:"uppercase-url", version:1, signature:{…}, errors:[]}`.
593-
- [ ] **14.2 Publish negative cases.** Malformed JSDoc → `400 stage:"jsdoc-parse"`; unknown kind → `400 stage:"signature-semantics"`; TS type error → `400 stage:"ts-check"` (from runner); duplicate of a **live** slug → `409 DUPLICATE_SLUG` (a *soft-deleted* slug re-POST **restores** instead — see 14.14).
594-
- [ ] **14.3 New version (update).** `PUT /api/dynamic-nodes/uppercase-url` with a modified script. **Pass:** `200 {version:2}`. `@name` ≠ path → `409 NAME_MISMATCH`; unknown/soft-deleted → `404`.
595-
- [ ] **14.4 List / detail.** `GET /api/dynamic-nodes` (+ `/:slug`). **Pass:** list sorted by slug, excludes soft-deleted, includes `headVersion`, `versionCount`, `usedInWorkflowCount`.
596-
- [ ] **14.5 Soft-delete.** `DELETE /api/dynamic-nodes/uppercase-url`. **Pass:** `200 {slug, deletedAt}`, idempotent, returns used-in-N count.
597-
- [ ] **14.6 Merged catalog.** `GET /api/activity-catalog`. **Pass:** includes `dyn.uppercase-url` with `dynamicNodeSlug/Version` + `colorHint:"dyn"` after static entries. A different group’s key does **not** see it (30s cache — allow a moment).
598-
- [ ] **14.14 Restore-on-republish.** Publish `uppercase-url` (v1) → **14.5 soft-delete** it → `POST /api/dynamic-nodes` with the **same** `@name`. **Pass:** `201` and the lineage is **restored**`version` continues the history (`v2`, not a fresh v1), `GET /:slug` is live again (`deletedAt:null`) with both versions. Re-POST once more while live → `409 DUPLICATE_SLUG` (the guard still fires for a genuine live clash). In the UI: delete a custom node, then **New custom node** with the same name — it re-appears instead of dead-ending. (`@infra` e2e: `tier1-dynamic-node`.)
597+
- [x] **14.2 Publish negative cases.** Malformed JSDoc → `400 stage:"jsdoc-parse"`; unknown kind → `400 stage:"signature-semantics"`; TS type error → `400 stage:"ts-check"` (from runner); duplicate of a **live** slug → `409 DUPLICATE_SLUG` (a *soft-deleted* slug re-POST **restores** instead — see 14.14).
598+
- [x] **14.3 New version (update).** `PUT /api/dynamic-nodes/uppercase-url` with a modified script. **Pass:** `200 {version:2}`. `@name` ≠ path → `409 NAME_MISMATCH`; unknown/soft-deleted → `404`.
599+
- [x] **14.4 List / detail.** `GET /api/dynamic-nodes` (+ `/:slug`). **Pass:** list sorted by slug, excludes soft-deleted, includes `headVersion`, `versionCount`, `usedInWorkflowCount`.
600+
- [x] **14.5 Soft-delete.** `DELETE /api/dynamic-nodes/uppercase-url`. **Pass:** `200 {slug, deletedAt}`, idempotent, returns used-in-N count.
601+
- [x] **14.6 Merged catalog.** `GET /api/activity-catalog`. **Pass:** includes `dyn.uppercase-url` with `dynamicNodeSlug/Version` + `colorHint:"dyn"` after static entries. A different group’s key does **not** see it (30s cache — allow a moment).
602+
- [x] **14.14 Restore-on-republish.** Publish `uppercase-url` (v1) → **14.5 soft-delete** it → `POST /api/dynamic-nodes` with the **same** `@name`. **Pass:** `201` and the lineage is **restored**`version` continues the history (`v2`, not a fresh v1), `GET /:slug` is live again (`deletedAt:null`) with both versions. Re-POST once more while live → `409 DUPLICATE_SLUG` (the guard still fires for a genuine live clash). In the UI: delete a custom node, then **New custom node** with the same name — it re-appears instead of dead-ending. (`@infra` e2e: `tier1-dynamic-node`.)
599603

600604
### Editor UI
601-
- [ ] **14.7 Management page.** Left-nav **Dynamic nodes**`/dynamic-nodes` list → **New dynamic node** → editor with prefilled boilerplate → edit → watch the **live parse strip** (300ms debounce) show green “Signature OK” or red line-anchored errors → Publish. **Pass:** on success the palette/catalog refresh **without a Vite restart**; on `400`, errors also show as Monaco gutter squiggles and clicking jumps to the line.
605+
- [x] **14.7 Management page.** Left-nav **Dynamic nodes**`/dynamic-nodes` list → **New dynamic node** → editor with prefilled boilerplate → edit → watch the **live parse strip** (300ms debounce) show green “Signature OK” or red line-anchored errors → Publish. **Pass:** on success the palette/catalog refresh **without a Vite restart**; on `400`, errors also show as Monaco gutter squiggles and clicking jumps to the line.
602606
- [ ] **14.8 In-canvas custom node.** Palette **Custom** section → **New custom node** modal → publish → node auto-drops as `dyn.<slug>` with a grape **DYN** badge. Right-click a `dyn.*` node → **Edit script**. **Pass:** deleted-lineage node shows a red **Deleted** badge, settings Alert, Try disabled.
603607

604608
### Execute + security
605609
- [ ] **14.9 Execute (Try).** Build `source.api → dyn.uppercase-url`, wire `document`, Save → **Try** with `{"documentUrl":"https://example.com/foo.pdf"}`. **Pass:** node goes blue→green; preview shows the uppercased URL. Publish v2 (reverse) → Try → cache miss → preview shows reversed URL.
606-
- [ ] **14.10 Runtime errors.** Script `throw``errorMessage` prefixed `[DynamicNodeRuntimeError] exitCode=1 …`; timeout (>60s) / stdout >5MB / invalid JSON / missing output port each map to their typed error (truncated 2KB).
607-
- [ ] **14.11 🔒 Network egress blocked.** Publish/run a node doing `await fetch("https://blocked.example.com")` with that host **not** in `DYNAMIC_NODE_ALLOW_NET`. Fast path — hit the runner directly:
610+
- [x] **14.10 Runtime errors.** Script `throw``errorMessage` prefixed `[DynamicNodeRuntimeError] exitCode=1 …`; timeout (>60s) / stdout >5MB / invalid JSON / missing output port each map to their typed error (truncated 2KB).
611+
- [x] **14.11 🔒 Network egress blocked.** Publish/run a node doing `await fetch("https://blocked.example.com")` with that host **not** in `DYNAMIC_NODE_ALLOW_NET`. Fast path — hit the runner directly:
608612
```bash
609613
curl -X POST http://localhost:9099/execute -H "content-type: application/json" -d '{
610614
"script":"export default async function(){ await fetch(\"https://blocked.example.com\"); return {ok:true}; }",
611615
"inputCtx":{},"parameters":{},"allowNet":[],"ambientEnv":{},"timeoutMs":5000,"maxMemoryMB":128}'
612616
```
613617
**Pass:** `exitCode != 0`, stderr mentions Deno net permission denied. Add the host to `allowNet` → same script succeeds (proves the allowlist is the gate). ⚠️ Locally the container still has NAT internet — you’re verifying the per-script Deno permission gate, not container isolation (true isolation only in OpenShift).
614-
- [ ] **14.12 🔒 Remote import blocked.** Script with `import … from "https://blocked.example.com/mod.ts"` where the host isn’t allowlisted. **Pass:** either `400 stage:"allowlist"` at publish (rejected host listed) or a runtime net-permission failure — never an actual outbound fetch.
615-
- [ ] **14.13 🔒 Env isolation.** A script reading `Deno.env.get("PATH")` (or anything beyond the 4 ambient vars `AI_DI_API_BASE_URL/API_KEY/GROUP_ID/WORKFLOW_RUN_ID`) returns undefined/fails. **Pass:** no host env leaks into the subprocess.
618+
- [x] **14.12 🔒 Remote import blocked.** Script with `import … from "https://blocked.example.com/mod.ts"` where the host isn’t allowlisted. **Pass:** either `400 stage:"allowlist"` at publish (rejected host listed) or a runtime net-permission failure — never an actual outbound fetch.
619+
- [x] **14.13 🔒 Env isolation.** A script reading `Deno.env.get("PATH")` (or anything beyond the 4 ambient vars `AI_DI_API_BASE_URL/API_KEY/GROUP_ID/WORKFLOW_RUN_ID`) returns undefined/fails. **Pass:** no host env leaks into the subprocess.
616620

617621
---
618622

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# Walkthrough — Parts 2 and 14 (2026-07-27)
2+
3+
Continuation of `WALKTHROUGH_PARTS_3_9.md`, which covered Parts 3–9 only. Parts
4+
2, 4, 10–16 had **never been walked in a browser**. This pass takes Part 2
5+
(smoke) and Part 14 (dynamic nodes) — the part that produced the
6+
`/dynamic-nodes` admin breakage Alex hit on 2026-07-27.
7+
8+
## Environment
9+
10+
Full local stack: backend + Temporal server + **Temporal worker** + postgres +
11+
minio + deno-runner (`{"ok":true,"denoVersion":"2.1.4"}`). Everything in Parts 2
12+
and 14 was runnable.
13+
14+
## Part 2 — Smoke
15+
16+
| # | Verdict | Evidence |
17+
|---|---|---|
18+
| 2.1 API reachable | ✅ PASS | `200`, 25 workflows. *Plan nit: it returns `{workflows:[…]}`, not a bare JSON array.* |
19+
| 2.2 deno-runner health | ✅ PASS | `{"ok":true,"denoVersion":"2.1.4"}` verbatim |
20+
| 2.3 IDIR login | ⏭ NOT AUTOMATABLE | needs real IDIR credentials; the mock-auth bypass reaches the app shell, which is as close as automation gets |
21+
| 2.4 Temporal UI | ✅ PASS | `200`; namespaces `["temporal-system","default"]` |
22+
| 2.5 Workflows list + kind filter | ✅ PASS | 25 rows; segmented control reads `Workflows / Libraries / All` |
23+
24+
## Part 14 — Dynamic (custom-code) nodes
25+
26+
| # | Verdict | Evidence |
27+
|---|---|---|
28+
| 14.1 Publish (create) | ⚠️ **PASS only after fixing the plan** | see D-9 |
29+
| 14.2 Publish negatives | ✅ PASS | `jsdoc-parse` "Missing @workflow-node marker"; `signature-semantics` "Unknown kind: NotAKind"; `ts-check` line 15; `409 DUPLICATE_SLUG` |
30+
| 14.3 New version | ✅ PASS | `200 {version:2}`; `@name` mismatch → `409 NAME_MISMATCH`; unknown slug → `404` |
31+
| 14.4 List / detail | ✅ PASS | slug-sorted; `versionCount:2`, `head:2`; detail newest-first `[2,1]` |
32+
| 14.5 Soft-delete | ✅ PASS | `200`, byte-identical `deletedAt` on the second call (idempotent), excluded from list |
33+
| 14.6 Merged catalog | ✅ PASS (one half) | both `dyn.*` entries carry `dynamicNodeSlug/Version` + `colorHint:"dyn"` and sit after all 41 static entries. **Cross-group isolation not provable with one API key** — the negative is: a non-admin naming a foreign group is refused outright |
34+
| 14.7 Management page | ✅ PASS | boilerplate prefilled; strip green→red→green; Publish **disabled** while JSDoc broken; on `400` a toast *"Publish failed (1 error) — see error markers"* plus a line-anchored Monaco marker (`line 14: Type 'Document' is not assignable to type 'number'`); on success *"Published v1"*`/dynamic-nodes/:slug` |
35+
| 14.8 In-canvas custom node | ✅ PASS after a fix / ⚠️ one gap | auto-drop was broken — see D-10. Deleted lineage: red **DELETED** badge, settings *"(deleted dynamic node)"*, validator *"Activity type 'dyn.walk-14-8-node' is not registered"* — but **Try is not disabled**, see D-11 |
36+
| 14.9 Execute (Try) | ⚠️ PARTIAL | the node really ran — both nodes `succeeded` through Temporal + deno-runner via the UI. The **preview does not show the value**, and says something untrue — see D-12 |
37+
| 14.10 Runtime errors | ✅ PASS | full typed-error matrix green against the live runner (14/14, `RUN_INTEGRATION=1`): timeout → `DynamicNodeTimeoutError`, 6MB stdout → `StdoutTooLarge`, throw → `RuntimeError` w/ stderrTail, bad JSON → `OutputInvalidJson`, missing port → `OutputShapeError` |
38+
| 14.11 🔒 Net egress | ✅ PASS | `allowNet:[]``NotCapable: Requires net access to "example.com:443"`; allowlist the host → `exit 0 {"status":200}`. The allowlist **is** the gate |
39+
| 14.12 🔒 Remote import | ✅ PASS | `Requires import access to "blocked.example.com:443"` — no outbound fetch |
40+
| 14.13 🔒 Env isolation | ✅ PASS | `Deno.env.get("PATH")``NotCapable: Requires env access to "PATH"` |
41+
| 14.14 Restore-on-republish | ✅ PASS | re-POST after soft-delete → `201 v3` (history continued, not a fresh v1), `deletedAt:null`, versions `[3,2,1]`; re-POST while live → `409 DUPLICATE_SLUG` |
42+
43+
---
44+
45+
## D-9 — the plan's own 14.1 example cannot publish (doc defect, fixed)
46+
47+
The `curl` body in 14.1 declares `dynamicNode(ctx, params)` with no parameter
48+
types. `deno check` runs under `noImplicitAny`, so the very first thing anyone
49+
following Part 14 does fails:
50+
51+
```
52+
400 ts-check line 8: Parameter 'ctx' implicitly has an 'any' type.
53+
```
54+
55+
Everything downstream cascades — 14.3 `400`, 14.4/14.5 `404`, 14.14 unreachable
56+
— because the lineage is never created. Both real sources of truth (the editor
57+
boilerplate and `seed-feature-demos.mjs`) type their parameters; only the plan
58+
does not. With a correctly typed script the whole chain passes on the first run.
59+
60+
## D-10 — publishing a custom node never dropped it on the canvas (fixed, `64d86d73`)
61+
62+
14.8's headline behaviour. The palette modal publishes, closes, toasts
63+
*"Published v1"* — and the canvas is unchanged. `addDynamicNode` looks the new
64+
`dyn.<slug>` up in the merged catalog and returns silently when absent, and it
65+
was absent for two independent reasons: the publish mutation fired
66+
`invalidateQueries` without returning the promise (so `mutateAsync` resolved a
67+
round-trip early), and even once awaited, React has not re-rendered at that
68+
instant — so a closed-over array *and* a ref updated during render are both a
69+
commit behind. Reads the TanStack cache first now. Verified live: 4 → 5 nodes.
70+
71+
## D-11 — Try stays enabled on a graph that cannot run (open — needs a ruling)
72+
73+
A workflow whose `dyn.*` lineage has been soft-deleted is correctly diagnosed:
74+
red **DELETED** badge, settings alert, and `1 error — Activity type
75+
"dyn.walk-14-8-node" is not registered`. But **Try and Run remain enabled**, and
76+
clicking Try opens the drawer as usual. Running it fails at
77+
`dynamicNode.resolveLineage` with `DynamicNodeDeletedError` — knowable at author
78+
time, which is what the *"Fail before the run"* invariant asks for.
79+
80+
14.8 states the criterion as "Try disabled". Not fixed unilaterally because the
81+
scope is a product decision: **does any validation error disable Try/Run, or
82+
only errors that make the graph structurally unrunnable?** Those are very
83+
different products. Needs Alex.
84+
85+
> ⚠️ Method note: my first reading of this said "Valid, Try enabled" and was
86+
> **wrong** — I sampled at 3.2s while the activity catalog was still loading,
87+
> and `isRegisteredActivityType` deliberately gives `dyn.*` the benefit of the
88+
> doubt until it resolves. At 6.5s the error is there. Any check touching
89+
> `dyn.*` validation must wait for the catalog.
90+
91+
## D-12 — a succeeded dynamic node previews an untrue message (open)
92+
93+
After a green run, the dyn node's preview reads:
94+
95+
> Preview unavailable — cache evicted. Re-run to repopulate.
96+
97+
Both halves are wrong. Nothing was evicted, and re-running will *never*
98+
repopulate it: the node is `@deterministic: false` (the default — the signature
99+
card even says **NON-DETERMINISTIC (NOT CACHED)**), and §3.3 says such scripts
100+
must re-execute every run and are deliberately not cached. So the widget offers
101+
an action that cannot work, for a reason that never happened.
102+
103+
14.9's "preview shows the uppercased URL" therefore holds only for a
104+
`@deterministic: true` node. The honest message is "not cached — this node is
105+
non-deterministic", which needs the widget to see the entry's `deterministic`
106+
flag.
107+
108+
## D-13 — the script editor loads Monaco from a public CDN (open)
109+
110+
The only external request the app makes:
111+
112+
```
113+
GET https://cdn.jsdelivr.net/npm/monaco-editor@0.55.1/min/vs/loader.js
114+
```
115+
116+
In this sandbox it fails with `ERR_CERT_AUTHORITY_INVALID` — TLS interception,
117+
exactly what a government network does — and the editor sits on **"Loading…"
118+
forever**: no error, no timeout, no fallback, and **Publish stays enabled**, so
119+
you can publish the untouched boilerplate without noticing. I did that twice by
120+
accident before spotting it.
121+
122+
Nothing blocks it in deployment (the frontend nginx sets no CSP), so whether the
123+
authoring surface works depends entirely on each user's network reaching
124+
jsdelivr. `monaco-editor@0.55.1` is already in `node_modules` at the exact
125+
version served, so the fix is `loader.config({ monaco })` against the local
126+
package. Not done here because it changes the build and needs the dependency
127+
declared — Alex's call.
128+
129+
For the walkthrough I served the byte-identical local copy via a Playwright
130+
route so the 14.7/14.8 editor checks could run.
131+
132+
---
133+
134+
## Artifacts created (safe to delete)
135+
136+
| Kind | Name / id |
137+
|---|---|
138+
| Library workflow | `WALK-7.8 typed ports``cms3tqu3z0000f2gc1b04jxz2` |
139+
| Workflow | `WALK-14.8 deleted-badge probe``cms3v1qds000hf2gcesg9ucy3` |
140+
| Workflow | `WALK-14.9 dyn execute probe``cms3v9c0x000kf2gcn1w3dtqn` |
141+
| Dynamic node | `uppercase-url` (v3, live) |
142+
| Dynamic node | `walk-14-7-node` (v1, live) |
143+
| Dynamic node | `walk-14-8-node`, `my-custom-node` (soft-deleted) |
144+
145+
## Still unwalked
146+
147+
Parts **4, 10, 11, 12, 13, 15, 16** in full, plus the open checks in Parts 3, 5,
148+
6, 7, 8, 9. Part 15 additionally needs cloud credentials.

0 commit comments

Comments
 (0)