Skip to content

Commit 6d3b4b2

Browse files
pmaxhoganclaude
andauthored
feat(ui): warn when include patterns defeat directory pruning (#162)
## What Both places a user edits exclusion rules - the add-source wizard's Exclusions step and the per-source inline editor in Settings > Sources - now show a small amber warning box under the **Include patterns** textarea whenever an entered include pattern would stop the scanner from pruning excluded directories. The scanner can only skip descending into an excluded folder (`node_modules`, etc.) when it can prove no include pattern could match inside it. That proof needs a root-anchored pattern of bounded depth (`/repo/.env`, `/*/.env`, `/a/*/b/.env`). A relative pattern (`.env`, `*/.env`, `blah/.env`) matches at any depth, and one containing `**` spans any number of levels - either forces the walk into every excluded directory just in case, which is what makes a scan crawl. ## How - `ui/src/stores/exclusionPreview.ts` (where the other pattern helpers live) gains `isUnconstrainedIncludePattern` / `unconstrainedIncludePatterns`: a trimmed, non-empty pattern is unconstrained iff it does not start with `/` **or** it contains `**`. `splitPatterns` is now exported from the same module and both editors use it instead of their own private copies, so the warning splits input exactly the way the saved rules do (newline **or** comma separated). - Each editor renders the box reactively off a `computed` over its textarea text, so it appears/disappears as the user types - the preview walk still only re-runs on blur. The box names the offending patterns and links to the anchored forms; styling matches the existing amber warning surfaces (`AccountList` reauth banner, `PausedBanner`). - New i18n keys nested under `settings.addSource.includeWarning` (`title`, `hint`), shared by both editors like the existing `includePatternsLabel`. ## Tests - `exclusion-preview-store.test.ts`: a vector table for the helper (`.env`, `*/.env`, `blah/.env`, `/*/x/**/.env` warn; `/x/.env`, `/*/.env`, `/a/*/b/.env` do not), plus trimming, blank lines, and comma vs newline splitting. - `settings-components.test.ts`: one test per editor asserting the box is absent with no rules, appears listing **only** the offending patterns once an unconstrained rule is typed, and disappears when the rules are anchored - without a blur or a re-preview. `npx vitest run` 454 passed, `prettier --check src` clean, `vue-tsc --noEmit` and `eslint .` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JLB3E2Jm7knNJd37fVpH8X Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent da17be9 commit 6d3b4b2

6 files changed

Lines changed: 245 additions & 16 deletions

File tree

ui/src/__tests__/exclusion-preview-store.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import {
3535
anchoredPatternForPath,
3636
appendPatternLine,
3737
createExclusionPreview,
38+
isUnconstrainedIncludePattern,
39+
unconstrainedIncludePatterns,
3840
type ExclusionPreviewController,
3941
} from "../stores/exclusionPreview";
4042
import type { ExclusionPreviewBatch, ExclusionPreviewNode } from "../ipc/types";
@@ -128,6 +130,67 @@ describe("appendPatternLine", () => {
128130
});
129131
});
130132

