Materialize workspace executables atomically; current sequence can permanently brick a workspace

[bobtista]

Problem

Materializing the workspace executable currently does copy → delete → move → chmod. That sequence has two crash windows:

1. After delete, before move — the destination is missing.
2. After move, before chmod — the destination exists but is not executable.

The second is the dangerous one: it permanently bricks the workspace, because verification never mutates the file. A workspace that lands in this state will fail every subsequent launch and will not self-heal, since nothing in the verification path fixes permissions.

Proposed approach

Make materialization atomic:

• Copy to a temporary file.
• chmod the temporary file.
• rename into place.

rename is atomic on both target platforms, so the destination is either the old valid file or the new valid file — never a half-materialized one.

Acceptance criteria

• No window exists in which the destination is missing or non-executable.
• A crash at any point leaves the workspace in a launchable state.
• Existing workspaces already in the bricked state are repaired or detected (decide and document which).

Notes

Stack PR 3 (feat/unix-filesystem).

[coderabbitai]

Coding Plan

Summary

• Remove any automatic injection of DYLD_LIBRARY_PATH / LD_LIBRARY_PATH from the native launch environment-building path (GameLauncher config construction and GameProcessManager env application), while leaving the generic user/profile EnvironmentVariables passthrough fully intact.
• Because this is PR 4 in the stacked feat/native-launch branch and the variables are absent in the current checkout, the change is framed to both delete stack-introduced injection and preventatively guarantee the pipeline never adds these keys.
• Lock in the behaviour with regression tests in the existing launch/process test suites and document the reliance on @executable_path / RPATH.

Design Choices

Design Choice 1: Where does the `DYLD_LIBRARY_PATH` / `LD_LIBRARY_PATH` injection currently live, given the current checkout has none?

Options Considered:

1. Treat as pure removal against the stacked-PR base (delete the injection code added upstream in the stack).
2. Treat as preventative/greenfield hardening (ensure the launch pipeline never auto-injects these keys and add regression coverage).

Chosen Option: 1

Rationale: The ticket describes removing variables that do not exist in this snapshot; they are introduced by earlier PRs in the feat/native-launch stack. Both options are combined: the implementer should first locate the injection introduced by the stack base and remove it; if absent in the working branch, the same tasks reduce to verifying and locking in (via tests) that the native launch environment never contains these keys. This is robust regardless of which stacked commits are present.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Remove native library-path injection from the launch environment

This phase eliminates the automatic population of DYLD_LIBRARY_PATH / LD_LIBRARY_PATH while preserving the generic, user-facing environment-variable passthrough that legitimately exists for profiles.

Task 1: Locate and remove the automatic library-path injection in the native launch flow

Strip out any code that auto-computes and sets the two dynamic-linker variables when building the launch environment.

