mirror of
https://github.com/block/buzz.git
synced 2026-11-01 15:01:12 +00:00
Merge origin/main (#1968 definition-authoritative config) into duncan/byoh-generic-acp-harness
Semantic merge: main's effective_config resolver (model/provider/prompt/mesh authority) composes with this PR's harness-descriptor resolver (command/args/ env). Model discovery in get_agent_models now resolves model/provider via resolve_effective_model_provider instead of raw record bytes, closing the live-persona seam Dawn flagged; saved_agent_model_discovery_config is gone (descriptor + resolver replace it) and its linked-agent test is rewritten onto the new path. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
commit
872148fd94
@@ -80,10 +80,16 @@ const overrides = new Map([
|
||||
// ratcheting 1443 -> 1295. Queued to split further in the A2 fold.
|
||||
// global-agent-config: resolve_deploy_model_provider + visibility exports
|
||||
// add ~40 lines on top of the 1A.1 ratchet. Queued to split.
|
||||
// +47: review fix for #2773 — load_global_agent_config hoisted out of
|
||||
// +29 (1340 -> 1369, main): agent-config-resolver — start_local_agent_with_preflight
|
||||
// uses resolve_effective_relay_mesh_model_id at both preflight call sites;
|
||||
// preview_prospective_persona_snapshot helper extracted; orphan guard threaded
|
||||
// through restore path; start_local_agent_pairs_with_preflight resolver
|
||||
// preflight. Load-bearing feature changes; queued to split.
|
||||
// +47 (#2773): review fix — load_global_agent_config hoisted out of
|
||||
// build_managed_agent_summary into callers, dangling-harness summaries render
|
||||
// the deleted id, and spawn errors surface as sentences (tests included).
|
||||
["src-tauri/src/commands/agents.rs", 1387],
|
||||
// +1: merge of the two deltas above (actual post-merge count).
|
||||
["src-tauri/src/commands/agents.rs", 1418],
|
||||
// agent-lifecycle-fixes: cascade-delete in delete_persona restructured into
|
||||
// 3-phase (stage/stop/commit) + commit_cascade_agents injectable helper for
|
||||
// retry-safety. Load-bearing reviewer-required change; queued to split.
|
||||
@@ -171,11 +177,18 @@ const overrides = new Map([
|
||||
// Windows PATH-correctness fix: 3 #[cfg(windows)] test functions covering
|
||||
// .cmd shim rejection, .bat shim rejection, and .exe acceptance for
|
||||
// configure_runtime_cli (fix #2397). Test-only growth; queued to split.
|
||||
// +7 (main): this PR's resolver tests land on top of main's #2397 Windows
|
||||
// shim tests, plus main's restart_eligible orphan-gate tests.
|
||||
// +34: BYOH custom-harness sweep condition unit tests — 3 tests validating
|
||||
// the OR-gate fix for custom-binary orphan cleanup.
|
||||
// +26: BYOH pass-2 I3 — 2 collector-decision tests for receipt path
|
||||
// ownership (valid_agent_runtime_receipt uses buzz_sweep_owns_process).
|
||||
["src-tauri/src/managed_agents/runtime/tests.rs", 1320],
|
||||
// runtime.rs re-entered the list after the #1968 merge: main's
|
||||
// definition-authoritative resolver comments grew it to 982, and this PR's
|
||||
// typed harness-descriptor resolution in spawn_agent_child (+38) lands on
|
||||
// top. Queued to shrink with the next runtime split pass (#2974 follow-up).
|
||||
["src-tauri/src/managed_agents/runtime.rs", 1020],
|
||||
// applyWorkspace reposDir parameter plus the validateReposDir binding,
|
||||
// threaded through Tauri invokes for configurable repos_dir, plus the
|
||||
// harness-persona-sync `harnessOverride` create-input bit — load-bearing
|
||||
@@ -210,7 +223,8 @@ const overrides = new Map([
|
||||
// mapper. This is the existing API boundary; split remains queued.
|
||||
// team-instructions-first-class: createManagedAgent Tauri bridge threads the
|
||||
// new teamId input through to the backend (+1 line).
|
||||
["src/shared/api/tauri.ts", 1305],
|
||||
// +2 for model_source field in RawManagedAgent + fromRawManagedAgent mapping.
|
||||
["src/shared/api/tauri.ts", 1307],
|
||||
// doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line).
|
||||
// codex-acp-package-swap: "adapter_outdated" variant added to AcpAvailabilityStatus (+1 line).
|
||||
// doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/
|
||||
@@ -232,21 +246,7 @@ const overrides = new Map([
|
||||
// (#2680) to indicate runtimes that need a separate CLI install.
|
||||
// +6: ManagedAgent.runtime record-level pin + JSDoc so the harness delete
|
||||
// confirmation can count referencing agents (review fix for #2773).
|
||||
["src/shared/api/types.ts", 1057],
|
||||
// readiness-gate: PersonaDialog.tsx threads computeLocalModeGate +
|
||||
// requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog
|
||||
// shows required markers and credential amber rows (parity with
|
||||
// CreateAgentDialog). +23 lines of gate wiring. Queued to split.
|
||||
// config-bridge-aware requirements: useRuntimeFileConfigQuery wiring adds
|
||||
// ~16 lines. Queued to split.
|
||||
// baked-env-required-badge: useBakedBuildEnvKeysQuery + bakedEnvKeys wiring
|
||||
// + correct exclusion-semantics for requiredEnvKeys adds ~14 lines.
|
||||
// +2 lines: filter managed provider key from requiredEnvKeys (suppress dead-input locked row).
|
||||
// global-agent-config parity: wire useGlobalAgentConfig into PersonaDialog
|
||||
// (Gap A: global-aware computeLocalModeGate + drop bare requiredCredentialEnvKeys;
|
||||
// Gap B: hasAutoOpenedAdvancedRef auto-expand effect) + effective-provider
|
||||
// save gate + Inherit/Select-a-provider label. Queued to split.
|
||||
["src/features/agents/ui/PersonaDialog.tsx", 1080],
|
||||
["src/shared/api/types.ts", 1058],
|
||||
// harness-persona-sync feature growth, queued to split in the resolver-unify
|
||||
// refactor followup. discovery.rs is dominated by the new test module
|
||||
// (the effective_agent_command / divergent / create-time override matrix);
|
||||
@@ -524,10 +524,19 @@ const overrides = new Map([
|
||||
// (if let Some(provider_update) = input.provider { record.provider = provider_update; }).
|
||||
// +8: harness_override thread-through in update_managed_agent so a deliberate
|
||||
// Custom pin routes to update_time_agent_command_override (comment + call).
|
||||
// +1: review round 2 (#2773) — model_discovery_error helper routes dangling-
|
||||
// harness resolution errors through user_facing_harness_error (sentence, not
|
||||
// raw DANGLING_HARNESS_ID sentinel) for the get_agent_models surface.
|
||||
["src-tauri/src/commands/agent_models.rs", 1080],
|
||||
// +22 (1079 -> 1101, main): Finding 2 — model discovery now resolves through
|
||||
// resolve_effective_model_provider instead of raw record bytes, plus
|
||||
// apply_model_provider_prompt_update's linked-instance write-guard
|
||||
// extraction and its regression tests.
|
||||
// +4 (1101 -> 1105): rebase onto agents-everywhere — agents.rs function
|
||||
// signatures updated for ManagedAgentRuntimeKey-keyed runtimes map.
|
||||
// +1 (#2773): model_discovery_error helper routes dangling-harness
|
||||
// resolution errors through user_facing_harness_error (sentence, not raw
|
||||
// DANGLING_HARNESS_ID sentinel) for the get_agent_models surface. The PR's
|
||||
// descriptor path also deletes saved_agent_model_discovery_config, whose
|
||||
// callers now use resolve_effective_model_provider + the descriptor env
|
||||
// directly (net wash after the merge of the deltas above).
|
||||
["src-tauri/src/commands/agent_models.rs", 1114],
|
||||
// global-agent-config: get_agent_config_surface / write_agent_config_field /
|
||||
// put_agent_session_config commands + GlobalAgentConfig serde types. New file
|
||||
// in this PR; queued to split with the command module refactor.
|
||||
@@ -543,7 +552,12 @@ const overrides = new Map([
|
||||
// relay from the harness-attached payload relayUrl (with effective-relay
|
||||
// fallback for older harnesses) instead of a required arg the frontend
|
||||
// wrapper never passed, which silently broke the session-config cache.
|
||||
["src-tauri/src/commands/agent_config.rs", 1050],
|
||||
// +60 (1050 -> 1110): agent-config-resolver — resolve_config_surface now
|
||||
// clears a linked instance's own system_prompt/model/provider before
|
||||
// computing had_* so stale materialized snapshot bytes can never be tagged
|
||||
// BuzzExplicit and shadow the definition/global fallthrough; the dead
|
||||
// persona-model re-tag branch replaced; two new regression tests added.
|
||||
["src-tauri/src/commands/agent_config.rs", 1110],
|
||||
// codex-install-auto-restart review-fixes: should_restart_after_install
|
||||
// takes pid_alive:bool (pure predicate, no OS-dependent call); 3 racy
|
||||
// cache tests replaced with 6 pure availability_drift predicate tests;
|
||||
@@ -565,9 +579,20 @@ const overrides = new Map([
|
||||
// Includes unit tests for detection, routing, and -Command body preservation.
|
||||
// +16: test_powershell_command_goose_catalog_dequoted proves the \$→$ escape
|
||||
// fix for the Goose Windows installer (PR #2680 interaction with #2750).
|
||||
// +126: BYOH — save_custom_harness (validate, atomic write, return entry) +
|
||||
// delete_custom_harness (id-guard, builtin reject, remove file) commands;
|
||||
// discover_acp_providers updated to pass AppHandle + custom_harnesses dir.
|
||||
// +10: pass an explicit PATH through Codex adapter install planning so unit
|
||||
// tests avoid the process-global login-shell PATH cache.
|
||||
// +59 (main): run install commands under `pipefail` so a failing `curl` in a
|
||||
// `curl … | bash` install fails the `cli` step instead of being masked by
|
||||
// `bash`'s exit 0, plus tests for the arg shape and the real pipeline status.
|
||||
// +81 (main): install_shell_args re-exports the composed PATH inside the command
|
||||
// body so login startup files can't clear or reorder it, plus an isolated
|
||||
// hostile-profile regression the pure composition tests structurally miss.
|
||||
// +42 (main): gate that re-export off Windows, where join_paths is `;`-separated
|
||||
// and bash would collapse it into one entry, plus a platform-shape test.
|
||||
// +126 (#2773): BYOH — save_custom_harness (validate, atomic write, return
|
||||
// entry) + delete_custom_harness (id-guard, builtin reject, remove file)
|
||||
// commands; discover_acp_providers updated to pass AppHandle +
|
||||
// custom_harnesses dir.
|
||||
// +30: BYOH F5 — atomic-write-file dep, original_id rename/delete support.
|
||||
// +13: BYOH pass-2 C1 — warm_harness_registry_from_dir call in save and
|
||||
// delete commands now verifies transactional registry refresh.
|
||||
@@ -575,10 +600,7 @@ const overrides = new Map([
|
||||
// return value so the frontend immediately has the updated env.
|
||||
// +1: rebase over main (#2680) — requires_external_cli: false added to
|
||||
// save_custom_harness catalog entry construction (new required field).
|
||||
// +10: merge of main (#2767) — pass an explicit PATH through Codex adapter
|
||||
// install planning so unit tests avoid the process-global login-shell PATH
|
||||
// cache. Independent of the BYOH growth above; the two deltas compose.
|
||||
["src-tauri/src/commands/agent_discovery.rs", 2048],
|
||||
["src-tauri/src/commands/agent_discovery.rs", 2167],
|
||||
// draft-persistence predicate: submit-time `loadDraft` check + inline comment
|
||||
// + deps-array entry in submitMessage closes the never-persisted-boundary
|
||||
// defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to
|
||||
@@ -624,7 +646,9 @@ const overrides = new Map([
|
||||
// for Databricks v1 gate; prospectiveRuntimeId default fallback for builtins.
|
||||
// PR-B moves default/API-key derivation into shared hooks; the explicit
|
||||
// hidden-key projection keeps the top-level secret out of Advanced rows.
|
||||
["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1195],
|
||||
// +6 (1195 -> 1201): rebase onto main — this PR's model-source label wiring
|
||||
// lands on top of main's dialog growth. Queued to split.
|
||||
["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1201],
|
||||
// AgentDefinitionDialog grew past 1000 with the following load-bearing fixes:
|
||||
// isRuntimeAutoSeededRef tracking for edit-mode seeding (Fizz shows models);
|
||||
// runtimeSupportsLlmProviderSelection guard on discovery provider (codex fix);
|
||||
|
||||
@@ -39,6 +39,13 @@ pub struct RuntimeFileConfigSubset {
|
||||
|
||||
/// Resolve the config surface with persona and global default values applied.
|
||||
///
|
||||
/// Linked instances are definition-authoritative: the record's own
|
||||
/// system_prompt/model/provider are cleared before applying, so a stale
|
||||
/// materialized snapshot can never shadow the persona's current values or a
|
||||
/// blank-definition fallthrough to global defaults (mirrors
|
||||
/// `effective_config::resolve_linked`). Definition-less instances keep their
|
||||
/// own explicit values.
|
||||
///
|
||||
/// The pipeline: resolve the linked persona's prompt/model/provider, inject
|
||||
/// each into the record only where the record lacks its own value, let
|
||||
/// `read_config_surface` tag those injected fields `BuzzExplicit`, then re-tag
|
||||
@@ -59,6 +66,22 @@ fn resolve_config_surface(
|
||||
session_cache: Option<&SessionConfigCache>,
|
||||
global: &GlobalAgentConfig,
|
||||
) -> RuntimeConfigSurface {
|
||||
// Linked instances are definition-authoritative (mirrors
|
||||
// `effective_config::resolve_linked`): the record's own
|
||||
// system_prompt/model/provider fields are, at best, a stale materialized
|
||||
// snapshot from the last `apply_persona_snapshot` — never a legitimate
|
||||
// live override, since `update_managed_agent` blocks writing these three
|
||||
// fields for linked instances. Clear them before computing `had_*` below
|
||||
// so a stale byte can never masquerade as BuzzExplicit and suppress
|
||||
// definition/global injection. Env var overrides (set via the advanced
|
||||
// env-vars editor) are untouched — those remain a legitimate
|
||||
// per-instance override regardless of link status.
|
||||
if record.persona_id.is_some() {
|
||||
record.system_prompt = None;
|
||||
record.model = None;
|
||||
record.provider = None;
|
||||
}
|
||||
|
||||
let had_prompt =
|
||||
record.system_prompt.is_some() || record.env_vars.contains_key("BUZZ_ACP_SYSTEM_PROMPT");
|
||||
let had_model = record.model.is_some();
|
||||
@@ -172,22 +195,6 @@ fn resolve_config_surface(
|
||||
retag_global_default(&mut surface.normalized.provider);
|
||||
}
|
||||
|
||||
// Re-tag persona-snapshotted model from BuzzExplicit to PersonaDefault.
|
||||
// Persona-created agents have record.model set at create time from the
|
||||
// persona snapshot — had_model is true, but the model came from the persona,
|
||||
// not an explicit user choice. Re-tag when the record model matches the
|
||||
// persona model and no live override is active. Only applies when a persona
|
||||
// is actually linked — non-persona agents with an explicit model keep BuzzExplicit.
|
||||
if had_model && !model_overridden && record.persona_id.is_some() {
|
||||
if let (Some(ref record_model), Some(ref persona_model_val)) =
|
||||
(&record.model, &persona_model)
|
||||
{
|
||||
if record_model == persona_model_val {
|
||||
retag_persona_default(&mut surface.normalized.model);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
surface
|
||||
}
|
||||
|
||||
@@ -728,11 +735,64 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// A model the user set explicitly in Buzz must never be re-tagged to
|
||||
/// `PersonaDefault`, even when the linked persona also has a model.
|
||||
/// Definition-authoritative: a stale materialized `record.model` on a
|
||||
/// linked instance must never outrank (or even be consulted against) the
|
||||
/// linked persona's model. `update_managed_agent` already blocks writing
|
||||
/// model/provider/prompt for linked instances, so a non-`None` value here
|
||||
/// can only be leftover snapshot bytes from before a persona edit — the
|
||||
/// panel must report the persona's current model, tagged `PersonaDefault`,
|
||||
/// not the stale byte as `BuzzExplicit`.
|
||||
#[test]
|
||||
fn explicit_record_model_outranks_persona_and_keeps_buzz_explicit_origin() {
|
||||
fn linked_stale_record_model_never_outranks_persona_model() {
|
||||
let mut record = agent_record();
|
||||
record.model = Some("stale-explicit-model".to_string());
|
||||
let personas = vec![persona_with_model("persona-model")];
|
||||
|
||||
let surface = resolve_config_surface(
|
||||
record,
|
||||
&personas,
|
||||
Some(goose_runtime()),
|
||||
None,
|
||||
&Default::default(),
|
||||
);
|
||||
|
||||
let model = surface.normalized.model.as_ref().expect("model resolved");
|
||||
assert_eq!(model.value.as_deref(), Some("persona-model"));
|
||||
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
|
||||
}
|
||||
|
||||
/// Definition-authoritative, blank-definition case: a linked instance
|
||||
/// whose persona has no model of its own must fall through to the global
|
||||
/// default, tagged `GlobalDefault` — mirroring
|
||||
/// `effective_config::resolve_linked`'s `None => global` arm. A stale
|
||||
/// materialized record model must not shadow this fallthrough either.
|
||||
#[test]
|
||||
fn linked_blank_definition_model_falls_through_to_global_default() {
|
||||
let mut record = agent_record();
|
||||
record.model = Some("stale-explicit-model".to_string());
|
||||
let mut persona = persona_with_model("unused");
|
||||
persona.model = None;
|
||||
let personas = vec![persona];
|
||||
let global = crate::managed_agents::GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let surface =
|
||||
resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global);
|
||||
|
||||
let model = surface.normalized.model.as_ref().expect("model resolved");
|
||||
assert_eq!(model.value.as_deref(), Some("global-model"));
|
||||
assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
|
||||
}
|
||||
|
||||
/// A definition-less (no `persona_id`) instance's own explicit model IS
|
||||
/// authoritative — the stale-record clearing above is scoped to linked
|
||||
/// instances only.
|
||||
#[test]
|
||||
fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() {
|
||||
let mut record = agent_record();
|
||||
record.persona_id = None;
|
||||
record.model = Some("explicit-model".to_string());
|
||||
let personas = vec![persona_with_model("persona-model")];
|
||||
|
||||
|
||||
@@ -692,6 +692,62 @@ fn persist_last_error_on_install(
|
||||
save_managed_agents(app, &records)
|
||||
}
|
||||
|
||||
/// Build the `-l -c` argument list for the install shell.
|
||||
///
|
||||
/// The body runs under `pipefail`: every CLI install command is a `curl … |
|
||||
/// bash` / `| sh` pipe, and without it the pipeline's status is the right-hand
|
||||
/// side's — `bash`/`sh` fed an empty stdin exits 0 — so a `curl` that fails (or
|
||||
/// isn't on PATH at all) was recorded as a successful `cli` step, leaving the
|
||||
/// user an unactionable `verify` error instead of curl's own stderr. Every
|
||||
/// install shell supports it; the Windows PowerShell path bypasses this shell.
|
||||
/// `SHELLOPTS` is not exported, so the piped-to vendor script keeps its own
|
||||
/// defaults.
|
||||
///
|
||||
/// Off Windows, `composed_path` is passed as a positional and re-exported
|
||||
/// *inside* the body, because `-l` sources the user's login startup files after
|
||||
/// the process environment is installed: a profile assigning PATH overwrites
|
||||
/// `cmd.env("PATH", …)` before the vendor command runs. `export PATH=` empties
|
||||
/// it outright; macOS `/etc/zprofile` runs `path_helper`, which reorders it and
|
||||
/// costs Buzz's managed Node/npm dirs their precedence. A positional rather than
|
||||
/// an interpolated body keeps entries containing spaces or quotes intact.
|
||||
///
|
||||
/// The prelude is omitted where it would do harm:
|
||||
/// - `composed_path` is `None` — `export PATH="$1"` with `$1` unset sets an
|
||||
/// *empty* PATH, worse than the ambient one.
|
||||
/// - `is_windows` — `join_paths` uses the platform separator, so the positional
|
||||
/// would be `;`-joined while bash splits PATH on `:`, collapsing every entry
|
||||
/// into one nonsense path; and Windows is where the inherited fallback always
|
||||
/// fires (`login_shell_path()` is unconditionally `None` there), so this would
|
||||
/// be the steady state. `cmd.env("PATH", …)` already delivers the native form
|
||||
/// Git Bash translates on entry, and Windows has no login startup files doing
|
||||
/// the clobbering this prelude defends against.
|
||||
///
|
||||
/// `is_windows` is a parameter rather than a `#[cfg]` so the Windows shape stays
|
||||
/// asserted on Unix CI — the same reason `should_skip_claude_executable` takes
|
||||
/// one. Extracted from `install_shell_command` for that testability, not because
|
||||
/// it has more than one caller.
|
||||
fn install_shell_args(
|
||||
command: &str,
|
||||
composed_path: Option<&std::ffi::OsStr>,
|
||||
is_windows: bool,
|
||||
) -> Vec<std::ffi::OsString> {
|
||||
let Some(path) = composed_path.filter(|_| !is_windows) else {
|
||||
return vec![
|
||||
"-l".into(),
|
||||
"-c".into(),
|
||||
format!("set -o pipefail; {command}").into(),
|
||||
];
|
||||
};
|
||||
vec![
|
||||
"-l".into(),
|
||||
"-c".into(),
|
||||
format!("export PATH=\"$1\"; set -o pipefail; {command}").into(),
|
||||
// `$0` is the shell-name slot, so the PATH must be the second positional.
|
||||
"buzz-install".into(),
|
||||
path.to_os_string(),
|
||||
]
|
||||
}
|
||||
|
||||
/// Build a login-shell `Command` for `command` with hermit env vars stripped,
|
||||
/// Buzz-managed npm locations set, and the user's PATH set. This is the
|
||||
/// single source of truth for
|
||||
@@ -706,7 +762,6 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
let shell: std::path::PathBuf = resolve_install_shell()?;
|
||||
|
||||
let mut cmd = std::process::Command::new(&shell);
|
||||
cmd.args(["-l", "-c", command]);
|
||||
|
||||
// Strip hermit vars and set managed npm paths (see apply_npm_env).
|
||||
apply_npm_env(&mut cmd);
|
||||
@@ -714,10 +769,18 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
// Compose the PATH for the install shell using the same kernel as the
|
||||
// runtime/probe path so the two can never drift. managed entries first
|
||||
// (Node/npm bins keep precedence); login-shell entries next; inherited
|
||||
// process PATH appended last on Windows when no login-shell PATH exists
|
||||
// (login_shell_path() always returns None on Windows — Git Bash paths are
|
||||
// POSIX-shaped and poison native children; cmd.env("PATH", …) replaces
|
||||
// rather than extends, so without inherited the install shell loses npm).
|
||||
// process PATH appended last when no login-shell PATH exists — the case
|
||||
// where the composed PATH would otherwise be Buzz's managed Node dirs
|
||||
// alone, with no `curl`/`sh`/`tar` for the vendor install pipes
|
||||
// (cmd.env("PATH", …) replaces rather than extends). On Windows that case
|
||||
// is the steady state: login_shell_path() always returns None there
|
||||
// because Git Bash paths are POSIX-shaped and poison native children.
|
||||
//
|
||||
// The composed PATH is set twice on purpose off Windows: `cmd.env` so the
|
||||
// login startup files themselves run with a usable PATH, and the `$1`
|
||||
// export in `install_shell_args` so their own PATH assignments cannot undo
|
||||
// it. Neither is redundant — see `install_shell_args`, which also explains
|
||||
// why the export is suppressed on Windows.
|
||||
let login_path = crate::managed_agents::login_shell_path();
|
||||
let had_login = login_path.is_some();
|
||||
let managed: Vec<std::path::PathBuf> = [
|
||||
@@ -734,14 +797,20 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
let inherited: Vec<std::path::PathBuf> = std::env::var_os("PATH")
|
||||
.map(|p| std::env::split_paths(&p).collect())
|
||||
.unwrap_or_default();
|
||||
let use_inherited = crate::managed_agents::should_use_inherited(had_login, true, cfg!(windows));
|
||||
let use_inherited = crate::managed_agents::should_use_inherited(had_login, true);
|
||||
let path_parts =
|
||||
crate::managed_agents::compose_path_entries(managed, login, inherited, use_inherited);
|
||||
if !path_parts.is_empty() {
|
||||
if let Ok(path) = std::env::join_paths(path_parts) {
|
||||
cmd.env("PATH", path);
|
||||
}
|
||||
let composed_path = (!path_parts.is_empty())
|
||||
.then(|| std::env::join_paths(path_parts).ok())
|
||||
.flatten();
|
||||
if let Some(path) = composed_path.as_deref() {
|
||||
cmd.env("PATH", path);
|
||||
}
|
||||
cmd.args(install_shell_args(
|
||||
command,
|
||||
composed_path.as_deref(),
|
||||
cfg!(windows),
|
||||
));
|
||||
|
||||
// Detach from the controlling terminal so install scripts that read from
|
||||
// /dev/tty (e.g. Codex's "Start Codex now? [y/N]") fall back to stdin
|
||||
@@ -1553,6 +1622,123 @@ mod tests {
|
||||
assert!(result.is_ok(), "install_shell_command must succeed on Unix");
|
||||
}
|
||||
|
||||
// ── pipefail: install pipes must not mask a failing left-hand side ────────
|
||||
|
||||
/// The command handed to the install shell must run under `set -o pipefail;`
|
||||
/// with the vendor command preserved verbatim, so `curl … | bash` fails when
|
||||
/// `curl` does. Platform-agnostic: only the PATH prelude differs by OS, and
|
||||
/// `test_install_shell_args_shape_per_platform` pins that.
|
||||
#[test]
|
||||
fn test_install_shell_command_enables_pipefail() {
|
||||
let cmd = super::install_shell_command("curl -fsSL https://example.test/i.sh | bash")
|
||||
.expect("install shell must resolve on a test host");
|
||||
let args: Vec<String> = cmd
|
||||
.get_args()
|
||||
.map(|a| a.to_string_lossy().into_owned())
|
||||
.collect();
|
||||
let body = &args[2];
|
||||
assert!(
|
||||
body.contains("set -o pipefail; "),
|
||||
"the install body must set pipefail; got: {body}"
|
||||
);
|
||||
assert!(
|
||||
body.ends_with("curl -fsSL https://example.test/i.sh | bash"),
|
||||
"the vendor command must be preserved verbatim; got: {body}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The PATH prelude is emitted only where it helps, and the exact argument
|
||||
/// vector is the contract: a stray trailing positional with no `$1` reader,
|
||||
/// or an export whose `$1` the shell cannot split, both corrupt PATH.
|
||||
/// Windows is excluded because `join_paths` is `;`-separated there while bash
|
||||
/// splits PATH on `:` — and it is the platform where the inherited fallback
|
||||
/// always fires. See `install_shell_args` for the full reasoning.
|
||||
#[test]
|
||||
fn test_install_shell_args_shape_per_platform() {
|
||||
let composed = std::ffi::OsString::from("/buzz/node/bin:/usr/bin");
|
||||
let windows_composed = std::ffi::OsString::from(r"C:\buzz\node;C:\Windows\system32");
|
||||
let bare = ["-l", "-c", "set -o pipefail; echo hi"].map(std::ffi::OsString::from);
|
||||
|
||||
assert_eq!(
|
||||
super::install_shell_args("echo hi", Some(&composed), false),
|
||||
[
|
||||
"-l",
|
||||
"-c",
|
||||
"export PATH=\"$1\"; set -o pipefail; echo hi",
|
||||
"buzz-install",
|
||||
"/buzz/node/bin:/usr/bin",
|
||||
]
|
||||
.map(std::ffi::OsString::from),
|
||||
"Unix must re-export the composed PATH after login init"
|
||||
);
|
||||
assert_eq!(
|
||||
super::install_shell_args("echo hi", Some(&windows_composed), true),
|
||||
bare,
|
||||
"Windows must not re-export a `;`-joined PATH inside bash"
|
||||
);
|
||||
assert_eq!(
|
||||
super::install_shell_args("echo hi", None, false),
|
||||
bare,
|
||||
"no composed PATH must yield the bare pipefail body and no positionals"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression for the login-startup-file overwrite: `cmd.env("PATH", …)` is
|
||||
/// installed *before* `-l` sources the user's profile, so a profile that
|
||||
/// assigns PATH silently discards the composed one. Uses `/bin/bash`
|
||||
/// explicitly — the planted profile is bash-specific, so resolving the host
|
||||
/// shell (which prefers zsh) would make this vacuous.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_composed_path_survives_a_profile_that_clears_it() {
|
||||
let home = tempfile::tempdir().expect("temp HOME");
|
||||
std::fs::write(home.path().join(".bash_profile"), "export PATH=\n")
|
||||
.expect("plant a hostile login profile");
|
||||
let composed = std::ffi::OsString::from("/buzz/sentinel/bin:/usr/bin:/bin");
|
||||
|
||||
// `echo` is a shell builtin, so the child needs no PATH to report one.
|
||||
let out = std::process::Command::new("/bin/bash")
|
||||
.args(super::install_shell_args(
|
||||
"echo \"$PATH\"",
|
||||
Some(&composed),
|
||||
false,
|
||||
))
|
||||
.env("HOME", home.path())
|
||||
.env("PATH", &composed)
|
||||
.stdin(std::process::Stdio::null())
|
||||
.output()
|
||||
.expect("bash must spawn");
|
||||
|
||||
let path = String::from_utf8_lossy(&out.stdout);
|
||||
assert!(
|
||||
path.contains("/buzz/sentinel/bin"),
|
||||
"the composed PATH must survive login init; got: {path:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// End-to-end on the real resolved install shell (no network): a pipeline
|
||||
/// whose left-hand side fails must exit non-zero, while a fully successful
|
||||
/// pipeline must still succeed. Without `pipefail` the status is the
|
||||
/// right-hand side's and the left-hand failure is invisible.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_install_shell_pipeline_status_follows_left_side() {
|
||||
for (command, expect_success) in [("false | true", false), ("echo ok | cat", true)] {
|
||||
let status = super::install_shell_command(command)
|
||||
.expect("Unix must always resolve an install shell")
|
||||
.stdin(std::process::Stdio::null())
|
||||
.stdout(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.status()
|
||||
.expect("install shell must spawn");
|
||||
assert_eq!(
|
||||
status.success(),
|
||||
expect_success,
|
||||
"`{command}` must report success={expect_success}; got {status:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Phase A: Windows install shell selection ───────────────────────────────
|
||||
|
||||
/// On Windows (CI runner has Git pre-installed), resolve_install_shell succeeds.
|
||||
|
||||
@@ -89,13 +89,21 @@ pub async fn get_agent_models(
|
||||
.map(|p| p.display().to_string())
|
||||
.unwrap_or_else(|| descriptor.command.clone());
|
||||
|
||||
let discovery_model = record.model.clone();
|
||||
// The record's saved provider is a deliberate choice and wins; when the
|
||||
// record has none, `effective_discovery_provider` below recovers the
|
||||
// provider the agent will actually launch with from the runtime's own
|
||||
// provider env var, read out of the descriptor env (which already
|
||||
// layers definition/persona/global values the same way spawn does).
|
||||
let discovery_provider = record.provider.clone();
|
||||
// ModelPicker can persist a selected model but not rewrite the saved
|
||||
// provider/env snapshot, and runtime spawn reads that same snapshot.
|
||||
// Model and provider come from the same authoritative resolver spawn
|
||||
// uses (`resolve_effective_model_provider`) — linked instances read
|
||||
// their definition, never a stale materialized `record.model` /
|
||||
// `record.provider`, so discovery cannot query a provider this agent
|
||||
// will not actually launch with. Definition-less instances keep their
|
||||
// own record values, matching spawn's `resolve_definition_less` arm.
|
||||
// When the resolver yields no provider, `effective_discovery_provider`
|
||||
// below recovers the provider the agent will actually launch with from
|
||||
// the runtime's own provider env var, read out of the descriptor env
|
||||
// (which already layers definition/persona/global values the same way
|
||||
// spawn does).
|
||||
let (discovery_model, discovery_provider) =
|
||||
crate::managed_agents::resolve_effective_model_provider(record, &personas, &global);
|
||||
let discovery_provider_env_var =
|
||||
known_acp_runtime(&descriptor.command).and_then(|meta| meta.provider_env_var);
|
||||
let discovery_env = descriptor.env;
|
||||
@@ -777,6 +785,35 @@ async fn discover_databricks_models(
|
||||
}))
|
||||
}
|
||||
|
||||
/// Apply an `UpdateManagedAgentRequest`'s model/provider/system_prompt patch
|
||||
/// to `record`, enforcing the linked-instance write guard: a definition-linked
|
||||
/// record's model/provider/prompt are definition-authoritative (see
|
||||
/// `effective_config::resolve_linked`), so writes to these three fields are
|
||||
/// silently dropped for a linked instance rather than persisting a byte the
|
||||
/// resolver will never read. Definition-less instances accept the patch
|
||||
/// as-is. Extracted so the guard is exercised by both `update_managed_agent`
|
||||
/// and its regression tests — a test that reimplements this check instead of
|
||||
/// calling it can go green after the real guard is deleted.
|
||||
fn apply_model_provider_prompt_update(
|
||||
record: &mut crate::managed_agents::ManagedAgentRecord,
|
||||
model: Option<Option<String>>,
|
||||
provider: Option<Option<String>>,
|
||||
system_prompt: Option<Option<String>>,
|
||||
) {
|
||||
if record.persona_id.is_some() {
|
||||
return;
|
||||
}
|
||||
if let Some(model_update) = model {
|
||||
record.model = model_update;
|
||||
}
|
||||
if let Some(provider_update) = provider {
|
||||
record.provider = provider_update;
|
||||
}
|
||||
if let Some(prompt_update) = system_prompt {
|
||||
record.system_prompt = prompt_update;
|
||||
}
|
||||
}
|
||||
|
||||
/// Update mutable fields on an existing managed agent record.
|
||||
///
|
||||
/// Does NOT auto-restart the agent. Runtime config changes (system prompt,
|
||||
@@ -816,15 +853,12 @@ pub async fn update_managed_agent(
|
||||
name_changed = true;
|
||||
}
|
||||
}
|
||||
if let Some(model_update) = input.model {
|
||||
record.model = model_update;
|
||||
}
|
||||
if let Some(provider_update) = input.provider {
|
||||
record.provider = provider_update;
|
||||
}
|
||||
if let Some(prompt_update) = input.system_prompt {
|
||||
record.system_prompt = prompt_update;
|
||||
}
|
||||
apply_model_provider_prompt_update(
|
||||
record,
|
||||
input.model,
|
||||
input.provider,
|
||||
input.system_prompt,
|
||||
);
|
||||
if let Some(parallelism) = input.parallelism {
|
||||
record.parallelism = parallelism;
|
||||
}
|
||||
|
||||
@@ -164,7 +164,7 @@ fn redaction_env_records_value_used_for_request() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saved_agent_model_discovery_uses_record_snapshot() {
|
||||
fn saved_agent_model_discovery_uses_record_snapshot_for_definition_less_agent() {
|
||||
let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str(
|
||||
r#"{
|
||||
"pubkey": "abcd1234",
|
||||
@@ -355,6 +355,84 @@ fn effective_discovery_provider_reads_the_runtimes_own_env_var() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Definition-authoritative: a linked agent's stale materialized
|
||||
/// `record.model`/`record.provider` must never drive model discovery — the
|
||||
/// linked definition's current model/provider wins, mirroring spawn's
|
||||
/// `resolve_effective_model_provider`.
|
||||
#[test]
|
||||
fn model_discovery_ignores_stale_record_for_linked_agent() {
|
||||
let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str(
|
||||
r#"{
|
||||
"pubkey": "abcd1234",
|
||||
"name": "test-agent",
|
||||
"persona_id": "persona-1",
|
||||
"private_key_nsec": "nsec1fake",
|
||||
"relay_url": "wss://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"system_prompt": null,
|
||||
"model": "stale-record-model",
|
||||
"provider": "stale-record-provider",
|
||||
"env_vars": {},
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"last_started_at": null,
|
||||
"last_stopped_at": null,
|
||||
"last_exit_code": null,
|
||||
"last_error": null
|
||||
}"#,
|
||||
)
|
||||
.expect("sample managed agent record");
|
||||
|
||||
let persona = crate::managed_agents::AgentDefinition {
|
||||
id: "persona-1".to_string(),
|
||||
display_name: "Persona".to_string(),
|
||||
avatar_url: None,
|
||||
system_prompt: "You are a persona.".to_string(),
|
||||
runtime: Some("goose".to_string()),
|
||||
model: Some("persona-model".to_string()),
|
||||
provider: Some("anthropic".to_string()),
|
||||
name_pool: Vec::new(),
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
env_vars: BTreeMap::new(),
|
||||
respond_to: None,
|
||||
respond_to_allowlist: Vec::new(),
|
||||
parallelism: None,
|
||||
created_at: "".to_string(),
|
||||
updated_at: "".to_string(),
|
||||
};
|
||||
|
||||
// get_agent_models resolves model/provider through the authoritative
|
||||
// resolver (the same one spawn uses) — the stale record bytes must lose.
|
||||
let personas = [persona];
|
||||
let global = crate::managed_agents::GlobalAgentConfig::default();
|
||||
let (model, provider) =
|
||||
crate::managed_agents::resolve_effective_model_provider(&record, &personas, &global);
|
||||
assert_eq!(model.as_deref(), Some("persona-model"));
|
||||
assert_eq!(provider.as_deref(), Some("anthropic"));
|
||||
|
||||
// And the discovery env comes from the descriptor, whose layering also
|
||||
// resolves through the definition — the derived model env var must carry
|
||||
// the persona's model, not the stale record snapshot.
|
||||
let descriptor =
|
||||
crate::managed_agents::resolve_effective_harness_descriptor(&record, &personas, &global)
|
||||
.expect("descriptor should resolve for a linked record");
|
||||
assert_eq!(
|
||||
descriptor.env.get("GOOSE_MODEL").map(String::as_str),
|
||||
Some("persona-model")
|
||||
);
|
||||
assert_eq!(
|
||||
descriptor.env.get("GOOSE_PROVIDER").map(String::as_str),
|
||||
Some("anthropic")
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Databricks provider detection
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -395,6 +473,104 @@ fn update_request_turn_timeout_parses_for_wire_compat() {
|
||||
assert_eq!(req.turn_timeout_seconds, Some(9999));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Linked-instance write guard (model/provider/prompt)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn linked_instance_ignores_model_provider_prompt_writes() {
|
||||
let mut record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str(
|
||||
r#"{
|
||||
"pubkey": "linked1",
|
||||
"name": "linked-agent",
|
||||
"persona_id": "p1",
|
||||
"private_key_nsec": "nsec1fake",
|
||||
"relay_url": "wss://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"system_prompt": null,
|
||||
"model": null,
|
||||
"provider": null,
|
||||
"env_vars": {},
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"last_started_at": null,
|
||||
"last_stopped_at": null,
|
||||
"last_exit_code": null,
|
||||
"last_error": null
|
||||
}"#,
|
||||
)
|
||||
.expect("linked agent record");
|
||||
|
||||
let is_linked = record.persona_id.is_some();
|
||||
assert!(is_linked, "test setup: record must be linked");
|
||||
|
||||
crate::commands::agent_models::apply_model_provider_prompt_update(
|
||||
&mut record,
|
||||
Some(Some("explicit-model".to_string())),
|
||||
Some(Some("explicit-prov".to_string())),
|
||||
Some(Some("explicit-prompt".to_string())),
|
||||
);
|
||||
|
||||
assert!(
|
||||
record.model.is_none(),
|
||||
"linked record model must not be updated"
|
||||
);
|
||||
assert!(
|
||||
record.provider.is_none(),
|
||||
"linked record provider must not be updated"
|
||||
);
|
||||
assert!(
|
||||
record.system_prompt.is_none(),
|
||||
"linked record system_prompt must not be updated"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn definition_less_instance_accepts_model_provider_prompt_writes() {
|
||||
let mut record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str(
|
||||
r#"{
|
||||
"pubkey": "standalone1",
|
||||
"name": "standalone-agent",
|
||||
"private_key_nsec": "nsec1fake",
|
||||
"relay_url": "wss://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"system_prompt": null,
|
||||
"model": null,
|
||||
"provider": null,
|
||||
"env_vars": {},
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"last_started_at": null,
|
||||
"last_stopped_at": null,
|
||||
"last_exit_code": null,
|
||||
"last_error": null
|
||||
}"#,
|
||||
)
|
||||
.expect("standalone agent record");
|
||||
|
||||
let is_linked = record.persona_id.is_some();
|
||||
assert!(!is_linked, "test setup: record must not be linked");
|
||||
|
||||
crate::commands::agent_models::apply_model_provider_prompt_update(
|
||||
&mut record,
|
||||
Some(Some("new-model".to_string())),
|
||||
Some(Some("new-prov".to_string())),
|
||||
Some(Some("new-prompt".to_string())),
|
||||
);
|
||||
|
||||
assert_eq!(record.model.as_deref(), Some("new-model"));
|
||||
assert_eq!(record.provider.as_deref(), Some("new-prov"));
|
||||
assert_eq!(record.system_prompt.as_deref(), Some("new-prompt"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_databricks_provider_matches_both_variants() {
|
||||
assert!(is_databricks_provider(Some("databricks")));
|
||||
|
||||
@@ -293,16 +293,16 @@ fn resolve_created_avatar_url(
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
async fn ensure_relay_mesh_for_record(
|
||||
app: &AppHandle,
|
||||
record: &ManagedAgentRecord,
|
||||
model_id: Option<&str>,
|
||||
allow_fresh_create_start: bool,
|
||||
) -> Result<(), String> {
|
||||
crate::commands::ensure_relay_mesh_for_record(app, record, allow_fresh_create_start).await
|
||||
crate::commands::ensure_relay_mesh_for_record(app, model_id, allow_fresh_create_start).await
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "mesh-llm"))]
|
||||
async fn ensure_relay_mesh_for_record(
|
||||
_app: &AppHandle,
|
||||
_record: &ManagedAgentRecord,
|
||||
_model_id: Option<&str>,
|
||||
_allow_fresh_create_start: bool,
|
||||
) -> Result<(), String> {
|
||||
Ok(())
|
||||
@@ -327,7 +327,16 @@ pub(super) async fn start_local_agent_pairs_with_preflight(
|
||||
if record_snapshot.backend != BackendKind::Local {
|
||||
return Err(format!("agent {pubkey} is not a local agent"));
|
||||
}
|
||||
ensure_relay_mesh_for_record(app, &record_snapshot, false).await?;
|
||||
let personas_for_preflight = load_personas(app).unwrap_or_default();
|
||||
let global_for_preflight =
|
||||
crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
let mesh_model_id =
|
||||
crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id(
|
||||
&record_snapshot,
|
||||
&personas_for_preflight,
|
||||
&global_for_preflight,
|
||||
);
|
||||
ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), false).await?;
|
||||
|
||||
{
|
||||
let _store_guard = state
|
||||
@@ -413,7 +422,22 @@ pub(super) async fn start_local_agent_with_preflight(
|
||||
return Err(format!("agent {pubkey} is not a local agent"));
|
||||
}
|
||||
|
||||
ensure_relay_mesh_for_record(app, &record_snapshot, allow_fresh_create_start).await?;
|
||||
// Preflight against the same resolution spawn uses — `resolve_effective_config`
|
||||
// (definition → global fallback). A linked instance's own `provider`/`model`/
|
||||
// `relay_mesh` bytes never contribute: this reads the CURRENT definition
|
||||
// directly, so a definition edit that flips `provider` to/from relay-mesh
|
||||
// between saves is reflected here without needing a prospective re-snapshot;
|
||||
// for a global-inherited blank definition, it also folds in the global
|
||||
// default, which record-byte sniffing could never see.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
let mesh_model_id =
|
||||
crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id(
|
||||
&record_snapshot,
|
||||
&personas,
|
||||
&global,
|
||||
);
|
||||
ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start).await?;
|
||||
|
||||
let _store_guard = state
|
||||
.managed_agents_store_lock
|
||||
@@ -437,9 +461,16 @@ pub(super) async fn start_local_agent_with_preflight(
|
||||
// at the end — avoids a second disk read for the same file in the same call.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
if let Some(persona_id) = record.persona_id.clone() {
|
||||
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
|
||||
crate::managed_agents::persona_events::apply_persona_snapshot(record, persona);
|
||||
record.updated_at = crate::util::now_iso();
|
||||
match personas.iter().find(|p| p.id == persona_id) {
|
||||
Some(persona) => {
|
||||
crate::managed_agents::persona_events::apply_persona_snapshot(record, persona);
|
||||
record.updated_at = crate::util::now_iso();
|
||||
}
|
||||
None => {
|
||||
return Err(
|
||||
crate::managed_agents::effective_config::ORPHANED_INSTANCE_ERROR.to_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
start_managed_agent_process(app, record, &mut runtimes, Some(owner_hex))?;
|
||||
|
||||
@@ -4,42 +4,36 @@
|
||||
|
||||
use tauri::AppHandle;
|
||||
|
||||
#[cfg(test)]
|
||||
use crate::managed_agents::AgentDefinition;
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{load_personas, AgentDefinition, ManagedAgentRecord},
|
||||
managed_agents::{load_personas, ManagedAgentRecord},
|
||||
relay::relay_ws_url_with_override,
|
||||
};
|
||||
|
||||
/// Resolve the deploy-specific structured model/provider for a managed agent.
|
||||
///
|
||||
/// Deploy uses **live-persona-first** precedence so remote agents receive
|
||||
/// current config after a persona update, without requiring delete+recreate.
|
||||
/// Unlike local spawn (which re-snapshots the persona onto `record` at the
|
||||
/// start of every spawn), provider start does not re-snapshot — so the
|
||||
/// record may hold a stale snapshot while the linked persona has moved on.
|
||||
/// Delegates to the single effective-config resolver which enforces
|
||||
/// definition-authoritative semantics for linked instances:
|
||||
/// - **Linked:** definition → global. Stale record bytes are never consulted.
|
||||
/// - **Definition-less:** instance → global.
|
||||
/// - **Orphaned:** returns `(None, None)` — spawn is blocked elsewhere.
|
||||
///
|
||||
/// Precedence: live-persona → record (snapshot fallback) → global.
|
||||
/// Symmetric for both model and provider.
|
||||
/// Both local spawn and deploy now use the same resolver, so they can never
|
||||
/// disagree on what model/provider an agent runs with.
|
||||
///
|
||||
/// Exported `pub(crate)` for unit testing.
|
||||
pub(crate) fn resolve_deploy_model_provider<'a>(
|
||||
record: &'a ManagedAgentRecord,
|
||||
personas: &'a [AgentDefinition],
|
||||
global: &'a crate::managed_agents::GlobalAgentConfig,
|
||||
) -> (Option<&'a str>, Option<&'a str>) {
|
||||
let live_persona = record
|
||||
.persona_id
|
||||
.as_deref()
|
||||
.and_then(|pid| personas.iter().find(|p| p.id == pid));
|
||||
let model = live_persona
|
||||
.and_then(|p| p.model.as_deref())
|
||||
.or(record.model.as_deref())
|
||||
.or(global.model.as_deref());
|
||||
let provider = live_persona
|
||||
.and_then(|p| p.provider.as_deref())
|
||||
.or(record.provider.as_deref())
|
||||
.or(global.provider.as_deref());
|
||||
(model, provider)
|
||||
#[cfg(test)]
|
||||
pub(crate) fn resolve_deploy_model_provider(
|
||||
record: &ManagedAgentRecord,
|
||||
personas: &[AgentDefinition],
|
||||
global: &crate::managed_agents::GlobalAgentConfig,
|
||||
) -> (Option<String>, Option<String>) {
|
||||
crate::managed_agents::effective_config::resolve_effective_model_provider_pair(
|
||||
record, personas, global,
|
||||
)
|
||||
.unwrap_or((None, None))
|
||||
}
|
||||
|
||||
/// Build the standard agent JSON payload for provider deploy calls.
|
||||
@@ -84,15 +78,16 @@ pub(super) fn build_deploy_payload(
|
||||
let merged_env =
|
||||
crate::managed_agents::merged_user_env(&global_persona_merged, &record.env_vars);
|
||||
|
||||
// Resolve the deploy-specific structured provider/model. Uses the deploy
|
||||
// resolver with live-persona → record → global precedence.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
let (effective_model, effective_provider) =
|
||||
resolve_deploy_model_provider(record, &personas, &global_config);
|
||||
let (effective_model, effective_provider) = (
|
||||
effective_model.map(str::to_string),
|
||||
effective_provider.map(str::to_string),
|
||||
);
|
||||
let cfg = crate::managed_agents::effective_config::resolve_effective_config(
|
||||
record,
|
||||
&personas,
|
||||
&global_config,
|
||||
)
|
||||
.require_resolved()?;
|
||||
let effective_model = cfg.model.value;
|
||||
let effective_provider = cfg.provider.value;
|
||||
let effective_prompt = cfg.system_prompt.value;
|
||||
|
||||
Ok(deploy_payload_json(
|
||||
record,
|
||||
@@ -102,6 +97,7 @@ pub(super) fn build_deploy_payload(
|
||||
),
|
||||
effective_model,
|
||||
effective_provider,
|
||||
effective_prompt,
|
||||
merged_env,
|
||||
))
|
||||
}
|
||||
@@ -114,36 +110,25 @@ pub(super) fn deploy_payload_json(
|
||||
relay_url: String,
|
||||
effective_model: Option<String>,
|
||||
effective_provider: Option<String>,
|
||||
effective_prompt: Option<String>,
|
||||
merged_env: std::collections::BTreeMap<String, String>,
|
||||
) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"name": &record.name,
|
||||
// Resolve the per-agent pin against the active workspace relay here:
|
||||
// this payload crosses the host boundary to a remote provider harness
|
||||
// that has no notion of the desktop's workspace, so the blank→workspace
|
||||
// fallback (otherwise applied at read-time in `effective_agent_relay_url`)
|
||||
// must be materialized into a concrete URL before serializing.
|
||||
"relay_url": relay_url,
|
||||
"private_key_nsec": &record.private_key_nsec,
|
||||
"auth_tag": &record.auth_tag,
|
||||
"agent_command": &record.agent_command,
|
||||
"agent_args": &record.agent_args,
|
||||
"system_prompt": &record.system_prompt,
|
||||
"system_prompt": effective_prompt,
|
||||
"model": effective_model,
|
||||
// Structured provider from the persona record. Providers that don't
|
||||
// yet read this field will fall back to env_vars or their own default
|
||||
// — no protocol break.
|
||||
"provider": effective_provider,
|
||||
"turn_timeout_seconds": record.turn_timeout_seconds,
|
||||
"idle_timeout_seconds": record.idle_timeout_seconds,
|
||||
"max_turn_duration_seconds": record.max_turn_duration_seconds,
|
||||
"parallelism": record.parallelism,
|
||||
// Inbound author gate. Providers that don't yet read these fall back
|
||||
// to the harness default (`owner-only`) — no protocol break.
|
||||
"respond_to": record.respond_to,
|
||||
"respond_to_allowlist": &record.respond_to_allowlist,
|
||||
// Merged persona + agent env vars. Providers that don't read this
|
||||
// field will simply ignore it — no protocol break.
|
||||
"env_vars": merged_env,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -126,57 +126,56 @@ fn build_agent_archive_request_attaches_owner_auth_and_retired_reason() {
|
||||
}));
|
||||
}
|
||||
|
||||
/// Deploy-path regression for Fix 1 of Thufir pass-2: a persona-linked
|
||||
/// provider agent with a stale record snapshot must use the live persona
|
||||
/// model/provider in the deploy payload, not the stale record values.
|
||||
///
|
||||
/// Scenario: agent was created with persona at model="old-model"/provider="old-prov".
|
||||
/// The persona was subsequently updated to "new-model"/"new-prov" but the record
|
||||
/// was NOT re-snapshotted (provider start skips re-snapshot; local spawn does it).
|
||||
/// The deploy resolver must use the current persona values.
|
||||
///
|
||||
/// Fails against `resolve_effective_model_provider` (record-first precedence),
|
||||
/// which would return "old-model"/"old-prov" from the stale record.
|
||||
/// Deploy resolver uses definition model/provider, ignoring stale record.
|
||||
#[test]
|
||||
fn deploy_resolver_uses_live_persona_over_stale_record_snapshot() {
|
||||
// Record holds the stale snapshot (created when persona had old values).
|
||||
fn deploy_resolver_uses_definition_over_stale_record() {
|
||||
let record = bare_agent_record(Some("p1"), Some("old-model"), Some("old-prov"));
|
||||
// Live persona has been updated since the record was snapshotted.
|
||||
let personas = vec![persona_record("p1", Some("new-model"), Some("new-prov"))];
|
||||
let global = crate::managed_agents::GlobalAgentConfig::default();
|
||||
|
||||
let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(
|
||||
model,
|
||||
model.as_deref(),
|
||||
Some("new-model"),
|
||||
"deploy must use live persona model, not stale record snapshot"
|
||||
"deploy must use definition model, not stale record snapshot"
|
||||
);
|
||||
assert_eq!(
|
||||
provider,
|
||||
provider.as_deref(),
|
||||
Some("new-prov"),
|
||||
"deploy must use live persona provider, not stale record snapshot"
|
||||
"deploy must use definition provider, not stale record snapshot"
|
||||
);
|
||||
}
|
||||
|
||||
/// Deploy resolver falls back to record when persona has no model/provider
|
||||
/// (persona without structured model — fallback to record snapshot).
|
||||
/// When a linked definition has blank model/provider (inherit), the deploy
|
||||
/// resolver must fall through to global — stale record bytes are inert.
|
||||
#[test]
|
||||
fn deploy_resolver_falls_back_to_record_when_persona_has_none() {
|
||||
let record = bare_agent_record(Some("p1"), Some("record-model"), Some("record-prov"));
|
||||
// Persona exists but has no model/provider.
|
||||
fn deploy_resolver_inherits_global_when_definition_blank() {
|
||||
let record = bare_agent_record(Some("p1"), Some("stale-model"), Some("stale-prov"));
|
||||
let personas = vec![persona_record("p1", None, None)];
|
||||
let global = crate::managed_agents::GlobalAgentConfig::default();
|
||||
let global = crate::managed_agents::GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-prov".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"));
|
||||
assert_eq!(provider, Some("record-prov"));
|
||||
assert_eq!(
|
||||
model.as_deref(),
|
||||
Some("global-model"),
|
||||
"definition blank → global; stale record ignored"
|
||||
);
|
||||
assert_eq!(
|
||||
provider.as_deref(),
|
||||
Some("global-prov"),
|
||||
"definition blank → global; stale record ignored"
|
||||
);
|
||||
}
|
||||
|
||||
/// Deploy resolver falls back to global when both persona and record have none.
|
||||
/// Deploy resolver falls back to global when both definition and record have none.
|
||||
#[test]
|
||||
fn deploy_resolver_falls_back_to_global_when_persona_and_record_have_none() {
|
||||
fn deploy_resolver_falls_back_to_global_when_definition_and_record_have_none() {
|
||||
let record = bare_agent_record(Some("p1"), None, None);
|
||||
let personas = vec![persona_record("p1", None, None)];
|
||||
let global = crate::managed_agents::GlobalAgentConfig {
|
||||
@@ -187,8 +186,35 @@ fn deploy_resolver_falls_back_to_global_when_persona_and_record_have_none() {
|
||||
|
||||
let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("global-model"));
|
||||
assert_eq!(provider, Some("global-prov"));
|
||||
assert_eq!(model.as_deref(), Some("global-model"));
|
||||
assert_eq!(provider.as_deref(), Some("global-prov"));
|
||||
}
|
||||
|
||||
/// Orphan: linked record with missing definition → the pure model/provider
|
||||
/// pair resolver returns `(None, None)`. This is NOT the deploy refusal
|
||||
/// boundary — `build_deploy_payload` refuses an orphan outright via
|
||||
/// `.require_resolved()?` before this pair is ever computed. This test pins
|
||||
/// the resolver's own orphan behavior, which readiness/hash also depend on.
|
||||
#[test]
|
||||
fn deploy_resolver_returns_none_for_orphaned_instance() {
|
||||
let record = bare_agent_record(Some("missing-def"), Some("stale-model"), Some("stale-prov"));
|
||||
let personas: Vec<AgentDefinition> = vec![];
|
||||
let global = crate::managed_agents::GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-prov".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global);
|
||||
|
||||
assert!(
|
||||
model.is_none(),
|
||||
"orphaned instance must not resolve to any model"
|
||||
);
|
||||
assert!(
|
||||
provider.is_none(),
|
||||
"orphaned instance must not resolve to any provider"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -404,6 +430,7 @@ fn deploy_payload_carries_the_full_behavioral_quad() {
|
||||
"wss://relay.example".to_string(),
|
||||
Some("gpt-x".to_string()),
|
||||
Some("openai".to_string()),
|
||||
None,
|
||||
std::collections::BTreeMap::new(),
|
||||
);
|
||||
|
||||
|
||||
@@ -481,11 +481,11 @@ fn pick_serve_target_for_model(
|
||||
/// Non relay-mesh records are a no-op.
|
||||
pub(crate) async fn ensure_relay_mesh_for_record(
|
||||
app: &AppHandle,
|
||||
record: &crate::managed_agents::ManagedAgentRecord,
|
||||
model_id: Option<&str>,
|
||||
_allow_fresh_create_start: bool,
|
||||
) -> Result<(), String> {
|
||||
let state = app.state::<AppState>();
|
||||
let Some(model_id) = crate::managed_agents::relay_mesh_model_id(record) else {
|
||||
let Some(model_id) = model_id else {
|
||||
return Ok(());
|
||||
};
|
||||
// A local serve/client runtime already owns the OpenAI ingress and its
|
||||
@@ -505,7 +505,7 @@ pub(crate) async fn ensure_relay_mesh_for_record(
|
||||
.await
|
||||
{
|
||||
mesh_llm::MeshRuntimeRecovery::Live => {
|
||||
return wait_for_mesh_inference(&model_id).await;
|
||||
return wait_for_mesh_inference(model_id).await;
|
||||
}
|
||||
mesh_llm::MeshRuntimeRecovery::Evicted | mesh_llm::MeshRuntimeRecovery::Absent => {}
|
||||
mesh_llm::MeshRuntimeRecovery::Debouncing => {
|
||||
@@ -521,7 +521,7 @@ pub(crate) async fn ensure_relay_mesh_for_record(
|
||||
);
|
||||
}
|
||||
mesh_llm::MeshRuntimeRecovery::Replaced => {
|
||||
return wait_for_mesh_inference(&model_id).await;
|
||||
return wait_for_mesh_inference(model_id).await;
|
||||
}
|
||||
mesh_llm::MeshRuntimeRecovery::RestartRequired => {
|
||||
app.request_restart();
|
||||
@@ -532,7 +532,7 @@ pub(crate) async fn ensure_relay_mesh_for_record(
|
||||
}
|
||||
}
|
||||
}
|
||||
let target = match resolve_mesh_bootstrap_target(&state, &model_id).await {
|
||||
let target = match resolve_mesh_bootstrap_target(&state, model_id).await {
|
||||
Ok(Some(target)) => target,
|
||||
Ok(None) => {
|
||||
return Err(
|
||||
@@ -556,8 +556,8 @@ pub(crate) async fn ensure_relay_mesh_for_record(
|
||||
// (the router resolves per-request), so it only cold-starts a Client when
|
||||
// there is genuinely no runtime. Falling back to Client if a serve node
|
||||
// crashed under local pressure is a desirable fail-safe, not a regression.
|
||||
ensure_client_node_for_model(&state, &model_id, Some(target.endpoint_addr)).await?;
|
||||
wait_for_mesh_inference(&model_id).await
|
||||
ensure_client_node_for_model(&state, model_id, Some(target.endpoint_addr)).await?;
|
||||
wait_for_mesh_inference(model_id).await
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
|
||||
318
desktop/src-tauri/src/managed_agents/effective_config/mod.rs
Normal file
318
desktop/src-tauri/src/managed_agents/effective_config/mod.rs
Normal file
@@ -0,0 +1,318 @@
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use serde::Serialize;
|
||||
|
||||
use super::global_config::GlobalAgentConfig;
|
||||
use super::relay_mesh::{
|
||||
RELAY_MESH_API_BASE_URL, RELAY_MESH_API_KEY_PLACEHOLDER, RELAY_MESH_AUTO_MODEL_ID,
|
||||
RELAY_MESH_PROVIDER_ID,
|
||||
};
|
||||
use super::types::{AgentDefinition, ManagedAgentRecord};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum ConfigSource {
|
||||
Definition,
|
||||
Global,
|
||||
InstanceLegacy,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ResolvedField<T> {
|
||||
pub value: Option<T>,
|
||||
pub source: ConfigSource,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct EffectiveAgentConfig {
|
||||
pub model: ResolvedField<String>,
|
||||
pub provider: ResolvedField<String>,
|
||||
pub system_prompt: ResolvedField<String>,
|
||||
}
|
||||
|
||||
impl EffectiveAgentConfig {
|
||||
/// The relay-mesh model id this config resolves to, or `None` when the
|
||||
/// effective provider isn't relay-mesh.
|
||||
///
|
||||
/// This is the single authoritative mesh decision for this config. Both
|
||||
/// the mesh preflight (interactive start, restore-on-launch) AND spawn's
|
||||
/// `apply_relay_mesh_env` block MUST derive their mesh gate from this
|
||||
/// method — never from a separate provider comparison — so the two paths
|
||||
/// are guaranteed to agree even when the stored provider string has leading
|
||||
/// or trailing whitespace. The provider is trimmed before matching;
|
||||
/// a blank effective model falls back to "auto", mirroring
|
||||
/// `apply_relay_mesh_env`'s own rule.
|
||||
pub fn relay_mesh_model_id(&self) -> Option<String> {
|
||||
if self.provider.value.as_deref().map(str::trim) != Some(RELAY_MESH_PROVIDER_ID) {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
self.model
|
||||
.value
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
.unwrap_or(RELAY_MESH_AUTO_MODEL_ID)
|
||||
.to_string(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum EffectiveConfigResult {
|
||||
Resolved(EffectiveAgentConfig),
|
||||
OrphanedInstance {
|
||||
record_pubkey: String,
|
||||
missing_persona_id: String,
|
||||
},
|
||||
}
|
||||
|
||||
fn non_blank(v: Option<&str>) -> Option<&str> {
|
||||
v.filter(|s| !s.trim().is_empty())
|
||||
}
|
||||
|
||||
fn resolve_linked(
|
||||
definition: &AgentDefinition,
|
||||
global: &GlobalAgentConfig,
|
||||
) -> EffectiveAgentConfig {
|
||||
let model = match non_blank(definition.model.as_deref()) {
|
||||
Some(m) => ResolvedField {
|
||||
value: Some(m.to_owned()),
|
||||
source: ConfigSource::Definition,
|
||||
},
|
||||
None => ResolvedField {
|
||||
value: global.model.clone(),
|
||||
source: ConfigSource::Global,
|
||||
},
|
||||
};
|
||||
|
||||
let provider = match non_blank(definition.provider.as_deref()) {
|
||||
Some(p) => ResolvedField {
|
||||
value: Some(p.to_owned()),
|
||||
source: ConfigSource::Definition,
|
||||
},
|
||||
None => ResolvedField {
|
||||
value: global.provider.clone(),
|
||||
source: ConfigSource::Global,
|
||||
},
|
||||
};
|
||||
|
||||
let system_prompt = ResolvedField {
|
||||
value: non_blank(Some(definition.system_prompt.as_str())).map(str::to_owned),
|
||||
source: ConfigSource::Definition,
|
||||
};
|
||||
|
||||
EffectiveAgentConfig {
|
||||
model,
|
||||
provider,
|
||||
system_prompt,
|
||||
}
|
||||
}
|
||||
|
||||
/// The API key value the relay-mesh preset wrote before the Jun-11 rename
|
||||
/// window (#960, `8f580f308`) changed `RELAY_MESH_API_KEY_PLACEHOLDER` from
|
||||
/// `"sprout-mesh-local"` to its current value. The old string is persisted as a
|
||||
/// *value* in `env_vars` on records created before that, and no migration ever
|
||||
/// rewrote it.
|
||||
const LEGACY_MESH_API_KEY_PLACEHOLDER: &str = "sprout-mesh-local";
|
||||
|
||||
/// The provider key the preset wrote before #971 (`8c8312932`) renamed it to
|
||||
/// `BUZZ_AGENT_PROVIDER`. That commit changed source literals only — persisted
|
||||
/// `env_vars` keys were never migrated.
|
||||
const LEGACY_MESH_PROVIDER_ENV_KEY: &str = "SPROUT_AGENT_PROVIDER";
|
||||
|
||||
/// The legacy env discriminator: recognizes the relay-mesh preset purely from
|
||||
/// the env vars a pre-typed-field record carries, returning its served model id.
|
||||
///
|
||||
/// All three sentinels must match — the local base URL alone is not enough,
|
||||
/// since a user may point their own OpenAI-compatible provider at the same
|
||||
/// port. The placeholder API key is what makes this Buzz's own preset.
|
||||
///
|
||||
/// Two of those sentinels were renamed in the same Jun-11 window, in separate
|
||||
/// commits, with neither migrating persisted records: the provider env *key*
|
||||
/// (#971) and the API key *value* (#960). Each is therefore accepted under
|
||||
/// either spelling, independently — a record straddling the window carries one
|
||||
/// old and one new. In both cases the current spelling is authoritative when
|
||||
/// present, so a record that has since been rewritten with a non-mesh value is
|
||||
/// not resurrected by the stale leftover beside it.
|
||||
///
|
||||
/// `OPENAI_COMPAT_BASE_URL` and `OPENAI_COMPAT_MODEL` were never renamed.
|
||||
/// Nothing beyond these two either/ors is loosened: every sentinel dropped
|
||||
/// widens the false-positive surface for a user's own openai-compatible agent.
|
||||
fn mesh_preset_env_model_id(env_vars: &BTreeMap<String, String>) -> Option<String> {
|
||||
let base_url = env_vars.get("OPENAI_COMPAT_BASE_URL")?.trim();
|
||||
if base_url.trim_end_matches('/') != RELAY_MESH_API_BASE_URL {
|
||||
return None;
|
||||
}
|
||||
let provider = env_vars
|
||||
.get("BUZZ_AGENT_PROVIDER")
|
||||
.or_else(|| env_vars.get(LEGACY_MESH_PROVIDER_ENV_KEY))?
|
||||
.trim();
|
||||
if provider != "openai" {
|
||||
return None;
|
||||
}
|
||||
let api_key = env_vars.get("OPENAI_COMPAT_API_KEY")?.trim();
|
||||
if api_key != RELAY_MESH_API_KEY_PLACEHOLDER && api_key != LEGACY_MESH_API_KEY_PLACEHOLDER {
|
||||
return None;
|
||||
}
|
||||
non_blank(env_vars.get("OPENAI_COMPAT_MODEL").map(String::as_str)).map(str::to_owned)
|
||||
}
|
||||
|
||||
/// The mesh model id a definition-less record carries in legacy form, or `None`
|
||||
/// when it is not a legacy mesh record.
|
||||
///
|
||||
/// Two shipped record generations predate `provider: "relay-mesh"` and are
|
||||
/// never rewritten on load, so they are still on disk:
|
||||
///
|
||||
/// - the typed `relay_mesh` marker, added before the record had a `provider`
|
||||
/// field at all;
|
||||
/// - before that, the mesh preset written straight into `env_vars`.
|
||||
///
|
||||
/// Consulted only by [`resolve_definition_less`]: a record with no definition
|
||||
/// has nothing authoritative to be overridden by, so its own bytes are the
|
||||
/// highest-precedence signal it has. A linked instance never reaches here.
|
||||
fn legacy_record_mesh_model_id(record: &ManagedAgentRecord) -> Option<String> {
|
||||
match &record.relay_mesh {
|
||||
// The marker itself is the mesh signal; a blank `model_ref` still means
|
||||
// mesh, resolved to the auto model exactly as `apply_relay_mesh_env`
|
||||
// and `relay_mesh_model_id` treat a blank model.
|
||||
Some(config) => Some(
|
||||
non_blank(Some(config.model_ref.as_str()))
|
||||
.unwrap_or(RELAY_MESH_AUTO_MODEL_ID)
|
||||
.to_owned(),
|
||||
),
|
||||
None => mesh_preset_env_model_id(&record.env_vars),
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_definition_less(
|
||||
record: &ManagedAgentRecord,
|
||||
global: &GlobalAgentConfig,
|
||||
) -> EffectiveAgentConfig {
|
||||
let model = match non_blank(record.model.as_deref()) {
|
||||
Some(m) => ResolvedField {
|
||||
value: Some(m.to_owned()),
|
||||
source: ConfigSource::InstanceLegacy,
|
||||
},
|
||||
None => ResolvedField {
|
||||
value: global.model.clone(),
|
||||
source: ConfigSource::Global,
|
||||
},
|
||||
};
|
||||
|
||||
let provider = match non_blank(record.provider.as_deref()) {
|
||||
Some(p) => ResolvedField {
|
||||
value: Some(p.to_owned()),
|
||||
source: ConfigSource::InstanceLegacy,
|
||||
},
|
||||
None => ResolvedField {
|
||||
value: global.provider.clone(),
|
||||
source: ConfigSource::Global,
|
||||
},
|
||||
};
|
||||
|
||||
let system_prompt = ResolvedField {
|
||||
value: non_blank(record.system_prompt.as_deref()).map(str::to_owned),
|
||||
source: ConfigSource::InstanceLegacy,
|
||||
};
|
||||
|
||||
let mut config = EffectiveAgentConfig {
|
||||
model,
|
||||
provider,
|
||||
system_prompt,
|
||||
};
|
||||
|
||||
// Legacy mesh compatibility. A record with an explicit `provider` has
|
||||
// already stated its intent — including switching AWAY from mesh, which
|
||||
// leaves the old marker and env bytes behind — so its legacy bytes are
|
||||
// never consulted. Only a record that never carried a provider at all
|
||||
// falls back, and both fields move together so the single mesh gate
|
||||
// (`relay_mesh_model_id`) and the spawned model agree.
|
||||
if non_blank(record.provider.as_deref()).is_none() {
|
||||
if let Some(model_ref) = legacy_record_mesh_model_id(record) {
|
||||
config.provider = ResolvedField {
|
||||
value: Some(RELAY_MESH_PROVIDER_ID.to_owned()),
|
||||
source: ConfigSource::InstanceLegacy,
|
||||
};
|
||||
config.model = ResolvedField {
|
||||
value: Some(model_ref),
|
||||
source: ConfigSource::InstanceLegacy,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
config
|
||||
}
|
||||
|
||||
pub fn resolve_effective_config(
|
||||
record: &ManagedAgentRecord,
|
||||
definitions: &[AgentDefinition],
|
||||
global: &GlobalAgentConfig,
|
||||
) -> EffectiveConfigResult {
|
||||
match &record.persona_id {
|
||||
Some(pid) => match definitions.iter().find(|d| d.id == *pid) {
|
||||
Some(def) => EffectiveConfigResult::Resolved(resolve_linked(def, global)),
|
||||
None => EffectiveConfigResult::OrphanedInstance {
|
||||
record_pubkey: record.pubkey.clone(),
|
||||
missing_persona_id: pid.clone(),
|
||||
},
|
||||
},
|
||||
None => EffectiveConfigResult::Resolved(resolve_definition_less(record, global)),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn resolve_effective_model_provider_pair(
|
||||
record: &ManagedAgentRecord,
|
||||
definitions: &[AgentDefinition],
|
||||
global: &GlobalAgentConfig,
|
||||
) -> Option<(Option<String>, Option<String>)> {
|
||||
match resolve_effective_config(record, definitions, global) {
|
||||
EffectiveConfigResult::Resolved(cfg) => Some((cfg.model.value, cfg.provider.value)),
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// The relay-mesh preflight decision for `record`, resolved the same way
|
||||
/// spawn resolves its mesh env: through `resolve_effective_config` (which
|
||||
/// folds in the definition → global fallback). A linked instance's own
|
||||
/// `provider`/`model`/`relay_mesh` bytes never contribute; a definition-less
|
||||
/// legacy record may fall back to them via `legacy_record_mesh_model_id`,
|
||||
/// which is confined to `resolve_definition_less`.
|
||||
///
|
||||
/// `None` covers both "not a mesh agent" and "orphaned instance" — an orphan
|
||||
/// never spawns (see `require_resolved`), so it never needs a mesh preflight
|
||||
/// either; the caller's own orphan handling downstream is unaffected, this
|
||||
/// just avoids tripping mesh bootstrap for a start that will be refused.
|
||||
pub fn resolve_effective_relay_mesh_model_id(
|
||||
record: &ManagedAgentRecord,
|
||||
definitions: &[AgentDefinition],
|
||||
global: &GlobalAgentConfig,
|
||||
) -> Option<String> {
|
||||
match resolve_effective_config(record, definitions, global) {
|
||||
EffectiveConfigResult::Resolved(cfg) => cfg.relay_mesh_model_id(),
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// The single user-facing message for a linked instance whose definition no
|
||||
/// longer exists. Shared by every path that must refuse to act on an orphan:
|
||||
/// the spawn boundary (`spawn_agent_child`), the interactive start command,
|
||||
/// and provider deploy.
|
||||
pub const ORPHANED_INSTANCE_ERROR: &str =
|
||||
"This agent's configuration is missing — it may still be \
|
||||
syncing or was deleted on another device.";
|
||||
|
||||
impl EffectiveConfigResult {
|
||||
/// Unwrap into the resolved config, or the shared orphan-refusal error.
|
||||
pub fn require_resolved(self) -> Result<EffectiveAgentConfig, String> {
|
||||
match self {
|
||||
EffectiveConfigResult::Resolved(cfg) => Ok(cfg),
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => {
|
||||
Err(ORPHANED_INSTANCE_ERROR.to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
872
desktop/src-tauri/src/managed_agents/effective_config/tests.rs
Normal file
872
desktop/src-tauri/src/managed_agents/effective_config/tests.rs
Normal file
@@ -0,0 +1,872 @@
|
||||
use super::*;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
fn definition(
|
||||
id: &str,
|
||||
model: Option<&str>,
|
||||
provider: Option<&str>,
|
||||
prompt: &str,
|
||||
) -> AgentDefinition {
|
||||
AgentDefinition {
|
||||
id: id.to_string(),
|
||||
display_name: "Test Definition".to_string(),
|
||||
avatar_url: None,
|
||||
system_prompt: prompt.to_string(),
|
||||
runtime: None,
|
||||
model: model.map(str::to_string),
|
||||
provider: provider.map(str::to_string),
|
||||
name_pool: vec![],
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
env_vars: BTreeMap::new(),
|
||||
respond_to: None,
|
||||
respond_to_allowlist: vec![],
|
||||
parallelism: None,
|
||||
created_at: "".to_string(),
|
||||
updated_at: "".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn record(
|
||||
persona_id: Option<&str>,
|
||||
model: Option<&str>,
|
||||
provider: Option<&str>,
|
||||
prompt: Option<&str>,
|
||||
) -> ManagedAgentRecord {
|
||||
use crate::managed_agents::{BackendKind, RespondTo};
|
||||
ManagedAgentRecord {
|
||||
pubkey: "agent-pk".to_string(),
|
||||
name: "Agent".to_string(),
|
||||
persona_id: persona_id.map(str::to_string),
|
||||
private_key_nsec: "".to_string(),
|
||||
auth_tag: None,
|
||||
relay_url: "ws://localhost:3000".to_string(),
|
||||
avatar_url: None,
|
||||
acp_command: "buzz-acp".to_string(),
|
||||
agent_command: "goose".to_string(),
|
||||
agent_command_override: None,
|
||||
agent_args: vec![],
|
||||
mcp_command: "".to_string(),
|
||||
turn_timeout_seconds: 300,
|
||||
idle_timeout_seconds: None,
|
||||
max_turn_duration_seconds: None,
|
||||
parallelism: 1,
|
||||
system_prompt: prompt.map(str::to_string),
|
||||
model: model.map(str::to_string),
|
||||
provider: provider.map(str::to_string),
|
||||
persona_source_version: None,
|
||||
env_vars: BTreeMap::new(),
|
||||
start_on_app_launch: false,
|
||||
runtime_pid: None,
|
||||
backend: BackendKind::Local,
|
||||
backend_agent_id: None,
|
||||
provider_binary_path: None,
|
||||
team_id: None,
|
||||
persona_team_dir: None,
|
||||
persona_name_in_team: None,
|
||||
created_at: "".to_string(),
|
||||
updated_at: "".to_string(),
|
||||
last_started_at: None,
|
||||
last_stopped_at: None,
|
||||
last_exit_code: None,
|
||||
last_error: None,
|
||||
last_error_code: None,
|
||||
respond_to: RespondTo::OwnerOnly,
|
||||
respond_to_allowlist: vec![],
|
||||
display_name: None,
|
||||
slug: None,
|
||||
runtime: None,
|
||||
name_pool: vec![],
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
relay_mesh: None,
|
||||
auto_restart_on_config_change: false,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
definition_parallelism: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn global(model: Option<&str>, provider: Option<&str>) -> GlobalAgentConfig {
|
||||
GlobalAgentConfig {
|
||||
model: model.map(str::to_string),
|
||||
provider: provider.map(str::to_string),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
// ── Linked instance: definition → global, record ignored ──
|
||||
|
||||
#[test]
|
||||
fn linked_definition_model_wins_over_stale_record() {
|
||||
let rec = record(
|
||||
Some("d1"),
|
||||
Some("stale-model"),
|
||||
Some("stale-prov"),
|
||||
Some("stale prompt"),
|
||||
);
|
||||
let defs = vec![definition(
|
||||
"d1",
|
||||
Some("def-model"),
|
||||
Some("def-prov"),
|
||||
"def prompt",
|
||||
)];
|
||||
let g = global(Some("global-model"), Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("def-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Definition);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("def-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Definition);
|
||||
assert_eq!(cfg.system_prompt.value.as_deref(), Some("def prompt"));
|
||||
assert_eq!(cfg.system_prompt.source, ConfigSource::Definition);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_inherit_global_when_definition_blank() {
|
||||
let rec = record(
|
||||
Some("d1"),
|
||||
Some("stale-model"),
|
||||
Some("stale-prov"),
|
||||
Some("stale prompt"),
|
||||
);
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
let g = global(Some("global-model"), Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("global-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("global-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.system_prompt.value, None);
|
||||
assert_eq!(cfg.system_prompt.source, ConfigSource::Definition);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_stale_record_model_is_inert() {
|
||||
let rec = record(Some("d1"), Some("stale-model"), Some("stale-prov"), None);
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
let g = global(None, None);
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value, None);
|
||||
assert_eq!(cfg.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.provider.value, None);
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_definition_model_set_provider_inherits() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("def-model"), None, "prompt")];
|
||||
let g = global(None, Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("def-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Definition);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("global-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_blank_prompt_means_no_prompt() {
|
||||
let rec = record(Some("d1"), None, None, Some("stale prompt on record"));
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
let g = global(None, None);
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.system_prompt.value, None);
|
||||
assert_eq!(cfg.system_prompt.source, ConfigSource::Definition);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_whitespace_only_definition_model_inherits_global() {
|
||||
let rec = record(Some("d1"), Some("stale"), None, None);
|
||||
let defs = vec![definition("d1", Some(" "), Some(" \t"), "")];
|
||||
let g = global(Some("global-model"), Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("global-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("global-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
}
|
||||
|
||||
// ── Orphaned instance ──
|
||||
|
||||
#[test]
|
||||
fn orphaned_linked_instance_returns_error() {
|
||||
let rec = record(Some("missing-def"), None, None, None);
|
||||
let defs = vec![];
|
||||
let g = global(Some("global-model"), None);
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
match result {
|
||||
EffectiveConfigResult::OrphanedInstance {
|
||||
record_pubkey,
|
||||
missing_persona_id,
|
||||
} => {
|
||||
assert_eq!(record_pubkey, "agent-pk");
|
||||
assert_eq!(missing_persona_id, "missing-def");
|
||||
}
|
||||
other => panic!("expected OrphanedInstance, got {:?}", other),
|
||||
}
|
||||
}
|
||||
|
||||
// ── Definition-less instance: instance → global ──
|
||||
|
||||
#[test]
|
||||
fn definition_less_uses_own_fields() {
|
||||
let rec = record(None, Some("my-model"), Some("my-prov"), Some("my prompt"));
|
||||
let defs = vec![];
|
||||
let g = global(Some("global-model"), Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("my-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::InstanceLegacy);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("my-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::InstanceLegacy);
|
||||
assert_eq!(cfg.system_prompt.value.as_deref(), Some("my prompt"));
|
||||
assert_eq!(cfg.system_prompt.source, ConfigSource::InstanceLegacy);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn definition_less_falls_back_to_global() {
|
||||
let rec = record(None, None, None, None);
|
||||
let defs = vec![];
|
||||
let g = global(Some("global-model"), Some("global-prov"));
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("global-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("global-prov"));
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn definition_less_blank_record_fields_fall_through() {
|
||||
let rec = record(None, Some(" "), Some(""), Some(" "));
|
||||
let defs = vec![];
|
||||
let g = global(Some("g-model"), None);
|
||||
|
||||
let result = resolve_effective_config(&rec, &defs, &g);
|
||||
let cfg = match result {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("expected Resolved, got {:?}", other),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("g-model"));
|
||||
assert_eq!(cfg.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.provider.value, None);
|
||||
assert_eq!(cfg.provider.source, ConfigSource::Global);
|
||||
assert_eq!(cfg.system_prompt.value, None);
|
||||
}
|
||||
|
||||
// ── Convenience helper ──
|
||||
|
||||
#[test]
|
||||
fn model_provider_pair_returns_none_for_orphan() {
|
||||
let rec = record(Some("missing"), None, None, None);
|
||||
assert_eq!(
|
||||
resolve_effective_model_provider_pair(&rec, &[], &global(None, None)),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn model_provider_pair_returns_resolved_values() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("m"), Some("p"), "")];
|
||||
let g = global(None, None);
|
||||
|
||||
let pair = resolve_effective_model_provider_pair(&rec, &defs, &g);
|
||||
assert_eq!(pair, Some((Some("m".to_string()), Some("p".to_string()))));
|
||||
}
|
||||
|
||||
// ── require_resolved: the shared orphan-refusal contract used by
|
||||
// build_deploy_payload and spawn_agent_child ──
|
||||
|
||||
#[test]
|
||||
fn require_resolved_returns_shared_error_for_orphan() {
|
||||
let rec = record(Some("missing"), None, None, None);
|
||||
let error = resolve_effective_config(&rec, &[], &global(None, None))
|
||||
.require_resolved()
|
||||
.expect_err("orphan must not resolve");
|
||||
assert_eq!(error, ORPHANED_INSTANCE_ERROR);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn require_resolved_returns_config_for_resolved() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("m"), Some("p"), "prompt")];
|
||||
let cfg = resolve_effective_config(&rec, &defs, &global(None, None))
|
||||
.require_resolved()
|
||||
.expect("linked instance with a live definition must resolve");
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("m"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn require_resolved_refuses_orphan_only() {
|
||||
let orphan = record(Some("missing"), None, None, None);
|
||||
assert_eq!(
|
||||
resolve_effective_config(&orphan, &[], &global(None, None))
|
||||
.require_resolved()
|
||||
.unwrap_err(),
|
||||
ORPHANED_INSTANCE_ERROR,
|
||||
);
|
||||
|
||||
let linked = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("m"), None, "")];
|
||||
assert!(
|
||||
resolve_effective_config(&linked, &defs, &global(None, None))
|
||||
.require_resolved()
|
||||
.is_ok()
|
||||
);
|
||||
|
||||
// Definition-less instances are never orphaned regardless of how bare
|
||||
// their own fields are — orphan status only applies to a dangling link.
|
||||
let bare = record(None, None, None, None);
|
||||
assert!(resolve_effective_config(&bare, &[], &global(None, None))
|
||||
.require_resolved()
|
||||
.is_ok());
|
||||
}
|
||||
|
||||
// ── Morgan's exact regression sequence ──
|
||||
|
||||
#[test]
|
||||
fn morgans_sequence_inherit_explicit_inherit() {
|
||||
let g = global(Some("claude-opus-4-6"), Some("anthropic"));
|
||||
|
||||
// Step 1: fresh agent with inherited model → resolves global
|
||||
let rec_step1 = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", None, None, "agent prompt")];
|
||||
let cfg1 = match resolve_effective_config(&rec_step1, &defs, &g) {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("step 1: {:?}", other),
|
||||
};
|
||||
assert_eq!(cfg1.model.value.as_deref(), Some("claude-opus-4-6"));
|
||||
assert_eq!(cfg1.model.source, ConfigSource::Global);
|
||||
|
||||
// Step 2: set explicit model on definition
|
||||
let defs_explicit = vec![definition(
|
||||
"d1",
|
||||
Some("goose-gpt-5-6-sol"),
|
||||
Some("databricks"),
|
||||
"agent prompt",
|
||||
)];
|
||||
let cfg2 = match resolve_effective_config(&rec_step1, &defs_explicit, &g) {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("step 2: {:?}", other),
|
||||
};
|
||||
assert_eq!(cfg2.model.value.as_deref(), Some("goose-gpt-5-6-sol"));
|
||||
assert_eq!(cfg2.model.source, ConfigSource::Definition);
|
||||
|
||||
// Step 3: switch back to inherit — even with stale record bytes
|
||||
let rec_stale = record(
|
||||
Some("d1"),
|
||||
Some("goose-gpt-5-6-sol"),
|
||||
Some("databricks"),
|
||||
None,
|
||||
);
|
||||
let defs_inherit = vec![definition("d1", None, None, "agent prompt")];
|
||||
let cfg3 = match resolve_effective_config(&rec_stale, &defs_inherit, &g) {
|
||||
EffectiveConfigResult::Resolved(c) => c,
|
||||
other => panic!("step 3: {:?}", other),
|
||||
};
|
||||
assert_eq!(cfg3.model.value.as_deref(), Some("claude-opus-4-6"));
|
||||
assert_eq!(cfg3.model.source, ConfigSource::Global);
|
||||
assert_eq!(cfg3.provider.value.as_deref(), Some("anthropic"));
|
||||
assert_eq!(cfg3.provider.source, ConfigSource::Global);
|
||||
}
|
||||
|
||||
// ── relay-mesh preflight resolution (Wes review 2 on #1968) ──
|
||||
//
|
||||
// Both regressions below reproduce the exact defects: the preflight must
|
||||
// key off `resolve_effective_config`'s resolution — the same one spawn's
|
||||
// mesh env consults — not the record's own `provider`/`model` bytes.
|
||||
|
||||
#[test]
|
||||
fn relay_mesh_model_id_none_for_non_mesh_config() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("m"), Some("anthropic"), "")];
|
||||
let g = global(None, None);
|
||||
|
||||
assert_eq!(resolve_effective_relay_mesh_model_id(&rec, &defs, &g), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relay_mesh_model_id_defaults_to_auto_when_model_blank() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", None, Some(RELAY_MESH_PROVIDER_ID), "")];
|
||||
let g = global(None, None);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &defs, &g).as_deref(),
|
||||
Some(RELAY_MESH_AUTO_MODEL_ID)
|
||||
);
|
||||
}
|
||||
|
||||
/// Switch-away regression (Wes finding 1): a linked definition that used to
|
||||
/// be relay-mesh but was edited to another provider must NOT trigger the mesh
|
||||
/// preflight — even though the record's own stale bytes still say
|
||||
/// `provider: relay-mesh`. The old `relay_mesh_config(record)` sniff read
|
||||
/// those stale record bytes directly and returned Some; the resolver-driven
|
||||
/// decision must read the definition's CURRENT provider and return None.
|
||||
#[test]
|
||||
fn switch_away_from_relay_mesh_clears_preflight_despite_stale_record_bytes() {
|
||||
let rec = record(Some("d1"), Some("auto"), Some(RELAY_MESH_PROVIDER_ID), None);
|
||||
let defs = vec![definition(
|
||||
"d1",
|
||||
Some("claude-opus-4-6"),
|
||||
Some("anthropic"),
|
||||
"",
|
||||
)];
|
||||
let g = global(None, None);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &defs, &g),
|
||||
None,
|
||||
"definition switched away from relay-mesh — no mesh preflight should fire"
|
||||
);
|
||||
}
|
||||
|
||||
/// Global-inheritance regression (Wes finding 2): a blank linked definition
|
||||
/// falling through to a relay-mesh GLOBAL default must trigger the mesh
|
||||
/// preflight, even though the record carries `provider: None` and no legacy
|
||||
/// env — all three of the old `relay_mesh_config` branches would miss here.
|
||||
#[test]
|
||||
fn global_relay_mesh_default_triggers_preflight_for_blank_definition() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
let g = global(Some("Qwen3"), Some(RELAY_MESH_PROVIDER_ID));
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &defs, &g).as_deref(),
|
||||
Some("Qwen3"),
|
||||
"global relay-mesh default must trigger the mesh preflight for a blank definition"
|
||||
);
|
||||
}
|
||||
|
||||
/// Symmetric case: a definition-less (legacy) instance inheriting the global
|
||||
/// relay-mesh default must also trigger the preflight.
|
||||
#[test]
|
||||
fn global_relay_mesh_default_triggers_preflight_for_definition_less_instance() {
|
||||
let rec = record(None, None, None, None);
|
||||
let defs = vec![];
|
||||
let g = global(None, Some(RELAY_MESH_PROVIDER_ID));
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &defs, &g).as_deref(),
|
||||
Some(RELAY_MESH_AUTO_MODEL_ID)
|
||||
);
|
||||
}
|
||||
|
||||
/// Orphaned instance: no mesh preflight regardless of stale record bytes —
|
||||
/// the caller's own orphan refusal is what matters, not a mesh bootstrap for
|
||||
/// a start that will never happen.
|
||||
#[test]
|
||||
fn orphaned_instance_never_triggers_mesh_preflight() {
|
||||
let rec = record(
|
||||
Some("missing-def"),
|
||||
Some("auto"),
|
||||
Some(RELAY_MESH_PROVIDER_ID),
|
||||
None,
|
||||
);
|
||||
let g = global(None, None);
|
||||
|
||||
assert_eq!(resolve_effective_relay_mesh_model_id(&rec, &[], &g), None);
|
||||
}
|
||||
|
||||
// ── Whitespace-provider parity (trim-mismatch regression, T1) ──
|
||||
//
|
||||
// The old spawn gate was `effective_provider.as_deref() == Some(RELAY_MESH_PROVIDER_ID)`
|
||||
// — an exact compare — while `relay_mesh_model_id()` trims before matching.
|
||||
// A stored provider of " relay-mesh " would make preflight fire while spawn
|
||||
// skipped the mesh-env block. After the fix spawn derives its gate from
|
||||
// `relay_mesh_model_id()` too, so the trim semantics are identical.
|
||||
//
|
||||
// These tests verify `relay_mesh_model_id()` returns Some for padded providers
|
||||
// on every resolution path, confirming the helper that both callers now use
|
||||
// handles whitespace correctly.
|
||||
|
||||
/// Definition provider with leading/trailing whitespace — resolver preserves
|
||||
/// the raw string, but `relay_mesh_model_id()` trims before matching, so the
|
||||
/// mesh preflight fires correctly.
|
||||
#[test]
|
||||
fn whitespace_provider_in_definition_triggers_mesh_decision() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", Some("qwen3"), Some(" relay-mesh "), "")];
|
||||
let g = global(None, None);
|
||||
|
||||
let mesh_id = resolve_effective_relay_mesh_model_id(&rec, &defs, &g);
|
||||
assert_eq!(
|
||||
mesh_id.as_deref(),
|
||||
Some("qwen3"),
|
||||
"padded definition provider must be treated as relay-mesh by the shared helper"
|
||||
);
|
||||
}
|
||||
|
||||
/// Global provider with leading/trailing whitespace — same assertion for the
|
||||
/// global-inherit path (blank definition falls through to padded global).
|
||||
#[test]
|
||||
fn whitespace_provider_in_global_triggers_mesh_decision() {
|
||||
let rec = record(Some("d1"), None, None, None);
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
let g = global(Some("qwen3"), Some("\trelay-mesh\t"));
|
||||
|
||||
let mesh_id = resolve_effective_relay_mesh_model_id(&rec, &defs, &g);
|
||||
assert_eq!(
|
||||
mesh_id.as_deref(),
|
||||
Some("qwen3"),
|
||||
"padded global provider must be treated as relay-mesh by the shared helper"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Legacy relay-mesh record compatibility (Wes review 3 on #1968) ──
|
||||
//
|
||||
// Two record generations shipped before `provider: "relay-mesh"` existed and
|
||||
// are never rewritten on load, so they are still on disk:
|
||||
//
|
||||
// Class A (#798) — the four mesh preset env vars in `env_vars`, no typed
|
||||
// field, no `provider` (the record had no `provider` field yet).
|
||||
// Class B (#879) — the typed `relay_mesh` marker, still no `provider` field.
|
||||
//
|
||||
// Without the fallback both classes resolve to a non-mesh provider while their
|
||||
// own stale env bytes still reach the child: a silent misroute, not a clean
|
||||
// failure. The fallback lives in `resolve_definition_less` only — a linked
|
||||
// instance's definition stays authoritative.
|
||||
|
||||
/// Class A: env-only legacy record resolves mesh via the four-env preset.
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_env_sniff() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1/".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)).as_deref(),
|
||||
Some("Qwen3"),
|
||||
"env-only legacy mesh record must still resolve to its served model"
|
||||
);
|
||||
}
|
||||
|
||||
/// Class A, oldest shipped record: both renamed sentinels in their original
|
||||
/// spelling. The preset first wrote `SPROUT_AGENT_PROVIDER` (#798) and the
|
||||
/// api-key value `sprout-mesh-local`; #971 and #960 renamed each in the same
|
||||
/// Jun-11 window without migrating persisted `env_vars`, so this exact shape is
|
||||
/// still on disk and must resolve mesh.
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_env_sniff_with_pre_rename_sentinels() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("SPROUT_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
"sprout-mesh-local".to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)).as_deref(),
|
||||
Some("Qwen3"),
|
||||
"the oldest shipped mesh record carries both pre-rename sentinels and was never migrated"
|
||||
);
|
||||
}
|
||||
|
||||
/// Class A straddling the rename window: old provider key with the new api-key
|
||||
/// value. Proves the two either/ors are independent rather than one either/or on
|
||||
/// a bundled old/new pair.
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_env_sniff_with_mixed_rename_sentinels() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("SPROUT_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)).as_deref(),
|
||||
Some("Qwen3"),
|
||||
"old provider key with new api key must resolve — the sentinels renamed independently"
|
||||
);
|
||||
}
|
||||
|
||||
/// The mirrored straddle: new provider key with the old api-key value.
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_env_sniff_with_new_provider_and_old_api_key() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
"sprout-mesh-local".to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)).as_deref(),
|
||||
Some("Qwen3"),
|
||||
"new provider key with old api key must resolve too"
|
||||
);
|
||||
}
|
||||
|
||||
/// Both spellings present and disagreeing: the renamed key is authoritative,
|
||||
/// so a record whose current provider env is non-mesh is not resurrected as
|
||||
/// mesh by the stale `SPROUT_` leftover.
|
||||
#[test]
|
||||
fn legacy_env_sniff_prefers_renamed_provider_key_over_stale_old_key() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()),
|
||||
("SPROUT_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)),
|
||||
None,
|
||||
"the renamed key states current intent — a stale SPROUT_ key must not override it"
|
||||
);
|
||||
}
|
||||
|
||||
/// Class A negative: a user's own OpenAI-compatible provider on the same local
|
||||
/// port is not Buzz's preset — the placeholder API key is the discriminator.
|
||||
#[test]
|
||||
fn legacy_env_sniff_ignores_user_openai_on_same_local_port() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
("OPENAI_COMPAT_API_KEY".to_string(), "real-key".to_string()),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)),
|
||||
None,
|
||||
"a real user key on the mesh port must not be mistaken for Buzz's preset"
|
||||
);
|
||||
}
|
||||
|
||||
/// Class B: typed marker with no `provider` resolves mesh, and its `model_ref`
|
||||
/// wins over an unrelated global model so preflight and spawn agree.
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_typed_marker_without_provider() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.relay_mesh = Some(crate::managed_agents::RelayMeshConfig {
|
||||
model_ref: "Qwen3".to_string(),
|
||||
});
|
||||
|
||||
let cfg = match resolve_effective_config(&rec, &[], &global(Some("gpt-5"), Some("openai"))) {
|
||||
EffectiveConfigResult::Resolved(cfg) => cfg,
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => {
|
||||
panic!("definition-less is never orphaned")
|
||||
}
|
||||
};
|
||||
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some(RELAY_MESH_PROVIDER_ID));
|
||||
assert_eq!(
|
||||
cfg.relay_mesh_model_id().as_deref(),
|
||||
Some("Qwen3"),
|
||||
"typed marker must resolve mesh even with no provider on the record"
|
||||
);
|
||||
}
|
||||
|
||||
/// Class B with a blank `model_ref`: the marker is still the mesh signal, and
|
||||
/// the model resolves to auto — same rule `apply_relay_mesh_env` applies.
|
||||
#[test]
|
||||
fn legacy_typed_marker_with_blank_model_resolves_auto() {
|
||||
let mut rec = record(None, None, None, None);
|
||||
rec.relay_mesh = Some(crate::managed_agents::RelayMeshConfig {
|
||||
model_ref: " ".to_string(),
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
resolve_effective_relay_mesh_model_id(&rec, &[], &global(None, None)).as_deref(),
|
||||
Some(RELAY_MESH_AUTO_MODEL_ID)
|
||||
);
|
||||
}
|
||||
|
||||
/// A definition-less record that was switched AWAY from mesh keeps its stale
|
||||
/// marker and env bytes on disk. Its explicit `provider` states current intent,
|
||||
/// so the legacy fallback must not resurrect mesh.
|
||||
#[test]
|
||||
fn definition_less_explicit_provider_wins_over_stale_legacy_mesh_bytes() {
|
||||
let mut rec = record(None, Some("claude-opus-4-6"), Some("anthropic"), None);
|
||||
rec.relay_mesh = Some(crate::managed_agents::RelayMeshConfig {
|
||||
model_ref: "Qwen3".to_string(),
|
||||
});
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
let cfg = match resolve_effective_config(&rec, &[], &global(None, None)) {
|
||||
EffectiveConfigResult::Resolved(cfg) => cfg,
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => {
|
||||
panic!("definition-less is never orphaned")
|
||||
}
|
||||
};
|
||||
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("anthropic"));
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("claude-opus-4-6"));
|
||||
assert_eq!(
|
||||
cfg.relay_mesh_model_id(),
|
||||
None,
|
||||
"a switched-away legacy record must not be dragged back onto mesh"
|
||||
);
|
||||
}
|
||||
|
||||
/// The test that proves the fallback did not reintroduce the bug this PR
|
||||
/// deletes: a LINKED record carrying both legacy mesh signals resolves purely
|
||||
/// from its definition and never trips mesh.
|
||||
#[test]
|
||||
fn linked_record_ignores_legacy_mesh_marker_and_env() {
|
||||
let mut rec = record(Some("d1"), Some("auto"), None, None);
|
||||
rec.relay_mesh = Some(crate::managed_agents::RelayMeshConfig {
|
||||
model_ref: "Qwen3".to_string(),
|
||||
});
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
let defs = vec![definition(
|
||||
"d1",
|
||||
Some("claude-opus-4-6"),
|
||||
Some("anthropic"),
|
||||
"",
|
||||
)];
|
||||
|
||||
let cfg = match resolve_effective_config(&rec, &defs, &global(None, None)) {
|
||||
EffectiveConfigResult::Resolved(cfg) => cfg,
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => panic!("definition exists"),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("anthropic"));
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("claude-opus-4-6"));
|
||||
assert_eq!(
|
||||
cfg.relay_mesh_model_id(),
|
||||
None,
|
||||
"legacy record bytes must never influence a linked instance"
|
||||
);
|
||||
}
|
||||
|
||||
/// A linked instance whose definition inherits a blank provider from a blank
|
||||
/// global must NOT pick up the record's legacy mesh bytes either — the linked
|
||||
/// path has no legacy fallback at all, by construction.
|
||||
#[test]
|
||||
fn linked_record_with_legacy_bytes_inherits_global_not_mesh() {
|
||||
let mut rec = record(Some("d1"), None, None, None);
|
||||
rec.relay_mesh = Some(crate::managed_agents::RelayMeshConfig {
|
||||
model_ref: "Qwen3".to_string(),
|
||||
});
|
||||
let defs = vec![definition("d1", None, None, "")];
|
||||
|
||||
let cfg = match resolve_effective_config(&rec, &defs, &global(Some("gpt-5"), Some("openai"))) {
|
||||
EffectiveConfigResult::Resolved(cfg) => cfg,
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => panic!("definition exists"),
|
||||
};
|
||||
|
||||
assert_eq!(cfg.provider.value.as_deref(), Some("openai"));
|
||||
assert_eq!(cfg.model.value.as_deref(), Some("gpt-5"));
|
||||
assert_eq!(cfg.relay_mesh_model_id(), None);
|
||||
}
|
||||
@@ -291,8 +291,12 @@ pub(crate) fn merged_user_env(
|
||||
|
||||
/// Look up the live env map of `persona_id` within an already-loaded persona
|
||||
/// slice. Returns an empty map for standalone agents (`None`) and for links
|
||||
/// to personas that no longer exist (an orphaned agent spawns from its own
|
||||
/// overrides alone — same fallback the prompt/model resolution uses).
|
||||
/// to personas that no longer exist. The latter is an orphaned instance,
|
||||
/// which `spawn_agent_child` refuses before this is ever read at spawn time
|
||||
/// (see `effective_config::resolve_effective_config`'s `OrphanedInstance`
|
||||
/// arm via `require_resolved`) — an empty map here only matters for
|
||||
/// non-spawn callers like readiness/hash that must still resolve
|
||||
/// a slice for a record whose orphan status hasn't been checked yet.
|
||||
pub(crate) fn live_persona_env(
|
||||
personas: &[super::types::AgentDefinition],
|
||||
persona_id: Option<&str>,
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
//! Global agent configuration defaults.
|
||||
//!
|
||||
//! A single `global-agent-config.json` record that applies to ALL managed
|
||||
//! agents. Per-agent config always wins; global provides the lowest
|
||||
//! user-settable layer below persona.
|
||||
//! agents. For a linked instance, the definition wins over global; for a
|
||||
//! definition-less (legacy) instance, the record's own fields win over
|
||||
//! global. Global is always the lowest user-settable layer.
|
||||
//!
|
||||
//! # Precedence (low → high)
|
||||
//!
|
||||
//! ```text
|
||||
//! baked build env < GLOBAL < persona < per-agent < Buzz-identity
|
||||
//! baked build env < GLOBAL < definition (linked) / instance (legacy) < Buzz-identity
|
||||
//! ```
|
||||
//!
|
||||
//! # Semantics
|
||||
@@ -38,8 +39,10 @@ use crate::managed_agents::types::{AgentDefinition, ManagedAgentRecord};
|
||||
/// so the config vocabulary is consistent across all three tiers.
|
||||
///
|
||||
/// `env_vars` is the lowest user-settable env layer — global < persona < agent.
|
||||
/// `provider` / `model` are fallback defaults: effective provider/model =
|
||||
/// `agent → persona → global → None`.
|
||||
/// `provider` / `model` are fallback defaults, resolved via
|
||||
/// `effective_config::resolve_effective_config`: for a linked instance,
|
||||
/// definition → global (the record's own `provider`/`model` bytes are never
|
||||
/// consulted); for a definition-less instance, instance → global.
|
||||
#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct GlobalAgentConfig {
|
||||
/// Global env vars injected into ALL agents unconditionally.
|
||||
@@ -204,44 +207,28 @@ pub fn save_global_agent_config(app: &AppHandle, config: &GlobalAgentConfig) ->
|
||||
atomic_write_json_restricted(&path, &payload)
|
||||
}
|
||||
|
||||
/// Resolve the effective model and provider for an agent using the
|
||||
/// precedence chain: `agent record → linked persona → global defaults → None`.
|
||||
/// Resolve the effective model and provider for an agent.
|
||||
///
|
||||
/// This is the single source of truth used by readiness evaluation, spawn,
|
||||
/// and deploy-payload construction. All three paths must use this function so
|
||||
/// they agree on what model/provider the agent will actually run with.
|
||||
/// Delegates to `effective_config::resolve_effective_config` which enforces
|
||||
/// definition-authoritative semantics for linked instances:
|
||||
/// - **Linked:** definition → global. Record bytes are never consulted.
|
||||
/// - **Definition-less:** instance → global.
|
||||
/// - **Orphaned:** returns `(None, None)`. This function is a display/
|
||||
/// readiness/hash convenience, not the spawn gate — an orphan must never
|
||||
/// reach a real spawn regardless of what this returns, because
|
||||
/// `spawn_agent_child` refuses orphans itself via
|
||||
/// `effective_config::resolve_effective_config(...).require_resolved()`
|
||||
/// before resolving model/provider/prompt for the process env.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `record` — the `ManagedAgentRecord` (may have `None` for model/provider)
|
||||
/// * `personas` — all current persona records (looked up by `record.persona_id`)
|
||||
/// * `global` — global agent config defaults
|
||||
///
|
||||
/// # Returns
|
||||
/// `(effective_model, effective_provider)` — both `Option<&str>`.
|
||||
pub(crate) fn resolve_effective_model_provider<'a>(
|
||||
record: &'a ManagedAgentRecord,
|
||||
personas: &'a [AgentDefinition],
|
||||
global: &'a GlobalAgentConfig,
|
||||
) -> (Option<&'a str>, Option<&'a str>) {
|
||||
let (persona_model, persona_provider) = record
|
||||
.persona_id
|
||||
.as_deref()
|
||||
.and_then(|pid| personas.iter().find(|p| p.id == pid))
|
||||
.map(|p| (p.model.as_deref(), p.provider.as_deref()))
|
||||
.unwrap_or((None, None));
|
||||
|
||||
let effective_model = record
|
||||
.model
|
||||
.as_deref()
|
||||
.or(persona_model)
|
||||
.or(global.model.as_deref());
|
||||
let effective_provider = record
|
||||
.provider
|
||||
.as_deref()
|
||||
.or(persona_provider)
|
||||
.or(global.provider.as_deref());
|
||||
|
||||
(effective_model, effective_provider)
|
||||
/// Returns owned `String`s because the effective value may come from a
|
||||
/// tier that outlives none of the input borrows (e.g. global config).
|
||||
pub(crate) fn resolve_effective_model_provider(
|
||||
record: &ManagedAgentRecord,
|
||||
personas: &[AgentDefinition],
|
||||
global: &GlobalAgentConfig,
|
||||
) -> (Option<String>, Option<String>) {
|
||||
super::effective_config::resolve_effective_model_provider_pair(record, personas, global)
|
||||
.unwrap_or((None, None))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -376,15 +376,14 @@ fn persona(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefini
|
||||
}
|
||||
}
|
||||
|
||||
/// Tier 1 — agent record wins: record has explicit model/provider; they must
|
||||
/// outrank both the linked persona and the global defaults. Fails against any
|
||||
/// implementation that prefers global or persona over the record.
|
||||
/// Linked instance: definition (persona) wins — stale record bytes are
|
||||
/// ignored. This is the core model-inheritance fix.
|
||||
#[test]
|
||||
fn resolve_agent_record_wins_over_persona_and_global() {
|
||||
fn resolve_definition_wins_over_stale_record_for_linked_instance() {
|
||||
let mut record = bare_record();
|
||||
record.persona_id = Some("p1".to_string());
|
||||
record.model = Some("record-model".to_string());
|
||||
record.provider = Some("record-provider".to_string());
|
||||
record.model = Some("stale-record-model".to_string());
|
||||
record.provider = Some("stale-record-provider".to_string());
|
||||
let personas = vec![persona(
|
||||
"p1",
|
||||
Some("persona-model"),
|
||||
@@ -398,11 +397,15 @@ fn resolve_agent_record_wins_over_persona_and_global() {
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"), "record model must win");
|
||||
assert_eq!(
|
||||
provider,
|
||||
Some("record-provider"),
|
||||
"record provider must win"
|
||||
model.as_deref(),
|
||||
Some("persona-model"),
|
||||
"definition model must win over stale record"
|
||||
);
|
||||
assert_eq!(
|
||||
provider.as_deref(),
|
||||
Some("persona-provider"),
|
||||
"definition provider must win over stale record"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -428,12 +431,12 @@ fn resolve_persona_fallback_when_record_has_none() {
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(
|
||||
model,
|
||||
model.as_deref(),
|
||||
Some("persona-model"),
|
||||
"persona model must be used when record has none"
|
||||
);
|
||||
assert_eq!(
|
||||
provider,
|
||||
provider.as_deref(),
|
||||
Some("persona-provider"),
|
||||
"persona provider must be used when record has none"
|
||||
);
|
||||
@@ -458,12 +461,12 @@ fn resolve_global_fallback_when_record_and_persona_have_none() {
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(
|
||||
model,
|
||||
model.as_deref(),
|
||||
Some("global-model"),
|
||||
"global model must be used when record and persona have none"
|
||||
);
|
||||
assert_eq!(
|
||||
provider,
|
||||
provider.as_deref(),
|
||||
Some("global-provider"),
|
||||
"global provider must be used when record and persona have none"
|
||||
);
|
||||
@@ -519,8 +522,8 @@ fn resolve_global_fallback_when_no_persona_linked() {
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("global-model"));
|
||||
assert_eq!(provider, Some("global-provider"));
|
||||
assert_eq!(model.as_deref(), Some("global-model"));
|
||||
assert_eq!(provider.as_deref(), Some("global-provider"));
|
||||
}
|
||||
|
||||
/// All-None: no source provides model/provider → both must be None.
|
||||
@@ -543,18 +546,15 @@ fn resolve_all_none_when_no_source_provides_values() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Partial tier — record has model but not provider; persona has provider but
|
||||
/// not model; global has both. Each field resolves independently through the
|
||||
/// three-tier chain.
|
||||
/// Each field resolves independently: definition model=None → global fills
|
||||
/// model; definition has provider → definition wins for provider. Stale
|
||||
/// record bytes are ignored for linked instances.
|
||||
#[test]
|
||||
fn resolve_each_field_resolves_independently_through_tiers() {
|
||||
let mut record = bare_record();
|
||||
record.persona_id = Some("p1".to_string());
|
||||
record.model = Some("record-model".to_string());
|
||||
// record.provider = None → falls through to persona
|
||||
record.model = Some("stale-record-model".to_string());
|
||||
let personas = vec![persona("p1", None, Some("persona-provider"))];
|
||||
// persona.model = None → global fills model if record also had none, but
|
||||
// record has model here so global is not needed for model.
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-provider".to_string()),
|
||||
@@ -563,11 +563,15 @@ fn resolve_each_field_resolves_independently_through_tiers() {
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"), "record wins for model");
|
||||
assert_eq!(
|
||||
provider,
|
||||
model.as_deref(),
|
||||
Some("global-model"),
|
||||
"definition model=None → global fills model; stale record ignored"
|
||||
);
|
||||
assert_eq!(
|
||||
provider.as_deref(),
|
||||
Some("persona-provider"),
|
||||
"persona wins for provider when record has none"
|
||||
"definition provider wins"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ mod backend;
|
||||
pub(crate) mod config_bridge;
|
||||
pub(crate) mod custom_harnesses;
|
||||
mod discovery;
|
||||
pub(crate) mod effective_config;
|
||||
mod env_vars;
|
||||
pub(crate) mod git_bash;
|
||||
pub(crate) mod global_config;
|
||||
|
||||
@@ -359,10 +359,14 @@ pub fn persona_event_content(record: &AgentDefinition) -> PersonaEventContent {
|
||||
}
|
||||
|
||||
/// A persona's spawn-relevant config, pinned onto a `ManagedAgentRecord` at
|
||||
/// create time. After the snapshot, spawn and deploy read these fields off the
|
||||
/// record and never the live persona, so an agent stays pinned to the config
|
||||
/// it was created with — restart reuses the snapshot, delete+respawn rewrites
|
||||
/// it.
|
||||
/// create time for display/backward-compat purposes. For a linked instance,
|
||||
/// spawn and deploy do NOT read these snapshotted fields — they resolve
|
||||
/// model/provider/prompt live from the definition on every spawn (see
|
||||
/// `effective_config::resolve_effective_config`), so a definition edit
|
||||
/// propagates on the next restart without delete+respawn. The snapshot still
|
||||
/// matters for `runtime` (materialized per B5, no live-read path yet) and for
|
||||
/// `persona_source_version`, the drift basis the Agents menu compares against
|
||||
/// the definition's current content hash.
|
||||
pub struct PersonaSnapshot {
|
||||
pub system_prompt: Option<String>,
|
||||
pub model: Option<String>,
|
||||
@@ -377,24 +381,6 @@ pub struct PersonaSnapshot {
|
||||
pub source_version: String,
|
||||
}
|
||||
|
||||
/// Apply persona-wins-when-set precedence for a single optional string field.
|
||||
///
|
||||
/// Returns the persona's value when it is non-`None` and non-whitespace-only;
|
||||
/// otherwise falls back to the record's value with the same blank filter applied.
|
||||
/// Returns `None` only when both are blank — a genuinely unconfigured field stays
|
||||
/// unconfigured.
|
||||
///
|
||||
/// This is the single source of truth for the precedence rule used by
|
||||
/// `persona_snapshot_with_agent_config_fallback`, `build_deploy_payload`, and
|
||||
/// `resolve_effective_prompt_model_provider` so the three paths cannot drift.
|
||||
pub fn persona_field_with_record_fallback(
|
||||
persona_value: Option<&str>,
|
||||
record_value: Option<&str>,
|
||||
) -> Option<String> {
|
||||
let non_blank = |v: Option<&str>| v.filter(|s| !s.trim().is_empty()).map(str::to_owned);
|
||||
non_blank(persona_value).or_else(|| non_blank(record_value))
|
||||
}
|
||||
|
||||
/// Build the pinned snapshot for an agent created from `persona`.
|
||||
///
|
||||
/// The persona's `system_prompt` is always present, so it is wrapped in
|
||||
@@ -411,51 +397,14 @@ pub fn persona_snapshot(persona: &AgentDefinition) -> PersonaSnapshot {
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the pinned snapshot for an **existing** agent record being re-snapshotted
|
||||
/// from its linked persona (on spawn or app-launch restore).
|
||||
/// Re-pin `record` to `persona`: build a snapshot via [`persona_snapshot`]
|
||||
/// and mirror it onto the record — the definition quad
|
||||
/// (`system_prompt`/`model`/`provider`/`runtime`), the env-override
|
||||
/// self-heal, and the `persona_source_version` drift basis.
|
||||
///
|
||||
/// Precedence rule: when the persona sets `model` or `provider` (non-`None`, non-empty),
|
||||
/// the persona wins — this is the expected inheritance. When the persona leaves
|
||||
/// these fields blank (`None` or empty string), the agent record's own values are
|
||||
/// preserved instead. This prevents a persona with no configured model/provider from
|
||||
/// clobbering a value the user already set on the agent, which would trap the agent
|
||||
/// in a permanent "needs configuration" loop that users cannot escape.
|
||||
///
|
||||
/// `source_version` is always updated to the current persona content hash so the
|
||||
/// drift badge clears correctly even when model/provider are not touched.
|
||||
///
|
||||
/// Env vars are not part of the snapshot: `record.env_vars` (agent overrides)
|
||||
/// is left untouched and the live persona env is merged underneath at read time.
|
||||
///
|
||||
/// The two fields (`model`, `provider`) are independent: a persona that sets only
|
||||
/// `model` wins on `model` while the agent's `provider` is preserved, and vice versa.
|
||||
pub fn persona_snapshot_with_agent_config_fallback(
|
||||
persona: &AgentDefinition,
|
||||
current_agent_model: Option<&str>,
|
||||
current_agent_provider: Option<&str>,
|
||||
) -> PersonaSnapshot {
|
||||
// Delegate system_prompt and source_version to persona_snapshot so future
|
||||
// PersonaSnapshot field additions stay automatically consistent.
|
||||
let base = persona_snapshot(persona);
|
||||
|
||||
// Apply the shared precedence rule: persona wins when non-blank, else
|
||||
// the agent record's value is preserved so a configured agent stays configured.
|
||||
let model = persona_field_with_record_fallback(base.model.as_deref(), current_agent_model);
|
||||
let provider =
|
||||
persona_field_with_record_fallback(base.provider.as_deref(), current_agent_provider);
|
||||
|
||||
PersonaSnapshot {
|
||||
model,
|
||||
provider,
|
||||
..base
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-pin `record` to `persona`: build the snapshot (via
|
||||
/// [`persona_snapshot_with_agent_config_fallback`], so blank persona
|
||||
/// `model`/`provider` preserve the record's own values) and mirror it onto the
|
||||
/// record — the definition quad (`system_prompt`/`model`/`provider`/`runtime`),
|
||||
/// the env-override self-heal, and the `persona_source_version` drift basis.
|
||||
/// Definition-authoritative: blank definition model/provider produce `None`
|
||||
/// on the record. The effective-config resolver falls through to the global
|
||||
/// default at read time; stale materialized record bytes are never preserved.
|
||||
///
|
||||
/// This is the single apply used by every snapshot-apply site: the spawn
|
||||
/// re-pin (`start_local_agent_with_preflight`), the launch backfill and
|
||||
@@ -467,11 +416,7 @@ pub fn persona_snapshot_with_agent_config_fallback(
|
||||
/// caller's concern, and `spawn_config_hash` (which applies this to a clone)
|
||||
/// must stay pure.
|
||||
pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDefinition) {
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
record.model.as_deref(), // fallback: record.model
|
||||
record.provider.as_deref(), // fallback: record.provider
|
||||
);
|
||||
let snapshot = persona_snapshot(persona);
|
||||
if let Some(prompt) = snapshot.system_prompt {
|
||||
record.system_prompt = Some(prompt);
|
||||
}
|
||||
@@ -507,5 +452,33 @@ pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDe
|
||||
.retain(|k, v| persona.env_vars.get(k) != Some(v));
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
}
|
||||
|
||||
/// Preview what `record` would look like immediately after the start/restore
|
||||
/// paths re-pin it to its linked persona, without mutating `record` itself.
|
||||
///
|
||||
/// Every decision made ahead of the real re-pin — the relay-mesh preflight in
|
||||
/// `start_local_agent_with_preflight`, the restart-badge hash in
|
||||
/// `spawn_config_hash` — needs to reason about spawn-time state, not
|
||||
/// pre-snapshot bytes, so a persona edit that flips a field (e.g. `provider`
|
||||
/// to/from relay-mesh) between saves is reflected in the decision instead of
|
||||
/// the stale value the real [`apply_persona_snapshot`] is about to overwrite
|
||||
/// anyway. Idempotent: applying it to an already-current record is a no-op,
|
||||
/// so the spawn-time stamp and later recomputes agree when nothing changed.
|
||||
///
|
||||
/// Orphaned records (persona deleted) pass through unchanged: the caller's
|
||||
/// own orphan handling — refusing to spawn, hashing as `(None, None, None)`
|
||||
/// — runs on the real record downstream, not on this preview.
|
||||
pub fn preview_prospective_persona_snapshot(
|
||||
record: &ManagedAgentRecord,
|
||||
personas: &[AgentDefinition],
|
||||
) -> ManagedAgentRecord {
|
||||
let mut preview = record.clone();
|
||||
if let Some(persona_id) = preview.persona_id.clone() {
|
||||
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
|
||||
apply_persona_snapshot(&mut preview, persona);
|
||||
}
|
||||
}
|
||||
preview
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -1,4 +1,141 @@
|
||||
use super::*;
|
||||
use crate::managed_agents::{BackendKind, ManagedAgentRecord, RespondTo};
|
||||
|
||||
/// A linked instance record with no persona-derived fields set yet — the
|
||||
/// state right after creation, before any snapshot apply.
|
||||
fn sample_record() -> ManagedAgentRecord {
|
||||
ManagedAgentRecord {
|
||||
pubkey: "p".repeat(64),
|
||||
name: "agent".into(),
|
||||
persona_id: Some("test-persona".into()),
|
||||
private_key_nsec: "nsec1fake".into(),
|
||||
auth_tag: None,
|
||||
relay_url: "ws://localhost:3000".into(),
|
||||
avatar_url: None,
|
||||
acp_command: "buzz-acp".into(),
|
||||
agent_command: "goose".into(),
|
||||
agent_command_override: None,
|
||||
agent_args: vec![],
|
||||
mcp_command: String::new(),
|
||||
turn_timeout_seconds: 320,
|
||||
idle_timeout_seconds: None,
|
||||
max_turn_duration_seconds: None,
|
||||
parallelism: 1,
|
||||
system_prompt: None,
|
||||
model: None,
|
||||
provider: None,
|
||||
persona_source_version: None,
|
||||
env_vars: BTreeMap::new(),
|
||||
start_on_app_launch: false,
|
||||
auto_restart_on_config_change: true,
|
||||
runtime_pid: None,
|
||||
backend: BackendKind::Local,
|
||||
backend_agent_id: None,
|
||||
provider_binary_path: None,
|
||||
team_id: None,
|
||||
persona_team_dir: None,
|
||||
persona_name_in_team: None,
|
||||
created_at: "now".into(),
|
||||
updated_at: "now".into(),
|
||||
last_started_at: None,
|
||||
last_stopped_at: None,
|
||||
last_exit_code: None,
|
||||
last_error: None,
|
||||
last_error_code: None,
|
||||
respond_to: RespondTo::OwnerOnly,
|
||||
respond_to_allowlist: vec![],
|
||||
display_name: None,
|
||||
slug: None,
|
||||
runtime: None,
|
||||
name_pool: Vec::new(),
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: Vec::new(),
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
}
|
||||
}
|
||||
|
||||
// ── preview_prospective_persona_snapshot (Finding 4: relay-mesh preflight
|
||||
// ordering) ───────────────────────────────────────────────────────────────
|
||||
|
||||
/// Regression for the relay-mesh preflight-ordering defect: the preflight
|
||||
/// used to check `ensure_relay_mesh_for_record` against the record's stale
|
||||
/// pre-snapshot bytes, so a persona edit that flips `provider` to
|
||||
/// `relay-mesh` between saves would not trigger the mesh preflight on the
|
||||
/// very next start — only on the restart after that, once the real
|
||||
/// `apply_persona_snapshot` had already landed. The preview must reflect the
|
||||
/// same prospective re-snapshot the real spawn path applies.
|
||||
#[test]
|
||||
fn preview_reflects_persona_edit_flipping_provider_into_relay_mesh() {
|
||||
let record = sample_record(); // provider: None (not relay-mesh)
|
||||
let mut persona = sample_persona();
|
||||
persona.id = "test-persona".into();
|
||||
persona.provider = Some("relay-mesh".into());
|
||||
persona.model = Some("auto".into());
|
||||
|
||||
let preview = preview_prospective_persona_snapshot(&record, &[persona]);
|
||||
|
||||
assert_eq!(
|
||||
preview.provider.as_deref(),
|
||||
Some("relay-mesh"),
|
||||
"preview must carry the persona's current provider, not the record's stale None"
|
||||
);
|
||||
assert_eq!(preview.model.as_deref(), Some("auto"));
|
||||
}
|
||||
|
||||
/// Preview reflects a persona edit flipping AWAY from relay-mesh too —
|
||||
/// symmetric with the into-mesh case above.
|
||||
#[test]
|
||||
fn preview_reflects_persona_edit_flipping_provider_out_of_relay_mesh() {
|
||||
let mut record = sample_record();
|
||||
record.provider = Some("relay-mesh".into());
|
||||
record.model = Some("auto".into());
|
||||
let mut persona = sample_persona();
|
||||
persona.id = "test-persona".into();
|
||||
persona.provider = Some("anthropic".into());
|
||||
persona.model = Some("claude-opus-4".into());
|
||||
|
||||
let preview = preview_prospective_persona_snapshot(&record, &[persona]);
|
||||
|
||||
assert_eq!(preview.provider.as_deref(), Some("anthropic"));
|
||||
assert_eq!(preview.model.as_deref(), Some("claude-opus-4"));
|
||||
}
|
||||
|
||||
/// The preview must not mutate the record passed in — callers (the mesh
|
||||
/// preflight) need the original bytes intact for the real snapshot apply
|
||||
/// that follows.
|
||||
#[test]
|
||||
fn preview_does_not_mutate_input_record() {
|
||||
let record = sample_record();
|
||||
let original = record.clone();
|
||||
let mut persona = sample_persona();
|
||||
persona.id = "test-persona".into();
|
||||
persona.provider = Some("relay-mesh".into());
|
||||
|
||||
let _ = preview_prospective_persona_snapshot(&record, &[persona]);
|
||||
|
||||
assert_eq!(record.provider, original.provider);
|
||||
assert_eq!(record.model, original.model);
|
||||
}
|
||||
|
||||
/// Orphaned instance (persona deleted): the preview must pass the record
|
||||
/// through unchanged so downstream orphan handling (refuse to spawn) sees
|
||||
/// the same bytes it always would.
|
||||
#[test]
|
||||
fn preview_passes_through_unchanged_when_persona_missing() {
|
||||
let mut record = sample_record();
|
||||
record.provider = Some("anthropic".into());
|
||||
record.persona_id = Some("deleted-persona".into());
|
||||
|
||||
let preview = preview_prospective_persona_snapshot(&record, &[]);
|
||||
|
||||
assert_eq!(preview.provider.as_deref(), Some("anthropic"));
|
||||
assert_eq!(preview.persona_id.as_deref(), Some("deleted-persona"));
|
||||
}
|
||||
|
||||
fn sample_persona() -> AgentDefinition {
|
||||
AgentDefinition {
|
||||
@@ -437,43 +574,6 @@ fn persona_content_hash_changes_on_edit() {
|
||||
);
|
||||
}
|
||||
|
||||
// ── persona_field_with_record_fallback ────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn field_fallback_persona_present_wins() {
|
||||
assert_eq!(
|
||||
persona_field_with_record_fallback(Some("persona-model"), Some("record-model")),
|
||||
Some("persona-model".to_owned()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn field_fallback_persona_blank_uses_record() {
|
||||
assert_eq!(
|
||||
persona_field_with_record_fallback(None, Some("record-model")),
|
||||
Some("record-model".to_owned()),
|
||||
);
|
||||
assert_eq!(
|
||||
persona_field_with_record_fallback(Some(" "), Some("record-model")),
|
||||
Some("record-model".to_owned()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn field_fallback_both_blank_is_none() {
|
||||
assert_eq!(persona_field_with_record_fallback(None, None), None);
|
||||
assert_eq!(persona_field_with_record_fallback(Some(""), Some("")), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn field_fallback_record_blank_is_none() {
|
||||
assert_eq!(
|
||||
persona_field_with_record_fallback(None, Some(" ")),
|
||||
None,
|
||||
"whitespace-only record value must also be treated as blank"
|
||||
);
|
||||
}
|
||||
|
||||
// ── PersonaSnapshot.runtime ───────────────────────────────────────────────
|
||||
|
||||
/// (b) The snapshot carries the persona's runtime VERBATIM — including None,
|
||||
@@ -484,7 +584,7 @@ fn field_fallback_record_blank_is_none() {
|
||||
#[test]
|
||||
fn snapshot_runtime_verbatim_from_persona() {
|
||||
let persona = sample_persona(); // runtime = Some("goose")
|
||||
let snap = persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4"), Some("openai"));
|
||||
let snap = persona_snapshot(&persona);
|
||||
assert_eq!(
|
||||
snap.runtime.as_deref(),
|
||||
Some("goose"),
|
||||
@@ -493,15 +593,14 @@ fn snapshot_runtime_verbatim_from_persona() {
|
||||
|
||||
let mut no_runtime = sample_persona();
|
||||
no_runtime.runtime = None;
|
||||
let snap =
|
||||
persona_snapshot_with_agent_config_fallback(&no_runtime, Some("gpt-4"), Some("openai"));
|
||||
let snap = persona_snapshot(&no_runtime);
|
||||
assert_eq!(
|
||||
snap.runtime, None,
|
||||
"persona runtime None must produce None snapshot (clears stale materialized value)"
|
||||
);
|
||||
}
|
||||
|
||||
// ── persona_snapshot_with_agent_config_fallback ────────────────────────────
|
||||
// ── persona_snapshot (definition-authoritative) ──────────────────────────
|
||||
|
||||
/// Helper: a persona with no model/provider configured.
|
||||
fn blank_model_persona() -> AgentDefinition {
|
||||
@@ -512,25 +611,22 @@ fn blank_model_persona() -> AgentDefinition {
|
||||
}
|
||||
}
|
||||
|
||||
/// (a) Persona leaves model/provider blank, agent record has values →
|
||||
/// record values preserved AND source_version still updated to current hash.
|
||||
/// Definition-authoritative: blank definition model/provider → snapshot is
|
||||
/// None. The effective-config resolver handles global fallback at read time.
|
||||
#[test]
|
||||
fn fallback_preserves_record_values_when_persona_blank() {
|
||||
fn blank_definition_clears_model_provider() {
|
||||
let persona = blank_model_persona();
|
||||
let expected_version = persona_content_hash(&persona_event_content(&persona));
|
||||
|
||||
let snapshot =
|
||||
persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4o"), Some("openai"));
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
Some("gpt-4o"),
|
||||
"blank persona model must fall back to agent record value"
|
||||
assert!(
|
||||
snapshot.model.is_none(),
|
||||
"blank definition model must produce None"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.provider.as_deref(),
|
||||
Some("openai"),
|
||||
"blank persona provider must fall back to agent record value"
|
||||
assert!(
|
||||
snapshot.provider.is_none(),
|
||||
"blank definition provider must produce None"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.source_version, expected_version,
|
||||
@@ -538,124 +634,102 @@ fn fallback_preserves_record_values_when_persona_blank() {
|
||||
);
|
||||
}
|
||||
|
||||
/// (b) Persona has model/provider set → persona wins over agent record.
|
||||
/// Persona has model/provider set → snapshot carries them verbatim.
|
||||
#[test]
|
||||
fn fallback_persona_wins_when_set() {
|
||||
fn snapshot_carries_persona_model_provider() {
|
||||
let persona = sample_persona(); // has model=Some("claude-opus-4"), provider=Some("anthropic")
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
Some("gpt-4o"), // agent had a different model
|
||||
Some("openai"), // agent had a different provider
|
||||
);
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
Some("claude-opus-4"),
|
||||
"persona model must win when persona has a value"
|
||||
"persona model must be carried verbatim"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.provider.as_deref(),
|
||||
Some("anthropic"),
|
||||
"persona provider must win when persona has a value"
|
||||
"persona provider must be carried verbatim"
|
||||
);
|
||||
}
|
||||
|
||||
/// (c) Both blank → snapshot keeps None; a genuinely unconfigured agent
|
||||
/// Both blank → snapshot keeps None; a genuinely unconfigured agent
|
||||
/// stays unconfigured (no fabricated values).
|
||||
#[test]
|
||||
fn fallback_both_blank_stays_none() {
|
||||
fn both_blank_stays_none() {
|
||||
let persona = blank_model_persona();
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona, None, // agent also has no model
|
||||
None, // agent also has no provider
|
||||
);
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert!(
|
||||
snapshot.model.is_none(),
|
||||
"neither persona nor agent has model — snapshot must be None"
|
||||
"blank persona model → snapshot None"
|
||||
);
|
||||
assert!(
|
||||
snapshot.provider.is_none(),
|
||||
"neither persona nor agent has provider — snapshot must be None"
|
||||
"blank persona provider → snapshot None"
|
||||
);
|
||||
}
|
||||
|
||||
/// Whitespace-only values on the persona are treated as blank; agent
|
||||
/// fallback applies.
|
||||
/// Whitespace-only values on the definition are preserved verbatim in the
|
||||
/// snapshot. The effective-config resolver treats whitespace as blank.
|
||||
#[test]
|
||||
fn fallback_treats_whitespace_only_persona_value_as_blank() {
|
||||
fn whitespace_only_definition_preserved_verbatim() {
|
||||
let mut persona = sample_persona();
|
||||
persona.model = Some(" ".to_string());
|
||||
persona.provider = Some("\t".to_string());
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
Some("claude-opus-4"),
|
||||
Some("anthropic"),
|
||||
);
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
Some("claude-opus-4"),
|
||||
"whitespace-only persona model must be treated as blank"
|
||||
Some(" "),
|
||||
"whitespace-only model is preserved verbatim in snapshot"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.provider.as_deref(),
|
||||
Some("anthropic"),
|
||||
"whitespace-only persona provider must be treated as blank"
|
||||
Some("\t"),
|
||||
"whitespace-only provider is preserved verbatim in snapshot"
|
||||
);
|
||||
}
|
||||
|
||||
/// Cross-field independence: persona sets model but not provider → model
|
||||
/// comes from persona, provider falls back to the record. This is the
|
||||
/// practically common case (model-only personas).
|
||||
/// Cross-field independence: definition sets model but not provider → model
|
||||
/// comes from definition, provider is None (global fallback at read time).
|
||||
#[test]
|
||||
fn fallback_persona_model_set_provider_blank_uses_record_provider() {
|
||||
let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic")
|
||||
persona.provider = None; // blank provider on persona
|
||||
fn definition_model_set_provider_blank_produces_none_provider() {
|
||||
let mut persona = sample_persona();
|
||||
persona.provider = None;
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
Some("gpt-4o"), // record model (should be overridden by persona)
|
||||
Some("openai"), // record provider (should be preserved)
|
||||
);
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
Some("claude-opus-4"),
|
||||
"persona model must win when persona has a value"
|
||||
"definition model must be used"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.provider.as_deref(),
|
||||
Some("openai"),
|
||||
"record provider must be used when persona provider is blank"
|
||||
assert!(
|
||||
snapshot.provider.is_none(),
|
||||
"definition provider=None → snapshot None"
|
||||
);
|
||||
}
|
||||
|
||||
/// Inverse: persona sets provider but not model → provider comes from
|
||||
/// persona, model falls back to the record.
|
||||
/// Definition provider set, model blank → model=None (global fallback at
|
||||
/// read time), provider from definition.
|
||||
#[test]
|
||||
fn fallback_persona_provider_set_model_blank_uses_record_model() {
|
||||
let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic")
|
||||
persona.model = None; // blank model on persona
|
||||
fn definition_provider_set_model_blank_produces_none_model() {
|
||||
let mut persona = sample_persona();
|
||||
persona.model = None;
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
Some("gpt-4o"), // record model (should be preserved)
|
||||
Some("openai"), // record provider (should be overridden by persona)
|
||||
);
|
||||
let snapshot = persona_snapshot(&persona);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
Some("gpt-4o"),
|
||||
"record model must be used when persona model is blank"
|
||||
assert!(
|
||||
snapshot.model.is_none(),
|
||||
"definition model=None → snapshot None"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.provider.as_deref(),
|
||||
Some("anthropic"),
|
||||
"persona provider must win when persona has a value"
|
||||
"definition provider must be used"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -226,11 +226,6 @@ fn resolve_effective_agent_env_with_def(
|
||||
// Layer 1: baked build defaults (floor — internal builds only; OSS = empty).
|
||||
let mut env = baked_build_env();
|
||||
|
||||
// Layer 2: runtime metadata env vars (model / provider keys derived from
|
||||
// the record's structured fields, with global as fallback).
|
||||
//
|
||||
// Uses the shared resolver to guarantee readiness and spawn agree on the
|
||||
// effective model/provider: agent → persona → global → None.
|
||||
let (effective_model, effective_provider) =
|
||||
super::global_config::resolve_effective_model_provider(record, personas, global);
|
||||
|
||||
@@ -239,8 +234,8 @@ fn resolve_effective_agent_env_with_def(
|
||||
rt.model_env_var,
|
||||
rt.provider_env_var,
|
||||
rt.provider_locked,
|
||||
effective_model,
|
||||
effective_provider,
|
||||
effective_model.as_deref(),
|
||||
effective_provider.as_deref(),
|
||||
) {
|
||||
env.insert(key.to_string(), value.to_string());
|
||||
}
|
||||
@@ -277,7 +272,11 @@ fn resolve_effective_agent_env_with_def(
|
||||
// Buzz shared compute is a native Buzz provider. Translate it to buzz-agent's
|
||||
// OpenAI-compatible transport only in the effective runtime environment.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
super::apply_relay_mesh_env(&mut env, effective_provider, effective_model);
|
||||
super::apply_relay_mesh_env(
|
||||
&mut env,
|
||||
effective_provider.as_deref(),
|
||||
effective_model.as_deref(),
|
||||
);
|
||||
|
||||
EffectiveAgentEnv {
|
||||
env,
|
||||
|
||||
@@ -1,6 +1,3 @@
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
use super::{ManagedAgentRecord, RelayMeshConfig};
|
||||
|
||||
pub const RELAY_MESH_API_BASE_URL: &str = "http://127.0.0.1:9337/v1";
|
||||
pub const RELAY_MESH_API_KEY_PLACEHOLDER: &str = "buzz-mesh-local";
|
||||
pub const RELAY_MESH_PROVIDER_ID: &str = "relay-mesh";
|
||||
@@ -46,146 +43,11 @@ pub fn apply_relay_mesh_env(
|
||||
env.insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "none".to_string());
|
||||
}
|
||||
|
||||
/// Resolve a record's relay-mesh config, typed field first.
|
||||
///
|
||||
/// Source of truth is the typed `record.relay_mesh` field. For records saved
|
||||
/// before that field existed, fall back to detecting the relay-mesh preset
|
||||
/// from `env_vars` (the legacy discriminator). New records carry the typed
|
||||
/// field and need no env-var sniffing at all.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
pub fn relay_mesh_config(record: &ManagedAgentRecord) -> Option<RelayMeshConfig> {
|
||||
if record.provider.as_deref() == Some(RELAY_MESH_PROVIDER_ID) {
|
||||
let model_ref = record
|
||||
.model
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
.unwrap_or(RELAY_MESH_AUTO_MODEL_ID)
|
||||
.to_string();
|
||||
return Some(RelayMeshConfig { model_ref });
|
||||
}
|
||||
if let Some(config) = &record.relay_mesh {
|
||||
return Some(config.clone());
|
||||
}
|
||||
relay_mesh_model_id_from_env(record).map(|model_ref| RelayMeshConfig { model_ref })
|
||||
}
|
||||
|
||||
/// Returns the relay-mesh model id for agents whose provider env points at the
|
||||
/// local mesh client endpoint created by Buzz's relay-mesh preset.
|
||||
///
|
||||
/// Prefer [`relay_mesh_config`]; this remains as a convenience for call sites
|
||||
/// that only need the model id.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
pub fn relay_mesh_model_id(record: &ManagedAgentRecord) -> Option<String> {
|
||||
relay_mesh_config(record).map(|config| config.model_ref)
|
||||
}
|
||||
|
||||
/// Legacy env-var discriminator: detects the relay-mesh preset purely from the
|
||||
/// four preset env vars. Used as a fallback for records saved before the typed
|
||||
/// `relay_mesh` field existed.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
fn relay_mesh_model_id_from_env(record: &ManagedAgentRecord) -> Option<String> {
|
||||
let base_url = record.env_vars.get("OPENAI_COMPAT_BASE_URL")?.trim();
|
||||
if base_url.trim_end_matches('/') != RELAY_MESH_API_BASE_URL {
|
||||
return None;
|
||||
}
|
||||
let provider = record.env_vars.get("BUZZ_AGENT_PROVIDER")?.trim();
|
||||
if provider != "openai" {
|
||||
return None;
|
||||
}
|
||||
let api_key = record.env_vars.get("OPENAI_COMPAT_API_KEY")?.trim();
|
||||
if api_key != RELAY_MESH_API_KEY_PLACEHOLDER {
|
||||
return None;
|
||||
}
|
||||
record
|
||||
.env_vars
|
||||
.get("OPENAI_COMPAT_MODEL")
|
||||
.map(|value| value.trim())
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "mesh-llm"))]
|
||||
mod tests {
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use super::*;
|
||||
use crate::managed_agents::{BackendKind, ManagedAgentRecord, RespondTo};
|
||||
|
||||
fn fixture() -> ManagedAgentRecord {
|
||||
ManagedAgentRecord {
|
||||
pubkey: "p".into(),
|
||||
name: "n".into(),
|
||||
persona_id: None,
|
||||
private_key_nsec: "nsec1fake".into(),
|
||||
auth_tag: Some("tag".into()),
|
||||
relay_url: "ws://localhost:3000".into(),
|
||||
avatar_url: None,
|
||||
acp_command: "buzz-acp".into(),
|
||||
agent_command: "goose".into(),
|
||||
agent_command_override: None,
|
||||
agent_args: vec![],
|
||||
mcp_command: String::new(),
|
||||
turn_timeout_seconds: 320,
|
||||
idle_timeout_seconds: None,
|
||||
max_turn_duration_seconds: None,
|
||||
parallelism: 1,
|
||||
system_prompt: None,
|
||||
model: None,
|
||||
env_vars: BTreeMap::new(),
|
||||
start_on_app_launch: false,
|
||||
auto_restart_on_config_change: true,
|
||||
runtime_pid: None,
|
||||
backend: BackendKind::Local,
|
||||
backend_agent_id: None,
|
||||
provider_binary_path: None,
|
||||
team_id: None,
|
||||
persona_team_dir: None,
|
||||
persona_name_in_team: None,
|
||||
persona_source_version: None,
|
||||
provider: None,
|
||||
created_at: "now".into(),
|
||||
updated_at: "now".into(),
|
||||
last_started_at: None,
|
||||
last_stopped_at: None,
|
||||
last_exit_code: None,
|
||||
last_error: None,
|
||||
last_error_code: None,
|
||||
respond_to: RespondTo::OwnerOnly,
|
||||
respond_to_allowlist: vec![],
|
||||
display_name: None,
|
||||
slug: None,
|
||||
runtime: None,
|
||||
name_pool: Vec::new(),
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: Vec::new(),
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relay_mesh_model_id_detects_mesh_preset_env() {
|
||||
let mut rec = fixture();
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1/".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(relay_mesh_model_id(&rec).as_deref(), Some("Qwen3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_provider_uses_context_safe_non_reasoning_budget() {
|
||||
@@ -205,95 +67,4 @@ mod tests {
|
||||
Some("none")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relay_mesh_model_id_ignores_non_mesh_openai_env() {
|
||||
let mut rec = fixture();
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"https://api.openai.com/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "gpt-5".to_string()),
|
||||
]);
|
||||
|
||||
assert_eq!(relay_mesh_model_id(&rec), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relay_mesh_model_id_ignores_user_openai_on_same_local_port() {
|
||||
let mut rec = fixture();
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
("OPENAI_COMPAT_API_KEY".to_string(), "real-key".to_string()),
|
||||
]);
|
||||
|
||||
assert_eq!(relay_mesh_model_id(&rec), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_provider_fields_are_authoritative() {
|
||||
// The whole point: a typed record needs no env-var sniffing to be
|
||||
// recognized as a relay-mesh agent.
|
||||
let mut rec = fixture();
|
||||
rec.provider = Some(RELAY_MESH_PROVIDER_ID.to_string());
|
||||
rec.model = Some("Qwen3".to_string());
|
||||
assert!(rec.env_vars.is_empty());
|
||||
assert_eq!(
|
||||
relay_mesh_config(&rec),
|
||||
Some(RelayMeshConfig {
|
||||
model_ref: "Qwen3".to_string()
|
||||
})
|
||||
);
|
||||
assert_eq!(relay_mesh_model_id(&rec).as_deref(), Some("Qwen3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_provider_fields_win_over_legacy_config() {
|
||||
let mut rec = fixture();
|
||||
rec.provider = Some(RELAY_MESH_PROVIDER_ID.to_string());
|
||||
rec.model = Some("native-model".to_string());
|
||||
rec.relay_mesh = Some(RelayMeshConfig {
|
||||
model_ref: "typed-model".to_string(),
|
||||
});
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "env-model".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
assert_eq!(relay_mesh_model_id(&rec).as_deref(), Some("native-model"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_record_falls_back_to_env_sniff() {
|
||||
// Records saved before the typed field still resolve via env vars.
|
||||
let mut rec = fixture();
|
||||
rec.relay_mesh = None;
|
||||
rec.env_vars = BTreeMap::from([
|
||||
("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_BASE_URL".to_string(),
|
||||
"http://127.0.0.1:9337/v1".to_string(),
|
||||
),
|
||||
("OPENAI_COMPAT_MODEL".to_string(), "Qwen3".to_string()),
|
||||
(
|
||||
"OPENAI_COMPAT_API_KEY".to_string(),
|
||||
RELAY_MESH_API_KEY_PLACEHOLDER.to_string(),
|
||||
),
|
||||
]);
|
||||
assert_eq!(relay_mesh_model_id(&rec).as_deref(), Some("Qwen3"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
use super::relay_mesh_model_id;
|
||||
use super::{
|
||||
find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, load_personas,
|
||||
save_managed_agents, spawn_agent_child, sync_managed_agent_processes, BackendKind,
|
||||
@@ -36,10 +34,9 @@ type AgentSpawnResult = (String, SpawnOutcome);
|
||||
/// `model`/`provider` were clobbered by the old unconditional snapshot code before
|
||||
/// this fix — are skipped here; they self-heal on the next manual start via the
|
||||
/// start-path re-snapshot in `start_local_agent_with_preflight`.
|
||||
/// If the linked persona is gone, we log loudly and leave the snapshot empty —
|
||||
/// the record's own `system_prompt`/`model` (possibly empty for persona-created
|
||||
/// agents) is then all the config that remains, which is the same fallback an
|
||||
/// orphaned agent already gets.
|
||||
/// If the linked persona is gone, we log loudly and leave the record untouched —
|
||||
/// it stays orphaned and `spawn_agent_child` refuses to start it (see
|
||||
/// `effective_config::resolve_effective_config`'s `OrphanedInstance` arm).
|
||||
pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String> {
|
||||
let state = app.state::<AppState>();
|
||||
let _store_guard = state
|
||||
@@ -66,7 +63,7 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String>
|
||||
}
|
||||
let Some(persona) = personas.iter().find(|p| p.id == persona_id) else {
|
||||
eprintln!(
|
||||
"buzz-desktop: persona-snapshot backfill: agent {} links persona {persona_id} which no longer exists; leaving snapshot empty — it will spawn from its record fields",
|
||||
"buzz-desktop: persona-snapshot backfill: agent {} links persona {persona_id} which no longer exists; leaving it orphaned — spawn will refuse it",
|
||||
record.pubkey
|
||||
);
|
||||
continue;
|
||||
@@ -206,6 +203,9 @@ pub async fn restore_managed_agents_on_launch(
|
||||
continue;
|
||||
};
|
||||
let Some(persona) = personas_for_snapshot.iter().find(|p| p.id == persona_id) else {
|
||||
// Orphaned: no current persona to re-snapshot from. Leave the
|
||||
// record as-is — `spawn_agent_child` (Phase B below) refuses to
|
||||
// spawn it and Phase C persists the refusal to `last_error`.
|
||||
continue;
|
||||
};
|
||||
super::persona_events::apply_persona_snapshot(record, persona);
|
||||
@@ -240,16 +240,26 @@ pub async fn restore_managed_agents_on_launch(
|
||||
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
let agents_to_start = {
|
||||
// Preflight against the same resolution spawn uses — `resolve_effective_config`
|
||||
// (definition → global fallback). A linked instance's own `provider`/`model`/
|
||||
// `relay_mesh` bytes never contribute. See `start_local_agent_with_preflight`
|
||||
// in `commands/agents.rs` for the identical rationale on the interactive path.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
let global = super::load_global_agent_config(app).unwrap_or_default();
|
||||
let mut mesh_preflight_failures = std::collections::HashSet::new();
|
||||
for record in &agents_to_start {
|
||||
if relay_mesh_model_id(record).is_none() {
|
||||
let mesh_model_id = super::effective_config::resolve_effective_relay_mesh_model_id(
|
||||
record, &personas, &global,
|
||||
);
|
||||
if mesh_model_id.is_none() {
|
||||
continue;
|
||||
}
|
||||
// Auto-start after relaunch: re-resolve a live bootstrap target and
|
||||
// dial it. Skip (with an actionable error) only when no live target
|
||||
// serves this model right now.
|
||||
if let Err(error) =
|
||||
crate::commands::ensure_relay_mesh_for_record(app, record, false).await
|
||||
crate::commands::ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), false)
|
||||
.await
|
||||
{
|
||||
persist_restore_error(app, &state, &record.pubkey, error)?;
|
||||
mesh_preflight_failures.insert(record.pubkey.clone());
|
||||
|
||||
@@ -192,12 +192,37 @@ pub fn build_managed_agent_summary(
|
||||
}
|
||||
};
|
||||
|
||||
// Display contract: show the pinned record snapshot — what the agent
|
||||
// actually runs — not the live persona. The drift flags below signal when
|
||||
// the snapshot has fallen behind an edited persona; showing live values
|
||||
// next to an "out of date" badge would contradict it.
|
||||
let (persona_out_of_date, persona_orphaned) = persona_drift_state(record, personas);
|
||||
|
||||
let global_for_summary =
|
||||
crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
let effective_cfg = crate::managed_agents::effective_config::resolve_effective_config(
|
||||
record,
|
||||
personas,
|
||||
&global_for_summary,
|
||||
);
|
||||
let (effective_model, effective_provider, effective_prompt, model_source) = match effective_cfg
|
||||
{
|
||||
crate::managed_agents::effective_config::EffectiveConfigResult::Resolved(cfg) => {
|
||||
let source = cfg.model.source.clone();
|
||||
(
|
||||
cfg.model.value,
|
||||
cfg.provider.value,
|
||||
cfg.system_prompt.value,
|
||||
Some(source),
|
||||
)
|
||||
}
|
||||
crate::managed_agents::effective_config::EffectiveConfigResult::OrphanedInstance {
|
||||
record_pubkey,
|
||||
missing_persona_id,
|
||||
} => {
|
||||
eprintln!(
|
||||
"orphaned agent instance: pubkey={record_pubkey}, missing_persona_id={missing_persona_id}"
|
||||
);
|
||||
(None, None, None, None)
|
||||
}
|
||||
};
|
||||
|
||||
// Restart badge: the running process stamped its effective spawn config
|
||||
// at launch; recompute from current disk state and flag drift. Only the
|
||||
// tracked live pair for THIS workspace can drift — stopped agents spawn
|
||||
@@ -233,7 +258,13 @@ pub fn build_managed_agent_summary(
|
||||
runtime.adapter_availability.as_ref(),
|
||||
super::adapter_availability_cached(),
|
||||
);
|
||||
hash_drift || availability_drift
|
||||
// An orphan can never be restarted successfully —
|
||||
// `spawn_agent_child` refuses it before any process side effect —
|
||||
// so `needs_restart` must never fire for one regardless of hash or
|
||||
// availability drift. Surfacing "Restart required" here would offer
|
||||
// an action guaranteed to fail; the UI shows `persona_orphaned`
|
||||
// instead (see `ManagedAgentSummary::persona_orphaned`).
|
||||
restart_eligible(persona_orphaned, hash_drift, availability_drift)
|
||||
});
|
||||
|
||||
// Resolve the effective harness via the single typed descriptor — same resolver
|
||||
@@ -279,10 +310,11 @@ pub fn build_managed_agent_summary(
|
||||
idle_timeout_seconds: record.idle_timeout_seconds,
|
||||
max_turn_duration_seconds: record.max_turn_duration_seconds,
|
||||
parallelism: record.parallelism,
|
||||
system_prompt: record.system_prompt.clone(),
|
||||
system_prompt: effective_prompt,
|
||||
avatar_url: record.avatar_url.clone(),
|
||||
model: record.model.clone(),
|
||||
provider: record.provider.clone(),
|
||||
model: effective_model,
|
||||
model_source,
|
||||
provider: effective_provider,
|
||||
persona_out_of_date,
|
||||
persona_orphaned,
|
||||
needs_restart,
|
||||
@@ -306,6 +338,19 @@ pub fn build_managed_agent_summary(
|
||||
})
|
||||
}
|
||||
|
||||
/// Pure predicate: should the "Restart required" badge fire?
|
||||
///
|
||||
/// An orphaned linked instance (its persona/definition no longer exists)
|
||||
/// can never be restarted successfully — `spawn_agent_child` refuses to
|
||||
/// spawn it before any process side effect. Surfacing "Restart required"
|
||||
/// for one would offer an action guaranteed to fail, so this always
|
||||
/// returns `false` for an orphan regardless of drift. Extracted for unit
|
||||
/// testing without `AppHandle`/global state, following the
|
||||
/// `availability_drift` pattern in `discovery.rs`.
|
||||
fn restart_eligible(persona_orphaned: bool, hash_drift: bool, availability_drift: bool) -> bool {
|
||||
!persona_orphaned && (hash_drift || availability_drift)
|
||||
}
|
||||
|
||||
pub fn find_managed_agent_mut<'a>(
|
||||
records: &'a mut [ManagedAgentRecord],
|
||||
pubkey: &str,
|
||||
@@ -414,6 +459,51 @@ pub fn spawn_agent_child(
|
||||
return Err(error);
|
||||
}
|
||||
let runtime_key = ManagedAgentRuntimeKey::new(record.pubkey.clone(), relay_url)?;
|
||||
// Resolve the effective harness (agent command) from the linked persona, so
|
||||
// persona harness edits propagate on the next spawn; an explicit per-agent
|
||||
// override wins. `agent_args` and `mcp_command` are pure derivations of the
|
||||
// command, so we recompute them from the effective value rather than the
|
||||
// frozen record snapshot. Mirrors the model resolution below.
|
||||
let personas = super::load_personas(app).unwrap_or_default();
|
||||
let teams = super::load_teams(app).unwrap_or_default();
|
||||
// Load global config once; used for runtime_metadata_env_vars (model/provider fallback)
|
||||
// and for the env-var merge at spawn time.
|
||||
let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
|
||||
// Resolve model/provider/prompt ONCE, here, at the shared spawn boundary —
|
||||
// the single source both the env writes below and `spawn_config_hash`
|
||||
// read from. Previously prompt was read from the record's own (possibly
|
||||
// stale, Phase-A-snapshot) bytes while model/provider were resolved live
|
||||
// from `personas`; a definition edit landing between a caller's snapshot
|
||||
// apply and this spawn could hand a fresh model/provider to a stale
|
||||
// prompt. This also folds in orphan refusal via `require_resolved`: every
|
||||
// caller (interactive start, launch restore, `start_managed_agent_process`)
|
||||
// inherits it — no caller can bypass this by reaching `spawn_agent_child`
|
||||
// directly. Checked before any side effect (log marker, log file, process
|
||||
// spawn) so a refused spawn leaves no trace.
|
||||
let effective_cfg = crate::managed_agents::effective_config::resolve_effective_config(
|
||||
record, &personas, &global,
|
||||
)
|
||||
.require_resolved()?;
|
||||
|
||||
// Single typed resolver: validates runtime id (dangling harness → Err), resolves
|
||||
// command, args (instance wins over definition default), and the full env layer stack.
|
||||
// This is the sole path for harness-definition lookup — spawn, hash, summary, and
|
||||
// model probes all consume this descriptor rather than assembling values inline.
|
||||
// Like the orphan refusal above, this runs before any side effect so a refused
|
||||
// spawn leaves no trace.
|
||||
let descriptor =
|
||||
crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global)
|
||||
.map_err(|e| {
|
||||
format!(
|
||||
"cannot spawn agent {}: {}",
|
||||
record.pubkey,
|
||||
crate::managed_agents::user_facing_harness_error(&e)
|
||||
)
|
||||
})?;
|
||||
let effective_command = &descriptor.command;
|
||||
let agent_args = &descriptor.args;
|
||||
|
||||
let log_path = super::managed_agent_runtime_log_path(app, &runtime_key)?;
|
||||
append_log_marker(
|
||||
&log_path,
|
||||
@@ -429,31 +519,6 @@ pub fn spawn_agent_child(
|
||||
let stderr = stdout
|
||||
.try_clone()
|
||||
.map_err(|error| format!("failed to clone log handle: {error}"))?;
|
||||
// Resolve the effective harness (agent command) from the linked persona, so
|
||||
// persona harness edits propagate on the next spawn; an explicit per-agent
|
||||
// override wins. `agent_args` and `mcp_command` are pure derivations of the
|
||||
// command, so we recompute them from the effective value rather than the
|
||||
// frozen record snapshot. Mirrors the model resolution below.
|
||||
let personas = super::load_personas(app).unwrap_or_default();
|
||||
let teams = super::load_teams(app).unwrap_or_default();
|
||||
// Load global config once; used for runtime_metadata_env_vars (model/provider fallback)
|
||||
// and for the env-var merge at spawn time.
|
||||
let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
// Single typed resolver: validates runtime id (dangling harness → Err), resolves
|
||||
// command, args (instance wins over definition default), and the full env layer stack.
|
||||
// This is the sole path for harness-definition lookup — spawn, hash, summary, and
|
||||
// model probes all consume this descriptor rather than assembling values inline.
|
||||
let descriptor =
|
||||
crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global)
|
||||
.map_err(|e| {
|
||||
format!(
|
||||
"cannot spawn agent {}: {}",
|
||||
record.pubkey,
|
||||
crate::managed_agents::user_facing_harness_error(&e)
|
||||
)
|
||||
})?;
|
||||
let effective_command = &descriptor.command;
|
||||
let agent_args = &descriptor.args;
|
||||
let resolved_acp_command = resolve_command(&record.acp_command)
|
||||
.ok_or_else(|| missing_command_message(&record.acp_command, "ACP harness command"))?;
|
||||
let effective_mcp_command = known_acp_runtime(effective_command)
|
||||
@@ -675,37 +740,40 @@ pub fn spawn_agent_child(
|
||||
command.env_remove("BUZZ_ACP_TEAM_INSTRUCTIONS");
|
||||
}
|
||||
|
||||
// System prompt via the shared spawn-effective filter — the SAME function the
|
||||
// config hash digests, so env write and badge cannot disagree (see
|
||||
// `effective_spawn_prompt` for the Some("")/None collapse). Model and provider
|
||||
// use the shared
|
||||
// resolver: agent → persona → global → None, so a global-default-only agent
|
||||
// spawns with the correct provider/model env.
|
||||
let effective_prompt = super::spawn_hash::effective_spawn_prompt(record);
|
||||
let (effective_model, effective_provider) =
|
||||
crate::managed_agents::resolve_effective_model_provider(record, &personas, &global);
|
||||
// Prompt, model, and provider all come from the single `effective_cfg`
|
||||
// resolved at the top of this function — the SAME resolve `spawn_config_hash`
|
||||
// performs below, so env write and restart badge cannot disagree. Linked
|
||||
// instances never consult the record's own model/provider/prompt bytes;
|
||||
// definition-less instances fall back to their own fields, then global.
|
||||
//
|
||||
// Derive the mesh decision BEFORE moving fields out — `relay_mesh_model_id`
|
||||
// is the single authoritative gate; the mesh-llm block below MUST use it
|
||||
// rather than re-deriving from `effective_provider` to keep preflight and
|
||||
// spawn semantics in lock-step (see `EffectiveAgentConfig::relay_mesh_model_id`).
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
let mesh_model_id = effective_cfg.relay_mesh_model_id();
|
||||
let effective_prompt = effective_cfg.system_prompt.value;
|
||||
let effective_model = effective_cfg.model.value;
|
||||
let effective_provider = effective_cfg.provider.value;
|
||||
|
||||
if let Some(prompt) = &effective_prompt {
|
||||
command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt);
|
||||
} else {
|
||||
command.env_remove("BUZZ_ACP_SYSTEM_PROMPT");
|
||||
}
|
||||
if let Some(model) = effective_model {
|
||||
if let Some(model) = effective_model.as_deref() {
|
||||
command.env("BUZZ_ACP_MODEL", model);
|
||||
} else {
|
||||
command.env_remove("BUZZ_ACP_MODEL");
|
||||
}
|
||||
// Baked-in provider defaults for internal builds (buzz-releases sets
|
||||
// BUZZ_BUILD_BUZZ_AGENT_* at compile time; OSS builds bake nothing).
|
||||
// Written FIRST so that record/persona metadata env vars below override them.
|
||||
build_buzz_agent_provider_defaults(&mut command);
|
||||
if let Some(meta) = runtime_meta {
|
||||
for (key, value) in runtime_metadata_env_vars(
|
||||
meta.model_env_var,
|
||||
meta.provider_env_var,
|
||||
meta.provider_locked,
|
||||
effective_model,
|
||||
effective_provider,
|
||||
effective_model.as_deref(),
|
||||
effective_provider.as_deref(),
|
||||
) {
|
||||
command.env(key, value);
|
||||
}
|
||||
@@ -783,10 +851,17 @@ pub fn spawn_agent_child(
|
||||
|
||||
// Buzz shared compute is stored as a native provider; derive the OpenAI-compatible
|
||||
// transport at spawn time and scrub any unrelated ambient OpenAI key.
|
||||
// Gate on `mesh_model_id` (derived from `effective_cfg.relay_mesh_model_id()`
|
||||
// above) — not on `effective_provider` directly — so the mesh gate here
|
||||
// uses the same trim semantics as the preflight callers.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
if effective_provider == Some(super::RELAY_MESH_PROVIDER_ID) {
|
||||
if let Some(ref mesh_model_id) = mesh_model_id {
|
||||
let mut mesh_env = std::collections::BTreeMap::new();
|
||||
super::apply_relay_mesh_env(&mut mesh_env, effective_provider, effective_model);
|
||||
super::apply_relay_mesh_env(
|
||||
&mut mesh_env,
|
||||
Some(super::RELAY_MESH_PROVIDER_ID),
|
||||
Some(mesh_model_id.as_str()),
|
||||
);
|
||||
command.env_remove("OPENAI_API_KEY");
|
||||
for (key, value) in mesh_env {
|
||||
command.env(key, value);
|
||||
|
||||
@@ -24,18 +24,11 @@ pub(crate) fn runtime_metadata_env_vars<'a>(
|
||||
vars
|
||||
}
|
||||
|
||||
/// Resolve the effective (prompt, model, provider) triple for a persona-linked agent.
|
||||
///
|
||||
/// Given a persona_id, finds the persona in the list and returns its system_prompt,
|
||||
/// model, and provider as the authoritative values. When the persona leaves `model`
|
||||
/// or `provider` blank (None or whitespace-only), falls back to the record's own
|
||||
/// field using the same precedence rule as `persona_snapshot_with_agent_config_fallback`
|
||||
/// so the display surface matches spawn behavior. Falls back to the record's own
|
||||
/// prompt/model/provider when no persona is linked or found.
|
||||
/// Resolve effective prompt/model/provider using definition-authoritative
|
||||
/// semantics for linked instances.
|
||||
///
|
||||
/// Used by `agent_config.rs` to inject persona defaults into the config surface
|
||||
/// before running the reader, so BuzzExplicit-tagged fields can be re-tagged to
|
||||
/// PersonaDefault for fields the record did not independently set.
|
||||
/// before running the reader.
|
||||
pub(crate) fn resolve_effective_prompt_model_provider(
|
||||
persona_id: Option<&str>,
|
||||
personas: &[crate::managed_agents::types::AgentDefinition],
|
||||
@@ -43,13 +36,16 @@ pub(crate) fn resolve_effective_prompt_model_provider(
|
||||
record_model: Option<String>,
|
||||
record_provider: Option<String>,
|
||||
) -> (Option<String>, Option<String>, Option<String>) {
|
||||
let fallback = crate::managed_agents::persona_events::persona_field_with_record_fallback;
|
||||
match persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid)) {
|
||||
Some(p) => (
|
||||
Some(p.system_prompt.clone()),
|
||||
fallback(p.model.as_deref(), record_model.as_deref()), // fallback: record.model
|
||||
fallback(p.provider.as_deref(), record_provider.as_deref()), // fallback: record.provider
|
||||
),
|
||||
Some(p) => {
|
||||
fn non_blank(v: Option<&str>) -> Option<String> {
|
||||
v.filter(|s| !s.trim().is_empty()).map(str::to_owned)
|
||||
}
|
||||
let prompt = non_blank(Some(&p.system_prompt));
|
||||
let model = non_blank(p.model.as_deref());
|
||||
let provider = non_blank(p.provider.as_deref());
|
||||
(prompt, model, provider)
|
||||
}
|
||||
None => (record_prompt, record_model, record_provider),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,30 +30,25 @@ pub(crate) fn should_skip_claude_executable(path: &std::path::Path, is_windows:
|
||||
/// Decide whether the inherited process PATH should be appended to the
|
||||
/// composed PATH.
|
||||
///
|
||||
/// On Windows, `login_shell_path()` always returns `None` because Git Bash
|
||||
/// returns POSIX colon-delimited paths that poison native children.
|
||||
/// `Command::env("PATH", …)` replaces rather than extends, so without the
|
||||
/// inherited PATH every child loses node/npm/git.
|
||||
///
|
||||
/// This pure function takes an explicit `is_windows` flag so it can be
|
||||
/// unit-tested cross-host (macOS CI can pass `true` to exercise the Windows
|
||||
/// policy without needing the `cfg!(windows)` target).
|
||||
/// `Command::env("PATH", …)` replaces rather than extends, so a child whose
|
||||
/// composed PATH carries no native entries loses every system binary. On
|
||||
/// Windows that is the steady state — `login_shell_path()` always returns
|
||||
/// `None` because Git Bash returns POSIX colon-delimited paths that poison
|
||||
/// native children. On Unix it is the failure mode: a login shell that exits
|
||||
/// non-zero or prints nothing also yields `None`, and the child is then left
|
||||
/// with only Buzz's managed Node dirs — no `curl`, `sh`, or `tar`, which
|
||||
/// silently breaks every `curl … | bash` install. The inherited PATH is the
|
||||
/// floor under both cases, appended last so managed dirs keep precedence.
|
||||
///
|
||||
/// Rules:
|
||||
/// - Only append when `is_windows` — on Unix the login-shell PATH always covers
|
||||
/// the needed runtimes.
|
||||
/// - Suppress when `had_shell_path` is `true` — if a login-shell PATH was
|
||||
/// supplied it already carries the user's native entries; appending the
|
||||
/// process PATH would double them.
|
||||
/// - Suppress when `has_local_context` is `false` — callers that pass no home
|
||||
/// or exe-parent context must not receive a PATH manufactured from ambient
|
||||
/// process state alone.
|
||||
pub(crate) fn should_use_inherited(
|
||||
had_shell_path: bool,
|
||||
has_local_context: bool,
|
||||
is_windows: bool,
|
||||
) -> bool {
|
||||
is_windows && !had_shell_path && has_local_context
|
||||
pub(crate) fn should_use_inherited(had_shell_path: bool, has_local_context: bool) -> bool {
|
||||
!had_shell_path && has_local_context
|
||||
}
|
||||
|
||||
/// Pure PATH composition kernel shared by the install shell and the runtime/probe paths.
|
||||
@@ -91,10 +86,14 @@ pub(crate) fn compose_path_entries(
|
||||
/// 4. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm)
|
||||
/// 5. exe parent dir — DMG sidecars under `Contents/MacOS/`
|
||||
/// 6. user's login-shell `PATH` — runtimes like node/python from other managers
|
||||
/// 7. Windows only: the current process `PATH` (appended when no login-shell
|
||||
/// PATH exists, because callers use `Command::env("PATH", …)` which
|
||||
/// *replaces* the child's PATH — without this, the child loses node/npm/git
|
||||
/// and every npm `.cmd` shim fails with `'node' is not recognized`)
|
||||
/// 7. the current process `PATH` — appended on every platform when no
|
||||
/// login-shell PATH exists, because callers use `Command::env("PATH", …)`
|
||||
/// which *replaces* the child's PATH. This is the steady state on Windows,
|
||||
/// where `login_shell_path()` always returns `None` and without it the
|
||||
/// child loses node/npm/git and every npm `.cmd` shim fails with
|
||||
/// `'node' is not recognized`; on Unix it is the login-shell-probe failure
|
||||
/// fallback, which keeps `curl`/`sh`/`tar` reachable. See
|
||||
/// [`should_use_inherited`] for the suppression rules.
|
||||
///
|
||||
/// `shell_path` is the raw colon-delimited string from a login shell, so it is
|
||||
/// split into individual entries before joining. Pushing it as a single segment
|
||||
@@ -145,7 +144,7 @@ pub(in crate::managed_agents) fn build_augmented_path(
|
||||
let inherited: Vec<PathBuf> = std::env::var_os("PATH")
|
||||
.map(|p| std::env::split_paths(&p).collect())
|
||||
.unwrap_or_default();
|
||||
let use_inherited = should_use_inherited(had_shell_path, has_local_context, cfg!(windows));
|
||||
let use_inherited = should_use_inherited(had_shell_path, has_local_context);
|
||||
|
||||
let parts = compose_path_entries(managed, login, inherited, use_inherited);
|
||||
if parts.is_empty() {
|
||||
@@ -223,29 +222,86 @@ mod tests {
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn nvm_bin_none_does_not_add_segment() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
// With no shell_path the inherited process PATH is appended last, so
|
||||
// pin it to a sentinel to keep the assertion deterministic.
|
||||
std::env::set_var("PATH", "/sentinel/inherited");
|
||||
|
||||
let result = build_augmented_path(
|
||||
Some(PathBuf::from("/home/user")),
|
||||
Some(PathBuf::from("/usr/local/bin")),
|
||||
None,
|
||||
None,
|
||||
);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let result = result.expect("path");
|
||||
assert!(result.starts_with("/home/user/.local/bin:"), "{result}");
|
||||
assert!(result.ends_with(":/usr/local/bin"), "{result}");
|
||||
assert!(!result.contains(".nvm"), "no nvm segment: {result}");
|
||||
assert!(
|
||||
result.contains(":/usr/local/bin:"),
|
||||
"exe parent must precede the inherited PATH: {result}"
|
||||
);
|
||||
assert!(
|
||||
result.ends_with(":/sentinel/inherited"),
|
||||
"inherited PATH must be appended last when no shell_path: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Unix, supplying a `shell_path` must NOT trigger the Windows process-PATH
|
||||
/// fallback — the output must be byte-identical to what it was before this
|
||||
/// fix.
|
||||
/// On Unix with no login-shell PATH, `build_augmented_path` must fall back to
|
||||
/// the inherited process PATH — otherwise the child gets only Buzz-managed
|
||||
/// dirs and loses every system binary (`curl`, `sh`, `tar`).
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn unix_shell_path_output_unchanged_by_windows_fallback_logic() {
|
||||
fn unix_appends_process_path_when_no_shell_path() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
std::env::set_var("PATH", "/usr/bin:/bin");
|
||||
|
||||
let result = build_augmented_path(Some(PathBuf::from("/home/user")), None, None, None);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let result = result.expect("path must not be None with a home dir");
|
||||
assert!(
|
||||
result.starts_with("/home/user/.local/bin:"),
|
||||
"home/.local/bin must be first: {result}"
|
||||
);
|
||||
assert!(
|
||||
result.ends_with(":/usr/bin:/bin"),
|
||||
"process PATH must be last: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Unix, supplying a `shell_path` must NOT also append the inherited
|
||||
/// process PATH — the login-shell PATH already carries the native entries.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn unix_shell_path_suppresses_inherited_fallback() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
std::env::set_var("PATH", "/should/not/appear");
|
||||
|
||||
let result = build_augmented_path(
|
||||
Some(PathBuf::from("/home/user")),
|
||||
None,
|
||||
Some("/usr/local/bin:/usr/bin:/bin".to_string()),
|
||||
None,
|
||||
);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let result = result.expect("path");
|
||||
assert!(
|
||||
result.ends_with(":/usr/local/bin:/usr/bin:/bin"),
|
||||
@@ -334,8 +390,8 @@ mod tests {
|
||||
// ── Pure policy and composition tests — run on every host ────────────────────
|
||||
//
|
||||
// These test `should_use_inherited` and `compose_path_entries` with explicit
|
||||
// inputs, so they run on macOS/Linux CI and validate the Windows policy
|
||||
// behavior without touching process state or requiring a Windows target.
|
||||
// inputs, so they run on macOS/Linux CI and validate the cross-platform
|
||||
// fallback policy without touching process state or requiring a Windows target.
|
||||
#[cfg(test)]
|
||||
mod compose_tests {
|
||||
use super::{compose_path_entries, is_batch_shim, should_use_inherited};
|
||||
@@ -347,43 +403,37 @@ mod compose_tests {
|
||||
|
||||
// ── should_use_inherited policy matrix ────────────────────────────────────
|
||||
|
||||
/// Windows + no shell path + has local context → must use inherited.
|
||||
/// No shell path + has local context → must use inherited, on every OS.
|
||||
/// This is the steady state on Windows (login_shell_path() is always None)
|
||||
/// and the failure mode on Unix (login shell exited non-zero or printed
|
||||
/// nothing); in both the child would otherwise get no native PATH entries.
|
||||
#[test]
|
||||
fn policy_windows_no_shell_with_context_uses_inherited() {
|
||||
fn policy_no_shell_with_context_uses_inherited() {
|
||||
assert!(
|
||||
should_use_inherited(false, true, true),
|
||||
"Windows, no shell path, has context → must append inherited"
|
||||
should_use_inherited(false, true),
|
||||
"no shell path, has context → must append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows + shell path present → must NOT use inherited (login path covers it).
|
||||
/// Shell path present → must NOT use inherited (login path covers it).
|
||||
#[test]
|
||||
fn policy_windows_shell_path_present_suppresses_inherited() {
|
||||
fn policy_shell_path_present_suppresses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(true, true, true),
|
||||
"Windows, shell path present → must not append inherited"
|
||||
!should_use_inherited(true, true),
|
||||
"shell path present → must not append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows + no local context → must NOT use inherited (no ambient state).
|
||||
/// No local context → must NOT use inherited (no ambient state).
|
||||
#[test]
|
||||
fn policy_windows_no_local_context_suppresses_inherited() {
|
||||
fn policy_no_local_context_suppresses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(false, false, true),
|
||||
"Windows, no local context → must not append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-Windows → never use inherited, regardless of other flags.
|
||||
#[test]
|
||||
fn policy_non_windows_never_uses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(false, true, false),
|
||||
"non-Windows must never append inherited PATH"
|
||||
!should_use_inherited(false, false),
|
||||
"no local context → must not append inherited"
|
||||
);
|
||||
assert!(
|
||||
!should_use_inherited(false, false, false),
|
||||
"non-Windows + no context must never append inherited PATH"
|
||||
!should_use_inherited(true, false),
|
||||
"no local context → must not append inherited even with a shell path"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -498,23 +548,25 @@ mod compose_tests {
|
||||
// compute the same `should_use_inherited` decision for equivalent inputs.
|
||||
// Tests the policy function directly to confirm the wrappers can't drift.
|
||||
|
||||
/// Exhaustive truth-table for `should_use_inherited` — all four input
|
||||
/// combinations that affect real callers. Confirms the policy is correct
|
||||
/// before either wrapper binds to it.
|
||||
/// Exhaustive truth-table for `should_use_inherited` — every input
|
||||
/// combination. Confirms the policy is correct before either wrapper binds
|
||||
/// to it. The rule is OS-independent: the inherited PATH is the floor
|
||||
/// whenever no login-shell PATH was obtained, because the alternative is a
|
||||
/// child with no native binaries at all.
|
||||
#[test]
|
||||
fn should_use_inherited_policy_truth_table() {
|
||||
// (had_shell, has_context, is_windows) → expected
|
||||
// (had_shell, has_context) → expected
|
||||
let cases = [
|
||||
(false, true, true, true), // Windows, no shell, context → USE
|
||||
(true, true, true, false), // Windows, shell present → NO
|
||||
(false, false, true, false), // Windows, no context → NO
|
||||
(false, true, false, false), // non-Windows → NO
|
||||
(false, true, true), // no shell PATH, context → USE (the floor)
|
||||
(true, true, false), // shell PATH present → NO (already covered)
|
||||
(false, false, false), // no context → NO (no ambient-only PATH)
|
||||
(true, false, false), // no context → NO, shell PATH irrelevant
|
||||
];
|
||||
for (had_shell, has_ctx, is_win, expected) in cases {
|
||||
let result = should_use_inherited(had_shell, has_ctx, is_win);
|
||||
for (had_shell, has_ctx, expected) in cases {
|
||||
let result = should_use_inherited(had_shell, has_ctx);
|
||||
assert_eq!(
|
||||
result, expected,
|
||||
"policy mismatch: had_shell={had_shell} has_ctx={has_ctx} is_win={is_win}"
|
||||
"policy mismatch: had_shell={had_shell} has_ctx={has_ctx}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -315,20 +315,16 @@ fn persona_with_provider(
|
||||
// that the old create-time env baking silently blocked.
|
||||
|
||||
use crate::managed_agents::env_vars::{live_persona_env, merged_user_env};
|
||||
use crate::managed_agents::persona_events::persona_snapshot;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// Apply a persona snapshot onto a record, mirroring `create_managed_agent`:
|
||||
/// snapshotted prompt/model/provider/source_version are pinned, with the
|
||||
/// system_prompt unwrapped (the persona always carries one). `env_vars` is
|
||||
/// deliberately NOT touched — it stays agent overrides only.
|
||||
/// links the record to `persona.id`, then delegates the actual snapshot
|
||||
/// (prompt/model/provider/runtime/source_version) to the real production
|
||||
/// `apply_persona_snapshot` — so a change to that function's behavior is
|
||||
/// exercised by these tests instead of silently diverging from it.
|
||||
fn pin_persona(record: &mut ManagedAgentRecord, persona: &crate::managed_agents::AgentDefinition) {
|
||||
let snapshot = persona_snapshot(persona);
|
||||
record.persona_id = Some(persona.id.clone());
|
||||
record.system_prompt = snapshot.system_prompt;
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
crate::managed_agents::persona_events::apply_persona_snapshot(record, persona);
|
||||
}
|
||||
|
||||
fn persona_v(
|
||||
@@ -428,19 +424,31 @@ fn agent_env_overrides_win_over_persona_env_at_spawn() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn orphaned_agent_spawns_from_its_own_overrides() {
|
||||
// Persona deleted: the live merge degrades to the record's own overrides.
|
||||
fn orphaned_agent_refused_at_spawn_boundary() {
|
||||
// Persona deleted: `spawn_agent_child` must refuse before any process
|
||||
// side effect, not silently degrade to the record's stale overrides.
|
||||
// `require_resolved` on the shared resolver is the pure predicate
|
||||
// `spawn_agent_child` checks first — this pins the contract without
|
||||
// needing a real `AppHandle`.
|
||||
let persona = persona_v("p", "prompt", &[("ANTHROPIC_API_KEY", "persona-key")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
record.env_vars = BTreeMap::from([("EXTRA".to_string(), "agent-value".to_string())]);
|
||||
pin_persona(&mut record, &persona);
|
||||
|
||||
let env = spawn_user_env(&record, &[]);
|
||||
assert_eq!(env.get("EXTRA").map(String::as_str), Some("agent-value"));
|
||||
// The persona is absent from the live catalog — same shape restore/start
|
||||
// see when a persona was deleted on another device.
|
||||
let no_personas: &[crate::managed_agents::AgentDefinition] = &[];
|
||||
let error = crate::managed_agents::effective_config::resolve_effective_config(
|
||||
&record,
|
||||
no_personas,
|
||||
&Default::default(),
|
||||
)
|
||||
.require_resolved()
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
env.get("ANTHROPIC_API_KEY"),
|
||||
None,
|
||||
"a deleted persona's env must not linger"
|
||||
error,
|
||||
crate::managed_agents::effective_config::ORPHANED_INSTANCE_ERROR.to_string(),
|
||||
"an orphaned linked instance must be refused, not spawned from its own overrides"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1270,3 +1278,37 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun
|
||||
};
|
||||
crate::managed_agents::ManagedAgentPairRuntime::starting(process)
|
||||
}
|
||||
|
||||
// ── restart_eligible tests ──────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_true_when_non_orphan_has_hash_drift() {
|
||||
assert!(super::restart_eligible(false, true, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_true_when_non_orphan_has_availability_drift() {
|
||||
assert!(super::restart_eligible(false, false, true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_false_when_orphan_has_hash_drift() {
|
||||
// An orphan can never be restarted successfully — spawn refuses it —
|
||||
// so hash drift alone must not surface "Restart required".
|
||||
assert!(!super::restart_eligible(true, true, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_false_when_orphan_has_availability_drift() {
|
||||
assert!(!super::restart_eligible(true, false, true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_false_when_orphan_has_no_drift() {
|
||||
assert!(!super::restart_eligible(true, false, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_eligible_false_when_non_orphan_has_no_drift() {
|
||||
assert!(!super::restart_eligible(false, false, false));
|
||||
}
|
||||
|
||||
@@ -28,23 +28,13 @@
|
||||
use std::hash::{DefaultHasher, Hash, Hasher};
|
||||
|
||||
use super::{
|
||||
effective_config::{resolve_effective_config, EffectiveConfigResult},
|
||||
known_acp_runtime, normalize_agent_args,
|
||||
persona_events::apply_persona_snapshot,
|
||||
persona_events::preview_prospective_persona_snapshot,
|
||||
types::{AgentDefinition, ManagedAgentRecord, TeamRecord},
|
||||
GlobalAgentConfig,
|
||||
};
|
||||
|
||||
/// The prompt a spawn would actually deliver: `Some("")` collapses to `None`
|
||||
/// because an empty `BUZZ_ACP_SYSTEM_PROMPT` is no prompt.
|
||||
///
|
||||
/// The single source of truth for the spawn env write AND the config hash.
|
||||
pub(crate) fn effective_spawn_prompt(record: &ManagedAgentRecord) -> Option<String> {
|
||||
record
|
||||
.system_prompt
|
||||
.clone()
|
||||
.filter(|prompt| !prompt.is_empty())
|
||||
}
|
||||
|
||||
/// Resolve the current instructions for this instance's deployment-time team binding.
|
||||
/// A deleted team deliberately degrades to no team section.
|
||||
pub(crate) fn effective_team_instructions(
|
||||
@@ -75,14 +65,9 @@ pub(crate) fn spawn_config_hash(
|
||||
// restart would actually run. Idempotent, so the spawn-time stamp
|
||||
// (post-snapshot record) and later recomputes (persisted record) agree
|
||||
// when nothing changed. The persona env itself reaches the hash through
|
||||
// `resolve_effective_agent_env` below; `persona_source_version` is set on
|
||||
// the descriptor's layered env below; `persona_source_version` is set on
|
||||
// the clone but is not a hash input.
|
||||
let mut record = record.clone();
|
||||
if let Some(persona_id) = record.persona_id.clone() {
|
||||
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
|
||||
apply_persona_snapshot(&mut record, persona);
|
||||
}
|
||||
}
|
||||
let record = preview_prospective_persona_snapshot(record, personas);
|
||||
let record = &record;
|
||||
|
||||
// Resolve command, args, and env via the single typed descriptor — same path
|
||||
@@ -102,9 +87,6 @@ pub(crate) fn spawn_config_hash(
|
||||
});
|
||||
let runtime_meta = known_acp_runtime(&descriptor.command);
|
||||
|
||||
// `resolve_effective_agent_env` now includes definition env as a floor
|
||||
// layer (below global/persona/agent), mirroring spawn_agent_child exactly.
|
||||
|
||||
let mut hasher = DefaultHasher::new();
|
||||
|
||||
// Harness identity and derivations (live-persona-resolved, like spawn).
|
||||
@@ -124,11 +106,24 @@ pub(crate) fn spawn_config_hash(
|
||||
// resolved: every record spawns on the workspace relay (legacy pins
|
||||
// ignored), so a workspace relay change must trip the badge.
|
||||
crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay).hash(&mut hasher);
|
||||
// Prompt and runtime-layered team instructions use the same resolver as spawn.
|
||||
effective_spawn_prompt(record).hash(&mut hasher);
|
||||
// Team instructions use the same resolver as spawn.
|
||||
effective_team_instructions(record, teams).hash(&mut hasher);
|
||||
record.model.hash(&mut hasher);
|
||||
record.provider.hash(&mut hasher);
|
||||
// Prompt, model, and provider all come from ONE `resolve_effective_config`
|
||||
// call — the SAME resolve `spawn_agent_child` performs for the env write,
|
||||
// so env write and this badge cannot disagree. An orphaned link (missing
|
||||
// definition) hashes as if all three were absent: `spawn_agent_child`
|
||||
// refuses to spawn an orphan regardless, so this is a display-only
|
||||
// convenience, not the spawn gate.
|
||||
let (resolved_prompt, resolved_model, resolved_provider) =
|
||||
match resolve_effective_config(record, personas, global) {
|
||||
EffectiveConfigResult::Resolved(cfg) => {
|
||||
(cfg.system_prompt.value, cfg.model.value, cfg.provider.value)
|
||||
}
|
||||
EffectiveConfigResult::OrphanedInstance { .. } => (None, None, None),
|
||||
};
|
||||
resolved_prompt.hash(&mut hasher);
|
||||
resolved_model.hash(&mut hasher);
|
||||
resolved_provider.hash(&mut hasher);
|
||||
record.auth_tag.hash(&mut hasher);
|
||||
record.respond_to.as_str().hash(&mut hasher);
|
||||
// The allowlist is hashed as the env receives it: spawn sets
|
||||
|
||||
@@ -441,20 +441,125 @@ fn missing_definition_leaves_materialized_runtime_in_hash() {
|
||||
);
|
||||
}
|
||||
|
||||
// ── Global default trips hash for linked inherited agents ─────────────────
|
||||
|
||||
#[test]
|
||||
fn effective_spawn_prompt_matches_hash_semantics() {
|
||||
// The env write and the hash share effective_spawn_prompt — this row
|
||||
// pins the helper's own semantics so a refactor of either caller cannot
|
||||
// silently diverge from the contract.
|
||||
let mut r = record();
|
||||
r.system_prompt = Some(String::new());
|
||||
assert_eq!(
|
||||
effective_spawn_prompt(&r),
|
||||
None,
|
||||
"empty collapses to absent"
|
||||
fn global_model_change_trips_hash_for_linked_inherited_agent() {
|
||||
let mut rec = record();
|
||||
rec.persona_id = Some("p1".into());
|
||||
rec.model = Some("stale-record-model".into());
|
||||
|
||||
let personas = vec![persona("p1", Some("goose"), "prompt")];
|
||||
|
||||
let global_a = GlobalAgentConfig {
|
||||
model: Some("model-a".to_string()),
|
||||
provider: Some("prov-a".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
let global_b = GlobalAgentConfig {
|
||||
model: Some("model-b".to_string()),
|
||||
provider: Some("prov-b".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let hash_a = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_a);
|
||||
let hash_b = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_b);
|
||||
|
||||
assert_ne!(
|
||||
hash_a, hash_b,
|
||||
"changing the global default must trip the hash for a linked inherited agent"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn global_model_change_trips_hash_without_model_env_var() {
|
||||
let mut rec = record();
|
||||
rec.persona_id = Some("p1".into());
|
||||
rec.agent_command = "some-harness-without-model-env".into();
|
||||
|
||||
let personas = vec![{
|
||||
let mut p = persona("p1", None, "prompt");
|
||||
p.model = None;
|
||||
p.provider = None;
|
||||
p
|
||||
}];
|
||||
|
||||
let global_a = GlobalAgentConfig {
|
||||
model: Some("model-a".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
let global_b = GlobalAgentConfig {
|
||||
model: Some("model-b".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let hash_a = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_a);
|
||||
let hash_b = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_b);
|
||||
|
||||
assert_ne!(
|
||||
hash_a, hash_b,
|
||||
"global model change must trip hash even without a model_env_var runtime"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_instance_stale_prompt_bytes_are_inert_at_hash_time() {
|
||||
// Regression for the split-resolve defect: prompt used to be read from
|
||||
// the record's own (possibly Phase-A-snapshot-stale) bytes while
|
||||
// model/provider were resolved live from the definition. A definition
|
||||
// edit landing between a caller's snapshot apply and spawn could hand a
|
||||
// fresh model/provider to a stale prompt, and the hash (which already
|
||||
// resolved model/provider live) would silently agree with a spawn that
|
||||
// wrote the stale prompt. Now both come from one `resolve_effective_config`
|
||||
// call, so a record whose own `system_prompt` bytes disagree with the
|
||||
// live definition must hash exactly as if the record carried the
|
||||
// definition's prompt verbatim — the record's prompt bytes are inert for
|
||||
// a linked instance.
|
||||
let mut rec = record();
|
||||
rec.persona_id = Some("p1".into());
|
||||
rec.system_prompt = Some("stale prompt on record".into());
|
||||
|
||||
let mut matching_bytes = rec.clone();
|
||||
matching_bytes.system_prompt = Some("live prompt".into());
|
||||
|
||||
let personas = [persona("p1", Some("goose"), "live prompt")];
|
||||
|
||||
assert_eq!(
|
||||
spawn_config_hash(
|
||||
&rec,
|
||||
&personas,
|
||||
&[],
|
||||
"wss://ws.example",
|
||||
&Default::default()
|
||||
),
|
||||
spawn_config_hash(
|
||||
&matching_bytes,
|
||||
&personas,
|
||||
&[],
|
||||
"wss://ws.example",
|
||||
&Default::default()
|
||||
),
|
||||
"record's own system_prompt bytes must not affect the hash of a linked instance"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linked_instance_prompt_model_provider_resolve_from_one_call() {
|
||||
// The prompt for a linked instance must track the definition, exactly
|
||||
// like model/provider — a definition prompt edit trips the hash even
|
||||
// though the record's own (stale) system_prompt bytes are unchanged.
|
||||
let mut rec = record();
|
||||
rec.persona_id = Some("p1".into());
|
||||
rec.system_prompt = Some("stale".into());
|
||||
|
||||
let before = [persona("p1", Some("goose"), "old definition prompt")];
|
||||
let after = [persona("p1", Some("goose"), "new definition prompt")];
|
||||
|
||||
assert_ne!(
|
||||
spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()),
|
||||
spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()),
|
||||
"linked instance prompt must resolve from the live definition, not stale record bytes"
|
||||
);
|
||||
r.system_prompt = Some("real".into());
|
||||
assert_eq!(effective_spawn_prompt(&r).as_deref(), Some("real"));
|
||||
}
|
||||
|
||||
// ── I2: definition args and env reach spawn_config_hash ──────────────────────
|
||||
|
||||
@@ -259,13 +259,19 @@ pub struct ManagedAgentRecord {
|
||||
/// Desired LLM model ID. Matches AgentModelInfo.id from discovery.
|
||||
/// The harness re-discovers the correct ACP switching metadata at session
|
||||
/// creation by matching this ID against the fresh session/new response.
|
||||
/// For a linked instance this is a legacy/display snapshot only — spawn
|
||||
/// and deploy resolve the effective model from the definition, never
|
||||
/// from this field (see `effective_config::resolve_effective_config`).
|
||||
/// For a definition-less instance this field is authoritative.
|
||||
#[serde(default)]
|
||||
pub model: Option<String>,
|
||||
/// LLM inference provider snapshotted from the persona at create time
|
||||
/// (e.g. 'databricks', 'anthropic'). Spawn and deploy read this, never the
|
||||
/// live persona — so the agent stays pinned to the provider it was created
|
||||
/// with across restarts. `#[serde(default)]` so pre-existing records
|
||||
/// deserialize as `None` and get backfilled on first load.
|
||||
/// LLM inference provider. For a linked instance this is a legacy/display
|
||||
/// snapshot only — spawn and deploy resolve the effective provider from
|
||||
/// the definition, never from this field (see
|
||||
/// `effective_config::resolve_effective_config`). For a definition-less
|
||||
/// instance this field is authoritative. `#[serde(default)]` so
|
||||
/// pre-existing records deserialize as `None` and get backfilled on
|
||||
/// first load.
|
||||
#[serde(default)]
|
||||
pub provider: Option<String>,
|
||||
/// Content hash of the persona at the time this agent was created — the
|
||||
@@ -390,12 +396,15 @@ pub struct ManagedAgentRecord {
|
||||
/// inference through Buzz's relay-mesh local endpoint; the `model_ref` is
|
||||
/// the served model id to route to. `None` is a normal agent.
|
||||
///
|
||||
/// This is the source of truth for "is this a mesh agent + which model" —
|
||||
/// replacing the old practice of sniffing it back out of `env_vars`
|
||||
/// (`relay_mesh_config`). Spawn-time env vars are *derived from* this, not
|
||||
/// the other way around. `#[serde(default)]` so pre-existing saved records
|
||||
/// deserialize as `None` and are resolved via the env-var fallback until
|
||||
/// they are rewritten with this field.
|
||||
/// Not the source of truth. `provider == "relay-mesh"` is, resolved through
|
||||
/// `effective_config::resolve_effective_config`; spawn-time env vars are
|
||||
/// derived from that resolution. This field is retained solely as a
|
||||
/// backward-compatibility signal for records written before the record had
|
||||
/// a `provider` field, and is consulted only for definition-less records
|
||||
/// that carry no provider — after which the env-var preset is the last
|
||||
/// fallback. A linked instance's marker is never read: its definition is
|
||||
/// authoritative. `#[serde(default)]` so records predating the field
|
||||
/// deserialize as `None`.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub relay_mesh: Option<RelayMeshConfig>,
|
||||
}
|
||||
@@ -482,7 +491,11 @@ pub struct ManagedAgentSummary {
|
||||
pub system_prompt: Option<String>,
|
||||
pub avatar_url: Option<String>,
|
||||
pub model: Option<String>,
|
||||
/// LLM inference provider, from the agent's pinned record snapshot.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub model_source: Option<super::effective_config::ConfigSource>,
|
||||
/// LLM inference provider, resolved the same way as `model`/`model_source`
|
||||
/// (definition → global for linked instances; instance → global for
|
||||
/// definition-less instances). `None` for an orphaned instance.
|
||||
pub provider: Option<String>,
|
||||
/// `true` when the linked persona has been edited since this agent was
|
||||
/// created — the running agent uses the older pinned snapshot. The UI
|
||||
@@ -491,9 +504,11 @@ pub struct ManagedAgentSummary {
|
||||
/// persona is gone, so there is nothing newer to drift toward).
|
||||
pub persona_out_of_date: bool,
|
||||
/// `true` when the agent was created from a persona that no longer exists.
|
||||
/// Distinct from out-of-date: there is no current persona to respawn into,
|
||||
/// so the UI should not prompt a respawn — the pinned snapshot is all the
|
||||
/// config that remains.
|
||||
/// Distinct from out-of-date: there is no current persona to respawn into.
|
||||
/// An orphaned agent also cannot be (re)started — `spawn_agent_child`
|
||||
/// refuses it (see `effective_config::resolve_effective_config`'s
|
||||
/// `OrphanedInstance` arm via `require_resolved`) — so the UI
|
||||
/// should surface that it's stuck, not merely stale.
|
||||
pub persona_orphaned: bool,
|
||||
/// `true` when the running process was spawned with a config that no
|
||||
/// longer matches what a spawn would use today — a plain restart would
|
||||
|
||||
58
desktop/src/features/agents/lib/agentCardModelLabel.test.mjs
Normal file
58
desktop/src/features/agents/lib/agentCardModelLabel.test.mjs
Normal file
@@ -0,0 +1,58 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { resolveAgentCardModelLabel } from "./agentCardModelLabel.ts";
|
||||
|
||||
test("resolveAgentCardModelLabel — unspawned definition with explicit model renders the model, not inherited", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: undefined,
|
||||
personaModel: "gpt-5",
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "gpt-5");
|
||||
});
|
||||
|
||||
test("resolveAgentCardModelLabel — unspawned definition with no model renders the default", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: undefined,
|
||||
personaModel: null,
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "Default model (claude-sonnet)");
|
||||
});
|
||||
|
||||
test("resolveAgentCardModelLabel — linked instance inheriting the global default ignores stale persona.model", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: { modelSource: "global", model: "stale-model" },
|
||||
personaModel: "gpt-5",
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "Default model (claude-sonnet)");
|
||||
});
|
||||
|
||||
test("resolveAgentCardModelLabel — linked instance with no modelSource (legacy/unset) is treated as inherited", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: { modelSource: null, model: "stale-model" },
|
||||
personaModel: "gpt-5",
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "Default model (claude-sonnet)");
|
||||
});
|
||||
|
||||
test("resolveAgentCardModelLabel — linked instance with an explicit resolved model renders that model", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: { modelSource: "definition", model: "gpt-5" },
|
||||
personaModel: "should-not-be-used",
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "gpt-5");
|
||||
});
|
||||
|
||||
test("resolveAgentCardModelLabel — non-inherited agent with a blank resolved model falls back to the default", () => {
|
||||
const label = resolveAgentCardModelLabel({
|
||||
agent: { modelSource: "instance_legacy", model: " " },
|
||||
personaModel: null,
|
||||
defaultModel: "claude-sonnet",
|
||||
});
|
||||
assert.equal(label, "Default model (claude-sonnet)");
|
||||
});
|
||||
44
desktop/src/features/agents/lib/agentCardModelLabel.ts
Normal file
44
desktop/src/features/agents/lib/agentCardModelLabel.ts
Normal file
@@ -0,0 +1,44 @@
|
||||
import { formatAgentModelLabel } from "./formatAgentModelLabel";
|
||||
import type { ManagedAgent } from "@/shared/api/types";
|
||||
|
||||
/**
|
||||
* Model label for the Agents-page card, shared by the definition-grouped
|
||||
* card (`AgentPersonaCard`) and the standalone/legacy card
|
||||
* (`StandaloneAgentCard`).
|
||||
*
|
||||
* A materialized `agent` is authoritative once it exists: its `modelSource`
|
||||
* says whether the *effective* config (from `resolve_effective_config` on
|
||||
* the backend) came from the global default or from an explicit
|
||||
* definition/instance value, so the card never has to re-derive that from
|
||||
* raw model bytes. Absent or `"global"` renders the default-model label;
|
||||
* anything else renders the agent's own resolved model.
|
||||
*
|
||||
* With NO materialized instance yet (a definition that has never been
|
||||
* started), there is no `modelSource` to read — the definition itself is
|
||||
* authoritative, so an explicit `personaModel` must render directly rather
|
||||
* than falling through to "inherited" for lack of an instance.
|
||||
*/
|
||||
export function resolveAgentCardModelLabel(input: {
|
||||
agent: Pick<ManagedAgent, "modelSource" | "model"> | undefined;
|
||||
personaModel: string | null | undefined;
|
||||
defaultModel: string;
|
||||
}): string {
|
||||
if (input.agent) {
|
||||
const isInherited =
|
||||
!input.agent.modelSource || input.agent.modelSource === "global";
|
||||
if (isInherited) {
|
||||
return formatDefaultModelLabel(input.defaultModel);
|
||||
}
|
||||
return input.agent.model?.trim()
|
||||
? formatAgentModelLabel(input.agent.model)
|
||||
: formatDefaultModelLabel(input.defaultModel);
|
||||
}
|
||||
return input.personaModel?.trim()
|
||||
? formatAgentModelLabel(input.personaModel)
|
||||
: formatDefaultModelLabel(input.defaultModel);
|
||||
}
|
||||
|
||||
export function formatDefaultModelLabel(defaultModel: string) {
|
||||
const model = defaultModel.trim();
|
||||
return model ? `Default model (${model})` : "Default model";
|
||||
}
|
||||
@@ -655,30 +655,36 @@ export function AgentInstanceEditDialog({
|
||||
parsedParallelism > 0 && parsedParallelism !== agent.parallelism
|
||||
? parsedParallelism
|
||||
: undefined,
|
||||
// Use tri-state: send null to clear, value to set, omit if unchanged.
|
||||
// Linked instances defer model/provider/systemPrompt to the definition.
|
||||
systemPrompt:
|
||||
(systemPrompt.trim() || null) !== agent.systemPrompt
|
||||
? systemPrompt.trim() || null
|
||||
: undefined,
|
||||
linkedPersona != null
|
||||
? undefined
|
||||
: (systemPrompt.trim() || null) !== agent.systemPrompt
|
||||
? systemPrompt.trim() || null
|
||||
: undefined,
|
||||
model:
|
||||
normalizedModel !== (agent.model ?? null)
|
||||
? normalizedModel
|
||||
: undefined,
|
||||
linkedPersona != null
|
||||
? undefined
|
||||
: normalizedModel !== (agent.model ?? null)
|
||||
? normalizedModel
|
||||
: undefined,
|
||||
// Tri-state provider persistence keyed on providerRuntimeCapability:
|
||||
// "capable" → persist: value if changed, omit if unchanged.
|
||||
// "locked" → clear: send null if provider was set, else omit.
|
||||
// "unknown" → omit always (never send null for a transient state).
|
||||
// llmProviderFieldVisible is for UX visibility only; not used here.
|
||||
provider:
|
||||
providerRuntimeCapability === "capable"
|
||||
? normalizedSubmitProvider !== (agent.provider ?? null)
|
||||
? normalizedSubmitProvider
|
||||
: undefined
|
||||
: providerRuntimeCapability === "locked"
|
||||
? (agent.provider ?? null) !== null
|
||||
? null
|
||||
linkedPersona != null
|
||||
? undefined
|
||||
: providerRuntimeCapability === "capable"
|
||||
? normalizedSubmitProvider !== (agent.provider ?? null)
|
||||
? normalizedSubmitProvider
|
||||
: undefined
|
||||
: undefined, // "unknown" → omit always
|
||||
: providerRuntimeCapability === "locked"
|
||||
? (agent.provider ?? null) !== null
|
||||
? null
|
||||
: undefined
|
||||
: undefined, // "unknown" → omit always
|
||||
envVars: envVarsEqual(submitEnvVars, agent.envVars)
|
||||
? undefined
|
||||
: submitEnvVars,
|
||||
|
||||
@@ -211,29 +211,32 @@ export function EditAgentAdvancedFields({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* System prompt override */}
|
||||
<div className="space-y-1.5">
|
||||
<label
|
||||
className="text-sm font-medium text-foreground"
|
||||
htmlFor="edit-agent-system-prompt"
|
||||
>
|
||||
System prompt override
|
||||
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
|
||||
</label>
|
||||
<div className={PERSONA_FIELD_SHELL_CLASS}>
|
||||
<Textarea
|
||||
className={cn(
|
||||
"min-h-24 resize-y px-3 py-3 leading-5",
|
||||
PERSONA_FIELD_CONTROL_CLASS,
|
||||
)}
|
||||
disabled={disabled}
|
||||
id="edit-agent-system-prompt"
|
||||
onChange={(event) => onSystemPromptChange(event.target.value)}
|
||||
placeholder="Leave blank to send no ACP system prompt"
|
||||
value={systemPrompt}
|
||||
/>
|
||||
{/* System prompt override — hidden for linked instances; the persona
|
||||
definition is authoritative and the backend will reject any override. */}
|
||||
{linkedPersona == null && (
|
||||
<div className="space-y-1.5">
|
||||
<label
|
||||
className="text-sm font-medium text-foreground"
|
||||
htmlFor="edit-agent-system-prompt"
|
||||
>
|
||||
System prompt override
|
||||
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
|
||||
</label>
|
||||
<div className={PERSONA_FIELD_SHELL_CLASS}>
|
||||
<Textarea
|
||||
className={cn(
|
||||
"min-h-24 resize-y px-3 py-3 leading-5",
|
||||
PERSONA_FIELD_CONTROL_CLASS,
|
||||
)}
|
||||
disabled={disabled}
|
||||
id="edit-agent-system-prompt"
|
||||
onChange={(event) => onSystemPromptChange(event.target.value)}
|
||||
placeholder="Leave blank to send no ACP system prompt"
|
||||
value={systemPrompt}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Env vars */}
|
||||
<EnvVarsEditor
|
||||
|
||||
@@ -238,7 +238,12 @@ function AgentSummary({
|
||||
<Badge variant="secondary">{personaLabel}</Badge>
|
||||
) : null}
|
||||
<AgentOriginBadge agent={agent} />
|
||||
{agent.needsRestart ? (
|
||||
{agent.personaOrphaned ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<AlertTriangle className="h-3 w-3" />
|
||||
Configuration missing
|
||||
</Badge>
|
||||
) : agent.needsRestart ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<RefreshCw className="h-3 w-3" />
|
||||
Restart required
|
||||
@@ -261,7 +266,12 @@ function AgentSummary({
|
||||
<span>Remote deployment</span>
|
||||
)}
|
||||
</div>
|
||||
{agent.needsRestart ? (
|
||||
{agent.personaOrphaned ? (
|
||||
<p className="mt-1.5 text-xs text-amber-600 dark:text-amber-400">
|
||||
This agent's configuration is missing — it may still be syncing or
|
||||
was deleted on another device.
|
||||
</p>
|
||||
) : agent.needsRestart ? (
|
||||
<p className="mt-1.5 text-xs text-amber-600 dark:text-amber-400">
|
||||
Configuration changed since this agent started. Restart to apply
|
||||
it.
|
||||
|
||||
@@ -1,163 +0,0 @@
|
||||
import { isCatalogPersonaSelected } from "@/features/agents/lib/catalog";
|
||||
import { ProfileAvatar } from "@/features/profile/ui/ProfileAvatar";
|
||||
import type { AgentPersona } from "@/shared/api/types";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { promptPreview } from "@/shared/lib/promptPreview";
|
||||
import {
|
||||
Sheet,
|
||||
SheetContent,
|
||||
SheetDescription,
|
||||
SheetHeader,
|
||||
SheetTitle,
|
||||
} from "@/shared/ui/sheet";
|
||||
|
||||
import { PersonaCatalogSelectionBadge } from "./PersonaCatalogSelectionBadge";
|
||||
import {
|
||||
getPersonaCatalogDetailSelectionCopy,
|
||||
getPersonaCatalogSelectionAriaLabel,
|
||||
} from "./personaLibraryCopy";
|
||||
|
||||
type PersonaCatalogDetailsSheetProps = {
|
||||
feedbackErrorMessage: string | null;
|
||||
feedbackNoticeMessage: string | null;
|
||||
isPending: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
onTogglePersona: (persona: AgentPersona) => void;
|
||||
open: boolean;
|
||||
persona: AgentPersona | null;
|
||||
};
|
||||
|
||||
export function PersonaCatalogDetailsSheet({
|
||||
feedbackErrorMessage,
|
||||
feedbackNoticeMessage,
|
||||
isPending,
|
||||
onOpenChange,
|
||||
onTogglePersona,
|
||||
open,
|
||||
persona,
|
||||
}: PersonaCatalogDetailsSheetProps) {
|
||||
const preview = persona ? promptPreview(persona.systemPrompt) : "";
|
||||
const isSelected = persona ? isCatalogPersonaSelected(persona) : false;
|
||||
const selectionCopy = getPersonaCatalogDetailSelectionCopy(isSelected);
|
||||
|
||||
return (
|
||||
<Sheet onOpenChange={onOpenChange} open={open}>
|
||||
<SheetContent
|
||||
className="flex w-full flex-col gap-0 overflow-hidden bg-background p-0 sm:max-w-xl"
|
||||
data-testid="persona-catalog-details-sheet"
|
||||
>
|
||||
{persona ? (
|
||||
<>
|
||||
<SheetHeader className="relative z-10 space-y-4 bg-background/25 px-6 py-6 pr-16 text-left shadow-[0_4px_24px_rgba(0,0,0,0.06)] backdrop-blur-xl supports-[backdrop-filter]:bg-background/20 dark:shadow-[0_4px_24px_rgba(0,0,0,0.25)]">
|
||||
<div className="flex items-start gap-3">
|
||||
<ProfileAvatar
|
||||
avatarUrl={persona.avatarUrl}
|
||||
className="h-12 w-12 text-sm"
|
||||
label={persona.displayName}
|
||||
/>
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<SheetTitle className="truncate text-xl">
|
||||
{persona.displayName}
|
||||
</SheetTitle>
|
||||
<PersonaCatalogSelectionBadge isActive={isSelected} />
|
||||
</div>
|
||||
<SheetDescription className="mt-2">
|
||||
{preview || "No summary available."}
|
||||
</SheetDescription>
|
||||
</div>
|
||||
</div>
|
||||
</SheetHeader>
|
||||
<div className="flex-1 space-y-6 overflow-y-auto px-6 py-6">
|
||||
<button
|
||||
aria-label={getPersonaCatalogSelectionAriaLabel(
|
||||
persona.displayName,
|
||||
isSelected,
|
||||
)}
|
||||
aria-pressed={isSelected}
|
||||
className={cn(
|
||||
"w-full rounded-xl border p-4 text-left transition-[background-color,border-color,box-shadow] focus:outline-hidden focus-visible:ring-2 focus-visible:ring-primary/40 focus-visible:ring-offset-2",
|
||||
isSelected
|
||||
? "border-primary bg-primary/10 text-foreground"
|
||||
: "border-border/80 bg-background/60 text-muted-foreground hover:bg-accent hover:text-accent-foreground",
|
||||
isPending && "cursor-not-allowed opacity-70",
|
||||
)}
|
||||
data-state={isSelected ? "selected" : "available"}
|
||||
data-testid={`persona-catalog-detail-selection-target-${persona.id}`}
|
||||
disabled={isPending}
|
||||
onClick={() => {
|
||||
onTogglePersona(persona);
|
||||
}}
|
||||
type="button"
|
||||
>
|
||||
<div className="flex items-start justify-between gap-4">
|
||||
<div>
|
||||
<p
|
||||
className="text-sm font-semibold tracking-tight"
|
||||
data-testid="persona-catalog-detail-selection-title"
|
||||
>
|
||||
{selectionCopy.title}
|
||||
</p>
|
||||
<p
|
||||
className="mt-1 text-sm text-muted-foreground"
|
||||
data-testid="persona-catalog-detail-selection-description"
|
||||
>
|
||||
{selectionCopy.description}
|
||||
</p>
|
||||
</div>
|
||||
<PersonaCatalogSelectionBadge isActive={isSelected} />
|
||||
</div>
|
||||
</button>
|
||||
|
||||
{feedbackNoticeMessage ? (
|
||||
<p className="rounded-2xl border border-primary/20 bg-primary/10 px-4 py-3 text-sm text-primary">
|
||||
{feedbackNoticeMessage}
|
||||
</p>
|
||||
) : null}
|
||||
|
||||
{feedbackErrorMessage ? (
|
||||
<p className="rounded-2xl border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive">
|
||||
{feedbackErrorMessage}
|
||||
</p>
|
||||
) : null}
|
||||
|
||||
<div className="grid gap-3 sm:grid-cols-2">
|
||||
<div className="rounded-xl border border-border/70 bg-card/70 p-4">
|
||||
<p className="text-2xs font-semibold uppercase tracking-wide text-muted-foreground">
|
||||
Type
|
||||
</p>
|
||||
<p className="mt-2 text-sm font-medium">Built-in agent</p>
|
||||
</div>
|
||||
<div className="rounded-xl border border-border/70 bg-card/70 p-4">
|
||||
<p className="text-2xs font-semibold uppercase tracking-wide text-muted-foreground">
|
||||
Preferred model
|
||||
</p>
|
||||
<p className="mt-2 text-sm font-medium">
|
||||
{persona.model ?? "Use app default"}
|
||||
</p>
|
||||
</div>
|
||||
<div className="rounded-xl border border-border/70 bg-card/70 p-4 sm:col-span-2">
|
||||
<p className="text-2xs font-semibold uppercase tracking-wide text-muted-foreground">
|
||||
Preferred runtime
|
||||
</p>
|
||||
<p className="mt-2 text-sm font-medium">
|
||||
{persona.runtime ?? "Use app default"}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="rounded-xl border border-border/70 bg-card/70 p-4">
|
||||
<p className="text-2xs font-semibold uppercase tracking-wide text-muted-foreground">
|
||||
System prompt
|
||||
</p>
|
||||
<pre className="mt-3 whitespace-pre-wrap break-words font-sans text-sm leading-6 text-foreground">
|
||||
{persona.systemPrompt}
|
||||
</pre>
|
||||
</div>
|
||||
</div>
|
||||
</>
|
||||
) : null}
|
||||
</SheetContent>
|
||||
</Sheet>
|
||||
);
|
||||
}
|
||||
@@ -1,166 +0,0 @@
|
||||
import { isCatalogPersonaSelected } from "@/features/agents/lib/catalog";
|
||||
import type { AgentPersona } from "@/shared/api/types";
|
||||
import { promptPreview } from "@/shared/lib/promptPreview";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { Card } from "@/shared/ui/card";
|
||||
import { Skeleton } from "@/shared/ui/skeleton";
|
||||
import { PersonaCatalogSelectionBadge } from "./PersonaCatalogSelectionBadge";
|
||||
import { PersonaIdentity } from "./PersonaIdentity";
|
||||
import {
|
||||
getPersonaCatalogSelectionActionCopy,
|
||||
getPersonaCatalogSelectionAriaLabel,
|
||||
personaCatalogCopy,
|
||||
} from "./personaLibraryCopy";
|
||||
|
||||
type PersonaCatalogSectionProps = {
|
||||
emptyDescription?: string;
|
||||
emptyTitle?: string;
|
||||
error: Error | null;
|
||||
isLoading: boolean;
|
||||
isPending: boolean;
|
||||
onTogglePersona: (persona: AgentPersona) => void;
|
||||
onViewDetails: (persona: AgentPersona) => void;
|
||||
personas: AgentPersona[];
|
||||
showHeader?: boolean;
|
||||
};
|
||||
|
||||
export function PersonaCatalogSection({
|
||||
emptyDescription = personaCatalogCopy.emptyCatalogDescription,
|
||||
emptyTitle = personaCatalogCopy.emptyCatalogTitle,
|
||||
error,
|
||||
isLoading,
|
||||
isPending,
|
||||
onTogglePersona,
|
||||
onViewDetails,
|
||||
personas,
|
||||
showHeader = true,
|
||||
}: PersonaCatalogSectionProps) {
|
||||
return (
|
||||
<section className="space-y-4" data-testid="agents-persona-catalog">
|
||||
{showHeader ? (
|
||||
<div>
|
||||
<h3 className="text-sm font-semibold tracking-tight">
|
||||
{personaCatalogCopy.title}
|
||||
</h3>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{personaCatalogCopy.description}
|
||||
</p>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{isLoading ? (
|
||||
<div className="grid gap-3 md:grid-cols-3 xl:grid-cols-4">
|
||||
{["first", "second", "third", "fourth"].map((key) => (
|
||||
<Card className="p-3" key={key}>
|
||||
<div className="flex items-center gap-2.5">
|
||||
<Skeleton className="h-8 w-8 rounded-lg" />
|
||||
<div className="space-y-2">
|
||||
<Skeleton className="h-4 w-24" />
|
||||
<Skeleton className="h-3 w-20" />
|
||||
</div>
|
||||
</div>
|
||||
</Card>
|
||||
))}
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{!isLoading && personas.length > 0 ? (
|
||||
<div className="grid gap-3 md:grid-cols-3 xl:grid-cols-4">
|
||||
{personas.map((persona) => {
|
||||
const preview = promptPreview(persona.systemPrompt);
|
||||
const isSelected = isCatalogPersonaSelected(persona);
|
||||
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
"group relative flex flex-col gap-4 rounded-xl border p-3 shadow-xs transition-[background-color,border-color,box-shadow]",
|
||||
isPending
|
||||
? "cursor-not-allowed opacity-70"
|
||||
: "cursor-pointer",
|
||||
isSelected
|
||||
? "border-primary bg-primary/10 text-foreground"
|
||||
: "border-border/80 bg-background/60 hover:bg-accent hover:text-accent-foreground",
|
||||
)}
|
||||
data-testid={`persona-catalog-card-${persona.id}`}
|
||||
data-state={isSelected ? "selected" : "available"}
|
||||
key={persona.id}
|
||||
>
|
||||
<button
|
||||
aria-label={getPersonaCatalogSelectionAriaLabel(
|
||||
persona.displayName,
|
||||
isSelected,
|
||||
)}
|
||||
aria-pressed={isSelected}
|
||||
className="absolute inset-0 z-0 rounded-xl focus:outline-hidden focus-visible:ring-2 focus-visible:ring-primary/40 focus-visible:ring-offset-2"
|
||||
data-testid={`persona-catalog-card-target-${persona.id}`}
|
||||
disabled={isPending}
|
||||
onClick={() => {
|
||||
onTogglePersona(persona);
|
||||
}}
|
||||
type="button"
|
||||
/>
|
||||
|
||||
<div className="pointer-events-none relative z-10 flex h-full flex-col gap-4">
|
||||
<div className="flex items-start justify-between gap-3">
|
||||
<PersonaIdentity
|
||||
className="min-w-0 flex-1"
|
||||
persona={persona}
|
||||
showBuiltInBadge={false}
|
||||
showPromptTooltip={false}
|
||||
/>
|
||||
|
||||
<PersonaCatalogSelectionBadge isActive={isSelected} />
|
||||
</div>
|
||||
|
||||
<p className="min-h-12 text-xs leading-5 text-muted-foreground">
|
||||
{preview}
|
||||
</p>
|
||||
|
||||
<div
|
||||
className={cn(
|
||||
"mt-auto flex items-center justify-between gap-3 border-t pt-3",
|
||||
isSelected ? "border-primary/20" : "border-border/60",
|
||||
)}
|
||||
>
|
||||
<Button
|
||||
className="pointer-events-auto"
|
||||
data-testid={`persona-catalog-details-${persona.id}`}
|
||||
onClick={() => {
|
||||
onViewDetails(persona);
|
||||
}}
|
||||
size="sm"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
{personaCatalogCopy.detailsAction}
|
||||
</Button>
|
||||
|
||||
<span className="text-xs font-medium text-muted-foreground">
|
||||
{getPersonaCatalogSelectionActionCopy(isSelected)}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{!isLoading && personas.length === 0 ? (
|
||||
<div className="rounded-xl border border-dashed border-border/70 px-6 py-10 text-center">
|
||||
<p className="text-sm font-semibold tracking-tight">{emptyTitle}</p>
|
||||
<p className="mt-2 text-sm text-muted-foreground">
|
||||
{emptyDescription}
|
||||
</p>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{error ? (
|
||||
<p className="rounded-2xl border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive">
|
||||
{error.message}
|
||||
</p>
|
||||
) : null}
|
||||
</section>
|
||||
);
|
||||
}
|
||||
@@ -1,29 +0,0 @@
|
||||
import { Check } from "lucide-react";
|
||||
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
|
||||
import { personaCatalogCopy } from "./personaLibraryCopy";
|
||||
|
||||
type PersonaCatalogSelectionBadgeProps = {
|
||||
isActive: boolean;
|
||||
};
|
||||
|
||||
export function PersonaCatalogSelectionBadge({
|
||||
isActive,
|
||||
}: PersonaCatalogSelectionBadgeProps) {
|
||||
return (
|
||||
<span
|
||||
className={cn(
|
||||
"inline-flex items-center gap-1 whitespace-nowrap rounded-full px-2.5 py-1 text-2xs font-semibold uppercase tracking-[0.14em]",
|
||||
isActive
|
||||
? "bg-primary text-primary-foreground shadow-xs"
|
||||
: "border border-border/70 bg-background/85 text-muted-foreground",
|
||||
)}
|
||||
>
|
||||
{isActive ? <Check className="h-4 w-4" /> : null}
|
||||
{isActive
|
||||
? personaCatalogCopy.selectedState
|
||||
: personaCatalogCopy.availableState}
|
||||
</span>
|
||||
);
|
||||
}
|
||||
@@ -1,79 +0,0 @@
|
||||
import * as React from "react";
|
||||
|
||||
import { isCatalogPersonaSelected } from "@/features/agents/lib/catalog";
|
||||
import type { AgentPersona } from "@/shared/api/types";
|
||||
import { useFeedbackToasts } from "@/shared/hooks/useToastEffect";
|
||||
|
||||
import { PersonaCatalogDetailsSheet } from "./PersonaCatalogDetailsSheet";
|
||||
import { PersonaCatalogSection } from "./PersonaCatalogSection";
|
||||
|
||||
type PersonaCatalogSurfaceProps = {
|
||||
error: Error | null;
|
||||
feedbackErrorMessage: string | null;
|
||||
feedbackNoticeMessage: string | null;
|
||||
isLoading: boolean;
|
||||
isPending: boolean;
|
||||
onClearFeedback: () => void;
|
||||
onSelectPersona: (persona: AgentPersona, active: boolean) => void;
|
||||
personas: AgentPersona[];
|
||||
showHeader?: boolean;
|
||||
};
|
||||
|
||||
export function PersonaCatalogSurface({
|
||||
error,
|
||||
feedbackErrorMessage,
|
||||
feedbackNoticeMessage,
|
||||
isLoading,
|
||||
isPending,
|
||||
onClearFeedback,
|
||||
onSelectPersona,
|
||||
personas,
|
||||
showHeader = true,
|
||||
}: PersonaCatalogSurfaceProps) {
|
||||
const [detailPersonaId, setDetailPersonaId] = React.useState<string | null>(
|
||||
null,
|
||||
);
|
||||
const detailPersona = React.useMemo(
|
||||
() =>
|
||||
detailPersonaId
|
||||
? (personas.find((persona) => persona.id === detailPersonaId) ?? null)
|
||||
: null,
|
||||
[detailPersonaId, personas],
|
||||
);
|
||||
const handleTogglePersona = (persona: AgentPersona) => {
|
||||
onSelectPersona(persona, !isCatalogPersonaSelected(persona));
|
||||
};
|
||||
|
||||
useFeedbackToasts(feedbackNoticeMessage, feedbackErrorMessage);
|
||||
|
||||
return (
|
||||
<>
|
||||
<PersonaCatalogSection
|
||||
error={error}
|
||||
isLoading={isLoading}
|
||||
isPending={isPending}
|
||||
onTogglePersona={handleTogglePersona}
|
||||
onViewDetails={(persona) => {
|
||||
onClearFeedback();
|
||||
setDetailPersonaId(persona.id);
|
||||
}}
|
||||
personas={personas}
|
||||
showHeader={showHeader}
|
||||
/>
|
||||
|
||||
<PersonaCatalogDetailsSheet
|
||||
feedbackErrorMessage={feedbackErrorMessage}
|
||||
feedbackNoticeMessage={feedbackNoticeMessage}
|
||||
isPending={isPending}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) {
|
||||
setDetailPersonaId(null);
|
||||
}
|
||||
}}
|
||||
onTogglePersona={handleTogglePersona}
|
||||
open={detailPersonaId !== null}
|
||||
persona={detailPersona}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -1,62 +0,0 @@
|
||||
import { Info } from "lucide-react";
|
||||
|
||||
import { ProfileAvatar } from "@/features/profile/ui/ProfileAvatar";
|
||||
import type { AgentPersona } from "@/shared/api/types";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { promptPreview } from "@/shared/lib/promptPreview";
|
||||
import { Badge } from "@/shared/ui/badge";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip";
|
||||
|
||||
type PersonaIdentityProps = {
|
||||
className?: string;
|
||||
persona: AgentPersona;
|
||||
showBuiltInBadge?: boolean;
|
||||
showPromptTooltip?: boolean;
|
||||
};
|
||||
|
||||
export function PersonaIdentity({
|
||||
className,
|
||||
persona,
|
||||
showBuiltInBadge = persona.isBuiltIn,
|
||||
showPromptTooltip = true,
|
||||
}: PersonaIdentityProps) {
|
||||
const preview = promptPreview(persona.systemPrompt);
|
||||
|
||||
return (
|
||||
<div className={cn("min-w-0 flex-1", className)}>
|
||||
<div className="flex min-w-0 items-center gap-2.5">
|
||||
<ProfileAvatar
|
||||
avatarUrl={persona.avatarUrl}
|
||||
className="h-8 w-8 rounded-lg text-xs"
|
||||
label={persona.displayName}
|
||||
/>
|
||||
<div className="min-w-0">
|
||||
<div className="flex min-w-0 items-center gap-2">
|
||||
<p className="truncate text-sm font-semibold tracking-tight">
|
||||
{persona.displayName}
|
||||
</p>
|
||||
{showBuiltInBadge ? (
|
||||
<Badge variant="secondary">Built-in</Badge>
|
||||
) : null}
|
||||
{showPromptTooltip && preview ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<button
|
||||
aria-label="View system prompt"
|
||||
className="flex h-4 w-4 shrink-0 items-center justify-center text-muted-foreground transition-colors hover:text-foreground"
|
||||
type="button"
|
||||
>
|
||||
<Info className="h-4 w-4" />
|
||||
</button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" className="max-w-xs">
|
||||
<p>{preview}</p>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -1,57 +0,0 @@
|
||||
import { Upload } from "lucide-react";
|
||||
|
||||
import { Button } from "@/shared/ui/button";
|
||||
|
||||
import { CreateNewButton } from "./CreateNewButton";
|
||||
import { personaLibraryCopy } from "./personaLibraryCopy";
|
||||
|
||||
type PersonaLibraryEntryPointsProps = {
|
||||
canChooseCatalog?: boolean;
|
||||
isPending: boolean;
|
||||
layout: "header" | "empty";
|
||||
onCreate: () => void;
|
||||
onChooseCatalog?: () => void;
|
||||
onImport?: () => void;
|
||||
};
|
||||
|
||||
export function PersonaLibraryEntryPoints({
|
||||
canChooseCatalog = false,
|
||||
isPending,
|
||||
layout,
|
||||
onCreate,
|
||||
onChooseCatalog,
|
||||
onImport,
|
||||
}: PersonaLibraryEntryPointsProps) {
|
||||
const isHeader = layout === "header";
|
||||
const chooseVariant = isHeader ? "outline" : "default";
|
||||
const createVariant = isHeader ? "default" : "outline";
|
||||
|
||||
return (
|
||||
<>
|
||||
{canChooseCatalog && onChooseCatalog ? (
|
||||
<Button
|
||||
data-testid="open-persona-catalog"
|
||||
disabled={isPending}
|
||||
onClick={onChooseCatalog}
|
||||
size="sm"
|
||||
type="button"
|
||||
variant={chooseVariant}
|
||||
>
|
||||
{personaLibraryCopy.chooseFromCatalog}
|
||||
</Button>
|
||||
) : null}
|
||||
<CreateNewButton
|
||||
disabled={isPending}
|
||||
label={personaLibraryCopy.createNew}
|
||||
onClick={onCreate}
|
||||
variant={createVariant}
|
||||
/>
|
||||
{!isHeader && onImport ? (
|
||||
<Button onClick={onImport} size="sm" type="button" variant="outline">
|
||||
<Upload className="h-4 w-4" />
|
||||
{personaLibraryCopy.import}
|
||||
</Button>
|
||||
) : null}
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -1,7 +1,12 @@
|
||||
import * as React from "react";
|
||||
import { ChevronDown, ChevronRight, RefreshCw } from "lucide-react";
|
||||
import {
|
||||
AlertTriangle,
|
||||
ChevronDown,
|
||||
ChevronRight,
|
||||
RefreshCw,
|
||||
} from "lucide-react";
|
||||
|
||||
import { formatAgentModelLabel } from "@/features/agents/lib/formatAgentModelLabel";
|
||||
import { resolveAgentCardModelLabel } from "@/features/agents/lib/agentCardModelLabel";
|
||||
import { friendlyAgentLastError } from "@/features/agents/lib/friendlyAgentLastError";
|
||||
import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions";
|
||||
import { useUserProfileQuery } from "@/features/profile/hooks";
|
||||
@@ -261,10 +266,11 @@ function AgentPersonaCard({
|
||||
onStartPersona: (persona: AgentPersona) => void;
|
||||
}) {
|
||||
const title = persona.displayName;
|
||||
const explicitModel = agent?.model ?? persona.model;
|
||||
const modelLabel = explicitModel?.trim()
|
||||
? formatAgentModelLabel(explicitModel)
|
||||
: formatDefaultModelLabel(defaultModel);
|
||||
const modelLabel = resolveAgentCardModelLabel({
|
||||
agent,
|
||||
personaModel: persona.model,
|
||||
defaultModel,
|
||||
});
|
||||
const isActive = agent ? isManagedAgentActive(agent) : false;
|
||||
const profileQuery = useUserProfileQuery(agent?.pubkey);
|
||||
const avatarUrl = agent
|
||||
@@ -322,7 +328,12 @@ function AgentPersonaCard({
|
||||
onOpenPersonaProfile(persona);
|
||||
}}
|
||||
statusBadge={
|
||||
agent?.needsRestart ? (
|
||||
agent?.personaOrphaned ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<AlertTriangle className="h-3 w-3" />
|
||||
Configuration missing
|
||||
</Badge>
|
||||
) : agent?.needsRestart ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<RefreshCw className="h-3 w-3" />
|
||||
Restart required
|
||||
@@ -380,11 +391,11 @@ function StandaloneAgentCard({
|
||||
avatarUrl={profileQuery.data?.avatarUrl}
|
||||
dataTestId={`managed-agent-${agent.pubkey}`}
|
||||
label={title}
|
||||
modelLabel={
|
||||
agent.model?.trim()
|
||||
? formatAgentModelLabel(agent.model)
|
||||
: formatDefaultModelLabel(defaultModel)
|
||||
}
|
||||
modelLabel={resolveAgentCardModelLabel({
|
||||
agent,
|
||||
personaModel: null,
|
||||
defaultModel,
|
||||
})}
|
||||
onClick={() => {
|
||||
onOpenAgentProfile(
|
||||
agent.pubkey,
|
||||
@@ -392,7 +403,12 @@ function StandaloneAgentCard({
|
||||
);
|
||||
}}
|
||||
statusBadge={
|
||||
agent.needsRestart ? (
|
||||
agent.personaOrphaned ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<AlertTriangle className="h-3 w-3" />
|
||||
Configuration missing
|
||||
</Badge>
|
||||
) : agent.needsRestart ? (
|
||||
<Badge className="gap-1" variant="warning">
|
||||
<RefreshCw className="h-3 w-3" />
|
||||
Restart required
|
||||
@@ -403,11 +419,6 @@ function StandaloneAgentCard({
|
||||
);
|
||||
}
|
||||
|
||||
function formatDefaultModelLabel(defaultModel: string) {
|
||||
const model = defaultModel.trim();
|
||||
return model ? `Default model (${model})` : "Default model";
|
||||
}
|
||||
|
||||
function firstAvatarUrl(
|
||||
...candidates: Array<string | null | undefined>
|
||||
): string | null {
|
||||
|
||||
@@ -109,24 +109,27 @@ export function hasMissingRequiredEnvKey(
|
||||
/**
|
||||
* Resolve the provider and env-vars to PERSIST on Save.
|
||||
*
|
||||
* The spawn path reads ONLY the record snapshot (`record.provider`/
|
||||
* `record.env_vars`), never the live persona, and the record is authoritative:
|
||||
* `env_vars` is the complete pinned map (persona env snapshotted at create,
|
||||
* already merged UNDER the agent's own overrides), and the provider field is
|
||||
* user-editable in the dialog even while inheriting. So the local edit state IS
|
||||
* the record's own value and is honored verbatim in the normal case.
|
||||
* This dialog edits an agent INSTANCE's own fields (`record.provider`/
|
||||
* `record.env_vars`), which remain the source of truth for a definition-less
|
||||
* (legacy) instance and for any local/display state before the next resolve.
|
||||
* For a LINKED instance, spawn/deploy resolve the effective provider/model
|
||||
* from the definition, never from the record's own `provider`/`model` bytes
|
||||
* (see `effective_config::resolve_effective_config` on the desktop backend) —
|
||||
* so persisting `record.provider` here only matters for definition-less
|
||||
* instances and for keeping the record's display/legacy fields consistent;
|
||||
* it does not change what a linked instance spawns with.
|
||||
*
|
||||
* The ONE exception is the inherit-TRANSITION-from-a-harness-pin: a previously
|
||||
* harness-pinned agent (e.g. Claude — `agent.agentCommandOverride != null` at
|
||||
* dialog open) has its `provider` cleared and carries no persona credential,
|
||||
* then the user checks "Inherit runtime from template" for a provider-backed
|
||||
* persona (e.g. buzz-agent/Anthropic). Persisting the local (empty) provider +
|
||||
* credential-less env would save an agent that fails readiness on next start.
|
||||
* The one case this function still has to handle carefully is the
|
||||
* inherit-TRANSITION-from-a-harness-pin: a previously harness-pinned agent
|
||||
* (e.g. Claude — `agent.agentCommandOverride != null` at dialog open) has its
|
||||
* `provider` cleared and carries no persona credential, then the user checks
|
||||
* "Inherit runtime from template" for a provider-backed persona (e.g.
|
||||
* buzz-agent/Anthropic). Persisting the local (empty) provider + credential-
|
||||
* less env would leave a definition-less instance's record fields blank.
|
||||
* Only in that case — inheriting AND the local provider is empty AND the agent
|
||||
* was harness-pinned at open — do we substitute the persona snapshot: the
|
||||
* persona's provider and the persona-layered env (`{ ...personaEnv,
|
||||
* ...agentEnv }`, agent layer wins to mirror spawn-time layering), matching
|
||||
* create-time record pinning.
|
||||
* ...agentEnv }`, agent layer wins to mirror spawn-time layering).
|
||||
*
|
||||
* A NON-empty local provider while inheriting (e.g. an Anthropic-persona agent
|
||||
* the user re-points to Databricks) is a deliberate edit and passes through
|
||||
@@ -135,11 +138,11 @@ export function hasMissingRequiredEnvKey(
|
||||
* MODEL follows the same transition rule. buzz-agent/goose readiness requires a
|
||||
* model, but a Claude-pinned agent's record often carries no model (Claude
|
||||
* resolves its own). On the inherit-transition to a provider-backed persona with
|
||||
* a set `persona.model`, persisting the empty local model would save an agent
|
||||
* that inherits the provider + credentials but no model and fails readiness on
|
||||
* next start — so we substitute `personaModel` in that same case. A non-empty
|
||||
* local model (deliberate pick) always passes through; an empty local model in
|
||||
* steady state stays empty (runtime default), same authoritative logic.
|
||||
* a set `persona.model`, persisting the empty local model would save a
|
||||
* record-level model gap — so we substitute `personaModel` in that same case. A
|
||||
* non-empty local model (deliberate pick) always passes through; an empty local
|
||||
* model in steady state stays empty (runtime default), same authoritative
|
||||
* logic.
|
||||
*
|
||||
* An EMPTY local provider while inheriting on an agent that was ALREADY
|
||||
* inheriting at open (`agentWasHarnessPinned` false) is ALSO authoritative: the
|
||||
@@ -148,8 +151,6 @@ export function hasMissingRequiredEnvKey(
|
||||
* resurrect the persona provider — otherwise the Default option could never
|
||||
* actually clear an inherited agent's provider override.
|
||||
*
|
||||
* The result is the SAME effective value the required-credential gate
|
||||
* validates, so the gate, the submitted record, and the spawn snapshot agree.
|
||||
* Provider is normalized: trimmed, empty → `null`.
|
||||
*/
|
||||
export function resolveInheritedRuntimeSubmission(input: {
|
||||
|
||||
@@ -134,6 +134,7 @@ export type RawManagedAgent = {
|
||||
system_prompt: string | null;
|
||||
avatar_url?: string | null;
|
||||
model: string | null;
|
||||
model_source?: ManagedAgent["modelSource"];
|
||||
provider: string | null;
|
||||
persona_out_of_date: boolean;
|
||||
persona_orphaned: boolean;
|
||||
@@ -710,6 +711,8 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent {
|
||||
systemPrompt: agent.system_prompt,
|
||||
avatarUrl: agent.avatar_url ?? null,
|
||||
model: agent.model,
|
||||
modelSource: agent.model_source ?? null,
|
||||
// Fallbacks for pre-feature mocks/fixtures. Real records always carry them.
|
||||
provider: agent.provider ?? null,
|
||||
personaOutOfDate: agent.persona_out_of_date ?? false,
|
||||
personaOrphaned: agent.persona_orphaned ?? false,
|
||||
|
||||
@@ -369,6 +369,7 @@ export type ManagedAgent = {
|
||||
systemPrompt: string | null;
|
||||
avatarUrl: string | null;
|
||||
model: string | null;
|
||||
modelSource: "definition" | "global" | "instance_legacy" | null;
|
||||
/** LLM inference provider, from the agent's pinned record snapshot. */
|
||||
provider: string | null;
|
||||
/**
|
||||
|
||||
@@ -7,6 +7,7 @@ import 'package:nostr/nostr.dart' as nostr;
|
||||
import '../../shared/auth/auth.dart';
|
||||
import '../../shared/deeplink/deep_link.dart';
|
||||
import '../../shared/relay/relay_session.dart';
|
||||
import '../../shared/relay/relay_validation.dart';
|
||||
|
||||
final inviteJoinHttpClientProvider = Provider<http.Client>((ref) {
|
||||
final client = http.Client();
|
||||
@@ -68,6 +69,7 @@ class InviteJoinNotifier extends Notifier<InviteJoinState> {
|
||||
InviteJoinState build() => const InviteJoinState();
|
||||
|
||||
Future<void> prepare(InviteDeepLink invite) async {
|
||||
validateInviteRelayUri(Uri.parse(invite.relayUrl));
|
||||
final communities = await ref.read(communityListProvider.future);
|
||||
final existing = _existingCommunity(communities, invite.relayUrl);
|
||||
if (existing != null) {
|
||||
@@ -121,22 +123,25 @@ class InviteJoinNotifier extends Notifier<InviteJoinState> {
|
||||
if (invite.policyReceipt != null)
|
||||
'policy_receipt': invite.policyReceipt,
|
||||
});
|
||||
final relayUri = Uri.parse(invite.relayUrl);
|
||||
validateInviteRelayUri(relayUri);
|
||||
final url = _claimUrlFromRelay(invite.relayUrl);
|
||||
final response = await ref
|
||||
final request = http.Request('POST', Uri.parse(url))
|
||||
..followRedirects = false
|
||||
..headers.addAll({
|
||||
'Authorization': buildNip98AuthHeader(
|
||||
method: 'POST',
|
||||
url: url,
|
||||
bodyBytes: utf8.encode(body),
|
||||
nsec: keys.nsec,
|
||||
),
|
||||
'Content-Type': 'application/json',
|
||||
})
|
||||
..body = body;
|
||||
final streamedResponse = await ref
|
||||
.read(inviteJoinHttpClientProvider)
|
||||
.post(
|
||||
Uri.parse(url),
|
||||
headers: {
|
||||
'Authorization': buildNip98AuthHeader(
|
||||
method: 'POST',
|
||||
url: url,
|
||||
bodyBytes: utf8.encode(body),
|
||||
nsec: keys.nsec,
|
||||
),
|
||||
'Content-Type': 'application/json',
|
||||
},
|
||||
body: body,
|
||||
);
|
||||
.send(request);
|
||||
final response = await http.Response.fromStream(streamedResponse);
|
||||
final decoded = jsonDecode(response.body.isEmpty ? '{}' : response.body);
|
||||
if (response.statusCode < 200 || response.statusCode >= 300) {
|
||||
final message = decoded is Map && decoded['error'] is String
|
||||
|
||||
@@ -7,6 +7,8 @@
|
||||
/// half-formed target.
|
||||
library;
|
||||
|
||||
import '../relay/relay_validation.dart';
|
||||
|
||||
/// A parsed deep link supported by the app.
|
||||
sealed class BuzzDeepLink {
|
||||
const BuzzDeepLink();
|
||||
@@ -107,8 +109,9 @@ MessageDeepLink? parseMessageDeepLink(Uri uri) {
|
||||
///
|
||||
/// Accepted forms:
|
||||
/// - `https://<relay>/invite/<code>` -> `wss://<relay>` + code
|
||||
/// - `http://<relay>/invite/<code>` -> `ws://<relay>` + code
|
||||
/// - `buzz://join?relay=<ws(s)://relay>&code=<code>` -> relay + code
|
||||
/// - `http://localhost/invite/<code>` -> `ws://localhost` + code in debug builds
|
||||
/// - `buzz://join?relay=<wss://relay>&code=<code>` -> relay + code
|
||||
/// - `buzz://join?relay=<ws://localhost>&code=<code>` -> local relay in debug
|
||||
///
|
||||
/// Rejects credentials, fragments, missing params, nested relay credentials, and
|
||||
/// non-invite paths so scanners do not accidentally treat arbitrary URLs as
|
||||
@@ -131,6 +134,11 @@ InviteDeepLink? parseInviteDeepLink(Uri uri) {
|
||||
relayUri.hasFragment) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
validateInviteRelayUri(relayUri);
|
||||
} on FormatException {
|
||||
return null;
|
||||
}
|
||||
final normalizedRelay = Uri(
|
||||
scheme: relayUri.scheme,
|
||||
host: relayUri.host,
|
||||
@@ -155,6 +163,16 @@ InviteDeepLink? parseInviteDeepLink(Uri uri) {
|
||||
return null;
|
||||
}
|
||||
final relayScheme = uri.scheme == 'https' ? 'wss' : 'ws';
|
||||
final relayUri = Uri(
|
||||
scheme: relayScheme,
|
||||
host: uri.host,
|
||||
port: uri.hasPort ? uri.port : null,
|
||||
);
|
||||
try {
|
||||
validateInviteRelayUri(relayUri);
|
||||
} on FormatException {
|
||||
return null;
|
||||
}
|
||||
final relay = Uri(
|
||||
scheme: relayScheme,
|
||||
host: uri.host,
|
||||
|
||||
164
mobile/lib/shared/relay/relay_validation.dart
Normal file
164
mobile/lib/shared/relay/relay_validation.dart
Normal file
@@ -0,0 +1,164 @@
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:flutter/foundation.dart';
|
||||
|
||||
/// Validates an untrusted relay origin before the mobile app connects to it.
|
||||
///
|
||||
/// Production relay URLs must use TLS and may not contain local or otherwise
|
||||
/// non-public IP literals. Debug builds retain plaintext localhost support for
|
||||
/// local development, but other non-public destinations remain blocked.
|
||||
void validateInviteRelayUri(
|
||||
Uri uri, {
|
||||
bool allowInsecureLocalhost = kDebugMode,
|
||||
}) {
|
||||
if (uri.host.isEmpty ||
|
||||
uri.userInfo.isNotEmpty ||
|
||||
uri.hasFragment ||
|
||||
(uri.path.isNotEmpty && uri.path != '/') ||
|
||||
uri.hasQuery) {
|
||||
throw const FormatException('Relay URL must be an origin');
|
||||
}
|
||||
|
||||
if (uri.scheme != 'ws' && uri.scheme != 'wss') {
|
||||
throw FormatException('Invalid relay URL scheme: ${uri.scheme}');
|
||||
}
|
||||
|
||||
final host = uri.host.toLowerCase();
|
||||
final isLocalhost = host == 'localhost' || host.endsWith('.localhost');
|
||||
if (uri.scheme != 'wss' && !(allowInsecureLocalhost && isLocalhost)) {
|
||||
throw const FormatException('Relay URL must use WSS');
|
||||
}
|
||||
if (isLocalhost) {
|
||||
if (!allowInsecureLocalhost) {
|
||||
throw const FormatException('Relay URL cannot target localhost');
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
final address = InternetAddress.tryParse(host);
|
||||
if (_looksLikeNumericAddress(host)) {
|
||||
// Only canonical dotted-decimal IPv4 is accepted. Ambiguous legacy forms
|
||||
// such as a single integer, hexadecimal, shortened components, or leading
|
||||
// zeroes have varied interpretation across URI/network stacks.
|
||||
final ipv4Parts = host.split('.');
|
||||
final hasLeadingZero = ipv4Parts.any(
|
||||
(part) => part.length > 1 && part.startsWith('0'),
|
||||
);
|
||||
if (address == null ||
|
||||
address.type != InternetAddressType.IPv4 ||
|
||||
address.address != host ||
|
||||
ipv4Parts.length != 4 ||
|
||||
hasLeadingZero) {
|
||||
throw const FormatException('Relay URL contains an invalid IP address');
|
||||
}
|
||||
}
|
||||
if (address != null) {
|
||||
if (!_isPublicAddress(address)) {
|
||||
throw const FormatException(
|
||||
'Relay URL cannot target non-public network addresses',
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// DNS hostnames are allowed here. Preventing a hostname from resolving or
|
||||
// rebinding to a non-public address requires connect-time resolution and
|
||||
// address pinning in the networking layer.
|
||||
}
|
||||
|
||||
bool _looksLikeNumericAddress(String host) {
|
||||
if (RegExp(r'^\d+$').hasMatch(host) ||
|
||||
RegExp(r'^0x[0-9a-f]+$', caseSensitive: false).hasMatch(host)) {
|
||||
return true;
|
||||
}
|
||||
final parts = host.split('.');
|
||||
return parts.length > 1 &&
|
||||
parts.every(
|
||||
(part) =>
|
||||
RegExp(r'^\d+$').hasMatch(part) ||
|
||||
RegExp(r'^0x[0-9a-f]+$', caseSensitive: false).hasMatch(part),
|
||||
);
|
||||
}
|
||||
|
||||
bool _isPublicAddress(InternetAddress address) {
|
||||
final bytes = address.rawAddress;
|
||||
if (address.type == InternetAddressType.IPv4) {
|
||||
return !_isNonPublicIpv4(bytes);
|
||||
}
|
||||
if (address.type != InternetAddressType.IPv6 || bytes.length != 16) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// IPv4-compatible and IPv4-mapped IPv6 addresses inherit the embedded
|
||||
// IPv4 address's classification.
|
||||
final firstTenZero = bytes.take(10).every((byte) => byte == 0);
|
||||
if (firstTenZero &&
|
||||
((bytes[10] == 0 && bytes[11] == 0) ||
|
||||
(bytes[10] == 0xff && bytes[11] == 0xff))) {
|
||||
return !_isNonPublicIpv4(bytes.sublist(12));
|
||||
}
|
||||
|
||||
// Accept only globally reachable IPv6 literals. Global unicast space is
|
||||
// 2000::/3, with special-purpose exclusions and narrow globally reachable
|
||||
// exceptions tracked by IANA (reconciled 2026-07-26):
|
||||
// https://www.iana.org/assignments/iana-ipv6-special-registry/
|
||||
if (!_hasPrefix(bytes, [0x20], 3)) return false;
|
||||
|
||||
// 2001::/23 is reserved for IETF protocol assignments. Only these entries
|
||||
// are currently classified by IANA as globally reachable.
|
||||
if (_hasPrefix(bytes, [0x20, 0x01, 0x00], 23) &&
|
||||
!_isGloballyReachableIetfAssignment(bytes)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Documentation and transition prefixes are not public relay destinations.
|
||||
if (_hasPrefix(bytes, [0x20, 0x01, 0x0d, 0xb8], 32)) return false;
|
||||
if (_hasPrefix(bytes, [0x20, 0x02], 16)) return false;
|
||||
if (_hasPrefix(bytes, [0x3f, 0xff, 0x00], 20)) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
bool _isGloballyReachableIetfAssignment(List<int> bytes) {
|
||||
final isGlobalAnycast =
|
||||
_hasPrefix(bytes, [0x20, 0x01, 0x00, 0x01, 0, 0, 0, 0], 64) &&
|
||||
bytes.sublist(8, 15).every((byte) => byte == 0) &&
|
||||
bytes[15] >= 1 &&
|
||||
bytes[15] <= 3;
|
||||
return isGlobalAnycast ||
|
||||
_hasPrefix(bytes, [0x20, 0x01, 0x00, 0x03], 32) ||
|
||||
_hasPrefix(bytes, [0x20, 0x01, 0x00, 0x04, 0x01, 0x12], 48) ||
|
||||
_hasPrefix(bytes, [0x20, 0x01, 0x00, 0x20], 28) ||
|
||||
_hasPrefix(bytes, [0x20, 0x01, 0x00, 0x30], 28);
|
||||
}
|
||||
|
||||
bool _hasPrefix(List<int> address, List<int> prefix, int prefixLength) {
|
||||
final wholeBytes = prefixLength ~/ 8;
|
||||
for (var i = 0; i < wholeBytes; i++) {
|
||||
if (address[i] != prefix[i]) return false;
|
||||
}
|
||||
final remainingBits = prefixLength % 8;
|
||||
if (remainingBits == 0) return true;
|
||||
final mask = (0xff << (8 - remainingBits)) & 0xff;
|
||||
return (address[wholeBytes] & mask) == (prefix[wholeBytes] & mask);
|
||||
}
|
||||
|
||||
bool _isNonPublicIpv4(List<int> bytes) {
|
||||
if (bytes.length != 4) return true;
|
||||
final a = bytes[0];
|
||||
final b = bytes[1];
|
||||
final c = bytes[2];
|
||||
|
||||
return a == 0 ||
|
||||
a == 10 ||
|
||||
a == 127 ||
|
||||
(a == 100 && b >= 64 && b <= 127) ||
|
||||
(a == 169 && b == 254) ||
|
||||
(a == 172 && b >= 16 && b <= 31) ||
|
||||
(a == 192 && b == 0 && c == 0) ||
|
||||
(a == 192 && b == 0 && c == 2) ||
|
||||
(a == 192 && b == 168) ||
|
||||
(a == 198 && (b == 18 || b == 19)) ||
|
||||
(a == 198 && b == 51 && c == 100) ||
|
||||
(a == 203 && b == 0 && c == 113) ||
|
||||
a >= 224;
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart' as http_testing;
|
||||
import 'package:nostr/nostr.dart' as nostr;
|
||||
import 'package:pointycastle/digests/sha256.dart';
|
||||
|
||||
import 'package:buzz/features/invites/invite_join_provider.dart';
|
||||
import 'package:buzz/shared/auth/auth.dart';
|
||||
@@ -128,7 +129,26 @@ void main() {
|
||||
'https://relay.example.com/api/invites/claim',
|
||||
);
|
||||
expect(capturedRequest!.body, jsonEncode({'code': 'code'}));
|
||||
expect(capturedRequest!.headers['Authorization'], startsWith('Nostr '));
|
||||
final authHeader = capturedRequest!.headers['Authorization'];
|
||||
expect(authHeader, startsWith('Nostr '));
|
||||
expect(capturedRequest!.followRedirects, isFalse);
|
||||
final encoded = authHeader!.substring('Nostr '.length);
|
||||
final authEvent =
|
||||
jsonDecode(
|
||||
utf8.decode(base64Url.decode(base64Url.normalize(encoded))),
|
||||
)
|
||||
as Map<String, dynamic>;
|
||||
final tags = (authEvent['tags'] as List<dynamic>)
|
||||
.map((tag) => (tag as List<dynamic>).cast<String>())
|
||||
.toList();
|
||||
final payloadHash = SHA256Digest()
|
||||
.process(capturedRequest!.bodyBytes)
|
||||
.map((byte) => byte.toRadixString(16).padLeft(2, '0'))
|
||||
.join();
|
||||
expect(authEvent['kind'], 27235);
|
||||
expect(tags, contains(equals(['u', capturedRequest!.url.toString()])));
|
||||
expect(tags, contains(equals(['method', 'POST'])));
|
||||
expect(tags, contains(equals(['payload', payloadHash])));
|
||||
expect(auth.authenticatedCommunities, hasLength(1));
|
||||
expect(
|
||||
auth.authenticatedCommunities.single.relayUrl,
|
||||
@@ -139,6 +159,21 @@ void main() {
|
||||
},
|
||||
);
|
||||
|
||||
test('prepare rejects an unsafe relay before showing confirmation', () async {
|
||||
final container = ProviderContainer();
|
||||
addTearDown(container.dispose);
|
||||
|
||||
await expectLater(
|
||||
container
|
||||
.read(inviteJoinProvider.notifier)
|
||||
.prepare(
|
||||
const InviteDeepLink(relayUrl: 'wss://127.0.0.1', code: 'code'),
|
||||
),
|
||||
throwsFormatException,
|
||||
);
|
||||
expect(container.read(inviteJoinProvider).status, InviteJoinStatus.idle);
|
||||
});
|
||||
|
||||
test('join_policy_required requires a fresh link and cannot retry', () async {
|
||||
final keys = nostr.Keys.generate();
|
||||
var attempts = 0;
|
||||
|
||||
@@ -104,6 +104,23 @@ void _inviteTests() {
|
||||
);
|
||||
});
|
||||
|
||||
test('normalizes trailing slash in buzz join handoff', () {
|
||||
final link = parseInviteDeepLink(
|
||||
Uri.parse(
|
||||
'buzz://join?relay=wss%3A%2F%2Frelay.example.com%2F&code=abc123',
|
||||
),
|
||||
);
|
||||
expect(link?.relayUrl, 'wss://relay.example.com');
|
||||
});
|
||||
|
||||
test('rejects plaintext public buzz join handoff', () {
|
||||
final relay = Uri.encodeQueryComponent('ws://relay.example.com');
|
||||
expect(
|
||||
parseInviteDeepLink(Uri.parse('buzz://join?relay=$relay&code=abc')),
|
||||
isNull,
|
||||
);
|
||||
});
|
||||
|
||||
test('preserves policy receipt in buzz join handoff', () {
|
||||
final link = parseInviteDeepLink(
|
||||
Uri.parse(
|
||||
@@ -177,6 +194,18 @@ void _inviteTests() {
|
||||
);
|
||||
});
|
||||
|
||||
test('rejects non-public invite relay destinations', () {
|
||||
for (final url in [
|
||||
'https://127.0.0.1/invite/abc',
|
||||
'https://169.254.169.254/invite/abc',
|
||||
'https://192.168.1.1/invite/abc',
|
||||
'https://[::1]/invite/abc',
|
||||
'https://[::ffff:127.0.0.1]/invite/abc',
|
||||
]) {
|
||||
expect(parseInviteDeepLink(Uri.parse(url)), isNull, reason: url);
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects buzz join with dangerous relay schemes', () {
|
||||
// The `relay=` param is an allowlist — only `ws` / `wss` are safe to
|
||||
// hand to a Nostr relay session. Anything else must be dropped by the
|
||||
|
||||
161
mobile/test/shared/relay/relay_validation_test.dart
Normal file
161
mobile/test/shared/relay/relay_validation_test.dart
Normal file
@@ -0,0 +1,161 @@
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
import 'package:buzz/shared/relay/relay_validation.dart';
|
||||
|
||||
void main() {
|
||||
group('validateInviteRelayUri', () {
|
||||
test('accepts public secure relay origins', () {
|
||||
for (final url in [
|
||||
'wss://relay.example.com',
|
||||
'wss://relay.example.com/',
|
||||
'wss://relay.example.com:8443',
|
||||
'wss://8.8.8.8',
|
||||
'wss://[2001:4860:4860::8888]',
|
||||
'wss://[2001:1::1]',
|
||||
'wss://[2001:1::2]',
|
||||
'wss://[2001:1::3]',
|
||||
'wss://[2001:3::1]',
|
||||
'wss://[2001:4:112::1]',
|
||||
'wss://[2001:20::1]',
|
||||
'wss://[2001:30::1]',
|
||||
'wss://[2001:200::1]',
|
||||
'wss://[2620:4f:8000::1]',
|
||||
'wss://[3fff:1000::1]',
|
||||
'wss://[2606:4700:4700::1111]',
|
||||
]) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse(url),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
returnsNormally,
|
||||
reason: url,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('allows plaintext localhost only when explicitly enabled', () {
|
||||
for (final url in ['ws://localhost:3000', 'ws://relay.localhost:3000']) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse(url),
|
||||
allowInsecureLocalhost: true,
|
||||
),
|
||||
returnsNormally,
|
||||
reason: url,
|
||||
);
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse(url),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
reason: url,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects plaintext public relays', () {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse('ws://relay.example.com'),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
);
|
||||
});
|
||||
|
||||
test('rejects non-public IPv4 literals', () {
|
||||
for (final host in [
|
||||
'0.0.0.0',
|
||||
'10.0.0.1',
|
||||
'100.64.0.1',
|
||||
'127.0.0.1',
|
||||
'169.254.169.254',
|
||||
'172.16.0.1',
|
||||
'192.168.0.1',
|
||||
'198.18.0.1',
|
||||
'224.0.0.1',
|
||||
]) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse('wss://$host'),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
reason: host,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects non-public IPv6 literals and mapped IPv4', () {
|
||||
for (final host in [
|
||||
'[::]',
|
||||
'[::1]',
|
||||
'[::ffff:127.0.0.1]',
|
||||
'[::ffff:169.254.169.254]',
|
||||
'[::ffff:a9fe:a9fe]',
|
||||
'[64:ff9b::a9fe:a9fe]',
|
||||
'[100::1]',
|
||||
'[100:0:0:1::1]',
|
||||
'[2001::1]',
|
||||
'[2001:1::4]',
|
||||
'[2001:2::1]',
|
||||
'[2001:4:111::1]',
|
||||
'[2001:10::1]',
|
||||
'[2001:40::1]',
|
||||
'[2001:db8::1]',
|
||||
'[2002:a9fe:a9fe::1]',
|
||||
'[3fff::1]',
|
||||
'[3fff:fff::1]',
|
||||
'[5f00::1]',
|
||||
'[fc00::1]',
|
||||
'[fd00::1]',
|
||||
'[fe80::1]',
|
||||
'[fec0::1]',
|
||||
'[ff02::1]',
|
||||
]) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse('wss://$host'),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
reason: host,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects legacy numeric IPv4 spellings', () {
|
||||
for (final host in ['2130706433', '0x7f000001', '0177.0.0.1', '127.1']) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse('wss://$host'),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
reason: host,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects non-origin URLs', () {
|
||||
for (final url in [
|
||||
'https://relay.example.com',
|
||||
'wss://user@relay.example.com',
|
||||
'wss://relay.example.com/path',
|
||||
'wss://relay.example.com?query=x',
|
||||
'wss://relay.example.com#fragment',
|
||||
]) {
|
||||
expect(
|
||||
() => validateInviteRelayUri(
|
||||
Uri.parse(url),
|
||||
allowInsecureLocalhost: false,
|
||||
),
|
||||
throwsFormatException,
|
||||
reason: url,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user