Skip to content

Commit 7c92ba8

Browse files
fix(config): enforce allow_edit_existing_files restriction in tool selection (#388) (#389)
Wire up the existing but unused `infer_task_edit_requirement()` heuristic to the tool selection pipeline. When a prompt indicates editing intent (fix, implement, refactor, etc.), tools with `allow_edit_existing_files = false` are now filtered out during tier-based selection. Three changes: - `resolve_tool_and_model()` accepts `needs_edit` and forwards it to `resolve_tier_tool_rotated()` (was hardcoded `false`) - New `resolve_tier_tool_filtered()` replaces the unfiltered fallback, `resolve_tier_tool()` now delegates to it with `needs_edit=false` - `run_cmd_execute` calls `infer_task_edit_requirement()` and passes the result through the strategy resolution chain Closes #388 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 780e55b commit 7c92ba8

11 files changed

Lines changed: 281 additions & 32 deletions

File tree

Cargo.lock

Lines changed: 15 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ members = ["crates/*"]
33
resolver = "2"
44

55
[workspace.package]
6-
version = "0.1.108"
6+
version = "0.1.109"
77
edition = "2024"
88
rust-version = "1.85"
99
license = "Apache-2.0"

crates/cli-sub-agent/src/claude_sub_agent_cmd.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub(crate) async fn handle_claude_sub_agent(
4343
&project_root,
4444
false, // claude-sub-agent does not support --force
4545
false, // claude-sub-agent does not support --force-override-user-config
46+
false, // sub-agent dispatch always uses explicit tool
4647
)?;
4748

4849
// 8. Build executor and validate tool

crates/cli-sub-agent/src/mcp_server.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ async fn handle_run_tool(args: Value) -> Result<Value> {
488488
&project_root,
489489
false, // MCP server does not support --force
490490
false, // MCP server does not support --force-override-user-config
491+
false, // MCP tool dispatch always uses explicit tool
491492
)?;
492493

493494
// Build executor

crates/cli-sub-agent/src/run_cmd_execute.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ pub(crate) async fn handle_run(
172172
};
173173
let run_timeout_seconds = resolve_run_timeout_seconds(timeout, skill.as_deref());
174174
let run_started_at = Instant::now();
175+
let needs_edit = crate::run_helpers::infer_task_edit_requirement(&prompt_text).unwrap_or(false);
175176
let strategy_result = resolve_tool_by_strategy(
176177
&strategy,
177178
model_spec.as_deref(),
@@ -181,6 +182,7 @@ pub(crate) async fn handle_run(
181182
&project_root,
182183
force,
183184
force_override_user_config,
185+
needs_edit,
184186
)?;
185187
let heterogeneous_runtime_fallback_candidates = strategy_result.runtime_fallback_candidates;
186188
let resolved_model_spec = strategy_result.model_spec;

crates/cli-sub-agent/src/run_cmd_tool_selection.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ pub(crate) fn resolve_tool_by_strategy(
133133
project_root: &Path,
134134
force: bool,
135135
force_override_user_config: bool,
136+
needs_edit: bool,
136137
) -> Result<StrategyResolution> {
137138
match strategy {
138139
ToolSelectionStrategy::Explicit(t) => {
@@ -144,6 +145,7 @@ pub(crate) fn resolve_tool_by_strategy(
144145
project_root,
145146
force,
146147
force_override_user_config,
148+
needs_edit,
147149
)?;
148150
Ok(StrategyResolution {
149151
tool,
@@ -161,6 +163,7 @@ pub(crate) fn resolve_tool_by_strategy(
161163
project_root,
162164
force,
163165
force_override_user_config,
166+
needs_edit,
164167
)?;
165168
Ok(StrategyResolution {
166169
tool,
@@ -177,6 +180,7 @@ pub(crate) fn resolve_tool_by_strategy(
177180
project_root,
178181
force,
179182
force_override_user_config,
183+
needs_edit,
180184
),
181185
ToolSelectionStrategy::HeterogeneousStrict => {
182186
let res = resolve_heterogeneous_strict(
@@ -187,6 +191,7 @@ pub(crate) fn resolve_tool_by_strategy(
187191
project_root,
188192
force,
189193
force_override_user_config,
194+
needs_edit,
190195
)?;
191196
Ok(StrategyResolution {
192197
tool: res.0,
@@ -216,6 +221,7 @@ fn collect_enabled_tools(
216221
}
217222
}
218223

224+
#[allow(clippy::too_many_arguments)]
219225
fn resolve_heterogeneous_preferred(
220226
model_spec: Option<&str>,
221227
model: Option<&str>,
@@ -224,6 +230,7 @@ fn resolve_heterogeneous_preferred(
224230
project_root: &Path,
225231
force: bool,
226232
force_override_user_config: bool,
233+
needs_edit: bool,
227234
) -> Result<StrategyResolution> {
228235
let detected_parent_tool = detect_parent_tool();
229236
let parent_tool_name = resolve_tool(detected_parent_tool, global_config);
@@ -245,6 +252,7 @@ fn resolve_heterogeneous_preferred(
245252
project_root,
246253
force,
247254
force_override_user_config,
255+
needs_edit,
248256
)?;
249257
Ok(StrategyResolution {
250258
tool: t,
@@ -267,6 +275,7 @@ fn resolve_heterogeneous_preferred(
267275
project_root,
268276
force,
269277
force_override_user_config,
278+
needs_edit,
270279
)?;
271280
Ok(StrategyResolution {
272281
tool: t,
@@ -288,6 +297,7 @@ fn resolve_heterogeneous_preferred(
288297
project_root,
289298
force,
290299
force_override_user_config,
300+
needs_edit,
291301
)?;
292302
Ok(StrategyResolution {
293303
tool: t,
@@ -298,6 +308,7 @@ fn resolve_heterogeneous_preferred(
298308
}
299309
}
300310

311+
#[allow(clippy::too_many_arguments)]
301312
fn resolve_heterogeneous_strict(
302313
model_spec: Option<&str>,
303314
model: Option<&str>,
@@ -306,6 +317,7 @@ fn resolve_heterogeneous_strict(
306317
project_root: &Path,
307318
force: bool,
308319
force_override_user_config: bool,
320+
needs_edit: bool,
309321
) -> Result<(ToolName, Option<String>, Option<String>)> {
310322
let detected_parent_tool = detect_parent_tool();
311323
let parent_tool_name = resolve_tool(detected_parent_tool, global_config);
@@ -323,6 +335,7 @@ fn resolve_heterogeneous_strict(
323335
project_root,
324336
force,
325337
force_override_user_config,
338+
needs_edit,
326339
),
327340
None => {
328341
anyhow::bail!(
@@ -346,6 +359,7 @@ fn resolve_heterogeneous_strict(
346359
project_root,
347360
force,
348361
force_override_user_config,
362+
needs_edit,
349363
)
350364
}
351365
}

crates/cli-sub-agent/src/run_helpers.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use csa_session::TokenUsage;
1717
/// - model: optional model string (from CLI, with alias resolution applied)
1818
///
1919
/// When tool is None, uses tier-based round-robin selection.
20+
/// `needs_edit`: when true, filters out tools with `allow_edit_existing_files = false`.
21+
#[allow(clippy::too_many_arguments)]
2022
pub(crate) fn resolve_tool_and_model(
2123
tool: Option<ToolName>,
2224
model_spec: Option<&str>,
@@ -25,6 +27,7 @@ pub(crate) fn resolve_tool_and_model(
2527
project_root: &Path,
2628
force: bool,
2729
force_override_user_config: bool,
30+
needs_edit: bool,
2831
) -> Result<(ToolName, Option<String>, Option<String>)> {
2932
// Case 1: model_spec provided → parse it to get tool
3033
if let Some(spec) = model_spec {
@@ -84,13 +87,15 @@ pub(crate) fn resolve_tool_and_model(
8487
if let Some(cfg) = config {
8588
// Try round-robin rotation first (needs project root to persist state)
8689
if let Ok(Some((tool_name_str, tier_model_spec))) =
87-
csa_scheduler::resolve_tier_tool_rotated(cfg, "default", project_root, false)
90+
csa_scheduler::resolve_tier_tool_rotated(cfg, "default", project_root, needs_edit)
8891
{
8992
let tool_name = parse_tool_name(&tool_name_str)?;
9093
return Ok((tool_name, Some(tier_model_spec), None));
9194
}
92-
// Fallback: original non-rotating selection
93-
if let Some((tool_name_str, tier_model_spec)) = cfg.resolve_tier_tool("default") {
95+
// Fallback: original non-rotating selection (also respects edit restrictions)
96+
if let Some((tool_name_str, tier_model_spec)) =
97+
cfg.resolve_tier_tool_filtered("default", needs_edit)
98+
{
9499
let tool_name = parse_tool_name(&tool_name_str)?;
95100
return Ok((tool_name, Some(tier_model_spec), None));
96101
}

crates/cli-sub-agent/src/run_helpers_tests.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ fn resolve_tool_and_model_disabled_tool_explicit_errors() {
549549
std::path::Path::new("/tmp"),
550550
true, // force tier bypass
551551
false, // no override
552+
false, // needs_edit
552553
);
553554
assert!(result.is_err());
554555
let msg = result.unwrap_err().to_string();
@@ -594,8 +595,9 @@ fn resolve_tool_and_model_disabled_tool_with_override_succeeds() {
594595
None,
595596
Some(&config),
596597
std::path::Path::new("/tmp"),
597-
true, // force tier bypass
598-
true, // override enabled
598+
true, // force tier bypass
599+
true, // override enabled
600+
false, // needs_edit
599601
);
600602
assert!(result.is_ok());
601603
let (tool, _, _) = result.unwrap();
@@ -643,6 +645,7 @@ fn resolve_tool_and_model_disabled_tool_model_spec_errors() {
643645
std::path::Path::new("/tmp"),
644646
true,
645647
false,
648+
false, // needs_edit
646649
);
647650
assert!(result.is_err());
648651
let msg = result.unwrap_err().to_string();

0 commit comments

Comments
 (0)