Visual Workflow Builder (Phases 1–8 + AI agent) → develop - #230
Draft
alex-struk wants to merge 532 commits into
Draft
Visual Workflow Builder (Phases 1–8 + AI agent) → develop#230alex-struk wants to merge 532 commits into
alex-struk wants to merge 532 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "04-list-after-delete.png", | ||
| ], | ||
| }; | ||
| writeFileSync(`${OUT_DIR}/summary.json`, JSON.stringify(summary, null, 2)); |
| if (m) results.workflowsCreated.push(m[1]); | ||
|
|
||
| console.log('S2: file drop'); | ||
| await fs.writeFile('/tmp/wb-phase7-verify/sample.pdf', '%PDF-1.4\n1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj\n2 0 obj<</Type/Pages/Count 1/Kids[3 0 R]>>endobj\n3 0 obj<</Type/Page/Parent 2 0 R/MediaBox[0 0 200 200]/Contents 4 0 R>>endobj\n4 0 obj<</Length 16>>stream\nBT /F1 12 Tf ET\nendstream endobj\nxref\n0 5\n0000000000 65535 f \n0000000009 00000 n \n0000000050 00000 n \n0000000091 00000 n \n0000000150 00000 n \ntrailer<</Size 5/Root 1 0 R>>\nstartxref\n210\n%%EOF\n'); |
| } | ||
|
|
||
| results.totalDurationMs = Date.now() - start; | ||
| await fs.writeFile(`${OUT}/summary.json`, JSON.stringify(results, null, 2)); |
| let i = 0; | ||
| while (i < blockLines.length) { | ||
| const line = blockLines[i]; | ||
| const tagMatch = line.match(/^\s*(@[a-zA-Z][a-zA-Z0-9_-]*)\b\s*(.*)$/); |
| } | ||
|
|
||
| function jsonResponse(status: number, body: unknown): Response { | ||
| return new Response(JSON.stringify(body), { |
Comment on lines
+521
to
+532
| const res = await fetch( | ||
| `/api/workflows/${currentWorkflowId}/sources/${sourceNodeId}/upload`, | ||
| { | ||
| method: "POST", | ||
| headers: Object.fromEntries( | ||
| Object.entries(getAgentAuthHeaders(activeGroupId)).filter( | ||
| ([k]) => k.toLowerCase() !== "content-type", | ||
| ), | ||
| ), | ||
| body: form, | ||
| }, | ||
| ); |
Comment on lines
+613
to
+615
| const wfRes = await fetch(`/api/workflows/${workflowId}`, { | ||
| headers: getAgentAuthHeaders(activeGroupId), | ||
| }); |
| } | ||
| } | ||
| }, | ||
| { root: null, rootMargin: "0px", threshold: 0 }, |
| // JSON5's line numbers are within `rawValue`. The rawValue's first line is | ||
| // the same source line as the tag itself, so file_line = tagFileLine + | ||
| // (parseLine - 1). | ||
| let line = tagFileLine; |
| ] as const; | ||
|
|
||
| /** Tags whose VALUE is a JSON-ish object or array. */ | ||
| const JSON_VALUE_TAGS = new Set<string>([ |
alex-struk
added a commit
that referenced
this pull request
Jul 8, 2026
…gs, cleanup Works through the PR #230 review of the visual workflow builder. All bug and blocker findings across sections 1-6 are fixed with tests; two items (artifact split, generic list editor) were deliberately deferred/declined with documented rationale. Infra: wire deno-runner env into worker + backend pods; build+push the deno-runner image (CI matrix, oc-build-push.sh, per-instance overlay); run the graph-workflow jest suite in CI; add linux-arm64 native-binary pins; document the deno-runner NetworkPolicy DNS rationale; fail fast on missing PLATFORM_API_KEY. Security/tenancy: agent workflow tools assert the workflow's group (closes cross-tenant read+write); getNodeStatuses scopes runId to the lineage; deleteDynamicNode slug bounded+encoded; conversation resume rejects group mismatch. Engine/cache: leaf-scoped cache snapshot + deep-merge restore (no sibling clobber); best-effort source-node write; P2002 dead-path made best-effort; honor DynamicNodeVersion.deterministic; version-pin resolution for name-referenced children; validator accepts source-produced ctx keys; Map fan-out propagates workflowLineageId; getInputCtx fail-safe 404 on retention + fail-closed cross-lineage. Frontend: NodePicker typeable on large graphs; Auto-arrange moves nodes; agent create->edit route; agent chat no longer stomps unsaved edits; chat-queued uploads fire; run-drawer duplicate run removed; cache-evicted alert gated on replay; renameCtxKey rewrites all refs; switch-case edge labels, error-fallback edges, fresh-Try polling, errorPolicy handle, and literal-input type-flip fixed. Housekeeping: remove the unimplemented Join "Any" strategy; gate the dev-only form-preview route behind import.meta.env.DEV; document the HumanGate continue-path test skips (test-env hang, not a bug); drop the last explicit any (deno-runner harness); delete the dead subprocess-harness placeholder. Cleanup: move benchmark-OCR replay knowledge to an ActivityCatalogEntry metadata field (engine stays workload-generic); add builderFetch (shared auth + cookies + ApiService 401 refresh) and migrate the 9 fetch-based builder hooks; extract a generic LruTtlCache and consolidate the two backend LRU+TTL caches; add a shared replaceNode helper + shared artifact-shape detectors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…im + rejection notice Task 7 (PORT_WIRING_DESIGN.md §6.2): isValidConnection rejects incompatible port-to-port drags (wildcard Artifact targets and node-level gestures stay permissive, self-connections always rejected); PortDragContext lets each input row self-classify as a compatible (enlarged handle) or incompatible (dimmed row) drop target while a port drag is in progress; onConnectEnd shows a plain-language "X can't be used as a Y" notice on a rejected port-to-port drop. Carry-overs from the Task 6 review: reworded a stale DOM-order comment, corrected the workflow-editor-canvas.css comment now that data wires are selectable/deletable (§6.3) and verified in-browser that the pointer-events override is likely redundant today but left in place, swapped the drag-to-bind test fixtures to a real catalog pair (file.prepare -> azureOcr.submit), and documented + pinned ensureEdgeBetween's same-pair-different-edge-type behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-ups on Task 7 (§6.2):
- A port drop on the SAME node reached the kind-mismatch notice
(isValidConnection rejects self first, so isValid is false even with
matching kinds), producing self-contradictory copy like "Document can't
be used as a Document". It now shows a distinct "A step can't feed
itself" notice, and the handler's doc comment no longer claims kind
mismatch is the only invalid-port-pair cause.
- Reworded the kind-mismatch message to drop the hardcoded indefinite
article ("as a OcrResult" was reachable for vowel-initial kinds):
"This input needs <target> — <source> can't be used here".
- Pinned the incompatible-row dim (opacity 0.35) in the PortRows test,
not just the data-drop-compatible attribute.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sults After a node-level connect (drag node-to-node, hover-extend pick), a transient popover on the target now lists what auto-wire did per port — checkmark rows for bound inputs (auto-bound/pinned/ctx-bound), warning rows with a Fix button for problem ports (unsatisfied/ambiguous/ locked-unbound) — replacing the previous silent auto-wiring (PORT_WIRING Phase 3, Task 8, §6.4). Port-to-port drags are unaffected — the pinned wire itself is already the feedback. Row semantics are extracted into a new shared settings/input-row-resolution.ts module (resolveWireableInputRows + effectiveResolution) reused by both InputsSection and the new popover, so the two surfaces can never drift. The canvas gains an onFixNodeInput prop (wired from the page's existing handleFixNodeInput) so a popover's Fix button deep-links into the settings-panel source picker at the right port. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re-renders The popover's 8s auto-dismiss effect was keyed on [opened, onClose], and the canvas passed onClose as an inline closure — a new identity on every canvas re-render (handle hover, selection, drags, config changes) — so each re-render cleared and re-armed the full 8s, indefinitely deferring dismissal while the user interacted. The effect now holds onClose in a ref and keys on [opened, nodeId], so identity churn can't touch the timer, and retargeting to a different node mid-window deliberately restarts the countdown. The canvas also passes a stable useCallback onClose. Regression-tested by re-rendering with a fresh onClose closure at t=4s and asserting dismissal at t=8s from open, not t=12s. Also: rewrite a stale RAF-era test comment, drop a dead if(rect) check after getBoundingClientRect, and make effectiveResolution module-private (no external consumers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng (spec §9) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t in extend popover Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, connect summary, kind-aware popover Adds dragConnectPorts (per-port drag-connect helper, mirrors dragConnect's outer-edge grab technique) and a new tier2-port-wiring spec covering PORT_WIRING_DESIGN.md §6/§9: drag-to-bind round-trip with reload persistence, connect-time kind rejection notice, wire delete → pinned-unbound → revert-to-automatic, the node-level connect-summary popover, and the kind-aware hover-extend popover.
…rt problems Since required base-Artifact identifier ports joined the validation problems surface, an unbound `documentId` on a `file.prepare` node reads as a warning. Bind it (to a declared ctx variable) on `buildLinearConfig`'s prep node and tier2-validation's orphan fixture so a valid fixture reads Valid and the unreachable-node test's badge count returns to 1. Full tests/e2e/workflow-builder sweep green (59/59, --workers=6).
…ics, kind-aware popover, limitations
…ist) example + resolve §4.2 ring note
Design for the port-wiring redesign §11: a 'From a step' picker as the default Ref experience in the condition editor, storing the same ctx path (ValueRef schema unchanged), with raw-key entry demoted to an advanced escape. Nested Ref|Literal layout, a separate ConditionProducerPicker that lists every upstream output port (no kind filter), producer-binding materialization helpers, and display resolution back to Node -> Port. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… node outputs Six-task TDD plan executing the §11 spec: pure producer-binding helpers, ConditionProducerPicker (no kind filter), ValueRefEditor step/manual sub-modes + prop threading, settings-form wiring, e2e, docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…in resolveCtxKeyToProducer
…utation Picking a step in a condition previously fired two onConfigChange calls from the same stale config; the ref update clobbered the producer output-binding update, so a producer consumed only by a condition never got its outputs[] binding persisted and the executor wrote nothing to ctx (condition read undefined at runtime). Now the step-picker emits only the ref change, and each control-flow settings form reconciles producer output bindings from its condition refs in the same committed config (idempotent; also self-heals hand-typed keys). Gate the step-picker on currentNodeId presence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Click a data wire → popover at the wire showing the value that flowed across it, via the existing preview cache + kind widgets, else a truncated-JSON fallback. Corrects §10's result[port] to outputCtx[ctxKey]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6-task TDD plan: renderKindValue extraction, JsonValuePreview fallback, WirePeekPopover state matrix, edge-selection mount, View-data context menu, @infra e2e + docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…says so (G-039) Two problems, both silent. The promotion was arbitrary: `Object.keys(nodesCopy)[0]` is insertion order, not graph order, so it usually promoted a node with inbound edges — an entry point that cannot be one — and the graph was invalid the instant the delete landed. `resolveNextEntryNodeId` now prefers a surviving source node, then a survivor with no inbound edges, and only falls back to "any survivor" for a fully-cyclic remainder. And it was unannounced. Where a workflow STARTS is at least as consequential as an orphaned variable, which has had a toast since G-002; this had nothing. The delete notice now carries the promotion, on its own when nothing is orphaned and appended to the orphan message when something is. The toast reads the promoted id from `resolveNextEntryNodeId` — the same function the delete uses — rather than recomputing it. Two implementations of "which node is promoted" would drift and the toast would eventually name a node the config never adopted. Three existing tests deleted the entry node incidentally while asserting orphan behaviour. Rather than loosen their assertions, each fixture keeps a surviving entry node so it stays about one thing, and the promotion gets its own cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s that throw (G-036, G-071) G-036 — `validateMapJoinNodes` checked only that `sourceMapNodeId` exists and is a map. `executeBranchSubgraph` allocates `mapBranchResults` per ITERATION, so a join outside that scope reads results that were discarded when the item finished, and throws `No results found for map node <id>` at run time. Two shapes are now refused: a source loop nested inside a body the join is outside of, and a join sitting inside its own source loop's body (the loop has not finished when it runs). The picker made both easy to build — `filterType="map"` was its only filter, so it listed every map with no dimming. It now reads `joinableMapIds`, the same helper `validateJoinScope` uses, so the editor cannot offer a choice Save then refuses. G-071 — the body-entry picker had no filter at all. It now excludes `source` (the workflow's front door; it runs once at intake, not per item), `join` (it exists to follow a loop) and `humanGate` (refused inside a body by G-070, so offering it was offering a guaranteed Save error). Nested loops stay listed — that shape is legitimate and `validateJoinScope` reasons about it explicitly. `collectMapBodyNodeIds` is now derived from a richer `collectMapBodyMembership` (nodeId → owning map ids), because "which bodies is this in" is what makes the scope rule expressible; "is it in some body" was not enough. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d a chip's Delete explains itself (G-095, G-091) G-095 — `getAggregateStatus` returned four of the six statuses, so collapsing a group DESTROYED information its expanded members were showing. `skipped` folded into `succeeded`, so a group served entirely from cache showed a green tick where every member showed the violet bolt; `cancelled` fell through to `pending`, so an aborted run's group read as "not started yet" — the opposite of what happened. That second case became reachable rather than hypothetical when G-047 made `cancelled` a real `NodeRunStatusValue`. Both statuses already had a badge treatment, so nothing new had to be drawn. G-091, first half — a selected chip was deletable, so Delete routed its synthetic `group-chip-<id>` into `handleDelete` where it matched no node, edge or group: nothing happened and nothing said why. It is now refused with the affordance that does work named. Refused rather than guessed: deleting a group is not the same act as deleting the steps inside it, and picking one for the author is how work gets lost. A mixed chip + node gesture still deletes the real nodes. G-091, second half — `deleteGroup` writes `nodeGroups` and nothing else, so the right rail stayed mounted on a group that no longer existed and fell through to its "Group not found" placeholder, a dead end reached by the panel's own Delete button. Guarded on derived state rather than inside `deleteGroup`, so it holds for every path that can remove a group: the panel, an undo, an agent write, or a group emptied by `pruneNodesFromGroups`. Two page tests drove `onGroupChipClick` with an id that was not in the config. A chip can only exist for a group that IS in `nodeGroups`, so the fixtures now declare it — more realistic, and no longer relying on the page tolerating a group that cannot exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion state (G-031) The badge-sync effect returned early for `source` and `group-chip` nodes, each with a stated reason that had since gone stale. Source nodes carry ERROR-severity rules (`nodes.<id>.sourceType`, `nodes.<id>.parameters<suffix>`) that reached the drawer and the top-bar count with nothing on the card, and a chip COLLAPSES its members — taking their badges off the canvas with them. So the top bar could read "N issues" while nothing on the graph was marked. Source cards mount the same corner badge every other node type does, with the same deep-link callback, so their issues are clickable rather than merely visible. Chips roll their members' counts up — the chip already aggregates run status this way; validation was the missing axis. `ValidationBadge` moves out of `WorkflowEditorCanvas` into its own module so renderers outside that file can mount it. The extraction is behaviour-neutral: the suite passed unchanged before any of the wiring was added. NOT included: the five edge anchor shapes the entry also names. Marking an edge needs a visual language for "this connection has a problem" that the canvas does not have yet, and inventing one is a design decision rather than a fix. The anchors are already reachable from the drawer (G-010), so they are navigable today — just not marked in place. Left in the register. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records each ruling against its register entry plus a decision-sheet summary of
the batch. Alex's steer ("not interested in new features, just ship bug free")
split the 27 confirmed gaps into 16 missing-capability entries, set aside, and
11 things broken in what already ships.
Two findings worth keeping outside the commit messages:
- Re-checking the top five against source (after G-051 turned out inverted)
found TWO that were worse than written. G-032 is real data corruption —
`writeToCtx` ends `current[finalKey] = value` with no undefined guard. G-070's
resume path is addressed by workflow id with a FIXED signal name, so there is
no per-iteration address at all, which is what made "refuse it" the honest fix
rather than "route it".
- Two entries were resolved by declining to guess. A gate in a loop cannot work,
so Save refuses and says why; a chip's Delete has two plausible readings, so
it refuses and names the affordance that does work.
Also records what was deliberately NOT fixed and why: G-031's edge half (needs
a visual language the canvas lacks), G-036's untaken-branch half (reachability,
not scope), G-052 (measured at 23/111 nodes — noise on day one) and G-034
(documented as intentional and shipped that way).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An earlier verification note called G-051 "inverted" and recommended rejecting it. That was wrong. It read `dynamic-node.repository.ts` — the backend CRUD path — and never reached `resolve-lineage.activity.ts`, which is what the runtime actually calls. The two contradict each other: the repository documents that version rows are kept "so workflows pinned to a specific version of a soft-deleted lineage continue to resolve at runtime", while the resolver throws `DynamicNodeDeletedError` as soon as `deletedAt !== null` — before the pinned-version branch is reached. The pin is never consulted, so soft-delete breaks every consumer, and the pinned case is the one the design promised would survive. Kept as a correction rather than a silent edit, because the method lesson generalises: verification that reads only the file the register cites will confirm whatever that file says. An entry describing a contract between two modules has to be checked at both ends, and only the honouring end is the truth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-pinned consumers (G-051) `DynamicNodeRepository.softDelete` is lineage-only and keeps every version row on purpose, documenting that "workflows pinned to a specific version of a soft-deleted lineage continue to resolve at runtime". `resolveLineage` checked `deletedAt` BEFORE the pinned-version branch, so the pin was never consulted: every consumer failed, and preserving the version rows bought nothing. The check now applies only to head-tracking consumers, which is what retiring a lineage is for — anything following the head stops, anything that deliberately pinned an immutable version keeps working. A missing lineage, a missing pinned version, and a head-tracking consumer all still fail exactly as before. I had previously verified this entry as "inverted" and recommended rejecting it. That was wrong: I read the repository (where the promise is written) and never reached the resolver (where it is broken). The register and decision sheet carry the correction, because the method lesson generalises — an entry describing a contract between two modules has to be checked at both ends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…le; delete the unreachable status aggregate (G-046, G-081) G-046 — 26 optional base-`Artifact` identifier ports across the catalog (`file.prepare.fileName`/`.fileType`/`.contentType`, `azureOcr.poll.modelId`, `azureOcr.extract.fileName`, …) each own an `in-<port>` canvas handle that `computePortRows` renders and a user can drag onto, while the Inputs panel hid them. A binding made by dragging was therefore invisible to the panel, the badge and the drawer, with nowhere to see or undo it short of the raw advanced-bindings editor. Hiding UNBOUND ones turned out to be deliberate, not an oversight — an existing test pins it, and without it `file.prepare` alone shows three always-empty rows. So the fix is narrower than the entry implies: hidden until it holds something. An unbound optional port stays out of the way; a bound one becomes visible and editable, which is the only state where hiding it destroyed information. An empty `ctxKey` does not count as bound — that stub can slip in on an edge delete. The entry's OTHER half stays closed: kindless input ports are excluded too, but zero of the catalog's activities declare one, so that branch has no population at all. G-081 — `computeNodeStatus` had no production caller. Every real surface goes through `autoWireIssuesToValidationErrors`, which applies a `manuallyBoundPorts` filter it did not, so the two disagreed about a ctx-bound port: one concept, two implementations, and the unreachable one was the wrong one. Deleted rather than reconciled — an unused second answer is a drift trap waiting for a caller. Its tests now assert the same thing through `computeNodeInputIssues`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the last three "broken in what exists" entries. G-046 is ruled as two separate things, which is what the verification pass argued for: the optional-Artifact half (26 ports) is fixed, the kindless half is closed with no work because zero catalog activities declare such a port. The fix is also narrower than the entry implied — hiding UNBOUND optional ports turned out to be deliberate and test-pinned, so only the bound case, where hiding destroyed information, changed. G-081 is closed by deletion rather than reconciliation. Two implementations of one concept, with the unreachable one being the wrong one, is a drift trap; the right move is to remove it, not to make it agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The v2 editor's "Edit script" action and the whole /dynamic-nodes page 400'd for any system-admin user: System-admin callers must include a `groupId` in the request body, query (`?groupId=...`), or `x-group-id` header. Root cause: 5c777d6 taught activity-catalog.controller and dynamic-nodes.controller to accept an explicit group hint (a system admin has no single group — getIdentityGroupIds returns undefined for that role), and wired the frontend half for useActivityCatalog only. The five wire helpers in dynamic-node-api.ts — list, detail, POST, PUT, DELETE — never learned to send it, so the backend fix never reached the surface its own commit message named as broken. All five now take an explicit `groupId: string | null` and send `x-group-id`; the four hooks read it from GroupContext. The two query keys gain a group suffix so switching groups refetches instead of serving another group's lineages from cache — the mutation hooks invalidate the bare keys, which still match by prefix. Also pins the other end of the contract: a backend spec asserting a system-admin caller WITH x-group-id resolves to that group (only the rejection case was covered). MANUAL_TEST_PLAN §1.3 now names all three endpoint families that need the hint, and records why our test setup hides this class of bug — the x-api-key path resolves to exactly one group, so neither an API-key caller nor the Playwright mock-auth bypass ever reaches the admin branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found walking MANUAL_TEST_PLAN 9.9b.
Replay already refused canvas and settings edits — `handleCanvasConfigChange`
returns early when `isReplay` — so the check's four stated criteria passed on
the first run. Undo/redo were the second door into the same room: they stepped
the EDITING config's history, which is hidden behind the historical graph while
replaying. Nothing on screen moves, so the author gets no feedback at all and
finds out only after leaving replay. Live: two Undo presses during replay
discarded two unsaved renames with the canvas looking identical throughout.
Refused at the `undo` / `redo` wrappers, the single choke point for all three
entry points (top-bar buttons, the Ctrl+Z / Ctrl+Shift+Z hotkeys, and the
canvas's own `onUndo`), and both buttons are disabled while replaying so the
mode is visible rather than silent. Two tests, both verified failing against
the pre-fix source.
9.9b's wording now names the Undo press, since the criteria as written did not
reach the defect the check exists to catch.
Also closes out the last three walkthrough checks:
7.8 Library port kinds PASS — authored a typed library; kinds annotate
the port editor, the picker preview and the
ChildWorkflow summary, and survive the round-trip
9.9b Replay safety PASS after the fix above
9.9c Unloadable version PASS — chip turns orange and says
"v3 unavailable, showing current graph"
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mock only fakes the frontend session; every backend call still goes out with x-api-key, which resolves to exactly one group. A real system admin resolves to none, so the endpoints that require an explicit groupId for admins take a branch the bypass never reaches — which is how the /dynamic-nodes breakage (dd6cdaf) stayed green under every browser check. Records the workaround: keep the API-key interception, but reject the admin case yourself so the admin server is reproduced without an admin JWT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s (14.8)
Found walking MANUAL_TEST_PLAN 14.8. The palette's "New custom node" modal
publishes, closes, shows a green "Published v1" toast — and left the canvas
unchanged. The node it promises to drop never appeared.
`addDynamicNode` looks the freshly published `dyn.<slug>` up in the merged
activity catalog and returns silently when it isn't there. Two independent
reasons it wasn't:
1. `useDynamicNodePublish.onSuccess` fired `invalidateQueries` without
returning the promise, so `mutateAsync` resolved — and the modal's
`onAfterPublish` ran — a whole network round-trip before the catalog held
the new entry; and
2. even once awaited, React has not re-rendered the page at that instant, so
both the closed-over array AND a ref updated during render are a commit
behind. The modal's callback is itself a closure captured before the
publish.
The TanStack cache is correct the moment the refetch settles, so the lookup now
reads `getQueriesData` first and falls back to what is rendered. The
invalidation is awaited as well, which is what makes the cache authoritative by
the time the callback runs.
Verified live: publishing from the palette modal takes the canvas from 4 nodes
to 5, with the grape DYN badge on the new one. The regression test models the
harder half — calling the stale callback after the catalog has moved on — and
was checked against the pre-fix source both ways.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
All three have seeded demos, so all three walked without building anything — which is what a gallery part should feel like. 12 of 14 checks pass outright. Two could not be finished, and both trace to ONE demo: `Demo — Library workflow (Part 10)` declares 0 inputs / 0 outputs and has only one version. So the signature summary a user is sent to look at renders empty (10.4), and 12.5's "pick v2 → stamps version:2" cannot be walked at all. Logged as demo gap D4 — fixing that single demo also closes D1. Adds COVERAGE_GAP_LOG.md, per the rule agreed with Alex: if walking a check required building a fixture, the check is not gallery-shaped and the gap gets logged rather than quietly papered over with a throwaway. Four demos to seed, two e2e specs to add (deliberately only two — a bug earns an e2e only where a unit test structurally cannot see the failure), two open product decisions, and a proposed plan split. On that split: if every row is either automated or in the gallery, the full matrix is not a document anyone walks — it is a coverage INDEX recording where each capability's assurance lives. Its own "unit/integration-backstopped" notes are already that index, written in the margins. Also records two non-findings, both my probe technique rather than the product: `force: true` manufactures clicks a user cannot make (the "disabled" Save-as-library item is genuinely unreachable by a real mouse), and Compare-to- head is disabled on the head row by design. The Part-12 demo's head was moved by walking 12.2 (Revert) and has been restored to its seeded v1 — revert is a pure head-pointer move, so nothing was lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Part 16 is a clean sweep — all 7 pass. The kind-aware extend popover (16.7) is the standout: hovering the typed `preparedData` output filters the catalog from 29 entries to exactly 2 — azureOcr.submit and mistralOcr.process, precisely the prepared-file consumers — with the "Show all" escape intact. Part 13: 13.4 and 13.5 pass outright, and 13.4's error copy is unusually good (the 400 lists the allowed MIME types, the 413 gives exact byte counts). Four checks are construction steps with no artifact to open, logged as D5 (no demo has BOTH source kinds, so the drawer's "both present" case is unobservable) and D6 (13.7 wants a second source added so the validator refuses it — better seeded already-broken). Refines the conversion worklist: the 29 construction-verb steps split THREE ways, not two. 16.2 says "build a map node with body nodes", but the Part-4 demo already has a map with a body and Part-6 already has two groups — so that step converts to "open this, toggle that" for free. Bucket 1 (already covered, reword only) may be the largest and costs nothing, so the 29 should be triaged before the split is estimated. Fourth self-caught false positive recorded: three passes at 16.2 clicked the More-menu row for Simplified view, which never flips the control — it is a visually-hidden Mantine input[role=switch] and data-checked stayed "false". The identical node counts that produced read exactly like "simplified view collapses nothing". It collapses fine (9 nodes + 1 map-body container → 4 nodes + 1 group chip, and back). Any first-pass failure is now treated as unverified until re-probed a second way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walking Part 4 led to running the workflow-builder e2e suite, which turned out to be the most valuable thing in this pass. first run: 14 failed, 51 passed after fix: 11 failed, 54 passed THREE OF THE FOURTEEN WERE MINE. f9049ab (G-063) gave UpdateWorkflowDto a whitelist so a save must state the version it was based on. The e2e helper had been updated to send expectedVersion but still sent groupId, which the whitelisted DTO now rejects with 400 "property groupId should not exist". A workflow's group is fixed at create time, so the helper was the wrong side. The other 11 are pre-existing and NOT triaged — 7 in tier2-control-flow (all the same selectNode timeout), 3 in tier2-port-wiring, 1 in tier2-validation. The product itself is fine: a genuine mouse click selects eachDoc/collect/ routeByType/pollOcr and opens each one's settings panel. This is test rot. Why it went unnoticed: .github/workflows/ has NO Playwright job. 76 specs exist and nothing runs them. That reframes the gap log — adding E1/E2 to a suite nobody runs buys very little. Wire the suite into CI first, then triage the 11, then add the new specs. My own regression sat there a day and was found only because I ran the suite by hand. Part 4 itself walks clean: all six control-flow types open their own settings panel, pollUntil carries the port rows G-016 gave it, and the condition editor renders a depth-3 AND/OR/NOT tree with a readable summary. Part 15 is not runnable here — no agent credentials in the environment (checked by presence only, never read). Its stubbed half passes via tier3-agent-stubbed. Ran with PLAYWRIGHT_SKIP_DB_RESET=1 throughout, so the dev DB was left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…07-25 walk The Parts 3-9 walkthrough recorded its verdicts in its own results doc and never ticked the plan, so 25 checks that were walked and confirmed working still read as untouched. Ticking them from the recorded evidence rather than re-walking. True remaining after reconciliation: 80 of 152. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st tables) My first reconciliation pass only parsed the walkthrough doc's TABLE rows. Parts 4 through 9 record their results as BULLET LISTS, so 17 checks that were walked and confirmed working in the 2026-07-25 pass were still showing as untouched — and I had told Alex Part 4 was never walked, which was wrong for the same reason. 51 check ids are evidenced in that doc, across all seven parts 3-9 including Part 4 (4.1, 4.2, 4.3, 4.5, 4.6, 4.7, 4.13, 4.15). Plan now reads 89 walked / 152 (58%), open 63. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
11 more checks walked directly (3.4, 3.5, 3.8, 3.9, 3.11, 6.7, 7.3, 7.9, 8.4, 8.8) and 6 resolved against passing e2e specs (6.2, 6.4, 13.2, 13.3, 13.6, 13.7). Plan now reads 105/152. Undo/redo is honest end to end: disabled on an empty graph, one node per click in both directions, and the redo branch drops the moment a new edit lands. 6.2/6.4 are not hand-walkable — neither Ctrl-click nor Shift box-drag produced a multi-selection under automation — but the gate is honest (menu item disabled with title 'Select 2+ nodes to group them') and tier2-node-swap-grouping covers the behaviour and passes. Method note recorded: matching open checks against e2e test titles by id alone over-claims. 6.2/6.4 matched '§6.2' in a tier2-port-wiring title, which is a section reference, not a check id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The agent chat drawer was completely non-functional in the browser. Every
message returned:
400 {"message":["property id should not exist",
"property trigger should not exist"]}
The AI SDK's `DefaultChatTransport` sends `id` and `trigger` alongside our
payload, `AgentChatRequestDto` never declared them, and the global pipe runs
with `forbidNonWhitelisted: true`. So the request died at the controller
boundary before reaching the agent at all — the composer accepted the message,
the thread showed "Agent", and nothing ever streamed.
Both fields are now declared optional and documented as accepted-and-ignored.
Nothing reads them; they exist so the whitelist stops rejecting the SDK's own
envelope.
Found walking Part 15. It was invisible from every other angle: the backend and
Azure are healthy (a hand-built request streams GPT-5.4 fine), the agent unit
tests pass because class-validator alone cannot see whitelisting — that is a
ValidationPipe concern — and no e2e runs the live chat. The new spec block
therefore goes through a real ValidationPipe, with a control asserting a
genuinely unknown property is still refused.
Verified live afterwards: 15.3 completes in 21s — streaming text, tool-call
cards (listActivityCatalog / listSourceCatalog / listLibraryWorkflows /
createWorkflow, all COMPLETE), the canvas re-rendering with the auto-seeded
source.upload node, and the app navigating to the new workflow mid-stream.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7 of 12 pass. The headline is D-15 (fixed in 672868d): the agent chat drawer was returning 400 on every single message, so the agent was completely unusable in the browser. Also corrects my earlier claim that Part 15 was not runnable for want of credentials — they were configured all along; I had probed my own shell instead of the backend's environment. 15.12 is recorded as INCONCLUSIVE rather than failed: only 1 of my 2 connect drags landed an edge, so the chain under test was never built. Consistent with the rule this walkthrough adopted after four self-caught false positives — a first-pass failure is unverified until re-probed a second way. 15.9 needs AGENT_MAX_CONVERSATION_TOKENS=1000 plus a backend restart, which is not something to do to a running stack unasked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9.1, 9.2 and 9.11 pass. 9.12 FAILS, and the behaviour turns out to be unimplemented rather than broken: RunWorkflowDrawer.handleTry calls startRun directly, with no dirty check, no save and no lazy deploy. Measured with the leave-guard as the dirty oracle: the rename lands on the canvas, the editor reports dirty, Try runs — and no version is created, with the server still holding the old label. The author watches badges light up on their canvas while Temporal ran the PREVIOUSLY SAVED graph, with nothing saying so. Same family as G-004 and D-12 (the canvas asserting one thing while the system does another), and arguably the worst of the three because it silently produces RESULTS for a graph the author is not looking at. Two defensible fixes — save first, or refuse while dirty — and the choice changes what version history means, so it needs Alex rather than a unilateral patch. 9.7 is recorded INCONCLUSIVE, not failed: my fixture's dyn node completes in ~85ms, leaving no window for a second Try to cancel the first. Its earlier 'failed' runs were also mine — the repeat Trys reused the drawer without re-filling the JSON, so they ran with empty ctx and the node correctly failed reading ctx.document.url. Also notes a copy nit: the version run-count badge reads '1 RUNS'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9.7 fails, and the diagnosis is precise. Against a purpose-built 15s dynamic node, a second Try leaves the first running: two concurrent 'running' runs, neither cancelled, both completing. The backend is correct and complete. startRun calls cancelInFlightTriesForLineage before starting, and G-021 deliberately narrows it so only runs stamped RunTrigger='try' are cancelled — production runs must complete regardless, or feeding 240 documents through would have document #2 cancel document #1. The frontend never stamps one. The drawer's Try and the Run tab both call useStartWorkflowRun -> POST /workflows/:id/runs, and that endpoint hard-codes the new run as 'api'. So the 'try' category has no producer at all, and the cancel set is always empty. G-021's narrowing is inert because the thing it protects is never created. Every editor Try therefore behaves as a production run: two Trys race, canvas badges follow whichever statuses land last, and on an OCR or LLM graph the abandoned run keeps spending. Two fix shapes, both with tradeoffs (a public-API 'trigger' field that callers could assert, vs a dedicated /tries route), so this needs Alex — and should likely be decided together with D-16, the other Try-path gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5.2 and 5.4 walked directly; 5.5 verified against the API; 5.7, 7.5 and 7.6 confirmed against their unit backstops (graph-workflow validator 161 tests, frontend picker/validation 92 tests, all green). 5.2 is a clean pass: the error-policy control offers exactly the three documented options, and choosing 'Follow the error path' adds a handle to the node (6 -> 7) and reveals the fallback-edge and retryable controls. Plan nit recorded on 5.5: the activity is registered as document.validateFields, but the doc's own curl uses the short name 'validateFields' and therefore 400s on 'not registered' before the rule-shape check it is trying to demonstrate ever runs. Same class of defect as D-9. 7.2 left open — the port tooltips render into a portal only while hovering and my locators never caught one. Content check, low risk. Walkthrough closes at 121/152 (79%), from 3 at the start. Five defects fixed, six left for a ruling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…reakages Alex approved seeding demos where appropriate, so D1/D4 land as a demo rather than as more throwaway fixtures. The Part-10 library demo now declares a real signature — input "Prepared file" (PreparedFile), output "OCR result" (OcrResult) — and carries two versions. One demo unblocked four checks: 7.8's kind annotations have something to annotate, 10.4's childWorkflow summary reads "V2 | 1 INPUT | 1 OUTPUT" instead of "0 INPUTS | 0 OUTPUTS", and 12.4/12.5 can finally pin a v2 (select offers head/v2/v1; picking v2 stamps the V2 badge). Re-seeding surfaced TWO breakages, both from my own earlier work this session: 1. `createDemo`'s second-version PUT still sent `groupId` and no `expectedVersion` — the same G-063 regression I fixed in the e2e helper (76bb745), third instance. Fixed the same way: a fresh lineage is always at v1. 2. The control-flow demo put its humanGate INSIDE the map body, which my own G-070 rule now (correctly) refuses — a gate in a loop cannot work, since every item registers the same signal name and an approval cannot say which item it is for. The seeder had already deleted 16 demos before hitting it, leaving the local set half-populated. The demo now models the shape that actually runs: the switch's default branch falls through to the body's exit node, and the gate sits after the join — approve the collected batch, then store. All 16 demos re-seed cleanly. Worth noting for the record: this is the second time a validator rule I added has invalidated existing seeded content. Adding a rule needs a pass over the seeder and the templates, not just the shipped workflows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
D2 — a new "Deleted custom node" demo that ships ALREADY tombstoned: the seeder
publishes a lineage, builds a workflow on it, then soft-deletes the lineage.
The reader opens it and sees the red Deleted badge, the "(deleted dynamic node)"
alert and the "Activity type not registered" error, with no destructive step and
nothing consumed for whoever walks next. Its guide steps name the D-11 gap
(Try/Run stay enabled) and the catalog-load timing that made me misread this
check the first time.
D3 — the Part-14 dynamic node is now tagged `@deterministic true` AND binds its
output to a ctx key. Both were needed: without the tag the script is treated as
non-deterministic and deliberately never cached, so there is nothing to preview;
without the binding the widget honestly reports "this step's output isn't bound
to a workflow value yet". With both, 14.9 finally shows what it promises —
{"fileName":"901E42EB-7094-4B67-B7FA-FD62618D2208-WALK-14-9.PDF"}.
That also sharpens D-12. The bogus "cache evicted — re-run to repopulate"
message appears specifically for a node that is never cached, so the fix is to
say "not cached: this node is non-deterministic" rather than to blame an
eviction that never happened.
14.9 ticked. 14.8's observable halves all pass on the new demo; it stays open
only on D-11's "Try disabled" criterion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…view copy Three findings from the walkthrough, all the same shape — the canvas asserting one thing while the run does another. D-11 (14.8) — a graph with validation errors was still runnable. A deleted `dyn.*` lineage is diagnosed at author time (red Deleted badge, "not registered") and the run then dies at dynamicNode.resolveLineage. Try and Run now refuse while errorCount > 0. Warnings are advisory and deliberately do NOT block. D-16 (9.12) — with unsaved edits, Try ran the PREVIOUSLY SAVED graph and said nothing. Measured live: the rename landed on the canvas, the editor reported dirty, and after Try no version existed and the server still held the old label. Try and Run now refuse while dirty. Chose this over auto-saving because auto-save mints a version on every dirty Try, which changes what version history means. Both funnel through one `runBlockedReason`, which doubles as the tooltip — a button can never be disabled here without saying why. The existing "Save the workflow first" string is reused verbatim so its assertions elsewhere keep passing. D-12 (14.9) — a succeeded node with no cache row always reported `evicted` and offered "Re-run to repopulate". For a `@deterministic:false` dynamic node that is untrue twice: nothing expired (it is never cached, §3.3) and re-running can never repopulate it. New `not-cached` reason with its own copy, no Re-run button, naming the actual cause and the fix. No backend work was needed — the catalog already ships `nonCacheable`; it just was not being read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Caught by verifying D-16 live rather than trusting the unit tests: on a freshly opened demo, Try and Run were disabled before the author touched anything. Every demo ships `metadata.arrangeOnLoad`, and the auto-arrange rewrites the config ~1.5s after mount. It correctly goes through `resetConfig` so it is not an undo step — but it still leaves `config !== lastHydratedConfigRef.current`, so the editor considered itself dirty from the moment it opened. That was already wrong before this batch: the G-027 leave-guard would warn on navigating away from a workflow nobody had edited. D-16 only made it visible, by turning "dirty" into a reason to refuse a run. `handleArrangeOnLoad` now re-bases the baseline alongside the reset, so `isDirty` keeps meaning "the AUTHOR changed something" — which is what both consumers want. Verified live: a fresh demo has Try/Run enabled; renaming a node disables them with "Save your changes first — a run always executes the saved graph, not the canvas"; and the deleted-lineage demo reports "Fix 1 validation error first — this graph cannot run as it stands". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y works (D-17) The editor's Try posted `/workflows/:id/runs`, which stamps `RunTrigger = "api"`. `cancelInFlightTriesForLineage` filters on `RunTrigger = "try"` — so the cancel ran on every start and always swept an empty set. Two Trys on a slow graph both ran to completion, the canvas badges fed by whichever statuses landed last, and on an OCR/LLM graph the abandoned run kept spending. The `"try"` side of G-021's distinction simply had no producer. `POST /workflows/:id/tries` shares one implementation with `/runs` and differs only in the trigger it stamps. The trigger is assigned by the ROUTE, not by a request field: under the alternative (a public `trigger` on StartRunRequestDto) an API caller could opt their own production runs into the set that editor activity cancels — which is exactly what G-021 narrowed the cancel to prevent (feeding 240 documents through must not have document #2 cancel document #1). Both editor preview paths move over: the Try tab, and the cache-evicted Re-run (repopulating a preview row is canvas iteration, not production). Also closes an audit gap in the code being restructured: neither run-start path recorded a `workflow_run_started` event, though the upload-and-Try path did. The shared helper now records one, carrying the trigger. Verified live against the 15-second `walk-slow-node` fixture — two Trys 1.8s apart, first `cancelled`, second `succeeded`. Previously both succeeded. Manual-test-plan 9.7 now passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the long-running visual workflow builder line of work up to date with
developand opens it for review. This is the full builder stack: the xyflow v2 canvas editor, control-flow nodes, typed I/O, library workflows + workflow-as-API, try-in-place previews, Windmill-style dynamic nodes (Deno sandbox), document-source nodes, and the Phase 7 AI workflow-builder agent (chat drawer, multi-provider, canvas live-update).Merge base with
develophad drifted ~197 commits;developwas merged in and 26 conflicts resolved.Notable merge resolutions
index.browser);config-hash(usesnode:crypto) moved behind a subpath so it never enters the webpack-bundled Temporal workflow. Node consumers import@ai-di/graph-workflow/dist/config-hash.configHash/workflowConfigOverrideswith our Phase 4 cache (workflowLineageId/cacheDeps) and Phase 6dyn.run(workflowRunId) plumbing. Adopted develop's by-reference childWorkflow + workflow-entry architecture and layered US-080 version pinning on top.uimodule,Header,ErrorBoundary, light theme); grafted in the agent-chat drawer/icon, dynamic-nodes routes, andModalsProvider; folded the library kind-filter + templates picker into develop'sPageHeader/DataTablelist page. Legacy JSON editor stays removed (v2 canvas is the sole workflow editor).@docsalias on develop's base; lockfile regenerated.Verification
useDocuments, and the global notifications mock.🤖 Generated with Claude Code
Update 2026-07-15 — port-wiring redesign complete
The workflow-builder port-wiring redesign (PORT_WIRING_DESIGN.md §15) is now feature-complete on this branch — all five phases:
Presentation/wiring layer only — no
GraphWorkflowConfig, engine, or resolver schema change.Verification (re-run 2026-07-15): frontend vitest 1639 green,
tscclean, and the@infratry-in-place e2e (extended to exercise the wire peek) green against the local Temporal worker stack. (The suite counts in the integration block above are from the original merge snapshot and predate this work.)