Skip to content

feat(magos-modificus): implement Phase 1 Enginseer-client launch façade - #20

Merged
ModifAmorphic merged 4 commits into
mainfrom
magos-modificus/enginseer-client
Jul 2, 2026
Merged

feat(magos-modificus): implement Phase 1 Enginseer-client launch façade#20
ModifAmorphic merged 4 commits into
mainfrom
magos-modificus/enginseer-client

Conversation

@ModifAmorphic

Copy link
Copy Markdown
Owner

What

Phase 1 Enginseer-client — the launch façade (the Phase 1 capstone). IEnginseerLaunchService.Launch(profileId) resolves the profile + discovery internally, then invokes magos_launcher.exe: Windows directly, Linux via proton run with both STEAM_COMPAT_* env vars + Z:\-translated paths. Returns LaunchResult (Launched / DiscoveryIncomplete + missing fields / Error). Consumes Profiles (PrepareModRoot--mod-path) + Steam (DiscoveryResult → env vars + proton + game-binary).

The launch smoke test is a USER-machine validation (no Darktide/Windows/Proton in CI). The PR provides a CLI smoke harness (dotnet run -- discover/list/launch) for the user to run the real launch on their Win + Linux boxes — the underlying path was already live-validated manually.

Spec: _local/phase1-enginseer-client-spec.md (approved).

