Skip to content

Simplify scope root normalization for nested extension roots#3720

Merged
Mpdreamz merged 1 commit into
mainfrom
simplify-scope-root-normalization
Jul 24, 2026
Merged

Simplify scope root normalization for nested extension roots#3720
Mpdreamz merged 1 commit into
mainfrom
simplify-scope-root-normalization

Conversation

@Mpdreamz

@Mpdreamz Mpdreamz commented Jul 24, 2026

Copy link
Copy Markdown
Member

Why

Follow-up to #3719. That PR fixes the ScopedFileSystem disjoint-roots crash for nested Codex configs by introducing a generic NormalizeDisjointRoots that sorts all roots by length and collapses parent–child pairs. This treats WorkingDirectoryRoot, ApplicationData, and extension roots as equals, but WorkingDirectoryRoot and ApplicationData are fixed and always disjoint — only extension roots can collide with WorkingDirectoryRoot.

Root cause

The codex-environments repo has configs nested inside the checkout:

/home/runner/work/codex-environments/codex-environments/   ← WorkingDirectoryRoot (has .git)
├── .git/
├── environments/
│   └── internal/
│       └── config.yml   ← config passed to `docs-builder codex clone`
└── ...

When a Codex command runs, it calls:

FileSystemFactory.ScopeCurrentWorkingDirectory(fs, [Paths.FindGitRoot(config.FullName)]);

FindGitRoot starts from the config file's directory and walks up looking for .git, but has a depth-1 limit in release builds. The .git is 2 levels up:

environments/internal/   ← depth 0: no .git
environments/            ← depth 1: no .git
repo root/               ← depth 2: .git found, but depth > 1 → REJECTED

Since .git is "too deep", FindGitRoot falls back to returning startDir — the config's own directory:

FindGitRoot("…/environments/internal/config.yml")
  → "/home/runner/…/codex-environments/environments/internal"

Now the original ScopeCurrentWorkingDirectory builds scope roots:

roots = [
  WorkingDirectoryRoot  →  "/home/runner/…/codex-environments"          ← ROOT
  ApplicationData       →  "/home/runner/.local/share/elastic/docs-builder"
  extensionRoot         →  "/home/runner/…/codex-environments/environments/internal"  ← CHILD OF ROOT!
]

.Distinct() doesn't remove anything (they're different strings). roots.Length == 3 ≠ 2, so it doesn't hit the fast path. ScopedFileSystem then enforces that all scope roots must be disjoint (no parent–child relationships):

   ❌ BOOM: ancestor/descendant relationship detected

   /home/runner/…/codex-environments/           ← scope root A
   └── environments/
       └── internal/                            ← scope root B (child of A)

