Skip to content

Commit 3f0e25f

Browse files
committed
feat(frontend): warn when a file action lacks an upstream file source
Executor.Run only populates vars["file.*"] (and the currentPath that file actions require) when a non-empty resourcePath reaches it. The scheduler always calls Run with an empty resourcePath, and the manual "Run now" panel only sends one if the user optionally types it in — only the File Event Trigger's SSE path reliably supplies a real file. Add a pure hasUpstreamFileSource() graph-walk (frontend/src/utils/flowValidation.ts) and surface a non-blocking warning in NodeDetailsPanel when a file action (tag/comment/move/copy/rename) has no such trigger upstream. Signed-off-by: Lukas Hirt <info@hirt.cz>
1 parent ddbce64 commit 3f0e25f

5 files changed

Lines changed: 253 additions & 2 deletions

File tree

‎frontend/src/components/NodeDetailsPanel.vue‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010
</oc-button>
1111
</div>
1212
<p v-if="nodeType" class="workflows-ndv-description">{{ nodeType.description }}</p>
13+
<p v-if="showFileSourceWarning" class="workflows-ndv-warning">
14+
{{
15+
$gettext(
16+
'This action needs a file to act on, but nothing upstream reliably provides one. Add a File Event Trigger upstream, or make sure a file path is supplied when this workflow runs.'
17+
)
18+
}}
19+
</p>
1320

