From cde87dbad779e17ef92a8410053f137fb050a977 Mon Sep 17 00:00:00 2001 From: tobwen <1864057+tobwen@users.noreply.github.com> Date: Fri, 19 Jun 2026 23:12:10 +0000 Subject: [PATCH 1/2] fix(edit_match): prefer literal paths over glob Existing paths with brackets are now treated as literal files instead of glob patterns. Falls back to glob when the path does not exist or validation fails. --- crates/aft/src/commands/edit_match.rs | 38 +++++- crates/aft/tests/integration/edit_test.rs | 144 ++++++++++++++++++++++ 2 files changed, 180 insertions(+), 2 deletions(-) diff --git a/crates/aft/src/commands/edit_match.rs b/crates/aft/src/commands/edit_match.rs index 3741fc1e..8c0949af 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 '{}', treating as glob: {:?}", + file, + resp.data + ); + true + } + } +} + /// 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 // ============================================================================ From d3d29ebfdf090d47ae92e454683b73896d96e945 Mon Sep 17 00:00:00 2001 From: tobwen <1864057+tobwen@users.noreply.github.com> Date: Sat, 20 Jun 2026 00:45:17 +0000 Subject: [PATCH 2/2] fix(edit_match): fail-safe on unknown validate_path errors Addresses Greptile P2: the Err(_) => true catch-all silently routed unknown validate_path errors to the glob handler. Return false instead so the single-file handler re-validates and surfaces the error. Dead code today (all errors use path_outside_root), zero behavior change. --- crates/aft/src/commands/edit_match.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/aft/src/commands/edit_match.rs b/crates/aft/src/commands/edit_match.rs index 8c0949af..c43c99e6 100644 --- a/crates/aft/src/commands/edit_match.rs +++ b/crates/aft/src/commands/edit_match.rs @@ -393,11 +393,11 @@ fn should_treat_as_glob(file: &str, ctx: &AppContext) -> bool { } Err(resp) => { log::debug!( - "edit_match: validate_path failed for '{}', treating as glob: {:?}", + "edit_match: validate_path failed for '{}', deferring to single-file handler: {:?}", file, resp.data ); - true + false } } }