fix(e2e): stop sending groupId on update; record the suite's rot (D-14)

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>

Author: alex-struk

docs-md/workflows/MANUAL_TEST_PLAN.md

@@ -269,39 +269,39 @@ Each item is one control-flow node's settings form. **4.1–4.7** use the **firs
 
 > **About the first demo workflow.** It is a **forms showcase, not a runnable pipeline** — one graph that packs all six control-flow node types so you can open each form in isolation. It maps over a `documents` array (**Run for each document**), and for each document a switch (**Branch by condition**) routes by type down one of three demo branches: *invoice* → a **Sub-workflow**, *receipt* → **Wait until condition** (poll) → **Extract OCR result**, and the *default* → **Wait for approval** (a human gate). A **join** (**Collect results**) would gather the per-document results into `results`, then **Store Results** persists them. **Two of the three branches (Sub-workflow, Wait for approval) deliberately dead-end** — they never reach the map body's exit node (**Extract OCR result**), so only the receipt branch completes a real chain. That is intentional here (the point is to inspect every form), but it would **not** be a valid production workflow — a real map body wants every branch to converge on the exit node. `documents`/`currentDoc` are **untyped trigger data** (no schema), so conditions on them (e.g. `currentDoc.type`) are typed by hand, not picked from a field list.
 
-- [ ] **4.1 Switch — route each item down one edge.** *A switch tests its cases top-to-bottom; the first match sends the item down that case's **edge**, else the default edge. Key model: a case does **not** point at a target node directly — it points at a **conditional edge** that runs from this switch to a target. So a case is "condition → which outgoing edge to take."*
+- [x] **4.1 Switch — route each item down one edge.** *A switch tests its cases top-to-bottom; the first match sends the item down that case's **edge**, else the default edge. Key model: a case does **not** point at a target node directly — it points at a **conditional edge** that runs from this switch to a target. So a case is "condition → which outgoing edge to take."*
   1. Click **Branch by condition** (the yellow diamond) → its settings open on the right. It already has two cases plus a default, each bound to one conditional edge leaving the diamond (to **Sub-workflow**, to **Wait until condition**, and — as default — to **Wait for approval**).
   2. Click **Add Case** → a new empty case row appears. Click its **condition** → build a test (e.g. `currentDoc.type` **equals** `"invoice"`). *(`currentDoc` is an untyped loop item, so type the `.type` path by hand — see the untyped-data note above.)*
   3. Open the case's **Edge** dropdown. **What this dropdown is:** the outgoing edge this case fires. It lists **every conditional edge that starts at *this* switch** — normal edges, error edges, and edges leaving *other* switches are excluded, because a switch can only route down its own conditional branches. In this demo that's the three existing edges (Sub-workflow / Wait until condition / Wait for approval). **To route a new case somewhere *new*, first draw a new conditional edge** on the canvas from the switch's output handle to the target node; it then appears in this dropdown alongside the others. *(Note: the dropdown does not currently grey out edges already used by another case, so it's on you not to point two cases at the same edge.)*
   4. Optionally set a **Default edge** (taken when no case matches).
   **Pass:** the Edge dropdown lists this switch's conditional edges (and only those); **Save + reload** keeps every case and its edge.
-- [ ] **4.2 Map — run a body once per item.** *A map iterates a collection and runs its "body" sub-graph for each element; the body is delimited by an entry and exit node (the dashed green box on the canvas).*
+- [x] **4.2 Map — run a body once per item.** *A map iterates a collection and runs its "body" sub-graph for each element; the body is delimited by an entry and exit node (the dashed green box on the canvas).*
   1. Click **Run for each document** → its settings open.
   2. Set **collection ctx key** (`documents`), **item ctx key** (`currentDoc`), optional **index ctx key**, and **max concurrency** (≥ 1).
   3. Set **body entry node** and **body exit node** via their pickers (each excludes the map node itself).
   **Pass:** all fields are editable; the green **body box** wraps the nodes between the entry and exit you chose.
-- [ ] **4.3 Join — collect a map's results.** *A join gathers the per-item outputs a map produced back into one array.*
+- [x] **4.3 Join — collect a map's results.** *A join gathers the per-item outputs a map produced back into one array.*
   1. Click **Collect results** → its settings open.
   2. Open the **Source Map node** picker → it lists **only map nodes** (not activities/switches).
   3. Set the **Results ctx key** (where the collected array is written).
   **Pass:** only map nodes are selectable as the source. *(Join strategy is fixed to `all` and isn't shown in the form.)*
-- [ ] **4.4 Sub-workflow (childWorkflow) — call another graph.** *Runs a whole other workflow — either a saved Library workflow or an inline graph shipped with this node.*
+- [x] **4.4 Sub-workflow (childWorkflow) — call another graph.** *Runs a whole other workflow — either a saved Library workflow or an inline graph shipped with this node.*
   1. Click **Sub-workflow (inline OCR)** → its settings open.
   2. Toggle **Library / Inline**. In **Inline** mode an **editable** JSON textarea of the embedded child graph shows, with a live problems list beneath it (see 5.7).
   3. Edit the **input** and **output mapping** lists (which parent ctx keys feed the child, and where its outputs land).
   **Pass:** the Library/Inline toggle switches modes; inline shows the JSON editor and, under it, either *"No problems in the inline graph."* or a red/amber panel listing them. *(Library-picker modal is a manual spot-check; inline round-trip is `tier2-control-flow` e2e.)*
-- [ ] **4.5 Wait until condition (pollUntil) — poll until true.** *Re-runs an activity on an interval until a condition holds (or it times out).*
+- [x] **4.5 Wait until condition (pollUntil) — poll until true.** *Re-runs an activity on an interval until a condition holds (or it times out).*
   1. Click **Wait until condition** → its settings open.
   2. Pick an **activity type** → its own parameters sub-form renders below.
   3. Set the **condition**, an **interval** (e.g. `30s`), **max attempts**, and optional **initial delay** / **timeout**.
   4. Type an invalid duration (e.g. `30` with no unit) in a duration field.
   **Pass:** the invalid duration shows an inline error; the picked activity's nested params render. *(Interval / invalid-duration / maxAttempts round-trips are `tier2-control-flow` e2e; the activity sub-form is a manual spot-check.)*
-- [ ] **4.6 Wait for approval (humanGate) — pause for a human.** *Suspends the run until a named signal arrives, with a timeout fallback.*
+- [x] **4.6 Wait for approval (humanGate) — pause for a human.** *Suspends the run until a named signal arrives, with a timeout fallback.*
   1. Click **Wait for approval** → its settings open.
   2. Note the **signal name**, the read-only **payload schema**, and the required **timeout**.
   3. Set **On timeout** to `fail`, then `continue`, then `fallback`.
   **Pass:** the **fallback edge** picker appears **only** when On timeout = `fallback` (gone for `fail`/`continue`).
-- [ ] **4.7 Condition editor — nested expressions.** *Builds boolean trees (AND/OR/NOT + comparisons) to any depth; each value is a **Ref** (a variable) or a **Literal**.*
+- [x] **4.7 Condition editor — nested expressions.** *Builds boolean trees (AND/OR/NOT + comparisons) to any depth; each value is a **Ref** (a variable) or a **Literal**.*
   1. On **Branch by condition**, open the **second case**'s condition — this demo ships a 3-level tree `AND( OR(EQ, GTE), NOT(IS-NULL) )`.
   2. Confirm the nesting renders indented per level.
   3. Pick a leaf value and toggle it **Ref** ↔ **Literal** (Ref = variable autocomplete; Literal = plain input).

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

@@ -50,6 +50,17 @@ cannot* see the failure.
 | **E1** | `tier1-dynamic-node` — **system-admin identity** variant | the `/dynamic-nodes` breakage (`dd6cdafb`) | The one that actually bit us. Unit tests mock `fetch` and never assert what reaches the wire; the `x-api-key` path resolves to exactly one group, so **no** existing layer reaches the admin branch. Needs the request-level admin simulation recorded in the `app-browser-auth` skill | S |
 | **E2** | `tier1-dynamic-node` — publish from the palette modal drops the node | 14.8 auto-drop (`64d86d73`) | The decisive argument: my unit test passes, but the live bug had a **second** cause (React render timing) the unit test did not model. I only found it in a browser | S |
 
+### ⚠️ G2 is now second priority — see D-14
+
+`.github/workflows/` contains **no Playwright job**. 76 e2e specs exist and
+nothing runs them, so **11 specs are failing today** and one of the failures was
+a regression I introduced a day earlier (`f9049ab3` whitelisted
+`UpdateWorkflowDto`; the e2e helper still sent `groupId`). It was caught only
+because I ran the suite by hand during this walk.
+
+Adding E1/E2 to a suite nobody runs buys very little. **Wire the existing suite
+into CI first**, then triage the 11 pre-existing failures, then add E1/E2.
+
 **Explicitly not adding e2e for:** undo-during-replay (`fd3194bb`) and the 9.9c
 replay chip. Both are fully pinned by falsifiable unit tests; an e2e would be
 duplicate cost.

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

@@ -250,3 +250,65 @@ Running tally of first-pass "failures" that were my technique, not the product:
 D-11 timing, `force: true` on a disabled item, Compare-to-head on the head row,
 and this. **Any first-pass failure is now unverified until re-probed a second
 way.**
+
+---
+
+# Part 4, the e2e suite, and Part 15
+
+## Part 4 — control-flow settings forms
+
+Walked manually on the Part-4 demo (which carries all six control-flow node
+types plus a map body).
+
+| # | Verdict | Evidence |
+|---|---|---|
+| 4.1–4.6 forms | ✅ PASS | every type opens its own panel on a genuine click: `switch-node-settings`, `map-node-settings`, `join-node-settings` (`source-map-node-id`, `results-ctx-key`), `child-workflow-node-settings` (`ref-type`, `inline-editor`, `inline-problems-none`, `input-row-0`), `poll-until-node-settings` (+ `poll-until-wrapped-pollOcr` and full port rows — G-016's "wrappers inherit affordances"), `human-gate-node-settings` (`signal-name`, `payload-schema-editor`, `timeout`) |
+| 4.7 Nested conditions | ✅ PASS | depth-3 tree — `case-1-condition-operand-0-editor-operand-0-editor-left-ref-input` — with the readable summary *"ALL OF THESE MUST BE TRUE — (ctx.currentDoc.type is "receipt" or ctx.currentDoc.confidence ≥ 0.8) and not (…)"* |
+| 4.8–4.15 | ◑ e2e-backstopped | `tier2-condition-step-ref` covers the step-picker/persistence half; 4.12 and 4.15 need a run |
+
+## D-14 — the e2e suite has rotted, and nothing runs it (open)
+
+Running the workflow-builder suite (`PLAYWRIGHT_SKIP_DB_RESET=1`, so Alex's dev
+DB was left alone):
+
+```
+first run:  14 failed, 51 passed
+after fix:  11 failed, 54 passed
+```
+
+**Three of the fourteen were mine, and I fixed them.** `f9049ab3` (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:
+
+```
+update workflow failed: 400 {"message":["property groupId should not exist"]}
+```
+
+A workflow's group is fixed at create time, so the helper was the wrong side —
+`groupId` removed from the PUT payload.
+
+**The other 11 are pre-existing and un-triaged**: 7 in `tier2-control-flow`
+(all the same `selectNode` timeout), 3 in `tier2-port-wiring`, 1 in
+`tier2-validation`. I did **not** establish their cause. What I did establish is
+that **the product works**: a genuine `page.mouse.click` selects `eachDoc`,
+`collect`, `routeByType` and `pollOcr` and opens `map-node-settings`,
+`join-node-settings`, `switch-node-settings` and `poll-until-node-settings`
+respectively. So this is test rot, not broken selection.
+
+**The reason it went unnoticed: `.github/workflows/` has no Playwright job at
+all.** 76 e2e specs exist and nothing runs them. That reframes gap **G2** — E1
+and E2 are worth much less than wiring the existing suite into CI, because a
+spec nobody runs is a spec that silently rots. My own regression sat there for a
+day and was caught only because I happened to run the suite by hand.
+
+Recommended order: **wire e2e into CI first**, then triage the 11, then add
+E1/E2.
+
+## Part 15 — AI agent ☁️🔑
+
+⏭ **NOT RUNNABLE HERE.** No agent credentials in the environment
+(`ANTHROPIC_API_KEY`, `AZURE_OPENAI_*` all unset — checked by presence only,
+never read). The stubbed half is covered by `tier3-agent-stubbed`, which passes
+in the suite run above. The live half (15.x against a real model) is manual-only
+and needs Alex.

tests/e2e/workflow-builder/helpers/workflow-api.ts

@@ -133,10 +133,12 @@ export async function updateWorkflow(
     opts.expectedVersion ?? (await getWorkflow(request, id)).version;
   const res = await request.put(`${BACKEND_URL}/api/workflows/${id}`, {
     headers,
+    // No `groupId`: a workflow's group is fixed at create time, and
+    // `UpdateWorkflowDto` is whitelisted, so sending it is a 400
+    // ("property groupId should not exist") rather than a no-op.
     data: {
       name: opts.name,
       config: opts.config,
-      groupId: SEED_GROUP_ID,
       expectedVersion,
     },
   });