fix(workflow): a workflow captured from a chat can be saved (D12), and the KB card says when it will have no default (D14)

crates/biorouter/src/agents/agent.rs

@@ -12302,11 +12302,25 @@ impl Agent {
             workflow_builder = workflow_builder.parameters(parameters);
         }
 
-        let workflow = workflow_builder.build().map_err(|e| {
+        let mut workflow = workflow_builder.build().map_err(|e| {
             tracing::error!("Failed to build workflow: {}", e);
             anyhow!("Workflow build failed: {}", e)
         })?;
 
+        // A parameter the document refers to nowhere makes the whole workflow
+        // unsavable: `workflow::service::validate` refuses it either way round
+        // — with no default as "Optional parameters missing default values", with
+        // one as "Unnecessary parameter definitions" — so a capture that emitted
+        // one could not be saved from any surface. Models produce them routinely,
+        // because `workflow.md` asks for parameters and for their `{{ key }}`
+        // references as two separate instructions.
+        //
+        // Pruned HERE rather than in each of the three capture surfaces: the
+        // route, the CLI's `/workflow` and the model's own `generate` all come
+        // through this function, which is the whole reason
+        // `tests/workflow_capture_parity.rs` exists.
+        crate::workflow::service::drop_unreferenced_parameters(&mut workflow);
+
         tracing::info!("Workflow creation completed successfully");
         Ok(workflow)
     }

crates/biorouter/src/workflow/service.rs

@@ -333,6 +333,86 @@ pub fn validate(workflow: &Workflow) -> Result<()> {
         .map_err(|err| anyhow::anyhow!("{err}"))
 }
 
+/// Drop the parameters no part of the document refers to, and return their keys.
+///
+/// [`validate`] refuses a parameter that appears in no `{{ key }}` anywhere in
+/// the document ("Unnecessary parameter definitions"), and separately refuses an
+/// `optional` parameter with no `default` ("Optional parameters missing default
+/// values"). Both rules are right on their own, and together they leave an
+/// unreferenced `optional` parameter with no accepted form at all: removing its
+/// default trades one refusal for the other. A generator that emits one
+/// therefore produces a workflow the user cannot save by any route — which is
+/// what "Create workflow from this chat" did, because `workflow.md` asks for
+/// parameters and for `{{ key }}` references as two separate instructions and a
+/// model routinely obeys only the first.
+///
+/// So the generated document is normalised instead of the rules being relaxed. A
+/// parameter nothing refers to is a value the run would collect and discard, and
+/// dropping it is the same stance `Agent::create_workflow` already takes towards
+/// a parameter list that does not parse: keep the expensive part, lose the part
+/// that cannot work, say so in the log.
+///
+/// ⚠ **The reference set is read the way [`validate`] reads it** — every
+/// `{{ … }}` in the serialized document, not just the ones in `prompt` and
+/// `instructions` — so the two can never disagree about what "referenced"
+/// means. A serialization failure drops nothing: an unpruned document still has
+/// a chance of being valid, and a silently emptied `parameters` list does not.
+pub fn drop_unreferenced_parameters(workflow: &mut Workflow) -> Vec<String> {
+    let Some(parameters) = workflow.parameters.as_ref() else {
+        return Vec::new();
+    };
+    if parameters.is_empty() {
+        return Vec::new();
+    }
+
+    let yaml = match workflow.to_yaml() {
+        Ok(yaml) => yaml,
+        Err(err) => {
+            tracing::warn!(
+                "Keeping the generated parameters: the workflow would not serialize \
+                 for a reference check: {err}"
+            );
+            return Vec::new();
+        }
+    };
+    let referenced = match crate::workflow::template_workflow::parse_workflow_content(&yaml, None) {
+        Ok((_, variables)) => variables,
+        Err(err) => {
+            tracing::warn!(
+                "Keeping the generated parameters: the workflow would not parse \
+                 for a reference check: {err}"
+            );
+            return Vec::new();
+        }
+    };
+
+    let mut dropped = Vec::new();
+    let kept: Vec<_> = parameters
+        .iter()
+        .filter(|parameter| {
+            if referenced.contains(&parameter.key) {
+                return true;
+            }
+            dropped.push(parameter.key.clone());
+            false
+        })
+        .cloned()
+        .collect();
+
+    if dropped.is_empty() {
+        return dropped;
+    }
+    tracing::warn!(
+        "Dropping generated workflow parameters the document never refers to: {}",
+        dropped.join(", ")
+    );
+    // `None`, not an empty list: `skip_serializing_if` keeps an empty `Vec` out
+    // of the YAML anyway, and `None` is what every other "there is nothing to
+    // say" path in this module means by it.
+    workflow.parameters = if kept.is_empty() { None } else { Some(kept) };
+    dropped
+}
+
 /// Substitute parameter values into a workflow template.
 ///
 /// `Ok(None)` means required parameters are still missing — the caller is

crates/biorouter/src/workflow/validate_workflow.rs

@@ -103,24 +103,34 @@ fn validate_parameters_in_template(
     let mut message = String::new();
 
     if !missing_keys.is_empty() {
+        // Sorted, like the arm below: both sides come out of a `HashSet`
+        // difference, so an unsorted join gives the same fault a different
+        // message on each run.
+        let mut names: Vec<String> = missing_keys.iter().map(|s| s.to_string()).collect();
+        names.sort();
         message.push_str(&format!(
             "Missing definitions for parameters in the workflow file: {}.",
-            missing_keys
-                .iter()
-                .map(|s| s.to_string())
-                .collect::<Vec<_>>()
-                .join(", ")
+            names.join(", ")
         ));
     }
 
     if !extra_keys.is_empty() {
+        // The prefix is load-bearing — it is what the messages users and tests
+        // have already matched on say. What follows it is new: the old message
+        // named the parameter and stopped, leaving "unnecessary" to be guessed
+        // at, and the obvious guess (remove its `default`) trades this refusal
+        // for `validate_optional_parameters`' one. Naming the fix rather than
+        // only the fault is the difference between a rule and a dead end.
+        let mut names: Vec<String> = extra_keys.iter().map(|s| s.to_string()).collect();
+        names.sort();
         message.push_str(&format!(
-            "\nUnnecessary parameter definitions: {}.",
-            extra_keys
-                .iter()
-                .map(|s| s.to_string())
-                .collect::<Vec<_>>()
-                .join(", ")
+            "\nUnnecessary parameter definitions: {}. Nothing in the workflow \
+             refers to {}, so a value for it would be collected and then \
+             discarded — write {{{{ {} }}}} into `prompt` or `instructions`, or \
+             remove the definition. Giving it a `default` does not help.",
+            names.join(", "),
+            if names.len() == 1 { "it" } else { "them" },
+            names[0]
         ));
     }
     Err(anyhow::anyhow!("{}", message.trim_end()))

crates/biorouter/tests/workflow_capture_parity.rs

@@ -61,7 +61,9 @@ fn sandbox_config_root_for_this_test_binary() {
 /// the two documents below therefore came from the capture path, which is
 /// exactly the fault being tested for.
 #[derive(Clone)]
-struct FixedWorkflowProvider;
+struct FixedWorkflowProvider {
+    json: &'static str,
+}
 
 const GENERATED_JSON: &str = r#"{
   "title": "Gene association summary",
@@ -98,7 +100,7 @@ impl Provider for FixedWorkflowProvider {
         _tools: &[Tool],
     ) -> Result<(Message, ProviderUsage), ProviderError> {
         Ok((
-            Message::assistant().with_text(GENERATED_JSON),
+            Message::assistant().with_text(self.json),
             ProviderUsage::new("fixed-model".to_string(), Usage::default()),
         ))
     }
@@ -115,6 +117,10 @@ struct Harness {
 }
 
 async fn harness() -> Harness {
+    harness_generating(GENERATED_JSON).await
+}
+
+async fn harness_generating(json: &'static str) -> Harness {
     let dir = TempDir::new().unwrap();
     let data_dir = dir.path().to_path_buf();
     let session_manager = Arc::new(SessionManager::new(data_dir.clone()));
@@ -136,7 +142,7 @@ async fn harness() -> Harness {
         .unwrap();
 
     agent
-        .update_provider(Arc::new(FixedWorkflowProvider), &session.id)
+        .update_provider(Arc::new(FixedWorkflowProvider { json }), &session.id)
         .await
         .expect("bind the fixed provider");
 
@@ -353,3 +359,163 @@ async fn the_settings_pin_names_the_bound_provider_and_never_panics() {
     );
     assert_eq!(settings.biorouter_model.as_deref(), Some("fixed-model"));
 }
+
+/// A generation whose optional parameter the document never refers to.
+///
+/// Models emit these constantly — `workflow.md` asks for parameters and for
+/// `{{ key }}` references separately, and a model that obeys the first half and
+/// forgets the second produces exactly this. `output_format` below is declared
+/// and then never used.
+const GENERATED_JSON_WITH_AN_UNREFERENCED_PARAMETER: &str = r#"{
+  "title": "Gene association summary",
+  "description": "Looks a gene up and summarises its disease associations.",
+  "instructions": "Query the graph and report the strongest associations first.",
+  "activities": ["Summarise APOE"],
+  "prompt": "Summarise the disease associations for {{ gene_symbol }}.",
+  "parameters": [
+    {
+      "key": "gene_symbol",
+      "input_type": "string",
+      "requirement": "user_prompt",
+      "description": "HGNC gene symbol"
+    },
+    {
+      "key": "output_format",
+      "input_type": "select",
+      "requirement": "optional",
+      "description": "How to format the summary",
+      "default": "markdown",
+      "options": ["markdown", "table"]
+    }
+  ],
+  "skills": []
+}"#;
+
+/// A workflow captured from a chat must be SAVABLE.
+///
+/// The two parameter validators are individually reasonable and jointly closed
+/// over a parameter the document never refers to:
+///
+///   * `optional` with no `default` → "Optional parameters missing default
+///     values in the workflow: output_format."
+///   * any requirement, referenced nowhere → "Unnecessary parameter
+///     definitions: output_format."
+///
+/// So there was no `default` — present or absent — for which `POST
+/// /workflows/save` accepted the generated document, and "Create workflow from
+/// this chat" could not complete at all.
+///
+/// Measured 2026-09-12. The refusal below, run against `origin/main`'s
+/// validator, read exactly `\nUnnecessary parameter definitions:
+/// output_format.` and nothing else; posting the same two documents to a
+/// running daemon answered **400** both times.
+///
+/// The generator is the half that is wrong. A parameter nothing refers to is a
+/// question the run would ask and then discard, so it is dropped at capture
+/// time — next to the existing rule that a parameter list which does not even
+/// parse is dropped rather than losing the whole document.
+#[tokio::test]
+async fn a_captured_workflow_never_declares_a_parameter_the_document_does_not_use() {
+    let h = harness_generating(GENERATED_JSON_WITH_AN_UNREFERENCED_PARAMETER).await;
+    let knowledge = biorouter_mcp::knowledge::service::KnowledgeService::new_default()
+        .expect("a knowledge service in the sandboxed root");
+    let session = h
+        .agent
+        .config
+        .session_manager
+        .get_session(&h.session_id, true)
+        .await
+        .unwrap();
+
+    let mut workflow = h
+        .agent
+        .create_workflow(session.conversation.clone().unwrap())
+        .await
+        .expect("the capture");
+    // The save path validates the ENRICHED document — the same one the modal
+    // posts — so the enrichment runs here too.
+    let enrichment = service::session_enrichment(&h.agent, &knowledge, &h.session_id, None)
+        .await
+        .expect("the route's enrichment");
+    service::apply_session_enrichment(&mut workflow, enrichment);
+
+    let keys: Vec<String> = workflow
+        .parameters
+        .iter()
+        .flatten()
+        .map(|parameter| parameter.key.clone())
+        .collect();
+    assert_eq!(
+        keys,
+        vec!["gene_symbol".to_string()],
+        "the referenced parameter is kept and the unreferenced one dropped"
+    );
+
+    // The whole point: what the capture produced is what `POST /workflows/save`
+    // will accept. `service::validate` is the function that route calls.
+    service::validate(&workflow).unwrap_or_else(|err| {
+        panic!(
+            "a workflow captured from a chat must save: {err}\n\n{}",
+            workflow.to_yaml().unwrap_or_default()
+        )
+    });
+}
+
+/// The two messages that closed the gap, pinned as a pair.
+///
+/// Read together they are the specification the generator now satisfies: a
+/// parameter must be referenced, and an `optional` one must also carry a
+/// default. Neither is relaxed — the "unnecessary" arm is the only thing that
+/// catches a key typo (`{{ gene }}` in the prompt beside a parameter keyed
+/// `gene_symbol` reports both halves of the mismatch), and a parameter nothing
+/// reads is dead weight in a document meant to be shared. What changed is that
+/// the message now says what to do about it.
+#[test]
+fn an_unreferenced_parameter_is_still_refused_and_the_refusal_says_why() {
+    let unreferenced_with_a_default = r#"
+version: 1.0.0
+title: Test
+description: Test
+instructions: Nothing refers to the parameter below.
+parameters:
+  - key: output_format
+    input_type: string
+    requirement: optional
+    description: How to format the summary
+    default: markdown
+"#;
+    let err = service::validate(
+        &biorouter::workflow::Workflow::from_content(unreferenced_with_a_default).unwrap(),
+    )
+    .expect_err("a parameter the document never refers to is refused");
+    let message = err.to_string();
+    assert!(
+        message.contains("Unnecessary parameter definitions: output_format."),
+        "the refusal still names the parameter: {message}"
+    );
+    assert!(
+        message.contains("{{ output_format }}"),
+        "and now says how to fix it, naming the key: {message}"
+    );
+
+    let unreferenced_without_a_default = r#"
+version: 1.0.0
+title: Test
+description: Test
+instructions: Nothing refers to the parameter below.
+parameters:
+  - key: output_format
+    input_type: string
+    requirement: optional
+    description: How to format the summary
+"#;
+    let message = service::validate(
+        &biorouter::workflow::Workflow::from_content(unreferenced_without_a_default).unwrap(),
+    )
+    .expect_err("dropping the default does not help")
+    .to_string();
+    assert!(
+        message.contains("Optional parameters missing default values"),
+        "the other half of the pair, unchanged: {message}"
+    );
+}

ui/desktop/src/components/workflows/shared/WorkflowFormFields.tsx

@@ -689,6 +689,18 @@ export function WorkflowFormFields({
                   onSelectedIdsChange={onKnowledgeBaseIdsChange}
                   defaultId={defaultKnowledgeBaseId}
                   onDefaultIdChange={onDefaultKnowledgeBaseIdChange}
+                  // A chat with no pinned primary is captured as `default: null`,
+                  // and the daemon never infers one from `visible`
+                  // (`plan_knowledge_selection`) — so the workflow really will
+                  // have no default, and every chat it starts will have no write
+                  // target. That is the intent, not a bug, and the card said
+                  // nothing about it while its own description promised "which
+                  // one is focused by default": the reader had no way to tell
+                  // which of the two they were looking at.
+                  noDefaultText={
+                    'No default — this workflow will not focus one. Chats it starts search ' +
+                    'every base above, and a write that names no base asks which one to use.'
+                  }
                   notice={knowledgeBaseNotice}
                   emptyText="No knowledge bases found"
                   searchPlaceholder="Search knowledge bases..."