Skip to content

feat(api/ui): set provider and model atomically #2717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tusharmath wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(api/ui): set provider and model atomically #2717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
609ed30
feat(api/ui): set provider and model atomically
tusharmath Mar 27, 2026
53fb829
[autofix.ci] apply automated fixes
autofix-ci[bot] Mar 27, 2026
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
14 changes: 9 additions & 5 deletions crates/forge_api/src/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,15 @@ pub trait API: Sync + Send {
/// Retrieves the provider configuration for the default agent
async fn get_default_provider(&self) -> anyhow::Result<Provider<Url>>;

/// Sets the default provider for all the agents
async fn set_default_provider(&self, provider_id: ProviderId) -> anyhow::Result<()>;
/// Sets the active provider and model simultaneously.
///
/// Both values must be supplied together to ensure the configuration always
/// contains a consistent provider/model pair.
async fn set_provider_and_model(
&self,
provider_id: ProviderId,
model_id: ModelId,
) -> anyhow::Result<()>;

/// Retrieves information about the currently authenticated user
async fn user_info(&self) -> anyhow::Result<Option<User>>;
Expand All @@ -142,9 +149,6 @@ pub trait API: Sync + Send {
/// Gets the default model
async fn get_default_model(&self) -> Option<ModelId>;

/// Sets the operating model
async fn set_default_model(&self, model_id: ModelId) -> anyhow::Result<()>;

/// Gets the commit configuration (provider and model for commit message
/// generation).
async fn get_commit_config(&self) -> anyhow::Result<Option<forge_domain::CommitConfig>>;
Expand Down
18 changes: 9 additions & 9 deletions crates/forge_api/src/forge_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,15 @@ impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInf
agent_provider_resolver.get_provider(Some(agent_id)).await
}

async fn set_default_provider(&self, provider_id: ProviderId) -> anyhow::Result<()> {
let result = self.services.set_default_provider(provider_id).await;
async fn set_provider_and_model(
&self,
provider_id: ProviderId,
model_id: ModelId,
) -> anyhow::Result<()> {
let result = self
.services
.set_provider_and_model(provider_id, model_id)
.await;
// Invalidate cache for agents
let _ = self.services.reload_agents().await;
result
Expand Down Expand Up @@ -261,13 +268,6 @@ impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInf
async fn get_default_model(&self) -> Option<ModelId> {
self.services.get_provider_model(None).await.ok()
}
async fn set_default_model(&self, model_id: ModelId) -> anyhow::Result<()> {
let result = self.services.set_default_model(model_id).await;
// Invalidate cache for agents
let _ = self.services.reload_agents().await;

result
}

async fn get_commit_config(&self) -> anyhow::Result<Option<CommitConfig>> {
self.services.get_commit_config().await
Expand Down
10 changes: 5 additions & 5 deletions crates/forge_app/src/command_generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ mod tests {
Ok(ProviderId::OPENAI)
}

async fn set_default_provider(&self, _provider_id: ProviderId) -> Result<()> {
async fn set_provider_and_model(
&self,
_provider_id: ProviderId,
_model: ModelId,
) -> Result<()> {
Ok(())
}

Expand All @@ -260,10 +264,6 @@ mod tests {
Ok(ModelId::new("test-model"))
}

async fn set_default_model(&self, _model: ModelId) -> Result<()> {
Ok(())
}

async fn get_commit_config(&self) -> Result<Option<forge_domain::CommitConfig>> {
Ok(None)
}
Expand Down
23 changes: 9 additions & 14 deletions crates/forge_app/src/services.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,14 @@ pub trait AppConfigService: Send + Sync {
/// Gets the user's default provider ID.
async fn get_default_provider(&self) -> anyhow::Result<ProviderId>;

/// Sets the user's default provider preference.
async fn set_default_provider(
/// Sets the active provider and model simultaneously.
///
/// Both values must be supplied together to ensure the persisted
/// configuration always contains a consistent provider/model pair.
async fn set_provider_and_model(
&self,
provider_id: forge_domain::ProviderId,
model: ModelId,
) -> anyhow::Result<()>;

/// Gets the user's default model for a specific provider or the currently
Expand All @@ -206,12 +210,6 @@ pub trait AppConfigService: Send + Sync {
provider_id: Option<&forge_domain::ProviderId>,
) -> anyhow::Result<ModelId>;

/// Sets the user's default model for the currently active provider.
///
/// # Errors
/// Returns an error if no default provider is configured.
async fn set_default_model(&self, model: ModelId) -> anyhow::Result<()>;

/// Gets the commit configuration (provider and model for commit message
/// generation).
async fn get_commit_config(&self) -> anyhow::Result<Option<forge_domain::CommitConfig>>;
Expand Down Expand Up @@ -985,12 +983,13 @@ impl<I: Services> AppConfigService for I {
self.config_service().get_default_provider().await
}

async fn set_default_provider(
async fn set_provider_and_model(
&self,
provider_id: forge_domain::ProviderId,
model: ModelId,
) -> anyhow::Result<()> {
self.config_service()
.set_default_provider(provider_id)
.set_provider_and_model(provider_id, model)
.await
}

Expand All @@ -1001,10 +1000,6 @@ impl<I: Services> AppConfigService for I {
self.config_service().get_provider_model(provider_id).await
}

async fn set_default_model(&self, model: ModelId) -> anyhow::Result<()> {
self.config_service().set_default_model(model).await
}

async fn get_commit_config(&self) -> anyhow::Result<Option<forge_domain::CommitConfig>> {
self.config_service().get_commit_config().await
}
Expand Down
72 changes: 16 additions & 56 deletions crates/forge_domain/src/env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ pub struct SessionConfig {
/// each in order, and persist the result atomically.
#[derive(Debug, Clone, PartialEq)]
pub enum ConfigOperation {
/// Set the active provider.
SetProvider(ProviderId),
/// Set the model for the given provider.
SetModel(ProviderId, ModelId),
/// Set the active provider and model together.
///
/// Both must be supplied at once to ensure the configuration always
/// contains a consistent provider/model pair.
SetProviderModel(ProviderId, ModelId),
/// Set the commit-message generation configuration.
SetCommitConfig(CommitConfig),
/// Set the shell-command suggestion configuration.
Expand Down Expand Up @@ -172,20 +173,10 @@ impl Environment {
/// Applies a single [`ConfigOperation`] to this environment in-place.
pub fn apply_op(&mut self, op: ConfigOperation) {
match op {
ConfigOperation::SetProvider(provider_id) => {
let pid = provider_id.as_ref().to_string();
self.session = Some(match self.session.take() {
Some(sc) => sc.provider_id(pid),
None => SessionConfig::default().provider_id(pid),
});
}
ConfigOperation::SetModel(provider_id, model_id) => {
ConfigOperation::SetProviderModel(provider_id, model_id) => {
let pid = provider_id.as_ref().to_string();
let mid = model_id.to_string();
self.session = Some(match self.session.take() {
Some(sc) if sc.provider_id.as_deref() == Some(&pid) => sc.model_id(mid),
_ => SessionConfig::default().provider_id(pid).model_id(mid),
});
self.session = Some(SessionConfig::default().provider_id(pid).model_id(mid));
}
ConfigOperation::SetCommitConfig(commit) => {
self.commit =
Expand Down Expand Up @@ -315,59 +306,28 @@ mod tests {
}

#[test]
fn test_apply_op_set_provider_creates_session_when_absent() {
let mut fixture = fixture_env();
fixture.apply_op(ConfigOperation::SetProvider(ProviderId::from(
"anthropic".to_string(),
)));
let expected = SessionConfig::default().provider_id("anthropic".to_string());
assert_eq!(fixture.session, Some(expected));
}

#[test]
fn test_apply_op_set_provider_updates_existing_session_keeping_model() {
let mut fixture = fixture_env();
fixture.session = Some(
SessionConfig::default()
.provider_id("openai".to_string())
.model_id("gpt-4".to_string()),
);
fixture.apply_op(ConfigOperation::SetProvider(ProviderId::from(
"anthropic".to_string(),
)));
let expected = SessionConfig::default()
.provider_id("anthropic".to_string())
.model_id("gpt-4".to_string());
assert_eq!(fixture.session, Some(expected));
}

#[test]
fn test_apply_op_set_model_for_matching_provider_updates_model() {
fn test_apply_op_set_provider_model_creates_session() {
let mut fixture = fixture_env();
fixture.session = Some(
SessionConfig::default()
.provider_id("openai".to_string())
.model_id("gpt-3.5".to_string()),
);
fixture.apply_op(ConfigOperation::SetModel(
ProviderId::from("openai".to_string()),
ModelId::new("gpt-4"),
fixture.session = None;
fixture.apply_op(ConfigOperation::SetProviderModel(
ProviderId::from("anthropic".to_string()),
ModelId::new("claude-3"),
));
let expected = SessionConfig::default()
.provider_id("openai".to_string())
.model_id("gpt-4".to_string());
.provider_id("anthropic".to_string())
.model_id("claude-3".to_string());
assert_eq!(fixture.session, Some(expected));
}

#[test]
fn test_apply_op_set_model_for_different_provider_replaces_session() {
fn test_apply_op_set_provider_model_replaces_existing_session() {
let mut fixture = fixture_env();
fixture.session = Some(
SessionConfig::default()
.provider_id("openai".to_string())
.model_id("gpt-4".to_string()),
);
fixture.apply_op(ConfigOperation::SetModel(
fixture.apply_op(ConfigOperation::SetProviderModel(
ProviderId::from("anthropic".to_string()),
ModelId::new("claude-3"),
));
Expand Down
32 changes: 2 additions & 30 deletions crates/forge_main/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,18 +517,13 @@ pub struct ConfigGetArgs {
/// Type-safe subcommands for `forge config set`.
#[derive(Subcommand, Debug, Clone)]
pub enum ConfigSetField {
/// Set the active model.
Model {
/// Model ID to set as default.
model: ModelId,
},
/// Set the active provider.
/// Set the active provider and model.
Provider {
/// Provider ID to set as default.
provider: ProviderId,

/// Optional model ID to set simultaneously, skipping interactive model
/// selection.
/// selection. When omitted, an interactive prompt is shown.
#[arg(long)]
model: Option<ModelId>,
},
Comment on lines 521 to 529
Copy link
Copy Markdown
Collaborator Author

@tusharmath tusharmath Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

model and provider are both required while setting model.

Expand Down Expand Up @@ -822,29 +817,6 @@ mod tests {
assert_eq!(actual, expected);
}

#[test]
fn test_config_set_with_model() {
let fixture = Cli::parse_from([
"forge",
"config",
"set",
"model",
"anthropic/claude-sonnet-4",
]);
let actual = match fixture.subcommands {
Some(TopLevelCommand::Config(config)) => match config.command {
ConfigCommand::Set(args) => match args.field {
ConfigSetField::Model { model } => Some(model.as_str().to_string()),
_ => None,
},
_ => None,
},
_ => None,
};
let expected = Some("anthropic/claude-sonnet-4".to_string());
assert_eq!(actual, expected);
}

#[test]
fn test_config_set_with_provider() {
let fixture = Cli::parse_from(["forge", "config", "set", "provider", "OpenAI"]);
Expand Down
Loading
Loading