Steam proxy preparation can leave the proxy installed after failure

[bobtista]

Problem

SteamLauncher.PrepareForProfileAsync replaces the game executable before validating all workspace prerequisites. A later failure or cancellation can return without restoring the original executable.

Expected behavior

Validate non-mutating prerequisites first. After replacement begins, failures and cancellation should restore the original executable, preserve pre-existing artifacts, and report rollback failures.

[coderabbitai]

Coding Plan

Summary

• Reorder PrepareForProfileAsync so all non-mutating prerequisite checks run and pass (as a non-throwing ValidationResult) before any executable replacement, eliminating the "proxy installed but prerequisite failed" window.
• Make the swap transactional: capture the original executable (and any manifest state), then wrap the mutation in try/catch/cleanup that restores the original on both failure and cancellation while preserving pre-existing artifacts.
• Report rollback outcomes explicitly, returning a distinct failure result (and error-level log) when restoration itself fails so the proxy-still-installed condition is visible to callers.
• Add the first tests for this behavior, asserting validate-first ordering, restore-on-failure/cancellation, artifact preservation, and rollback-failure reporting, following existing Moq-based launcher/strategy test patterns.

File-Level Changes and Follow-Up Guidance

• New interface in GenHub/GenHub.Core/Interfaces/ (e.g., ISteamLauncher): defines PrepareForProfileAsync returning a result type; model contract shape on ISteamManifestPatcher.cs.
• New/updated component SteamLauncher under GenHub/GenHub/Features/Launching/ (or Features/Manifest/): implements validate-first ordering, backup capture, transactional swap, idempotent restore helper, and rollback reporting.
• DI registration in the relevant module (ManifestModule.cs or GameLaunchingModule.cs): register the new component consistent with AddScoped<ISteamManifestPatcher, SteamManifestPatcher>().
• Possible touch to GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs: only if the proxy transaction must revert a manifest flag change during rollback — add a revert path invoked by SteamLauncher's cleanup; do not refactor its existing scan logic.
• New test class under GenHub/GenHub.Tests/GenHub.Tests.Core/Features/: establishes the validate-first and restore-on-failure/cancellation contract.
• Follow-up agents: reuse UserDataTrackerService.BackupExistingFileAsync semantics for backup/restore and WorkspaceStrategyBase.CleanupWorkspaceOnFailure semantics for the idempotent, non-throwing cleanup helper; do not adopt SteamManifestPatcher's direct-overwrite-without-backup approach for the executable swap.

Design Choices

Design Choice 1: Does `SteamLauncher.PrepareForProfileAsync` already exist, and is the "proxy" a physical executable swap or the existing manifest flag-flip?

Options Considered:

1. Treat the ticket as referring to a physical executable replacement performed by a (to-be-introduced/aligned) SteamLauncher.PrepareForProfileAsync, and implement validate-first ordering plus backup/restore around that mutation.
2. Treat the "proxy" as the existing SteamManifestPatcher flag-flip and add validate-first ordering plus manifest snapshot/restore around PatchManifestAsync.
3. Do nothing until the class is located, assuming the exploration missed it.

Chosen Option: 1

Rationale: The ticket explicitly names SteamLauncher.PrepareForProfileAsync and describes restoring an installed proxy executable, so the plan implements/aligns that component with a real backup/restore transaction; the same validate-first + rollback structure is applied to whichever underlying mutation (physical swap and/or manifest patch) the method drives. The exploration confirms no SteamLauncher/PrepareForProfileAsync exists and that the only current proxy mechanism is a manifest IsExecutable toggle with no backup/restore. The ticket's language ("proxy installed after failure", "restore the original executable") describes a mutating, file-level replacement requiring rollback.

💡 User Tips

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

Implementation Steps

Phase 1: Separate Non-Mutating Prerequisite Validation From the Mutating Swap

This phase reorders PrepareForProfileAsync so that every check that does not alter disk state runs and must pass before any executable replacement begins. This directly removes the window where the proxy is installed but a prerequisite later fails.

Task 1: Introduce/locate the SteamLauncher proxy-preparation entry point

Establish the component that owns the proxy-preparation transaction.

• Place the SteamLauncher component alongside existing Steam/launch logic (e.g., under GenHub/GenHub/Features/Launching/ or Features/Manifest/), defining an interface in GenHub/GenHub.Core/Interfaces/ mirroring the shape of ISteamManifestPatcher and returning an OperationResult<T>-style result rather than throwing for expected failures.
• Ensure PrepareForProfileAsync accepts the profile and a CancellationToken, and register the component in the appropriate DI module (following ManifestModule.cs/GameLaunchingModule.cs conventions).

