M20: [Major] Rules matcher hardening — picomatch reuse, Windows paths, glob guard, tiebreaker, date compare

[curtyo18]

Summary

Five related defects in packages/engine/src/rules/matcher.ts (35-line file). They span correctness, perf, and platform support; bundled because all live in one tiny module and one PR can address them together.

1. picomatch is recompiled per file × per rule (perf)

matcher.ts:20 constructs picomatch(m.pathGlob, { dot: true }) inside matches, which is called for every (file, rule) pair during planning. On a 100k-file scan with 20 rules that's 2 million regex compilations. The compiled matcher should be cached per rule.

2. pathGlob matches raw file.path with no separator normalization (Windows correctness)

matcher.ts:21 runs isMatch(file.path). picomatch is forward-slash by default. On Windows where file.path contains backslashes, the documented glob **/Downloads/** silently never fires. README declares Windows the primary target.

3. No guard against catastrophic glob input (correctness / DoS)

A user-edited rule with a pathological glob (**/**/**/**.{a,b,c,d,e} or similar) makes picomatch produce a regex with exponential backtracking. Currently this would throw a raw error at match time; users get no feedback that their rule is malformed until planning hangs or errors.

4. firstMatch is order-dependent for equal-priority rules

matcher.ts:29 iterates the array as given; RulesRepo.list() orders by priority ASC only. Two rules at the same priority have implementation-defined evaluation order — silent first-match flapping if SQLite's row order changes.

5. ISO timestamp compared lexicographically against date-only string (correctness; discussion)

matcher.ts:10-11: if (m.dateBefore && fileDate >= m.dateBefore) return false;. fileDate is a full ISO timestamp ('2025-06-01T00:00:00.000Z'); m.dateBefore per Rule.match.dateBefore is documented as an ISO date string but historically users have set it as either '2024-01-01' or '2024-01-01T00:00:00Z'. Lexicographic comparison works correctly across that boundary by accident (ISO sorts correctly). Worth a deliberate decision: either parse both sides into Date objects, or document the invariant ("dateBefore must be a prefix of ISO timestamp; lex comparison is the documented semantics") and add a boundary test.

Background

From the 2026-05-17 multi-agent full-repo review. Items #1-#4 are clear defects; item #5 needs a deliberate call.

Acceptance criteria

picomatch reuse (#1)

• Compiled matcher is cached per rule. Either: lift compilation into firstMatch (compile each enabled rule's glob once before the loop), or attach the compiled matcher to the Rule shape via a memo map keyed by rule ID + pathGlob string.
• Perf test or benchmark added showing reduced compilation count on a 100-file × 20-rule synthetic dataset.

Windows path normalization (#2)

• Before matching, normalize file.path separators to / (and document this contract on the pathGlob field's type comment in @fileorganizer/shared/rules.ts). The catalog stores Windows paths with backslashes; converting just for match input doesn't affect storage.
• Test added: pathGlob: '**/Downloads/**' matches file.path: 'C:\\Users\\foo\\Downloads\\bar.jpg' on a Windows-style input.

Catastrophic glob guard (#3)

• picomatch() construction in firstMatch (or wherever compilation lands) is wrapped in try; on throw, translate to RuleError('INVALID_GLOB', \rule ${rule.id} has malformed pathGlob: ${(err as Error).message}`)` and surface to the planner.
• The planner endpoint returns the rule's invalid-glob error as a per-rule validation issue rather than failing the whole plan.
• Test added: a deliberately bad glob is reported per-rule, the other rules plan normally.

Equal-priority tiebreaker (#4)

• RulesRepo.list() adds a secondary ORDER BY clause that produces a stable, user-meaningful order. Options: created_at ASC, name ASC or name ASC alone. Pick one and document.
• Test added: two rules at the same priority — assert their evaluation order is deterministic and matches the chosen tiebreaker.

Date comparison (#5) — needs discussion

• Resolve the question: keep the lex comparison (and document the invariant + add boundary test), or parse both sides into Date (and accept a small perf hit on every match call).
• If keeping lex: a JSDoc on Rule.match.dateBefore / dateAfter documenting "must be an ISO 8601 timestamp prefix; lex comparison is intentional"; a boundary test at '2024-01-01T00:00:00.000Z'.
• If parsing: factor into matches(); ensure no per-call new Date() if the rule's dates can be parsed once at load.

Files affected (likely)

• packages/engine/src/rules/matcher.ts
• packages/engine/src/rules/matcher.test.ts
• packages/engine/src/rules/repo.ts — tiebreaker ORDER BY
• packages/engine/src/rules/repo.test.ts — tiebreaker test
• packages/shared/src/rules.ts — pathGlob contract JSDoc
• packages/shared/src/errors.ts — RuleError('INVALID_GLOB', ...) if not already present

Suggested approach

Address in this order: #4 (smallest), #2 (one line + test), #3 (guard around compile), #1 (refactor firstMatch to hoist compilation), then #5 after a quick discussion.

Out of scope

• Template rendering on Windows (separate concern; the path.posix.join vs path.win32.join question is in rules/template.ts).
• Glob debug UI ("which files would this rule match?").

References

• Code: packages/engine/src/rules/matcher.ts
• Spec §6.1 — Rule.match.path_glob
• README — "Windows is the primary target"