Skip to content

Let a TOC entry source its content from outside the docset root - #4020

Closed
clintandrewhall wants to merge 3 commits into
mainfrom
feat/toc-source-outside-docset-3798
Closed

Let a TOC entry source its content from outside the docset root#4020
clintandrewhall wants to merge 3 commits into
mainfrom
feat/toc-source-outside-docset-3798

Conversation

@clintandrewhall

@clintandrewhall clintandrewhall commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

A source: key on a file: or hidden: table of contents entry reads the page's content from a markdown file elsewhere in the repository. A page can now live next to the code it documents without moving the documentation set root.

Affects: Navigation, Authoring, Configuration

Prompt summary: Implement #3798. Kibana publishes internal developer docs to Codex from a docs-dev/ docset while the packages those docs describe live under src/platform/, and the maintainers want each page checked in beside the code it documents so a component change and its documentation land in the same review. The ask is to let a table of contents entry keep its docset-relative position while sourcing content from above the root.

Why

All table of contents paths resolve relative to the documentation set root, and a repository supports one docset per registry. Colocating a page with its code therefore means moving the docset root inside a package directory, which concedes the repository to a single internal docset. Keeping the docset where it is means the documentation cannot sit beside the code.

What

The new source: key

An entry keeps file: as its position in the documentation set and gains source: for where the content is read:

toc:
  - file: index.md
  - file: feedback.md
    source: ../packages/kbn-ui/feedback/feedback.md

file: still drives the URL, the navigation entry, the output path and the cross-repository link reference. source: is resolved against the directory holding the docset.yml or toc.yml that declares the entry, and nothing else. It works on a hidden: entry and on the folder: + file: form.

Separating position from content

DocumentationFile.RelativePath is the page's position in the set and no longer has to equal SourceFile relative to the root. A sourced page parses as though it lived at that position, so relative links, images and includes inside it resolve from the virtual path rather than from where the file sits on disk. Because generated URLs and output paths derive from a path that cannot contain ../, no page can write outside the output directory or emit ../ in a URL.

Registering a page the file scan never sees

The scan walks the documentation set root, so a sourced page has to be registered from the table of contents. MarkdownFileFactory keys it under its virtual path — the same path navigation resolves against — and registers it after everything else so it can be refused with a diagnostic when:

  • the source escapes the repository checkout, either lexically or through a symlink on its path
  • the source does not exist, or is not a markdown file
  • the source resolves back inside the documentation set root, where the scan already owns it
  • the file: position escapes the root, or is already held by a scanned file, an extension-generated page, or another sourced entry

A non-public docset can also declare its root index.md with source:; the root-index requirement accepts the registered position.

The edit link

The "Edit this page" link now derives from the file that was actually read instead of joining the docset directory with the position in the set. That is required for a sourced page, and it also corrects detection rule pages, whose .toml source produced a path with ../ segments in it.

ResolveFileRef rebuilds refs with with

Resolution previously reconstructed each ref through a three-arm new switch, which silently drops any key added to FileRef later. A with expression preserves the concrete ref type and every off-path key, so source: cannot be lost between parsing and navigation.

Verify

./build.sh unit-test
# TocSourceTests — parsing, resolution anchoring, and the two path-shape errors
# ExternalSourceTests — registration, output path, relative links, edit link, root index, and the five registration errors

Then try it against this repository's own docs, pointing an entry at a file above docs/:

dotnet run --project src/tooling/docs-builder -- --path docs --strict

Out of scope: single markdown files only. There is no source: on folder:, listing: or toc:, and no glob form, per the issue. A sourced entry's file: is now checked for containment, but the general case the issue describes under Alternative Solutions — a bare - file: ../../x.md with no source: — is still accepted with no check. The author offered to track that separately and this PR leaves it alone.

Risk: assembled builds clone a repository with a partial checkout of docs only. A source: above that path needs the repository's sparse_paths or checkout_strategy widened in config/assembler.yml, the same requirement the detection rules extension already has. Codex checkouts are not sparse, so the case in the issue works without a config change.

clintandrewhall and others added 2 commits September 2, 2026 14:16
A `source:` key on a `file:` or `hidden:` entry points at a markdown file elsewhere in the repository, so a page can live next to the code it documents without moving the documentation set root. `file:` stays the virtual position that drives the URL, navigation, output path and link reference; `source:` only says which file to read.

