Accept .. globs within project root in catalog directive - #275
Merged
Conversation
Catalog now mirrors include's path-resolution semantics: ".." segments in glob patterns are allowed when the resolved path stays inside the project root, rejected with "glob escapes project root" otherwise. This unblocks cross-tree catalogs (e.g. a skill cataloging architecture docs several levels up the tree) that previously had to ship hand-maintained reference-style link-def workarounds. Adds globpath.ResolveAgainstRoot/ContainsDotDotSegment helpers, threads a globResolution struct through buildCatalogEntries so root-relative matches are displayed via filepath.Rel, and wires MDS019 fixtures onto RootFS in the integration runner so the new behavior is testable. Updates the solid-architecture skill to consume the new behavior with a <?catalog?>-driven slug link-def block.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
========================================
Coverage 95.80% 95.81%
========================================
Files 184 186 +2
Lines 24237 24405 +168
========================================
+ Hits 23220 23383 +163
- Misses 623 626 +3
- Partials 394 396 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements plan 153 by updating the MDS019 (<?catalog?>) directive to allow glob patterns containing .. segments, as long as the resolved glob stays within the configured project root (mirroring <?include?> behavior).
Changes:
- Refactors catalog glob rooting/resolution to be project-root-aware (including new diagnostics for “escapes root” and “missing root” cases).
- Extracts shared path utilities into
internal/globpath(ResolveAgainstRoot,ContainsDotDotSegment) and adds unit tests. - Updates docs/fixtures and adjusts the integration fixture runner so MDS019 fixtures have
RootFSset.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/153_catalog-dotdot-globs.md | Marks the plan complete and documents what landed. |
| PLAN.md | Updates the plan index to show plan 153 as completed. |
| internal/rules/MDS019-catalog/README.md | Documents new .. behavior + updated diagnostics. |
| internal/rules/MDS019-catalog/good/dotdot.md | Adds a passing fixture covering .. staying inside root. |
| internal/rules/MDS019-catalog/bad/dotdot.md | Adds a failing fixture covering a .. glob that escapes root. |
| internal/rules/catalog/rule.go | Implements project-root-aware glob resolution and new diagnostics. |
| internal/rules/catalog/rule_test.go | Updates expectations and adds coverage for accept/reject cases. |
| internal/integration/rules_test.go | Ensures MDS019 fixtures have RootFS pinned so .. resolution works. |
| internal/globpath/globpath.go | Adds shared helpers for .. detection and root-safe resolution. |
| internal/globpath/globpath_test.go | Adds tests for the new globpath helpers. |
| .claude/skills/solid-architecture/SKILL.md | Replaces hand-maintained link defs with a <?catalog?> block using .. globs. |
- README: missing-root diagnostic table entry now matches the actual rule message (added "is"). - catalog: resolveBaseRel only rejects real ".." traversal, not filenames coincidentally starting with "..". - Two cosmetic comment/plan typos where ".." was followed by ":".
Two follow-up fixes from Copilot review on PR #275: - Patterns like {..,sibling}/*.md previously slipped past the containment check because path.Clean does not expand braces, so doublestar would silently match nothing for the .. branch. Reject these at validate time with a clear "rewrite as separate patterns" message. - When the catalog-owning file is supplied via an absolute path (e.g. CLI glob expansion) and a project RootDir is configured, derive the file directory via Rel(RootDir, Dir(f.Path)) so .. globs resolve against the real project root instead of bailing out. Files outside the configured root surface the existing missing-root diagnostic rather than silently failing. Adds direct tests for the new helpers (containsDotDotInsideBraces, braceSegmentBoundary, projectRelFileDir, displayPath) and an exclude-escapes-root scenario, lifting coverage on the new code paths to 100%.
- internal/integration: MDS019 fixtures now also set f.RootDir so the catalog rule's gitignore base resolves the same way it does in real runs. Previously RootDir was nil and gitignoreBase silently degraded to "" (gitignore filtering disabled) even when GitignoreFunc was set. - internal/globpath: fix a stray colon in a doc comment.
- Add a separate "catalog file is outside project root" diagnostic so users do not see a misleading "project root is not configured" message when RootFS *is* set but the file's path cannot be related to RootDir (e.g. it lives above the configured root or on a different Windows volume). - Stop ignoring errors from filepath.Abs/Rel in projectRelFileDir: Windows cross-volume Rel can fail, and quietly returning a bogus rel path would let the safety check pass for unrelated paths.
The Runner passes file paths through verbatim from the command line, so a relative f.Path may be CWD-relative rather than RootDir-relative (e.g. running `mdsmith check index.md` from a subdirectory). Previously projectRelFileDir treated any non-absolute f.Path as already root-relative, which made `..` glob resolution and the resulting display paths incorrect for that invocation style. Mirror duplicatedcontent.rootRelative: when RootDir is set, absolutize f.Path first and then compute Rel against RootDir. With RootDir empty the path is still assumed to be root-relative, matching the prior non-CLI usage. Update the MDS019 integration fixture runner to pass the fixture's absolute path as f.Path so the new absolutize-first logic lands on the fixture directory instead of the test process CWD. Adds a regression test that chdirs into a subdirectory of the project root and lints a CWD-relative path through Fix.
When source-dir is invalid (absolute or escaping), the rule used to fall back to localFSResolution unconditionally. For ".." patterns that fallback can't resolve the segment at all, so the user saw a silent empty match instead of the intended root-aware diagnostic. Keep the existing source-dir-only fallback (no dotdot) but, when patterns contain "..", ignore the bad source-dir and continue resolution against the file's directory so escape-root / outside-root diagnostics still fire when warranted. Also fix a stale test comment that referred to the prior f.FS fallback for the outside-root case (now replaced by the dedicated "catalog file is outside project root" diagnostic).
The original loop only asserted the absence of the dotdot/root-escape messages, so the test would have passed even if Check returned no diagnostics at all. Pin it to exactly one drift diagnostic and assert the expected "out of date" message, then keep the negative assertions on that diagnostic.
jeduden
pushed a commit
that referenced
this pull request
May 14, 2026
Copilot flagged that <?include?> on pattern/bad and pattern/good rewrote relative link targets so the displayed paths (e.g. pattern/bad/data/alpha.md) did not match the source files (data/alpha.md). The rewrite is stable so mdsmith fix did not flag the body as stale, but the cosmetic mismatch confuses a reader trying to copy the snippet verbatim. Drop the include directives from the ## Pattern sections. The snippets in each directive README now match the source pattern/ files character-for-character, and the prose points to pattern/bad and pattern/good as the canonical folders the integration test still enforces. Also add bad/.nofix on MDS019 for the new dotdot.md fixture from main (#275). The dotdot escape is a validation-only diagnostic with no auto-fix. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP
jeduden
pushed a commit
that referenced
this pull request
May 14, 2026
Copilot flagged that <?include?> on pattern/bad and pattern/good rewrote relative link targets so the displayed paths (e.g. pattern/bad/data/alpha.md) did not match the source files (data/alpha.md). The rewrite is stable so mdsmith fix did not flag the body as stale, but the cosmetic mismatch confuses a reader trying to copy the snippet verbatim. Drop the include directives from the ## Pattern sections. The snippets in each directive README now match the source pattern/ files character-for-character, and the prose points to pattern/bad and pattern/good as the canonical folders the integration test still enforces. Also add bad/.nofix on MDS019 for the new dotdot.md fixture from main (#275). The dotdot escape is a validation-only diagnostic with no auto-fix. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP
jeduden
pushed a commit
that referenced
this pull request
May 14, 2026
Copilot flagged that <?include?> on pattern/bad and pattern/good rewrote relative link targets so the displayed paths (e.g. pattern/bad/data/alpha.md) did not match the source files (data/alpha.md). The rewrite is stable so mdsmith fix did not flag the body as stale, but the cosmetic mismatch confuses a reader trying to copy the snippet verbatim. Drop the include directives from the ## Pattern sections. The snippets in each directive README now match the source pattern/ files character-for-character, and the prose points to pattern/bad and pattern/good as the canonical folders the integration test still enforces. Also add bad/.nofix on MDS019 for the new dotdot.md fixture from main (#275). The dotdot escape is a validation-only diagnostic with no auto-fix. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP
jeduden
added a commit
that referenced
this pull request
May 14, 2026
* Enforce bad/good/fixed examples for directive rules Every rule that implements gensection.Directive (MDS019 catalog, MDS021 include, MDS038 toc, MDS039 build) must ship bad/, good/, and — when fixable — fixed/ example folders. A new integration test in internal/integration/directive_examples_test.go walks rule.All() and fails when any of those folders are missing or when bad/ has a fix-able entry with no matching fixed/ counterpart. Non-fixable bad fixtures (cycle detection, build validation) opt out via a bad/.nofix sentinel file. Backfills MDS019-catalog/fixed/ so the loop tests the catalog fix path, and adjusts the MDS019 bad fixture so the post-fix body satisfies the default-enabled blank-line-around-lists rule. The markdown-audit skill (.claude/skills/markdown-audit/) now points at these example folders as the single source of truth for the <?catalog?> and <?include?> fix recipes. Check 6 (kind without schema) gains an inline-schema vs proto.md section that links to the four MDS020 example flavors so the audit reader picks the right shape per kind. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Add pattern/ folders showing authored→directive transformations The bad/good folders under each rule test diagnostics — bad/* must fire a diagnostic, good/* must pass all rules. That layer is the wrong shape for "what does the hand-maintained anti-pattern look like before someone adopts this directive?", which is what the markdown-audit skill needs to show users. Each directive rule (MDS019 catalog, MDS021 include, MDS038 toc, MDS039 build) now ships a separate pattern/bad/ and pattern/good/ pair: - pattern/bad — the user-authored Markdown someone would write today without the directive (hand-maintained index, duplicated section, hand-maintained TOC, hand-maintained build output snippet). - pattern/good — the same content rewritten with the directive, plus any sibling files the directive needs. The four directive rule READMEs grew a ## Pattern section that <?include?>s the new fixtures. A new directive-rule-readme kind (with proto at internal/rules/directive-proto.md) layers a stricter schema over rule-readme that requires that Pattern section with Without/With subsections. Until kind-schema composition lands (plan/156_kind-schema-composition.md), the directive-rule-readme schema duplicates rule-readme's structure and the four directive READMEs are excluded from the rule-readme glob so only one schema resolves per file. TestDirectiveRulesHaveExamples now also requires pattern/bad/*.md and pattern/good/*.md for every gensection.Directive rule. The markdown-audit skill patterns.md + SKILL.md point at pattern/ as the canonical before/after pair. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Plan 157: catalog filter by front matter property Files the use case (a "Directive rules" listing inside internal/rules/index.md that selects on a nature property) and the proposed surface (a where: parameter on the catalog directive that reuses the CUE expression matcher from mdsmith list query). https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Inline ## Pattern snippets in directive rule READMEs Copilot flagged that <?include?> on pattern/bad and pattern/good rewrote relative link targets so the displayed paths (e.g. pattern/bad/data/alpha.md) did not match the source files (data/alpha.md). The rewrite is stable so mdsmith fix did not flag the body as stale, but the cosmetic mismatch confuses a reader trying to copy the snippet verbatim. Drop the include directives from the ## Pattern sections. The snippets in each directive README now match the source pattern/ files character-for-character, and the prose points to pattern/bad and pattern/good as the canonical folders the integration test still enforces. Also add bad/.nofix on MDS019 for the new dotdot.md fixture from main (#275). The dotdot escape is a validation-only diagnostic with no auto-fix. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Inline ## Examples snippets in MDS019 README Copilot flagged that the <?include?> for bad/default.md in the ## Examples section displayed paths with a `bad/` prefix (`[data/alpha.md](bad/data/alpha.md)`), while the source fixture uses `[data/alpha.md](data/alpha.md)`. The displayed body IS what `mdsmith fix` produces — the include rule's adjustLinks rewrites relative link targets so they resolve from the host file's directory. The body is stable, not stale. But the cosmetic mismatch makes the example misleading, and the row template `{filename}` placeholder gets the same rewrite applied. Replace both <?include?> blocks with inline snippets that match the fixture files character-for-character; link to good/default.md and bad/default.md by path for readers who want the source. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Add where: filter to catalog directive Extend <?catalog?> with a `where:` parameter that evaluates a CUE expression against each matched file's parsed front matter, dropping non-matching files before sort and render. Reuses the matcher from internal/query/ so the catalog and `mdsmith list query` accept the same grammar. Invalid CUE expressions emit an MDS019 diagnostic on the directive's opening line; missing fields and type mismatches silently exclude the file, matching list-query semantics. Covers tasks 1, 5, and 6 from plan/157. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Add nature key to rule-readme schema and backfill READMEs Add a required `nature` front-matter key to the rule-readme schema (internal/rules/proto.md) and the directive-rule-readme schema (internal/rules/directive-proto.md). The field labels each rule's kind so the catalog directive can filter listings by it. Allowed values: directive, generator, content, style, structure. Backfill `nature:` on every MDS rule README and add a fixture under MDS020 (bad/nature-missing.md) that exercises the new schema requirement. Covers tasks 2 and 3 from plan/157. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Add Directive rules section to internal/rules/index.md Uses the new catalog where: filter (plan 157 task 1) and the nature front-matter property (plan 157 tasks 2 + 3) to surface a filtered listing of just the four directive rules. Closes the remaining acceptance criterion in plan/157. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP * Use catalog where: filter in markdown-audit SKILL Replace the hand-maintained list of four directive rule pattern folders with a <?catalog?> directive that selects every rule README with nature: directive in its front matter. The list now stays in sync with the rule catalog automatically — a new directive rule starts showing up once its README declares nature: directive. https://claude.ai/code/session_01K7ZGjbkxFHTdTekshbN3kP --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement plan 153 to allow
..segments in catalog glob patterns as long as the resolved pattern stays within the project root. This mirrors how the<?include?>directive resolves itsfileparameter.Key Changes
Refactored glob resolution logic (
resolveGlobFS): Replaced the simplecontainsDotDotrejection with a project-root-aware resolution that:..segments in patterns via newdotDotInPatternshelper..is present orsource-diris setglobResolutionstruct encapsulating the resolved fs.FS, rewritten patterns, and metadata"glob escapes project root"diagnostic..patterns when project root is not configured with"glob contains ".." but project root is not configured"diagnosticExtracted shared path-resolution helpers to
internal/globpath:ResolveAgainstRoot: Resolves a path against a base directory and detects escape attemptsContainsDotDotSegment: Checks for..path elements (replaces removedcontainsDotDot)Simplified gitignore handling (
resolveGitignoreMatcher): Removed source-dir-specific logic since glob resolution now handles path rewriting centrallyUpdated test expectations: Changed error messages from
".." path traversalto the new diagnostic messages; added tests for both accept case (../sibling/*.mdstaying inside root) and reject case (../../secret/*.mdescaping root)Updated documentation (MDS019 README): Clarified that
..is allowed within project root, documented the new diagnostics, and aligned wording with MDS021 include behaviorAdded integration fixtures:
good/dotdot.mdandbad/dotdot.mdunderinternal/rules/MDS019-catalog/to exercise the new behaviorUpdated integration test runner (
attachFixtureFS): PinsRootFS = FSfor MDS019 fixtures so..resolution works in testsRegenerated SKILL.md: Added
<?catalog?>block targeting architecture docs with../../../docs/development/architecture/*.mdglob, replacing hand-maintained reference-style link definitionsImplementation Details
The refactored
resolveGlobFSnow returns aglobResolutionstruct that encapsulates:This centralizes path resolution logic and makes it easier to reason about when patterns are rewritten and which filesystem is used. The
displayPathmethod handles converting root-relative matches back to paths relative to the catalog-owning file's directory.https://claude.ai/code/session_019Dtf2aTG3WvQuGLfEjmGy7