diff --git a/crates/aft/src/commands/edit_match.rs b/crates/aft/src/commands/edit_match.rs index 3741fc1e..c43c99e6 100644 --- a/crates/aft/src/commands/edit_match.rs +++ b/crates/aft/src/commands/edit_match.rs @@ -82,8 +82,12 @@ pub fn handle_edit_match(req: &RawRequest, ctx: &AppContext) -> Response { // sequences before the string reaches us. Adding unescape_str on top caused // double-interpretation that corrupted source code with literal escapes. - // Detect glob pattern - if is_glob_pattern(file) { + // Detect glob pattern. Prefer the literal interpretation when the path + // already exists on disk, even if its name contains glob metacharacters + // such as `[`, `]`, `*`, `?`, or `{`. This prevents files in directories + // with brackets (e.g. `src/[another]/file.rs`) from being misclassified as + // globs. + if should_treat_as_glob(file, ctx) { return handle_glob_edit_match(req, ctx, file, match_str, replacement, &op_id); } @@ -368,6 +372,36 @@ fn is_glob_pattern(path: &str) -> bool { path.contains('*') || path.contains('?') || path.contains('{') || path.contains('[') } +/// Returns true when `file` should be treated as a glob pattern rather than a +/// literal path. A path that exists on disk is always treated literally, even +/// if its name contains glob metacharacters such as `[`, `]`, `*`, `?`, or `{`. +/// This defends against directories or files with brackets in their +/// names being misclassified as glob patterns (see issue #132). +/// +/// Resolution mirrors `ctx.validate_path` so the literal-vs-glob decision stays +/// consistent with the single-file path handler's interpretation. +fn should_treat_as_glob(file: &str, ctx: &AppContext) -> bool { + if !is_glob_pattern(file) { + return false; + } + match ctx.validate_path("literal-check", Path::new(file)) { + Ok(candidate) => !candidate.exists(), + Err(resp) + if resp.data.get("code").and_then(|c| c.as_str()) == Some("path_outside_root") => + { + false + } + Err(resp) => { + log::debug!( + "edit_match: validate_path failed for '{}', deferring to single-file handler: {:?}", + file, + resp.data + ); + false + } + } +} + /// Handle a glob-based multi-file edit_match. fn handle_glob_edit_match( req: &RawRequest, diff --git a/crates/aft/tests/integration/edit_test.rs b/crates/aft/tests/integration/edit_test.rs index 201e2a10..c7cab808 100644 --- a/crates/aft/tests/integration/edit_test.rs +++ b/crates/aft/tests/integration/edit_test.rs @@ -814,6 +814,150 @@ fn edit_match_no_match() { assert!(status.success()); } +// Regression for #132: edit_match must treat literal paths with brackets as +// single-file edits, not glob patterns. Brackets (`[]`) are glob metacharacters, +// so the old character-detection heuristic misclassified real file paths. +#[test] +fn edit_match_handles_brackets_in_literal_path() { + let mut aft = AftProcess::spawn(); + let dir = tempfile::tempdir().unwrap(); + let subdir = dir.path().join("[another]"); + fs::create_dir_all(&subdir).unwrap(); + let target = subdir.join("file.ts"); + fs::write(&target, "const value = OLD;\n").unwrap(); + + let req = serde_json::json!({ + "id": "em-brackets", + "command": "edit_match", + "file": target.display().to_string(), + "match": "OLD", + "replacement": "NEW" + }); + let resp = aft.send(&serde_json::to_string(&req).unwrap()); + + assert_eq!( + resp["success"], true, + "edit_match should succeed for path with brackets: {:?}", + resp + ); + assert_eq!(resp["replacements"], 1); + let content = fs::read_to_string(&target).unwrap(); + assert!( + content.contains("NEW"), + "replacement should apply: {}", + content + ); + assert!( + !content.contains("OLD"), + "original text should be gone: {}", + content + ); + + let status = aft.shutdown(); + assert!(status.success()); +} + +// Sanity check: parentheses are also called out in the issue, but they are +// not glob metacharacters, so they were never misclassified. This test only +// ensures literal paths containing them remain single-file edits. +#[test] +fn edit_match_handles_parentheses_in_literal_path() { + let mut aft = AftProcess::spawn(); + let dir = tempfile::tempdir().unwrap(); + let subdir = dir.path().join("(something)"); + fs::create_dir_all(&subdir).unwrap(); + let target = subdir.join("file.ts"); + fs::write(&target, "const value = OLD;\n").unwrap(); + + let req = serde_json::json!({ + "id": "em-parens", + "command": "edit_match", + "file": target.display().to_string(), + "match": "OLD", + "replacement": "NEW" + }); + let resp = aft.send(&serde_json::to_string(&req).unwrap()); + + assert_eq!( + resp["success"], true, + "edit_match should succeed for path with parentheses: {:?}", + resp + ); + assert_eq!(resp["replacements"], 1); + let content = fs::read_to_string(&target).unwrap(); + assert!( + content.contains("NEW"), + "replacement should apply: {}", + content + ); + assert!( + !content.contains("OLD"), + "original text should be gone: {}", + content + ); + + let status = aft.shutdown(); + assert!(status.success()); +} + +// Regression for #132: relative paths with brackets must also be treated as +// literal single-file edits when resolved against the configured project root. +#[test] +fn edit_match_handles_brackets_in_relative_literal_path() { + let mut aft = AftProcess::spawn(); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let subdir = root.join("[another]"); + fs::create_dir_all(&subdir).unwrap(); + let target = subdir.join("file.ts"); + fs::write(&target, "const value = OLD;\n").unwrap(); + + let configure = aft.send( + &serde_json::json!({ + "id": "cfg-restrict", + "command": "configure", + "harness": "opencode", + "project_root": root.to_string_lossy(), + "config": user_config(serde_json::json!({ "restrict_to_project_root": true })) + }) + .to_string(), + ); + assert_eq!( + configure["success"], true, + "configure should succeed: {configure:?}" + ); + + let req = serde_json::json!({ + "id": "em-brackets-relative", + "command": "edit_match", + "file": "[another]/file.ts", + "match": "OLD", + "replacement": "NEW" + }); + let resp = aft.send(&serde_json::to_string(&req).unwrap()); + + assert_eq!( + resp["success"], true, + "edit_match should succeed for relative path with brackets: {:?}", + resp + ); + assert_eq!(resp["replacements"], 1); + let content = fs::read_to_string(&target).unwrap(); + assert!( + content.contains("NEW"), + "replacement should apply: {}", + content + ); + assert!( + !content.contains("OLD"), + "original text should be gone: {}", + content + ); + + let status = aft.shutdown(); + assert!(status.success()); +} + // ============================================================================ // batch command tests // ============================================================================