diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 7d1b60778..ec10bb7a2 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,6 +1,6 @@ { "crates/rho": "1.33.1", - "crates/rho-sdk": "1.17.3", + "crates/rho-sdk": "1.18.0", "crates/rho-providers": "0.18.2", "crates/rho-tools": "0.14.0" } diff --git a/Cargo.lock b/Cargo.lock index 822eae0db..dfcbb3f2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4483,7 +4483,7 @@ dependencies = [ [[package]] name = "rho-sdk" -version = "1.17.3" +version = "1.18.0" dependencies = [ "pretty_assertions", "proptest", diff --git a/crates/rho-providers/Cargo.toml b/crates/rho-providers/Cargo.toml index 250bf2595..c2a6349bd 100644 --- a/crates/rho-providers/Cargo.toml +++ b/crates/rho-providers/Cargo.toml @@ -9,7 +9,7 @@ repository = "https://github.com/matthewyjiang/rho" readme = "README.md" [dependencies] -rho-sdk = { version = "1.17.3", path = "../rho-sdk" } +rho-sdk = { version = "1.18.0", path = "../rho-sdk" } tokio = { version = "1", features = ["full"] } reqwest = { version = "0.12", features = ["json", "stream"] } serde = { version = "1", features = ["derive"] } diff --git a/crates/rho-sdk/Cargo.toml b/crates/rho-sdk/Cargo.toml index 00a70bfc8..ddb0e2253 100644 --- a/crates/rho-sdk/Cargo.toml +++ b/crates/rho-sdk/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rho-sdk" -version = "1.17.3" +version = "1.18.0" edition = "2021" rust-version = "1.86" description = "Embeddable, headless agent runtime for Rho" diff --git a/crates/rho-sdk/src/session.rs b/crates/rho-sdk/src/session.rs index 95080b3ed..9ffed1f97 100644 --- a/crates/rho-sdk/src/session.rs +++ b/crates/rho-sdk/src/session.rs @@ -648,6 +648,16 @@ impl Session { self.core.commit(history) } + /// Replaces committed history while the session is idle. + /// + /// Hosts use this after durable persistence fails following + /// [`Self::append_message`], so model-visible history stays aligned with + /// storage instead of remaining one message ahead of a failed snapshot. + pub fn replace_history(&self, history: Vec) -> Result { + let _inactive = self.core.lock_inactive()?; + self.core.commit(history) + } + pub fn reset(&self) -> Result<(), Error> { let _inactive = self.core.lock_inactive()?; let system_prompt = match &self.core.runtime().system_prompt { diff --git a/crates/rho-sdk/src/session_tests.rs b/crates/rho-sdk/src/session_tests.rs index 29401a93b..c030dddfa 100644 --- a/crates/rho-sdk/src/session_tests.rs +++ b/crates/rho-sdk/src/session_tests.rs @@ -169,3 +169,23 @@ async fn stale_finalization_cannot_clear_a_newer_run_owner() { assert!(!session.is_running()); assert_eq!(session.state(), SessionState::Idle); } + +#[tokio::test] +async fn replace_history_restores_model_visible_state_after_a_failed_host_persist() { + let runtime = Rho::builder() + .provider(ScriptedProvider::new(identity(), [])) + .build() + .unwrap(); + let session = runtime.session(SessionOptions::default()).await.unwrap(); + let before = session.history(); + + session + .append_message(crate::model::Message::user_text( + "notice that never persisted", + )) + .unwrap(); + assert_eq!(session.history().len(), before.len() + 1); + + session.replace_history(before.clone()).unwrap(); + assert_eq!(session.history(), before); +} diff --git a/crates/rho-tools/Cargo.toml b/crates/rho-tools/Cargo.toml index 3052a67df..5c2c38365 100644 --- a/crates/rho-tools/Cargo.toml +++ b/crates/rho-tools/Cargo.toml @@ -15,7 +15,7 @@ document-pdf = ["dep:flate2", "dep:lopdf", "dep:pdf-inspector"] document-spreadsheets = ["dep:calamine", "dep:zip"] [dependencies] -rho-sdk = { version = "1.17.3", path = "../rho-sdk" } +rho-sdk = { version = "1.18.0", path = "../rho-sdk" } tokio = { version = "1", features = ["full"] } serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/crates/rho/Cargo.toml b/crates/rho/Cargo.toml index 157b4edad..b692aaf2e 100644 --- a/crates/rho/Cargo.toml +++ b/crates/rho/Cargo.toml @@ -22,7 +22,7 @@ name = "rho" path = "src/main.rs" [dependencies] -rho-sdk = { version = "1.17.3", path = "../rho-sdk" } +rho-sdk = { version = "1.18.0", path = "../rho-sdk" } rho-providers = { version = "0.18.2", path = "../rho-providers", default-features = false } rho-tools = { version = "0.14.0", path = "../rho-tools", package = "rho-agent-tools" } tokio = { version = "1", features = ["full"] } diff --git a/crates/rho/src/app/automation.rs b/crates/rho/src/app/automation.rs index f15fe5190..41a078f9e 100644 --- a/crates/rho/src/app/automation.rs +++ b/crates/rho/src/app/automation.rs @@ -504,7 +504,7 @@ async fn run_session_with_output( workspace, workspace_policy: AppPolicy::for_mode(startup.config.permission_mode), approval_session: startup.approval_session.clone(), - system_prompt: system_prompt.for_advisor_mode(tool_set.advisor_registered()), + system_prompt, reasoning: sdk_options.runtime.reasoning, service_tier: sdk_options.runtime.service_tier, compaction, diff --git a/crates/rho/src/app/config_repository.rs b/crates/rho/src/app/config_repository.rs index 4442944be..2b495ccf7 100644 --- a/crates/rho/src/app/config_repository.rs +++ b/crates/rho/src/app/config_repository.rs @@ -1,6 +1,9 @@ use std::path::PathBuf; #[cfg(test)] -use std::sync::Arc; +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, +}; use crate::config::Config; @@ -10,6 +13,11 @@ pub(crate) struct ConfigRepository { path: Option, #[cfg(test)] _temp_dir: Option>, + /// When set, the next [`Self::save`] fails once after load/mutation so + /// callers can exercise durable-save rollback without OS-specific FS locks. + /// Shared across clones so the request is not tied to an OS thread. + #[cfg(test)] + fail_next_save: Arc, } impl ConfigRepository { @@ -18,6 +26,8 @@ impl ConfigRepository { path, #[cfg(test)] _temp_dir: None, + #[cfg(test)] + fail_next_save: Arc::new(AtomicBool::new(false)), } } @@ -27,9 +37,17 @@ impl ConfigRepository { Ok(Self { path: Some(temp_dir.path().join("config.toml")), _temp_dir: Some(temp_dir), + fail_next_save: Arc::new(AtomicBool::new(false)), }) } + /// Fail this repository's next `save` once. Shared across clones; not + /// thread-local, so Tokio worker hops still observe the request. + #[cfg(test)] + pub(crate) fn fail_next_save_for_tests(&self) { + self.fail_next_save.store(true, Ordering::SeqCst); + } + pub(crate) fn configured_path(&self) -> anyhow::Result { self.path .clone() @@ -42,6 +60,12 @@ impl ConfigRepository { } pub(crate) fn save(&self, config: &Config) -> anyhow::Result<()> { + #[cfg(test)] + { + if self.fail_next_save.swap(false, Ordering::SeqCst) { + anyhow::bail!("injected config save failure"); + } + } config.save(self.path.clone()) } diff --git a/crates/rho/src/app/config_repository_tests.rs b/crates/rho/src/app/config_repository_tests.rs index 4793df129..6e773aef5 100644 --- a/crates/rho/src/app/config_repository_tests.rs +++ b/crates/rho/src/app/config_repository_tests.rs @@ -16,3 +16,33 @@ fn failed_save_does_not_return_the_update_value() { assert!(result.is_err()); } + +// Covers: injected save failure is stored on the repository and shared by clones +// Owner: config repository +#[test] +fn injected_save_failure_is_instance_scoped_and_shared_by_clones() { + let repository = ConfigRepository::temporary_for_tests().unwrap(); + let clone = repository.clone(); + repository.fail_next_save_for_tests(); + + let failed = clone + .update(|config| { + config.max_output_bytes = 42; + config.max_output_bytes + }) + .expect_err("clone must observe the injected save failure"); + assert!( + failed.to_string().contains("injected config save failure"), + "{failed}" + ); + + // One-shot: the next save on either handle succeeds. + let value = repository + .update(|config| { + config.max_output_bytes = 7; + config.max_output_bytes + }) + .expect("injection is consumed after one save"); + assert_eq!(value, 7); + assert_eq!(repository.load().unwrap().max_output_bytes, 7); +} diff --git a/crates/rho/src/app/interactive_runtime.rs b/crates/rho/src/app/interactive_runtime.rs index 753f3a7bb..6f98135b8 100644 --- a/crates/rho/src/app/interactive_runtime.rs +++ b/crates/rho/src/app/interactive_runtime.rs @@ -15,6 +15,8 @@ use { #[path = "interactive_runtime_advisor.rs"] mod advisor; +#[path = "interactive_runtime_edit_tool.rs"] +pub(crate) mod edit_tool; #[path = "interactive_runtime_hooks.rs"] mod session_hooks; #[path = "interactive_runtime_startup.rs"] @@ -29,7 +31,6 @@ use super::{ policy::AppPolicy, provider_controller::ProviderController, runtime_builder::{build_compaction, build_runtime, RuntimeBuildOptions}, - tools_prompt::SystemPromptVariants, }; pub(crate) use super::interactive_run_controller::{ @@ -72,7 +73,7 @@ pub(crate) struct InteractiveRuntime { mcp_report: crate::tools::mcp::McpSessionReport, plugins_report: crate::plugins::PluginLoadReport, workspace: Workspace, - system_prompt: SystemPromptVariants, + system_prompt: rho_sdk::SystemPrompt, compaction: CompactionConfig, context_window: Option, usage_recording: rho_sdk::ProviderRequestUsageRecording, @@ -713,13 +714,45 @@ impl InteractiveRuntime { model: String, display: String, ) -> anyhow::Result<()> { - self.sessions - .session() - .append_message(Message::user_text(model))?; - self.sessions - .save_snapshot(&[Message::user_text(display)])?; - self.refresh_context_usage(); - Ok(()) + let session = self.sessions.session(); + let history_before = session.history(); + session.append_message(Message::user_text(model))?; + + let save_result = { + #[cfg(test)] + { + if advisor::take_fail_next_advisor_notice_snapshot_save_for_tests() { + Err(anyhow::anyhow!( + "injected advisor switch notice snapshot save failure" + )) + } else { + self.sessions.save_snapshot(&[Message::user_text(display)]) + } + } + #[cfg(not(test))] + { + self.sessions.save_snapshot(&[Message::user_text(display)]) + } + }; + + match save_result { + Ok(()) => { + self.refresh_context_usage(); + Ok(()) + } + Err(error) => { + // Append already advanced model-visible history. Roll it back so a + // failed durable write cannot leave the live session describing a + // notice the host never persisted. + if let Err(rollback_error) = self.sessions.session().replace_history(history_before) + { + return Err(error.context(format!( + "failed to roll back live history after snapshot save failure: {rollback_error}" + ))); + } + Err(error) + } + } } pub(crate) async fn shutdown(&mut self) { @@ -889,3 +922,11 @@ impl InteractiveRuntime { #[cfg(test)] #[path = "interactive_runtime_tests.rs"] mod tests; + +/// Test factory for TUI seams that need a live edit-capable runtime. +#[cfg(test)] +pub(crate) async fn test_edit_tool_runtime( + edit_tool: crate::config::EditTool, +) -> InteractiveRuntime { + tests::edit_tool_runtime(edit_tool).await +} diff --git a/crates/rho/src/app/interactive_runtime_advisor.rs b/crates/rho/src/app/interactive_runtime_advisor.rs index 27fb2806d..a75c982fb 100644 --- a/crates/rho/src/app/interactive_runtime_advisor.rs +++ b/crates/rho/src/app/interactive_runtime_advisor.rs @@ -1,10 +1,13 @@ //! Advisor mode as a runtime state transition. //! -//! Advisor mode changes the tool list and the system prompt, neither of which -//! the SDK can swap on a live runtime. Turning it on or off therefore rebuilds -//! the runtime and rebinds the session, the same move a permission-mode change -//! makes, so the change lands on the next turn and the session ID and history -//! survive it. +//! Advisor mode changes the advertised tool list, which the SDK cannot swap on +//! a live runtime. Turning it on or off therefore rebuilds the runtime and +//! rebinds the session so the change lands on the next turn. The session ID and +//! history survive it. +//! +//! The system prompt stays fixed for prompt-cache stability. The model learns +//! about the tool list change from an appended context notice (with the tool +//! schema when enabling) rather than a rewritten system prompt. use std::sync::Arc; @@ -19,11 +22,31 @@ use super::super::{ use super::InteractiveRuntime; +#[cfg(test)] +thread_local! { + /// When set, the next advisor notice appends model-visible history, then + /// fails snapshot persistence so rollback must cover the partial commit. + static FAIL_NEXT_ADVISOR_NOTICE_SNAPSHOT_SAVE: std::cell::Cell = + const { std::cell::Cell::new(false) }; +} + +#[cfg(test)] +pub(crate) fn fail_next_advisor_switch_notice_for_tests() { + FAIL_NEXT_ADVISOR_NOTICE_SNAPSHOT_SAVE.with(|flag| flag.set(true)); +} + +#[cfg(test)] +pub(super) fn take_fail_next_advisor_notice_snapshot_save_for_tests() -> bool { + FAIL_NEXT_ADVISOR_NOTICE_SNAPSHOT_SAVE.with(|flag| flag.replace(false)) +} + impl InteractiveRuntime { - /// The system prompt for the tools this run currently offers. + /// Fixed system prompt for this session. + /// + /// Mid-session tool list changes keep this value stable and tell the model + /// through appended context instead. pub(super) fn active_system_prompt(&self) -> SystemPrompt { - self.system_prompt - .for_advisor_mode(self.tools.advisor_registered()) + self.system_prompt.clone() } /// Applies an advisor mode or advisor model change to the next turn. @@ -31,20 +54,20 @@ impl InteractiveRuntime { /// `model` is the advisor model to use, or `None` when advisor mode is off /// or has no model yet; those are the same thing to the executor. The live /// tool reads the new model at once. Registering or removing the `advisor` - /// tool also changes the tool list and the system prompt, so it needs the - /// same runtime rebuild as a permission-mode change; the session ID and - /// history survive it. + /// tool rebuilds the runtime without rewriting the system prompt, then + /// appends a context notice. Returns display text for a transcript notice + /// when the tool list changed. pub(crate) async fn set_advisor( &mut self, model: Option, - ) -> anyhow::Result<()> { + ) -> anyhow::Result> { let Some(store) = self.tools.advisor().cloned() else { - return Ok(()); + return Ok(None); }; let registered = model.is_some(); if registered == self.tools.advisor_registered() { store.set_model(model); - return Ok(()); + return Ok(None); } if self.runs.is_active() { anyhow::bail!("advisor mode cannot change while a run is active"); @@ -52,23 +75,63 @@ impl InteractiveRuntime { // The model lands only after the rebuild succeeds, so a failed // transition leaves both the tool list and the store untouched. + let previous_registered = self.tools.advisor_registered(); + let previous_model = store.model(); + let history_before = self.sessions.history(); self.tools.set_advisor_registered(registered); match self.rebind_current_session().await { Ok(()) => { store.set_model(model); - Ok(()) + match self.append_advisor_switch_notice(registered) { + Ok(display) => Ok(Some(display)), + Err(error) => { + // Mirror edit-tool: a notice failure must not leave the + // session advertising a tool list the model was never + // told about. Also restore model-visible history when a + // partial append-before-save left a notice in place. + store.set_model(previous_model); + self.tools.set_advisor_registered(previous_registered); + if self.sessions.history() != history_before { + let _ = self.sessions.session().replace_history(history_before); + } + let _ = self.rebind_current_session().await; + Err(error) + } + } } Err(error) => { - self.tools.set_advisor_registered(!registered); + self.tools.set_advisor_registered(previous_registered); Err(error) } } } + fn append_advisor_switch_notice(&mut self, enabled: bool) -> anyhow::Result { + let (model, display) = if enabled { + let spec = self + .tools + .specs() + .into_iter() + .find(|spec| spec.name == crate::tools::advisor::TOOL_NAME) + .ok_or_else(|| { + anyhow::anyhow!("advisor tool is missing after it was registered") + })?; + crate::prompt::advisor_enabled_context(&spec) + } else { + crate::prompt::advisor_disabled_context() + }; + self.append_user_context_with_display(model, display.clone())?; + Ok(display) + } + /// Rebuilds the SDK runtime around the current tools and prompt, then /// rebinds the live session onto it. The live runtime is replaced only /// after the replacement is ready, so a failure leaves the session intact. - async fn rebind_current_session(&mut self) -> anyhow::Result<()> { + /// + /// Callers that change the advertised tool list should keep the system + /// prompt fixed for prompt-cache stability and tell the model about the + /// change with an appended context message instead. + pub(super) async fn rebind_current_session(&mut self) -> anyhow::Result<()> { let snapshot = self.sessions.session().snapshot(); let replacement_runtime = build_runtime(RuntimeBuildOptions { provider: Arc::clone(self.provider.provider()), diff --git a/crates/rho/src/app/interactive_runtime_edit_tool.rs b/crates/rho/src/app/interactive_runtime_edit_tool.rs new file mode 100644 index 000000000..29aebe451 --- /dev/null +++ b/crates/rho/src/app/interactive_runtime_edit_tool.rs @@ -0,0 +1,89 @@ +//! File edit tool as a runtime state transition. +//! +//! Swapping the advertised edit surface changes the tool list, which the SDK +//! cannot hot-swap on a live runtime. The change rebuilds the runtime and +//! rebinds the session so it lands on the next turn. Session ID and history +//! survive it. +//! +//! The system prompt stays fixed and format-agnostic. The model learns about the +//! new surface from an appended context notice that carries the live schema. + +use super::InteractiveRuntime; + +/// Result of a successful mid-session edit-tool switch. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct EditToolChange { + pub(crate) previous: rho_tools::EditFormat, + pub(crate) display: String, +} + +impl InteractiveRuntime { + /// Swaps the advertised file edit tool for the next turn. + /// + /// Keeps the system prompt fixed for prompt-cache stability, rebuilds the + /// runtime tool list, and appends a model-facing schema notice when the + /// surface actually changes. Returns [`None`] when this run has no edit tool + /// or the selection is already active. + pub(crate) async fn set_edit_tool( + &mut self, + edit_tool: rho_tools::EditFormat, + max_output_bytes: usize, + ) -> anyhow::Result> { + if self.runs.is_active() { + anyhow::bail!("edit tool cannot change while a run is active"); + } + let Some(previous) = self.tools.set_edit_tool(edit_tool, max_output_bytes) else { + return Ok(None); + }; + if let Err(error) = self.rebind_current_session().await { + let _ = self.tools.set_edit_tool(previous, max_output_bytes); + return Err(error); + } + match self.append_edit_tool_switch_notice(previous, edit_tool) { + Ok(display) => Ok(Some(EditToolChange { previous, display })), + Err(error) => { + // Restore so a notice failure does not leave the session + // advertising a tool the model was never told about. Surface + // rollback failures instead of dropping them. + if self + .tools + .set_edit_tool(previous, max_output_bytes) + .is_none() + { + return Err(anyhow::anyhow!( + "{error}; rollback failed: could not restore previous edit tool" + )); + } + if let Err(rebind_error) = self.rebind_current_session().await { + return Err(anyhow::anyhow!("{error}; rollback failed: {rebind_error}")); + } + Err(error) + } + } + } + + fn append_edit_tool_switch_notice( + &mut self, + previous: rho_tools::EditFormat, + current: rho_tools::EditFormat, + ) -> anyhow::Result { + let spec = self + .tools + .specs() + .into_iter() + .find(|spec| spec.name == current.tool_name()) + .ok_or_else(|| { + anyhow::anyhow!( + "edit tool `{}` is missing after the mid-session switch", + current.tool_name() + ) + })?; + let (model, display) = crate::prompt::edit_tool_switch_context(previous, current, &spec); + self.append_user_context_with_display(model, display.clone())?; + Ok(display) + } + + pub(crate) fn tool_specs(&self) -> Vec { + self.tools.specs() + } +} diff --git a/crates/rho/src/app/interactive_runtime_startup.rs b/crates/rho/src/app/interactive_runtime_startup.rs index e314740e2..ebe53ae13 100644 --- a/crates/rho/src/app/interactive_runtime_startup.rs +++ b/crates/rho/src/app/interactive_runtime_startup.rs @@ -101,7 +101,7 @@ pub(super) async fn initialize( approval_session: approval_handler .clone() .map(rho_sdk::ApprovalSession::from_shared), - system_prompt: system_prompt.for_advisor_mode(tools.advisor_registered()), + system_prompt: system_prompt.clone(), reasoning: sdk_options.runtime.reasoning, service_tier: sdk_options.runtime.service_tier, compaction: compaction.clone(), diff --git a/crates/rho/src/app/interactive_runtime_tests.rs b/crates/rho/src/app/interactive_runtime_tests.rs index dc9f1b017..50c752564 100644 --- a/crates/rho/src/app/interactive_runtime_tests.rs +++ b/crates/rho/src/app/interactive_runtime_tests.rs @@ -208,7 +208,7 @@ async fn test_runtime(turns: Vec) -> InteractiveRuntime { mcp_report: Default::default(), plugins_report: Default::default(), workspace, - system_prompt: super::SystemPromptVariants::uniform(SystemPrompt::None), + system_prompt: SystemPrompt::None, compaction: CompactionConfig::default(), context_window: None, usage_recording: Default::default(), @@ -638,12 +638,15 @@ fn advisor_model() -> crate::config::InternalAgentModelConfig { } // Covers: toggling advisor mode mid-session must add and remove the advisor tool -// for the next turn while the session ID and history survive the rebuild. +// for the next turn without rewriting the system prompt, while the session ID +// survives and a model-facing schema notice is appended. // Owner: interactive runtime advisor state transition. #[tokio::test] async fn advisor_mode_changes_the_tool_list_without_replacing_the_session() { let mut interactive = advisor_test_runtime().await; let session_id = interactive.sessions.session().id().clone(); + let history_before = interactive.history().len(); + let system_before = interactive.system_prompt.clone(); let advertised = |interactive: &InteractiveRuntime| { interactive .runtime @@ -655,18 +658,212 @@ async fn advisor_mode_changes_the_tool_list_without_replacing_the_session() { assert!(!interactive.tools.advisor_registered()); - interactive + let enabled = interactive .set_advisor(Some(advisor_model())) .await .unwrap(); - + assert_eq!(enabled.as_deref(), Some("advisor mode on")); assert!(interactive.tools.advisor_registered()); assert!(advertised(&interactive)); assert_eq!(interactive.sessions.session().id(), &session_id); + assert_eq!(interactive.system_prompt, system_before); + assert_eq!(interactive.history().len(), history_before + 1); + let enabled_notice = interactive.history().last().expect("enable notice").clone(); + let Message::User(blocks) = &enabled_notice else { + panic!("expected user notice, got {enabled_notice:?}"); + }; + let enabled_text = blocks + .iter() + .filter_map(|block| match block { + ContentBlock::Text(text) => Some(text.as_str()), + _ => None, + }) + .collect::(); + assert!(enabled_text.contains("[advisor mode on]")); + assert!(enabled_text.contains("input_schema:")); + assert!(enabled_text.contains("`advisor`")); + + let disabled = interactive.set_advisor(None).await.unwrap(); + assert_eq!(disabled.as_deref(), Some("advisor mode off")); + assert!(!interactive.tools.advisor_registered()); + assert!(!advertised(&interactive)); + assert_eq!(interactive.sessions.session().id(), &session_id); + assert_eq!(interactive.system_prompt, system_before); + assert_eq!(interactive.history().len(), history_before + 2); + let disabled_notice = interactive + .history() + .last() + .expect("disable notice") + .clone(); + let Message::User(blocks) = &disabled_notice else { + panic!("expected user notice, got {disabled_notice:?}"); + }; + let disabled_text = blocks + .iter() + .filter_map(|block| match block { + ContentBlock::Text(text) => Some(text.as_str()), + _ => None, + }) + .collect::(); + assert!(disabled_text.contains("[advisor mode off]")); + assert!(disabled_text.contains("no longer available")); +} + +// Covers: append-success / snapshot-save-failure after a successful rebuild must +// restore previous advisor registration, model, and model-visible history. +// Owner: interactive runtime advisor state transition. +#[tokio::test] +async fn advisor_notice_failure_restores_previous_registration_and_model() { + let mut interactive = advisor_test_runtime().await; + let store = interactive.tools.advisor().cloned().expect("advisor store"); + assert!(!interactive.tools.advisor_registered()); + assert!(store.model().is_none()); + let history_before = interactive.history(); + + super::advisor::fail_next_advisor_switch_notice_for_tests(); + let error = interactive + .set_advisor(Some(advisor_model())) + .await + .expect_err("notice failure should abort the transition"); + assert!( + error + .to_string() + .contains("injected advisor switch notice snapshot save failure"), + "unexpected error: {error}" + ); + assert!( + !interactive.tools.advisor_registered(), + "registration must roll back" + ); + assert!(store.model().is_none(), "model must roll back"); + assert_eq!( + interactive.history(), + history_before, + "model-visible history must not keep a notice that never persisted" + ); + assert!( + !interactive + .runtime + .diagnostics() + .tools() + .iter() + .any(|tool| tool.name() == "advisor"), + "runtime must not advertise advisor after rollback" + ); +} - interactive.set_advisor(None).await.unwrap(); +// Covers: advisor mode cannot change while a provider run is active. +// Owner: interactive runtime advisor state transition. +#[tokio::test] +async fn advisor_mode_rejects_change_while_a_run_is_active() { + let mut interactive = pending_compaction_runtime("still going").await; + let config = Config::default(); + interactive.tools = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(AgentCapabilities::new( + [ToolCapability::Advisor].into_iter().collect(), + )) + .advisor(crate::tools::advisor::AdvisorSessionStore::new()), + ); + interactive + .start(UserInput::text("keep running"), None) + .await + .unwrap(); + assert!(interactive.is_run_active()); + let error = interactive + .set_advisor(Some(advisor_model())) + .await + .expect_err("active run must block advisor transitions"); + assert!( + error + .to_string() + .contains("cannot change while a run is active"), + "unexpected error: {error}" + ); assert!(!interactive.tools.advisor_registered()); - assert!(!advertised(&interactive)); + interactive.shutdown().await; +} + +async fn edit_tool_test_runtime() -> InteractiveRuntime { + edit_tool_runtime(crate::config::EditTool::Pinned( + rho_tools::EditFormat::Hashline, + )) + .await +} + +/// Shared factory for TUI tests that exercise Auto edit-tool handoff. +pub(super) async fn edit_tool_runtime(edit_tool: crate::config::EditTool) -> InteractiveRuntime { + let mut interactive = pending_compaction_runtime("done").await; + let config = Config { + edit_tool, + ..Config::default() + }; + interactive.tools = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(AgentCapabilities::new( + [ToolCapability::Edit, ToolCapability::ReadFile] + .into_iter() + .collect(), + )), + ); + interactive +} + +// Covers: /config edit-tool selection must swap the advertised edit surface for +// the next turn without rewriting the system prompt, while the session ID +// survives and a model-facing schema notice is appended. +// Owner: interactive runtime edit-tool state transition. +#[tokio::test] +async fn edit_tool_switch_rebuilds_tools_and_appends_schema_notice() { + let mut interactive = edit_tool_test_runtime().await; + let session_id = interactive.sessions.session().id().clone(); + let history_before = interactive.history().len(); + let system_before = interactive.system_prompt.clone(); + let advertised = |interactive: &InteractiveRuntime, name: &str| { + interactive + .runtime + .diagnostics() + .tools() + .iter() + .any(|tool| tool.name() == name) + }; + + assert!(interactive.tools.contains("edit")); + assert!(!interactive.tools.contains("str_replace")); + + let change = interactive + .set_edit_tool( + rho_tools::EditFormat::StrReplace, + Config::default().max_output_bytes, + ) + .await + .unwrap() + .expect("edit tool should change"); + assert_eq!(change.previous, rho_tools::EditFormat::Hashline); + assert_eq!(change.display, "edit tool switched to str_replace"); + assert_eq!(interactive.system_prompt, system_before); + assert!(interactive.tools.contains("str_replace")); + assert!(!interactive.tools.contains("edit")); + assert!(advertised(&interactive, "str_replace")); + assert!(!advertised(&interactive, "edit")); assert_eq!(interactive.sessions.session().id(), &session_id); + assert_eq!(interactive.history().len(), history_before + 1); + let notice = interactive.history().last().expect("switch notice").clone(); + let Message::User(blocks) = ¬ice else { + panic!("expected user notice, got {notice:?}"); + }; + let text = blocks + .iter() + .filter_map(|block| match block { + ContentBlock::Text(text) => Some(text.as_str()), + _ => None, + }) + .collect::(); + assert!(text.contains("[edit tool switched]")); + assert!(text.contains("`str_replace`")); + assert!(text.contains("input_schema:")); + assert!(!text.contains("restart")); } diff --git a/crates/rho/src/app/tools_prompt.rs b/crates/rho/src/app/tools_prompt.rs index 8676294a9..813f70fe0 100644 --- a/crates/rho/src/app/tools_prompt.rs +++ b/crates/rho/src/app/tools_prompt.rs @@ -50,7 +50,9 @@ pub(crate) struct StartupInventory { pub(crate) struct ToolsAndPrompt { pub(crate) tools: AppToolSet, - pub(crate) system_prompt: SystemPromptVariants, + /// Fixed for the session so prompt cache stays stable across mid-session + /// tool-list changes (advisor / edit tool). Those changes use context notices. + pub(crate) system_prompt: SystemPrompt, pub(crate) inventory: StartupInventory, /// Late-bound model handle for MCP sampling. Bound once the runtime exists, /// and rebound whenever the user changes models. Left unbound, every @@ -58,36 +60,6 @@ pub(crate) struct ToolsAndPrompt { pub(crate) mcp_sampling: crate::tools::mcp::McpSamplingBridge, } -/// The system prompt with and without the advisor steering text. -/// -/// Advisor mode turns on and off mid-session, and the prompt must never -/// describe a tool the run does not have, so both forms are built once and one -/// is selected on every runtime build. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct SystemPromptVariants { - without_advisor: SystemPrompt, - with_advisor: SystemPrompt, -} - -impl SystemPromptVariants { - /// One prompt for both modes, for runs whose prompt cannot carry advisor - /// steering. - pub(crate) fn uniform(prompt: SystemPrompt) -> Self { - Self { - without_advisor: prompt.clone(), - with_advisor: prompt, - } - } - - pub(crate) fn for_advisor_mode(&self, enabled: bool) -> SystemPrompt { - if enabled { - self.with_advisor.clone() - } else { - self.without_advisor.clone() - } - } -} - /// Capability resolution plus system prompt assembly for root interactive and /// automation startup. Claude-cli agents bind no Rho host tools; root runs still /// use the Rho loop and parent config, with Claude execution via AgentExecutor @@ -183,10 +155,10 @@ pub(crate) async fn assemble_tools_and_prompt( let specs = tools.specs(); let system_prompt = if options.no_system_prompt { options.diagnostics.update_prompt_sources(Vec::new()); - SystemPromptVariants::uniform(SystemPrompt::None) + SystemPrompt::None } else { - let (mut text, mut advisor_text) = match options.agent.prompt() { - PromptPolicy::Replace(text) => (text.clone(), text.clone()), + let mut text = match options.agent.prompt() { + PromptPolicy::Replace(text) => text.clone(), PromptPolicy::Extend(extra) => { let mut built = prompt::system_prompt_with_plugin_skills(&specs, options.cwd, plugin_skills); @@ -194,38 +166,32 @@ pub(crate) async fn assemble_tools_and_prompt( if !launch_delegation_enabled { prompt::append_subagents_disabled_instruction(&mut built.text); } - // Server guidance describes the MCP tools this run actually has, - // so it belongs in both prompt variants. + // Server guidance describes the MCP tools this run actually has. let mcp_instructions = mcp_report .servers .iter() .filter_map(|server| Some((server.identity.as_str(), server.instructions()?))) .collect::>(); prompt::append_mcp_instructions(&mut built.text, mcp_instructions.iter().copied()); - let mut advisor_text = built.text.clone(); - prompt::append_advisor_instruction(&mut advisor_text); if !extra.is_empty() { - let instructions = format!("\n\n# Agent instructions\n\n{extra}"); - built.text.push_str(&instructions); - advisor_text.push_str(&instructions); + built + .text + .push_str(&format!("\n\n# Agent instructions\n\n{extra}")); } - (built.text, advisor_text) + built.text } }; if text.is_empty() { text = "You are a coding agent.".into(); - advisor_text = text.clone(); - } - SystemPromptVariants { - without_advisor: SystemPrompt::Custom(text), - with_advisor: SystemPrompt::Custom(advisor_text), } + // Advisor steering lives on the tool description / enable notice, not + // here, so mid-session /advisor toggles never require a prompt rewrite. + SystemPrompt::Custom(text) }; if let Some(store) = tools.advisor() { - // The advisor reviews what the executor was told, and it only ever runs - // while advisor mode is on, so it reads the advisor variant. - store.bind_system_prompt(match system_prompt.for_advisor_mode(true) { - SystemPrompt::Custom(text) => Some(text), + // The advisor reviews what the executor was told. + store.bind_system_prompt(match &system_prompt { + SystemPrompt::Custom(text) => Some(text.clone()), // `SystemPrompt` is non-exhaustive; only custom text is reviewable. _ => None, }); diff --git a/crates/rho/src/app/tools_prompt_tests.rs b/crates/rho/src/app/tools_prompt_tests.rs index abfffd845..072f4a62c 100644 --- a/crates/rho/src/app/tools_prompt_tests.rs +++ b/crates/rho/src/app/tools_prompt_tests.rs @@ -79,7 +79,7 @@ async fn assemble(config: &Config, cwd: &std::path::Path) -> (bool, String) { let tools = assembled.tools; let prompt = assembled.system_prompt; let registered = tools.advisor_registered(); - let text = match prompt.for_advisor_mode(registered) { + let text = match prompt { SystemPrompt::Custom(text) => text, SystemPrompt::None => String::new(), _ => String::new(), @@ -88,7 +88,7 @@ async fn assemble(config: &Config, cwd: &std::path::Path) -> (bool, String) { } // Covers: the advisor tool must appear only when advisor mode is on and an -// advisor model is configured, and the steering text must track the tool. +// advisor model is configured. Steering stays off the system prompt. // Owner: root tool/prompt assembly. #[tokio::test] async fn the_advisor_tool_needs_both_the_mode_and_a_model() { @@ -109,10 +109,9 @@ async fn the_advisor_tool_needs_both_the_mode_and_a_model() { registered, expected, "advisor_mode={advisor_mode} with_model={with_model}" ); - assert_eq!( - prompt.contains("You have access to an `advisor` tool"), - expected, - "steering text for advisor_mode={advisor_mode} with_model={with_model}" + assert!( + !prompt.contains("Call advisor BEFORE substantive work"), + "system prompt must stay advisor-agnostic; advisor_mode={advisor_mode} with_model={with_model}" ); } } @@ -145,20 +144,19 @@ async fn the_advisor_receives_the_executor_system_prompt() { let tools = assembled.tools; let prompt = assembled.system_prompt; - let SystemPrompt::Custom(text) = prompt.for_advisor_mode(true) else { + let SystemPrompt::Custom(text) = prompt else { panic!("expected a custom system prompt"); }; let store = tools.advisor().expect("advisor store"); assert_eq!(store.system_prompt(), Some(text)); } -// Covers: both prompt forms are built once so a mid-session /advisor toggle can -// swap them, and the executor is never told about a tool it does not have. +// Covers: the executor system prompt is a single form that does not encode +// advisor registration. Mid-session toggles must not rely on swapping prompts. // Owner: root tool/prompt assembly. #[tokio::test] -async fn both_prompt_variants_are_available_whatever_the_saved_mode_is() { +async fn system_prompt_stays_advisor_agnostic() { let cwd = tempfile::tempdir().unwrap(); - let steering = "You have access to an `advisor` tool"; for advisor_mode in [false, true] { let config = advisor_config(advisor_mode, /*with_model*/ true); @@ -183,18 +181,17 @@ async fn both_prompt_variants_are_available_whatever_the_saved_mode_is() { .unwrap() .system_prompt; - let text = |enabled| match prompt.for_advisor_mode(enabled) { + let text = match prompt { SystemPrompt::Custom(text) => text, SystemPrompt::None => String::new(), _ => String::new(), }; - - assert_eq!( - ( - text(/*enabled*/ true).contains(steering), - text(/*enabled*/ false).contains(steering) - ), - (true, false), + assert!( + !text.contains("Call advisor BEFORE substantive work"), + "advisor_mode={advisor_mode}" + ); + assert!( + !text.contains("You have access to an `advisor` tool"), "advisor_mode={advisor_mode}" ); } diff --git a/crates/rho/src/builtin_skills/rho-config/SKILL.md b/crates/rho/src/builtin_skills/rho-config/SKILL.md index 24a039fe1..bd3389546 100644 --- a/crates/rho/src/builtin_skills/rho-config/SKILL.md +++ b/crates/rho/src/builtin_skills/rho-config/SKILL.md @@ -57,11 +57,11 @@ Use the read-only `rho` tool with action `config` to see the sanitized live conf - **Permission mode**: `permission_mode` must be `auto`, `plan`, or `supervised`. Set it under Agent behavior in `/config`, or in config. `auto` allows, `plan` denies file writes and process execution, `supervised` asks before file writes and process execution. The change applies before the next turn and clears session approvals. - **Auto compaction**: under Context & limits in `/config`. `compact_target_percent` must stay below `compact_threshold_percent`; values at or above the threshold are clamped. - **Web search**: under Tools in `/config`. `hosted` enables provider-hosted search; `provider` selects the backup backend (`auto`, `openai`, `exa`, `brave`, `disabled`). Set `hosted` to `false` and `provider` to `disabled` to disable search entirely. -- **Edit tool**: under Tools in `/config`, choose `hashline`, `apply_patch`, or `str_replace`. Exactly one edit schema is exposed under its own tool name (`edit`, `apply_patch`, or `str_replace`). The saved choice applies on the next Rho startup, so restart Rho after changing it. +- **Edit tool**: under Tools in `/config`, choose `auto`, `hashline`, `apply_patch`, or `str_replace`. Exactly one edit schema is exposed under its own tool name (`edit`, `apply_patch`, or `str_replace`). `auto` keeps that preference in config and advertises the preferred format for the active provider, switching live when the provider changes. Prefer `auto` so models get the edit tool their first-party harness trained them on (Codex → `apply_patch`, Anthropic/xAI → `str_replace`, otherwise Rho `hashline`). Pinned formats stay fixed. The change applies before the next turn: the tool list rebuilds and the session gets a short notice with the new tool schema. It cannot change while a model turn is running. - **Prompt templates**: add a file under `~/.rho/prompts/` or `/.rho/prompts/`, or define `[prompt_templates]` inline in config. The filename or key becomes the slash command. Restart rho after adding or editing templates. - **Global instructions**: to change rules that apply to every session, edit `~/.rho/AGENTS.md` (see above). - **Keybindings**: edit `[keybindings]` in config. Values use `+`-separated modifiers and keys. Keybinding changes take effect at startup. ## Applying a change -State which mechanism you recommend and, when relevant, when it takes effect. Settings that apply to the current session or the next turn need no restart, including `advisor_mode`; `enable_subagents`, web search hosted state, templates, and keybindings apply on the next session or at startup. When the user edits config directly, tell them a restart may be required and offer to check whether the setting is restart-only. +State which mechanism you recommend and, when relevant, when it takes effect. Settings that apply to the current session or the next turn need no restart, including `advisor_mode` and `edit_tool`; `enable_subagents`, web search hosted state, templates, and keybindings apply on the next session or at startup. When the user edits config directly, tell them a restart may be required and offer to check whether the setting is restart-only. diff --git a/crates/rho/src/config.rs b/crates/rho/src/config.rs index bdf07a60f..20e62b051 100644 --- a/crates/rho/src/config.rs +++ b/crates/rho/src/config.rs @@ -81,7 +81,9 @@ pub struct Config { pub web_search_hosted: bool, /// Client-side backup backend used when hosted search is off or unsupported. pub web_search_provider: SearchProvider, - /// Selects the one built-in file edit tool exposed to models. + /// Selects the preferred built-in file edit tool exposed to models. + /// + /// [`EditTool::Auto`] resolves a concrete format from the active provider. pub edit_tool: EditTool, pub check_for_updates: bool, pub enable_subagents: bool, @@ -260,8 +262,148 @@ impl<'de> Deserialize<'de> for SearchProvider { } } -/// Built-in file edit surface exposed to models. -pub type EditTool = rho_tools::EditFormat; +/// Configured file-edit preference. +/// +/// [`Self::Auto`] picks a built-in preferred format for the active chat +/// provider. [`Self::Pinned`] freezes one [`rho_tools::EditFormat`] across +/// provider changes. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +#[non_exhaustive] +pub enum EditTool { + /// Prefer the built-in format for the active provider. + #[default] + Auto, + /// Pin one concrete model-facing edit format. + Pinned(rho_tools::EditFormat), +} + +impl EditTool { + /// Every supported preference, in UI display order. + /// + /// Pinned variants track [`rho_tools::EditFormat::ALL`] so newly added + /// formats appear here without a second hard-coded list. + pub fn all() -> Vec { + let mut all = Vec::with_capacity(1 + rho_tools::EditFormat::ALL.len()); + all.push(Self::Auto); + all.extend(rho_tools::EditFormat::ALL.iter().copied().map(Self::Pinned)); + all + } + + /// Configured value and selector label (`behavior.edit_tool`). + pub const fn as_str(self) -> &'static str { + match self { + Self::Auto => "auto", + Self::Pinned(format) => format.as_str(), + } + } + + /// Short human-readable preference label. + pub const fn label(self) -> &'static str { + self.as_str() + } + + /// Detail shown when selecting an edit preference. + pub fn detail(self) -> &'static str { + match self { + Self::Auto => { + "Pick the preferred format for the active provider and switch when the provider changes." + } + Self::Pinned(format) => match format { + rho_tools::EditFormat::Hashline => { + "Always expose `edit` with snapshot tags and line-anchored PUT/CUT operations." + } + rho_tools::EditFormat::ApplyPatch => { + "Always expose `apply_patch` with a Codex-style multi-file patch document." + } + rho_tools::EditFormat::StrReplace => { + "Always expose `str_replace` with exact old_string/new_string replacement." + } + _ => "Always expose this pinned file edit format.", + }, + } + } + + /// Resolves the model-facing format for `provider`. + pub fn resolve(self, provider: &str) -> rho_tools::EditFormat { + match self { + Self::Auto => preferred_edit_format_for_provider(provider), + Self::Pinned(format) => format, + } + } + + /// Label for config rows, including the resolved format when Auto. + pub fn display_label(self, provider: &str) -> String { + match self { + Self::Auto => format!("auto ({})", self.resolve(provider).label()), + Self::Pinned(format) => format.label().into(), + } + } +} + +/// Built-in preferred edit format for a chat provider. +/// +/// Product defaults, not user config. Many models train inside a first-party +/// harness that supplies one edit tool, so Auto picks that familiar surface. +/// Pin a concrete [`EditTool`] when a session should ignore this table. +/// Unknown providers fall back to hashline. +pub fn preferred_edit_format_for_provider(provider: &str) -> rho_tools::EditFormat { + match provider { + // Codex harness trains on apply_patch documents. + "openai-codex" => rho_tools::EditFormat::ApplyPatch, + // Claude Code and xAI first-party agent tooling train on string replace. + "anthropic" | "xai" => rho_tools::EditFormat::StrReplace, + // Rho default when no first-party match is known. + _ => rho_tools::EditFormat::Hashline, + } +} + +impl fmt::Display for EditTool { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.as_str()) + } +} + +impl FromStr for EditTool { + type Err = String; + + fn from_str(value: &str) -> Result { + let normalized = value.trim().to_ascii_lowercase(); + if normalized == "auto" { + return Ok(Self::Auto); + } + if let Some(format) = rho_tools::EditFormat::from_config_value(&normalized) { + return Ok(Self::Pinned(format)); + } + let expected = Self::all() + .into_iter() + .map(Self::as_str) + .collect::>() + .join(", "); + Err(format!( + "unknown edit tool {normalized:?}; expected {expected}" + )) + } +} + +impl Serialize for EditTool { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(self.as_str()) + } +} + +impl<'de> Deserialize<'de> for EditTool { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + String::deserialize(deserializer)? + .parse() + .map_err(serde::de::Error::custom) + } +} #[derive(Clone, Default, Deserialize, Serialize, PartialEq, Eq)] pub(crate) struct LegacyWebSearchCredentials { diff --git a/crates/rho/src/config_load_tests.rs b/crates/rho/src/config_load_tests.rs index 4dc5f0c05..bcdcd3ea2 100644 --- a/crates/rho/src/config_load_tests.rs +++ b/crates/rho/src/config_load_tests.rs @@ -121,9 +121,19 @@ provider = "unknown" #[test] fn edit_tool_preferences_load_from_behavior_config() { for (value, expected) in [ - ("hashline", super::super::EditTool::Hashline), - ("apply_patch", super::super::EditTool::ApplyPatch), - ("str_replace", super::super::EditTool::StrReplace), + ("auto", super::super::EditTool::Auto), + ( + "hashline", + super::super::EditTool::Pinned(rho_tools::EditFormat::Hashline), + ), + ( + "apply_patch", + super::super::EditTool::Pinned(rho_tools::EditFormat::ApplyPatch), + ), + ( + "str_replace", + super::super::EditTool::Pinned(rho_tools::EditFormat::StrReplace), + ), ] { let (config, warnings) = parse_settings(&format!("[behavior]\nedit_tool = {value:?}\n")).unwrap(); @@ -454,3 +464,20 @@ fn conversation_triple(config: &super::Config, id: &str) -> (String, String, Str selection.auth.clone(), ) } + +// Covers: Auto preference resolves EditTool and composes display labels for +// the active provider. +// Owner: config EditTool preference +#[test] +fn auto_edit_tool_resolves_preferred_format_for_provider() { + use super::EditTool; + + assert_eq!( + EditTool::Auto.resolve("openai-codex"), + rho_tools::EditFormat::ApplyPatch + ); + assert_eq!( + EditTool::Auto.display_label("anthropic"), + "auto (str_replace)" + ); +} diff --git a/crates/rho/src/diagnostics.rs b/crates/rho/src/diagnostics.rs index 73eaccce2..0465bdda9 100644 --- a/crates/rho/src/diagnostics.rs +++ b/crates/rho/src/diagnostics.rs @@ -156,6 +156,12 @@ impl RuntimeDiagnostics { self.write().config.advisor_mode = advisor_mode; } + /// Edit tool selection can change mid-session, so the mirror follows the + /// live value rather than the process startup snapshot. + pub fn update_edit_tool(&self, edit_tool: &str) { + self.write().config.edit_tool = edit_tool.into(); + } + pub fn update_prompt_sources(&self, sources: Vec) { self.write().prompt_sources = sources; } diff --git a/crates/rho/src/diagnostics_tests.rs b/crates/rho/src/diagnostics_tests.rs index fbd3d1b96..1edbe9758 100644 --- a/crates/rho/src/diagnostics_tests.rs +++ b/crates/rho/src/diagnostics_tests.rs @@ -59,6 +59,7 @@ fn runtime_updates_do_not_replace_restart_only_config() { let diagnostics = RuntimeDiagnostics::new(&config); diagnostics.update_max_tool_output_lines(25); diagnostics.update_check_for_updates(false); + diagnostics.update_edit_tool("str_replace"); diagnostics.update_compaction_config(&CompactionConfig { auto_compact: true, threshold_percent: 70, @@ -81,7 +82,7 @@ fn runtime_updates_do_not_replace_restart_only_config() { "check_for_updates": false, "enable_subagents": true, "advisor_mode": false, - "edit_tool": "hashline", + "edit_tool": "str_replace", "rtk": true, "source": "live values used by this process; restart-only settings may differ from saved config" }) diff --git a/crates/rho/src/prompt.rs b/crates/rho/src/prompt.rs index 492a6ed82..29afccaf1 100644 --- a/crates/rho/src/prompt.rs +++ b/crates/rho/src/prompt.rs @@ -91,28 +91,15 @@ Prefer the `grep` tool over shell `rg` or `grep` for workspace content search. U "#, ); } - let selected_edit_tool = tools + // Format-agnostic: mid-session edit-tool switches keep this system prompt + // fixed, so do not name a concrete edit surface or embed hashline policy. + // Concrete contracts live on the live tool description/schema. + if tools .iter() - .find(|tool| rho_tools::EditFormat::is_edit_tool_name(tool.name.as_str())); - if let Some(tool) = selected_edit_tool { - text.push_str(&format!( - "\nPrefer the `{}` tool over shell or script-based rewrites for existing UTF-8 files. Prefer `write` only to create or fully rewrite a file.\n", - tool.name - )); - } - // Hashline-only policy: only the hashline `edit` surface needs TAG/PUT guidance. - if tools.iter().any(|tool| tool.name == "edit") { - if grep_available { - text.push_str( - r#" -`grep` content mode returns chainable `[path#TAG]` headers and match line numbers (`N | preview`) for hash-line edit anchors. Match text is preview only and may be truncated - copy TAG and line numbers, not preview bodies, into PUT rows; use `read_file` when you need exact line text. -"#, - ); - } + .any(|tool| rho_tools::EditFormat::is_edit_tool_name(tool.name.as_str())) + { text.push_str( - r#" -Use `edit` (not shell or Python rewrites) for existing UTF-8 files once you have a fresh `[path#TAG]`. Copy locator forms and the PUT body/span contract from the tool description (`PUT 12:` never `PUT 12.:`). Put every hunk for one path in a single document; do not stack two `edit` calls on the same path in one batch. After a structural edit the tool returns TAG + ops summary without chainable body lines - re-read before further ops on that path. Prefer `write` only to create or fully rewrite a file. -"#, + "\nUse the live file-edit tool from the tool list for existing UTF-8 files. Prefer `write` only to create or fully rewrite a file.\n", ); } if tools.iter().any(|tool| tool.name == "agent") { @@ -237,15 +224,73 @@ fn neutralize_mcp_server_instruction_close_tags(text: &str) -> String { text.replace(NEEDLE, REPLACEMENT) } -/// Tells the executor when to consult the `advisor` tool. +/// Model and display text when the `advisor` tool becomes available. +/// +/// Steering lives on the tool description so the system prompt stays free of +/// tool-list-dependent text. This notice only announces availability + schema. +pub fn advisor_enabled_context(spec: &ToolSpec) -> (String, String) { + let model = format!( + "[advisor mode on]\n\n\ +The `advisor` tool is now available. Do not skip it when the live tool list includes it.\n\n\ +{}\n", + tool_schema_block(spec), + ); + let display = "advisor mode on".into(); + (model, display) +} + +/// Model and display text when the `advisor` tool is removed. +pub fn advisor_disabled_context() -> (String, String) { + let model = "\ +[advisor mode off]\n\n\ +The `advisor` tool is no longer available. Do not call `advisor`. \ +Follow the live tool list.\n" + .into(); + let display = "advisor mode off".into(); + (model, display) +} + +/// Model and display text for a mid-session edit-tool switch. /// -/// Appended only while advisor mode is active and an advisor model is set, so -/// the prompt never describes a tool the run does not have. -pub fn append_advisor_instruction(text: &mut String) { - text.push_str(ADVISOR_INSTRUCTION); +/// The system prompt stays format-agnostic. This notice carries the new tool +/// contract so the model stops using the previous surface. +pub fn edit_tool_switch_context( + previous: rho_tools::EditFormat, + current: rho_tools::EditFormat, + spec: &ToolSpec, +) -> (String, String) { + let previous_name = previous.tool_name(); + let current_name = current.tool_name(); + let model = format!( + "[edit tool switched]\n\n\ +The file edit tool changed mid-session. Do not call `{previous_name}` anymore.\n\ +Use `{current_name}` for edits to existing UTF-8 files from now on.\n\ +Prefer `write` only to create or fully rewrite a file.\n\ +Follow the live tool list.\n\n\ +Previous tool: `{previous_name}` ({previous_label})\n\ +Current tool: `{current_name}` ({current_label})\n\n\ +{schema}\n", + previous_label = previous.as_str(), + current_label = current.as_str(), + schema = tool_schema_block(spec), + ); + let display = format!("edit tool switched to {}", current.as_str()); + (model, display) } -const ADVISOR_INSTRUCTION: &str = "\n\n# Advisor\n\nYou have access to an `advisor` tool backed by a stronger reviewer model. It takes NO parameters. When you call advisor, your entire conversation history is forwarded automatically. The advisor sees the task, every tool call you have made, and every result you have seen.\n\nCall advisor BEFORE substantive work: before writing, before committing to an interpretation, before building on an assumption. If the task needs orientation first (finding files, fetching a source, seeing what is there), do that, then call advisor. Orientation is not substantive work. Writing, editing, and declaring an answer are.\n\nAlso call advisor:\n- When you believe the task is complete. BEFORE this call, make your deliverable durable: write the file, save the result, commit the change.\n- When stuck: errors recurring, approach not converging, results that do not fit.\n- When considering a change of approach.\n\nOn tasks longer than a few steps, call advisor at least once before committing to an approach and once before declaring done. On short reactive tasks where the next action follows from tool output you just read, you do not need to keep calling. The advisor adds most of its value on the first call, before the approach hardens.\n\nGive the advice serious weight. If you follow a step and it fails in practice, or you have primary-source evidence that contradicts a specific claim, adapt. If you have already retrieved data pointing one way and the advisor points another, do not switch silently: surface the conflict in one more advisor call.\n"; +fn tool_schema_block(spec: &ToolSpec) -> String { + let schema = serde_json::to_string_pretty(&spec.input_schema).unwrap_or_else(|_| "{}".into()); + format!( + "Tool schema for `{name}`:\n\ +description:\n\ +{description}\n\n\ +input_schema:\n\ +{schema}", + name = spec.name, + description = spec.description, + schema = schema, + ) +} fn push_context_file(out: &mut String, tag: &str, path: &Path, contents: &str) { out.push('\n'); @@ -498,13 +543,9 @@ mod tests { } #[test] - fn includes_selected_edit_policy_and_hashline_details_only_for_edit() { + fn includes_format_agnostic_edit_policy_when_any_edit_tool_is_present() { let project = TempDir::new().unwrap(); - for (config_name, tool_name, expect_hashline) in [ - ("hashline", "edit", true), - ("apply_patch", "apply_patch", false), - ("str_replace", "str_replace", false), - ] { + for tool_name in ["edit", "apply_patch", "str_replace"] { let tool = ToolSpec { name: tool_name.into(), description: "edit".into(), @@ -514,23 +555,22 @@ mod tests { let prompt = system_prompt_with_home(&[tool], project.path(), None).text; assert!( - prompt.contains(&format!("Prefer the `{tool_name}` tool")), - "config {config_name}" + prompt.contains("Use the live file-edit tool from the tool list"), + "tool {tool_name}" ); - assert_eq!( - prompt.contains("never `PUT 12.:`"), - expect_hashline, - "config {config_name}" + assert!( + !prompt.contains(&format!("Prefer the `{tool_name}` tool")), + "tool {tool_name}" ); - assert_eq!( - prompt.contains("without chainable body lines"), - expect_hashline, - "config {config_name}" + assert!(!prompt.contains("never `PUT 12.:`"), "tool {tool_name}"); + assert!( + !prompt.contains("without chainable body lines"), + "tool {tool_name}" ); } let disabled = system_prompt_with_home(&[], project.path(), None).text; - assert!(!disabled.contains("over shell or script-based rewrites")); + assert!(!disabled.contains("live file-edit tool from the tool list")); } #[test] diff --git a/crates/rho/src/tools/advisor/mod.rs b/crates/rho/src/tools/advisor/mod.rs index 0e5f3bfa8..33f333c2b 100644 --- a/crates/rho/src/tools/advisor/mod.rs +++ b/crates/rho/src/tools/advisor/mod.rs @@ -39,7 +39,7 @@ pub(crate) const TOOL_NAME: &str = "advisor"; const USAGE_PURPOSE: &str = "advisor"; -const TOOL_DESCRIPTION: &str = "Consult a stronger reviewer model about this session. Takes no parameters: your whole conversation history, including the task, every tool call you made, and every result you saw, is forwarded automatically. Returns strategic guidance on what to do next. Call it before substantive work, when you are stuck, when you consider changing approach, and when you believe the task is complete."; +const TOOL_DESCRIPTION: &str = "Consult a stronger reviewer model about this session. Takes NO parameters: your whole conversation history, including the task, every tool call you made, and every result you saw, is forwarded automatically. Returns strategic guidance on what to do next.\n\nCall advisor BEFORE substantive work: before writing, before committing to an interpretation, before building on an assumption. If the task needs orientation first (finding files, fetching a source, seeing what is there), do that, then call advisor. Orientation is not substantive work. Writing, editing, and declaring an answer are.\n\nAlso call advisor:\n- When you believe the task is complete. BEFORE this call, make your deliverable durable: write the file, save the result, commit the change.\n- When stuck: errors recurring, approach not converging, results that do not fit.\n- When considering a change of approach.\n\nOn tasks longer than a few steps, call advisor at least once before committing to an approach and once before declaring done. On short reactive tasks where the next action follows from tool output you just read, you do not need to keep calling. The advisor adds most of its value on the first call, before the approach hardens.\n\nGive the advice serious weight. If you follow a step and it fails in practice, or you have primary-source evidence that contradicts a specific claim, adapt. If you have already retrieved data pointing one way and the advisor points another, do not switch silently: surface the conflict in one more advisor call."; const NO_MODEL_MESSAGE: &str = "advisor mode has no advisor model. Choose one with /advisor, then call advisor again."; @@ -141,6 +141,11 @@ impl AdvisorSessionStore { self.lock().model = model; } + /// Currently configured advisor model, if any. + pub fn model(&self) -> Option { + self.lock().model.clone() + } + /// Fold provider-reported cost from a finished advisor call into the /// unclaimed total. /// diff --git a/crates/rho/src/tools/coding.rs b/crates/rho/src/tools/coding.rs index cfbbbc030..da6e12101 100644 --- a/crates/rho/src/tools/coding.rs +++ b/crates/rho/src/tools/coding.rs @@ -1,12 +1,27 @@ use std::sync::Arc; use crate::agent::{AgentCapabilities, ToolCapability}; -use rho_sdk::ProcessEnvironment; +use rho_sdk::{tool::Tool, ProcessEnvironment}; + +/// Build the mid-session/startup edit tool with one shared options policy. +pub(super) fn edit_tool( + edit_format: rho_tools::EditFormat, + max_output_bytes: usize, + mutation_observer: Arc, +) -> Arc { + rho_tools::coding_tool( + rho_tools::CodingToolKind::Edit, + rho_tools::CodingToolOptions::new() + .max_output_bytes(max_output_bytes) + .edit_tool(edit_format) + .mutation_observer(mutation_observer), + ) +} pub(super) fn sdk_bundle( capabilities: &AgentCapabilities, max_output_bytes: usize, - config_edit_tool: crate::config::EditTool, + config_edit_tool: rho_tools::EditFormat, process_environment: ProcessEnvironment, mutation_observer: Arc, ) -> super::sdk_registry::StaticToolBundle { @@ -14,7 +29,6 @@ pub(super) fn sdk_bundle( let options = rho_tools::CodingToolOptions::new() .max_output_bytes(max_output_bytes) - .edit_tool(config_edit_tool) .mutation_observer(Arc::clone(&mutation_observer)); let mut tools = Vec::new(); for (capability, kind) in [ @@ -25,8 +39,16 @@ pub(super) fn sdk_bundle( (ToolCapability::Grep, CodingToolKind::Grep), (ToolCapability::Glob, CodingToolKind::Glob), ] { - if capabilities.contains(&capability) { - tools.push(rho_tools::coding_tool(kind, options.clone())); + if !capabilities.contains(&capability) { + continue; + } + match kind { + CodingToolKind::Edit => tools.push(edit_tool( + config_edit_tool, + max_output_bytes, + Arc::clone(&mutation_observer), + )), + kind => tools.push(rho_tools::coding_tool(kind, options.clone())), } } #[cfg(unix)] diff --git a/crates/rho/src/tools/sdk_registry.rs b/crates/rho/src/tools/sdk_registry.rs index 92433615f..f459000e8 100644 --- a/crates/rho/src/tools/sdk_registry.rs +++ b/crates/rho/src/tools/sdk_registry.rs @@ -188,7 +188,7 @@ impl AppToolSet { tool_set.add_bundle(super::coding::sdk_bundle( &capabilities, config.max_output_bytes, - config.edit_tool, + config.edit_tool.resolve(&config.provider), process_environment.clone(), tool_set.checkpoint_tracker.clone(), )); @@ -300,6 +300,12 @@ impl AppToolSet { self.unfiltered_names().any(|registered| registered == name) } + /// Test-only: append a tool without capability filtering. + #[cfg(test)] + pub(crate) fn push_tool_for_tests(&mut self, tool: Arc) { + self.tools.push(tool); + } + /// The advisor's session store, present whenever the run may offer the /// advisor, even while advisor mode is off. pub fn advisor(&self) -> Option<&AdvisorSessionStore> { @@ -335,6 +341,47 @@ impl AppToolSet { true } + /// Replaces the advertised built-in file edit tool. + /// + /// Returns the previous format when the advertised tool list changed so + /// callers can rebuild the runtime and tell the model. No-ops when this + /// run has no edit tool or the selection is already active. + /// + /// Matches only canonical built-in names (`edit`, `apply_patch`, + /// `str_replace`). Legacy aliases such as `edit_file` still classify as + /// edit for transcripts via [`rho_tools::EditFormat::is_edit_tool_name`], + /// but are never treated as the swappable built-in slot. + pub fn set_edit_tool( + &mut self, + edit_tool: rho_tools::EditFormat, + max_output_bytes: usize, + ) -> Option { + let previous = self.edit_tool()?; + if previous == edit_tool { + return None; + } + let position = self + .tools + .iter() + .position(|tool| is_canonical_edit_tool_name(tool.spec().name.as_str()))?; + let mutation_observer: Arc = + self.checkpoint_tracker.clone(); + self.tools[position] = + super::coding::edit_tool(edit_tool, max_output_bytes, mutation_observer); + Some(previous) + } + + /// The currently advertised built-in edit format, when this run exposes one. + pub fn edit_tool(&self) -> Option { + self.tools.iter().find_map(|tool| { + let name = tool.spec().name; + rho_tools::EditFormat::ALL + .iter() + .copied() + .find(|format| format.tool_name() == name.as_str()) + }) + } + pub fn subagents(&self) -> Option<&SubagentManager> { self.subagents.as_ref() } @@ -360,6 +407,14 @@ impl AppToolSet { } } +/// Whether `name` is a canonical built-in edit tool name (`edit`, +/// `apply_patch`, `str_replace`). Excludes legacy aliases such as `edit_file`. +fn is_canonical_edit_tool_name(name: &str) -> bool { + rho_tools::EditFormat::ALL + .iter() + .any(|format| format.tool_name() == name) +} + #[cfg(test)] #[path = "sdk_registry_tests.rs"] mod tests; diff --git a/crates/rho/src/tools/sdk_registry_tests.rs b/crates/rho/src/tools/sdk_registry_tests.rs index ce7818e07..ce62529e5 100644 --- a/crates/rho/src/tools/sdk_registry_tests.rs +++ b/crates/rho/src/tools/sdk_registry_tests.rs @@ -71,7 +71,11 @@ fn canonical_tool_names_match_the_unfiltered_registry() { let root = tempfile::tempdir().unwrap(); let mut model_names = Vec::new(); - for &edit_tool in rho_tools::EditFormat::ALL { + for edit_tool in [ + crate::config::EditTool::Pinned(rho_tools::EditFormat::Hashline), + crate::config::EditTool::Pinned(rho_tools::EditFormat::ApplyPatch), + crate::config::EditTool::Pinned(rho_tools::EditFormat::StrReplace), + ] { let config = Config { edit_tool, ..Config::default() @@ -88,7 +92,7 @@ fn canonical_tool_names_match_the_unfiltered_registry() { // Advisor mode is off by default; the registry still owns the name. tools.set_advisor_registered(true); let names = tools.unfiltered_names().collect::>(); - let selected = edit_tool.tool_name(); + let selected = edit_tool.resolve(&config.provider).tool_name(); // Model-facing names only; legacy `edit_file` is not registered. // NEXT_MAJOR(rho-tools): drop edit_file alias recognition entirely in 2.0. for name in ["edit", "apply_patch", "str_replace"] { @@ -598,3 +602,139 @@ fn advisor_registration_toggles_without_rebuilding_the_tool_set() { // the model the user already chose. assert!(tools.advisor().is_some()); } + +// Covers: /config edit-tool selection must swap the single advertised edit +// surface without rebuilding the rest of the tool set. +// Owner: application tool registry. +#[test] +fn edit_tool_selection_swaps_the_advertised_edit_surface() { + let config = Config { + edit_tool: crate::config::EditTool::Pinned(rho_tools::EditFormat::Hashline), + ..Config::default() + }; + let mut tools = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(capabilities(&["edit", "read_file"])), + ); + let before = tools.unfiltered_names().collect::>(); + assert_eq!(tools.edit_tool(), Some(rho_tools::EditFormat::Hashline)); + assert!(tools.contains("edit")); + assert!(!tools.contains("str_replace")); + + assert_eq!( + tools.set_edit_tool(rho_tools::EditFormat::Hashline, config.max_output_bytes), + None + ); + assert_eq!( + tools.set_edit_tool(rho_tools::EditFormat::StrReplace, config.max_output_bytes), + Some(rho_tools::EditFormat::Hashline) + ); + assert_eq!(tools.edit_tool(), Some(rho_tools::EditFormat::StrReplace)); + assert!(!tools.contains("edit")); + assert!(tools.contains("str_replace")); + + let after = tools.unfiltered_names().collect::>(); + assert_eq!(before.len(), after.len()); + assert!(after.iter().any(|name| name == "read_file")); + + let mut without_edit = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(capabilities(&["read_file"])), + ); + assert_eq!( + without_edit.set_edit_tool(rho_tools::EditFormat::ApplyPatch, config.max_output_bytes), + None + ); +} + +// Covers: Auto preference advertises the provider-preferred format at construction. +// Owner: application tool registry. +#[test] +fn auto_edit_tool_constructs_the_preferred_provider_format() { + let config = Config { + provider: "anthropic".into(), + edit_tool: crate::config::EditTool::Auto, + ..Config::default() + }; + let tools = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(capabilities(&["edit", "read_file"])), + ); + assert_eq!(tools.edit_tool(), Some(rho_tools::EditFormat::StrReplace)); + assert!(tools.contains("str_replace")); + assert!(!tools.contains("edit")); + assert!(!tools.contains("apply_patch")); +} + +// Covers: legacy edit_file alias names must not be treated as the swappable +// built-in edit slot. set_edit_tool matches only canonical tool names. +// Owner: application tool registry. +#[test] +fn set_edit_tool_ignores_legacy_edit_file_alias_tools() { + use rho_sdk::{ + model::ToolSpec, + tool::{Tool as SdkTool, ToolContext, ToolFuture, ToolInvocation, ToolOutput}, + }; + + struct AliasTool; + + impl SdkTool for AliasTool { + fn spec(&self) -> ToolSpec { + ToolSpec { + name: "edit_file".into(), + description: "unrelated alias-named tool".into(), + input_schema: json!({"type": "object"}), + } + } + + fn call<'a>( + &'a self, + _invocation: ToolInvocation, + _context: ToolContext, + ) -> ToolFuture<'a> { + Box::pin(async { Ok(ToolOutput::text("unused")) }) + } + } + + // No Edit capability: only the alias-named intruder is present. + let config = Config::default(); + let mut tools = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(capabilities(&["read_file"])), + ); + tools.push_tool_for_tests(Arc::new(AliasTool)); + assert!(tools.contains("edit_file")); + assert!(rho_tools::EditFormat::is_edit_tool_name("edit_file")); + assert_eq!(tools.edit_tool(), None); + assert_eq!( + tools.set_edit_tool(rho_tools::EditFormat::StrReplace, config.max_output_bytes), + None, + "must not replace a non-canonical edit_file tool" + ); + assert!(tools.contains("edit_file")); + assert!(!tools.contains("str_replace")); + + // With a real built-in edit tool, the alias must stay put while the + // canonical slot swaps. + let config = Config { + edit_tool: crate::config::EditTool::Pinned(rho_tools::EditFormat::Hashline), + ..Config::default() + }; + let mut with_edit = AppToolSet::new( + &config, + RuntimeDiagnostics::new(&config), + ToolSetOptions::new(capabilities(&["edit", "read_file"])), + ); + with_edit.push_tool_for_tests(Arc::new(AliasTool)); + assert_eq!( + with_edit.set_edit_tool(rho_tools::EditFormat::StrReplace, config.max_output_bytes), + Some(rho_tools::EditFormat::Hashline) + ); + assert!(with_edit.contains("str_replace")); + assert!(with_edit.contains("edit_file")); + assert!(!with_edit.contains("edit")); +} diff --git a/crates/rho/src/tui/advisor_command.rs b/crates/rho/src/tui/advisor_command.rs index b32713f5b..181e72b3c 100644 --- a/crates/rho/src/tui/advisor_command.rs +++ b/crates/rho/src/tui/advisor_command.rs @@ -12,24 +12,35 @@ const SELECT_ADVISOR_MODEL_EDIT_STATUS: &str = "select an advisor model"; /// The runtime side of advisor mode. /// -/// Advisor mode changes the tool list and the system prompt, so the runtime has -/// to act on it rather than only save it. Implementors apply the change to the -/// next turn and leave the current session ID and history alone. +/// Advisor mode changes the tool list, so the runtime has to act on it rather +/// than only save it. Implementors apply the change to the next turn, keep the +/// system prompt fixed, append a context notice when the tool list changes, and +/// leave the current session ID alone. pub(super) trait AdvisorRuntime { /// Points the advisor at `model`, or turns the advisor off with `None`. + /// + /// Returns display text for a transcript notice when the advertised tool + /// list changed. fn set_advisor( &mut self, model: Option, - ) -> impl Future> + Send; + ) -> impl Future>> + Send; + + /// Live tool specs after an advisor change, for diagnostics mirrors. + fn tool_specs(&self) -> Vec; } impl AdvisorRuntime for InteractiveRuntime { fn set_advisor( &mut self, model: Option, - ) -> impl Future> + Send { + ) -> impl Future>> + Send { InteractiveRuntime::set_advisor(self, model) } + + fn tool_specs(&self) -> Vec { + InteractiveRuntime::tool_specs(self) + } } impl App { @@ -123,22 +134,65 @@ impl App { enabled: bool, agent: &mut impl AdvisorRuntime, ) -> anyhow::Result<()> { - if self.info.runtime.advisor_mode != enabled { + let previous_mode = self.info.runtime.advisor_mode; + let previous_model = self + .info + .runtime + .internal_agents + .get(ADVISOR_AGENT_ID) + .cloned(); + + // Apply the runtime transition against the *desired* mode before + // touching config or in-memory mode, so a failure leaves both alone. + let desired_model = enabled.then(|| previous_model.clone()).flatten(); + let notice = match agent.set_advisor(desired_model).await { + Ok(notice) => notice, + Err(error) => { + self.insert_entry(&Entry::Error(format!( + "advisor mode could not be applied to this session: {error}" + ))); + self.set_status("advisor mode change failed"); + // Surface the runtime error to callers (active run, etc.). + return Err(error); + } + }; + + if previous_mode != enabled { if let Err(error) = self .info .services .config_repository .update(|config| config.advisor_mode = enabled) { - self.insert_entry(&Entry::Error(format!( - "could not save advisor mode: {error}" - ))); + // Runtime already moved; best-effort restore so mode and tool + // list stay aligned with the failed save. + let rollback_model = previous_mode.then_some(previous_model).flatten(); + if let Err(rollback_error) = agent.set_advisor(rollback_model).await { + self.insert_entry(&Entry::Error(format!( + "could not save advisor mode: {error}; runtime rollback failed: {rollback_error}" + ))); + } else { + self.insert_entry(&Entry::Error(format!( + "could not save advisor mode: {error}" + ))); + } self.set_status("config save failed"); return Ok(()); } self.info.runtime.advisor_mode = enabled; } - self.sync_advisor_runtime(agent).await; + + self.info + .services + .diagnostics + .update_advisor_mode(self.info.runtime.advisor_mode); + if let Some(display) = notice { + self.insert_entry(&Entry::Notice(display)); + self.info + .services + .diagnostics + .update_tools(&agent.tool_specs()); + } // Mode and model both feed the statusline; refresh before the toast so // the bottom row and the status message describe the same state. self.statusline.update_model(&self.info.runtime); @@ -184,7 +238,9 @@ impl App { /// Applies the saved advisor state to the live runtime. /// /// The advisor model and the mode reach the runtime as one value, because - /// advisor mode without a model offers the executor nothing. + /// advisor mode without a model offers the executor nothing. Callers that + /// must keep config and memory aligned with a failed transition should + /// prefer [`Self::set_advisor_mode`], which applies the runtime first. pub(super) async fn sync_advisor_runtime(&mut self, agent: &mut impl AdvisorRuntime) { let model = self.info.runtime.advisor_mode.then(|| { self.info @@ -197,10 +253,20 @@ impl App { .services .diagnostics .update_advisor_mode(self.info.runtime.advisor_mode); - if let Err(error) = agent.set_advisor(model.flatten()).await { - self.insert_entry(&Entry::Error(format!( - "advisor mode could not be applied to this session: {error}" - ))); + match agent.set_advisor(model.flatten()).await { + Ok(Some(display)) => { + self.insert_entry(&Entry::Notice(display)); + self.info + .services + .diagnostics + .update_tools(&agent.tool_specs()); + } + Ok(None) => {} + Err(error) => { + self.insert_entry(&Entry::Error(format!( + "advisor mode could not be applied to this session: {error}" + ))); + } } } diff --git a/crates/rho/src/tui/advisor_command_tests.rs b/crates/rho/src/tui/advisor_command_tests.rs index f2613cb36..25c5bfccb 100644 --- a/crates/rho/src/tui/advisor_command_tests.rs +++ b/crates/rho/src/tui/advisor_command_tests.rs @@ -66,9 +66,13 @@ impl AdvisorRuntime for FakeAdvisorRuntime { fn set_advisor( &mut self, model: Option, - ) -> impl std::future::Future> + Send { + ) -> impl std::future::Future>> + Send { self.applied.push(model); - std::future::ready(Ok(())) + std::future::ready(Ok(None)) + } + + fn tool_specs(&self) -> Vec { + Vec::new() } } @@ -286,19 +290,56 @@ fn dismissing_the_advisor_model_prompt_leaves_the_mode_off() { }); } -// Covers: an unknown argument reports usage without changing the mode +// Covers: an active-run runtime error must not flip saved or in-memory mode. // Owner: advisor command #[tokio::test] -async fn unknown_advisor_argument_reports_usage() { - let mut app = app_with_advisor_model(); - let mut agent = FakeAdvisorRuntime::default(); +async fn advisor_mode_runtime_failure_leaves_mode_unchanged() { + #[derive(Debug)] + struct ActiveRunError; + + impl std::fmt::Display for ActiveRunError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("advisor mode cannot change while a run is active") + } + } - app.execute_advisor_command_with_runtime(invocation("/advisor maybe"), &mut agent) - .await - .unwrap(); + impl std::error::Error for ActiveRunError {} + + struct ActiveRunRuntime; + + impl AdvisorRuntime for ActiveRunRuntime { + fn set_advisor( + &mut self, + _model: Option, + ) -> impl std::future::Future>> + Send { + std::future::ready(Err(anyhow::Error::new(ActiveRunError))) + } + fn tool_specs(&self) -> Vec { + Vec::new() + } + } + + let mut app = app_with_advisor_model(); + let mut agent = ActiveRunRuntime; + let error = app + .execute_advisor_command_with_runtime(invocation("/advisor on"), &mut agent) + .await + .expect_err("active-run failure should propagate"); + assert!( + error.downcast_ref::().is_some(), + "expected typed ActiveRunError, got: {error:#}" + ); assert!(!app.info.runtime.advisor_mode); - assert_eq!(app.status(), "invalid advisor mode"); + assert!( + !app.info + .services + .config_repository + .load() + .unwrap() + .advisor_mode + ); + assert_eq!(app.status(), "advisor mode change failed"); } // Covers: editing the advisor model from /config does not claim it enables mode. diff --git a/crates/rho/src/tui/app_state/history_ui.rs b/crates/rho/src/tui/app_state/history_ui.rs index 4fe2641c8..11b5a1830 100644 --- a/crates/rho/src/tui/app_state/history_ui.rs +++ b/crates/rho/src/tui/app_state/history_ui.rs @@ -64,10 +64,6 @@ impl HistoryUi { self.transcript.get(index) } - pub(in crate::tui) fn get_mut(&mut self, index: usize) -> Option<&mut Entry> { - self.transcript.get_mut(index) - } - pub(in crate::tui) fn lines_mut(&mut self) -> &mut HistoryLineCache { &mut self.lines } diff --git a/crates/rho/src/tui/config_actions.rs b/crates/rho/src/tui/config_actions.rs index a35999cba..cca3689eb 100644 --- a/crates/rho/src/tui/config_actions.rs +++ b/crates/rho/src/tui/config_actions.rs @@ -113,11 +113,10 @@ impl App { let selected = &value[config_picker::EDIT_TOOL_PREFIX.len()..]; let edit_tool: crate::config::EditTool = selected.parse().map_err(anyhow::Error::msg)?; - self.info.services.config_repository.update(|config| { - config.edit_tool = edit_tool; - })?; + self.apply_edit_tool(edit_tool, agent).await?; + let status = self.status().to_string(); self.open_main_config_picker_selected(config_picker::EDIT_TOOL_VALUE)?; - self.set_status(format!("edit tool: {edit_tool}; restart Rho to apply")); + self.set_status(status); Ok(()) } value if value.starts_with(config_picker::INLINE_SHELL_PREFIX) => { @@ -548,4 +547,147 @@ impl App { config.reasoning = self.info.runtime.reasoning; }) } + + /// Saves the edit-tool preference and applies the resolved format when possible. + /// + /// The system prompt stays fixed. A successful live switch rebuilds the tool + /// list for the next turn and appends a model-facing schema notice. + /// [`crate::config::EditTool::Auto`] keeps `auto` in config and advertises the + /// preferred format for the active provider. + pub(super) async fn apply_edit_tool( + &mut self, + edit_tool: crate::config::EditTool, + agent: &mut InteractiveRuntime, + ) -> anyhow::Result<()> { + let config = self.info.services.config_repository.load()?; + let provider = self.info.runtime.provider.clone(); + let resolved = edit_tool.resolve(&provider); + let change = match self + .apply_resolved_edit_tool(agent, resolved, config.max_output_bytes, |error| { + format!("could not apply edit tool: {error}") + }) + .await + { + Ok(change) => change, + Err(()) => { + self.set_status("edit tool change failed"); + return Ok(()); + } + }; + + if let Err(error) = self.info.services.config_repository.update(|config| { + config.edit_tool = edit_tool; + }) { + if let Some(change) = change { + // Forward switch already landed in model-visible and persisted + // display history. Rollback records the reverse the same way. + // Mirror both into the transcript so UI, model context, and + // display history describe the same transition sequence. + match agent + .set_edit_tool(change.previous, config.max_output_bytes) + .await + { + Ok(rollback) => { + self.insert_entry(&Entry::Notice(change.display.clone())); + if let Some(rollback) = rollback { + self.insert_entry(&Entry::Notice(rollback.display)); + } + } + Err(rollback_error) => { + self.insert_entry(&Entry::Notice(change.display.clone())); + return Err(anyhow::anyhow!( + "could not save edit tool: {error}; runtime rollback failed: {rollback_error}" + )); + } + } + } + self.insert_entry(&Entry::Error(format!( + "could not save edit tool setting: {error}" + ))); + self.set_status("config save failed"); + return Ok(()); + } + + // UI mirrors only after the preference is durable, so a save failure + // cannot leave diagnostics/notices ahead of config. + self.info + .services + .diagnostics + .update_edit_tool(edit_tool.as_str()); + if let Some(change) = change.as_ref() { + self.info + .services + .diagnostics + .update_tools(&agent.tool_specs()); + self.insert_entry(&Entry::Notice(change.display.clone())); + } + self.set_status(format!("edit tool: {}", edit_tool.display_label(&provider))); + Ok(()) + } + + /// When edit preference is Auto, advertise the preferred format for + /// `provider`. Failures are reported as notices and do not undo a model + /// switch. + pub(super) async fn apply_auto_edit_tool_for_provider( + &mut self, + provider: &str, + agent: &mut InteractiveRuntime, + ) -> anyhow::Result<()> { + let config = self.info.services.config_repository.load()?; + if config.edit_tool != crate::config::EditTool::Auto { + return Ok(()); + } + let resolved = config.edit_tool.resolve(provider); + match self + .apply_resolved_edit_tool(agent, resolved, config.max_output_bytes, |error| { + format!("model switched, but auto edit tool could not follow the provider: {error}") + }) + .await + { + Ok(Some(change)) => { + // Auto provider-follow does not persist a preference; mirror + // the live tool list and notice immediately. Leave the caller's + // status toast alone (model switch should keep its own feedback). + self.info + .services + .diagnostics + .update_tools(&agent.tool_specs()); + self.insert_entry(&Entry::Notice(change.display)); + } + Ok(None) => {} + Err(()) => {} + } + Ok(()) + } + + /// Applies a concrete edit format on the live runtime. + /// + /// On runtime failure inserts an error entry built by `on_error` and returns + /// `Err(())`. `Ok(None)` means the advertised surface did not change. + /// Callers that persist a preference must apply diagnostics/notice updates + /// only after that save succeeds; the Auto provider-switch path updates UI + /// immediately because it does not write config. + async fn apply_resolved_edit_tool( + &mut self, + agent: &mut InteractiveRuntime, + resolved: rho_tools::EditFormat, + max_output_bytes: usize, + on_error: impl FnOnce(&anyhow::Error) -> String, + ) -> Result, ()> { + match agent.set_edit_tool(resolved, max_output_bytes).await { + Ok(change) => Ok(change), + Err(error) => { + self.insert_entry(&Entry::Error(on_error(&error))); + Err(()) + } + } + } + + pub(super) fn reject_edit_tool_change(&mut self) { + self.set_status("edit tool cannot change until the current turn finishes"); + } } + +#[cfg(test)] +#[path = "config_actions_tests.rs"] +mod tests; diff --git a/crates/rho/src/tui/config_actions_tests.rs b/crates/rho/src/tui/config_actions_tests.rs new file mode 100644 index 000000000..028d2f13f --- /dev/null +++ b/crates/rho/src/tui/config_actions_tests.rs @@ -0,0 +1,110 @@ +use super::*; +use crate::{ + app::{config_repository::ConfigRepository, interactive_runtime::test_edit_tool_runtime}, + config::EditTool, + tui::tests::test_app, +}; + +// Covers: a failed edit-tool config save must roll runtime back and leave the +// transcript describing the same forward+reverse transition sequence already +// written to model-visible and persisted display history. +// Owner: tui config edit-tool apply path +#[tokio::test] +async fn failed_edit_tool_save_keeps_rollback_histories_aligned() { + let mut app = test_app(); + // Instance-scoped injection keeps load working and fails only this + // repository's next durable write, including across Tokio worker hops. + let repository = ConfigRepository::temporary_for_tests().unwrap(); + repository.fail_next_save_for_tests(); + app.info.services.config_repository = repository; + + let mut agent = test_edit_tool_runtime(EditTool::Pinned(rho_tools::EditFormat::Hashline)).await; + assert!(agent.has_tool("edit")); + assert!(!agent.has_tool("str_replace")); + let history_before = agent.history().len(); + let diagnostics_before = app + .info + .services + .diagnostics + .response("config") + .expect("diagnostics config"); + + let result = app + .apply_edit_tool( + EditTool::Pinned(rho_tools::EditFormat::StrReplace), + &mut agent, + ) + .await; + + result.expect("save failure should stay Ok after successful runtime rollback"); + + // Live runtime restored. + assert!(agent.has_tool("edit"), "runtime must roll back to hashline"); + assert!(!agent.has_tool("str_replace")); + + // Model history recorded both transitions. + let history = agent.history(); + assert_eq!(history.len(), history_before + 2); + let notices: Vec = history[history_before..] + .iter() + .map(|message| match message { + rho_sdk::model::Message::User(blocks) => blocks + .iter() + .filter_map(|block| match block { + rho_sdk::model::ContentBlock::Text(text) => Some(text.as_str()), + _ => None, + }) + .collect::(), + other => panic!("expected user switch notice, got {other:?}"), + }) + .collect(); + assert!( + notices[0].contains("[edit tool switched]") && notices[0].contains("`str_replace`"), + "forward model notice missing: {}", + notices[0] + ); + assert!( + notices[1].contains("[edit tool switched]") && notices[1].contains("`edit`"), + "rollback model notice missing: {}", + notices[1] + ); + + // Transcript mirrors the same sequence, then the injected save error. + // If save succeeded, status would be the success label and this error + // entry would be missing — fail closed on the injection path. + let entries = app.history.entries(); + assert_eq!(entries.len(), 3); + assert!(matches!( + &entries[0], + Entry::Notice(text) if text == "edit tool switched to str_replace" + )); + assert!(matches!( + &entries[1], + Entry::Notice(text) if text == "edit tool switched to hashline" + )); + match &entries[2] { + Entry::Error(text) => { + assert!( + text.contains("could not save edit tool setting"), + "save-failure notice missing: {text}" + ); + assert!( + text.contains("injected config save failure"), + "expected injected save failure path, got successful save UI: {text}" + ); + } + other => panic!("expected save-failure error entry, got {other:?}"), + } + assert_eq!(app.status(), "config save failed"); + + // Preference diagnostics stay on the pre-save value. + let diagnostics_after = app + .info + .services + .diagnostics + .response("config") + .expect("diagnostics config"); + assert_eq!(diagnostics_before, diagnostics_after); + + agent.shutdown().await; +} diff --git a/crates/rho/src/tui/config_picker.rs b/crates/rho/src/tui/config_picker.rs index e7bb34ac2..7e61c0b55 100644 --- a/crates/rho/src/tui/config_picker.rs +++ b/crates/rho/src/tui/config_picker.rs @@ -157,7 +157,7 @@ pub(super) fn config_picker(info: &super::RuntimeModelView, config: &Config) -> Some(format!( "{} shell · {} · {}", config.inline_shell, - config.edit_tool.label(), + config.edit_tool.display_label(&info.provider), web_search_summary(config) )), TOOLS_CATEGORY_VALUE, @@ -322,8 +322,8 @@ pub(super) fn category_picker( ), item( "Edit tool", - "Choose the file edit format exposed to models. Restart Rho to apply changes.", - Some(config.edit_tool.label().into()), + "File edit format exposed to models. Auto follows the active provider.", + Some(config.edit_tool.display_label(&info.provider)), EDIT_TOOL_VALUE, ), item( @@ -480,9 +480,8 @@ pub(super) fn inline_shell_picker(config: &Config) -> UiPicker { pub(super) fn edit_tool_picker(selected: EditTool) -> UiPicker { UiPicker::new( "Edit tool", - EditTool::ALL - .iter() - .copied() + EditTool::all() + .into_iter() .map(|edit_tool| PickerItem { section: None, label: edit_tool.label().into(), diff --git a/crates/rho/src/tui/context_handoff.rs b/crates/rho/src/tui/context_handoff.rs index c27e859d0..05998e3d8 100644 --- a/crates/rho/src/tui/context_handoff.rs +++ b/crates/rho/src/tui/context_handoff.rs @@ -197,23 +197,25 @@ impl ContextHandoffImpact { } impl App { - pub(super) fn request_model_selection( + pub(super) async fn request_model_selection( &mut self, selection: InteractiveModelSelection, agent: &mut InteractiveRuntime, ) -> anyhow::Result<()> { self.prepare_model_selection(selection, AfterHandoff::None, agent) + .await } - pub(super) fn request_model_selection_after_turn( + pub(super) async fn request_model_selection_after_turn( &mut self, selection: InteractiveModelSelection, agent: &mut InteractiveRuntime, ) -> anyhow::Result<()> { self.prepare_model_selection(selection, AfterHandoff::ContinueTurnWork, agent) + .await } - pub(super) fn request_model_selection_from_config_picker( + pub(super) async fn request_model_selection_from_config_picker( &mut self, selection: InteractiveModelSelection, picker: UiPicker, @@ -228,9 +230,10 @@ impl App { }, agent, ) + .await } - fn prepare_model_selection( + async fn prepare_model_selection( &mut self, selection: InteractiveModelSelection, after: AfterHandoff, @@ -241,7 +244,7 @@ impl App { && target.model == self.info.runtime.model && target.auth == self.info.runtime.auth; if same_model { - self.select_model(selection, agent)?; + self.select_model(selection, agent).await?; return self.finish_after_handoff_sync(after); } @@ -262,7 +265,8 @@ impl App { { Ok(identity) => identity, Err(_) => { - self.select_model_with_omission_notice(selection, agent)?; + self.select_model_with_omission_notice(selection, agent) + .await?; return self.finish_after_handoff_sync(after); } }; @@ -276,7 +280,8 @@ impl App { cache_warm: agent.live_context_warm(), }; if !impact.should_prompt() { - self.select_model_with_omission_notice(selection, agent)?; + self.select_model_with_omission_notice(selection, agent) + .await?; return self.finish_after_handoff_sync(after); } @@ -512,7 +517,7 @@ impl App { let Some(source) = source_selection else { anyhow::bail!("session model is unavailable"); }; - self.select_model(source, agent)?; + self.select_model(source, agent).await?; self.materialize_if_needed(materialize, terminal, agent) .await?; } @@ -520,7 +525,7 @@ impl App { let had_source = source_selection.is_some(); if let Some(source) = source_selection { if !selection_matches_runtime(self, &source) { - self.select_model(source, agent)?; + self.select_model(source, agent).await?; } } self.materialize_if_needed(materialize, terminal, agent) @@ -529,7 +534,7 @@ impl App { Ok(true) => { if let Some(target) = target_selection { if !selection_matches_runtime(self, &target) { - self.select_model(target, agent)?; + self.select_model(target, agent).await?; } } } @@ -550,7 +555,7 @@ impl App { .await?; if let Some(target) = target_selection { if !selection_matches_runtime(self, &target) { - self.select_model(target, agent)?; + self.select_model(target, agent).await?; } else if !materialized { self.set_status("ready"); } @@ -675,12 +680,12 @@ impl App { Ok(()) } - pub(super) fn select_model_with_omission_notice( + pub(super) async fn select_model_with_omission_notice( &mut self, selection: InteractiveModelSelection, agent: &mut InteractiveRuntime, ) -> anyhow::Result<()> { - let report = self.select_model_report(selection, agent)?; + let report = self.select_model_report(selection, agent).await?; if let Some(report) = report.filter(HandoffReport::has_omissions) { self.insert_entry(&Entry::Notice(format!( "model handoff omitted {} nonportable provider context block(s): {}; assistant text, tool history, and reasoning summaries were preserved", diff --git a/crates/rho/src/tui/during_turn.rs b/crates/rho/src/tui/during_turn.rs index e26fd45ce..75350c249 100644 --- a/crates/rho/src/tui/during_turn.rs +++ b/crates/rho/src/tui/during_turn.rs @@ -337,7 +337,7 @@ impl App { Ok(()) } - pub(super) fn apply_pending_model_selection( + pub(super) async fn apply_pending_model_selection( &mut self, agent: &mut InteractiveRuntime, after_successful_turn: bool, @@ -347,8 +347,9 @@ impl App { }; if after_successful_turn { self.request_model_selection_after_turn(pending, agent) + .await } else { - self.select_model_with_omission_notice(pending, agent) + self.select_model_with_omission_notice(pending, agent).await } } @@ -614,6 +615,14 @@ impl App { self.open_child_picker(config_picker::inline_shell_picker(&config)); self.set_status("select inline shell"); } + config_picker::EDIT_TOOL_VALUE => { + let config = self.info.services.config_repository.load()?; + self.open_child_picker(config_picker::edit_tool_picker(config.edit_tool)); + self.set_status("select edit tool"); + } + value if value.starts_with(config_picker::EDIT_TOOL_PREFIX) => { + self.reject_edit_tool_change(); + } value if value.starts_with(config_picker::INLINE_SHELL_PREFIX) => { let shell = value[config_picker::INLINE_SHELL_PREFIX.len()..].to_string(); self.info.services.config_repository.update(|config| { diff --git a/crates/rho/src/tui/feed_image.rs b/crates/rho/src/tui/feed_image.rs index a766f569c..ef4537743 100644 --- a/crates/rho/src/tui/feed_image.rs +++ b/crates/rho/src/tui/feed_image.rs @@ -100,6 +100,10 @@ impl RenderedImagePlacements { } } + pub(super) fn from_placements(placements: Vec) -> Self { + Self { placements } + } + pub(super) fn iter(&self) -> impl Iterator { self.placements.iter() } diff --git a/crates/rho/src/tui/history_cache.rs b/crates/rho/src/tui/history_cache.rs index 15b8eb477..4e3c18653 100644 --- a/crates/rho/src/tui/history_cache.rs +++ b/crates/rho/src/tui/history_cache.rs @@ -3,7 +3,7 @@ use std::{ops::Range, sync::Arc}; use ratatui::text::Line; use super::{ - feed_image::{FeedImage, RenderedImagePlacements}, + feed_image::{FeedImage, RenderedImagePlacement, RenderedImagePlacements}, markdown::incremental_markdown_tail_start, markdown_image::MarkdownImageSource, message_render::render_assistant_content, @@ -62,6 +62,10 @@ pub(super) struct HistoryLineCache { code_blocks: Vec, image_placements: Vec, dirty_from: Option, + /// Entry indices to re-render in place (height may change). Applied on the + /// next `ensure_current` without rebuilding the history suffix when the + /// cache is already warm — used by tool expand/collapse. + resplice: Vec, appended_assistant: Option, /// When set, the last entry is still being streamed and must not own a trailing blank. open_stream_tail: bool, @@ -70,7 +74,36 @@ pub(super) struct HistoryLineCache { impl HistoryLineCache { pub(super) fn invalidate_from(&mut self, index: usize) { self.appended_assistant = None; - self.dirty_from = Some(self.dirty_from.map_or(index, |dirty| dirty.min(index))); + // Fold pending surgical marks into the suffix rebuild so an earlier + // resplice is not lost when a later invalidation starts. Marks at or + // after `index` are already covered by rebuilding from `index`. + let mut dirty = index; + for &resplice_index in &self.resplice { + dirty = dirty.min(resplice_index); + } + self.resplice.clear(); + self.dirty_from = Some( + self.dirty_from + .map_or(dirty, |existing| existing.min(dirty)), + ); + } + + /// Re-render these entries on the next paint without dropping the cached + /// suffix after them. Falls back to [`Self::invalidate_from`] when the + /// cache is cold or already suffix-dirty. + /// + /// Used when tool cards toggle expand/collapse (height changes, content of + /// later entries does not). + pub(super) fn resplice_entries(&mut self, indices: impl IntoIterator) { + self.appended_assistant = None; + if self.dirty_from.is_some() { + // Already doing a suffix rebuild; fold the earliest index into it. + for index in indices { + self.dirty_from = Some(self.dirty_from.map_or(index, |dirty| dirty.min(index))); + } + return; + } + self.resplice.extend(indices); } /// Suppress the trailing separator on the last entry while a stream is still open. @@ -83,13 +116,15 @@ impl HistoryLineCache { } self.open_stream_tail = open; if let Some(last) = self.entry_ranges.len().checked_sub(1) { - self.invalidate_from(last); + // Surgical: only the last entry's trailing blank changes. + self.resplice_entries([last]); } } pub(super) fn assistant_appended(&mut self, index: usize) { let can_extend = index + 1 == self.entry_ranges.len() && self.dirty_from.is_none() + && self.resplice.is_empty() && self .assistant_caches .get(index) @@ -199,6 +234,7 @@ impl HistoryLineCache { self.code_blocks.clear(); self.image_placements.clear(); self.appended_assistant = None; + self.resplice.clear(); self.dirty_from = Some(0); } @@ -208,6 +244,28 @@ impl HistoryLineCache { std::cmp::Ordering::Greater => self.invalidate_from(self.entry_ranges.len()), } + // Prefer surgical resplice when the cache is warm and only discrete + // entries changed height (tool expand/collapse). Fall back to a suffix + // rebuild if anything looks inconsistent. + if self.dirty_from.is_none() && !self.resplice.is_empty() { + let mut indices = std::mem::take(&mut self.resplice); + indices.sort_unstable(); + indices.dedup(); + if !self.try_resplice_entries(entries, &indices, settings, image_resolver) { + // try_resplice may have already forced a full rebuild (dirty 0). + if self.dirty_from.is_none() { + let min = indices.first().copied().unwrap_or(0); + self.dirty_from = Some(min); + } + } + } else if !self.resplice.is_empty() { + // Suffix rebuild wins; fold pending marks into the earliest dirty + // index so they are not dropped before the rebuild. + for index in self.resplice.drain(..) { + self.dirty_from = Some(self.dirty_from.map_or(index, |dirty| dirty.min(index))); + } + } + let Some(dirty_from) = self.dirty_from.take() else { return; }; @@ -233,57 +291,101 @@ impl HistoryLineCache { .collect(); for (entry_index, entry) in entries.iter().enumerate().skip(rebuild_from) { - let range_start = self.lines.len(); - if settings.hides_entry(entry) { - // Keep a zero-height range so entry indices stay aligned with history. - self.entry_ranges.push(range_start..range_start); - self.assistant_caches.push(None); - continue; + self.push_rendered_entry(entry_index, entry, entries.len(), settings, image_resolver); + } + } + + /// Re-render `indices` in place and shift later line offsets by the height + /// delta. Returns false when the cache cannot support a surgical update. + fn try_resplice_entries( + &mut self, + entries: &[Entry], + indices: &[usize], + settings: HistoryRenderSettings, + image_resolver: EntryImageResolver<'_>, + ) -> bool { + if indices.is_empty() { + return true; + } + if self.entry_ranges.len() != entries.len() + || self.assistant_caches.len() != entries.len() + || self.settings != Some(settings) + { + return false; + } + for &index in indices { + if index >= entries.len() || index >= self.entry_ranges.len() { + return false; } + } - let entry_start = self.lines.len(); - let trailing_blank = if self.open_stream_tail && entry_index + 1 == entries.len() { - TrailingBlank::Omit - } else { - TrailingBlank::Include - }; - let mut rendered = render_entry_with_options( + for &index in indices { + let old_range = self.entry_ranges[index].clone(); + if old_range.end > self.lines.len() || old_range.start > old_range.end { + return false; + } + + let entry = &entries[index]; + let (new_lines, new_code_blocks, new_image) = match prepare_cache_entry_render( entry, - settings.width, - settings.max_tool_output_lines, - trailing_blank, - ); - if !rendered.image_sources.is_empty() { - let images = image_resolver(entry_index, &rendered.image_sources); - apply_markdown_images(&mut rendered, &images, settings.width); + index, + entries.len(), + settings, + self.open_stream_tail, + image_resolver, + ) { + None => (Vec::new(), Vec::new(), None), + Some(rendered) => ( + rendered.lines, + rendered.code_blocks, + rendered.image_placement, + ), + }; + + let start = old_range.start; + let end = old_range.end; + let old_len = end - start; + let new_len = new_lines.len(); + let delta = new_len as isize - old_len as isize; + + self.lines.splice(start..end, new_lines); + + // Code blocks inside the old span are replaced; later ones shift. + self.code_blocks + .retain(|block| block.line < start || block.line >= end); + for block in &mut self.code_blocks { + if block.line >= end { + block.line = offset_usize(block.line, delta); + } } self.code_blocks - .extend( - rendered - .code_blocks - .into_iter() - .map(|block| CachedCodeBlock { - // Content starts at the first entry row; trailing blank is after. - line: entry_start.saturating_add(block.top_line), - // render_entry also pads markdown by one column on each side. - copy_columns: block.copy_columns.start.saturating_add(1) - ..block.copy_columns.end.saturating_add(1), - text: Arc::from(block.text), - }), - ); - if let Some(placement) = rendered.image_placement { - self.image_placements - .push(placement.offset_rows(entry_start)); + .extend(new_code_blocks.into_iter().map(|mut block| { + block.line = start.saturating_add(block.line); + block + })); + // Keep code_blocks ordered by line for stable hit-testing. + self.code_blocks.sort_by_key(|block| block.line); + + self.image_placements = shift_image_placements_for_splice( + &self.image_placements, + start, + end, + delta, + new_image.map(|placement| placement.offset_rows(start)), + ); + + self.entry_ranges[index] = start..start + new_len; + for range in self.entry_ranges.iter_mut().skip(index + 1) { + range.start = offset_usize(range.start, delta); + range.end = offset_usize(range.end, delta); } - self.lines.extend(rendered.lines); - self.entry_ranges.push(range_start..self.lines.len()); - // Only the last entry can be appended to, so only its cache is ever - // read (see `assistant_appended`). Building one for every entry would - // re-render each assistant message's stable prefix a second time, - // doubling the markdown work on every resize. - let is_last = entry_index + 1 == entries.len(); - self.assistant_caches.push(match entry { - Entry::Assistant(text) if is_last => { + + // Tool/reasoning toggles never own incremental assistant state. + // Last-assistant open-stream blank changes also clear it; rebuild + // only if this is still the last assistant entry. + self.assistant_caches[index] = None; + if index + 1 == entries.len() { + if let Entry::Assistant(text) = entry { let stable_source_len = incremental_markdown_tail_start(text); let stable_line_count = if stable_source_len == 0 { 0 @@ -292,14 +394,91 @@ impl HistoryLineCache { .lines .len() }; - Some(IncrementalAssistantCache { + self.assistant_caches[index] = Some(IncrementalAssistantCache { stable_source_len, stable_line_count, - }) + }); } - _ => None, - }); + } + } + + // Sanity: flat line buffer length matches last range end. + let ok = match self.entry_ranges.last() { + Some(last) => last.end == self.lines.len(), + None => self.lines.is_empty(), + }; + if !ok { + // Should be unreachable if ranges stayed coherent; force a full + // rebuild rather than paint a torn cache. + self.lines.clear(); + self.entry_ranges.clear(); + self.assistant_caches.clear(); + self.code_blocks.clear(); + self.image_placements.clear(); + self.dirty_from = Some(0); + return false; + } + true + } + + fn push_rendered_entry( + &mut self, + entry_index: usize, + entry: &Entry, + entries_len: usize, + settings: HistoryRenderSettings, + image_resolver: EntryImageResolver<'_>, + ) { + let range_start = self.lines.len(); + let Some(rendered) = prepare_cache_entry_render( + entry, + entry_index, + entries_len, + settings, + self.open_stream_tail, + image_resolver, + ) else { + // Keep a zero-height range so entry indices stay aligned with history. + self.entry_ranges.push(range_start..range_start); + self.assistant_caches.push(None); + return; + }; + + let entry_start = self.lines.len(); + self.code_blocks + .extend(rendered.code_blocks.into_iter().map(|mut block| { + // Content starts at the first entry row; trailing blank is after. + block.line = entry_start.saturating_add(block.line); + block + })); + if let Some(placement) = rendered.image_placement { + self.image_placements + .push(placement.offset_rows(entry_start)); } + self.lines.extend(rendered.lines); + self.entry_ranges.push(range_start..self.lines.len()); + // Only the last entry can be appended to, so only its cache is ever + // read (see `assistant_appended`). Building one for every entry would + // re-render each assistant message's stable prefix a second time, + // doubling the markdown work on every resize. + let is_last = entry_index + 1 == entries_len; + self.assistant_caches.push(match entry { + Entry::Assistant(text) if is_last => { + let stable_source_len = incremental_markdown_tail_start(text); + let stable_line_count = if stable_source_len == 0 { + 0 + } else { + render_assistant_content(&text[..stable_source_len], settings.width) + .lines + .len() + }; + Some(IncrementalAssistantCache { + stable_source_len, + stable_line_count, + }) + } + _ => None, + }); } fn try_extend_last_assistant(&mut self, entries: &[Entry], index: usize, width: usize) -> bool { @@ -391,6 +570,107 @@ impl HistoryLineCache { } } +fn offset_usize(value: usize, delta: isize) -> usize { + if delta >= 0 { + value.saturating_add(delta as usize) + } else { + value.saturating_sub((-delta) as usize) + } +} + +/// Shared entry render for full rebuild and surgical resplice paths. +/// +/// Returns `None` for hidden entries. Code-block line numbers and image +/// placements are relative to the entry start; callers relocate them. +struct PreparedCacheEntry { + lines: Vec>, + code_blocks: Vec, + image_placement: Option, +} + +fn prepare_cache_entry_render( + entry: &Entry, + entry_index: usize, + entries_len: usize, + settings: HistoryRenderSettings, + open_stream_tail: bool, + image_resolver: EntryImageResolver<'_>, +) -> Option { + if settings.hides_entry(entry) { + return None; + } + let trailing_blank = if open_stream_tail && entry_index + 1 == entries_len { + TrailingBlank::Omit + } else { + TrailingBlank::Include + }; + let mut rendered = render_entry_with_options( + entry, + settings.width, + settings.max_tool_output_lines, + trailing_blank, + ); + if !rendered.image_sources.is_empty() { + let images = image_resolver(entry_index, &rendered.image_sources); + apply_markdown_images(&mut rendered, &images, settings.width); + } + let code_blocks = rendered + .code_blocks + .into_iter() + .map(|block| CachedCodeBlock { + // Relative to entry start; callers offset when placing. + line: block.top_line, + // render_entry also pads markdown by one column on each side. + copy_columns: block.copy_columns.start.saturating_add(1) + ..block.copy_columns.end.saturating_add(1), + text: Arc::from(block.text), + }) + .collect(); + Some(PreparedCacheEntry { + lines: rendered.lines, + code_blocks, + image_placement: rendered.image_placement, + }) +} + +/// Drop placements overlapping `[start, end)`, shift those at/after `end` by +/// `delta`, then append any replacement placement for the respliced entry. +fn shift_image_placements_for_splice( + existing: &[RenderedImagePlacements], + start: usize, + end: usize, + delta: isize, + replacement: Option, +) -> Vec { + let mut out = Vec::with_capacity(existing.len() + usize::from(replacement.is_some())); + for group in existing { + let placements: Vec<_> = group + .iter() + .filter_map(|placement| { + if placement.rows.end <= start { + Some(placement.clone()) + } else if placement.rows.start >= end { + let offset_start = offset_usize(placement.rows.start, delta); + let offset_end = offset_usize(placement.rows.end, delta); + Some(RenderedImagePlacement { + image: placement.image.clone(), + rows: offset_start..offset_end, + }) + } else { + None + } + }) + .collect(); + if !placements.is_empty() { + out.push(RenderedImagePlacements::from_placements(placements)); + } + } + if let Some(replacement) = replacement { + out.push(replacement); + } + out +} + #[cfg(test)] #[path = "history_cache_tests.rs"] mod tests; diff --git a/crates/rho/src/tui/history_cache_tests.rs b/crates/rho/src/tui/history_cache_tests.rs index 1a087a3a8..e6eb9beb9 100644 --- a/crates/rho/src/tui/history_cache_tests.rs +++ b/crates/rho/src/tui/history_cache_tests.rs @@ -371,3 +371,122 @@ fn zen_mode_hides_tool_and_reasoning_lines_and_restores_them() { let restored = cache.line_count(&entries, settings(40), &no_images); assert_eq!(restored, full); } + +// Covers: tool expand/collapse resplices only the toggled card; later assistant +// markdown is not re-rendered (line identity of the suffix is preserved). +// Owner: history line cache surgical update +#[test] +fn resplice_tool_expand_preserves_later_assistant_lines() { + use crate::tui::ToolEntry; + use rho_tools::tool_card::{ + DiffRow, DiffRowKind, ToolBody, ToolCard, ToolFamily, ToolHeader, ToolStatus, + }; + + // Long body so collapsed vs expanded heights differ under max_tool_output_lines=2. + let rows: Vec<_> = (0..20) + .map(|i| DiffRow::new(DiffRowKind::Added, Some(i + 1), format!("line_{i}"))) + .collect(); + let card = ToolCard::new( + ToolStatus::Ok, + ToolFamily::FileDiff, + ToolHeader::call("str_replace", Some("f.rs".into())), + ) + .with_body(ToolBody::Diff(rows)); + + let mut entries = vec![ + Entry::User("go".into()), + Entry::Tool(ToolEntry { + card, + expanded: false, + image: None, + }), + Entry::Assistant("# big\n\n".to_string() + &"paragraph\n\n".repeat(30)), + ]; + + let mut cache = HistoryLineCache::default(); + let width = 40usize; + let max_lines = 2usize; + let s = settings_with(width, max_lines, false); + + let mut before = Vec::new(); + cache.extend_visible_lines( + &entries, + s, + HistoryLineSlice { + start: 0, + count: usize::MAX, + }, + &mut before, + &no_images, + ); + let assistant_range = cache.entry_ranges[2].clone(); + let assistant_before = before[assistant_range.clone()].to_vec(); + let total_before = before.len(); + + // Expand the tool surgically. + if let Entry::Tool(tool) = &mut entries[1] { + tool.expanded = true; + } + cache.resplice_entries([1]); + let mut after = Vec::new(); + cache.extend_visible_lines( + &entries, + s, + HistoryLineSlice { + start: 0, + count: usize::MAX, + }, + &mut after, + &no_images, + ); + + let assistant_range_after = cache.entry_ranges[2].clone(); + assert!( + after.len() > total_before, + "expanded tool should grow the transcript" + ); + assert_eq!( + &after[assistant_range_after.clone()], + &assistant_before[..], + "assistant suffix lines must be preserved by content" + ); + // Range must have shifted by the tool height delta. + let delta = after.len() as isize - total_before as isize; + assert_eq!( + assistant_range_after.start as isize, + assistant_range.start as isize + delta + ); + + // Full rebuild must match surgical result (correctness oracle). + cache.invalidate_from(0); + let mut rebuilt = Vec::new(); + cache.extend_visible_lines( + &entries, + s, + HistoryLineSlice { + start: 0, + count: usize::MAX, + }, + &mut rebuilt, + &no_images, + ); + assert_eq!(after, rebuilt); + + // Collapse again. + if let Entry::Tool(tool) = &mut entries[1] { + tool.expanded = false; + } + cache.resplice_entries([1]); + let mut collapsed = Vec::new(); + cache.extend_visible_lines( + &entries, + s, + HistoryLineSlice { + start: 0, + count: usize::MAX, + }, + &mut collapsed, + &no_images, + ); + assert_eq!(collapsed.len(), total_before); +} diff --git a/crates/rho/src/tui/model_actions.rs b/crates/rho/src/tui/model_actions.rs index 2d287c916..e41784de9 100644 --- a/crates/rho/src/tui/model_actions.rs +++ b/crates/rho/src/tui/model_actions.rs @@ -167,7 +167,7 @@ impl App { &self.info.runtime.provider, &self.info.runtime.auth, ) { - Ok(selection) => self.request_model_selection(selection, agent), + Ok(selection) => self.request_model_selection(selection, agent).await, Err(err) => { self.insert_entry(&Entry::Error(err.to_string())); self.set_status("model switch failed"); @@ -243,8 +243,9 @@ impl App { selected_value, agent, ) + .await } else { - self.request_model_selection(selection, agent) + self.request_model_selection(selection, agent).await } } Err(err) => { @@ -596,16 +597,16 @@ impl App { .map(|item| (picker.action, item.value.clone())) } - pub(super) fn select_model( + pub(super) async fn select_model( &mut self, resolved: InteractiveModelSelection, agent: &mut InteractiveRuntime, ) -> anyhow::Result<()> { - let _ = self.select_model_report(resolved, agent)?; + let _ = self.select_model_report(resolved, agent).await?; Ok(()) } - pub(super) fn select_model_report( + pub(super) async fn select_model_report( &mut self, resolved: InteractiveModelSelection, agent: &mut InteractiveRuntime, @@ -677,6 +678,10 @@ impl App { self.set_status("config save failed"); } } + // Auto edit preference follows the new provider while the session is + // idle (model switches never land mid-run). + self.apply_auto_edit_tool_for_provider(&provider, agent) + .await?; self.finish_setup_screen(); Ok(Some(handoff)) } diff --git a/crates/rho/src/tui/model_actions_tests.rs b/crates/rho/src/tui/model_actions_tests.rs index 29c09c29a..68a48d5a7 100644 --- a/crates/rho/src/tui/model_actions_tests.rs +++ b/crates/rho/src/tui/model_actions_tests.rs @@ -175,3 +175,116 @@ fn selecting_an_internal_agent_model_carries_explicit_reasoning() { Some(ReasoningLevel::High) ); } + +// Covers: select_model_report must apply Auto's provider-preferred edit format +// after a provider change, while a pinned preference stays put. +// Owner: model switch edit-tool handoff +#[tokio::test] +async fn select_model_report_auto_edit_tool_follows_provider_change() { + use std::sync::Arc; + + use rho_providers::credentials::{save_provider_api_key, MemoryCredentialStore}; + + use crate::{ + app::interactive_runtime::test_edit_tool_runtime, + config::EditTool, + tui::{tests::test_bootstrap, App, InteractiveRuntime}, + }; + + async fn switch_to_anthropic(app: &mut App, agent: &mut InteractiveRuntime) { + app.select_model_report( + InteractiveModelSelection { + selection: ModelSelection { + provider: "anthropic".into(), + model: "claude-fable-5".into(), + auth: "api-key".into(), + from_catalog: true, + }, + alias: None, + }, + agent, + ) + .await + .expect("model switch should succeed") + .expect("handoff report"); + } + + fn advertised_edit_name(agent: &InteractiveRuntime) -> Option<&'static str> { + ["edit", "apply_patch", "str_replace"] + .into_iter() + .find(|name| agent.has_tool(name)) + } + + // --- Auto follows the provider --- + let store = Arc::new(MemoryCredentialStore::default()); + save_provider_api_key(store.as_ref(), "openai", "sk-test").unwrap(); + save_provider_api_key(store.as_ref(), "anthropic", "sk-ant-test").unwrap(); + let mut app = App::new_with_credentials( + test_bootstrap(), + store, + crate::herdr::HerdrGraphicsCapability::NotHerdr, + crate::tools::mcp::McpSessionReport::default(), + crate::tools::mcp::McpCatalog::default(), + crate::plugins::PluginLoadReport::default(), + ); + app.info + .services + .config_repository + .update(|config| { + config.edit_tool = EditTool::Auto; + config.provider = "openai".into(); + config.model = "gpt-5.5".into(); + config.auth = "api-key".into(); + }) + .unwrap(); + + let mut agent = test_edit_tool_runtime(EditTool::Auto).await; + assert_eq!( + advertised_edit_name(&agent), + Some("edit"), + "Auto + openai should start on hashline (`edit`)" + ); + + switch_to_anthropic(&mut app, &mut agent).await; + assert_eq!(app.info.runtime.provider, "anthropic"); + assert_eq!( + advertised_edit_name(&agent), + Some("str_replace"), + "Auto must follow anthropic to str_replace after select_model_report" + ); + + // --- Pinned does not follow --- + let store = Arc::new(MemoryCredentialStore::default()); + save_provider_api_key(store.as_ref(), "openai", "sk-test").unwrap(); + save_provider_api_key(store.as_ref(), "anthropic", "sk-ant-test").unwrap(); + let mut app = App::new_with_credentials( + test_bootstrap(), + store, + crate::herdr::HerdrGraphicsCapability::NotHerdr, + crate::tools::mcp::McpSessionReport::default(), + crate::tools::mcp::McpCatalog::default(), + crate::plugins::PluginLoadReport::default(), + ); + app.info + .services + .config_repository + .update(|config| { + config.edit_tool = EditTool::Pinned(rho_tools::EditFormat::Hashline); + config.provider = "openai".into(); + config.model = "gpt-5.5".into(); + config.auth = "api-key".into(); + }) + .unwrap(); + + let mut agent = test_edit_tool_runtime(EditTool::Pinned(rho_tools::EditFormat::Hashline)).await; + assert_eq!(advertised_edit_name(&agent), Some("edit")); + + switch_to_anthropic(&mut app, &mut agent).await; + assert_eq!(app.info.runtime.provider, "anthropic"); + assert_eq!( + advertised_edit_name(&agent), + Some("edit"), + "pinned hashline must not follow the anthropic provider change" + ); + assert!(!agent.has_tool("str_replace")); +} diff --git a/crates/rho/src/tui/prompt_turn.rs b/crates/rho/src/tui/prompt_turn.rs index cd0a4be0c..aab60f6b1 100644 --- a/crates/rho/src/tui/prompt_turn.rs +++ b/crates/rho/src/tui/prompt_turn.rs @@ -581,7 +581,7 @@ impl App { self.preserve_unapplied_steering_as_follow_ups(); } self.clear_accepted_steering(); - self.apply_pending_model_selection(agent, completed)?; + self.apply_pending_model_selection(agent, completed).await?; if self.pending_subagent_questionnaire.is_some() { self.set_status(HerdrUserWait::Questionnaire.message()); } diff --git a/crates/rho/src/tui/syntax.rs b/crates/rho/src/tui/syntax.rs index 3bf52d29f..44f85f03d 100644 --- a/crates/rho/src/tui/syntax.rs +++ b/crates/rho/src/tui/syntax.rs @@ -23,6 +23,15 @@ static SYNTAX_SET: LazyLock = LazyLock::new(two_face::syntax::extra_n /// interactive. pub(in crate::tui) const MAX_TOOL_SYNTAX_LINES: usize = 2_500; +/// Soft cap on bytes per line for language-aware tool-card paint. Longer rows +/// keep solid add/remove/context colors. +/// +/// Syntect's Markdown grammar is pathological on dense inline-code spans: a +/// single ~800-byte docs prose line with many `` `backticks` `` can take tens +/// of milliseconds. Expand paints both diff sides on the UI thread, so one +/// long `.md` edit felt like a stall. Line-count caps alone do not catch this. +pub(in crate::tui) const MAX_TOOL_SYNTAX_LINE_BYTES: usize = 256; + // Per-thread counter of syntect line parses (highlight + advance). Thread-local // so parallel unit tests do not race the measurement. thread_local! { diff --git a/crates/rho/src/tui/tool_diff.rs b/crates/rho/src/tui/tool_diff.rs index 402429a24..d09f8d0b3 100644 --- a/crates/rho/src/tui/tool_diff.rs +++ b/crates/rho/src/tui/tool_diff.rs @@ -1,6 +1,8 @@ use rho_tools::tool_card::{DiffRow, DiffRowKind, ToolFamily, ToolHeader}; -use super::syntax::{BlockHighlighter, HighlightSegment, MAX_TOOL_SYNTAX_LINES}; +use super::syntax::{ + BlockHighlighter, HighlightSegment, MAX_TOOL_SYNTAX_LINES, MAX_TOOL_SYNTAX_LINE_BYTES, +}; /// Width of the line-number gutter for a diff body. /// @@ -103,7 +105,12 @@ impl DiffSyntax { if is_diff_chrome(&row.text) { return None; } - if !self.should_paint_content() { + if !self.should_paint_content_line(&row.text) { + // Long / over-budget lines skip syntect entirely. Restart so + // the next short line does not inherit a desynced stack. + if row.text.len() > MAX_TOOL_SYNTAX_LINE_BYTES { + self.restart(); + } return None; } // Advance old without segment alloc; styles come from new. @@ -119,9 +126,18 @@ impl DiffSyntax { self.highlighted_lines < MAX_TOOL_SYNTAX_LINES } + fn should_paint_content_line(&self, text: &str) -> bool { + self.should_paint_content() && text.len() <= MAX_TOOL_SYNTAX_LINE_BYTES + } + fn paint_content(&mut self, side: Side, text: &str) -> Option> { - if !self.should_paint_content() { - // Soft cap: plain row colors, no more syntect work this pass. + if !self.should_paint_content_line(text) { + // Soft caps: plain row colors, no more syntect work this pass. + // Over-long add/remove lines restart only their side so the other + // stream keeps multi-line token state. + if text.len() > MAX_TOOL_SYNTAX_LINE_BYTES { + self.restart_side(side); + } return None; } let segments = match side { @@ -154,6 +170,16 @@ impl DiffSyntax { self.new = BlockHighlighter::for_path(&path); } } + + fn restart_side(&mut self, side: Side) { + let Some(path) = self.path.clone() else { + return; + }; + match side { + Side::Old => self.old = BlockHighlighter::for_path(&path), + Side::New => self.new = BlockHighlighter::for_path(&path), + } + } } enum Side { diff --git a/crates/rho/src/tui/tool_diff_tests.rs b/crates/rho/src/tui/tool_diff_tests.rs index 2ba5b1032..41986d8ca 100644 --- a/crates/rho/src/tui/tool_diff_tests.rs +++ b/crates/rho/src/tui/tool_diff_tests.rs @@ -64,6 +64,56 @@ fn highlights_rust_tokens_after_file_row() { .any(|s| s.role.is_none() && s.style(plain) == plain)); } +// Covers: over-long rows skip syntect so expand stays interactive on dense +// Markdown prose (docs diffs with many inline `code` spans). +// Owner: pure unit (diff syntax line-byte budget) +#[test] +fn skips_language_paint_for_overlong_lines() { + use crate::tui::syntax::{ + reset_highlight_line_calls, take_highlight_line_calls, warm_syntax_set, + MAX_TOOL_SYNTAX_LINE_BYTES, + }; + + warm_syntax_set(); + let mut syntax = DiffSyntax::new(Some("docs/configuration.md")); + let long = format!( + "The `x` span. {}", + "word ".repeat(MAX_TOOL_SYNTAX_LINE_BYTES) + ); + assert!(long.len() > MAX_TOOL_SYNTAX_LINE_BYTES); + + reset_highlight_line_calls(); + let removed = DiffRow::new(DiffRowKind::Removed, Some(1), long.clone()); + assert!( + syntax.paint_row(&removed).is_none(), + "over-long removed line must stay plain" + ); + let added = DiffRow::new(DiffRowKind::Added, Some(1), long); + assert!( + syntax.paint_row(&added).is_none(), + "over-long added line must stay plain" + ); + assert_eq!( + take_highlight_line_calls(), + 0, + "over-long lines must not enter syntect" + ); + + // Short lines after a skipped long row still highlight (restart kept state sane). + let mut rust_syntax = DiffSyntax::new(Some("src/lib.rs")); + let long_rs = "a".repeat(MAX_TOOL_SYNTAX_LINE_BYTES + 1); + assert!(rust_syntax + .paint_row(&DiffRow::new(DiffRowKind::Removed, Some(1), long_rs)) + .is_none()); + let short = DiffRow::new(DiffRowKind::Added, Some(2), "let answer = 1;"); + let segments = rust_syntax + .paint_row(&short) + .expect("short line after long skip still paints"); + assert!(segments + .iter() + .any(|s| s.text.contains("let") && s.role == Some(SyntaxRole::Keyword))); +} + // Covers: /diff +++ headers switch language without a File row // Owner: pure unit (diff header path observe) #[test] diff --git a/crates/rho/src/tui/tool_output_ui.rs b/crates/rho/src/tui/tool_output_ui.rs index 58ef022a0..bc8d09901 100644 --- a/crates/rho/src/tui/tool_output_ui.rs +++ b/crates/rho/src/tui/tool_output_ui.rs @@ -69,18 +69,22 @@ impl App { pub(super) fn toggle_transcript_tool_output(&mut self, index: usize) { let expand = !matches!(self.history.get(index), Some(Entry::Tool(tool)) if tool.expanded); - let mut dirty_from = index; + let mut changed = Vec::new(); for (entry_index, entry) in self.history.entries_mut().iter_mut().enumerate() { if let Entry::Tool(tool) = entry { - if tool.expanded { - dirty_from = dirty_from.min(entry_index); + // Accordion: at most one tool body expanded. Only entries whose + // expanded bit actually flips need a cache resplice. + let next = expand && entry_index == index; + if tool.expanded != next { + tool.expanded = next; + changed.push(entry_index); } - tool.expanded = false; } } - if let Some(Entry::Tool(tool)) = self.history.get_mut(index) { - tool.expanded = expand; - self.history.lines_mut().invalidate_from(dirty_from); + if !changed.is_empty() { + // Surgical height update — do not rebuild assistant markdown (etc.) + // after the toggled tool(s). + self.history.lines_mut().resplice_entries(changed); } self.set_status(if expand { "tool output expanded" diff --git a/crates/rho/src/tui/tool_search.rs b/crates/rho/src/tui/tool_search.rs index 9c7164563..ab2ac4ddc 100644 --- a/crates/rho/src/tui/tool_search.rs +++ b/crates/rho/src/tui/tool_search.rs @@ -10,6 +10,7 @@ use super::{ syntax::{ match_byte_ranges, spans_from_segments_with_matches, spans_plain_with_matches, BlockHighlighter, HighlightSegment, MatchQuery, MAX_TOOL_SYNTAX_LINES, + MAX_TOOL_SYNTAX_LINE_BYTES, }, theme::Theme, }; @@ -99,6 +100,17 @@ impl SearchSyntax { } fn highlight_source(&mut self, source: &str) -> Vec { + if source.len() > MAX_TOOL_SYNTAX_LINE_BYTES { + // Match DiffSyntax: overlong rows skip paint and restart so the + // next short line does not inherit a desynced stack. + if let Some(path) = self.path.clone() { + self.highlighter = BlockHighlighter::for_path(&path); + } + return vec![HighlightSegment { + text: source.to_string(), + role: None, + }]; + } if self.highlighted_lines >= MAX_TOOL_SYNTAX_LINES { return vec![HighlightSegment { text: source.to_string(), diff --git a/crates/rho/tests/automation_cli.rs b/crates/rho/tests/automation_cli.rs index 120bd9b05..5e4d9815c 100644 --- a/crates/rho/tests/automation_cli.rs +++ b/crates/rho/tests/automation_cli.rs @@ -84,7 +84,8 @@ provider = "disabled" "list_dir", "read_file", "write", - "edit", + // Auto edit preference for xai resolves to str_replace. + "str_replace", "grep", "glob", "process", @@ -100,6 +101,8 @@ provider = "disabled" ); } assert!(!names.contains(&"web_search")); + assert!(!names.contains(&"edit")); + assert!(!names.contains(&"apply_patch")); let config = std::fs::read_to_string(root.path().join("config.toml")).unwrap(); // CLI overrides are session-only unless --save is set. diff --git a/docs/configuration.md b/docs/configuration.md index 97ddf7267..0f1a81fdf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -34,7 +34,7 @@ Unknown keys in `config.toml` are a load error so typos fail loudly. Values that In the [interactive TUI](/interactive-tui), [`/config`](/interactive-tui#commands) opens a category browser. **Models & reasoning** contains the conversation model, reasoning level, reasoning-output toggle, zen mode, and theme. **Agent behavior** contains permission mode, delegation, and advisor mode. **Context & limits** contains auto compaction and output limits. **Tools** contains the inline shell, edit tool, and Web search settings. **Providers** contains login, logout, and model-list refresh actions. **Updates** contains the startup update check. Type in the category browser to find a category by any setting it contains, then press `enter` to open it. Press `esc` to return to the category browser. -Settings save as soon as they change. The `permission_mode` row applies the selected policy before the next turn. The `reasoning` row cycles through `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, and `max` and applies to the current session. The `show_reasoning_output`, `zen_mode`, and `theme` rows apply immediately, including during the current model turn. The `check_for_updates` row controls startup checks against GitHub releases. The `enable_subagents` row applies to the next session. The `edit_tool` row applies on the next Rho startup. The `advisor_mode` row applies before the next turn; turning it on without an advisor model opens the model picker first. The auto-compaction rows edit its threshold and target percentages. The `max_output_bytes` row saves for the next session. +Settings save as soon as they change. The `permission_mode` row applies the selected policy before the next turn. The `reasoning` row cycles through `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, and `max` and applies to the current session. The `show_reasoning_output`, `zen_mode`, and `theme` rows apply immediately, including during the current model turn. The `check_for_updates` row controls startup checks against GitHub releases. The `enable_subagents` row applies to the next session. The `edit_tool` row applies before the next turn; Auto also follows provider changes mid-session. The `advisor_mode` row applies before the next turn; turning it on without an advisor model opens the model picker first. The auto-compaction rows edit its threshold and target percentages. The `max_output_bytes` row saves for the next session. [`/login`](/interactive-tui#commands), [`/logout`](/interactive-tui#commands), and [`/model`](/interactive-tui#commands) remain direct shortcuts for provider credentials and conversation-model selection. The corresponding `/config` rows provide the same picker flows. Use `/agents` to inspect reserved internal agents and configure their optional model overrides. Model pickers show entries from Rho's [model catalog](/authentication-and-models#selecting-models) and cached dynamic provider model lists for providers with available auth, and `/model provider/model` can switch explicitly. See the [provider pages](/authentication-and-models#providers) for per-provider auth and model details. @@ -166,22 +166,34 @@ Model aliases work in these entries. Rho keeps reading the old `[title]` section ## Edit tool -`edit_tool` under `[behavior]` selects the one file edit schema exposed to the model. It defaults to `hashline`. Supported values are: +`edit_tool` under `[behavior]` selects the file edit preference exposed to the model. It defaults to `auto`. Supported values are: | Value | Exposed tool | Format | | --- | --- | --- | +| `auto` | preferred for the active provider | Built-in catalog; switches when the provider changes | | `hashline` | `edit` | Snapshot-tagged, line-anchored `PUT` and `CUT` operations | | `apply_patch` | `apply_patch` | Codex-style, multi-file patch documents | | `str_replace` | `str_replace` | Exact `old_string` to `new_string` replacement in one file | -Only the selected tool is registered. Each format keeps its own model-facing name. Change it from **Tools** > **Edit tool** in `/config`, or set it directly: +Only one edit tool is registered at a time. Each concrete format keeps its own model-facing name. Change it from **Tools** > **Edit tool** in `/config`, or set it directly: ```toml [behavior] -edit_tool = "apply_patch" +edit_tool = "auto" ``` -The change applies on the next Rho startup because the process fixes tool schemas when it starts. Restart Rho after changing it. Use `hashline` when you want stale-file checks, `apply_patch` for models trained on that patch format, or `str_replace` for models that work best with exact string replacement. +`auto` is a preference, not a tool name. Rho keeps `auto` in config and advertises the preferred concrete format for the active chat provider. + +Many models learn to edit files inside a first-party harness that only offers one edit tool. Codex trains with `apply_patch`. Claude Code and several other agent stacks train with exact string replacement. Auto picks that familiar surface so the model uses the format it was trained on. Providers without a clear first-party match fall back to Rho's `hashline` `edit` tool. + +| Provider | Preferred format | Why | +| --- | --- | --- | +| `openai-codex` | `apply_patch` | Codex harness trains on Codex-style patches | +| `anthropic` | `str_replace` | Claude Code harness trains on exact string replace | +| `xai` | `str_replace` | First-party agent tooling favors string replace | +| all others | `hashline` | Rho default when no first-party match is known | + +Pinned values (`hashline`, `apply_patch`, `str_replace`) stay fixed across provider changes. From `/config`, the change applies before the next turn: the tool list rebuilds and the session gets a short notice with the new tool schema. Auto mode also applies that live switch when you change providers mid-session. Direct `config.toml` edits still need a restart. Pin a format when you want one surface for every provider. ## Web search diff --git a/docs/configuration/full-example.md b/docs/configuration/full-example.md index 93f2b238e..79b7f8122 100644 --- a/docs/configuration/full-example.md +++ b/docs/configuration/full-example.md @@ -61,7 +61,7 @@ advisor_mode = false check_for_updates = true enable_subagents = true experimental_workspace_rewind = false -edit_tool = "hashline" # hashline, apply_patch, or str_replace +edit_tool = "auto" # auto, hashline, apply_patch, or str_replace permission_mode = "auto" # auto, plan, or supervised rtk = true inline_shell = "bash" # bash default on macOS/Linux; powershell on Windows diff --git a/docs/tools-workspace.md b/docs/tools-workspace.md index 601161633..9ea5aedd4 100644 --- a/docs/tools-workspace.md +++ b/docs/tools-workspace.md @@ -29,7 +29,7 @@ Core workspace tools on every platform: | `grep` | Search file contents with a regex (in-process) | | `glob` | List paths that match a glob (in-process) | -Rho exposes exactly one edit tool per session. Select it with [`behavior.edit_tool`](/configuration#edit-tool) or `/config` > **Tools** > **Edit tool**. The default is `hashline`, which exposes the hash-line `edit` tool. +Rho exposes exactly one edit tool at a time. Select it with [`behavior.edit_tool`](/configuration#edit-tool) or `/config` > **Tools** > **Edit tool**. The default is `auto`, which picks the format the active provider's models were trained to use in their first-party harness (`apply_patch` for Codex, `str_replace` for Anthropic and xAI, `hashline` otherwise). See [Edit tool](/configuration#edit-tool) for the full catalog and pin options. Additional tools: diff --git a/docs/tools-workspace/edit-format.md b/docs/tools-workspace/edit-format.md index fa6099ef8..a3f0010aa 100644 --- a/docs/tools-workspace/edit-format.md +++ b/docs/tools-workspace/edit-format.md @@ -2,7 +2,7 @@ Parent: [Tools and workspace](/tools-workspace). -This page applies when [`behavior.edit_tool`](/configuration#edit-tool) is `hashline`, the default. +This page applies when the resolved edit format is `hashline` (the default for most providers under `edit_tool = "auto"`, or when you pin `edit_tool = "hashline"`). `edit` changes existing UTF-8 files with line-anchored hunks. You pass one hashline document in `input`. Each section names a path and a snapshot tag from diff --git a/fixtures/downstream/Cargo.lock b/fixtures/downstream/Cargo.lock index c98d44e7d..237815a46 100644 --- a/fixtures/downstream/Cargo.lock +++ b/fixtures/downstream/Cargo.lock @@ -16,13 +16,13 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "displaydoc" -version = "0.2.6" +version = "0.2.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ac70aa55017e108007fbaf5aa0f54b021c98f92ff8af59d42eda9da96e3dd4f" +checksum = "c6232dd377dcc64799954cbd3a9bb882e9cdc1308ccd87b1c098f1fb2eaf82a8" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.3", ] [[package]] @@ -36,21 +36,21 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" +checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" [[package]] name = "futures-task" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" +checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" [[package]] name = "futures-util" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" dependencies = [ "futures-core", "futures-task", @@ -180,9 +180,9 @@ checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" [[package]] name = "js-sys" -version = "0.3.103" +version = "0.3.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" +checksum = "0e0c1080212aad755ea003d18543e8768dd432c48819efd73a7bf1e39b7a5a3a" dependencies = [ "cfg-if", "futures-util", @@ -191,9 +191,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.186" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "litemap" @@ -236,18 +236,18 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.106" +version = "1.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.46" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfbc457d0c7a0759a614551b11a6409e5951f6c7537be1f1b7682b9ae9230368" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" dependencies = [ "proc-macro2", ] @@ -260,7 +260,7 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rho-sdk" -version = "1.17.3" +version = "1.18.0" dependencies = [ "serde", "serde_json", @@ -293,9 +293,9 @@ checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" [[package]] name = "serde" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" dependencies = [ "serde_core", "serde_derive", @@ -303,29 +303,29 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.3", ] [[package]] name = "serde_json" -version = "1.0.150" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8014e44b4736ed0538adeecded0fce2a272f22dc9578a7eb6b2d9993c74cfb9" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", @@ -363,6 +363,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "synstructure" version = "0.13.2" @@ -371,27 +382,27 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", ] [[package]] name = "thiserror" -version = "2.0.18" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.18" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.3", ] [[package]] @@ -406,9 +417,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.52.3" +version = "1.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" +checksum = "202caea871b69668250d242070849eb495be178ed697a3e98aebce5bc81a0bed" dependencies = [ "pin-project-lite", "tokio-macros", @@ -416,13 +427,13 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.7.0" +version = "2.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" +checksum = "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.3", ] [[package]] @@ -444,7 +455,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", ] [[package]] @@ -482,9 +493,9 @@ checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" [[package]] name = "uuid" -version = "1.23.5" +version = "1.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea5fab0d6c3c01ae70085a09cb03d4c7a1d6314e2b3e075392783396d724ca0a" +checksum = "bf3923a6f5c4c6382e0b653c4117f48d631ea17f38ed86e2a828e6f7412f5239" dependencies = [ "getrandom", "js-sys", @@ -494,9 +505,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" +checksum = "1b70935747edd64d89de3efa29d73789b806c15798f8e7dca4d8ac356b50ce70" dependencies = [ "cfg-if", "once_cell", @@ -507,9 +518,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" +checksum = "77775f8f3f7217702089053b94958f8f54061a3f663417df76e19cbdcca29bc1" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -517,22 +528,22 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" +checksum = "e11d33f857dc2fb11b8bc75aee111aa9cbeb12cd9f25efd3d4c2a3dd4e235284" dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn", + "syn 2.0.119", "wasm-bindgen-shared", ] [[package]] name = "wasm-bindgen-shared" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" +checksum = "7ef64dbcc55df09c7e5a46182d181c2cfa3e925f3da937ea764728b4bbb9dcbf" dependencies = [ "unicode-ident", ] @@ -562,7 +573,7 @@ checksum = "de844c262c8848816172cef550288e7dc6c7b7814b4ee56b3e1553f275f1858e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", "synstructure", ] @@ -583,7 +594,7 @@ checksum = "11532158c46691caf0f2593ea8358fed6bbf68a0315e80aae9bd41fbade684a1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", "synstructure", ] @@ -617,7 +628,7 @@ checksum = "625dc425cab0dca6dc3c3319506e6593dcb08a9f387ea3b284dbd52a92c40555" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.119", ] [[package]]