diff --git a/CLAUDE.md b/CLAUDE.md index 8b4f16132..990d9444f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -745,7 +745,10 @@ prompt both catalog models made structured tool calls reliably. sticky per instance, text held until the size check passes. - **Every request streams**, blocking ones collected, so `OLLAMA_TIMEOUT` (600 s) bounds silence, never a whole generation; a timeout is never retried - (`TimeoutRetry::GiveUp`), a connection failure still is. An unpinned turn gets + (`TimeoutRetry::GiveUp`), a connection failure still is. One before the reply + begins is `ProviderError::IdleTimeout`, naming the seconds, the prompt's rough + size and the setting, and the agent does not re-send it either (it used to, + as a network blip: diagnostics 20261006_2). An unpinned turn gets `num_predict` 16384, or 32768 when the model may think. - **Thinking:** `think` only to models whose `/api/show` capabilities include `thinking` (others answer `think:true` with a 400). Quick sends `false`; @@ -856,14 +859,22 @@ prompt both catalog models made structured tool calls reliably. beyond a ceiling for the reasoning (characters/2 + 16 tokens) plus 8, else "returned reasoning only (N output tokens)"; a reply cut at `length`, or one with no reasoning and at most 8 tokens, fails at once; (3) a WHOLE reply - that is a call recovery declined (`text_tool_calls::attempted_call`: a name + that is a call recovery declined (`text_tool_calls::attempted_calls`: a name without its `extension__` prefix, arguments that do not fit, the - `` tag), whose name stands for an offered tool. That one + `` tag; several objects or an array, each one naming an + offered tool), whose name stands for an offered tool. That one asks rather than orders, is skipped when the call echoes the conversation (made by an earlier turn, or quoted in a tool result or earlier assistant reply; `echoes_conversation`), and is best-effort: if it cannot be sent, fails or comes back empty, the turn ends with the reply already shown. A retry after shown text starts a new paragraph. + - **A reply of calls nothing ran is SAID, never run** (diagnostics + 20261006_2): when a turn that offered tools ends on a reply made only of + call objects written as text (`whole_reply_call_names`), `agents::reply_parts` + adds one user-only inline notice after it ("Nothing ran: ..."), for every + provider. It is stored, so a reload shows it, and every wire format drops it + because it is not agent-visible. Calls Code Execution hides (`shell`, + `text_editor`) still get no retry: the provider cannot see them. - **Tool roster for templates that hide tools** (R8): when `/api/show`'s template shows tools only next to the last user message (llama3.1, mistral-small3.2, mistral-nemo; `ModelFacts::tools_only_near_last_user`), diff --git a/biorouter-self-test.yaml b/biorouter-self-test.yaml index aa1f06600..7bee9bbd0 100644 --- a/biorouter-self-test.yaml +++ b/biorouter-self-test.yaml @@ -163,6 +163,7 @@ instructions: | - Enable/disable extensions - Test extension interactions - Verify isolation between extensions + - In the desktop, verify Crew appears in Settings > Chat > Capabilities and is absent from the Extensions page and the composer's Tools popover. Preserve the fixture's existing Crew on/off choice. - In a disposable chat, browse `/` and `/ext:` and confirm current capability labels including Biorouter Copilot. Select a session-only extension and verify its exact reference identity. - Submit `/extend Biorouter Copilot` and verify guidance names `/ext:BiorouterCopilot` without model execution or a permission grant. - Verify `/effort` and a harmless configured workflow in the desktop, TUI, and classic CLI. A missing workflow must report its problem instead of falling through to the model. @@ -210,6 +211,7 @@ instructions: | fixture or desktop leaves GUI evidence unavailable; never claim a skipped action passed. For a Crew-enabled fixture, additionally: + - Toggle Crew in Settings > Chat > Capabilities, reload the disposable fixture, and verify the choice persists; restore its original value. - Confirm the empty `/crew` view identifies the isolated profile and does not imply authentication success when no workspace exists - Exercise the workspace form's required connection, invitation, and pinned-key validation using synthetic values only - Verify first-host preparation is explicit, host-key trust is visible, and no private key or provider credential is entered into the fixture @@ -687,7 +689,7 @@ prompt: | 1. Call `agent_drafter__list_apps`. If that exact app id already exists or matches the preview id `{{ preview_app_id }}`, stop this phase and report the collision without modifying any app. Do not overwrite, reuse, or remove a prior run's app. 2. Call `create_app` with that id, kind `agentic`, archetype `dashboard`, a concise title and description, and a system prompt that updates dashboard metrics through the declared action. 3. Call `read_app` and verify the resolved manifest has the dashboard state schema, `set_metric` action, `refresh_requested` signal, and nonblank `surface.state_initial` metric labels. Missing measurements must be explicitly unavailable, not invented zero values. Keep declared initial state as the source of first-load values; do not add duplicate imperative initialization to work around the smoke harness. - 4. Call `build_app`; require a successful bundle. + 4. Call `build_app`; require a successful bundle and a passing default browser-free page check. This checks page files and JavaScript structure; it does not establish browser execution. A quality-check error must fail the build rather than produce a success card. 5. Call `build_app` with `check_only: true`; require zero ERROR findings. 6. Call `smoke_app`; require the real-browser harness to execute successfully. A missing browser or skipped execution is a failure, not a pass. 7. Record setup, manifest assertions, build output, lint output, and smoke output in {{ workspace_dir }}/phase2b_apps.md. diff --git a/crates/biorouter-cli/src/session/tui/mod.rs b/crates/biorouter-cli/src/session/tui/mod.rs index f70ad5ae9..ca5ffedb2 100644 --- a/crates/biorouter-cli/src/session/tui/mod.rs +++ b/crates/biorouter-cli/src/session/tui/mod.rs @@ -1590,33 +1590,17 @@ fn count_skills() -> usize { count_user_skills_in(&list_skills()) } -/// The six foundational built-ins surfaced as their own "Capabilities" section -/// in the GUI and intentionally excluded from the extension list/count there -/// (see ui/desktop/.../capabilities/capabilities.ts). The CLI status line was -/// counting them as extensions, so it showed e.g. 11 where the GUI shows 5. -/// Keys are normalized like the GUI's `nameToKey` (whitespace removed, lowercased). -const CAPABILITY_KEYS: [&str; 6] = [ - "developer", - "extensionmanager", - "skills", - "todo", - "memory", - "knowledge", -]; - -fn name_to_key(name: &str) -> String { - name.chars() - .filter(|c| !c.is_whitespace()) - .collect::() - .to_lowercase() -} - -/// Enabled extensions, excluding the foundational capabilities — matches the -/// count the GUI shows. +/// Enabled extensions, excluding Biorouter's own capabilities, which the GUI +/// lists in Settings → Chat → Capabilities and leaves out of every extension +/// count (`ui/desktop/.../capabilities/capabilities.ts`). It asks +/// `ExtensionConfig::is_capability`, the daemon's own classification, rather +/// than a list of keys here: the list this replaced named six of the fourteen +/// capabilities, so the status line counted Biorouter Copilot, Crew, Workspace +/// Control and five more as extensions the person had installed. fn count_non_capability_extensions() -> usize { biorouter::config::get_enabled_extensions() .iter() - .filter(|ext| !CAPABILITY_KEYS.contains(&name_to_key(&ext.name()).as_str())) + .filter(|ext| !ext.is_capability()) .count() } @@ -2411,7 +2395,7 @@ mod tests { /// (#77). The five Contexts are seeded into the same directory on every /// start, so before this exclusion the terminal reported "5 skills" on a /// machine with none and disagreed with the GUI chip about the same word — - /// the drift `CAPABILITY_KEYS` already fixes for extensions. + /// the drift `count_non_capability_extensions` already fixes for extensions. /// /// Nothing asserted it. `count_skills` reads the real config directory, and /// on the common machine (contexts seeded, no user skills) the filtered and diff --git a/crates/biorouter-mcp/src/agent_drafter/bundle.rs b/crates/biorouter-mcp/src/agent_drafter/bundle.rs index 0cb6bc100..d44f146c1 100644 --- a/crates/biorouter-mcp/src/agent_drafter/bundle.rs +++ b/crates/biorouter-mcp/src/agent_drafter/bundle.rs @@ -81,6 +81,11 @@ pub fn lint_app(project_dir: &Path) -> Vec { let manifest: Option = std::fs::read_to_string(project_dir.join("manifest.json")) .ok() .and_then(|s| serde_json::from_str(&s).ok()); + out.extend(super::page_check::findings( + project_dir, + manifest.as_ref().map_or("index.html", |m| m.entry.as_str()), + true, + )); let is_agentic = manifest .as_ref() .is_none_or(|manifest| manifest.kind == ArtifactKind::Agentic); @@ -934,7 +939,7 @@ pub fn format_lint(findings: &[LintFinding]) -> String { s.push_str(&format!(" {tag}: {}\n", f.msg)); } if findings.iter().any(|f| f.level == LintLevel::Error) { - s.push_str("Fix the ERRORs (they break backend access or portability), then rebuild."); + s.push_str("Fix the ERRORs, then rebuild."); } s } @@ -958,7 +963,7 @@ pub struct BuildReport { } impl BuildReport { - fn not_built(used: &str, log: impl Into) -> Self { + pub(super) fn not_built(used: &str, log: impl Into) -> Self { Self { ok: false, used: used.to_string(), @@ -1741,6 +1746,15 @@ impl BundleMaking<'_> { let staging = Staging::new(&dist)?; match run_esbuild(&cmd, &entry, &staging.0.join("app.js")) { Ok(mut report) if report.ok => { + let js = std::fs::read_to_string(staging.0.join("app.js"))?; + if let Some(finding) = + super::page_check::javascript_error(&js, "dist/app.js", false) + { + return Ok(BuildReport::not_built("esbuild", finding.msg)); + } + if let Some(error) = staged_page_error(self.project_dir, &staging.0, true) { + return Ok(BuildReport::not_built("esbuild", error)); + } let banner = BundleBanner { builder: "esbuild".into(), sdk: self.sdk.clone(), @@ -1782,6 +1796,23 @@ impl BundleMaking<'_> { } } +fn staged_page_error(project_dir: &Path, staging: &Path, replaces_css: bool) -> Option { + let manifest: Option = std::fs::read_to_string(project_dir.join("manifest.json")) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()); + if manifest.is_none() && !project_dir.join("index.html").is_file() { + return None; + } + let entry = manifest.as_ref().map_or("index.html", |m| m.entry.as_str()); + let findings = super::page_check::staged_findings(project_dir, entry, staging, replaces_css); + (!findings.is_empty()).then(|| { + format!( + "Browser-free page check failed:\n{}", + format_lint(&findings) + ) + }) +} + /// A fresh hidden directory in `dist/` for esbuild to write into, removed when /// dropped. /// @@ -2186,6 +2217,16 @@ fn fallback_bundle( strip_module_syntax(&sdk_source), strip_module_syntax(main) ); + if let Some(finding) = + super::page_check::javascript_error(&js, "dist/app.js fallback output", false) + { + return Ok(BuildReport::not_built("fallback", format!("The fallback bundler's output does not parse, so it was not installed:\n{}\n{FALLBACK_NEEDS_ESBUILD}", finding.msg))); + } + let staging = Staging::new(out.parent().unwrap_or(project_dir))?; + write_atomic(&staging.0.join("app.js"), js.as_bytes())?; + if let Some(error) = staged_page_error(project_dir, &staging.0, false) { + return Ok(BuildReport::not_built("fallback", error)); + } let verified = match node_check(&js, out.parent().unwrap_or(project_dir)) { Ok(None) => "It passed `node --check`.", Ok(Some(error)) => { @@ -3035,6 +3076,104 @@ mod tests { assert_eq!(names, vec!["app.js".to_string()]); } + #[test] + #[cfg(unix)] + fn a_page_check_failure_keeps_the_previous_javascript_and_stylesheet() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join("src")).unwrap(); + std::fs::write( + dir.path().join("src/main.ts"), + "document.body.dataset.ready = 'new';", + ) + .unwrap(); + std::fs::write( + dir.path().join("index.html"), + r#"

App

"#, + ) + .unwrap(); + let dist = dir.path().join("dist"); + std::fs::create_dir(&dist).unwrap(); + std::fs::write(dist.join("app.js"), "old();\n").unwrap(); + std::fs::write(dist.join("app.css"), "body{color:red}\n").unwrap(); + let make = || BundleMaking { + project_dir: dir.path(), + main: "document.body.dataset.ready = 'new';", + includes_sdk: false, + sdk: "none".into(), + sdk_version: None, + }; + let compiler = || { + EsbuildCommand { + program: "/bin/sh".into(), + lead: vec!["-c".into(), r#"for arg in "$@"; do case "$arg" in --outfile=*) printf 'updated();\n' > "${arg#--outfile=}";; esac; done"#.into(), "fixture-compiler".into()], + child_path: None, + source: "fixture", + } + }; + let failed = make().build(Some(compiler())).unwrap(); + assert!( + !failed.ok && failed.log.contains("dist/app.css"), + "{}", + failed.log + ); + assert_eq!( + std::fs::read_to_string(dist.join("app.js")).unwrap(), + "old();\n" + ); + assert_eq!( + std::fs::read_to_string(dist.join("app.css")).unwrap(), + "body{color:red}\n" + ); + std::fs::write(dir.path().join("index.html"), "

No stylesheet needed

").unwrap(); + assert!(make().build(Some(compiler())).unwrap().ok); + assert!(!dist.join("app.css").exists()); + } + + #[test] + fn a_failed_fallback_page_check_does_not_replace_its_previous_bundle() { + let dir = project(None, "document.body.dataset.ready = 'old';\n"); + let first = fallback_of(&dir); + assert!(first.ok, "{}", first.log); + let old = std::fs::read_to_string(dir.path().join("dist/app.js")).unwrap(); + std::fs::write(dir.path().join("dist/app.css"), "body{color:red}\n").unwrap(); + std::fs::write( + dir.path().join("index.html"), + r#"

App

"#, + ) + .unwrap(); + std::fs::write( + dir.path().join("src/main.ts"), + "document.body.dataset.ready = 'new';\n", + ) + .unwrap(); + let report = fallback_of(&dir); + assert!( + !report.ok && report.log.contains("assets/missing.js"), + "{}", + report.log + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("dist/app.js")).unwrap(), + old + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("dist/app.css")).unwrap(), + "body{color:red}\n" + ); + std::fs::create_dir(dir.path().join("assets")).unwrap(); + std::fs::write(dir.path().join("assets/missing.js"), "const value = 1;").unwrap(); + let fixed = fallback_of(&dir); + assert!(fixed.ok, "{}", fixed.log); + assert_ne!( + std::fs::read_to_string(dir.path().join("dist/app.js")).unwrap(), + old + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("dist/app.css")).unwrap(), + "body{color:red}\n" + ); + } + /// ⚠ esbuild used to write straight to a hidden temp outfile, and it puts /// an imported stylesheet beside the outfile with the extension swapped, so /// `dist/app.css` was never written and hidden `.css` files piled up. Needs @@ -3347,6 +3486,54 @@ mod tests { ); } + /// Issue #386, the input that was reported. Without esbuild the stripper + /// turned `{ omop_rules: OMOP_RULES, required_outputs: Object.keys(patient) }` + /// into `{ omop_rules , required_outputs , … }`, and dropped the local + /// modules `./prevent` and `./preventData` without a word. The literal on + /// one line now keeps its values and builds; the same literal across lines + /// and the local modules are refused, by line, with nothing written. + #[test] + fn the_reported_literal_keeps_its_values_and_local_modules_are_refused() { + let literal = + "const req = { omop_rules: OMOP_RULES, required_outputs: Object.keys(patient) };"; + assert_eq!(strip_colon_types(literal), literal); + let call = " br.agent.send({ omop_rules: OMOP_RULES, required_outputs: Object.keys(p) });"; + assert_eq!(strip_colon_types(call), call); + + let sdk = "export function createApp(x: unknown) { globalThis.req = x; }\n"; + let main = format!( + "import {{ createApp }} from \"./sdk\";\nconst OMOP_RULES = [\"r1\"];\n\ + const patient = {{ age: 1 }};\n{literal}\ncreateApp(req);\n" + ); + let dir = project(Some(sdk), &main); + let report = fallback_of(&dir); + assert!(report.ok, "{}", report.log); + let js = std::fs::read_to_string(dir.path().join("dist/app.js")).unwrap(); + assert!(js.contains(literal), "{js}"); + if let Some(parsed) = node_parses(&js) { + parsed.expect("the fallback bundle parses"); + } + + for (main, expect) in [ + ( + "import { createApp } from \"./sdk\";\nimport { risk } from \"./prevent\";\n\ + import { OMOP_RULES } from \"./preventData\";\ncreateApp(risk(OMOP_RULES));\n", + "./prevent\"", + ), + ( + "import { createApp } from \"./sdk\";\nconst req = {\n omop_rules: OMOP_RULES,\n \ + required_outputs: Object.keys(patient),\n};\ncreateApp(req);\n", + "line 3", + ), + ] { + let dir = project(Some(sdk), main); + let report = fallback_of(&dir); + assert!(!report.ok, "refused: {main}"); + assert!(report.log.contains(expect), "{main}\n---\n{}", report.log); + assert!(!dir.path().join("dist/app.js").exists(), "{main}"); + } + } + /// The banner is the bundle's own record of what built it, read back from /// the file; a bundle from before banners is recognised as a fallback only /// by the marker the stripper used to write. diff --git a/crates/biorouter-mcp/src/agent_drafter/export.rs b/crates/biorouter-mcp/src/agent_drafter/export.rs index b72090472..805ae6c38 100644 --- a/crates/biorouter-mcp/src/agent_drafter/export.rs +++ b/crates/biorouter-mcp/src/agent_drafter/export.rs @@ -693,6 +693,12 @@ fn prepare( // races a page load's build, and a build that failed on these very inputs // is not run again. match super::ensure_bundle_fresh(&store, id) { + Ok(super::BundleCheck { + quality_error: Some(error), + .. + }) => { + return Err(ExportError::Unexportable(error)); + } Ok(check) if check.build.as_ref().is_some_and(|build| !build.ok) => { report.notes.push(if check.has_bundle { "the app's bundle could not be rebuilt; the export carries the last one that \ @@ -2287,6 +2293,34 @@ mod tests { PathBuf::from(&report.target) } + #[test] + fn a_broken_page_is_not_exported_even_with_the_lint_override() { + let f = fixture(); + let app = f.app("broken-page", ArtifactKind::Static); + std::fs::write( + app.join("index.html"), + "", + ) + .unwrap(); + + let error = f + .export( + "broken-page", + &ExportRequest { + allow_lint_errors: true, + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!(error, ExportError::Unexportable(_)), "{error}"); + assert!(error.to_string().contains("index.html"), "{error}"); + assert_eq!(std::fs::read_dir(&f.work).unwrap().count(), 0); + + let error = legacy_export_map(&f.store_root, "broken-page").unwrap_err(); + assert!(matches!(error, ExportError::Unexportable(_)), "{error}"); + assert!(error.to_string().contains("index.html"), "{error}"); + } + #[test] fn binary_assets_round_trip_byte_identical_and_runtime_data_stays_home() { let f = fixture(); diff --git a/crates/biorouter-mcp/src/agent_drafter/mod.rs b/crates/biorouter-mcp/src/agent_drafter/mod.rs index 505905b5a..b67ed8ea0 100644 --- a/crates/biorouter-mcp/src/agent_drafter/mod.rs +++ b/crates/biorouter-mcp/src/agent_drafter/mod.rs @@ -21,6 +21,7 @@ pub mod declare; pub mod evidence; pub mod export; pub mod manifest; +mod page_check; pub mod render; pub mod resolved; pub mod store; @@ -1463,6 +1464,7 @@ fn build_inputs_fingerprint(dir: &Path, manifest: &Manifest) -> u64 { .hash(&mut h); manifest.built_at.hash(&mut h); manifest.sdk_hash.hash(&mut h); + page_check::inputs_fingerprint(dir, &manifest.entry).hash(&mut h); bundle::sdk_fingerprint().hash(&mut h); bundle::esbuild_available().hash(&mut h); h.finish() @@ -1490,6 +1492,36 @@ fn build_and_stamp_locked( id: &str, dir: &Path, ) -> std::io::Result { + let manifest = store.load_manifest(id)?; + let findings = page_check::findings(dir, &manifest.entry, true); + if !findings.is_empty() { + return Ok(bundle::BuildReport::not_built( + "none", + format!( + "Browser-free page check failed:\n{}", + bundle::format_lint(&findings) + ), + )); + } + if manifest.kind == ArtifactKind::Static && !dir.join("src/main.ts").is_file() { + let findings = page_check::findings(dir, &manifest.entry, false); + return Ok(if findings.is_empty() { + bundle::BuildReport { + ok: true, + used: "none".into(), + log: page_check::PASSED.into(), + stamp: None, + } + } else { + bundle::BuildReport::not_built( + "none", + format!( + "Browser-free page check failed:\n{}", + bundle::format_lint(&findings) + ), + ) + }); + } // Keep a vendored SDK that a NEWER Biorouter put there: its bundle's banner // names that version and the very SDK the app still vendors. Refreshing it // would rebuild the app down to this binary's older runtime. The version @@ -1511,6 +1543,20 @@ fn build_and_stamp_locked( keep_sdk_from: keep_sdk_from.clone(), }, )?; + if report.ok { + let findings = page_check::findings(dir, &manifest.entry, false); + if findings.is_empty() { + report.log = format!("{}\n{}", report.log, page_check::PASSED); + } else { + report.ok = false; + report.stamp = None; + report.log = format!( + "Browser-free page check failed:\n{}\n{}", + bundle::format_lint(&findings), + report.log + ); + } + } if let Some(kept) = keep_sdk_from { report.log = format!( "Kept src/sdk.ts: it is the App SDK of Biorouter {kept}, which is newer than this one ({}).\n{}", @@ -1546,6 +1592,8 @@ pub fn rebuild_and_stamp(store: &ArtifactStore, id: &str) -> std::io::Result, /// Why the bundle was stale, `None` when it was fresh. pub stale_because: Option<&'static str>, /// The build that ran — or the remembered result of an identical earlier @@ -1564,6 +1612,9 @@ impl BundleCheck { /// The failed build behind a missing bundle, when that is the situation: /// the app needs a bundle, has none, and the build said why. pub fn missing_bundle_log(&self) -> Option { + if let Some(error) = &self.quality_error { + return Some(error.clone()); + } if !self.needs_bundle || self.has_bundle { return None; } @@ -1609,7 +1660,15 @@ pub fn ensure_bundle_fresh(store: &ArtifactStore, id: &str) -> std::io::Result { } #[tokio::test] - async fn harness_errors_block_launch_and_export() { + async fn harness_errors_block_build_launch_and_export() { let (_d, s) = server(); let mut p = create("Broken Harness", None); p.html = Some( @@ -4315,8 +4392,8 @@ br.run("hello", "#missing"); check_only: None, })) .await - .unwrap(); - let build_text = text_of(&build); + .unwrap_err(); + let build_text = build.message; assert!(build_text.contains("ERROR")); assert!(build_text.contains("#missing")); @@ -4404,6 +4481,7 @@ br.run("hello", "#missing"); .await .unwrap(); assert!(text_of(&res).contains("dist/app.js")); + assert!(text_of(&res).contains(page_check::PASSED)); assert!(s.store().file_exists("launchy", "dist/app.js")); assert!(s .store() @@ -4419,6 +4497,136 @@ br.run("hello", "#missing"); .await .unwrap(); assert!(text_of(&res).contains("/apps/launchy/")); + assert!(text_of(&res).contains("runtime behavior was not verified")); + } + + #[tokio::test] + async fn browser_free_checks_block_broken_static_pages_without_a_bundler() { + let (_d, s) = server(); + let mut p = create("Broken static", Some("static")); + p.html = Some("

Page

".into()); + s.create_app_inner(p, None, &KbCaller::restricted()) + .await + .unwrap(); + let check = ensure_bundle_fresh(&s.store(), "broken-static").unwrap(); + assert!(!check.needs_bundle && check.build.is_none()); + assert!(check.quality_error.as_ref().unwrap().contains("index.html")); + assert_eq!(check.missing_bundle_log(), check.quality_error); + let build = s + .build_app(Parameters(BuildAppParams { + id: "broken-static".into(), + check_only: None, + })) + .await + .unwrap_err(); + assert!(build.message.contains("index.html")); + let launch = s + .launch_app(Parameters(AppIdParams { + id: "broken-static".into(), + })) + .await + .unwrap_err(); + assert!(launch.message.contains("invalid browser JavaScript")); + assert!(s + .store() + .load_manifest("broken-static") + .unwrap() + .built_at + .is_none()); + } + + #[tokio::test] + async fn browser_free_checks_allow_static_pages_with_advisory_lint_warnings() { + let (_d, s) = server(); + let mut p = create("Checked static", Some("static")); + p.html = Some("

Static content

".into()); + s.create_app_inner(p, None, &KbCaller::restricted()) + .await + .unwrap(); + let findings = bundle::lint_app(&s.store().artifact_dir("checked-static").unwrap()); + assert!(findings.iter().any(|f| f.level == bundle::LintLevel::Warn)); + assert!(!findings.iter().any(|f| f.level == bundle::LintLevel::Error)); + let result = s + .build_app(Parameters(BuildAppParams { + id: "checked-static".into(), + check_only: None, + })) + .await + .unwrap(); + assert!(text_of(&result).contains("no TypeScript bundle needed")); + assert!(text_of(&result).contains(page_check::PASSED)); + assert!(!s.store().file_exists("checked-static", "dist/app.js")); + assert!(s + .launch_app(Parameters(AppIdParams { + id: "checked-static".into() + })) + .await + .is_ok()); + } + + #[test] + fn page_edits_retry_a_failed_build_and_preserve_the_previous_bundle() { + bundle::tests_without_esbuild::run(|| { + let (_d, store) = agentic_app("page-fix", DEFAULT_MAIN); + let first = rebuild_and_stamp(&store, "page-fix").unwrap(); + assert!(first.ok, "{}", first.log); + let original = store.read_file("page-fix", "dist/app.js").unwrap(); + store + .write_file("page-fix", "index.html", "") + .unwrap(); + let broken = ensure_bundle_fresh(&store, "page-fix").unwrap(); + assert!(broken.quality_error.is_some()); + assert_eq!( + store.read_file("page-fix", "dist/app.js").unwrap(), + original + ); + store + .write_file("page-fix", "index.html", "

Fixed page

