diff --git a/app/src/hooks/useFlowChanged.ts b/app/src/hooks/useFlowChanged.ts new file mode 100644 index 0000000000..d9111704b7 --- /dev/null +++ b/app/src/hooks/useFlowChanged.ts @@ -0,0 +1,85 @@ +/** + * useFlowChanged (Phase 3 — flow-mutation observability, audit F6) + * ---------------------------------------------------------------- + * + * Subscribes to the core's flow-mutation feed so an open Workflows list or + * canvas reacts when a flow's definition changes underneath it — most + * importantly, so an agent `save_workflow` becomes visible instead of the user + * silently working against (and later clobbering) stale state. + * + * The backend publishes `DomainEvent::FlowChanged` on create/update/delete/ + * enable; the core socket bridge (`src/core/socketio.rs`) re-emits it as both + * `flow:changed` and `flow_changed` (colon + underscore aliases) with the + * payload `{ flow_id, kind, actor }`. + * + * Best-effort (broadcast bridges drop on lag); the UI's own refetch-on-focus + * remains the backstop, exactly as {@link useFlowRunProgress} pairs with the 2s + * poller. Pass `flowId` to filter to a single flow (canvas), or omit it to + * receive every change (list). + */ +import debug from 'debug'; +import { useCallback, useEffect } from 'react'; + +import { socketService } from '../services/socketService'; + +const log = debug('flows:changed'); + +/** Socket event aliases the core bridge emits (colon + underscore forms). */ +const EVENT_COLON = 'flow:changed'; +const EVENT_UNDERSCORE = 'flow_changed'; + +/** What happened to the flow. */ +export type FlowChangeKind = 'created' | 'updated' | 'deleted' | 'enabled_changed' | (string & {}); + +/** Payload of a `flow:changed` socket event (`DomainEvent::FlowChanged`). */ +export interface FlowChangedEvent { + flow_id: string; + kind: FlowChangeKind; + /** Coarse hint: `agent` | `user` | `system`. */ + actor: string; +} + +function parsePayload(data: unknown): FlowChangedEvent | null { + if (!data || typeof data !== 'object') return null; + const obj = data as Record; + if (typeof obj.flow_id !== 'string' || typeof obj.kind !== 'string') return null; + return { + flow_id: obj.flow_id, + kind: obj.kind, + actor: typeof obj.actor === 'string' ? obj.actor : 'system', + }; +} + +/** + * Invokes `onChange` whenever a flow changes. When `flowId` is provided, only + * changes to that flow are delivered; otherwise every change is. + */ +export function useFlowChanged( + onChange: (event: FlowChangedEvent) => void, + flowId?: string | null +): void { + const handle = useCallback( + (data: unknown) => { + const payload = parsePayload(data); + if (!payload) { + log('changed: dropped — invalid payload %o', data); + return; + } + if (flowId && payload.flow_id !== flowId) return; + log('changed: flow=%s kind=%s actor=%s', payload.flow_id, payload.kind, payload.actor); + onChange(payload); + }, + [onChange, flowId] + ); + + useEffect(() => { + socketService.on(EVENT_COLON, handle); + socketService.on(EVENT_UNDERSCORE, handle); + return () => { + socketService.off(EVENT_COLON, handle); + socketService.off(EVENT_UNDERSCORE, handle); + }; + }, [handle]); +} + +export default useFlowChanged; diff --git a/app/src/pages/FlowsPage.tsx b/app/src/pages/FlowsPage.tsx index 86717a1e21..047cfd6fe8 100644 --- a/app/src/pages/FlowsPage.tsx +++ b/app/src/pages/FlowsPage.tsx @@ -31,6 +31,7 @@ import BetaBanner from '../components/ui/BetaBanner'; import Button from '../components/ui/Button'; import { CenteredLoadingState, ErrorBanner } from '../components/ui/LoadingState'; import { ModalShell } from '../components/ui/ModalShell'; +import { useFlowChanged } from '../hooks/useFlowChanged'; import { FLOW_CANVAS_DRAFT_ROUTE, type FlowCanvasDraftState } from '../lib/flows/canvasDraft'; import { downloadFlowGraph } from '../lib/flows/exportFlow'; import { type FlowTemplate, templateNameKey } from '../lib/flows/templates'; @@ -105,6 +106,17 @@ export default function FlowsPage() { void loadFlows(); }, [loadFlows]); + // Refetch (silently, no spinner) whenever any flow changes underneath us — + // e.g. an agent save_workflow — so the list never shows stale state (F6). + useFlowChanged( + useCallback(() => { + log('flow:changed — refetching list'); + void listFlows() + .then(setFlows) + .catch(err => log('refetch failed: %o', err)); + }, []) + ); + const handleToggle = useCallback( async (flow: Flow) => { if (busyKey) return; diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index 31bc2e8d03..13539d52ea 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -142,10 +142,45 @@ export interface Flow { */ export interface FlowValidation { valid: boolean; + /** All structural errors in one pass (multi-error validation), not just the first. */ errors: string[]; + /** Structured, per-node counterpart to {@link errors} (additive). */ + error_details?: FlowValidationErrorDetail[]; warnings: string[]; } +/** One structured validation error (`src/openhuman/flows/types.rs::FlowValidationError`). */ +export interface FlowValidationErrorDetail { + /** Stable machine-readable code, e.g. `missing_trigger`, `unknown_node`. */ + code: string; + /** Human-readable message (identical to the matching {@link FlowValidation.errors} entry). */ + message: string; + /** The node this error anchors to, when node-specific. */ + node_id?: string; + /** The offending config field, when field-specific (reserved). */ + field?: string; +} + +/** Where a {@link FlowDraft} originated (`src/openhuman/flows/types.rs::DraftOrigin`). */ +export type DraftOrigin = 'chat' | 'canvas' | 'import'; + +/** + * A core-managed, durable workflow draft (`src/openhuman/flows/types.rs::FlowDraft`) + * — the shared working copy the agent tools and the canvas both read/write by + * id across turns and reloads. Never live; promote runs the normal save gates. + */ +export interface FlowDraft { + id: string; + /** The saved flow this draft edits, if any (promote → update vs create). */ + flow_id?: string; + name: string; + /** Work-in-progress graph (may be incomplete/invalid) — opaque to this client. */ + graph: unknown; + origin: DraftOrigin; + created_at: string; + updated_at: string; +} + /** * Source format for {@link importFlow}. `native` is a tinyflows `WorkflowGraph` * JSON; `n8n` is an n8n workflow export (mapped best-effort host-side); `auto` @@ -185,6 +220,54 @@ export interface FlowUpdate { name?: string; graph?: unknown; requireApproval?: boolean; + /** + * Optimistic-concurrency token: the flow's `updated_at` as last observed. If + * the flow changed since, the update is refused with a {@link FlowVersionConflict} + * error instead of clobbering. Omit for last-write-wins. + */ + expectedVersion?: string; + /** Run the agent author hard-gates before persisting (F3). */ + strict?: boolean; +} + +/** A revision snapshot (`src/openhuman/flows/types.rs::FlowRevision`). */ +export interface FlowRevision { + id: string; + flow_id: string; + graph: unknown; + name: string; + require_approval: boolean; + created_at: string; +} + +/** + * The structured error `flows_update` returns on an optimistic-concurrency + * conflict (encoded in the RPC error message as JSON). Detect it by parsing a + * caught update error — see {@link parseFlowVersionConflict}. + */ +export interface FlowVersionConflict { + code: 'version_conflict'; + message: string; + current: Flow; +} + +/** + * If `err` is a `flows_update` version-conflict error, returns the structured + * conflict (with the current server flow) so the UI can offer reload/diff; + * otherwise `null`. + */ +export function parseFlowVersionConflict(err: unknown): FlowVersionConflict | null { + const message = err instanceof Error ? err.message : typeof err === 'string' ? err : ''; + if (!message.includes('version_conflict')) return null; + try { + const parsed = JSON.parse(message) as Partial; + if (parsed?.code === 'version_conflict' && parsed.current) { + return parsed as FlowVersionConflict; + } + } catch { + // Not a JSON conflict payload. + } + return null; } /** Lifecycle status of a {@link FlowSuggestion} (`src/openhuman/flows/types.rs::SuggestionStatus`). */ @@ -464,12 +547,42 @@ export async function updateFlow(id: string, update: FlowUpdate): Promise if (update.name !== undefined) params.name = update.name; if (update.graph !== undefined) params.graph = update.graph; if (update.requireApproval !== undefined) params.require_approval = update.requireApproval; + if (update.expectedVersion !== undefined) params.expected_version = update.expectedVersion; + if (update.strict !== undefined) params.strict = update.strict; const response = await callCoreRpc({ method: 'openhuman.flows_update', params }); const flow = unwrapCliEnvelope(response); log('updateFlow: response id=%s name=%s', flow.id, flow.name); return flow; } +/** List a flow's revision history via `openhuman.flows_get_history` (newest first). */ +export async function getFlowHistory(id: string, limit?: number): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_get_history', + params: { id, limit }, + }); + const result = unwrapCliEnvelope<{ revisions: FlowRevision[] }>(response); + return result.revisions ?? []; +} + +/** + * Roll a flow back to a prior revision via `openhuman.flows_rollback` (restores + * that revision's graph through the normal update path — itself snapshotted, so + * a rollback is undoable). Honours optimistic concurrency via `expectedVersion`. + */ +export async function rollbackFlow( + id: string, + revisionId: string, + expectedVersion?: string +): Promise { + log('rollbackFlow: request id=%s revision=%s', id, revisionId); + const response = await callCoreRpc({ + method: 'openhuman.flows_rollback', + params: { id, revision_id: revisionId, expected_version: expectedVersion }, + }); + return unwrapCliEnvelope(response); +} + /** * Validate a candidate `WorkflowGraph` via `openhuman.flows_validate`. Pure and * cheap server-side (no config load), so it's safe to call on a debounce while @@ -532,6 +645,139 @@ export async function importFlow( return result; } +// ── Catalog RPCs for the UI (Phase 5, item 16) ─────────────────────────────── + +/** One search hit from `openhuman.flows_search_tool_catalog` (secret-free). */ +export interface ToolCatalogEntry { + slug: string; + toolkit: string; + description?: string | null; + required_args?: string[]; + output_fields?: string[]; + primary_array_path?: string | null; + /** Curated/featured toolkits rank first. */ + featured?: boolean; +} + +/** Search the live Composio tool catalog via `openhuman.flows_search_tool_catalog`. */ +export async function searchToolCatalog( + query: string, + opts?: { toolkit?: string; limit?: number } +): Promise { + log('searchToolCatalog: query=%s toolkit=%s', query, opts?.toolkit ?? '(all)'); + const response = await callCoreRpc({ + method: 'openhuman.flows_search_tool_catalog', + params: { query, toolkit: opts?.toolkit, limit: opts?.limit }, + timeoutMs: 60_000, + }); + const result = unwrapCliEnvelope<{ tools: ToolCatalogEntry[] }>(response); + return result.tools ?? []; +} + +/** A toolkit a graph needs, with its connected state (Phase 5, item 18). */ +export interface RequiredConnection { + toolkit: string; + status: 'connected' | 'missing'; +} + +/** + * Compute which Composio toolkits a candidate graph needs and whether each is + * connected, via `openhuman.flows_required_connections` — the data behind the + * "Connect " CTAs. Also surfaced on the workflow_proposal payload. + */ +export async function requiredConnections(graph: unknown): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_required_connections', + params: { graph }, + }); + const result = unwrapCliEnvelope<{ required_connections: RequiredConnection[] }>(response); + return result.required_connections ?? []; +} + +/** Fetch one action's full contract via `openhuman.flows_get_tool_contract`. */ +export async function getToolContract(slug: string): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_get_tool_contract', + params: { slug }, + timeoutMs: 60_000, + }); + const result = unwrapCliEnvelope<{ contract: unknown }>(response); + return result.contract; +} + +// ── Core-managed drafts (F5) ───────────────────────────────────────────────── + +/** Create a durable draft via `openhuman.flows_draft_create`. */ +export async function createDraft(params: { + name: string; + graph: unknown; + flowId?: string; + origin?: DraftOrigin; +}): Promise { + log('createDraft: request origin=%s', params.origin ?? 'canvas'); + const response = await callCoreRpc({ + method: 'openhuman.flows_draft_create', + params: { + name: params.name, + graph: params.graph, + flow_id: params.flowId, + origin: params.origin, + }, + }); + return unwrapCliEnvelope(response); +} + +/** Fetch a draft by id via `openhuman.flows_draft_get`. */ +export async function getDraft(id: string): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_draft_get', + params: { id }, + }); + return unwrapCliEnvelope(response); +} + +/** Patch a draft's name/graph/flow_id via `openhuman.flows_draft_update`. */ +export async function updateDraft( + id: string, + patch: { name?: string; graph?: unknown; flowId?: string } +): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_draft_update', + params: { id, name: patch.name, graph: patch.graph, flow_id: patch.flowId }, + }); + return unwrapCliEnvelope(response); +} + +/** List all drafts (newest-updated first) via `openhuman.flows_draft_list`. */ +export async function listDrafts(): Promise { + const response = await callCoreRpc({ method: 'openhuman.flows_draft_list', params: {} }); + const result = unwrapCliEnvelope<{ drafts: FlowDraft[] }>(response); + return result.drafts ?? []; +} + +/** Delete a draft via `openhuman.flows_draft_delete`. */ +export async function deleteDraft(id: string): Promise { + const response = await callCoreRpc({ + method: 'openhuman.flows_draft_delete', + params: { id }, + }); + const result = unwrapCliEnvelope<{ id: string; deleted: boolean }>(response); + return result.deleted; +} + +/** + * Promote a draft into a saved flow via `openhuman.flows_draft_promote` (runs + * the normal create/update gates, then removes the draft). Returns the Flow. + */ +export async function promoteDraft(id: string, requireApproval?: boolean): Promise { + log('promoteDraft: request id=%s', id); + const response = await callCoreRpc({ + method: 'openhuman.flows_draft_promote', + params: { id, require_approval: requireApproval }, + }); + return unwrapCliEnvelope(response); +} + /** * `openhuman.flows_discover` runs the read-only Flow Scout agent, which reasons * over the user's memory/threads/connections/flows and can take up to ~300s diff --git a/docs/flows-agent-friendliness-audit.md b/docs/flows-agent-friendliness-audit.md new file mode 100644 index 0000000000..710c667a4a --- /dev/null +++ b/docs/flows-agent-friendliness-audit.md @@ -0,0 +1,290 @@ +# Flows: Agent-Friendliness Audit & Improvement Plan + +**Date:** 2026-07-15 · **Scope:** how well the Workflows (Flows) product supports an AI agent +creating, editing, saving, testing, and debugging automations — across the Rust core +(`src/openhuman/flows/`, `tinyflows` engine seam), the agent tool belt +(`flows/tools.rs`, `flows/builder_tools.rs`), and the frontend canvas +(`app/src/pages/FlowCanvasPage.tsx` and friends). + +--- + +## 1. Current architecture (baseline) + +- A flow is a typed JSON node graph (`tinyflows::model::WorkflowGraph` — nodes, edges, 12 + `NodeKind`s, jq `=`-expressions for bindings), persisted in SQLite + (`{workspace}/flows/flows.db`, `flow_definitions.graph_json`). +- 21 RPC controllers under `openhuman.flows_*` (`src/openhuman/flows/schemas.rs`). +- Agent surface: `propose_workflow` / `revise_workflow` (validate-only, never persist), + `save_workflow` (update existing flows only), `dry_run_workflow` (mock capabilities), + `run_flow` (saved flows, real effects), plus read tools (`list_flows`, `get_flow`, + `get_flow_run`, `list_flow_connections`, `search_tool_catalog`, `get_tool_contract`, + `get_tool_output_sample`, `list_agent_profiles`). +- Human-in-the-loop invariant is enforced **structurally**: the agent has no create tool; + proposals render as `WorkflowProposalCard` / canvas diff previews, and only the user's + explicit Save persists. `save_workflow` never touches `enabled` / `require_approval`. +- Frontend: React Flow canvas, explicit Save only (no autosave — a saved+enabled flow is + live), client-side-only drafts (`/flows/draft` graph rides in router `location.state`), + copilot panel driving `flows_build`. + +Strengths worth preserving: the validate-gates return model-consumable, per-node, +"fix-and-retry" errors; the propose→accept→save loop gives real human oversight; +`get_tool_contract` / `get_tool_output_sample` are exactly the right shape of +machine-readable introspection. + +--- + +## 2. Audit findings — where agents get hurt + +### F1. Whole-graph blobs are the only edit unit (highest friction) +Every mutating/validating tool (`propose_workflow`, `revise_workflow`, `save_workflow`, +`dry_run_workflow`) requires the **entire** `WorkflowGraph` per call +(`flows/builder_tools.rs:114-122`, `:1712-1720`). There is no add-node / patch-node-config / +rewire-edge operation, at either the tool or RPC layer (`flows_update`'s `graph` param is +"Replacement WorkflowGraph", `schemas.rs:513`). For a 20-node flow, one config tweak means +re-emitting the whole graph — token-heavy, slow, and the #1 source of accidental +regressions (dropped nodes, mangled edges). `revise_workflow`'s "NOT a regeneration from +scratch" rule is advisory prose, not enforced. + +### F2. The flow DSL has no queryable schema +The 12 node kinds, per-kind config shapes, port rules, and `.item` / `.item.json` envelope +semantics live only in prose: the 618-line `workflow_builder` prompt +(`flows/agents/workflow_builder/prompt.md`) and `propose_workflow`'s giant description +string (`flows/tools.rs:52-76`). `parameters_schema` describes node `config` as +"Kind-specific configuration; see tool description". Contrast with Composio actions, where +`get_tool_contract` returns real JSON schemas. Any agent outside the workflow_builder +persona (or a future model with a trimmed prompt) is flying blind, and prompt/code drift +is unchecked. + +### F3. Validation iterates one error at a time, and paths diverge +`FlowValidation.errors` carries **at most one** error — tinyflows validation stops at the +first structural failure (`flows/types.rs:53-55`), so a graph with five problems costs +five round-trips. Several paths surface raw `.map_err(|e| e.to_string())` strings +(`ops.rs:89-92`, `:2204`). Additionally, the agent-tool save path layers extra hard gates +(binding resolvability, tool contracts, required-arg resolvability) that `flows_update` +itself doesn't run (`builder_tools.rs:1765-1770`) — so agent saves and UI saves are +validated differently, and a UI-saved flow can fail gates the agent is required to pass. + +### F4. Capability asymmetries — RPC ops with no agent tool +| Operation | RPC | Agent tool | Consequence | +|---|---|---|---| +| Create flow | `flows_create` | none (by design) | OK as a safety choice, but there's no gated alternative for "create disabled draft" either | +| Enable/disable | `flows_set_enabled` | none | orchestrator prompt even references it; agent can only ask the user | +| Delete / duplicate | `flows_delete` / `flows_duplicate` | none | agent told to clone via get_flow→revise | +| List runs | `flows_list_runs` | none | agent has `get_flow_run` but must be handed a `run_id` externally — breaks the self-debug loop | +| Resume / cancel run | `flows_resume` / `flows_cancel_run` | none | agent can't progress a run paused on approval, or stop a runaway one | +| Standalone validate | `flows_validate` | none | validation only reachable bundled inside propose/revise/save | +| Import (n8n) | `flows_import` | none | agent can't help migrate automations | + +### F5. No server-side draft; agent and UI can't share working state +Drafts exist only client-side: the `/flows/draft` graph lives in router `location.state` +(dropped on reload), and agent proposals live in the chat/canvas proposal card. There is +no durable draft the agent can iterate on across turns, no way for a chat-initiated build +and the canvas to reference the same in-progress graph by id, and a crash/reload loses +everything. This is also *why* F1 exists — with no server-side working copy, every tool +call must carry the full graph. + +### F6. Last-write-wins everywhere; the UI is blind to agent edits +`update_flow_graph` has no version/etag/`updated_at` precondition (`store.rs:329-343`); +`Flow` has no version field. There are **no** socket/domain events for flow +create/update/delete/enable — only run-progress and approval events +(`useFlowRunProgress.ts`; `useFlowRunPoller.ts` notes "the flows engine emits no socket +events"). So: an agent `save_workflow` while the user has the canvas open is invisible, +and the user's next Save silently clobbers it (and vice versa). No revision history or +rollback exists, which is precisely what makes granting the agent more write power scary. + +### F7. The agent cannot actually test what it built +`dry_run_workflow` runs against deterministic **mock** capabilities only — the prompt +spends ~200 lines (`prompt.md:540-605`) teaching the agent to reason around sandbox +artifacts. Real execution (`run_flow`) requires the flow to already be **saved** plus an +explicit user "yes" — a draft cannot be end-to-end tested at all. And the read-only +autonomy tier blocks even the side-effect-free mock dry-run +(`builder_tools.rs:1218-1229`), so a read-only agent cannot self-verify its own proposal. + +### F8. "Workflow" means four different things +`flows/` (the product), the legacy `skills` tools literally named +`list_workflows`/`run_workflow`/`create_workflow` (SKILL.md system), `rhai_workflows` (an +in-turn scripting cell), and cron jobs. The orchestrator prompt has to explicitly warn +against crossing them (`agent_registry/agents/orchestrator/prompt.rs:105-110`), and +`run_flow` was named to dodge a collision with the legacy `run_workflow`. This taxes every +agent turn and every new contributor. + +--- + +## 3. Improvement plan + +Ordering principle: make edits cheap and correct first (F1–F3), then make shared state +durable (F5), then make concurrent writes safe + observable (F6) — because F6's +safety rails are the prerequisite for widening agent write capabilities (F4, F7). + +### Phase 1 — Structured editing & introspection (F1, F2, F3) + +1. **Graph patch operations.** New core op `flows::apply_graph_ops(base_graph, ops[]) -> + Result` with ops like `add_node`, `update_node_config` + (JSON-merge-patch on `config`), `rename_node`, `remove_node`, `add_edge`, + `remove_edge`, `set_node_position`. Expose as a new agent tool `edit_workflow + { flow_id | graph, ops[] }` that applies ops, runs the full validate+gate stack, and + returns the updated graph + proposal payload (same contract as `revise_workflow`). + Whole-graph tools stay for initial generation; iteration switches to ops. +2. **Queryable DSL schema.** Move per-node-kind config shapes into typed Rust schemas + (source of truth), and expose `get_node_kind_contract { kind }` / + `list_node_kinds` agent tools mirroring `get_tool_contract`. Generate the prompt.md + node-kind section and `propose_workflow`'s description from the same source + (docs-drift-style check) so prose can never diverge from code. +3. **Multi-error validation.** Change `FlowValidation.errors` to collect all + structural errors (tinyflows `validate` change: accumulate instead of first-error), + with structured entries `{node_id, field, code, message}`. Replace bare + `.to_string()` error paths in `ops.rs` with the structured form. +4. **Unify validation planes.** Run the host hard gates (binding/contract/required-arg) + inside `flows_update`/`flows_create` too — or, if UI permissiveness must stay, + downgrade them to blocking-for-agents via an explicit `strict: bool` param rather + than a divergent code path. Also give the agent a standalone `validate_workflow` tool + (thin wrapper over `flows_validate` + gates) so it can check without proposing. + +### Phase 2 — Core-managed local drafts (F5, and the real fix for F1's token cost) + +5. **Draft entity — local JSON files, not a new table (for now).** Drafts are stored + as plain JSON files on disk, managed by the core: + `{workspace_dir}/flows/drafts/.json`, each holding + `{id, flow_id?, name, graph, origin: chat|canvas|import, created_at, updated_at}`. + No SQLite schema/migration. Same thin RPC surface on top: + `flows_draft_create/get/update/list/delete/promote` — `promote` runs the existing + create/update path (same gates, same forced `require_approval` floor) and removes + the file. Key constraint this preserves: drafts must be readable/writable by **both** + the agent tools (Rust core) and the canvas — which rules out frontend-only + `localStorage`. File-based storage keeps drafts trivially inspectable and deletable, + and can be migrated into a `flow_drafts` table later if drafts ever need querying, + retention caps, or cross-device sync; the RPC contract stays identical either way. +6. **Agent tools on drafts.** `propose_workflow`/`revise_workflow`/`edit_workflow`/ + `dry_run_workflow` gain a `draft_id` mode: the graph lives in the core-managed + draft file; tool calls + carry only ops/instructions and get back diffs + validation. Cuts per-turn tokens + dramatically and survives reloads/session hops. +7. **Frontend adoption.** `/flows/draft/:draftId` loads from core instead of + `location.state`; import and proposal-accept create drafts; the unsaved-changes + guard becomes "draft is saved, flow is not yet updated". Copilot and canvas now + share one working copy by id. + +### Phase 3 — Concurrency safety & observability (F6) + +8. **Optimistic concurrency.** Add `version: i64` (or reuse `updated_at` as etag) to + `Flow`; `flows_update` and `save_workflow` take `expected_version` and return a + structured conflict error (with the current server graph) instead of clobbering. + UI surfaces "flow changed since you opened it" with a reload/diff option. +9. **Flow mutation events.** Publish `DomainEvent::FlowChanged{flow_id, kind: + created|updated|deleted|enabled_changed, actor}` on every mutation; bridge to a + `flow:changed` socket event. FlowsPage refetches on it; FlowCanvasPage shows a + banner when its open flow changes underneath (agent edits become visible in real + time instead of silently). +10. **Revision history + rollback.** `flow_revisions` table capturing the prior + `graph_json` on every update (capped, e.g. last 20), plus `flows_rollback` RPC and + a `get_flow_history` agent tool. This is the safety rail that justifies Phase 4. + +### Phase 4 — Widen agent capabilities behind existing gates (F4, F7) + +11. **Debug loop tools:** `list_flow_runs { flow_id, limit }`, `resume_flow_run` + (Execute + approval-gated), `cancel_flow_run` (Write). The agent can then find a + failing run, diagnose it via `get_flow_run`, patch via `edit_workflow`, and verify. +12. **Gated create:** `create_workflow` agent tool → `flows_create` with hard-coded + `enabled: false` + the existing forced `require_approval` floor, + `PermissionLevel::Write` (approval gate). Enable/disable stays human-only + (`flows_set_enabled` deliberately remains toolless). Add `duplicate_flow` + (creates disabled copy) for the clone-then-edit pattern. +13. **Testability:** (a) allow `dry_run_workflow` on the read-only tier — it is + mock-only and side-effect-free; (b) add a `test_run` mode that executes a draft + with *real read-scope* capabilities only (generalizing `get_tool_output_sample`'s + read-only-real-call precedent), refusing Write/Admin-class nodes; (c) let + `run_flow` accept `draft_id` once drafts exist, still behind explicit user + confirmation. + +### Phase 5 — Builder UX: connector onboarding, tool discovery, chat separation + +Today the builder assumes the user already knows their Composio landscape: the +`tool_call` config drawer makes them hand-type a toolkit slug before the per-toolkit +action dropdown appears (`app/src/components/flows/canvas/nodeConfig/composioFields.tsx`), +the connection selector only lists connections that already exist +(`flows_list_connections` → `nodeConfigFields.tsx:559`), and full-catalog search +(`search_tool_catalog` / `get_tool_contract`) is agent-tool-only with no RPC the UI can +call. When a graph needs an unconnected toolkit, the tool-contract gate just errors — +nothing walks the user through connecting it. + +16. **Catalog RPCs for the UI.** Expose the existing agent-tool logic as controllers: + `flows_search_tool_catalog { query, toolkit? }` and `flows_get_tool_contract + { slug }` (thin wrappers over the same core code as `search_tool_catalog` / + `get_tool_contract`, secret-free). One implementation, two consumers. +17. **In-canvas tool browser.** Replace the hand-typed toolkit slug with a searchable + catalog picker in the `tool_call` config drawer (and a "browse tools" entry in the + NodePalette): search across all toolkits, show description / required args / + connected-state per result, select → fills toolkit+action and shows the contract's + arg schema. Connected toolkits rank first; unconnected results carry a Connect + badge. +18. **Required-connections surfacing.** Compute `required_connections: + [{toolkit, connection_ref?, status: connected|missing}]` for any graph (derivable + from the existing tool-contract gate) and include it in `flows_validate` output, + the `workflow_proposal` payload, and `flows_get`. The proposal card and canvas + validation banner render missing ones as explicit "Connect " CTAs + deep-linking into the existing `/connections` connect flow, instead of a bare + gate error. On return (connection created), re-validate automatically. +19. **Agent-side guidance.** Teach the workflow_builder prompt to treat a missing + connection as a first-class outcome: propose the flow anyway, enumerate the + required connections in the proposal summary, and tell the user which toolkits + need connecting (the card's CTAs do the rest). Optionally add a read-only + `list_connectable_toolkits` tool (catalog toolkits + connected flag) so the agent + can steer toolkit choice toward what's already connected. +20. **Tag workflow chats separately from general chat.** Workflow-copilot + conversations are ordinary core threads today, distinguishable only by a + client-side `localStorage` mapping (`workflowCopilotThreads.ts` — user-scoped + `copilot-thread:` keys). Instead, tag the thread server-side at creation: + add a thread `kind` (e.g. `workflow_copilot`, with the associated + `flow_id`/`draft_id` in thread metadata) in the `threads` domain, set it whenever + `flows_build` / the copilot panel spawns a thread, and let thread-list queries + filter by kind. The general `/chat` thread list excludes `workflow_copilot` + threads; the flows UI lists a flow's builder threads from the core by tag. This + also lets the copilot resolve "which thread belongs to this flow" server-side, + demoting the fragile `localStorage` mapping to a cache (or deleting it). + +### Phase 6 — Naming & prompt hygiene (F8) + +21. Rename the legacy skills tools (`list_workflows`→`list_skills`, + `run_workflow`→`run_skill`, etc., with deprecation aliases for one release) so + "workflow" unambiguously means Flows in the agent's tool belt; delete the + orchestrator-prompt disambiguation paragraph once done. +22. Shrink the workflow_builder prompt by replacing the node-kind reference and mock + behavior table with pointers to the Phase 1 introspection tools (generated docs + keep parity). + +### Delivery: one PR, phased commits + +All six phases ship together as **a single PR**, not as separate PRs per item. The +phases above define the *internal build order and commit structure* of that PR — each +numbered item lands as one or more focused commits, in phase order, so the branch is +reviewable commit-by-commit and bisectable — but the feature is reviewed, tested, and +merged as one unit. Rationale: the pieces are interdependent (drafts remove the need for +whole-graph tools, versioning/events are the safety rails that justify the wider tool +belt, the prompt shrink depends on the introspection tools existing), and shipping them +piecemeal would leave the agent surface in inconsistent intermediate states across +releases. + +| Phase | Depends on | Rough size | Risk | +|---|---|---|---| +| 1 (patch ops, schema tools, multi-error) | — | M–L (tinyflows crate change for multi-error) | Low | +| 2 (core-managed local drafts) | 1 helps | S–M | Low — additive files/RPC, no DB migration | +| 3 (versioning, events, history) | — (parallel to 2) | M | Medium — touches UI save path | +| 4 (new agent tools) | 3 (safety rails) | S–M each | Medium — permission review each | +| 5 (connector onboarding, tool browser, chat tagging) | 1 (contract gate reuse) | M | Low–Medium — UI + additive RPCs + thread `kind` | +| 6 (renames) | — | S | Low (needs deprecation window) | + +Build within the branch in phase order (1 → 2 → 3 → 4 → 5 → 6): items 1–3 (Phase 1) deliver +the biggest agent-experience win and require no changes to the human-in-the-loop model; +Phase 3's rails must be in place before Phase 4's write tools are enabled. The PR +description should map commits to plan items so reviewers can follow the same structure. + +--- + +## 4. Non-goals / explicitly preserved invariants + +- **No autosave** on the canvas — a saved+enabled flow is live; the explicit Save gate stays. +- **Enable/disable remains human-only**; agent-created flows are born disabled. +- The forced `require_approval = true` floor for side-effect graphs stays uncloseable + by the agent. +- `propose → user accepts → save` remains the default chat flow; new write tools are + additive and approval-gated, not a replacement for proposals. diff --git a/src/core/event_bus/events.rs b/src/core/event_bus/events.rs index dbc5b643e5..355153dead 100644 --- a/src/core/event_bus/events.rs +++ b/src/core/event_bus/events.rs @@ -463,6 +463,23 @@ pub enum DomainEvent { status: String, }, + /// A saved flow's definition changed (created / updated / deleted / + /// enable-toggled). Bridged to a `flow:changed` socket event so an open + /// Workflows list or canvas refetches instead of silently showing stale + /// state — most importantly, so an agent `save_workflow` becomes visible in + /// a canvas the user has open (audit F6). Best-effort (broadcast bridges + /// drop on lag); the UI's own refetch-on-focus remains the backstop. + FlowChanged { + /// The affected flow's id. + flow_id: String, + /// What happened: `"created"` | `"updated"` | `"deleted"` | + /// `"enabled_changed"`. + kind: String, + /// Who made the change: `"agent"` | `"user"` | `"system"` — a coarse + /// hint for the UI banner ("an assistant edited this flow"). + actor: String, + }, + // ── Skills ────────────────────────────────────────────────────────── /// A skill was loaded into the runtime. WorkflowLoaded { skill_id: String, runtime: String }, @@ -1379,7 +1396,8 @@ impl DomainEvent { | Self::CronDeliveryRequested { .. } | Self::ProactiveMessageRequested { .. } | Self::FlowScheduleTick { .. } - | Self::FlowRunProgress { .. } => "cron", + | Self::FlowRunProgress { .. } + | Self::FlowChanged { .. } => "cron", Self::WorkflowLoaded { .. } | Self::WorkflowStopped { .. } @@ -1543,6 +1561,7 @@ impl DomainEvent { Self::ProactiveMessageRequested { .. } => "ProactiveMessageRequested", Self::FlowScheduleTick { .. } => "FlowScheduleTick", Self::FlowRunProgress { .. } => "FlowRunProgress", + Self::FlowChanged { .. } => "FlowChanged", Self::WorkflowLoaded { .. } => "WorkflowLoaded", Self::WorkflowStopped { .. } => "WorkflowStopped", Self::WorkflowStartFailed { .. } => "WorkflowStartFailed", diff --git a/src/core/socketio.rs b/src/core/socketio.rs index 1931aa198b..84afc6fc84 100644 --- a/src/core/socketio.rs +++ b/src/core/socketio.rs @@ -1078,6 +1078,30 @@ pub fn spawn_web_channel_bridge(io: SocketIo) { let _ = io_memory_sync.emit("flow:run_progress", &payload); let _ = io_memory_sync.emit("flow_run_progress", &payload); } + // A saved flow's definition changed (create/update/delete/ + // enable). Broadcast so an open Workflows list/canvas refetches + // — most importantly, so an agent `save_workflow` becomes + // visible in a canvas the user has open (audit F6). Best-effort; + // the UI's refetch-on-focus is the backstop. + crate::core::event_bus::DomainEvent::FlowChanged { + flow_id, + kind, + actor, + } => { + let payload = serde_json::json!({ + "flow_id": flow_id, + "kind": kind, + "actor": actor, + }); + log::debug!( + "[socketio] broadcast flow_changed flow_id={} kind={} actor={}", + flow_id, + kind, + actor + ); + let _ = io_memory_sync.emit("flow:changed", &payload); + let _ = io_memory_sync.emit("flow_changed", &payload); + } // A Workflow-origin tool call parked in the `ApprovalGate` // (flow-approval-surface, PR2/PR3). Broadcast — not // room-scoped like `ApprovalRequested`'s `approval_request` diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 6b09db83c9..5159dbdf6e 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -49,16 +49,27 @@ hint = "reasoning" named = [ "propose_workflow", "revise_workflow", + "edit_workflow", + "validate_workflow", "save_workflow", "list_flows", "get_flow", + "get_flow_history", "get_flow_run", "list_flow_connections", "search_tool_catalog", "get_tool_contract", "get_tool_output_sample", "list_agent_profiles", + "list_connectable_toolkits", + "list_node_kinds", + "get_node_kind_contract", "dry_run_workflow", + "list_flow_runs", + "resume_flow_run", + "cancel_flow_run", + "create_workflow", + "duplicate_flow", "run_flow", "composio_list_toolkits", "composio_list_connections", diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 4b9e3711d2..4988fa6292 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -106,6 +106,34 @@ it as real). Rules: integrations" below — you can help the user link it before you build, rather than dead-ending. +## Your authoring tools (prefer these — don't re-emit whole graphs) + +You have a machine-readable belt; use it instead of relying on memory: + +- **Introspect the DSL:** `list_node_kinds` → the 12 kinds; `get_node_kind_contract + { kind }` → one kind's exact config fields, ports, an example, and its + gotchas. Consult these instead of guessing config shapes (this is the source + of truth; the summary below is just orientation). +- **Iterate cheaply:** once a draft exists, prefer `edit_workflow { draft_id | + flow_id | graph, ops[] }` (add_node / update_node_config[merge-patch] / + rename_node / add_edge / remove_edge / …) over re-emitting the whole graph + with `revise_workflow` — it's fewer tokens and won't drop a node or mangle an + edge. Edits to a `draft_id` are written back to the shared draft. +- **Check without proposing:** `validate_workflow { graph | flow_id }` runs the + same structural + hard-gate stack and returns every problem at once, so you + can self-verify mid-build without emitting a proposal card. +- **Steer connections:** `list_connectable_toolkits` flags which toolkits are + already connected — prefer those; the proposal's `required_connections` + enumerates what still needs linking. +- **Debug a run:** `list_flow_runs { flow_id }` → find a failing run; + `get_flow_run` → diagnose it; patch with `edit_workflow`; `resume_flow_run` + (approval-gated) or `cancel_flow_run` to progress/stop a run. `get_flow_history` + → prior graph snapshots. +- **Persist (only when the user explicitly asks):** `create_workflow` makes a + NEW flow (always born disabled); `duplicate_flow` clones one (disabled) for + clone-then-edit; `save_workflow` writes onto an existing flow. Enabling stays + the user's job. + ## Connecting integrations A workflow often needs an app the user hasn't linked yet (a `tool_call` on @@ -183,6 +211,11 @@ A `WorkflowGraph` is `{ name?, nodes: [...], edges: [...] }`. ### The 12 node kinds +> The authoritative, always-current config shapes, ports, examples, and gotchas +> for each kind live in the `list_node_kinds` / `get_node_kind_contract { kind }` +> tools — call those when you need the exact fields. The summary below is +> orientation; when it and the contract tool disagree, the tool wins. + 1. **`trigger`** — the entry point (`config.trigger_kind`, see triggers below). 2. **`agent`** — an LLM step. **`config.input_context` carries the DATA; `config.prompt` stays a PLAIN instruction — never a `=` expression.** diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index abc3fc3381..599d90b0ab 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -56,7 +56,7 @@ use crate::openhuman::config::Config; use crate::openhuman::flows::ops; use crate::openhuman::flows::ops::validate_and_migrate_graph; use crate::openhuman::flows::tools; -use crate::openhuman::security::{AutonomyLevel, SecurityPolicy}; +use crate::openhuman::security::SecurityPolicy; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; /// Wall-clock bound on a single `dry_run_workflow` mock execution. A malformed @@ -180,93 +180,956 @@ impl Tool for ReviseWorkflowTool { } }; - // Enforcing binding-resolvability gate (see - // `ops::validate_binding_resolvability`): reject outright — rather - // than merely warn — a `tool_call` binding that is guaranteed to - // resolve null (or the wrong value) at runtime, so the builder must - // fix the graph before the revision can even be proposed. - let binding_errors = ops::validate_binding_resolvability(&graph); - if !binding_errors.is_empty() { + // Full builder hard-gate stack (binding-resolvability → tool-contract → + // required-arg resolvability) + summary/warning assembly, shared with + // edit_workflow so the two proposal paths can't drift. + match ops::build_builder_proposal( + &self.config, + "revise_workflow", + &name, + &graph, + require_approval, + true, + instruction, + ) + .await + { + Ok(payload) => Ok(ToolResult::success(serde_json::to_string_pretty(&payload)?)), + Err(message) => { + tracing::debug!(target: "flows", %name, "[flows] revise_workflow: a hard gate rejected the revised graph"); + Ok(ToolResult::error(message)) + } + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// edit_workflow — structured incremental edits (proposal only) — F1 +// ───────────────────────────────────────────────────────────────────────────── + +/// `edit_workflow`: apply a small list of structured graph ops to a base graph +/// (a saved flow by `flow_id`, or an inline `graph`) instead of re-emitting the +/// whole graph. Applies the ops, runs the full validate + hard-gate stack, and +/// returns the same `workflow_proposal` payload as `revise_workflow`. +/// +/// This is the cheap, low-regression iteration path (audit F1): a one-field +/// tweak on a 20-node flow is one `update_node_config` op, not a full re-emit. +/// Still proposal-only — never persists or enables. +pub struct EditWorkflowTool { + config: Arc, +} + +impl EditWorkflowTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for EditWorkflowTool { + fn name(&self) -> &str { + "edit_workflow" + } + + fn description(&self) -> &str { + "Iterate on a workflow with STRUCTURED EDITS instead of re-emitting the whole graph — the \ + cheap, low-regression path for changing a draft, saved, or inline flow. Provide the base \ + (draft_id for a working draft — the applied edit is written back to it; flow_id for a \ + saved flow; or an inline graph) plus ops[]: a list of edits applied in \ + order. Op shapes (each is { \"op\": , ... }): add_node {node}, update_node_config \ + {id, config} (JSON merge-patch — a null value deletes that config key), set_node_name \ + {id, name}, rename_node {id, new_id} (rewires edges), remove_node {id} (drops its edges), \ + add_edge {edge}, remove_edge {from_node, to_node, from_port?, to_port?}, set_node_position \ + {id, position}. Like propose/revise_workflow this ONLY VALIDATES and returns a proposal \ + for the user to review — it never creates, updates, or enables the flow. If an op fails or \ + the resulting graph is invalid, the error names the failing op / node; fix it and call \ + edit_workflow again." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "draft_id": { + "type": "string", + "description": "A working draft to edit as the base; the applied edit is written back to it. Provide one of draft_id / flow_id / graph." + }, + "flow_id": { + "type": "string", + "description": "The saved flow to edit as the base graph. Provide one of draft_id / flow_id / graph." + }, + "graph": { + "type": "object", + "description": "An inline base tinyflows WorkflowGraph to edit. Provide one of draft_id / flow_id / graph.", + "properties": { + "nodes": { "type": "array" }, + "edges": { "type": "array" } + } + }, + "ops": { + "type": "array", + "description": "The structured edits, applied in order. Each item is { op, ... } — see the tool description for op shapes.", + "items": { "type": "object", "properties": { "op": { "type": "string" } }, "required": ["op"] }, + "minItems": 1 + }, + "name": { + "type": "string", + "description": "Name for the resulting proposed flow. Defaults to the base flow's name." + }, + "instruction": { + "type": "string", + "description": "The change that motivated these ops (echoed back on the review card)." + }, + "require_approval": { + "type": "boolean", + "description": "Force a human-approval gate on every outbound action once saved. Defaults to true." + } + }, + "required": ["ops"] + }) + } + + fn permission_level(&self) -> PermissionLevel { + // Pure validation, no side effect — mirrors propose/revise_workflow. + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + // Resolve the base graph + a default name from exactly one of: a draft + // (the shared working copy — edits are written back to it), a saved + // flow, or an inline graph. + let draft_id = args + .get("draft_id") + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()); + let flow_id = args + .get("flow_id") + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()); + let inline_graph = args.get("graph").filter(|v| !v.is_null()); + + // When editing a draft, remember its id so the applied edit is written + // back — the draft is the durable working copy across turns/reloads. + let mut write_back_draft: Option = None; + + let (base_graph, default_name) = match (draft_id, flow_id, inline_graph) { + (Some(id), _, _) => match ops::flows_draft_get(&self.config, id) { + Ok(outcome) => { + let draft = outcome.value; + match ops::migrate_and_deserialize_graph(draft.graph.clone()) { + Ok(graph) => { + write_back_draft = Some(draft.id.clone()); + (graph, draft.name) + } + Err(e) => { + return Ok(ToolResult::error(format!( + "Draft '{id}' holds a graph that could not be parsed: {e}." + ))); + } + } + } + Err(e) => { + return Ok(ToolResult::error(format!( + "Could not load draft '{id}' to edit: {e}" + ))); + } + }, + (None, Some(id), _) => match ops::flows_get(&self.config, id).await { + Ok(outcome) => (outcome.value.graph, outcome.value.name), + Err(e) => { + return Ok(ToolResult::error(format!( + "Could not load flow '{id}' to edit: {e}" + ))); + } + }, + (None, None, Some(graph_json)) => { + match ops::migrate_and_deserialize_graph(graph_json.clone()) { + Ok(graph) => { + let name = graph.name.clone(); + (graph, name) + } + Err(e) => { + return Ok(ToolResult::error(format!( + "The inline base `graph` could not be parsed: {e}." + ))); + } + } + } + (None, None, None) => { + return Ok(ToolResult::error( + "Provide one of `draft_id` (a working draft), `flow_id` (a saved flow), or \ + `graph` (an inline base graph) to edit." + .to_string(), + )); + } + }; + + // Parse the ops list. + let ops_value = match args.get("ops") { + Some(v) if v.is_array() => v.clone(), + _ => { + return Ok(ToolResult::error( + "Missing 'ops' parameter (a non-empty array of structured edits).".to_string(), + )); + } + }; + let graph_ops: Vec = match serde_json::from_value(ops_value) + { + Ok(ops) => ops, + Err(e) => { + return Ok(ToolResult::error(format!( + "Could not parse `ops`: {e}. Each op is {{ \"op\": , ... }} — valid \ + types: add_node, update_node_config, set_node_name, rename_node, \ + remove_node, add_edge, remove_edge, set_node_position." + ))); + } + }; + if graph_ops.is_empty() { + return Ok(ToolResult::error( + "`ops` is empty — provide at least one edit.".to_string(), + )); + } + + let name = args + .get("name") + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .unwrap_or(default_name); + let name = if name.is_empty() { + "Untitled workflow".to_string() + } else { + name + }; + let instruction = args + .get("instruction") + .and_then(Value::as_str) + .map(str::to_string); + let require_approval = args + .get("require_approval") + .and_then(Value::as_bool) + .unwrap_or(true); + + tracing::debug!( + target: "flows", + %name, + op_count = graph_ops.len(), + from_flow = flow_id.is_some(), + "[flows] edit_workflow: applying structured ops to base graph" + ); + + // Apply the ops (structural mutation, precise per-op errors). + let edited = match tinyflows::graph_ops::apply_ops(&base_graph, &graph_ops) { + Ok(graph) => graph, + Err(e) => { + tracing::debug!(target: "flows", %name, error = %e, "[flows] edit_workflow: an op failed to apply"); + return Ok(ToolResult::error(format!( + "{e}\n\nFix the ops and call edit_workflow again." + ))); + } + }; + + // Write the applied edit back to the draft (the durable working copy), + // so it survives across turns/reloads even if validation/gates below + // still flag something to fix next. + if let Some(ref draft_id) = write_back_draft { + let edited_json = serde_json::to_value(&edited)?; + if let Err(e) = ops::flows_draft_update( + &self.config, + draft_id, + Some(name.clone()), + Some(edited_json), + None, + ) { + tracing::warn!(target: "flows", %draft_id, error = %e, "[flows] edit_workflow: could not write edit back to draft"); + } + } + + // Structural validation of the RESULT — surface every problem at once. + let structural = tinyflows::validate::validate_all(&edited); + if !structural.is_empty() { + let messages: Vec = structural.iter().map(ToString::to_string).collect(); tracing::debug!( target: "flows", %name, - error_count = binding_errors.len(), - "[flows] revise_workflow: binding-resolvability check rejected the revised graph" + error_count = messages.len(), + "[flows] edit_workflow: the edited graph is structurally invalid" ); return Ok(ToolResult::error(format!( - "{}\n\nFix these bindings and call revise_workflow again.", - binding_errors.join("\n\n") + "The edited graph is invalid:\n\n{}\n\nFix the ops and call edit_workflow again.", + messages.join("\n") ))); } - // Tool-contract enforcement gate (systemic tool-contract fix, Part 2): - // reject a `tool_call` node whose slug isn't a REAL action in the - // live Composio catalog, or whose real required args aren't all wired. - let contract_errors = ops::validate_tool_contracts(&self.config, &graph).await; - if !contract_errors.is_empty() { - tracing::debug!( - target: "flows", - %name, - error_count = contract_errors.len(), - "[flows] revise_workflow: tool-contract check rejected the revised graph" - ); + // Full builder hard-gate stack + proposal payload (shared with revise). + match ops::build_builder_proposal( + &self.config, + "edit_workflow", + &name, + &edited, + require_approval, + true, + instruction, + ) + .await + { + Ok(mut payload) => { + if let Some(draft_id) = write_back_draft { + payload["draft_id"] = json!(draft_id); + } + Ok(ToolResult::success(serde_json::to_string_pretty(&payload)?)) + } + Err(message) => { + tracing::debug!(target: "flows", %name, "[flows] edit_workflow: a hard gate rejected the edited graph"); + Ok(ToolResult::error(message)) + } + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// validate_workflow — standalone check without proposing (F3) +// ───────────────────────────────────────────────────────────────────────────── + +/// `validate_workflow`: run the SAME structural validation + hard-gate stack +/// the propose/revise/edit/save tools use, but WITHOUT emitting a proposal — +/// a pure check so the agent can verify a draft (or a saved flow) mid-build. +/// +/// Returns a structured report `{ ok, structurally_valid, errors[], +/// error_details[], gate_errors[], warnings[] }`, so a failing check is +/// fix-and-retry rather than a proposal the user has to reject. +pub struct ValidateWorkflowTool { + config: Arc, +} + +impl ValidateWorkflowTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for ValidateWorkflowTool { + fn name(&self) -> &str { + "validate_workflow" + } + + fn description(&self) -> &str { + "Check a workflow graph WITHOUT proposing or saving it — the same validation the \ + propose/revise/edit/save tools run, surfaced on its own so you can verify a draft mid-\ + build. Provide the graph to check (inline `graph`, or `flow_id` for a saved flow). \ + Returns { ok, structurally_valid, errors, error_details:[{code, message, node_id}], \ + gate_errors, warnings }: `errors` lists EVERY structural problem at once; `gate_errors` \ + lists the hard author-gate failures (unresolvable bindings, unreal tool slugs, unwired \ + required args) checked only once the graph is structurally valid; `warnings` are \ + non-fatal. `ok` is true only when there are no errors and no gate_errors. Read-only." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "flow_id": { + "type": "string", + "description": "A saved flow to validate. Provide this OR `graph`." + }, + "graph": { + "type": "object", + "description": "An inline tinyflows WorkflowGraph to validate. Provide this OR `flow_id`.", + "properties": { + "nodes": { "type": "array" }, + "edges": { "type": "array" } + } + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + // Resolve the graph to check from either a saved flow or an inline graph. + let flow_id = args + .get("flow_id") + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()); + let inline_graph = args.get("graph").filter(|v| !v.is_null()); + + let graph_json = match (flow_id, inline_graph) { + (Some(id), _) => match ops::load_flow_graph(&self.config, id) { + Ok(Some(graph)) => serde_json::to_value(&graph)?, + Ok(None) => { + return Ok(ToolResult::error(format!("flow '{id}' not found"))); + } + Err(e) => { + return Ok(ToolResult::error(format!( + "Could not load flow '{id}' to validate: {e}" + ))); + } + }, + (None, Some(graph)) => graph.clone(), + (None, None) => { + return Ok(ToolResult::error( + "Provide either `flow_id` (a saved flow) or `graph` (an inline graph) to \ + validate." + .to_string(), + )); + } + }; + + tracing::debug!( + target: "flows", + from_flow = flow_id.is_some(), + "[flows] validate_workflow: checking graph (read-only)" + ); + + // Structural validation first (every error at once). + let validation = ops::flows_validate(graph_json.clone()).value; + + // Only run the (expensive) hard gates on a structurally-valid graph. + let gate_errors = if validation.valid { + match ops::migrate_and_deserialize_graph(graph_json) { + Ok(graph) => ops::run_builder_gates(&self.config, &graph).await, + Err(_) => Vec::new(), + } + } else { + Vec::new() + }; + + let ok = validation.valid && gate_errors.is_empty(); + let report = json!({ + "ok": ok, + "structurally_valid": validation.valid, + "errors": validation.errors, + "error_details": validation.error_details, + "gate_errors": gate_errors, + "warnings": validation.warnings, + }); + Ok(ToolResult::success(serde_json::to_string_pretty(&report)?)) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// get_flow_history — read-only: prior graph snapshots (F6) +// ───────────────────────────────────────────────────────────────────────────── + +/// `get_flow_history`: read a saved flow's revision history — the prior graph +/// snapshots captured on each update. Lets the agent see what changed and pick +/// a revision to roll back to (the user drives the actual rollback RPC). +pub struct GetFlowHistoryTool { + config: Arc, +} + +impl GetFlowHistoryTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for GetFlowHistoryTool { + fn name(&self) -> &str { + "get_flow_history" + } + + fn description(&self) -> &str { + "List a saved flow's revision history — the prior graph snapshots captured automatically \ + on each update (newest first, capped). Read-only. Returns a JSON array of { id, flow_id, \ + graph, name, require_approval, created_at }. Use it to see what a flow looked like before \ + a change, or to find the revision id the user can roll back to." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "flow_id": { "type": "string", "description": "The saved flow whose history to list." }, + "limit": { "type": "integer", "description": "Max revisions to return (default 20)." } + }, + "required": ["flow_id"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let flow_id = match args.get("flow_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'flow_id' parameter".to_string())), + }; + let limit = args + .get("limit") + .and_then(Value::as_u64) + .map(|n| n as usize) + .unwrap_or(20); + tracing::debug!(target: "flows", %flow_id, limit, "[flows] get_flow_history: listing revisions (read-only)"); + match ops::flows_get_history(&self.config, &flow_id, limit) { + Ok(outcome) => Ok(ToolResult::success(serde_json::to_string_pretty( + &json!({ "revisions": outcome.value }), + )?)), + Err(e) => Ok(ToolResult::error(format!( + "Could not load history for flow '{flow_id}': {e}" + ))), + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Phase 4 — the self-debug loop + gated create (F4, F7) +// ───────────────────────────────────────────────────────────────────────────── + +/// `list_flow_runs`: read-only listing of a saved flow's recent runs (id / +/// status / timestamps), so the agent can FIND a failing run to diagnose +/// instead of needing a run_id handed to it externally — the missing first step +/// of the self-debug loop (audit F4). +pub struct ListFlowRunsTool { + config: Arc, +} + +impl ListFlowRunsTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for ListFlowRunsTool { + fn name(&self) -> &str { + "list_flow_runs" + } + + fn description(&self) -> &str { + "List a saved flow's recent runs (newest first) so you can find one to diagnose with \ + get_flow_run. Read-only. Returns a JSON array of runs { id, flow_id, thread_id, status, \ + started_at, finished_at?, error? }. `id`/`thread_id` is the run id you pass to \ + get_flow_run / resume_flow_run / cancel_flow_run." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "flow_id": { "type": "string", "description": "The saved flow whose runs to list." }, + "limit": { "type": "integer", "description": "Max runs to return (default 20)." } + }, + "required": ["flow_id"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let flow_id = match args.get("flow_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'flow_id' parameter".to_string())), + }; + let limit = args + .get("limit") + .and_then(Value::as_u64) + .map(|n| n as usize) + .unwrap_or(20); + tracing::debug!(target: "flows", %flow_id, limit, "[flows] list_flow_runs: listing runs (read-only)"); + match ops::flows_list_runs(&self.config, &flow_id, limit).await { + Ok(outcome) => Ok(ToolResult::success(serde_json::to_string_pretty( + &json!({ "runs": outcome.value }), + )?)), + Err(e) => Ok(ToolResult::error(format!( + "Could not list runs for flow '{flow_id}': {e}" + ))), + } + } +} + +/// `resume_flow_run`: progress a run parked on a human approval by +/// approving/rejecting its pending node(s). Execute + approval-gated — it +/// advances a REAL run that can fire real outbound effects. +pub struct ResumeFlowRunTool { + config: Arc, +} + +impl ResumeFlowRunTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for ResumeFlowRunTool { + fn name(&self) -> &str { + "resume_flow_run" + } + + fn description(&self) -> &str { + "Resume a flow run that is paused on a human approval, approving and/or rejecting its \ + pending node(s). This ADVANCES A REAL RUN — approved outbound nodes will fire — so it is \ + approval-gated. Params: { flow_id, run_id, approve?: [node_id...], reject?: [node_id...] }. \ + Use list_flow_runs / get_flow_run to find a run with status pending_approval and its \ + pending node ids first." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "flow_id": { "type": "string", "description": "The run's flow id." }, + "run_id": { "type": "string", "description": "The run (thread) id to resume (from list_flow_runs)." }, + "approve": { "type": "array", "items": { "type": "string" }, "description": "Node ids to approve." }, + "reject": { "type": "array", "items": { "type": "string" }, "description": "Node ids to reject." } + }, + "required": ["flow_id", "run_id"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + // Advances a real run (approved nodes fire) — gate like an execute-class, + // approval-parked action. + PermissionLevel::Execute + } + + fn external_effect(&self) -> bool { + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let flow_id = match args.get("flow_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'flow_id' parameter".to_string())), + }; + let run_id = match args.get("run_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'run_id' parameter".to_string())), + }; + let approve = string_array(&args, "approve"); + let reject = string_array(&args, "reject"); + tracing::debug!(target: "flows", %flow_id, %run_id, approve = approve.len(), reject = reject.len(), "[flows] resume_flow_run: resuming parked run"); + match ops::flows_resume(&self.config, &flow_id, &run_id, approve, reject).await { + Ok(outcome) => Ok(ToolResult::success(serde_json::to_string_pretty( + &outcome.value, + )?)), + Err(e) => Ok(ToolResult::error(format!("Could not resume run: {e}"))), + } + } +} + +/// `cancel_flow_run`: stop an in-flight or parked run. Write-class — it changes +/// run state but fires no new outbound effect. +pub struct CancelFlowRunTool { + config: Arc, +} + +impl CancelFlowRunTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for CancelFlowRunTool { + fn name(&self) -> &str { + "cancel_flow_run" + } + + fn description(&self) -> &str { + "Cancel an in-flight or approval-parked flow run by its run_id (from list_flow_runs). \ + Stops a runaway or stuck run; fires no new outbound effect. Params: { run_id }." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "run_id": { "type": "string", "description": "The run (thread) id to cancel." } + }, + "required": ["run_id"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let run_id = match args.get("run_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'run_id' parameter".to_string())), + }; + tracing::debug!(target: "flows", %run_id, "[flows] cancel_flow_run: cancelling run"); + match ops::flows_cancel_run(&self.config, &run_id).await { + Ok(outcome) => Ok(ToolResult::success(serde_json::to_string_pretty( + &outcome.value, + )?)), + Err(e) => Ok(ToolResult::error(format!("Could not cancel run: {e}"))), + } + } +} + +/// `create_workflow`: the gated create tool (audit F4/F12). Persists a NEW +/// flow, always **born disabled** (enable stays human-only) and behind the +/// forced `require_approval` floor for side-effect graphs. Write + approval +/// gated. This is the deliberate widening the Phase 3 rails (versioning, +/// events, history) make safe. +pub struct CreateWorkflowTool { + config: Arc, +} + +impl CreateWorkflowTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for CreateWorkflowTool { + fn name(&self) -> &str { + "create_workflow" + } + + fn description(&self) -> &str { + "Create a NEW saved flow from a graph. Approval-gated. The flow is ALWAYS created DISABLED \ + (only the user can enable it via the UI) and inherits the forced approval gate for any \ + outbound action — so a created flow can never fire on its own without an explicit human \ + enable. Runs the same author hard-gates as save. Params: { name, graph, require_approval? }. \ + Prefer propose_workflow when the user just wants to review a design; use this when they've \ + explicitly asked you to create the flow." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "name": { "type": "string", "description": "Human-readable flow name." }, + "graph": { + "type": "object", + "description": "The tinyflows WorkflowGraph: { nodes: [...], edges: [...] }.", + "properties": { "nodes": { "type": "array" }, "edges": { "type": "array" } }, + "required": ["nodes", "edges"] + }, + "require_approval": { "type": "boolean", "description": "Force the approval gate (defaults true)." } + }, + "required": ["name", "graph"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + fn external_effect(&self) -> bool { + // Persists a new flow definition. + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let name = match args.get("name").and_then(Value::as_str).map(str::trim) { + Some(n) if !n.is_empty() => n.to_string(), + _ => return Ok(ToolResult::error("Missing 'name' parameter".to_string())), + }; + let graph_json = match args.get("graph") { + Some(v) if !v.is_null() => v.clone(), + _ => return Ok(ToolResult::error("Missing 'graph' parameter".to_string())), + }; + let require_approval = args + .get("require_approval") + .and_then(Value::as_bool) + .unwrap_or(true); + + // Same structural + hard-gate stack an agent save must pass. + if let Err(msg) = ops::strict_gate(&self.config, &graph_json).await { return Ok(ToolResult::error(format!( - "{}\n\nFix these tool_call nodes and call revise_workflow again.", - contract_errors.join("\n\n") + "{msg}\n\nFix the graph and call create_workflow again." ))); } - // Required-arg resolvability gate (issue B18): reject outright — not - // just warn — a REQUIRED outbound arg that LOOKS wired but resolves - // to `null` in a sandboxed test run. See - // `ops::validate_required_arg_resolvability`. - let null_arg_errors = ops::validate_required_arg_resolvability(&graph).await; - if !null_arg_errors.is_empty() { - tracing::debug!( - target: "flows", - %name, - error_count = null_arg_errors.len(), - "[flows] revise_workflow: required-arg resolvability check rejected the revised graph" - ); - return Ok(ToolResult::error(format!( - "{}\n\nFix these bindings and call revise_workflow again.", - null_arg_errors.join("\n\n") - ))); + tracing::info!(target: "flows", %name, "[flows] create_workflow: agent-initiated create (born disabled)"); + let flow = match ops::flows_create(&self.config, name, graph_json, require_approval).await { + Ok(outcome) => outcome.value, + Err(e) => return Ok(ToolResult::error(format!("Could not create flow: {e}"))), + }; + + // Force born-disabled: enable stays human-only, even for a manual-trigger + // graph that flows_create would otherwise create enabled. + if flow.enabled { + if let Err(e) = ops::flows_set_enabled(&self.config, &flow.id, false).await { + tracing::warn!(target: "flows", flow_id = %flow.id, error = %e, "[flows] create_workflow: could not force-disable the new flow"); + } } - let summary = super::tools::build_summary(&graph); - let mut warnings = ops::graph_trigger_warnings(&graph); - // Author-time wiring check: unwired REQUIRED Composio args come back - // as warnings naming the field, before the user ever saves. - warnings.extend(ops::graph_wiring_warnings(&self.config, &graph).await); - let graph_value = serde_json::to_value(&graph)?; + Ok(ToolResult::success(serde_json::to_string_pretty(&json!({ + "type": "workflow_created", + "flow_id": flow.id, + "name": flow.name, + "enabled": false, + "require_approval": flow.require_approval, + "note": "Flow created DISABLED. The user must enable it explicitly before it can run.", + }))?)) + } +} - tracing::info!( - target: "flows", - %name, - node_count = graph.nodes.len(), - require_approval, - warning_count = warnings.len(), - "[flows] revise_workflow: revised proposal ready for user review" - ); +/// `duplicate_flow`: create an independent, DISABLED copy of a saved flow — the +/// clone-then-edit pattern. Write-class. +pub struct DuplicateFlowTool { + config: Arc, +} - let mut payload = json!({ - "type": "workflow_proposal", - "revision": true, - "name": name, - "graph": graph_value, - "require_approval": require_approval, - "summary": summary, - "warnings": warnings, - }); - if let Some(instruction) = instruction { - payload["instruction"] = json!(instruction); +impl DuplicateFlowTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for DuplicateFlowTool { + fn name(&self) -> &str { + "duplicate_flow" + } + + fn description(&self) -> &str { + "Duplicate a saved flow: create an independent, DISABLED copy of its graph under a new id \ + (name suffixed \" (copy)\"). The copy never fires until the user enables it. Use this for \ + the clone-then-edit pattern (edit_workflow the copy). Params: { flow_id }." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { "flow_id": { "type": "string", "description": "The saved flow to duplicate." } }, + "required": ["flow_id"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + fn external_effect(&self) -> bool { + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let flow_id = match args.get("flow_id").and_then(Value::as_str).map(str::trim) { + Some(id) if !id.is_empty() => id.to_string(), + _ => return Ok(ToolResult::error("Missing 'flow_id' parameter".to_string())), + }; + tracing::info!(target: "flows", %flow_id, "[flows] duplicate_flow: agent-initiated duplicate"); + match ops::flows_duplicate(&self.config, &flow_id).await { + Ok(outcome) => { + let flow = outcome.value; + Ok(ToolResult::success(serde_json::to_string_pretty(&json!({ + "type": "workflow_duplicated", + "flow_id": flow.id, + "name": flow.name, + "enabled": flow.enabled, + }))?)) + } + Err(e) => Ok(ToolResult::error(format!("Could not duplicate flow: {e}"))), } + } +} + +/// `list_connectable_toolkits`: read-only list of the Composio toolkits the +/// builder can wire, each tagged connected/unconnected — so the agent can steer +/// toolkit choice toward what's already connected (audit Phase 5, item 19). +pub struct ListConnectableToolkitsTool { + config: Arc, +} + +impl ListConnectableToolkitsTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for ListConnectableToolkitsTool { + fn name(&self) -> &str { + "list_connectable_toolkits" + } + + fn description(&self) -> &str { + "List the Composio toolkits available to wire into a tool_call/app_event, each flagged \ + `connected: true/false`. Read-only. Use it to prefer an ALREADY-connected toolkit when \ + several would work, and to tell the user which toolkits a proposed flow still needs \ + connecting. Returns a JSON array of { toolkit, connected }." + } + + fn parameters_schema(&self) -> Value { + json!({ "type": "object", "properties": {}, "additionalProperties": false }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } - Ok(ToolResult::success(serde_json::to_string_pretty(&payload)?)) + async fn execute(&self, _args: Value) -> anyhow::Result { + use crate::openhuman::memory_sync::composio::providers::agent_ready_toolkits; + tracing::debug!(target: "flows", "[flows] list_connectable_toolkits: listing toolkits + connected state (read-only)"); + let connected = ops::connected_toolkits(&self.config).await; + let toolkits: Vec = agent_ready_toolkits() + .into_iter() + .map(|tk| { + let tk_lc = tk.to_ascii_lowercase(); + json!({ "toolkit": tk_lc, "connected": connected.contains(&tk_lc) }) + }) + .collect(); + Ok(ToolResult::success(serde_json::to_string_pretty( + &json!({ "toolkits": toolkits }), + )?)) } } +/// Extracts a string array from `args[key]`, ignoring non-strings; empty when +/// absent. Shared by the resume tool's approve/reject lists. +fn string_array(args: &Value, key: &str) -> Vec { + args.get(key) + .and_then(Value::as_array) + .map(|a| { + a.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default() +} + // ───────────────────────────────────────────────────────────────────────────── // list_flows — read-only: saved flow summaries // ───────────────────────────────────────────────────────────────────────────── @@ -1064,6 +1927,162 @@ impl Tool for ListAgentProfilesTool { } } +// ───────────────────────────────────────────────────────────────────────────── +// list_node_kinds / get_node_kind_contract — queryable DSL schema (F2) +// ───────────────────────────────────────────────────────────────────────────── + +/// `list_node_kinds`: enumerate the 12 tinyflows node kinds with a one-line +/// summary each. The DSL counterpart of `search_tool_catalog` for Composio +/// actions — a cheap first call to orient before fetching a full contract. +pub struct ListNodeKindsTool; + +impl ListNodeKindsTool { + /// Builds the tool (no configuration — the contracts are static). + #[must_use] + pub fn new() -> Self { + Self + } +} + +impl Default for ListNodeKindsTool { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl Tool for ListNodeKindsTool { + fn name(&self) -> &str { + "list_node_kinds" + } + + fn description(&self) -> &str { + "List the 12 tinyflows node kinds you can put in a WorkflowGraph, each with a one-line \ + summary and its config field names. Read-only, no args. Returns a JSON array of { kind, \ + summary, required_config, optional_config }. Call get_node_kind_contract { kind } for the \ + full config-field shapes, ports, an example node, and authoring gotchas of any one kind — \ + this is the machine-readable DSL schema, so you don't have to rely on prose or memory." + } + + fn parameters_schema(&self) -> Value { + json!({ "type": "object", "properties": {}, "additionalProperties": false }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, _args: Value) -> anyhow::Result { + tracing::debug!(target: "flows", "[flows] list_node_kinds: enumerating node kinds (read-only)"); + let kinds: Vec = crate::openhuman::flows::all_node_kind_contracts() + .iter() + .map(|c| { + let required: Vec<&str> = c + .config_fields + .iter() + .filter(|f| f.required) + .map(|f| f.name.as_str()) + .collect(); + let optional: Vec<&str> = c + .config_fields + .iter() + .filter(|f| !f.required) + .map(|f| f.name.as_str()) + .collect(); + json!({ + "kind": c.kind, + "summary": c.summary, + "required_config": required, + "optional_config": optional, + }) + }) + .collect(); + Ok(ToolResult::success(serde_json::to_string_pretty( + &json!({ "node_kinds": kinds }), + )?)) + } +} + +/// `get_node_kind_contract`: the FULL machine-readable contract for one node +/// kind — config fields (name/required/type/description/enum), ports, a valid +/// example node, and the authoring gotchas. Mirrors `get_tool_contract` for +/// Composio actions but for the DSL itself. +pub struct GetNodeKindContractTool; + +impl GetNodeKindContractTool { + /// Builds the tool (no configuration — the contracts are static). + #[must_use] + pub fn new() -> Self { + Self + } +} + +impl Default for GetNodeKindContractTool { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl Tool for GetNodeKindContractTool { + fn name(&self) -> &str { + "get_node_kind_contract" + } + + fn description(&self) -> &str { + "Fetch the FULL contract for ONE tinyflows node kind before you author a node of that \ + kind. Read-only. Returns { kind, summary, description, config_fields:[{name, required, \ + value_type, description, enum_values?}], ports:{inputs, outputs}, example, notes }. Use \ + config_fields for exactly what to put in config, ports for how to wire branch edges (the \ + branch label goes on the edge's from_port), and notes for the envelope/gotcha rules that \ + otherwise silently resolve to null. Find the kind names via list_node_kinds." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "kind": { + "type": "string", + "description": "One of the 12 node kinds, e.g. 'tool_call' (from list_node_kinds).", + "enum": crate::openhuman::flows::NODE_KINDS, + } + }, + "required": ["kind"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::None + } + + fn external_effect(&self) -> bool { + false + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let kind = match args.get("kind").and_then(Value::as_str).map(str::trim) { + Some(k) if !k.is_empty() => k.to_string(), + _ => return Ok(ToolResult::error("Missing 'kind' parameter".to_string())), + }; + tracing::debug!(target: "flows", %kind, "[flows] get_node_kind_contract: fetching contract (read-only)"); + match crate::openhuman::flows::node_kind_contract(&kind) { + Some(contract) => Ok(ToolResult::success(serde_json::to_string_pretty( + &contract, + )?)), + None => Ok(ToolResult::error(format!( + "'{kind}' is not a tinyflows node kind — call list_node_kinds for the 12 valid \ + kinds." + ))), + } + } +} + // ───────────────────────────────────────────────────────────────────────────── // dry_run_workflow — execute a DRAFT against MOCK capabilities (tier-gated) // ───────────────────────────────────────────────────────────────────────────── @@ -1182,9 +2201,13 @@ impl Tool for DryRunWorkflowTool { json!({ "type": "object", "properties": { + "draft_id": { + "type": "string", + "description": "A working draft to simulate. Provide this OR `graph`." + }, "graph": { "type": "object", - "description": "The DRAFT tinyflows WorkflowGraph to simulate: { nodes: [...], edges: [...] }.", + "description": "The DRAFT tinyflows WorkflowGraph to simulate: { nodes: [...], edges: [...] }. Provide this OR `draft_id`.", "properties": { "nodes": { "type": "array" }, "edges": { "type": "array" } @@ -1194,43 +2217,50 @@ impl Tool for DryRunWorkflowTool { "input": { "description": "Optional trigger input passed to the run (defaults to {})." } - }, - "required": ["graph"] + } }) } fn permission_level(&self) -> PermissionLevel { - // Represents executable capability (a full sandbox could run code/http), - // so it is gated like an execute-class tool even though the mock backend - // means no real side effect can fire. - PermissionLevel::Execute + // Mock-only and side-effect-free: nothing external ever fires (all + // capabilities are echo stubs). So it needs no elevated permission and + // is available on EVERY tier, read-only included (audit F7) — a + // read-only agent must be able to self-verify its own proposal. + PermissionLevel::None } fn external_effect(&self) -> bool { - // Mock capabilities only — no real outbound effect. The `Execute` - // permission above plus the read-only tier refusal below carry the gate. + // Mock capabilities only — no real outbound effect. false } async fn execute(&self, args: Value) -> anyhow::Result { - // Autonomy-tier gate: a read-only session cannot dry-run (executable - // capability, even simulated). Supervised / Full may. - if self.security.autonomy == AutonomyLevel::ReadOnly { - tracing::debug!( - target: "flows", - "[flows] dry_run_workflow: refused — autonomy tier is read-only" - ); - return Ok(ToolResult::error( - "dry_run_workflow requires at least 'supervised' autonomy — the current \ - tier is read-only. Propose the workflow instead (propose_workflow), or \ - raise autonomy in Settings → Agent access." - .to_string(), - )); - } - - let graph_json = match args.get("graph") { - Some(v) if !v.is_null() => v.clone(), - _ => return Ok(ToolResult::error("Missing 'graph' parameter".to_string())), + // Graph source: a working draft (draft_id) or an inline graph. + let graph_json = if let Some(draft_id) = args + .get("draft_id") + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()) + { + match ops::flows_draft_get(&self.config, draft_id) { + Ok(outcome) => outcome.value.graph, + Err(e) => { + return Ok(ToolResult::error(format!( + "Could not load draft '{draft_id}' to dry-run: {e}" + ))); + } + } + } else { + match args.get("graph") { + Some(v) if !v.is_null() => v.clone(), + _ => { + return Ok(ToolResult::error( + "Provide `graph` (an inline graph) or `draft_id` (a working draft) to \ + dry-run." + .to_string(), + )); + } + } }; let input = args.get("input").cloned().unwrap_or_else(|| json!({})); @@ -1777,51 +2807,21 @@ impl Tool for SaveWorkflowTool { ))); } }; - let binding_errors = ops::validate_binding_resolvability(&graph); - if !binding_errors.is_empty() { - tracing::debug!( - target: "flows", - %flow_id, - error_count = binding_errors.len(), - "[flows] save_workflow: binding-resolvability check rejected the graph" - ); - return Ok(ToolResult::error(format!( - "{}\n\nFix these bindings and call save_workflow again.", - binding_errors.join("\n\n") - ))); - } - // Tool-contract enforcement gate (systemic tool-contract fix, Part 2): - // reject a `tool_call` node whose slug isn't a REAL action in the - // live Composio catalog, or whose real required args aren't all - // wired — before the graph is ever persisted. - let contract_errors = ops::validate_tool_contracts(&self.config, &graph).await; - if !contract_errors.is_empty() { - tracing::debug!( - target: "flows", - %flow_id, - error_count = contract_errors.len(), - "[flows] save_workflow: tool-contract check rejected the graph" - ); - return Ok(ToolResult::error(format!( - "{}\n\nFix these tool_call nodes and call save_workflow again.", - contract_errors.join("\n\n") - ))); - } - // Required-arg resolvability gate (issue B18): reject outright — not - // just warn — a REQUIRED outbound arg that LOOKS wired but resolves - // to `null` in a sandboxed test run, before the graph is ever - // persisted. See `ops::validate_required_arg_resolvability`. - let null_arg_errors = ops::validate_required_arg_resolvability(&graph).await; - if !null_arg_errors.is_empty() { + // The full builder hard-gate stack, run through the single canonical + // runner shared with propose/revise/edit and the strict create/update + // RPC path (F3) — so an agent can never persist a graph that would fail + // gates the other planes enforce. + let gate_errors = ops::run_builder_gates(&self.config, &graph).await; + if !gate_errors.is_empty() { tracing::debug!( target: "flows", %flow_id, - error_count = null_arg_errors.len(), - "[flows] save_workflow: required-arg resolvability check rejected the graph" + error_count = gate_errors.len(), + "[flows] save_workflow: a hard gate rejected the graph" ); return Ok(ToolResult::error(format!( - "{}\n\nFix these bindings and call save_workflow again.", - null_arg_errors.join("\n\n") + "{}\n\nFix these and call save_workflow again.", + gate_errors.join("\n\n") ))); } // Author-time warnings (unfired trigger kinds + unwired REQUIRED @@ -1838,7 +2838,7 @@ impl Tool for SaveWorkflowTool { "[flows] save_workflow: agent-initiated save to existing flow" ); - match ops::flows_update(&self.config, &flow_id, name, Some(graph_json), None).await { + match ops::flows_update(&self.config, &flow_id, name, Some(graph_json), None, None).await { Ok(outcome) => { let flow = outcome.value; tracing::info!( diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index bd4e6dcb00..fc207c73fe 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -503,29 +503,34 @@ async fn get_tool_output_sample_refuses_an_unconnected_toolkit() { // ── dry_run_workflow ───────────────────────────────────────────────────────── #[test] -fn dry_run_is_execute_permission() { +fn dry_run_is_side_effect_free_and_ungated() { let tool = DryRunWorkflowTool::new( policy(AutonomyLevel::Supervised), test_config(&TempDir::new().unwrap()), ); assert_eq!(tool.name(), "dry_run_workflow"); - assert_eq!(tool.permission_level(), PermissionLevel::Execute); - // Mock-backed: no real outbound effect. + // Mock-only + side-effect-free → PermissionLevel::None, available on every + // tier including read-only (audit F7). + assert_eq!(tool.permission_level(), PermissionLevel::None); assert!(!tool.external_effect()); } #[tokio::test] -async fn dry_run_refused_under_readonly_tier() { +async fn dry_run_allowed_under_readonly_tier() { + // F7: dry_run is mock-only and side-effect-free, so a read-only agent must + // be able to self-verify its own proposal (previously refused). let tool = DryRunWorkflowTool::new( policy(AutonomyLevel::ReadOnly), test_config(&TempDir::new().unwrap()), ); + assert_eq!(tool.permission_level(), PermissionLevel::None); let result = tool .execute(json!({ "graph": valid_graph() })) .await .unwrap(); - assert!(result.is_error); - assert!(result.output().to_lowercase().contains("read-only")); + // Not refused for tier reasons — it actually runs against the mocks. + assert!(!result.is_error, "{}", result.output()); + assert!(!result.output().to_lowercase().contains("read-only")); } #[tokio::test] @@ -1374,3 +1379,348 @@ async fn save_workflow_accepts_correctly_schemad_graph() { assert_eq!(saved.name, "Summarize and notify"); assert_eq!(saved.graph.nodes.len(), 3); } + +#[tokio::test] +async fn list_node_kinds_tool_returns_all_twelve() { + let tool = ListNodeKindsTool::new(); + let result = tool.execute(json!({})).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + let kinds = parsed["node_kinds"].as_array().unwrap(); + assert_eq!(kinds.len(), 12); + // Each entry carries a kind + summary + the config-field name lists. + assert!(kinds.iter().any(|k| k["kind"] == "tool_call")); + assert!(kinds.iter().all(|k| k.get("summary").is_some())); +} + +#[tokio::test] +async fn get_node_kind_contract_tool_returns_contract_and_rejects_unknown() { + let tool = GetNodeKindContractTool::new(); + + let ok = tool.execute(json!({ "kind": "tool_call" })).await.unwrap(); + assert!(!ok.is_error, "{}", ok.output()); + let parsed: Value = serde_json::from_str(&ok.output()).unwrap(); + assert_eq!(parsed["kind"], "tool_call"); + assert!(parsed["config_fields"] + .as_array() + .unwrap() + .iter() + .any(|f| f["name"] == "slug")); + // Host overlay is present on the tool's output. + assert!(parsed["notes"] + .as_array() + .unwrap() + .iter() + .any(|n| n.as_str().unwrap_or("").contains("Composio"))); + + let bad = tool.execute(json!({ "kind": "nope" })).await.unwrap(); + assert!(bad.is_error); + assert!(bad.output().contains("list_node_kinds")); + + let missing = tool.execute(json!({})).await.unwrap(); + assert!(missing.is_error); +} + +// ── edit_workflow (F1: structured incremental edits) ───────────────────────── + +#[tokio::test] +async fn edit_workflow_applies_ops_to_inline_graph_and_returns_proposal() { + let tmp = TempDir::new().unwrap(); + let tool = EditWorkflowTool::new(test_config(&tmp)); + + // Add a merge node `b` and wire the agent into it. + let result = tool + .execute(json!({ + "graph": valid_graph(), + "name": "Edited flow", + "instruction": "add a merge step", + "ops": [ + { "op": "add_node", "node": { "id": "b", "kind": "merge", "name": "Join" } }, + { "op": "add_edge", "edge": { "from_node": "a", "to_node": "b" } } + ] + })) + .await + .unwrap(); + + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["type"], "workflow_proposal"); + assert_eq!(parsed["name"], "Edited flow"); + assert_eq!(parsed["graph"]["nodes"].as_array().unwrap().len(), 3); + assert_eq!(parsed["graph"]["edges"].as_array().unwrap().len(), 2); +} + +#[tokio::test] +async fn edit_workflow_update_node_config_merge_patches() { + let tmp = TempDir::new().unwrap(); + let tool = EditWorkflowTool::new(test_config(&tmp)); + + let result = tool + .execute(json!({ + "graph": valid_graph(), + "ops": [ + { "op": "update_node_config", "id": "a", "config": { "prompt": "new instruction" } } + ] + })) + .await + .unwrap(); + + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + let nodes = parsed["graph"]["nodes"].as_array().unwrap(); + let agent = nodes.iter().find(|n| n["id"] == "a").unwrap(); + assert_eq!(agent["config"]["prompt"], "new instruction"); +} + +#[tokio::test] +async fn edit_workflow_requires_a_base() { + let tmp = TempDir::new().unwrap(); + let tool = EditWorkflowTool::new(test_config(&tmp)); + let result = tool + .execute(json!({ "ops": [ { "op": "remove_node", "id": "a" } ] })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("flow_id")); +} + +#[tokio::test] +async fn edit_workflow_reports_failing_op_with_guidance() { + let tmp = TempDir::new().unwrap(); + let tool = EditWorkflowTool::new(test_config(&tmp)); + let result = tool + .execute(json!({ + "graph": valid_graph(), + "ops": [ { "op": "remove_node", "id": "ghost" } ] + })) + .await + .unwrap(); + assert!(result.is_error); + let out = result.output(); + assert!(out.contains("remove_node"), "{out}"); + assert!(out.contains("edit_workflow again"), "{out}"); +} + +#[tokio::test] +async fn edit_workflow_rejects_a_result_that_is_structurally_invalid() { + let tmp = TempDir::new().unwrap(); + let tool = EditWorkflowTool::new(test_config(&tmp)); + // Removing the only trigger leaves the graph structurally invalid. + let result = tool + .execute(json!({ + "graph": valid_graph(), + "ops": [ { "op": "remove_node", "id": "t" } ] + })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("trigger"), "{}", result.output()); +} + +#[tokio::test] +async fn edit_workflow_edits_a_saved_flow_by_id() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + // Create a saved flow to edit. + let flow = ops::flows_create(&config, "Base flow".to_string(), valid_graph(), false) + .await + .unwrap() + .value; + + let tool = EditWorkflowTool::new(config.clone()); + let result = tool + .execute(json!({ + "flow_id": flow.id, + "ops": [ { "op": "set_node_name", "id": "a", "name": "Renamed step" } ] + })) + .await + .unwrap(); + + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + // Default name falls back to the base flow's name. + assert_eq!(parsed["name"], "Base flow"); + let nodes = parsed["graph"]["nodes"].as_array().unwrap(); + let agent = nodes.iter().find(|n| n["id"] == "a").unwrap(); + assert_eq!(agent["name"], "Renamed step"); +} + +// ── validate_workflow (F3: standalone check) ───────────────────────────────── + +#[tokio::test] +async fn validate_workflow_reports_ok_for_a_valid_graph() { + let tmp = TempDir::new().unwrap(); + let tool = ValidateWorkflowTool::new(test_config(&tmp)); + let result = tool + .execute(json!({ "graph": valid_graph() })) + .await + .unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["structurally_valid"], true); + assert_eq!(parsed["errors"].as_array().unwrap().len(), 0); + assert_eq!(parsed["gate_errors"].as_array().unwrap().len(), 0); +} + +#[tokio::test] +async fn validate_workflow_surfaces_all_structural_errors() { + let tmp = TempDir::new().unwrap(); + let tool = ValidateWorkflowTool::new(test_config(&tmp)); + // No trigger + a dangling edge. + let graph = json!({ + "nodes": [ { "id": "a", "kind": "agent", "name": "A", "config": { "prompt": "hi" } } ], + "edges": [ { "from_node": "a", "to_node": "ghost" } ] + }); + let result = tool.execute(json!({ "graph": graph })).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["ok"], false); + assert_eq!(parsed["structurally_valid"], false); + let codes: Vec<&str> = parsed["error_details"] + .as_array() + .unwrap() + .iter() + .map(|e| e["code"].as_str().unwrap()) + .collect(); + assert!(codes.contains(&"missing_trigger"), "{codes:?}"); + assert!(codes.contains(&"unknown_node"), "{codes:?}"); +} + +#[tokio::test] +async fn validate_workflow_requires_a_base() { + let tmp = TempDir::new().unwrap(); + let tool = ValidateWorkflowTool::new(test_config(&tmp)); + let result = tool.execute(json!({})).await.unwrap(); + assert!(result.is_error); + assert!(result.output().contains("flow_id")); +} + +#[tokio::test] +async fn edit_workflow_edits_a_draft_and_writes_back() { + use crate::openhuman::flows::DraftOrigin; + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + // A draft holding the base graph. + let draft = ops::flows_draft_create( + &config, + None, + "Draft flow".to_string(), + valid_graph(), + DraftOrigin::Chat, + ) + .unwrap() + .value; + + let tool = EditWorkflowTool::new(config.clone()); + let result = tool + .execute(json!({ + "draft_id": draft.id, + "ops": [ { "op": "add_node", "node": { "id": "b", "kind": "merge", "name": "Join" } } ] + })) + .await + .unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["draft_id"], draft.id); + assert_eq!(parsed["graph"]["nodes"].as_array().unwrap().len(), 3); + + // The edit was written back to the draft (survives for the next turn). + let reloaded = ops::flows_draft_get(&config, &draft.id).unwrap().value; + assert_eq!(reloaded.graph["nodes"].as_array().unwrap().len(), 3); +} + +// ── Phase 4: gated create / duplicate / debug loop (F4) ────────────────────── + +#[tokio::test] +async fn create_workflow_creates_a_disabled_flow() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let tool = CreateWorkflowTool::new(config.clone()); + // valid_graph has a manual trigger — flows_create would normally make it + // enabled; create_workflow must force it DISABLED. + let result = tool + .execute(json!({ "name": "Agent-made", "graph": valid_graph() })) + .await + .unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["type"], "workflow_created"); + assert_eq!(parsed["enabled"], false); + // Persisted and really disabled. + let flow_id = parsed["flow_id"].as_str().unwrap(); + let flow = ops::flows_get(&config, flow_id).await.unwrap().value; + assert!(!flow.enabled, "agent-created flows are born disabled"); +} + +#[tokio::test] +async fn create_workflow_rejects_an_invalid_graph() { + let tmp = TempDir::new().unwrap(); + let tool = CreateWorkflowTool::new(test_config(&tmp)); + let bad = json!({ + "nodes": [ { "id": "a", "kind": "output_parser", "name": "A" } ], + "edges": [] + }); + let result = tool + .execute(json!({ "name": "Bad", "graph": bad })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("create_workflow again")); +} + +#[tokio::test] +async fn duplicate_flow_creates_a_disabled_copy() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let flow = ops::flows_create(&config, "Original".to_string(), valid_graph(), false) + .await + .unwrap() + .value; + let tool = DuplicateFlowTool::new(config.clone()); + let result = tool.execute(json!({ "flow_id": flow.id })).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["type"], "workflow_duplicated"); + assert_eq!(parsed["enabled"], false); + assert_ne!(parsed["flow_id"].as_str().unwrap(), flow.id); +} + +#[tokio::test] +async fn list_flow_runs_is_empty_for_a_fresh_flow() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let flow = ops::flows_create(&config, "F".to_string(), valid_graph(), false) + .await + .unwrap() + .value; + let tool = ListFlowRunsTool::new(config.clone()); + let result = tool.execute(json!({ "flow_id": flow.id })).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!(parsed["runs"].as_array().unwrap().len(), 0); +} + +#[test] +fn phase4_write_tools_have_the_right_permissions() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + assert_eq!( + CreateWorkflowTool::new(config.clone()).permission_level(), + PermissionLevel::Write + ); + assert!(CreateWorkflowTool::new(config.clone()).external_effect()); + assert_eq!( + CancelFlowRunTool::new(config.clone()).permission_level(), + PermissionLevel::Write + ); + assert_eq!( + ResumeFlowRunTool::new(config.clone()).permission_level(), + PermissionLevel::Execute + ); + assert_eq!( + ListFlowRunsTool::new(config.clone()).permission_level(), + PermissionLevel::None + ); +} diff --git a/src/openhuman/flows/draft_store.rs b/src/openhuman/flows/draft_store.rs new file mode 100644 index 0000000000..1a53433f90 --- /dev/null +++ b/src/openhuman/flows/draft_store.rs @@ -0,0 +1,282 @@ +//! File-based persistence for [`FlowDraft`]s (audit F5). +//! +//! Drafts are plain JSON files under `{workspace_dir}/flows/drafts/.json`, +//! one file per draft — deliberately NOT a SQLite table (no schema/migration, +//! trivially inspectable and deletable). The draft is the shared working copy +//! the agent tools (Rust core) and the canvas both read/write by id across +//! turns and reloads, which rules out frontend-only `localStorage`. +//! +//! This module is the thin storage layer; business logic (promote → the +//! existing create/update gates) lives in [`super::ops`]. The same RPC contract +//! (`flows_draft_*`) would hold if drafts later migrate into a table. + +use std::path::PathBuf; + +use anyhow::{bail, Context, Result}; +use chrono::Utc; +use serde_json::Value; +use uuid::Uuid; + +use crate::openhuman::config::Config; +use crate::openhuman::flows::types::{DraftOrigin, FlowDraft}; + +/// The directory holding draft files, `{workspace_dir}/flows/drafts`. +fn drafts_dir(config: &Config) -> PathBuf { + config.workspace_dir.join("flows").join("drafts") +} + +/// Whether `id` is a safe draft-file stem — guards `get`/`update`/`delete` +/// against path traversal (`..`, separators) since the id reaches the +/// filesystem. Server-minted ids are UUIDs; this only accepts that shape. +fn is_safe_draft_id(id: &str) -> bool { + !id.is_empty() + && id.len() <= 64 + && id + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') +} + +/// The on-disk path for draft `id` (validated). +fn draft_path(config: &Config, id: &str) -> Result { + if !is_safe_draft_id(id) { + bail!("invalid draft id: {id:?}"); + } + Ok(drafts_dir(config).join(format!("{id}.json"))) +} + +/// Creates a new draft, writes it to disk, and returns it. +pub fn create_draft( + config: &Config, + flow_id: Option, + name: String, + graph: Value, + origin: DraftOrigin, +) -> Result { + let now = Utc::now().to_rfc3339(); + let draft = FlowDraft { + id: Uuid::new_v4().to_string(), + flow_id, + name, + graph, + origin, + created_at: now.clone(), + updated_at: now, + }; + write_draft(config, &draft)?; + tracing::debug!( + target: "flows", + draft_id = %draft.id, + origin = draft.origin.as_str(), + "[flows] draft_store: created draft" + ); + Ok(draft) +} + +/// Reads a draft by id, or `None` if no such file exists. +pub fn get_draft(config: &Config, id: &str) -> Result> { + let path = draft_path(config, id)?; + match std::fs::read(&path) { + Ok(bytes) => { + let draft: FlowDraft = + serde_json::from_slice(&bytes).with_context(|| format!("draft {id} is corrupt"))?; + Ok(Some(draft)) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).with_context(|| format!("reading draft {id}")), + } +} + +/// Patches a draft's mutable fields (any `Some` is applied), bumps +/// `updated_at`, persists, and returns the updated draft. Errors if the draft +/// does not exist. +pub fn update_draft( + config: &Config, + id: &str, + name: Option, + graph: Option, + flow_id: Option>, +) -> Result { + let mut draft = get_draft(config, id)?.with_context(|| format!("draft {id} not found"))?; + if let Some(name) = name { + draft.name = name; + } + if let Some(graph) = graph { + draft.graph = graph; + } + if let Some(flow_id) = flow_id { + draft.flow_id = flow_id; + } + draft.updated_at = Utc::now().to_rfc3339(); + write_draft(config, &draft)?; + tracing::debug!(target: "flows", draft_id = %id, "[flows] draft_store: updated draft"); + Ok(draft) +} + +/// Lists all drafts, newest-updated first. Skips (and logs) any corrupt file +/// rather than failing the whole listing. +pub fn list_drafts(config: &Config) -> Result> { + let dir = drafts_dir(config); + let entries = match std::fs::read_dir(&dir) { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(e).with_context(|| format!("listing drafts in {}", dir.display())), + }; + let mut drafts = Vec::new(); + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + match std::fs::read(&path).map(|b| serde_json::from_slice::(&b)) { + Ok(Ok(draft)) => drafts.push(draft), + Ok(Err(e)) => { + tracing::warn!(target: "flows", path = %path.display(), error = %e, "[flows] draft_store: skipping corrupt draft file"); + } + Err(e) => { + tracing::warn!(target: "flows", path = %path.display(), error = %e, "[flows] draft_store: could not read draft file"); + } + } + } + // Newest-updated first (RFC3339 with a fixed +00:00 offset sorts lexically). + drafts.sort_by(|a, b| b.updated_at.cmp(&a.updated_at)); + Ok(drafts) +} + +/// Deletes a draft file. Returns `true` if a file was removed, `false` if it +/// was already absent. +pub fn delete_draft(config: &Config, id: &str) -> Result { + let path = draft_path(config, id)?; + match std::fs::remove_file(&path) { + Ok(()) => { + tracing::debug!(target: "flows", draft_id = %id, "[flows] draft_store: deleted draft"); + Ok(true) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(e).with_context(|| format!("deleting draft {id}")), + } +} + +/// Serializes a draft to its file, creating the drafts dir if needed. Writes to +/// a temp file then renames, so a crash mid-write never leaves a corrupt draft. +fn write_draft(config: &Config, draft: &FlowDraft) -> Result<()> { + let dir = drafts_dir(config); + std::fs::create_dir_all(&dir) + .with_context(|| format!("creating drafts dir {}", dir.display()))?; + let path = draft_path(config, &draft.id)?; + let tmp = dir.join(format!(".{}.json.tmp", draft.id)); + let json = serde_json::to_vec_pretty(draft).context("serializing draft")?; + std::fs::write(&tmp, &json).with_context(|| format!("writing {}", tmp.display()))?; + std::fs::rename(&tmp, &path).with_context(|| format!("renaming into {}", path.display()))?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + use tempfile::TempDir; + + fn test_config(tmp: &TempDir) -> Config { + Config { + workspace_dir: tmp.path().join("workspace"), + action_dir: tmp.path().join("workspace"), + config_path: tmp.path().join("config.toml"), + ..Config::default() + } + } + + fn sample_graph() -> Value { + json!({ "nodes": [ { "id": "t", "kind": "trigger", "name": "Manual" } ], "edges": [] }) + } + + #[test] + fn create_get_roundtrip() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let draft = create_draft( + &config, + None, + "My draft".into(), + sample_graph(), + DraftOrigin::Chat, + ) + .unwrap(); + let loaded = get_draft(&config, &draft.id).unwrap().unwrap(); + assert_eq!(loaded, draft); + assert_eq!(loaded.name, "My draft"); + assert_eq!(loaded.origin, DraftOrigin::Chat); + assert!(loaded.flow_id.is_none()); + } + + #[test] + fn get_missing_is_none() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + assert!(get_draft(&config, "does-not-exist").unwrap().is_none()); + } + + #[test] + fn update_patches_fields_and_bumps_updated_at() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let draft = create_draft( + &config, + None, + "Old".into(), + sample_graph(), + DraftOrigin::Canvas, + ) + .unwrap(); + let updated = update_draft( + &config, + &draft.id, + Some("New name".into()), + Some(json!({ "nodes": [], "edges": [] })), + Some(Some("flow-42".into())), + ) + .unwrap(); + assert_eq!(updated.name, "New name"); + assert_eq!(updated.flow_id.as_deref(), Some("flow-42")); + assert_eq!(updated.graph["nodes"].as_array().unwrap().len(), 0); + assert!(updated.updated_at >= draft.updated_at); + assert_eq!(updated.created_at, draft.created_at); + } + + #[test] + fn list_returns_newest_first_and_delete_removes() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let a = create_draft(&config, None, "A".into(), sample_graph(), DraftOrigin::Chat).unwrap(); + // Bump a second draft's updated_at by updating it after creation. + let b = create_draft(&config, None, "B".into(), sample_graph(), DraftOrigin::Chat).unwrap(); + let b = update_draft(&config, &b.id, Some("B2".into()), None, None).unwrap(); + + let list = list_drafts(&config).unwrap(); + assert_eq!(list.len(), 2); + assert_eq!(list[0].id, b.id, "newest-updated first"); + + assert!(delete_draft(&config, &a.id).unwrap()); + assert!( + !delete_draft(&config, &a.id).unwrap(), + "second delete is a no-op" + ); + assert_eq!(list_drafts(&config).unwrap().len(), 1); + } + + #[test] + fn list_on_missing_dir_is_empty() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + assert!(list_drafts(&config).unwrap().is_empty()); + } + + #[test] + fn rejects_path_traversal_ids() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + assert!(get_draft(&config, "../secret").is_err()); + assert!(draft_path(&config, "a/b").is_err()); + assert!(draft_path(&config, "..").is_err()); + assert!(draft_path(&config, "ok-123_ID").is_ok()); + } +} diff --git a/src/openhuman/flows/mod.rs b/src/openhuman/flows/mod.rs index 1a79a53990..2eddc37cdc 100644 --- a/src/openhuman/flows/mod.rs +++ b/src/openhuman/flows/mod.rs @@ -11,7 +11,9 @@ pub mod agents; pub mod builder_tools; pub mod bus; pub mod discovery_tools; +mod draft_store; mod n8n_import; +pub mod node_contracts; pub mod ops; mod run_registry; mod schemas; @@ -32,8 +34,12 @@ pub use schemas::{ // live run observer (`tinyflows::observability::FlowRunObserver`, issue G2) // lives in the sibling `tinyflows` domain and persists each finished step onto // the `flow_runs` row through this function as the run executes. +pub use node_contracts::{ + all_node_kind_contracts, node_kind_contract, render_node_kinds_line, ConfigField, + NodeKindContract, PortSpec, NODE_KINDS, +}; pub use store::{kv_get, kv_set, upsert_flow_run_step}; pub use types::{ - Flow, FlowConnection, FlowImport, FlowRun, FlowRunStep, FlowRunTrigger, FlowSuggestion, - FlowValidation, SuggestionStatus, + DraftOrigin, Flow, FlowConnection, FlowDraft, FlowImport, FlowRevision, FlowRun, FlowRunStep, + FlowRunTrigger, FlowSuggestion, FlowValidation, FlowValidationError, SuggestionStatus, }; diff --git a/src/openhuman/flows/node_contracts.rs b/src/openhuman/flows/node_contracts.rs new file mode 100644 index 0000000000..e2e14ebd88 --- /dev/null +++ b/src/openhuman/flows/node_contracts.rs @@ -0,0 +1,199 @@ +//! Host overlay on the tinyflows node-kind catalog (P1.2 / audit finding F2). +//! +//! The portable, model-level contracts live in the tinyflows crate +//! ([`tinyflows::catalog`]) — config fields, ports, examples, and the +//! structural gotchas that are true of the DSL everywhere. This module is the +//! **thin host layer**: it takes those contracts and appends the facts only +//! *this* host knows — what a `tool_call` slug resolves to (a Composio action +//! slug or an `oh:` native tool), that a Composio result is wrapped in `data`, +//! how an `agent` node receives its data (`input_context`), and which trigger +//! kinds actually dispatch here. Keeping the vendor-specific knowledge here +//! preserves tinyflows' host-agnostic invariant. +//! +//! The `list_node_kinds` / `get_node_kind_contract` agent tools and the +//! `propose_workflow` description all read the *overlaid* view via +//! [`all_node_kind_contracts`] / [`node_kind_contract`]. + +pub use tinyflows::catalog::{ConfigField, NodeKindContract, PortSpec, NODE_KINDS}; + +/// Appends this host's vendor-specific caveats to a portable tinyflows +/// contract. One arm per kind that has host-owned facts; the rest pass through +/// unchanged. +fn apply_host_overlay(contract: NodeKindContract) -> NodeKindContract { + match contract.kind.as_str() { + "trigger" => contract + .with_note( + "In THIS host only manual / schedule / app_event actually dispatch today; the \ + other kinds save but never self-run (flows_validate warns).", + ) + .with_note( + "trigger_kind=app_event also needs config.toolkit + config.trigger_slug (the \ + Composio app + event to match).", + ), + "agent" => contract + .with_note( + "Data reaches the agent via config.input_context — an explicit =-binding: \ + \"=item\" (direct predecessor), \"=items\" (all inputs, for a fan-in), or \ + \"=nodes..item.json\" (a specific upstream node). The agent has NO automatic \ + access to the upstream item.", + ) + .with_note( + "config.prompt must be PLAIN natural-language text — no leading = and no .item \ + woven into the prose. A prompt written as a =expression built from prose silently \ + resolves to null and hands the agent an EMPTY prompt (rejected by the \ + binding-resolvability gate).", + ), + "tool_call" => contract + .with_note( + "config.slug is a real Composio action slug (from search_tool_catalog, e.g. \ + GMAIL_SEND_EMAIL) OR oh: for a native OpenHuman tool (e.g. \ + oh:web_search). A hallucinated/typo'd slug is a hard reject.", + ) + .with_note( + "Before wiring, call get_tool_contract { slug }: wire EVERY required_arg into \ + config.args using the input_schema's REAL property names (a guessed key is \ + rejected). Composio actions also need config.connection_ref for the account; oh: \ + tools do not.", + ) + .with_note( + "A Composio tool_call's output is wrapped in `data` (ComposioExecuteResponse) — \ + bind downstream as =nodes..item.json.data., NOT .item.json.. To \ + split_out over its result list, use get_tool_contract's primary_array_path \ + prefixed with `json.` (e.g. \"json.data.messages\").", + ), + "http_request" => contract.with_note( + "config.connection_ref is an http_cred: credential for authentication.", + ), + "code" => contract.with_note( + "A code node's output is NOT `data`-wrapped (unlike a Composio tool_call) — bind \ + downstream as =nodes..item.json..", + ), + "split_out" => contract.with_note( + "For a Composio source whose get_tool_contract primary_array_path is null, do NOT \ + default the path to \"json.data\" (that targets the whole payload container and \ + yields one item) — probe the real array path with get_tool_output_sample instead.", + ), + _ => contract, + } +} + +/// All 12 node-kind contracts with this host's overlay applied, in +/// [`NODE_KINDS`] order. +pub fn all_node_kind_contracts() -> Vec { + tinyflows::catalog::all_contracts() + .into_iter() + .map(apply_host_overlay) + .collect() +} + +/// The overlaid contract for one node kind, or `None` if `kind` is not one of +/// the 12. +pub fn node_kind_contract(kind: &str) -> Option { + tinyflows::catalog::contract_for(kind).map(apply_host_overlay) +} + +/// Renders the compact, one-line-per-kind node-kind enumeration used to keep +/// `propose_workflow`'s description honest against the typed contracts (drift +/// test). Format: `kind [required config.a/config.b; optional config.c] — +/// summary`, joined by ` | `. +pub fn render_node_kinds_line() -> String { + all_node_kind_contracts() + .iter() + .map(|c| { + let required: Vec<&str> = c + .config_fields + .iter() + .filter(|f| f.required) + .map(|f| f.name.as_str()) + .collect(); + let optional: Vec<&str> = c + .config_fields + .iter() + .filter(|f| !f.required) + .map(|f| f.name.as_str()) + .collect(); + let mut cfg = String::new(); + if !required.is_empty() { + cfg.push_str(&format!("required config.{}", required.join("/config."))); + } + if !optional.is_empty() { + if !cfg.is_empty() { + cfg.push_str("; "); + } + cfg.push_str(&format!("optional config.{}", optional.join("/config."))); + } + if cfg.is_empty() { + format!("{} ({})", c.kind, c.summary) + } else { + format!("{} [{}] — {}", c.kind, cfg, c.summary) + } + }) + .collect::>() + .join(" | ") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn overlay_preserves_all_12_kinds() { + assert_eq!(all_node_kind_contracts().len(), 12); + for kind in NODE_KINDS { + assert!(node_kind_contract(kind).is_some(), "missing {kind}"); + } + assert!(node_kind_contract("not_a_kind").is_none()); + } + + #[test] + fn tool_call_overlay_adds_host_composio_facts() { + let c = node_kind_contract("tool_call").unwrap(); + let notes = c.notes.join("\n"); + // Host facts that must NOT live in the portable crate. + assert!(notes.contains("Composio"), "{notes}"); + assert!(notes.contains("oh:"), "{notes}"); + assert!(notes.contains("data"), "{notes}"); + assert!(notes.contains("get_tool_contract"), "{notes}"); + } + + #[test] + fn agent_overlay_adds_input_context_guidance() { + let c = node_kind_contract("agent").unwrap(); + assert!(c.notes.iter().any(|n| n.contains("input_context"))); + } + + #[test] + fn trigger_overlay_names_the_host_dispatch_set() { + let c = node_kind_contract("trigger").unwrap(); + assert!(c.notes.iter().any(|n| n.contains("app_event"))); + } + + #[test] + fn merge_has_no_overlay_and_stays_portable() { + // A kind with no host facts is byte-identical to the portable contract. + assert_eq!( + node_kind_contract("merge").unwrap(), + tinyflows::catalog::contract_for("merge").unwrap() + ); + } + + #[test] + fn rendered_line_covers_every_kind_and_required_field() { + let line = render_node_kinds_line(); + for c in all_node_kind_contracts() { + assert!( + line.contains(&c.kind), + "rendered line missing kind {}", + c.kind + ); + for f in c.config_fields.iter().filter(|f| f.required) { + assert!( + line.contains(&format!("config.{}", f.name)), + "rendered line missing required field config.{} for {}", + f.name, + c.kind + ); + } + } + } +} diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 61ec81c24e..5f343e0ab3 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -13,6 +13,7 @@ use crate::openhuman::agent::turn_origin::{with_origin, AgentTurnOrigin, Trusted use crate::openhuman::approval::{FlowRunContext, APPROVAL_FLOW_RUN_CONTEXT}; use crate::openhuman::config::Config; use crate::openhuman::flows::bus; +use crate::openhuman::flows::draft_store; use crate::openhuman::flows::run_registry; use crate::openhuman::flows::store; use crate::openhuman::flows::types::{ @@ -87,12 +88,169 @@ const FLOW_PARKED_TTL_SECS: i64 = 600; /// which is what keeps the "the agent can never create a flow" invariant /// intact: this function validates and returns, it has no persistence effect. pub(crate) fn validate_and_migrate_graph(graph_json: Value) -> Result { + let graph = migrate_and_deserialize_graph(graph_json)?; + tinyflows::validate::validate(&graph).map_err(|e| e.to_string())?; + Ok(graph) +} + +/// Runs a raw graph JSON value through migration + deserialization **without** +/// the structural `validate` step. Splits the two so a caller that wants +/// *every* structural error (via `tinyflows::validate::validate_all`) can run +/// validation itself — a pre-validation failure here (unparseable JSON, an +/// unmigrateable schema) is genuinely a single error, whereas structural +/// validation can surface many at once. +pub(crate) fn migrate_and_deserialize_graph(graph_json: Value) -> Result { let migrated = tinyflows::migrate::migrate(graph_json).map_err(|e| e.to_string())?; let graph: WorkflowGraph = serde_json::from_value(migrated).map_err(|e| e.to_string())?; - tinyflows::validate::validate(&graph).map_err(|e| e.to_string())?; Ok(graph) } +/// Maps a portable `tinyflows` [`ValidationError`](tinyflows::error::ValidationError) +/// into the host's structured [`FlowValidationError`], carrying its stable +/// `code`, anchoring `node_id`, and human `message`. One place so the mapping +/// stays consistent across `flows_validate` and the builder gate stack. +pub(crate) fn to_flow_validation_error( + err: &tinyflows::error::ValidationError, +) -> crate::openhuman::flows::FlowValidationError { + crate::openhuman::flows::FlowValidationError { + code: err.code().to_string(), + message: err.to_string(), + node_id: err.node_id().map(str::to_string), + field: None, + } +} + +/// The single canonical definition of the builder hard-gate stack: the three +/// author-time gates that reject (not warn) a graph an agent must not propose +/// or persist — binding-resolvability, tool-contract, and required-arg +/// resolvability, in increasing cost order. +/// +/// Returns an empty `Vec` when the graph passes; otherwise the first failing +/// gate's node-level error messages (short-circuiting, so an expensive later +/// gate never runs on a graph already known to be broken). Every plane that +/// gates an agent-authored graph — `build_builder_proposal` (propose / revise / +/// edit), `save_workflow`, and the `strict` create/update RPC path — routes +/// through here, so they cannot drift (audit F3: agent saves and UI saves used +/// to validate differently). +/// +/// Assumes `graph` is already structurally valid (run +/// `validate_and_migrate_graph` / `validate_all` first) — these gates check +/// resolvability/contracts on a compilable graph. +pub(crate) async fn run_builder_gates(config: &Config, graph: &WorkflowGraph) -> Vec { + // Cheap, sync: a binding guaranteed to resolve null / wrong at runtime. + let binding_errors = validate_binding_resolvability(graph); + if !binding_errors.is_empty() { + return binding_errors; + } + // Async, live catalog: a tool_call whose slug isn't a real Composio action + // or whose real required args aren't all wired. + let contract_errors = validate_tool_contracts(config, graph).await; + if !contract_errors.is_empty() { + return contract_errors; + } + // Async, sandbox run: a required outbound arg that looks wired but resolves + // null in a mock execution. + validate_required_arg_resolvability(graph).await +} + +/// Strict-mode gate for the create/update RPC path (audit F3): validates +/// `graph_json` structurally (surfacing every error at once) and then runs the +/// same [`run_builder_gates`] the agent tools enforce, returning `Err` with a +/// combined, model-consumable message if anything fails. +/// +/// The UI/RPC create/update path stays permissive by default (a human editing +/// on the canvas may save a work-in-progress graph); passing `strict: true` +/// opts that call into the *same* gates an agent save must pass, so the two +/// planes converge on one definition instead of diverging. +pub(crate) async fn strict_gate(config: &Config, graph_json: &Value) -> Result<(), String> { + let graph = migrate_and_deserialize_graph(graph_json.clone())?; + let structural = tinyflows::validate::validate_all(&graph); + if !structural.is_empty() { + let messages: Vec = structural.iter().map(ToString::to_string).collect(); + return Err(format!( + "strict validation failed — the graph is structurally invalid:\n{}", + messages.join("\n") + )); + } + let gate_errors = run_builder_gates(config, &graph).await; + if !gate_errors.is_empty() { + return Err(format!( + "strict validation failed:\n{}", + gate_errors.join("\n\n") + )); + } + Ok(()) +} + +/// Runs the full builder hard-gate stack on an already structurally-valid +/// `graph` and, if it passes, builds the `workflow_proposal` payload the +/// propose/revise/edit tools all return. +/// +/// The single home for the gate sequence (binding-resolvability → +/// tool-contract → required-arg resolvability) plus summary/warning assembly, +/// so `revise_workflow` and `edit_workflow` cannot drift. `retry_tool` names +/// the tool in the "fix … and call `` again" guidance so each caller's +/// error text points the agent back at the right tool. +/// +/// Returns `Ok(payload)` on success, or `Err(message)` with a +/// model-consumable, fix-and-retry error when a gate rejects the graph. The +/// caller is responsible for structural validation (`validate_and_migrate_graph` +/// / `validate_all`) *before* calling this — these gates assume a compilable +/// graph. +pub(crate) async fn build_builder_proposal( + config: &Config, + retry_tool: &str, + name: &str, + graph: &WorkflowGraph, + require_approval: bool, + revision: bool, + instruction: Option, +) -> Result { + // The full builder hard-gate stack, run through the single canonical + // runner so every proposal/save/strict-RPC path gates identically (F3). + let gate_errors = run_builder_gates(config, graph).await; + if !gate_errors.is_empty() { + return Err(format!( + "{}\n\nFix these and call {retry_tool} again.", + gate_errors.join("\n\n") + )); + } + + let summary = crate::openhuman::flows::tools::build_summary(graph); + let mut warnings = graph_trigger_warnings(graph); + warnings.extend(graph_wiring_warnings(config, graph).await); + // Connector onboarding (Phase 5, item 18): tell the proposal card which + // toolkits this graph needs and whether they're connected, so it can render + // "Connect " CTAs instead of a bare gate error later. + let required_connections = compute_required_connections(config, graph).await; + let graph_value = serde_json::to_value(graph).map_err(|e| e.to_string())?; + + tracing::info!( + target: "flows", + %name, + node_count = graph.nodes.len(), + require_approval, + warning_count = warnings.len(), + revision, + "[flows] build_builder_proposal: proposal ready for user review" + ); + + let mut payload = json!({ + "type": "workflow_proposal", + "revision": revision, + "name": name, + "graph": graph_value, + "require_approval": require_approval, + "summary": summary, + "warnings": warnings, + "required_connections": required_connections, + }); + if let Some(instruction) = instruction { + payload["instruction"] = json!(instruction); + } + Ok(payload) +} + /// Stable snake_case label for a [`TriggerKind`], matching its serde wire /// discriminator — used in loud author-facing warnings (not derived via serde /// so the exact human string is unmistakable at the call site). @@ -1394,39 +1552,71 @@ fn is_trigger_scoped_expression( pub fn flows_validate(graph_json: Value) -> RpcOutcome { use crate::openhuman::flows::FlowValidation; tracing::debug!(target: "flows", "[flows] flows_validate: validating candidate graph"); - match validate_and_migrate_graph(graph_json) { - Ok(graph) => { - let warnings = graph_trigger_warnings(&graph); - for warning in &warnings { - tracing::warn!(target: "flows", warning = %warning, "[flows] flows_validate: non-fatal validation warning"); - } - tracing::debug!( - target: "flows", - node_count = graph.nodes.len(), - warning_count = warnings.len(), - "[flows] flows_validate: graph is structurally valid" - ); - RpcOutcome::single_log( - FlowValidation { - valid: true, - errors: Vec::new(), - warnings, - }, - "flow validated", - ) - } + // Split migrate/deserialize (a genuinely single failure) from structural + // validation (which can surface many problems at once). A pre-validation + // failure short-circuits with one error; a deserializable graph is then run + // through `validate_all` so the author sees every structural problem in one + // pass instead of one round-trip per error. + let graph = match migrate_and_deserialize_graph(graph_json) { + Ok(graph) => graph, Err(error) => { - tracing::debug!(target: "flows", %error, "[flows] flows_validate: graph is structurally invalid"); - RpcOutcome::single_log( + tracing::debug!(target: "flows", %error, "[flows] flows_validate: graph could not be migrated/parsed"); + return RpcOutcome::single_log( FlowValidation { valid: false, - errors: vec![error], + errors: vec![error.clone()], + error_details: vec![crate::openhuman::flows::FlowValidationError { + code: "unparseable_graph".to_string(), + message: error, + node_id: None, + field: None, + }], warnings: Vec::new(), }, "flow validation failed", - ) + ); } + }; + + let structural = tinyflows::validate::validate_all(&graph); + if !structural.is_empty() { + let error_details: Vec<_> = structural.iter().map(to_flow_validation_error).collect(); + let errors: Vec = error_details.iter().map(|e| e.message.clone()).collect(); + tracing::debug!( + target: "flows", + error_count = errors.len(), + "[flows] flows_validate: graph is structurally invalid" + ); + return RpcOutcome::single_log( + FlowValidation { + valid: false, + errors, + error_details, + warnings: Vec::new(), + }, + "flow validation failed", + ); + } + + let warnings = graph_trigger_warnings(&graph); + for warning in &warnings { + tracing::warn!(target: "flows", warning = %warning, "[flows] flows_validate: non-fatal validation warning"); } + tracing::debug!( + target: "flows", + node_count = graph.nodes.len(), + warning_count = warnings.len(), + "[flows] flows_validate: graph is structurally valid" + ); + RpcOutcome::single_log( + FlowValidation { + valid: true, + errors: Vec::new(), + error_details: Vec::new(), + warnings, + }, + "flow validated", + ) } /// Imports a workflow definition WITHOUT persisting it (PHASE 4d), normalizing @@ -1606,6 +1796,7 @@ pub async fn flows_create( ); } + publish_flow_changed(&flow.id, "created", "system"); Ok(RpcOutcome::new(flow, logs)) } @@ -1845,6 +2036,37 @@ fn title_case_toolkit(toolkit: &str) -> String { .join(" ") } +/// Publishes a [`DomainEvent::FlowChanged`](crate::core::event_bus::DomainEvent::FlowChanged) +/// so an open Workflows list/canvas refetches (bridged to a `flow:changed` +/// socket event) — the observability half of audit F6. Best-effort broadcast; +/// `actor` is a coarse hint (`"system"` for RPC-driven changes today). +fn publish_flow_changed(flow_id: &str, kind: &str, actor: &str) { + tracing::debug!(target: "flows", %flow_id, kind, actor, "[flows] publishing FlowChanged"); + crate::core::event_bus::publish_global(crate::core::event_bus::DomainEvent::FlowChanged { + flow_id: flow_id.to_string(), + kind: kind.to_string(), + actor: actor.to_string(), + }); +} + +/// Maps a store-level [`FlowUpdateError`](store::FlowUpdateError) to the RPC +/// error string. A concurrency conflict is encoded as a JSON object the UI can +/// parse (`{ code: "version_conflict", message, current }`) so it can offer a +/// reload/diff instead of silently clobbering; other variants are plain text. +fn map_flow_update_error(e: store::FlowUpdateError) -> String { + match e { + store::FlowUpdateError::NotFound => "flow not found".to_string(), + store::FlowUpdateError::Conflict(current) => serde_json::to_string(&json!({ + "code": "version_conflict", + "message": "This flow changed since you loaded it. Reload to see the latest \ + version, then reapply your change.", + "current": *current, + })) + .unwrap_or_else(|_| "version_conflict".to_string()), + store::FlowUpdateError::Store(err) => err.to_string(), + } +} + /// Updates a flow's name, graph, and/or `require_approval` toggle. /// Re-validates the graph (whether newly supplied or the existing one) /// before persisting, same as `flows_create`. @@ -1862,6 +2084,7 @@ pub async fn flows_update( name: Option, graph_json: Option, require_approval: Option, + expected_version: Option, ) -> Result, String> { let existing = store::get_flow(config, id) .map_err(|e| e.to_string())? @@ -1878,9 +2101,16 @@ pub async fn flows_update( } }; - tracing::debug!(target: "flows", flow_id = %id, "[flows] flows_update: persisting changes"); - let updated = store::update_flow_graph(config, id, new_name, graph, new_require_approval) - .map_err(|e| e.to_string())?; + tracing::debug!(target: "flows", flow_id = %id, has_expected = expected_version.is_some(), "[flows] flows_update: persisting changes"); + let updated = store::update_flow_graph( + config, + id, + new_name, + graph, + new_require_approval, + expected_version.as_deref(), + ) + .map_err(map_flow_update_error)?; if graph_changed && updated.enabled { let trigger_unchanged = bus::extract_trigger_kind(&existing) @@ -1893,12 +2123,54 @@ pub async fn flows_update( } } + publish_flow_changed(id, "updated", "system"); Ok(RpcOutcome::single_log( updated, format!("flow updated: {id}"), )) } +/// Lists a flow's revision history (prior graph snapshots), newest first, +/// capped at `limit` (audit F6). The safety rail that makes rollback possible. +pub fn flows_get_history( + config: &Config, + id: &str, + limit: usize, +) -> Result>, String> { + let revisions = store::list_revisions(config, id, limit).map_err(|e| e.to_string())?; + let count = revisions.len(); + Ok(RpcOutcome::single_log( + revisions, + format!("flow history: {id} ({count} revisions)"), + )) +} + +/// Rolls a flow back to a prior revision by restoring that revision's graph +/// through the normal update path — which itself snapshots the current graph as +/// a new revision, so a rollback is itself undoable. Honours optimistic +/// concurrency via `expected_version`. +pub async fn flows_rollback( + config: &Config, + id: &str, + revision_id: &str, + expected_version: Option, +) -> Result, String> { + let rev = store::revision_by_id(config, id, revision_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("revision '{revision_id}' not found for flow '{id}'"))?; + + tracing::debug!(target: "flows", flow_id = %id, %revision_id, "[flows] flows_rollback: restoring prior revision"); + flows_update( + config, + id, + Some(rev.name), + Some(rev.graph), + Some(rev.require_approval), + expected_version, + ) + .await +} + /// Deletes a flow by id. /// /// Unbinds the flow's automatic-dispatch trigger (e.g. the schedule-trigger @@ -1921,6 +2193,7 @@ pub async fn flows_delete(config: &Config, id: &str) -> Result store::remove_flow(config, id).map_err(|e| e.to_string())?; tracing::debug!(target: "flows", flow_id = %id, "[flows] flows_delete: removed"); + publish_flow_changed(id, "deleted", "system"); Ok(RpcOutcome::new( json!({ "id": id, "removed": true }), vec![format!("flow removed: {id}")], @@ -1975,6 +2248,7 @@ pub async fn flows_set_enabled( } } + publish_flow_changed(id, "enabled_changed", "system"); Ok(RpcOutcome::new(flow, logs)) } @@ -3588,6 +3862,271 @@ pub async fn flows_mark_suggestion_built( )) } +// ───────────────────────────────────────────────────────────────────────────── +// Connector onboarding (Phase 5, item 18) — which toolkits a graph needs +// ───────────────────────────────────────────────────────────────────────────── + +/// The set of Composio toolkits currently connected (lowercased), derived from +/// the same picker source the node-config credential dropdown uses. +pub(crate) async fn connected_toolkits(config: &Config) -> std::collections::HashSet { + match flows_list_connections(config).await { + Ok(outcome) => outcome + .value + .iter() + .filter_map(|c| c.toolkit.as_deref()) + .map(|t| t.to_ascii_lowercase()) + .collect(), + Err(e) => { + tracing::warn!(target: "flows", error = %e, "[flows] connected_toolkits: could not list connections — treating all as unconnected"); + std::collections::HashSet::new() + } + } +} + +/// The Composio toolkits a graph needs (from its `tool_call` slugs and any +/// `app_event` trigger), each tagged connected/missing — the data behind the +/// canvas/proposal "Connect " CTAs (audit Phase 5, item 18). Native +/// `oh:` tools and `http_request` nodes need no Composio connection and are +/// skipped. +pub async fn compute_required_connections(config: &Config, graph: &WorkflowGraph) -> Vec { + use crate::openhuman::memory_sync::composio::providers::toolkit_from_slug; + + // Collect required toolkits (deduped, order-preserving). + let mut required: Vec = Vec::new(); + let mut seen = std::collections::HashSet::new(); + let mut push = |tk: String| { + let tk = tk.to_ascii_lowercase(); + if !tk.is_empty() && seen.insert(tk.clone()) { + required.push(tk); + } + }; + + for node in &graph.nodes { + if node.kind == NodeKind::ToolCall { + if let Some(slug) = node.config.get("slug").and_then(Value::as_str) { + // Native OpenHuman tools (`oh:`) need no connection. + if slug.starts_with("oh:") { + continue; + } + if let Some(tk) = toolkit_from_slug(slug) { + push(tk.to_string()); + } + } + } + } + // An app_event trigger names its toolkit directly. + if let Some(trigger) = graph.trigger() { + if let Some(tk) = trigger.config.get("toolkit").and_then(Value::as_str) { + push(tk.to_string()); + } + } + + if required.is_empty() { + return Vec::new(); + } + + let connected = connected_toolkits(config).await; + required + .into_iter() + .map(|toolkit| { + let status = if connected.contains(&toolkit) { + "connected" + } else { + "missing" + }; + json!({ "toolkit": toolkit, "status": status }) + }) + .collect() +} + +/// RPC: compute the toolkits a candidate graph needs and their connected +/// status, so the canvas/proposal can render "Connect " CTAs. +pub async fn flows_required_connections( + config: &Config, + graph_json: Value, +) -> Result, String> { + let graph = migrate_and_deserialize_graph(graph_json)?; + let required = compute_required_connections(config, &graph).await; + Ok(RpcOutcome::single_log( + json!({ "required_connections": required }), + "required connections computed", + )) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Catalog RPCs for the UI (Phase 5, item 16) — one implementation, two consumers +// ───────────────────────────────────────────────────────────────────────────── + +/// Searches the live Composio tool catalog (secret-free) — the RPC the in-canvas +/// tool browser calls, reusing the exact same core as the agent's +/// `search_tool_catalog` tool so the two can't drift. +pub async fn flows_search_tool_catalog( + config: &Config, + query: &str, + toolkit: Option<&str>, + limit: usize, +) -> Result, String> { + tracing::debug!(target: "flows", %query, toolkit = toolkit.unwrap_or(""), "[flows] flows_search_tool_catalog: searching live catalog"); + let tools = + crate::openhuman::flows::builder_tools::search_live_catalog(config, query, toolkit, limit) + .await; + Ok(RpcOutcome::single_log( + json!({ "tools": tools }), + "tool catalog searched", + )) +} + +/// Fetches one Composio action's full contract (secret-free) — the RPC the +/// canvas tool browser calls to fill in an action's arg schema, reusing the same +/// core as the agent's `get_tool_contract` tool. +pub async fn flows_get_tool_contract( + config: &Config, + slug: &str, +) -> Result, String> { + let slug = slug.trim(); + let Some(toolkit) = crate::openhuman::memory_sync::composio::providers::toolkit_from_slug(slug) + else { + return Err(format!( + "Could not extract a toolkit from slug '{slug}' — it must look like \ + '_' (e.g. 'GMAIL_SEND_EMAIL')." + )); + }; + tracing::debug!(target: "flows", %slug, %toolkit, "[flows] flows_get_tool_contract: fetching contract"); + let Some(catalog) = + crate::openhuman::tinyflows::caps::fetch_live_toolkit_catalog(config, &toolkit).await + else { + return Err(format!( + "Could not fetch the live Composio catalog for toolkit '{toolkit}'." + )); + }; + match catalog.iter().find(|c| c.slug.eq_ignore_ascii_case(slug)) { + Some(contract) => { + let contract = + crate::openhuman::tinyflows::caps::apply_probe_override(contract.clone()); + let value = serde_json::to_value(&contract).map_err(|e| e.to_string())?; + Ok(RpcOutcome::single_log( + json!({ "contract": value }), + "tool contract fetched", + )) + } + None => Err(format!( + "'{slug}' is not a real action in the '{toolkit}' toolkit's live catalog." + )), + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Core-managed local drafts (F5) — the shared agent/canvas working copy +// ───────────────────────────────────────────────────────────────────────────── + +/// Creates a new draft (a durable, non-live working copy) from a graph. +pub fn flows_draft_create( + config: &Config, + flow_id: Option, + name: String, + graph: Value, + origin: crate::openhuman::flows::DraftOrigin, +) -> Result, String> { + let draft = draft_store::create_draft(config, flow_id, name, graph, origin) + .map_err(|e| e.to_string())?; + Ok(RpcOutcome::single_log(draft, "draft created")) +} + +/// Reads a draft by id (errors if it does not exist). +pub fn flows_draft_get( + config: &Config, + id: &str, +) -> Result, String> { + let draft = draft_store::get_draft(config, id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("draft '{id}' not found"))?; + Ok(RpcOutcome::single_log(draft, format!("draft loaded: {id}"))) +} + +/// Patches a draft's `name`/`graph`/`flow_id` (any `Some` applied) and bumps +/// `updated_at`. +pub fn flows_draft_update( + config: &Config, + id: &str, + name: Option, + graph: Option, + flow_id: Option>, +) -> Result, String> { + let draft = + draft_store::update_draft(config, id, name, graph, flow_id).map_err(|e| e.to_string())?; + Ok(RpcOutcome::single_log(draft, "draft updated")) +} + +/// Lists all drafts, newest-updated first. +pub fn flows_draft_list( + config: &Config, +) -> Result>, String> { + let drafts = draft_store::list_drafts(config).map_err(|e| e.to_string())?; + Ok(RpcOutcome::single_log(drafts, "drafts listed")) +} + +/// Deletes a draft by id (idempotent — reports whether a file was removed). +pub fn flows_draft_delete(config: &Config, id: &str) -> Result, String> { + let deleted = draft_store::delete_draft(config, id).map_err(|e| e.to_string())?; + Ok(RpcOutcome::single_log( + json!({ "id": id, "deleted": deleted }), + "draft deleted", + )) +} + +/// Promotes a draft into a saved flow, then removes the draft file. +/// +/// Runs the SAME create/update gates as a normal save (structural validation, +/// the forced `require_approval` floor for side-effect graphs, born-disabled +/// for automatic triggers) — a draft is never a back-door around them. A draft +/// with a `flow_id` updates that flow; otherwise it creates a new one. The +/// draft file is deleted only on a successful promote. +pub async fn flows_draft_promote( + config: &Config, + id: &str, + require_approval: Option, +) -> Result, String> { + let draft = draft_store::get_draft(config, id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("draft '{id}' not found"))?; + + tracing::debug!( + target: "flows", + draft_id = %id, + promotes_to = draft.flow_id.as_deref().unwrap_or(""), + "[flows] flows_draft_promote: promoting draft through the create/update gates" + ); + + let outcome = match &draft.flow_id { + Some(flow_id) => { + flows_update( + config, + flow_id, + Some(draft.name.clone()), + Some(draft.graph.clone()), + require_approval, + None, + ) + .await? + } + None => { + flows_create( + config, + draft.name.clone(), + draft.graph.clone(), + require_approval.unwrap_or(false), + ) + .await? + } + }; + + // Only remove the draft once the flow write succeeded. + if let Err(e) = draft_store::delete_draft(config, id) { + tracing::warn!(target: "flows", draft_id = %id, error = %e, "[flows] flows_draft_promote: flow saved but draft file could not be removed"); + } + Ok(outcome) +} + #[cfg(test)] #[path = "ops_tests.rs"] mod tests; diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 95f9939891..ee807f06cc 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -143,6 +143,7 @@ async fn flows_update_replaces_name_and_graph() { Some("renamed".to_string()), Some(new_graph), None, + None, ) .await .unwrap(); @@ -160,13 +161,13 @@ async fn flows_update_can_set_require_approval() { .unwrap(); assert!(!created.value.require_approval); - let updated = flows_update(&config, &created.value.id, None, None, Some(true)) + let updated = flows_update(&config, &created.value.id, None, None, Some(true), None) .await .unwrap(); assert!(updated.value.require_approval); // Omitting `require_approval` on a later update preserves the current value. - let unchanged = flows_update(&config, &created.value.id, None, None, None) + let unchanged = flows_update(&config, &created.value.id, None, None, None, None) .await .unwrap(); assert!(unchanged.value.require_approval); @@ -186,9 +187,16 @@ async fn flows_update_rejects_invalid_replacement_graph() { "edges": [] }); - let err = flows_update(&config, &created.value.id, None, Some(invalid_graph), None) - .await - .expect_err("invalid replacement graph must be rejected"); + let err = flows_update( + &config, + &created.value.id, + None, + Some(invalid_graph), + None, + None, + ) + .await + .expect_err("invalid replacement graph must be rejected"); assert!(err.contains("trigger")); } @@ -490,6 +498,7 @@ async fn flows_update_rebinds_schedule_cron_job_when_trigger_schedule_changes() None, Some(schedule_trigger_graph("30 8 * * *")), None, + None, ) .await .unwrap(); @@ -538,6 +547,7 @@ async fn flows_update_does_not_rebind_when_graph_is_not_supplied() { Some("renamed".to_string()), None, None, + None, ) .await .unwrap(); @@ -1461,6 +1471,76 @@ fn flows_validate_reports_error_for_graph_without_trigger() { ); } +#[test] +fn flows_validate_accumulates_every_structural_error() { + // A graph with several independent problems: no trigger, a duplicate node + // id, and a dangling edge. Multi-error validation must surface all of them + // in one call (fail-fast would report only the first). + let graph = json!({ + "name": "riddled", + "nodes": [ + { "id": "dup", "kind": "agent", "name": "One" }, + { "id": "dup", "kind": "agent", "name": "Two" } + ], + "edges": [ { "from_node": "dup", "to_node": "ghost" } ] + }); + let outcome = flows_validate(graph); + assert!(!outcome.value.valid); + // errors[] and error_details[] must be 1:1. + assert_eq!( + outcome.value.errors.len(), + outcome.value.error_details.len(), + "errors and error_details must be parallel: {:?} vs {:?}", + outcome.value.errors, + outcome.value.error_details + ); + assert!( + outcome.value.errors.len() >= 3, + "expected >=3 accumulated errors, got {:?}", + outcome.value.errors + ); + let codes: Vec<&str> = outcome + .value + .error_details + .iter() + .map(|e| e.code.as_str()) + .collect(); + assert!(codes.contains(&"missing_trigger"), "{codes:?}"); + assert!(codes.contains(&"duplicate_node_id"), "{codes:?}"); + assert!(codes.contains(&"unknown_node"), "{codes:?}"); + // A node-anchored error carries its node id; a graph-wide one does not. + let dup = outcome + .value + .error_details + .iter() + .find(|e| e.code == "duplicate_node_id") + .unwrap(); + assert_eq!(dup.node_id.as_deref(), Some("dup")); + let missing = outcome + .value + .error_details + .iter() + .find(|e| e.code == "missing_trigger") + .unwrap(); + assert_eq!(missing.node_id, None); +} + +#[test] +fn flows_validate_reports_unparseable_graph_as_single_error() { + // A pre-validation failure (an unknown node kind can't deserialize) is a + // genuine single error, not a structural-error accumulation. + let graph = json!({ + "name": "bad", + "nodes": [ { "id": "a", "kind": "not_a_real_kind", "name": "A" } ], + "edges": [] + }); + let outcome = flows_validate(graph); + assert!(!outcome.value.valid); + assert_eq!(outcome.value.errors.len(), 1); + assert_eq!(outcome.value.error_details.len(), 1); + assert_eq!(outcome.value.error_details[0].code, "unparseable_graph"); +} + #[tokio::test] async fn flows_set_enabled_surfaces_unfired_trigger_warning_at_enable() { let tmp = TempDir::new().unwrap(); @@ -3624,3 +3704,229 @@ fn trigger_is_automatic_no_trigger_kind() { let g = graph(trigger_only_graph()); assert!(!trigger_is_automatic(&g)); } + +#[tokio::test] +async fn strict_gate_passes_a_valid_graph_and_rejects_a_structurally_invalid_one() { + let config = Config::default(); + // A trigger-only graph is structurally valid and has no outbound gates. + assert!(strict_gate(&config, &trigger_only_graph()).await.is_ok()); + + // No trigger → structural failure surfaced by strict mode. + let bad = json!({ + "nodes": [ { "id": "a", "kind": "output_parser", "name": "A" } ], + "edges": [] + }); + let err = strict_gate(&config, &bad).await.unwrap_err(); + assert!(err.contains("structurally invalid"), "{err}"); + assert!(err.contains("trigger"), "{err}"); +} + +// ── core-managed drafts (F5) ───────────────────────────────────────────────── + +#[tokio::test] +async fn draft_promote_creates_a_new_flow_and_removes_the_draft() { + use crate::openhuman::flows::DraftOrigin; + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let draft = flows_draft_create( + &config, + None, + "From draft".to_string(), + trigger_only_graph(), + DraftOrigin::Chat, + ) + .unwrap() + .value; + + let flow = flows_draft_promote(&config, &draft.id, None) + .await + .unwrap() + .value; + assert_eq!(flow.name, "From draft"); + // The draft file is gone once promoted. + assert!(flows_draft_get(&config, &draft.id).is_err()); + // The flow really exists. + assert!(flows_get(&config, &flow.id).await.is_ok()); +} + +#[tokio::test] +async fn draft_promote_with_flow_id_updates_the_existing_flow() { + use crate::openhuman::flows::DraftOrigin; + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let flow = flows_create(&config, "Original".to_string(), trigger_only_graph(), false) + .await + .unwrap() + .value; + + let draft = flows_draft_create( + &config, + Some(flow.id.clone()), + "Renamed via draft".to_string(), + trigger_only_graph(), + DraftOrigin::Canvas, + ) + .unwrap() + .value; + + let updated = flows_draft_promote(&config, &draft.id, None) + .await + .unwrap() + .value; + assert_eq!(updated.id, flow.id, "same flow, not a new one"); + assert_eq!(updated.name, "Renamed via draft"); + assert!( + flows_draft_get(&config, &draft.id).is_err(), + "draft removed" + ); +} + +#[tokio::test] +async fn draft_promote_of_invalid_graph_is_rejected_and_keeps_the_draft() { + use crate::openhuman::flows::DraftOrigin; + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + // A graph with no trigger fails the create gate. + let bad = json!({ + "nodes": [ { "id": "a", "kind": "output_parser", "name": "A" } ], + "edges": [] + }); + let draft = flows_draft_create(&config, None, "Bad".to_string(), bad, DraftOrigin::Chat) + .unwrap() + .value; + + assert!(flows_draft_promote(&config, &draft.id, None).await.is_err()); + // The draft survives a failed promote so the user can fix it. + assert!(flows_draft_get(&config, &draft.id).is_ok()); +} + +// ── Phase 3: optimistic concurrency + revisions + rollback (F6) ─────────────── + +#[tokio::test] +async fn flows_update_rejects_a_stale_expected_version() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let flow = flows_create(&config, "V".to_string(), trigger_only_graph(), false) + .await + .unwrap() + .value; + + // A correct expected_version succeeds. + let ok = flows_update( + &config, + &flow.id, + Some("renamed".to_string()), + None, + None, + Some(flow.updated_at.clone()), + ) + .await + .unwrap(); + assert_eq!(ok.value.name, "renamed"); + + // The OLD version is now stale → conflict. + let err = flows_update( + &config, + &flow.id, + Some("again".to_string()), + None, + None, + Some(flow.updated_at.clone()), + ) + .await + .unwrap_err(); + assert!(err.contains("version_conflict"), "{err}"); + // The structured error carries the current flow. + let parsed: serde_json::Value = serde_json::from_str(&err).unwrap(); + assert_eq!(parsed["code"], "version_conflict"); + assert_eq!(parsed["current"]["name"], "renamed"); +} + +#[tokio::test] +async fn update_records_revisions_and_rollback_restores() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let flow = flows_create(&config, "Orig".to_string(), trigger_only_graph(), false) + .await + .unwrap() + .value; + + // Update the graph → the prior graph is snapshotted as a revision. + let two_node = json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "a", "kind": "agent", "name": "Step", "config": { "prompt": "hi" } } + ], + "edges": [ { "from_node": "t", "to_node": "a" } ] + }); + flows_update(&config, &flow.id, None, Some(two_node), None, None) + .await + .unwrap(); + + let history = flows_get_history(&config, &flow.id, 20).unwrap().value; + assert_eq!(history.len(), 1, "one prior snapshot"); + let rev = &history[0]; + // The snapshot holds the ORIGINAL (single-node trigger-only) graph. + assert_eq!(rev.graph["nodes"].as_array().unwrap().len(), 1); + + // Roll back → the flow returns to the single-node graph. + let rolled = flows_rollback(&config, &flow.id, &rev.id, None) + .await + .unwrap() + .value; + assert_eq!(rolled.graph.nodes.len(), 1); + + // Rollback is itself undoable — it snapshotted the pre-rollback (2-node) graph. + let history2 = flows_get_history(&config, &flow.id, 20).unwrap().value; + assert_eq!(history2.len(), 2); +} + +// ── Phase 5: connector onboarding (required_connections, item 18) ───────────── + +#[tokio::test] +async fn compute_required_connections_flags_missing_composio_toolkits() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + // A tool_call to a Gmail action (no connections in a fresh workspace). + let graph_json = json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "send", "kind": "tool_call", "name": "Send", + "config": { "slug": "GMAIL_SEND_EMAIL", "args": {} } } + ], + "edges": [ { "from_node": "t", "to_node": "send" } ] + }); + let graph = migrate_and_deserialize_graph(graph_json).unwrap(); + let required = compute_required_connections(&config, &graph).await; + assert_eq!(required.len(), 1); + assert_eq!(required[0]["toolkit"], "gmail"); + assert_eq!(required[0]["status"], "missing"); +} + +#[tokio::test] +async fn compute_required_connections_skips_native_and_http_nodes() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let graph_json = json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "search", "kind": "tool_call", "name": "Search", + "config": { "slug": "oh:web_search", "args": {} } }, + { "id": "http", "kind": "http_request", "name": "Fetch", + "config": { "method": "GET", "url": "https://example.com" } } + ], + "edges": [ + { "from_node": "t", "to_node": "search" }, + { "from_node": "search", "to_node": "http" } + ] + }); + let graph = migrate_and_deserialize_graph(graph_json).unwrap(); + let required = compute_required_connections(&config, &graph).await; + assert!( + required.is_empty(), + "native oh: and http_request need no connection: {required:?}" + ); +} diff --git a/src/openhuman/flows/schemas.rs b/src/openhuman/flows/schemas.rs index 0736d3a275..f82dbfacce 100644 --- a/src/openhuman/flows/schemas.rs +++ b/src/openhuman/flows/schemas.rs @@ -32,6 +32,15 @@ fn flow_output() -> FieldSchema { } } +fn draft_output() -> FieldSchema { + FieldSchema { + name: "draft", + ty: TypeSchema::Json, + comment: "The draft: { id, flow_id?, name, graph, origin, created_at, updated_at }.", + required: true, + } +} + /// Output field for the suggestion-returning controllers (`discover`, /// `list_suggestions`). Kept in one place so the schema mirrors /// `flows::types::FlowSuggestion`. @@ -167,6 +176,29 @@ fn require_approval_input() -> FieldSchema { } } +fn expected_version_input() -> FieldSchema { + FieldSchema { + name: "expected_version", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: + "Optimistic-concurrency token: the flow's `updated_at` as last observed. If the \ + flow has changed since, the write is refused with a structured version_conflict \ + error carrying the current flow, instead of clobbering. Omit for last-write-wins.", + required: false, + } +} + +fn strict_input() -> FieldSchema { + FieldSchema { + name: "strict", + ty: TypeSchema::Option(Box::new(TypeSchema::Bool)), + comment: "Run the same author hard-gates an agent save must pass (unresolvable bindings, \ + unreal tool slugs, unwired required args) before persisting, rejecting the \ + write if any fail. Defaults to `false` — the permissive human-canvas path.", + required: false, + } +} + fn run_output_fields() -> Vec { vec![ FieldSchema { @@ -255,6 +287,17 @@ pub fn all_controller_schemas() -> Vec { schemas("list_suggestions"), schemas("dismiss_suggestion"), schemas("mark_suggestion_built"), + schemas("draft_create"), + schemas("draft_get"), + schemas("draft_update"), + schemas("draft_list"), + schemas("draft_delete"), + schemas("draft_promote"), + schemas("get_history"), + schemas("rollback"), + schemas("search_tool_catalog"), + schemas("get_tool_contract"), + schemas("required_connections"), ] } @@ -344,6 +387,50 @@ pub fn all_registered_controllers() -> Vec { schema: schemas("mark_suggestion_built"), handler: handle_mark_suggestion_built, }, + RegisteredController { + schema: schemas("draft_create"), + handler: handle_draft_create, + }, + RegisteredController { + schema: schemas("draft_get"), + handler: handle_draft_get, + }, + RegisteredController { + schema: schemas("draft_update"), + handler: handle_draft_update, + }, + RegisteredController { + schema: schemas("draft_list"), + handler: handle_draft_list, + }, + RegisteredController { + schema: schemas("draft_delete"), + handler: handle_draft_delete, + }, + RegisteredController { + schema: schemas("draft_promote"), + handler: handle_draft_promote, + }, + RegisteredController { + schema: schemas("get_history"), + handler: handle_get_history, + }, + RegisteredController { + schema: schemas("rollback"), + handler: handle_rollback, + }, + RegisteredController { + schema: schemas("search_tool_catalog"), + handler: handle_search_tool_catalog, + }, + RegisteredController { + schema: schemas("get_tool_contract"), + handler: handle_get_tool_contract, + }, + RegisteredController { + schema: schemas("required_connections"), + handler: handle_required_connections, + }, ] } @@ -368,6 +455,7 @@ pub fn schemas(function: &str) -> ControllerSchema { required: true, }, require_approval_input(), + strict_input(), ], outputs: vec![flow_output()], }, @@ -516,6 +604,8 @@ pub fn schemas(function: &str) -> ControllerSchema { required: false, }, require_approval_input(), + strict_input(), + expected_version_input(), ], outputs: vec![flow_output()], }, @@ -867,6 +957,219 @@ pub fn schemas(function: &str) -> ControllerSchema { required: true, }], }, + "required_connections" => ControllerSchema { + namespace: "flows", + function: "required_connections", + description: "Compute which Composio toolkits a candidate graph needs and whether each \ + is connected — the data behind the canvas/proposal \"Connect \" \ + CTAs. Native oh: tools and http_request nodes need no connection.", + inputs: vec![FieldSchema { + name: "graph", + ty: TypeSchema::Json, + comment: "The WorkflowGraph to inspect.", + required: true, + }], + outputs: vec![FieldSchema { + name: "required_connections", + ty: TypeSchema::Array(Box::new(TypeSchema::Json)), + comment: "One per needed toolkit: { toolkit, status: connected|missing }.", + required: true, + }], + }, + "search_tool_catalog" => ControllerSchema { + namespace: "flows", + function: "search_tool_catalog", + description: "Search the live Composio tool catalog (secret-free) for the in-canvas \ + tool browser — the same core as the agent's search_tool_catalog tool.", + inputs: vec![ + FieldSchema { + name: "query", + ty: TypeSchema::String, + comment: "Keyword query matched against slug / toolkit / description.", + required: true, + }, + FieldSchema { + name: "toolkit", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "Restrict to one toolkit slug (e.g. `gmail`); omit to search all.", + required: false, + }, + FieldSchema { + name: "limit", + ty: TypeSchema::Option(Box::new(TypeSchema::U64)), + comment: "Max results (default 25).", + required: false, + }, + ], + outputs: vec![FieldSchema { + name: "tools", + ty: TypeSchema::Array(Box::new(TypeSchema::Json)), + comment: "Matches: { slug, toolkit, description, required_args, output_fields, primary_array_path, featured }.", + required: true, + }], + }, + "get_tool_contract" => ControllerSchema { + namespace: "flows", + function: "get_tool_contract", + description: "Fetch one Composio action's full contract (secret-free) for the canvas \ + tool browser — the same core as the agent's get_tool_contract tool.", + inputs: vec![FieldSchema { + name: "slug", + ty: TypeSchema::String, + comment: "The exact Composio action slug (e.g. `GMAIL_SEND_EMAIL`).", + required: true, + }], + outputs: vec![FieldSchema { + name: "contract", + ty: TypeSchema::Json, + comment: "The action contract: { slug, toolkit, description, required_args, input_schema, output_fields, output_schema, primary_array_path, is_curated }.", + required: true, + }], + }, + "get_history" => ControllerSchema { + namespace: "flows", + function: "get_history", + description: "List a flow's revision history — prior graph snapshots captured on each \ + update (capped, newest first). The safety rail behind rollback.", + inputs: vec![ + id_input("Identifier of the flow whose history to list."), + FieldSchema { + name: "limit", + ty: TypeSchema::Option(Box::new(TypeSchema::U64)), + comment: "Max revisions to return (defaults to the retention cap).", + required: false, + }, + ], + outputs: vec![FieldSchema { + name: "revisions", + ty: TypeSchema::Array(Box::new(TypeSchema::Json)), + comment: "Revision snapshots: { id, flow_id, graph, name, require_approval, created_at }.", + required: true, + }], + }, + "rollback" => ControllerSchema { + namespace: "flows", + function: "rollback", + description: "Roll a flow back to a prior revision (restores that revision's graph \ + through the normal update path — itself snapshotted, so rollback is \ + undoable). Honours optimistic concurrency via expected_version.", + inputs: vec![ + id_input("Identifier of the flow to roll back."), + FieldSchema { + name: "revision_id", + ty: TypeSchema::String, + comment: "The revision (from get_history) to restore.", + required: true, + }, + expected_version_input(), + ], + outputs: vec![flow_output()], + }, + "draft_create" => ControllerSchema { + namespace: "flows", + function: "draft_create", + description: "Create a core-managed draft (a durable, non-live working copy of a graph) \ + shared by the agent tools and the canvas. Never persists a flow.", + inputs: vec![ + FieldSchema { + name: "name", + ty: TypeSchema::String, + comment: "Human-readable draft name (carried into the flow on promote).", + required: true, + }, + FieldSchema { + name: "graph", + ty: TypeSchema::Json, + comment: "The (possibly incomplete) WorkflowGraph JSON to hold in the draft.", + required: true, + }, + FieldSchema { + name: "flow_id", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "The saved flow this draft edits, if any (promote → update vs create).", + required: false, + }, + FieldSchema { + name: "origin", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "Where the draft came from: `chat` | `canvas` | `import`. Defaults to `canvas`.", + required: false, + }, + ], + outputs: vec![draft_output()], + }, + "draft_get" => ControllerSchema { + namespace: "flows", + function: "draft_get", + description: "Fetch a draft by id.", + inputs: vec![id_input("Identifier of the draft to fetch.")], + outputs: vec![draft_output()], + }, + "draft_update" => ControllerSchema { + namespace: "flows", + function: "draft_update", + description: "Patch a draft's name/graph/flow_id (any provided field) and bump its \ + updated_at. Never persists a flow.", + inputs: vec![ + id_input("Identifier of the draft to update."), + FieldSchema { + name: "name", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "New name, if changing it.", + required: false, + }, + FieldSchema { + name: "graph", + ty: TypeSchema::Option(Box::new(TypeSchema::Json)), + comment: "New graph JSON, if changing it.", + required: false, + }, + FieldSchema { + name: "flow_id", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "New linked flow id, if changing it.", + required: false, + }, + ], + outputs: vec![draft_output()], + }, + "draft_list" => ControllerSchema { + namespace: "flows", + function: "draft_list", + description: "List all drafts, newest-updated first.", + inputs: vec![], + outputs: vec![FieldSchema { + name: "drafts", + ty: TypeSchema::Array(Box::new(TypeSchema::Json)), + comment: "The drafts (each { id, flow_id?, name, graph, origin, created_at, updated_at }).", + required: true, + }], + }, + "draft_delete" => ControllerSchema { + namespace: "flows", + function: "draft_delete", + description: "Delete a draft by id (idempotent).", + inputs: vec![id_input("Identifier of the draft to delete.")], + outputs: vec![FieldSchema { + name: "result", + ty: TypeSchema::Json, + comment: "`{ id, deleted }` — `deleted` is false if the id was already absent.", + required: true, + }], + }, + "draft_promote" => ControllerSchema { + namespace: "flows", + function: "draft_promote", + description: "Promote a draft into a saved flow through the same create/update gates \ + (structural validation, forced require_approval floor, born-disabled for \ + automatic triggers), then delete the draft file. A draft with a flow_id \ + updates that flow; otherwise it creates a new one.", + inputs: vec![ + id_input("Identifier of the draft to promote."), + require_approval_input(), + ], + outputs: vec![flow_output()], + }, _other => ControllerSchema { namespace: "flows", function: "unknown", @@ -896,6 +1199,16 @@ fn handle_create(params: Map) -> ControllerFuture { .get("require_approval") .and_then(Value::as_bool) .unwrap_or(false); + // Opt-in strict mode (F3): run the same author hard-gates an agent save + // must pass, before persisting. Default off — the human canvas save + // path stays permissive. + if params + .get("strict") + .and_then(Value::as_bool) + .unwrap_or(false) + { + ops::strict_gate(&config, &graph).await?; + } to_json(ops::flows_create(&config, name, graph, require_approval).await?) }) } @@ -964,7 +1277,33 @@ fn handle_update(params: Map) -> ControllerFuture { .map_err(|e| format!("invalid 'name': {e}"))?; let graph = params.get("graph").filter(|v| !v.is_null()).cloned(); let require_approval = params.get("require_approval").and_then(Value::as_bool); - to_json(ops::flows_update(&config, id.trim(), name, graph, require_approval).await?) + let expected_version = params + .get("expected_version") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_string); + // Opt-in strict mode (F3): when a new graph is supplied, run the same + // author hard-gates an agent save must pass, before persisting. + if params + .get("strict") + .and_then(Value::as_bool) + .unwrap_or(false) + { + if let Some(graph_json) = graph.as_ref() { + ops::strict_gate(&config, graph_json).await?; + } + } + to_json( + ops::flows_update( + &config, + id.trim(), + name, + graph, + require_approval, + expected_version, + ) + .await?, + ) }) } @@ -1142,6 +1481,146 @@ fn handle_mark_suggestion_built(params: Map) -> ControllerFuture }) } +fn handle_required_connections(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let graph = read_required::(¶ms, "graph")?; + to_json(ops::flows_required_connections(&config, graph).await?) + }) +} + +fn handle_search_tool_catalog(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let query = read_required::(¶ms, "query")?; + let toolkit = params + .get("toolkit") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()); + let limit = params + .get("limit") + .and_then(Value::as_u64) + .map(|n| n as usize) + .unwrap_or(25); + to_json(ops::flows_search_tool_catalog(&config, query.trim(), toolkit, limit).await?) + }) +} + +fn handle_get_tool_contract(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let slug = read_required::(¶ms, "slug")?; + to_json(ops::flows_get_tool_contract(&config, slug.trim()).await?) + }) +} + +fn handle_get_history(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + let limit = params + .get("limit") + .and_then(Value::as_u64) + .map(|n| n as usize) + .unwrap_or(20); + to_json(ops::flows_get_history(&config, id.trim(), limit)?) + }) +} + +fn handle_rollback(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + let revision_id = read_required::(¶ms, "revision_id")?; + let expected_version = params + .get("expected_version") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_string); + to_json( + ops::flows_rollback(&config, id.trim(), revision_id.trim(), expected_version).await?, + ) + }) +} + +fn handle_draft_create(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let name = read_required::(¶ms, "name")?; + let graph = read_required::(¶ms, "graph")?; + let flow_id = params + .get("flow_id") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_string); + let origin = params + .get("origin") + .and_then(Value::as_str) + .and_then(|s| serde_json::from_value(Value::String(s.to_string())).ok()) + .unwrap_or(crate::openhuman::flows::DraftOrigin::Canvas); + to_json(ops::flows_draft_create( + &config, flow_id, name, graph, origin, + )?) + }) +} + +fn handle_draft_get(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + to_json(ops::flows_draft_get(&config, id.trim())?) + }) +} + +fn handle_draft_update(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + let name = params + .get("name") + .filter(|v| !v.is_null()) + .map(|v| serde_json::from_value(v.clone())) + .transpose() + .map_err(|e| format!("invalid 'name': {e}"))?; + let graph = params.get("graph").filter(|v| !v.is_null()).cloned(); + // A present `flow_id` (even null) re-links the draft; absent leaves it. + let flow_id = params + .get("flow_id") + .map(|v| v.as_str().filter(|s| !s.is_empty()).map(str::to_string)); + to_json(ops::flows_draft_update( + &config, + id.trim(), + name, + graph, + flow_id, + )?) + }) +} + +fn handle_draft_list(_params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + to_json(ops::flows_draft_list(&config)?) + }) +} + +fn handle_draft_delete(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + to_json(ops::flows_draft_delete(&config, id.trim())?) + }) +} + +fn handle_draft_promote(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let id = read_required::(¶ms, "id")?; + let require_approval = params.get("require_approval").and_then(Value::as_bool); + to_json(ops::flows_draft_promote(&config, id.trim(), require_approval).await?) + }) +} + fn read_required(params: &Map, key: &str) -> Result { let value = params .get(key) @@ -1188,6 +1667,17 @@ mod tests { "list_suggestions", "dismiss_suggestion", "mark_suggestion_built", + "draft_create", + "draft_get", + "draft_update", + "draft_list", + "draft_delete", + "draft_promote", + "get_history", + "rollback", + "search_tool_catalog", + "get_tool_contract", + "required_connections", ] ); } @@ -1195,7 +1685,7 @@ mod tests { #[test] fn all_registered_controllers_has_handler_per_schema() { let controllers = all_registered_controllers(); - assert_eq!(controllers.len(), 21); + assert_eq!(controllers.len(), 32); let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect(); assert_eq!( names, @@ -1221,6 +1711,17 @@ mod tests { "list_suggestions", "dismiss_suggestion", "mark_suggestion_built", + "draft_create", + "draft_get", + "draft_update", + "draft_list", + "draft_delete", + "draft_promote", + "get_history", + "rollback", + "search_tool_catalog", + "get_tool_contract", + "required_connections", ] ); } diff --git a/src/openhuman/flows/store.rs b/src/openhuman/flows/store.rs index c684881016..a0f3b21583 100644 --- a/src/openhuman/flows/store.rs +++ b/src/openhuman/flows/store.rs @@ -15,7 +15,9 @@ //! `checkpoints.db` (see `src/openhuman/tinyflows/mod.rs::open_flow_checkpointer`). use crate::openhuman::config::Config; -use crate::openhuman::flows::types::{FlowRun, FlowRunStep, FlowSuggestion, SuggestionStatus}; +use crate::openhuman::flows::types::{ + FlowRevision, FlowRun, FlowRunStep, FlowSuggestion, SuggestionStatus, +}; use crate::openhuman::flows::Flow; use anyhow::{Context, Result}; use chrono::Utc; @@ -93,7 +95,18 @@ fn with_connection(config: &Config, f: impl FnOnce(&Connection) -> Result) source_run_id TEXT ); CREATE INDEX IF NOT EXISTS idx_flow_suggestions_status ON flow_suggestions(status); - CREATE INDEX IF NOT EXISTS idx_flow_suggestions_created_at ON flow_suggestions(created_at);", + CREATE INDEX IF NOT EXISTS idx_flow_suggestions_created_at ON flow_suggestions(created_at); + + CREATE TABLE IF NOT EXISTS flow_revisions ( + id TEXT PRIMARY KEY, + flow_id TEXT NOT NULL, + graph_json TEXT NOT NULL, + name TEXT NOT NULL, + require_approval INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL, + FOREIGN KEY (flow_id) REFERENCES flow_definitions(id) ON DELETE CASCADE + ); + CREATE INDEX IF NOT EXISTS idx_flow_revisions_flow_id ON flow_revisions(flow_id, created_at);", ) .context("Failed to initialize flows schema")?; @@ -324,38 +337,180 @@ pub fn set_enabled(config: &Config, id: &str, enabled: bool) -> Result { get_flow(config, id)?.ok_or_else(|| anyhow::anyhow!("flow '{id}' not found after update")) } -/// Replaces a flow's name/graph/`require_approval` (re-validated by the -/// caller before this is invoked) in place, bumping `updated_at`. +/// How many revision snapshots to retain per flow (audit F6). Older ones are +/// pruned on each new capture. +const MAX_REVISIONS_PER_FLOW: usize = 20; + +/// Failure modes of [`update_flow_graph`] that the caller must distinguish: +/// a genuine not-found, an optimistic-concurrency conflict (carrying the +/// current server flow so the UI can diff/reload), or a store error. +#[derive(Debug)] +pub enum FlowUpdateError { + /// No flow with that id exists. + NotFound, + /// The flow changed since `expected_updated_at` was observed — the write + /// was refused to avoid clobbering. Carries the current server flow. + Conflict(Box), + /// An underlying store failure. + Store(anyhow::Error), +} + +impl std::fmt::Display for FlowUpdateError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotFound => write!(f, "flow not found"), + Self::Conflict(_) => write!(f, "flow changed since it was loaded"), + Self::Store(e) => write!(f, "{e}"), + } + } +} + +/// Replaces a flow's name/graph/`require_approval` (re-validated by the caller +/// before this is invoked) in place, bumping `updated_at`, capturing the prior +/// graph as a revision, and enforcing optimistic concurrency. +/// +/// When `expected_updated_at` is `Some`, the write is refused with +/// [`FlowUpdateError::Conflict`] (carrying the current server flow) if the +/// flow's `updated_at` no longer matches — so an agent save and a concurrent +/// canvas save can't silently clobber each other. `None` keeps the prior +/// last-write-wins behaviour for callers that don't track a version. pub fn update_flow_graph( config: &Config, id: &str, name: String, graph: tinyflows::model::WorkflowGraph, require_approval: bool, -) -> Result { - let graph_json = serde_json::to_string(&graph).context("Failed to serialize graph")?; + expected_updated_at: Option<&str>, +) -> std::result::Result { + let current = get_flow(config, id) + .map_err(FlowUpdateError::Store)? + .ok_or(FlowUpdateError::NotFound)?; + + // Optimistic-concurrency check: refuse if the flow moved on since the + // caller observed `expected_updated_at`. + if let Some(expected) = expected_updated_at { + if current.updated_at != expected { + return Err(FlowUpdateError::Conflict(Box::new(current))); + } + } + + let graph_json = serde_json::to_string(&graph) + .context("Failed to serialize graph") + .map_err(FlowUpdateError::Store)?; + let prior_graph_json = + serde_json::to_string(¤t.graph).unwrap_or_else(|_| "null".to_string()); let now = Utc::now().to_rfc3339(); - // Targeted UPDATE of only the editable columns, so a concurrent - // `set_enabled` / `record_run` isn't clobbered by writing back a stale - // `enabled` / `last_run_at` / `last_status` from a read-modify-write. - let changed = with_connection(config, |conn| { + + with_connection(config, |conn| { + // Guarded UPDATE keyed on the observed updated_at (race-safe even + // without an explicit expected version) — a concurrent writer that + // moved updated_at makes this match 0 rows. Targeted columns only, so a + // concurrent set_enabled/record_run isn't clobbered. + let changed = conn + .execute( + "UPDATE flow_definitions SET name = ?1, graph_json = ?2, updated_at = ?3, \ + require_approval = ?4 WHERE id = ?5 AND updated_at = ?6", + params![ + name, + graph_json, + now, + if require_approval { 1 } else { 0 }, + id, + current.updated_at, + ], + ) + .context("Failed to update flow")?; + if changed == 0 { + // Someone raced us between the read and the write. + anyhow::bail!("__conflict__"); + } + // Capture the prior graph as a revision, then prune to the cap. + let rev_id = Uuid::new_v4().to_string(); conn.execute( - "UPDATE flow_definitions SET name = ?1, graph_json = ?2, updated_at = ?3, \ - require_approval = ?4 WHERE id = ?5", + "INSERT INTO flow_revisions (id, flow_id, graph_json, name, require_approval, \ + created_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6)", params![ - name, - graph_json, + rev_id, + id, + prior_graph_json, + current.name, + if current.require_approval { 1 } else { 0 }, now, - if require_approval { 1 } else { 0 }, - id ], ) - .context("Failed to update flow") + .context("Failed to record flow revision")?; + conn.execute( + "DELETE FROM flow_revisions WHERE flow_id = ?1 AND id NOT IN (\ + SELECT id FROM flow_revisions WHERE flow_id = ?1 \ + ORDER BY created_at DESC, id DESC LIMIT ?2)", + params![id, MAX_REVISIONS_PER_FLOW as i64], + ) + .context("Failed to prune flow revisions")?; + Ok(()) + }) + .map_err(|e| { + if e.to_string().contains("__conflict__") { + // Re-read to hand back the current state. + match get_flow(config, id) { + Ok(Some(f)) => FlowUpdateError::Conflict(Box::new(f)), + Ok(None) => FlowUpdateError::NotFound, + Err(e) => FlowUpdateError::Store(e), + } + } else { + FlowUpdateError::Store(e) + } })?; - if changed == 0 { - anyhow::bail!("flow '{id}' not found"); - } - get_flow(config, id)?.ok_or_else(|| anyhow::anyhow!("flow '{id}' not found")) + + get_flow(config, id) + .map_err(FlowUpdateError::Store)? + .ok_or(FlowUpdateError::NotFound) +} + +/// Lists a flow's revision snapshots, newest first, up to `limit`. +pub fn list_revisions(config: &Config, flow_id: &str, limit: usize) -> Result> { + with_connection(config, |conn| { + let mut stmt = conn.prepare( + "SELECT id, flow_id, graph_json, name, require_approval, created_at \ + FROM flow_revisions WHERE flow_id = ?1 ORDER BY created_at DESC, id DESC LIMIT ?2", + )?; + let rows = stmt + .query_map(params![flow_id, limit as i64], map_revision_row)? + .collect::>>()?; + Ok(rows) + }) +} + +/// Fetches one revision by id (scoped to `flow_id`), or `None`. +pub fn revision_by_id( + config: &Config, + flow_id: &str, + revision_id: &str, +) -> Result> { + with_connection(config, |conn| { + let mut stmt = conn.prepare( + "SELECT id, flow_id, graph_json, name, require_approval, created_at \ + FROM flow_revisions WHERE flow_id = ?1 AND id = ?2", + )?; + let mut rows = stmt.query_map(params![flow_id, revision_id], map_revision_row)?; + match rows.next() { + Some(row) => Ok(Some(row?)), + None => Ok(None), + } + }) +} + +fn map_revision_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let graph_str: String = row.get(2)?; + let graph: serde_json::Value = + serde_json::from_str(&graph_str).unwrap_or(serde_json::Value::Null); + Ok(FlowRevision { + id: row.get(0)?, + flow_id: row.get(1)?, + graph, + name: row.get(3)?, + require_approval: row.get::<_, i64>(4)? != 0, + created_at: row.get(5)?, + }) } /// Records the outcome of a `flows_run` invocation onto the flow's summary diff --git a/src/openhuman/flows/store_tests.rs b/src/openhuman/flows/store_tests.rs index cd73275ffe..8192cb0e73 100644 --- a/src/openhuman/flows/store_tests.rs +++ b/src/openhuman/flows/store_tests.rs @@ -91,8 +91,15 @@ fn update_flow_graph_bumps_updated_at_and_preserves_created_at() { let mut new_graph = trigger_graph(); new_graph.name = "renamed-graph".to_string(); - let updated = - update_flow_graph(&config, &flow.id, "renamed".to_string(), new_graph, false).unwrap(); + let updated = update_flow_graph( + &config, + &flow.id, + "renamed".to_string(), + new_graph, + false, + None, + ) + .unwrap(); assert_eq!(updated.name, "renamed"); assert_eq!(updated.created_at, flow.created_at); @@ -190,8 +197,15 @@ fn update_flow_graph_can_change_require_approval() { let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); assert!(!flow.require_approval); - let updated = - update_flow_graph(&config, &flow.id, flow.name.clone(), trigger_graph(), true).unwrap(); + let updated = update_flow_graph( + &config, + &flow.id, + flow.name.clone(), + trigger_graph(), + true, + None, + ) + .unwrap(); assert!(updated.require_approval); let reloaded = get_flow(&config, &flow.id).unwrap().unwrap(); diff --git a/src/openhuman/flows/tools_tests.rs b/src/openhuman/flows/tools_tests.rs index 3bd893121a..3ce228532e 100644 --- a/src/openhuman/flows/tools_tests.rs +++ b/src/openhuman/flows/tools_tests.rs @@ -319,3 +319,32 @@ async fn propose_workflow_rejects_unschemad_agent_binding() { "must name the missing schema as the fix: {output}" ); } + +/// Docs-drift guard (F2): `propose_workflow`'s hand-written description and the +/// typed node-kind contracts are two views of the SAME DSL, and they must not +/// diverge. If a node kind is added/renamed or a required config field changes +/// in `node_contracts.rs`, this fails until the tool description is updated to +/// match — the "prose can never diverge from code" check the plan calls for. +#[test] +fn propose_workflow_description_matches_typed_node_contracts() { + let tmp = TempDir::new().unwrap(); + let tool = ProposeWorkflowTool::new(test_config(&tmp)); + let desc = tool.description(); + for contract in crate::openhuman::flows::all_node_kind_contracts() { + assert!( + desc.contains(&contract.kind), + "propose_workflow description is missing node kind `{}` — update it to match \ + node_contracts.rs", + contract.kind + ); + for field in contract.config_fields.iter().filter(|f| f.required) { + assert!( + desc.contains(&field.name), + "propose_workflow description is missing required field `config.{}` of node kind \ + `{}` — update it to match node_contracts.rs", + field.name, + contract.kind + ); + } + } +} diff --git a/src/openhuman/flows/types.rs b/src/openhuman/flows/types.rs index 5c6837650f..30e872ca4b 100644 --- a/src/openhuman/flows/types.rs +++ b/src/openhuman/flows/types.rs @@ -6,6 +6,7 @@ //! struct is the OpenHuman-side record around it. use serde::{Deserialize, Serialize}; +use serde_json::Value; use tinyflows::model::WorkflowGraph; /// How a flow run was started. Stamped onto the run's Langfuse trace as a @@ -40,24 +41,62 @@ impl FlowRunTrigger { /// structural errors and non-fatal warnings (e.g. "this trigger kind never /// fires automatically yet") to an authoring surface *before* a flow is saved. /// -/// A graph is `valid` when it passes `tinyflows::validate::validate` after -/// migration; `errors` carries the single structural error when it does not. -/// `warnings` is orthogonal to validity — a `valid` graph can still carry -/// warnings (it saves and enables fine, it just won't behave as an author -/// might expect), and an invalid graph reports no warnings (there's nothing to -/// warn about a graph that won't compile). +/// A graph is `valid` when it passes `tinyflows::validate::validate_all` after +/// migration; `errors` carries **every** structural error when it does not (a +/// pre-validation failure — unparseable JSON or an unmigrateable schema — is +/// still a single entry). `warnings` is orthogonal to validity — a `valid` +/// graph can still carry warnings (it saves and enables fine, it just won't +/// behave as an author might expect), and an invalid graph reports no warnings +/// (there's nothing to warn about a graph that won't compile). #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct FlowValidation { /// True when the graph is structurally valid (migrates + validates). pub valid: bool, - /// Structural validation errors (empty when `valid`). Today at most one — - /// `tinyflows::validate::validate` returns the first error it hits. + /// Human-readable structural validation errors (empty when `valid`). As of + /// the multi-error work this carries **all** independent structural + /// problems in one pass — an author fixing five costs one validate call, + /// not five round-trips. See [`FlowValidation::error_details`] for the + /// machine-readable, per-node form. pub errors: Vec, + /// Structured, machine-readable counterpart to [`FlowValidation::errors`]: + /// one entry per structural error, carrying a stable `code`, the anchoring + /// `node_id` when node-specific, and the human `message`. Additive and + /// `#[serde(default)]` so existing clients that only read `errors` are + /// unaffected; agent tools and richer UIs consume this to attach errors to + /// the right node and switch on `code`. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub error_details: Vec, /// Non-fatal warnings: the graph is accepted, but something about it is /// worth flagging (e.g. an unfired trigger kind). Never blocks save/enable. pub warnings: Vec, } +/// A single structural validation error in machine-readable form — the +/// structured counterpart to a [`FlowValidation::errors`] string. +/// +/// Mirrors `tinyflows::error::ValidationError` (via its `code()` / `node_id()` +/// accessors) so a host surface can attach the error to a specific node and +/// switch on a stable `code` rather than parsing the `message`. `field` is +/// reserved for future config-level errors that can name the offending config +/// key; it is `None` for today's graph/edge-level checks. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +pub struct FlowValidationError { + /// Stable, machine-readable identifier for the error kind (e.g. + /// `missing_trigger`, `unknown_node`, `invalid_condition_routing`). + pub code: String, + /// Human-readable description — identical to the matching + /// [`FlowValidation::errors`] string. + pub message: String, + /// The node id this error is anchored to, when node-specific; `None` for + /// graph-wide errors (missing trigger, schema-too-new, multiple triggers). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub node_id: Option, + /// The offending config field, when the error is config-key-specific. + /// Reserved for future use; `None` today. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub field: Option, +} + /// The result of importing a workflow definition (native tinyflows JSON or an /// n8n export) via `openhuman.flows_import` (PHASE 4d) — the normalized, /// migrated + validated [`WorkflowGraph`] plus any non-fatal import warnings @@ -80,6 +119,84 @@ pub struct FlowImport { pub warnings: Vec, } +/// A snapshot of a flow's graph captured just before an update overwrote it — +/// the safety rail behind `flows_rollback` / `get_flow_history` (audit F6). +/// +/// Rows live in the `flow_revisions` table, capped (e.g. last 20 per flow). +/// `graph` is the prior graph as raw JSON so a snapshot never fails to load +/// even if the schema later evolves. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct FlowRevision { + /// Stable revision id (UUID). + pub id: String, + /// The flow this snapshot belongs to. + pub flow_id: String, + /// The flow's graph at the time this revision was captured (raw JSON). + pub graph: Value, + /// The flow's name at capture time. + pub name: String, + /// The flow's `require_approval` at capture time. + pub require_approval: bool, + /// RFC3339 time the snapshot was captured (i.e. when it was superseded). + pub created_at: String, +} + +/// Where a [`FlowDraft`] came from — carried through so the UI can label a +/// draft and the agent can reason about it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DraftOrigin { + /// Created from a chat/copilot build turn. + Chat, + /// Created from the canvas (e.g. "new workflow", or an accepted proposal). + Canvas, + /// Created from an import (native tinyflows JSON or an n8n export). + Import, +} + +impl DraftOrigin { + /// The serde wire discriminator, for logging. + pub fn as_str(&self) -> &'static str { + match self { + Self::Chat => "chat", + Self::Canvas => "canvas", + Self::Import => "import", + } + } +} + +/// A durable, core-managed **draft** of a workflow graph — the shared working +/// copy the agent tools and the canvas both read/write across turns and reloads +/// (audit F5). +/// +/// Stored as a plain JSON file on disk (`{workspace_dir}/flows/drafts/.json`), +/// not in SQLite — trivially inspectable and deletable, no schema/migration. +/// A draft is **never live**: promoting it (`flows_draft_promote`) runs the +/// existing `flows_create`/`flows_update` gates (same forced `require_approval` +/// floor, same human-in-the-loop) and removes the file. `graph` is a raw JSON +/// value (not a typed `WorkflowGraph`) because a work-in-progress draft is +/// explicitly allowed to be incomplete or not-yet-valid. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct FlowDraft { + /// Stable draft id (UUID). Distinct from any `flow_id`. + pub id: String, + /// The saved flow this draft edits, if any. `None` for a from-scratch draft + /// (promote → `flows_create`); `Some` for an edit of an existing flow + /// (promote → `flows_update`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub flow_id: Option, + /// Human-readable name carried into the flow on promote. + pub name: String, + /// The work-in-progress graph as raw JSON (may be incomplete/invalid). + pub graph: Value, + /// Where the draft originated. + pub origin: DraftOrigin, + /// RFC3339 creation time. + pub created_at: String, + /// RFC3339 last-update time. + pub updated_at: String, +} + /// A saved automation workflow: a `tinyflows` graph plus OpenHuman-side /// bookkeeping (enablement, run history summary). #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/openhuman/skills/tools.rs b/src/openhuman/skills/tools.rs index bd24747365..144a0b3bbd 100644 --- a/src/openhuman/skills/tools.rs +++ b/src/openhuman/skills/tools.rs @@ -402,14 +402,17 @@ impl WorkflowCreateTool { #[async_trait] impl Tool for WorkflowCreateTool { fn name(&self) -> &str { - "create_workflow" + // Renamed from `create_workflow` (audit F8): "workflow" now + // unambiguously means a Flows automation — this scaffolds a SKILL.md + // *skill*. The flows domain owns `create_workflow`. + "create_skill" } fn description(&self) -> &str { - "Scaffold a new workflow (SKILL.md, plus skill.toml when inputs are \ - declared). Requires `name` and `description`; optional `scope` \ - (user|project), `tags`, `allowed_tools`, and `inputs`. Use when the \ - user wants to capture a repeatable procedure as a packaged workflow." + "Scaffold a new SKILL.md skill (a packaged, repeatable procedure), plus skill.toml when \ + inputs are declared. Requires `name` and `description`; optional `scope` (user|project), \ + `tags`, `allowed_tools`, and `inputs`. NOTE: this creates a *skill*, not a Flows \ + automation workflow — use create_workflow for that." } fn parameters_schema(&self) -> serde_json::Value { @@ -436,9 +439,9 @@ impl Tool for WorkflowCreateTool { async fn execute(&self, args: serde_json::Value) -> anyhow::Result { log::debug!("[tool][skills] create invoked"); let params: CreateWorkflowParams = serde_json::from_value(args) - .map_err(|e| anyhow::anyhow!("create_workflow: invalid params: {e}"))?; + .map_err(|e| anyhow::anyhow!("create_skill: invalid params: {e}"))?; let skill = create_workflow(&self.workspace_dir, params) - .map_err(|e| anyhow::anyhow!("create_workflow: {e}"))?; + .map_err(|e| anyhow::anyhow!("create_skill: {e}"))?; Ok(ToolResult::success(serde_json::to_string(&skill)?)) } } diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index d96dac8688..95fb2736bb 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -286,6 +286,28 @@ pub fn all_tools_with_runtime( // read tools are `PermissionLevel::None`, and `dry_run_workflow` is // autonomy-tier gated + wired to deterministic mock capabilities. Box::new(ReviseWorkflowTool::new(config.clone())), + // Structured incremental edits (F1): apply a small ops[] list to a base + // graph (saved flow or inline) instead of re-emitting the whole graph, + // then validate + gate + return a proposal (same contract as revise). + // Proposal-only — never persists. + Box::new(EditWorkflowTool::new(config.clone())), + // Standalone validate (F3): run the SAME structural + hard-gate stack + // the propose/save tools use, without emitting a proposal — a pure + // check so the agent can self-verify a draft mid-build. Read-only. + Box::new(ValidateWorkflowTool::new(config.clone())), + // Read a saved flow's revision history (F6) — prior graph snapshots the + // agent can inspect / pick a rollback target from. Read-only. + Box::new(GetFlowHistoryTool::new(config.clone())), + // Phase 4 self-debug loop (F4): find a failing run, resume a parked + // run (approval-gated), or cancel a runaway one. + Box::new(ListFlowRunsTool::new(config.clone())), + Box::new(ResumeFlowRunTool::new(config.clone())), + Box::new(CancelFlowRunTool::new(config.clone())), + // Gated create (F4/F12): create a NEW flow — born disabled, approval + // gated — and duplicate an existing one (disabled copy) for + // clone-then-edit. Behind the Phase 3 safety rails. + Box::new(CreateWorkflowTool::new(config.clone())), + Box::new(DuplicateFlowTool::new(config.clone())), Box::new(ListFlowsTool::new(config.clone())), Box::new(GetFlowTool::new(config.clone())), Box::new(GetFlowRunTool::new(config.clone())), @@ -309,6 +331,15 @@ pub fn all_tools_with_runtime( // (researcher / code_executor / …) — the agent analogue of // search_tool_catalog. Read-only. Box::new(ListAgentProfilesTool::new()), + // Steer toolkit choice toward what's already connected + surface which + // toolkits a flow still needs (Phase 5, item 19). Read-only. + Box::new(ListConnectableToolkitsTool::new(config.clone())), + // Queryable DSL schema (F2): enumerate the 12 node kinds and fetch one + // kind's full config-field/port/example/gotcha contract — the DSL + // analogue of search_tool_catalog + get_tool_contract, so an agent need + // not rely on prompt prose or memory for node config shapes. Read-only. + Box::new(ListNodeKindsTool::new()), + Box::new(GetNodeKindContractTool::new()), Box::new(DryRunWorkflowTool::new(security.clone(), config.clone())), // Real end-to-end test run of a SAVED flow (Write / external-effect). The // workflow-builder prompt requires it to ask the user for confirmation @@ -1108,7 +1139,7 @@ fn tool_group(name: &str) -> crate::core::all::DomainGroup { "run_workflow", "await_workflow", "list_workflows", - "create_workflow", + "create_skill", "describe_workflow", "read_workflow_resource", "list_workflow_runs", diff --git a/src/openhuman/tools/ops_tests.rs b/src/openhuman/tools/ops_tests.rs index 1d762f46f3..3e1e0d5b7e 100644 --- a/src/openhuman/tools/ops_tests.rs +++ b/src/openhuman/tools/ops_tests.rs @@ -1743,7 +1743,7 @@ const KNOWLEDGE_TOOLS: &[&str] = &[ "read_workflow_resource", "list_workflow_runs", "read_workflow_run_log", - "create_workflow", + "create_skill", "install_workflow_from_url", "uninstall_workflow", "thread_list", @@ -1777,7 +1777,7 @@ const KNOWLEDGE_TOOLS: &[&str] = &[ const KNOWLEDGE_DEFAULT_OFF: &[&str] = &[ "people_refresh_address_book", - "create_workflow", + "create_skill", "install_workflow_from_url", "uninstall_workflow", "thread_delete", diff --git a/src/openhuman/tools/user_filter.rs b/src/openhuman/tools/user_filter.rs index f5089418f6..584c0bda77 100644 --- a/src/openhuman/tools/user_filter.rs +++ b/src/openhuman/tools/user_filter.rs @@ -167,7 +167,7 @@ const TOOL_FAMILIES: &[ToolFamily] = &[ ToolFamily { id: "workflow_manage", rust_names: &[ - "create_workflow", + "create_skill", "install_workflow_from_url", "uninstall_workflow", ], @@ -180,7 +180,7 @@ const TOOL_FAMILIES: &[ToolFamily] = &[ ToolFamily { id: "skill_manage", rust_names: &[ - "create_workflow", + "create_skill", "install_workflow_from_url", "uninstall_workflow", ], diff --git a/vendor/tinyflows b/vendor/tinyflows index be8ae65718..f18238904f 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit be8ae6571883d9ce2e8f23c11779b2ee04e00ce6 +Subproject commit f18238904f48c395dce99c0c1d759c3dd1bfb678