Skip to content

fix: MockFileSystem.GetTempPath() returns real OS temp path by default#1454

Open
Mpdreamz wants to merge 4 commits intoTestableIO:mainfrom
Mpdreamz:fix/mock-filesystem-temp-path-uses-real-os-path
Open

fix: MockFileSystem.GetTempPath() returns real OS temp path by default#1454
Mpdreamz wants to merge 4 commits intoTestableIO:mainfrom
Mpdreamz:fix/mock-filesystem-temp-path-uses-real-os-path

Conversation

@Mpdreamz
Copy link
Copy Markdown

@Mpdreamz Mpdreamz commented Apr 1, 2026

Problem

MockFileSystem.Path.GetTempPath() returns a hardcoded path derived by mechanically converting the Windows constant C:\temp\ via MockUnixSupport.Path. On non-Windows this produces /temp/ rather than the standard /tmp/, and on any platform where the temp directory has been customised (TMPDIR on Linux/macOS, TEMP/TMP on Windows) it will also diverge from what System.IO.Path.GetTempPath() actually returns.

This became apparent while developing scoped-filesystem, a library that restricts file access to known locations and supports injection of special folders like the temp directory. It uses System.IO.Path.GetTempPath() to determine allowed prefixes and mockFs.Path.GetTempPath() to produce paths inside those prefixes. When the two values diverge, paths that should be permitted are rejected — failures that are easy to miss because the mock returns a well-formed, rooted, separator-terminated path with no hint that it differs from reality.

Fix

Replace the hardcoded constant with System.IO.Path.GetTempPath() when initialising the default temp directory. Callers who need a fixed, reproducible temp path can still pass an explicit value via MockFileSystemOptions or the MockPath constructor — that behaviour is unchanged.

Test

Added GetTempPath_Default_MatchesRealOsTempPath which asserts that new MockFileSystem().Path.GetTempPath() equals System.IO.Path.GetTempPath().

…fault

Replace the hardcoded C:\temp\ constant (converted via XFS.Path) with
System.IO.Path.GetTempPath() so the default mirrors the actual environment.
Callers can still override by passing an explicit temp directory.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 1, 2026 07:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes MockFileSystem.Path.GetTempPath() so the mock’s default temp directory matches the real OS temp directory (System.IO.Path.GetTempPath()), improving parity with real-world behavior (especially on non-Windows and when temp is customized via environment variables).

Changes:

  • Update MockFileSystem default temp directory initialization to use System.IO.Path.GetTempPath() instead of a hardcoded C:\temp\-derived value.
  • Add a unit test asserting the default mock temp path matches System.IO.Path.GetTempPath().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileSystem.cs Switches the default temp directory used by MockFileSystem.Path to the real OS temp path.
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockPathTests.cs Adds a regression test validating the new default behavior matches System.IO.Path.GetTempPath().

Mpdreamz added a commit to elastic/docs-builder that referenced this pull request Apr 1, 2026
0.4.0 exposes ScopedFileSystem.InnerType, removing the need to check
the outer wrapper's type name when inspecting the underlying filesystem.