Relative links, images and includes inside a sourced page resolve from that virtual position, so a page cannot smuggle `../` segments into generated URLs or write output outside the output directory. The "Edit this page" link now derives from the file actually read rather than from the position in the set, which also corrects it for detection rule pages.

Closes #3798

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…straint

`SourceFullPath` is derived and absolute, so serializing a table of contents would leak a machine path into generated YAML.

Assembled builds clone a repository with a partial checkout of `docs` only, so a `source:` above that path needs `sparse_paths` or `checkout_strategy` widened for that repository.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Virtual-path traversal, symlink escape, root-index validation, and generated-page collisions remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds off-root Markdown sourcing while preserving each page’s virtual docset position.

Changes:

  • Adds source: parsing, validation, and registration.
  • Separates virtual paths from physical content paths.
  • Updates documentation and test coverage.
File summaries
File Description
tests/Elastic.Markdown.Tests/DocSet/ExternalSourceTests.cs Tests rendering and registration.
tests/Elastic.Documentation.Configuration.Tests/TocSourceTests.cs Tests TOC parsing and resolution.
src/Elastic.Markdown/Myst/MarkdownParser.cs Supports virtual parse paths.
src/Elastic.Markdown/IO/MarkdownFileFactory.cs Registers sourced files.
src/Elastic.Markdown/IO/MarkdownFile.cs Accepts virtual relative paths.
src/Elastic.Markdown/IO/ExternallySourcedMarkdownFile.cs Models sourced Markdown pages.
src/Elastic.Markdown/IO/DocumentationFile.cs Separates position from source.
src/Elastic.Markdown/HtmlWriter.cs Corrects edit links.
src/Elastic.Documentation/Extensions/IFileInfoExtensions.cs Adds containment checking.
src/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cs Improves sourced-file diagnostics.
src/Elastic.Documentation.Configuration/Toc/TableOfContentsYamlConverters.cs Parses source: entries.
src/Elastic.Documentation.Configuration/Toc/TableOfContentsItems.cs Stores source paths.
src/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cs Resolves and collects sources.
docs/documentation/isolated/navigation.md Documents navigation usage.
docs/documentation/isolated/configure/index.md Documents configuration rules.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 6
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +162 to +167
var sourceFile = context.ReadFileSystem.FileInfo.New(fileRef.SourceFullPath!);
if (!sourceFile.Exists)
{
context.Collector.EmitError(fileRef.Context, $"'source: {fileRef.Source}' does not exist.");
continue;
}
Comment on lines +169 to +170
var virtualRelativePath = fileRef.PathRelativeToDocumentationSet.OptionalWindowsReplace();
var file = new ExternallySourcedMarkdownFile(sourceFile, virtualRelativePath, _markdownParser, context);
{
if (fileOnly == "index.md")
return new IndexFileRef(fileOnly, fileOnly, false, children, placeholderContext);
return new IndexFileRef(fileOnly, fileOnly, false, children, placeholderContext) { Source = contentSource };
Comment on lines 59 to 63
Files = files
.Concat(additionalSources)
.Concat(externallySourced)
.Where(t => t.Item2 is not ExcludedFile)
.ToDictionary(kv => new FilePath(kv.Item1, context.DocumentationSourceDirectory), kv => kv.Item2)
Comment thread src/Elastic.Markdown/IO/ExternallySourcedMarkdownFile.cs Outdated
Comment on lines +55 to +59
private Task<MarkdownDocument> ParseFromFile(
IFileInfo path,
IFileInfo contentFile,
YamlFrontMatter? matter,
MarkdownPipeline pipeline,
`TocSourceTests` compared a mock filesystem path against a real `Path.GetFullPath`, which resolves on whichever drive the tests run from. Windows saw `C:\repo\...` against `D:\repo\...`. Assertions now compute their expected paths from the same filesystem the resolution used.

Four gaps in registering a sourced page:

A symlink inside the checkout passes the lexical containment check while resolving outside it. `SymlinkValidator.ValidateFileAccess` now guards the source and its ancestors, matching what the ordinary file scan and the control files already do.

`file:` is not itself checked for containment, so a virtual path could carry `../` into URL generation and write output outside the output directory. A sourced entry whose position escapes the documentation set root is now refused.

Collision detection compared the virtual path against files on disk and earlier `source:` entries, but not against pages an extension generates — those exist nowhere on disk, so a clash reached `ToDictionary` and threw. Sourced pages are now registered last and checked against every position already taken.

`ValidateRootIndexExists` only stats `<docset>/index.md`, so a non-public docset whose root index is sourced always failed. It now accepts the registered position too.

`ParseFromFile` groups its pipeline and validation flag into a `ParseMode`, which keeps it within the parameter limit and stops a caller pairing the minimal pipeline with full validation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Mpdreamz

Mpdreamz commented Sep 2, 2026

Copy link
Copy Markdown
Member

That risk section is spot on and exactly why I am weary of this change. We also do redirect detection for removed/renamed files.

This kind of breaks our contract that the url is the file. There are quite a few assumptions around that first principle.

The system is more designed around docset/toc.yml as the composable building blocks from which you can re-arange/assemble your final site.

Codex is much simpler it's fixed url pattern. But the system has to work for both.

As I understand it packages want their docs to live in the package repository?

I wonder if we should allow packages to define their own toc.yml files and the dev-docs can then reference those instead to compose it's overal nav?

Codex should be doing sparse clones, especially for Kibana. Although it's been forever since I benchmarked that vs shallow clones, @reakaleek can you remember?

We still need a way to ensure/declare this is a codex only thing? Codex docsets.yml have to set registry: key so we can determine if a doc set is intended for codex publishing.

@clintandrewhall

clintandrewhall commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@Mpdreamz I would certainly defer to your expertise and preference here. I was putting forward a "what about this?" agent-assisted PR to spur the conversation.

But yes: the purpose of this is have a docset that can reach out and collect documents from other places. In this case, kibana/docs-dev/docset.yml can include, say, kibana/src/platform/kbn-ui/callout/*.md... so the docs can be colocated with the code, (and therefore stay fresh).

You could opt for kibana/docs-dev/docset.yml referencing kibana/src/platform/kbn-ui/callout/docs/toc.yml... that would certainly be easy to maintain on our end.

@Mpdreamz

Mpdreamz commented Sep 2, 2026

Copy link
Copy Markdown
Member

No worries about submitting the PR, super helpful indeed to spur the conversation: working code > issue but in this case both are great :)

If that works for y'all too, let's explore what toc.yml in packages looks like to unlock package docs collocation.

@clintandrewhall

Copy link
Copy Markdown
Contributor Author

Lemme see if I can revise this PR, but I may close it (and the issue) in favor of another. Stay tuned!

@clintandrewhall

Copy link
Copy Markdown
Contributor Author

Closing in favour of the package-hosted toc.yml direction. Summarising what this PR established so the next one does not relitigate it.

Why the source: mechanism does not survive. @Mpdreamz is right that it breaks "the URL is the file". The invariant is load-bearing in more places than navigation — redirect detection for removed and renamed files keys on docset-relative paths, and source: makes a page's docset-relative path a name with no file behind it.

What a package-hosted toc.yml has to answer. Given kibana/docs-dev/docset.yml referencing kibana/src/platform/kbn-ui/callout/docs/toc.yml, the pages there either mount at a position declared in the referencing docset, or take their real repository path and produce /src/platform/… URLs. It will be the former, which means the virtual/physical split does not disappear — it moves from per file to once per subtree. The argument for it is that the split then lands on IsolatedTableOfContentsRef, which already rewrites child paths by prepending the TOC's own position, so it extends a composable building block instead of adding a parallel mechanism beside it. Worth settling explicitly rather than assuming the objection is answered.

Codex is not doing sparse clones today. CodexGitRepository.EnableSparseCheckout exists with no caller anywhere in the repo; only RepositorySourcesFetcher calls its own. So a Kibana Codex checkout currently has the whole package tree present. That cuts both ways for the new design: it works today without a config change, and if Codex adopts sparse clones the referenced package directories have to be in the sparse set.

One correction to this PR's description. It claimed the edit-link change also fixed detection rule pages. It does not. UrlPath.Join does no dot-segment removal, so the old code did emit docs/../rules/x.toml — but RFC 3986 §5.2.2 applies remove_dot_segments to absolute references too, so every compliant client normalises it before the request. The link works; it only reads as a defect. Cosmetic, not a bug.

Issue #3798 stays open and is being reframed around this direction — the problem is unchanged, only the proposed mechanism moves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants