fix #406: launcher session exit observation carries the real code + captures client stderr

GameWindow.Dispose() (via Program.cs's `using var window = ...`) runs
unconditionally even when invoked mid-unwind of an exception that escaped
Run()'s Silk.NET frame loop. Resource teardown itself can converge
cleanly regardless, so CompleteShutdown had no way to tell "normal Run()
return" from "a crash is propagating through me right now" and always
wrote the hardcoded exited{code:0,reason:"graceful"} — exactly the
symptom #406 observed against a real 0xE0434352 crash. Fixed by latching
_runFailure in Run()'s existing catch block (before the pre-existing
throw) and consulting it from a new ReportExited method, the one call
site for the terminal status write: crashed(1)/graceful(0)/
shutdown-incomplete(1) as appropriate. No wire-contract amendment needed
— §LA1 pins the exited event NAME, and reason is already free text that
StatusEventParser round-trips unchanged.

Sibling gap fixed in the same commit: the launcher discarded the child's
stdout/stderr entirely, which is why diagnosing this exact crash required
a manual console re-run. Added BoundedProcessOutputCapture, a 2 MiB-capped
sink mirroring SessionStatusWriter's open-append-flush-close-per-write
posture (a long-lived write handle is not actually concurrently readable
on Windows even with FileShare.Read — confirmed by isolated repro), wired
into both SystemChildProcess (ProcessStartInfo.RedirectStandardError;
Linux + Windows graphical children, i.e. this bug's own scenario) and
WindowsSystemChildProcess (a real native pipe via CreateChildOutputPipe,
mirroring the existing stdin pipe; Windows console-capable/Headless
children). Opt-in via LauncherProcessSpec.StderrLogPath (null = unchanged
behavior), threaded through SessionConfigComposer -> client.err.log
beside status.jsonl -> LauncherExecutableSet -> LauncherOrchestrator.

Tests: GameWindowCrashStatusTests (source-shape, matching the existing
GameWindow test pattern — the class cannot be constructed without a live
GPU/window), BoundedProcessOutputCaptureTests (10 unit tests), and three
new LauncherProcessSupervisorTests spawning real child processes through
both capture code paths.

Launcher.Core.Tests: 337/0 (was 324/0). Launcher.Tests: 67/0 (unchanged).
Full solution build green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-16 11:44:46 +02:00
parent 1d9de5e095
commit 691b925952
13 changed files with 1150 additions and 38 deletions

View file

@ -103,26 +103,62 @@ amendment. Immediate workaround (confirmed live): drag-resize the
windowed client — resize events rebuild the swapchain (#387) and the
retail UI rescales from its 800x600 authored canvas.
## #406 — Launcher records a crashed client as `exited{code:0,reason:"graceful"}`
## #406 CLOSED: Launcher records a crashed client as `exited{code:0,reason:"graceful"}`
**Status:** OPEN (Campaign CC gate round 1, 2026-08-16)
**Status:** DONE (this commit, 2026-08-16)
**Severity:** MEDIUM (diagnosis-misleading, not data-loss)
Found while diagnosing #405: the client process died with exit code
`0xE0434352` (.NET unhandled exception, stack on stderr), but the
launcher's session status stream recorded `{"e":"exited","code":0,
"reason":"graceful"}` — the exact opposite of what happened. Running the
identical binary + session config from a console shows the true nonzero
exit code, so the corruption is in the launcher's session-orchestrator
exit observation (wrong process handle/exit-code read, or a default that
masks the real code), not in the client. §LA1 explicitly promises
`exited{code,reason}` carries the real termination; a launcher that
reports "graceful" for a crash sends any future gate/automation
diagnosis in the wrong direction (it did exactly that this round until
the console repro). Investigate the launcher-side session orchestrator's
exit capture; a test should pin a nonzero-exit child producing
`exited{code:<nonzero>,reason:"crashed"|"failed"}` per the LA contract's
vocabulary.
"reason":"graceful"}` — the exact opposite of what happened.
Root cause was NOT in the launcher's process supervision (its own
OS-level exit-code read was always correct) — it was in the CLIENT's own
self-report. `GameWindow.Dispose()` (`src/AcDream.App/Rendering/GameWindow.cs`)
runs unconditionally via `Program.cs`'s `using var window = new
GameWindow(...)` even when invoked mid-unwind of an exception that
escaped `Run()`'s Silk.NET frame loop — the resource-shutdown transaction
itself can converge cleanly (nothing it tears down touches the crash),
so `CompleteShutdown` had no way to tell "normal `Run()` return" from "an
exception is propagating through me right now" and always wrote the
hardcoded `exited{code:0,reason:"graceful"}`. Fixed by latching
`_runFailure` in `Run()`'s existing `catch (Exception failure)` block
(right before the `throw;` that already existed for the
`_constructionCleanup.RetainFrom(failure)` ledger) and consulting it from
a new `ReportExited` method that is now the ONE call site for the
terminal status write: `exited{code:1,reason:"crashed"}` when a crash was
observed, `exited{code:0,reason:"graceful"}` on a real graceful
Dispose(), `exited{code:1,reason:"shutdown-incomplete"}` unchanged for a
non-crash teardown failure. `"crashed"` is a new value for the already-
free-text `reason` field (§LA1's `exited{code,reason}` vocabulary pins
the EVENT name, not an enum of `reason` strings — `StatusEventParser`
already round-trips any string there) so no wire-contract amendment was
needed. Pinned as a source-shape test (`GameWindowCrashStatusTests`) since
`GameWindow` cannot be constructed without a live GPU/window.
Sibling gap fixed in the same commit: the launcher previously discarded
the child's stdout/stderr entirely, which is why diagnosing this exact
crash required a manual console re-run. Added
`BoundedProcessOutputCapture` (`src/AcDream.Launcher.Core/Launching/`) —
a 2 MiB-capped, additive-only sink mirroring `SessionStatusWriter`'s
open-append-flush-close-per-write posture (a long-lived write handle is
NOT actually concurrently readable on Windows even with
`FileShare.Read` — confirmed by isolated repro) — wired into BOTH
`SystemChildProcess` (`ProcessStartInfo.RedirectStandardError` +
`ErrorDataReceived`; used on Linux for every child and on Windows for
graphical/non-console children, i.e. exactly this bug's own App/GUI
scenario) and `WindowsSystemChildProcess` (a real native pipe via a new
`CreateChildOutputPipe`, mirroring the existing stdin pipe in the
opposite direction, drained on a background pump thread; used on Windows
for console-capable children, i.e. Headless). The capture path is opt-in
via a new `LauncherProcessSpec.StderrLogPath` (null = behave exactly as
before) threaded through `SessionConfigComposer``client.err.log`
beside `status.jsonl` in the per-session directory →
`LauncherExecutableSet.CreatePlaySpec`/`CreateProbeSpec`
`LauncherOrchestrator`. Real end-to-end tests
(`LauncherProcessSupervisorTests`) spawn an actual child via both code
paths and assert the captured file.
## #405 — CLOSED: chargen/summary preview leases missing Transfer killed every retail-UI window load