Skip to content

Commit 7e8a51d

Browse files
fix(config): re-read ForgeConfig via infra get_config (#2878)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 54b2ccc commit 7e8a51d

42 files changed

Lines changed: 867 additions & 976 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/forge_api/src/api.rs‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,13 @@ pub trait API: Sync + Send {
122122
/// Retrieves the provider configuration for the default agent
123123
async fn get_default_provider(&self) -> anyhow::Result<Provider<Url>>;
124124

125-
/// Sets the default provider for all the agents
126-
async fn set_default_provider(&self, provider_id: ProviderId) -> anyhow::Result<()>;
127-
128-
/// Updates the caller's default provider and model together, ensuring all
129-
/// commands resolve a consistent pair without requiring a follow-up model
130-
/// selection call.
131-
async fn set_default_provider_and_model(
132-
&self,
133-
provider_id: ProviderId,
134-
model: ModelId,
135-
) -> anyhow::Result<()>;
125+
/// Applies one or more configuration mutations atomically.
126+
///
127+
/// Each operation in `ops` is applied in order and persisted as a single
128+
/// atomic write. Use [`forge_domain::ConfigOperation`] variants to describe
129+
/// each mutation. Provider and model changes also invalidate the agent
130+
/// cache so the next request picks up the updated configuration.
131+
async fn update_config(&self, ops: Vec<forge_domain::ConfigOperation>) -> anyhow::Result<()>;
136132

137133
/// Retrieves information about the currently authenticated user
138134
async fn user_info(&self) -> anyhow::Result<Option<User>>;
@@ -152,31 +148,17 @@ pub trait API: Sync + Send {
152148
/// Gets the default model
153149
async fn get_default_model(&self) -> Option<ModelId>;
154150

155-
/// Sets the operating model
156-
async fn set_default_model(&self, model_id: ModelId) -> anyhow::Result<()>;
157-
158151
/// Gets the commit configuration (provider and model for commit message
159152
/// generation).
160-
async fn get_commit_config(&self) -> anyhow::Result<Option<forge_domain::CommitConfig>>;
161-
162-
/// Sets the commit configuration (provider and model for commit message
163-
/// generation).
164-
async fn set_commit_config(&self, config: forge_domain::CommitConfig) -> anyhow::Result<()>;
153+
async fn get_commit_config(&self) -> anyhow::Result<Option<forge_domain::ModelConfig>>;
165154

166155
/// Gets the suggest configuration (provider and model for command
167156
/// suggestion generation).
168-
async fn get_suggest_config(&self) -> anyhow::Result<Option<forge_domain::SuggestConfig>>;
169-
170-
/// Sets the suggest configuration (provider and model for command
171-
/// suggestion generation).
172-
async fn set_suggest_config(&self, config: forge_domain::SuggestConfig) -> anyhow::Result<()>;
157+
async fn get_suggest_config(&self) -> anyhow::Result<Option<forge_domain::ModelConfig>>;
173158

174159
/// Gets the current reasoning effort setting.
175160
async fn get_reasoning_effort(&self) -> anyhow::Result<Option<Effort>>;
176161

177-
/// Sets the reasoning effort level applied to all agents.
178-
async fn set_reasoning_effort(&self, effort: Effort) -> anyhow::Result<()>;
179-
180162
/// Refresh MCP caches by fetching fresh data
181163
async fn reload_mcp(&self) -> Result<()>;
182164

‎crates/forge_api/src/forge_api.rs‎

Lines changed: 40 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,20 @@ use crate::API;
2424
pub struct ForgeAPI<S, F> {
2525
services: Arc<S>,
2626
infra: Arc<F>,
27-
config: forge_config::ForgeConfig,
2827
}
2928

3029
impl<A, F> ForgeAPI<A, F> {
31-
pub fn new(services: Arc<A>, infra: Arc<F>, config: forge_config::ForgeConfig) -> Self {
32-
Self { services, infra, config }
30+
pub fn new(services: Arc<A>, infra: Arc<F>) -> Self {
31+
Self { services, infra }
3332
}
3433

35-
/// Creates a ForgeApp instance with the current services
34+
/// Creates a ForgeApp instance with the current services and latest config.
3635
fn app(&self) -> ForgeApp<A>
3736
where
38-
A: Services,
37+
A: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>,
38+
F: EnvironmentInfra<Config = forge_config::ForgeConfig>,
3939
{
40-
ForgeApp::new(self.services.clone(), self.config.clone())
40+
ForgeApp::new(self.services.clone())
4141
}
4242
}
4343

@@ -49,10 +49,10 @@ impl ForgeAPI<ForgeServices<ForgeRepo<ForgeInfra>>, ForgeRepo<ForgeInfra>> {
4949
/// * `config` - Pre-read application configuration (from startup)
5050
/// * `services_url` - Pre-validated URL for the gRPC workspace server
5151
pub fn init(cwd: PathBuf, config: ForgeConfig, services_url: Url) -> Self {
52-
let infra = Arc::new(ForgeInfra::new(cwd, config.clone(), services_url));
53-
let repo = Arc::new(ForgeRepo::new(infra.clone(), config.clone()));
54-
let app = Arc::new(ForgeServices::new(repo.clone(), config.clone()));
55-
ForgeAPI::new(app, repo, config)
52+
let infra = Arc::new(ForgeInfra::new(cwd, config, services_url));
53+
let repo = Arc::new(ForgeRepo::new(infra.clone()));
54+
let app = Arc::new(ForgeServices::new(repo.clone()));
55+
ForgeAPI::new(app, repo)
5656
}
5757

5858
pub async fn get_skills_internal(&self) -> Result<Vec<Skill>> {
@@ -62,8 +62,13 @@ impl ForgeAPI<ForgeServices<ForgeRepo<ForgeInfra>>, ForgeRepo<ForgeInfra>> {
6262
}
6363

6464
#[async_trait::async_trait]
65-
impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInfra> API
66-
for ForgeAPI<A, F>
65+
impl<
66+
A: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>,
67+
F: CommandInfra
68+
+ EnvironmentInfra<Config = forge_config::ForgeConfig>
69+
+ SkillRepository
70+
+ GrpcInfra,
71+
> API for ForgeAPI<A, F>
6772
{
6873
async fn discover(&self) -> Result<Vec<File>> {
6974
let environment = self.services.get_environment();
@@ -98,7 +103,7 @@ impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInf
98103
diff: Option<String>,
99104
additional_context: Option<String>,
100105
) -> Result<forge_app::CommitResult> {
101-
let git_app = GitApp::new(self.services.clone(), self.config.clone());
106+
let git_app = GitApp::new(self.services.clone());
102107
let result = git_app
103108
.commit_message(max_diff_size, diff, additional_context)
104109
.await?;
@@ -225,13 +230,31 @@ impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInf
225230
agent_provider_resolver.get_provider(Some(agent_id)).await
226231
}
227232

228-
async fn set_default_provider(&self, provider_id: ProviderId) -> anyhow::Result<()> {
229-
let result = self.services.set_default_provider(provider_id).await;
230-
// Invalidate cache for agents
231-
let _ = self.services.reload_agents().await;
233+
async fn update_config(&self, ops: Vec<forge_domain::ConfigOperation>) -> anyhow::Result<()> {
234+
// Determine whether any op affects provider/model resolution before writing,
235+
// so we can invalidate the agent cache afterwards.
236+
let needs_agent_reload = ops
237+
.iter()
238+
.any(|op| matches!(op, forge_domain::ConfigOperation::SetSessionConfig(_)));
239+
let result = self.services.update_config(ops).await;
240+
if needs_agent_reload {
241+
let _ = self.services.reload_agents().await;
242+
}
232243
result
233244
}
234245

246+
async fn get_commit_config(&self) -> anyhow::Result<Option<ModelConfig>> {
247+
self.services.get_commit_config().await
248+
}
249+
250+
async fn get_suggest_config(&self) -> anyhow::Result<Option<ModelConfig>> {
251+
self.services.get_suggest_config().await
252+
}
253+
254+
async fn get_reasoning_effort(&self) -> anyhow::Result<Option<Effort>> {
255+
self.services.get_reasoning_effort().await
256+
}
257+
235258
async fn user_info(&self) -> Result<Option<User>> {
236259
let provider = self.get_default_provider().await?;
237260
if let Some(api_key) = provider.api_key() {
@@ -273,50 +296,6 @@ impl<A: Services, F: CommandInfra + EnvironmentInfra + SkillRepository + GrpcInf
273296
async fn get_default_model(&self) -> Option<ModelId> {
274297
self.services.get_provider_model(None).await.ok()
275298
}
276-
async fn set_default_model(&self, model_id: ModelId) -> anyhow::Result<()> {
277-
let result = self.services.set_default_model(model_id).await;
278-
// Invalidate cache for agents
279-
let _ = self.services.reload_agents().await;
280-
281-
result
282-
}
283-
284-
async fn set_default_provider_and_model(
285-
&self,
286-
provider_id: ProviderId,
287-
model: ModelId,
288-
) -> anyhow::Result<()> {
289-
let result = self
290-
.services
291-
.set_default_provider_and_model(provider_id, model)
292-
.await;
293-
let _ = self.services.reload_agents().await;
294-
result
295-
}
296-
297-
async fn get_commit_config(&self) -> anyhow::Result<Option<CommitConfig>> {
298-
self.services.get_commit_config().await
299-
}
300-
301-
async fn set_commit_config(&self, config: CommitConfig) -> anyhow::Result<()> {
302-
self.services.set_commit_config(config).await
303-
}
304-
305-
async fn get_suggest_config(&self) -> anyhow::Result<Option<SuggestConfig>> {
306-
self.services.get_suggest_config().await
307-
}
308-
309-
async fn set_suggest_config(&self, config: SuggestConfig) -> anyhow::Result<()> {
310-
self.services.set_suggest_config(config).await
311-
}
312-
313-
async fn get_reasoning_effort(&self) -> anyhow::Result<Option<Effort>> {
314-
self.services.get_reasoning_effort().await
315-
}
316-
317-
async fn set_reasoning_effort(&self, effort: Effort) -> anyhow::Result<()> {
318-
self.services.set_reasoning_effort(effort).await
319-
}
320299

321300
async fn reload_mcp(&self) -> Result<()> {
322301
self.services.mcp_service().reload_mcp().await

‎crates/forge_app/src/agent.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use merge::Merge;
1010

1111
use crate::services::AppConfigService;
1212
use crate::tool_registry::ToolRegistry;
13-
use crate::{ConversationService, ProviderService, Services};
13+
use crate::{ConversationService, EnvironmentInfra, ProviderService, Services};
1414

1515
/// Agent service trait that provides core chat and tool call functionality.
1616
/// This trait abstracts the essential operations needed by the Orchestrator.
@@ -30,7 +30,6 @@ pub trait AgentService: Send + Sync + 'static {
3030
agent: &Agent,
3131
context: &ToolCallContext,
3232
call: ToolCallFull,
33-
config: &ForgeConfig,
3433
) -> ToolResult;
3534

3635
/// Synchronize the on-going conversation
@@ -39,7 +38,7 @@ pub trait AgentService: Send + Sync + 'static {
3938

4039
/// Blanket implementation of AgentService for any type that implements Services
4140
#[async_trait::async_trait]
42-
impl<T: Services> AgentService for T {
41+
impl<T: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> AgentService for T {
4342
async fn chat_agent(
4443
&self,
4544
id: &ModelId,
@@ -61,9 +60,8 @@ impl<T: Services> AgentService for T {
6160
agent: &Agent,
6261
context: &ToolCallContext,
6362
call: ToolCallFull,
64-
config: &ForgeConfig,
6563
) -> ToolResult {
66-
let registry = ToolRegistry::new(Arc::new(self.clone()), config.clone());
64+
let registry = ToolRegistry::new(Arc::new(self.clone()));
6765
registry.call(agent, context, call).await
6866
}
6967

‎crates/forge_app/src/agent_executor.rs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::sync::Arc;
22

33
use anyhow::Context;
44
use convert_case::{Case, Casing};
5-
use forge_config::ForgeConfig;
65
use forge_domain::{
76
AgentId, ChatRequest, ChatResponse, ChatResponseContent, Conversation, ConversationId, Event,
87
TitleFormat, ToolCallContext, ToolDefinition, ToolName, ToolOutput,
@@ -12,18 +11,16 @@ use futures::StreamExt;
1211
use tokio::sync::RwLock;
1312

1413
use crate::error::Error;
15-
use crate::{AgentRegistry, ConversationService, Services};
16-
14+
use crate::{AgentRegistry, ConversationService, EnvironmentInfra, Services};
1715
#[derive(Clone)]
1816
pub struct AgentExecutor<S> {
1917
services: Arc<S>,
20-
config: ForgeConfig,
2118
pub tool_agents: Arc<RwLock<Option<Vec<ToolDefinition>>>>,
2219
}
2320

24-
impl<S: Services> AgentExecutor<S> {
25-
pub fn new(services: Arc<S>, config: ForgeConfig) -> Self {
26-
Self { services, config, tool_agents: Arc::new(RwLock::new(None)) }
21+
impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> AgentExecutor<S> {
22+
pub fn new(services: Arc<S>) -> Self {
23+
Self { services, tool_agents: Arc::new(RwLock::new(None)) }
2724
}
2825

2926
/// Returns a list of tool definitions for all available agents.
@@ -79,7 +76,7 @@ impl<S: Services> AgentExecutor<S> {
7976
conversation
8077
};
8178
// Execute the request through the ForgeApp
82-
let app = crate::ForgeApp::new(self.services.clone(), self.config.clone());
79+
let app = crate::ForgeApp::new(self.services.clone());
8380
let mut response_stream = app
8481
.chat(
8582
agent_id.clone(),

‎crates/forge_app/src/app.rs‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use crate::tool_registry::ToolRegistry;
1919
use crate::tool_resolver::ToolResolver;
2020
use crate::user_prompt::UserPromptGenerator;
2121
use crate::{
22-
AgentExt, AgentProviderResolver, ConversationService, FileDiscoveryService, ProviderService,
23-
Services,
22+
AgentExt, AgentProviderResolver, ConversationService, EnvironmentInfra, FileDiscoveryService,
23+
ProviderService, Services,
2424
};
2525

2626
/// Builds a [`TemplateConfig`] from a [`ForgeConfig`].
@@ -44,17 +44,12 @@ pub(crate) fn build_template_config(config: &ForgeConfig) -> forge_domain::Templ
4444
pub struct ForgeApp<S> {
4545
services: Arc<S>,
4646
tool_registry: ToolRegistry<S>,
47-
config: ForgeConfig,
4847
}
4948

50-
impl<S: Services> ForgeApp<S> {
51-
/// Creates a new ForgeApp instance with the provided services and config.
52-
pub fn new(services: Arc<S>, config: ForgeConfig) -> Self {
53-
Self {
54-
tool_registry: ToolRegistry::new(services.clone(), config.clone()),
55-
services,
56-
config,
57-
}
49+
impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeApp<S> {
50+
/// Creates a new ForgeApp instance with the provided services.
51+
pub fn new(services: Arc<S>) -> Self {
52+
Self { tool_registry: ToolRegistry::new(services.clone()), services }
5853
}
5954

6055
/// Executes a chat request and returns a stream of responses.
@@ -73,7 +68,7 @@ impl<S: Services> ForgeApp<S> {
7368
.ok_or_else(|| forge_domain::Error::ConversationNotFound(chat.conversation_id))?;
7469

7570
// Discover files using the discovery service
76-
let forge_config = self.config.clone();
71+
let forge_config = self.services.get_config()?;
7772
let environment = services.get_environment();
7873

7974
let files = services.list_current_directory().await?;
@@ -136,7 +131,7 @@ impl<S: Services> ForgeApp<S> {
136131

137132
// Detect and render externally changed files notification
138133
let conversation = ChangedFiles::new(services.clone(), agent.clone())
139-
.update_file_stats(conversation, forge_config.max_parallel_file_reads)
134+
.update_file_stats(conversation)
140135
.await;
141136

142137
let conversation = InitConversationMetrics::new(current_time).apply(conversation);
@@ -159,14 +154,11 @@ impl<S: Services> ForgeApp<S> {
159154
.on_toolcall_end(tracing_handler.clone())
160155
.on_end(tracing_handler.and(title_handler));
161156

162-
let retry_config = forge_config.retry.clone().unwrap_or_default();
163-
164157
let orch = Orchestrator::new(
165158
services.clone(),
166-
retry_config,
167159
conversation,
168160
agent,
169-
forge_config,
161+
self.services.get_config()?,
170162
)
171163
.error_tracker(ToolErrorTracker::new(max_tool_failure_per_turn))
172164
.tool_definitions(tool_definitions)
@@ -229,7 +221,7 @@ impl<S: Services> ForgeApp<S> {
229221
let original_messages = context.messages.len();
230222
let original_token_count = *context.token_count();
231223

232-
let forge_config = self.config.clone();
224+
let forge_config = self.services.get_config()?;
233225

234226
// Get agent and apply workflow config
235227
let agent = self.services.get_agent(&active_agent_id).await?;

0 commit comments

Comments
 (0)