Parcourir la source

docs: analysis of the editor's modes and toolbar behaviour

Eleven distinct modes and six inconsistencies, with file and line for each.
Written before any of the toolbar work so the changes had something to be
measured against, and kept because the mode matrix is the thing that was
missing when these controls drifted apart in the first place.
fszontagh il y a 1 mois
Parent
commit
75a07b92a1
1 fichiers modifiés avec 235 ajouts et 0 suppressions
  1. 235 0
      docs/superpowers/analysis/2026-08-10-editor-toolbar-and-views.md

+ 235 - 0
docs/superpowers/analysis/2026-08-10-editor-toolbar-and-views.md

@@ -0,0 +1,235 @@
+# Workflow editor: views and toolbar analysis
+
+Scope: `webui/src/pages/WorkflowEditorPage.tsx`, `webui/src/components/workflow/EditorHeader.tsx`,
+`ExecutionViewerBanner.tsx`, `ExecutionListPanel.tsx`, `ExecutionResultsPanel.tsx`,
+`VersionHistoryPanel.tsx`, `PinnedDataBadge.tsx`, `webui/src/stores/executionPinStore.ts`,
+`webui/src/hooks/useAutosave.ts`. No code was changed for this analysis.
+
+## 1. Inventory of modes
+
+The editor mounts at route `workflows/:id` only (`webui/src/App.tsx:47`) - a workflow is created via
+API before the editor ever loads, so there is no "brand new, no id yet" mode inside this page. There
+is also no permission/role check anywhere in `WorkflowEditorPage.tsx` (checked for `readOnly`,
+`canEdit`, role, 403 - none found), so read-only-by-permission is not a mode this page implements
+either; access must be gated one level up, before this component mounts.
+
+What *is* implemented, gated by real state:
+
+| Mode | Entered by | Left by | What visibly changes |
+|---|---|---|---|
+| **Normal editing** | Default state once workflow loads | - | Canvas editable, autosave active, all toolbar controls live |
+| **Viewing a past execution** (`isViewingExecution`, `executionPinStore.ts:10`) | Clicking "View" on an execution row (`ExecutionListPanel.tsx:346` -> `handleViewExecution`, `WorkflowEditorPage.tsx:553-559`), or "Pin This Data" in the banner while pinned-but-not-viewing | "Return to Editor" (banner, `ExecutionViewerBanner.tsx:59-65` -> `handleReturnToEditor`, `WorkflowEditorPage.tsx:570-572`), or closing the results panel (`WorkflowEditorPage.tsx:3297-3305`, which also unpins) | Canvas becomes non-interactive: `snapToGrid`, `nodesDraggable`, `nodesConnectable`, `elementsSelectable` all false, connect/context-menu handlers all `undefined` (`WorkflowEditorPage.tsx:3205-3254`); canvas shows the execution's own node/edge snapshot (`viewedNodes`/`viewedEdges`, built at `WorkflowEditorPage.tsx:1428-1490`) rather than live `nodes`/`edges`; a full-width blue banner appears (`ExecutionViewerBanner.tsx`); undo history is disabled (`useGraphHistory(..., !isViewingExecution)`, `WorkflowEditorPage.tsx:528`); autosave is disabled (`WorkflowEditorPage.tsx:912`) |
+| **Pinned execution, not viewing** (`pinnedExecution` set, `isViewingExecution` false) | Clicking "Pin Data" on an execution row (`ExecutionListPanel.tsx:357-369` -> `handlePinExecution`, `WorkflowEditorPage.tsx:562-567`), or leaving Viewing mode while a pin remains | "Clear" button in the header (`EditorHeader.tsx:308-317` -> `onUnpinExecution`), or closing the results panel | Canvas is live/editable again, but node outputs shown are the pinned execution's outputs wherever a node is unchanged since that execution (`effectiveExecutionState`, `WorkflowEditorPage.tsx:427-511`); pinned-data badges appear on nodes (`PinnedDataBadge.tsx`); header "Executions" button carries a green "Pinned" tag (`EditorHeader.tsx:290-307`) |
+| **Live execution running** (`isRunning` = `executionState.status === 'running'`) | Clicking Execute / Execute-trigger, or testing a single node (`executingNodeId` set) | Execution finishes | Header shows a spinning "Running.../Testing node..." pill (`EditorHeader.tsx:185-190`); Execute button disabled (`EditorHeader.tsx:370,404`); edges animate (`WorkflowEditorPage.tsx:1623-1628`) |
+| **Active vs inactive workflow** (`isActive` = `!!workflow?.active`) | Clicking Activate/Deactivate (`toggleActiveMutation`, `WorkflowEditorPage.tsx:934+`) | Same button, opposite direction | Only the Activate/Deactivate button's color and label change (`EditorHeader.tsx:335-355`); nothing else in the toolbar reacts to it |
+| **Unsaved changes** (`hasChanges`) | Any edit: node/edge change, connect, rename, settings change, undo/redo, restoring a version | A successful save (autosave or manual) | Amber "Unsaved changes" pill (`EditorHeader.tsx:180-184`); Save button enabled; Publish button disabled (`EditorHeader.tsx:262`) |
+| **Save in flight** (`isSaving` = `saveMutation.isPending`) | Autosave firing, or clicking Save, or Ctrl+S | Save resolves | Save button reads "Saving..." and stays disabled (`EditorHeader.tsx:358-363`); autosave is also gated off while a save is pending (`WorkflowEditorPage.tsx:912`) |
+| **Publishing in flight** (`isPublishing`) | Clicking Publish | Publish resolves | Publish button reads "Publishing..." |
+| **Unpublished changes** (`hasUnpublishedChanges`) | Server-computed diff between stored and published graph (`WorkflowEditorPage.tsx:623-625`) after any save/publish | Publishing | Publish button switches from grey "Published" to green "Publish" (`EditorHeader.tsx:272-280`) |
+| **Duplicating in flight** (`isDuplicating`) | Clicking Duplicate | Duplicate resolves and navigates away | Button shows a spinner in place of the copy icon (`EditorHeader.tsx:324`) |
+| **Multiple triggers present** (`hasMultipleTriggers`) | Structural - workflow has >1 trigger node | - | Execute becomes a split button with a dropdown instead of a single action (`EditorHeader.tsx:366-410`) |
+| **Editing the name inline** (`isEditingName`, local `EditorHeader` state) | Clicking the workflow name (only when `onRename` is set and not viewing an execution, `EditorHeader.tsx:122-126`) | Enter/blur (save) or Escape (cancel) | Name becomes a text input |
+| **Save conflict / stale copy** (`saveConflict`, `savedByOther`) | Server rejects a save with 409, or presence detects another tab/user saved | Dismissed via the merge banner (`WorkflowEditorPage.tsx:2975-3070`) | A banner appears above the canvas; suppressed entirely while `isViewingExecution` (`WorkflowEditorPage.tsx:2975`) |
+
+Total distinct modes/flags found and confirmed by reading the gating code: **11** (Normal editing,
+Viewing execution, Pinned-not-viewing, Running, Active/inactive, Unsaved changes, Saving, Publishing,
+Duplicating, Multiple-triggers execute variant, Save-conflict). Inline name-editing is a minor UI
+sub-state layered on top rather than an editor mode in its own right.
+
+## 2. Toolbar control x mode matrix
+
+Read directly from `EditorHeader.tsx` JSX conditions (props are computed in `WorkflowEditorPage.tsx`,
+cited where the gating actually originates).
+
+| Control | Normal | Viewing execution | Pinned (not viewing) | Running | Notes / citation |
+|---|---|---|---|---|---|
+| Back | visible, enabled | visible, enabled | visible, enabled | visible, enabled | Never gated (`EditorHeader.tsx:152-157`) |
+| Workflow name (editable) | click-to-edit | **not editable** - click handler and pencil icon suppressed | click-to-edit | click-to-edit | `EditorHeader.tsx:122-126,171-177` |
+| Save | enabled iff `hasChanges` | enabled iff `hasChanges` (view mode does not block it directly) | enabled iff `hasChanges` | enabled iff `hasChanges` | `EditorHeader.tsx:358` - only checks `hasChanges`/`isSaving`, not `isViewingExecution`. In practice `hasChanges` is normally false while viewing since edits are disabled, but nothing in the button itself prevents a stray Save from firing if `hasChanges` were ever true in that mode (see 3.5) |
+| Publish | enabled iff has unpublished changes and no pending edits | **disabled** | enabled iff unpublished changes | enabled iff unpublished changes | `EditorHeader.tsx:262` explicitly includes `isViewingExecution` in `disabled` |
+| Execute (single) | enabled | enabled (not gated) | enabled | **disabled** | `EditorHeader.tsx:404` only checks `isRunning`; not gated by `isViewingExecution` |
+| Execute-trigger dropdown | enabled | enabled (not gated) | enabled | **disabled** | `EditorHeader.tsx:370` same as above |
+| Activate/Deactivate | enabled | enabled (not gated) | enabled | enabled | `EditorHeader.tsx:337` only checks `isTogglingActive` |
+| Add Node | enabled | **disabled** | enabled | enabled | `EditorHeader.tsx:196` |
+| Undo | enabled iff `canUndo` | **disabled** | enabled iff `canUndo` | enabled iff `canUndo` | `EditorHeader.tsx:236`; history itself is turned off while viewing (`WorkflowEditorPage.tsx:528`) |
+| Redo | enabled iff `canRedo` | **disabled** | enabled iff `canRedo` | enabled iff `canRedo` | `EditorHeader.tsx:244` |
+| Version history | visible | **disabled** | visible | visible | `EditorHeader.tsx:256` |
+| Auto-arrange | visible | **disabled** | visible | visible | `EditorHeader.tsx:285` |
+| Execution history ("Executions") | visible, shows "Pinned" tag if pinned | visible | visible, shows "Pinned" tag | visible | Never gated (`EditorHeader.tsx:290-307`) |
+| Clear pinned data | **hidden** (nothing pinned) | **hidden** even if `pinnedExecution` is set | **visible** | hidden unless pinned | `EditorHeader.tsx:308`: `pinnedExecution && !isViewingExecution` - see 3.1 |
+| Duplicate | enabled | enabled (not gated) | enabled | enabled | `EditorHeader.tsx:320` only checks `isDuplicating` |
+| Settings | visible, enabled | **visible, enabled** (not gated) | visible, enabled | visible, enabled | `EditorHeader.tsx:327-334` - the one editing-adjacent control never disabled while viewing; see 3.5 |
+| Results (panel toggle) | visible iff `hasExecutionData` | visible (always true while viewing, since pinned data populates `effectiveExecutionState`) | visible | visible once any run has happened | `EditorHeader.tsx:412` gated on `hasExecutionData`, computed at `WorkflowEditorPage.tsx:3120` |
+
+## 3. Inconsistencies, concretely
+
+### 3.1 Pinned-data controls: badge, "Pinned" tag, and "Clear" do not agree on when there is something to act on
+
+- The **"Pinned" tag** on the Executions button appears whenever `pinnedExecution` is non-null, in
+  every mode including while viewing (`EditorHeader.tsx:301-306`).
+- The **"Clear" button** only appears when `pinnedExecution && !isViewingExecution`
+  (`EditorHeader.tsx:308`). So while actually viewing a pinned execution's read-only snapshot - the
+  moment the badge is loudest and most relevant - the control to clear it is gone. A user has to
+  first "Return to Editor" before "Clear" reappears, even though there is exactly as much pinned
+  data to clear at that moment as a second later.
+- Rule today: *Clear is shown only outside Viewing mode.* A consistent rule would be: *Clear is
+  shown whenever `pinnedExecution` is set, in every mode* - it already does nothing destructive to
+  the canvas (it only clears the pin store), so there's no reason to hide it during Viewing.
+- Separately, the **per-node `PinnedDataBadge`** (`PinnedDataBadge.tsx:43-50`) is driven by a
+  different signal entirely - `nodeChangeStatus`, computed once at pin time by diffing the pinned
+  snapshot against the graph as it stood *then* (`executionPinStore.ts:49-100`). It does not
+  recompute as the user keeps editing after pinning, so a node's badge can read "unchanged / pinned
+  data available" (green) even after the user has since changed that very node, until the user
+  unpins and re-pins. This is a second inconsistency of the same shape: a badge is shown as if it
+  reflects current truth, but reflects a stale snapshot instead.
+- The banner's own **"Pin This Data"** button (`ExecutionViewerBanner.tsx:50-58`) is really "stop
+  viewing, keep the pin that already exists" - it calls `setViewingExecution(false)` directly
+  (`WorkflowEditorPage.tsx:3100-3103`), not `handlePinExecution`. Since `handleViewExecution` already
+  called `pinExecution` before entering Viewing mode (`WorkflowEditorPage.tsx:553-559`), the data was
+  pinned from the moment you clicked "View", not from the moment you click "Pin This Data". The
+  button's label overstates what it does; it should read something like "Keep this data" or "Return
+  and keep pinned".
+
+### 3.2 The read-only execution view duplicates the results panel, not just a control
+
+While `isViewingExecution` is true, two panels render at once showing overlapping information about
+the *same* execution, from two different data shapes:
+
+- `ExecutionViewerBanner` (`WorkflowEditorPage.tsx:3096-3105`): status icon + word ("completed" /
+  "failed"), start timestamp, and a "Return to Editor" action - fixed at the top of the whole page.
+- `ExecutionResultsPanel` (`WorkflowEditorPage.tsx:3290-3311`), forced open by the same condition
+  (`showExecutionPanel || isViewingExecution || pinnedExecution`): its own "Status:" line
+  (`ExecutionResultsPanel.tsx:204-215`), its own execution ID, its own duration - a second
+  presentation of the same execution metadata, in the docked side panel that also exists for a live
+  run's results.
+
+So the duplication is exactly: **execution status and timing are shown twice on screen
+simultaneously** - once in the top banner, once at the head of the results panel - because Viewing
+mode doesn't reuse the results panel's summary, it adds a second, independent summary on top of it.
+There is also a second duplication in the *entry points* to this same state: from the header,
+`onShowExecutionList` opens the list panel, and `onToggleExecutionPanel` toggles the results panel;
+from the list panel, "View" and "Pin Data" are two more entry points into what is functionally the
+same read-only state machine (`ExecutionListPanel.tsx:344-369`). Four buttons across two components
+all lead into the same `pinnedExecution`/`isViewingExecution` pair.
+
+Compounding this: closing the results panel's `X` while viewing does three things at once - closes
+the panel, exits Viewing mode, *and* unpins (`WorkflowEditorPage.tsx:3297-3305`) - while the banner's
+"Return to Editor" only exits Viewing mode and leaves the pin intact
+(`WorkflowEditorPage.tsx:570-572`). Two visually distinct "close" affordances for the same screen do
+different things to the same state.
+
+### 3.3 Settings is the one editing-adjacent control not gated by Viewing mode
+
+Every other control that can mutate the graph or its metadata - Add Node, Undo, Redo, Version
+History, Auto Layout, inline rename - is disabled while `isViewingExecution`
+(`EditorHeader.tsx:196,236,244,256,285` and `122-126`). Settings is not
+(`EditorHeader.tsx:327-334`, no `disabled` prop at all). Opening it while viewing lets the user
+change `workflowSettings` and calls `setHasChanges(true)`
+(`WorkflowEditorPage.tsx:3403-3406`) while the canvas is showing a frozen, unrelated snapshot
+(`viewedNodes`/`viewedEdges`) - so a real edit gets recorded as pending while the editor is telling
+the user, via the banner, that they are looking at read-only history. Autosave is disabled while
+viewing (`WorkflowEditorPage.tsx:912`), so this change sits as `hasChanges` until the user returns to
+the editor, at which point it silently becomes a pending save with no indication it happened while
+viewing an old execution.
+
+### 3.4 Execute and Activate/Deactivate ignore Viewing mode
+
+Execute, the trigger dropdown, Duplicate, and Activate/Deactivate are all reachable while
+`isViewingExecution` is true (`EditorHeader.tsx:370,404,320,337` - none check
+`isViewingExecution`). Execute only checks `isRunning`. This does not corrupt anything (each is an
+independent API call, not a mutation of the frozen canvas), but it is inconsistent with the page's
+own framing: the banner says "Read-Only Snapshot," yet three of the most consequential actions in the
+whole editor - run it, publish-adjacent activation, duplicate it - are fully live from that screen.
+A user who clicks Execute while looking at last week's failed run has no visual cue that they just
+started a brand new run of the *current* graph, not the one on screen.
+
+### 3.5 A button whose enabled state does not match whether the action can succeed: Save
+
+Save's `disabled` is `!hasChanges || isSaving` (`EditorHeader.tsx:358`) - it does not check
+`isViewingExecution`. In today's flow this rarely surfaces because editing is disabled while
+viewing, so `hasChanges` should stay false - except 3.3 just showed a path (Settings) where
+`hasChanges` can become true while viewing. In that case Save would render enabled and clickable
+while the banner claims read-only. This is the same shape as the pinned-badge staleness in 3.1: a
+control's visible state (enabled/disabled) is derived from a flag (`hasChanges`) that can be true for
+reasons the control's label doesn't account for.
+
+### 3.6 Autosave's own edit token misses edge-only edits
+
+`useAutosave`'s doc comment says the token should be "the node and edge arrays, typically"
+(`useAutosave.ts:27`), but the call site passes only `nodes`:
+
+```
+useAutosave(nodes, hasChanges, !isViewingExecution && !!id && !saveMutation.isPending, () => {
+  handleSaveRef.current()
+})
+```
+(`WorkflowEditorPage.tsx:912`)
+
+**Verdict: an edge-only edit is still saved - no data is lost - but the debounce guarantee the hook
+exists to provide is broken for edge-only bursts.**
+
+Reasoning: `useAutosave`'s effect re-runs (and restarts the idle timer) only when `nodes`,
+`hasChanges`, or `enabled` change (`useAutosave.ts:44,65`). Reconnecting or deleting an edge goes
+through `onConnect` (`WorkflowEditorPage.tsx:1678-1681`) or `onEdgesChange`
+(`WorkflowEditorPage.tsx:3236-3241`), both of which call `setHasChanges(true)` but never touch the
+`nodes` array. The *first* edge edit in a burst still restarts the timer, because `hasChanges` itself
+flips `false -> true`, which is a dependency change. But every subsequent edge edit in the same burst
+changes neither `nodes` nor `hasChanges` (already `true`), so the effect does not re-run and the
+timer set on the *first* edit keeps its original deadline. The result: a user rewiring several edges
+in a row can have the autosave fire mid-burst - the opposite of "one burst becomes one version," which
+is the entire stated purpose of the hook (`useAutosave.ts:6-14`). When the timer does fire, it calls
+`handleSaveRef.current()`, and `handleSaveRef.current` is reassigned to the latest `handleSave` on
+every render (`WorkflowEditorPage.tsx:1756`), and `handleSave` reads the live graph via
+`currentGraph()` (`WorkflowEditorPage.tsx:1742-1745`) - so whatever fires, it saves the *current*
+edges correctly. Nothing is lost; the version history is just noisier for edge-only work than the
+hook intends, exactly as the hook's own comment warns generic keystroke-level saving would be. If
+Save is removed from the toolbar, this should be fixed first (pass `[nodes, edges]` or an
+edge-inclusive token) so the debounce behaves the same for every kind of edit - otherwise removing
+the visible Save button removes the one remaining way a careful user could force a single clean save
+point instead of relying on a timer that behaves differently for edges than for nodes.
+
+## 4. Recommendation
+
+Given the user's direction - primary actions stay visible, no overflow menu, shrink buttons instead
+- and given Save's redundancy is now confirmed (autosave always eventually persists edge-only edits
+too, once 3.6 is fixed):
+
+**Stays on the bar** (primary, always relevant to "am I looking at my workflow and can I act on it"):
+Back, workflow name, Execute (+ trigger dropdown), Activate/Deactivate, Add Node, Undo/Redo,
+Publish, Results toggle.
+
+**Remove outright:** Save. Autosave already covers it (`useAutosave.ts`, `WorkflowEditorPage.tsx:912`),
+and the "Unsaved changes (Ctrl+S)" pill already tells the user their state; Ctrl+S can keep working as
+a shortcut without a persistent button. Loses: a manual "save right now" affordance for someone who
+distrusts the timer, and the one deliberate save point before a risky edit - both minor once 3.6 is
+fixed and the autosave delay (2s idle / 30s max) is short enough that "did it save" is rarely in
+doubt.
+
+**Merge:**
+- **Clear pinned data** into the **Executions** button itself: once pinned, the button's own
+  "Pinned" tag becomes clickable (an `x`) to unpin, in every mode per 3.1, instead of being a
+  separate button that disappears in Viewing mode. Loses nothing - it's the same action, reachable
+  everywhere the state it clears is visible.
+- **Duplicate** into **Settings** (or a right-click context menu on the canvas background) as a
+  workflow-lifecycle action rather than a toolbar-level one - it's used far less often than Execute
+  or Publish. Loses one click of directness; gains bar space.
+- **Version History** into the **Publish** control's dropdown/adjacent affordance, since both are
+  about "what version is this" - clicking the version number/"Published" label could open history.
+  Loses a dedicated icon; the two are already conceptually adjacent (publish reads version, history
+  browses versions).
+
+**Move into a panel/context menu:**
+- **Auto Layout** into a canvas right-click context menu ("Arrange nodes") - it's a one-off tidy-up
+  action, not something reached for per-session. Loses one click of visibility; gains bar space.
+- **Settings** into the canvas context menu or a small icon-only affordance near the workflow name,
+  since it's opened rarely relative to Execute/Publish. Before moving it, fix 3.3/3.5 (gate it behind
+  `!isViewingExecution` like every other mutating control) - moving a button doesn't fix a control
+  that can silently create unsaved changes from a read-only screen.
+
+**Fix regardless of layout, before shrinking anything:**
+1. Gate Settings (and by extension Save's enabled-state) behind `!isViewingExecution`, matching every
+   other mutating control (3.3, 3.5).
+2. Make "Clear pinned data" available whenever `pinnedExecution` is set, not only outside Viewing
+   mode (3.1).
+3. Pass an edge-inclusive token (e.g. `[nodes, edges]`) into `useAutosave` at
+   `WorkflowEditorPage.tsx:912` so edge-only bursts debounce the same as node-only bursts (3.6).