Remove BDD process-global state (#492)

[leynos]

Summary

This branch removes the last process-global environment and working-directory
coordination from rstest-bdd scenarios. It retains in-process assertions via
injected seams, preserves process-CWD semantics for explicit relative
configuration selectors, and removes BDD scenario serialization.

Closes #492.

Routing inventory

Scenario class	Route	Reason
Spawned netsuke command scenarios	A	assert_cmd builds a child with env_clear() and forwards only scenario-tracked variables.
In-process configuration, manifest, IR and stdlib scenarios	B	Their Cli, Manifest, BuildGraph or render assertions require the injected environment boundary.
CWD sites in configuration discovery, manifest parsing and IR compilation	B	Their assertions are in process; -C or absolute manifest paths remove the need for a process CWD change.

Review walkthrough

• BDD fixture state no longer carries a global environment/CWD guard.
• Configuration BDD steps and discovery retain hermetic in-process behaviour without chdir.
• Child-process regression coverage proves --config and NETSUKE_CONFIG remain process-CWD-relative under -C.

Validation

• make markdownlint: passed.
• make check-fmt: passed.
• make lint: passed, including Rustdoc, Clippy and Whitaker.
• make test: passed — 2,317 tests, 3 skipped, plus doctests.
• Focused configuration-discovery E2E suite: passed, including 32 generated selector/anchor cases.
• Focused configuration-discovery and glob/Jinja/foreach BDD scenarios: passed.
• coderabbit review --agent: completed with zero comments after each milestone.

References

• Lody session

[coderabbitai]

Warning

Your free Security trial is over. An organization admin can activate billing to continue.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5c2cdcc-1cf8-43e8-b3c1-51abf8235bad

📥 Commits

Reviewing files that changed from the base of the PR and between d533911 and 7125e89.

📒 Files selected for processing (6)

• src/cli/discovery.rs
• src/cli/discovery_layer_tests.rs
• tests/bdd/fixtures/mod.rs
• tests/bdd/steps/configuration_discovery.rs
• tests/bdd/steps/ir.rs
• tests/bdd/steps/manifest/mod.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• leynos/monotony (auto-detected)
• leynos/rstest-bdd (auto-detected)
• leynos/ortho-config (auto-detected)
• leynos/whitaker (auto-detected)
• leynos/shared-actions (auto-detected)

💤 Files with no reviewable changes (1)

• tests/bdd/fixtures/mod.rs

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

Summary

• Remove process-global environment and working-directory coordination from rstest-bdd scenarios.
• Route spawned netsuke scenarios through child-process environment isolation.
• Use injected environment boundaries for in-process configuration, manifest, IR, and stdlib scenarios.
• Remove EnvLock, CwdGuard, env_mutation.rs, and related TestWorld state and cleanup.
• Resolve explicit relative configuration paths against cli.directory, following the -C/--directory contract.
• Keep manifest and IR processing in-process by using absolute test manifest paths.
• Simplify configuration, manifest, and IR BDD steps so they do not mutate process-global state.
• Add regression coverage for configuration discovery.
• Update routing documentation, BDD steps, and fixtures for issue #492.
• Pass formatting, linting, 2,315 tests with three skipped tests, doctests, and focused BDD scenarios.

Walkthrough

Resolve relative configuration paths against the CLI directory. Remove process-global working-directory mutation and related guards from BDD fixtures and steps. Add coverage for relative configuration discovery.

Changes

Configuration discovery and BDD state handling

Layer / File(s)	Summary
Resolve explicit configuration paths src/cli/discovery.rs, src/cli/discovery_layer_tests.rs	Resolve relative explicit configuration paths against cli.directory. Keep absolute paths unchanged. Test successful loading from the CLI directory.
Remove BDD working-directory mutation tests/bdd/fixtures/mod.rs, tests/bdd/steps/configuration_discovery.rs, tests/bdd/steps/ir.rs, tests/bdd/steps/manifest/mod.rs	Remove global-state guards and process working-directory changes. Keep relative test paths rooted at the project directory. Simplify configuration-file helpers and callers.

Suggested labels: Issue

Poem

Relative paths find their way,
The CLI marks where they stay.
BDD steps leave CWD still,
Guards retire by testing’s will.
Config loads, clean and bright.

🚥 Pre-merge checks | ✅ 16 | ❌ 4

❌ Failed checks (4 warnings)

Check name	Status	Explanation	Resolution
User-Facing Documentation	⚠️ Warning	src/cli/discovery.rs now resolves relative explicit paths under --directory, but docs/users-guide.md still documents the opposite and the PR changes no documentation.	Update docs/users-guide.md to document relative --config and NETSUKE_CONFIG resolution under --directory; add the required migration note for this changed behaviour.
Developer Documentation	⚠️ Warning	Update the developer guide and ADR: the commit removes TestWorld::global_state_lock and CWD mutation, but the guide still documents both and omits the new BDD Route A/Route B boundary.	Document the injected in-process versus isolated-child routing, absolute path handling, and revised TestWorld fields; remove stale EnvLock/CwdGuard guidance and record the decision in an ADR.
Testing (Unit And Behavioural)	⚠️ Warning	The commit adds only a unit test calling private discover_file_layers; it adds no binary-level test for the externally visible -C/--directory plus relative --config workflow.	Add an assert_cmd end-to-end test that runs netsuke -C <dir> --config relative.toml ... and asserts that the selected configuration affects the command outcome.
Testing (Property / Proof)	⚠️ Warning	The patch introduces effective-path behaviour over arbitrary relative/absolute paths and directories, but adds only one fixed relative-path test; existing proptest covers selector precedence, not t...	Add a substantive proptest for generated directories and relative/absolute config paths, asserting relative paths join the directory and absolute paths remain unchanged.

✅ Passed checks (16 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Accept the implementation: it addresses issue #492 by removing BDD global-state coordination and preserving in-process coverage.
Out of Scope Changes check	✅ Passed	Accept the changes: relative-path resolution and its regression test directly support CWD removal and preserve the -C/--directory contract.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files.
Testing (Overall)	✅ Passed	Pass this check: the new isolated test loads a temp config via relative --config and cli.directory, asserts the effective path, and BDD scenarios assert the loaded emoji value.
Module-Level Documentation	✅ Passed	Keep this change: all six changed Rust modules begin with //! documentation that states their purpose and, where needed, their relationship to related components.
Testing (Compile-Time / Ui)	✅ Passed	PASS: The PR changes runtime configuration discovery, not compile-time behaviour. A focused test asserts loading and effective paths, and existing discovery tracing uses normalised snapshots.
Unit Architecture	✅ Passed	Keep this change: the diff removes BDD CWD mutation and global locks; discovery keeps injected environment access, explicit load errors, and deferred diagnostics visible.
Domain Architecture	✅ Passed	Keep this change: the diff is confined to CLI configuration discovery and BDD infrastructure; no core domain code gains filesystem, environment, transport, or persistence coupling.
Observability	✅ Passed	The path-resolution change reuses existing bounded discovery diagnostics: selector, effective-path hash, branch, and classified load failures are emitted; no new service boundary or metric need is...
Security And Privacy	✅ Passed	The diff adds only PathBuf-based config resolution and test isolation. Existing diagnostics remain bounded, and no new secret, auth, permission, unsafe-parser, injection, or data-exposure path is i...
Performance And Resource Use	✅ Passed	Accept the change: the diff adds one bounded path join and one diagnostic path allocation per explicit discovery, with no new unbounded collection, loop, retry, blocking work, or repeated I/O.
Concurrency And State	✅ Passed	Pass: remove-only changes eliminate BDD CWD/global-state mutation; per-scenario state, injected environments, isolated child commands, absolute manifest paths, and a relative-config regression test...
Architectural Complexity And Maintainability	✅ Passed	The diff adds no abstraction, dependency, layer, or extension point; it removes global-state coordination and adds one local effective-path calculation with a focused test.
Rust Compiler Lint Integrity	✅ Passed	The commit adds no broad suppression or artificial anchor. It removes obsolete CWD-lock code, and its single new PathBuf copy supports the effective-path ownership boundary.
Title check	✅ Passed	The title accurately describes removal of BDD process-global state and references the linked issue as (#492).
Description check	✅ Passed	The description clearly relates to the changes and explains the removal of process-global environment and working-directory coordination.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-492-migrate-rstest-bdd-scenarios-off-in-process-environment-and-cwd-mutation

---

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

[sourcery-ai]

Reviewer's Guide

Removes the remaining process-global environment and CWD guarding from BDD test infrastructure by relying on explicit paths and the CLI -C/--directory contract, while adding a regression test to lock in the new discovery behavior for relative config paths.

Flow diagram for explicit configuration discovery with -C

flowchart TD
    A[CLI with directory and config path] --> B{path.is_relative}
    B -->|yes and directory set| C[directory.join path]
    B -->|no or directory unset| D[path.to_path_buf]
    C --> E[load_layers_from_path_with_warning]
    D --> E
    E --> F[Explicit FileLayerTrace with effective path]

Loading

File-Level Changes

Change	Details	Files
Make explicit config discovery obey the CLI working-directory (-C/--directory) semantics instead of the process CWD.	Compute an effective_path for explicit configuration files by resolving relative paths against cli.directory when present Pass the effective_path to load_layers_from_path_with_warning and to BoundedConfigPath::from_path to keep tracing aligned with the resolved file location Document the relationship between -C and CLI path handling in discovery code comments	src/cli/discovery.rs
Add a regression test ensuring relative explicit configs are resolved against the CLI directory.	Create a temporary directory containing cli.toml and construct a Cli with config set to a relative path and directory set to that temp dir Invoke discover_file_layers and assert there are no discovery errors Assert that the discovered layer path matches the absolute path of the temp cli.toml file	src/cli/discovery_layer_tests.rs
Remove scenario-scoped global environment/CWD locking from the BDD test world and its teardown.	Delete the GlobalStateGuard struct and its EnvLock/CwdGuard members Remove the global_state_lock field from TestWorld and its management in Drop Stop importing and using EnvLock and CwdGuard from test_support	tests/bdd/fixtures/mod.rs
Stop mutating the process CWD in configuration-discovery BDD steps; rely on the new discovery semantics instead.	Simplify write_config_file to only write files into the scenario temp directory without a chdir flag Remove calls to world.ensure_global_state_lock and std::env::set_current_dir from configuration step helpers Update all step constructors that previously passed chdir=true/false to call the simplified write_config_file	tests/bdd/steps/configuration_discovery.rs
Stop using global CWD changes in manifest/IR compilation BDD steps by relying on absolute test-data paths.	In IR steps, keep the process CWD at the project root and resolve tests/... manifest paths to absolute paths using CARGO_MANIFEST_DIR without acquiring a global state lock In manifest parsing steps, likewise resolve tests/... paths to absolute paths while removing the ensure_global_state_lock calls and process set_current_dir invocations Update comments to clarify that glob patterns now rely on a stable project-root CWD plus absolute manifest paths rather than runtime CWD mutation	tests/bdd/steps/ir.rs tests/bdd/steps/manifest/mod.rs

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#492	Migrate all BDD scenarios away from direct process-global environment mutation by routing spawned command scenarios through child-process environment configuration and retaining injected/tracked environment state for in-process scenarios.	✅
#492	Remove process-global working-directory mutation from BDD scenarios by replacing CWD changes with absolute paths or explicit CLI directory handling, while preserving relative configuration and manifest behavior.	✅
#492	Remove the BDD global-state coordination machinery, including the EnvLock/CWD guard state from TestWorld, so scenarios no longer serialize on process-global state.	✅

Possibly linked issues

• Migrate rstest-bdd scenarios off in-process environment and CWD mutation #492: The PR directly implements the issue’s required environment/CWD migration and removes the global state guards.
• Migrate rstest-bdd scenarios off in-process environment and CWD mutation #492: The PR directly implements issue Migrate rstest-bdd scenarios off in-process environment and CWD mutation #492 by removing BDD global-state guards and replacing CWD-dependent behavior with explicit paths.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7125e89296

ℹ️ 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]

Keep explicit config paths anchored to the process CWD

When -C/--directory is combined with a relative --config (or relative NETSUKE_CONFIG), this now resolves the selector beneath the project directory rather than the invoking shell's working directory. That contradicts the public contract in docs/users-guide.md:896-897 and the design invariant in docs/netsuke-design.md:3117-3120, and can silently load a same-named project config instead of the explicitly selected file; keep the resolved selector path unchanged while continuing to use cli.directory only for automatic project discovery.

Useful? React with 👍 / 👎.

[codescene-access]

No quality gates enabled for this code.