feat(backend): add create-folder action node - #24
Conversation
5b43843 to
f125aab
Compare
Expose webdavfile.Client's existing private mkcol as a public CreateFolder(ctx, authHeader, davPath) method, wire it into the executor's FileClient interface, and add a createFolder action type that renders its `path` template against the run's vars and issues a WebDAV MKCOL. Idempotent like the existing internal mkcol usage in Comment (201/405/409 all treated as success). Unlike move/copy/rename, createFolder does not change currentPath — it creates a location rather than relocating the file under consideration. Signed-off-by: Lukas Hirt <info@hirt.cz>
Add a "Create Folder" action-picker entry (action-create-folder) under the Actions category, backed by the new createFolder ActionType, and wire its `path` template-string config field into NodeDetailsPanel.vue following the same v-model/actionParam pattern used by move/copy's destination and rename's newName fields. Signed-off-by: Lukas Hirt <info@hirt.cz>
f125aab to
094c98a
Compare
dj4oC
left a comment
There was a problem hiding this comment.
Review: feat(backend): add create-folder action node
Stats: +230/-6 across 8 files (backend + frontend)
Overview
Adds a createFolder action node: configure a template-rendered path, and the workflow issues a WebDAV MKCOL there when it runs. Backend exposes webdavfile.Client's existing private mkcol as a public CreateFolder, wires a new runAction case, and adds the frontend picker entry + config field. Deliberately doesn't touch currentPath — this creates a location rather than relocating the file under consideration, which is the right call and is directly tested (TestRunCreateFolderActionDoesNotChangeCurrentPath).
Potential issues & risks
1. The PR's own stated downstream-reuse claim doesn't hold — createFolder's output isn't actually referenceable (Medium-High)
The PR body says: "result.Output is set to the rendered path so a subsequent move/copy action can reuse it as its own destination." Looking at the runAction case added here:
case "createFolder":
folderPath := param("path")
...
result.Output = folderPath
return currentPath, nilThis only sets result.Output on the NodeResult — it never writes into the shared vars map. render() (the template engine every node's params go through) only substitutes from vars, so a later node configured with destination: "{{createFolder.output}}/report.pdf" would get the literal, unexpanded string back, not the created path. Compare with runLLM, which does both (vars["llm.output"] = output and result.Output = output) — that's the only reason {{llm.output}} actually works today. Neither of this PR's two new tests exercises a downstream node referencing {{createFolder.output}} (the first workflow has nothing after createFolder; the second's tag node uses a literal "done", not a template reference), so this gap isn't caught. Note there's a currently-open sibling PR (#23) that fixes this exact same class of bug for the other six action types (tag/comment/move/copy/rename/notify all got vars[actionType+".output"] = ... added) — createFolder would be the one action type left out of that convention unless it's added here too. Suggested fix: vars["createFolder.output"] = folderPath alongside result.Output = folderPath.
2. mkcol's 409 handling may have "already exists" and "missing parent" backwards (Medium)
CreateFolder calls the pre-existing mkcol helper unchanged, which treats 405 and 409 both as "already exists, treat as success." Per RFC 4918 §9.3 (WebDAV MKCOL), these mean different things: 405 ("MKCOL can only be executed on an unmapped URL") is genuinely "already exists," but 409 ("a collection cannot be made at the Request-URI until one or more intermediate collections have been created") means the opposite — the parent doesn't exist yet, and the folder was not created. If oCIS follows the spec here, mkcol currently swallows that failure silently and reports success.
This is pre-existing code (unchanged by this PR), but two things make it newly relevant here:
- The codebase's own
Comment()method — right abovemkcolin the same file — already works around exactly this by callingmkcol(".workflows")and thenmkcol(".workflows/comments")as two explicit, ordered calls, rather than relying on one call to create a nested path. That pattern only makes sense if the author already knew MKCOL doesn't auto-create intermediate collections — which is consistent with the RFC 409 case being a real "parent missing" error, not "already exists." CreateFolderis now a first-class, user-facing "create a folder at this path" primitive, and nested paths are an entirely expected use (the PR's own example is/Archive/{{llm.output}}; a classification value with a/in it, or any path more than one level deep where the parent doesn't already exist, would plausibly hit exactly this case). The newTestCreateFolderTreatsAlreadyExistsAsSuccesstest bakes the 405-and-409-are-equivalent assumption in as a first-class assertion, against a hand-written fake server — it can't tell us whether that assumption matches real oCIS behavior.
Worth verifying directly against a real oCIS instance: does MKCOL against a path with a missing intermediate parent actually return 409? If so, either (a) stop treating 409 as success and surface it as a real error telling the user to create the parent first, or (b) make CreateFolder create each path segment in turn (mirroring Comment's own pattern) so multi-level paths work as users would expect from a "create folder" action.
Code quality & style
CreateFolder's doc comment is honest about the limitation ("and only it — parent collections must already exist"), which is good — but see finding #2 above for why that limitation may be silently masked rather than surfaced as an error today.- Reusing the existing
mkcolhelper rather than duplicating MKCOL-request logic is the right call, keeping this consistent withComment's existing usage. - Frontend wiring (
nodeTypes.tsentry,ActionTypeunion,NodeDetailsPanel.vuefield) closely follows the existingmove/renamefield patterns — nothing novel, easy to follow.
Test coverage
Good shape overall: backend tests cover template rendering into CreateFolder's argument, that currentPath is left untouched for a following action, and (at the webdavfile layer) the MKCOL request shape plus all three status-code branches. The gap is the one called out in finding #1 — no test drives a value through {{createFolder.output}} into a later node, which is exactly the kind of test that would have caught the missing vars write (the sibling PR #23 demonstrates this pattern for the other action types). Frontend coverage (nodeTypes.spec.ts) is adequate for what's a fairly mechanical registration.
Security
No new attack surface beyond what already exists for move/copy/rename (whose destination/newName params are template-rendered from the same untrusted vars, including LLM output, with no path-traversal validation before reaching davURL). createFolder is arguably the least-constrained of the bunch, though, since unlike those three it doesn't require an existing resolved currentPath first — it's usable purely off a rendered string with zero other precondition. Not a new class of issue introduced by this PR specifically, but worth a follow-up across the whole webdavfile layer given {{llm.output}} (and, if PR #30 lands, {{webhook.body.*}}) can inject arbitrary attacker-influenced text into these paths with no sanitization.
Summary
Small, mostly clean addition, but finding #1 means the feature doesn't yet deliver what its own PR description promises (chained folder-then-move/copy workflows), and finding #2 is worth a real answer before this ships, since silent no-op folder creation on nested paths would be a confusing failure mode for end users relying on this action.
🤖 Generated with Claude Code
Summary
createFolderaction node end to end: pick aCreate Folderentry on the canvas, configure a template-renderedpath(e.g./Archive/{{llm.output}}), and the workflow issues a WebDAVMKCOLat that path when it runs.webdavfile.Client's existing privatemkcolmethod as a publicCreateFolder(ctx, authHeader, davPath) error, adds it to the executor'sFileClientinterface, and adds acase "createFolder":arm torunActionthat renders thepathtemplate againstvarsand callsCreateFolder. Idempotent (201/405/409 all treated as success), matching the existing internalmkcolusage inComment.move/copy/rename, this action does not changecurrentPath— it creates a location rather than relocating the file under consideration.result.Outputis set to the rendered path so a subsequentmove/copyaction can reuse it as its owndestination.action-create-folderentry toNODE_TYPESunder the Actions category (iconfolder-add), extendsActionTypewith'createFolder', and wires thepathconfig field intoNodeDetailsPanel.vuefollowing the samev-model/actionParampattern used bymove'sdestinationfield.Test plan
cd backend && go test ./...— all packages pass, including new tests inpkg/executor(renderspathtemplates, callsCreateFolderwith the rendered path, setsresult.Output, and does not mutatecurrentPath) and newpkg/webdavfile/webdavfile_test.go(exercisesCreateFolder's MKCOL request shape and its 201/405/409 idempotency, plus propagation of unexpected statuses).go vet ./...andgofmt -l .clean.npm run test:unit— newtests/unit/nodeTypes.spec.tsassertsaction-create-folderexists inNODE_TYPESwith the right category/actionType/defaultData and resolves viafindNodeTypeForNode.npm run check:typesandnpm run lintclean.🤖 Generated with Claude Code