Revert "Wrap all IFileSystem usage in ScopedFileSystem"#3011
Merged
Conversation
This reverts commit 17eea57.
|
Label error. Requires exactly 1 of: automation, breaking, bug, changelog:skip, chore, ci, dependencies, documentation, enhancement, feature, fix, redesign. Found: breaking, dependencies |
Member
Author
|
Caused errors in CI |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (130)
📝 WalkthroughWalkthroughThis is a significant refactoring that removes the Possibly related PRs
Suggested labels
✨ Finishing Touches✨ Simplify code
Comment |
Mpdreamz
added a commit
that referenced
this pull request
Apr 1, 2026
3 tasks
Mpdreamz
added a commit
that referenced
this pull request
Apr 2, 2026
Mpdreamz
added a commit
that referenced
this pull request
Apr 2, 2026
…ontainers (#3012) * Reapply "Wrap all IFileSystem usage in ScopedFileSystem (#3001)" (#3011) This reverts commit 108282e. * Fix ApplicationData scope-root collision when HOME is unset in Docker In Docker/CI containers where neither XDG_DATA_HOME nor HOME is set, Environment.GetFolderPath(LocalApplicationData) returns "". Path.Join("", "elastic", "docs-builder") then produces a relative path that resolves to {CWD}/elastic/docs-builder — a subdirectory of WorkingDirectoryRoot. ScopedFileSystem 0.4.0's ValidateRootsAreDisjoint check then throws in FileSystemFactory's static constructor, crashing the process with TypeInitializationException before anything runs. Observed in elastic/elasticsearch (workspace /github/workspace) and elastic/chainguard-image-sync where the resolved ApplicationData path fell inside the runner workspace. Fix: fall back to Path.GetTempPath() when LocalApplicationData is empty so the ApplicationData path is always an absolute non-workspace path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Misceleanous fixes, one old pending coderabbit task * Address code review feedback on ScopedFileSystem usage - Use FileSystemFactory.AppData for default checkout dirs (CodexContext, AssembleContext) instead of ReadFileSystem to avoid scope mismatches - Fix RealGitRootForPathWrite to use BuildWriteOptions so AllowedSpecialFolders.Temp is included - Fix DetermineWorkingDirectoryRoot: *.slnx check now respects depth guard (was bypassing it entirely) - Fix GitLinkIndexReader.CloneDirectory to use Paths.ApplicationData instead of raw LocalApplicationData (handles Docker/CI empty HOME) - Normalize ExternalScopeRoots in DetectionRulesDocsBuilderExtension to absolute paths - Replace static Directory.GetCurrentDirectory() with fileSystem.Directory.GetCurrentDirectory() in DeployUpdateRedirectsService - Replace new FileStream / static File.ReadAllTextAsync with scoped fileSystem equivalents in IncrementalDeployService - Fix StaticWebHost to pass _contentRoot to RealGitRootForPath for consistent scoping - Split AssemblerBuildService.BuildAll fs param into readFs/writeFs; update AssemblerCommands callers - Fix test paths in CodexNavigationTestBase to fall within WorkingDirectoryRoot/ApplicationData scopes - Make changelog test reportPath unique to prevent parallel-test file collisions Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix build errors: AssemblerIndexService and MockFileSystem constructor - Pass fileSystem as both readFs/writeFs in AssemblerIndexService.Index() to match the updated BuildAll(readFs, writeFs) signature - Fix MockFileSystem construction in CodexNavigationTestBase: use constructor argument for currentDirectory instead of missing property Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix FindGitRoot file-path fallback and split AssemblerIndexService read/write scopes - Paths.FindGitRoot: capture startDir before the loop so fallback returns always return a directory path, not the raw startPath which can be a file - AssemblerIndexService.Index: replace single fileSystem param with separate readFs/writeFs to avoid forwarding the read scope into write operations; update AssemblerIndexCommand to pass RealRead and RealWrite respectively Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Restore unlimited-depth *.slnx anchor in DetermineWorkingDirectoryRoot The previous commit applied a depth guard to *.slnx in release builds, but this broke both authoring tests and the assembler integration tests: binaries run by Aspire and dotnet test start from the build output or project directory (4+ levels below the solution root), so restricting *.slnx to depth <= 1 caused WorkingDirectoryRoot to resolve to the build output directory instead of the repo root, making docs/ and all other repo paths fall outside the scoped filesystem roots. *.slnx is intentionally a depth-unlimited anchor for DetermineWorkingDirectoryRoot — it is the primary way to locate the solution root regardless of where the binary is launched from. The .git depth guard is kept unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
cotti
pushed a commit
that referenced
this pull request
Apr 2, 2026
…ontainers (#3012) * Reapply "Wrap all IFileSystem usage in ScopedFileSystem (#3001)" (#3011) This reverts commit 108282e. * Fix ApplicationData scope-root collision when HOME is unset in Docker In Docker/CI containers where neither XDG_DATA_HOME nor HOME is set, Environment.GetFolderPath(LocalApplicationData) returns "". Path.Join("", "elastic", "docs-builder") then produces a relative path that resolves to {CWD}/elastic/docs-builder — a subdirectory of WorkingDirectoryRoot. ScopedFileSystem 0.4.0's ValidateRootsAreDisjoint check then throws in FileSystemFactory's static constructor, crashing the process with TypeInitializationException before anything runs. Observed in elastic/elasticsearch (workspace /github/workspace) and elastic/chainguard-image-sync where the resolved ApplicationData path fell inside the runner workspace. Fix: fall back to Path.GetTempPath() when LocalApplicationData is empty so the ApplicationData path is always an absolute non-workspace path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Misceleanous fixes, one old pending coderabbit task * Address code review feedback on ScopedFileSystem usage - Use FileSystemFactory.AppData for default checkout dirs (CodexContext, AssembleContext) instead of ReadFileSystem to avoid scope mismatches - Fix RealGitRootForPathWrite to use BuildWriteOptions so AllowedSpecialFolders.Temp is included - Fix DetermineWorkingDirectoryRoot: *.slnx check now respects depth guard (was bypassing it entirely) - Fix GitLinkIndexReader.CloneDirectory to use Paths.ApplicationData instead of raw LocalApplicationData (handles Docker/CI empty HOME) - Normalize ExternalScopeRoots in DetectionRulesDocsBuilderExtension to absolute paths - Replace static Directory.GetCurrentDirectory() with fileSystem.Directory.GetCurrentDirectory() in DeployUpdateRedirectsService - Replace new FileStream / static File.ReadAllTextAsync with scoped fileSystem equivalents in IncrementalDeployService - Fix StaticWebHost to pass _contentRoot to RealGitRootForPath for consistent scoping - Split AssemblerBuildService.BuildAll fs param into readFs/writeFs; update AssemblerCommands callers - Fix test paths in CodexNavigationTestBase to fall within WorkingDirectoryRoot/ApplicationData scopes - Make changelog test reportPath unique to prevent parallel-test file collisions Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix build errors: AssemblerIndexService and MockFileSystem constructor - Pass fileSystem as both readFs/writeFs in AssemblerIndexService.Index() to match the updated BuildAll(readFs, writeFs) signature - Fix MockFileSystem construction in CodexNavigationTestBase: use constructor argument for currentDirectory instead of missing property Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix FindGitRoot file-path fallback and split AssemblerIndexService read/write scopes - Paths.FindGitRoot: capture startDir before the loop so fallback returns always return a directory path, not the raw startPath which can be a file - AssemblerIndexService.Index: replace single fileSystem param with separate readFs/writeFs to avoid forwarding the read scope into write operations; update AssemblerIndexCommand to pass RealRead and RealWrite respectively Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Restore unlimited-depth *.slnx anchor in DetermineWorkingDirectoryRoot The previous commit applied a depth guard to *.slnx in release builds, but this broke both authoring tests and the assembler integration tests: binaries run by Aspire and dotnet test start from the build output or project directory (4+ levels below the solution root), so restricting *.slnx to depth <= 1 caused WorkingDirectoryRoot to resolve to the build output directory instead of the repo root, making docs/ and all other repo paths fall outside the scoped filesystem roots. *.slnx is intentionally a depth-unlimited anchor for DetermineWorkingDirectoryRoot — it is the primary way to locate the solution root regardless of where the binary is launched from. The .git depth guard is kept unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
cotti
pushed a commit
that referenced
this pull request
Apr 7, 2026
…ontainers (#3012) * Reapply "Wrap all IFileSystem usage in ScopedFileSystem (#3001)" (#3011) This reverts commit 108282e. * Fix ApplicationData scope-root collision when HOME is unset in Docker In Docker/CI containers where neither XDG_DATA_HOME nor HOME is set, Environment.GetFolderPath(LocalApplicationData) returns "". Path.Join("", "elastic", "docs-builder") then produces a relative path that resolves to {CWD}/elastic/docs-builder — a subdirectory of WorkingDirectoryRoot. ScopedFileSystem 0.4.0's ValidateRootsAreDisjoint check then throws in FileSystemFactory's static constructor, crashing the process with TypeInitializationException before anything runs. Observed in elastic/elasticsearch (workspace /github/workspace) and elastic/chainguard-image-sync where the resolved ApplicationData path fell inside the runner workspace. Fix: fall back to Path.GetTempPath() when LocalApplicationData is empty so the ApplicationData path is always an absolute non-workspace path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Misceleanous fixes, one old pending coderabbit task * Address code review feedback on ScopedFileSystem usage - Use FileSystemFactory.AppData for default checkout dirs (CodexContext, AssembleContext) instead of ReadFileSystem to avoid scope mismatches - Fix RealGitRootForPathWrite to use BuildWriteOptions so AllowedSpecialFolders.Temp is included - Fix DetermineWorkingDirectoryRoot: *.slnx check now respects depth guard (was bypassing it entirely) - Fix GitLinkIndexReader.CloneDirectory to use Paths.ApplicationData instead of raw LocalApplicationData (handles Docker/CI empty HOME) - Normalize ExternalScopeRoots in DetectionRulesDocsBuilderExtension to absolute paths - Replace static Directory.GetCurrentDirectory() with fileSystem.Directory.GetCurrentDirectory() in DeployUpdateRedirectsService - Replace new FileStream / static File.ReadAllTextAsync with scoped fileSystem equivalents in IncrementalDeployService - Fix StaticWebHost to pass _contentRoot to RealGitRootForPath for consistent scoping - Split AssemblerBuildService.BuildAll fs param into readFs/writeFs; update AssemblerCommands callers - Fix test paths in CodexNavigationTestBase to fall within WorkingDirectoryRoot/ApplicationData scopes - Make changelog test reportPath unique to prevent parallel-test file collisions Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix build errors: AssemblerIndexService and MockFileSystem constructor - Pass fileSystem as both readFs/writeFs in AssemblerIndexService.Index() to match the updated BuildAll(readFs, writeFs) signature - Fix MockFileSystem construction in CodexNavigationTestBase: use constructor argument for currentDirectory instead of missing property Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix FindGitRoot file-path fallback and split AssemblerIndexService read/write scopes - Paths.FindGitRoot: capture startDir before the loop so fallback returns always return a directory path, not the raw startPath which can be a file - AssemblerIndexService.Index: replace single fileSystem param with separate readFs/writeFs to avoid forwarding the read scope into write operations; update AssemblerIndexCommand to pass RealRead and RealWrite respectively Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Restore unlimited-depth *.slnx anchor in DetermineWorkingDirectoryRoot The previous commit applied a depth guard to *.slnx in release builds, but this broke both authoring tests and the assembler integration tests: binaries run by Aspire and dotnet test start from the build output or project directory (4+ levels below the solution root), so restricting *.slnx to depth <= 1 caused WorkingDirectoryRoot to resolve to the build output directory instead of the repo root, making docs/ and all other repo paths fall outside the scoped filesystem roots. *.slnx is intentionally a depth-unlimited anchor for DetermineWorkingDirectoryRoot — it is the primary way to locate the solution root regardless of where the binary is launched from. The .git depth guard is kept unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
cotti
added a commit
that referenced
this pull request
Apr 7, 2026
* Move S3 upload to a new integrations project and reuse it for changelog uploads. Add tests. * Update tests/Elastic.Documentation.Integrations.Tests/S3/S3EtagCalculatorTests.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Ne'er forget using * Adjust entities being used * Update src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add changelog bundle support for hiding private links (#3002) Co-authored-by: Felipe Cotti <felipe.cotti@elastic.co> * Fix ApplicationData path collision with workspace root in Docker/CI containers (#3012) * Reapply "Wrap all IFileSystem usage in ScopedFileSystem (#3001)" (#3011) This reverts commit 108282e. * Fix ApplicationData scope-root collision when HOME is unset in Docker In Docker/CI containers where neither XDG_DATA_HOME nor HOME is set, Environment.GetFolderPath(LocalApplicationData) returns "". Path.Join("", "elastic", "docs-builder") then produces a relative path that resolves to {CWD}/elastic/docs-builder — a subdirectory of WorkingDirectoryRoot. ScopedFileSystem 0.4.0's ValidateRootsAreDisjoint check then throws in FileSystemFactory's static constructor, crashing the process with TypeInitializationException before anything runs. Observed in elastic/elasticsearch (workspace /github/workspace) and elastic/chainguard-image-sync where the resolved ApplicationData path fell inside the runner workspace. Fix: fall back to Path.GetTempPath() when LocalApplicationData is empty so the ApplicationData path is always an absolute non-workspace path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Misceleanous fixes, one old pending coderabbit task * Address code review feedback on ScopedFileSystem usage - Use FileSystemFactory.AppData for default checkout dirs (CodexContext, AssembleContext) instead of ReadFileSystem to avoid scope mismatches - Fix RealGitRootForPathWrite to use BuildWriteOptions so AllowedSpecialFolders.Temp is included - Fix DetermineWorkingDirectoryRoot: *.slnx check now respects depth guard (was bypassing it entirely) - Fix GitLinkIndexReader.CloneDirectory to use Paths.ApplicationData instead of raw LocalApplicationData (handles Docker/CI empty HOME) - Normalize ExternalScopeRoots in DetectionRulesDocsBuilderExtension to absolute paths - Replace static Directory.GetCurrentDirectory() with fileSystem.Directory.GetCurrentDirectory() in DeployUpdateRedirectsService - Replace new FileStream / static File.ReadAllTextAsync with scoped fileSystem equivalents in IncrementalDeployService - Fix StaticWebHost to pass _contentRoot to RealGitRootForPath for consistent scoping - Split AssemblerBuildService.BuildAll fs param into readFs/writeFs; update AssemblerCommands callers - Fix test paths in CodexNavigationTestBase to fall within WorkingDirectoryRoot/ApplicationData scopes - Make changelog test reportPath unique to prevent parallel-test file collisions Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix build errors: AssemblerIndexService and MockFileSystem constructor - Pass fileSystem as both readFs/writeFs in AssemblerIndexService.Index() to match the updated BuildAll(readFs, writeFs) signature - Fix MockFileSystem construction in CodexNavigationTestBase: use constructor argument for currentDirectory instead of missing property Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix FindGitRoot file-path fallback and split AssemblerIndexService read/write scopes - Paths.FindGitRoot: capture startDir before the loop so fallback returns always return a directory path, not the raw startPath which can be a file - AssemblerIndexService.Index: replace single fileSystem param with separate readFs/writeFs to avoid forwarding the read scope into write operations; update AssemblerIndexCommand to pass RealRead and RealWrite respectively Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Restore unlimited-depth *.slnx anchor in DetermineWorkingDirectoryRoot The previous commit applied a depth guard to *.slnx in release builds, but this broke both authoring tests and the assembler integration tests: binaries run by Aspire and dotnet test start from the build output or project directory (4+ levels below the solution root), so restricting *.slnx to depth <= 1 caused WorkingDirectoryRoot to resolve to the build output directory instead of the repo root, making docs/ and all other repo paths fall outside the scoped filesystem roots. *.slnx is intentionally a depth-unlimited anchor for DetermineWorkingDirectoryRoot — it is the primary way to locate the solution root regardless of where the binary is launched from. The .git depth guard is kept unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix docs-builder redirect tests (#3008) * Fix redirect tests * Remove changelog redirects implemented elsewhere * Search: Use default semantic_text inference, remove Jina mappings (#3014) Elasticsearch Serverless now defaults semantic_text to Jina, making the explicit Jina sub-fields redundant and the ELSER inference ID unnecessary. This removes both inference ID constants, all .jina field mappings, and lets semantic_text fields use the platform default. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: Update config/versions.yml eck 3.3.2 (#3019) Made with ❤️️ by updatecli Co-authored-by: elastic-observability-automation[bot] <180520183+elastic-observability-automation[bot]@users.noreply.github.com> * Deploy: Use write-scoped filesystem for apply command (#3021) The deploy apply command used RealRead which lacks AllowedSpecialFolder.Temp, causing ScopedFileSystemException when AwsS3SyncApplyStrategy stages files in /tmp/ for S3 upload. Switch to RealWrite which permits temp directory access. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use ScopedFileSystem and inject interfaces. * Fix exception filtering --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lisa Cawley <lcawley@elastic.co> Co-authored-by: Martijn Laarman <Mpdreamz@gmail.com> Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Jan Calanog <jan.calanog@elastic.co> Co-authored-by: elastic-observability-automation[bot] <180520183+elastic-observability-automation[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #3001