FileSystemFactory.BuildWriteOptions: use sf.InnerType where the inner FS
is available through a ScopedFileSystem to check for MockFileSystem, with
a note linking to the upstream fix PR
(TestableIO/System.IO.Abstractions#1454).

GitCheckoutInformation.Create: replace fileSystem.GetType().Name.Contains("Mock")
with (fileSystem is ScopedFileSystem sf ? sf.InnerType : fileSystem.GetType())
so the check correctly sees through the ScopedFileSystem wrapper.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Addresses CoPilot review feedback: the PR description referenced
MockFileSystemOptions as a way to set a custom temp directory, but the
option did not exist. Added TemporaryDirectory (nullable string) that,
when set, is used instead of System.IO.Path.GetTempPath().

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Mpdreamz added a commit to elastic/docs-builder that referenced this pull request Apr 1, 2026
* Wrap all IFileSystem usage in ScopedFileSystem

Introduces `Nullean.ScopedFileSystem` (v0.2.0) as a security boundary
around every file system operation in the codebase.

`ScopedFileSystem` is a `System.IO.Abstractions.IFileSystem` decorator
that enforces at runtime that no file read or write can escape a set of
configured root directories. Any path outside the roots — including paths
reached via symlink, `..` traversal, or hidden directories — throws a
`ScopedFileSystemException` (extends `SecurityException`) before the
underlying OS call is ever made. `File.Exists` / `Directory.Exists` return
`false` for out-of-scope paths instead of throwing, so probing code stays
safe. The library supports multiple disjoint roots, an allowlist for
specific hidden names (e.g. `.git`), and opt-in OS special-folder access
(e.g. `Temp`).

Changes:
- New `FileSystemFactory` in `Elastic.Documentation.Configuration` with:
  - `FileSystemFactory.Real` — pre-allocated singleton scoped to the
    working-directory root and `Paths.ApplicationData`
    (`LocalApplicationData/elastic/docs-builder`), with `.git`
    folder/file allowlisted and `Temp` special-folder enabled
  - `FileSystemFactory.InMemory()` — wraps a fresh `MockFileSystem` in
    the same scope options; each call returns a new independent instance
  - `FileSystemFactory.CreateScoped(IFileSystem inner, ...)` — for
    wrapping an existing FS with optional extra roots from extensions
  - `FileSystemFactory.CreateForUserData()` — user-profile scope with
    `.docs-builder` allowlisted, used by `GitLinkIndexReader`
- All 33+ `new FileSystem()` call sites replaced with factory methods
- `CrossLinkFetcher` and `CheckForUpdatesFilter` converted from direct
  `System.IO.File` / `Directory` static calls to `IFileSystem` injection
- `IDocsBuilderExtension` gains `ExternalScopeRoots` (default `[]`);
  `DetectionRulesDocsBuilderExtension` implements it to expose the
  detection-rules folders so builds can widen the scope when needed
- `IFileSystem` registered as DI singleton (`FileSystemFactory.Real`)
  in `DocumentationTooling`
- Service constructors that previously required the concrete `FileSystem`
  type widened to `IFileSystem`

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

* Tighten ScopedFileSystem allowlists and consolidate app data paths

Fix a critical bug where real disk builds would throw ScopedFileSystemException
when writing to the default output directory (.artifacts/docs/html), because
.artifacts was missing from AllowedHiddenFolderNames.

Changes:
- FileSystemFactory.Real: remove speculative hidden-file allowances
  (.gitignore, .gitmodules, .gitattributes, .editorconfig, .nojekyll — none
  accessed via IFileSystem), remove AllowedSpecialFolders.Temp, add
  .artifacts (folder) and .doc.state (file) which are confirmed accessed
- FileSystemFactory.AppData: new pre-allocated FS scoped to only the elastic
  /docs-builder app data directory; used by components that never touch
  workspace files (CrossLinkFetcher, CheckForUpdatesFilter, GitLinkIndexReader)
- ConfigurationFileProvider: move temp staging directory from OS temp to
  {ApplicationData}/config-runtime/, eliminating the only Temp usage
- GitLinkIndexReader: normalize CloneDirectory from ~/.docs-builder/codex-link-index
  to {LocalAppData}/elastic/docs-builder/codex-link-index so all app data
  lives under one root; switch FS fallback from custom user-profile scope
  to FileSystemFactory.AppData
- CrossLinkFetcher: default FS changed to FileSystemFactory.AppData
- CheckForUpdatesFilter: use FileSystemFactory.AppData directly; remove
  IFileSystem DI injection (AppData is the correct and only scope needed)

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

* Split FileSystemFactory.Real into RealRead and RealWrite

Builds that write HTML output never need .git access. Introducing
RealWrite enforces this at the ScopedFileSystem level: any accidental
write into .git would throw ScopedFileSystemException rather than
silently succeeding.

RealRead retains .git access (needed by GitCheckoutInformation for
branch/remote metadata and worktree resolution), plus .artifacts and
.doc.state for incremental build state reads.

RealWrite has the same scope roots but omits .git from both
AllowedHiddenFolderNames and AllowedHiddenFileNames.

IsolatedBuildCommand now passes FileSystemFactory.RealWrite explicitly
as the writeFileSystem argument for non-in-memory builds. In-memory
builds pass null so IsolatedBuildService falls back to the same
MockFileSystem used for reads.

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

* Tighten GitLinkIndexReader scope and rename CreateScoped

Pass FileSystemFactory.AppData to GitLinkIndexReader at the three call
sites that previously passed context.ReadFileSystem or the build fileSystem
parameter. GitLinkIndexReader only ever accesses {AppData}/codex-link-index
so the workspace scope was unnecessarily broad. The IFileSystem? parameter
is retained for test injection.

Rename CreateScoped -> WrapToRead (read options, .git allowed) and add
WrapToWrite (write options, .git not allowed) to match the RealRead/RealWrite
naming convention. The extension-roots overload WrapToRead(inner, extensionRoots)
is the hook for DetectionRules external scope wiring.

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

* Enforce ScopedFileSystem at compile time across the codebase

Addresses the CodeRabbit concern: services accepted IFileSystem? with ??
defaults, allowing any unscoped instance to bypass the security boundary
silently. The fix is a compile-time contract — no runtime is/as checks.

Changes:
- FileSystemFactory: return types changed from IFileSystem to ScopedFileSystem
  for all factory members (RealRead, RealWrite, AppData, InMemory,
  WrapToRead, WrapToWrite). Logic unchanged; this is annotation only.
- IDocumentationContext: ReadFileSystem/WriteFileSystem typed ScopedFileSystem
- BuildContext: properties and constructor params typed ScopedFileSystem
- 12 optional IFileSystem? service parameters → ScopedFileSystem?
  (GitLinkIndexReader, CrossLinkFetcher, CsvReader, all changelog services,
  IsolatedBuildService.writeFileSystem)
- DocumentationSetFile.LoadAndResolve: ScopedFileSystem? with cast guard on
  the IFileInfo/IDirectoryInfo.FileSystem fallback
- AssembleContext, CodexContext, authoring services: updated to ScopedFileSystem
- All IDocumentationContext mock implementations in tests updated
- Tests: new MockFileSystem() → FileSystemFactory.WrapToRead(new MockFileSystem())
         new FileSystem()    → FileSystemFactory.RealRead

The compiler now rejects any call site that tries to pass an unscoped
new FileSystem() or bare MockFileSystem to a service boundary.

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

* Scope file systems to the path argument rather than CWD

RealRead/RealWrite are pre-allocated singletons scoped to
Paths.WorkingDirectoryRoot (the CWD git root). Commands that accept an
explicit --path or --output argument may target a repo outside that root,
which would cause ScopedFileSystemException.

Add FileSystemFactory.ForPath(string? path) and ForPathWrite(string? path,
string? output) that derive the scope root dynamically by walking up from
the given path to find its .git boundary. Both fall back to RealRead/RealWrite
when no path is provided (CWD-relative operation). ForPathWrite also adds
the output directory as an extra scope root when it falls outside the git root.

Update all commands that accept --path/--output to use these:
IndexCommand, IsolatedBuildCommand, ServeCommand, FormatCommand,
MoveCommand, DiffCommands, InMemoryBuildState, StaticWebHost.

Commands that always operate relative to CWD (assembler, codex, changelog)
continue using RealRead since their scope is determined by assembler config
or git context, not an arbitrary user-provided path.

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

* Consolidate git root discovery and remove Paths.GitCommonRoot static

Three changes:

1. FileSystemFactory.FindGitRoot removed — ForPath/ForPathWrite now call
   Paths.FindGitRoot, which is the single canonical git-root walker.

2. Paths.DetermineSourceDirectoryRoot simplified — the loop body had a
   dead branch (checked GetDirectories.Length == 0 in the while condition
   then re-checked inside). Rewritten to a clean while-loop matching
   FindGitRoot's structure; semantics unchanged.

3. Paths.GitCommonRoot static field and InitGitCommonRoot removed —
   the static ran complex logic (ScopedFileSystem creation + file read)
   at class init time and was only used by AssembleContext and CodexContext.
   Both now call Paths.ResolveGitCommonRoot(readFileSystem, workingRoot)
   directly, which is already public and takes explicit dependencies.
   Paths.cs no longer references Nullean.ScopedFileSystem.

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

* Fix scope gaps, depth-protect git root discovery, consolidate API

Move checkout directories to ApplicationData:
- AssembleContext: .artifacts/checkouts/{env} → ApplicationData/checkouts/{env}
- CodexContext:    .artifacts/codex/clone     → ApplicationData/codex/clone
Both now resolve within the existing ApplicationData scope root; no
worktree detection or scope expansion needed. Paths.ResolveGitCommonRoot
(IFileSystem) had no remaining callers and is removed along with its tests.

Depth protection on git root discovery:
- DetermineWorkingDirectoryRoot: only adopts a .git anchor ≤ 1 directory
  above CWD in release builds. Debug builds allow deeper traversal when
  a *.slnx is adjacent (developer running from IDE output directory).
- FindGitRoot(string) gets the same depth limit — documentation is not
  expected to live deep inside a repo tree.

Consolidate FindGitRoot / DetermineSourceDirectoryRoot:
- DetermineSourceDirectoryRoot removed; FindGitRoot(IDirectoryInfo)
  overload added with the same semantics (IFileSystem-abstracted,
  same depth protection, returns IDirectoryInfo?).
- Callers updated: BuildContext, ChangelogCommand, LocalChangesService.

Rename ForPath/ForPathWrite → RealForPath/RealForPathWrite:
- Signals these always create a real FileSystem.
- Doc comment notes suitability for command layer; service layer is
  tested via InMemory() at unit test level.

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

* Fix CI failures: restore Temp in WriteOptions, fix import ordering

WriteOptions had AllowedSpecialFolders.Temp removed on the assumption
that only ConfigurationFileProvider used it. AwsS3SyncApplyStrategy
also uses temp to stage files before S3 upload — restore Temp to
WriteOptions to fix the DocsSyncTests.TestApply integration failure.

Fix IMPORTS lint errors across all files that gained new Nullean.ScopedFileSystem
using directives during the ScopedFileSystem migration; dotnet format
reorders them into the expected alphabetical/grouped order.

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

* Fix remaining CI failures

Three issues:

1. Changelog tests used FileSystem.Path.GetTempPath() to generate unique
   paths within MockFileSystem. Since MockFS is in-memory, the paths can be
   anything in scope — replace with Paths.WorkingDirectoryRoot.FullName as
   the base so they stay within the ScopedFileSystem scope bounds. No real
   filesystem is touched; all operations remain in-memory.

2. DocsSyncTests.TestApply passed the same WrapToRead FS for both read and
   write in AssembleContext. AwsS3SyncApplyStrategy uses context.WriteFileSystem
   and needs AllowedSpecialFolders.Temp (present in WriteOptions). Fix by
   using WrapToWrite for the write FS.

3. IsolatedBuildService CI fallback defaulted the output path to
   Paths.WorkingDirectoryRoot/.artifacts/docs/html. When --path points to a
   different repo the write FS is scoped to that repo's root, not
   WorkingDirectoryRoot. Derive the default output from `path` instead so it
   stays within the write FS scope (same logic as BuildContext uses for the
   normal build path).

   FileSystemFactory.RealForPathWrite: replace StartsWith string check with
   IDirectoryInfo.IsSubPathOf from Nullean.ScopedFileSystem which does a
   proper directory-tree walk, preventing sibling-prefix false positives.

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

* Fix last import ordering lint error in CrossLinkRegistryTests

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

* Address CodeRabbit review comments

CsvReader: remove GetType().Name == "MockFileSystem" type check — the check
never matched after ScopedFileSystem wrapping was introduced, causing Sep to
read from the real filesystem via FromFile() instead of going through the
scoped IFileSystem. Always use IFileSystem.File.ReadAllText + Sep.FromText so
all filesystem access (real, scoped, or mock) goes through the abstraction.

ChangelogCommand: four ChangelogConfigurationLoader instantiations were still
passing new System.IO.Abstractions.FileSystem() directly, bypassing the
scoped _fileSystem field. Replace with _fileSystem.

ChangelogRenderingService: service writes output files (CreateDirectory,
WriteAllTextAsync) so its default should be FileSystemFactory.RealWrite not
RealRead. RealWrite has the same scope but correctly excludes .git write access.

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

* Fix changelog test paths: replace out-of-scope absolute paths with WorkingDirectoryRoot-relative

Tests that extend ChangelogTestBase use a ScopedFileSystem wrapping MockFileSystem,
bounded to [WorkingDirectoryRoot, ApplicationData]. Several tests used absolute paths
like /tmp/config, /docs/changelog, /test-root, and relative paths like docs/changelog
that resolved outside the scope root on CI runners.

- ChangelogPrEvaluationServiceTests: /tmp/config/changelog.yml →
  Path.Join(WorkingDirectoryRoot, "config/changelog.yml"); "docs/changelog" →
  Path.Join(WorkingDirectoryRoot, "docs/changelog")
- ChangelogCreationServiceTests: /tmp/config → Path.Join(WorkingDirectoryRoot, "config");
  /tmp/output → Path.Join(WorkingDirectoryRoot, "output")
- BundleChangelogsTests: /test-root → WorkingDirectoryRoot; use MockFileSystemOptions
  { CurrentDirectory = root } so relative paths within service code resolve correctly;
  switch YAML template config strings to $$""" raw literals so {version}/{lifecycle}
  stay as literal text while {{Path.Join(...)}} is interpolated
- ChangelogTestBase: set MockFileSystem CurrentDirectory to WorkingDirectoryRoot so
  paths within the test base resolve within scope

BundleLoaderTests (standalone, unscoped MockFileSystem) does not extend ChangelogTestBase
and is not affected.

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

* remove RFC data

* Rename WrapToRead/WrapToWrite to explicit Scope* methods; fix navigation test scope

WrapToRead/WrapToWrite implicitly injected Paths.WorkingDirectoryRoot into every
scope, which was a wrong assumption when the inner FS contained files outside
the working directory (e.g. navigation tests with MockFS at /checkouts/...).

New explicit API in FileSystemFactory:
- ScopeCurrentWorkingDirectory(inner) — scopes to WorkingDirectoryRoot + ApplicationData
- ScopeCurrentWorkingDirectory(inner, extensionRoots) — adds detection-rules roots
- ScopeCurrentWorkingDirectoryForWrite(inner) — write variant (no .git)
- ScopeSourceDirectory(inner, sourceRoot) — scopes to an explicit root + ApplicationData
- ScopeSourceDirectoryForWrite(inner, sourceRoot) — write variant

All production options fields renamed: ReadOptions → WorkingDirectoryReadOptions,
WriteOptions → WorkingDirectoryWriteOptions (makes scope intent visible in code).

DocumentationSetFile.LoadAndResolve: replace the InvalidOperationException cast guard
with FileSystemFactory.ScopeSourceDirectory(fs, directory.FullName) so callers with
unscoped IFileSystem/IDirectoryInfo (e.g. from a raw MockFileSystem) are automatically
given the tightest correct scope.

Navigation.Tests/Assembler: change ScopeCurrentWorkingDirectory(fileSystem) →
ScopeSourceDirectory(fileSystem, "/checkouts") for all tests that use
SiteNavigationTestFixture (MockFS rooted at /checkouts/current/...).

TestDocumentationSetContext: use ScopeSourceDirectory(fileSystem, sourceDirectory.FullName)
and ScopeSourceDirectoryForWrite(fileSystem, outputDirectory.FullName) so each test gets
a scope precisely matching its mock FS tree.

Fix 3 semantic bugs where WriteFileSystem was assigned ScopeCurrentWorkingDirectory
instead of ScopeCurrentWorkingDirectoryForWrite:
- CodexNavigationTestBase, GroupNavigationTests, CrossLinkRegistryTests

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

* Rename RealForPath* to RealGitRootForPath* for clarity

RealForPath → RealGitRootForPath
RealForPathWrite → RealGitRootForPathWrite

The Git prefix makes explicit that these factory methods resolve the scope
root by walking up to the nearest .git boundary from the given path.

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

* Fix ConfigurationFileProvider file lock and GitCheckoutInformation type check

ConfigurationFileProvider: use a unique subdirectory per instance under
ApplicationData/config-runtime/{guid} to avoid Windows file-locking
collisions when multiple tests run in parallel writing the same
versions.yml/products.yml files. Original CreateTempSubdirectory gave
unique dirs; the fixed path broke parallel test isolation on Windows.

GitCheckoutInformation.Create: the guard `fileSystem is not FileSystem`
returned hardcoded test data for any ScopedFileSystem, including RealRead
wrapping a real FileSystem. RealRead is used in GitCheckoutInformationTests
which expects real git metadata. Change to check the type name for "Mock"
so only MockFileSystem-backed instances return test data.

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

* Fix MockFileSystem temp path mismatch in write-scope methods

MockFileSystem on non-Windows returns a hardcoded Unix-ified path ('/temp/')
for GetTempPath() — it unixifies 'C:\temp' rather than calling
System.IO.Path.GetTempPath(). This diverges from the real API which reads
TMPDIR. AllowedSpecialFolder.Temp in the ValidationContext uses the real
System.IO.Path.GetTempPath() (resolves to '/tmp/' on standard Linux), creating
a mismatch: the scope allows '/tmp/' but the mock FS creates staging paths
at '/temp/zzyysk35.wbk'.

Fix: ScopeCurrentWorkingDirectoryForWrite and ScopeSourceDirectoryForWrite
now call inner.Path.GetTempPath() on non-Windows and add the result as an
explicit scope root alongside AllowedSpecialFolders.Temp. This covers both
the real OS temp ('/tmp/' via AllowedSpecialFolders) and the mock's hardcoded
path ('/temp/' via the explicit root), without relaxing permissions on Windows
where MockFileSystem temp is consistent with the real API.

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

* Bump Nullean.ScopedFileSystem to 0.4.0; use InnerType for mock detection

0.4.0 exposes ScopedFileSystem.InnerType, removing the need to check
the outer wrapper's type name when inspecting the underlying filesystem.

FileSystemFactory.BuildWriteOptions: use sf.InnerType where the inner FS
is available through a ScopedFileSystem to check for MockFileSystem, with
a note linking to the upstream fix PR
(TestableIO/System.IO.Abstractions#1454).

GitCheckoutInformation.Create: replace fileSystem.GetType().Name.Contains("Mock")
with (fileSystem is ScopedFileSystem sf ? sf.InnerType : fileSystem.GetType())
so the check correctly sees through the ScopedFileSystem wrapper.

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

* Fix double-wrapping bug in PhysicalDocsetCanBeNavigated

PhysicalDocsetCanBeNavigated was the only test in PhysicalDocsetTests.cs
that passed FileSystemFactory.RealRead (already a ScopedFileSystem) to
TestDocumentationSetContext, which then tried to scope it again via
ScopeSourceDirectory — causing 'Cannot wrap a ScopedFileSystem inside
another ScopedFileSystem' with Nullean.ScopedFileSystem 0.4.0's new
validation.

All other tests in the same file correctly pass new FileSystem() (raw).
This was an over-eager substitution during the new FileSystem() → RealRead
migration. Fix: use new FileSystem() and derive the ScopedFileSystem via
context.ReadFileSystem for the DocumentationSetFile.LoadAndResolve call.

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

* Fix CodeRabbit: use WriteFileSystem for OutputDirectory; fix File.Exists bypass

AssembleContext and CodexContext both materialised OutputDirectory from
ReadFileSystem. Since OutputDirectory is the write target (HTML files,
state files, redirects) it must be resolved via WriteFileSystem so that
scope enforcement applies to writes. CheckoutDirectory remains on ReadFileSystem
as it is a read-only source.

AssemblerBuildService had a direct System.IO.File.Exists(redirectsPath) call
that bypassed the scoped filesystem boundary; replaced with fs.File.Exists
using the ScopedFileSystem parameter already in scope.

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

* Fix SiteNavigationTests.DisposeAsync collection-modified race

RecordedLogs was being iterated in DisposeAsync while Aspire was still
writing to it concurrently, causing 'Collection was modified; enumeration
operation may not execute.' Snapshot with .ToList() before iterating.

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

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The project does not enable nullable reference types, so string? causes
CS8632 across all target frameworks. Plain string with IsNullOrEmpty
guard achieves the same result.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@vbreuss vbreuss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @Mpdreamz. Looks good in general, I have just some minor comments.

Thanks @vbreuss applied your updates !

Co-authored-by: Valentin Breuß <vbreuss@gmail.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.

3 participants