Skip to content

Commit 1b23e2e

Browse files
alex-strukclaude
andcommitted
docs(workflow-builder): record the six rulings and re-walk 9.7, 9.12, 14.8
All five approved findings are shipped (D-14/CI deferred by Alex). Re-walked the three checks whose expected behaviour the fixes changed: - 9.7 cancel-on-new-Try now passes against the 15s slow-node fixture. - 9.12 is rewritten: it documented auto-save-on-Try, and the shipped decision (D-16) is refuse-while-dirty. Both satisfy the invariant "never run a graph the author is not looking at"; the refusal is the one that doesn't mint a version on every Try. Verified with 0 run/try POSTs across the pass. - 14.8 passes end to end on the deleted-lineage demo — badge, top-bar error, settings Alert, Try and Run both refusing. Also records the D-16 regression that only live verification caught: every demo ships arrangeOnLoad, so the auto-arrange left the editor "dirty" from the moment it opened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 907ceaa commit 1b23e2e

2 files changed

Lines changed: 54 additions & 7 deletions

File tree

docs-md/workflows/MANUAL_TEST_PLAN.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,8 @@ Requires Temporal server + **worker** + visibility store + `activity_output_cach
492492
- **Control-flow node (switch/map/join/humanGate/childWorkflow/pollUntil):** draws no copy (it never writes an output row), and must NOT show a cache-evicted alert.
493493
- Inspect via devtools: the preview wrapper carries `data-state="not-started" | "running" | "branch-not-taken" | "failed" | "cancelled" | "evicted" | "not-previewable" | "no-run"` — one state per situation, and the same names on the wire-peek popover.
494494
- [x] **9.11 Version run-count badge.** **More ▸ History** after some runs. **Pass:** each version row shows a `{n} runs` badge.
495-
- [ ] **9.12 Lazy deploy + auto-save on first Try.** Try on a workflow with unsaved changes. **Pass:** a new version is saved before Temporal starts; if validation fails, Try aborts with the validator toast and **no** Temporal resource is used.
495+
- [x] **9.12 A Try never runs a graph you are not looking at.** Try on a workflow with unsaved changes, and on one with a validation error. **Pass:** Try and Run are both **disabled**, each explaining itself — *"Save your changes first — a run always executes the saved graph, not the canvas"* / *"Fix N validation errors first — this graph cannot run as it stands"* — and **no** Temporal resource is used.
496+
- *Changed by D-16.* This check used to read "a new version is saved before Temporal starts". The shipped behaviour is refuse-while-dirty, not auto-save: auto-saving would mint a version on every Try and quietly change what the version history means. Either satisfies the underlying invariant (never run a graph the author is not looking at); the refusal is the one that leaves versioning alone.
496497
- [x] **9.13 Wire data peek.** After a Try completes, **click a data wire** (a coloured port-to-port wire) on the canvas. **Pass:** a popover opens at the wire midpoint showing the value that flowed across it — a kind widget where one exists, otherwise a truncated JSON snippet. Click a wire **before any run**: **Pass:** the popover reads **“Run this workflow to see what this step produces.”** — and mid-run it reports the producer’s actual state (“Running now…”, “Waiting — the run hasn’t reached this step yet.”) rather than the same sentence (G-012). Peeking an **OCR** wire shows the dereferenced values, not the blob pointer (G-022). **Right-click** a data wire: **Pass:** the menu shows **“View data”** only after a run has happened (it’s absent before the first run), and choosing it opens the same popover. ⚠️ The peek is scoped to the current run only — after a page reload (which clears the active run) a wire shows the “Run to see…” state and “View data” disappears until you Try again.
497498

498499
---
@@ -603,7 +604,7 @@ One `source.api` and one `source.upload` max per workflow.
603604

604605
### Editor UI
605606
- [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.
606-
- [ ] **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.
607+
- [x] **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.
607608

608609
### Execute + security
609610
- [x] **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.

feature-docs/20260724-workflow-builder-spec-completion/WALKTHROUGH_PARTS_2_14.md

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ round-trip early), and even once awaited, React has not re-rendered at that
6868
instant — so a closed-over array *and* a ref updated during render are both a
6969
commit behind. Reads the TanStack cache first now. Verified live: 4 → 5 nodes.
7070

71-
## D-11 — Try stays enabled on a graph that cannot run (open — needs a ruling)
71+
## D-11 — Try stays enabled on a graph that cannot run (FIXED, `20624c1f`)
7272

7373
A workflow whose `dyn.*` lineage has been soft-deleted is correctly diagnosed:
7474
red **DELETED** badge, settings alert, and `1 error — Activity type
@@ -88,7 +88,7 @@ different products. Needs Alex.
8888
> doubt until it resolves. At 6.5s the error is there. Any check touching
8989
> `dyn.*` validation must wait for the catalog.
9090
91-
## D-12 — a succeeded dynamic node previews an untrue message (open)
91+
## D-12 — a succeeded dynamic node previews an untrue message (FIXED, `20624c1f`)
9292

9393
After a green run, the dyn node's preview reads:
9494

@@ -105,7 +105,7 @@ an action that cannot work, for a reason that never happened.
105105
non-deterministic", which needs the widget to see the entry's `deterministic`
106106
flag.
107107

108-
## D-13 — the script editor loads Monaco from a public CDN (open)
108+
## D-13 — the script editor loads Monaco from a public CDN (FIXED, `907ceaac`)
109109