ScopedFileSystem throws here by design — nested roots create policy ambiguity (which root's hidden-file/folder allowlist wins?), so the library forces callers to provide disjoint roots.

What

Replace the generic shortest-first normalizer from #3719 with a targeted filter: drop extension roots that are already covered by WorkingDirectoryRoot using the existing IsSubPathOf extension. No new helper methods needed.

Includes the same two regression tests from #3719 (nested in-repo config, genuinely external config root).

@Mpdreamz
Mpdreamz requested a review from a team as a code owner July 24, 2026 11:34
@Mpdreamz Mpdreamz added the bug label Jul 24, 2026
@Mpdreamz
Mpdreamz requested a review from technige July 24, 2026 11:34
Filter extension roots against WorkingDirectoryRoot using the existing
IsSubPathOf extension instead of a generic shortest-first normalizer.
WorkingDirectoryRoot and ApplicationData are fixed and always disjoint;
only extension roots can collide, so only they need filtering.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@Mpdreamz
Mpdreamz force-pushed the simplify-scope-root-normalization branch from 2685be6 to a43362e Compare July 24, 2026 11:40
@Mpdreamz
Mpdreamz enabled auto-merge (squash) July 24, 2026 11:48
@Mpdreamz
Mpdreamz merged commit 3c5f970 into main Jul 24, 2026
24 checks passed
@Mpdreamz
Mpdreamz deleted the simplify-scope-root-normalization branch July 24, 2026 11:48
Mpdreamz added a commit that referenced this pull request Jul 24, 2026
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Mpdreamz added a commit that referenced this pull request Jul 24, 2026
* Emit Mermaid diagrams as external SVG files loaded via img (#3713)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Fix CopyBrandingResources failure during serve in-memory build (#3718)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix overlapping scope roots for nested Codex config paths (#3719)

docs-builder 1.29.0 added extension roots from FindGitRoot for Codex
commands. When config.yml lives under environments/internal/, the resolved
root is nested inside WorkingDirectoryRoot and ScopedFileSystem rejects
the overlapping roots. Normalize extension roots before constructing the
scoped filesystem so redundant nested paths are dropped.

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Simplify scope root normalization for nested extension roots (#3720)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Replace pagefind native binary with Pagefind.Net managed library

Replace PR #3709's native pagefind binary subprocess approach with the
Pagefind.Net and Pagefind.Net.Frontend NuGet packages — pure managed
.NET, no native binary, AOT-compatible.

The pagefind indexer is now a proper IMarkdownExporter that eagerly
indexes each page during ExportAsync (O(1) memory) and flushes the
index to disk in FinishExportAsync. It is included in both
ExportOptions.Default and ExportOptions.Validation, so it works in
isolated builds and docs-builder serve mode automatically.

Key changes:
- New PagefindMarkdownExporter in Elastic.Markdown/Exporters/Pagefind
- New Exporter.Pagefind enum value, registered in both ExporterParsers
- Removed native binary pipeline (package-pagefind.mjs, embedded
  resource, pagefind npm dep, PagefindSearchIndexer subprocess)
- Removed static-search feature flag — search is always available
  for isolated builds and serve mode
- Serve mode reads pagefind files from the background build's
  in-memory filesystem via a route handler
- Added .pagefind-net-frontend-version to AllowedHiddenFileNames

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Jan Calanog <jan.calanog@elastic.co>
Mpdreamz added a commit that referenced this pull request Jul 25, 2026
* Emit Mermaid diagrams as external SVG files loaded via img (#3713)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Fix CopyBrandingResources failure during serve in-memory build (#3718)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix overlapping scope roots for nested Codex config paths (#3719)

docs-builder 1.29.0 added extension roots from FindGitRoot for Codex
commands. When config.yml lives under environments/internal/, the resolved
root is nested inside WorkingDirectoryRoot and ScopedFileSystem rejects
the overlapping roots. Normalize extension roots before constructing the
scoped filesystem so redundant nested paths are dropped.

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Simplify scope root normalization for nested extension roots (#3720)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Replace pagefind native binary with Pagefind.Net managed library

Replace PR #3709's native pagefind binary subprocess approach with the
Pagefind.Net and Pagefind.Net.Frontend NuGet packages — pure managed
.NET, no native binary, AOT-compatible.

The pagefind indexer is now a proper IMarkdownExporter that eagerly
indexes each page during ExportAsync (O(1) memory) and flushes the
index to disk in FinishExportAsync. It is included in both
ExportOptions.Default and ExportOptions.Validation, so it works in
isolated builds and docs-builder serve mode automatically.

Key changes:
- New PagefindMarkdownExporter in Elastic.Markdown/Exporters/Pagefind
- New Exporter.Pagefind enum value, registered in both ExporterParsers
- Removed native binary pipeline (package-pagefind.mjs, embedded
  resource, pagefind npm dep, PagefindSearchIndexer subprocess)
- Removed static-search feature flag — search is always available
  for isolated builds and serve mode
- Serve mode reads pagefind files from the background build's
  in-memory filesystem via a route handler
- Added .pagefind-net-frontend-version to AllowedHiddenFileNames

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Jan Calanog <jan.calanog@elastic.co>
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.

2 participants