fix(launcher): close Campaign LA LA1 review findings

This commit is contained in:
Erik 2026-08-14 17:00:09 +02:00
parent 4edc122085
commit d511e4c348
12 changed files with 413 additions and 65 deletions

View file

@ -171,6 +171,16 @@ sides. Unknown `e` values must parse to a typed Unknown event, never
throw; a known `e` with a wrong payload shape should be distinguishable
from an unknown `e` (LA3 review finding 12).
**Known LA1 status limitation:** the stream has no independent mid-play
wire-drop detector. If a transport becomes silent without raising through the
host's tick/teardown path, no immediate `disconnected` line can be promised;
the launcher must not treat the absence of that line as proof that the socket
is healthy. Explicit reconnect is ordered and observable — it emits
`disconnected{reason:"reconnect"}` before the replacement connection's second
`connected` — and normal stop/process teardown closes any still-open
connection before `exited`. A future transport-health signal may improve the
timing without changing this pinned event vocabulary.
Three pieces, one slice, because they share the session-config/status seam:
1. **App `--session-config <path>`:** parsed once in `Program.cs` into

View file

@ -29,11 +29,12 @@ internal sealed class AppCredentialResolver
private readonly bool _isLinux;
/// <summary>
/// <paramref name="isLinux"/> is caller-supplied, never detected in this
/// file — <c>LinuxPlatformBoundaryTests</c>'s platform-owner guard
/// requires every OS-family check to live under <c>Platform/</c>;
/// callers pass <c>GraphicalHostPlatformServices</c>'s already-detected
/// value instead of this file re-detecting it itself.
/// <paramref name="isLinux"/> is the caller-supplied platform-policy
/// value from <c>GraphicalHostPlatformServices</c>. This file still uses
/// <c>RuntimePlatformGuard.IsLinuxRuntime</c> below as the narrow
/// CA1416-recognized runtime guard required before calling
/// <c>File.GetUnixFileMode</c>; it does not independently select the host
/// platform or bypass the platform-services owner.
/// </summary>
internal AppCredentialResolver(
TextReader standardInput,

View file

@ -1665,7 +1665,7 @@ public sealed class GameWindow :
// OnClosing() native-window-close-request pass) represents the
// process actually being done.
if (releaseNativeWindow)
_statusWriter.Exited(_options.SessionId ?? "app", 0, "disposed");
_statusWriter.Exited(_options.SessionId ?? "app", 0, "graceful");
return;
}

View file

@ -2,6 +2,8 @@ using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Reflection;
using System.Text;
using AcDream.App.Configuration;
using AcDream.App.Rendering.Residency;
using AcDream.App.Streaming;
@ -266,6 +268,44 @@ public sealed record RuntimeOptions(
selector.Id,
selector.Name);
private static readonly PropertyInfo[] PrintableProperties =
typeof(RuntimeOptions)
.GetProperties(
BindingFlags.Instance
| BindingFlags.Public
| BindingFlags.DeclaredOnly)
.Where(static property =>
property.GetMethod is not null
&& property.GetIndexParameters().Length == 0)
.OrderBy(static property => property.MetadataToken)
.ToArray();
/// <summary>
/// Campaign LA LA1 defense in depth: positional records normally print
/// every public property, including the live password. Preserve that
/// ordinary diagnostic property set while substituting the one sensitive
/// value before it can reach a log, debugger display, or exception.
/// Reflection is cached once and runs only on the diagnostic
/// <see cref="object.ToString"/> path.
/// </summary>
private bool PrintMembers(StringBuilder builder)
{
ArgumentNullException.ThrowIfNull(builder);
for (int index = 0; index < PrintableProperties.Length; index++)
{
PropertyInfo property = PrintableProperties[index];
if (index != 0)
builder.Append(", ");
builder.Append(property.Name);
builder.Append(" = ");
builder.Append(
property.Name == nameof(LivePass) && LivePass is not null
? "<redacted>"
: property.GetValue(this));
}
return PrintableProperties.Length != 0;
}
/// <summary>True iff live-mode credentials are present and valid for connecting.</summary>
public bool HasLiveCredentials =>
LiveMode && !string.IsNullOrEmpty(LiveUser) && !string.IsNullOrEmpty(LivePass);

View file

@ -491,8 +491,9 @@ internal sealed class HeadlessSessionHost : IDisposable
_policy.Tick(Runtime, Commands);
}
internal RuntimeTeardownAcknowledgement Stop()
internal RuntimeTeardownAcknowledgement Stop(string reason = "stopped")
{
ArgumentException.ThrowIfNullOrWhiteSpace(reason);
RuntimeTeardownAcknowledgement result =
Commands.Session.Stop(Runtime.Generation);
// R9 review fix (2026-08-03): _currentSession is cached across
@ -510,7 +511,7 @@ internal sealed class HeadlessSessionHost : IDisposable
if (_hasConnected)
{
_hasConnected = false;
_statusWriter.Disconnected(_descriptor.Id, "stopped");
_statusWriter.Disconnected(_descriptor.Id, reason);
}
return result;
}
@ -640,7 +641,7 @@ internal sealed class HeadlessSessionHost : IDisposable
_statusWriter.Exited(
_descriptor.Id,
_faulted ? 1 : 0,
_faulted ? "fault" : "disposed");
_faulted ? "runtime-fault" : "graceful");
_disposeStage++;
_disposed = true;
break;
@ -670,8 +671,12 @@ internal sealed class HeadlessSessionHost : IDisposable
if (reconnect)
{
RuntimeTeardownAcknowledgement stopped =
_liveSession.Stop(expectedGeneration);
// Campaign LA LA1 review fix F3: route reconnect teardown
// through the same status-aware Stop boundary as every other
// host stop. The retiring connection therefore publishes a
// truthful disconnected(reason: "reconnect") edge before the
// fresh LiveSessionHost reports its second connected edge.
RuntimeTeardownAcknowledgement stopped = Stop("reconnect");
if (!stopped.IsComplete)
{
return new RuntimeSessionStartResult(

View file

@ -35,6 +35,18 @@ namespace AcDream.Runtime.Session;
/// </para>
///
/// <para>
/// The writer also owns the small amount of stream-ordering state needed to
/// keep the external contract coherent across host implementations. A second
/// <c>connected</c> edge while the prior connection is still open first emits
/// <c>disconnected(reason: "reconnect")</c>; a terminal <c>exited</c> edge
/// closes any still-open connection with
/// <c>disconnected(reason: "process-exit")</c>. <c>exited</c> is terminal and
/// idempotent: the first call wins and every later event is ignored. This is
/// deliberately enforced here because both graphical and no-window hosts use
/// this exact sink, while their reconnect command adapters are separate.
/// </para>
///
/// <para>
/// <strong>This writer can never fail or stall the session transaction it
/// observes</strong> (Campaign LA LA1 review fix F1). Every call site sits
/// inside a caller-owned try block that treats a throw as a real failure —
@ -91,6 +103,8 @@ public sealed class SessionStatusWriter
private readonly object _gate = new();
private bool _directoryEnsured;
private bool _latchedOff;
private bool _connected;
private bool _exited;
public SessionStatusWriter(string? path, TimeProvider? timeProvider = null)
{
@ -116,14 +130,44 @@ public sealed class SessionStatusWriter
sessionId,
});
public void Connected(string sessionId) =>
Write(new
public void Connected(string sessionId)
{
if (!IsEnabled)
return;
lock (_gate)
{
v = VocabularyVersion,
e = "connected",
t = Now(),
sessionId,
});
if (_latchedOff || _exited)
return;
if (_connected)
{
if (!TryWriteLocked(new
{
v = VocabularyVersion,
e = "disconnected",
t = Now(),
sessionId,
reason = "reconnect",
}))
{
return;
}
_connected = false;
}
if (TryWriteLocked(new
{
v = VocabularyVersion,
e = "connected",
t = Now(),
sessionId,
}))
{
_connected = true;
}
}
}
public void CharacterList(string sessionId, LiveSessionRosterReport roster)
{
@ -161,26 +205,70 @@ public sealed class SessionStatusWriter
characterName,
});
public void Disconnected(string sessionId, string reason) =>
Write(new
{
v = VocabularyVersion,
e = "disconnected",
t = Now(),
sessionId,
reason,
});
public void Disconnected(string sessionId, string reason)
{
if (!IsEnabled)
return;
public void Exited(string sessionId, int code, string reason) =>
Write(new
lock (_gate)
{
v = VocabularyVersion,
e = "exited",
t = Now(),
sessionId,
code,
reason,
});
if (_latchedOff || _exited)
return;
if (TryWriteLocked(new
{
v = VocabularyVersion,
e = "disconnected",
t = Now(),
sessionId,
reason,
}))
{
_connected = false;
}
}
}
public void Exited(string sessionId, int code, string reason)
{
if (!IsEnabled)
return;
lock (_gate)
{
if (_latchedOff || _exited)
return;
if (_connected)
{
if (!TryWriteLocked(new
{
v = VocabularyVersion,
e = "disconnected",
t = Now(),
sessionId,
reason = "process-exit",
}))
{
return;
}
_connected = false;
}
if (TryWriteLocked(new
{
v = VocabularyVersion,
e = "exited",
t = Now(),
sessionId,
code,
reason,
}))
{
_exited = true;
}
}
}
private string Now() =>
_timeProvider.GetUtcNow().ToString(
@ -189,7 +277,7 @@ public sealed class SessionStatusWriter
private void Write<T>(T value)
{
if (_path is not { } path || _latchedOff)
if (_path is null || _latchedOff)
return;
lock (_gate)
@ -197,26 +285,41 @@ public sealed class SessionStatusWriter
// Re-check inside the lock: another thread may have latched the
// writer off (or already ensured the directory) between the
// fast check above and taking the gate.
if (_latchedOff)
if (_latchedOff || _exited)
return;
try
{
EnsureDirectory(path);
string line = JsonSerializer.Serialize(value, JsonOptions);
using FileStream stream = new(
path,
FileMode.Append,
FileAccess.Write,
FileShare.Read);
using var writer = new StreamWriter(stream);
writer.WriteLine(line);
writer.Flush();
}
catch (Exception error) when (IsRecoverableIoFailure(error))
{
LatchOff(path, error);
}
_ = TryWriteLocked(value);
}
}
/// <summary>
/// Writes one event while <see cref="_gate"/> is held. Returning success
/// lets the lifecycle methods publish their state transition only after
/// the matching line has reached the stream. A recoverable I/O failure
/// latches the writer off, so there is never a retry that could duplicate
/// an uncertain terminal edge.
/// </summary>
private bool TryWriteLocked<T>(T value)
{
string path = _path!;
try
{
EnsureDirectory(path);
string line = JsonSerializer.Serialize(value, JsonOptions);
using FileStream stream = new(
path,
FileMode.Append,
FileAccess.Write,
FileShare.Read);
using var writer = new StreamWriter(stream);
writer.WriteLine(line);
writer.Flush();
return true;
}
catch (Exception error) when (IsRecoverableIoFailure(error))
{
LatchOff(path, error);
return false;
}
}
@ -234,10 +337,22 @@ public sealed class SessionStatusWriter
private void LatchOff(string path, Exception error)
{
_latchedOff = true;
Console.Error.WriteLine(
$"[status-writer] disabling status stream at '{path}' after a "
+ $"write failure ({error.GetType().Name}: {error.Message}); no "
+ "further events for this session will be written.");
try
{
Console.Error.WriteLine(
$"[status-writer] disabling status stream at '{path}' after a "
+ $"write failure ({error.GetType().Name}: {error.Message}); no "
+ "further events for this session will be written.");
}
catch (Exception diagnosticError)
when (IsRecoverableIoFailure(diagnosticError)
|| diagnosticError is ObjectDisposedException
or InvalidOperationException)
{
// This is the fallback diagnostic for an already-failed
// observability sink. A closed/broken stderr must not turn it
// back into a session-transaction failure.
}
}
/// <summary>

View file

@ -19,12 +19,18 @@ namespace AcDream.App.Tests.Configuration;
public sealed class SessionConfigurationSharedFixtureTests
{
[Fact]
public void AppReaderAcceptsTheSharedFixtureAndParsesTheFiveNewFields()
public void AppReaderAcceptsTheProductionShapedSharedFixture()
{
(SessionConfiguration configuration, SessionDescriptor session) =
SessionConfigurationLoader.Load(SharedFixturePath());
Assert.Equal(1, configuration.Version);
Assert.Equal(
"shared-fixture-dats",
configuration.Process?.Content?.DatDirectory);
Assert.Equal(
"shared-fixture-dats/acdream.pak",
configuration.Process?.Content?.PreparedAssetPath);
Assert.Equal("shared-fixture", session.Id);
Assert.Equal("127.0.0.1", session.Endpoint.Host);
Assert.Equal(9000, session.Endpoint.Port);
@ -34,9 +40,9 @@ public sealed class SessionConfigurationSharedFixtureTests
// the pinned contract's "parsed-and-ignored" clause.
Assert.Equal("idle", session.Policy?.Id);
Assert.Equal(
SessionCredentialProviderKind.Environment,
SessionCredentialProviderKind.StandardInput,
session.Credential.Provider);
Assert.Equal("SHARED_FIXTURE_PASSWORD", session.Credential.Reference);
Assert.Equal("session", session.Credential.Reference);
Assert.Equal(["ExamplePlugin", "AnotherPlugin"], session.Plugins);
Assert.Equal(

View file

@ -87,6 +87,27 @@ public sealed class LinuxPlatformBoundaryTests
Assert.Empty(offenders);
}
[Fact]
public void RuntimePlatformGuardHasOneDefinitionAndOneApprovedConsumer()
{
string app = AppSourceRoot();
string[] files = Directory
.EnumerateFiles(app, "*.cs", SearchOption.AllDirectories)
.Where(path => File.ReadAllText(path).Contains(
"RuntimePlatformGuard",
StringComparison.Ordinal))
.Select(path => Path.GetRelativePath(app, path).Replace('\\', '/'))
.OrderBy(static path => path, StringComparer.Ordinal)
.ToArray();
Assert.Equal(
[
"Credentials/AppCredentialResolver.cs",
"Platform/GraphicalHostPlatformServices.cs",
],
files);
}
[Fact]
public void SmokePluginCopyUsesRidAwarePortableBuildAndPublishPaths()
{

View file

@ -234,6 +234,26 @@ public sealed class RuntimeOptionsTests
Assert.Equal("testpassword", realValues.LivePass);
}
[Fact]
public void RecordPrintMembersRedactsTheLivePassword()
{
RuntimeOptions options = RuntimeOptions.Parse(
AnyDatDir,
Env(new()
{
["ACDREAM_LIVE"] = "1",
["ACDREAM_TEST_USER"] = "testaccount",
["ACDREAM_TEST_PASS"] = "top-secret-value",
}));
string printed = options.ToString();
Assert.DoesNotContain("top-secret-value", printed, StringComparison.Ordinal);
Assert.Contains("LivePass = <redacted>", printed, StringComparison.Ordinal);
Assert.Contains("LiveHost = 127.0.0.1", printed, StringComparison.Ordinal);
Assert.Contains("HasLiveCredentials = True", printed, StringComparison.Ordinal);
}
[Fact]
public void HasLiveCredentials_RequiresLiveModeAndBothUserAndPass()
{

View file

@ -62,6 +62,73 @@ public sealed class HeadlessSessionHostTests
Assert.DoesNotContain("AcDream.App", diagnostics);
}
/// <summary>
/// Campaign LA LA1 review fixes F3/F6: reconnect is a visible lifecycle
/// replacement, so the retiring connection must publish disconnected
/// before the new connection publishes connected. Its reason is distinct
/// from the final host stop and from the terminal process outcome.
/// </summary>
[Fact]
public void ReconnectPublishesDisconnectedBeforeTheSecondConnectedEdge()
{
string statusPath = Path.Combine(
Path.GetTempPath(),
$"acdream-headless-reconnect-status-{Guid.NewGuid():N}.jsonl");
try
{
var operations = new FixtureSessionOperations();
using var diagnosticsOutput = new StringWriter();
using var credential = new HeadlessCredentialSecret(
"fixture",
"password");
using var host = new HeadlessSessionHost(
Descriptor(statusFile: statusPath),
credential,
new HeadlessDiagnosticWriter(diagnosticsOutput),
operations);
Assert.Equal(
RuntimeSessionStartStatus.Connected,
host.Start().Status);
Assert.Equal(
RuntimeSessionStartStatus.Connected,
host.Reconnect().Status);
host.Dispose();
JsonElement[] events = File.ReadAllLines(statusPath)
.Select(static line => JsonDocument.Parse(line).RootElement.Clone())
.ToArray();
Assert.Equal(
[
"started", "connected", "characterList", "enteredWorld",
"disconnected", "connected", "characterList", "enteredWorld",
"disconnected", "exited",
],
events.Select(static item => item.GetProperty("e").GetString()));
JsonElement[] disconnected = events
.Where(static item =>
item.GetProperty("e").GetString() == "disconnected")
.ToArray();
Assert.Equal(2, disconnected.Length);
Assert.Equal(
"reconnect",
disconnected[0].GetProperty("reason").GetString());
Assert.Equal(
"stopped",
disconnected[1].GetProperty("reason").GetString());
JsonElement exited = events[^1];
Assert.Equal(0, exited.GetProperty("code").GetInt32());
Assert.Equal("graceful", exited.GetProperty("reason").GetString());
}
finally
{
if (File.Exists(statusPath))
File.Delete(statusPath);
}
}
/// <summary>
/// Campaign LA slice LA1: proves the status-event writer fires the
/// pinned lifecycle vocabulary — started/connected/characterList/

View file

@ -18,11 +18,17 @@ namespace AcDream.Headless.Tests;
public sealed class SessionConfigurationSharedFixtureTests
{
[Fact]
public void HeadlessReaderAcceptsTheSharedFixtureAndParsesTheFiveNewFields()
public void HeadlessReaderAcceptsTheProductionShapedSharedFixture()
{
HeadlessConfiguration configuration =
HeadlessConfigurationLoader.Load(SharedFixturePath());
Assert.Equal(
"shared-fixture-dats",
configuration.Process.Content?.DatDirectory);
Assert.Equal(
"shared-fixture-dats/acdream.pak",
configuration.Process.Content?.PreparedAssetPath);
HeadlessSessionDescriptor session = Assert.Single(configuration.Sessions)!;
Assert.Equal("shared-fixture", session.Id);
Assert.Equal("127.0.0.1", session.Endpoint.Host);
@ -31,9 +37,9 @@ public sealed class SessionConfigurationSharedFixtureTests
Assert.Equal("SharedToon", session.Character.Name);
Assert.Equal("idle", session.Policy.Id);
Assert.Equal(
HeadlessCredentialProviderKind.Environment,
HeadlessCredentialProviderKind.StandardInput,
session.Credential.Provider);
Assert.Equal("SHARED_FIXTURE_PASSWORD", session.Credential.Reference);
Assert.Equal("session", session.Credential.Reference);
Assert.Equal(["ExamplePlugin", "AnotherPlugin"], session.Plugins);
Assert.Equal(

View file

@ -97,6 +97,63 @@ public sealed class SessionStatusWriterTests
writer.Started("s1");
}
/// <summary>
/// F3: both hosts share this writer but have separate reconnect command
/// adapters. The sink therefore closes an open connection before it
/// accepts another connected edge, preserving a coherent external
/// lifecycle even if a host has no reconnect-specific status hook.
/// </summary>
[Fact]
public void SecondConnectedEdgeFirstClosesTheRetiringConnection()
{
using TemporaryFile file = TemporaryFile.Create();
var writer = new SessionStatusWriter(file.Path);
writer.Connected("s1");
writer.Connected("s1");
JsonElement[] events = File.ReadAllLines(file.Path)
.Select(Parse)
.ToArray();
Assert.Equal(
["connected", "disconnected", "connected"],
events.Select(static item => item.GetProperty("e").GetString()));
Assert.Equal(
"reconnect",
events[1].GetProperty("reason").GetString());
}
/// <summary>
/// F6: exited is a terminal fact, not an append request. Repeated host
/// disposal and any late callback after disposal must not create a second
/// terminal edge or resurrect the stream. If a host exits while still
/// connected, the writer closes that connection first with a distinct,
/// truthful reason.
/// </summary>
[Fact]
public void ExitedIsIdempotentTerminalAndClosesAnOpenConnection()
{
using TemporaryFile file = TemporaryFile.Create();
var writer = new SessionStatusWriter(file.Path);
writer.Connected("s1");
writer.Exited("s1", 0, "graceful");
writer.Exited("s1", 1, "duplicate-must-not-win");
writer.Connected("s1");
JsonElement[] events = File.ReadAllLines(file.Path)
.Select(Parse)
.ToArray();
Assert.Equal(
["connected", "disconnected", "exited"],
events.Select(static item => item.GetProperty("e").GetString()));
Assert.Equal(
"process-exit",
events[1].GetProperty("reason").GetString());
Assert.Equal(0, events[2].GetProperty("code").GetInt32());
Assert.Equal("graceful", events[2].GetProperty("reason").GetString());
}
/// <summary>
/// F7 (Campaign LA LA1 review fix round): replaces the earlier
/// "DoesNotContain 'hunter2'/'password'" assertion, which could never