From 6f01c6b28bfa1ca060130eb54cf4cbf71065c461 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 09:20:24 +0000 Subject: [PATCH 01/14] docs: PRD + ADRs for flow skills and MCP servers Add planning docs (no code) for two new capabilities: - Upload a SKILL.md to a conversational step (reusable library + inline override), injected as a cache-stable prompt block. - MCP server integration: admin-registered remote servers, a deterministic `mcp` node, and tool-calling in conversational steps via a separate agentic runner (ILanguageModel unchanged). PRD: flow-skills-and-mcp.prd.md (target v1.52.0 then v1.53.0, MINOR per phase) ADR-031: runtime skills as injected step instructions ADR-032: MCP integration and tool-calling architecture Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01GtujrmX4ywLjDUF3j6X2Yh --- ...e-skills-injected-step-instructions.adr.md | 112 +++++++++++ ...32-mcp-integration-and-tool-calling.adr.md | 154 +++++++++++++++ .../prd/flow-skills-and-mcp.prd.md | 184 ++++++++++++++++++ 3 files changed, 450 insertions(+) create mode 100644 docs/development/adr/031-runtime-skills-injected-step-instructions.adr.md create mode 100644 docs/development/adr/032-mcp-integration-and-tool-calling.adr.md create mode 100644 docs/development/prd/flow-skills-and-mcp.prd.md diff --git a/docs/development/adr/031-runtime-skills-injected-step-instructions.adr.md b/docs/development/adr/031-runtime-skills-injected-step-instructions.adr.md new file mode 100644 index 00000000..e9b934a3 --- /dev/null +++ b/docs/development/adr/031-runtime-skills-injected-step-instructions.adr.md @@ -0,0 +1,112 @@ +# ADR-031 — Runtime Skills as Injected Step Instructions + +- **Status**: Proposed +- **Date**: 2026-06-30 +- **Relates to**: Flow Skills & MCP PRD, ADR-016 (prompt structure / cache), + ADR-032 (MCP integration — the `allowed-tools` bridge) + +## Context + +Flow authors steer a conversational step with the free-text `aiInstruction` field +(`ConversationalNodeConfig`). The wider ecosystem distributes reusable expertise +as **`SKILL.md`** files — YAML frontmatter (`name`, `description`, optional +`allowed-tools`, etc.) plus a markdown body of instructions, sometimes with +bundled scripts/assets. Authors want to drop such a skill into a Wayfinder step +without retyping it, and to reuse the same skill across flows. + +Note: the repo's existing `docs/guides/skills.md` describes Claude Code's +*authoring-time* routing layer. That is unrelated to this ADR. "Skill" here means +a **runtime** artefact that shapes a conversational step's behaviour. + +Two shapes were considered for how a skill attaches to a step: + +1. **Inline only** — paste the SKILL.md onto a node; lives in `flow_nodes.config`. + Simple, but no reuse and no governance. +2. **Library only** — upload once into a managed store; steps reference by id. + Reuse + governance, but blocks quick one-offs. + +## Decision + +### Support both: a reusable library *and* an inline per-step override + +- A new **`app_skills`** table holds reusable skills (name, description, + `frontmatter` jsonb, `body` text, `allowed_tools` jsonb, `version` int, `status`). + Updating a skill bumps its `version`; references resolve to the current version + (flow-level pinning is deferred — see Consequences). +- `ConversationalNodeConfig` gains: + - `skillRefs?: string[]` — ids of library skills applied to the step. + - `inlineSkill?: ParsedSkill` — a one-off skill stored only in the node config. + Both may be present; inline is appended after referenced skills. + +### A skill is parsed, not executed + +A `SKILL.md` is parsed into a `ParsedSkill` value: + +- `name`, `description` — from frontmatter (fall back to first heading / filename). +- `body` — the markdown instructions. +- `allowedTools` — from the frontmatter `allowed-tools` list, normalised to + `McpToolRef`s where they resolve to a registered MCP tool (ADR-032). This is the + **only** bridge between a skill and tool access. + +Anything else a skill bundles — scripts, binaries, file references — is **ignored +at runtime**. Wayfinder never executes skill-bundled code; a skill contributes +instructions and a tool-allowance declaration, nothing more. Parsing lives behind +`ISkillParser` (adapter), returning `Result` — a malformed file is a +`DomainError`, never a throw. + +### Skills inject as a dedicated, cache-stable prompt block + +`buildSystemPrompt` (`FlowSessionGraph`) gains a `` block, placed as a +sibling of the existing `` block — i.e. in the **stable** +region of the prompt, above the per-turn `` chunks, to +preserve prompt-cache hits (ADR-016 Decision 5). Each applied skill renders as: + +``` + + ...body... + +``` + +Referenced (library) skills render first, in author order; the inline skill last. +Skills shape *how* the step behaves; they do not replace `aiInstruction`, +`doneWhen`, or the structured `` contract — those remain authoritative. + +### `allowed-tools` is advisory at author time, enforced at runtime by MCP + +A skill naming tools that are not registered MCP tools is **not** fatal: its +instructions still inject, and the editor surfaces the unresolved names as a +warning. Actual tool access for a conversational step is governed by the node's +`allowedMcpToolRefs` (ADR-032), which the editor pre-populates from the applied +skills' `allowedTools`. Injecting prose and granting tool access are deliberately +separate concerns. + +## Consequences + +**Positive** + +- Authors reuse external skills with one upload; the library gives a single place + to govern and version them. +- Inline override keeps experimentation friction-free with no library clutter. +- The cache-stable `` block adds capability without disturbing the + generation mechanism or the per-turn cost profile. +- "Ignore bundled code" keeps the security surface small: a skill is text, not an + execution vector. + +**Negative** + +- Two attachment paths (refs + inline) mean two code paths in the editor and in + prompt assembly. Mitigated by both resolving to the same `ParsedSkill` list + before injection. +- Library skills resolve to their **current** version, so editing a skill changes + every referencing flow's behaviour. Acceptable for v1; per-flow version pinning + is deferred until an author hits the need (YAGNI), and `flow_versioning` already + gives a snapshot escape hatch. +- Large skill bodies consume context budget. Surfaced as an editor concern, not a + hard limit, in this ADR. + +## Alternatives considered + +- **Fold the skill body into `aiInstruction`.** Rejected: no reuse, no governance, + and it conflates author intent with imported expertise. +- **Treat skills as RAG documents.** Rejected: skills are behavioural instructions + that must apply every turn deterministically, not retrieved-on-relevance chunks. diff --git a/docs/development/adr/032-mcp-integration-and-tool-calling.adr.md b/docs/development/adr/032-mcp-integration-and-tool-calling.adr.md new file mode 100644 index 00000000..974c9432 --- /dev/null +++ b/docs/development/adr/032-mcp-integration-and-tool-calling.adr.md @@ -0,0 +1,154 @@ +# ADR-032 — MCP Integration and Tool-Calling Architecture + +- **Status**: Proposed +- **Date**: 2026-06-30 +- **Relates to**: Flow Skills & MCP PRD, ADR-031 (runtime skills), + ADR-010/ADR-013 (external workflow integration / auto-node structured data), + ADR-020 (reuse of `session_step_outputs`), ADR-016 (prompt structure), + ADR-026/027 (usage governance) + +## Context + +Wayfinder steps can talk to the user (`conversational`) or call a pre-registered +n8n webhook (`auto`). They cannot reach the growing ecosystem of tools exposed via +the **Model Context Protocol (MCP)**. We want two things: + +1. A deterministic step that calls one MCP tool (like the auto-node, but MCP). +2. A conversational step that can call MCP tools mid-conversation, bounded by what + a skill's `allowed-tools` permits (ADR-031). + +Two hard constraints frame the decision: + +- **Architecture rules.** `packages/domain` has zero external deps; + `packages/application` imports only domain + shared (no SDKs); SDKs live in + `packages/adapters`. So any MCP client SDK and any tool-calling SDK feature must + live in adapters, behind domain ports. +- **The conversational turn is a single structured generation today.** + `buildSystemPrompt` → `generateObject`/`streamObject` emits + `{response, rationale, stepCompleteConfidence, contextGathered}`. `ILanguageModel` + has no tool-calling surface. Introducing tools must not break that contract or + the confidence/auto-advance semantics. + +## Decision + +### 1. Remote-only, admin-registered MCP servers + +The hosted multi-tenant runtime talks only to **remote** MCP servers over +HTTP/SSE. Local/stdio (process-spawning) transports are out of scope — they are a +sandbox-escape and operability risk in a shared container. + +Registration is an **admin** action, mirroring the n8n governance model: + +- `admin_mcp_servers` — `label`, `transport` (`http` | `sse`), `url`, + `auth_kind`, `credential_ref`, `status`. +- `admin_mcp_tools` (optional cache) — discovered tool schemas per server, + refreshable; the flow editor reads this, falling back to a live `listTools`. + +Flow authors **select** from registered servers; they never see credentials and +never register servers themselves. + +### 2. Ports in domain, SDK in adapters + +- `IMcpClient` (domain port) — `listTools(server)` and + `callTool(server, name, args)`, both `Result`-typed. No SDK types leak across. +- `IMcpServerDirectory` (domain port) — lists servers + tools for the editor, + directly analogous to `IN8nWorkflowDirectory`. +- `IMcpServerRepository` (domain port) — admin CRUD. +- The **MCP client adapter** (`packages/adapters/src/mcp/`) implements `IMcpClient` + using the Vercel AI SDK MCP client (verify the exact API in `node_modules` + before building — do not rely on training data). Credentials are resolved inside + the adapter from `credential_ref`; the plaintext value never crosses a boundary. + +### 3. Credentials are referenced, never stored or returned in the clear + +`admin_mcp_servers.credential_ref` points at the secret store; the secret is +resolved only inside the MCP adapter at call time. No read endpoint returns it. +(Open question: which store backs the ref — see below.) + +### 4. The deterministic `mcp` node reuses the existing executor path + +Add `"mcp"` to `FlowNodeType` and an `McpNodeConfig` (`serverId`, `toolName`, +`requestFields` + `requestFieldValues` keyed by `FieldValueSource`, +`responseFields`). Implement `McpNodeExecutor implements INodeExecutor` and add it +to the `NodeExecutors` registry beside `n8n`/`mock`. It: + +1. Resolves `requestFields` to tool arguments via the existing `FieldValueSource` + machinery (same as the auto-node). +2. Calls `IMcpClient.callTool`. +3. Coerces the result into `responseFields` and persists them to + `session_step_outputs` via the **existing** path (ADR-020) — no new persistence. + +This keeps the deterministic case entirely within proven infrastructure. The +synchronous MCP call returns `completed` (or `failed`) directly — unlike n8n, there +is no inbound-webhook `pending` round-trip. + +### 5. Tool-calling in conversational steps lives in a separate agentic runner — `ILanguageModel` is unchanged + +This is the load-bearing choice. Two options were weighed: + +- **(a) Extend `ILanguageModel`** with a tool-enabled generate. Rejected: it + pushes tool-loop concerns into the port every provider implements and into the + simple structured-turn fast path, for a feature only some steps use. +- **(b) A separate agentic runner in the agents adapter** owns the tool loop and + the `IMcpClient`, and is invoked **only** when a conversational step has + `allowedMcpToolRefs`. Chosen. + +The runner (alongside `FlowSessionGraph`/`langgraph-agent-runner`) runs the +tool-call loop, then produces the **same** +`{response, rationale, stepCompleteConfidence, contextGathered}` structured output +the existing path returns — so confidence, auto-advance, and step-completion +semantics are untouched downstream. Steps without allowed tools keep using the +existing single-generation path with zero behavioural change. + +Tool exposure is **deny-by-default**: the runner offers the model only the tools in +the node's `allowedMcpToolRefs`. A call to anything else is refused before any +network request. The editor pre-fills `allowedMcpToolRefs` from the applied +skills' `allowedTools` (ADR-031), but the node config — not the skill — is the +enforcement boundary. + +### 6. Tool output is untrusted external data + +MCP results entering the prompt are framed and isolated the way +`` are (ADR-016): clearly delimited, never treated as +instructions. The agent is told tool output is data, not direction. + +### 7. Audit and usage reuse existing infrastructure + +Every tool call writes an `audit-logger` entry attributed to session/flow/user and +a usage record through the existing usage repository (so tool turns count toward +ADR-026/027 governance). No new ledger table (ADR-020 reasoning): introduce one +only if a concrete audit/retention requirement proves step/usage records +insufficient. + +## Consequences + +**Positive** + +- Domain/application stay SDK-free; all MCP and tool-loop machinery is confined to + adapters, satisfying the architecture rules and `validate.sh`. +- The deterministic `mcp` node is almost entirely existing infrastructure + (executor registry + `FieldValueSource` + ADR-020 persistence). +- The simple conversational turn keeps its exact shape and cost profile; the tool + loop is opt-in and isolated, limiting blast radius. +- Admin-only remote registration gives one governed, auditable egress surface. + +**Negative** + +- Two AI execution paths for conversational steps (with/without tools) to maintain. + Mitigated by both emitting the identical structured contract. +- Synchronous MCP calls block the `mcp` step for the tool's duration; slow tools + need timeouts and surface as failed steps (no async callback like n8n). +- Tool loops add latency and tokens; usage governance must account for them. +- Live tool discovery depends on server reachability; the `admin_mcp_tools` cache + mitigates editor-time flakiness but can drift until refreshed. + +## Open questions (resolve during build) + +- **Secret store for `credential_ref`** — encrypted column vs. env-referenced + secret vs. `system_setting`. Must guarantee no client exposure. +- **Tool-loop + streaming + structured output** — confirm via a spike that the + chosen SDK can stream a tool-using turn and still yield the structured object. +- **Per-tool timeouts / retry** — defaults and whether they are admin-configurable + per server. +- **`allowed-tools` resolution** — soft warning when a skill names an unregistered + tool (ADR-031 leans soft); confirm the editor UX. diff --git a/docs/development/prd/flow-skills-and-mcp.prd.md b/docs/development/prd/flow-skills-and-mcp.prd.md new file mode 100644 index 00000000..d5ddd139 --- /dev/null +++ b/docs/development/prd/flow-skills-and-mcp.prd.md @@ -0,0 +1,184 @@ +# PRD — Flow Skills & MCP Servers + +- **Status**: Draft +- **Date**: 2026-06-30 +- **Author**: John (john769160@gmail.com) +- **Target version**: 1.52.0 then 1.53.0 (bump: MINOR per phase — see `docs/guides/versioning.md`) + +## 1. Problem + +Flow authors can only steer a conversational step with free-text instructions +typed into the node. Reusable, battle-tested know-how authored elsewhere — a +`SKILL.md` found in another project or shared by a colleague — cannot be dropped +into a Wayfinder step. There is also no way for a step to *do* anything beyond +talk to the user or call a pre-registered n8n webhook: the AI cannot reach an +external system through a standard tool protocol. Both gaps push authors toward +copy-pasting prose and toward bespoke n8n workflows for things the wider +ecosystem already exposes as MCP tools. + +## 2. Users / Personas + +- **Flow author (ops/HR/procurement lead)** — wants to reuse a skill they found + elsewhere in a step, and wants steps that can call external tools, without + writing code. +- **Platform admin** — must govern which external MCP servers the platform may + reach and with what credentials, the same way they govern n8n and HR data. +- **End-user operator** — runs the flow; benefits from more capable steps but + should see no new configuration burden. + +## 3. Goals + +- A flow author can upload a `SKILL.md` and have its instructions apply to a + conversational step — either from a reusable library or as a one-off on the step. +- The same skill can be referenced by many steps and versioned independently of + any one flow. +- An admin can register remote MCP servers (HTTP/SSE) once, with stored + credentials, and flow authors can use their tools without seeing secrets. +- A flow author can add a deterministic `mcp` step that calls one MCP tool with + mapped inputs and captures the result as step output. +- A conversational step can call MCP tools mid-conversation, limited to the tools + a skill's `allowed-tools` permits. +- Every MCP tool call is auditable and counts toward existing usage records. + +## 4. Non-goals + +- **Local/stdio MCP servers.** Hosted multi-tenant runtime only talks to remote + (HTTP/SSE) servers (ADR-032). Process-spawning transports are explicitly out. +- **Author-supplied MCP servers per flow.** Registration is an admin action only. +- **Executing arbitrary scripts/code bundled in a skill.** A skill is parsed for + frontmatter + markdown instructions and an `allowed-tools` declaration; any + bundled scripts/assets are ignored at runtime (ADR-031). +- **MCP elicitation / sampling / server-initiated prompts.** Phase 1 of MCP uses + request/response tool calls only. +- **A skill marketplace / sharing between tenants.** Upload only. + +## 5. Key entities + +| Entity | Lives in | New / existing | Notes | +| ------ | -------- | -------------- | ----- | +| `Skill` | `packages/domain/src/entities/skill.ts` | new | Parsed SKILL.md: name, description, frontmatter, body, `allowedTools`, version, status. | +| `ParsedSkill` (value) | `packages/domain/src/entities/skill.ts` | new | Result of parsing a raw SKILL.md before it becomes a stored `Skill` or inline config. | +| `McpServer` | `packages/domain/src/entities/mcp-server.ts` | new | Admin-registered remote server: label, transport, url, credential ref, status. | +| `McpTool` (value) | `packages/domain/src/entities/mcp-server.ts` | new | A discovered tool: name, description, JSON-schema input. Cached, refreshable. | +| `ConversationalNodeConfig` | `packages/domain/src/entities/flow-node.ts` | existing → extend | Add `skillRefs?: string[]`, `inlineSkill?: ParsedSkill`, `allowedMcpToolRefs?: McpToolRef[]`. | +| `FlowNodeType` | `packages/domain/src/entities/flow-node.ts` | existing → extend | Add `"mcp"`. | +| `McpNodeConfig` | `packages/domain/src/entities/flow-node.ts` | new | `serverId`, `toolName`, `requestFields` + `requestFieldValues`, `responseFields`. | +| `ISkillRepository` | `packages/domain/src/ports/skill-repository.ts` | new | CRUD + list for the library. | +| `IMcpServerRepository` | `packages/domain/src/ports/mcp-server-repository.ts` | new | CRUD + list for registered servers (admin). | +| `IMcpClient` | `packages/domain/src/ports/mcp-client.ts` | new | `listTools(server)` / `callTool(server, name, args)` — Result pattern. | +| `IMcpServerDirectory` | `packages/domain/src/ports/mcp-server-directory.ts` | new | Lists servers + their tools for the flow editor (mirrors `IN8nWorkflowDirectory`). | + +## 6. User stories + +1. As a flow author, I can upload a `SKILL.md` to my skill library and give it a + name, so I can reuse it across flows. +2. As a flow author, I can attach one or more library skills to a conversational + step, so the AI follows that skill's instructions. +3. As a flow author, I can paste/upload a one-off skill directly onto a single + step without adding it to the library, so quick experiments stay local. +4. As an admin, I can register a remote MCP server with its URL and credentials, + so the platform can reach its tools under governance. +5. As a flow author, I can add an `mcp` step, pick a server + tool, map inputs + from earlier steps, and capture the output, so a step performs a concrete + external action deterministically. +6. As a flow author, I can allow a conversational step to use specific MCP tools + (typically the ones a skill declares), so the AI can act mid-conversation. +7. As an admin/auditor, I can see every MCP tool call attributed to a session, + flow, and user, so external actions are traceable. + +## 7. Pages / surfaces affected + +- `/admin/skills` — new: skill library (upload, list, view, version, archive). +- `/admin/mcp-servers` — new: register/list/test/disable remote MCP servers. +- Conversational step editor — new: skill picker (library multi-select + inline + upload) and an "allowed MCP tools" picker. +- Canvas — new `mcp` node type with its own editor (server + tool + field mapping), + styled alongside the existing `auto` node. +- `apps/api` tRPC: `skill.*` (list/get/create/update/archive/parse), + `mcpServer.*` (list/get/create/update/disable/test), `mcpDirectory.listTools`. +- Wiring: `apps/*/lib/container.ts` registers the new repositories, the MCP client + adapter, the directory, and the `mcp` node executor. + +## 8. Database changes + +| Table | Change | Prefix valid? | +| ----- | ------ | ------------- | +| `app_skills` | NEW — uploaded skills (name, description, frontmatter jsonb, body text, allowed_tools jsonb, version int, status) | yes (app_) | +| `admin_mcp_servers` | NEW — registered remote servers (label, transport, url, auth_kind, credential_ref, status) | yes (admin_) | +| `admin_mcp_tools` | NEW (optional cache) — discovered tool schemas per server (server_id, name, description, input_schema jsonb, last_synced_at) | yes (admin_) | + +Node-config additions (`ConversationalNodeConfig`, new `McpNodeConfig`) live in the +existing `flow_nodes.config` jsonb column — **no migration**. + +Credentials are **not** stored in plaintext: `credential_ref` points at the +existing system-secret mechanism; the value never leaves the adapter layer and is +never returned to the client. (See ADR-032 §Credentials — open question on the +secret store flagged in §12.) + +Tool-call usage/audit reuses the existing `audit-logger` and usage repository — +**no new ledger table** (consistent with ADR-020's reuse-first stance). + +## 9. Architectural decisions + +- **ADR-031** — Runtime skills as injected step instructions (library + inline). + Introduces the `Skill` entity, parsing rules, and the `` system-prompt + block; establishes that bundled scripts are ignored and only frontmatter + + body + `allowed-tools` are honoured. +- **ADR-032** — MCP integration: admin-registered remote servers, the `IMcpClient` + port + adapter, governance mirroring the n8n directory, and the tool-calling + architecture — a **separate agentic runner** in the agents adapter owns the + conversational tool loop, leaving `ILanguageModel` unchanged; the `mcp` node + uses the existing `INodeExecutor` path. +- Assumes ADR-013 (auto-node structured data) and ADR-020 (reuse of + `session_step_outputs` for end-of-step metadata). + +## 10. Acceptance criteria + +- [ ] A valid `SKILL.md` uploaded to the library is parsed into name, description, + body, and `allowedTools`; an invalid one returns a `DomainError`, not a throw. +- [ ] A conversational step with `skillRefs` renders each skill body inside a + `` block in the system prompt, above per-turn retrieved chunks. +- [ ] An inline skill on a step applies without a library row existing. +- [ ] An admin can create, list, disable, and connection-test an MCP server; + credentials are never returned by any read endpoint. +- [ ] `mcpDirectory.listTools` returns the tools of an enabled server (cached, + refreshable), and an empty/typed error for a disabled or unreachable one. +- [ ] An `mcp` node maps `requestFields` via `FieldValueSource`, calls the tool, + and persists `responseFields` to `session_step_outputs` via the existing path. +- [ ] A conversational step restricted to `allowedMcpToolRefs` can call only those + tools; a tool not in the list is refused before any external call. +- [ ] Every tool call writes an audit entry attributed to session/flow/user and a + usage record; failures are captured as a failed step/turn, never a throw + across a boundary. +- [ ] `packages/domain` and `packages/application` gain no MCP/AI-SDK dependency + (the MCP client lives in `packages/adapters`); `validate.sh` passes. + +## 11. Out of scope / future work + +- Local/stdio MCP transports. +- Per-flow or per-author server registration; org-scoped server visibility. +- MCP resources, prompts, elicitation, and sampling. +- A cross-tenant skill marketplace and skill dependency resolution. +- A dedicated tool-call ledger table (revisit only if audit/usage proves + insufficient, per ADR-020 reasoning). + +## 12. Risks / open questions + +- **Tool-calling changes the conversational turn shape.** Today a turn is a single + structured generation (`buildSystemPrompt` → `generateObject`/`streamObject`). + A tool loop must preserve the `{response, confidence, contextGathered}` contract + and confidence/auto-advance semantics. ADR-032 isolates this in a separate + agentic runner to keep the simple path unchanged — needs a build-time spike to + confirm streaming + structured output coexist with tool calls. +- **Credential storage.** Which secret store backs `credential_ref` — an encrypted + column, env-referenced secret, or `system_setting`? Pick during ADR-032 build; + must guarantee secrets never reach the client. +- **Untrusted tool output.** MCP tool results are external data entering the + prompt; need framing/guarding so results can't redirect the agent (treat like + ``). +- **Cost/latency.** Tool loops and tool discovery add latency and tokens; ensure + usage governance (ADR-026/027) counts tool turns. +- **Skill ↔ tool coupling.** A skill's `allowed-tools` names may not match any + registered MCP tool. Decide whether that's a soft warning (skill still injects) + or a hard block (ADR-031 leans soft: instructions inject; missing tools are + surfaced in the editor, not fatal at runtime). From 1e021b689fad447b3ee1e2a24d1740e761de25d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 09:48:59 +0000 Subject: [PATCH 02/14] feat: upload SKILL.md and attach skills to conversational steps (v1.52.0) Phase 1 of the Flow Skills & MCP PRD. Flow authors can upload an externally-authored SKILL.md to a library and attach skills to a conversational step; skill bodies are injected into the step's system prompt as a cache-stable block (ADR-031). - domain: Skill/ParsedSkill/ResolvedSkill entities, ISkillParser + ISkillRepository ports; ConversationalNodeConfig.skillRefs/inlineSkill; BuildSystemPromptInput.resolvedSkills - adapters: dependency-free SkillParser, app_skills table, DrizzleSkillRepository, prompt block - application: create/update/list/get/archive/restore skill use-cases + ResolveStepSkills - web: skill tRPC router, container wiring, prompt resolution at both call sites, /admin/skills library page, step-editor skill picker MCP/tool-calling (Phase 2) is out of scope; allowedTools is parsed and stored but not yet enforced. Version bump: MINOR (1.51.0 -> 1.52.0; new app_skills table). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01GtujrmX4ywLjDUF3j6X2Yh --- VERSION | 2 +- apps/web/e2e/phase-flow-skills.spec.ts | 49 ++++ .../app/(admin)/admin/flows/[id]/_content.tsx | 2 + .../src/app/(admin)/admin/skills/_content.tsx | 159 +++++++++++++ .../web/src/app/(admin)/admin/skills/page.tsx | 12 + .../app/api/chat/[sessionId]/stream/route.ts | 4 + .../[sessionId]/stream/turn-helpers.test.ts | 2 + .../chat/[sessionId]/stream/turn-helpers.ts | 4 + .../components/canvas/node-config-modal.tsx | 45 ++++ .../canvas/scheduled-node-config.test.ts | 1 + apps/web/src/components/sidebar.tsx | 2 + apps/web/src/lib/container.ts | 22 +- apps/web/src/server/router.ts | 2 + apps/web/src/server/routers/skill.ts | 82 +++++++ .../v1.52.0/_implementation-summary.md | 82 +++++++ .../v1.52.0/phase-flow-skills.phase.md | 46 ++++ package.json | 2 +- .../src/agents/flow-session-graph.test.ts | 26 +++ .../adapters/src/agents/flow-session-graph.ts | 16 +- packages/adapters/src/db/schema/app.ts | 29 ++- packages/adapters/src/index.ts | 1 + .../repositories/drizzle-skill-repository.ts | 137 ++++++++++++ packages/adapters/src/repositories/index.ts | 1 + packages/adapters/src/skills/index.ts | 1 + .../adapters/src/skills/skill-parser.test.ts | 96 ++++++++ packages/adapters/src/skills/skill-parser.ts | 128 +++++++++++ packages/application/src/use-cases/index.ts | 1 + .../application/src/use-cases/skill/index.ts | 1 + .../src/use-cases/skill/skill.test.ts | 211 ++++++++++++++++++ .../application/src/use-cases/skill/skill.ts | 116 ++++++++++ packages/domain/src/entities/flow-node.ts | 7 + packages/domain/src/entities/index.ts | 1 + packages/domain/src/entities/skill.ts | 53 +++++ packages/domain/src/ports/index.ts | 2 + packages/domain/src/ports/session-agent.ts | 4 + packages/domain/src/ports/skill-parser.ts | 8 + packages/domain/src/ports/skill-repository.ts | 17 ++ 37 files changed, 1368 insertions(+), 6 deletions(-) create mode 100644 apps/web/e2e/phase-flow-skills.spec.ts create mode 100644 apps/web/src/app/(admin)/admin/skills/_content.tsx create mode 100644 apps/web/src/app/(admin)/admin/skills/page.tsx create mode 100644 apps/web/src/server/routers/skill.ts create mode 100644 docs/development/implemented/v1.52.0/_implementation-summary.md create mode 100644 docs/development/implemented/v1.52.0/phase-flow-skills.phase.md create mode 100644 packages/adapters/src/repositories/drizzle-skill-repository.ts create mode 100644 packages/adapters/src/skills/index.ts create mode 100644 packages/adapters/src/skills/skill-parser.test.ts create mode 100644 packages/adapters/src/skills/skill-parser.ts create mode 100644 packages/application/src/use-cases/skill/index.ts create mode 100644 packages/application/src/use-cases/skill/skill.test.ts create mode 100644 packages/application/src/use-cases/skill/skill.ts create mode 100644 packages/domain/src/entities/skill.ts create mode 100644 packages/domain/src/ports/skill-parser.ts create mode 100644 packages/domain/src/ports/skill-repository.ts diff --git a/VERSION b/VERSION index ba0a7191..a63cb35e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.51.0 +1.52.0 diff --git a/apps/web/e2e/phase-flow-skills.spec.ts b/apps/web/e2e/phase-flow-skills.spec.ts new file mode 100644 index 00000000..40dca9ec --- /dev/null +++ b/apps/web/e2e/phase-flow-skills.spec.ts @@ -0,0 +1,49 @@ +import { expect, test } from "@playwright/test"; + +// E2E for Flow Skills — uploading a SKILL.md to the library and attaching it to a +// conversational step (PRD: flow-skills-and-mcp, ADR-031). +// +// Driven by the /e2e (Playwright MCP) skill against a running stack — it is +// excluded from the vitest unit run. Assumes an authenticated admin storageState +// (the same global setup the other admin specs rely on). + +const SKILL_MD = `--- +name: E2E Contract Reviewer +description: Flags unusual contract clauses +--- + +# Contract review + +Read the contract and flag unusual indemnity clauses.`; + +test.describe("flow skills", () => { + test("an admin can upload a SKILL.md and see it in the library", async ({ page }) => { + await page.goto("/admin/skills"); + + await page.getByLabel("SKILL.md").fill(SKILL_MD); + await page.getByRole("button", { name: /upload skill/i }).click(); + + // The parsed name appears in the library table. + await expect(page.getByText("E2E Contract Reviewer")).toBeVisible(); + await expect(page.getByText("Flags unusual contract clauses")).toBeVisible(); + }); + + test("an invalid SKILL.md surfaces a validation error and stores nothing", async ({ page }) => { + await page.goto("/admin/skills"); + + // Frontmatter with no name and no heading cannot be parsed into a skill. + await page.getByLabel("SKILL.md").fill("---\ndescription: no name\n---\n"); + await page.getByRole("button", { name: /upload skill/i }).click(); + + await expect(page.getByText(/must declare a name/i)).toBeVisible(); + }); + + test("an uploaded skill can be archived from the library", async ({ page }) => { + await page.goto("/admin/skills"); + + const row = page.getByRole("row", { name: /E2E Contract Reviewer/i }).first(); + await row.getByRole("button", { name: /archive/i }).click(); + + await expect(row.getByRole("button", { name: /restore/i })).toBeVisible(); + }); +}); diff --git a/apps/web/src/app/(admin)/admin/flows/[id]/_content.tsx b/apps/web/src/app/(admin)/admin/flows/[id]/_content.tsx index 63127c20..90c3b217 100644 --- a/apps/web/src/app/(admin)/admin/flows/[id]/_content.tsx +++ b/apps/web/src/app/(admin)/admin/flows/[id]/_content.tsx @@ -378,6 +378,7 @@ function CanvasInner({ flowId }: { flowId: string }) { documentTemplateStructuredContent: hasTemplate ? (existingNodeConfig.documentTemplateStructuredContent ?? null) : null, allowManualEdit: values.allowManualEdit, requireConfirmation: values.neverDone ? false : values.requireConfirmation, + skillRefs: values.skillRefs, notifyOnComplete: values.notifyOnComplete, }; }; @@ -611,6 +612,7 @@ function CanvasInner({ flowId }: { flowId: string }) { documentTemplateContent: (editingConfig.documentTemplateContent as string | null) ?? null, allowManualEdit: (editingConfig.allowManualEdit as boolean | undefined) ?? true, requireConfirmation: Boolean(editingConfig.requireConfirmation), + skillRefs: (editingConfig.skillRefs as string[] | undefined) ?? [], instruction: (editingConfig.instruction as string | null) ?? "", executor: (editingConfig.executor as "n8n" | "mock" | undefined) ?? "n8n", workflowId: (editingConfig.workflowId as string | null) ?? null, diff --git a/apps/web/src/app/(admin)/admin/skills/_content.tsx b/apps/web/src/app/(admin)/admin/skills/_content.tsx new file mode 100644 index 00000000..b0a13b13 --- /dev/null +++ b/apps/web/src/app/(admin)/admin/skills/_content.tsx @@ -0,0 +1,159 @@ +"use client"; + +import { useState } from "react"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Label } from "@/components/ui/label"; +import { Textarea } from "@/components/ui/textarea"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; +import { trpc } from "@/trpc/client"; + +const EXAMPLE_SKILL = `--- +name: Contract Reviewer +description: Reviews procurement contracts for risk +--- + +# Contract review + +Read the contract carefully and flag unusual indemnity, liability, or +termination clauses. Ask the user about anything ambiguous.`; + +export function AdminSkillsContent() { + const utils = trpc.useUtils(); + const skillsQuery = trpc.skill.list.useQuery({ includeArchived: true }); + + const [raw, setRaw] = useState(""); + const [error, setError] = useState(null); + + const create = trpc.skill.create.useMutation({ + onSuccess: () => { + setRaw(""); + setError(null); + void utils.skill.list.invalidate(); + }, + onError: (cause) => setError(cause.message), + }); + const archive = trpc.skill.archive.useMutation({ + onSuccess: () => void utils.skill.list.invalidate(), + }); + const restore = trpc.skill.restore.useMutation({ + onSuccess: () => void utils.skill.list.invalidate(), + }); + + const submit = () => { + if (!raw.trim()) return; + create.mutate({ raw }); + }; + + return ( +
+
+ + + Upload a skill + + +

+ Paste a SKILL.md (YAML frontmatter + + markdown body). Uploaded skills can be attached to any conversational + step to steer the AI's behaviour. +

+
+ +