Task 2: Extract and front-load prerequisite validation

Move all read-only checks ahead of any mutation.

• Model the validation after WorkspaceValidator.ValidatePrerequisitesAsync: verify the target profile/workspace exists, the original executable and proxy source are present and readable, target paths are writable (write-probe), platform/permission requirements are satisfied, and there is sufficient disk space for a backup copy.
• Return a non-throwing ValidationResult classifying issues as Error (blocking — abort before swapping) vs Warning (log only), and short-circuit PrepareForProfileAsync with a failure result if any blocking error is found, so no swap is attempted.
• Perform an explicit CancellationToken.ThrowIfCancellationRequested() at the end of validation, before the first mutating step.

🤖 Prompt for AI agents

Implement Phase 1: reorder `PrepareForProfileAsync` so all non-mutating
prerequisite checks run and must pass before any executable replacement begins,
removing the window where the proxy is installed but a prerequisite later fails.

- Introduce or locate the `SteamLauncher` proxy-preparation entry point: place
it alongside existing Steam/launch logic (e.g., under
`GenHub/GenHub/Features/Launching/` or `Features/Manifest/`).
- Define an interface in `GenHub/GenHub.Core/Interfaces/` mirroring the shape of
`ISteamManifestPatcher`, returning an `OperationResult<T>`-style result rather
than throwing for expected failures.
- Ensure `PrepareForProfileAsync` accepts the profile and a `CancellationToken`.
- Register the component in the appropriate DI module, following
`ManifestModule.cs`/`GameLaunchingModule.cs` conventions.
- Extract and front-load all read-only prerequisite checks ahead of any
mutation, modeling the validation after
`WorkspaceValidator.ValidatePrerequisitesAsync`: verify the target
profile/workspace exists, the original executable and proxy source are present
and readable, target paths are writable (write-probe), platform/permission
requirements are satisfied, and there is sufficient disk space for a backup
copy.
- Return a non-throwing `ValidationResult` classifying issues as `Error`
(blocking — abort before swapping) vs `Warning` (log only); short-circuit
`PrepareForProfileAsync` with a failure result if any blocking error is found so
no swap is attempted.
- Perform an explicit `CancellationToken.ThrowIfCancellationRequested()` at the
end of validation, before the first mutating step.

Phase 2: Make the Proxy Swap Transactional With Backup, Rollback, and Reporting

This phase wraps the executable replacement in a try/catch/cleanup transaction so that any failure or cancellation after the swap begins restores the original executable, leaves pre-existing artifacts untouched, and surfaces rollback outcomes.

Task 1: Capture original state before replacement

Record enough state to fully undo the swap.

• Before replacing the executable, snapshot the original (model after UserDataTrackerService.BackupExistingFileAsync: copy to a deterministic/timestamped backup path and retain the backup location in memory for the duration of the operation).
• Detect whether a proxy/backup artifact already existed from a prior run and record that fact so rollback does not delete or overwrite pre-existing artifacts that were not created by this operation ("preserve pre-existing artifacts").

Task 2: Wrap the mutating swap in a rollback-guaranteed transaction

Ensure every post-mutation exit path restores the original.