What's in it

  • IEnginseerLaunchService.Launch(Guid profileId) → LaunchResult — resolves profile (IProfileService.PrepareModRoot) + discovery (ISteamService.Discover) internally; never throws.
  • Windows path: Process.Start(launcher, args) directly — no Proton, no translation. Args: --game-binary, --mod-path (from PrepareModRoot), --log-file.
  • Linux path: sets STEAM_COMPAT_DATA_PATH + STEAM_COMPAT_CLIENT_INSTALL_PATH from DiscoveryResult; Z:\-translates --game-binary + --mod-path + --log-file; invokes <ProtonBinaryPath> run <launcher.exe> <args>.
  • DiscoveryIncomplete: derived from the null required fields (≡ Steam's Status != Complete by construction — verified equivalent per-platform) + returned with the missing field names (nameof-based) for the Phase 3 UI's escape-hatch prompt.
  • IProcessLauncher seam — mockable; real ProcessLauncher uses ProcessStartInfo.ArgumentList (argv-correct, no shell — paths-with-spaces safe, no injection surface).
  • CLI smoke harness — dual-purpose test project (dotnet test = xUnit; dotnet run -- discover/list/launch = real-composition harness); README + instructions embedded.
  • 28 tests (Windows/Linux arg assembly, Z:\ translation, both env vars, proton run invocation, DiscoveryIncomplete per-platform, profile integration, error cases, DI).

Verification trail

  • coder — implemented to spec (resumed cleanly after a session-stall abort); 144 tests pass (28 EnginseerClient + 116 existing); no new NuGet deps; all pins latest-stable. Caught + the lead folded two real fixes (below).
  • qa → PASS — all 10 acceptance criteria met; all 5 deviations sound; the smoke harness executes (dotnet run -- discover → real composition → DiscoveryIncomplete/exit 2 in this env, correct); DiscoveryIncompleteStatus equivalence verified; no new deps.
  • code-review → REQUEST CHANGES on one should-fix (now fixed): the --log-level vocabulary mismatch — Magos forwarded its Serilog level name to the shell, but the shell only recognizes error/warn/info/debug/traceWarning silently became info (more noise), etc. Fixed by omitting --log-level entirely (rely on the shell's info default; decouple the two logs — they serve different purposes; consistent with the --steam-app-id deviation). The reviewer verified everything else sound (the Status equivalence, Z:\, proton run, both env vars, the IProcessLauncher seam, the smoke-harness design). Fixing this was the condition to flip to APPROVE.
  • CI — gates Win + Linux on this PR; Linux build/test green locally (144/144).

Tracked follow-ups (not blockers)

  • #2 dead DarktideSteamAppId constant — documented placeholder for a future --steam-app-id config-override field.
  • #3 generic Error message on process-start failure — the specific exception lands in the log, not LaunchResult.Message. Thread it out when Phase 3 UI consumes Error + wants to show why.
  • --steam-app-id + a dedicated shell-log-level config field — add if/when a knob is needed (Phase 1 relies on the launcher's defaults).

After this merges

Phase 1 (core domain libraries) is complete — Profiles + Steam + Integrations + Enginseer-client all in main. → Phase 2: shared-mod storage (the version-policy allocation model + staging). The IEnginseerLaunchService interface is designed so Phase 3 (UI) consumes Launch + handles DiscoveryIncomplete (escape-hatch prompt) cleanly, with a future Launch(profileId, DiscoveryResult) overload for cached discovery.

Builds on merged #16 (scaffold) + #17 (Profiles) + #18 (Steam) + #19 (Integrations).

ModifAmorphic added a commit that referenced this pull request Jul 2, 2026
…refix (#21)

## What
Small follow-up fix to the Phase 1 Steam discovery library (#18): the
compatdata (Proton prefix) resolution now searches **all Steam
libraries**, not just the main install.

## Why
A launch-smoke-test on a real Linux machine surfaced this: the user's
Darktide compatdata lives at `/games/steamapps/compatdata/1361210/` —
under a **Steam library drive**, not the main Steam install. Steam
places the prefix on whichever drive it chose at install time, which is
frequently a library. The previous logic checked only
`<SteamInstallPath>/steamapps/compatdata/<appid>/`, so it reported
`CompatdataPath: (missing)` → `DiscoveryIncomplete`, blocking the
launch.

## The fix
`SteamService.FindCompatdata` now probes candidates across **the main
install + each library** (main install first for prior-behavior
preservation, then libraries in `libraryfolders.vdf` order, deduped),
first existing wins. The discovery already parsed `libraryfolders.vdf`
for the Darktide search — reuses that list.
`DiscoveryResult.CompatdataPath` stays a single string — **no interface
change**. Windows discovery + Proton/Darktide resolution untouched.

## Test
New `Compatdata_in_secondary_library_is_found` — fixtures a secondary
library with the compatdata (not under the main install) → asserts
`Status: Complete` + the correct library path. Existing
main-install-compatdata test still passes (main install probed first).

## Verification
- **coder** — impl + test; `dotnet build` 0/0; `dotnet test` green
(Steam 39 = 38 + 1; full main suite 117). No new deps.
- **Light review path** — this is a one-method targeted fix with a clear
test; skipping the formal qa/code-review cycle. CI gates Win + Linux on
this PR; the diff is small for direct review.
- After merge: re-run `dotnet run --project
tests/Magos.Modificus.EnginseerClient.Tests -- discover` → `Status:
Complete` (compatdata found under the library).

Builds on merged #18 (Steam). Independent of #20 (Enginseer-client,
still open).
Replace the Phase-0 stub with the v1 launch façade over the Enginseer
runtime. IEnginseerLaunchService.Launch(profileId) resolves the profile
(via IProfileService.PrepareModRoot) + Steam discovery (via
ISteamService.Discover) internally, assembles the launcher args, and
invokes magos_launcher.exe — directly on Windows, under 'proton run' on
Linux (with the Steam compat env vars + Z:\-translated paths).

Outcome is reported via LaunchResult: Launched (fire-and-forget),
DiscoveryIncomplete (carrying the missing field names for the UI's
escape-hatch prompt — Enginseer-client does NOT prompt itself), or Error
(unknown profile / missing runtime dir / process-start failure).

Process invocation is abstracted behind IProcessLauncher (mirrors the
Steam library's IProcessLookup seam) so the launch path is unit-testable
without spawning a real process. The platform branches on a runtime
detection (forced via an internal constructor in tests).

Consumes Profiles (the mod-path) + Steam (DiscoveryResult) + MagosConfig
(EnginseerRuntimeDir + Logging); AddEnginseerClient() registers
IEnginseerLaunchService + the IProcessLauncher seam (TryAdd).
Unit-test the launch path via mocked IProcessLauncher + fake
IProfileService/ISteamService (no real process, no game): Windows arg
assembly, Linux Z:\ translation + both Steam compat env vars + proton
run invocation, DiscoveryIncomplete with the missing field names, profile
integration (PrepareModRoot called), and the Error cases (unknown
profile, missing runtime dir, process-start failure). WinePath is covered
exhaustively; DI tests prove AddEnginseerClient() registers
IEnginseerLaunchService + the IProcessLauncher seam (with TryAdd override
semantics).

The test project is dual-purpose: 'dotnet test' runs the xUnit suite,
and 'dotnet run -- {discover,list,launch <id>}' drives the launch
smoke-test harness — a CLI hook over IEnginseerLaunchService.Launch that
builds the REAL Magos composition so the user can validate an actual
modded Darktide launch on their Win/Linux box (the agent env has no
game/Proton). The harness is the user-facing smoke-test delivery per the
spec's Acceptance.
BuildLauncherArgs translated only --game-binary + --mod-path on Linux (per
the spec's literal Z:\ enumeration). But magos_launcher.exe runs under
Wine and opens --log-file itself, so a POSIX path there can't be opened
under Wine — magos_enginseer.log wouldn't be written where Magos expects
on Linux. Translate --log-file with the same flag (one line); --log-level
is a level name, not a path, and stays as-is. Spec oversight confirmed by
the lead (the lead wrote the spec).

Tests: add Linux_translates_log_file_to_wine_path, and extend the
Windows guard to assert --log-file passes through untranslated (no Z:\).
…ocabulary mismatch)

BuildLauncherArgs was forwarding MagosConfig.Logging.Level to the launcher's
--log-level, but that value is a Serilog LogEventLevel name
(Verbose/Debug/Information/Warning/Error/Fatal). The Enginseer shell's
resolve_log_level recognizes only error/warn/info/debug/trace
(case-insensitive) and falls back to info for unknown names — so 4/6
Serilog levels silently mis-resolved: Warning -> info (more noise than
intended), Verbose -> info (wanted trace), Fatal -> info (wanted error-
only). The bug was invisible to tests since they only asserted the value
was forwarded verbatim, never that the shell understood it.

The Magos Serilog log and the Enginseer shell log serve different purposes;
coupling their levels was the mistake. Drop the --log-level flag entirely
(and the now-unused logLevel parameter through Launch/LaunchWindows/
LaunchLinux) and rely on the launcher's info default — consistent with
the --steam-app-id omission (both rely on the launcher's own default).
A dedicated shell-level config field can be added if a future need arises.

Tests: drop the --log-level assertions from the Windows/Linux arg-assembly
tests and assert --log-level is NOT emitted (both platforms).
@ModifAmorphic
ModifAmorphic force-pushed the magos-modificus/enginseer-client branch from 45579b1 to 9a19636 Compare July 2, 2026 03:59
@ModifAmorphic
ModifAmorphic merged commit 7950ed1 into main Jul 2, 2026
2 checks passed
@ModifAmorphic
ModifAmorphic deleted the magos-modificus/enginseer-client branch July 2, 2026 04:19
ModifAmorphic added a commit that referenced this pull request Jul 2, 2026
## Problem

`ISteamService.IsGameRunning()` false-negatived under Linux/Proton. It
delegated to `Process.GetProcessesByName("Darktide")`, which on Unix
reads `/proc/<pid>/comm` (the kernel process name, 15-char cap). Under
Proton, Darktide's `comm` is literally **`main`** — so the call returned
`0` while the game was actually running. This would break the (Phase 3)
"block profile switching while the game runs" guard and permit a
double-launch.

## Root cause (validated empirically against a live Proton Darktide)

- `GetProcessesByName("Darktide")` → `0`; `GetProcessesByName("main")` →
`1` (the actual game).
- The game's **`argv[0]`** (`/proc/<pid>/cmdline`, first NUL token) is
`S:\common\Warhammer 40,000 DARKTIDE\binaries\Darktide.exe` — stable,
set at exec — whose basename-stem `Darktide` matches the existing
`GameProcessName` option.
- Ruled out: `pfx.lock` flock (Proton holds it only during prefix
*setup*, not the session — confirmed no locks on the compatdata device
while running); whole-cmdline substring match (too permissive — matches
the wine `steam.exe` wrapper and the detector process itself).

## Fix

Split the single `ProcessLookup` into two `IProcessLookup`
implementations selected **once** at DI registration (no per-call OS
branching inside the lookups):
- `WinProcessLookup` — wraps `Process.GetProcessesByName` (unchanged
Windows behavior).
- `LinuxProcessLookup` — scans `/proc/<pid>/cmdline`, takes `argv[0]`,
stem-matches to `GameProcessName`. Never throws; per-entry read failures
are skipped (degrades to not-running).

`AddSteam()` picks the impl via
`RuntimeInformation.IsOSPlatform(OSPlatform.Linux)` at registration.
`IProcessLookup` signature, `GameProcessName` value (`"Darktide"`), and
`SteamService` are unchanged; the test fixture's `FakeProcessLookup`
still wins over `TryAddSingleton`.

### Notable detail
`MatchesArgv0` normalizes `\`→`/` before
`Path.GetFileNameWithoutExtension`. This is load-bearing: on a Linux
runtime, `Path.GetFileNameWithoutExtension` does **not** split
backslashes, so without it the live `S:\...\Darktide.exe` would still
false-negative. (Caught empirically — first test run failed on exactly
this case.)

## Validation
- **Live-verified** against a running Proton Darktide on the operator's
box: real `SteamService.IsGameRunning()` via the SmokeHarness returns
`Darktide running? True` (was `false`). Discovery `Complete`.
- **QA**: PASS — all acceptance criteria, evidence-based; 193 tests
green.
- **Code-review**: APPROVE — every critical invariant verified
(backslash normalization, argv[0]-only matching, DI selection once,
never-throws).
- `dotnet build` 0 warnings / 0 errors; `dotnet test` 193/193 (Steam 47
incl. 8 new `ArgvMatchTests`, Profiles 59, Integrations 29,
EnginseerClient 28, SharedMods 22, General 8).

## Also in this PR
`docs(agents)`: `AGENTS.md` labeled `integrations/`, `steam/`, and
`enginseer-client/` as stubs though all three were implemented in Phase
1 (#18/#19/#20). Corrected the directory map + main-branch summary.
Bundled here (not a separate doc PR) since we're working in the `steam/`
component.

## Follow-up (separate, not in this PR)
An audit of OS-branching across `magos-modificus` found two spots that
don't follow the resolve-once/interface+DI discipline established here:
`SteamService.Discover()` (if/else enum dispatch →
`DiscoverLinux`/`DiscoverWindows`) and `SteamRegistryReader` (runtime
`OperatingSystem.IsWindows()` guard). A small consistency refactor for
those is a candidate for a later PR.
ModifAmorphic added a commit that referenced this pull request Jul 8, 2026
🤖 I have created a release *beep* *boop*
---


## 0.1.0 (2026-07-08)


### Features

* **component-a:** Hybrid Rust+C discovery + shell + launcher
([#1](#1))
([491e5d1](491e5d1))
* **magos-modificus:** implement Phase 1 Enginseer-client launch façade
([#20](#20))
([7950ed1](7950ed1))
* **magos-modificus:** implement Phase 1 Integrations (GitHub Releases
client)
([#19](#19))
([781d65c](781d65c))
* **magos-modificus:** implement Phase 1 Profiles library
([#17](#17))
([f355ceb](f355ceb))
* **magos-modificus:** implement Phase 1 Steam discovery library
([#18](#18))
([8f6ec00](8f6ec00))
* **magos-modificus:** implement Phase 2 shared-first mod storage
([#22](#22))
([cf2af80](cf2af80))
* **magos-modificus:** Phase 3 Track B mod-list, import, source model
([#29](#29))
([5075cee](5075cee))
* **magos-modificus:** Phase 3 Track C launch + Settings + escape-hatch
+ base-folder mod loading
([#32](#32))
([c595700](c595700))
* **magos-modificus:** Phase 4 Stage 1 nxm scheme handler + IPC
([#34](#34))
([0017529](0017529))
* **magos-modificus:** Phase 4 Stage 2 Nexus auth + Integrations dialog
([#35](#35))
([0790a8e](0790a8e))
* **magos-modificus:** Phase 4 Stage 3 Nexus mod acquisition
([#36](#36))
([d01105f](d01105f))
* **magos-modificus:** Phase 4 Stage 4 Nexus update-check service
([#39](#39))
([1749e76](1749e76))
* **magos-modificus:** Phase 4 Stage 5 mod-list update badges + per-mod
update
([#43](#43))
([423e146](423e146))
* **magos-modificus:** Phase 4 Stage 6 DMF new-profile/auth prompt
([#44](#44))
([994b4f8](994b4f8))
* **magos-modificus:** scaffold .NET 10 + Avalonia 12 app + libraries
([#16](#16))
([d1fac91](d1fac91))
* **mod-loader:** own the load-order contract (mods.lst), drop DMF
prepend
([#14](#14))
([1ccb891](1ccb891))
* **release:** add Curator release pipeline
([#49](#49))
([01517e4](01517e4))
* **runtime:** engine-context proven — trampoline, Enginseer v1,
launcher fail-fast
([#4](#4))
([4565ba8](4565ba8))
* **runtime:** Enginseer v2 — mod loader + launcher config + logging
([#5](#5))
([1e65b3f](1e65b3f))
* **runtime:** package Enginseer with the runtime; relocate build files
to runtime/
([#6](#6))
([7221bdb](7221bdb))
* **ui:** Phase 3 Track A — app shell + profile management
([#27](#27))
([f7f8250](f7f8250))
* **ui:** Phase 3 Track D — Preferences + i18n + custom title bars +
icon
([#28](#28))
([c921650](c921650))


### Bug Fixes

* **enginseer:** DMF integration fixes — IO re-root, load timing,
destroy
([#7](#7))
([401759c](401759c))
* **magos-modificus:** multi-format archive import (zip + 7z + rar)
([#41](#41))
([ee4f5c6](ee4f5c6))
* **magos-modificus:** search all Steam libraries for the compatdata
prefix
([#21](#21))
([895fa2b](895fa2b))
* **steam:** detect running Darktide via /proc argv[0] under Proton
([#23](#23))
([c5f38c4](c5f38c4))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: ModifAmorphic <86930443+ModifAmorphic@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant