Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions crates/forge_config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ pub struct ProviderUrlParam {
/// UI.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub options: Vec<String>,
/// Whether this parameter is optional. When `true`, the parameter may be
/// left blank without causing an error.
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub optional: bool,
}

/// A single provider entry defined inline in `forge.toml`.
Expand Down
15 changes: 13 additions & 2 deletions crates/forge_domain/src/auth/new_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,36 @@ pub struct URLParamValue(String);
///
/// When `options` is `Some`, the UI presents a dropdown for selection.
/// When `options` is `None`, the UI presents a free-text input.
/// When `optional` is `true`, the parameter may be left blank and missing
/// values are silently ignored during credential creation and URL rendering.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct URLParamSpec {
/// The parameter name used as the template variable and credential map key.
pub name: URLParam,
/// Optional list of allowed values. When present, the UI renders a
/// dropdown.
pub options: Option<Vec<String>>,
/// Whether this parameter is optional. When `true`, the parameter may be
/// left blank without causing an error.
#[serde(default)]
pub optional: bool,
}

impl URLParamSpec {
/// Creates a `URLParamSpec` with only a name, rendering as a free-text
/// input.
pub fn new(name: impl Into<URLParam>) -> Self {
Self { name: name.into(), options: None }
Self { name: name.into(), options: None, optional: false }
}

/// Creates a `URLParamSpec` with preset options, rendering as a dropdown.
pub fn with_options(name: impl Into<URLParam>, options: Vec<String>) -> Self {
Self { name: name.into(), options: Some(options) }
Self { name: name.into(), options: Some(options), optional: false }
}

/// Creates an optional `URLParamSpec` that may be left blank.
pub fn optional(name: impl Into<URLParam>) -> Self {
Self { name: name.into(), options: None, optional: true }
}
}

Expand Down
25 changes: 18 additions & 7 deletions crates/forge_main/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2996,8 +2996,13 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
.prompt()?
.context("Parameter selection cancelled")?
} else {
// Free-text path (existing behavior)
let mut input = ForgeWidget::input(format!("Enter {}", param.name));
// Free-text path
let label = if param.optional {
format!("Enter {} (optional, press Enter to skip)", param.name)
} else {
format!("Enter {}", param.name)
};
let mut input = ForgeWidget::input(label);

// Add default value if it exists in the credential
if let Some(params) = existing_url_params
Expand All @@ -3006,13 +3011,19 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
input = input.with_default(default_value.as_str());
}

if param.optional {
input = input.allow_empty(true);
}

let param_value = input.prompt()?.context("Parameter input cancelled")?;

anyhow::ensure!(
!param_value.trim().is_empty(),
"{} cannot be empty",
param.name
);
if !param.optional {
anyhow::ensure!(
!param_value.trim().is_empty(),
"{} cannot be empty",
param.name
);
}

