mirror of
https://github.com/block/buzz.git
synced 2026-10-31 14:31:02 +00:00
Merge remote-tracking branch 'origin/main' into projects-design-v3-layout-test
Signed-off-by: Thomas Petersen <thomasp@squareup.com>
This commit is contained in:
4
Cargo.lock
generated
4
Cargo.lock
generated
@@ -10833,9 +10833,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "webbrowser"
|
||||
version = "1.2.2"
|
||||
version = "1.2.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ef62a3d5f7b2411119a11b6f62570dbff91d7105e011a20fb83fbf8f5761c40f"
|
||||
checksum = "62c35be770821a214dbc362fc26908c853e776c0004294d0b10b8a6bad582f94"
|
||||
dependencies = [
|
||||
"jni 0.22.4",
|
||||
"log",
|
||||
|
||||
@@ -5031,6 +5031,7 @@ mod author_gate_tests {
|
||||
relay::ChannelInfo {
|
||||
name: "dm".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
@@ -5038,6 +5039,7 @@ mod author_gate_tests {
|
||||
relay::ChannelInfo {
|
||||
name: "stream".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
]);
|
||||
@@ -5054,6 +5056,7 @@ mod author_gate_tests {
|
||||
relay::ChannelInfo {
|
||||
name: "unknown".into(),
|
||||
channel_type: "unknown".into(),
|
||||
description: None,
|
||||
},
|
||||
)]);
|
||||
assert!(
|
||||
|
||||
@@ -524,6 +524,7 @@ impl ChannelInfoResolver {
|
||||
PromptChannelInfo {
|
||||
name: info.name,
|
||||
channel_type: info.channel_type,
|
||||
description: info.description,
|
||||
},
|
||||
))
|
||||
})
|
||||
@@ -2577,17 +2578,25 @@ pub(crate) async fn fetch_channel_info(
|
||||
let ev = events.first()?;
|
||||
let tags = ev.get("tags")?.as_array()?;
|
||||
let mut name = None;
|
||||
let mut description = None;
|
||||
for tag in tags {
|
||||
if let Some(arr) = tag.as_array() {
|
||||
if arr.first().and_then(|v| v.as_str()) == Some("name") {
|
||||
name = arr.get(1).and_then(|v| v.as_str());
|
||||
match arr.first().and_then(|v| v.as_str()) {
|
||||
Some("name") => name = arr.get(1).and_then(|v| v.as_str()),
|
||||
Some("about") => description = arr.get(1).and_then(|v| v.as_str()),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
let channel_type = crate::relay::channel_type_from_tags(tags);
|
||||
let description = description
|
||||
.map(|s| s.trim())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(str::to_string);
|
||||
Some(PromptChannelInfo {
|
||||
name: name.unwrap_or(UNKNOWN_CHANNEL_NAME).to_string(),
|
||||
channel_type,
|
||||
description,
|
||||
})
|
||||
}
|
||||
Ok(Err(e)) => {
|
||||
@@ -5803,6 +5812,7 @@ done"#
|
||||
crate::relay::ChannelInfo {
|
||||
name: "test-dm".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
},
|
||||
)]),
|
||||
RestClient {
|
||||
@@ -5964,6 +5974,7 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'"
|
||||
crate::relay::ChannelInfo {
|
||||
name: "test-dm".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
},
|
||||
)]),
|
||||
RestClient {
|
||||
@@ -7855,6 +7866,38 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'"
|
||||
server.abort();
|
||||
}
|
||||
|
||||
/// A channel's `about` tag is parsed through the lazy-fetch path and
|
||||
/// delivered as the resolved description.
|
||||
#[tokio::test]
|
||||
async fn test_channel_resolver_delivers_description() {
|
||||
let id = Uuid::new_v4();
|
||||
let response = channel_metadata_response(
|
||||
id,
|
||||
&[
|
||||
["name", "team-chat"],
|
||||
["t", "stream"],
|
||||
["about", "Engineering discussions"],
|
||||
],
|
||||
);
|
||||
let (resolver, _requests, server) = counting_resolver(response).await;
|
||||
|
||||
let info = resolver.resolve(id).await.expect("should resolve");
|
||||
assert_eq!(info.description.as_deref(), Some("Engineering discussions"));
|
||||
server.abort();
|
||||
}
|
||||
|
||||
/// A metadata event with no `about` tag yields no description.
|
||||
#[tokio::test]
|
||||
async fn test_channel_resolver_absent_description_when_no_about_tag() {
|
||||
let id = Uuid::new_v4();
|
||||
let response = channel_metadata_response(id, &[["name", "buzz-dev"], ["t", "stream"]]);
|
||||
let (resolver, _requests, server) = counting_resolver(response).await;
|
||||
|
||||
let info = resolver.resolve(id).await.expect("should resolve");
|
||||
assert_eq!(info.description, None);
|
||||
server.abort();
|
||||
}
|
||||
|
||||
/// A DM carries no useful name, so it gets the bare agent title (and no
|
||||
/// canvas section).
|
||||
#[tokio::test]
|
||||
|
||||
@@ -1003,6 +1003,8 @@ pub struct ContextMessage {
|
||||
pub struct PromptChannelInfo {
|
||||
pub name: String,
|
||||
pub channel_type: String,
|
||||
/// Channel description from the kind-39000 `about` tag, if present.
|
||||
pub description: Option<String>,
|
||||
}
|
||||
|
||||
/// Minimal profile fields needed to label users in ACP prompts.
|
||||
@@ -1231,6 +1233,48 @@ fn resolve_reply_anchor(
|
||||
)
|
||||
}
|
||||
|
||||
/// Maximum length (in characters) of a channel description rendered into `[Context]`.
|
||||
///
|
||||
/// Limits prompt bloat from unusually long descriptions; a raw embedded newline
|
||||
/// in a description must not be able to spoof another `[Context]` field, so
|
||||
/// multiline text is collapsed to single-space-joined lines before truncation.
|
||||
const MAX_DESCRIPTION_LEN: usize = 500;
|
||||
|
||||
/// Append a `Description: …` line to a `[Context]` block when non-empty.
|
||||
///
|
||||
/// Collapses internal newlines (any `\r\n`, `\r`, or `\n`) to a single space
|
||||
/// so a multi-line description cannot inject a fake `[Context]` field line.
|
||||
/// Truncates at [`MAX_DESCRIPTION_LEN`] characters with a `…` marker.
|
||||
fn append_channel_description(s: &mut String, channel_info: Option<&PromptChannelInfo>) {
|
||||
let desc = match channel_info.and_then(|ci| ci.description.as_deref()) {
|
||||
Some(d) if !d.is_empty() => d,
|
||||
_ => return,
|
||||
};
|
||||
// Collapse newlines to spaces so the description can never spoof another field.
|
||||
let collapsed: String = desc
|
||||
.split(['\n', '\r'])
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
if collapsed.is_empty() {
|
||||
return;
|
||||
}
|
||||
// Truncate at a character boundary (not byte boundary) to avoid splitting
|
||||
// multi-byte sequences.
|
||||
let truncated = if collapsed.chars().count() > MAX_DESCRIPTION_LEN {
|
||||
let end = collapsed
|
||||
.char_indices()
|
||||
.nth(MAX_DESCRIPTION_LEN)
|
||||
.map(|(i, _)| i)
|
||||
.unwrap_or(collapsed.len());
|
||||
format!("{}…", &collapsed[..end])
|
||||
} else {
|
||||
collapsed
|
||||
};
|
||||
s.push_str(&format!("\nDescription: {truncated}"));
|
||||
}
|
||||
|
||||
/// Format a `[Context]` hints section based on event scope.
|
||||
///
|
||||
/// `reply_anchor` is the pre-resolved `--reply-to` target for this turn (see
|
||||
@@ -1301,9 +1345,10 @@ fn format_context_hints(
|
||||
let mut s = format!(
|
||||
"[Context]\n\
|
||||
Scope: thread\n\
|
||||
Channel: {channel_display}\n\
|
||||
Thread root: {root}"
|
||||
Channel: {channel_display}"
|
||||
);
|
||||
append_channel_description(&mut s, channel_info);
|
||||
s.push_str(&format!("\nThread root: {root}"));
|
||||
if let Some(ref parent) = thread_tags.parent_event_id {
|
||||
if parent != root {
|
||||
s.push_str(&format!("\nParent: {parent}"));
|
||||
@@ -1318,8 +1363,11 @@ fn format_context_hints(
|
||||
let mut s = format!(
|
||||
"[Context]\n\
|
||||
Scope: channel\n\
|
||||
Channel: {channel_display}\n\
|
||||
Hint: Use `buzz messages get --channel <UUID>` for recent messages if needed."
|
||||
Channel: {channel_display}"
|
||||
);
|
||||
append_channel_description(&mut s, channel_info);
|
||||
s.push_str(
|
||||
"\nHint: Use `buzz messages get --channel <UUID>` for recent messages if needed.",
|
||||
);
|
||||
if let Some(event_id) = reply_anchor {
|
||||
append_new_thread_reply_instruction(&mut s, event_id);
|
||||
@@ -3087,6 +3135,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "engineering".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
let prompt = format_prompt(
|
||||
@@ -3118,6 +3167,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
let prompt = format_prompt(
|
||||
@@ -3230,6 +3280,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
let ctx = ConversationContext::Dm {
|
||||
messages: vec![ContextMessage {
|
||||
@@ -3488,6 +3539,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
// Thread context fetched (as the fetch path does for DM replies).
|
||||
let ctx = ConversationContext::Thread {
|
||||
@@ -3587,6 +3639,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
let trigger_only_prompt = format_prompt(
|
||||
@@ -3635,6 +3688,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
// No context fetched — hints only.
|
||||
@@ -4130,6 +4184,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
let prompt = format_prompt(
|
||||
@@ -4193,6 +4248,7 @@ mod tests {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: None,
|
||||
};
|
||||
|
||||
let prompt = format_prompt(
|
||||
@@ -4961,4 +5017,254 @@ mod tests {
|
||||
"second extend must not move deadline backward (monotonic)"
|
||||
);
|
||||
}
|
||||
|
||||
// ── channel description delivery ─────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_adds_description_line() {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some("Engineering discussions".into()),
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel\nChannel: team (#abc)".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
assert!(
|
||||
s.contains("\nDescription: Engineering discussions"),
|
||||
"description must be appended; got: {s}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_absent_when_none() {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: None,
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
assert!(
|
||||
!s.contains("Description:"),
|
||||
"no description must be appended when None; got: {s}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_absent_when_channel_info_none() {
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, None);
|
||||
assert!(
|
||||
!s.contains("Description:"),
|
||||
"no description must be appended when channel_info is None; got: {s}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_collapses_newlines_spoof_prevention() {
|
||||
// A multiline description must not be able to inject a fake [Context] field.
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some("Line one\nScope: injected\nLine two".into()),
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
// The whole description is on a single Description line — no injected field.
|
||||
let desc_line = s.lines().find(|l| l.starts_with("Description:")).unwrap();
|
||||
assert_eq!(
|
||||
desc_line, "Description: Line one Scope: injected Line two",
|
||||
"multiline description must collapse to one line, never a fake field"
|
||||
);
|
||||
assert_eq!(
|
||||
s.lines().filter(|l| l.starts_with("Description:")).count(),
|
||||
1,
|
||||
"exactly one Description line is rendered"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_truncates_at_cap() {
|
||||
let long_desc = "x".repeat(600);
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some(long_desc),
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
let desc_line = s.lines().find(|l| l.starts_with("Description:")).unwrap();
|
||||
assert!(
|
||||
desc_line.ends_with('…'),
|
||||
"truncated description must end with '…'; got: {desc_line}"
|
||||
);
|
||||
// Value = first MAX_DESCRIPTION_LEN chars + the "…" marker.
|
||||
let value = desc_line.strip_prefix("Description: ").unwrap();
|
||||
assert_eq!(
|
||||
value.chars().count(),
|
||||
MAX_DESCRIPTION_LEN + 1,
|
||||
"truncated value is exactly the cap plus the ellipsis marker"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_multibyte_truncation_is_char_safe() {
|
||||
// Truncation must land on a char boundary, never split a multi-byte code point.
|
||||
let long_desc = "é".repeat(600);
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some(long_desc),
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
let desc_line = s.lines().find(|l| l.starts_with("Description:")).unwrap();
|
||||
let value = desc_line.strip_prefix("Description: ").unwrap();
|
||||
assert_eq!(value.chars().count(), MAX_DESCRIPTION_LEN + 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_channel_description_whitespace_only_is_absent() {
|
||||
let ci = PromptChannelInfo {
|
||||
name: "team".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some("\n \r\n \n".into()),
|
||||
};
|
||||
let mut s = "[Context]\nScope: channel".to_string();
|
||||
append_channel_description(&mut s, Some(&ci));
|
||||
assert!(
|
||||
!s.contains("Description:"),
|
||||
"a whitespace-only description collapses to empty and is not rendered; got: {s}"
|
||||
);
|
||||
}
|
||||
|
||||
fn description_batch(ch: Uuid, event: Event) -> FlushBatch {
|
||||
FlushBatch {
|
||||
channel_id: ch,
|
||||
events: vec![BatchEvent {
|
||||
event,
|
||||
prompt_tag: "test".into(),
|
||||
received_at: Instant::now(),
|
||||
}],
|
||||
cancelled_events: vec![],
|
||||
cancel_reason: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_prompt_includes_description_in_context_for_channel_turn() {
|
||||
let ch = Uuid::new_v4();
|
||||
let batch = description_batch(ch, make_event("what should we build?"));
|
||||
let ci = PromptChannelInfo {
|
||||
name: "engineering".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some("Engineering discussions and planning.".into()),
|
||||
};
|
||||
let prompt = format_prompt(
|
||||
&batch,
|
||||
&FormatPromptArgs {
|
||||
channel_info: Some(&ci),
|
||||
has_system_prompt_support: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.join("\n\n");
|
||||
assert!(
|
||||
prompt.contains("Scope: channel"),
|
||||
"channel-scope turn expected; got: {prompt}"
|
||||
);
|
||||
assert!(
|
||||
prompt.contains("Description: Engineering discussions and planning."),
|
||||
"description must appear in [Context] for channel turns; got: {prompt}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_prompt_includes_description_in_context_for_thread_turn() {
|
||||
let ch = Uuid::new_v4();
|
||||
let event = make_event_with_tags(
|
||||
"reply in thread",
|
||||
vec![vec![
|
||||
"e".into(),
|
||||
"root123".into(),
|
||||
"".into(),
|
||||
"reply".into(),
|
||||
]],
|
||||
);
|
||||
let batch = description_batch(ch, event);
|
||||
let ci = PromptChannelInfo {
|
||||
name: "engineering".into(),
|
||||
channel_type: "stream".into(),
|
||||
description: Some("Engineering discussions and planning.".into()),
|
||||
};
|
||||
let prompt = format_prompt(
|
||||
&batch,
|
||||
&FormatPromptArgs {
|
||||
channel_info: Some(&ci),
|
||||
has_system_prompt_support: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.join("\n\n");
|
||||
assert!(
|
||||
prompt.contains("Scope: thread"),
|
||||
"thread-scope turn expected; got: {prompt}"
|
||||
);
|
||||
assert!(
|
||||
prompt.contains("Description: Engineering discussions and planning."),
|
||||
"description must appear in [Context] for thread turns; got: {prompt}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_prompt_excludes_description_for_dm_turn() {
|
||||
let ch = Uuid::new_v4();
|
||||
let batch = description_batch(ch, make_event("hey"));
|
||||
let ci = PromptChannelInfo {
|
||||
name: "DM".into(),
|
||||
channel_type: "dm".into(),
|
||||
description: Some("This should not appear.".into()),
|
||||
};
|
||||
let prompt = format_prompt(
|
||||
&batch,
|
||||
&FormatPromptArgs {
|
||||
channel_info: Some(&ci),
|
||||
has_system_prompt_support: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.join("\n\n");
|
||||
assert!(
|
||||
prompt.contains("Scope: dm"),
|
||||
"dm-scope turn expected; got: {prompt}"
|
||||
);
|
||||
assert!(
|
||||
!prompt.contains("Description:"),
|
||||
"DM turn must not include a Description field; got: {prompt}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_prompt_no_description_when_channel_metadata_unresolved() {
|
||||
let ch = Uuid::new_v4();
|
||||
let batch = description_batch(ch, make_event("what should we build?"));
|
||||
// channel_info None models unresolved metadata: no name, no description.
|
||||
let prompt = format_prompt(
|
||||
&batch,
|
||||
&FormatPromptArgs {
|
||||
channel_info: None,
|
||||
has_system_prompt_support: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.join("\n\n");
|
||||
assert!(
|
||||
prompt.contains("Scope: channel"),
|
||||
"channel-scope turn expected; got: {prompt}"
|
||||
);
|
||||
assert!(
|
||||
!prompt.contains("Description:"),
|
||||
"unresolved metadata must not render a Description field; got: {prompt}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -136,6 +136,8 @@ use crate::config::ChannelFilter;
|
||||
pub struct ChannelInfo {
|
||||
pub name: String,
|
||||
pub channel_type: String,
|
||||
/// Channel description from the kind-39000 `about` tag, if present.
|
||||
pub description: Option<String>,
|
||||
}
|
||||
|
||||
pub(crate) fn channel_type_from_tags(tags: &[serde_json::Value]) -> String {
|
||||
@@ -175,7 +177,7 @@ pub(crate) fn merge_discovered_channels(
|
||||
channel_uuids: Vec<Uuid>,
|
||||
meta_events: &serde_json::Value,
|
||||
) -> HashMap<Uuid, ChannelInfo> {
|
||||
let mut meta_map: HashMap<Uuid, (String, String)> = HashMap::new();
|
||||
let mut meta_map: HashMap<Uuid, (String, String, Option<String>)> = HashMap::new();
|
||||
let mut archived: std::collections::HashSet<Uuid> = std::collections::HashSet::new();
|
||||
if let Some(arr) = meta_events.as_array() {
|
||||
for ev in arr {
|
||||
@@ -186,11 +188,13 @@ pub(crate) fn merge_discovered_channels(
|
||||
let mut d_val = None;
|
||||
let mut name = None;
|
||||
let mut is_archived = false;
|
||||
let mut description = None;
|
||||
for tag in tags {
|
||||
if let Some(arr) = tag.as_array() {
|
||||
match arr.first().and_then(|v| v.as_str()) {
|
||||
Some("d") => d_val = arr.get(1).and_then(|v| v.as_str()),
|
||||
Some("name") => name = arr.get(1).and_then(|v| v.as_str()),
|
||||
Some("about") => description = arr.get(1).and_then(|v| v.as_str()),
|
||||
Some("archived") => {
|
||||
is_archived = arr.get(1).and_then(|v| v.as_str()) == Some("true")
|
||||
}
|
||||
@@ -206,7 +210,11 @@ pub(crate) fn merge_discovered_channels(
|
||||
}
|
||||
let ch_name = name.unwrap_or("unknown").to_string();
|
||||
let ch_type = channel_type_from_tags(tags);
|
||||
meta_map.insert(uuid, (ch_name, ch_type));
|
||||
let ch_desc = description
|
||||
.map(|s| s.trim())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(str::to_string);
|
||||
meta_map.insert(uuid, (ch_name, ch_type, ch_desc));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -217,10 +225,17 @@ pub(crate) fn merge_discovered_channels(
|
||||
if archived.contains(&uuid) {
|
||||
continue;
|
||||
}
|
||||
let (name, channel_type) = meta_map
|
||||
let (name, channel_type, description) = meta_map
|
||||
.remove(&uuid)
|
||||
.unwrap_or_else(|| ("unknown".to_string(), "unknown".to_string()));
|
||||
map.insert(uuid, ChannelInfo { name, channel_type });
|
||||
.unwrap_or_else(|| ("unknown".to_string(), "unknown".to_string(), None));
|
||||
map.insert(
|
||||
uuid,
|
||||
ChannelInfo {
|
||||
name,
|
||||
channel_type,
|
||||
description,
|
||||
},
|
||||
);
|
||||
}
|
||||
map
|
||||
}
|
||||
@@ -4163,6 +4178,46 @@ mod tests {
|
||||
assert!(map.contains_key(&ch), "archived=false is treated as live");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_discovered_channels_parses_about_as_description() {
|
||||
let ch = Uuid::new_v4();
|
||||
let meta = serde_json::json!([meta_event(
|
||||
ch,
|
||||
"team",
|
||||
&["t", "stream", "about", "Engineering discussions"]
|
||||
)]);
|
||||
|
||||
let map = merge_discovered_channels(vec![ch], &meta);
|
||||
|
||||
assert_eq!(
|
||||
map[&ch].description.as_deref(),
|
||||
Some("Engineering discussions")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_discovered_channels_blank_about_is_none() {
|
||||
let ch = Uuid::new_v4();
|
||||
let meta = serde_json::json!([meta_event(ch, "team", &["about", " "])]);
|
||||
|
||||
let map = merge_discovered_channels(vec![ch], &meta);
|
||||
|
||||
assert_eq!(
|
||||
map[&ch].description, None,
|
||||
"a whitespace-only about tag is trimmed away to None"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_discovered_channels_missing_about_is_none() {
|
||||
let ch = Uuid::new_v4();
|
||||
let meta = serde_json::json!([meta_event(ch, "team", &["t", "stream"])]);
|
||||
|
||||
let map = merge_discovered_channels(vec![ch], &meta);
|
||||
|
||||
assert_eq!(map[&ch].description, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_ok_accepted() {
|
||||
let text = r#"["OK","abc123",true,""]"#;
|
||||
|
||||
50
desktop/src-tauri/Cargo.lock
generated
50
desktop/src-tauri/Cargo.lock
generated
@@ -210,7 +210,7 @@ version = "1.1.5"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc"
|
||||
dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -221,7 +221,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d"
|
||||
dependencies = [
|
||||
"anstyle",
|
||||
"once_cell_polyfill",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1624,7 +1624,7 @@ version = "3.1.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34"
|
||||
dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.48.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2291,7 +2291,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ccc2776f0c61eca1ca32528f85548abd1a4be8fb53d1b21c013e4f18da1e7090"
|
||||
dependencies = [
|
||||
"data-encoding",
|
||||
"syn 2.0.118",
|
||||
"syn 1.0.109",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2469,7 +2469,7 @@ dependencies = [
|
||||
"libc",
|
||||
"option-ext",
|
||||
"redox_users",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.59.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2750,7 +2750,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -3268,7 +3268,7 @@ dependencies = [
|
||||
"libc",
|
||||
"log",
|
||||
"rustversion",
|
||||
"windows-link 0.2.1",
|
||||
"windows-link 0.1.3",
|
||||
"windows-result 0.4.1",
|
||||
]
|
||||
|
||||
@@ -5726,7 +5726,7 @@ version = "0.6.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "536bfad37a309d62069485248eeaba1e8d9853aaf951caaeaed0585a95346f08"
|
||||
dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -5885,7 +5885,7 @@ dependencies = [
|
||||
"png 0.18.1",
|
||||
"serde",
|
||||
"thiserror 2.0.18",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -6398,7 +6398,7 @@ version = "0.50.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5"
|
||||
dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.59.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -7102,7 +7102,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.45.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -8128,7 +8128,7 @@ dependencies = [
|
||||
"once_cell",
|
||||
"socket2",
|
||||
"tracing",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -8859,7 +8859,7 @@ dependencies = [
|
||||
"errno",
|
||||
"libc",
|
||||
"linux-raw-sys 0.12.1",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -8929,7 +8929,7 @@ dependencies = [
|
||||
"security-framework 3.7.0",
|
||||
"security-framework-sys",
|
||||
"webpki-root-certs",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -9196,7 +9196,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "5b55fb86dfd3a2f5f76ea78310a88f96c4ea21a3031f8d212443d56123fd0521"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -9815,7 +9815,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "52d1cfed4120b4d927bf7c0f86d2087a4a7d6027c906d9f9d525a80573b9be51"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -10829,10 +10829,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd"
|
||||
dependencies = [
|
||||
"fastrand",
|
||||
"getrandom 0.4.3",
|
||||
"getrandom 0.3.4",
|
||||
"once_cell",
|
||||
"rustix 1.1.4",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.52.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -10854,7 +10854,7 @@ dependencies = [
|
||||
"parking_lot",
|
||||
"rustix 1.1.4",
|
||||
"signal-hook 0.3.18",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -11596,7 +11596,7 @@ dependencies = [
|
||||
"png 0.18.1",
|
||||
"serde",
|
||||
"thiserror 2.0.18",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -11697,7 +11697,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e"
|
||||
dependencies = [
|
||||
"memoffset",
|
||||
"tempfile",
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -12308,15 +12308,15 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "webbrowser"
|
||||
version = "1.2.1"
|
||||
version = "1.2.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "0fc95580916af1e68ff6a7be07446fc5db73ebf71cf092de939bbf5f7e189f72"
|
||||
checksum = "62c35be770821a214dbc362fc26908c853e776c0004294d0b10b8a6bad582f94"
|
||||
dependencies = [
|
||||
"core-foundation 0.10.1",
|
||||
"jni 0.22.4",
|
||||
"log",
|
||||
"ndk-context",
|
||||
"objc2",
|
||||
"objc2-app-kit",
|
||||
"objc2-foundation",
|
||||
"url",
|
||||
"web-sys",
|
||||
@@ -12548,7 +12548,7 @@ version = "0.1.11"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
|
||||
dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
"windows-sys 0.48.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
//! Databricks v1/v2 model discovery and interactive reauthentication.
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::LazyLock;
|
||||
use std::collections::{BTreeMap, HashMap};
|
||||
use std::sync::{LazyLock, Mutex, MutexGuard};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crate::commands::agent_models_env::{
|
||||
env_or_process_value, redaction_env_with_value, DiscoveryProvider,
|
||||
@@ -13,6 +14,77 @@ use crate::managed_agents::AgentModelsResponse;
|
||||
// callback listener/browser flow for the process-wide OAuth cache.
|
||||
static AUTH_GATE: LazyLock<tokio::sync::Mutex<()>> = LazyLock::new(|| tokio::sync::Mutex::new(()));
|
||||
|
||||
// Hard cap on the interactive browser flow launched from a discovery surface.
|
||||
// An abandoned SSO tab must fail discovery cleanly rather than wedge the
|
||||
// dropdown forever. (`authenticate_databricks` has its own 60s callback wait;
|
||||
// this outer bound also covers endpoint discovery and token exchange.)
|
||||
const AUTH_FLOW_TIMEOUT: Duration = Duration::from_secs(150);
|
||||
|
||||
// How long a failed/cancelled interactive sign-in suppresses re-launching the
|
||||
// browser from passive surfaces.
|
||||
pub(super) const AUTH_COOLDOWN: Duration = Duration::from_secs(5 * 60);
|
||||
|
||||
/// Per-host record of a recently failed, cancelled, or timed-out interactive
|
||||
/// sign-in.
|
||||
///
|
||||
/// Passive discovery surfaces fire on every form-state change, so without this
|
||||
/// a cancelled SSO page would re-pop the browser on the very next keystroke.
|
||||
/// Entries expire so a genuine later retry still launches; the saved-model
|
||||
/// picker bypasses the cooldown and a success clears it.
|
||||
#[derive(Default)]
|
||||
pub(super) struct AuthCooldown {
|
||||
until: Mutex<HashMap<String, Instant>>,
|
||||
}
|
||||
|
||||
impl AuthCooldown {
|
||||
fn map(&self) -> MutexGuard<'_, HashMap<String, Instant>> {
|
||||
// The critical sections below are panic-free map ops, so recover from a
|
||||
// poisoned lock rather than wedge every future sign-in on one panic.
|
||||
self.until
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner())
|
||||
}
|
||||
|
||||
pub(super) fn is_active(&self, host: &str, now: Instant) -> bool {
|
||||
let mut map = self.map();
|
||||
match map.get(host) {
|
||||
Some(&expiry) if now < expiry => true,
|
||||
Some(_) => {
|
||||
map.remove(host);
|
||||
false
|
||||
}
|
||||
None => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn record(&self, host: &str, now: Instant) {
|
||||
self.map().insert(host.to_string(), now + AUTH_COOLDOWN);
|
||||
}
|
||||
|
||||
pub(super) fn clear(&self, host: &str) {
|
||||
self.map().remove(host);
|
||||
}
|
||||
|
||||
/// Whether the interactive browser flow may launch now under `auth_intent`.
|
||||
/// Passive surfaces are suppressed while a per-host cooldown is active; the
|
||||
/// explicit picker path always launches and clears any stale suppression.
|
||||
pub(super) fn permits_launch(
|
||||
&self,
|
||||
auth_intent: DatabricksAuthIntent,
|
||||
host: &str,
|
||||
now: Instant,
|
||||
) -> bool {
|
||||
if auth_intent.respects_cooldown() {
|
||||
!self.is_active(host, now)
|
||||
} else {
|
||||
self.clear(host);
|
||||
true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static AUTH_COOLDOWNS: LazyLock<AuthCooldown> = LazyLock::new(AuthCooldown::default);
|
||||
|
||||
pub(super) fn is_databricks_provider(provider: Option<&str>) -> bool {
|
||||
matches!(
|
||||
provider
|
||||
@@ -50,8 +122,14 @@ pub(super) enum DatabricksAuthIntent {
|
||||
}
|
||||
|
||||
impl DatabricksAuthIntent {
|
||||
fn allows_interactive_auth(self) -> bool {
|
||||
matches!(self, Self::InteractiveModelPicker)
|
||||
/// Passive draft discovery honors (and, on failure, writes) the per-host
|
||||
/// cooldown so a cancelled SSO page does not re-pop on the next form
|
||||
/// keystroke. The saved-model picker is an explicit user action, so it
|
||||
/// bypasses the cooldown and clears it before launching. Both surfaces
|
||||
/// launch the browser flow (Phase 2 goose-parity); this predicate is the
|
||||
/// only behavioral difference between them.
|
||||
fn respects_cooldown(self) -> bool {
|
||||
matches!(self, Self::PassiveDraftDiscovery)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -60,11 +138,16 @@ pub(super) fn databricks_sign_in_required_error() -> String {
|
||||
.to_string()
|
||||
}
|
||||
|
||||
pub(super) fn should_start_interactive_auth(
|
||||
api_key: &str,
|
||||
auth_intent: DatabricksAuthIntent,
|
||||
) -> bool {
|
||||
api_key.is_empty() && auth_intent.allows_interactive_auth()
|
||||
pub(super) fn databricks_sign_in_timed_out_error() -> String {
|
||||
"Databricks sign-in timed out; open the model picker to retry, or run `buzz-agent auth databricks`"
|
||||
.to_string()
|
||||
}
|
||||
|
||||
pub(super) fn should_start_interactive_auth(api_key: &str) -> bool {
|
||||
// Phase 2: both discovery surfaces launch the browser flow when no static
|
||||
// token is configured. Which surface is allowed to actually pop the browser
|
||||
// (vs. respect a cooldown) is decided via `AuthCooldown::permits_launch`.
|
||||
api_key.is_empty()
|
||||
}
|
||||
|
||||
pub(super) async fn discover_databricks_models(
|
||||
@@ -93,22 +176,26 @@ pub(super) async fn discover_databricks_models(
|
||||
|
||||
let entries = match buzz_agent_pkg::discover_databricks_models(&config).await {
|
||||
Ok(entries) => entries,
|
||||
Err(buzz_agent_pkg::AgentError::LlmAuth(_))
|
||||
if should_start_interactive_auth(&api_key, auth_intent) =>
|
||||
{
|
||||
Err(buzz_agent_pkg::AgentError::LlmAuth(_)) if should_start_interactive_auth(&api_key) => {
|
||||
let _auth = AUTH_GATE.lock().await;
|
||||
match buzz_agent_pkg::discover_databricks_models(&config).await {
|
||||
// A peer sign-in under the gate already succeeded.
|
||||
Ok(entries) => entries,
|
||||
Err(buzz_agent_pkg::AgentError::LlmAuth(_)) => {
|
||||
buzz_agent_pkg::authenticate_databricks(&host)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
format_redacted_error(
|
||||
"Databricks sign-in failed",
|
||||
&error,
|
||||
&redaction_env,
|
||||
)
|
||||
})?;
|
||||
// Passive surfaces suppress the browser while a recent
|
||||
// failure/cancel is cooling down; the explicit picker path
|
||||
// always launches (and clears any stale cooldown).
|
||||
if !AUTH_COOLDOWNS.permits_launch(auth_intent, &host, Instant::now()) {
|
||||
return Err(databricks_sign_in_required_error());
|
||||
}
|
||||
run_interactive_databricks_auth(
|
||||
buzz_agent_pkg::authenticate_databricks(&host),
|
||||
AUTH_FLOW_TIMEOUT,
|
||||
&AUTH_COOLDOWNS,
|
||||
&host,
|
||||
&redaction_env,
|
||||
)
|
||||
.await?;
|
||||
buzz_agent_pkg::discover_databricks_models(&config)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
@@ -172,3 +259,43 @@ fn format_redacted_error(
|
||||
let message = crate::managed_agents::redact_env_values_in(&error.to_string(), redaction_env);
|
||||
format!("{context}: {message}")
|
||||
}
|
||||
|
||||
/// Run the interactive browser OAuth flow under a hard timeout and maintain the
|
||||
/// per-host cooldown. Success clears the cooldown; a failure, cancel, or
|
||||
/// timeout records it so passive surfaces stop re-launching the browser on the
|
||||
/// next form keystroke. `timeout` is injected (production passes
|
||||
/// [`AUTH_FLOW_TIMEOUT`]) so the timeout/cooldown policy is unit-testable
|
||||
/// without a live browser.
|
||||
pub(super) async fn run_interactive_databricks_auth<Fut>(
|
||||
auth: Fut,
|
||||
timeout: Duration,
|
||||
cooldowns: &AuthCooldown,
|
||||
host: &str,
|
||||
redaction_env: &BTreeMap<String, String>,
|
||||
) -> Result<(), String>
|
||||
where
|
||||
Fut: std::future::Future<Output = Result<(), buzz_agent_pkg::AgentError>>,
|
||||
{
|
||||
match tokio::time::timeout(timeout, auth).await {
|
||||
Ok(Ok(())) => {
|
||||
cooldowns.clear(host);
|
||||
Ok(())
|
||||
}
|
||||
Ok(Err(error)) => {
|
||||
cooldowns.record(host, Instant::now());
|
||||
Err(format_redacted_error(
|
||||
"Databricks sign-in failed",
|
||||
&error,
|
||||
redaction_env,
|
||||
))
|
||||
}
|
||||
Err(_elapsed) => {
|
||||
cooldowns.record(host, Instant::now());
|
||||
Err(databricks_sign_in_timed_out_error())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "agent_models_databricks_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
109
desktop/src-tauri/src/commands/agent_models_databricks_tests.rs
Normal file
109
desktop/src-tauri/src/commands/agent_models_databricks_tests.rs
Normal file
@@ -0,0 +1,109 @@
|
||||
//! Cooldown and interactive-auth policy tests for Databricks discovery.
|
||||
//!
|
||||
//! Housed as a child of `agent_models_databricks` (not the shared
|
||||
//! `agent_models_tests`) so the async timeout/cooldown cases sit next to the
|
||||
//! code they exercise and reach its `pub(super)` items directly via
|
||||
//! `use super::*` — and so the shared test file stays under its size ratchet.
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn databricks_cooldown_suppresses_passive_relaunch_but_never_the_picker() {
|
||||
let cooldowns = AuthCooldown::default();
|
||||
let host = "https://example.cloud.databricks.com";
|
||||
let now = Instant::now();
|
||||
|
||||
// A fresh host permits either surface to launch.
|
||||
assert!(cooldowns.permits_launch(DatabricksAuthIntent::PassiveDraftDiscovery, host, now));
|
||||
assert!(cooldowns.permits_launch(DatabricksAuthIntent::InteractiveModelPicker, host, now));
|
||||
|
||||
// After a failed/cancelled attempt, passive discovery must NOT re-pop the
|
||||
// browser while the window is active...
|
||||
cooldowns.record(host, now);
|
||||
assert!(!cooldowns.permits_launch(DatabricksAuthIntent::PassiveDraftDiscovery, host, now));
|
||||
|
||||
// ...but an explicit picker click always launches, and clears the window so
|
||||
// a later passive read is unblocked too.
|
||||
assert!(cooldowns.permits_launch(DatabricksAuthIntent::InteractiveModelPicker, host, now));
|
||||
assert!(cooldowns.permits_launch(DatabricksAuthIntent::PassiveDraftDiscovery, host, now));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn databricks_cooldown_expires_after_its_window_and_is_host_scoped() {
|
||||
let cooldowns = AuthCooldown::default();
|
||||
let host = "https://a.cloud.databricks.com";
|
||||
let other = "https://b.cloud.databricks.com";
|
||||
let now = Instant::now();
|
||||
|
||||
cooldowns.record(host, now);
|
||||
// A cooldown on one host never suppresses another.
|
||||
assert!(!cooldowns.is_active(other, now));
|
||||
assert!(cooldowns.is_active(host, now));
|
||||
|
||||
// The window is closed the instant it elapses, so a genuine later retry
|
||||
// launches again.
|
||||
let after = now + AUTH_COOLDOWN;
|
||||
assert!(!cooldowns.is_active(host, after));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn databricks_interactive_auth_success_clears_a_prior_cooldown() {
|
||||
let cooldowns = AuthCooldown::default();
|
||||
let host = "https://example.cloud.databricks.com";
|
||||
let redaction = BTreeMap::new();
|
||||
cooldowns.record(host, Instant::now());
|
||||
|
||||
let result = run_interactive_databricks_auth(
|
||||
async { Ok(()) },
|
||||
Duration::from_secs(150),
|
||||
&cooldowns,
|
||||
host,
|
||||
&redaction,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(result.is_ok());
|
||||
assert!(!cooldowns.is_active(host, Instant::now()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn databricks_interactive_auth_failure_records_a_cooldown() {
|
||||
let cooldowns = AuthCooldown::default();
|
||||
let host = "https://example.cloud.databricks.com";
|
||||
let redaction = BTreeMap::new();
|
||||
|
||||
let result = run_interactive_databricks_auth(
|
||||
async { Err(buzz_agent_pkg::AgentError::LlmAuth("closed the tab".into())) },
|
||||
Duration::from_secs(150),
|
||||
&cooldowns,
|
||||
host,
|
||||
&redaction,
|
||||
)
|
||||
.await;
|
||||
|
||||
let error = result.expect_err("a failed sign-in must surface an error");
|
||||
assert!(error.contains("Databricks sign-in failed"));
|
||||
assert!(cooldowns.is_active(host, Instant::now()));
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn databricks_interactive_auth_timeout_records_cooldown_and_returns_timeout_copy() {
|
||||
let cooldowns = AuthCooldown::default();
|
||||
let host = "https://example.cloud.databricks.com";
|
||||
let redaction = BTreeMap::new();
|
||||
|
||||
// An abandoned SSO tab: the flow never resolves. Under the paused clock the
|
||||
// injected timeout fires deterministically without real waiting.
|
||||
let result = run_interactive_databricks_auth(
|
||||
std::future::pending::<Result<(), buzz_agent_pkg::AgentError>>(),
|
||||
Duration::from_secs(150),
|
||||
&cooldowns,
|
||||
host,
|
||||
&redaction,
|
||||
)
|
||||
.await;
|
||||
|
||||
let error = result.expect_err("a timed-out sign-in must surface an error");
|
||||
assert_eq!(error, databricks_sign_in_timed_out_error());
|
||||
assert!(cooldowns.is_active(host, Instant::now()));
|
||||
}
|
||||
@@ -577,19 +577,12 @@ fn is_databricks_provider_matches_both_variants() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn databricks_interactive_auth_requires_explicit_intent_and_no_static_token() {
|
||||
assert!(should_start_interactive_auth(
|
||||
"",
|
||||
DatabricksAuthIntent::InteractiveModelPicker
|
||||
));
|
||||
assert!(!should_start_interactive_auth(
|
||||
"",
|
||||
DatabricksAuthIntent::PassiveDraftDiscovery
|
||||
));
|
||||
assert!(!should_start_interactive_auth(
|
||||
"static-token",
|
||||
DatabricksAuthIntent::InteractiveModelPicker
|
||||
));
|
||||
fn databricks_interactive_auth_launches_only_without_a_static_token() {
|
||||
// Phase 2: both surfaces launch the browser flow when the token is empty;
|
||||
// the surface distinction is now cooldown-only (asserted separately). A
|
||||
// configured static token still short-circuits interactive auth entirely.
|
||||
assert!(should_start_interactive_auth(""));
|
||||
assert!(!should_start_interactive_auth("static-token"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -69,6 +69,53 @@ test("model discovery status stays quiet for missing Databricks defaults", () =>
|
||||
assert.equal(status, null);
|
||||
});
|
||||
|
||||
test("Databricks sign-in-required is a muted note pointing at the picker and CLI", () => {
|
||||
const status = formatModelDiscoveryErrorStatus(
|
||||
new Error(
|
||||
"Databricks sign-in is required; save this agent, then open its model picker to sign in, or run `buzz-agent auth databricks`",
|
||||
),
|
||||
"databricks_v2",
|
||||
);
|
||||
|
||||
assert.equal(status?.tone, "muted");
|
||||
assert.match(status?.message ?? "", /model picker/);
|
||||
assert.match(status?.message ?? "", /buzz-agent auth databricks/);
|
||||
});
|
||||
|
||||
test("Databricks sign-in failure warns and points at the explicit retry", () => {
|
||||
const status = formatModelDiscoveryErrorStatus(
|
||||
new Error("Databricks sign-in failed: oauth callback: access_denied"),
|
||||
"databricks_v2",
|
||||
);
|
||||
|
||||
assert.equal(status?.tone, "warning");
|
||||
assert.match(status?.message ?? "", /didn't complete/);
|
||||
assert.match(status?.message ?? "", /model picker/);
|
||||
});
|
||||
|
||||
test("Databricks sign-in timeout warns and points at the explicit retry", () => {
|
||||
const status = formatModelDiscoveryErrorStatus(
|
||||
new Error(
|
||||
"Databricks sign-in timed out; open the model picker to retry, or run `buzz-agent auth databricks`",
|
||||
),
|
||||
"databricks_v2",
|
||||
);
|
||||
|
||||
assert.equal(status?.tone, "warning");
|
||||
assert.match(status?.message ?? "", /didn't complete/);
|
||||
assert.match(status?.message ?? "", /buzz-agent auth databricks/);
|
||||
});
|
||||
|
||||
test("other Databricks discovery failures fall through to the generic notice", () => {
|
||||
const status = formatModelDiscoveryErrorStatus(
|
||||
new Error("Databricks model discovery failed: relay offline"),
|
||||
"databricks_v2",
|
||||
);
|
||||
|
||||
assert.equal(status?.tone, "warning");
|
||||
assert.match(status?.message ?? "", /Using built-in model options/);
|
||||
});
|
||||
|
||||
test("auth-required errors name the agent and ask for sign-in", () => {
|
||||
// Real shape from run_agent_models_command wrapping buzz-acp stderr when
|
||||
// cursor-agent is signed out (spec ErrorCode::AuthRequired text).
|
||||
|
||||
@@ -124,6 +124,16 @@ export function formatModelDiscoveryErrorStatus(
|
||||
return null;
|
||||
}
|
||||
|
||||
// Databricks transparent auth (agent_models_databricks.rs). The backend
|
||||
// launches the browser OAuth flow itself from every discovery surface, so
|
||||
// these are terminal outcomes the user should see, not raw error text.
|
||||
// Matched on the stable error strings the backend emits (string matching is
|
||||
// this file's convention until typed error codes arrive).
|
||||
const databricksStatus = formatDatabricksAuthStatus(message);
|
||||
if (databricksStatus !== null) {
|
||||
return databricksStatus;
|
||||
}
|
||||
|
||||
return {
|
||||
message: `Using built-in model options. Could not load live models for ${providerObjectLabel(
|
||||
provider,
|
||||
@@ -131,3 +141,35 @@ export function formatModelDiscoveryErrorStatus(
|
||||
tone: "warning",
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Maps the terminal Databricks sign-in states to user-facing guidance, or null
|
||||
* when the error is not a Databricks sign-in outcome. "Sign-in required" is a
|
||||
* quiet muted note (a passive surface hit its cooldown, or an unsaved draft
|
||||
* can't launch the browser); a failed, cancelled, or timed-out sign-in is a
|
||||
* warning that points the user at the explicit retry path.
|
||||
*/
|
||||
function formatDatabricksAuthStatus(
|
||||
message: string,
|
||||
): PersonaModelDiscoveryStatus | null {
|
||||
if (message.includes("Databricks sign-in is required")) {
|
||||
return {
|
||||
message:
|
||||
"Databricks sign-in is required. Open the model picker to sign in, or run `buzz-agent auth databricks` in a terminal.",
|
||||
tone: "muted",
|
||||
};
|
||||
}
|
||||
|
||||
if (
|
||||
message.includes("Databricks sign-in failed") ||
|
||||
message.includes("Databricks sign-in timed out")
|
||||
) {
|
||||
return {
|
||||
message:
|
||||
"Databricks sign-in didn't complete. Open the model picker to retry, or run `buzz-agent auth databricks` in a terminal.",
|
||||
tone: "warning",
|
||||
};
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user