133+
describe("isUnconstrainedIncludePattern", () => {
134+
// The scanner may only PRUNE an excluded directory when no include pattern
135+
// could match beneath it, which needs a root-anchored pattern of bounded
136+
// depth. Anything relative, or spanning levels with a double-star, forces the
137+
// walk into every excluded folder - those are the ones the editors warn about.
138+
const vectors: Array<[string, boolean]> = [
139+
// Unconstrained: no leading slash, so it matches at any depth.
140+
[".env", true],
141+
["*/.env", true],
142+
["blah/.env", true],
143+
["**/.env", true],
144+
// Unconstrained: anchored, but a double-star spans any number of levels.
145+
["/*/x/**/.env", true],
146+
["/**", true],
147+
["/a/**/b", true],
148+
// Constrained: anchored AND depth-bounded.
149+
["/x/.env", false],
150+
["/*/.env", false],
151+
["/a/*/b/.env", false],
152+
["/.env", false],
153+
["/node_modules/", false],
154+
// Blank lines are not patterns at all.
155+
["", false],
156+
[" ", false],
157+
];
158+
159+
it.each(vectors)("treats %j as unconstrained=%s", (pattern, expected) => {
160+
expect(isUnconstrainedIncludePattern(pattern)).toBe(expected);
161+
});
162+
163+
it("judges a pattern by its trimmed form", () => {
164+
// The editors trim each line before sending it, so surrounding whitespace
165+
// must not change the verdict either way.
166+
expect(isUnconstrainedIncludePattern(" /x/.env ")).toBe(false);
167+
expect(isUnconstrainedIncludePattern(" .env ")).toBe(true);
168+
});
169+
});
170+
171+
describe("unconstrainedIncludePatterns", () => {
172+
it("reports only the offending patterns, in the order they were typed", () => {
173+
expect(unconstrainedIncludePatterns("/x/.env\n.env\n/a/*/b/.env\n/*/x/**/.env")).toEqual([
174+
".env",
175+
"/*/x/**/.env",
176+
]);
177+
});
178+
179+
it("splits on commas as well as newlines, the way both editors do", () => {
180+
expect(unconstrainedIncludePatterns("/x/.env,blah/.env,/*/.env")).toEqual(["blah/.env"]);
181+
});
182+
183+
it("ignores blank lines and surrounding whitespace", () => {
184+
expect(unconstrainedIncludePatterns("\n \n /x/.env \n\n,, \t\n")).toEqual([]);
185+
expect(unconstrainedIncludePatterns(" */.env \n\n")).toEqual(["*/.env"]);
186+
});
187+
188+
it("is empty for empty input and for an all-anchored list", () => {
189+
expect(unconstrainedIncludePatterns("")).toEqual([]);
190+
expect(unconstrainedIncludePatterns("/x/.env\n/*/.env\n/a/*/b/.env")).toEqual([]);
191+
});
192+
});
193+
131194
describe("createExclusionPreview", () => {
132195
it("starts a streaming preview and reports it as scanning", async () => {
133196
const preview = await started();

ui/src/__tests__/settings-components.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,44 @@ describe("SourceTable", () => {
444444
);
445445
});
446446

447+
it("warns while typing an include pattern that defeats directory pruning", async () => {
448+
// The scanner can only skip descending into an excluded folder when no
449+
// include rule could match beneath it. A relative rule (or one with a
450+
// double-star) forces it into every node_modules, so the editor calls that
451+
// out AS THE USER TYPES - the preview walk only re-runs on blur, and the
452+
// guidance must not wait for it.
453+
invokeMock.mockImplementation((cmd: string) => {
454+
if (cmd === "list_sources") return Promise.resolve([makeSource()]);
455+
if (cmd === "list_accounts") return Promise.resolve([]);
456+
if (cmd === "preview_exclusions_start") return Promise.resolve("gen-1");
457+
return Promise.resolve(undefined);
458+
});
459+
const wrapper = mount(SourceTable, { global: globalMountOptions });
460+
await flushPromises();
461+
await wrapper
462+
.findAll("button")
463+
.find((b) => b.text() === i18n.global.t("settings.sources.editExclusionsButton"))!
464+
.trigger("click");
465+
await flushPromises();
466+
467+
const editor = wrapper.get('[data-testid="exclusion-editor"]');
468+
// A source with no include rules at all has nothing to warn about.
469+
expect(wrapper.find('[data-testid="include-pattern-warning"]').exists()).toBe(false);
470+
471+
const includeArea = editor.findAll("textarea")[0];
472+
await includeArea.setValue("/keep/.env\n.env\n/*/x/**/.env");
473+
const warning = wrapper.get('[data-testid="include-pattern-warning"]');
474+
expect(warning.text()).toContain(i18n.global.t("settings.addSource.includeWarning.title"));
475+
// Only the offending rules are listed - the anchored, depth-bounded one is
476+
// fine and must not be named.
477+
const listed = warning.findAll("li").map((li) => li.text());
478+
expect(listed).toEqual([".env", "/*/x/**/.env"]);
479+
480+
// Anchoring them clears the box without a blur or a re-preview.
481+
await includeArea.setValue("/keep/.env\n/*/.env");
482+
expect(wrapper.find('[data-testid="include-pattern-warning"]').exists()).toBe(false);
483+
});
484+
447485
it("Edit exclusions opens the inline editor and saves a patch", async () => {
448486
invokeMock.mockImplementation((cmd: string) => {
449487
if (cmd === "list_sources") return Promise.resolve([makeSource()]);
@@ -747,6 +785,61 @@ describe("AddSourceWizard", () => {
747785
);
748786
});
749787

788+
it("warns on the exclusions step when an include pattern defeats directory pruning", async () => {
789+
invokeMock.mockImplementation((cmd: string) => {
790+
if (cmd === "list_accounts")
791+
return Promise.resolve([
792+
{
793+
id: "acc-1",
794+
email: "user@example.com",
795+
displayName: null,
796+
state: "ok",
797+
encryptionEnabled: false,
798+
createdAt: 0,
799+
lastSyncedAt: null,
800+
},
801+
]);
802+
if (cmd === "pick_drive_folder")
803+
return Promise.resolve({ currentFolderId: "root", currentFolderPath: "", folders: [] });
804+
if (cmd === "preview_exclusions_start") return Promise.resolve("gen-1");
805+
if (cmd === "pick_folder_dialog")
806+
return Promise.resolve({ path: "/home/u/docs", token: "tok-folder" });
807+
return Promise.resolve(undefined);
808+
});
809+
810+
const wrapper = mount(AddSourceWizard, { global: globalMountOptions });
811+
await (wrapper.vm as unknown as { start: () => Promise<void> }).start();
812+
await flushPromises();
813+
await wrapper
814+
.findAll("button")
815+
.find((b) => b.text() === i18n.global.t("settings.addSource.chooseLocalButton"))!
816+
.trigger("click");
817+
await flushPromises();
818+
const clickNext = async () => {
819+
await wrapper
820+
.findAll("button")
821+
.find((b) => b.text() === i18n.global.t("common.next"))!
822+
.trigger("click");
823+
await flushPromises();
824+
};
825+
await clickNext(); // -> Drive
826+
await clickNext(); // -> Exclusions
827+
828+
// A fresh wizard starts with no rules, so nothing to warn about.
829+
expect(wrapper.find('[data-testid="include-pattern-warning"]').exists()).toBe(false);
830+
831+
const includeArea = wrapper.findAll("textarea")[0];
832+
await includeArea.setValue("blah/.env,/a/*/b/.env");
833+
const warning = wrapper.get('[data-testid="include-pattern-warning"]');
834+
expect(warning.text()).toContain(i18n.global.t("settings.addSource.includeWarning.hint"));
835+
expect(warning.findAll("li").map((li) => li.text())).toEqual(["blah/.env"]);
836+
837+
// Anchoring the rule clears the box, and typing never triggered a re-walk
838+
// (that still belongs to blur).
839+
await includeArea.setValue("/blah/.env,/a/*/b/.env");
840+
expect(wrapper.find('[data-testid="include-pattern-warning"]').exists()).toBe(false);
841+
});
842+
750843
it("issue #4: checking the cloud-only toggle sends placeholderPolicy force_download; default is skip", async () => {
751844
let addArgs: unknown = null;
752845
invokeMock.mockImplementation((cmd: string, args?: unknown) => {

ui/src/components/AddSourceWizard.vue

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import RecoveryPhraseReveal from "./RecoveryPhraseReveal.vue";
88
import * as ipc from "../ipc/commands";
99
import { toErrorCode } from "../ipc/errors";
1010
import { useAccountsStore } from "../stores/accounts";
11-
import { appendPatternLine } from "../stores/exclusionPreview";
11+
import {
12+
appendPatternLine,
13+
splitPatterns,
14+
unconstrainedIncludePatterns,
15+
} from "../stores/exclusionPreview";
1216
import { useSourcesStore } from "../stores/sources";
1317
import type { SourceDto } from "../ipc/types";
1418
@@ -105,17 +109,16 @@ const revealErrorCode = ref<string | null>(null);
105109
106110
const includePatterns = computed(() => splitPatterns(includePatternsText.value));
107111
const excludePatterns = computed(() => splitPatterns(excludePatternsText.value));
112+
// The include rules that stop the scanner from pruning excluded folders, so the
113+
// exclusions step can warn about them AS THEY ARE TYPED (the walk itself only
114+
// re-runs on blur, but the guidance should not wait for it).
115+
const unconstrainedIncludes = computed(() =>
116+
unconstrainedIncludePatterns(includePatternsText.value)
117+
);
108118
109119
const canLeaveLocal = computed(() => accountId.value !== null && localPathToken.value !== null);
110120
const canLeaveDrive = computed(() => driveFolderId.value !== null);
111121
112-
function splitPatterns(text: string): string[] {
113-
return text
114-
.split(/[\n,]/)
115-
.map((p) => p.trim())
116-
.filter((p) => p.length > 0);
117-
}
118-
119122
async function start(): Promise<void> {
120123
reset();
121124
open.value = true;
@@ -402,6 +405,26 @@ defineExpose({ start });
402405
@blur="refreshPreview"
403406
/>
404407
</label>
408+
<!-- An include rule the scanner cannot bound to a fixed depth forces it
409+
into every excluded folder, so the walk stops being prunable. -->
410+
<div
411+
v-if="unconstrainedIncludes.length > 0"
412+
class="rounded-lg border border-amber-400 bg-amber-50 p-3 text-xs text-amber-800 dark:border-amber-700 dark:bg-amber-950/40 dark:text-amber-200"
413+
data-testid="include-pattern-warning"
414+
role="status"
415+
>
416+
<p class="font-medium">
417+
{{ t("settings.addSource.includeWarning.title") }}
418+
</p>
419+
<ul class="mt-1 list-disc space-y-0.5 pl-5 font-mono break-all">
420+
<li v-for="pattern in unconstrainedIncludes" :key="pattern">
421+
{{ pattern }}
422+
</li>
423+
</ul>
424+
<p class="mt-2">
425+
{{ t("settings.addSource.includeWarning.hint") }}
426+
</p>
427+
</div>
405428
<label class="block space-y-1 text-sm">
406429
<span class="text-zinc-600 dark:text-zinc-400">{{
407430
t("settings.addSource.excludePatternsLabel")

ui/src/components/SourceTable.vue

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import RecoveryPhraseReveal from "./RecoveryPhraseReveal.vue";
88
import * as ipc from "../ipc/commands";
99
import { toErrorCode } from "../ipc/errors";
1010
import { useAccountsStore } from "../stores/accounts";
11-
import { appendPatternLine } from "../stores/exclusionPreview";
11+
import {
12+
appendPatternLine,
13+
splitPatterns,
14+
unconstrainedIncludePatterns,
15+
} from "../stores/exclusionPreview";
1216
import { useSourcesStore } from "../stores/sources";
1317
import type { SourceDto } from "../ipc/types";
1418
@@ -101,12 +105,10 @@ onMounted(async () => {
101105
await Promise.all([sources.refresh(), accounts.refresh()]);
102106
});
103107
104-
function splitPatterns(text: string): string[] {
105-
return text
106-
.split(/[\n,]/)
107-
.map((p) => p.trim())
108-
.filter((p) => p.length > 0);
109-
}
108+
// The include rules in the open editor that stop the scanner from pruning
109+
// excluded folders, recomputed AS THE USER TYPES (the preview walk itself only
110+
// re-runs on blur, but the guidance should not wait for it).
111+
const unconstrainedIncludes = computed(() => unconstrainedIncludePatterns(editIncludeText.value));
110112
111113
function openWizard(): void {
112114
void wizard.value?.start();
@@ -534,6 +536,26 @@ async function confirmRevealAck(sourceId: string): Promise<void> {
534536
@blur="refreshEditPreview"
535537
/>
536538
</label>
539+
<!-- An include rule the scanner cannot bound to a fixed depth forces
540+
it into every excluded folder, so the walk stops being prunable. -->
541+
<div
542+
v-if="unconstrainedIncludes.length > 0"
543+
class="rounded-lg border border-amber-400 bg-amber-50 p-3 text-xs text-amber-800 dark:border-amber-700 dark:bg-amber-950/40 dark:text-amber-200"
544+
data-testid="include-pattern-warning"
545+
role="status"
546+
>
547+
<p class="font-medium">
548+
{{ t("settings.addSource.includeWarning.title") }}
549+
</p>
550+
<ul class="mt-1 list-disc space-y-0.5 pl-5 font-mono break-all">
551+
<li v-for="pattern in unconstrainedIncludes" :key="pattern">
552+
{{ pattern }}
553+
</li>
554+
</ul>
555+
<p class="mt-2">
556+
{{ t("settings.addSource.includeWarning.hint") }}
557+
</p>
558+
</div>
537559
<label class="block space-y-1 text-sm">
538560
<span class="text-zinc-600 dark:text-zinc-400">{{
539561
t("settings.addSource.excludePatternsLabel")

ui/src/locales/en-US.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@
191191
"respectGitignoreLabel": "Respect .gitignore files",
192192
"includePatternsLabel": "Include patterns",
193193
"excludePatternsLabel": "Exclude patterns",
194+
"includeWarning": {
195+
"title": "These include patterns slow scanning:",
196+
"hint": "Patterns without a leading / (or containing **) force the scanner to search inside every excluded folder such as node_modules. Anchor them to fixed depths instead, e.g. /*/.env, /path/to/.env, or /path/*/deploy/.env."
197+
},
194198
"placeholderPolicyLabel": "Back up OneDrive cloud-only files",
195199
"placeholderPolicyCaption": "Applies to OneDrive / cloud-only placeholder files on Windows (harmless elsewhere). When on, Driven downloads and backs up files that are stored only in the cloud.",
196200
"preview": {

ui/src/stores/exclusionPreview.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,37 @@ export function anchoredPatternForPath(rel: string, isDir: boolean): string | nu
9797

9898
/** Split a patterns textarea into the glob list the IPC layer takes. Mirrors the
9999
* `splitPatterns` both editors already use (newline OR comma separated). */
100-
function splitPatterns(text: string): string[] {
100+
export function splitPatterns(text: string): string[] {
101101
return text
102102
.split(/[\n,]/)
103103
.map((p) => p.trim())
104104
.filter((p) => p.length > 0);
105105
}
106106

107+
/** Would this include pattern stop the scanner from PRUNING excluded folders?
108+
*
109+
* The walker skips descending into an excluded directory only when it can prove
110+
* no include pattern could ever re-include something below it. That proof needs
111+
* a pattern that is anchored to the source root AND bounded in depth, like
112+
* `/repo/.env` or a leading-slash pattern whose wildcards each cover a single
113+
* segment - those can only match at a known depth, so `node_modules` can be
114+
* skipped outright. A relative pattern (`.env`, `blah/.env`) matches at ANY
115+
* depth, and one containing a double-star spans any number of levels, so either
116+
* forces the walker to descend into every excluded directory just in case -
117+
* which is what makes a scan crawl.
118+
*/
119+
export function isUnconstrainedIncludePattern(pattern: string): boolean {
120+
const trimmed = pattern.trim();
121+
if (trimmed === "") return false;
122+
return !trimmed.startsWith("/") || trimmed.includes("**");
123+
}
124+
125+
/** The include patterns in a textarea's raw text that defeat directory pruning,
126+
* in the order the user typed them. Empty when the rules are all prune-safe. */
127+
export function unconstrainedIncludePatterns(text: string): string[] {
128+
return splitPatterns(text).filter(isUnconstrainedIncludePattern);
129+
}
130+
107131
/** Append `pattern` as a NEW LINE to a patterns textarea's text, skipping the
108132
* append when the exact pattern is already present (clicking "-" twice on the
109133
* same row must not stack duplicate rules against the source's 256-pattern cap).

0 commit comments

Comments
 (0)