Remove DYLD_LIBRARY_PATH and LD_LIBRARY_PATH from native launch

[bobtista]

Problem

Native launch currently sets DYLD_LIBRARY_PATH / LD_LIBRARY_PATH. Measurement showed these are not required for the engine to resolve its libraries.

Keeping them is actively harmful:

• dyld searches DYLD_LIBRARY_PATH before @executable_path, so any same-named library elsewhere on the path silently overrides the intended one. This is a real correctness and supply-chain risk, not a theoretical one.
• Hardened-runtime signing ignores DYLD_LIBRARY_PATH anyway, so the variable will stop having any effect the moment GenHub is signed and notarized — meaning current behaviour would change silently at signing time.

Proposed approach

Remove both variables from the native launch environment and confirm the engine still resolves its libraries via @executable_path / RPATH.

Acceptance criteria

• Native launch sets neither variable.
• Engine launches and reaches its main loop on macOS and Linux.
• Behaviour is unchanged before and after hardened-runtime signing.

Notes

Stack PR 4 (feat/native-launch). Related: the notarization issue, where this becomes load-bearing.

[coderabbitai]

Coding Plan

Summary

• Introduce a single synchronized head+tail bounded buffer primitive that owns all truncation/byte/line-length limits and concurrency safety.
• In GameProcessManager, redirect stdout/stderr and start the read pumps before any wait, treating Data == null as EOF, and follow the timed WaitForExit(timeout) with a parameterless WaitForExit()/EOF-signal await to eliminate the drain race.
• Apply the same drain-then-read discipline to the immediate-crash path and surface the captured output through GameProcessInfo/failure messages/exit-event args.
• Note the ticket title is a misnomer: no DYLD_LIBRARY_PATH/LD_LIBRARY_PATH references exist, so no env-var removal is performed — the work is entirely the bounded, race-free capture fix.

Design Choices

Design Choice 1: Ticket title vs. body mismatch regarding DYLD_LIBRARY_PATH/LD_LIBRARY_PATH

Options Considered:

1. Treat the title literally and add code/docs to strip those env vars from the launch environment.
2. Treat the body + acceptance criteria as authoritative and implement the output-capture race fix; disregard the non-existent env vars.

Chosen Option: 2

Rationale: There is nothing to remove (zero references exist), and every acceptance criterion concerns bounded, race-free output capture — so the capture fix is the real deliverable.

Design Choice 2: Existence of output-capture path from earlier stack PR

Options Considered:

1. Assume capture already exists upstream and only add the drain/ordering fix.
2. Implement the full capture path (redirection + handlers + buffer) plus the drain/ordering fix, so the plan is self-contained.

Chosen Option: 2

Rationale: Planning the complete path is safe and idempotent — if pieces already exist from the stack, they can be reconciled during implementation; if not, the ticket's acceptance criteria are still fully met.

💡 User Tips

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

Implementation Steps

Phase 1: Bounded Head+Tail Output Buffer Utility

Introduce a reusable, thread-safe capture buffer that retains both the startup context (first N lines) and the failure tail (last N lines), enforces total-byte and per-line-length limits, and reports whether truncation occurred. This isolates all bounding/concurrency logic away from process-lifecycle code and directly satisfies the "bounded memory," "head and tail retained," and "no data race" criteria.

Task 1: Add capture limit constants

Define the tunable bounds used by the buffer, following the existing constant conventions.

• Add public const int limits to GenHub.Core/Constants/ProcessConstants.cs (or a small dedicated constants class if it grows), e.g. max retained head lines, max retained tail lines, max total captured bytes, and max characters per line.
• Follow the established Max<Noun><Unit> naming and include XML doc comments per each member, mirroring ValidationLimits.cs/DownloadDefaults.cs.

Task 2: Create the synchronized head+tail ring buffer class

Build the greenfield capture primitive that all process output flows through.