param_value.trim_end_matches('/').to_string()
};
Expand Down
6 changes: 3 additions & 3 deletions crates/forge_repo/src/provider/provider.json
Original file line number Diff line number Diff line change
Expand Up @@ -1228,11 +1228,11 @@
"url_param_vars": [
{"name": "VLLM_SSL_SCHEME", "options": ["http", "https"]},
"VLLM_HOST",
"VLLM_PORT"
{"name": "VLLM_PORT", "optional": true}
],
"response_type": "OpenAI",
"url": "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}:{{VLLM_PORT}}/v1/chat/completions",
"models": "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}:{{VLLM_PORT}}/v1/models",
"url": "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}{{#if VLLM_PORT}}:{{VLLM_PORT}}{{/if}}/v1/chat/completions",
"models": "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}{{#if VLLM_PORT}}:{{VLLM_PORT}}{{/if}}/v1/models",
"auth_methods": ["api_key"]
},
{
Expand Down
87 changes: 81 additions & 6 deletions crates/forge_repo/src/provider/provider_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,15 @@ enum Models {
enum UrlParamVarConfig {
/// A plain environment variable name with free-text UI input.
Plain(String),
/// A parameter with a constrained set of options, rendered as a dropdown.
WithOptions { name: String, options: Vec<String> },
/// A parameter with a constrained set of options, rendered as a dropdown,
/// and an optional flag indicating the param may be left blank.
WithOptions {
name: String,
#[serde(default)]
options: Vec<String>,
#[serde(default)]
optional: bool,
},
}

impl UrlParamVarConfig {
Expand All @@ -40,12 +47,26 @@ impl UrlParamVarConfig {
}
}

/// Returns whether this parameter is optional.
fn is_optional(&self) -> bool {
match self {
Self::Plain(_) => false,
Self::WithOptions { optional, .. } => *optional,
}
}

/// Converts into a `URLParamSpec` for use in the domain layer.
fn into_spec(self) -> URLParamSpec {
match self {
Self::Plain(s) => URLParamSpec::new(URLParam::from(s)),
Self::WithOptions { name, options } => {
URLParamSpec::with_options(URLParam::from(name), options)
Self::WithOptions { name, options, optional } => {
let mut spec = if options.is_empty() {
URLParamSpec::new(URLParam::from(name))
} else {
URLParamSpec::with_options(URLParam::from(name), options)
};
spec.optional = optional;
spec
}
}
}
Expand Down Expand Up @@ -132,10 +153,14 @@ fn merge_configs(base: &mut Vec<ProviderConfig>, other: Vec<ProviderConfig>) {

impl From<forge_config::ProviderUrlParam> for UrlParamVarConfig {
fn from(param: forge_config::ProviderUrlParam) -> Self {
if param.options.is_empty() {
if param.options.is_empty() && !param.optional {
UrlParamVarConfig::Plain(param.name)
} else {
UrlParamVarConfig::WithOptions { name: param.name, options: param.options }
UrlParamVarConfig::WithOptions {
name: param.name,
options: param.options,
optional: param.optional,
}
}
}
}
Expand Down Expand Up @@ -392,6 +417,11 @@ impl<
URLParam::from(name.to_string()),
URLParamValue::from(value.to_string()),
);
} else if env_var.is_optional() {
// Optional param absent from env — omit from credential
// entirely. `render_url_template` injects null
// for absent optional params so `{{#if PARAM}}`
// evaluates to false.
} else {
return Err(Error::env_var_not_found(config.id.clone(), name).into());
}
Expand Down Expand Up @@ -1746,4 +1776,49 @@ mod env_tests {
.find(|c| c.id == ProviderId::OPEN_ROUTER);
assert!(openrouter_config.is_some());
}

#[tokio::test]
async fn test_vllm_port_is_optional() {
let configs = get_provider_configs();
let vllm_id = ProviderId::from("vllm".to_string());
let config = configs.iter().find(|c| c.id == vllm_id).unwrap();

let port_param = config
.url_param_vars
.iter()
.find(|v| v.param_name() == "VLLM_PORT")
.unwrap();

assert!(port_param.is_optional(), "VLLM_PORT should be optional");
}

#[tokio::test]
async fn test_vllm_migration_without_port() {
// vLLM behind a reverse proxy — no port needed
let mut env_vars = HashMap::new();
env_vars.insert("VLLM_API_KEY".to_string(), "vllm-key".to_string());
env_vars.insert("VLLM_HOST".to_string(), "my.server.url".to_string());
env_vars.insert("VLLM_SSL_SCHEME".to_string(), "https".to_string());
// VLLM_PORT intentionally absent

let infra = Arc::new(MockInfra::new(env_vars));
let registry = ForgeProviderRepository::new(infra.clone());

registry.migrate_env_to_file().await.unwrap();

let credentials = infra.credentials.lock().await;
let creds = credentials.as_ref().unwrap();

let vllm_id = ProviderId::from("vllm".to_string());
let vllm_cred = creds.iter().find(|c| c.id == vllm_id).unwrap();

// Optional absent param should not be stored in the credential at all.
// `render_url_template` handles the absent key by injecting null.
assert!(
!vllm_cred
.url_params
.contains_key(&URLParam::from("VLLM_PORT".to_string())),
"VLLM_PORT should be absent from credential when not provided"
);
}
}
101 changes: 95 additions & 6 deletions crates/forge_services/src/provider_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,37 @@ impl<R> ForgeProviderService<R> {
Self { repository }
}

/// Renders a URL template with provided parameters
/// Renders a URL template with provided parameters.
///
/// Params present in the credential are passed through as strings.
/// Optional specs that are absent from the credential entirely are
/// inserted as JSON `null` so that `{{#if PARAM}}` blocks evaluate
/// to false in the Handlebars template (e.g. a port not provided).
fn render_url_template(
&self,
template: &str,
params: &HashMap<forge_domain::URLParam, forge_domain::URLParamValue>,
specs: &[forge_domain::URLParamSpec],
) -> Result<Url> {
let template_data: HashMap<&str, &str> = params
// Start with all stored params as string values.
let mut template_data: HashMap<&str, serde_json::Value> = params
.iter()
.map(|(k, v)| (k.as_str(), v.as_str()))
.map(|(k, v)| {
(
k.as_str(),
serde_json::Value::String(v.as_str().to_string()),
)
})
.collect();

// For optional specs that are entirely absent from the credential,
// inject null so {{#if PARAM}} is falsy instead of erroring.
for spec in specs {
if spec.optional && !params.contains_key(&spec.name) {
template_data.insert(spec.name.as_str(), serde_json::Value::Null);
}
}

let handlebars = forge_app::TemplateEngine::handlebar_instance();
let rendered = handlebars.render_template(template, &template_data)?;

Expand All @@ -48,14 +68,21 @@ impl<R> ForgeProviderService<R> {
.ok_or_else(|| anyhow::anyhow!("Provider has no credential"))?;

// Render main URL
let url =
self.render_url_template(&template_provider.url.template, &credential.url_params)?;
let url = self.render_url_template(
&template_provider.url.template,
&credential.url_params,
&template_provider.url_params,
)?;

// Render model source URLs
let models = template_provider.models.as_ref().and_then(|m| match m {
ModelSource::Url(template) => {
let model_url = self
.render_url_template(&template.template, &credential.url_params)
.render_url_template(
&template.template,
&credential.url_params,
&template_provider.url_params,
)
.ok();
model_url.map(ModelSource::Url)
}
Expand Down Expand Up @@ -299,4 +326,66 @@ mod tests {
);
}
}

#[test]
fn test_render_url_template_optional_port_absent() {
// VLLM_PORT is absent from the credential (user left it blank).
// render_url_template must inject null so {{#if VLLM_PORT}} is falsy.
let service = ForgeProviderService::new(Arc::new(MockProviderRepository::new(vec![])));
let template = "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}{{#if VLLM_PORT}}:{{VLLM_PORT}}{{/if}}/v1/chat/completions";

let mut params = HashMap::new();
params.insert(
forge_domain::URLParam::from("VLLM_SSL_SCHEME".to_string()),
forge_domain::URLParamValue::from("https".to_string()),
);
params.insert(
forge_domain::URLParam::from("VLLM_HOST".to_string()),
forge_domain::URLParamValue::from("my.server.url".to_string()),
);
// VLLM_PORT intentionally absent — not in the credential map at all.

let specs = vec![forge_domain::URLParamSpec::optional(
forge_domain::URLParam::from("VLLM_PORT".to_string()),
)];

let actual = service
.render_url_template(template, &params, &specs)
.unwrap();
let expected = "https://my.server.url/v1/chat/completions";

assert_eq!(actual.as_str(), expected);
}

#[test]
fn test_render_url_template_optional_port_with_value() {
// When VLLM_PORT has a value, it should appear in the URL.
let service = ForgeProviderService::new(Arc::new(MockProviderRepository::new(vec![])));
let template = "{{VLLM_SSL_SCHEME}}://{{VLLM_HOST}}{{#if VLLM_PORT}}:{{VLLM_PORT}}{{/if}}/v1/chat/completions";

let mut params = HashMap::new();
params.insert(
forge_domain::URLParam::from("VLLM_SSL_SCHEME".to_string()),
forge_domain::URLParamValue::from("https".to_string()),
);
params.insert(
forge_domain::URLParam::from("VLLM_HOST".to_string()),
forge_domain::URLParamValue::from("my.server.url".to_string()),
);
params.insert(
forge_domain::URLParam::from("VLLM_PORT".to_string()),
forge_domain::URLParamValue::from("8000".to_string()),
);

let specs = vec![forge_domain::URLParamSpec::optional(
forge_domain::URLParam::from("VLLM_PORT".to_string()),
)];

let actual = service
.render_url_template(template, &params, &specs)
.unwrap();
let expected = "https://my.server.url:8000/v1/chat/completions";

assert_eq!(actual.as_str(), expected);
}
}
4 changes: 4 additions & 0 deletions forge.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,10 @@
"description": "The environment variable name used as the template variable key.",
"type": "string"
},
"optional": {
"description": "Whether this parameter is optional. When `true`, the parameter may be\nleft blank without causing an error.",
"type": "boolean"
},
"options": {
"description": "Optional preset values for this parameter shown as suggestions in the\nUI.",
"type": "array",
Expand Down
Loading