Skip to content

fix(file): follow symbolic links in findFilesByGlobs - #1802

Merged
dyoshikawa merged 5 commits into
dyoshikawa:mainfrom
saitota:i1707
Jun 10, 2026
Merged

fix(file): follow symbolic links in findFilesByGlobs#1802
dyoshikawa merged 5 commits into
dyoshikawa:mainfrom
saitota:i1707

Conversation

@saitota

@saitota saitota commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Refs #1707

Summary

  • Change followSymbolicLinks: false to true in findFilesByGlobs

Scope

findFilesByGlobs is the single discovery function used by all features — rules, commands, subagents, hooks, and skills. This one-line fix covers every feature, not just skills.

Two distinct symlink scenarios are resolved:

  1. A symlinked skill/rule/command directory under .rulesync/ was not detected at all (original report by @mocyuto)
  2. A file inside a skill directory that is a symlink (e.g. pulled in from a dotfiles repo) was not exported (reported in comments by @saitota)

Design note

The alternative — preserving the symlink itself in the output — was considered but does not fit the architecture. findFilesByGlobs feeds into AiDir.collectOtherFiles, which reads every file as a Buffer and re-writes it as a plain file via writeFileContent. There is no layer in the pipeline that carries symlink metadata, so the output is always a regular file regardless. Following and copying is the natural behavior here.

Test plan

  • pnpm vitest run src/utils/file.test.ts — added two tests: symlinked file is found, symlinked directory and files inside it are found

saitota and others added 4 commits June 10, 2026 02:12
Symlinked skill directories and files were silently ignored because
globbySync ran with followSymbolicLinks: false. Setting it to true lets
users centralize shared skills in one location and reference them via
symlinks across multiple directories without duplication.
- Add inline comment explaining why followSymbolicLinks: true is safe
  for callers and how it differs from git-client.ts (which skips symlinks
  during remote fetch for unrelated reasons)
- Fix misleading test description comments
- Expand the Symlinks-and-trust section in separate-input-root.md to
  explain that file discovery now follows symlinks and what the trust
  boundary is
@github-actions

Copy link
Copy Markdown
Contributor

@saitota The CI checks on this pull request are currently failing.

Please review the failing checks and push a fix. If you need help, leave a comment and a maintainer will follow up.

@dyoshikawa
dyoshikawa merged commit 5e62dee into dyoshikawa:main Jun 10, 2026
5 checks passed
@dyoshikawa

Copy link
Copy Markdown
Owner

@saitota Thank you!

rudironsoni pushed a commit to rudironsoni/rulesync that referenced this pull request Jun 13, 2026
… model

Follow-ups from PR dyoshikawa#1802 review (issue dyoshikawa#1808):
- Deduplicate findFilesByGlobs results by real path so directory symlink cycles
  (which globby follows up to the kernel ELOOP limit) no longer yield ~40x
  duplicated entries that get read and re-emitted (dyoshikawa#1)
- Rewrite the findFilesByGlobs comment: git-client.ts skips symlinks as a
  security hardening for untrusted remote content (commit 51bf044), not because
  a bare clone makes them meaningless — it does a non-bare checkout (dyoshikawa#2)
- Document the intentional trade-off of not enforcing realpath containment
  (it would break the issue dyoshikawa#1707 shared-file use case); record the decision in
  the comment and docs rather than adding a containment check (dyoshikawa#3)
- Add a skills-processor test loading a symlinked skill directory (dyoshikawa#4)
- Guard symlink unit tests with skipIf(win32) since fs.symlink needs admin /
  Developer Mode on Windows (dyoshikawa#5)
- Add a feature-wide Symlinks section to docs/reference/file-formats.md and
  cross-link it from the input-root guide; sync skills/rulesync (dyoshikawa#6)

Closes dyoshikawa#1808

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants