fix(reasoning): respect user config and Effort::None for models without effort support

crates/forge_app/src/agent.rs

@@ -144,7 +144,7 @@ impl AgentExt for Agent {
         }
 
         // Apply workflow reasoning configuration to agents.
-        // Agent-level fields take priority; config fills in any unset fields.
+        // User config takes priority; agent defaults fill in any unset fields.
         if let Some(ref config_reasoning) = config.reasoning {
             use forge_config::Effort as ConfigEffort;
             let config_as_domain = ReasoningConfig {
@@ -161,9 +161,9 @@ impl AgentExt for Agent {
                 exclude: config_reasoning.exclude,
                 enabled: config_reasoning.enabled,
             };
-            // Start from the agent's own settings and fill unset fields from config.
-            let mut merged = agent.reasoning.clone().unwrap_or_default();
-            merged.merge(config_as_domain);
+            // Start from user config; agent defaults fill unset fields.
+            let mut merged = config_as_domain;
+            merged.merge(agent.reasoning.clone().unwrap_or_default());
             agent.reasoning = Some(merged);
         }
 
@@ -208,29 +208,79 @@ mod tests {
         assert_eq!(actual, expected);
     }
 
-    /// When the agent already has reasoning fields set, those fields take
-    /// priority; config only fills in fields the agent left unset.
+    /// When both the agent and config set reasoning fields, user config
+    /// takes priority; agent values only fill in fields the config left unset.
     #[test]
-    fn test_reasoning_agent_fields_take_priority_over_config() {
+    fn test_reasoning_config_fields_take_priority_over_agent() {
         let config = ForgeConfig::default().reasoning(
             ConfigReasoningConfig::default()
                 .enabled(true)
                 .effort(ConfigEffort::Low)
                 .max_tokens(1024_usize),
         );
 
-        // Agent overrides effort but leaves enabled and max_tokens unset.
+        // Agent overrides effort but config takes priority.
         let agent = fixture_agent().reasoning(ReasoningConfig::default().effort(Effort::High));
 
         let actual = agent.apply_config(&config).reasoning;
 
         let expected = Some(
             ReasoningConfig::default()
-                .effort(Effort::High) // agent's value wins
+                .effort(Effort::Low) // config's value wins
                 .enabled(true) // filled in from config
                 .max_tokens(1024_usize), // filled in from config
         );
 
         assert_eq!(actual, expected);
     }
+
+    /// When the user sets enabled=false in config, it overrides the agent's
+    /// enabled=true.  This is the core scenario from issue #2924.
+    #[test]
+    fn test_reasoning_config_enabled_false_overrides_agent() {
+        let config = ForgeConfig::default().reasoning(
+            ConfigReasoningConfig::default()
+                .enabled(false)
+                .effort(ConfigEffort::None),
+        );
+
+        // Agent has enabled=true (like forge.md).
+        let agent = fixture_agent().reasoning(ReasoningConfig::default().enabled(true));
+
+        let actual = agent.apply_config(&config).reasoning;
+
+        let expected = Some(
+            ReasoningConfig::default()
+                .enabled(false)
+                .effort(Effort::None),
+        );
+
+        assert_eq!(actual, expected);
+    }
+
+    /// When the user config leaves fields unset, the agent's defaults fill
+    /// them.
+    #[test]
+    fn test_reasoning_agent_fills_unset_config_fields() {
+        let config = ForgeConfig::default()
+            .reasoning(ConfigReasoningConfig::default().effort(ConfigEffort::Medium));
+
+        // Agent has enabled=true and max_tokens=2048; config has neither.
+        let agent = fixture_agent().reasoning(
+            ReasoningConfig::default()
+                .enabled(true)
+                .max_tokens(2048_usize),
+        );
+
+        let actual = agent.apply_config(&config).reasoning;
+
+        let expected = Some(
+            ReasoningConfig::default()
+                .effort(Effort::Medium) // from config
+                .enabled(true) // from agent (config had None)
+                .max_tokens(2048_usize), // from agent (config had None)
+        );
+
+        assert_eq!(actual, expected);
+    }
 }

crates/forge_app/src/dto/anthropic/request.rs

@@ -144,19 +144,24 @@ impl TryFrom<forge_domain::Context> for Request {
                         None,
                     )
                 } else if let Some(effort) = reasoning.effort {
-                    // Effort without budget → newer output_config API.
-                    let output_effort = match effort {
-                        forge_domain::Effort::Low => OutputEffort::Low,
-                        forge_domain::Effort::High => OutputEffort::High,
-                        forge_domain::Effort::Max => OutputEffort::Max,
-                        // Map unsupported variants to the nearest Anthropic-valid effort.
-                        forge_domain::Effort::None | forge_domain::Effort::Minimal => {
-                            OutputEffort::Low
-                        }
-                        forge_domain::Effort::Medium => OutputEffort::Medium,
-                        forge_domain::Effort::XHigh => OutputEffort::Max,
-                    };
-                    (None, Some(OutputConfig { effort: output_effort }))
+                    if matches!(effort, forge_domain::Effort::None) {
+                        // "None" means skip reasoning entirely.
+                        (None, None)
+                    } else {
+                        // Effort without budget → newer output_config API.
+                        let output_effort = match effort {
+                            forge_domain::Effort::Low | forge_domain::Effort::Minimal => {
+                                OutputEffort::Low
+                            }
+                            forge_domain::Effort::Medium => OutputEffort::Medium,
+                            forge_domain::Effort::High => OutputEffort::High,
+                            forge_domain::Effort::Max | forge_domain::Effort::XHigh => {
+                                OutputEffort::Max
+                            }
+                            forge_domain::Effort::None => unreachable!(),
+                        };
+                        (None, Some(OutputConfig { effort: output_effort }))
+                    }
                 } else {
                     // Enabled-only → thinking with default budget.
                     (
@@ -680,6 +685,55 @@ mod tests {
         assert_eq!(actual.thinking, None);
     }
 
+    #[test]
+    fn test_effort_none_with_enabled_skips_reasoning() {
+        // Effort::None means "skip reasoning entirely", even if enabled=true.
+        let fixture = Context::default().reasoning(ReasoningConfig {
+            enabled: Some(true),
+            effort: Some(forge_domain::Effort::None),
+            max_tokens: None,
+            exclude: None,
+        });
+
+        let actual = Request::try_from(fixture).unwrap();
+
+        assert_eq!(actual.thinking, None);
+        assert_eq!(actual.output_config, None);
+    }
+
+    #[test]
+    fn test_effort_none_with_disabled_skips_reasoning() {
+        let fixture = Context::default().reasoning(ReasoningConfig {
+            enabled: Some(false),
+            effort: Some(forge_domain::Effort::None),
+            max_tokens: None,
+            exclude: None,
+        });
+
+        let actual = Request::try_from(fixture).unwrap();
+
+        assert_eq!(actual.thinking, None);
+        assert_eq!(actual.output_config, None);
+    }
+
+    #[test]
+    fn test_effort_minimal_maps_to_low() {
+        let fixture = Context::default().reasoning(ReasoningConfig {
+            enabled: Some(true),
+            effort: Some(forge_domain::Effort::Minimal),
+            max_tokens: None,
+            exclude: None,
+        });
+
+        let actual = Request::try_from(fixture).unwrap();
+
+        assert_eq!(
+            actual.output_config,
+            Some(OutputConfig { effort: OutputEffort::Low })
+        );
+        assert_eq!(actual.thinking, None);
+    }
+
     #[test]
     fn test_context_conversion_stream_defaults_to_true() {
         let fixture = Context::default();