From 9902e8754d54b8e51c0bd83a2c042844aeb28f9e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 04:24:29 +0300 Subject: [PATCH 01/15] docs: audit of Flows agent-friendliness with phased improvement plan --- docs/flows-agent-friendliness-audit.md | 222 +++++++++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 docs/flows-agent-friendliness-audit.md diff --git a/docs/flows-agent-friendliness-audit.md b/docs/flows-agent-friendliness-audit.md new file mode 100644 index 0000000000..f575a26405 --- /dev/null +++ b/docs/flows-agent-friendliness-audit.md @@ -0,0 +1,222 @@ +# 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 — Server-side drafts (F5, and the real fix for F1's token cost) + +5. **Draft entity.** New `flow_drafts` table (id, optional `flow_id` it revises, name, + graph_json, created/updated, `origin: chat|canvas|import`). RPC: + `flows_draft_create/get/update/list/delete/promote` — `promote` runs the existing + create/update path (same gates, same forced `require_approval` floor). +6. **Agent tools on drafts.** `propose_workflow`/`revise_workflow`/`edit_workflow`/ + `dry_run_workflow` gain a `draft_id` mode: the graph lives server-side; 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 — Naming & prompt hygiene (F8) + +14. 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. +15. 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). + +### Sequencing & sizing + +| Phase | Depends on | Rough size | Risk | +|---|---|---|---| +| 1 (patch ops, schema tools, multi-error) | — | M–L (tinyflows crate change for multi-error) | Low | +| 2 (server drafts) | 1 helps | M | Low — additive tables/RPC | +| 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 (renames) | — | S | Low (needs deprecation window) | + +Each numbered item is a natural PR-sized slice; items 1–3 (Phase 1) deliver the biggest +immediate agent-experience win and require no changes to the human-in-the-loop model. + +--- + +## 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. From 9c9a0ca41d579f8aca8b517461eaf76c3ad3f16c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 04:26:29 +0300 Subject: [PATCH 02/15] docs: deliver the flows agent-friendliness plan as a single PR with phased commits --- docs/flows-agent-friendliness-audit.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/flows-agent-friendliness-audit.md b/docs/flows-agent-friendliness-audit.md index f575a26405..c46bf8d68d 100644 --- a/docs/flows-agent-friendliness-audit.md +++ b/docs/flows-agent-friendliness-audit.md @@ -197,7 +197,17 @@ safety rails are the prerequisite for widening agent write capabilities (F4, F7) behavior table with pointers to the Phase 1 introspection tools (generated docs keep parity). -### Sequencing & sizing +### Delivery: one PR, phased commits + +All five 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 | |---|---|---|---| @@ -207,8 +217,10 @@ safety rails are the prerequisite for widening agent write capabilities (F4, F7) | 4 (new agent tools) | 3 (safety rails) | S–M each | Medium — permission review each | | 5 (renames) | — | S | Low (needs deprecation window) | -Each numbered item is a natural PR-sized slice; items 1–3 (Phase 1) deliver the biggest -immediate agent-experience win and require no changes to the human-in-the-loop model. +Build within the branch in phase order (1 → 2 → 3 → 4 → 5): 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. --- From 432c0698d6742b07471312e679605b2c24feb2b1 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 04:27:49 +0300 Subject: [PATCH 03/15] docs: store flow drafts as core-managed local JSON files instead of a new table --- docs/flows-agent-friendliness-audit.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/flows-agent-friendliness-audit.md b/docs/flows-agent-friendliness-audit.md index c46bf8d68d..64958790e9 100644 --- a/docs/flows-agent-friendliness-audit.md +++ b/docs/flows-agent-friendliness-audit.md @@ -140,14 +140,23 @@ safety rails are the prerequisite for widening agent write capabilities (F4, F7) 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 — Server-side drafts (F5, and the real fix for F1's token cost) +### Phase 2 — Core-managed local drafts (F5, and the real fix for F1's token cost) -5. **Draft entity.** New `flow_drafts` table (id, optional `flow_id` it revises, name, - graph_json, created/updated, `origin: chat|canvas|import`). RPC: +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). + 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 server-side; tool calls + `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 @@ -212,7 +221,7 @@ releases. | Phase | Depends on | Rough size | Risk | |---|---|---|---| | 1 (patch ops, schema tools, multi-error) | — | M–L (tinyflows crate change for multi-error) | Low | -| 2 (server drafts) | 1 helps | M | Low — additive tables/RPC | +| 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 (renames) | — | S | Low (needs deprecation window) | From bd356f996dd9572e55707928ab0da24fb8db7ceb Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 04:31:23 +0300 Subject: [PATCH 04/15] =?UTF-8?q?docs:=20add=20builder=20UX=20phase=20?= =?UTF-8?q?=E2=80=94=20connector=20onboarding,=20Composio=20tool=20browser?= =?UTF-8?q?,=20workflow-chat=20tagging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/flows-agent-friendliness-audit.md | 61 +++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/docs/flows-agent-friendliness-audit.md b/docs/flows-agent-friendliness-audit.md index 64958790e9..710c667a4a 100644 --- a/docs/flows-agent-friendliness-audit.md +++ b/docs/flows-agent-friendliness-audit.md @@ -196,19 +196,65 @@ safety rails are the prerequisite for widening agent write capabilities (F4, F7) `run_flow` accept `draft_id` once drafts exist, still behind explicit user confirmation. -### Phase 5 — Naming & prompt hygiene (F8) - -14. Rename the legacy skills tools (`list_workflows`→`list_skills`, +### 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. -15. Shrink the workflow_builder prompt by replacing the node-kind reference and mock +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 five phases ship together as **a single PR**, not as separate PRs per item. The +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 @@ -224,9 +270,10 @@ releases. | 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 (renames) | — | S | Low (needs deprecation window) | +| 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): items 1–3 (Phase 1) deliver +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. From 7b59608e1b6a28b5a92e4e50e8d1ec3d849fd53d Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 04:58:08 +0300 Subject: [PATCH 05/15] feat(flows): multi-error validation (P1.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface every structural problem in a candidate graph in one flows_validate call instead of one round-trip per error — the #1 friction the audit (F3) calls out for an agent iterating on a flow. - tinyflows submodule: add validate_all() accumulating all independent structural errors; validate() is now its first element (fail-fast contract and all existing tests preserved). Add ValidationError::code()/node_id() for structured host reporting. (submodule commit 63ad5c4) - host: split migrate/deserialize (a genuine single failure) from structural validation; flows_validate runs validate_all and returns ALL errors. - add structured FlowValidationError {code, message, node_id, field}; expose as additive FlowValidation.error_details (serde default, back-compat — the existing errors: Vec now simply carries all messages). Refs #flows-agent-friendliness F3. --- src/openhuman/flows/mod.rs | 2 +- src/openhuman/flows/ops.rs | 110 +++++++++++++++++++++++-------- src/openhuman/flows/ops_tests.rs | 70 ++++++++++++++++++++ src/openhuman/flows/types.rs | 54 ++++++++++++--- vendor/tinyflows | 2 +- 5 files changed, 202 insertions(+), 36 deletions(-) diff --git a/src/openhuman/flows/mod.rs b/src/openhuman/flows/mod.rs index 1a79a53990..5fceb034f0 100644 --- a/src/openhuman/flows/mod.rs +++ b/src/openhuman/flows/mod.rs @@ -35,5 +35,5 @@ pub use schemas::{ pub use store::{kv_get, kv_set, upsert_flow_run_step}; pub use types::{ Flow, FlowConnection, FlowImport, FlowRun, FlowRunStep, FlowRunTrigger, FlowSuggestion, - FlowValidation, SuggestionStatus, + FlowValidation, FlowValidationError, SuggestionStatus, }; diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 61ec81c24e..953440adc7 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -87,12 +87,38 @@ 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, + } +} + /// 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 +1420,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 diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 95f9939891..8cb63315cb 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -1461,6 +1461,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(); diff --git a/src/openhuman/flows/types.rs b/src/openhuman/flows/types.rs index 5c6837650f..ee933cf4af 100644 --- a/src/openhuman/flows/types.rs +++ b/src/openhuman/flows/types.rs @@ -40,24 +40,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 diff --git a/vendor/tinyflows b/vendor/tinyflows index be8ae65718..63ad5c487f 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit be8ae6571883d9ce2e8f23c11779b2ee04e00ce6 +Subproject commit 63ad5c487fa3d307b64f8171f1251a35c6948241 From ac818912e003a0aeccff565d0dea462dc79bc785 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 05:19:47 +0300 Subject: [PATCH 06/15] feat(flows): queryable node-kind contracts + introspection tools (P1.2) Give agents a machine-readable DSL schema so they need not rely on prompt prose or memory for node config shapes (audit F2). - consume tinyflows::catalog (the portable, host-agnostic source of truth) and layer a thin host overlay adding only vendor facts: Composio slug semantics, oh: native tools, the data-envelope, agent input_context, and which trigger kinds actually dispatch here. - new read-only agent tools list_node_kinds + get_node_kind_contract{kind} (the DSL analogue of search_tool_catalog + get_tool_contract), on the workflow_builder belt. - docs-drift guard: a test ties propose_workflow's hand-written description to the typed contracts so prose can't silently diverge from code. Bumps tinyflows submodule to 9ccc141 (catalog module). Refs #flows-agent-friendliness F2. --- .../flows/agents/workflow_builder/agent.toml | 2 + src/openhuman/flows/builder_tools.rs | 156 ++++++++++++++ src/openhuman/flows/builder_tools_tests.rs | 41 ++++ src/openhuman/flows/mod.rs | 5 + src/openhuman/flows/node_contracts.rs | 199 ++++++++++++++++++ src/openhuman/flows/tools_tests.rs | 29 +++ src/openhuman/tools/ops.rs | 6 + vendor/tinyflows | 2 +- 8 files changed, 439 insertions(+), 1 deletion(-) create mode 100644 src/openhuman/flows/node_contracts.rs diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 6b09db83c9..d291f1dc2f 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -58,6 +58,8 @@ named = [ "get_tool_contract", "get_tool_output_sample", "list_agent_profiles", + "list_node_kinds", + "get_node_kind_contract", "dry_run_workflow", "run_flow", "composio_list_toolkits", diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index abc3fc3381..ba44650410 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -1064,6 +1064,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) // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index bd4e6dcb00..146ad02f57 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -1374,3 +1374,44 @@ 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); +} diff --git a/src/openhuman/flows/mod.rs b/src/openhuman/flows/mod.rs index 5fceb034f0..d713d28b69 100644 --- a/src/openhuman/flows/mod.rs +++ b/src/openhuman/flows/mod.rs @@ -12,6 +12,7 @@ pub mod builder_tools; pub mod bus; pub mod discovery_tools; mod n8n_import; +pub mod node_contracts; pub mod ops; mod run_registry; mod schemas; @@ -32,6 +33,10 @@ 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, 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/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/tools/ops.rs b/src/openhuman/tools/ops.rs index d96dac8688..b6633555e4 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -309,6 +309,12 @@ pub fn all_tools_with_runtime( // (researcher / code_executor / …) — the agent analogue of // search_tool_catalog. Read-only. Box::new(ListAgentProfilesTool::new()), + // 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 diff --git a/vendor/tinyflows b/vendor/tinyflows index 63ad5c487f..9ccc1413b4 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit 63ad5c487fa3d307b64f8171f1251a35c6948241 +Subproject commit 9ccc1413b407af45676d7a85b97dbbc5f1a90a68 From e1e1aa4fbe2b3181cb5cded8f812525dc4c358f4 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 05:34:58 +0300 Subject: [PATCH 07/15] =?UTF-8?q?feat(flows):=20edit=5Fworkflow=20?= =?UTF-8?q?=E2=80=94=20structured=20incremental=20graph=20edits=20(P1.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give the agent a cheap, low-regression iteration path (audit F1): change a saved or draft flow with a small ops[] list instead of re-emitting the whole graph (token-heavy, the top source of dropped-node/mangled-edge mistakes). - new edit_workflow agent tool: base = flow_id (saved) or inline graph; applies ops via tinyflows::graph_ops::apply_ops; runs validate_all (all structural errors at once) + the full hard-gate stack; returns the same workflow_proposal payload as revise_workflow. Proposal-only — never persists or enables. - extract ops::build_builder_proposal — the shared gate + summary/warning + payload logic — so revise_workflow and edit_workflow can't drift; keeps the host tool bodies thin. - bumps tinyflows submodule to bc68254 (graph_ops module). Refs #flows-agent-friendliness F1. --- .../flows/agents/workflow_builder/agent.toml | 1 + src/openhuman/flows/builder_tools.rs | 310 ++++++++++++++---- src/openhuman/flows/builder_tools_tests.rs | 124 +++++++ src/openhuman/flows/ops.rs | 84 +++++ src/openhuman/tools/ops.rs | 5 + vendor/tinyflows | 2 +- 6 files changed, 455 insertions(+), 71 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index d291f1dc2f..3a8697d770 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -49,6 +49,7 @@ hint = "reasoning" named = [ "propose_workflow", "revise_workflow", + "edit_workflow", "save_workflow", "list_flows", "get_flow", diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index ba44650410..407dd544b2 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -180,90 +180,260 @@ 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() { - tracing::debug!( - target: "flows", - %name, - error_count = binding_errors.len(), - "[flows] revise_workflow: binding-resolvability check rejected the revised graph" - ); - return Ok(ToolResult::error(format!( - "{}\n\nFix these bindings and call revise_workflow again.", - binding_errors.join("\n\n") - ))); + // 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)) + } } + } +} - // 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" - ); - return Ok(ToolResult::error(format!( - "{}\n\nFix these tool_call nodes and call revise_workflow again.", - contract_errors.join("\n\n") - ))); +// ───────────────────────────────────────────────────────────────────────────── +// 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 saved or draft flow. Provide the base (either \ + 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": { + "flow_id": { + "type": "string", + "description": "The saved flow to edit as the base graph. Provide this OR `graph`." + }, + "graph": { + "type": "object", + "description": "An inline base tinyflows WorkflowGraph to edit. Provide this OR `flow_id`.", + "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 either a saved flow or + // an inline graph (exactly one required). + 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 (base_graph, default_name) = match (flow_id, inline_graph) { + (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, 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) => { + return Ok(ToolResult::error( + "Provide either `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(), + )); } - // 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() { + 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." + ))); + } + }; + + // 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 = null_arg_errors.len(), - "[flows] revise_workflow: required-arg 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.", - null_arg_errors.join("\n\n") + "The edited graph is invalid:\n\n{}\n\nFix the ops and call edit_workflow again.", + messages.join("\n") ))); } - 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)?; - - tracing::info!( - target: "flows", - %name, - node_count = graph.nodes.len(), + // Full builder hard-gate stack + proposal payload (shared with revise). + match ops::build_builder_proposal( + &self.config, + "edit_workflow", + &name, + &edited, require_approval, - warning_count = warnings.len(), - "[flows] revise_workflow: revised proposal ready for user review" - ); - - 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); + true, + instruction, + ) + .await + { + Ok(payload) => 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)) + } } - - Ok(ToolResult::success(serde_json::to_string_pretty(&payload)?)) } } diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index 146ad02f57..48ac282b58 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -1415,3 +1415,127 @@ async fn get_node_kind_contract_tool_returns_contract_and_rejects_unknown() { 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"); +} diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 953440adc7..efcd62a9b5 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -119,6 +119,90 @@ pub(crate) fn to_flow_validation_error( } } +/// 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 { + // Binding-resolvability gate: reject (not warn) a binding guaranteed to + // resolve null / wrong at runtime. + let binding_errors = validate_binding_resolvability(graph); + if !binding_errors.is_empty() { + return Err(format!( + "{}\n\nFix these bindings and call {retry_tool} again.", + binding_errors.join("\n\n") + )); + } + + // Tool-contract gate: reject a tool_call whose slug isn't a real live + // 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 Err(format!( + "{}\n\nFix these tool_call nodes and call {retry_tool} again.", + contract_errors.join("\n\n") + )); + } + + // Required-arg resolvability gate: reject a required outbound arg that looks + // wired but resolves null in a sandbox run. + let null_arg_errors = validate_required_arg_resolvability(graph).await; + if !null_arg_errors.is_empty() { + return Err(format!( + "{}\n\nFix these bindings and call {retry_tool} again.", + null_arg_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); + 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, + }); + 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). diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index b6633555e4..3a01359c5b 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -286,6 +286,11 @@ 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())), Box::new(ListFlowsTool::new(config.clone())), Box::new(GetFlowTool::new(config.clone())), Box::new(GetFlowRunTool::new(config.clone())), diff --git a/vendor/tinyflows b/vendor/tinyflows index 9ccc1413b4..bc68254b07 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit 9ccc1413b407af45676d7a85b97dbbc5f1a90a68 +Subproject commit bc68254b0718385829b6fac2e9a16550f5443c65 From 9081f58252fee67ddb0e15118ae155ad21eeafca Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 05:46:26 +0300 Subject: [PATCH 08/15] feat(flows): unify validation planes + validate_workflow tool (P1.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the gap where agent saves and UI saves validated differently (audit F3), and give the agent a standalone check. - ops::run_builder_gates: the single canonical definition of the three author hard-gates (binding / tool-contract / required-arg). propose/revise/ edit (build_builder_proposal) and save_workflow now route through it — no more duplicated, drift-prone gate sequences. - new validate_workflow agent tool: runs the SAME structural + gate stack without emitting a proposal, returning { ok, structurally_valid, errors, error_details, gate_errors, warnings } so a check is fix-and-retry. - opt-in strict:bool on flows_create/flows_update RPC (ops::strict_gate): runs the agent gates before persisting. Default off keeps the human-canvas path permissive; on, the UI plane converges with the agent plane. Refs #flows-agent-friendliness F3. --- .../flows/agents/workflow_builder/agent.toml | 1 + src/openhuman/flows/builder_tools.rs | 179 ++++++++++++++---- src/openhuman/flows/builder_tools_tests.rs | 51 +++++ src/openhuman/flows/ops.rs | 94 ++++++--- src/openhuman/flows/ops_tests.rs | 16 ++ src/openhuman/flows/schemas.rs | 34 ++++ src/openhuman/tools/ops.rs | 4 + 7 files changed, 313 insertions(+), 66 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 3a8697d770..d8e0ee948a 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -50,6 +50,7 @@ named = [ "propose_workflow", "revise_workflow", "edit_workflow", + "validate_workflow", "save_workflow", "list_flows", "get_flow", diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 407dd544b2..d6bda7b364 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -437,6 +437,135 @@ impl Tool for EditWorkflowTool { } } +// ───────────────────────────────────────────────────────────────────────────── +// 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)?)) + } +} + // ───────────────────────────────────────────────────────────────────────────── // list_flows — read-only: saved flow summaries // ───────────────────────────────────────────────────────────────────────────── @@ -2103,51 +2232,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 diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index 48ac282b58..3e5b108daa 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -1539,3 +1539,54 @@ async fn edit_workflow_edits_a_saved_flow_by_id() { 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")); +} diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index efcd62a9b5..98356616a6 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -119,6 +119,68 @@ pub(crate) fn to_flow_validation_error( } } +/// 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. @@ -143,33 +205,13 @@ pub(crate) async fn build_builder_proposal( revision: bool, instruction: Option, ) -> Result { - // Binding-resolvability gate: reject (not warn) a binding guaranteed to - // resolve null / wrong at runtime. - let binding_errors = validate_binding_resolvability(graph); - if !binding_errors.is_empty() { - return Err(format!( - "{}\n\nFix these bindings and call {retry_tool} again.", - binding_errors.join("\n\n") - )); - } - - // Tool-contract gate: reject a tool_call whose slug isn't a real live - // 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 Err(format!( - "{}\n\nFix these tool_call nodes and call {retry_tool} again.", - contract_errors.join("\n\n") - )); - } - - // Required-arg resolvability gate: reject a required outbound arg that looks - // wired but resolves null in a sandbox run. - let null_arg_errors = validate_required_arg_resolvability(graph).await; - if !null_arg_errors.is_empty() { + // 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 bindings and call {retry_tool} again.", - null_arg_errors.join("\n\n") + "{}\n\nFix these and call {retry_tool} again.", + gate_errors.join("\n\n") )); } diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 8cb63315cb..faaca921d0 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3694,3 +3694,19 @@ 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}"); +} diff --git a/src/openhuman/flows/schemas.rs b/src/openhuman/flows/schemas.rs index 0736d3a275..67e8f9e3c0 100644 --- a/src/openhuman/flows/schemas.rs +++ b/src/openhuman/flows/schemas.rs @@ -167,6 +167,17 @@ fn require_approval_input() -> FieldSchema { } } +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 { @@ -368,6 +379,7 @@ pub fn schemas(function: &str) -> ControllerSchema { required: true, }, require_approval_input(), + strict_input(), ], outputs: vec![flow_output()], }, @@ -516,6 +528,7 @@ pub fn schemas(function: &str) -> ControllerSchema { required: false, }, require_approval_input(), + strict_input(), ], outputs: vec![flow_output()], }, @@ -896,6 +909,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,6 +987,17 @@ 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); + // 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).await?) }) } diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 3a01359c5b..a169a8adb2 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -291,6 +291,10 @@ pub fn all_tools_with_runtime( // 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())), Box::new(ListFlowsTool::new(config.clone())), Box::new(GetFlowTool::new(config.clone())), Box::new(GetFlowRunTool::new(config.clone())), From d9b430d4776bc02e689c90015e4ba2356ef84404 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 06:33:18 +0300 Subject: [PATCH 09/15] feat(flows): core-managed local JSON drafts (Phase 2, F5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A durable, core-managed draft the agent tools and the canvas both read/write by id across turns and reloads — replacing the client-only, reload-losing router-state draft. - FlowDraft stored as plain JSON at {workspace_dir}/flows/drafts/.json (no SQLite/migration; atomic temp-write+rename; path-traversal guarded). - RPC flows_draft_create/get/update/list/delete/promote. promote runs the SAME create/update gates (structural validation, forced require_approval floor, born-disabled auto-triggers) then removes the draft; never a back-door around them. - edit_workflow gains draft_id mode: base from the draft + the applied edit written back to it (the shared working copy). dry_run_workflow gains draft_id. flowsApi.ts draft client methods + FlowDraft/error_details types. Refs #flows-agent-friendliness F5. --- app/src/services/api/flowsApi.ts | 114 +++++++++ src/openhuman/flows/builder_tools.rs | 121 +++++++-- src/openhuman/flows/builder_tools_tests.rs | 35 +++ src/openhuman/flows/draft_store.rs | 282 +++++++++++++++++++++ src/openhuman/flows/mod.rs | 5 +- src/openhuman/flows/ops.rs | 112 ++++++++ src/openhuman/flows/ops_tests.rs | 82 ++++++ src/openhuman/flows/schemas.rs | 236 ++++++++++++++++- src/openhuman/flows/types.rs | 57 +++++ 9 files changed, 1022 insertions(+), 22 deletions(-) create mode 100644 src/openhuman/flows/draft_store.rs diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index 31bc2e8d03..086e785bfc 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` @@ -532,6 +567,85 @@ export async function importFlow( return result; } +// ── 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/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index d6bda7b364..73c6da42a4 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -233,8 +233,9 @@ impl Tool for EditWorkflowTool { 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 saved or draft flow. Provide the base (either \ - flow_id for a saved flow, or an inline graph) plus ops[]: a list of edits applied in \ + 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), \ @@ -249,13 +250,17 @@ impl Tool for EditWorkflowTool { 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 this OR `graph`." + "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 this OR `flow_id`.", + "description": "An inline base tinyflows WorkflowGraph to edit. Provide one of draft_id / flow_id / graph.", "properties": { "nodes": { "type": "array" }, "edges": { "type": "array" } @@ -294,8 +299,14 @@ impl Tool for EditWorkflowTool { } async fn execute(&self, args: Value) -> anyhow::Result { - // Resolve the base graph + a default name from either a saved flow or - // an inline graph (exactly one required). + // 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) @@ -303,8 +314,33 @@ impl Tool for EditWorkflowTool { .filter(|s| !s.is_empty()); let inline_graph = args.get("graph").filter(|v| !v.is_null()); - let (base_graph, default_name) = match (flow_id, inline_graph) { - (Some(id), _) => match ops::flows_get(&self.config, id).await { + // 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!( @@ -312,7 +348,7 @@ impl Tool for EditWorkflowTool { ))); } }, - (None, Some(graph_json)) => { + (None, None, Some(graph_json)) => { match ops::migrate_and_deserialize_graph(graph_json.clone()) { Ok(graph) => { let name = graph.name.clone(); @@ -325,10 +361,10 @@ impl Tool for EditWorkflowTool { } } } - (None, None) => { + (None, None, None) => { return Ok(ToolResult::error( - "Provide either `flow_id` (a saved flow) or `graph` (an inline base graph) to \ - edit." + "Provide one of `draft_id` (a working draft), `flow_id` (a saved flow), or \ + `graph` (an inline base graph) to edit." .to_string(), )); } @@ -400,6 +436,22 @@ impl Tool for EditWorkflowTool { } }; + // 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() { @@ -428,7 +480,12 @@ impl Tool for EditWorkflowTool { ) .await { - Ok(payload) => Ok(ToolResult::success(serde_json::to_string_pretty(&payload)?)), + 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)) @@ -1637,9 +1694,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" } @@ -1649,8 +1710,7 @@ impl Tool for DryRunWorkflowTool { "input": { "description": "Optional trigger input passed to the run (defaults to {})." } - }, - "required": ["graph"] + } }) } @@ -1683,9 +1743,32 @@ impl Tool for DryRunWorkflowTool { )); } - 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!({})); diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index 3e5b108daa..d4be4485e5 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -1590,3 +1590,38 @@ async fn validate_workflow_requires_a_base() { 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); +} 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 d713d28b69..f89dd5e924 100644 --- a/src/openhuman/flows/mod.rs +++ b/src/openhuman/flows/mod.rs @@ -11,6 +11,7 @@ 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; @@ -39,6 +40,6 @@ pub use node_contracts::{ }; pub use store::{kv_get, kv_set, upsert_flow_run_step}; pub use types::{ - Flow, FlowConnection, FlowImport, FlowRun, FlowRunStep, FlowRunTrigger, FlowSuggestion, - FlowValidation, FlowValidationError, SuggestionStatus, + DraftOrigin, Flow, FlowConnection, FlowDraft, FlowImport, FlowRun, FlowRunStep, FlowRunTrigger, + FlowSuggestion, FlowValidation, FlowValidationError, SuggestionStatus, }; diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 98356616a6..d2844e7ff9 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::{ @@ -3772,6 +3773,117 @@ pub async fn flows_mark_suggestion_built( )) } +// ───────────────────────────────────────────────────────────────────────────── +// 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, + ) + .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 faaca921d0..6934752488 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3710,3 +3710,85 @@ async fn strict_gate_passes_a_valid_graph_and_rejects_a_structurally_invalid_one 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()); +} diff --git a/src/openhuman/flows/schemas.rs b/src/openhuman/flows/schemas.rs index 67e8f9e3c0..f92f92adf8 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`. @@ -266,6 +275,12 @@ 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"), ] } @@ -355,6 +370,30 @@ 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, + }, ] } @@ -880,6 +919,111 @@ pub fn schemas(function: &str) -> ControllerSchema { required: true, }], }, + "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", @@ -1176,6 +1320,84 @@ fn handle_mark_suggestion_built(params: Map) -> ControllerFuture }) } +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) @@ -1222,6 +1444,12 @@ mod tests { "list_suggestions", "dismiss_suggestion", "mark_suggestion_built", + "draft_create", + "draft_get", + "draft_update", + "draft_list", + "draft_delete", + "draft_promote", ] ); } @@ -1229,7 +1457,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(), 27); let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect(); assert_eq!( names, @@ -1255,6 +1483,12 @@ mod tests { "list_suggestions", "dismiss_suggestion", "mark_suggestion_built", + "draft_create", + "draft_get", + "draft_update", + "draft_list", + "draft_delete", + "draft_promote", ] ); } diff --git a/src/openhuman/flows/types.rs b/src/openhuman/flows/types.rs index ee933cf4af..a2373c270a 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 @@ -118,6 +119,62 @@ pub struct FlowImport { pub warnings: Vec, } +/// 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)] From d3076f3816ee225c34cb345df6aec14551f5cc32 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 06:52:58 +0300 Subject: [PATCH 10/15] feat(flows): concurrency safety, mutation events, revision history (Phase 3, F6) The safety rails that make widening the agent's write surface (Phase 4) safe. - Optimistic concurrency: update_flow_graph reuses updated_at as an etag; flows_update/rollback take expected_version and return a structured version_conflict error (carrying the current server flow) instead of silently clobbering. flowsApi parseFlowVersionConflict + updateFlow wiring. - Flow mutation events: DomainEvent::FlowChanged published on create/update/delete/enable, bridged to a flow:changed socket event. New useFlowChanged hook; FlowsPage refetches on it so an agent save_workflow becomes visible instead of silently stale. - Revision history: flow_revisions table captures the prior graph on every update (capped at 20); flows_get_history + flows_rollback RPCs (rollback is itself snapshotted, so undoable); get_flow_history agent tool. Refs #flows-agent-friendliness F6. --- app/src/hooks/useFlowChanged.ts | 90 ++++++++ app/src/pages/FlowsPage.tsx | 12 ++ app/src/services/api/flowsApi.ts | 78 +++++++ src/core/event_bus/events.rs | 21 +- src/core/socketio.rs | 24 +++ .../flows/agents/workflow_builder/agent.toml | 1 + src/openhuman/flows/builder_tools.rs | 74 ++++++- src/openhuman/flows/mod.rs | 4 +- src/openhuman/flows/ops.rs | 91 +++++++- src/openhuman/flows/ops_tests.rs | 101 ++++++++- src/openhuman/flows/schemas.rs | 114 +++++++++- src/openhuman/flows/store.rs | 197 ++++++++++++++++-- src/openhuman/flows/store_tests.rs | 22 +- src/openhuman/flows/types.rs | 22 ++ src/openhuman/tools/ops.rs | 3 + 15 files changed, 815 insertions(+), 39 deletions(-) create mode 100644 app/src/hooks/useFlowChanged.ts diff --git a/app/src/hooks/useFlowChanged.ts b/app/src/hooks/useFlowChanged.ts new file mode 100644 index 0000000000..2abbd1839f --- /dev/null +++ b/app/src/hooks/useFlowChanged.ts @@ -0,0 +1,90 @@ +/** + * 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..07eef75e24 100644 --- a/app/src/pages/FlowsPage.tsx +++ b/app/src/pages/FlowsPage.tsx @@ -21,6 +21,7 @@ import FlowTemplateGallery from '../components/flows/FlowTemplateGallery'; import NewWorkflowModal from '../components/flows/NewWorkflowModal'; import SuggestedWorkflows from '../components/flows/SuggestedWorkflows'; import { useCreateFlow } from '../components/flows/useCreateFlow'; +import { useFlowChanged } from '../hooks/useFlowChanged'; import WorkflowPromptBar from '../components/flows/WorkflowPromptBar'; import { ToastContainer } from '../components/intelligence/Toast'; import PageSectionHeader from '../components/layout/PageSectionHeader'; @@ -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 086e785bfc..a90b965168 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -220,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`). */ @@ -499,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 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 d8e0ee948a..48f5b520c2 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -54,6 +54,7 @@ named = [ "save_workflow", "list_flows", "get_flow", + "get_flow_history", "get_flow_run", "list_flow_connections", "search_tool_catalog", diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 73c6da42a4..e01e905d8a 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -623,6 +623,78 @@ impl Tool for ValidateWorkflowTool { } } +// ───────────────────────────────────────────────────────────────────────────── +// 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}" + ))), + } + } +} + // ───────────────────────────────────────────────────────────────────────────── // list_flows — read-only: saved flow summaries // ───────────────────────────────────────────────────────────────────────────── @@ -2346,7 +2418,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/mod.rs b/src/openhuman/flows/mod.rs index f89dd5e924..2eddc37cdc 100644 --- a/src/openhuman/flows/mod.rs +++ b/src/openhuman/flows/mod.rs @@ -40,6 +40,6 @@ pub use node_contracts::{ }; pub use store::{kv_get, kv_set, upsert_flow_run_step}; pub use types::{ - DraftOrigin, Flow, FlowConnection, FlowDraft, FlowImport, FlowRun, FlowRunStep, FlowRunTrigger, - FlowSuggestion, FlowValidation, FlowValidationError, SuggestionStatus, + DraftOrigin, Flow, FlowConnection, FlowDraft, FlowImport, FlowRevision, FlowRun, FlowRunStep, + FlowRunTrigger, FlowSuggestion, FlowValidation, FlowValidationError, SuggestionStatus, }; diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index d2844e7ff9..05e7430115 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -1791,6 +1791,7 @@ pub async fn flows_create( ); } + publish_flow_changed(&flow.id, "created", "system"); Ok(RpcOutcome::new(flow, logs)) } @@ -2030,6 +2031,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`. @@ -2047,6 +2079,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())? @@ -2063,9 +2096,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) @@ -2078,12 +2118,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 @@ -2106,6 +2188,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}")], @@ -2160,6 +2243,7 @@ pub async fn flows_set_enabled( } } + publish_flow_changed(id, "enabled_changed", "system"); Ok(RpcOutcome::new(flow, logs)) } @@ -3863,6 +3947,7 @@ pub async fn flows_draft_promote( Some(draft.name.clone()), Some(draft.graph.clone()), require_approval, + None, ) .await? } diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 6934752488..30f73edcea 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(); @@ -3792,3 +3802,84 @@ async fn draft_promote_of_invalid_graph_is_rejected_and_keeps_the_draft() { // 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); +} diff --git a/src/openhuman/flows/schemas.rs b/src/openhuman/flows/schemas.rs index f92f92adf8..a40535e79f 100644 --- a/src/openhuman/flows/schemas.rs +++ b/src/openhuman/flows/schemas.rs @@ -176,6 +176,18 @@ 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", @@ -281,6 +293,8 @@ pub fn all_controller_schemas() -> Vec { schemas("draft_list"), schemas("draft_delete"), schemas("draft_promote"), + schemas("get_history"), + schemas("rollback"), ] } @@ -394,6 +408,14 @@ pub fn all_registered_controllers() -> Vec { schema: schemas("draft_promote"), handler: handle_draft_promote, }, + RegisteredController { + schema: schemas("get_history"), + handler: handle_get_history, + }, + RegisteredController { + schema: schemas("rollback"), + handler: handle_rollback, + }, ] } @@ -568,6 +590,7 @@ pub fn schemas(function: &str) -> ControllerSchema { }, require_approval_input(), strict_input(), + expected_version_input(), ], outputs: vec![flow_output()], }, @@ -919,6 +942,45 @@ pub fn schemas(function: &str) -> ControllerSchema { 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", @@ -1131,6 +1193,11 @@ 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); + 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 @@ -1142,7 +1209,17 @@ fn handle_update(params: Map) -> ControllerFuture { ops::strict_gate(&config, graph_json).await?; } } - to_json(ops::flows_update(&config, id.trim(), name, graph, require_approval).await?) + to_json( + ops::flows_update( + &config, + id.trim(), + name, + graph, + require_approval, + expected_version, + ) + .await?, + ) }) } @@ -1320,6 +1397,35 @@ fn handle_mark_suggestion_built(params: Map) -> ControllerFuture }) } +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?; @@ -1450,6 +1556,8 @@ mod tests { "draft_list", "draft_delete", "draft_promote", + "get_history", + "rollback", ] ); } @@ -1457,7 +1565,7 @@ mod tests { #[test] fn all_registered_controllers_has_handler_per_schema() { let controllers = all_registered_controllers(); - assert_eq!(controllers.len(), 27); + assert_eq!(controllers.len(), 29); let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect(); assert_eq!( names, @@ -1489,6 +1597,8 @@ mod tests { "draft_list", "draft_delete", "draft_promote", + "get_history", + "rollback", ] ); } 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/types.rs b/src/openhuman/flows/types.rs index a2373c270a..30e872ca4b 100644 --- a/src/openhuman/flows/types.rs +++ b/src/openhuman/flows/types.rs @@ -119,6 +119,28 @@ 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)] diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index a169a8adb2..15a231feb5 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -295,6 +295,9 @@ pub fn all_tools_with_runtime( // 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())), Box::new(ListFlowsTool::new(config.clone())), Box::new(GetFlowTool::new(config.clone())), Box::new(GetFlowRunTool::new(config.clone())), From 8aed34c27b857b8dcb5b690c5e0d604c437c2f4f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 07:02:07 +0300 Subject: [PATCH 11/15] feat(flows): widen agent capabilities behind the Phase 3 rails (Phase 4, F4/F7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that versioning/events/history make agent writes safe and visible, open the self-debug loop and gated authoring — all approval/permission gated. - self-debug loop: list_flow_runs (read), resume_flow_run (Execute + approval — advances a real parked run), cancel_flow_run (Write). The agent can now find a failing run, diagnose via get_flow_run, patch via edit_workflow, and verify — without a run_id handed in externally. - gated create: create_workflow (Write + approval) persists a NEW flow ALWAYS born disabled (enable stays human-only) behind the forced require_approval floor and the same author gates as save; duplicate_flow makes a disabled copy for clone-then-edit. - testability: dry_run_workflow is now PermissionLevel::None (mock-only, side-effect-free) so a read-only agent can self-verify its own proposal. Deferred as scoped follow-ups (need new execution plumbing, not tool wrappers): a real read-scope test_run mode, and run_flow accepting a draft_id. Refs #flows-agent-friendliness F4 F7. --- .../flows/agents/workflow_builder/agent.toml | 5 + src/openhuman/flows/builder_tools.rs | 409 +++++++++++++++++- src/openhuman/flows/builder_tools_tests.rs | 111 ++++- src/openhuman/tools/ops.rs | 10 + 4 files changed, 507 insertions(+), 28 deletions(-) diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 48f5b520c2..41097e6e73 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -64,6 +64,11 @@ named = [ "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/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index e01e905d8a..c1da38dea8 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 @@ -695,6 +695,386 @@ impl Tool for GetFlowHistoryTool { } } +// ───────────────────────────────────────────────────────────────────────────── +// 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!( + "{msg}\n\nFix the graph and call create_workflow again." + ))); + } + + 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"); + } + } + + 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.", + }))?)) + } +} + +/// `duplicate_flow`: create an independent, DISABLED copy of a saved flow — the +/// clone-then-edit pattern. Write-class. +pub struct DuplicateFlowTool { + config: Arc, +} + +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}"))), + } + } +} + +/// 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 // ───────────────────────────────────────────────────────────────────────────── @@ -1787,34 +2167,19 @@ impl Tool for DryRunWorkflowTool { } 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(), - )); - } - // Graph source: a working draft (draft_id) or an inline graph. let graph_json = if let Some(draft_id) = args .get("draft_id") diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index d4be4485e5..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] @@ -1625,3 +1630,97 @@ async fn edit_workflow_edits_a_draft_and_writes_back() { 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/tools/ops.rs b/src/openhuman/tools/ops.rs index 15a231feb5..f79687edfd 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -298,6 +298,16 @@ pub fn all_tools_with_runtime( // 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())), From a373e1c881570bc5ac396d2bb9dc22dfbd2f771c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 07:12:53 +0300 Subject: [PATCH 12/15] feat(flows): connector onboarding + catalog RPCs (Phase 5, F: builder UX) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connector-onboarding and tool-discovery core, reusing the agent-tool logic so the UI and agent can't drift. - catalog RPCs (item 16): flows_search_tool_catalog + flows_get_tool_contract — thin, secret-free wrappers over the SAME core as the agent's search_tool_catalog / get_tool_contract tools. flowsApi searchToolCatalog / getToolContract. - required-connections (item 18): compute_required_connections tags each toolkit a graph needs connected|missing (native oh: + http_request skipped); surfaced via flows_required_connections RPC AND on the workflow_proposal payload, so the proposal card can render Connect CTAs instead of a bare gate error. flowsApi requiredConnections. - agent guidance (item 19): list_connectable_toolkits tool so the agent steers toolkit choice toward what's already connected and can enumerate what a flow still needs. Deferred as frontend/cross-domain follow-ups: the in-canvas tool browser UI (item 17), the Connect-CTA rendering in the proposal/canvas (item 18 UI), and server-side workflow_copilot thread tagging (item 20). Refs #flows-agent-friendliness Phase 5. --- app/src/services/api/flowsApi.ts | 60 +++++++ .../flows/agents/workflow_builder/agent.toml | 1 + src/openhuman/flows/builder_tools.rs | 55 ++++++ src/openhuman/flows/ops.rs | 158 ++++++++++++++++++ src/openhuman/flows/ops_tests.rs | 47 ++++++ src/openhuman/flows/schemas.rs | 125 +++++++++++++- src/openhuman/tools/ops.rs | 3 + 7 files changed, 448 insertions(+), 1 deletion(-) diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index a90b965168..b0300484f6 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -645,6 +645,66 @@ 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`. */ diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 41097e6e73..5159dbdf6e 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -61,6 +61,7 @@ named = [ "get_tool_contract", "get_tool_output_sample", "list_agent_profiles", + "list_connectable_toolkits", "list_node_kinds", "get_node_kind_contract", "dry_run_workflow", diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index c1da38dea8..599d90b0ab 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -1062,6 +1062,61 @@ impl Tool for DuplicateFlowTool { } } +/// `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 + } + + 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 { diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 05e7430115..5f343e0ab3 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -219,6 +219,10 @@ pub(crate) async fn build_builder_proposal( 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!( @@ -239,6 +243,7 @@ pub(crate) async fn build_builder_proposal( "require_approval": require_approval, "summary": summary, "warnings": warnings, + "required_connections": required_connections, }); if let Some(instruction) = instruction { payload["instruction"] = json!(instruction); @@ -3857,6 +3862,159 @@ 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 // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 30f73edcea..ee807f06cc 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3883,3 +3883,50 @@ async fn update_records_revisions_and_rollback_restores() { 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 a40535e79f..f82dbfacce 100644 --- a/src/openhuman/flows/schemas.rs +++ b/src/openhuman/flows/schemas.rs @@ -295,6 +295,9 @@ pub fn all_controller_schemas() -> Vec { schemas("draft_promote"), schemas("get_history"), schemas("rollback"), + schemas("search_tool_catalog"), + schemas("get_tool_contract"), + schemas("required_connections"), ] } @@ -416,6 +419,18 @@ pub fn all_registered_controllers() -> Vec { 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, + }, ] } @@ -942,6 +957,75 @@ 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", @@ -1397,6 +1481,39 @@ 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?; @@ -1558,6 +1675,9 @@ mod tests { "draft_promote", "get_history", "rollback", + "search_tool_catalog", + "get_tool_contract", + "required_connections", ] ); } @@ -1565,7 +1685,7 @@ mod tests { #[test] fn all_registered_controllers_has_handler_per_schema() { let controllers = all_registered_controllers(); - assert_eq!(controllers.len(), 29); + assert_eq!(controllers.len(), 32); let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect(); assert_eq!( names, @@ -1599,6 +1719,9 @@ mod tests { "draft_promote", "get_history", "rollback", + "search_tool_catalog", + "get_tool_contract", + "required_connections", ] ); } diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index f79687edfd..4e8a515e40 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -331,6 +331,9 @@ 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 From ea66da7a6954d0dabcdfdb1594299d5ce15ddeda Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 07:21:53 +0300 Subject: [PATCH 13/15] feat(flows): naming disambiguation + prompt hygiene (Phase 6, F8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename the legacy skills tool create_workflow -> create_skill (audit F8): 'workflow' now unambiguously means a Flows automation. This also fixes the hard registry collision Phase 4 introduced when the flows domain claimed the create_workflow name — the duplicate-tool-name guard is green again. Updates the Skills domain-group list, the user_filter tool families, and the domain-membership tests. - workflow_builder prompt: add a concise 'authoring tools' section pointing at the new belt (list_node_kinds/get_node_kind_contract for the DSL schema, edit_workflow for cheap iteration, validate_workflow to self-check, list_connectable_toolkits, the debug-loop + create/duplicate tools, draft_id mode); point the 12-node-kind reference at the introspection tools as the source of truth. Scoped follow-up: the broader list_workflows->list_skills / run_workflow-> run_skill rename (with deprecation aliases) + removing the orchestrator disambiguation paragraph — run_workflow is special-cased across the harness turn handler, rhai bridge, and agent tomls, so it warrants its own reviewed change rather than riding this PR. Refs #flows-agent-friendliness F8. --- .../flows/agents/workflow_builder/prompt.md | 33 +++++++++++++++++++ src/openhuman/skills/tools.rs | 17 ++++++---- src/openhuman/tools/ops.rs | 2 +- src/openhuman/tools/ops_tests.rs | 4 +-- src/openhuman/tools/user_filter.rs | 4 +-- 5 files changed, 48 insertions(+), 12 deletions(-) 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/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 4e8a515e40..95fb2736bb 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -1139,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", ], From 881143e651c3600f33822e90dd3e98d2466271b5 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 07:33:44 +0300 Subject: [PATCH 14/15] chore(tinyflows): bump submodule to merged f182389 (tinyflows#11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The validate_all / catalog / graph_ops changes this branch depends on merged upstream (tinyhumansai/tinyflows#11, squash-merged as f182389). Point the submodule at the merged commit so CI can check it out — the earlier per-phase pointers referenced the pre-merge branch commits, which are not on the tinyflows remote. --- vendor/tinyflows | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/tinyflows b/vendor/tinyflows index bc68254b07..f18238904f 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit bc68254b0718385829b6fac2e9a16550f5443c65 +Subproject commit f18238904f48c395dce99c0c1d759c3dd1bfb678 From 3a54ef140af6337962c05e08e76730f64ab7ee60 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Wed, 15 Jul 2026 07:39:03 +0300 Subject: [PATCH 15/15] chore: apply prettier auto-fixes to flows frontend --- app/src/hooks/useFlowChanged.ts | 7 +------ app/src/pages/FlowsPage.tsx | 2 +- app/src/services/api/flowsApi.ts | 10 ++-------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/src/hooks/useFlowChanged.ts b/app/src/hooks/useFlowChanged.ts index 2abbd1839f..d9111704b7 100644 --- a/app/src/hooks/useFlowChanged.ts +++ b/app/src/hooks/useFlowChanged.ts @@ -29,12 +29,7 @@ const EVENT_COLON = 'flow:changed'; const EVENT_UNDERSCORE = 'flow_changed'; /** What happened to the flow. */ -export type FlowChangeKind = - | 'created' - | 'updated' - | 'deleted' - | 'enabled_changed' - | (string & {}); +export type FlowChangeKind = 'created' | 'updated' | 'deleted' | 'enabled_changed' | (string & {}); /** Payload of a `flow:changed` socket event (`DomainEvent::FlowChanged`). */ export interface FlowChangedEvent { diff --git a/app/src/pages/FlowsPage.tsx b/app/src/pages/FlowsPage.tsx index 07eef75e24..047cfd6fe8 100644 --- a/app/src/pages/FlowsPage.tsx +++ b/app/src/pages/FlowsPage.tsx @@ -21,7 +21,6 @@ import FlowTemplateGallery from '../components/flows/FlowTemplateGallery'; import NewWorkflowModal from '../components/flows/NewWorkflowModal'; import SuggestedWorkflows from '../components/flows/SuggestedWorkflows'; import { useCreateFlow } from '../components/flows/useCreateFlow'; -import { useFlowChanged } from '../hooks/useFlowChanged'; import WorkflowPromptBar from '../components/flows/WorkflowPromptBar'; import { ToastContainer } from '../components/intelligence/Toast'; import PageSectionHeader from '../components/layout/PageSectionHeader'; @@ -32,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'; diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index b0300484f6..13539d52ea 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -750,10 +750,7 @@ export async function updateDraft( /** 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 response = await callCoreRpc({ method: 'openhuman.flows_draft_list', params: {} }); const result = unwrapCliEnvelope<{ drafts: FlowDraft[] }>(response); return result.drafts ?? []; } @@ -772,10 +769,7 @@ export async function deleteDraft(id: string): Promise { * 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 { +export async function promoteDraft(id: string, requireApproval?: boolean): Promise { log('promoteDraft: request id=%s', id); const response = await callCoreRpc({ method: 'openhuman.flows_draft_promote',