• In GenHub/Features/Launching/GameLauncher.cs, inspect the GameLaunchConfiguration construction inside the private LaunchProfileAsync overload for any logic that inserts DYLD_LIBRARY_PATH or LD_LIBRARY_PATH (e.g., derived from the workspace path) and remove it.
• Check GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs StartProcessAsync for any hardcoded addition of these keys to ProcessStartInfo.EnvironmentVariables and remove it; the generic loop copying configuration.EnvironmentVariables must remain untouched.
• Search any platform helpers (GenHub.Linux/**, and any macOS-oriented helper added by the stack base) for the same injection and remove it there as well.

Task 2: Preserve the generic profile environment-variable passthrough

Ensure only the automatic library-path behaviour is removed, not the user-configurable environment mechanism.

• Confirm GameLaunchConfiguration.EnvironmentVariables and GameProfile.EnvironmentVariables remain intact so users can still set arbitrary environment variables explicitly.
• Confirm the profile-defined dictionary continues to flow from GameLauncher → GameProcessManager → ProcessStartInfo unchanged, so a user who deliberately adds these keys is still honored (removal targets automatic injection only).

🤖 Prompt for AI agents

Implement the removal of automatic native library-path injection from the launch
environment while keeping the user-facing environment-variable passthrough
intact.

- In `GenHub/Features/Launching/GameLauncher.cs`, inspect the
`GameLaunchConfiguration` construction inside the private `LaunchProfileAsync`
overload for any logic that inserts `DYLD_LIBRARY_PATH` or `LD_LIBRARY_PATH`
(e.g., derived from the workspace path), and remove that logic.
- In `GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs`,
inspect `StartProcessAsync` for any hardcoded addition of these two keys to
`ProcessStartInfo.EnvironmentVariables` and remove it. Do not touch the generic
loop that copies `configuration.EnvironmentVariables` into
`ProcessStartInfo.EnvironmentVariables` — this must remain functioning exactly
as before.
- Search platform-specific helpers (e.g., `GenHub.Linux/**`, and any
macOS-oriented helper introduced by the stack base) for the same injection
pattern and remove it there as well.
- After removal, verify that `GameLaunchConfiguration.EnvironmentVariables` and
`GameProfile.EnvironmentVariables` remain intact and unmodified in structure, so
users can still explicitly set arbitrary environment variables including these
two keys if they choose to.
- Verify the profile-defined dictionary still flows unchanged from
`GameLauncher` through to `GameProcessManager` and into `ProcessStartInfo`,
confirming that only the automatic injection was removed and not the
user-configurable mechanism.

Phase 2: Verify behaviour and update documentation

This phase locks in the removal with regression coverage and reflects the reliance on @executable_path / RPATH in the docs.

Task 1: Add regression tests asserting the two variables are never auto-injected

Prevent reintroduction of the harmful behaviour.

• Extend GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/GameLauncherTests.cs to assert that a launch driven by a profile with no explicit environment variables produces a GameLaunchConfiguration whose EnvironmentVariables contains neither DYLD_LIBRARY_PATH nor LD_LIBRARY_PATH.
• Extend GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs to assert that StartProcessAsync does not add these keys to the process environment beyond what the caller supplies.
• Ensure a positive-path test still confirms user-supplied environment variables are propagated (guards Task 1.2).

Task 2: Update launch/environment documentation

Record the intended library-resolution behaviour so the removal is not accidentally reverted.

• Update docs/architecture.md (Launch section describing GameLaunchConfiguration / EnvironmentVariables) to state that native launch does not set DYLD_LIBRARY_PATH / LD_LIBRARY_PATH, and that the engine resolves libraries via @executable_path / RPATH, which keeps behaviour identical before and after hardened-runtime signing.
• Note the manual validation step (out-of-band, not automatable in CI): confirm the engine launches and reaches its main loop on both macOS and Linux with the variables absent.

🤖 Prompt for AI agents

Add regression coverage for the removal and update documentation to reflect the
new library-resolution behaviour.

- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/GameLauncherTests.cs`
with a test asserting that launching a profile with no explicit environment
variables produces a `GameLaunchConfiguration` whose `EnvironmentVariables`
contains neither `DYLD_LIBRARY_PATH` nor `LD_LIBRARY_PATH`.
- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs`
with a test asserting that `StartProcessAsync` does not add these two keys to
the process environment beyond what the caller explicitly supplies.
- Add or confirm a positive-path test verifying that user-supplied environment
variables (set explicitly by the caller/profile) are still correctly propagated
through the pipeline, to guard the passthrough behaviour preserved in Phase 1.
- Update `docs/architecture.md` in the Launch section describing
`GameLaunchConfiguration` / `EnvironmentVariables`: state explicitly that native
launch does not set `DYLD_LIBRARY_PATH` / `LD_LIBRARY_PATH`, and that the engine
resolves libraries via `@executable_path` / `RPATH`, ensuring identical
behaviour before and after hardened-runtime signing.
- Document the manual validation step (out-of-band, not automatable in CI):
confirm the engine launches and reaches its main loop on both macOS and Linux
with the variables absent.

Research

GenHub launches games through a two-stage pipeline. GameLauncher.LaunchProfileAsync (GenHub/Features/Launching/GameLauncher.cs) prepares a workspace and constructs a GameLaunchConfiguration (GenHub.Core/Models/Launching/GameLaunchConfiguration.cs), populating its EnvironmentVariables dictionary solely from the user-defined GameProfile.EnvironmentVariables. That configuration flows to GameProcessManager.StartProcessAsync (GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs), which copies each entry into ProcessStartInfo.EnvironmentVariables (only when UseShellExecute = false) — this is the single point where any environment variable actually reaches the child process.

A repository-wide search found no current references to DYLD_LIBRARY_PATH/LD_LIBRARY_PATH/RPATH; this work is PR 4 in the stacked feat/native-launch branch, so the injection being removed is expected to originate from earlier, not-yet-present PRs in that stack.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement the removal of automatic native library-path injection from the launch
environment while keeping the user-facing environment-variable passthrough
intact.

- In `GenHub/Features/Launching/GameLauncher.cs`, inspect the
`GameLaunchConfiguration` construction inside the private `LaunchProfileAsync`
overload for any logic that inserts `DYLD_LIBRARY_PATH` or `LD_LIBRARY_PATH`
(e.g., derived from the workspace path), and remove that logic.
- In `GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs`,
inspect `StartProcessAsync` for any hardcoded addition of these two keys to
`ProcessStartInfo.EnvironmentVariables` and remove it. Do not touch the generic
loop that copies `configuration.EnvironmentVariables` into
`ProcessStartInfo.EnvironmentVariables` — this must remain functioning exactly
as before.
- Search platform-specific helpers (e.g., `GenHub.Linux/**`, and any
macOS-oriented helper introduced by the stack base) for the same injection
pattern and remove it there as well.
- After removal, verify that `GameLaunchConfiguration.EnvironmentVariables` and
`GameProfile.EnvironmentVariables` remain intact and unmodified in structure, so
users can still explicitly set arbitrary environment variables including these
two keys if they choose to.
- Verify the profile-defined dictionary still flows unchanged from
`GameLauncher` through to `GameProcessManager` and into `ProcessStartInfo`,
confirming that only the automatic injection was removed and not the
user-configurable mechanism.
===============================================================================

Task: 2

Add regression coverage for the removal and update documentation to reflect the
new library-resolution behaviour.

- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/GameLauncherTests.cs`
with a test asserting that launching a profile with no explicit environment
variables produces a `GameLaunchConfiguration` whose `EnvironmentVariables`
contains neither `DYLD_LIBRARY_PATH` nor `LD_LIBRARY_PATH`.
- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs`
with a test asserting that `StartProcessAsync` does not add these two keys to
the process environment beyond what the caller explicitly supplies.
- Add or confirm a positive-path test verifying that user-supplied environment
variables (set explicitly by the caller/profile) are still correctly propagated
through the pipeline, to guard the passthrough behaviour preserved in Phase 1.
- Update `docs/architecture.md` in the Launch section describing
`GameLaunchConfiguration` / `EnvironmentVariables`: state explicitly that native
launch does not set `DYLD_LIBRARY_PATH` / `LD_LIBRARY_PATH`, and that the engine
resolves libraries via `@executable_path` / `RPATH`, ensuring identical
behaviour before and after hardened-runtime signing.
- Document the manual validation step (out-of-band, not automatable in CI):
confirm the engine launches and reaches its main loop on both macOS and Linux
with the variables absent.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!

[bobtista]

How this closes. Fixed inside the mac stack rather than by its own PR — folded into the owning branch before the PR opens, so there will be no separate "fixes #316" diff. Closed by PR A (feat/cross-platform-fixes). See #327 for the PR structure.

Filed as its own issue despite that, because this is data loss rather than tidiness: the post-move, pre-chmod window permanently bricks a workspace, and verification never mutates so it cannot self-heal. If a bricked workspace is ever reported, this should be findable.

[bobtista]

Completed by #338.

ExecutableFileSwap.cs provides the atomic swap, and WorkspaceValidator gained the repair path. Both are on development, and were backported to release/alpha-4 via #351.