") + .unwrap(); + let fixed = ensure_bundle_fresh(&store, "page-fix").unwrap(); + assert!(fixed.quality_error.is_none()); + assert!(fixed.missing_bundle_log().is_none()); + + store + .write_file( + "page-fix", + "src/main.ts", + "import { createApp } from \"./sdk\";\ncreateApp();\n", + ) + .unwrap(); + store + .write_file( + "page-fix", + "index.html", + "", + ) + .unwrap(); + let missing = ensure_bundle_fresh(&store, "page-fix").unwrap(); + assert!(missing.build.as_ref().is_some_and(|b| !b.ok)); + store + .write_file("page-fix", "assets/helper.js", "const value = 1;") + .unwrap(); + let added = ensure_bundle_fresh(&store, "page-fix").unwrap(); + assert!(!added.remembered); + assert!(added.build.as_ref().is_some_and(|b| b.ok), "{added:?}"); + }); + } + + #[test] + fn a_corrupted_fresh_bundle_is_blocked_without_rebuilding_it() { + let (_d, store) = agentic_app("corrupt", DEFAULT_MAIN); + fake_bundle(&store, "corrupt", "esbuild", bundle::biorouter_version()); + store + .write_file("corrupt", "dist/app.js", "(()=>{const value = ;") + .unwrap(); + let check = ensure_bundle_fresh(&store, "corrupt").unwrap(); + assert!(check.build.is_none()); + assert!(check + .quality_error + .as_ref() + .unwrap() + .contains("dist/app.js")); + assert_eq!(check.missing_bundle_log(), check.quality_error); } // --- build freshness, the build lock, and what a build is stamped with --- diff --git a/crates/biorouter-mcp/src/agent_drafter/page_check.rs b/crates/biorouter-mcp/src/agent_drafter/page_check.rs new file mode 100644 index 000000000..ff8175675 --- /dev/null +++ b/crates/biorouter-mcp/src/agent_drafter/page_check.rs @@ -0,0 +1,451 @@ +use std::collections::HashSet; +use std::hash::{Hash, Hasher}; +use std::io::{self, Read}; +use std::path::{Path, PathBuf}; + +use super::bundle::{LintFinding, LintLevel}; +use super::render::{is_handler_name, scan_tags, script_is_executable, Tag, TagAttr}; + +pub(super) const PASSED: &str = "Browser-free page check passed: entry, local scripts/styles and JavaScript structure checked. Runtime behavior was not verified."; +const MAX_SOURCE_BYTES: u64 = 16 * 1024 * 1024; + +fn error(msg: impl Into) -> LintFinding { + LintFinding { + level: LintLevel::Error, + msg: msg.into(), + } +} + +fn read_source(root: &Path, path: &Path) -> io::Result { + let canonical = path.canonicalize()?; + if !canonical.starts_with(root.canonicalize()?) { + return Err(io::Error::other("the file is outside the app directory")); + } + if !canonical.metadata()?.is_file() { + return Err(io::Error::other("the path is not a regular source file")); + } + let mut source = String::new(); + std::fs::File::open(canonical)? + .take(MAX_SOURCE_BYTES + 1) + .read_to_string(&mut source)?; + if source.len() as u64 > MAX_SOURCE_BYTES { + return Err(io::Error::other( + "the source exceeds the 16 MiB check limit", + )); + } + Ok(source) +} + +fn attr<'a>(attrs: &'a [TagAttr], key: &str) -> Option<&'a str> { + attrs + .iter() + .find(|a| !a.duplicate && a.key == key) + .map(|a| a.value.as_str()) +} + +/// Resolve against the same app-root base the served page receives. URLs are +/// parsed, never fetched; canonicalization below also catches encoded traversal. +fn local_asset(root: &Path, value: &str) -> Result, String> { + let base = url::Url::parse("https://biorouter.invalid/apps/check/").expect("static URL"); + let value = value.trim(); + let resolved = base.join(value).map_err(|e| e.to_string())?; + if resolved.origin() != base.origin() { + return Ok(None); + } + let relative = resolved.path().strip_prefix(base.path()).ok_or_else(|| { + "the URL leaves the app's serving directory; use an app-relative path".to_string() + })?; + let file_base = url::Url::from_directory_path(root) + .map_err(|()| "the app directory is not an absolute path".to_string())?; + let decoded = file_base + .join(relative) + .map_err(|e| e.to_string())? + .to_file_path() + .map_err(|()| "the URL is not a local file path".to_string())?; + let relative = decoded.strip_prefix(root).map_err(|e| e.to_string())?; + if !relative.starts_with("assets") && !relative.starts_with("dist") { + return Err("the app serves local scripts and styles only from assets/ or dist/; move the file there and update the URL".into()); + } + Ok(Some(decoded)) +} + +pub(super) fn inputs_fingerprint(root: &Path, entry: &str) -> u64 { + let mut hash = std::collections::hash_map::DefaultHasher::new(); + if let Ok(html) = read_source(root, &root.join(entry)) { + html.hash(&mut hash); + scan_tags(&html, |tag| { + if let Tag::Start { name, attrs, .. } = tag { + let value = match name.as_str() { + "script" => attr(&attrs, "src"), + "link" => attr(&attrs, "href"), + _ => None, + }; + if let Some(Ok(Some(path))) = value.map(|v| local_asset(root, v)) { + path.hash(&mut hash); + let metadata = path + .canonicalize() + .ok() + .filter(|p| root.canonicalize().is_ok_and(|r| p.starts_with(r))) + .and_then(|p| p.metadata().ok()); + metadata + .as_ref() + .map(std::fs::Metadata::len) + .hash(&mut hash); + metadata.and_then(|m| m.modified().ok()).hash(&mut hash); + } + } + }); + } + hash.finish() +} + +pub(super) fn javascript_error(source: &str, label: &str, module: bool) -> Option { + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&tree_sitter_javascript::LANGUAGE.into()) + .expect("the compiled JavaScript grammar is valid"); + let Some(tree) = parser.parse(source, None) else { + return Some(error(format!( + "{label}: JavaScript could not be parsed. Fix the script and rebuild." + ))); + }; + let mut nodes = vec![tree.root_node()]; + while let Some(node) = nodes.pop() { + let invalid = node.is_error() + || node.is_missing() + || node.kind().starts_with("jsx_") + || (!module && matches!(node.kind(), "import_statement" | "export_statement")); + if invalid { + let at = node.start_position(); + return Some(error(format!( + "{label}: invalid browser JavaScript near line {}, column {}. Fix the syntax (and use type=\"module\" for imports/exports), then rebuild.", + at.row + 1, + at.column + 1 + ))); + } + let mut cursor = node.walk(); + nodes.extend(node.children(&mut cursor)); + } + None +} + +pub(super) fn findings(root: &Path, entry: &str, building: bool) -> Vec { + findings_with(root, entry, building, None, false) +} + +pub(super) fn staged_findings( + root: &Path, + entry: &str, + staging: &Path, + replaces_css: bool, +) -> Vec { + findings_with(root, entry, false, Some(staging), replaces_css) +} + +fn authored_asset<'a>( + name: &str, + attrs: &'a [TagAttr], + script_text: Option<&str>, + entry: &str, + script_number: &mut usize, + out: &mut Vec, +) -> Option<(&'a str, bool, bool)> { + for handler in attrs + .iter() + .filter(|a| !a.duplicate && is_handler_name(&a.key)) + { + let source = format!("function __handler(event){{\n{}\n}}", handler.value); + if let Some(finding) = + javascript_error(&source, &format!("{entry} {} handler", handler.key), false) + { + out.push(finding); + } + } + if name == "script" { + let kind = attr(attrs, "type"); + if !script_is_executable(kind, attr(attrs, "language")) { + return None; + } + if kind.is_some_and(|t| t.trim().eq_ignore_ascii_case("importmap")) { + if let Err(e) = + serde_json::from_str::(script_text.unwrap_or_default()) + { + out.push(error(format!( + "{entry} importmap: invalid JSON ({e}). Fix the import map before launch." + ))); + } + return None; + } + let module = kind.is_some_and(|t| t.trim().eq_ignore_ascii_case("module")); + if let Some(src) = attr(attrs, "src") { + return Some((src, true, module)); + } + *script_number += 1; + if let Some(finding) = javascript_error( + script_text.unwrap_or_default(), + &format!("{entry} inline script {script_number}"), + module, + ) { + out.push(finding); + } + } else if name == "link" + && attr(attrs, "rel").is_some_and(|rel| { + rel.split_ascii_whitespace() + .any(|word| word.eq_ignore_ascii_case("stylesheet")) + }) + { + return attr(attrs, "href").map(|href| (href, false, false)); + } + None +} + +fn findings_with( + root: &Path, + entry: &str, + building: bool, + staging: Option<&Path>, + replaces_css: bool, +) -> Vec { + let mut out = Vec::new(); + let html = match read_source(root, &root.join(entry)) { + Ok(html) if !html.trim().is_empty() => html, + Ok(_) => return vec![error(format!("{entry}: the entry page is empty. Add its HTML before building or launching."))], + Err(e) => return vec![error(format!("{entry}: the entry page could not be read ({e}). Restore the file or fix the manifest entry."))], + }; + let mut checked = HashSet::new(); + let mut bundle_loaded = false; + let mut template_depth = 0usize; + let mut script_number = 0usize; + scan_tags(&html, |tag| { + let Tag::Start { + name, + attrs, + script_text, + } = tag + else { + if let Tag::End { name } = tag { + if name == "template" { + template_depth = template_depth.saturating_sub(1); + } + } + return; + }; + if name == "template" { + template_depth += 1; + } + if template_depth > 0 { + return; + } + let asset = authored_asset( + &name, + &attrs, + script_text, + entry, + &mut script_number, + &mut out, + ); + if let Some((url, script, module)) = asset { + match local_asset(root, url) { + Ok(Some(path)) => { + let generated = + path == root.join("dist/app.js") || path == root.join("dist/app.css"); + if building && generated && root.join("src/main.ts").is_file() { + return; + } + if script && path == root.join("dist/app.js") { + bundle_loaded = true; + } + if !checked.insert((path.clone(), script, module)) { + return; + } + let replaced = path == root.join("dist/app.js") + || (replaces_css && path == root.join("dist/app.css")); + let path = staging.filter(|_| replaced).map_or(path.clone(), |dir| { + dir.join(path.file_name().expect("generated asset has a name")) + }); + match read_source(root, &path) { + Ok(source) if script => { + if let Some(finding) = javascript_error(&source, &format!("{entry} script '{url}'"), module) { + out.push(finding); + } + } + Ok(_) => {} + Err(e) => out.push(error(format!("{entry}: local {} '{url}' could not be read ({e}). Add the file or correct its path.", if script { "script" } else { "stylesheet" }))), + } + } + Ok(None) => {} + Err(e) => out.push(error(format!( + "{entry}: asset '{url}' is invalid ({e}). Correct the URL." + ))), + } + } + }); + let bundle = staging.map_or_else(|| root.join("dist/app.js"), |dir| dir.join("app.js")); + if !building && bundle.is_file() && !bundle_loaded { + match read_source(root, &bundle) { + Ok(source) => { + if let Some(finding) = javascript_error(&source, "dist/app.js", false) { + out.push(finding); + } + } + Err(e) => out.push(error(format!( + "dist/app.js could not be checked ({e}). Rebuild the bundle." + ))), + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn page(html: &str) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("index.html"), html).unwrap(); + dir + } + + #[test] + fn broken_scripts_handlers_and_local_assets_are_actionable() { + let dir = page( + r#""#, + ); + let errors = findings(dir.path(), "index.html", false); + assert_eq!(errors.len(), 4); + assert!(errors.iter().all(|f| f.msg.contains("index.html"))); + assert!(errors.iter().any(|f| f.msg.contains("missing.js?v=1#x"))); + assert!(errors.iter().any(|f| f.msg.contains("missing.css"))); + } + + #[test] + fn valid_modules_data_templates_and_query_urls_are_not_runtime_tests() { + let dir = page( + r#""#, + ); + std::fs::create_dir(dir.path().join("assets")).unwrap(); + std::fs::write(dir.path().join("assets/helper.js"), "const x = 1;").unwrap(); + std::fs::write(dir.path().join("assets/styles.css"), "body { color: red; }").unwrap(); + assert!(findings(dir.path(), "index.html", false).is_empty()); + assert!(PASSED.contains("Runtime behavior was not verified")); + } + + #[test] + fn nested_entries_use_the_served_app_root_and_encoded_paths() { + let dir = page("placeholder"); + std::fs::create_dir(dir.path().join("pages")).unwrap(); + std::fs::write( + dir.path().join("pages/home.html"), + r#""#, + ) + .unwrap(); + std::fs::create_dir(dir.path().join("assets")).unwrap(); + std::fs::write(dir.path().join("assets/helper file.js"), "const x = 1;").unwrap(); + assert!(findings(dir.path(), "pages/home.html", false).is_empty()); + std::fs::write( + dir.path().join("pages/home.html"), + r#""#, + ) + .unwrap(); + assert!(findings(dir.path(), "pages/home.html", false)[0] + .msg + .contains("leaves the app")); + } + + #[test] + fn empty_entries_importmaps_and_corrupted_existing_bundles_fail() { + let dir = page(""); + assert!(findings(dir.path(), "index.html", false)[0] + .msg + .contains("empty")); + std::fs::write( + dir.path().join("index.html"), + "", + ) + .unwrap(); + assert!(findings(dir.path(), "index.html", false)[0] + .msg + .contains("importmap")); + std::fs::write(dir.path().join("index.html"), "

Valid page

