feat(frontend): dynamically disable file-dependent actions in the node picker

[LukasHirt]

Backend investigation

Read backend/pkg/executor/executor.go end-to-end plus the callers of Executor.Run (backend/pkg/service/workflows.go, backend/pkg/scheduler/scheduler.go, backend/pkg/sse/manager.go):

• vars["file.name"]/vars["file.content"] are only populated when resourcePath != "" at the top of Run() (executor.go:66-76). If GetContent fails or resourcePath is empty, those vars are simply never set — no error, no fallback.
• runAction's tag/comment/move/copy/rename branches all require a non-empty currentPath (seeded from resourcePath) and return a hard runtime error ("...action needs both a target file and...") when it's empty. notify is the only action that doesn't touch currentPath at all.
• Whether resourcePath is non-empty depends entirely on who calls Run, not on the graph itself:
  • scheduler.go:147 — s.executor.Run(ctx, authHeader, *wf, "schedule", "") — always an empty string. A Schedule Trigger can never provide a file. This is structurally impossible, not just unlikely.
  • service/workflows.go Run handler (manual "Run now") — resourcePath comes from an optional free-text field in ExecutionsPanel.vue ("File to run against (WebDAV path)"). It can work — Manual Trigger + file action is a legitimate, common workflow shape — but nothing in the graph guarantees the user filled it in.
  • sse/manager.go:306 — passes the real path from the file event that fired. The File Event Trigger is the only trigger that reliably supplies a file on every run.

Decision (evolved across this PR)

1. Started with a non-blocking warning in NodeDetailsPanel.vue.
2. Replaced/supplemented that with pick-time restriction in NodePicker.vue: disable file-dependent action entries when the picker's source node's upstream trigger doesn't reliably provide a file.
3. CI caught a real bug in step 2: build-workflow.spec.ts and run-workflow.spec.ts both build a Manual Trigger -> LLM -> Add Tag chain, and the picker was disabling "Add Tag" because the disable-check treated Manual Trigger the same as Schedule Trigger. That's too strict — Manual Trigger is only conditionally file-less (depends on what the user types into "Run now"), whereas Schedule Trigger is structurally file-less (the scheduler always calls Run with an empty resourcePath, full stop). Fixed by splitting the check in two:
   • hasUpstreamFileSource() keeps its original, stricter "only Event Trigger is reliable" semantics — still used for the informational NodeDetailsPanel nudge (fires for both Manual and Schedule).
   • canUpstreamProvideFile() is new and looser — only Schedule Trigger fails it — and is what NodePicker.vue now uses to decide what to hard-disable. Manual Trigger no longer blocks file-dependent actions in the picker.

What changed

• frontend/src/utils/flowValidation.ts (new): pure, tested graph-walk.
  • hasUpstreamFileSource(nodeId, nodes, edges) — true only if the upstream trigger is event (or none is reachable at all). Used by NodeDetailsPanel's warning.
  • canUpstreamProvideFile(nodeId, nodes, edges) — true unless the upstream trigger is schedule (the only structurally-impossible case). Used by NodePicker's hard-disable.
  • isFileDependentActionType(actionType) — tag/comment/move/copy/rename are file-dependent; notify is not.
• frontend/src/components/NodePicker.vue: new sourceNodeId/nodes/edges props. Disables file-dependent entries (grayed out, inline reason + tooltip, click no-ops) only when the source's upstream trigger is a Schedule Trigger.
• frontend/src/components/NodeDetailsPanel.vue: shows a non-blocking warning banner for file-dependent actions whose upstream trigger isn't an Event Trigger (Manual or Schedule) — kept as a secondary safety net, since Vue Flow lets users rewire an action node's incoming edge by dragging directly between existing nodes (bypassing the picker), and workflows created via the REST API never go through the picker at all.
• frontend/src/views/WorkflowBuilder.vue: threads pickerConnectFrom/nodes/edges into NodePicker, and nodes/edges into NodeDetailsPanel.

Left the node picker's categorization, the "+" button, node positioning, config-modal OK-button/auto-open, and output hints untouched — those are owned by other in-flight changes.

Test plan

TDD throughout, including for the CI-driven follow-up fix (tests updated/added first, confirmed failing against the pre-fix code, then implementation, then confirmed passing):

• frontend/tests/unit/flowValidation.spec.ts (14 cases): hasUpstreamFileSource covers direct manual→move flags, direct event→move doesn't, manual→llm→move still flags, event→llm→move doesn't, schedule-trigger case, orphan-node case. canUpstreamProvideFile covers the same shapes but asserts Manual Trigger is not flagged and only Schedule Trigger is.

• frontend/tests/unit/NodePicker.spec.ts (5 cases): Move File not disabled for a manual-trigger source; disabled (with reason text) for a schedule-trigger source; not disabled for an event-trigger source; clicking a disabled entry doesn't emit select; non-file action (Send Notification) stays enabled regardless of source.

• frontend/tests/unit/NodeDetailsPanel.spec.ts (3 cases): warns for manual→move, doesn't warn for event→move, doesn't warn for notify.

• npm run test:unit — 5 test files, 26 tests, all passing

• npm run check:types — clean

• npm run lint — clean

• CI (gh pr checks 22) re-run after the fix — see checks on this PR

[dj4oC]

Review: feat(frontend): dynamically disable file-dependent actions in the node picker

Stats: +448/-9 across 7 files (frontend only)

Overview

Prevents users from wiring up file-dependent actions (tag/comment/move/copy/rename) downstream of a trigger that can't reliably supply a file. Adds a new pure module flowValidation.ts with two intentionally different checks — hasUpstreamFileSource (strict: only File Event Trigger counts, used for a non-blocking warning banner in NodeDetailsPanel) and canUpstreamProvideFile (loose: only Schedule Trigger is disqualifying, used to hard-disable entries in NodePicker) — plus isFileDependentActionType. The PR body documents a real CI-caught bug from an earlier iteration (Manual Trigger was wrongly treated as strictly as Schedule Trigger, breaking the "Manual Trigger + Run now with a supplied path" workflow shape) and the fix that split the check in two. I independently verified the backend claims underpinning this split against main: scheduler.go:147 does call Run(ctx, authHeader, *wf, "schedule", "") with a hardcoded empty resourcePath, and executor.go's runAction does hard-fail tag/comment/move/copy/rename when currentPath == "" — so the Schedule-Trigger-is-structurally-impossible / Manual-Trigger-is-only-conditional distinction is accurate, not just asserted.

Code quality & style

• flowValidation.ts is clean, dependency-free, and well-commented on the why (each function's doc comment explains the reasoning and points at the specific backend line it mirrors) — this is exactly the kind of comment worth keeping, since the two functions' near-identical shape makes it easy to reach for the wrong one without that context.
• The BFS in upstreamTriggerType is straightforward and correct: builds a reverse adjacency map once, then walks backwards with a visited set, terminating on the first type === 'trigger' node found, returning undefined for an unreachable/orphan node (deliberately treated as "nothing to flag" rather than an error).
• Good separation of concerns: NodePicker hard-disables (with a visible reason + tooltip), NodeDetailsPanel only warns — and the PR explains why both are needed (Vue Flow lets users rewire an edge by dragging directly between existing nodes, bypassing the picker entirely; workflows created via the REST API never touch the picker at all).

Minor observations (non-blocking)

• upstreamTriggerType's BFS enqueues a node's predecessors before checking whether they're already visited, only skipping work on dequeue. For graphs where multiple paths reconverge on a shared ancestor, this can enqueue the same id more than once — harmless (idempotent, and visited still prevents infinite loops on cycles) but slightly wasteful. Not worth changing given realistic workflow-graph sizes.
• The BFS also implicitly assumes exactly one trigger node is reachable upstream of any given node. That matches how workflows are modeled today (one WorkflowTrigger per workflow), so it's a safe assumption now — just flagging that if a future graph shape ever allowed two distinct trigger nodes to reconverge on one downstream node, the "first found via BFS" result would be traversal-order-dependent rather than deterministic by design.
• NodePicker.vue sets both the native disabled attribute and aria-disabled on the button. The native attribute alone removes the item from tab order, so a keyboard-only screen reader user tabbing through won't land on it to hear the reason text — though since the reason renders as a plain sibling <span> in the DOM (not only a title tooltip), it's still discoverable via non-tab browsing (arrow-key/virtual-cursor reading). Not a blocker, just worth knowing if a11y is a priority for this component.

Test coverage

Thorough and well-targeted: 14 cases in flowValidation.spec.ts covering both functions across direct/indirect chains, all three trigger types, and the orphan-node case; 5 cases in NodePicker.spec.ts covering enable/disable per trigger type, the disabled-click-no-ops guard, and that non-file actions stay enabled regardless of trigger; 3 new cases in NodeDetailsPanel.spec.ts for the warning banner. The PR body's description of a TDD, CI-driven fix cycle (test added/updated first, confirmed red, then the hasUpstreamFileSource/canUpstreamProvideFile split, confirmed green) is a good practice worth calling out positively. One small gap: no direct NodeDetailsPanel mount test for the Schedule Trigger case specifically (only Manual/Event/notify are exercised there) — low risk since the underlying hasUpstreamFileSource util already covers schedule directly and the component's computed is a thin pass-through.

Security / performance

No concerns — this is pure client-side UX/validation with no new network calls, no new state persisted, and graph sizes here are small enough that the BFS's minor inefficiency is a non-issue.

Summary

Solid, self-contained PR. The two-tier strict/loose validation split is the right call and is backed by verified backend behavior rather than assumption. Nothing here blocks merge; the observations above are minor polish, not defects.

---

🤖 Generated with Claude Code