Deploy: separate read/write scoped filesystems in IncrementalDeployService

[Mpdreamz]

Summary

Alternative to #3021 — fixes the same ScopedFileSystemException on /tmp/ during incremental deploy, but with proper read/write filesystem separation rather than a single-FS swap.

• IncrementalDeployService now accepts separate readFileSystem and writeFileSystem parameters instead of a single fileSystem used for both
• DeployCommands passes RealRead + RealWrite to both Plan and Apply commands
• Each operation uses the correct scoped FS: reads through readFileSystem, writes (including plan output and temp staging) through writeFileSystem

Why this over #3021

#3021 swaps RealRead → RealWrite for the Apply command only, still passing a single FS as both the read and write filesystem to IncrementalDeployService (and through it to AssembleContext). This works but:

1. Mismatched semantics — AssembleContext has distinct ReadFileSystem / WriteFileSystem properties, but both point to the same RealWrite instance. Future code relying on ReadFileSystem having read-scoped permissions (e.g. .git access) would silently get the wrong FS.
2. Plan command unchanged — Plan still uses RealRead for writing its output file (fileSystem.File.Create(@out)). This happens to work today because the output path is within the working directory scope, but it's using a read FS for a write operation.
3. Single responsibility — the service should declare what it needs (a reader and a writer) rather than accepting one FS and hoping the caller picked the right one.

Test plan

• dotnet build passes with 0 errors
• Existing DocsSyncTests.TestApply uses separate read/write scoped filesystems and passes
• Deploy apply in CI should no longer throw ScopedFileSystemException for /tmp/ paths

Fixes https://github.com/elastic/docs-internal-workflows/actions/runs/23897412121/job/69685376318

Made with Cursor

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4809261b-1850-4e34-b6cd-e5e4e5f4b7d0

📥 Commits

Reviewing files that changed from the base of the PR and between 5b4db85 and d91576a.

📒 Files selected for processing (2)

• src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs
• src/tooling/docs-builder/Commands/Assembler/DeployCommands.cs

✅ Files skipped from review due to trivial changes (1)

• src/tooling/docs-builder/Commands/Assembler/DeployCommands.cs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs

---

📝 Walkthrough

Walkthrough

The pull request updates IncrementalDeployService to accept separate ScopedFileSystem instances for reading and writing (readFileSystem, writeFileSystem) instead of a single shared filesystem. AssembleContext is adjusted to use the distinct read/write file systems. In Plan, the serialized sync plan is written via writeFileSystem.File.Create(...). In Apply, plan existence checks and reading use readFileSystem.File.Exists(...) and readFileSystem.File.ReadAllTextAsync(...). DeployCommands now supplies FileSystemFactory.RealRead and FileSystemFactory.RealWrite when constructing the service.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant CLI as DeployCommands
participant Service as IncrementalDeployService
participant ReadFS as readFileSystem
participant WriteFS as writeFileSystem

CLI->>Service: instantiate(read=RealRead, write=RealWrite)
CLI->>Service: Plan(...)
Service->>WriteFS: File.Create(@out) / write plan JSON
WriteFS-->>Service: file stream written
Service-->>CLI: Plan result

CLI->>Service: Apply(...)
Service->>ReadFS: File.Exists(planPath)
ReadFS-->>Service: exists? (true/false)
alt exists
Service->>ReadFS: File.ReadAllTextAsync(planPath)
ReadFS-->>Service: plan JSON
Service-->>CLI: Apply result
else not exists
Service-->>CLI: error / no plan found
end

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: separating read/write scoped filesystems in IncrementalDeployService, which is the core objective of the PR.
Description check	✅ Passed	The description clearly explains the changes, rationale, and test plan. It provides meaningful context about why this approach is better than the alternative (#3021).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/incremental-deploy

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}