").unwrap(); + std::fs::create_dir(dir.path().join("dist")).unwrap(); + std::fs::write(dir.path().join("dist/app.js"), "(()=>{ const x = ;").unwrap(); + assert!(findings(dir.path(), "index.html", false)[0] + .msg + .contains("dist/app.js")); + assert!(javascript_error("
", "inline", false).is_some()); + assert!(javascript_error("import x from './x.js';", "inline", false).is_some()); + } + + #[test] + fn less_common_handlers_share_the_renderers_predicate() { + let dir = page( + r#"
"#, + ); + let errors = findings(dir.path(), "index.html", false); + assert_eq!(errors.len(), 2); + assert!(errors.iter().any(|f| f.msg.contains("onwheel handler"))); + assert!(errors.iter().any(|f| f.msg.contains("onscroll handler"))); + } + + #[test] + fn source_paths_cannot_read_directories_or_escape_the_app() { + let dir = page(r#""#); + std::fs::create_dir(dir.path().join("assets")).unwrap(); + assert!(findings(dir.path(), "index.html", false)[0] + .msg + .contains("not a regular source file")); + let outside = tempfile::tempdir().unwrap(); + std::fs::write(outside.path().join("external.js"), "const x = 1;").unwrap(); + assert!(read_source(dir.path(), &outside.path().join("external.js")) + .unwrap_err() + .to_string() + .contains("outside the app")); + #[cfg(unix)] + { + std::os::unix::fs::symlink( + outside.path().join("external.js"), + dir.path().join("assets/escape.js"), + ) + .unwrap(); + std::fs::write( + dir.path().join("index.html"), + r#""#, + ) + .unwrap(); + assert!(findings(dir.path(), "index.html", false)[0] + .msg + .contains("outside the app")); + } + } + + #[test] + fn each_script_loading_context_is_checked_even_after_a_stylesheet() { + let dir = page( + r#""#, + ); + std::fs::create_dir(dir.path().join("assets")).unwrap(); + std::fs::write(dir.path().join("assets/shared.js"), "export const x = 1;").unwrap(); + assert_eq!(findings(dir.path(), "index.html", false).len(), 1); + std::fs::write(dir.path().join("index.html"), r#""#).unwrap(); + assert_eq!(findings(dir.path(), "index.html", false).len(), 1); + std::fs::write( + dir.path().join("index.html"), + r#""#, + ) + .unwrap(); + assert!(findings(dir.path(), "index.html", false).is_empty()); + } + + #[test] + fn local_files_must_have_a_served_asset_path() { + let dir = + page(r#""#); + std::fs::write(dir.path().join("helper.js"), "const x = 1;").unwrap(); + std::fs::write(dir.path().join("style.css"), "body{}").unwrap(); + let errors = findings(dir.path(), "index.html", false); + assert_eq!(errors.len(), 2); + assert!(errors + .iter() + .all(|f| f.msg.contains("only from assets/ or dist/"))); + } +} diff --git a/crates/biorouter-mcp/src/agent_drafter/render.rs b/crates/biorouter-mcp/src/agent_drafter/render.rs index dcb08e6b4..bf9537e7e 100644 --- a/crates/biorouter-mcp/src/agent_drafter/render.rs +++ b/crates/biorouter-mcp/src/agent_drafter/render.rs @@ -476,7 +476,7 @@ const HANDLER_BINDER_ID: &str = "biorouter-inline-handlers"; const HANDLER_MARKER: &str = "data-br-on-"; /// Whether an attribute name is an `on…` event handler's. -fn is_handler_name(key: &str) -> bool { +pub(crate) fn is_handler_name(key: &str) -> bool { key.len() > 2 && key.starts_with("on") } @@ -692,20 +692,20 @@ const JS_MIME_TYPES: &[&str] = &[ /// One attribute of a start tag, as the tokenizer reads it. #[derive(Debug, Clone)] -struct TagAttr { +pub(crate) struct TagAttr { /// Lowercase name. - key: String, + pub(crate) key: String, /// Decoded value. - value: String, + pub(crate) value: String, /// Where the whole attribute, name through value, sits in the page. - span: std::ops::Range, + pub(crate) span: std::ops::Range, /// A later attribute of the same name on the same tag, which a browser /// drops (the first occurrence wins). - duplicate: bool, + pub(crate) duplicate: bool, } /// A tag as [`scan_tags`] reports it. -enum Tag<'a> { +pub(crate) enum Tag<'a> { Start { /// Lowercase name. name: String, @@ -727,7 +727,7 @@ enum Tag<'a> { /// It is deliberately a scanner, not a parser, and errs toward MISSING code /// (which then stays blocked, as it always was) rather than inventing it: a /// hash can only ever allow text that is really in the page. -fn scan_tags<'a>(html: &'a str, mut visit: impl FnMut(Tag<'a>)) { +pub(crate) fn scan_tags<'a>(html: &'a str, mut visit: impl FnMut(Tag<'a>)) { let bytes = html.as_bytes(); let mut i = 0; while i < bytes.len() { @@ -864,7 +864,7 @@ pub fn authored_inline_code(html: &str) -> InlineCode { code } -fn script_is_executable(type_attr: Option<&str>, language: Option<&str>) -> bool { +pub(crate) fn script_is_executable(type_attr: Option<&str>, language: Option<&str>) -> bool { let mime = match (type_attr.map(str::trim), language.map(str::trim)) { (Some(t), _) if !t.is_empty() => t.to_ascii_lowercase(), (Some(_), _) => return true, @@ -1619,28 +1619,30 @@ port_alive() { curl -sf -o /dev/null --max-time 1 "http://127.0.0.1:$1/status" 2 # ── 3. A daemon whose secret this launcher knows ────────────────────────── # A daemon serves an app's page only to a browser holding that app's access # cookie, and the one-time link that sets the cookie is handed only to a caller -# holding the daemon's secret. So this launcher uses a daemon it started (its -# port and secret are kept, readable by you alone, in $LAUNCHER_STATE) or one you -# name with BIOROUTERD_PORT and BIOROUTER_SERVER__SECRET_KEY. It cannot open the -# app on any other daemon, whatever answers /status. +# holding the daemon's secret. The profile's verified background service comes +# first, then a daemon this launcher started or one you explicitly name. LAUNCHER_STATE="$LAUNCHER_HOME/daemon" SECRET="" LOG="" +SHARED_SOCKET="" +SHARED_IDENTITY="" new_secret() { od -An -N32 -tx1 /dev/urandom | tr -d ' \n'; } # Every request that carries the secret hands it to curl on stdin, never on a # command line, where other accounts on this machine can read it. secret_ok() { + if [ -n "$SHARED_SOCKET" ]; then shared_daemon apps >/dev/null; return; fi printf 'X-Secret-Key: %s\n' "$2" \ | curl -sf -o /dev/null --max-time 2 -H @- "http://127.0.0.1:$1/apps" 2>/dev/null } # The path of this app's one-time launch link, from the daemon on :$1. launch_path() { - printf 'X-Secret-Key: %s\n' "$2" \ - | curl -sf --max-time 5 -X POST -H @- "http://127.0.0.1:$1/apps/$APP_ID/launch" 2>/dev/null \ - | sed -n 's|.*"path":"\(/apps/[A-Za-z0-9_-]*/?t=[0-9a-f]*\)".*|\1|p' + { if [ -n "$SHARED_SOCKET" ]; then shared_daemon launch "$APP_ID" + else printf 'X-Secret-Key: %s\n' "$2" \ + | curl -sf --max-time 5 -X POST -H @- "http://127.0.0.1:$1/apps/$APP_ID/launch" 2>/dev/null; fi + } | sed -n 's|.*"path":"\(/apps/[A-Za-z0-9_-]*/?t=[0-9a-f]*\)".*|\1|p' } save_daemon() { @@ -1650,6 +1652,132 @@ save_daemon() { ) || echo " (could not remember this daemon; the next run starts another)" >&2 } +# Read the existing shared-daemon protocol, not a port scan. Python is optional: +# an installation without it keeps the standalone launcher path below. +shared_daemon() { + command -v python3 >/dev/null 2>&1 || return 1 + python3 - "$CFG" "$SHARED_IDENTITY" "$@" <<'PY' +import hashlib, http.client, json, os, re, socket, stat, sys +from urllib.parse import urlsplit + +def private(path, kind): + s = os.lstat(path) + valid = {'file': stat.S_ISREG, 'directory': stat.S_ISDIR, 'socket': stat.S_ISSOCK}[kind] + if not valid(s.st_mode) or s.st_uid != os.getuid() or s.st_mode & 0o7777 != (0o700 if kind == 'directory' else 0o600): + raise ValueError('private metadata required') + return s + +def record(path): + private(path, 'file') + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + try: + s = os.fstat(fd) + if not stat.S_ISREG(s.st_mode) or s.st_uid != os.getuid() or s.st_mode & 0o7777 != 0o600 or s.st_nlink != 1 or s.st_size > 16384: + raise ValueError('invalid metadata') + with os.fdopen(fd, 'rb', closefd=False) as f: + data = f.read(16385) + if len(data) > 16384: + raise ValueError('oversized metadata') + return json.loads(data) + finally: + os.close(fd) + +try: + root = os.environ.get('BIOROUTER_PATH_ROOT', '') + xdg = os.environ.get('XDG_STATE_HOME', '') + state = os.path.join(root, 'state') if root.strip() else os.path.join(xdg if os.path.isabs(xdg) else os.path.expanduser('~/.local/state'), 'biorouter') + directory = os.path.join(state, 'daemon') + private(directory, 'directory') + config = os.path.realpath(sys.argv[1]) + profile = record(os.path.join(config, 'daemon-profile.json')) + runtime = record(os.path.join(directory, 'runtime.json')) + uuid = re.compile(r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', re.I) + if profile.get('version') != 1 or not isinstance(profile.get('profile_id'), str) or not uuid.fullmatch(profile['profile_id']) or profile.get('config_dir') != config: + raise ValueError('profile mismatch') + endpoint = runtime.get('endpoint', {}) + path = endpoint.get('path') + secret = runtime.get('api_secret') + if runtime.get('version') != 1 or runtime.get('profile_id') != profile['profile_id'] or not isinstance(runtime.get('instance_id'), str) or not uuid.fullmatch(runtime['instance_id']) or type(runtime.get('pid')) is not int or runtime['pid'] < 1 or type(runtime.get('user_action_installed')) is not bool or endpoint.get('kind') != 'unix' or path != os.path.join(directory, 'daemon.sock') or not os.path.isabs(path) or '\n' in path or not isinstance(secret, str) or len(secret) < 32 or not re.fullmatch(r'[A-Za-z0-9._~+/=-]+', secret): + raise ValueError('runtime mismatch') + expected = {key: runtime[key] for key in ('version', 'profile_id', 'instance_id', 'pid', 'user_action_installed')} + pinned = json.loads(sys.argv[2]) if sys.argv[2] else None + if pinned and (pinned['identity'] != expected or pinned['key_digest'] != hashlib.sha256(secret.encode()).hexdigest()): + raise ValueError('daemon instance changed') + private(path, 'socket') + class Connection(http.client.HTTPConnection): + connected = False + def connect(self): + if self.connected: + raise ValueError('daemon connection changed') + self.connected = True + private(path, 'socket') + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self.sock.settimeout(2) + self.sock.connect(path) + connection = Connection('localhost', timeout=2) + def request(route): + connection.request('GET', route, headers={'X-Secret-Key': secret}) + response = connection.getresponse() + body = response.read(16385) + if response.status != 200 or len(body) > 16384: + raise ValueError('daemon refused') + return json.loads(body) + identity = request('/daemon/identity') + if identity != expected: + raise ValueError('identity mismatch') + origin = urlsplit(request('/headless/health').get('api_base_url', '')) + if origin.scheme != 'http' or origin.hostname not in ('127.0.0.1', 'localhost', '0.0.0.0') or origin.username or origin.password or origin.path not in ('', '/') or origin.query or origin.fragment or not origin.port: + raise ValueError('not a local listener') + named = os.environ.get('BIOROUTERD_PORT') + if (named and int(named) != origin.port) or (pinned and pinned['port'] != origin.port): + raise ValueError('another port was requested') + if len(sys.argv) == 3: + print(origin.port) + print(secret) + print(path) + print(json.dumps({'identity': expected, 'port': origin.port, 'key_digest': hashlib.sha256(secret.encode()).hexdigest()}, separators=(',', ':'))) + else: + operation = sys.argv[3] + if operation == 'import': + archive = sys.argv[4] + source = os.path.basename(archive)[:-5] + if not re.fullmatch(r'[a-z0-9][a-z0-9-]{0,79}', source): + raise ValueError('invalid archive filename') + size = os.stat(archive).st_size + if size > 128 * 1024 * 1024: + raise ValueError('oversized archive') + boundary = 'biorouter-' + os.urandom(16).hex() + header = (f'--{boundary}\r\nContent-Disposition: form-data; name="file"; filename="{source}.brkb"\r\nContent-Type: application/octet-stream\r\n\r\n').encode() + footer = f'\r\n--{boundary}--\r\n'.encode() + connection.sock.settimeout(120) + connection.putrequest('POST', '/knowledge/bases/import') + connection.putheader('X-Secret-Key', secret) + connection.putheader('Content-Type', f'multipart/form-data; boundary={boundary}') + connection.putheader('Content-Length', str(len(header) + size + len(footer))) + connection.endheaders(header) + with open(archive, 'rb') as stream: + while True: + chunk = stream.read(65536) + if not chunk: + break + connection.send(chunk) + connection.send(footer) + else: + route = '/apps' if operation == 'apps' else '/apps/' + sys.argv[4] + '/launch' + connection.request('GET' if operation == 'apps' else 'POST', route, headers={'X-Secret-Key': secret}) + response = connection.getresponse() + body = response.read(1024 * 1024 + 1) + if response.status != 200 or len(body) > 1024 * 1024: + raise ValueError('daemon refused') + print(body.decode('utf-8')) + connection.close() +except Exception: + if sys.argv[2]: + print("Biorouter's background service changed or closed; run the launcher again.", file=sys.stderr) + sys.exit(1) +PY +} + start_daemon() { if [ -n "${BIOROUTER_SERVER__SECRET_KEY:-}" ] && [ -n "${BIOROUTERD_PORT:-}" ] \ && secret_ok "$BIOROUTERD_PORT" "$BIOROUTER_SERVER__SECRET_KEY"; then @@ -1657,6 +1785,13 @@ start_daemon() { echo "Using the Biorouter daemon on :$PORT" return 0 fi + shared="$(shared_daemon)" || shared="" + if [ -n "$shared" ]; then + { IFS= read -r PORT; IFS= read -r SECRET; IFS= read -r shared_socket; IFS= read -r SHARED_IDENTITY; } <<< "$shared" + SHARED_SOCKET="$shared_socket" + echo "Using this profile's Biorouter background service on :$PORT" + return 0 + fi if [ -f "$LAUNCHER_STATE" ]; then saved_port=""; saved_secret="" read -r saved_port saved_secret < "$LAUNCHER_STATE" || true @@ -1890,9 +2025,13 @@ import_knowledge_archive() { [ -n "$PORT" ] && [ -n "$SECRET" ] || return 1 [ "$(wc -c < "$f")" -le 134217728 ] || return 1 escaped="${f//\\/\\\\}"; escaped="${escaped//\"/\\\"}" - response="$(printf 'X-Secret-Key: %s\n' "$SECRET" | curl -fsS --max-time 120 -H @- \ - -F "file=@\"$escaped\";filename=\"$source.brkb\";type=application/octet-stream" \ - "http://127.0.0.1:$PORT/knowledge/bases/import")" || return 1 + if [ -n "$SHARED_SOCKET" ]; then + response="$(shared_daemon import "$f")" || return 1 + else + response="$(printf 'X-Secret-Key: %s\n' "$SECRET" | curl -fsS --max-time 120 -H @- \ + -F "file=@\"$escaped\";filename=\"$source.brkb\";type=application/octet-stream" \ + "http://127.0.0.1:$PORT/knowledge/bases/import")" || return 1 + fi imported="$(printf '%s' "$response" | sed -n 's/^[[:space:]]*{[[:space:]]*"id"[[:space:]]*:[[:space:]]*"\([a-z0-9-]*\)"[[:space:]]*}[[:space:]]*$/\1/p')" knowledge_id_ok "$imported" || return 1 printf '%s\n' "$imported" @@ -2519,7 +2658,8 @@ function Test-Port([int]$Port) { # cookie, and the one-time link that sets it is handed only to a caller holding # the daemon's secret. So this launcher uses a daemon it started (its port and # secret are kept in $StateFile, under your profile) or one you name with -# BIOROUTERD_PORT and BIOROUTER_SERVER__SECRET_KEY, and no other. +# BIOROUTERD_PORT and BIOROUTER_SERVER__SECRET_KEY. Windows has no shared +# background-service discovery transport in this Biorouter build. $StateFile = Join-Path $LauncherHome "daemon" function New-Secret { @@ -2672,10 +2812,11 @@ const SERVE_MJS_TEMPLATE: &str = r##"// Dev server for the exported Biorouter ap // daemon, including the agent WebSocket at /apps//agent, so the page and its // agent share an origin and no port is ever hard-coded. The app's dist/ and // authored files are safely installed on refresh and served by the daemon. -import { createServer, request as httpRequest } from "node:http"; +import { Agent as HttpAgent, createServer, request as httpRequest } from "node:http"; +import { createConnection } from "node:net"; import { spawn, spawnSync } from "node:child_process"; import { readFile, writeFile, rename, mkdir, cp, rm, access } from "node:fs/promises"; -import { readdirSync, lstatSync, statSync, readFileSync, realpathSync, createReadStream, constants } from "node:fs"; +import { readdirSync, lstatSync, statSync, fstatSync, openSync, closeSync, readSync, readFileSync, realpathSync, createReadStream, constants } from "node:fs"; import { randomBytes, createHash } from "node:crypto"; import { join, dirname, isAbsolute, win32, posix } from "node:path"; import { fileURLToPath } from "node:url"; @@ -2742,10 +2883,10 @@ const STORE = join(CFG, "agent_drafter", APP_ID); const LAUNCHER_HOME = join(CFG, "app-launcher"); // A daemon serves an app's page only to a browser holding that app's access // cookie, and the one-time link that sets it is handed only to a caller holding -// the daemon's secret. So this server uses a daemon it (or run.sh) started, whose -// port and secret are kept here readable by you alone, or one you name with -// BIOROUTERD_PORT and BIOROUTER_SERVER__SECRET_KEY, and no other. +// the daemon's secret. Prefer this profile's verified background service, then +// a daemon started for these apps. Explicit daemon settings take precedence. const LAUNCHER_STATE = join(LAUNCHER_HOME, "daemon"); +let sharedDaemon; const APP_PREFIX = `/apps/${APP_ID}/`; const PAYLOAD_STATE = join(LAUNCHER_HOME, "payload", APP_ID, SOURCE_DIGEST); const exists = (p) => access(p, constants.F_OK).then(() => true, () => false); @@ -3013,27 +3154,133 @@ function probe(port) { const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +function daemonTransport(port, connection) { + return connection && connection.socketPath && (connection.port === undefined || connection.port === port) + ? { socketPath: connection.socketPath, agent: connection.agent } + : { host: "127.0.0.1", port }; +} + +async function verifiedDaemonConnection(port, secret) { + if (!sharedDaemon || sharedDaemon.port !== port) return; + const connection = await profileDaemon(sharedDaemon.identity); + if (!connection || connection.port !== port || connection.secret !== secret) { + connection?.agent.destroy(); + throw new Error("Biorouter's background service changed or closed; run the launcher again."); + } + return connection; +} + /** One request to the daemon on `port` carrying its secret; resolves to { status, body }. */ -function withSecret(port, secret, method, path) { - return new Promise((res) => { +async function withSecret(port, secret, method, path, connection) { + const owned = connection === undefined; + if (owned) connection = await verifiedDaemonConnection(port, secret); + try { return await new Promise((res) => { const r = httpRequest( - { host: "127.0.0.1", port, path, method, timeout: 5000, headers: { "X-Secret-Key": secret } }, + { ...daemonTransport(port, connection), path, method, timeout: 5000, headers: { "X-Secret-Key": secret } }, (resp) => { let body = ""; resp.setEncoding("utf8"); - resp.on("data", (chunk) => { body += chunk; }); + resp.on("data", (chunk) => { + body += chunk; + if (connection && connection.metadata && body.length > 16384) resp.destroy(); + }); + resp.on("error", () => res({ status: 0, body: "" })); + resp.on("close", () => { if (!resp.complete) res({ status: 0, body: "" }); }); resp.on("end", () => res({ status: resp.statusCode, body })); } ); r.on("error", () => res({ status: 0, body: "" })); r.on("timeout", () => { r.destroy(); res({ status: 0, body: "" }); }); r.end(); - }); + }); } finally { if (owned) connection?.agent.destroy(); } } const secretWorks = async (port, secret) => (await withSecret(port, secret, "GET", "/apps")).status === 200; +function privateOwned(path, kind) { + const s = lstatSync(path); + const valid = kind === "directory" ? s.isDirectory() : kind === "socket" ? s.isSocket() : s.isFile(); + if (!valid || s.uid !== process.getuid() || (s.mode & 0o7777) !== (kind === "directory" ? 0o700 : 0o600)) { + throw new Error("Invalid private daemon metadata"); + } +} + +function privateRecord(path) { + privateOwned(path, "file"); + const fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + try { + const s = fstatSync(fd); + if (!s.isFile() || s.uid !== process.getuid() || (s.mode & 0o7777) !== 0o600 || s.nlink !== 1 || s.size > 16384) { + throw new Error("Invalid private daemon metadata"); + } + const bytes = Buffer.alloc(16385); + const count = readSync(fd, bytes, 0, bytes.length, 0); + if (count > 16384) throw new Error("Invalid private daemon metadata"); + return JSON.parse(bytes.subarray(0, count).toString("utf8")); + } finally { closeSync(fd); } +} + +async function profileDaemon(expectedIdentity) { + // Shared daemon IPC has no Windows transport in this Biorouter build. + if (process.platform === "win32") return; + const root = process.env.BIOROUTER_PATH_ROOT; + const xdg = process.env.XDG_STATE_HOME; + const state = root && root.trim() ? join(root, "state") + : join(xdg && isAbsolute(xdg) ? xdg : join(homedir(), ".local", "state"), "biorouter"); + const directory = join(state, "daemon"); + const location = join(directory, "runtime.json"); + try { lstatSync(location); } catch (error) { if (error.code === "ENOENT") return; } + let agent; + try { + privateOwned(directory, "directory"); + const config = realpathSync.native(CFG); + const profile = privateRecord(join(config, "daemon-profile.json")); + const runtime = privateRecord(location); + const uuid = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!profile || profile.version !== 1 || typeof profile.profile_id !== "string" || !uuid.test(profile.profile_id) || profile.config_dir !== config) { + throw new Error("Daemon profile mismatch"); + } + if (!runtime || runtime.version !== 1 || runtime.profile_id !== profile.profile_id || typeof runtime.instance_id !== "string" || !uuid.test(runtime.instance_id) + || !Number.isSafeInteger(runtime.pid) || runtime.pid < 1 || typeof runtime.user_action_installed !== "boolean" + || runtime.endpoint?.kind !== "unix" || runtime.endpoint.path !== join(directory, "daemon.sock") || !isAbsolute(runtime.endpoint.path) + || typeof runtime.api_secret !== "string" || runtime.api_secret.length < 32 || !/^[A-Za-z0-9._~+/=-]+$/.test(runtime.api_secret)) { + throw new Error("Daemon runtime mismatch"); + } + const keys = ["version", "profile_id", "instance_id", "pid", "user_action_installed"]; + if (expectedIdentity && keys.some(key => expectedIdentity[key] !== runtime[key])) { + throw new Error("Daemon instance changed"); + } + privateOwned(runtime.endpoint.path, "socket"); + agent = new HttpAgent({ keepAlive: true, maxSockets: 1 }); + let connected = false; + agent.createConnection = () => { + if (connected) throw new Error("Biorouter's background service connection closed; run the launcher again."); + connected = true; + privateOwned(runtime.endpoint.path, "socket"); + return createConnection({ path: runtime.endpoint.path }); + }; + const connection = { socketPath: runtime.endpoint.path, agent, metadata: true }; + const identityReply = await withSecret(0, runtime.api_secret, "GET", "/daemon/identity", connection); + const identity = JSON.parse(identityReply.body); + if (identityReply.status !== 200 || !identity || Object.keys(identity).length !== keys.length || keys.some(key => identity[key] !== runtime[key])) { + throw new Error("Daemon identity mismatch"); + } + const healthReply = await withSecret(0, runtime.api_secret, "GET", "/headless/health", connection); + const origin = new URL(JSON.parse(healthReply.body).api_base_url); + const port = Number(origin.port); + if (healthReply.status !== 200 || origin.protocol !== "http:" || !["127.0.0.1", "localhost", "0.0.0.0"].includes(origin.hostname) + || origin.username || origin.password || origin.pathname !== "/" || origin.search || origin.hash || !Number.isInteger(port) || port < 1 || port > 65535) { + throw new Error("Daemon has no local listener"); + } + if (process.env.BIOROUTERD_PORT && Number(process.env.BIOROUTERD_PORT) !== port) { agent.destroy(); return; } + return { port, secret: runtime.api_secret, socketPath: runtime.endpoint.path, agent, identity }; + } catch { + agent?.destroy(); + if (!expectedIdentity) console.warn("This profile's background service could not be verified; using the standalone launcher."); + } +} + async function rememberDaemon(port, secret) { try { await mkdir(dirname(LAUNCHER_STATE), { recursive: true, mode: 0o700 }); @@ -3053,6 +3300,12 @@ async function ensureDaemon() { console.log(`Using the Biorouter daemon on :${preferred}`); return { port: preferred, secret: named }; } + sharedDaemon = await profileDaemon(); + if (sharedDaemon) { + sharedDaemon.agent.destroy(); + console.log(`Using this profile's Biorouter background service on :${sharedDaemon.port}`); + return sharedDaemon; + } try { const [port, secret] = (await readFile(LAUNCHER_STATE, "utf8")).trim().split(/\s+/); if (port && secret && (await secretWorks(Number(port), secret))) { @@ -3228,10 +3481,12 @@ async function importPayloadKnowledge(port, secret, kb) { const boundary = `biorouter-${randomBytes(16).toString("hex")}`; const header = Buffer.from(`--${boundary}\r\nContent-Disposition: form-data; name="file"; filename="${kb.source}.brkb"\r\nContent-Type: application/octet-stream\r\n\r\n`); const footer = Buffer.from(`\r\n--${boundary}--\r\n`); - const response = await new Promise((resolve, reject) => { + const connection = await verifiedDaemonConnection(port, secret); + let response; + try { response = await new Promise((resolve, reject) => { const stream = createReadStream(kb.path); const request = httpRequest({ - host: "127.0.0.1", port, path: "/knowledge/bases/import", method: "POST", timeout: 120000, + ...daemonTransport(port, connection), path: "/knowledge/bases/import", method: "POST", timeout: 120000, headers: { "X-Secret-Key": secret, "Content-Type": `multipart/form-data; boundary=${boundary}`, "Content-Length": header.length + size + footer.length }, }, (reply) => { let body = ""; @@ -3249,7 +3504,7 @@ async function importPayloadKnowledge(port, secret, kb) { request.write(header); stream.pipe(request, { end: false }); stream.on("end", () => request.end(footer)); - }); + }); } finally { connection?.agent.destroy(); } if (response.status !== 200) throw new Error(`daemon returned HTTP ${response.status}`); let allocated; try { allocated = JSON.parse(response.body).id; } catch { } @@ -3460,8 +3715,8 @@ That's the whole thing. The launcher: 1. installs the app into your local Biorouter store (`manifest.json` and all), 2. installs any first-run payload this export carries (`payload/`: knowledge bases and skills, with your consent; `export.json` lists them), -3. reuses the `biorouterd` it started last time, or starts one on the first - free port, +3. reuses your profile's running Biorouter background service when available, + otherwise reuses its earlier daemon or starts one on the first free port, 4. asks that daemon for a one-time link to the app, and 5. opens the link in your web browser, and prints it in the terminal too. @@ -3477,9 +3732,13 @@ put there. A daemon serves an app only to a browser that opened such a link, and hands one out only to a caller that knows the daemon's secret, so another account on the -same computer cannot open your app. That is why the launcher starts a daemon of -its own rather than using one that happens to be running: it keeps that -daemon's port and secret, readable by you alone, in `app-launcher/daemon` in +same computer cannot open your app. On macOS and Linux, the launcher verifies +your profile's private discovery records and the running service's identity +before using it. Closing the launcher never stops that shared service. The +shell launchers use Python 3 for this optional discovery; without it, they use +the standalone path. Windows uses the standalone path because this build has +no shared-service discovery transport there. When starting its own daemon, +the launcher keeps its port and secret, readable by you alone, in `app-launcher/daemon` in your Biorouter folder (`~/.config/biorouter` on macOS and Linux, `%APPDATA%\{windows_author}\{windows_app}\config` on Windows (or the older Biorouter folder in `%APPDATA%`, until Biorouter has moved it), or @@ -3586,6 +3845,417 @@ mod tests { find_command("bash", work_dir) } + #[cfg(unix)] + struct LauncherFixture(std::process::Child); + + #[cfg(unix)] + impl Drop for LauncherFixture { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + #[cfg(unix)] + #[test] + fn generated_launchers_reuse_verified_profile_without_owning_it() { + use std::os::unix::fs::PermissionsExt; + use std::process::{Command, Stdio}; + + let directory = tempfile::Builder::new() + .prefix("br-launch-") + .tempdir_in("/tmp") + .unwrap(); + let Some(node) = find_command("node", directory.path()) else { + eprintln!("skipped: no node for shared launcher lifecycle test"); + return; + }; + let root = directory.path().join("profile"); + let folder = directory.path().join("export"); + std::fs::create_dir_all(&folder).unwrap(); + let m = manifest(ArtifactKind::Static); + for (name, content) in launcher_files(&m, None) { + std::fs::write(folder.join(name), content).unwrap(); + } + std::fs::write(folder.join("manifest.json"), exported_manifest_json(&m)).unwrap(); + std::fs::write(folder.join("index.html"), "exported copy").unwrap(); + std::fs::create_dir_all(folder.join("payload/knowledge")).unwrap(); + std::fs::write( + folder.join("payload/knowledge/demo-kb.brkb"), + "test archive", + ) + .unwrap(); + std::fs::write(folder.join("export.json"), "{}").unwrap(); + let fixture_script = directory.path().join("daemon.cjs"); + std::fs::write( + &fixture_script, + r#"const http = require('node:http'), fs = require('node:fs'), path = require('node:path'); +const root = process.env.BIOROUTER_PATH_ROOT; +const config = path.join(root, 'config'), directory = path.join(root, 'state/daemon'); +fs.mkdirSync(config, {recursive:true});fs.mkdirSync(directory, {recursive:true, mode:0o700});fs.chmodSync(directory, 0o700); +const profile = 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa'; +const instance = 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbbbb'; +const secret = 'dd'.repeat(32), explicit = 'ee'.repeat(32); +const socket = path.join(directory, 'daemon.sock'); +const identity = {version:1, profile_id:profile, instance_id:instance, pid:process.pid, user_action_installed:true}; +const controlPath = path.join(root, 'control.json');fs.writeFileSync(controlPath, '{}'); +let port; +const handle = transport => (req, res) => { + const control = JSON.parse(fs.readFileSync(controlPath)); + fs.appendFileSync(path.join(root, 'requests.jsonl'), JSON.stringify({transport, route:req.url, shared_secret:req.headers['x-secret-key']===secret})+'\n'); + const json = value => {res.setHeader('content-type','application/json');res.end(JSON.stringify(value));}; + if (transport==='http' && req.url==='/status') return res.end('ok'); + if (transport==='http' && req.url.startsWith('/apps/demo/?t=') && !req.headers['x-secret-key']) return res.end(fs.readFileSync(path.join(config, 'agent_drafter/demo/index.html'))); + if (req.headers['x-secret-key'] !== (transport==='unix' ? secret : explicit)) {res.writeHead(401);return res.end();} + if (req.url==='/daemon/identity') return json(control.badIdentity ? {...identity,pid:identity.pid+1} : identity); + if (req.url==='/headless/health') { + if (control.closeAfterHealth) { + fs.writeFileSync(controlPath, '{}'); + res.setHeader('connection','close'); + res.on('finish', () => { + if (control.changedDescriptor) { + const record = JSON.parse(fs.readFileSync(path.join(directory, 'runtime.json'))); + record.instance_id = 'cccccccc-cccc-4ccc-cccc-cccccccccccc'; + fs.writeFileSync(path.join(directory, 'runtime.json'), JSON.stringify(record)); + } + if (control.replaceAfterHealth) { + unix.close(() => { + if (fs.existsSync(socket)) fs.unlinkSync(socket); + fs.writeFileSync(controlPath, JSON.stringify({badIdentity:true})); + unix = http.createServer(handle('unix')); + unix.listen(socket, () => {fs.chmodSync(socket,0o600);fs.writeFileSync(path.join(root,'lifecycle-ready'),'ready');}); + }); + } else fs.writeFileSync(path.join(root,'lifecycle-ready'),'ready'); + }); + } + return json({api_base_url:control.origin || `http://127.0.0.1:${port}`}); + } + if (req.url==='/apps') return json([]); + if (req.method==='POST' && req.url==='/knowledge/bases/import') { + let body='';req.on('data',chunk=>body+=chunk);req.on('end',()=> { + if (!body.includes('filename="demo-kb.brkb"') || !body.includes('test archive')) {res.writeHead(400);return res.end();} + json({id:'demo-kb'}); + });return; + } + if (req.method==='POST' && req.url==='/apps/demo/launch') return json({path:'/apps/demo/?t='+'ab'.repeat(32)}); + res.writeHead(404);res.end(); +}; +const tcp = http.createServer(handle('http')); +let unix = http.createServer(handle('unix')); +tcp.listen(0, '127.0.0.1', () => { + port = tcp.address().port; + unix.listen(socket, () => { + fs.chmodSync(socket, 0o600); + fs.writeFileSync(path.join(config, 'daemon-profile.json'),JSON.stringify({version:1,profile_id:profile,config_dir:fs.realpathSync(config)}),{mode:0o600}); + fs.writeFileSync(path.join(directory, 'runtime.json'),JSON.stringify({...identity,endpoint:{kind:'unix',path:socket},api_secret:secret}),{mode:0o600}); + fs.writeFileSync(path.join(root, 'ready'),String(port)); + }); +}); +setTimeout(()=>process.exit(0),120000).unref(); +"#, + ) + .unwrap(); + let mut fixture = LauncherFixture( + Command::new(&node) + .arg(&fixture_script) + .env("BIOROUTER_PATH_ROOT", &root) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(), + ); + let ready = root.join("ready"); + for _ in 0..100 { + if ready.exists() { + break; + } + assert!(fixture.0.try_wait().unwrap().is_none(), "fixture exited"); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + let port: u16 = std::fs::read_to_string(&ready) + .expect("shared fixture ready") + .parse() + .unwrap(); + let runtime_path = root.join("state/daemon/runtime.json"); + let original = std::fs::read_to_string(&runtime_path).unwrap(); + let (definitions, _) = serve_mjs("demo", 5123, "") + .split_once("\nlet daemonPort;") + .map(|(prefix, suffix)| (prefix.to_string(), suffix.to_string())) + .unwrap(); + let probe = folder.join("probe.mjs"); + std::fs::write( + &probe, + format!( + "{definitions}\ndaemonPaths = () => [];\ntry {{ await installApp(); const daemon = await ensureDaemon(); if(process.env.LAUNCHER_TEST_LIFECYCLE==='1') {{const marker=join(process.env.BIOROUTER_PATH_ROOT,'lifecycle-ready');for(let i=0;!payloadKind(marker)?.isFile()&&i<100;i++)await sleep(10);if(!payloadKind(marker)?.isFile())throw Error('lifecycle fixture not ready');}} await installPayload(daemon.port,daemon.secret); const launch = await launchPath(daemon.port,daemon.secret); console.log('SELECTED:'+daemon.port+':'+(sharedDaemon?'socket':'http')); if(sharedDaemon) {{ const page = await new Promise((resolve,reject)=>{{ const request=httpRequest({{host:'127.0.0.1',port:daemon.port,path:launch}},response=>{{let body='';response.on('data',chunk=>body+=chunk);response.on('end',()=>resolve(body));}});request.on('error',reject);request.end();}});if(!page.includes('exported copy'))throw Error('not exported app'); }} }} catch(error) {{console.error(error.message);process.exitCode=1;}} finally {{sharedDaemon?.agent.destroy();}}\n" + ), + ) + .unwrap(); + let bash = find_usable_bash(&folder).unwrap(); + let shell_probe = folder.join("probe.sh"); + std::fs::write( + &shell_probe, + "set -euo pipefail\n. ./biorouter-launch.sh\nfind_biorouterd() { echo standalone-fallback >&2; return 1; }\ninstall_app\nstart_daemon\nif [ \"${LAUNCHER_TEST_LIFECYCLE:-}\" = 1 ]; then for i in {1..100}; do [ -f \"$BIOROUTER_PATH_ROOT/lifecycle-ready\" ] && break; sleep 0.01; done; [ -f \"$BIOROUTER_PATH_ROOT/lifecycle-ready\" ]; fi\ninstall_payload\nlaunch_url\nprintf 'SELECTED:%s:%s\\n' \"$PORT\" \"${SHARED_SOCKET:+socket}\"\n", + ) + .unwrap(); + let python = find_command("python3", &folder); + let run = |backend: &str, env: &[(&str, String)]| { + let mut command = Command::new(if backend == "node" { &node } else { &bash }); + command + .arg(if backend == "node" { + &probe + } else { + &shell_probe + }) + .current_dir(&folder) + .env("BIOROUTER_PATH_ROOT", &root) + .env("BIOROUTER_EXPORT_YES", "1") + .env_remove("BIOROUTERD_PORT") + .env_remove("BIOROUTER_SERVER__SECRET_KEY") + .env_remove("BIOROUTERD_BIN") + .stdin(Stdio::null()); + for (key, value) in env { + command.env(key, value); + } + let out = command.output().unwrap(); + let printed = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!(!printed.contains(&"dd".repeat(32)), "shared secret printed"); + assert!( + !printed.contains(&"ee".repeat(32)), + "explicit secret printed" + ); + (out.status.success(), printed) + }; + let import_count = || { + std::fs::read_to_string(root.join("requests.jsonl")) + .unwrap_or_default() + .lines() + .filter(|line| { + serde_json::from_str::(line).unwrap()["route"] + == "/knowledge/bases/import" + }) + .count() + }; + let backends: Vec<&str> = if python.is_some() { + vec!["node", "bash"] + } else { + vec!["node"] + }; + for backend in &backends { + let _ = std::fs::remove_dir_all(root.join("config/app-launcher/payload")); + let before_imports = import_count(); + let (ok, printed) = run(backend, &[]); + assert!(ok, "{backend}: {printed}"); + assert_eq!( + import_count() - before_imports, + 1, + "{backend} duplicated the shared payload import" + ); + assert!( + printed.contains(&format!("SELECTED:{port}:socket")), + "{printed}" + ); + assert!( + printed.contains("Payload installed."), + "{backend} did not import full payload: {printed}" + ); + assert!( + fixture.0.try_wait().unwrap().is_none(), + "launcher killed reused daemon" + ); + } + for case in [ + "wrong-profile", + "wrong-identity", + "unsafe-record", + "unsafe-socket", + "remote-origin", + "named-other-port", + "stale-record", + "oversized-record", + "wrong-socket", + ] { + let mut descriptor: serde_json::Value = serde_json::from_str(&original).unwrap(); + let mut control = serde_json::json!({}); + let mut env = Vec::new(); + match case { + "wrong-profile" => { + descriptor["profile_id"] = "cccccccc-cccc-4ccc-cccc-cccccccccccc".into() + } + "wrong-identity" => control["badIdentity"] = true.into(), + "unsafe-record" => { + std::fs::set_permissions(&runtime_path, std::fs::Permissions::from_mode(0o644)) + .unwrap() + } + "unsafe-socket" => std::fs::set_permissions( + root.join("state/daemon/daemon.sock"), + std::fs::Permissions::from_mode(0o666), + ) + .unwrap(), + "remote-origin" => control["origin"] = "http://example.test:1234".into(), + "named-other-port" => env.push(( + "BIOROUTERD_PORT", + (if port == 65535 { port - 1 } else { port + 1 }).to_string(), + )), + "stale-record" => std::fs::remove_file(&runtime_path).unwrap(), + "oversized-record" => descriptor["padding"] = "x".repeat(16385).into(), + "wrong-socket" => { + descriptor["endpoint"]["path"] = root + .join("another.sock") + .to_string_lossy() + .into_owned() + .into() + } + _ => unreachable!(), + } + if case != "stale-record" { + std::fs::write(&runtime_path, serde_json::to_vec(&descriptor).unwrap()).unwrap(); + } + std::fs::write( + root.join("control.json"), + serde_json::to_vec(&control).unwrap(), + ) + .unwrap(); + for backend in &backends { + let (ok, printed) = run(backend, &env); + assert!(!ok, "{backend} reused {case}: {printed}"); + assert!( + printed.contains(if *backend == "bash" { + "standalone-fallback" + } else { + "Could not find or start biorouterd" + }), + "{backend} did not reach standalone fallback for {case}: {printed}" + ); + assert!( + fixture.0.try_wait().unwrap().is_none(), + "refusal killed shared daemon" + ); + } + std::fs::write(&runtime_path, &original).unwrap(); + std::fs::set_permissions(&runtime_path, std::fs::Permissions::from_mode(0o600)) + .unwrap(); + std::fs::set_permissions( + root.join("state/daemon/daemon.sock"), + std::fs::Permissions::from_mode(0o600), + ) + .unwrap(); + std::fs::write(root.join("control.json"), "{}").unwrap(); + } + for case in ["idle-close", "replacement-socket", "changed-descriptor"] { + for backend in &backends { + let before = std::fs::read_to_string(root.join("requests.jsonl")).unwrap(); + let _ = std::fs::remove_file(root.join("lifecycle-ready")); + let _ = std::fs::remove_dir_all(root.join("config/app-launcher/payload")); + std::fs::write( + root.join("control.json"), + serde_json::to_vec(&serde_json::json!({ + "closeAfterHealth": true, + "replaceAfterHealth": case == "replacement-socket", + "changedDescriptor": case == "changed-descriptor", + })) + .unwrap(), + ) + .unwrap(); + let before_imports = import_count(); + let (ok, printed) = run(backend, &[("LAUNCHER_TEST_LIFECYCLE", "1".to_string())]); + if case == "idle-close" { + assert!(ok, "idle-close did not reverify and recover: {printed}"); + assert!(printed.contains("Payload installed."), "{printed}"); + assert_eq!( + import_count() - before_imports, + 1, + "{backend} duplicated the idle-close import" + ); + } else { + assert!(!ok, "stale connection followed {case}: {printed}"); + assert!( + printed.contains("run the launcher again"), + "no recovery advice: {printed}" + ); + let after = std::fs::read_to_string(root.join("requests.jsonl")).unwrap(); + let added = after + .strip_prefix(&before) + .expect("requests retain their prefix"); + let requests: Vec = added + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert!( + requests + .iter() + .all(|request| request["route"] == "/daemon/identity" + || request["route"] == "/headless/health"), + "an app request reached the replacement service: {added}" + ); + if case == "changed-descriptor" { + assert_eq!( + requests.len(), + 2, + "secret sent after descriptor changed: {added}" + ); + } + } + assert!( + fixture.0.try_wait().unwrap().is_none(), + "connection change killed shared daemon" + ); + std::fs::write(root.join("control.json"), "{}").unwrap(); + std::fs::write(&runtime_path, &original).unwrap(); + } + } + for backend in &backends { + let _ = std::fs::remove_dir_all(root.join("config/app-launcher/payload")); + let before_imports = import_count(); + let (ok, printed) = run( + backend, + &[ + ("BIOROUTERD_PORT", port.to_string()), + ("BIOROUTER_SERVER__SECRET_KEY", "ee".repeat(32)), + ], + ); + assert!(ok, "explicit daemon configuration: {backend}: {printed}"); + assert_eq!( + import_count() - before_imports, + 1, + "{backend} duplicated the explicit HTTP import" + ); + assert!( + !printed.contains("background service"), + "explicit daemon lost precedence: {printed}" + ); + assert!( + printed.contains("Payload installed."), + "explicit HTTP payload import failed: {printed}" + ); + } + let requests = std::fs::read_to_string(root.join("requests.jsonl")).unwrap(); + let mut imports = 0; + for line in requests.lines() { + let request: serde_json::Value = serde_json::from_str(line).unwrap(); + if request["route"] == "/knowledge/bases/import" { + imports += 1; + } + assert!( + request["transport"] != "http" || request["shared_secret"] != true, + "shared secret left private socket" + ); + } + assert_eq!( + imports, + backends.len() * 3, + "one full payload import per backend for shared, recovered and explicit transports" + ); + assert!( + fixture.0.try_wait().unwrap().is_none(), + "launcher exit killed shared daemon" + ); + } + #[cfg(windows)] fn push_git_bash_candidates(candidates: &mut Vec, git_root: &Path) { candidates.push(git_root.join("bin").join("bash.exe")); diff --git a/crates/biorouter-server/src/routes/apps.rs b/crates/biorouter-server/src/routes/apps.rs index 0593a5754..26c8ff900 100644 --- a/crates/biorouter-server/src/routes/apps.rs +++ b/crates/biorouter-server/src/routes/apps.rs @@ -2710,8 +2710,7 @@ fn app_platform_tools(name: &str) -> Option<&'static [&'static str]> { None } -/// The extension config an app arms `name` with, or `None` when an app may not -/// have it at all. +/// The extension config an app arms `name` with, or why it is not armed. /// /// Scoped HERE and not at the auto-arm call sites, because both app paths build /// their configs through this one function — the main agent's @@ -2721,26 +2720,109 @@ fn app_platform_tools(name: &str) -> Option<&'static [&'static str]> { /// comment: `arm_extensions` is the only caller of `Agent::add_extension` in /// this file, and `an_app_arms_extensions_through_exactly_one_call_site` reads /// this file's own source to keep it that way. -fn manifest_extension_config(name: &str) -> Option { +fn manifest_extension_config(name: &str) -> Result { + resolve_manifest_extension(name, configured_extension) +} + +/// Why an extension an app named was not armed, worded for the daemon log. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UnarmedExtension { + /// A platform extension outside [`app_platform_tools`]. + Platform, + /// Not built in, and nothing on this machine is configured under the name. + NotInstalled, + /// Configured on this machine, and turned off there. + TurnedOff, + /// Configured as a kind an app may not arm (named). + Kind(&'static str), +} + +impl UnarmedExtension { + fn reason(self) -> String { + match self { + Self::Platform => { + "refused: an app may not arm this platform extension (see `app_platform_tools`)" + .to_string() + } + Self::NotInstalled => "not armed: it is not built in and no extension with this name \ + is installed on this machine; install it under the same name" + .to_string(), + Self::TurnedOff => "not armed: the extension with this name is turned off on this \ + machine; turn it on under Extensions in Biorouter" + .to_string(), + Self::Kind(kind) => { + format!("refused: the extension with this name is a {kind} entry, which an app may not arm") + } + } + } +} + +/// The extension an app arms for `name`, from Biorouter's own servers or from +/// `configured`, this machine's extension configuration. +/// +/// Issue #386. A manifest names an extension and nothing else: no command, path, +/// address or credential, so it reads the same on the author's machine and on a +/// recipient's. A name that is neither a platform extension nor one of +/// Biorouter's built-in servers is therefore an extension the person installed, +/// and it resolves against what THIS machine has configured under that name, +/// with this machine's own command and credentials. It used to be armed as a +/// built-in server whatever it was, so an installed stdio extension the app +/// catalog offered (`ucsfomopagent`) failed every time with "Unknown builtin +/// extension". +/// +/// An entry the person turned off is not armed: an app is not a way around +/// their switch. Nor is a platform entry (that would side-step +/// [`app_platform_tools`]) or a frontend one (its tools run in a desktop +/// window, which an app's page is not). +fn resolve_manifest_extension( + name: &str, + configured: impl FnOnce(&str) -> Option, +) -> Result { if PLATFORM_EXTENSIONS.contains_key(name) { - return Some(ExtensionConfig::Platform { + let tools = app_platform_tools(name).ok_or(UnarmedExtension::Platform)?; + return Ok(ExtensionConfig::Platform { name: name.to_string(), bundled: None, description: name.to_string(), - available_tools: app_platform_tools(name)? - .iter() - .map(|t| (*t).to_string()) - .collect(), + available_tools: tools.iter().map(|t| (*t).to_string()).collect(), }); } - Some(ExtensionConfig::Builtin { - name: name.to_string(), - display_name: None, - timeout: None, - bundled: None, - description: name.to_string(), - available_tools: Vec::new(), - }) + if biorouter_mcp::BUILTIN_EXTENSIONS.contains_key(name) { + return Ok(ExtensionConfig::Builtin { + name: name.to_string(), + display_name: None, + timeout: None, + bundled: None, + description: name.to_string(), + available_tools: Vec::new(), + }); + } + let entry = configured(name).ok_or(UnarmedExtension::NotInstalled)?; + if !entry.enabled { + return Err(UnarmedExtension::TurnedOff); + } + match entry.config { + config @ (ExtensionConfig::Stdio { .. } + | ExtensionConfig::StreamableHttp { .. } + | ExtensionConfig::Sse { .. } + | ExtensionConfig::InlinePython { .. }) => Ok(config), + ExtensionConfig::Platform { .. } => Err(UnarmedExtension::Kind("platform")), + ExtensionConfig::Frontend { .. } => Err(UnarmedExtension::Kind("frontend")), + ExtensionConfig::Builtin { .. } => Err(UnarmedExtension::Kind("built-in")), + } +} + +/// The entry this machine's configuration holds for the extension `name`: by +/// its exact name, else by the key the configuration files it under (case and +/// spaces folded), which is how an installer names it. +fn configured_extension(name: &str) -> Option { + if let Some(entry) = biorouter::config::get_extension_entry_by_name(name) { + return Some(entry); + } + let key = biorouter::config::extensions::name_to_key(name); + biorouter::config::get_all_extensions() + .into_iter() + .find(|entry| biorouter::config::extensions::name_to_key(&entry.config.name()) == key) } /// Arm one app agent's extensions. The **only** `Agent::add_extension` call site @@ -2754,14 +2836,17 @@ async fn arm_extensions( names: Vec, ) { for name in names { - let Some(config) = manifest_extension_config(&name) else { - // Refused, and said out loud: a silently missing toolset is - // indistinguishable from a broken extension. - warn!( - app = %manifest.id, profile = ?profile, extension = %name, - "refused: an app may not arm this platform extension (see `app_platform_tools`)" - ); - continue; + let config = match manifest_extension_config(&name) { + Ok(config) => config, + Err(unarmed) => { + // Refused, and said out loud: a silently missing toolset is + // indistinguishable from a broken extension. + warn!( + app = %manifest.id, profile = ?profile, extension = %name, + "{}", unarmed.reason() + ); + continue; + } }; if let Err(e) = agent.add_extension(config).await { warn!(app = %manifest.id, profile = ?profile, extension = %name, "add_extension failed: {e}"); @@ -12890,7 +12975,7 @@ mod tests { continue; } assert!( - manifest_extension_config(name).is_none(), + manifest_extension_config(name).is_err(), "an app was armed with the platform extension `{name}`; \ `available_tools: Vec::new()` means ALL of its tools" ); @@ -12905,7 +12990,7 @@ mod tests { // `skills` is armed, and bounded. match manifest_extension_config(skills) { - Some(ExtensionConfig::Platform { + Ok(ExtensionConfig::Platform { available_tools, .. }) => assert_eq!( available_tools, @@ -12923,7 +13008,7 @@ mod tests { assert!( matches!( manifest_extension_config("developer"), - Some(ExtensionConfig::Builtin { .. }) + Ok(ExtensionConfig::Builtin { .. }) ), "a builtin MCP extension must still arm for an app" ); @@ -12961,6 +13046,107 @@ mod tests { ); } + /// Issue #386. An app that names an extension the person installed + /// (`ucsfomopagent`, a stdio server in their configuration) is armed with + /// THAT entry: its command, arguments and credential keys come from this + /// machine, never from the manifest. It used to be armed as a built-in + /// server whatever it was, and failed every time with "Unknown builtin + /// extension: ucsfomopagent". + #[test] + fn an_app_arms_an_installed_extension_from_this_machines_configuration() { + use super::super::{resolve_manifest_extension, UnarmedExtension}; + use biorouter::agents::ExtensionConfig; + use biorouter::config::ExtensionEntry; + + let entry = |value: serde_json::Value| -> ExtensionEntry { + serde_json::from_value(value).expect("a config.yaml extension entry") + }; + let installed = |enabled: bool| { + entry(serde_json::json!({ + "enabled": enabled, + "type": "stdio", + "name": "ucsfomopagent", + "description": "UCSF OMOP", + "cmd": "uvx", + "args": ["ucsf-omop-agent"], + "env_keys": ["UCSFOMOP_PASSWORD"], + "timeout": 300 + })) + }; + let only = |want: &'static str, found: ExtensionEntry| { + move |name: &str| (name == want).then(|| found.clone()) + }; + + match resolve_manifest_extension( + "ucsfomopagent", + only("ucsfomopagent", installed(true)), + ) { + Ok(ExtensionConfig::Stdio { + name, + cmd, + args, + env_keys, + .. + }) => { + assert_eq!(name, "ucsfomopagent"); + assert_eq!(cmd, "uvx"); + assert_eq!(args, vec!["ucsf-omop-agent".to_string()]); + assert_eq!(env_keys, vec!["UCSFOMOP_PASSWORD".to_string()]); + } + other => panic!("an installed stdio extension must arm as itself: {other:?}"), + } + + // The person's off switch holds: an app is not a way around it. + assert_eq!( + resolve_manifest_extension( + "ucsfomopagent", + only("ucsfomopagent", installed(false)) + ), + Err(UnarmedExtension::TurnedOff) + ); + // Nothing installed under the name: said as that, not as a broken + // built-in server. + assert_eq!( + resolve_manifest_extension("ucsfomopagent", |_: &str| None), + Err(UnarmedExtension::NotInstalled) + ); + assert!(UnarmedExtension::NotInstalled + .reason() + .contains("install it under the same name")); + + // A configured entry may not smuggle in what the platform allow-list + // refuses, nor tools that only a desktop window can run. + let platform = entry(serde_json::json!({ + "enabled": true, "type": "platform", "name": "mytools", "description": "" + })); + assert_eq!( + resolve_manifest_extension("mytools", only("mytools", platform)), + Err(UnarmedExtension::Kind("platform")) + ); + let frontend = entry(serde_json::json!({ + "enabled": true, "type": "frontend", "name": "panel", "description": "", + "tools": [], "instructions": null + })); + assert_eq!( + resolve_manifest_extension("panel", only("panel", frontend)), + Err(UnarmedExtension::Kind("frontend")) + ); + + // Biorouter's own servers and the platform allow-list are decided + // before the configuration is read at all. + let unread = |name: &str| -> Option { + panic!("the configuration was read for `{name}`") + }; + assert!(matches!( + resolve_manifest_extension("autovisualiser", unread), + Ok(ExtensionConfig::Builtin { .. }) + )); + assert_eq!( + resolve_manifest_extension("workspace", unread), + Err(UnarmedExtension::Platform) + ); + } + /// A blank knowledge base is not a declared one. /// /// `resolve_worker_grants` read `cfg.knowledge_base.is_some()`, so diff --git a/crates/biorouter/src/agents/extension.rs b/crates/biorouter/src/agents/extension.rs index 66d030ead..60e88ad13 100644 --- a/crates/biorouter/src/agents/extension.rs +++ b/crates/biorouter/src/agents/extension.rs @@ -954,9 +954,25 @@ mod tests { assert!(PLATFORM_EXTENSIONS["extensionmanager"].default_enabled); assert!(PLATFORM_EXTENSIONS["skills"].default_enabled); assert!(PLATFORM_EXTENSIONS["code_execution"].default_enabled); + assert!(PLATFORM_EXTENSIONS["crew"].default_enabled); assert!(!PLATFORM_EXTENSIONS["chatrecall"].default_enabled); } + /// Crew is a capability, not an extension: the desktop lists it in Settings → Chat → + /// Capabilities and leaves it off the Extensions page, and this side agrees, so Extension + /// Manager refuses to enable or disable it (`capability_management_refusal`) and the + /// agent's extension listings leave it out. + #[test] + fn crew_is_a_capability() { + let crew = ExtensionConfig::Platform { + name: "crew".to_string(), + description: String::new(), + bundled: Some(true), + available_tools: Vec::new(), + }; + assert!(crew.is_capability()); + } + #[test] fn builtin_transport_is_not_itself_a_capability_classification() { let capability = ExtensionConfig::Builtin { diff --git a/crates/biorouter/src/agents/extension_manager.rs b/crates/biorouter/src/agents/extension_manager.rs index 30c995a1b..0841b000e 100644 --- a/crates/biorouter/src/agents/extension_manager.rs +++ b/crates/biorouter/src/agents/extension_manager.rs @@ -5615,6 +5615,7 @@ mod tests { ("Todo", "todo"), ("Web & Documents", "webdocuments"), ("Agent Drafter", "agent_drafter"), + ("Crew", "crew"), ] { let target = resolve_bundled_extension(label).expect(label); assert_eq!(target.key(), id); diff --git a/crates/biorouter/src/agents/mistakes.rs b/crates/biorouter/src/agents/mistakes.rs index b4675968e..27e8f4e0c 100644 --- a/crates/biorouter/src/agents/mistakes.rs +++ b/crates/biorouter/src/agents/mistakes.rs @@ -186,6 +186,9 @@ pub fn is_recoverable(error: &ProviderError) -> bool { | crate::providers::errors::ProviderErrorKind::Network | crate::providers::errors::ProviderErrorKind::Other ), + // Already given up on by the provider, for a reason a re-send cannot + // change: the local server is still reading the first copy. + ProviderError::IdleTimeout(_) => false, ProviderError::Authentication(_) | ProviderError::RateLimitExceeded { .. } | ProviderError::NotImplemented(_) diff --git a/crates/biorouter/src/agents/reply_parts.rs b/crates/biorouter/src/agents/reply_parts.rs index a0df0e5bf..b3cb55984 100644 --- a/crates/biorouter/src/agents/reply_parts.rs +++ b/crates/biorouter/src/agents/reply_parts.rs @@ -508,16 +508,90 @@ impl HeldThinking { /// /// Split out of [`Agent::stream_response_from_provider`] so the request /// plumbing and what is done to each item read separately. +/// Watches a reply for the one shape that leaves a person reading JSON where an +/// answer should be: a WHOLE reply of tool calls written as text, in a turn +/// that offered tools and made no call. Nothing in it ran, and before this +/// nothing said so. +/// +/// Diagnostics 20261006_2: llama3.1:8b, in a Code Execution chat, answered +/// three requests with `text_editor`, `shell` and `agent_drafter` calls +/// written as JSON. None names a tool that chat offered, so the provider +/// neither recovered them nor asked the model again, and each turn ended on the +/// JSON. The person gave up and asked for code to run by hand. The calls are +/// never run from here either; the person is only told. +#[derive(Default)] +struct UnrunCallWatch { + text: String, + called: bool, +} + +impl UnrunCallWatch { + fn observe(&mut self, message: &Message) { + self.called |= message.content.iter().any(|content| { + matches!( + content, + MessageContent::ToolRequest(_) | MessageContent::FrontendToolRequest(_) + ) + }); + for content in &message.content { + if let MessageContent::Text(text) = content { + self.text.push_str(&text.text); + } + } + } + + /// The notice for a finished reply, when it needs one. + fn notice(&self) -> Option { + if self.called { + return None; + } + let names = crate::providers::formats::text_tool_calls::whole_reply_call_names(&self.text)?; + Some(unrun_call_notice(&names)) + } +} + +/// What the person reads under a reply that is only tool calls written as +/// text. User-only: the model never sees it. +fn unrun_call_notice(names: &[String]) -> Message { + const SHOWN: usize = 3; + let mut quoted: Vec = names + .iter() + .take(SHOWN) + .map(|name| format!("'{name}'")) + .collect(); + if names.len() > SHOWN { + quoted.push(format!("{} more", names.len() - SHOWN)); + } + let what = match quoted.as_slice() { + [one] => format!("a call to {one}"), + [first, second] => format!("calls to {first} and {second}"), + [init @ .., last] => format!("calls to {} and {last}", init.join(", ")), + [] => "a tool call".to_string(), + }; + Message::assistant() + .with_system_notification( + crate::conversation::message::SystemNotificationType::InlineMessage, + format!( + "Nothing ran: this reply is {what} written as text, not a tool call Biorouter \ + can run. Smaller local models often do this. A larger model, or fewer \ + extensions turned on in this chat, may help." + ), + ) + .user_only() +} + fn agent_facing_stream( mut stream: MessageStream, toolshim_interpreter: Option, toolshim_tools: Vec, + tools_offered: bool, streaming: bool, stream_open_start: std::time::Instant, ) -> MessageStream { Box::pin(try_stream! { let mut toolshim_reply = ToolshimReply::default(); let mut held_thinking = HeldThinking::default(); + let mut unrun_calls = UnrunCallWatch::default(); while let Some(result) = stream.next().await { let (message, usage, pending) = match result { Ok(item) => item, @@ -542,6 +616,9 @@ fn agent_facing_stream( if let (Some(message), Some(_)) = (message.as_ref(), toolshim_interpreter.as_ref()) { toolshim_reply.observe(message); } + if let (Some(message), true) = (message.as_ref(), tools_offered) { + unrun_calls.observe(message); + } // Thinking waits for the end of its phase; see `HeldThinking`. let message = match message { @@ -577,10 +654,17 @@ fn agent_facing_stream( // provider's own text-then-call stream has. if let Some(interpreter) = toolshim_interpreter.as_ref() { if let Some(calls) = toolshim_reply.interpret(interpreter, &toolshim_tools).await { + unrun_calls.observe(&calls); yield (Some(calls), None, None); } } + // Last, after any call the toolshim found: a reply that is nothing but + // calls written as text, none of which ran, gets one line saying so. + if let Some(notice) = unrun_calls.notice() { + yield (Some(notice), None, None); + } + // The stream is drained: generation is genuinely finished. This is // the marker the old instrumentation lacked entirely, which is why // generation time was invisible and prior investigations attributed @@ -947,10 +1031,12 @@ impl Agent { } }; + let tools_offered = !tools.is_empty() || !toolshim_tools.is_empty(); Ok(agent_facing_stream( stream, toolshim_interpreter, toolshim_tools, + tools_offered, streaming, stream_open_start, )) @@ -1028,6 +1114,10 @@ impl Agent { if let Some(id) = response.id.clone() { filtered_message = filtered_message.with_id(id); } + // And who the message is for: this copy is the one the client is + // shown live, and a user-only notice (`UnrunCallWatch`) streamed as + // visible to the model while its stored row said otherwise. + filtered_message.metadata = response.metadata.clone(); // Categorize tool requests let mut frontend_requests = Vec::new(); @@ -2389,6 +2479,14 @@ mod tests { /// one entry per yielded item. async fn through_the_agent_stream( items: Vec>, + ) -> Vec> { + through_the_agent_stream_offering(items, &[]).await + } + + /// [`through_the_agent_stream`] for a request that offered `tools`. + async fn through_the_agent_stream_offering( + items: Vec>, + tools: &[Tool], ) -> Vec> { let provider: Arc = Arc::new(ScriptedStreamProvider { items: std::sync::Mutex::new(Some(items)), @@ -2397,7 +2495,7 @@ mod tests { provider, "system", &[Message::user().with_text("how many rows?")], - &[], + tools, &[], None, ) @@ -2585,4 +2683,151 @@ mod tests { ("second item".to_string(), Some("rs_2".to_string())) ); } + + /// Diagnostics 20261006_2: one of the reporter's exact assistant replies. + fn reporter_reply(index: u64) -> String { + let doc: Value = serde_json::from_str(include_str!( + "../../tests/fixtures/ollama/diagnostics_20261006_2/replies.json" + )) + .unwrap(); + doc["replies"] + .as_array() + .unwrap() + .iter() + .find(|reply| reply["index"] == index) + .and_then(|reply| reply["reply"].as_str()) + .unwrap_or_else(|| panic!("no reply {index} in the fixture")) + .to_string() + } + + fn reply_item(text: &str) -> Result { + Ok(( + Some(Message::assistant().with_text(text).with_id("r1")), + None, + None, + )) + } + + /// The inline notice text of an item, when it is one. + fn notice_text( + item: &Result, + ) -> Option { + let (Some(message), None, None) = item.as_ref().ok()? else { + return None; + }; + match message.content.as_slice() { + [MessageContent::SystemNotification(note)] + if note.notification_type + == crate::conversation::message::SystemNotificationType::InlineMessage => + { + assert!( + !message.is_agent_visible() && message.is_user_visible(), + "the notice is for the person only: {message:?}" + ); + assert_ne!(message.id.as_deref(), Some("r1"), "its own row"); + Some(note.msg.clone()) + } + _ => None, + } + } + + /// Diagnostics 20261006_2: llama3.1:8b, in a chat with Code Execution on, + /// answered three requests with a call written as JSON to `text_editor`, + /// `shell` and `agent_drafter`, none of them a tool that chat offered. + /// Nothing ran and nothing said so; the reporter read the JSON as the + /// answer and gave up. The reply still shows as it came, and one line + /// after it, for the person only, says that nothing ran. + #[tokio::test] + async fn a_reply_of_calls_written_as_text_says_that_nothing_ran() { + for (index, name) in [ + (1, "'text_editor'"), + (7, "'shell'"), + (15, "'agent_drafter'"), + ] { + let reply = reporter_reply(index); + let out = through_the_agent_stream_offering( + vec![reply_item(&reply), usage_item()], + &[shell_tool()], + ) + .await; + assert_eq!(out.len(), 3, "reply {index}: {out:?}"); + let (Some(shown), None, None) = out[0].as_ref().unwrap() else { + panic!("reply {index}: the reply first: {:?}", out[0]); + }; + assert_eq!( + shown.as_concat_text(), + reply, + "the reply is shown as it came" + ); + assert!(matches!(out[1].as_ref().unwrap(), (None, Some(_), None))); + let notice = notice_text(&out[2]) + .unwrap_or_else(|| panic!("reply {index}: a notice last: {:?}", out[2])); + assert!( + notice.starts_with("Nothing ran: this reply is a call to ") + && notice.contains(name) + && notice.contains("written as text"), + "{notice}" + ); + } + + // Reply 11, two calls in one reply. + let out = through_the_agent_stream_offering( + vec![reply_item(&reporter_reply(11)), usage_item()], + &[shell_tool()], + ) + .await; + let notice = out.iter().find_map(notice_text).expect("a notice"); + assert!( + notice.contains("calls to 'workspace_read_conversation' and 'workspace_read_panel'"), + "{notice}" + ); + } + + /// Answers are never noticed: the reporter's prose, its code for the person + /// to run, and the JSON it was asked for. Nor is a reply in a turn that + /// offered no tools, nor one whose call was made after all (Ollama's one + /// corrective retry turning reply 9 into a real `search_modules` call). + #[tokio::test] + async fn answers_and_made_calls_get_no_notice() { + for index in [3, 5, 13, 17, 19] { + let out = through_the_agent_stream_offering( + vec![reply_item(&reporter_reply(index)), usage_item()], + &[shell_tool()], + ) + .await; + assert!( + out.iter().all(|item| notice_text(item).is_none()), + "reply {index}: {out:?}" + ); + } + + let out = + through_the_agent_stream(vec![reply_item(&reporter_reply(7)), usage_item()]).await; + assert!(out.iter().all(|item| notice_text(item).is_none())); + + let call = Message::assistant() + .with_tool_request( + "call-1", + Ok(rmcp::model::CallToolRequestParams { + task: None, + name: "code_execution__search_modules".into(), + arguments: Some(object!({"terms": "llama"})), + meta: None, + }), + ) + .with_id("r1"); + let out = through_the_agent_stream_offering( + vec![ + reply_item(&reporter_reply(9)), + Ok((Some(call), None, None)), + usage_item(), + ], + &[shell_tool()], + ) + .await; + assert!( + out.iter().all(|item| notice_text(item).is_none()), + "{out:?}" + ); + } } diff --git a/crates/biorouter/src/agents/skills_extension.rs b/crates/biorouter/src/agents/skills_extension.rs index 593947d0a..6f61406b9 100644 --- a/crates/biorouter/src/agents/skills_extension.rs +++ b/crates/biorouter/src/agents/skills_extension.rs @@ -404,6 +404,43 @@ pub fn count_user_skills() -> usize { .count() } +/// Skills Biorouter itself renamed: (old name, current name). A workflow saved +/// before the rename still names the old one. +/// +/// Diagnostics 20261006_2: a Daily Meditation copied in v1.85.2 named +/// `soul-writer`, which v1.85.3 renamed to `update-soul` (and startup deletes +/// the old folder), so once workflow skills became required every run failed +/// with "requires skill 'soul-writer'", a skill nothing can install. A stock +/// copy is now upgraded at startup (`knowledge::soul`); this covers a copy the +/// person edited, which startup leaves alone. +const RENAMED_SKILLS: &[(&str, &str)] = &[( + crate::knowledge::soul::SOUL_SKILL_DIR_LEGACY, + crate::knowledge::soul::SOUL_SKILL_DIR, +)]; + +/// `name`, or the current name of a skill Biorouter renamed when the old name +/// is neither a skill nor a bundle here. An installed skill that happens to +/// carry an old name keeps it. +fn current_skill_name(name: &str, view: &skill_catalog::CatalogView) -> String { + let installed = view.skills.iter().any(|skill| skill.name == name) + || view.bundles.iter().any(|bundle| bundle.name == name); + if !installed { + if let Some((_, current)) = RENAMED_SKILLS.iter().find(|(old, _)| *old == name) { + return (*current).to_string(); + } + } + name.to_string() +} + +/// The refusal for a workflow skill this machine does not have, saying what +/// the person can do about it. +fn missing_workflow_skill(name: &str) -> anyhow::Error { + anyhow::anyhow!( + "workflow requires skill '{name}', but it is not installed. Add the skill under Skills, \ + or remove '{name}' from the workflow's skills." + ) +} + /// Resolve workflow-declared skills to the exact installed bodies visible to /// one session. This is intentionally stricter than the model-facing search: /// a workflow names required procedure, so missing or disabled instructions @@ -436,6 +473,7 @@ pub async fn workflow_skill_instructions( let expanded: Vec = { let mut out: Vec = Vec::new(); for name in requested { + let name = ¤t_skill_name(name, &view); match view.bundles.iter().find(|bundle| bundle.name == *name) { Some(bundle) => { if bundle.skills.is_empty() { @@ -460,17 +498,15 @@ pub async fn workflow_skill_instructions( .skills .iter() .find(|entry| entry.name == *name) - .ok_or_else(|| { - anyhow::anyhow!("workflow requires skill '{name}', but it is not installed") - })?; + .ok_or_else(|| missing_workflow_skill(name))?; if !visible.state.effective { anyhow::bail!( "workflow requires skill '{name}', but it is disabled for this conversation" ); } - let skill = skills.get(name).ok_or_else(|| { - anyhow::anyhow!("workflow requires skill '{name}', but it is not installed") - })?; + let skill = skills + .get(name) + .ok_or_else(|| missing_workflow_skill(name))?; if !rendered.is_empty() { rendered.push_str("\n\n"); @@ -6299,6 +6335,70 @@ Working dir biorouter content skill_catalog::invalidate(); } + + /// Diagnostics 20261006_2: a Daily Meditation saved in v1.85.2 names + /// `soul-writer`, the Soul skill's name before v1.85.3, and startup deletes + /// that folder. A stock copy is upgraded at startup; a copy the person + /// edited is not, and failed every night with a skill nothing can install. + /// The old name now loads the skill it became, and a skill that really is + /// missing is refused with what to do about it. + #[tokio::test(flavor = "current_thread")] + async fn a_workflow_naming_the_renamed_soul_skill_loads_update_soul() { + if !crate::test_sandbox::in_a_process_of_its_own() { + return; + } + let temp = TempDir::new().unwrap(); + let _env = crate::test_sandbox::relocate_path_root(temp.path().to_str().unwrap()); + crate::knowledge::soul::ensure_soul_skill(&Paths::config_dir()).unwrap(); + skill_catalog::invalidate(); + + let manager = SessionManager::new(temp.path().join("sessions")); + let session = manager + .create_session( + temp.path().to_path_buf(), + "Scheduled job: daily-meditation".into(), + crate::session::SessionType::Scheduled, + ) + .await + .unwrap(); + + let rendered = workflow_skill_instructions( + &manager, + &session.id, + &[crate::knowledge::soul::SOUL_SKILL_DIR_LEGACY.to_string()], + ) + .await + .expect("the Soul skill's old name must load the renamed skill"); + assert!( + rendered.contains("# Required workflow skill: update-soul"), + "{rendered}" + ); + assert!(rendered.contains("# Writing the Soul"), "{rendered}"); + + // Naming both inlines the skill once. + let both = workflow_skill_instructions( + &manager, + &session.id, + &["soul-writer".to_string(), "update-soul".to_string()], + ) + .await + .unwrap(); + assert_eq!(both.matches("# Writing the Soul").count(), 1, "{both}"); + + let error = + workflow_skill_instructions(&manager, &session.id, &["no-such-skill".to_string()]) + .await + .unwrap_err() + .to_string(); + assert!( + error.contains("requires skill 'no-such-skill', but it is not installed") + && error.contains("Add the skill under Skills") + && error.contains("remove 'no-such-skill' from the workflow's skills"), + "{error}" + ); + + skill_catalog::invalidate(); + } /// **`/skills` is spelled once.** /// /// Three helpers claim to resolve the same path — `skill_catalog::roots()`'s diff --git a/crates/biorouter/src/config/extensions.rs b/crates/biorouter/src/config/extensions.rs index fe1e1709a..a4f9c1a76 100644 --- a/crates/biorouter/src/config/extensions.rs +++ b/crates/biorouter/src/config/extensions.rs @@ -230,6 +230,8 @@ pub fn bundled_extension_display_name(name: &str) -> Option<&'static str> { "code_execution" => return Some("Code Execution"), "skills" => return Some("Skills"), "todo" => return Some("Todo"), + // A capability in the desktop's Settings, named as its row there names it. + "crew" => return Some("Crew"), _ => {} } BUILTIN_EXTENSION_METADATA diff --git a/crates/biorouter/src/knowledge/soul.rs b/crates/biorouter/src/knowledge/soul.rs index 91ca365ef..15dabbd39 100644 --- a/crates/biorouter/src/knowledge/soul.rs +++ b/crates/biorouter/src/knowledge/soul.rs @@ -69,7 +69,41 @@ pub const MEDITATION_CRON: &str = "0 0 3 * * *"; pub const SOUL_COLOR: &str = "#9c6b3f"; const SOUL_RECONCILE_LOCK: &str = ".soul-reconcile.lock"; +/// The SHA-256 of every Meditation workflow text Biorouter shipped before the +/// current [`MEDITATION_WORKFLOW_YAML`]. A library or schedule copy whose bytes +/// hash to one of these is stock, and startup replaces it with the current +/// text; any other copy is the person's own and is left alone. +/// +/// ⚠ Each entry is the hash of REAL shipped bytes, and a test reads every one +/// of those texts back from `tests/fixtures/meditation/` and hashes it +/// (`every_meditation_text_biorouter_shipped_is_recognised_as_stock`). Until +/// diagnostics 20261006_2 this list held only the last two entries, which +/// match no text ever committed, so no install was ever upgraded. A v1.85.2 +/// copy kept requiring `soul-writer`, the skill v1.85.3 renamed to +/// `update-soul`, and once v1.90.0 made a workflow's skills required, Daily +/// Meditation failed every night with "requires skill 'soul-writer'". The +/// upgrade tests used made-up hashes, so nothing tied this list to what had +/// shipped. +/// +/// When [`MEDITATION_WORKFLOW_YAML`] changes, add the outgoing text's hash +/// here and its bytes as a fixture; the test that pins the current hash fails +/// until both are done. const PREVIOUS_MEDITATION_WORKFLOW_SHA256: &[&str] = &[ + // 6ba0bf201, before v1.85.2: requires `soul-writer`. + "e457b997531525b0c023b9b6929e12c4045aa037649f9268296b572433af0fb8", + // v1.85.2 (60703d513): requires `soul-writer`. The copy in diagnostics + // 20261006_2, byte for byte. + "b03af2d3bf2143573ac1229eee7d56e22ba92a9577ac598c4099c22623124bab", + // v1.85.3 (6151978bb): the skill renamed to `update-soul`. + "c1dcdbd5268789e8dd1e612c4508ed32de65fdf2d620a1dc9581e6acff823ab5", + // 8c1ea5d34, before v1.89.0. + "7d316d31f17e6d4e8599b971f90c3bd2d9614221bba14d83d9eb6cdb2ad68656", + // v1.89.0 (cd7de3504). + "573f3f9c6ce2c9b821632d77a64ee029d525db7aaa832562cebb7d5d7cb397d1", + // v1.89.7 (eb594ded3). + "c060d3b563139f2d1a7fa8c8425fec1f3d178241cb939482e95b1959f2db762c", + // Listed before any of the above, and matching no text ever committed on + // any ref. Kept: a hash that matches nothing upgrades nothing. "142edbf98aca3e521649ddec05ed156ab16936303cc42cbe8cce1bc79c630772", "a0558805057ffae5257fa805b73253a1b615300bec563a0126f4a23884f9884c", ]; @@ -1336,6 +1370,145 @@ mod tests { assert_eq!(std::fs::read_to_string(scheduled_copy).unwrap(), customized); } + /// Every Meditation text Biorouter has shipped, as the bytes it wrote to + /// disk, with where it came from. Read back from + /// `tests/fixtures/meditation/`, extracted from soul.rs at each commit that + /// changed the text. + const SHIPPED_MEDITATION_TEXTS: &[(&str, &[u8])] = &[ + ( + "6ba0bf201, before v1.85.2", + include_bytes!("../../tests/fixtures/meditation/e457b997.yaml"), + ), + ( + "v1.85.2 (60703d513), diagnostics 20261006_2", + include_bytes!("../../tests/fixtures/meditation/b03af2d3.yaml"), + ), + ( + "v1.85.3 (6151978bb)", + include_bytes!("../../tests/fixtures/meditation/c1dcdbd5.yaml"), + ), + ( + "8c1ea5d34, before v1.89.0", + include_bytes!("../../tests/fixtures/meditation/7d316d31.yaml"), + ), + ( + "v1.89.0 (cd7de3504)", + include_bytes!("../../tests/fixtures/meditation/573f3f9c.yaml"), + ), + ( + "v1.89.7 (eb594ded3)", + include_bytes!("../../tests/fixtures/meditation/c060d3b5.yaml"), + ), + ]; + + /// The hash of [`MEDITATION_WORKFLOW_YAML`] as it ships today (v1.90.0, + /// f81f5d64c). Changing the text fails this pin, and the message says what + /// else has to move with it. + const CURRENT_MEDITATION_WORKFLOW_SHA256: &str = + "4c3e2f773574c46a7ed4572aabf475fd90bf197020a83fdd86fb02084c0a1ac1"; + + fn sha256_hex(bytes: &[u8]) -> String { + format!("{:x}", Sha256::digest(bytes)) + } + + /// Diagnostics 20261006_2: the stock-copy list held two hashes that match + /// no Meditation text ever committed, so every install kept whatever text + /// it was first given. This holds the list to the real bytes. + #[test] + fn every_meditation_text_biorouter_shipped_is_recognised_as_stock() { + for (shipped, bytes) in SHIPPED_MEDITATION_TEXTS { + let hash = sha256_hex(bytes); + assert!( + PREVIOUS_MEDITATION_WORKFLOW_SHA256.contains(&hash.as_str()), + "the Meditation text shipped in {shipped} ({hash}) is not in \ + PREVIOUS_MEDITATION_WORKFLOW_SHA256, so an install that has it is never upgraded" + ); + // A fixture that is the current text would make this test pass + // while proving nothing about an older install. + assert_ne!(bytes, &MEDITATION_WORKFLOW_YAML.as_bytes(), "{shipped}"); + } + let current = sha256_hex(MEDITATION_WORKFLOW_YAML.as_bytes()); + assert_eq!( + current, CURRENT_MEDITATION_WORKFLOW_SHA256, + "MEDITATION_WORKFLOW_YAML changed. Add the outgoing text's hash \ + ({CURRENT_MEDITATION_WORKFLOW_SHA256}) to PREVIOUS_MEDITATION_WORKFLOW_SHA256, save \ + its bytes as tests/fixtures/meditation/.yaml and list it in \ + SHIPPED_MEDITATION_TEXTS, then update this pin" + ); + assert!( + !PREVIOUS_MEDITATION_WORKFLOW_SHA256.contains(¤t.as_str()), + "the current text must not be listed as a previous one, or every startup rewrites it" + ); + } + + /// Diagnostics 20261006_2, end to end on the reporter's own bytes: the + /// v1.85.2 text in both the workflow library and the schedule's copy. + /// Startup must replace both with today's text, which needs `update-soul`, + /// and keep the schedule's bookkeeping. Before the fix both copies kept + /// `skills: [soul-writer]` and the job failed every night. + #[test] + fn a_v1_85_2_meditation_copy_is_upgraded_in_the_library_and_the_schedule() { + let reporter_copy = SHIPPED_MEDITATION_TEXTS[1].1; + assert!(String::from_utf8_lossy(reporter_copy).contains("soul-writer")); + let tmp = tempfile::tempdir().unwrap(); + + let library = tmp.path().join("workflows").join(MEDITATION_WORKFLOW_FILE); + std::fs::create_dir_all(library.parent().unwrap()).unwrap(); + std::fs::write(&library, reporter_copy).unwrap(); + assert_eq!(ensure_meditation_workflow(tmp.path()).unwrap(), library); + assert_eq!( + std::fs::read_to_string(&library).unwrap(), + MEDITATION_WORKFLOW_YAML + ); + + let scheduled_copy = tmp.path().join("daily-meditation.yaml"); + std::fs::write(&scheduled_copy, reporter_copy).unwrap(); + let jobs = vec![ScheduledJob { + id: MEDITATION_SCHEDULE_ID.to_string(), + source: scheduled_copy.to_string_lossy().into_owned(), + cron: MEDITATION_CRON.to_string(), + last_run: Some(chrono::Utc::now()), + currently_running: false, + paused: false, + current_session_id: None, + process_start_time: None, + run_count: 94, + max_runs: None, + creator_session_id: None, + last_error: Some( + "workflow requires skill 'soul-writer', but it is not installed".to_string(), + ), + owns_source: None, + armed_with_private_reach: None, + }]; + let metadata_before = serde_json::to_value(&jobs[0]).unwrap(); + assert_eq!( + upgrade_existing_meditation_workflow( + &jobs, + MEDITATION_WORKFLOW_YAML, + PREVIOUS_MEDITATION_WORKFLOW_SHA256, + ) + .unwrap(), + Some(true) + ); + let upgraded = std::fs::read_to_string(&scheduled_copy).unwrap(); + assert_eq!(upgraded, MEDITATION_WORKFLOW_YAML); + assert_eq!(serde_json::to_value(&jobs[0]).unwrap(), metadata_before); + let workflow: Workflow = serde_yaml::from_str(&upgraded).unwrap(); + assert_eq!(workflow.skills, Some(vec![SOUL_SKILL_DIR.to_string()])); + + // The next startup finds today's text and leaves it alone. + assert_eq!( + upgrade_existing_meditation_workflow( + &jobs, + MEDITATION_WORKFLOW_YAML, + PREVIOUS_MEDITATION_WORKFLOW_SHA256, + ) + .unwrap(), + Some(false) + ); + } + fn make_legacy(svc: &KnowledgeService, id: &str, name: &str) { svc.create_base(id, name, None).unwrap(); let root = biorouter_mcp::knowledge::paths::kb_root(svc.root(), id); diff --git a/crates/biorouter/src/providers/errors.rs b/crates/biorouter/src/providers/errors.rs index 7a9d07876..4fe055d20 100644 --- a/crates/biorouter/src/providers/errors.rs +++ b/crates/biorouter/src/providers/errors.rs @@ -30,6 +30,21 @@ pub enum ProviderError { #[error("Unsupported operation: {0}")] NotImplemented(String), + + /// A local inference server sent nothing for the provider's idle bound + /// (`OLLAMA_TIMEOUT`). The text is a whole sentence naming the setting to + /// raise. + /// + /// Its own variant because it must not be re-sent by ANYONE. The provider + /// already hands a timeout straight back (`retry::TimeoutRetry::GiveUp`): + /// the server is still reading the abandoned request, and a second copy + /// queues behind it. As a `RequestFailed` it was classed a network blip + /// and the agent re-sent the whole prompt once anyway. Diagnostics + /// 20261006_2: on an Intel Mac reading about 25 prompt tokens a second, a + /// first prompt of about 37,000 tokens outlasts a 600-second bound, so the + /// person waited twenty minutes for "error sending request ... (timeout)". + #[error("{0}")] + IdleTimeout(String), } /// The coarse class of a provider failure — enough for a caller to decide @@ -252,6 +267,7 @@ impl ProviderError { | ProviderError::ExecutionError(details) | ProviderError::UsageError(details) => classify_provider_details(details), ProviderError::NotImplemented(_) => ProviderErrorKind::InvalidRequest, + ProviderError::IdleTimeout(_) => ProviderErrorKind::Network, } } @@ -265,6 +281,7 @@ impl ProviderError { ProviderError::ExecutionError(_) => "execution", ProviderError::UsageError(_) => "usage", ProviderError::NotImplemented(_) => "not_implemented", + ProviderError::IdleTimeout(_) => "timeout", } } } diff --git a/crates/biorouter/src/providers/formats/text_tool_calls.rs b/crates/biorouter/src/providers/formats/text_tool_calls.rs index 282473d7e..0d00e9a36 100644 --- a/crates/biorouter/src/providers/formats/text_tool_calls.rs +++ b/crates/biorouter/src/providers/formats/text_tool_calls.rs @@ -314,13 +314,14 @@ fn fence_body(text: &str) -> Option<&str> { Some(body) } -/// `callseq | callarray`: every element a valid call (G7), or `None`. -fn payload(text: &str, tools: &[Tool]) -> Option<(Vec, bool)> { - let objects: Vec = if text.starts_with('[') { +/// The JSON values of `callseq | callarray`, unchecked: one non-empty array, +/// or one or more values separated only by whitespace, and nothing else. +fn payload_values(text: &str) -> Option> { + if text.starts_with('[') { // One array, and nothing after it (serde rejects trailing characters). match serde_json::from_str::(text).ok()? { - Value::Array(items) if !items.is_empty() => items, - _ => return None, + Value::Array(items) if !items.is_empty() => Some(items), + _ => None, } } else if text.starts_with('{') { // One object, or several separated only by whitespace (llama3.1's raw @@ -333,10 +334,15 @@ fn payload(text: &str, tools: &[Tool]) -> Option<(Vec, bool)> { if objects.is_empty() || values.byte_offset() != text.len() { return None; } - objects + Some(objects) } else { - return None; - }; + None + } +} + +/// `callseq | callarray`: every element a valid call (G7), or `None`. +fn payload(text: &str, tools: &[Tool]) -> Option<(Vec, bool)> { + let objects = payload_values(text)?; let mut calls = Vec::with_capacity(objects.len()); let mut phi4 = false; for object in &objects { @@ -479,14 +485,40 @@ pub struct AttemptedCall { /// a quote becomes an execution. Measured, it did not even help: llama3.1:8b, /// asked once after "I will use the following tool call: {..}", repeated /// itself. +/// +/// The one-call case of [`attempted_calls`]. pub fn attempted_call(text: &str, tools: &[Tool]) -> Option { + let mut calls = attempted_calls(text, tools)?; + (calls.len() == 1).then(|| calls.remove(0)) +} + +/// [`attempted_call`] for a whole reply of one or more call objects, in the +/// shapes recovery reads (L2): objects separated only by whitespace, or one +/// array, optionally fenced or tag-wrapped as one block. Every object must be +/// a call whose name stands for an advertised tool, or nothing is an attempt, +/// just as one bad call rejects a whole recovery. +/// +/// Diagnostics 20261006_2: llama3.1:8b answered "find the blasts in these +/// notes" with two objects, `workspace_read_conversation` then +/// `workspace_read_panel`, each a real workspace tool without its prefix. +/// Read as one value, the pair was neither a call nor an attempt, so the turn +/// ended on the JSON with nothing asked or run, while either object alone +/// drew the corrective question. +pub fn attempted_calls(text: &str, tools: &[Tool]) -> Option> { if tools.is_empty() { return None; } let trimmed = text.trim(); let inner = unwrap_tags(trimmed, ATTEMPT_TAGS).unwrap_or(trimmed).trim(); let inner = fence_body(inner).map(str::trim).unwrap_or(inner); - let value = serde_json::from_str::(inner).ok()?; + payload_values(inner)? + .iter() + .map(|value| attempted_object(value, tools)) + .collect() +} + +/// One call object of an attempt: its name must stand for an advertised tool. +fn attempted_object(value: &Value, tools: &[Tool]) -> Option { let object = value.as_object()?; let (name, arguments) = match flat_call(object, true) { Some(call) => call, @@ -608,6 +640,109 @@ pub fn forged_tool_result(text: &str, tools: &[Tool]) -> Option { }) } +/// The names of the calls a WHOLE reply writes as text, as written, or `None`. +/// +/// Every block of the reply must be a call object, a whitespace-separated run +/// of them or an array of them, bare or in a ```` ```json ```` fence; the +/// whole may sit in a call or result tag. A name need not be a tool: this +/// answers "is this reply nothing but calls?", not "which tool was meant?". +/// Any prose, any other JSON, or one object that is not call-shaped, and it +/// is `None`. +/// +/// Detection only, for the notice the agent puts under such a reply +/// (`agents::reply_parts`): the reply is shown as it is, and the person is +/// told that nothing ran. Diagnostics 20261006_2: llama3.1:8b in a Code +/// Execution chat answered with `text_editor`, `shell` and `agent_drafter` +/// calls written as JSON, none of them a tool that chat offered, so nothing +/// recovered or asked about them, and the person read raw JSON as the answer +/// with no word that nothing had happened. Several blocks are read because a +/// corrective retry that repeats the call joins a second block to the first. +pub fn whole_reply_call_names(text: &str) -> Option> { + let notice_text = escape_unknown_json_string_escapes(text); + let trimmed = notice_text.trim(); + let mut rest = unwrap_tags(trimmed, ATTEMPT_TAGS).unwrap_or(trimmed).trim(); + let mut names = Vec::new(); + while !rest.is_empty() { + let values = if let Some(after_open) = rest.strip_prefix("```") { + let after_tag = after_open + .strip_prefix("json") + .or_else(|| after_open.strip_prefix("JSON")) + .unwrap_or(after_open); + let (tag, body) = after_tag.split_once('\n')?; + if !tag.trim().is_empty() { + // ```python and the like: code, not a call. + return None; + } + let (inner, after_fence) = body.split_once("```")?; + rest = after_fence.trim_start(); + payload_values(inner.trim())? + } else { + // One JSON value, then whitespace, another block, or the end. + let mut values = serde_json::Deserializer::from_str(rest).into_iter::(); + let value = values.next()?.ok()?; + rest = rest.get(values.byte_offset()..)?.trim_start(); + match value { + Value::Array(items) if !items.is_empty() => items, + Value::Object(_) => vec![value], + _ => return None, + } + }; + for value in &values { + names.push(written_call_name(value)?); + } + } + (!names.is_empty()).then_some(names) +} + +// Models sometimes embed code with JSON-invalid escapes such as `\d` or +// `\${n}`. Only the notice detector reads this copy; recovery and retry still +// require the original arguments to be valid JSON. +fn escape_unknown_json_string_escapes(text: &str) -> Cow<'_, str> { + let mut inside_string = false; + let mut escaped = false; + let mut changed = false; + let mut result = String::with_capacity(text.len()); + for character in text.chars() { + if escaped { + if !matches!( + character, + '"' | '\\' | '/' | 'b' | 'f' | 'n' | 'r' | 't' | 'u' + ) { + result.push('\\'); + changed = true; + } + escaped = false; + } else if inside_string && character == '\\' { + escaped = true; + } else if character == '"' { + inside_string = !inside_string; + } + result.push(character); + } + if changed { + Cow::Owned(result) + } else { + Cow::Borrowed(text) + } +} + +/// The name of one call object as written (S1-S3), or `None` when the object +/// is not call-shaped or names nothing. +fn written_call_name(value: &Value) -> Option { + let object = value.as_object()?; + let (name, _) = match flat_call(object, true) { + Some(call) => call, + None if has_exactly(object, &["type", "function"]) + && object.get("type").and_then(Value::as_str) == Some("function") => + { + flat_call(object.get("function")?.as_object()?, false)? + } + None => return None, + }; + let name = name.trim(); + (!name.is_empty()).then(|| name.to_string()) +} + // --------------------------------------------------------------------------- // R16: argument normalisation, for structured AND recovered calls. // --------------------------------------------------------------------------- @@ -1851,4 +1986,252 @@ mod tests { } assert_eq!(forged_tool_result(cases[0].0, &[]), None); } + + /// Diagnostics 20261006_2: every assistant reply of the reporter's chat, + /// verbatim (`tests/fixtures/ollama/diagnostics_20261006_2/replies.json`). + fn reporter_reply(index: u64) -> String { + let doc: Value = serde_json::from_str(include_str!( + "../../../tests/fixtures/ollama/diagnostics_20261006_2/replies.json" + )) + .unwrap(); + doc["replies"] + .as_array() + .unwrap() + .iter() + .find(|reply| reply["index"] == index) + .and_then(|reply| reply["reply"].as_str()) + .unwrap_or_else(|| panic!("no reply {index} in the fixture")) + .to_string() + } + + /// What the reporter's Code Execution chat offered the model directly: + /// Code Execution's three tools (schemas mirroring its parameter structs), + /// a todo tool, and the real workspace tools. Developer and Agent Drafter + /// were reachable only from a script. + fn reporter_code_execution_roster() -> Vec { + let mut tools = vec![ + tool( + "code_execution__execute_code", + json!({"type": "object", "properties": { + "code": {"type": "string"}, + "tool_graph": {"type": "array"} + }, "required": ["code"]}), + ), + tool( + "code_execution__read_module", + json!({"type": "object", "properties": {"module_path": {"type": "string"}}, "required": ["module_path"]}), + ), + tool( + "code_execution__search_modules", + json!({"type": "object", "properties": { + "terms": {"anyOf": [{"type": "string"}, {"type": "array", "items": {"type": "string"}}]}, + "regex": {"type": "boolean"} + }, "required": ["terms"]}), + ), + tool( + "todo__todo_write", + json!({"type": "object", "properties": {"content": {"type": "string"}}, "required": ["content"]}), + ), + ]; + for mut workspace in crate::agents::workspace_extension::WorkspaceClient::get_tools() { + workspace.name = format!("workspace__{}", workspace.name).into(); + tools.push(workspace); + } + tools + } + + /// The same chat with Code Execution off: Developer offered directly. + fn reporter_direct_roster() -> Vec { + let mut tools: Vec = reporter_code_execution_roster() + .into_iter() + .filter(|tool| !tool.name.starts_with("code_execution__")) + .collect(); + tools.push(tool( + "developer__shell", + json!({"type": "object", "properties": {"command": {"type": "string"}}, "required": ["command"]}), + )); + tools.push(tool( + "developer__text_editor", + json!({"type": "object", "properties": { + "command": {"type": "string"}, "path": {"type": "string"}, "file_text": {"type": "string"} + }, "required": ["command", "path"]}), + )); + tools + } + + fn attempt_names(text: &str, tools: &[Tool]) -> Option> { + attempted_calls(text, tools).map(|calls| { + calls + .into_iter() + .flat_map(|call| call.meant) + .collect::>() + }) + } + + /// Diagnostics 20261006_2, reply by reply: what Biorouter makes of each of + /// the reporter's exact assistant replies. llama3.1:8b on Ollama, in a chat + /// with Code Execution on; the reporter saw raw JSON as the answer to every + /// request that needed a tool. + /// + /// - Nothing is RUN from text: no reply names an offered tool byte for byte + /// with arguments that fit (G5, G6). + /// - A whole reply naming an offered tool without its prefix is ASKED for + /// once (`attempted_calls`): reply 9, and since this report reply 11, + /// whose two objects each name a workspace tool. + /// - A whole reply of calls that cannot be asked for (a tool Code Execution + /// hides, an extension's name) is shown with a notice that nothing ran + /// (`whole_reply_call_names`): replies 1, 7 and 15, and the same three of + /// 9 and 11 if their retry fails. + /// - Prose, code for the person to run, and the JSON answer the person asked + /// for (reply 13) are answers: never asked about, never noticed. + #[test] + fn the_replies_in_diagnostics_20261006_2_are_classified() { + let code = reporter_code_execution_roster(); + let direct = reporter_direct_roster(); + for index in [1, 3, 5, 7, 9, 11, 13, 15, 17, 19] { + let text = reporter_reply(index); + for roster in [&code, &direct] { + assert_eq!( + recover(&text, roster, Tiers::WholeMessage), + None, + "reply {index} must not run from text" + ); + assert_eq!(forged_tool_result(&text, roster), None, "reply {index}"); + } + } + + let asked = |index: u64, roster: &[Tool]| attempt_names(&reporter_reply(index), roster); + assert_eq!( + asked(9, &code), + Some(vec!["code_execution__search_modules".to_string()]) + ); + assert_eq!( + asked(11, &code), + Some(vec![ + "workspace__workspace_read_conversation".to_string(), + "workspace__workspace_read_panel".to_string(), + ]), + "each of reply 11's two objects names a workspace tool without its prefix" + ); + // `text_editor` and `shell` are Developer's, which Code Execution + // offers only from a script: nothing to ask the model to call there. + // Offered directly, they are asked for. + assert_eq!(asked(1, &code), None); + assert_eq!(asked(7, &code), None); + assert_eq!( + asked(1, &direct), + Some(vec!["developer__text_editor".to_string()]) + ); + assert_eq!( + asked(7, &direct), + Some(vec!["developer__shell".to_string()]) + ); + // An extension's name is not a tool's. + assert_eq!(asked(15, &code), None); + for answer in [3, 5, 13, 17, 19] { + assert_eq!(asked(answer, &code), None, "reply {answer}"); + assert_eq!(asked(answer, &direct), None, "reply {answer}"); + } + + let noticed = |index: u64| whole_reply_call_names(&reporter_reply(index)); + assert_eq!(noticed(1), Some(vec!["text_editor".to_string()])); + assert_eq!(noticed(7), Some(vec!["shell".to_string()])); + assert_eq!(noticed(9), Some(vec!["search_modules".to_string()])); + assert_eq!( + noticed(11), + Some(vec![ + "workspace_read_conversation".to_string(), + "workspace_read_panel".to_string(), + ]) + ); + assert_eq!(noticed(15), Some(vec!["agent_drafter".to_string()])); + for answer in [3, 5, 13, 17, 19] { + assert_eq!(noticed(answer), None, "reply {answer} is an answer"); + } + } + + /// The shapes `whole_reply_call_names` reads, beyond the reporter's: a + /// retry that repeats the call joins a second block to the first, and an + /// array or tag-wrapped payload is still only calls. + #[test] + fn a_whole_reply_of_calls_is_named_and_anything_else_is_not() { + let fenced = "```json\n{\"name\": \"shell\", \"arguments\": {\"commands\": [\"ls\"]}}\n```"; + for (text, names) in [ + ( + format!("{fenced}\n\n{fenced}"), + vec!["shell", "shell"], + ), + ( + "[{\"name\": \"a\", \"arguments\": {}}, {\"name\": \"b\", \"parameters\": {}}]" + .to_string(), + vec!["a", "b"], + ), + ( + "\n{\"type\": \"function\", \"function\": {\"name\": \"read\", \"arguments\": \"{}\"}}\n" + .to_string(), + vec!["read"], + ), + ] { + assert_eq!( + whole_reply_call_names(&text), + Some(names.iter().map(|name| name.to_string()).collect()), + "{text}" + ); + } + for text in [ + "", + "42", + "[]", + r#"{"sum": 42}"#, + r#"{"name": "", "arguments": {}}"#, + r#"{"name": "shell", "arguments": {}, "note": "extra key"}"#, + // A call followed or preceded by prose is never noticed. + "{\"name\": \"shell\", \"arguments\": {}}\nThat lists the folder.", + "I will run {\"name\": \"shell\", \"arguments\": {}}", + "```python\nprint(1)\n```", + // One call and one non-call: not a reply of calls. + "{\"name\": \"shell\", \"arguments\": {}}\n{\"total\": 3}", + ] { + assert_eq!(whole_reply_call_names(text), None, "{text}"); + } + } + + #[test] + fn the_live_qwen_reply_with_invalid_code_escapes_is_noticed_but_never_run() { + let text = include_str!( + "../../../tests/fixtures/ollama/diagnostics_20261006_2/qwen-malformed-call.txt" + ); + let tools = [tool("agent_drafter__create_app", json!({"type": "object"}))]; + assert_eq!( + whole_reply_call_names(text), + Some(vec!["agent_drafter__create_app".to_string()]) + ); + assert_eq!(recover(text, &tools, Tiers::WholeMessage), None); + assert_eq!(attempted_calls(text, &tools), None); + } + + #[test] + fn noticing_invalid_string_escapes_still_requires_a_whole_call_envelope() { + let call = r#"{"name": "shell", "arguments": {"command": "printf '\d'"}}"#; + for text in [ + call.to_string(), + format!("```json\n{call}\n```"), + format!("[{call}]"), + format!("{call}"), + ] { + assert_eq!(whole_reply_call_names(&text), Some(vec!["shell".into()])); + } + for text in [ + format!("Here is an example:\n{call}"), + format!("{call}\nThis is code for you to run."), + format!("```python\n{call}\n```"), + r#"{"name": "regex example", "pattern": "\d"}"#.to_string(), + r#"{"name": "shell", "arguments": {"command": "printf '\d'"}, "note": "example"}"# + .to_string(), + format!("{call}\n{{\"count\": 3}}"), + r#"{"name": "shell", "arguments": {"command": "\uZZZZ"}}"#.to_string(), + ] { + assert_eq!(whole_reply_call_names(&text), None, "{text}"); + } + } } diff --git a/crates/biorouter/src/providers/ollama.rs b/crates/biorouter/src/providers/ollama.rs index 641e63201..6d063af2f 100644 --- a/crates/biorouter/src/providers/ollama.rs +++ b/crates/biorouter/src/providers/ollama.rs @@ -72,7 +72,7 @@ use std::io; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, LazyLock, Mutex}; use std::time::Duration; -use tokio_util::codec::{FramedRead, LinesCodec}; +use tokio_util::codec::{FramedRead, LinesCodec, LinesCodecError}; use tokio_util::io::StreamReader; use url::Url; @@ -128,6 +128,21 @@ const BUILTIN_WINDOW_SETTING: &str = "OLLAMA_NUM_CTX"; /// which `BIOROUTER_CONTEXT_LIMIT` is the way to set. const CUSTOM_WINDOW_SETTING: &str = "BIOROUTER_CONTEXT_LIMIT"; +/// The setting that bounds how long the built-in provider waits on a silent +/// server. +const BUILTIN_TIMEOUT_SETTING: &str = "OLLAMA_TIMEOUT"; +/// The same for an Ollama-engine provider declared in a `custom_providers` +/// file: that file's `timeout_seconds`. +const CUSTOM_TIMEOUT_SETTING: &str = "timeout_seconds"; + +/// The settings a provider's errors tell the person to change: the built-in +/// provider's, or a custom provider's. +#[derive(Clone, Copy)] +struct SettingNames { + window: &'static str, + timeout: &'static str, +} + /// An output budget below this is spent on reasoning before an answer begins, /// so a request that pins one gets thinking off unless Deep effort asked for it. /// `biorouter configure` probes with 50 tokens: qwen3.8:27b at its default @@ -1234,31 +1249,56 @@ fn echoes_conversation( /// tells the model to run anything, and it never claims what did or did not /// run before — only that text is not a call. fn attempted_call_nudge( - call: &super::formats::text_tool_calls::AttemptedCall, + calls: &[super::formats::text_tool_calls::AttemptedCall], tools: &[Tool], ) -> String { - let meant = call - .meant + let quoted = |names: &[String], joiner: &str| { + names + .iter() + .map(|name| format!("'{name}'")) + .collect::>() + .join(joiner) + }; + let written = quoted( + &calls + .iter() + .map(|call| call.name.clone()) + .collect::>(), + " and ", + ); + let meant = calls .iter() - .map(|name| format!("'{name}'")) + .map(|call| quoted(&call.meant, " or ")) .collect::>() - .join(" or "); - if call.meant.len() == 1 && call.meant[0] == call.name { - format!( - "Your last reply was a call to '{}' written as text, and text in a reply is never \ - run as a tool call. If you intended to call {meant}, make it as a tool call with \ - arguments that match its parameters; otherwise answer in text.", - call.name - ) - } else { - format!( - "Your last reply was a call to '{}' written as text, and text in a reply is never \ - run as a tool call; there is also no tool with that name. If you intended to call \ - {meant}, make it as a tool call; otherwise answer in text. The tools you can call \ - are: {}.", - call.name, + .join(" and "); + let exact = calls + .iter() + .all(|call| call.meant.len() == 1 && call.meant[0] == call.name); + match (calls.len(), exact) { + (1, true) => format!( + "Your last reply was a call to {written} written as text, and text in a reply is \ + never run as a tool call. If you intended to call {meant}, make it as a tool call \ + with arguments that match its parameters; otherwise answer in text." + ), + (1, false) => format!( + "Your last reply was a call to {written} written as text, and text in a reply is \ + never run as a tool call; there is also no tool with that name. If you intended to \ + call {meant}, make it as a tool call; otherwise answer in text. The tools you can \ + call are: {}.", correction_tool_names(tools) - ) + ), + (_, true) => format!( + "Your last reply was calls to {written} written as text, and text in a reply is \ + never run as a tool call. If you intended to call {meant}, make them as tool calls \ + with arguments that match their parameters; otherwise answer in text." + ), + (_, false) => format!( + "Your last reply was calls to {written} written as text, and text in a reply is \ + never run as a tool call; not every one of those names is a tool. If you intended \ + to call {meant}, make them as tool calls; otherwise answer in text. The tools you \ + can call are: {}.", + correction_tool_names(tools) + ), } } @@ -1385,7 +1425,28 @@ async fn collect_reply( /// An error out of the format module: the [`ProviderError`] it carries, or a /// decode failure. -fn provider_error(error: anyhow::Error) -> ProviderError { +fn provider_error( + error: anyhow::Error, + idle: Duration, + timeout_setting: &str, + text_bytes: usize, +) -> ProviderError { + if error.chain().any(|cause| { + cause + .downcast_ref::() + .or_else(|| match cause.downcast_ref::()? { + LinesCodecError::Io(error) => error.get_ref()?.downcast_ref::(), + _ => None, + }) + .is_some_and(reqwest::Error::is_timeout) + }) { + return waited_too_long( + ProviderError::RequestFailed("response body read timed out".to_string()), + idle, + timeout_setting, + text_bytes, + ); + } error.downcast::().unwrap_or_else(|error| { ProviderError::RequestFailed(format!("Stream decode error: {error}")) }) @@ -1484,6 +1545,9 @@ fn decode_native( mut log: RequestLog, options: DecodeOptions, unknown_tool: Arc>>, + idle: Duration, + timeout_setting: &'static str, + text_bytes: usize, ) -> MessageStream { let bytes = response.bytes_stream().map_err(io::Error::other); Box::pin(try_stream! { @@ -1501,7 +1565,7 @@ fn decode_native( } mapped } - Err(error) => provider_error(error), + Err(error) => provider_error(error, idle, timeout_setting, text_bytes), }; let _ = log.provider_error(&error); error @@ -1520,6 +1584,8 @@ struct NativeTurn { model_config: ModelConfig, check: TurnCheck, window_setting: &'static str, + idle: Duration, + timeout_setting: &'static str, /// Every item of the turn, retry included, carries this id, so the agent /// folds them into one assistant message. message_id: String, @@ -1555,6 +1621,8 @@ fn native_turn(turn: NativeTurn, first: Response, first_log: RequestLog) -> Mess model_config, mut check, window_setting, + idle, + timeout_setting, message_id, tools, mut retried, @@ -1591,6 +1659,9 @@ fn native_turn(turn: NativeTurn, first: Response, first_log: RequestLog) -> Mess message_id: Some(message_id.clone()), }, unknown_tool.clone(), + idle, + timeout_setting, + check.text_bytes, ); let mut recovered = recover_leaked_tool_calls( decoded, @@ -1704,11 +1775,13 @@ fn native_turn(turn: NativeTurn, first: Response, first_log: RequestLog) -> Mess && !called_a_tool && guard.finish_reason.as_deref() != Some("length")) .then(|| { - super::formats::text_tool_calls::attempted_call(&shown_text, &tools) - .filter(|call| !echoes_conversation(&payload, call)) - .map(|call| { + super::formats::text_tool_calls::attempted_calls(&shown_text, &tools) + .filter(|calls| { + !calls.iter().any(|call| echoes_conversation(&payload, call)) + }) + .map(|calls| { ( - attempted_call_nudge(&call, &tools), + attempted_call_nudge(&calls, &tools), "the model's whole reply was a tool call written as text; asking \ it once more to make it as a call", ) @@ -1838,6 +1911,13 @@ pub struct OllamaProvider { /// nothing. #[serde(skip)] window_setting: &'static str, + /// How long this server may stay silent before a request is given up on, + /// and the setting that changes it, both named when that happens + /// ([`waited_too_long`]). + #[serde(skip)] + idle: Duration, + #[serde(skip)] + timeout_setting: &'static str, /// The `num_ctx` chosen per model, once. Only filled from a resolution that /// had `/api/show`'s answer, so a server that was briefly unreachable is /// asked again rather than pinned to a window chosen without the model's @@ -1882,7 +1962,10 @@ impl OllamaProvider { true, Self::metadata().name, read_settings(config), - BUILTIN_WINDOW_SETTING, + SettingNames { + window: BUILTIN_WINDOW_SETTING, + timeout: BUILTIN_TIMEOUT_SETTING, + }, ) } @@ -1904,7 +1987,10 @@ impl OllamaProvider { config.supports_streaming.unwrap_or(true), config.name.clone(), OllamaSettings::defaults(), - CUSTOM_WINDOW_SETTING, + SettingNames { + window: CUSTOM_WINDOW_SETTING, + timeout: CUSTOM_TIMEOUT_SETTING, + }, ) } @@ -1915,7 +2001,7 @@ impl OllamaProvider { supports_streaming: bool, name: String, settings: OllamaSettings, - window_setting: &'static str, + names: SettingNames, ) -> Result { let auth = AuthMethod::Custom(Box::new(NoAuth)); let resolved_base_url = base_url.to_string(); @@ -1931,7 +2017,9 @@ impl OllamaProvider { name, resolved_base_url, settings, - window_setting, + window_setting: names.window, + idle, + timeout_setting: names.timeout, windows: Mutex::new(HashMap::new()), live_context_limit: AtomicUsize::new(0), openai_compat: AtomicBool::new(false), @@ -2373,6 +2461,7 @@ impl OllamaProvider { .await }) .await + .map_err(|e| waited_too_long(e, self.idle, self.timeout_setting, check.text_bytes)) .inspect_err(|e| { let _ = log.provider_error(e); })?; @@ -2382,6 +2471,8 @@ impl OllamaProvider { model_config: model_config.clone(), check, window_setting: self.window_setting, + idle: self.idle, + timeout_setting: self.timeout_setting, message_id: format!("ollama_{}", uuid::Uuid::new_v4().simple()), tools: tools.to_vec(), retried: false, @@ -2552,11 +2643,18 @@ impl OllamaProvider { }) .await .map_err(|e| v1_capability_error(e, &self.model.model_name)) + .map_err(|e| waited_too_long(e, self.idle, self.timeout_setting, check.text_bytes)) .inspect_err(|e| { let _ = log.provider_error(e); })?; let decoded = recover_leaked_tool_calls( - stream_openai_compat_checked(response, log), + stream_openai_compat_checked( + response, + log, + self.idle, + self.timeout_setting, + check.text_bytes, + ), RecoveryContext { tools: tools.to_vec(), provider: "ollama", @@ -2572,6 +2670,50 @@ impl OllamaProvider { } } +/// A request this server answered with nothing at all for `idle`, as the error +/// the person reads; any other error is returned as it came. +/// +/// Ollama sends no byte while it reads a prompt, so the first sign of a prompt +/// too long for this computer is this bound firing before the reply begins. +/// It used to arrive as "Request failed: error sending request for url +/// (.../api/chat) (timeout)", which names neither the bound nor the prompt, and +/// as a network blip the agent re-sent the whole prompt once more behind the +/// first. Diagnostics 20261006_2: an Intel Mac reading about 25 prompt tokens a +/// second against a first prompt of about 37,000 tokens and a 600-second +/// bound. [`ProviderError::IdleTimeout`] is re-sent by nobody. +/// +/// The token count is the request's text at four bytes a token, rounded: an +/// order of magnitude to act on, not a measurement. +fn waited_too_long( + error: ProviderError, + idle: Duration, + timeout_setting: &str, + text_bytes: usize, +) -> ProviderError { + if !super::retry::is_timeout(&error) || matches!(error, ProviderError::IdleTimeout(_)) { + return error; + } + let tokens = ((text_bytes / 4 + 500) / 1000).max(1) * 1000; + let tokens = tokens + .to_string() + .as_bytes() + .rchunks(3) + .rev() + .map(|digits| String::from_utf8_lossy(digits).into_owned()) + .collect::>() + .join(","); + let waited = match idle.as_secs() { + 1 => "1 second".to_string(), + seconds => format!("{seconds} seconds"), + }; + ProviderError::IdleTimeout(format!( + "Ollama sent nothing back for {waited}, so Biorouter stopped waiting. This request's \ + prompt is about {tokens} tokens, and a computer without a fast GPU can take longer \ + than that just to read it. Raise {timeout_setting}, or turn off extensions this chat \ + does not need so the prompt is shorter." + )) +} + /// R19 on the `/v1` fallback: OpenAI's error shape carries the same sentence /// as the native 400 (`{"error":{"message":"... does not support tools", /// "type":"invalid_request_error"}}` from gemma3:4b), and is answered the same @@ -2593,7 +2735,13 @@ fn v1_capability_error(error: ProviderError, model: &str) -> ProviderError { /// Measured on Ollama 0.40.2: when ministral-3:8b called an undeclared tool /// after streaming text, `/v1` silently dropped the error and just ended the /// body, which the generic decoder handed on as a complete, successful reply. -fn stream_openai_compat_checked(response: Response, mut log: RequestLog) -> MessageStream { +fn stream_openai_compat_checked( + response: Response, + mut log: RequestLog, + idle: Duration, + timeout_setting: &'static str, + text_bytes: usize, +) -> MessageStream { let bytes = response.bytes_stream().map_err(io::Error::other); Box::pin(try_stream! { let ended = Arc::new(AtomicBool::new(false)); @@ -2616,7 +2764,7 @@ fn stream_openai_compat_checked(response: Response, mut log: RequestLog) -> Mess tokio::pin!(decoded); while let Some(item) = decoded.next().await { let item = item.map_err(|error| { - let error = provider_error(error); + let error = provider_error(error, idle, timeout_setting, text_bytes); let _ = log.provider_error(&error); error })?; @@ -5358,4 +5506,66 @@ mod tests { assert!(text.contains("reasoning only"), "{text}"); assert!(!text.contains("could not parse"), "{text}"); } + + /// Diagnostics 20261006_2: what the reporter would read when the built-in + /// provider's 600-second bound fires before Ollama has read their 164,005 + /// bytes of prompt. Anything that is not a timeout passes through as it + /// came. + #[test] + fn a_timeout_before_the_reply_names_the_setting_and_the_prompt_size() { + let timed_out = ProviderError::RequestFailed( + "error sending request for url (http://127.0.0.1:11434/api/chat) (timeout)".to_string(), + ); + let error = waited_too_long( + timed_out, + Duration::from_secs(OLLAMA_TIMEOUT), + BUILTIN_TIMEOUT_SETTING, + 164_005, + ); + let ProviderError::IdleTimeout(text) = &error else { + panic!("{error:?}"); + }; + assert_eq!( + text, + "Ollama sent nothing back for 600 seconds, so Biorouter stopped waiting. This \ + request's prompt is about 41,000 tokens, and a computer without a fast GPU can take \ + longer than that just to read it. Raise OLLAMA_TIMEOUT, or turn off extensions this \ + chat does not need so the prompt is shorter." + ); + assert!(!crate::agents::mistakes::is_recoverable(&error)); + assert!(super::super::retry::is_timeout(&error)); + + let others = || { + [ + ProviderError::RequestFailed( + "error sending request for url (http://127.0.0.1:9/api/chat) (failed to \ + connect to 127.0.0.1:9)" + .to_string(), + ), + ProviderError::ServerError("502".to_string()), + ProviderError::IdleTimeout("already said".to_string()), + ] + }; + for (other, unchanged) in others().into_iter().zip(others()) { + assert_eq!( + waited_too_long(other, Duration::from_secs(600), "X", 10), + unchanged + ); + } + } + + #[test] + fn a_decode_error_that_mentions_timeout_keeps_its_original_classification() { + let error = provider_error( + anyhow::anyhow!("invalid JSON in a reply about a timeout"), + Duration::from_secs(600), + BUILTIN_TIMEOUT_SETTING, + 160_000, + ); + assert!(matches!(error, ProviderError::RequestFailed(_))); + assert_eq!( + error.to_string(), + "Request failed: Stream decode error: invalid JSON in a reply about a timeout" + ); + } } diff --git a/crates/biorouter/src/providers/retry.rs b/crates/biorouter/src/providers/retry.rs index 8e7f2aa06..698a06446 100644 --- a/crates/biorouter/src/providers/retry.rs +++ b/crates/biorouter/src/providers/retry.rs @@ -147,6 +147,7 @@ pub enum TimeoutRetry { /// reqwest timeout `(timeout)`; a body read that stalls says "timed out". pub fn is_timeout(error: &ProviderError) -> bool { match error { + ProviderError::IdleTimeout(_) => true, ProviderError::RequestFailed(details) | ProviderError::ExecutionError(details) => { let details = details.to_ascii_lowercase(); error.kind() == super::errors::ProviderErrorKind::Network diff --git a/crates/biorouter/src/scheduler.rs b/crates/biorouter/src/scheduler.rs index 4ccf541e0..6c7d67b27 100644 --- a/crates/biorouter/src/scheduler.rs +++ b/crates/biorouter/src/scheduler.rs @@ -2657,6 +2657,20 @@ fn scheduled_prompt(job: &ScheduledJob, workflow: &Workflow) -> String { ) } +/// Delete the chat a scheduled run made for itself when the run failed before +/// its first message: nothing happened in it, and nobody can open it, because +/// History lists only chats with messages. A failure to delete is logged and +/// the run's own error still stands. +async fn discard_unstarted_run_session(session_manager: &SessionManager, session_id: &str) { + if let Err(error) = session_manager.delete_session(session_id).await { + tracing::warn!( + session_id, + "could not delete the empty chat of a scheduled run that failed before it started: \ + {error:#}" + ); + } +} + #[allow(clippy::too_many_lines)] async fn execute_job( job: ScheduledJob, @@ -2754,36 +2768,51 @@ async fn execute_job( ) .await?; - agent.update_provider(agent_provider, &session.id).await?; + // Everything from here to the first message can still refuse the run: the + // bind, a required skill that is missing or switched off, the workflow's + // own setup. Such a run never started, so it must not leave its chat + // behind. Diagnostics 20261006_2: Daily Meditation failed its skill check + // every night, and each failure left one empty "Scheduled job: + // daily-meditation" row in the store, hidden from History because it has + // no messages. + let setup = async { + agent.update_provider(agent_provider, &session.id).await?; + + let prepared_workflow_prompt = crate::workflow::runtime::prepare_prompt( + agent.config.session_manager.as_ref(), + &session.id, + &workflow, + ) + .await?; - let prepared_workflow_prompt = crate::workflow::runtime::prepare_prompt( - agent.config.session_manager.as_ref(), - &session.id, - &workflow, - ) - .await?; + // Persist and apply the workflow before the first model call. Scheduled + // runs used to load only `extensions`; their declared knowledge base, + // required skills, instructions, and structured-output components were + // silently ignored until after the turn had already finished. + agent + .config + .session_manager + .update(&session.id) + .schedule_id(Some(job.id.clone())) + .workflow(Some(workflow.clone())) + .apply() + .await?; - // Persist and apply the workflow before the first model call. Scheduled - // runs used to load only `extensions`; their declared knowledge base, - // required skills, instructions, and structured-output components were - // silently ignored until after the turn had already finished. - agent - .config - .session_manager - .update(&session.id) - .schedule_id(Some(job.id.clone())) - .workflow(Some(workflow.clone())) - .apply() + crate::workflow::runtime::install_prepared( + &agent, + &session.id, + &workflow, + true, + prepared_workflow_prompt, + ) .await?; - - crate::workflow::runtime::install_prepared( - &agent, - &session.id, - &workflow, - true, - prepared_workflow_prompt, - ) - .await?; + Ok::<(), anyhow::Error>(()) + } + .await; + if let Err(error) = setup { + discard_unstarted_run_session(agent.config.session_manager.as_ref(), &session.id).await; + return Err(error); + } let mut jobs_guard = jobs.lock().await; if let Some((_, job_def)) = jobs_guard.get_mut(job_id.as_str()) { @@ -3021,6 +3050,17 @@ fn finish_scheduled_run( } } +/// Fold one stream item into the run's transcript, or fail the run. +/// +/// ⚠ **`TurnAborted` fails the run.** The agent ends a turn that did not do +/// its work (the model call failed, the loop guard stopped it, the store +/// failed) with a "Ran into this error" message and then this event, and the +/// stream ends normally. Read as a completed run, that cleared `last_error` +/// and advanced `last_run`, which for `daily-meditation` is the Chat Recall +/// cursor: each night the model could not answer, a day of the person's chats +/// was skipped and the schedule reported success. Found re-running the Daily +/// Meditation of diagnostics 20261006_2 once its skill check passed, against a +/// local model that was not running. fn apply_scheduled_stream_item( conversation: &mut Conversation, item: Result, @@ -3028,6 +3068,13 @@ fn apply_scheduled_stream_item( match item? { AgentEvent::Message(message) => conversation.push(message), AgentEvent::HistoryReplaced(updated) => *conversation = updated, + AgentEvent::TurnAborted { code, message } => { + return Err(anyhow!( + "the run stopped before it did its work ({}), and the schedule's last-run \ + cursor was not advanced: {message}", + code.wire_code() + )); + } _ => {} } Ok(()) @@ -3974,6 +4021,63 @@ mod tests { ); } + /// Diagnostics 20261006_2: Daily Meditation failed its required-skill check + /// every night, and that check runs after the run has made its chat, so + /// each failure left an empty "Scheduled job: daily-meditation" row in the + /// store. A run that fails before its first message now leaves no chat. + /// + /// Driven through `execute_job` itself, on a loopback Ollama that is not + /// listening: the model is never asked anything, because the skill check + /// comes first. The rows live in `SessionManager::instance()`, which + /// `Agent::new()` reads, so the run's chat is found by its job id. + #[tokio::test(flavor = "current_thread")] + async fn a_run_that_fails_before_its_first_message_leaves_no_chat() { + let dir = tempdir().unwrap(); + let workflow = dir.path().join("needs_a_missing_skill.yaml"); + fs::write( + &workflow, + "version: 1.0.0\ntitle: Needs a missing skill\ndescription: probe\nprompt: test\n\ + extensions: []\nskills:\n- no-such-skill-20261006\n", + ) + .unwrap(); + let job_id = format!("missing-skill-{}", uuid::Uuid::new_v4().simple()); + let overrides = HashMap::from([ + ("BIOROUTER_PROVIDER".to_string(), "ollama".to_string()), + ("BIOROUTER_MODEL".to_string(), "llama3.1:8b".to_string()), + ("OLLAMA_HOST".to_string(), "http://127.0.0.1:9".to_string()), + ]); + let error = crate::config::with_config_overrides( + overrides, + execute_job( + dormant_job(&job_id, &workflow), + Arc::new(Mutex::new(JobsMap::new())), + job_id.clone(), + CancellationToken::new(), + ), + ) + .await + .expect_err("a workflow whose required skill is missing must not run"); + assert!( + error.to_string().contains("no-such-skill-20261006"), + "precondition: the run failed at its skill check, not before it made its chat: \ + {error:#}" + ); + + let name = format!("Scheduled job: {job_id}"); + let left: Vec = SessionManager::instance() + .list_sessions_by_types_including_empty(&[SessionType::Scheduled]) + .await + .unwrap() + .into_iter() + .filter(|session| session.name == name) + .map(|session| session.id) + .collect(); + assert!( + left.is_empty(), + "a run that never started left its empty chat behind: {left:?}" + ); + } + /// Issue #148B, the half that loses data. Cancellation `break`s the reply /// loop rather than erroring, so `execute_job` returned `Ok` and both /// completion paths advanced `last_run` — which, for `daily-meditation`, IS @@ -4915,6 +5019,43 @@ mod tests { .contains("fixture scheduled stream failed")); } + /// A turn the agent aborted (here, the model call failed) ends its stream + /// normally, after the "Ran into this error" message. Before this, the run + /// was then classified a success: `last_error` cleared and, for Daily + /// Meditation, the Chat Recall cursor advanced past a day nobody digested. + #[test] + fn an_aborted_turn_fails_the_scheduled_run() { + let mut conversation = Conversation::default(); + apply_scheduled_stream_item( + &mut conversation, + Ok(AgentEvent::Message(Message::assistant().with_text( + "Ran into this error: Request failed: error sending request for url \ + (http://127.0.0.1:9/api/chat) (failed to connect to 127.0.0.1:9).", + ))), + ) + .unwrap(); + let error = apply_scheduled_stream_item( + &mut conversation, + Ok(AgentEvent::TurnAborted { + code: crate::agents::turn_abort::TurnAbortCode::ProviderFailure { + kind: crate::providers::errors::ProviderErrorKind::Network, + }, + message: "Request failed: failed to connect to 127.0.0.1:9".to_string(), + }), + ) + .expect_err("an aborted turn must fail the scheduled run"); + let error = error.to_string(); + assert!( + error.contains("provider_failure") + && error.contains("failed to connect to 127.0.0.1:9") + && error.contains("cursor was not advanced"), + "{error}" + ); + // The person-facing explanation the agent wrote is still in the run's + // transcript. + assert_eq!(conversation.len(), 1); + } + #[test] fn meditation_prompt_uses_the_last_successful_run_as_its_recall_cursor() { let cursor = chrono::DateTime::parse_from_rfc3339("2026-08-28T10:11:12Z") diff --git a/crates/biorouter/tests/fixtures/meditation/573f3f9c.yaml b/crates/biorouter/tests/fixtures/meditation/573f3f9c.yaml new file mode 100644 index 000000000..4bf1b45f9 --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/573f3f9c.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Review the user's recent Biorouter sessions and save what matters about them + into the built-in "Soul" knowledge base: how they approach scientific + questions, the tools and commands they use, the tool responses they rely on, + and lasting personal details such as name, role, and preferences. Runs daily + at 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `update-soul` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER, not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:"), especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: link related entities/concepts with [[wiki-links]], + avoid duplicating facts already present, and prefer updating an existing + page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Search the skills installed on this machine and load the one that matches the task in hand + bundled: true + available_tools: [] +- type: platform + name: todo + description: Keep a running checklist through a multi-step task, so Biorouter tracks what is done and what is left + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search your earlier chats and load a summary of one, so work you already did can be picked up here + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- update-soul +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/meditation/7d316d31.yaml b/crates/biorouter/tests/fixtures/meditation/7d316d31.yaml new file mode 100644 index 000000000..47392c3ec --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/7d316d31.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Review the user's recent Biorouter sessions and save what matters about them + into the built-in "Soul" knowledge base: how they approach scientific + questions, the tools and commands they use, the tool responses they rely on, + and lasting personal details such as name, role, and preferences. Runs daily + at 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `update-soul` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER, not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:"), especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: link related entities/concepts with [[wiki-links]], + avoid duplicating facts already present, and prefer updating an existing + page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Load and use skills from relevant directories + bundled: true + available_tools: [] +- type: platform + name: todo + description: Enable a todo list for biorouter so it can keep track of what it is doing + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search past conversations and load session summaries for contextual memory + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- update-soul +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/meditation/b03af2d3.yaml b/crates/biorouter/tests/fixtures/meditation/b03af2d3.yaml new file mode 100644 index 000000000..638efe51d --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/b03af2d3.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Review the user's recent Biorouter sessions and save what matters about them + into the built-in "Soul" knowledge base: how they approach scientific + questions, the tools and commands they use, the tool responses they rely on, + and lasting personal details such as name, role, and preferences. Runs daily + at 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `soul-writer` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER — not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:") — especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: link related entities/concepts with [[wiki-links]], + avoid duplicating facts already present, and prefer updating an existing + page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Load and use skills from relevant directories + bundled: true + available_tools: [] +- type: platform + name: todo + description: Enable a todo list for biorouter so it can keep track of what it is doing + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search past conversations and load session summaries for contextual memory + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- soul-writer +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/meditation/c060d3b5.yaml b/crates/biorouter/tests/fixtures/meditation/c060d3b5.yaml new file mode 100644 index 000000000..af8eccc27 --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/c060d3b5.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Review the user's recent Biorouter sessions and save what matters about them + into the built-in "Soul" knowledge base: how they approach scientific + questions, the tools and commands they use, the tool responses they rely on, + and lasting personal details such as name, role, and preferences. Runs daily + at 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `update-soul` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER, not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:"), especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: use ordinary OKF markdown links to related page + paths, avoid duplicating facts already present, and prefer updating an + existing page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Search the skills installed on this machine and load the one that matches the task in hand + bundled: true + available_tools: [] +- type: platform + name: todo + description: Keep a running checklist through a multi-step task, so Biorouter tracks what is done and what is left + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search your earlier chats and load a summary of one, so work you already did can be picked up here + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- update-soul +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/meditation/c1dcdbd5.yaml b/crates/biorouter/tests/fixtures/meditation/c1dcdbd5.yaml new file mode 100644 index 000000000..3f5809ed6 --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/c1dcdbd5.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Review the user's recent Biorouter sessions and save what matters about them + into the built-in "Soul" knowledge base: how they approach scientific + questions, the tools and commands they use, the tool responses they rely on, + and lasting personal details such as name, role, and preferences. Runs daily + at 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `update-soul` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER — not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:") — especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: link related entities/concepts with [[wiki-links]], + avoid duplicating facts already present, and prefer updating an existing + page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Load and use skills from relevant directories + bundled: true + available_tools: [] +- type: platform + name: todo + description: Enable a todo list for biorouter so it can keep track of what it is doing + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search past conversations and load session summaries for contextual memory + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- update-soul +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/meditation/e457b997.yaml b/crates/biorouter/tests/fixtures/meditation/e457b997.yaml new file mode 100644 index 000000000..6a5238396 --- /dev/null +++ b/crates/biorouter/tests/fixtures/meditation/e457b997.yaml @@ -0,0 +1,67 @@ +version: 1.0.0 +title: Meditation +description: >- + Reflect on the user's recent BioRouter interactions and digest them into the + built-in "Soul" knowledge base — capturing how they approach scientific + questions, the tools and commands they use, the tool responses they act on, + and durable personal details (name, occupation, preferences). Runs daily at + 3:00 AM by default as "Daily Meditation". +instructions: |- + You are maintaining the user's personal "Soul" knowledge base (id: soul). + Follow the `soul-writer` skill exactly. + + Goal: turn the user's recent interaction history into durable, high-signal, + personalised knowledge about THE USER — not a summary of every chat. + + Procedure: + 1. Find the user's recent REAL chat sessions with the `chatrecall` tool + (search mode, with a recent date range and broad queries). Collect their + session ids. Skip scheduled-job sessions (names starting with + "Scheduled job:") — especially this very session. + 2. Call `platform__ingest_conversation` with EXPLICIT `session_ids` for the + most relevant recent session(s), targeting the `soul` knowledge base. + Never omit `session_ids`: omitting it defaults to the current scheduled + session, which contains nothing about the user. Prefer sessions since + the last Meditation. If chatrecall surfaces no real user sessions, stop + and make no changes. + 3. Prioritise capturing: + - the way the user approaches different scientific questions, + - the tools and extensions they use and how they call them, + - the commands they run and the responses they rely on, + - personal information they reveal: name, role/occupation, affiliations, + stated preferences and working style. + 4. Explicitly DISCARD low-value noise: greetings, chit-chat, one-off + irrelevant details, and anything that would not help a future assistant + serve this user better. + 5. Keep the Soul coherent: link related entities/concepts with [[wiki-links]], + avoid duplicating facts already present, and prefer updating an existing + page over creating a near-duplicate. + + If there is nothing new worth recording, say so and make no changes. +extensions: +- type: platform + name: skills + description: Load and use skills from relevant directories + bundled: true + available_tools: [] +- type: platform + name: todo + description: Enable a todo list for biorouter so it can keep track of what it is doing + bundled: true + available_tools: [] +- type: platform + name: chatrecall + description: Search past conversations and load session summaries for contextual memory + bundled: true + available_tools: [] +knowledge_bases: + default: soul + visible: + - soul +skills: +- soul-writer +activities: +- Update my Soul from recent interactions +- Learn my preferences and working style +- Record the tools and commands I use +parameters: [] diff --git a/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/qwen-malformed-call.txt b/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/qwen-malformed-call.txt new file mode 100644 index 000000000..f8a9ff06a --- /dev/null +++ b/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/qwen-malformed-call.txt @@ -0,0 +1,3 @@ +```json +{"name": "agent_drafter__create_app", "arguments": {"title": "Factorial Calculator", "description": "Calculate the factorial of a number using a Python script", "archetype": "explorer", "src/main.ts": "import { createApp } from \"./sdk\";\n\nconst br = createApp({ autoChat: false });\n\nbr.actions.register(\"calculate_factorial\", async (n) => {\n const response = await fetch(\"http://localhost:8000/factorial?n=\${n}\");\n const result = await response.text();\n return result;\n});\n\nbr.on(\"message\", async (e) => {\n if (e.type === \"message\") {\n const { content } = e;\n const match = content.match(/calculate factorial (\d+)/);\n if (match) {\n const n = parseInt(match[1], 10);\n const result = await br.call(\"calculate_factorial\", n);\n await br.run(`Factorial of ${n} is ${result}`, \"#result\");\n }\n }\n});", "system_prompt": "You are a calculator that can compute the factorial of a number using a Python script. Respond to user messages by calculating the factorial and displaying the result.", "greeting": "Welcome to the Factorial Calculator! Enter 'calculate factorial n' to get the factorial of n.", "model": "qwen2_5-coder-7b", "extensions": [], "skills": [], "knowledge_base": [], "capabilities": {"data_sources": []}, "guardrails": [], "reliability": [], "orchestration": {}, "output_type": "json"}} +``` \ No newline at end of file diff --git a/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/replies.json b/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/replies.json new file mode 100644 index 000000000..fb7f99c9a --- /dev/null +++ b/crates/biorouter/tests/fixtures/ollama/diagnostics_20261006_2/replies.json @@ -0,0 +1,45 @@ +{ + "note": "Every assistant reply in session 20261006_2 of diagnostics bundle 20261006_2 (Biorouter 1.92.0 on an Intel Mac, Ollama with llama3.1:8b and qwen2.5-coder:7b, Code Execution on). Verbatim, except that the account name in file paths is replaced with reporter. index is the message index in the session; the user message before each reply is at index - 1.", + "replies": [ + { + "index": 1, + "reply": "```json\n{\n \"name\": \"text_editor\",\n \"arguments\": {\n \"language\": \"python\",\n \"content\": \"def factorial(n):\\n if n == 0:\\n return 0\\n elif n == 1:\\n return 1\\n else:\\n return n * factorial(n - 1)\"\n }\n}\n```" + }, + { + "index": 3, + "reply": "Sure! Here's the equivalent data in straight Python with JSON formatted strings as function arguments:\n\n```python\nimport json\n\nfunction_calls = [\n {\n \"name\": \"workspace__workspace_read_panel\",\n \"arguments\": json.dumps({\n \"capture\": False,\n \"max_chars\": 20000,\n \"session_id\": None\n })\n },\n {\n \"name\": \"workspace__workspace_send_prompt\",\n \"arguments\": json.dumps({\n \"mode\": \"turn\",\n \"session_id\": None,\n \"text\": \"Inject some prompt here\",\n \"timeout_s\": None,\n \"wait\": \"final_message\"\n })\n },\n {\n \"name\": \"workspace__workspace_set_tools\",\n \"arguments\": json.dumps({\n \"add_extensions\": [],\n \"add_skills\": [\"skill1\", \"skill2\"],\n \"model\": None,\n \"primary_knowledge_base\": None,\n \"provider\": None,\n \"remove_extensions\": [],\n \"remove_skills\": [],\n \"session_id\": None,\n \"set_knowledge_bases\": [\"kb1\", \"kb2\"]\n })\n },\n {\n \"name\": \"workspace__workspace_watch\",\n \"arguments\": json.dumps({\n \"assume_running\": False,\n \"mode\": \"any\",\n \"session_ids\": [\"session_id1\", \"session_id2\"],\n \"timeout_s\": 120\n })\n }\n]\n```\n\nTo invoke these functions, you can create a function that takes the function name and arguments, and calls the appropriate Python function based on the name. However, since the actual function implementations aren't provided, I'll show you how you might structure this:\n\n```python\ndef call_function(function_name, arguments):\n if function_name == \"workspace__workspace_read_panel\":\n return workspace__workspace_read_panel(json.loads(arguments))\n elif function_name == \"workspace__workspace_send_prompt\":\n return workspace__workspace_send_prompt(json.loads(arguments))\n elif function_name == \"workspace__workspace_set_tools\":\n return workspace__workspace_set_tools(json.loads(arguments))\n elif function_name == \"workspace__workspace_watch\":\n return workspace__workspace_watch(json.loads(arguments))\n else:\n raise ValueError(f\"Unknown function: {function_name}\")\n\n# Example usage:\nresults = [call_function(call[\"name\"], call[\"arguments\"]) for call in function_calls]\n```\n\nIn this example, `workspace__workspace_read_panel`, `workspace__workspace_send_prompt`, `workspace__workspace_set_tools`, and `workspace__workspace_watch` should be defined elsewhere in your codebase to handle the actual logic. Each function would take a single argument, which is a dictionary of the function's arguments." + }, + { + "index": 5, + "reply": "Understood. To create a simple Python script that calculates the factorial of a given number and exits, you can follow these steps:\n\n1. Create a new file called `factorial.py`.\n2. Write the Python code to calculate the factorial.\n3. Ensure the script halts and exits after calculating the factorial.\n\nHere's an example of what the `factorial.py` file might look like:\n\n```python\nimport sys\n\ndef factorial(n):\n if n < 0:\n return \"Factorial is not defined for negative numbers\"\n elif n == 0:\n return 1\n else:\n result = 1\n for i in range(1, n + 1):\n result *= i\n return result\n\nif __name__ == \"__main__\":\n if len(sys.argv) != 2:\n print(\"Usage: python factorial.py \")\n sys.exit(1)\n\n try:\n n = int(sys.argv[1])\n except ValueError:\n print(\"Please provide a valid integer\")\n sys.exit(1)\n\n print(factorial(n))\n sys.exit(0)\n```\n\nExplanation:\n- The script takes one argument from the command line, which is converted to an integer.\n- It defines a function `factorial` that calculates the factorial of a non-negative integer.\n- The script checks if the number of arguments is correct and if the argument is a valid integer.\n- It prints the factorial of the number and then exits using `sys.exit(0)`.\n\nTo use this script, you would save it as `factorial.py`, and then run it from the command line like this:\n\n```sh\npython factorial.py 5\n```\n\nThis should output:\n\n```\n120\n```\n\nAnd the script will exit." + }, + { + "index": 7, + "reply": "{\"name\": \"shell\", \"arguments\": {\"commands\": [\"cat ~/temp\"]}}" + }, + { + "index": 9, + "reply": "```json\n{\n \"name\": \"search_modules\",\n \"arguments\": {\n \"query\": \"llama3.1:8b\"\n }\n}\n```" + }, + { + "index": 11, + "reply": "{\n \"name\": \"workspace_read_conversation\",\n \"arguments\": {\n \"session_id\": \"/Users/reporter/temp\"\n }\n}\n\n{\n \"name\": \"workspace_read_panel\",\n \"arguments\": {\n \"capture\": false,\n \"session_id\": \"/Users/reporter/temp\"\n }\n}" + }, + { + "index": 13, + "reply": "{\n \"diagnoses\": 0,\n \"medications\": 0,\n \"procedures\": 0,\n \"symptoms\": 0,\n \"relevant_dates\": 0,\n \"negated_findings\": 0,\n \"family_history_only_findings\": 0,\n \"uncertainty_rule_out_statements\": 0\n}" + }, + { + "index": 15, + "reply": "{\n \"name\": \"agent_drafter\",\n \"arguments\": {\n \"text\": \"In the provided notes, count all occurrences of the word 'urine', case insensitive.\"\n }\n}" + }, + { + "index": 17, + "reply": "Based on the provided information, the code that solves the problem is as follows:\n\n```javascript\nimport { shell } from \"developer\";\n\nconst tempDir = \"/Users/reporter/temp\";\nconst notesFilePath = `${tempDir}/clinical_notes.txt`;\n\n// Read the notes file and count occurrences of \"urine\" (case insensitive)\nshell.cat(notesFilePath).then(notes => {\n const count = (notes.match(/\\braine/gi) || []).length;\n console.log(`The word \"urine\" appears ${count} times.`);\n});\n```\n\nPlease note that, as mentioned in the description, the `shell.cat` tool is not defined by default, and you need to make sure it's available in your local environment. If it's not, you can define a custom function that reads the file and returns its contents.\n\nAdditionally, you might want to consider handling errors and edge cases, depending on your specific requirements." + }, + { + "index": 19, + "reply": "Here is the Python code as a .py file based on the specified calling parameter to return the count of occurrences of \"urine\" (case-insensitive):\n```python\nimport sys\n\ndef countoccurrences():\n result = {}\n with open('/var/log/brat.log', 'r') as f:\n for line in f:\n linesplit = line.split()\n for word in linesplit:\n word = word.lower()\n if word in result:\n result[word] += 1\n else:\n result[word] = 1\n return result\n\ndef searchparams(params_str, word):\n word = word.lower()\n paramsplit = params_str.split()\n count = 0\n newparams = []\n for param in paramsplit:\n if word in param:\n count += 1\n else:\n newparams.append(param)\n return count, ' '.join(newparams)\n\ndef main():\n if len(sys.argv) < 2:\n print(\"Error: missing parameter\")\n return\n with open('/var/log/brat.log', 'r') as f:\n for line in f:\n linesplit = line.split()\n for word in linesplit:\n if word.lower() == sys.argv[1]:\n print(1)\n return\n\n params_str = ''\n with open('/var/log/brat.log', 'r') as f:\n found = False\n for line in f:\n linesplit = line.split()\n if found:\n params_str += ' ' + ' '.join(linesplit)\n else:\n for word in linesplit:\n if word.lower() == sys.argv[1]:\n params_str = ' '.join(linesplit)\n found = True\n if not found:\n print(\"No occurrences found\")\n return\n\n try:\n if sys.argv[1].lower() == 'workspace_read_conversation':\n params_str = workspace__workspace_read_conversation(sys.argv[1])\n elif sys.argv[1].lower() == 'workspace_send_prompt':\n params_str = workspace__workspace_send_prompt(sys.argv[1])\n elif sys.argv[1].lower() == 'workspace_set_tools':\n params_str = workspace__workspace_set_tools(sys.argv[1])\n elif sys.argv[1].lower() == 'workspace_watch':\n params_str = workspace__workspace_watch(sys.argv[1])\n\n except Exception as e:\n print(\"exception found:\", e)\n\n count = 0\n paramsdict = {}\n words = sys.argv[1].strip().lower().split()\n for word in words:\n count += 1\n if count == 0:\n params_str = sys.argv[1]\n count, newparams = searchparams(params_str, sys.argv[1])\n\n print(\"Parameter: \" + params_str)\n print(\"Parameter after removal of '\" + sys.argv[1] + \"': \" + newparams)\n print(\"Count of '\" + sys.argv[1] + \"': \" + str(count))\n else:\n print(str(count))\nif __name__ == \"__main__\":\n main()\n```\nJust copy and paste this code into a .py file and run it, passing in the desired string as a parameter. For example, for searching for the word \"hepatitis b\" you should call it with `python stringcount.py \"hepatitis b\"`. The output will be the count of occurrences of the string. \n\nNote: The code assumes that the brat.log file contains information needed for the workspace tools (i.e. `workspace_read_conversation`, `workspace_send_prompt`, etc.) and the counts are obtained by reading this file." + } + ] +} diff --git a/crates/biorouter/tests/ollama_diagnostics_20261006_2.rs b/crates/biorouter/tests/ollama_diagnostics_20261006_2.rs new file mode 100644 index 000000000..3f11e6c0c --- /dev/null +++ b/crates/biorouter/tests/ollama_diagnostics_20261006_2.rs @@ -0,0 +1,1027 @@ +//! Diagnostics bundle 20261006_2, replayed against the real Ollama provider. +//! +//! The report: Biorouter 1.92.0 on an Intel Mac, Ollama with llama3.1:8b and +//! qwen2.5-coder:7b, a chat with Code Execution on. Every request that needed a +//! tool came back as JSON, and the person "decided just to ask for the code and +//! run it myself". The cause (fixed in #393) was the `/v1` route, which cannot +//! set Ollama's context window: each ~160 KB request was cut to 2,050 tokens, +//! so the model saw the end of the tool list and the question, and wrote calls +//! to tools it could not see as text. +//! +//! Each test serves one of the reporter's exact assistant replies +//! (`tests/fixtures/ollama/diagnostics_20261006_2/replies.json`, the account +//! name in paths replaced) from a mock Ollama, with the reporter's measured +//! usage where it matters, and asserts what Biorouter does with it today: the +//! cut turn is refused, a call to an offered tool written as text is asked for +//! once, a call nothing can run is shown and never run (the agent adds a line +//! saying so; `agents::reply_parts` tests that), an answer is an answer, and a +//! first prompt that outlasts the idle bound says which setting to raise and is +//! sent once. +//! +//! Nothing here reaches a network beyond the loopback mock server. + +use biorouter::agents::workspace_extension::WorkspaceClient; +use biorouter::config::declarative_providers::{DeclarativeProviderConfig, ProviderEngine}; +use biorouter::conversation::message::{Message, MessageContent, ToolRequest}; +use biorouter::model::ModelConfig; +use biorouter::providers::base::{Provider, ProviderStreamItem}; +use biorouter::providers::errors::{ProviderError, ProviderErrorKind}; +use biorouter::providers::ollama::OllamaProvider; +use futures::StreamExt; +use rmcp::model::Tool; +use serde_json::{json, Value}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// Sandbox this binary's config root before any test runs: the request log +/// directory and `Config::global()` resolve from `BIOROUTER_PATH_ROOT` the +/// first time anything touches them. An outer root wins. +#[ctor::ctor] +fn sandbox_config_root_for_this_test_binary() { + if biorouter::config::paths::Paths::path_root_override().is_some() { + return; + } + let root = tempfile::TempDir::new().expect("a scratch config root"); + std::env::set_var("BIOROUTER_PATH_ROOT", root.path()); + static ROOT: std::sync::OnceLock = std::sync::OnceLock::new(); + let _ = ROOT.set(root); +} + +const FIXTURES: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/ollama"); + +/// The reporter's exact reply at `index` in the session. +fn reply(index: u64) -> String { + let doc: Value = serde_json::from_str( + &std::fs::read_to_string(format!("{FIXTURES}/diagnostics_20261006_2/replies.json")) + .unwrap(), + ) + .unwrap(); + doc["replies"] + .as_array() + .unwrap() + .iter() + .find(|reply| reply["index"] == index) + .and_then(|reply| reply["reply"].as_str()) + .unwrap_or_else(|| panic!("no reply {index} in the fixture")) + .to_string() +} + +fn tool(name: &str, schema: Value) -> Tool { + Tool::new( + name.to_string(), + format!("{name} tool"), + rmcp::model::object(schema), + ) +} + +/// What the reporter's chat offered the model directly with Code Execution +/// on: its three tools (schemas mirroring its parameter structs), a todo tool, +/// and the real workspace tools. Developer and Agent Drafter were reachable +/// only from a script. +fn code_execution_roster() -> Vec { + let mut tools = vec![ + tool( + "code_execution__execute_code", + json!({"type": "object", "properties": { + "code": {"type": "string"}, "tool_graph": {"type": "array"} + }, "required": ["code"]}), + ), + tool( + "code_execution__read_module", + json!({"type": "object", "properties": {"module_path": {"type": "string"}}, "required": ["module_path"]}), + ), + tool( + "code_execution__search_modules", + json!({"type": "object", "properties": { + "terms": {"anyOf": [{"type": "string"}, {"type": "array", "items": {"type": "string"}}]}, + "regex": {"type": "boolean"} + }, "required": ["terms"]}), + ), + tool( + "todo__todo_write", + json!({"type": "object", "properties": {"content": {"type": "string"}}, "required": ["content"]}), + ), + ]; + for mut workspace in WorkspaceClient::get_tools() { + workspace.name = format!("workspace__{}", workspace.name).into(); + tools.push(workspace); + } + tools +} + +/// The same chat with Code Execution off: Developer offered directly. +fn direct_roster() -> Vec { + let mut tools: Vec = code_execution_roster() + .into_iter() + .filter(|tool| !tool.name.starts_with("code_execution__")) + .collect(); + tools.push(tool( + "developer__shell", + json!({"type": "object", "properties": {"command": {"type": "string"}}, "required": ["command"]}), + )); + tools +} + +/// `/api/show` for a model that calls tools, trained on `context_length`. +fn show(context_length: u64, template: Option) -> Value { + let mut body = json!({ + "capabilities": ["completion", "tools"], + "model_info": {"general.architecture": "llama", "llama.context_length": context_length}, + }); + if let Some(template) = template { + body["template"] = json!(template); + } + body +} + +/// One non-streamed native body answering `content`, with the prompt counts +/// the server reported. +fn text_body(content: &str, prompt: u64, cached: u64) -> String { + json!({ + "model": "model", + "message": {"role": "assistant", "content": content}, + "done": true, "done_reason": "stop", + "prompt_eval_count": prompt, "prompt_eval_cached_count": cached, "eval_count": 120, + }) + .to_string() +} + +/// One native body making a structured call. +fn structured_call(name: &str, arguments: Value) -> String { + json!({ + "model": "model", + "message": {"role": "assistant", "content": "", + "tool_calls": [{"id": "call_ok", "function": {"name": name, "arguments": arguments}}]}, + "done": true, "done_reason": "stop", "prompt_eval_count": 31_000, "eval_count": 40, + }) + .to_string() +} + +/// A mock Ollama whose `/api/chat` answers `replies` in order (the last one +/// repeats), each after `delay`, and a provider pointed at it with an idle +/// bound of `timeout_seconds`. +async fn serve( + model: &str, + show: Value, + replies: &[(u16, String)], + delay: std::time::Duration, + timeout_seconds: u64, +) -> (MockServer, OllamaProvider) { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/show")) + .respond_with(ResponseTemplate::new(200).set_body_json(show)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/api/version")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"version": "0.40.2"}))) + .mount(&server) + .await; + for (index, (status, body)) in replies.iter().enumerate() { + let mock = Mock::given(method("POST")) + .and(path("/api/chat")) + .respond_with( + ResponseTemplate::new(*status) + .set_delay(delay) + .set_body_raw(body.clone(), "application/x-ndjson"), + ); + let mock = if index + 1 < replies.len() { + mock.up_to_n_times(1) + } else { + mock + }; + mock.mount(&server).await; + } + let provider = OllamaProvider::from_custom_config( + ModelConfig::new_or_fail(&format!("{model}-{}", uuid::Uuid::new_v4().simple())), + DeclarativeProviderConfig { + name: "ollama-diagnostics-20261006-2".to_string(), + engine: ProviderEngine::Ollama, + display_name: "Ollama diagnostics 20261006_2".to_string(), + description: None, + api_key_env: "NOT_USED".to_string(), + base_url: server.uri(), + models: vec![], + headers: None, + timeout_seconds: Some(timeout_seconds), + supports_streaming: Some(true), + }, + ) + .expect("the provider constructs"); + (server, provider) +} + +async fn serve_replies(model: &str, replies: &[(u16, String)]) -> (MockServer, OllamaProvider) { + serve( + model, + show(131_072, None), + replies, + std::time::Duration::ZERO, + 10, + ) + .await +} + +async fn chat_requests(server: &MockServer) -> Vec { + server + .received_requests() + .await + .unwrap() + .iter() + .filter(|request| request.url.path() == "/api/chat") + .map(|request| serde_json::from_slice(&request.body).unwrap()) + .collect() +} + +/// The user message the provider sent last in `request`: on a corrective +/// retry, the question it asked. +fn last_user_text(request: &Value) -> String { + request["messages"] + .as_array() + .and_then(|messages| messages.last()) + .and_then(|message| message["content"].as_str()) + .unwrap_or_default() + .to_string() +} + +struct Turn { + items: Vec, + error: Option, +} + +impl Turn { + fn text(&self) -> String { + self.items + .iter() + .filter_map(|(message, _, _)| message.as_ref()) + .map(Message::as_concat_text) + .collect() + } + + fn calls(&self) -> Vec<&ToolRequest> { + self.items + .iter() + .filter_map(|(message, _, _)| message.as_ref()) + .flat_map(|message| message.content.iter()) + .filter_map(MessageContent::as_tool_request) + .collect() + } + + fn call_names(&self) -> Vec { + self.calls() + .iter() + .map(|call| call.tool_call.as_ref().unwrap().name.to_string()) + .collect() + } +} + +async fn turn(provider: &OllamaProvider, system: &str, user: &str, tools: &[Tool]) -> Turn { + let mut stream = match provider + .stream(system, &[Message::user().with_text(user)], tools) + .await + { + Ok(stream) => stream, + Err(error) => { + return Turn { + items: Vec::new(), + error: Some(error), + } + } + }; + let mut items = Vec::new(); + while let Some(item) = stream.next().await { + match item { + Ok(item) => items.push(item), + Err(error) => { + return Turn { + items, + error: Some(error), + } + } + } + } + Turn { items, error: None } +} + +/// The reporter's first turn as 1.92.0 sent it: a 164,005-byte request +/// (`llm_request.0.jsonl`) that Ollama answered from 2,046 new and 4 cached +/// prompt tokens, its 4,096-token default window keeping 4 head tokens and the +/// last 2,046. Today the request asks for the window it needs and refuses a +/// cut, and a reply that comes back cut anyway is an error, never the JSON the +/// reporter read. +#[tokio::test] +async fn the_reporters_cut_first_turn_is_refused_and_the_window_is_asked_for() { + let template = + std::fs::read_to_string(format!("{FIXTURES}/templates/llama3.1_8b.tmpl")).unwrap(); + let (server, provider) = serve( + "llama3.1:8b", + show(131_072, Some(template)), + &[(200, text_body(&reply(1), 2046, 4))], + std::time::Duration::ZERO, + 10, + ) + .await; + let system = "s".repeat(164_005 - 60); + let turn = turn( + &provider, + &system, + "write me some code to find the factorial of any number", + &code_execution_roster(), + ) + .await; + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 1); + assert!(requests[0]["options"]["num_ctx"].as_u64().unwrap() >= 65_536); + assert_eq!(requests[0]["truncate"], false); + assert!( + matches!(turn.error, Some(ProviderError::ContextLengthExceeded(_))), + "{:?}", + turn.error + ); + assert!(turn.calls().is_empty()); + assert!( + !turn.text().contains("text_editor"), + "the cut reply is never shown" + ); + assert_eq!( + provider.get_model_config().context_limit, + requests[0]["options"]["num_ctx"] + .as_u64() + .map(|n| n as usize), + "the gauge and compaction read the window really asked for, not the name's guess" + ); +} + +/// qwen2.5-coder:7b was trained on 32,768 tokens, and the reporter's prompt +/// was about 37,000. The request asks for the trained window and Ollama's +/// over-window refusal is a context error, sent once. (Whether a small model +/// should get a smaller prompt is an open product decision, not tested here.) +#[tokio::test] +async fn qwen25_coder_at_its_trained_window_refuses_the_reporters_prompt_size() { + let overflow = r#"{"error":"{\"error\":{\"code\":400,\"message\":\"request (37274 tokens) exceeds the available context size (32768 tokens), try increasing it\",\"type\":\"exceed_context_size_error\",\"n_prompt_tokens\":37274,\"n_ctx\":32768}}"}"#; + let (server, provider) = serve( + "qwen2.5-coder:7b", + show(32_768, None), + &[(400, overflow.to_string())], + std::time::Duration::ZERO, + 10, + ) + .await; + let turn = turn(&provider, "system", "hello", &code_execution_roster()).await; + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 1); + assert_eq!(requests[0]["options"]["num_ctx"], 32_768); + assert!( + matches!(turn.error, Some(ProviderError::ContextLengthExceeded(_))), + "{:?}", + turn.error + ); +} + +/// Reply 9: `search_modules` without its prefix, `query` for `terms`. The +/// model is asked once, naming the real tool, and its call completes the turn. +#[tokio::test] +async fn reply_9_is_asked_for_once_naming_code_execution_search_modules() { + let (server, provider) = serve_replies( + "qwen2.5-coder:7b", + &[ + (200, text_body(&reply(9), 31_000, 30_500)), + ( + 200, + structured_call("code_execution__search_modules", json!({"terms": "llama"})), + ), + ], + ) + .await; + let turn = turn( + &provider, + "system", + "can you actually execute llama3.1:8b against those notes", + &code_execution_roster(), + ) + .await; + assert!(turn.error.is_none(), "{:?}", turn.error); + assert_eq!(turn.call_names(), ["code_execution__search_modules"]); + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 2, "one corrective retry"); + let nudge = last_user_text(&requests[1]); + assert!( + nudge.contains("a call to 'search_modules' written as text") + && nudge.contains("If you intended to call 'code_execution__search_modules'"), + "{nudge}" + ); +} + +/// Reply 11: two objects, `workspace_read_conversation` then +/// `workspace_read_panel`, each a real workspace tool without its prefix. +/// Read as one value the pair was neither a call nor an attempt, so the turn +/// ended on the JSON; now the model is asked once about both. +#[tokio::test] +async fn reply_11_two_calls_in_one_reply_are_asked_for_once() { + let (server, provider) = serve_replies( + "llama3.1:8b", + &[ + (200, text_body(&reply(11), 31_000, 30_500)), + ( + 200, + structured_call( + "workspace__workspace_read_conversation", + json!({"session_id": "20261006_2"}), + ), + ), + ], + ) + .await; + let turn = turn( + &provider, + "system", + "take the file called ~/temp and find any occurrences of blasts", + &code_execution_roster(), + ) + .await; + assert!(turn.error.is_none(), "{:?}", turn.error); + assert_eq!( + turn.call_names(), + ["workspace__workspace_read_conversation"] + ); + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 2, "one corrective retry"); + let nudge = last_user_text(&requests[1]); + assert!( + nudge.contains("calls to 'workspace_read_conversation' and 'workspace_read_panel'") + && nudge.contains( + "'workspace__workspace_read_conversation' and 'workspace__workspace_read_panel'" + ) + && nudge.contains("otherwise answer in text"), + "{nudge}" + ); +} + +/// Replies 1 (`text_editor`), 7 (`shell`) and 15 (`agent_drafter`) in the +/// reporter's Code Execution chat: none names a tool that chat offered, so +/// nothing is run and nothing is asked. The reply is shown as it came; the +/// agent then adds a line saying nothing ran. +#[tokio::test] +async fn replies_1_7_and_15_in_a_code_execution_chat_are_shown_and_never_run() { + for index in [1, 7, 15] { + let (server, provider) = serve_replies( + "llama3.1:8b", + &[(200, text_body(&reply(index), 31_000, 30_500))], + ) + .await; + let turn = turn(&provider, "system", "do it", &code_execution_roster()).await; + assert!(turn.error.is_none(), "reply {index}: {:?}", turn.error); + assert!(turn.calls().is_empty(), "reply {index}"); + assert_eq!(turn.text().trim(), reply(index).trim(), "reply {index}"); + assert_eq!( + chat_requests(&server).await.len(), + 1, + "reply {index}: no retry" + ); + } +} + +/// Reply 7 where Developer IS offered directly: the same `shell` call is +/// asked for once, naming `developer__shell`. +#[tokio::test] +async fn reply_7_with_developer_offered_is_asked_for_once() { + let (server, provider) = serve_replies( + "llama3.1:8b", + &[ + (200, text_body(&reply(7), 31_000, 30_500)), + ( + 200, + structured_call("developer__shell", json!({"command": "cat ~/temp"})), + ), + ], + ) + .await; + let turn = turn(&provider, "system", "do it", &direct_roster()).await; + assert!(turn.error.is_none(), "{:?}", turn.error); + assert_eq!(turn.call_names(), ["developer__shell"]); + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 2); + assert!( + last_user_text(&requests[1]).contains("If you intended to call 'developer__shell'"), + "{}", + last_user_text(&requests[1]) + ); +} + +/// The reporter's answers: prose with code for the person to run (3, 5, 17, +/// 19) and the JSON the person asked for (13). Shown, never asked about. +#[tokio::test] +async fn the_reporters_answers_are_never_asked_about() { + for index in [3, 5, 13, 17, 19] { + let (server, provider) = serve_replies( + "llama3.1:8b", + &[(200, text_body(&reply(index), 31_000, 30_500))], + ) + .await; + let turn = turn(&provider, "system", "do it", &code_execution_roster()).await; + assert!(turn.error.is_none(), "reply {index}: {:?}", turn.error); + assert!(turn.calls().is_empty(), "reply {index}"); + assert_eq!(turn.text().trim(), reply(index).trim(), "reply {index}"); + assert_eq!( + chat_requests(&server).await.len(), + 1, + "reply {index}: no retry" + ); + } +} + +/// The reporter's machine read about 25 prompt tokens a second (llama3.1:8b) +/// with `OLLAMA_TIMEOUT` at 600 s, so a whole first prompt of about 37,000 +/// tokens would be a quarter of an hour of silence before Ollama's first byte. +/// Scaled down (a 1 s bound, 3 s of silence): the person reads which setting to +/// raise and how large the prompt is, the request is sent once, and the agent +/// does not send it again either. +#[tokio::test] +async fn a_first_prompt_slower_than_the_idle_bound_names_the_bound_and_is_sent_once() { + let (server, provider) = serve( + "llama3.1:8b", + show(131_072, None), + &[(200, text_body("hello", 37_000, 0))], + std::time::Duration::from_secs(3), + 1, + ) + .await; + let system = "s".repeat(160_000); + let turn = turn(&provider, &system, "hello", &code_execution_roster()).await; + let error = turn.error.expect("the bound fires before the first byte"); + let text = error.to_string(); + assert!(matches!(error, ProviderError::IdleTimeout(_)), "{error:?}"); + assert!( + text.contains("sent nothing back for 1 second,") + && text.contains("Raise timeout_seconds") + && text.contains("about 4") + && text.contains(",000 tokens"), + "{text}" + ); + assert_eq!(error.kind(), ProviderErrorKind::Network); + assert!( + !biorouter::agents::mistakes::is_recoverable(&error), + "the agent must not send the whole prompt again behind the first" + ); + assert_eq!(chat_requests(&server).await.len(), 1, "sent once"); +} + +#[tokio::test] +async fn a_body_timeout_after_headers_names_the_bound_and_is_sent_once() { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let requests = Arc::new(AtomicUsize::new(0)); + let counted = requests.clone(); + let server = tokio::spawn(async move { + let mut clients = tokio::task::JoinSet::new(); + loop { + let (mut socket, _) = listener.accept().await.unwrap(); + let counted = counted.clone(); + clients.spawn(async move { + let mut request = Vec::new(); + let mut buf = [0_u8; 4096]; + let header_end = loop { + let n = socket.read(&mut buf).await.unwrap(); + if n == 0 { + return; + } + request.extend_from_slice(&buf[..n]); + if let Some(end) = request.windows(4).position(|w| w == b"\r\n\r\n") { + break end + 4; + } + }; + let headers = String::from_utf8_lossy(&request[..header_end]).into_owned(); + let content_length = headers + .lines() + .find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().unwrap()) + }) + .unwrap_or(0); + while request.len() < header_end + content_length { + let n = socket.read(&mut buf).await.unwrap(); + if n == 0 { + return; + } + request.extend_from_slice(&buf[..n]); + } + let route = headers.split_whitespace().nth(1).unwrap(); + let body = match route { + "/api/show" => show(131_072, None).to_string(), + "/api/version" => json!({"version": "0.40.2"}).to_string(), + "/api/ps" => json!({"models": []}).to_string(), + "/api/chat" => { + counted.fetch_add(1, Ordering::SeqCst); + text_body("hello", 37_000, 0) + } + _ => panic!("unexpected route {route}"), + }; + socket + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/x-ndjson\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ) + .as_bytes(), + ) + .await + .unwrap(); + if route == "/api/chat" { + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + } + let _ = socket.write_all(body.as_bytes()).await; + }); + } + }); + let provider = OllamaProvider::from_custom_config( + ModelConfig::new_or_fail(&format!("llama3.1:8b-{}", uuid::Uuid::new_v4().simple())), + DeclarativeProviderConfig { + name: "ollama-body-timeout".to_string(), + engine: ProviderEngine::Ollama, + display_name: "Ollama body timeout".to_string(), + description: None, + api_key_env: "NOT_USED".to_string(), + base_url: format!("http://{address}"), + models: vec![], + headers: None, + timeout_seconds: Some(1), + supports_streaming: Some(true), + }, + ) + .unwrap(); + let turn = turn( + &provider, + &"s".repeat(160_000), + "hello", + &code_execution_roster(), + ) + .await; + server.abort(); + let error = turn + .error + .expect("headers alone do not satisfy the idle bound"); + assert!(matches!(error, ProviderError::IdleTimeout(_)), "{error:?}"); + assert!( + error.to_string().contains("Raise timeout_seconds"), + "{error}" + ); + assert_eq!(error.kind(), ProviderErrorKind::Network); + assert!(!biorouter::agents::mistakes::is_recoverable(&error)); + assert_eq!(requests.load(Ordering::SeqCst), 1, "sent once"); +} + +// --------------------------------------------------------------------------- +// Through the agent: what the person reads, and what the model is sent next. +// --------------------------------------------------------------------------- + +/// A model that answers its first request with the reporter's reply 7 and +/// every later one with "ok", recording what each request carried. +struct ReporterModel { + requests: std::sync::Mutex>>, +} + +#[async_trait::async_trait] +impl Provider for ReporterModel { + async fn complete( + &self, + _system: &str, + messages: &[Message], + _tools: &[Tool], + ) -> Result<(Message, biorouter::providers::base::ProviderUsage), ProviderError> { + let first = { + let mut requests = self.requests.lock().unwrap(); + requests.push(messages.to_vec()); + requests.len() == 1 + }; + let text = if first { reply(7) } else { "ok".to_string() }; + let mut usage = biorouter::providers::base::ProviderUsage::new( + "llama3.1:8b".to_string(), + biorouter::providers::base::Usage::new(Some(31_000), Some(20), Some(31_020)), + ); + usage.finish_reason = Some("stop".to_string()); + Ok((Message::assistant().with_text(text), usage)) + } + + async fn complete_with_model( + &self, + _model: &ModelConfig, + system: &str, + messages: &[Message], + tools: &[Tool], + ) -> Result<(Message, biorouter::providers::base::ProviderUsage), ProviderError> { + self.complete(system, messages, tools).await + } + + fn get_model_config(&self) -> ModelConfig { + ModelConfig::new_or_fail("llama3.1:8b") + } + + fn metadata() -> biorouter::providers::base::ProviderMetadata { + biorouter::providers::base::ProviderMetadata::empty() + } + + fn get_name(&self) -> &str { + "diagnostics-20261006-2-model" + } +} + +fn is_unrun_call_notice(message: &Message) -> bool { + message.content.iter().any(|content| { + matches!( + content, + MessageContent::SystemNotification(note) if note.msg.starts_with("Nothing ran") + ) + }) +} + +/// End to end through the agent, on the reporter's reply 7 (`shell` written as +/// JSON) in a turn that offered tools: the reply is shown and stored as it +/// came, one line after it tells the person that nothing ran, that line is +/// stored for the person only, and the model's next request does not carry it. +#[tokio::test] +async fn the_agent_tells_the_person_nothing_ran_and_keeps_it_from_the_model() { + use biorouter::agents::extension::ExtensionConfig; + use biorouter::agents::{Agent, AgentConfig, AgentEvent, SessionConfig}; + + let work = tempfile::TempDir::new().unwrap(); + let data = tempfile::TempDir::new().unwrap(); + let permissions = tempfile::TempDir::new().unwrap(); + let manager = std::sync::Arc::new(biorouter::session::SessionManager::new( + data.path().to_path_buf(), + )); + let agent = Agent::with_config(AgentConfig::new( + manager.clone(), + std::sync::Arc::new(biorouter::config::permission::PermissionManager::new( + permissions.path().to_path_buf(), + )), + None, + biorouter::config::BioRouterMode::Auto, + )); + let session = manager + .create_session( + work.path().to_path_buf(), + "diagnostics 20261006_2".to_string(), + biorouter::session::session_manager::SessionType::Hidden, + ) + .await + .unwrap(); + let model = std::sync::Arc::new(ReporterModel { + requests: std::sync::Mutex::new(Vec::new()), + }); + agent + .update_provider(model.clone(), &session.id) + .await + .unwrap(); + agent + .add_extension(ExtensionConfig::Frontend { + name: "notes".to_string(), + description: "Read the person's notes".to_string(), + tools: vec![tool( + "notes__read", + json!({"type": "object", "properties": {"path": {"type": "string"}}}), + )], + instructions: None, + bundled: None, + available_tools: Vec::new(), + }) + .await + .unwrap(); + let config = || SessionConfig { + id: session.id.clone(), + schedule_id: None, + max_turns: Some(3), + max_tool_calls: Some(3), + budget: None, + retry_config: None, + reasoning_effort: None, + }; + + let mut shown = Vec::new(); + for user in ["count the word urine in ~/temp", "thanks"] { + let stream = agent + .reply(Message::user().with_text(user), config(), None) + .await + .unwrap(); + tokio::pin!(stream); + tokio::time::timeout(std::time::Duration::from_secs(30), async { + while let Some(event) = stream.next().await { + if let AgentEvent::Message(message) = event.unwrap() { + shown.push(message); + } + } + }) + .await + .expect("a bounded turn"); + } + + let notices: Vec<&Message> = shown + .iter() + .filter(|message| is_unrun_call_notice(message)) + .collect(); + assert_eq!( + notices.len(), + 1, + "one notice, after the first turn: {shown:?}" + ); + let MessageContent::SystemNotification(note) = ¬ices[0].content[0] else { + unreachable!() + }; + assert!( + note.msg.contains("a call to 'shell' written as text"), + "{}", + note.msg + ); + assert!( + notices[0].is_user_visible() && !notices[0].is_agent_visible(), + "the live copy says who the notice is for, as the stored row does: {:?}", + notices[0].metadata + ); + assert!(shown + .iter() + .any(|message| message.as_concat_text() == reply(7))); + + let stored = manager + .get_session(&session.id, true) + .await + .unwrap() + .conversation + .unwrap(); + let stored_notice = stored + .iter() + .find(|message| is_unrun_call_notice(message)) + .expect("the notice is stored, so a reload shows it"); + assert!(stored_notice.is_user_visible() && !stored_notice.is_agent_visible()); + + let requests = model.requests.lock().unwrap(); + let second = requests + .iter() + .find(|request| { + request + .iter() + .any(|message| message.as_concat_text().contains("thanks")) + }) + .unwrap_or_else(|| panic!("the second turn reached the model: {requests:?}")); + // Providers are handed the whole history and every wire format keeps only + // the agent-visible messages (the mechanism compaction relies on too), so + // what the model is sent is exactly this filter. + let sent_to_the_model: Vec<&Message> = second + .iter() + .filter(|message| message.is_agent_visible()) + .collect(); + assert!( + sent_to_the_model + .iter() + .all(|message| !is_unrun_call_notice(message) + && !message.as_concat_text().contains("Nothing ran")), + "the notice is the person's, never the model's: {sent_to_the_model:?}" + ); + assert!( + sent_to_the_model + .iter() + .any(|message| message.as_concat_text() == reply(7)), + "the reply itself stays in the model's history" + ); + // And through Ollama's own request builder, which is what the reporter's + // model was sent. + let wire = biorouter::providers::formats::ollama::create_request( + &biorouter::providers::formats::ollama::ChatRequest { + model: "llama3.1:8b", + system: "system", + messages: second, + tools: &[], + stream: true, + num_ctx: 65_536, + temperature: None, + num_predict: None, + think: None, + keep_alive: None, + vision: false, + calls_without_text: false, + }, + ) + .unwrap() + .to_string(); + assert!(!wire.contains("Nothing ran"), "{wire}"); +} + +#[tokio::test] +async fn the_live_malformed_qwen_reply_gets_a_user_only_notice_without_dispatch() { + use biorouter::agents::extension::ExtensionConfig; + use biorouter::agents::{Agent, AgentConfig, AgentEvent, SessionConfig}; + + let text = include_str!("fixtures/ollama/diagnostics_20261006_2/qwen-malformed-call.txt"); + let (server, provider) = serve_replies( + "llama3.1:8b", + &[ + (200, text_body(text, 31_000, 30_500)), + (200, text_body("ok", 31_000, 30_500)), + ], + ) + .await; + let work = tempfile::TempDir::new().unwrap(); + let data = tempfile::TempDir::new().unwrap(); + let permissions = tempfile::TempDir::new().unwrap(); + let manager = std::sync::Arc::new(biorouter::session::SessionManager::new( + data.path().to_path_buf(), + )); + let agent = Agent::with_config(AgentConfig::new( + manager.clone(), + std::sync::Arc::new(biorouter::config::permission::PermissionManager::new( + permissions.path().to_path_buf(), + )), + None, + biorouter::config::BioRouterMode::Auto, + )); + let session = manager + .create_session( + work.path().to_path_buf(), + "synthetic malformed qwen replay".to_string(), + biorouter::session::session_manager::SessionType::Hidden, + ) + .await + .unwrap(); + agent + .update_provider(std::sync::Arc::new(provider), &session.id) + .await + .unwrap(); + agent + .add_extension(ExtensionConfig::Frontend { + name: "notes".to_string(), + description: "Read synthetic notes".to_string(), + tools: vec![tool("notes__read", json!({"type": "object"}))], + instructions: None, + bundled: None, + available_tools: Vec::new(), + }) + .await + .unwrap(); + let mut shown = Vec::new(); + for user in ["count urine in the synthetic notes", "thanks"] { + let stream = agent + .reply( + Message::user().with_text(user), + SessionConfig { + id: session.id.clone(), + schedule_id: None, + max_turns: Some(3), + max_tool_calls: Some(3), + budget: None, + retry_config: None, + reasoning_effort: None, + }, + None, + ) + .await + .unwrap(); + tokio::pin!(stream); + tokio::time::timeout(std::time::Duration::from_secs(30), async { + while let Some(event) = stream.next().await { + if let AgentEvent::Message(message) = event.unwrap() { + shown.push(message); + } + } + }) + .await + .expect("a bounded fixture replay"); + } + assert!(shown.iter().any(|message| message.as_concat_text() == text)); + assert!(shown + .iter() + .all(|message| message.content.iter().all(|content| !matches!( + content, + MessageContent::ToolRequest(_) | MessageContent::FrontendToolRequest(_) + )))); + let notices: Vec<_> = shown + .iter() + .filter(|message| is_unrun_call_notice(message)) + .collect(); + assert_eq!(notices.len(), 1, "{shown:?}"); + assert!(notices[0].is_user_visible() && !notices[0].is_agent_visible()); + let MessageContent::SystemNotification(note) = ¬ices[0].content[0] else { + unreachable!() + }; + assert!(note.msg.contains("'agent_drafter__create_app'")); + let stored = manager + .get_session(&session.id, true) + .await + .unwrap() + .conversation + .unwrap(); + let stored_notice = stored + .iter() + .find(|message| is_unrun_call_notice(message)) + .expect("the notice remains after reload"); + assert!(stored_notice.is_user_visible() && !stored_notice.is_agent_visible()); + let requests = chat_requests(&server).await; + assert_eq!(requests.len(), 2, "no corrective retry or tool dispatch"); + assert!(!requests[1].to_string().contains("Nothing ran")); + assert!(requests[1] + .to_string() + .contains("agent_drafter__create_app")); +} diff --git a/docs/apps-sdk/sdk-reference.md b/docs/apps-sdk/sdk-reference.md index bbd954c24..7f26c66b1 100644 --- a/docs/apps-sdk/sdk-reference.md +++ b/docs/apps-sdk/sdk-reference.md @@ -38,7 +38,7 @@ create_app → (edit src/main.ts + index.html) → build_app → launch_ap 1. **Create.** `create_app { title, description, archetype?, extensions?, skills?, knowledge_base?, capabilities?, … }`. Omitting `html`/`src/main.ts` seeds a working, lint-clean **archetype** starter (explorer / dashboard / workbench / wizard / canvas / chat) plus a matching declared `manifest.surface`. Default kind is `agentic`; pass `kind: "static"` for a plain page with no agent. An explicit `id` that is already taken is refused unless this chat created that app, or the call passes `replace: true` (see [deleting, re-creating and broken apps](#deleting-re-creating-and-broken-apps)). 2. **Edit.** `update_app { id, path?, content? | old_str+new_str }`. Editing anything under `src/` marks the bundle stale. Author your UI in `src/main.ts` (which `import`s `./sdk`) and your shell in `index.html`. 3. **Configure (optional).** `configure_app { id, system_prompt?, model?, extensions?, skills?, knowledge_base?, max_turns?, capabilities?, guardrails?, reliability?, orchestration?, output_type?, durable_session? }`. -4. **Build.** `build_app { id }` bundles `src/main.ts → dist/app.js` with esbuild, refreshes the vendored `src/sdk.ts`, stamps `manifest.sdk_hash`, and runs the lint harness. Fix any harness **ERROR** before launch or export. See [building and serving](#building-and-serving) for where esbuild comes from and when an app is rebuilt on its own. +4. **Build.** `build_app { id }` bundles `src/main.ts → dist/app.js` with esbuild, refreshes the vendored `src/sdk.ts`, and runs the lint harness and default browser-free page check. A check error fails the build; only a successful bundle receives a current `manifest.sdk_hash` stamp. Static pages without TypeScript are checked without creating a bundle. Fix the reported errors before launch or export. See [building and serving](#building-and-serving) for where esbuild comes from and when an app is rebuilt on its own. 5. **Launch.** `launch_app { id }` returns `http:///apps//`. That address carries no access cookie, so it is a name for the app rather than a way in. The desktop opens it as a live preview in the chat's artifact panel, on the window's own daemon whatever port the link names (a link from an earlier run names a port that is gone), through a launch link it mints itself. Only `http://127.0.0.1:/apps//`, the spelling the daemon writes, is taken for an app this way; a `localhost` link is left alone, because that is usually a web development server and the person's own project. The preview's **Open in browser** opens the app in the default browser the same way, and it is always the app the preview shows, whatever address the app's page has put in the toolbar. On Linux, a default browser packaged as a snap (stock Ubuntu's Firefox and Chromium) cannot read the page the desktop hands the link over in, so the desktop offers to copy a one-time link to paste into it instead. The CLI prints the URL, and `biorouter apps open ` opens the app from a terminal. 6. **Export.** `export_app { id, target_dir, replace?, mode?, include?, bundle_daemon? }` writes a folder that runs the app on another computer; the Applications view's **Export** does the same through the daemon. See the [export guide](#export-guide). @@ -90,9 +90,9 @@ Serving `GET /apps//` and `launch_app` rebuild an app first when its bundle - the fallback bundler built it and esbuild can run now; - it was built against a different App SDK than this daemon ships, unless the SDK it carries came from a newer Biorouter (`sdkv=`, or the builder's version when that is absent). A daemon never rebuilds an app down to its own older SDK, and when it rebuilds such an app for another reason it keeps the newer SDK and records where it came from, so two daemons of different versions serving one store no longer rebuild each app back and forth. -Builds of one app are serialised: a second request waits for the first, then finds the bundle fresh. Bundles and the refreshed `src/sdk.ts` are written to a temporary file and renamed into place, so a browser never loads half a bundle. A build that failed is remembered against its inputs (the files under `src/`, the bundle and the manifest stamps), so an app whose code does not compile is not rebuilt on every page load; any edit is a new attempt. +Builds of one app are serialised: a second request waits for the first, then finds the bundle fresh. Bundles and the refreshed `src/sdk.ts` are written to a temporary file and renamed into place, so a browser never loads half a bundle. A build that failed is remembered against its inputs (source files, page and asset metadata, the bundle and the manifest stamps), so an app whose code does not compile is not rebuilt on every page load. Fixing the page or adding a missing asset permits another attempt. -When an app that cannot run without a bundle (an agentic app, or one with a `src/main.ts`) has none and its build failed, `GET /apps//` answers `500` with a short page that shows the escaped build log and asks the person to have the agent fix the app and rebuild it. An app that still has an older bundle keeps serving it, and `launch_app` then returns the address with a warning that the browser gets the previous bundle, without the latest changes, and the build log. +When an app that cannot run without a bundle (an agentic app, or one with a `src/main.ts`) has none and its build failed, `GET /apps//` answers `500` with a short page that shows the escaped build log and asks the person to have the agent fix the app and rebuild it. An app that still has an older bundle can keep serving it if its current page passes the default check; `launch_app` then returns the address with a warning that the browser gets the previous bundle, without the latest changes, and the build log. A page-check error blocks serving and launch even when an older bundle exists. ### Static apps and inline scripts @@ -100,7 +100,7 @@ A static app is a page written in `index.html`. Inline `