From 88d72789f4826d125a008d6dd0e59edf777d8ff3 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 12 Jun 2026 00:34:47 +0200 Subject: [PATCH] apply include/exclude globs in search.replace (T-364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit computeReplacements accepted the query's glob filters and silently dropped them — replace could rewrite files the equivalent search would never have matched. The grep engine's glob helpers are now public and shared, so search and replace can't disagree on scope; both the preview and the apply path go through the filtered list. Co-Authored-By: Claude Fable 5 --- .pql/changelog/ticket_history/2026-06.sql | 2 ++ .pql/changelog/tickets/2026-06.sql | 6 ++++++ CHANGELOG.md | 5 +++++ lib/src/search/grep_engine.dart | 13 +++++++----- lib/src/search/replace_engine.dart | 6 ++++++ test/search/replace_engine_test.dart | 24 +++++++++++++++++++++++ 6 files changed, 51 insertions(+), 5 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index f43bccfe..72895458 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3947,3 +3947,5 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC0HYRZ86CWW0DDQJ5CAQM', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:30:24', '2026-06-11 22:30:24', '2026-06-11 22:30:24', NULL, 'aa1fe975e147be83905f24c63662254d', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:30:48', '2026-06-11 22:30:48', '2026-06-11 22:30:48', NULL, '2c5c8e5cdf68c96e25705f014dcd6acc', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:32:08', '2026-06-11 22:32:08', '2026-06-11 22:32:08', NULL, 'f3dbb31fb6f6f416a2d4d8ef37723316', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:32:29', '2026-06-11 22:32:29', '2026-06-11 22:32:29', NULL, '7b833a606da59ab523e0bb43f1753f6a', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:34:31', '2026-06-11 22:34:31', '2026-06-11 22:34:31', NULL, '254ae09f42b4da439a04d59d675d8f42', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index fc42bd70..b350f497 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -4007,3 +4007,9 @@ Fix: use lstat (FileStat via Link check / FileSystemEntity.isLinkSync on the raw INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBSG6356MZJ2DCCCSBMBGM', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'listDir symlink detection is dead code; walkFiles descends symlinked dirs', 'lib/src/files/listing.dart:46-54 — stat() follows links, so isSymlink is always false; walkFiles therefore descends symlinked directories the docs claim it skips (escape hatch out of the workspace, plus cycle risk). Fix: use lstat (FileStat via Link check / FileSystemEntity.isLinkSync on the raw path) for symlink detection. Tests: symlinked dir is reported as symlink and not descended; symlink cycle does not hang the walk.', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:55:49', '2026-06-11 22:32:08', NULL, 'f83e31fd330c98a311a3da10cba1c655', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'search.replace silently ignores its include/exclude globs', 'lib/src/search/replace_engine.dart:124-143 — the include/exclude glob filters the user typed are accepted but never applied; replace will happily rewrite files outside the filter. + +Fix: apply the same glob filtering the search side uses before rewriting. Test: replace with an include glob touches only matching files; exclude glob is honored.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-11 21:55:38', '2026-06-11 22:32:29', NULL, '0d067ce0eded0fcc158d09ea793bb223', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBR4636GSRJBWFJDAZ6ZA0', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'search.replace silently ignores its include/exclude globs', 'lib/src/search/replace_engine.dart:124-143 — the include/exclude glob filters the user typed are accepted but never applied; replace will happily rewrite files outside the filter. + +Fix: apply the same glob filtering the search side uses before rewriting. Test: replace with an include glob touches only matching files; exclude glob is honored.', 'done', 'high', NULL, NULL, NULL, '2026-06-11 21:55:38', '2026-06-11 22:34:31', NULL, 'e86c024d6f8a6d9cd9b7273522b54511', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acf87e5..11e69869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. search engine on top of it) silently descended symlinked directories — including ones pointing outside the workspace. (T-365) +- **Search-and-replace now honors its include/exclude globs.** The filters + were accepted but never applied, so replace could rewrite files outside + the scope the user typed; replace now uses the same glob filtering as + search. (T-364) + - **Accepting ExitPlanMode now leaves plan mode in the conversation panel.** Approving Claude's plan (the ExitPlanMode tool) transitioned the underlying session out of plan mode, but clide's tracked permission mode didn't follow, diff --git a/lib/src/search/grep_engine.dart b/lib/src/search/grep_engine.dart index d4ba1e64..4b61a33e 100644 --- a/lib/src/search/grep_engine.dart +++ b/lib/src/search/grep_engine.dart @@ -56,11 +56,11 @@ Stream> grepWorkspace({ final walk = await walkFiles(root: root, ignore: ignore); if (cancel?.isCancelled ?? false) return; - final includes = [for (final g in query.include) _globToRegExp(g)]; - final excludes = [for (final g in query.exclude) _globToRegExp(g)]; + final includes = [for (final g in query.include) globToRegExp(g)]; + final excludes = [for (final g in query.exclude) globToRegExp(g)]; final candidates = []; for (final e in walk.files) { - if (_acceptGlobs(e.path, includes, excludes)) candidates.add(e.path); + if (acceptGlobs(e.path, includes, excludes)) candidates.add(e.path); } if (candidates.isEmpty) return; @@ -193,7 +193,10 @@ class CompiledQuery { // -- Glob filtering ---------------------------------------------------------- -bool _acceptGlobs(String path, List includes, List excludes) { +/// Whether [path] passes the compiled include/exclude filters. Shared with +/// the replace engine so search and replace can never disagree on scope +/// (T-364). +bool acceptGlobs(String path, List includes, List excludes) { if (includes.isNotEmpty && !includes.any((r) => r.hasMatch(path))) return false; if (excludes.any((r) => r.hasMatch(path))) return false; return true; @@ -202,7 +205,7 @@ bool _acceptGlobs(String path, List includes, List excludes) { /// Compile a gitignore-flavoured glob to a full-path regex. A `/` in /// the glob anchors it to the workspace root; otherwise it may match at /// any depth (basename-style). Supports `*`, `**`, `?`. -RegExp _globToRegExp(String glob) { +RegExp globToRegExp(String glob) { final anchored = glob.contains('/'); final b = StringBuffer('^'); if (!anchored) b.write(r'(?:.*/)?'); diff --git a/lib/src/search/replace_engine.dart b/lib/src/search/replace_engine.dart index 2dd38831..7ff9cf9b 100644 --- a/lib/src/search/replace_engine.dart +++ b/lib/src/search/replace_engine.dart @@ -16,6 +16,7 @@ import 'dart:io'; import '../files/ignore.dart'; import '../files/listing.dart'; +import 'grep_engine.dart' show acceptGlobs, globToRegExp; import 'match.dart'; /// One changed line within a file. @@ -133,9 +134,14 @@ Future> computeReplacements({ final walk = await walkFiles(root: root, ignore: ignore); final rootPath = root.absolute.path; + // Same compiled glob filters as the grep engine — replace must never + // touch a file the equivalent search wouldn't have matched (T-364). + final includes = [for (final g in query.include) globToRegExp(g)]; + final excludes = [for (final g in query.exclude) globToRegExp(g)]; final out = []; for (final entry in walk.files) { if (out.length >= maxFiles) break; + if (!acceptGlobs(entry.path, includes, excludes)) continue; final fr = _replaceInFile(rootPath, entry.path, query, replacement); if (fr != null) out.add(fr); } diff --git a/test/search/replace_engine_test.dart b/test/search/replace_engine_test.dart index 828429a5..ea6c79dd 100644 --- a/test/search/replace_engine_test.dart +++ b/test/search/replace_engine_test.dart @@ -104,6 +104,30 @@ void main() { ); expect(r.any((f) => f.path == 'blob.bin'), isFalse); }); + + // T-364: the globs were accepted and silently dropped — replace touched + // files the equivalent search would never have matched. + test('include glob restricts replacement to matching files', () async { + File('${root.path}/c.txt').writeAsStringSync('foo here too\n'); + final r = await computeReplacements( + root: root, + ignore: IgnoreSet([]), + query: const SearchQuery(pattern: 'foo', include: ['*.dart']), + replacement: 'baz', + ); + expect(r.map((f) => f.path).toList(), ['a.dart']); + }); + + test('exclude glob is honored', () async { + File('${root.path}/c.txt').writeAsStringSync('foo here too\n'); + final r = await computeReplacements( + root: root, + ignore: IgnoreSet([]), + query: const SearchQuery(pattern: 'foo', exclude: ['*.dart']), + replacement: 'baz', + ); + expect(r.map((f) => f.path).toList(), ['c.txt']); + }); }); group('rewriteFileContent', () {