Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions crates/tui/src/fleet/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,48 @@ mod tests {
);
}

#[test]
fn worker_command_threads_custom_profile_provider_name() {
let mut task = task("format");
task.worker.as_mut().unwrap().agent_profile = Some("local".to_string());

let mut profile = agent_profile("local", "formatter", "Keep edits tight.");
profile.profile.provider = Some("lm-studio".to_string());
profile.profile.model = Some("qwen-2.5-7b".to_string());

let cmd = build_worker_exec_command_with_profiles(
"codewhale",
&task,
&FleetExecConfig::default(),
Some("deepseek-v4-pro"),
&[profile],
)
.unwrap();

let provider_idx = cmd
.args
.iter()
.position(|a| a == "--provider")
.expect("--provider must be threaded for a custom provider pin");
assert_eq!(
cmd.args.get(provider_idx + 1).map(String::as_str),
Some("lm-studio"),
"{:?}",
cmd.args
);
let model_idx = cmd
.args
.iter()
.position(|a| a == "--model")
.expect("--model must be present");
assert_eq!(
cmd.args.get(model_idx + 1).map(String::as_str),
Some("qwen-2.5-7b"),
"{:?}",
cmd.args
);
}

/// A worker with no profile-bound provider preserves today's behavior: the
/// run-level model on `--model`, and NO `--provider` (the worker keeps its
/// own session default). Guards against regressing profile-less workers.
Expand Down
51 changes: 38 additions & 13 deletions crates/tui/src/fleet/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,21 @@ fn validate_agent_profile_model_hint(path: &Path, value: Option<&str>) -> Result
Ok(())
}

