mirror of
https://github.com/block/buzz.git
synced 2026-10-30 22:11:36 +00:00
fix: fail closed on ambiguous openai credential migration
Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -95,6 +95,8 @@ pub(crate) enum RequirementPayload {
|
||||
NormalizedField { field: String },
|
||||
/// An env-backed credential that is absent.
|
||||
EnvKey { key: String },
|
||||
/// Invalid or conflicting configuration with actionable remediation copy.
|
||||
ConfigInvalid { message: String },
|
||||
/// A CLI authentication step that must be completed interactively.
|
||||
CliLogin {
|
||||
probe_args: Vec<String>,
|
||||
@@ -127,6 +129,7 @@ impl RequirementPayload {
|
||||
RequirementPayload::EnvKey { key } => {
|
||||
format!("set `{}` in Edit Agent → Environment variables", key)
|
||||
}
|
||||
RequirementPayload::ConfigInvalid { message } => message.clone(),
|
||||
RequirementPayload::CliLogin {
|
||||
setup_copy,
|
||||
availability,
|
||||
@@ -725,6 +728,25 @@ mod tests {
|
||||
assert!(body.contains("Fizz"), "nudge body should name the agent");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nudge_body_explains_conflicting_openai_origin() {
|
||||
let payload = SetupPayload {
|
||||
agent_name: "OpenAI Agent".to_string(),
|
||||
agent_pubkey: "test".to_string(),
|
||||
requirements: vec![RequirementPayload::ConfigInvalid {
|
||||
message:
|
||||
"remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`"
|
||||
.to_string(),
|
||||
}],
|
||||
};
|
||||
|
||||
let body = payload.nudge_body();
|
||||
|
||||
assert!(body.contains("remove `OPENAI_COMPAT_BASE_URL`"));
|
||||
assert!(body.contains("switch the provider to `openai-compat`"));
|
||||
assert!(body.contains(r#""surface":"config_invalid""#));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nudge_body_codex_copy_does_not_mention_openai_api_key() {
|
||||
let payload = SetupPayload {
|
||||
|
||||
@@ -286,7 +286,6 @@ fn resolve_effective_agent_env_with_def(
|
||||
}
|
||||
|
||||
// ── Requirement types ─────────────────────────────────────────────────────────
|
||||
|
||||
/// A single missing piece of configuration, tagged with the UI surface that
|
||||
/// owns it so the UI can route each gap to the right affordance.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
@@ -299,11 +298,12 @@ pub enum Requirement {
|
||||
field: String,
|
||||
},
|
||||
/// An env-backed credential that is absent from the effective env.
|
||||
/// Routes to the env-var row editor in the Edit Agent dialog.
|
||||
EnvKey {
|
||||
/// The env var key name (e.g. `"ANTHROPIC_API_KEY"`).
|
||||
key: String,
|
||||
},
|
||||
/// Invalid configuration with actionable remediation copy.
|
||||
ConfigInvalid { message: String },
|
||||
/// A CLI authentication step that must be completed interactively.
|
||||
/// Routes to a setup instruction panel in the Edit Agent dialog.
|
||||
CliLogin {
|
||||
|
||||
@@ -11,8 +11,9 @@ pub(super) fn require_safe_official_origin(
|
||||
.filter(|value| !value.is_empty())
|
||||
.is_none_or(|value| value.trim_end_matches('/') == "https://api.openai.com/v1");
|
||||
if !safe {
|
||||
missing.push(Requirement::EnvKey {
|
||||
key: "OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
missing.push(Requirement::ConfigInvalid {
|
||||
message: "remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -39,8 +40,10 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
missing,
|
||||
vec![Requirement::EnvKey {
|
||||
key: "OPENAI_COMPAT_BASE_URL".to_string()
|
||||
vec![Requirement::ConfigInvalid {
|
||||
message:
|
||||
"remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`"
|
||||
.to_string()
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -596,6 +596,9 @@ pub fn spawn_agent_child(
|
||||
"surface": "env_key",
|
||||
"key": key,
|
||||
}),
|
||||
Requirement::ConfigInvalid { message } => serde_json::json!({
|
||||
"surface": "config_invalid", "message": message,
|
||||
}),
|
||||
Requirement::CliLogin {
|
||||
probe_args,
|
||||
setup_copy,
|
||||
|
||||
@@ -95,6 +95,22 @@ struct DefinitionContext {
|
||||
env: Map<String, Value>,
|
||||
}
|
||||
|
||||
fn harness_env_for_runtime(
|
||||
runtime: Option<&str>,
|
||||
harness_env_by_runtime: &std::collections::HashMap<String, Map<String, Value>>,
|
||||
) -> Result<Map<String, Value>, String> {
|
||||
let Some(runtime) = runtime.map(str::trim).filter(|value| !value.is_empty()) else {
|
||||
return Ok(Map::new());
|
||||
};
|
||||
if let Some(env) = harness_env_by_runtime.get(runtime) {
|
||||
return Ok(env.clone());
|
||||
}
|
||||
if crate::managed_agents::custom_harnesses::check_id_collision(runtime).is_ok() {
|
||||
return Err(format!("unresolved custom harness {runtime:?}"));
|
||||
}
|
||||
Ok(Map::new())
|
||||
}
|
||||
|
||||
fn sibling_path(path: &Path, suffix: &str) -> PathBuf {
|
||||
let file_name = path
|
||||
.file_name()
|
||||
@@ -201,10 +217,8 @@ fn migrate_openai_credentials_in_file(
|
||||
.filter(|runtime| !runtime.is_empty())
|
||||
.or(effective_runtime);
|
||||
let merged_inherited_env = {
|
||||
let mut merged = selected_runtime
|
||||
.and_then(|runtime| harness_env_by_runtime.get(runtime))
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
let mut merged =
|
||||
harness_env_for_runtime(selected_runtime, harness_env_by_runtime)?;
|
||||
if let Some(inherited) = inherited_env_vars {
|
||||
merged.extend(inherited.clone());
|
||||
}
|
||||
@@ -222,20 +236,7 @@ fn migrate_openai_credentials_in_file(
|
||||
changed
|
||||
}
|
||||
Value::Object(record) => {
|
||||
let selected_runtime = record
|
||||
.get("preferred_runtime")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::trim)
|
||||
.filter(|runtime| !runtime.is_empty())
|
||||
.or(effective_runtime);
|
||||
let merged_inherited_env = selected_runtime
|
||||
.and_then(|runtime| harness_env_by_runtime.get(runtime))
|
||||
.cloned();
|
||||
migrate_openai_credential_record(
|
||||
record,
|
||||
effective_provider,
|
||||
merged_inherited_env.as_ref().or(inherited_env_vars),
|
||||
)
|
||||
migrate_openai_credential_record(record, effective_provider, inherited_env_vars)
|
||||
}
|
||||
_ => {
|
||||
return Err(format!(
|
||||
@@ -562,6 +563,66 @@ mod tests {
|
||||
assert!(records[0]["env_vars"].get(OPENAI_API_KEY).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unresolved_custom_harness_leaves_file_unmarked() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("managed-agents.json");
|
||||
let original = serde_json::to_vec(&serde_json::json!([{
|
||||
"pubkey": "",
|
||||
"slug": "custom-definition",
|
||||
"runtime": "missing-custom-harness",
|
||||
"provider": "openai",
|
||||
"env_vars": { "OPENAI_COMPAT_API_KEY": "compat-secret" }
|
||||
}]))
|
||||
.unwrap();
|
||||
std::fs::write(&path, &original).unwrap();
|
||||
|
||||
let error = migrate_openai_credentials_in_file(
|
||||
&path,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
&std::collections::HashMap::new(),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert!(error.contains("unresolved custom harness"), "{error}");
|
||||
assert_eq!(std::fs::read(&path).unwrap(), original);
|
||||
assert!(!sibling_path(&path, MIGRATION_SUFFIX).exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn global_credentials_ignore_preferred_runtime_harness_env() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("global-agent-config.json");
|
||||
std::fs::write(
|
||||
&path,
|
||||
serde_json::to_vec(&serde_json::json!({
|
||||
"preferred_runtime": "custom-gateway",
|
||||
"provider": "openai",
|
||||
"env_vars": { "OPENAI_COMPAT_API_KEY": "official-secret" }
|
||||
}))
|
||||
.unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
let harness_env = std::collections::HashMap::from([(
|
||||
"custom-gateway".to_string(),
|
||||
serde_json::json!({
|
||||
"OPENAI_COMPAT_BASE_URL": "https://gateway.example/v1"
|
||||
})
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone(),
|
||||
)]);
|
||||
|
||||
migrate_openai_credentials_in_file(&path, None, None, None, &harness_env).unwrap();
|
||||
let global: Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap();
|
||||
|
||||
assert_eq!(global["provider"], "openai");
|
||||
assert_eq!(global["env_vars"][OPENAI_API_KEY], "official-secret");
|
||||
assert!(global["env_vars"].get(OPENAI_COMPAT_API_KEY).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_instance_uses_definition_provider_and_layered_endpoint() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -23,6 +23,7 @@ import type { AcpAvailabilityStatus } from "@/shared/api/types";
|
||||
export type ConfigNudgeRequirement =
|
||||
| { surface: "normalized_field"; field: string }
|
||||
| { surface: "env_key"; key: string }
|
||||
| { surface: "config_invalid"; message: string }
|
||||
| {
|
||||
surface: "cli_login";
|
||||
probe_args: string[];
|
||||
@@ -145,6 +146,8 @@ function isConfigNudgeRequirement(v: unknown): v is ConfigNudgeRequirement {
|
||||
return typeof r.field === "string";
|
||||
case "env_key":
|
||||
return typeof r.key === "string";
|
||||
case "config_invalid":
|
||||
return typeof r.message === "string";
|
||||
case "cli_login":
|
||||
return (
|
||||
Array.isArray(r.probe_args) &&
|
||||
|
||||
@@ -29,6 +29,8 @@ function requirementKey(
|
||||
switch (req.surface) {
|
||||
case "env_key":
|
||||
return `env_key:${req.key}:${index}`;
|
||||
case "config_invalid":
|
||||
return `config_invalid:${req.message}:${index}`;
|
||||
case "normalized_field":
|
||||
return `normalized_field:${req.field}:${index}`;
|
||||
case "cli_login":
|
||||
@@ -330,6 +332,21 @@ function RequirementRow({
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
case "config_invalid":
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs leading-4 text-muted-foreground">
|
||||
<span className="flex-1 [overflow-wrap:anywhere]">
|
||||
{requirement.message}
|
||||
</span>
|
||||
<button
|
||||
className="relative z-20 shrink-0 font-medium text-muted-foreground hover:underline"
|
||||
onClick={(e) => onOpenEditAgent(e, undefined)}
|
||||
type="button"
|
||||
>
|
||||
Edit Agent →
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
case "normalized_field":
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs leading-4 text-muted-foreground">
|
||||
|
||||
Reference in New Issue
Block a user