[POC] feat: implement skill management in tools#6362
[POC] feat: implement skill management in tools#6362prd-hoang-doan wants to merge 1 commit intoFlowiseAI:mainfrom
Conversation
- Components: SkillPublishBar, SkillCreateNodeDialog, and reference pickers for files and tools. - Hooks & Utils: Added useNodeBlobUrl for resource handling, along with tree management, file validation, and extension utilities. - Feature Integration: Enhanced the Tools view to support full skill lifecycle management.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive "Skills V2" feature, enabling the creation, management, and execution of self-contained skill bundles within an E2B sandbox environment. It includes a full-screen UI editor with file tree and graph visualizations, a server-side compiler for transitive dependency resolution, and enhanced storage providers with raw blob primitives. Feedback focuses on improving code robustness and data integrity, specifically by simplifying boolean logic, ensuring file cleanup in error scenarios, and addressing potential race conditions and state inconsistencies during database and storage operations. Additionally, a suggestion was made to use a more portable UUID generation function in PostgreSQL migrations.
| // Capability + manifest. The manifest is only needed for the bash | ||
| // path, so building it when the user opted out keeps init() cheap. | ||
| // ------------------------------------------------------------------ | ||
| const bashEnabledByUser = nodeData.inputs?.enableBash === false ? false : true |
There was a problem hiding this comment.
The ternary operator can be simplified for better readability, prioritizing code understandability.
const bashEnabledByUser = nodeData.inputs?.enableBash !== falseReferences
- Prioritize code readability and understandability over conciseness.
| try { | ||
| fs.unlinkSync(file.path) | ||
| } catch { | ||
| /* best-effort cleanup */ | ||
| } |
There was a problem hiding this comment.
The file cleanup logic (fs.unlinkSync) should be placed in a finally block to ensure the temporary file is deleted even if fs.readFileSync fails, preventing file leaks.
try {
buffer = fs.readFileSync(file.path)
} finally {
try {
fs.unlinkSync(file.path)
} catch {
/* best-effort cleanup */
}
}| const existing = await repo().findOneBy({ workspaceId, name: dto.name.trim() }) | ||
| if (existing) { | ||
| throw new InternalFlowiseError(StatusCodes.CONFLICT, `A skill named "${dto.name}" already exists`) | ||
| } |
There was a problem hiding this comment.
The check for an existing skill name followed by a save is prone to race conditions. Consider using a database-level unique constraint to ensure data integrity.
| await Promise.all([ | ||
| writeBuffer(JSON_MIME, buf, `${nodeId}.json`, ...prefix(workspaceId, skillId), 'nodes'), | ||
| putNodeMeta(workspaceId, skillId, nodeId, meta) | ||
| ]) |
There was a problem hiding this comment.
Using Promise.all for writing the payload and metadata can lead to an inconsistent state if one operation fails. Consider sequential execution or a transaction-like approach to ensure both succeed.
| for (const id of toDelete) { | ||
| await SkillStorage.deleteNodeAssets(workspaceId, skillId, id) | ||
| } | ||
|
|
||
| await saveFileTree(workspaceId, skillId, tree) |
There was a problem hiding this comment.
If saveFileTree fails, the assets are already deleted, leaving the tree in an inconsistent state. Consider wrapping these operations in a transaction or ensuring the tree is updated before deleting assets.
|
Hi @HenryHengZJ , @harshit-flowise, @jchui-wd. Could you review the general approach? If you’re happy with this solution, I’m ready to dive back in and handle any further requirements to get this production-ready. |
1 participant
Proof-of-Concept: Skill management in tools
Ticket:
Flowise Roadmap
Overview:
A Skill is a new authoring primitive in Flowise — a self-contained app composed of markdown prompts, code, data, and binary assets that can be published once and invoked from any Agentflow as one or more LangChain tools. This PR implements the full Skill lifecycle across three packages: server, components, and UI.
Database
Execution Modes
The runtime node collapses into one of two modes depending on environment configuration:
/home/user/skills/in a sandboxed E2B VM and lets the model run shell commandsMarkdown execution flow: Each selected .md file becomes a SkillFileTool. When the LLM calls it, the tool returns the pre-compiled markdown content (with all
{{skill.*}}and{{tool.*}}placeholders resolved at publish time) plus an optional tool hint block. Runtime placeholders ({{question}},{{$vars.*}}) are resolved by Flowise's agent layer after the tool returns.Bash execution flow: A single SandboxBashTool is registered per skill. On first invocation it lazily boots an E2B VM, materialises all reachable assets (markdown, code, data, binaries) under
/home/user/skills/, and executes the command. Results are returned as a JSON envelope { status, stdout, stderr, exitCode, durationMs }. Built-in helper scripts handle document extraction (PDF, DOCX, PPTX, XLSX, HTML, TXT).File Kind Classification:
Files in the skill tree are classified into four kinds that determine how they are compiled and surfaced at runtime:
How LLM detect the bash command
For example:
Testing
Demo Recording
example-testing.zip