• Structure the mutating section following the Workspace-strategy convention: try { swap; further steps; token.ThrowIfCancellationRequested() at checkpoints } catch (OperationCanceledException) { restore; rethrow } catch (Exception) { restore; return failure }.
• Implement an idempotent private RestoreOriginalExecutable/CleanupOnFailure helper that moves the backup back over the proxy (mirroring UserDataTrackerService's File.Move restore), only removing artifacts this operation created, and wrapping its own body in try/catch so a secondary failure is captured rather than masking the primary failure.
• If the underlying proxy mechanism also mutates manifest state (SteamManifestPatcher.PatchManifestAsync), include reverting that flag change in the same rollback path so state is consistent.

Task 3: Report rollback outcomes in the result

Surface both the primary failure and any rollback failure to callers.

• On successful restore after a failure/cancellation, return a failure result describing the original problem.
• On a failed restore, log at error level with full context and return a distinct, clearly-worded failure result indicating the proxy may still be installed and manual intervention may be required, following the OperationResult<T>/LaunchOperationResult<T> CreateFailure conventions.

🤖 Prompt for AI agents

Implement Phase 2: wrap the executable replacement in a try/catch/cleanup
transaction so any failure or cancellation after the swap begins restores the
original executable, leaves pre-existing artifacts untouched, and surfaces
rollback outcomes.

- Before replacing the executable, snapshot the original by copying it to a
deterministic/timestamped backup path, modeling this after
`UserDataTrackerService.BackupExistingFileAsync`, and retain the backup location
in memory for the duration of the operation.
- Detect whether a proxy/backup artifact already existed from a prior run and
record that fact so rollback does not delete or overwrite pre-existing artifacts
not created by this operation.
- Structure the mutating section following the Workspace-strategy convention:
try block performs swap and further steps with
`token.ThrowIfCancellationRequested()` at checkpoints; catch
`OperationCanceledException` to restore then rethrow; catch `Exception` to
restore then return failure.
- Implement an idempotent private `RestoreOriginalExecutable`/`CleanupOnFailure`
helper that moves the backup back over the proxy (mirroring
`UserDataTrackerService`'s `File.Move` restore), only removing artifacts this
operation created, and wraps its own body in try/catch so a secondary failure is
captured rather than masking the primary failure.
- If the underlying proxy mechanism also mutates manifest state via
`SteamManifestPatcher.PatchManifestAsync`, include reverting that flag change in
the same rollback path so state remains consistent.
- On successful restore after a failure/cancellation, return a failure result
describing the original problem.
- On a failed restore, log at error level with full context and return a
distinct, clearly-worded failure result indicating the proxy may still be
installed and manual intervention may be required, following
`OperationResult<T>`/`LaunchOperationResult<T>` `CreateFailure` conventions.

Phase 3: Tests Establishing the Transactional Contract

Since no Steam proxy/launcher tests exist, add tests that lock in the validate-first and restore-on-failure guarantees, using the mocking patterns already in GameLauncherTests and StrategyTests.

Add SteamLauncher prerequisite and rollback tests

Create the first behavioral contract for this component.

• Add a new test class under GenHub/GenHub.Tests/GenHub.Tests.Core/Features/ (mirroring the Launching/Workspace test layout), mocking the file-operations/manifest-patcher dependencies via Moq.
• Cover: blocking prerequisite failure aborts before any swap (verify no backup/replace call occurs); pre-cancelled token throws before mutation; failure after swap invokes restore (verify the restore/File.Move interaction) and returns a failure result; cancellation after swap restores then rethrows (TaskCanceledException/OperationCanceledException consistent with existing launcher/strategy tests); a failing restore produces the distinct "rollback failed" result; and a pre-existing artifact is preserved (not deleted) during rollback.

🤖 Prompt for AI agents

Implement Phase 3: add the first tests establishing the transactional contract
for the Steam proxy preparation flow, using the mocking patterns already present
in `GameLauncherTests` and `StrategyTests`.

- Add a new test class under `GenHub/GenHub.Tests/GenHub.Tests.Core/Features/`,
mirroring the Launching/Workspace test layout, and mock the
file-operations/manifest-patcher dependencies via Moq.
- Cover the following cases:
- A blocking prerequisite failure aborts before any swap (verify no
backup/replace call occurs).
  - A pre-cancelled token throws before mutation.
- A failure after the swap invokes restore (verify the restore/`File.Move`
interaction) and returns a failure result.
- A cancellation after the swap restores then rethrows
(`TaskCanceledException`/`OperationCanceledException`, consistent with existing
launcher/strategy tests).
  - A failing restore produces the distinct "rollback failed" result.
  - A pre-existing artifact is preserved (not deleted) during rollback.

Research

GenHub is a .NET application whose game-launch flow spans the Workspace, Launching, and Manifest features. Steam "proxy" handling today lives in GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs (implementing ISteamManifestPatcher), which toggles IsExecutable flags between generals.exe and game.dat in cached manifest JSON — a best-effort mutation with no backup/restore. The SteamLauncher class and PrepareForProfileAsync method named in the ticket do not currently exist. The codebase already establishes strong conventions to model this work on: non-throwing prerequisite validation returning ValidationResult (WorkspaceValidator.ValidatePrerequisitesAsync), try/catch/cleanup rollback with an idempotent cleanup helper (WorkspaceStrategyBase.CleanupWorkspaceOnFailure), file backup/restore via timestamped .bak files (UserDataTrackerService.BackupExistingFileAsync), and OperationResult<T>/LaunchOperationResult<T> result objects. No existing tests cover Steam proxy or launcher behavior.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement Phase 1: reorder `PrepareForProfileAsync` so all non-mutating
prerequisite checks run and must pass before any executable replacement begins,
removing the window where the proxy is installed but a prerequisite later fails.

- Introduce or locate the `SteamLauncher` proxy-preparation entry point: place
it alongside existing Steam/launch logic (e.g., under
`GenHub/GenHub/Features/Launching/` or `Features/Manifest/`).
- Define an interface in `GenHub/GenHub.Core/Interfaces/` mirroring the shape of
`ISteamManifestPatcher`, returning an `OperationResult<T>`-style result rather
than throwing for expected failures.
- Ensure `PrepareForProfileAsync` accepts the profile and a `CancellationToken`.
- Register the component in the appropriate DI module, following
`ManifestModule.cs`/`GameLaunchingModule.cs` conventions.
- Extract and front-load all read-only prerequisite checks ahead of any
mutation, modeling the validation after
`WorkspaceValidator.ValidatePrerequisitesAsync`: verify the target
profile/workspace exists, the original executable and proxy source are present
and readable, target paths are writable (write-probe), platform/permission
requirements are satisfied, and there is sufficient disk space for a backup
copy.
- Return a non-throwing `ValidationResult` classifying issues as `Error`
(blocking — abort before swapping) vs `Warning` (log only); short-circuit
`PrepareForProfileAsync` with a failure result if any blocking error is found so
no swap is attempted.
- Perform an explicit `CancellationToken.ThrowIfCancellationRequested()` at the
end of validation, before the first mutating step.
===============================================================================

Task: 2

Implement Phase 2: wrap the executable replacement in a try/catch/cleanup
transaction so any failure or cancellation after the swap begins restores the
original executable, leaves pre-existing artifacts untouched, and surfaces
rollback outcomes.

- Before replacing the executable, snapshot the original by copying it to a
deterministic/timestamped backup path, modeling this after
`UserDataTrackerService.BackupExistingFileAsync`, and retain the backup location
in memory for the duration of the operation.
- Detect whether a proxy/backup artifact already existed from a prior run and
record that fact so rollback does not delete or overwrite pre-existing artifacts
not created by this operation.
- Structure the mutating section following the Workspace-strategy convention:
try block performs swap and further steps with
`token.ThrowIfCancellationRequested()` at checkpoints; catch
`OperationCanceledException` to restore then rethrow; catch `Exception` to
restore then return failure.
- Implement an idempotent private `RestoreOriginalExecutable`/`CleanupOnFailure`
helper that moves the backup back over the proxy (mirroring
`UserDataTrackerService`'s `File.Move` restore), only removing artifacts this
operation created, and wraps its own body in try/catch so a secondary failure is
captured rather than masking the primary failure.
- If the underlying proxy mechanism also mutates manifest state via
`SteamManifestPatcher.PatchManifestAsync`, include reverting that flag change in
the same rollback path so state remains consistent.
- On successful restore after a failure/cancellation, return a failure result
describing the original problem.
- On a failed restore, log at error level with full context and return a
distinct, clearly-worded failure result indicating the proxy may still be
installed and manual intervention may be required, following
`OperationResult<T>`/`LaunchOperationResult<T>` `CreateFailure` conventions.
===============================================================================

Task: 3

Implement Phase 3: add the first tests establishing the transactional contract
for the Steam proxy preparation flow, using the mocking patterns already present
in `GameLauncherTests` and `StrategyTests`.

- Add a new test class under `GenHub/GenHub.Tests/GenHub.Tests.Core/Features/`,
mirroring the Launching/Workspace test layout, and mock the
file-operations/manifest-patcher dependencies via Moq.
- Cover the following cases:
- A blocking prerequisite failure aborts before any swap (verify no
backup/replace call occurs).
  - A pre-cancelled token throws before mutation.
- A failure after the swap invokes restore (verify the restore/`File.Move`
interaction) and returns a failure result.
- A cancellation after the swap restores then rethrows
(`TaskCanceledException`/`OperationCanceledException`, consistent with existing
launcher/strategy tests).
  - A failing restore produces the distinct "rollback failed" result.
  - A pre-existing artifact is preserved (not deleted) during rollback.

💡 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!