• Add a new internal/public class (e.g. ProcessOutputBuffer) under an appropriate location such as GenHub/GenHub/Features/GameProfiles/Infrastructure/ (or GenHub.Core helpers if it must be shared), with nullable annotations enabled.
• Maintain a fixed-capacity head list (first N lines, never overwritten) and a fixed-capacity tail ring (last N lines), plus a running total-byte counter and a Truncated flag.
• On each appended line: truncate the line to the max-per-line character limit, enforce the total-byte cap (stop retaining / set Truncated once exceeded), and place the line into head or roll it through the tail ring accordingly.
• Guard all mutation and snapshot reads with a single private readonly object _lock = new(); (per codebase precedent), since OutputDataReceived/ErrorDataReceived fire on background threads.
• Expose a snapshot accessor that returns an ordered, immutable view (head, an optional truncation marker line, then tail) suitable for embedding in diagnostics.

🤖 Prompt for AI agents

Implement a reusable, thread-safe, bounded head+tail output capture buffer
utility to isolate truncation/byte/line-length limits and concurrency safety
from process-lifecycle code.

- Add `public const int` capture limit constants to
`GenHub.Core/Constants/ProcessConstants.cs` (or a new small dedicated constants
class if warranted): max retained head lines, max retained tail lines, max total
captured bytes, and max characters per line. Follow the established
`Max<Noun><Unit>` naming convention and add XML doc comments per member,
mirroring the style of `ValidationLimits.cs`/`DownloadDefaults.cs`.
- Create a new class (e.g. `ProcessOutputBuffer`) under an appropriate location
such as `GenHub/GenHub/Features/GameProfiles/Infrastructure/` (or a shared
`GenHub.Core` helper location), with nullable annotations enabled.
- The buffer must maintain: a fixed-capacity head list (first N lines, never
overwritten), a fixed-capacity tail ring (last N lines), a running total-byte
counter, and a `Truncated` flag.
- On each appended line: truncate the line to the max-per-line character limit;
enforce the total-byte cap by stopping retention and setting `Truncated` once
exceeded; place the line into the head list or roll it through the tail ring
depending on capacity state.
- Guard all mutation and snapshot reads with a single `private readonly object
_lock = new();`, since the data will be appended from
`OutputDataReceived`/`ErrorDataReceived` background threads.
- Expose a snapshot accessor returning an ordered, immutable view (head lines,
an optional truncation marker line, then tail lines) suitable for embedding
directly into diagnostic messages.

Phase 2: Correct Capture Wiring and Wait Sequencing in GameProcessManager

Redirect and drain stdout/stderr through the new buffer, and fix the exit/drain ordering so diagnostics are complete even for a process that exits immediately. This phase implements the core race fix and the "complete diagnostics on immediate exit" and "same drain check to stdout" requirements.

Task 1: Enable redirection and attach data handlers before any wait

Ensure output pumps are running before the process can exit.

• In GameProcessManager.StartProcessAsync (GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs), set RedirectStandardOutput = true and RedirectStandardError = true on the ProcessStartInfo construction block (UseShellExecute = false is already present).
• Immediately after Process.Start() succeeds and before the crash-detection delay, create a ProcessOutputBuffer for the process, subscribe OutputDataReceived/ErrorDataReceived, and call BeginOutputReadLine()/BeginErrorReadLine().
• In both handlers, treat a null e.Data as the EOF signal (mark that stream drained via a per-stream completion signal such as a TaskCompletionSource/ManualResetEventSlim) rather than appending; append non-null lines to the buffer.
• Store each process's buffer and stream-completion signals in a new ConcurrentDictionary<int, ...> alongside _managedProcesses, keyed by process id.

Task 2: Fix the WaitForExit drain sequencing

Guarantee async handlers have fully drained before reading captured output.

• Replace the current if (configuration.WaitForExit) { process.WaitForExit(timeoutMs); } block so that, after the timed wait indicates the process has exited, a parameterless process.WaitForExit() (and/or awaiting both stream-EOF signals) is issued to ensure OutputDataReceived/ErrorDataReceived have completed.
• Apply the same drain wait to the immediate-crash-detection path (the ProcessStartDelayMs delay + HasExited branch): before reading the buffer for a failed early-exit, wait for both stdout and stderr EOF signals so diagnostics are complete for a process that exits immediately.
• Ensure EnableRaisingEvents = true / Exited subscription happens as part of the early setup (Task 2.1) so fast-exiting processes are not missed.

Task 3: Surface captured diagnostics to callers

Make the drained buffer available on both the failure and success paths.

• On the early-exit failure path, append the captured stderr/stdout snapshot to the OperationResult failure message (alongside the existing exit-code mapping) so a crashing launch reports the actual engine output.
• Add non-breaking properties to GenHub.Core/Models/Launching/GameProcessInfo.cs (plain mutable class) to expose the recent captured output/error snapshot and a bool OutputTruncated flag; populate them when constructing GameProcessInfo.
• Optionally enrich GameProcessExitedEventArgs and the OnProcessExited handler with the final buffer snapshot so subscribers receive the failure tail on exit.

Task 4: Clean up per-process buffers on termination/exit/dispose

Prevent unbounded growth across many launches.

• Remove and release the buffer/completion entries for a process in OnProcessExited, TerminateProcessAsync, CleanupDeadProcesses, and Dispose() — mirroring how _managedProcesses entries are already pruned.
• Ensure redirected streams/read loops are unsubscribed/closed when the process object is disposed to avoid leaked handlers.

🤖 Prompt for AI agents

Fix the EOF race in `GameProcessManager.cs` by wiring correct stdout/stderr
redirection and capture, and correcting the wait/drain sequencing so diagnostics
are complete even when a process exits immediately.

- In `StartProcessAsync` in
`GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs`, set
`RedirectStandardOutput = true` and `RedirectStandardError = true` on the
`ProcessStartInfo` (alongside the existing `UseShellExecute = false`).
- Immediately after `Process.Start()` succeeds and before the crash-detection
delay: create a `ProcessOutputBuffer` for the process, subscribe
`OutputDataReceived`/`ErrorDataReceived` handlers, and call
`BeginOutputReadLine()`/`BeginErrorReadLine()`. Also ensure `EnableRaisingEvents
= true` and the `Exited` subscription happen at this same early point so
fast-exiting processes are not missed.
- In the output/error handlers, treat `e.Data == null` as the EOF signal for
that stream (mark completion via a per-stream signal such as
`TaskCompletionSource`/`ManualResetEventSlim`) rather than appending it as data;
append non-null lines to the buffer.
- Store each process's buffer and stream-completion signals in a new
`ConcurrentDictionary<int, ...>` alongside the existing `_managedProcesses`
collection, keyed by process id.
- Replace the current `if (configuration.WaitForExit) {
process.WaitForExit(timeoutMs); }` logic: after the timed wait indicates the
process exited, issue a parameterless `process.WaitForExit()` and/or await both
stream-EOF signals to ensure `OutputDataReceived`/`ErrorDataReceived` have fully
completed before reading captured output.
- Apply this same drain-then-read discipline to the immediate-crash-detection
path (the `ProcessStartDelayMs` delay + `HasExited` branch): wait for both
stdout and stderr EOF signals before reading the buffer for a failed early-exit.
- On the early-exit failure path, append the captured stderr/stdout snapshot to
the `OperationResult` failure message alongside the existing exit-code mapping.
- Add non-breaking properties to
`GenHub.Core/Models/Launching/GameProcessInfo.cs` to expose the recent captured
output/error snapshot and a `bool OutputTruncated` flag, and populate them when
constructing `GameProcessInfo`.
- Optionally enrich `GameProcessExitedEventArgs` and the `OnProcessExited`
handler with the final buffer snapshot so subscribers receive the failure tail
on exit.
- Clean up: remove and release the buffer/completion entries for a process in
`OnProcessExited`, `TerminateProcessAsync`, `CleanupDeadProcesses`, and
`Dispose()`, mirroring the existing pruning of `_managedProcesses` entries;
ensure redirected streams/read loops are unsubscribed/closed on disposal to
avoid leaked handlers.

Phase 3: Tests for Bounded, Race-Free Capture

Add focused tests validating each acceptance criterion, following the existing real-process test style in GameProcessManagerTests.cs.

Task 1: Unit-test the output buffer

Add tests (under GenHub.Tests.Core) for the new buffer verifying head+tail retention on overflow, the Truncated flag, per-line length capping, and total-byte bounding.

Task 2: Integration-test capture on a real short-lived process

Extend GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs with a script that writes to stdout/stderr and exits immediately, asserting captured diagnostics are non-empty and complete.

Add a high-volume writer test asserting memory stays bounded and both head and tail lines are present after truncation.

🤖 Prompt for AI agents

Add tests validating the bounded, race-free output capture behavior implemented
in Phases 1 and 2, following the existing real-process test style used in
`GameProcessManagerTests.cs`.

- Add unit tests under `GenHub.Tests.Core` for the new `ProcessOutputBuffer`,
verifying: head+tail retention on overflow, correct setting of the `Truncated`
flag, per-line length capping, and total-byte bounding.
- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs`
with an integration test using a script that writes to stdout/stderr and exits
immediately, asserting the captured diagnostics are non-empty and complete
(validating the drain-sequencing fix for immediate exits).
- Add a high-volume writer test asserting memory usage stays bounded and that
both head and tail lines are present in the snapshot after truncation occurs.

Research

Native game processes are launched through GameLauncher.cs (orchestration and EnvironmentVariables pass-through) and GameProcessManager.cs (actual ProcessStartInfo construction, start, crash detection, termination, lifecycle events). Today the process manager starts processes with UseShellExecute = false but never redirects stdout/stderr, has no output-capture logic, and wires EnableRaisingEvents/Exited only after an optional blocking WaitForExit(timeoutMs). Diagnostics on failure are limited to the numeric exit code mapped to a small Windows-only NTSTATUS lookup. No ring-buffer/bounded-buffer utility exists anywhere. Constants live in GenHub.Core/Constants/ (e.g. ProcessConstants.cs, ValidationLimits.cs) with a public const int Max<Noun><Unit> convention; locking uses private readonly object _xxxLock = new(); per codebase precedent. Result/info models (GameProcessInfo, GameLaunchConfiguration) are plain mutable classes, so new properties are non-breaking.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement a reusable, thread-safe, bounded head+tail output capture buffer
utility to isolate truncation/byte/line-length limits and concurrency safety
from process-lifecycle code.

- Add `public const int` capture limit constants to
`GenHub.Core/Constants/ProcessConstants.cs` (or a new small dedicated constants
class if warranted): max retained head lines, max retained tail lines, max total
captured bytes, and max characters per line. Follow the established
`Max<Noun><Unit>` naming convention and add XML doc comments per member,
mirroring the style of `ValidationLimits.cs`/`DownloadDefaults.cs`.
- Create a new class (e.g. `ProcessOutputBuffer`) under an appropriate location
such as `GenHub/GenHub/Features/GameProfiles/Infrastructure/` (or a shared
`GenHub.Core` helper location), with nullable annotations enabled.
- The buffer must maintain: a fixed-capacity head list (first N lines, never
overwritten), a fixed-capacity tail ring (last N lines), a running total-byte
counter, and a `Truncated` flag.
- On each appended line: truncate the line to the max-per-line character limit;
enforce the total-byte cap by stopping retention and setting `Truncated` once
exceeded; place the line into the head list or roll it through the tail ring
depending on capacity state.
- Guard all mutation and snapshot reads with a single `private readonly object
_lock = new();`, since the data will be appended from
`OutputDataReceived`/`ErrorDataReceived` background threads.
- Expose a snapshot accessor returning an ordered, immutable view (head lines,
an optional truncation marker line, then tail lines) suitable for embedding
directly into diagnostic messages.
===============================================================================

Task: 2

Fix the EOF race in `GameProcessManager.cs` by wiring correct stdout/stderr
redirection and capture, and correcting the wait/drain sequencing so diagnostics
are complete even when a process exits immediately.

- In `StartProcessAsync` in
`GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs`, set
`RedirectStandardOutput = true` and `RedirectStandardError = true` on the
`ProcessStartInfo` (alongside the existing `UseShellExecute = false`).
- Immediately after `Process.Start()` succeeds and before the crash-detection
delay: create a `ProcessOutputBuffer` for the process, subscribe
`OutputDataReceived`/`ErrorDataReceived` handlers, and call
`BeginOutputReadLine()`/`BeginErrorReadLine()`. Also ensure `EnableRaisingEvents
= true` and the `Exited` subscription happen at this same early point so
fast-exiting processes are not missed.
- In the output/error handlers, treat `e.Data == null` as the EOF signal for
that stream (mark completion via a per-stream signal such as
`TaskCompletionSource`/`ManualResetEventSlim`) rather than appending it as data;
append non-null lines to the buffer.
- Store each process's buffer and stream-completion signals in a new
`ConcurrentDictionary<int, ...>` alongside the existing `_managedProcesses`
collection, keyed by process id.
- Replace the current `if (configuration.WaitForExit) {
process.WaitForExit(timeoutMs); }` logic: after the timed wait indicates the
process exited, issue a parameterless `process.WaitForExit()` and/or await both
stream-EOF signals to ensure `OutputDataReceived`/`ErrorDataReceived` have fully
completed before reading captured output.
- Apply this same drain-then-read discipline to the immediate-crash-detection
path (the `ProcessStartDelayMs` delay + `HasExited` branch): wait for both
stdout and stderr EOF signals before reading the buffer for a failed early-exit.
- On the early-exit failure path, append the captured stderr/stdout snapshot to
the `OperationResult` failure message alongside the existing exit-code mapping.
- Add non-breaking properties to
`GenHub.Core/Models/Launching/GameProcessInfo.cs` to expose the recent captured
output/error snapshot and a `bool OutputTruncated` flag, and populate them when
constructing `GameProcessInfo`.
- Optionally enrich `GameProcessExitedEventArgs` and the `OnProcessExited`
handler with the final buffer snapshot so subscribers receive the failure tail
on exit.
- Clean up: remove and release the buffer/completion entries for a process in
`OnProcessExited`, `TerminateProcessAsync`, `CleanupDeadProcesses`, and
`Dispose()`, mirroring the existing pruning of `_managedProcesses` entries;
ensure redirected streams/read loops are unsubscribed/closed on disposal to
avoid leaked handlers.
===============================================================================

Task: 3

Add tests validating the bounded, race-free output capture behavior implemented
in Phases 1 and 2, following the existing real-process test style used in
`GameProcessManagerTests.cs`.

- Add unit tests under `GenHub.Tests.Core` for the new `ProcessOutputBuffer`,
verifying: head+tail retention on overflow, correct setting of the `Truncated`
flag, per-line length capping, and total-byte bounding.
- Extend
`GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/GameProcessManagerTests.cs`
with an integration test using a script that writes to stdout/stderr and exits
immediately, asserting the captured diagnostics are non-empty and complete
(validating the drain-sequencing fix for immediate exits).
- Add a high-volume writer test asserting memory usage stays bounded and that
both head and tail lines are present in the snapshot after truncation occurs.

💡 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 #317" diff. Closed by PR C (feat/native-launch). See #327 for the PR structure.

Why this is not just hygiene: hardened-runtime signing ignores DYLD_LIBRARY_PATH, so the variable stops having any effect the moment GenHub is signed and notarized (#322). Leaving it in place means behaviour would change silently at signing time. Removing it now makes pre- and post-notarization behaviour identical.

[bobtista]

Completed by #332.

DYLD_LIBRARY_PATH and LD_LIBRARY_PATH are no longer set. The only remaining references in GameLauncher.cs are documentation comments recording why the variables are deliberately not used — including that the hardened runtime ignores DYLD_LIBRARY_PATH, which is what makes this load-bearing for #322.