diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index cbba657ca..7fd4eca00 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -1017,7 +1017,12 @@ mod tests { // (hard-refused otherwise, regardless of the user's scope preference) // against an already-connected toolkit — see `builder_tools.rs`'s // module doc. This pins the invariant in the agent definition itself, - // not just the tool implementations. + // not just the tool implementations. It also has read-only grounding + // in the user's memory via `memory_recall` (direct lookups) and + // `memory_hybrid_search` (keyword/lexical lookups — pairs with + // `memory_recall` the same way the sibling `flow_discovery` agent + // does) — no `memory_store`, so it can look up context but never + // write it. let def = find("workflow_builder"); assert_eq!(def.agent_tier, AgentTier::Worker); assert_eq!(def.delegate_name.as_deref(), Some("build_workflow")); @@ -1051,6 +1056,8 @@ mod tests { "composio_list_toolkits", "composio_list_connections", "composio_connect", + "memory_recall", + "memory_hybrid_search", ]; for required in expected { assert!( @@ -1078,6 +1085,8 @@ mod tests { "apply_patch", "composio_execute", "spawn_subagent", + // Memory access must stay read-only: no write tool. + "memory_store", ] { assert!( !names.iter().any(|n| n == forbidden), diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 6b09db83c..3a0d73b89 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -46,6 +46,25 @@ hint = "reasoning" # listing publishes no output schema at all (verified for every GitHub # action), so a downstream split_out.path is never guessed. See # `builder_tools.rs`'s module doc. +# `memory_recall` is the raw (un-delegated) memory read tool — READ-ONLY, no +# write counterpart (`memory_store` is deliberately absent). This agent is a +# Named-scope agent, so the orchestrator's `retrieve_memory` delegate would +# not synthesize context for it; the raw tool is the correct mechanism here. +# `omit_memory_context = true` stays set above, so nothing is injected +# automatically — the agent must call `memory_recall` explicitly when it +# needs the user's context. +# `memory_hybrid_search` is paired alongside it (same convention as the +# sibling `flow_discovery` agent) for keyword/lexical lookups — the +# `[learning]`-gated `MemoryAccessSection` bias instruction +# (`src/openhuman/learning/prompt_sections.rs`) tells every agent with a +# retrieval tool visible to "call `memory_recall` (or `memory_search` for +# keyword lookups)"; `memory_search` itself is not a registered tool name +# anywhere in this codebase (a pre-existing, wider naming inconsistency +# across several agent configs, out of scope here), so without a real +# keyword-capable tool visible, a keyword-memory turn on this Named scope +# could only ever reach `memory_recall` — leaving the guidance's "keyword +# lookups" half unsatisfiable. `memory_hybrid_search`'s `lexical` mode is +# the real keyword-heavy retrieval this agent needs; both stay READ-ONLY. named = [ "propose_workflow", "revise_workflow", @@ -63,4 +82,6 @@ named = [ "composio_list_toolkits", "composio_list_connections", "composio_connect", + "memory_recall", + "memory_hybrid_search", ] diff --git a/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs b/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs index 0babd0fc1..97f7a8f05 100644 --- a/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs +++ b/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs @@ -293,6 +293,50 @@ mod tests { assert!(p.contains("END-TO-END")); } + /// The standing archetype (`prompt.md`, the always-loaded system prompt — + /// as opposed to the per-turn directives rendered above) carries the same + /// B27 banned-phrase regression, plus positive coverage for the plain- + /// language style rule and the read-only memory grounding tool added + /// alongside it. Guards against reintroducing jargon-leaking or + /// phantom-review-card language, and against silently losing the + /// `memory_recall` guidance if the prompt is ever rewritten. + #[test] + fn standing_prompt_teaches_plain_language_and_readonly_memory() { + const STANDING_PROMPT: &str = include_str!("prompt.md"); + + // Negative (B27): the phantom "review card" phrasing must never + // reappear in the standing prompt either. + for banned in ["review card", "Accept the proposal explicitly"] { + assert!( + !STANDING_PROMPT.contains(banned), + "standing prompt must not carry phantom review-card phrasing `{banned}` (B27)" + ); + } + + // Positive: the anti-jargon Style rule — replies must stay in plain + // language, never leak response_format/schema/expression internals. + assert!( + STANDING_PROMPT.contains("Speak to a non-technical user"), + "standing prompt must teach the anti-jargon Style rule" + ); + + // Positive: read-only memory grounding via the raw `memory_recall` + // tool (no `memory_store` — see the agent.toml regression test). + assert!( + STANDING_PROMPT.contains("memory_recall"), + "standing prompt must teach the builder to ground itself with memory_recall" + ); + + // Positive: the prompt must state the read-only contract explicitly — + // not just mention the tool name — so a future edit can't silently + // drop the "can't change their memory" guarantee this agent's tool + // scope depends on (no `memory_store` in agent.toml). + assert!( + STANDING_PROMPT.contains("Read-only — you can't change their memory"), + "standing prompt must state the memory read-only guarantee, not just mention memory_recall" + ); + } + #[test] fn repair_includes_run_id_error_and_failing_nodes() { let mut r = req(BuildMode::Repair); diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 4b9e3711d..31dffca82 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -19,45 +19,37 @@ no tool that does — by design. Your authoring outputs are: - **`save_workflow`** — the ONE persistence tool you have, and it only writes to a flow that **already exists** (you need its `flow_id`). See below. -If there is no existing flow to save to, only the user's own explicit click -persists it — never yours, and the click looks different depending on where -your proposal shows up: on the canvas copilot, Accept applies it to the -draft and the canvas's own **Save** persists it; in main chat or Suggested -Workflows (no flow open yet), your proposal renders as a card whose -**"Save & enable"** button calls the save RPC directly. Either way, saving -is the user's action, not a tool you have. If a user says "just turn it on -for me", explain that enabling stays in their hands — you cannot enable a -flow. +Persisting is otherwise the user's own action, not a tool you have — the one +exception is `save_workflow` on an **existing** flow id, and only when the +user **explicitly asks** (see below). If a user says "just turn it on for +me", explain that enabling stays in their hands — you cannot enable a flow. ## Saving your work: `save_workflow` (only on the user's explicit ask) -Every authoring turn — including a **build** turn seeded from the Flows -prompt bar (which creates the flow first and delegates with its id) and every -**revise** turn on the canvas copilot — is **propose-only** by default. Your -arc is: +Every authoring turn — build, revise, or repair — is **propose-only** by +default. Your arc is: 1. Ground + build the graph (below), `dry_run_workflow` until it's clean. -2. `revise_workflow` / `propose_workflow` so the user sees the proposal. - **Stop there** and hand back — the user reviews and persists it - themselves. On the canvas copilot: Accept applies it to the draft, then - the canvas's own Save persists it. In main chat or Suggested Workflows - (no flow open yet): the proposal renders as a card and "Save & enable" - persists it directly. Do NOT call `save_workflow` unless the user - explicitly asks. +2. `propose_workflow` / `revise_workflow` so the user sees the proposal, then + **stop and hand back** — persisting it is their action, not yours. Don't + over-explain how to save: give one short line for the current surface + ("accept it on the canvas and hit Save", or "use Save & enable on the + card") — never recite every persist path, and never repeat it across + turns. -**Do NOT auto-`save_workflow` when the request carries a `flow_id`.** The id -is context — the user may later ask you to save/test that flow — but the -persistence gate stays with the user. Auto-saving would leave the flow's -graph persisted even if the user Rejects the proposal. +**When the user says "save it":** if you have a `save_workflow` action +available — an **existing** `flow_id` plus their explicit ask ("save this", +"yes save it onto flow_X") — just call `save_workflow { flow_id, graph, +name? }` and confirm in one plain line what you saved (trigger, steps, and — +if the flow is enabled with a schedule/app_event trigger — that it's now +live and will fire on its own). If you don't have that (no flow yet, or they +haven't asked), give the one short line above instead of re-explaining. -Use **`save_workflow { flow_id, graph, name? }`** only when the user -**explicitly asks** you to save it ("save this", "yes save it onto flow_X"). -When you do, tell them plainly what you saved (trigger, steps, and — if the -flow is enabled with a schedule/app_event trigger — that it is now live and -will fire on its own). Never `save_workflow` onto a flow the user did NOT -ask you to build/update — editing some other saved flow requires their -explicit ask naming it. It cannot create flows, and it never changes -`enabled` or the approval gate. +**Do NOT auto-`save_workflow`** just because the request carries a +`flow_id` — the id is context for a later ask, but the persistence gate +stays with the user until they explicitly ask. Never `save_workflow` onto a +flow the user did NOT ask you to build/update. It cannot create flows, and +it never changes `enabled` or the approval gate. ## Testing a saved flow: `run_flow` (ask first!) @@ -78,6 +70,16 @@ it as real). Rules: report what happened; if it failed, `get_flow_run` for the steps and propose a fix. +## Grounding in what you already know: `memory_recall` + +You can `memory_recall` to look up the user's context — connected channels, +teammates/people, stated preferences, past decisions. Use it to resolve a +genuinely-ambiguous target/recipient/preference **before** asking or +guessing (e.g. recall their default channel or their team's names). For a +keyword-style lookup (a specific name, term, or phrase you need to find +rather than a general context recall), use `memory_hybrid_search` in its +`lexical` mode instead. Read-only — you can't change their memory. + ## Your authoring loop 1. **Understand the trigger and the steps.** What starts the flow? What should @@ -229,6 +231,12 @@ A `WorkflowGraph` is `{ name?, nodes: [...], edges: [...] }`. the `true` branch instead. Typing the field as `boolean` in the schema is what makes the output-parser coerce/validate it into a real boolean rather than a string that merely looks like one. + + An `agent` node inside a workflow can also **read and write the user's + memory at run time**. If a workflow genuinely needs the user's context + (recall a preference) or should remember a result/state across runs, wire + an `agent` node that uses memory instead of hardcoding context memory + already holds. Use sparingly — only when the workflow truly needs it. 3. **`tool_call`** — an action. Two flavours by `config.slug`: - **Composio app action** — `config.slug` = a real action slug (from `search_tool_catalog`, e.g. `GMAIL_SEND_EMAIL`) + `config.connection_ref` @@ -486,6 +494,15 @@ failure at runtime. Rules of thumb: ## Style +**Speak to a non-technical user.** Describe what the workflow *does* in plain +language; never surface implementation internals in your replies — no +`response_format`, `output_parser.schema`, jq/`=`-expressions, node config +JSON, tool slugs, or envelope-path talk — unless the user explicitly asks how +it's wired. Say "it'll read your unread email and post a summary to +`#team-product` every morning", not "I added an agent node with an +output_parser.schema and bound the Slack node to +=nodes.research.item.json…". + Be concise. Your posture is **clarify genuinely-ambiguous inputs, verify before you propose, and don't stop until the graph is right** — but a workflow that needs zero questions is still the happy path. Don't let "ask when truly @@ -550,8 +567,10 @@ values appear: PLACEHOLDER (e.g. `""`) rather than null, so the dry run reports `ok: true`. This is expected — the schema is correctly declared, the binding path is correct, and at runtime a real LLM will produce real - values. You may note this: "the dry run passed; the agent node's output - is a mock placeholder — at runtime the real model fills these in." + values. Treat this as your own internal confirmation that the wiring is + correct; don't narrate the mock/placeholder mechanics to the user — that's + sandbox internals, not something they need to hear. Just tell them, + plainly, that the workflow checks out. 2. **Real binding nulls** — a `=nodes..item.json.` expression that resolves to `null` because the path is WRONG (missing `.json.`, diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 95f993989..5e8d93b01 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -2170,13 +2170,20 @@ async fn validate_tool_contracts_skips_rather_than_rejects_when_the_catalog_is_u // `missing_required_args` because SOME value was present, just under the // wrong key) ──────────────────────────────────────────────────────────── -/// `SLACK_SEND_MESSAGE` with a real `input_schema` naming `channel` and -/// `markdown_text` — models the live bug this fixes: `markdown_text` is the -/// real field, `text` is not. +/// Models `SLACK_SEND_MESSAGE`'s real `input_schema` (naming `channel` and +/// `markdown_text` — the live bug this fixes: `markdown_text` is the real +/// field, `text` is not) but under a **fictional toolkit key** +/// (`slackargnametest`), never the real `"slack"` key: `seeded_slack_send_contract` +/// above (input_schema: `None`) also seeds `"slack"` and is used by several +/// sibling tests in this file whose `args` still carry `text` — sharing the +/// real key would race those tests over the process-global +/// `LIVE_CATALOG_CACHE` entry for `"slack"` (same discipline +/// `builder_tools_tests.rs` already applies for its own `slack`/`gmail` +/// fixtures that don't match the shared-key contract byte-for-byte). fn seeded_slack_send_message_contract_with_schema() -> ToolContract { ToolContract { - slug: "SLACK_SEND_MESSAGE".to_string(), - toolkit: "slack".to_string(), + slug: "SLACKARGNAMETEST_SEND_MESSAGE".to_string(), + toolkit: "slackargnametest".to_string(), description: None, required_args: vec![], input_schema: Some(json!({ @@ -2196,7 +2203,7 @@ fn seeded_slack_send_message_contract_with_schema() -> ToolContract { #[tokio::test] async fn validate_tool_contracts_rejects_an_arg_name_not_in_the_input_schema() { seed_live_catalog_cache( - "slack", + "slackargnametest", vec![seeded_slack_send_message_contract_with_schema()], ); let config = Config::default(); @@ -2204,7 +2211,7 @@ async fn validate_tool_contracts_rejects_an_arg_name_not_in_the_input_schema() { "nodes": [ { "id": "t", "kind": "trigger", "name": "Manual" }, { "id": "post", "kind": "tool_call", "name": "Post", - "config": { "slug": "SLACK_SEND_MESSAGE", + "config": { "slug": "SLACKARGNAMETEST_SEND_MESSAGE", "args": { "channel": "#general", "text": "hi" } } } ], "edges": [ { "from_node": "t", "to_node": "post" } ] @@ -2220,7 +2227,7 @@ async fn validate_tool_contracts_rejects_an_arg_name_not_in_the_input_schema() { #[tokio::test] async fn validate_tool_contracts_passes_the_real_arg_name_from_the_input_schema() { seed_live_catalog_cache( - "slack", + "slackargnametest", vec![seeded_slack_send_message_contract_with_schema()], ); let config = Config::default(); @@ -2228,7 +2235,7 @@ async fn validate_tool_contracts_passes_the_real_arg_name_from_the_input_schema( "nodes": [ { "id": "t", "kind": "trigger", "name": "Manual" }, { "id": "post", "kind": "tool_call", "name": "Post", - "config": { "slug": "SLACK_SEND_MESSAGE", + "config": { "slug": "SLACKARGNAMETEST_SEND_MESSAGE", "args": { "channel": "#general", "markdown_text": "hi" } } } ], "edges": [ { "from_node": "t", "to_node": "post" } ]