Add pipeline-scoped working directories

[thomhurst]

Closes #3770

Summary

• add a pipeline-scoped working directory used by command and file contexts
• add built-in pipeline-project and Git-root resolvers
• remove template CWD mutation and its 53-line helper

Validation

• ModularPipelines.slnx Release build: 0 warnings, 0 errors
• PipelineWorkingDirectoryTests: 3/3 passed
• PipelineBuilderRegistrationTests: 16/16 passed
• targeted formatter verification passed

The focused test project currently needs the unrelated stale RunReportTests initializers for removed PrintLogo/PrintResults properties omitted locally to compile; those lines were restored and are not part of this PR.

[thomhurst]

CI diagnosis: all six failures fan out from Fast fail. The template pipeline itself succeeds, then the smoke test fails test ! -d "$RUNNER_TEMP/template-caller/artifacts" because existing CI auto run-report output still resolves relative to the process CWD after this PR removes the template's global CWD mutation. This is the sibling run-report anchoring problem tracked and implemented by #3840, not a command/file-context defect in this PR. Avoiding duplicate/conflicting run-report changes here; refresh this branch from main after #3840 merges, then rerun.

[thomhurst]

Refreshed onto current main at ccea314 after the prior Fast-fail artifact-upload failure. Current-head validation: PipelineWorkingDirectoryTests 3/3; PipelineBuilderRegistrationTests 16/16; core Release build 0 warnings / 0 errors. Please re-review current head.

[claude]

Code review

Nice cleanup — this replaces the template's 53-line PipelineProjectDirectory + Environment.CurrentDirectory mutation hack with a proper PipelineWorkingDirectory abstraction that's injected through DI and consistently threaded through Command, FilesContext, FileSystemContext, and EnvironmentContext. The new PipelineWorkingDirectoryTests do a good job of proving the process CWD is left untouched while pipeline-internal operations correctly scope to the configured directory.

One design point worth discussing before merge, plus one follow-up suggestion:

Pipeline.CreateBuilder(args) now silently changes its default working directory for all existing pipelines, not just the template

Pipeline.CreateBuilder (src/ModularPipelines/Pipeline.cs:34-38) now always populates PipelineBuilderOptions.WorkingDirectory via PipelineDirectory.TryFindPipelineProject(sourceFilePath), using the [CallerFilePath]-captured, compile-time directory of Program.cs. Previously (confirmed against origin/main), this overload just forwarded args and the working directory came purely from Directory.GetCurrentDirectory().

Since TryFindPipelineProject succeeds whenever Program.cs sits next to appsettings.json + a .csproj — which is true for essentially every generated pipeline project, including this repo's own src/ModularPipelines.Build — this is a behavioral change for every pre-existing pipeline that upgrades, not only for template-generated ones. If a pipeline binary/​dotnet run is ever invoked from a directory other than its own source directory (a wrapper script, a monorepo task runner, a CI step that cds elsewhere first), context.Environment.WorkingDirectory, relative file resolution, and command working directories will now silently point at the pipeline's source directory instead of the directory the process was actually launched from.

I can see this matches the ask in #3770 ("The template then shrinks to Pipeline.CreateBuilder(args) + AddModule calls"), so it's intentional — but it's a meaningful, essentially non-optional default-behavior change hidden inside an existing overload's default parameter (the only opt-out is switching to the PipelineBuilderOptions overload and leaving WorkingDirectory unset, since both PipelineBuilder constructors are internal). Worth calling out explicitly in release notes, and/or worth updating CreateBuilder's XML doc (which still just shows the old CWD-implying example) to describe the new resolution order, so existing consumers aren't surprised on upgrade.

Suggestion: the working-directory abstraction doesn't yet reach module caching/artifacts

ModuleCacheOptions.WorkingDirectory (src/ModularPipelines/Caching/ModuleCacheOptions.cs) still defaults to raw Directory.GetCurrentDirectory(), and ArtifactLifecycleManager's fallback does the same — neither is wired to the new PipelineWorkingDirectory singleton. This is pre-existing code the PR doesn't touch, so not a regression from this diff, but it does mean that once a pipeline sets PipelineBuilderOptions.WorkingDirectory to something other than the process CWD (exactly the scenario this PR's own test exercises), [CacheInputs] glob resolution and artifact snapshotting will resolve against a different base directory than context.Files/context.Cmd do. Might be worth a fast-follow to post-configure ModuleCacheOptions.WorkingDirectory from the same PipelineWorkingDirectory singleton so there's one consistent notion of "the pipeline's directory" everywhere, rather than two.

Everything else — the Command/FilesContext/FileSystemContext resolution changes, the PipelineDirectory static helper, and the DI wiring — looks correct and consistent; I didn't find any compile or logic issues in the diff itself.

[thomhurst]

Addressed the current-head review at 15e9687778: both Pipeline.CreateBuilder overloads now explicitly document the intentional working-directory behavior, resolution/fallback order, and how callers preserve process-CWD semantics. The cache/artifact suggestion remains outside #3770's command/file API scope (run-report anchoring is likewise a sibling issue).

Revalidation: PipelineWorkingDirectoryTests 3/3; exact external-temp template smoke exits 0, writes the publish DLL under the pipeline project, and leaves the caller artifact-free; core Release build 0 warnings/errors; format and diff checks clean.

@claude review
@codex review

[claude]

Code review (re-review at 15e9687778)

Re-reviewed the current head against the earlier review on this PR. Both points raised previously have been handled reasonably:

1. "Pipeline.CreateBuilder(args) silently changes the default working directory for existing pipelines" — now addressed. Both CreateBuilder overloads (src/ModularPipelines/Pipeline.cs:9-45) carry <remarks> explaining the resolution/fallback order and how to opt out (use the PipelineBuilderOptions overload and leave WorkingDirectory unset). I also confirmed this repo's own src/ModularPipelines.Build/Program.cs:20 uses Pipeline.CreateBuilder(args) and is invoked from its own source directory per CLAUDE.md, so the new caller-path inference is a no-op for it — no self-inflicted regression.
2. Module cache / artifact working-directory wiring — the author's response that this is out of scope for Give pipelines a first-class working-directory concept so templates stop mutating Environment.CurrentDirectory #3770 (a sibling concern, same as the run-report anchoring issue closed out in Stabilize run-report storage paths #3840) is reasonable; not a regression introduced by this diff, just a known gap worth a fast-follow.

Fresh pass over the current diff

Traced the PipelineWorkingDirectory singleton end-to-end: PipelineBuilder → DI registration (PipelineBuilder.cs:358) → Command, FilesContext, FileSystemContext, EnvironmentContext, EnvironmentDomainContext. Path resolution is consistently applied once per call site (no double-resolution, since ResolvePath is idempotent for already-absolute paths), and the new _configuration.SetBasePath(_environment.WorkingDirectory) correctly lets the template's AddJsonFile("appsettings.json") resolve without the old CWD mutation. I also double-checked the FirstNonEmpty(fallback, candidates...) precedence in CreateHostEnvironment (PipelineBuilder.cs:293-301) — despite Directory.GetCurrentDirectory() appearing textually first in the call, it's bound to the fallback parameter, so options.WorkingDirectory correctly takes priority; no bug there despite how it reads at a glance.

One minor, non-blocking observation: IEnvironmentContext.WorkingDirectory (EnvironmentContext.cs) and IEnvironmentDomainContext.WorkingDirectory (EnvironmentDomainContext.cs) are two separate properties on two separate context interfaces that now both just read PipelineWorkingDirectory.Path. That duplication predates this PR, but this PR touches both call sites to wire them independently to the same singleton rather than having one delegate to the other — a small consolidation opportunity (e.g. domain context sourcing from IEnvironmentContext) if either interface is touched again, not worth blocking on here.

No compile/logic issues found in the diff. Tests (PipelineWorkingDirectoryTests, updated PipelineBuilderRegistrationTests) look like they exercise the right invariants: process CWD left untouched, command/file/environment resolution scoped to the configured directory, and caller-path project inference. Good to merge from my read.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15e9687778

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Scope nested file helpers to the pipeline directory

When PipelineBuilderOptions.WorkingDirectory is set and the process CWD is somewhere else, only the top-level IFilesContext methods use _workingDirectory; the injected Zip and Checksum helpers still receive raw relative paths and resolve them against the process CWD. A module that writes relative.txt with context.Files.WriteAsync(...) and then calls context.Files.Checksum.Md5("relative.txt"), or zips to "out.zip", will fail or create artifacts outside the configured pipeline directory, so these nested helpers need the same path resolver or scoped wrappers.

Useful? React with 👍 / 👎.

[thomhurst]

Fixed in 48b417c. Zip and Checksum now receive PipelineWorkingDirectory and resolve all relative ZIP input/output and checksum paths against it. The integration test writes, hashes, zips, and unzips using relative paths and asserts the configured directory.

[chatgpt-codex-connector]

Walk source ancestors for project discovery

If Pipeline.CreateBuilder is called from a helper/source file under a subdirectory of the pipeline project and the pipeline is launched from a copied or published output whose AppContext.BaseDirectory is no longer under that source project, this exact-directory check misses the project root even though the caller file is inside it. The builder then falls back to the process CWD, so AddJsonFile("appsettings.json") and relative file operations use the wrong directory; start the existing ancestor search from sourceDirectory before falling back to AppContext.BaseDirectory.

Useful? React with 👍 / 👎.

[thomhurst]

Fixed in 48b417c. Pipeline project discovery now walks ancestors from the source-file directory before falling back to AppContext.BaseDirectory. The regression uses a caller path nested under src/Pipeline.

[chatgpt-codex-connector]

Preserve the existing CreateBuilder entry point

Changing the public CreateBuilder(string[]?) method into CreateBuilder(string[]?, string) removes the one-argument IL signature that existing compiled pipeline executables reference. In the package-upgrade-without-rebuild scenario those binaries will throw MissingMethodException before source compatibility helps, so keep a binary-compatible shim or move caller-path discovery behind an additional API.

Useful? React with 👍 / 👎.

[thomhurst]

Fixed in 48b417c. The exact CreateBuilder(string[]?) entry point is restored, with a reflection regression for that IL signature. Caller-path inference moved to the additional CreateBuilderFromSource API, which the template now uses.

[thomhurst]

@claude review

[thomhurst]

@codex review

[claude]

Code review (re-review at 48b417cc)

Re-reviewed the diff since the last review (15e9687778 → 48b417cc), which is a focused fix commit ("complete working directory scoping").

The main outstanding concern from earlier is now resolved architecturally, not just documented

Previously, Pipeline.CreateBuilder(args) silently inferred the working directory from the caller's [CallerFilePath]-captured source location for every pipeline, which was a behavioral change for existing consumers hidden inside an overload's default parameter. This commit splits that into two explicit methods (src/ModularPipelines/Pipeline.cs:9-51):

• CreateBuilder(string[]? args = null) — reverted to the original, pre-PR behavior: just forwards args, no source-path inference. This is what src/ModularPipelines.Build/Program.cs and every doc example already use, so it's a genuine no-op for existing consumers rather than a "trust the docstring" fix.
• CreateBuilderFromSource(args, [CallerFilePath] sourceFilePath) — the new opt-in method that does the project-directory inference. Only the source-generator template (src/ModularPipelines.Templates/templates/modularpipeline/Program.cs:9) uses it.

This is a better fix than the docs-only approach from the previous round: the two behaviors are now distinguished by method name instead of by an implicit default parameter, so a reader (or IntelliSense) sees the working-directory inference happening explicitly at the call site. There's also a new reflection-based test (PipelineWorkingDirectoryTests.CreateBuilderRetainsSingleArgumentBinarySignature) guarding the plain CreateBuilder(string[]) signature stays intact.

PipelineDirectory.TryFindPipelineProject now walks ancestors from the source directory

src/ModularPipelines/PipelineDirectory.cs:45-50 changed from checking only the exact source directory before falling back to AppContext.BaseDirectory, to walking all ancestors of the source directory first via FindAncestor. This is an intentional generalization to support pipeline entry points living in a nested folder (e.g. src/Pipeline/Program.cs), and is covered by the updated CreateBuilderRetainsSourceInferenceFromNestedDirectory test. Reasonable — no issue.

Checksum/Zip now correctly resolve relative paths through PipelineWorkingDirectory

Checksum.Md5 and Zip.ZipFolder/UnZipToFolder (src/ModularPipelines/Context/Checksum.cs, src/ModularPipelines/Context/Zip.cs) now resolve their path arguments via the injected PipelineWorkingDirectory before touching the filesystem, closing a gap where these two context APIs previously didn't participate in the new working-directory abstraction while Command/FilesContext/FileSystemContext/EnvironmentContext already did. The new PipelineWorkingDirectoryTests observation record now asserts checksum/zip/unzip paths resolve against the configured pipeline working directory, not just environment/file/command paths as before.

No compile or logic issues found in this diff. The fix commit is well-targeted at exactly the concern raised previously, and the design (explicit opt-in method vs. implicit default-parameter behavior change) is the right call — good to merge from my read.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48b417cc26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Set module cache roots to the pipeline working directory

When a pipeline is created with WorkingDirectory/CreateBuilderFromSource and module caching is enabled, this new scoped directory is only registered for command/file contexts; ModuleCacheOptions.WorkingDirectory still defaults to Directory.GetCurrentDirectory() (src/ModularPipelines/Caching/ModuleCacheOptions.cs:11), and cache input/artifact resolution uses that option. In the published-output or template scenario where process CWD differs from the pipeline project, context.Files.WriteAsync("input.txt") writes under the pipeline directory but [CacheInputs("input.txt")] / [ProducesArtifact("out.txt")] fingerprint and restore under the process CWD; if the CWD has no matching input, the real input is omitted from the fingerprint and stale cache hits can be reused. Initialize the cache option from _environment.WorkingDirectory unless the user overrides it.

Useful? React with 👍 / 👎.