110110
The only external request the app makes:
111111

@@ -398,7 +398,7 @@ wrong at the join — and only reachable by driving the real UI.**
398398
| 9.7 Cancel-on-new-Try | ⚠️ INCONCLUSIVE | my fixture cannot test it — the dyn node completes in ~85 ms, so there is no window in which a second Try could cancel the first. Needs a deliberately slow workflow (a sleeping dynamic node or a `pollUntil`) |
399399
| 9.5 / 9.5a / 9.5b / 9.5c / 9.10 / 9.10a / 9.4a | ⏳ NOT WALKED | need OCR-shaped outputs and a switch-heavy graph |
400400

401-
## D-16 — Try silently runs a different graph than the one on screen (open)
401+
## D-16 — Try silently runs a different graph than the one on screen (FIXED, `20624c1f` + `1a8ab34b`)
402402

403403
9.12 says: *"Try on a workflow with unsaved changes. **Pass:** a new version is
404404
saved before Temporal starts."* That does not happen, and the behaviour is not
@@ -433,7 +433,7 @@ Two defensible fixes, and the choice is a product decision:
433433

434434
Not implemented unilaterally. Needs Alex.
435435

436-
## D-17 — no editor run is ever stamped "try", so cancel-on-new-Try never fires (open)
436+
## D-17 — no editor run is ever stamped "try", so cancel-on-new-Try never fires (FIXED, `5b37af8d`)
437437

438438
9.7 asks: start a Try, Try again mid-run → *"prior run cancelled server-side
439439
(shows cancelled in Run history); **exactly one active run**."*
@@ -516,3 +516,49 @@ preview family (9.5/9.5a/9.5b/9.5c/9.10/9.10a/9.4a), which needs OCR-shaped
516516
outputs and a switch-heavy graph; then Part 4's condition deep-half
517517
(4.8–4.12, 4.14); then Part 15's three long agent builds. 12.4/12.5 stay blocked
518518
on demo gap **D4**, 14.8/14.9 on rulings **D-11/D-12**, and 2.3 needs real IDIR.
519+
520+
---
521+
522+
# Dispositions — the six findings that needed a ruling (2026-07-27)
523+
524+
Alex approved the recommended option on all six, deferring only CI:
525+
*"don't worry about ci for now, but you can run heavy tests locally (temporal,
526+
deno, etc) as needed. Otherwise i'm good with your choices."*
527+
528+
| # | Ruling | Shipped as |
529+
|---|---|---|
530+
| **D-11** | Refuse to Try/Run a graph with validation errors | `20624c1f` |
531+
| **D-12** | Say "not cached" for a never-cached node; offer no Re-run | `20624c1f` |
532+
| **D-13** | Bundle Monaco locally + bound the bring-up | `907ceaac` |
533+
| **D-14** | **Deferred** — no CI work this pass ||
534+
| **D-16** | Refuse while dirty (option 2), NOT auto-save on Try | `20624c1f`, `1a8ab34b` |
535+
| **D-17** | Dedicated `POST /:id/tries` (option b), NOT a public `trigger` field | `5b37af8d` |
536+
537+
D-16 and D-17 were decided together, as flagged — both are the Try path, and
538+
taking option 2/b for each gives a clean split: Try has its own endpoint that
539+
stamps `"try"` server-side and refuses while the canvas is ahead of the saved
540+
graph, and the public run API is untouched.
541+
542+
**One regression, caught only by live verification.** D-16's gate disabled
543+
Try/Run on every *freshly opened* demo, before any edit: every demo ships
544+
`metadata.arrangeOnLoad`, and the auto-arrange rewrites `config` ~1.5s after
545+
mount, so `config !== lastHydratedConfigRef.current` and the editor considered
546+
itself dirty from the moment it opened. That was already wrong for the G-027
547+
leave-guard (it would warn on a workflow nobody had touched); D-16 only made it
548+
visible by turning "dirty" into a refusal. Fixed in `1a8ab34b` by re-basing the
549+
baseline inside `handleArrangeOnLoad`. The unit suite was green throughout.
550+
551+
**Re-walked after the fixes**
552+
553+
| # | Verdict | Evidence |
554+
|---|---|---|
555+
| 9.7 Cancel-on-new-Try | ✅ PASS | two Trys 1.8s apart on the 15s `walk-slow-node` fixture → first `cancelled`, second `succeeded`. Before D-17: both `succeeded`, none cancelled |
556+
| 9.12 (rewritten) | ✅ PASS | clean demo → Try enabled; after dragging a node → both disabled with *"Save your changes first…"*; **0** run/try POSTs across the whole pass |
557+
| 14.8 In-canvas custom node | ✅ PASS | deleted-lineage demo → red **DELETED** badge on the node, `1 error` on the top bar, settings Alert *"This dynamic node was deleted. Restore from the management page…"*, Try + Run both disabled with *"Fix 1 validation error first"* |
558+
559+
**9.12's expectation changed, and the plan now says so.** It used to read "a new
560+
version is saved before Temporal starts" — the auto-save design. The shipped
561+
behaviour is the refusal. Both satisfy the underlying invariant (never run a
562+
graph the author is not looking at); the refusal is the one that does not mint
563+
a version on every Try.
564+

0 commit comments

Comments
 (0)