mirror of
https://github.com/block/buzz.git
synced 2026-11-08 18:31:56 +00:00
fix(desktop): admit buzz_agent logs to per-agent log files
buzz-acp inherits its stderr to the agent subprocess (acp.rs:463), so the desktop's RUST_LOG filter for the harness also decides what the agent can write to disk. It named only buzz_acp, so every buzz-agent diagnostic was filtered out before reaching the per-agent log. That is invisible until something goes wrong: an agent misbehaving in the desktop left no trace of compaction, the _Stop veto, max-rounds, or provider errors -- exactly the lines you need to tell whether a turn did what it should. Found while trying to confirm from the logs alone that a desktop agent was running the new loop; the logs could not answer it. Adds buzz_agent=info to the default, appends only the missing directive when an operator filter is present (so RUST_LOG=buzz_agent=debug is not downgraded to info), and takes a fully-specified filter verbatim. Co-authored-by: Michael Neale <michael.neale@gmail.com> Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -1055,23 +1055,11 @@ pub fn run() -> Result<(), Box<dyn std::error::Error>> {
|
||||
/// The cached token is what lets both the agent loop and the desktop model
|
||||
/// picker work without a static `DATABRICKS_TOKEN`.
|
||||
async fn auth_subcommand(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
|
||||
use buzz_model_catalog::auth::{PkceOAuthConfig, PkceOAuthTokenSource};
|
||||
|
||||
match args.first().map(String::as_str) {
|
||||
Some("databricks" | "databricks_v2" | "databricks-v2") => {
|
||||
let host = std::env::var("DATABRICKS_HOST")
|
||||
.map_err(|_| "auth databricks: DATABRICKS_HOST required")?;
|
||||
let pkce = PkceOAuthConfig {
|
||||
discovery_url: format!(
|
||||
"{}/oidc/.well-known/oauth-authorization-server",
|
||||
host.trim_end_matches('/')
|
||||
),
|
||||
client_id: "databricks-cli".into(),
|
||||
scopes: vec!["all-apis".into(), "offline_access".into()],
|
||||
cache_namespace: "databricks".into(),
|
||||
cache_dir_override: None,
|
||||
};
|
||||
PkceOAuthTokenSource::new(pkce)?.interactive_login().await?;
|
||||
buzz_model_catalog::authenticate_databricks(&host).await?;
|
||||
eprintln!("Authenticated. Token cached under ~/.config/buzz-agent/oauth/databricks/.");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -22,6 +22,33 @@ pub use catalog::{discover_databricks_models, ModelEntry, DATABRICKS_V2_KNOWN_MO
|
||||
pub use config::{Config, Provider};
|
||||
pub use types::AgentError;
|
||||
|
||||
/// PKCE configuration for Databricks OAuth against `host`.
|
||||
///
|
||||
/// Shared so the desktop model picker and `buzz-agent auth databricks` cache
|
||||
/// tokens in the same place.
|
||||
pub fn databricks_pkce_config(host: &str) -> auth::PkceOAuthConfig {
|
||||
auth::PkceOAuthConfig {
|
||||
discovery_url: format!(
|
||||
"{}/oidc/.well-known/oauth-authorization-server",
|
||||
host.trim_end_matches('/')
|
||||
),
|
||||
client_id: "databricks-cli".into(),
|
||||
scopes: vec!["all-apis".into(), "offline_access".into()],
|
||||
cache_namespace: "databricks".into(),
|
||||
cache_dir_override: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the interactive Databricks OAuth PKCE login and cache the token.
|
||||
///
|
||||
/// Needs a browser on the machine. The desktop model picker calls this when
|
||||
/// discovery fails with an auth error.
|
||||
pub async fn authenticate_databricks(host: &str) -> Result<(), AgentError> {
|
||||
auth::PkceOAuthTokenSource::new(databricks_pkce_config(host))?
|
||||
.interactive_login()
|
||||
.await
|
||||
}
|
||||
|
||||
/// Environment keys the Windows Git Bash resolver may inspect.
|
||||
///
|
||||
/// The MCP child is spawned with an otherwise-cleared environment, so every
|
||||
|
||||
692
desktop/src-tauri/Cargo.lock
generated
692
desktop/src-tauri/Cargo.lock
generated
File diff suppressed because it is too large
Load Diff
@@ -911,11 +911,90 @@ pub fn spawn_agent_child(
|
||||
})
|
||||
}
|
||||
|
||||
/// Log filter for the spawned harness and, through it, the agent.
|
||||
///
|
||||
/// `buzz-acp` inherits its stderr to the agent subprocess (`acp.rs:463`), so
|
||||
/// whatever this filter admits is what lands in the per-agent log file. That
|
||||
/// makes `buzz_agent` as load-bearing as `buzz_acp`: without it the agent's own
|
||||
/// turn diagnostics — compaction, the `_Stop` veto, max-rounds — are filtered
|
||||
/// out before they reach disk, and an agent misbehaving in the desktop leaves
|
||||
/// no trace to debug it with.
|
||||
fn child_rust_log_filter() -> String {
|
||||
const DEFAULTS: &str = "buzz_acp=info,buzz_agent=info";
|
||||
match std::env::var("RUST_LOG") {
|
||||
Ok(existing) if existing.contains("buzz_acp") => existing,
|
||||
Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"),
|
||||
_ => "buzz_acp=info".to_string(),
|
||||
// An operator filter that already names both crates is taken verbatim.
|
||||
Ok(existing) if existing.contains("buzz_acp") && existing.contains("buzz_agent") => {
|
||||
existing
|
||||
}
|
||||
// Otherwise append only the directive that is missing, so an explicit
|
||||
// `buzz_agent=debug` is not overridden by our default.
|
||||
Ok(existing) if !existing.trim().is_empty() => {
|
||||
let mut filter = existing;
|
||||
if !filter.contains("buzz_acp") {
|
||||
filter.push_str(",buzz_acp=info");
|
||||
}
|
||||
if !filter.contains("buzz_agent") {
|
||||
filter.push_str(",buzz_agent=info");
|
||||
}
|
||||
filter
|
||||
}
|
||||
_ => DEFAULTS.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod rust_log_filter_tests {
|
||||
use super::child_rust_log_filter;
|
||||
|
||||
/// Serialize: `RUST_LOG` is process-global and these tests mutate it.
|
||||
fn with_rust_log<T>(value: Option<&str>, body: impl FnOnce() -> T) -> T {
|
||||
static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
let _guard = LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let previous = std::env::var("RUST_LOG").ok();
|
||||
match value {
|
||||
Some(v) => std::env::set_var("RUST_LOG", v),
|
||||
None => std::env::remove_var("RUST_LOG"),
|
||||
}
|
||||
let result = body();
|
||||
match previous {
|
||||
Some(v) => std::env::set_var("RUST_LOG", v),
|
||||
None => std::env::remove_var("RUST_LOG"),
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_admits_both_the_harness_and_the_agent() {
|
||||
// buzz_agent was missing here, which silently discarded every agent
|
||||
// turn diagnostic from the desktop's per-agent log files.
|
||||
let filter = with_rust_log(None, child_rust_log_filter);
|
||||
assert!(filter.contains("buzz_acp=info"));
|
||||
assert!(filter.contains("buzz_agent=info"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unrelated_operator_filter_is_extended_not_replaced() {
|
||||
let filter = with_rust_log(Some("warn"), child_rust_log_filter);
|
||||
assert!(filter.starts_with("warn"), "operator filter must survive");
|
||||
assert!(filter.contains("buzz_acp=info"));
|
||||
assert!(filter.contains("buzz_agent=info"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_explicit_agent_directive_is_not_downgraded() {
|
||||
let filter = with_rust_log(Some("buzz_agent=debug"), child_rust_log_filter);
|
||||
assert!(filter.contains("buzz_agent=debug"));
|
||||
assert!(!filter.contains("buzz_agent=info"), "must not override");
|
||||
assert!(filter.contains("buzz_acp=info"), "still needs the harness");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_fully_specified_filter_is_taken_verbatim() {
|
||||
let filter = with_rust_log(
|
||||
Some("buzz_acp=warn,buzz_agent=trace"),
|
||||
child_rust_log_filter,
|
||||
);
|
||||
assert_eq!(filter, "buzz_acp=warn,buzz_agent=trace");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user