[BUGFIX] Skip symbolic links instead of aborting the file listing

[CybotTM]

Problem

FlySystemAdapter::createForPath() builds the Local adapter with the default link-handling mode, which is DISALLOW_LINKS in both league/flysystem v1 and v3. Any symbolic link met during listContents() aborts the caller — v1 with NotSupportedException, v3 with SymbolicLinkEncountered inside UnableToListContents.

ParseDirectoryHandler uses that listing to find the entrypoint of an input directory, so one symlink anywhere in the tree kills the whole render, including links to files the parser would never read — CLAUDE.md -> AGENTS.md for cross-tool AI instructions, vendored references, build artefacts. Excluding the path does not help: the listing walks it before any exclusion applies.

Downstream report in the TYPO3 render-guides wrapper: TYPO3-Documentation/render-guides#1234.

Fix

SKIP_LINKS as the linkHandling argument on both adapters. Two lines.

-            $filesystem = new FlysystemV1(new LeagueFilesystem(new Local($path)));
+            $filesystem = new FlysystemV1(new LeagueFilesystem(new Local($path, linkHandling: Local::SKIP_LINKS)));
         } else {
             $filesystem = new FlysystemV3(
                 new LeagueFilesystem(
-                    new LocalFilesystemAdapter($path),
+                    new LocalFilesystemAdapter($path, linkHandling: LocalFilesystemAdapter::SKIP_LINKS),

The stance towards symbolic links is unchanged — they are still not followed. Only the reaction changes, from aborting the run to leaving the entry out of the listing.

DISALLOW_LINKS with the exception caught at the call sites would have been the alternative. SKIP_LINKS is narrower: it fixes every caller at once and needs no error handling in ParseDirectoryHandler or in any future consumer.

What a missing document still looks like

Skipping is not silent where it matters. A symlinked document referenced from a toctree is reported by the existing menu resolution, naming both the entry and the file that references it:

app.WARNING: Menu entry "chapter/page" was not found in the document tree. Ignoring it. {"rst-file":"index.rst"}

An earlier revision of this branch added a dedicated warning for skipped links. It was dropped: it restated what the menu already says, and it fired for links whose target does not exist — the kind a partial composer install --no-dev leaves in vendor/bin — turning a passing build into a failing one for a document that was never missing.

One case is not improved by this PR and is worth naming: when the index file itself is a symlink, the run still aborts, now with Could not find an index file instead of the Flysystem exception. The file is sitting in plain sight, so that message is misleading. Fixing it needs the handler to explain why the file was not seen, which is a separate change.

Tests

packages/filesystem/tests/unit/FlySystemAdapterTest.php lists a directory holding a regular file next to a symbolic link and asserts the regular file is listed while the link is not.

The guard was checked against a reverted fix on both branches: without SKIP_LINKS it fails with NotSupportedException on flysystem v1 and with SymbolicLinkEncountered on v3.

Run locally across the full CI matrix, 5 PHP versions times lowest/locked/highest, 15 cells, 829 tests each, no failures. The lowest column resolves flysystem to 1.1.4 and exercises the v1 branch; locked and highest run v3.

PHP	lowest (v1)	locked (v3)	highest (v3)
8.1	pass	pass	pass
8.2	pass	pass	pass
8.3	pass	pass	pass
8.4	pass	pass	pass
8.5	pass	pass	pass

Reproduction

ln -s AGENTS.md Documentation/CLAUDE.md
docker run --rm -v "$PWD:/project" -w /project \
  ghcr.io/typo3-documentation/render-guides:latest \
  render --config=Documentation --output=out Documentation

Fails on main, passes with this branch.

Assisted by claude-code:claude-fable-5 — Session

[linawolf]

SKIP_LINKS affects every listContents() call on this filesystem, not just the index lookup. FileCollector::collect() uses the same adapter (via Flyfinder's recursive listContents()) to build the whole parse queue, so a symlinked .rst file anywhere in the tree — not just the index — is now silently dropped with no warning. listSkippedLinks() is only wired into the index-not-found error path, so this case never surfaces. Should FileCollector also check listSkippedLinks() and warn when non-empty?

[CybotTM]

You are right, and the missing warning was a deliberate omission I now think was the wrong call: FileCollector had no logger, and I did not want to add one as a new constructor argument. That is not a good enough reason to let a document disappear from the output silently, so 0998496 adds it.

FileCollector takes an optional LoggerInterface defaulting to a NullLogger, the same shape ParseDirectoryHandler already uses for its SettingsManager. It is wired through inline_service(FileCollector::class)->autowire(), so no container change was needed. After find() it asks the filesystem what was skipped and warns when the list is not empty.

That required a second change: listSkippedLinks() was deliberately non-recursive, because the index lookup only ever needs one directory. Collecting walks the whole tree, so the method gained a bool $recursive = false parameter. The default is unchanged, and the recursion records a linked directory rather than descending into it, so a cycle of links cannot be walked into.

The warning is not filtered by extension. Every symbolic link under the collected directory is named. Before this branch any of them aborted the whole run, so naming all of them is strictly quieter than the old behaviour.

Tests: FlySystemAdapterTest covers the recursive and the non-recursive case, FileCollectorTest covers the warning and its absence. Both new guards were checked against a reverted fix: disabling the recursion fails testItNamesTheSkippedSymbolicLinksBelowTheGivenDirectory, disabling the warning call fails testCollectWarnsAboutTheSymbolicLinksThatWereNotCollected.

One thing I noticed while wiring this up, unrelated to this branch: psr/log sits in require-dev of packages/guides/composer.json while LoggerInterface is a required constructor argument in production classes such as ParseFileHandler. It resolves today because symfony/http-client pulls psr/log in, and composer-require-checker is present in .phive/phars.xml but is not invoked by CI or a composer script, so nothing flags it. Happy to open a separate issue for that if you want it tracked.

Assisted by claude-code:claude-fable-5 — Session

[linawolf]

Yes it is a good idea to move psr/log to the production requirements in a separate PR. As long as the requirements are no stricter then what symfony/http-client currently requires it can cause no breaking change but is cleaner for current updates

[CybotTM]

A review of this branch turned up a build-breaking regression in the warning I added, plus a test of mine that guarded nothing. 05fb4f2 fixes both. Reproductions below, all on the same tree, base branch versus this branch.

The regression. Flysystem does not merely skip a symbolic link whose target is missing, it ignores it entirely: LocalFilesystemAdapter catches SymbolicLinkEncountered and only rethrows when file_exists() on the link is true. So a dangling link never reached the listing and never cost anything. My warning named it anyway, and since any warning ends this CLI with exit 1, a tree containing one went from passing to failing with no document missing anywhere. A partial composer install --no-dev or a stale node_modules/.bin leaves exactly such links behind.

tree: docs/index.rst + docs/vendor/bin/dangling -> (missing target)
main          exit 0
before fix    exit 1   "symbolic links and were skipped: vendor/bin/dangling"
after fix     exit 0

For contrast, an unrelated warning (an unresolvable :ref:) exits 1 on main too, so the "any warning fails the build" part is pre-existing and not something this branch introduces.

The fix. Links are now held against the very specification the listing used, instead of a second rule invented next to it. A foreign extension or an excluded path therefore stays quiet, and Flyfinder's directory pruning is mirrored via CompositeSpecification::thatCanBeSatisfiedBySomethingBelow() rather than reimplemented. The adapter drops a link whose target does not exist, and answers a directory outside the root with nothing, the way listContents() refuses one with PathTraversalDetected. A genuinely symlinked .rst is still reported, which is the case the feature exists for.

The test that guarded nothing. testItNamesTheSkippedSymbolicLinksBelowTheGivenDirectory asserted that linked-dir/real.rst was absent, to prove a linked directory is not descended into. real.rst is a regular file and the method returns links only, so that path could never appear and deleting the continue that implements the guard kept the suite green. The fixture now puts a link inside the linked directory and the assertion is on that.

Every new assertion was checked by disabling its fix and watching it fail: dangling filter, traversal guard, non-descent, extension filter, exclusion filter — five for five, then restored.

Full suite 835 tests 0 failures, PHPStan clean, PHPCS clean.

Two things I did not fold in. The undeclared psr/log this branch surfaced is now its own change, #1354, since it affects seven packages and not just this one. And listSkippedLinks() still costs a second directory walk (measured about +23% on collect for this repository, 55 ms against 297 ms); observing the links during the listing itself would remove that along with the need to re-derive the exclusion, but it is a larger change than this bug fix and I would rather propose it separately than smuggle it in here.

Assisted by claude-code:claude-fable-5 — Session

[CybotTM]

I have cut this branch back to the two-line change it started as. The diagnostics I built on top are gone: no listSkippedLinks(), no logger in FileCollector, no enriched index-not-found message. What remains is SKIP_LINKS on both local adapters plus one test that fails with SymbolicLinkEncountered inside UnableToListContents when the flag is removed — the exact abort reported downstream.

@linawolf, your question deserves a straight answer rather than the growth it triggered. You were right that the diagnostic was wired into one path while SKIP_LINKS affects every listing. I took that as "wire it everywhere", and the right answer was "take it out".

The reason is that the pipeline already reports the case. With the warning disabled, a symlinked document referenced from a toctree produces:

app.WARNING: Menu entry "chapter/page" was not found in the document tree. Ignoring it. {"rst-file":"index.rst"}

That names the referencing file and the entry, which my warning did not. The only case it did not cover is a symlinked document nothing references, and such a document produces no menu entry and no link either way.

It also cost more than it was worth. The warning fired for links whose target does not exist — the kind a partial composer install --no-dev leaves in vendor/bin — and since any warning ends this CLI with exit 1, a tree holding one went from exit 0 to exit 1 with no document missing anywhere. Fixing that pulled in an extension filter, the exclusion specification, Flyfinder's directory pruning, a path-traversal guard and a second full directory walk costing about 23% on collect. All of that to restate something the menu already says.

Two findings from that work stand on their own and are not folded back in:

psr/log is used in the production code of seven packages without being declared by any of them, which is #1354 and independent of this branch.

GuidesExtension tests its config with isset($config['fail_on_log']), and isset() is true for the value false, so this repository's own guides.xml carrying fail-on-log="false" turns the setting on rather than off. I have not touched it; say the word and I will open an issue.

Assisted by claude-code:claude-fable-5 — Session

[linawolf]

The solution as is now seems better to me. Did you check what happens if you use a symlink in an include or literalinclude?

[CybotTM]

Checked, and both work — but only with this branch. On main the same document cannot be built at all.

Fixture: docs/index.rst with .. include:: part.rst and .. literalinclude:: code.php, where both part.rst and code.php are symbolic links pointing outside the input directory.

main         UnableToListContents / SymbolicLinkEncountered, exit 1 — the run never reaches the parser
this branch  exit 0, both markers present in index.html

So include and literalinclude follow symbolic links exactly as before, and this branch turns the case from "whole build aborts" into "renders normally".

The reason they are unaffected is that SKIP_LINKS is a listing-time option. IncludeDirective.php:50,56 and LiteralincludeDirective.php:47,53 resolve their target through $origin->has($path) and $origin->read($path), neither of which goes through listContents(), so link handling never enters the picture. The same holds for anything else reading a known path rather than discovering it — only discovery is affected, which is the parse queue and toctree globs.

One consequence worth stating explicitly, since it is the flip side of the same coin: a symbolic link is no longer discovered, so a linked .rst that is not reached by an include or a toctree entry simply does not become a document. If a toctree does point at it, the existing menu resolution reports it by name:

app.WARNING: Menu entry "chapter/page" was not found in the document tree. Ignoring it. {"rst-file":"index.rst"}

Assisted by claude-code:claude-fable-5 — Session