/// Validate an explicit `provider` field against the known `ApiProvider`
/// vocabulary (#4093). This is the ONLY place a profile's provider is
/// established — a name that doesn't parse is rejected outright rather than
/// silently ignored or guessed from `model` (EPIC #2608: explicit config
/// only, never a model-id prefix/substring sniff).
/// Validate an explicit `provider` field as a safe provider id (#4093).
///
/// Built-in providers are accepted by the runtime vocabulary, and user-named
/// OpenAI-compatible custom providers are accepted as simple tokens so the
/// launch path can resolve `[providers.<id>]` from the session config (#3965).
/// This field remains the ONLY place a profile's provider is established:
/// callers never infer it from `model` (EPIC #2608).
fn validate_agent_profile_provider(path: &Path, value: &str) -> Result<()> {
if crate::config::ApiProvider::parse(value).is_none() {
let trimmed = value.trim();
if trimmed.is_empty() {
bail!("agent profile {} provider cannot be empty", path.display());
}
if trimmed != value || !trimmed.chars().all(is_agent_profile_token_char) {
bail!(
"agent profile {} provider {value:?} is not a recognized provider id",
"agent profile {} provider must be a simple provider id",
path.display()
);
}
Expand Down Expand Up @@ -1015,17 +1021,36 @@ model = "deepseek/deepseek-v4-pro"
}

#[test]
fn agent_profile_loader_rejects_unrecognized_provider_name() {
// EPIC #2608 explicit-config-only mandate: an unrecognized provider
// name is rejected outright at load time — never silently ignored,
// and never guessed from `model`.
fn agent_profile_loader_accepts_custom_provider_name() {
// #3965: LM Studio and other user-named OpenAI-compatible providers
// are resolved from `[providers.<id>]` at launch time, so the profile
// loader must preserve the safe id instead of requiring a built-in.
let tmp = TempDir::new().unwrap();
write_profile(
tmp.path(),
"reviewer.toml",
r#"
name = "reviewer"
provider = "lm-studio"
model = "qwen-2.5-7b"
"#,
);

let profiles = load_agent_profiles_from_dir(tmp.path()).expect("profile loads");

assert_eq!(profiles[0].profile.provider.as_deref(), Some("lm-studio"));
assert_eq!(profiles[0].profile.model.as_deref(), Some("qwen-2.5-7b"));
}

#[test]
fn agent_profile_loader_rejects_malformed_provider_name() {
let tmp = TempDir::new().unwrap();
write_profile(
tmp.path(),
"reviewer.toml",
r#"
name = "reviewer"
provider = "not-a-real-provider"
provider = "lm studio"
model = "some-model"
"#,
);
Expand All @@ -1035,7 +1060,7 @@ model = "some-model"
.to_string();

assert!(
err.contains("not a recognized provider"),
err.contains("provider must be a simple provider id"),
"unexpected error: {err}"
);
}
Expand Down
130 changes: 100 additions & 30 deletions crates/tui/src/fleet/worker_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ pub fn fleet_task_to_worker_spec_with_profiles(
&loadout,
model_source,
);
requested_runtime.provider =
explicit_fleet_provider(agent_profile).map(|provider| provider.as_str().to_string());
requested_runtime.provider = explicit_fleet_provider_id(agent_profile);
requested_runtime.reasoning_effort = effective_fleet_reasoning_effort(agent_profile);
if let Some(agent_profile) = agent_profile
&& let Some(profile_depth) = agent_profile.profile.delegation.max_spawn_depth
Expand Down Expand Up @@ -159,8 +158,13 @@ pub(crate) fn resolve_fleet_route(
// Resolve within the profile's own explicit provider scope when it has
// one (#4093); otherwise fall back to the existing default scope (mirrors
// `ProviderKind::default()`). The resolver is fully offline/hermetic and
// never reads secrets, env, or config.
let provider = effective_fleet_provider(agent_profile);
// never reads secrets, env, or config. User-named custom providers need the
// session Config to resolve their table, so this receipt path omits the
// route instead of fabricating DeepSeek details for them (#3965).
let provider = match explicit_fleet_provider_id(agent_profile).as_deref() {
Some(provider_id) => ApiProvider::parse(provider_id)?,
None => ApiProvider::Deepseek,
};
let candidate = resolve_route_candidate(provider, model_selector, None, None, None).ok()?;

Some(FleetResolvedRoute {
Expand Down Expand Up @@ -427,33 +431,42 @@ fn effective_fleet_model_with_source(
(run_model.to_string(), "run.model")
}

/// The provider this task's resolved route should use (#4093).
/// The provider id a resolved agent profile EXPLICITLY pins, if any (#4093).
///
/// This preserves user-named OpenAI-compatible custom providers such as
/// `lm-studio` instead of collapsing them through [`ApiProvider`]. Runtime
/// launch paths can set `Config.provider` to this exact id so the normal config
/// resolver finds `[providers.<id>]` (#3965).
///
/// Only an explicit `provider` field on the resolved agent profile ever
/// selects a provider here — EPIC #2608 forbids inferring one by sniffing a
/// substring/prefix out of `model`. Absent an explicit pin (no profile, or a
/// profile that never named a provider), the worker profile carries no
/// provider authority and resolution falls back to the existing default
/// scope, unchanged from prior behavior.
fn effective_fleet_provider(agent_profile: Option<&AgentProfile>) -> ApiProvider {
explicit_fleet_provider(agent_profile).unwrap_or(ApiProvider::Deepseek)
/// Returns `None` when no profile names a provider — never invents a DeepSeek
/// default — so launch paths can omit `--provider` and leave profile-less
/// workers on their own session default. EPIC #2608: never inferred from
/// `model`.
pub(crate) fn explicit_fleet_provider_id(agent_profile: Option<&AgentProfile>) -> Option<String> {
agent_profile
.and_then(|profile| profile.profile.provider.as_deref())
.map(str::trim)
.filter(|provider| !provider.is_empty())
.map(str::to_string)
}

/// The provider a resolved agent profile EXPLICITLY pins, if any (#4093).
/// The built-in provider a resolved agent profile EXPLICITLY pins, if any (#4093).
///
/// Unlike [`effective_fleet_provider`], this returns `None` (never the DeepSeek
/// default) when no profile names a provider, so the launch path can leave
/// `--provider` off the worker argv and preserve today's behavior for
/// profile-less / provider-less workers (they resolve their provider from
/// their own session default). EPIC #2608: never inferred from `model`.
/// This returns `None` (never the DeepSeek default) when no profile names a
/// provider, so call sites can leave `--provider` off the worker argv and
/// preserve today's behavior for profile-less / provider-less workers (they
/// resolve their provider from their own session default). EPIC #2608: never
/// inferred from `model`.
///
/// `pub(crate)` so the interactive-TUI in-process spawn path
/// (`tools::subagent`) resolves the pinned provider from the SAME
/// explicit-only source as the headless `codewhale exec` launch route (#4193),
/// instead of re-deriving it and risking a second, divergent policy.
/// instead of re-deriving it and risking a second, divergent policy. User-named
/// custom providers intentionally return `None` here; launch paths that can
/// carry strings should use [`explicit_fleet_provider_id`].
pub(crate) fn explicit_fleet_provider(agent_profile: Option<&AgentProfile>) -> Option<ApiProvider> {
agent_profile
.and_then(|profile| profile.profile.provider.as_deref())
explicit_fleet_provider_id(agent_profile)
.as_deref()
.and_then(ApiProvider::parse)
}

Expand Down Expand Up @@ -488,18 +501,18 @@ pub(crate) fn fleet_worker_launch_reasoning_effort(
/// This is the launch-side twin of [`resolve_fleet_route`] (the receipt): both
/// read the worker's model from the same task/profile/run precedence
/// ([`effective_fleet_model`]) and the provider from the same explicit-only
/// source ([`explicit_fleet_provider`]), so a worker whose profile is pinned to
/// provider B launches on provider B even when the parent session is on
/// source ([`explicit_fleet_provider_id`]), so a worker whose profile is pinned
/// to provider B launches on provider B even when the parent session is on
/// provider A.
///
/// - `model`: never empty in practice — falls back to `run_model` when neither
/// the task nor the profile pins a model, matching pre-#4093 dispatch.
/// - `provider`: `Some(canonical_id)` ONLY when the resolved agent profile
/// - `provider`: `Some(provider_id)` ONLY when the resolved agent profile
/// explicitly pins a provider. `None` means "no provider authority" — the
/// caller omits `--provider` and the worker keeps its own session default,
/// preserving today's behavior for profile-less workers. The id is
/// [`ApiProvider::as_str`], which round-trips through `ApiProvider::parse` on
/// the worker's `--provider` flag.
/// preserving today's behavior for profile-less workers. Built-ins use their
/// canonical ids; user-named custom providers preserve the profile's id so
/// `codewhale exec --provider <id>` can resolve `[providers.<id>]`.
pub(crate) fn fleet_worker_launch_route(
task_spec: &FleetTaskSpec,
agent_profiles: &[AgentProfile],
Expand All @@ -510,8 +523,7 @@ pub(crate) fn fleet_worker_launch_route(
.flatten();
let worker_profile = task_spec.worker.as_ref();
let model = effective_fleet_model(run_model, worker_profile, agent_profile);
let provider =
explicit_fleet_provider(agent_profile).map(|provider| provider.as_str().to_string());
let provider = explicit_fleet_provider_id(agent_profile);
(model, provider)
}

Expand Down Expand Up @@ -1428,6 +1440,36 @@ mod tests {
assert_ne!(route.provider_id, "deepseek");
}

#[test]
fn resolve_fleet_route_omits_custom_provider_without_config_snapshot() {
let mut profile = agent_profile(
"local",
"scout",
None,
codewhale_config::FleetLoadout::Inherit,
);
profile.profile.model = Some("qwen-2.5-7b".to_string());
profile.profile.provider = Some("lm-studio".to_string());
let task = fleet_task(
"custom-receipt",
Some(worker_profile(
Some("local"),
None,
None,
None,
None,
vec![],
)),
);

let route = resolve_fleet_route(&task, &[profile], Some("deepseek-v4-pro"));

assert!(
route.is_none(),
"custom providers need the session Config snapshot; do not fabricate a DeepSeek receipt"
);
}

#[test]
fn fleet_worker_launch_route_is_explicit_provider_only() {
// The LAUNCH resolver (twin of the receipt) must emit a provider ONLY
Expand Down Expand Up @@ -1466,6 +1508,34 @@ mod tests {
Some("high")
);

// 1b) User-named OpenAI-compatible providers are launchable too: keep
// the exact provider id so `codewhale exec --provider lm-studio`
// can resolve `[providers.lm-studio]` from config (#3965).
let mut custom = agent_profile(
"local",
"scout",
None,
codewhale_config::FleetLoadout::Inherit,
);
custom.profile.model = Some("qwen-2.5-7b".to_string());
custom.profile.provider = Some("lm-studio".to_string());
let custom_task = fleet_task(
"launch-custom",
Some(worker_profile(
Some("local"),
None,
None,
None,
None,
vec![],
)),
);
let custom_profiles = vec![custom];
let (model, provider) =
fleet_worker_launch_route(&custom_task, &custom_profiles, "deepseek-v4-pro");
assert_eq!(model, "qwen-2.5-7b");
assert_eq!(provider.as_deref(), Some("lm-studio"));

// 2) A DeepSeek-shaped model with NO explicit provider must NOT infer a
// provider — provider stays None so the worker keeps its own session
// default, and no `--provider` is emitted.
Expand Down
Loading
Loading