1421
<div class="workflows-ndv-body">
1522
<template v-if="node.type === 'trigger'">
@@ -126,14 +133,22 @@
126133
import { computed } from 'vue'
127134
import { useGettext } from 'vue3-gettext'
128135
import { findNodeTypeForNode } from '../nodeTypes'
129-
import type { EventTriggerType, WorkflowNode, WorkflowNodeData } from '../types/workflow'
136+
import { hasUpstreamFileSource, isFileDependentActionType } from '../utils/flowValidation'
137+
import type { EventTriggerType, WorkflowEdge, WorkflowNode, WorkflowNodeData } from '../types/workflow'
130138
131-
const props = defineProps<{ node: WorkflowNode }>()
139+
const props = defineProps<{ node: WorkflowNode; nodes: WorkflowNode[]; edges: WorkflowEdge[] }>()
132140
const emit = defineEmits<{ (e: 'update', data: WorkflowNodeData): void; (e: 'close'): void }>()
133141
const { $gettext } = useGettext()
134142
135143
const nodeType = computed(() => findNodeTypeForNode(props.node.type, props.node.data.actionType))
136144
145+
const showFileSourceWarning = computed(
146+
() =>
147+
props.node.type === 'action' &&
148+
isFileDependentActionType(props.node.data.actionType) &&
149+
!hasUpstreamFileSource(props.node.id, props.nodes, props.edges)
150+
)
151+
137152
const patch = (partial: Partial<WorkflowNodeData>) => emit('update', { ...props.node.data, ...partial })
138153
139154
const field = <K extends keyof WorkflowNodeData>(key: K) =>
@@ -209,6 +224,14 @@ const paramMessage = actionParam('message')
209224
opacity: 0.7;
210225
margin-top: 0.25rem;
211226
}
227+
.workflows-ndv-warning {
228+
margin-top: 0.75rem;
229+
padding: 0.6rem 0.8rem;
230+
border-radius: 4px;
231+
background: var(--oc-color-swatch-warning-default, #fff4e5);
232+
color: var(--oc-color-swatch-warning-contrastText, #7a4a00);
233+
font-size: 0.9rem;
234+
}
212235
.workflows-ndv-body {
213236
margin-top: 1.5rem;
214237
display: flex;
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import type { ActionType, WorkflowEdge, WorkflowNode } from '../types/workflow'
2+
3+
// Action types whose backend implementation (backend/pkg/executor/executor.go, runAction)
4+
// requires a non-empty `currentPath` and fails the node otherwise. `notify` is the only
5+
// action that doesn't touch a file at all, so it's deliberately excluded.
6+
const FILE_DEPENDENT_ACTION_TYPES: ReadonlySet<ActionType> = new Set(['tag', 'comment', 'move', 'copy', 'rename'])
7+
8+
/** Whether `actionType` operates on a target file and therefore needs `currentPath` to be set. */
9+
export function isFileDependentActionType(actionType?: ActionType): boolean {
10+
return !!actionType && FILE_DEPENDENT_ACTION_TYPES.has(actionType)
11+
}
12+
13+
/**
14+
* Walks the graph backwards from `nodeId` to the trigger it descends from and reports
15+
* whether that trigger reliably supplies a file.
16+
*
17+
* This mirrors real backend behavior (backend/pkg/executor/executor.go): `vars["file.*"]`
18+
* is only populated when a non-empty `resourcePath` reaches Executor.Run, and `currentPath`
19+
* for file actions comes from that same value. Only the File Event Trigger is guaranteed to
20+
* carry one — the SSE event manager always passes the actual path of the file that fired the
21+
* event (backend/pkg/sse/manager.go). A Schedule Trigger never does: the scheduler always
22+
* calls Run with an empty resourcePath (backend/pkg/scheduler/scheduler.go). A Manual Trigger
23+
* only *might*: "Run now" lets a user optionally type a WebDAV path into a free-text field,
24+
* but nothing about the graph guarantees it's filled in, so it's treated the same as having
25+
* no file source.
26+
*
27+
* If no trigger is reachable upstream at all (e.g. a disconnected/orphan node), this returns
28+
* true — there's nothing to flag against yet, and other validation should own that concern.
29+
*/
30+
export function hasUpstreamFileSource(nodeId: string, nodes: WorkflowNode[], edges: WorkflowEdge[]): boolean {
31+
const byId = new Map(nodes.map((n) => [n.id, n]))
32+
const incoming = new Map<string, string[]>()
33+
for (const e of edges) {
34+
const list = incoming.get(e.target)
35+
if (list) {
36+
list.push(e.source)
37+
} else {
38+
incoming.set(e.target, [e.source])
39+
}
40+
}
41+
42+
const visited = new Set<string>()
43+
const queue = [nodeId]
44+
while (queue.length) {
45+
const id = queue.shift()!
46+
if (visited.has(id)) continue
47+
visited.add(id)
48+
49+
const node = byId.get(id)
50+
if (node?.type === 'trigger') {
51+
return node.data.triggerType === 'event'
52+
}
53+
54+
for (const source of incoming.get(id) ?? []) {
55+
queue.push(source)
56+
}
57+
}
58+
59+
// No trigger found upstream at all.
60+
return true
61+
}

‎frontend/src/views/WorkflowBuilder.vue‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@
8383
<NodeDetailsPanel
8484
v-if="selectedNode"
8585
:node="selectedNode"
86+
:nodes="nodes"
87+
:edges="edges"
8688
@update="(data) => updateNodeData(selectedNode!.id, data)"
8789
@close="selectedNodeId = null"
8890
/>
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { mount } from '@vue/test-utils'
3+
import { createGettext } from 'vue3-gettext'
4+
import NodeDetailsPanel from '../../src/components/NodeDetailsPanel.vue'
5+
import type { WorkflowEdge, WorkflowNode } from '../../src/types/workflow'
6+
7+
const stubs = {
8+
'oc-icon': true,
9+
'oc-button': true,
10+
'oc-text-input': true
11+
}
12+
13+
const gettext = createGettext({ availableLanguages: { en: 'English' }, defaultLanguage: 'en' })
14+
15+
const mountPanel = (node: WorkflowNode, nodes: WorkflowNode[], edges: WorkflowEdge[]) =>
16+
mount(NodeDetailsPanel, {
17+
props: { node, nodes, edges },
18+
global: { plugins: [gettext], stubs }
19+
})
20+
21+
describe('NodeDetailsPanel file-source warning', () => {
22+
const moveAction: WorkflowNode = {
23+
id: 'action-1',
24+
type: 'action',
25+
position: { x: 0, y: 0 },
26+
data: { actionType: 'move' }
27+
}
28+
29+
it('warns when configuring a move action fed only by a manual trigger', () => {
30+
const nodes: WorkflowNode[] = [
31+
{ id: 'trigger', type: 'trigger', position: { x: 0, y: 0 }, data: { triggerType: 'manual' } },
32+
moveAction
33+
]
34+
const edges: WorkflowEdge[] = [{ id: 'e1', source: 'trigger', target: 'action-1' }]
35+
36+
const wrapper = mountPanel(moveAction, nodes, edges)
37+
38+
expect(wrapper.find('.workflows-ndv-warning').exists()).toBe(true)
39+
})
40+
41+
it('does not warn when configuring a move action fed by a File Event Trigger', () => {
42+
const nodes: WorkflowNode[] = [
43+
{
44+
id: 'trigger',
45+
type: 'trigger',
46+
position: { x: 0, y: 0 },
47+
data: { triggerType: 'event', event: { type: 'upload' } }
48+
},
49+
moveAction
50+
]
51+
const edges: WorkflowEdge[] = [{ id: 'e1', source: 'trigger', target: 'action-1' }]
52+
53+
const wrapper = mountPanel(moveAction, nodes, edges)
54+
55+
expect(wrapper.find('.workflows-ndv-warning').exists()).toBe(false)
56+
})
57+
58+
it('does not warn for a non-file-dependent action such as notify', () => {
59+
const notifyAction: WorkflowNode = {
60+
id: 'action-1',
61+
type: 'action',
62+
position: { x: 0, y: 0 },
63+
data: { actionType: 'notify' }
64+
}
65+
const nodes: WorkflowNode[] = [
66+
{ id: 'trigger', type: 'trigger', position: { x: 0, y: 0 }, data: { triggerType: 'manual' } },
67+
notifyAction
68+
]
69+
const edges: WorkflowEdge[] = [{ id: 'e1', source: 'trigger', target: 'action-1' }]
70+
71+
const wrapper = mountPanel(notifyAction, nodes, edges)
72+
73+
expect(wrapper.find('.workflows-ndv-warning').exists()).toBe(false)
74+
})
75+
})
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { hasUpstreamFileSource, isFileDependentActionType } from '../../src/utils/flowValidation'
3+
import type { WorkflowEdge, WorkflowNode } from '../../src/types/workflow'
4+
5+
// Only the File Event Trigger reliably supplies a file to the run's template-variable
6+
// context (backend/pkg/executor/executor.go only fills `vars["file.*"]` when a non-empty
7+
// resourcePath is passed to Executor.Run, and only the SSE event path always provides one:
8+
// - the scheduler always calls Run(..., "schedule", "") — resourcePath is hardcoded empty.
9+
// - the manual "Run now" panel sends whatever the user optionally typed into a free-text
10+
// field, so it isn't guaranteed either.
11+
// - the SSE event manager passes the actual file path from the triggering event.
12+
describe('hasUpstreamFileSource', () => {
13+
const trigger = (id: string, data: WorkflowNode['data']): WorkflowNode => ({
14+
id,
15+
type: 'trigger',
16+
position: { x: 0, y: 0 },
17+
data
18+
})
19+
const llm = (id: string): WorkflowNode => ({
20+
id,
21+
type: 'llm',
22+
position: { x: 0, y: 0 },
23+
data: { prompt: 'summarize {{file.content}}' }
24+
})
25+
const action = (id: string, actionType: 'move' | 'tag'): WorkflowNode => ({
26+
id,
27+
type: 'action',
28+
position: { x: 0, y: 0 },
29+
data: { actionType }
30+
})
31+
const edge = (source: string, target: string): WorkflowEdge => ({ id: `${source}-${target}`, source, target })
32+
33+
it('flags a manual trigger chained directly into a move-file action', () => {
34+
const nodes = [trigger('trigger', { triggerType: 'manual' }), action('action-1', 'move')]
35+
const edges = [edge('trigger', 'action-1')]
36+
37+
expect(hasUpstreamFileSource('action-1', nodes, edges)).toBe(false)
38+
})
39+
40+
it('does not flag an event trigger chained directly into a move-file action', () => {
41+
const nodes = [trigger('trigger', { triggerType: 'event', event: { type: 'upload' } }), action('action-1', 'move')]
42+
const edges = [edge('trigger', 'action-1')]
43+
44+
expect(hasUpstreamFileSource('action-1', nodes, edges)).toBe(true)
45+
})
46+
47+
it('still flags manual trigger -> llm -> move-file (no file anywhere upstream)', () => {
48+
const nodes = [trigger('trigger', { triggerType: 'manual' }), llm('llm-1'), action('action-1', 'move')]
49+
const edges = [edge('trigger', 'llm-1'), edge('llm-1', 'action-1')]
50+
51+
expect(hasUpstreamFileSource('action-1', nodes, edges)).toBe(false)
52+
})
53+
54+
it('does not flag event trigger -> llm -> move-file', () => {
55+
const nodes = [
56+
trigger('trigger', { triggerType: 'event', event: { type: 'upload' } }),
57+
llm('llm-1'),
58+
action('action-1', 'move')
59+
]
60+
const edges = [edge('trigger', 'llm-1'), edge('llm-1', 'action-1')]
61+
62+
expect(hasUpstreamFileSource('action-1', nodes, edges)).toBe(true)
63+
})
64+
65+
it('flags a schedule trigger too, since the scheduler always runs with an empty resourcePath', () => {
66+
const nodes = [trigger('trigger', { triggerType: 'schedule', schedule: '0 * * * *' }), action('action-1', 'move')]
67+
const edges = [edge('trigger', 'action-1')]
68+
69+
expect(hasUpstreamFileSource('action-1', nodes, edges)).toBe(false)
70+
})
71+
72+
it('returns true for a node with no upstream trigger at all (nothing to flag against)', () => {
73+
const nodes = [action('action-1', 'move')]
74+
expect(hasUpstreamFileSource('action-1', nodes, [])).toBe(true)
75+
})
76+
})
77+
78+
describe('isFileDependentActionType', () => {
79+
it('flags actions that operate on a target file', () => {
80+
expect(isFileDependentActionType('tag')).toBe(true)
81+
expect(isFileDependentActionType('comment')).toBe(true)
82+
expect(isFileDependentActionType('move')).toBe(true)
83+
expect(isFileDependentActionType('copy')).toBe(true)
84+
expect(isFileDependentActionType('rename')).toBe(true)
85+
})
86+
87+
it('does not flag notify, which needs no target file', () => {
88+
expect(isFileDependentActionType('notify')).toBe(false)
89+
})
90+
})

0 commit comments

Comments
 (0)