feat(magos-modificus): implement Phase 1 Integrations (GitHub Releases client) - #19
Merged
Merged
Conversation
Adds the Integrations.GitHub section to MagosConfig for the Phase 1 GitHub Releases client: BaseUrl (default https://api.github.com) + optional Token (PAT). Every field carries a default so an absent section yields a usable (anonymous) client. Consumed by AddIntegrations() in the Integrations library.
Replaces the stub Integrations library with the real IGitHubClient: - IGitHubClient: ListReleases / GetLatestRelease (sync wrappers) + DownloadAssetAsync (streamed, with progress). - GitHubClient over the GitHub REST API via an IHttpClientFactory- provided HttpClient (AddHttpClient<IGitHubClient, GitHubClient>). - Rate-limit detection (X-RateLimit-Remaining: 0 + 403/429) throws GitHubRateLimitException carrying the reset time; other non-2xx throw GitHubApiException(status, message). 404 -> null/empty for the release lookups. - AddIntegrations() configures BaseAddress + User-Agent + Accept + optional Bearer auth from MagosConfig.Integrations.GitHub. System.Text.Json (in-box) for deserialization; Microsoft.Extensions.Http 10.0.9 (latest stable, .NET 10 LTS) is the only new dep. Also swaps the Phase-0 IModSourceService probe in App.axaml.cs for IGitHubClient (forced by removing the stub; one-line mechanical fix).
Adds the Magos.Modificus.Integrations.Tests xUnit project and registers it in the solution. All tests run against a stub HttpMessageHandler (no real network calls): - ListReleases/GetLatestRelease parsing, 404 -> empty/null, 500/403 -> GitHubApiException, non-JSON error fallback, missing-field robustness. - Rate-limit (X-RateLimit-Remaining: 0) -> GitHubRateLimitException (which is itself a GitHubApiException). - DownloadAssetAsync: writes bytes, reports progress, creates the destination dir, honors cancellation, 404 -> GitHubApiException. - AddIntegrations(): resolves IGitHubClient, exposes IHttpClientFactory, and wires BaseAddress + headers + auth from MagosConfig (verified end-to-end via a stub handler on the outgoing request). 98% line coverage on the Integrations library; existing tests unaffected (114 total, 0 failures).
Folds in code-review fixes for the GitHub client: - User-Agent: the constructor's TryParseAdd always appends, so production (where AddIntegrations already sets the UA) sent a duplicated 'Magos-Modificus Magos-Modificus'. Now only adds a UA when none is set (guard on UserAgent.Count == 0); comment corrected. - DownloadAssetAsync: a failure mid-copy (network drop / cancellation) previously left a partial destination file. Wrap the stream copy in try/catch and best-effort delete the partial file before rethrowing (TryDelete swallows IOException/UnauthorizedAccessException so the original exception propagates). New test with a stream that throws mid-read asserts no partial file remains. - GitHubRateLimitException now carries the actual response status (was hardcoded 403); a 429-driven limit surfaces as 429, not 403. New 429 test covers the path (existing tests cover 403). - IsRateLimited uses HttpStatusCode.TooManyRequests instead of the magic (HttpStatusCode)429 cast.
ListReleases silently caps at GitHub's default page size (~30). Non-issue for the Phase-4 DMF prompt (GetLatestRelease via /releases/latest is the right tool and doesn't paginate), but a caller doing a version-history scan would get a truncated list. Doc-only; pagination deferred to a later phase.
ModifAmorphic
added a commit
that referenced
this pull request
Jul 2, 2026
…de (#20) ## 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); `DiscoveryIncomplete`↔`Status` 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`/`trace` → `Warning` 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
## 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.
This was referenced Jul 8, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 Integrations library for Magos Modificus — the GitHub Releases client (
IGitHubClient:ListReleases/GetLatestRelease/DownloadAssetAsync) viaIHttpClientFactory, for mod-source download + version checks. Consumed by Phase 4 (DMF new-profile prompt + mod-install orchestration); not on the Phase 1 launch critical path.Spec:
_local/phase1-integrations-spec.md(approved).What's in it
IGitHubClient+ types (GitHubRepo,GitHubRelease,GitHubReleaseAsset) — exactly per spec.GitHubApiException(base,StatusCode) +GitHubRateLimitException : GitHubApiException(carriesResetAtfromX-RateLimit-Reset); callers catch uniformly or specialize.MagosConfig.Integrations.GitHubadded (BaseUrldefaulthttps://api.github.com, optionalTokenfor a PAT) — defaulted, first-run safe.AddIntegrations()wiresAddHttpClient<IGitHubClient, GitHubClient>with the BaseUrl (trailing-slash normalized),User-Agent,Accept, and optionalAuthorization: Bearerfrom config.ResponseHeadersRead+ReadAsStreamAsync, 80KB buffer) — assets aren't buffered into memory; progress reports cumulative bytes; cancellation honored.HttpMessageHandler— no real network calls): happy parsing, missing optional fields, 404→null/empty, 500/non-JSON errors, precise rate-limit detection (403/429 +X-RateLimit-Remaining: 0), 403-vs-rate-limit disambiguation, download bytes/progress/cancellation/partial-cleanup, DI + config wiring.Verification trail
Microsoft.Extensions.Http10.0.9, latest stable); all test deps match existing projects.ui/App.axaml.cstouch — minimal/mechanical, the integrations stubIModSourceServicefully removed); no real network calls; rate-limit precise detection verified;Microsoft.Extensions.Httplatest stable.User-Agentin production (real bug —TryParseAddalways appends); (2) partial-file cleanup on failed/cancelled downloads (the client owns the file it created) + a regression test; (3) documentedListReleases's 30-release page-size cap; (4)GitHubRateLimitExceptionnow carries the actual status (was hardcoded 403 even on 429) + a 429 test; (5)HttpStatusCode.TooManyRequestsnamed enum.The
ui/App.axaml.cstouch (deviation, lead-accepted)The Phase-0 UI startup probe referenced the integrations stub
IModSourceService; replacing the stub withIGitHubClient(different name) forced a one-line fix in the probe. Minimal + mechanical; the stub is fully removed (no dead interface). This cascade is unique to Integrations — Profiles/Steam kept their interface names (no UI touch), and Enginseer-client will too.Tracked follow-ups (not blockers; Phase 4+)
ListReleasespagination (per_page=100+ page-walking) — file if a Phase-4 caller needs >100 releases.ListReleasesAsyncetc.) — add when Phase 4 orchestration arrives (currently sync-over-async via.GetAwaiter().GetResult()+.ConfigureAwait(false); safe in Avalonia, but the orchestrator should be async-native).MapAssetnull-handling on a missingBrowserDownloadUrl(minor robustness).Prerelease/Draftfields onGitHubRelease— Phase 4 version-history-filter concern.Note for Phase 4
The DMF-acquisition strategy is a separate, open Phase-4 decision (DMF's GitHub repo has zero releases/tags — the original "fetch DMF from GitHub Releases" plan doesn't apply to DMF specifically; this client is correct for mods that do publish releases). The client's job stops at "give me releases + download bytes to a path"; asset selection, extraction, placement, and
IProfileService.AddModlive in the Phase-4 orchestrator.