diff --git a/docs/plans/2026-08-14-launcher-campaign.md b/docs/plans/2026-08-14-launcher-campaign.md index ae9586e1..264a209b 100644 --- a/docs/plans/2026-08-14-launcher-campaign.md +++ b/docs/plans/2026-08-14-launcher-campaign.md @@ -478,7 +478,7 @@ LA6 adds CH-regression scrutiny; LA0 adds guard-integrity scrutiny. |---|---|---|---|---| | LA0 | **DONE 2026-08-14** | `cb6502c8`, `a49e92df` | Opus dual-lens PASS; all 6 findings CLOSED in narrow re-review | Byte-identity proven; Linux CI lanes restored; Platform BCL-only self-guard added; K0 guard untouched | | LA1 | implemented; Opus review in flight | `db9ad53c` (mixed — see `e1322a06`) | review in flight | Runtime 1630 / Headless 126 / App 5025+3skip / Core.Net 905 green; Runtime+Headless green on WSL; shared fixture parsed by BOTH host readers | -| LA2 | implementation complete; automated gates **GREEN**; Opus dual-lens review pending | `c6019424` + completion (this commit) | pending | Probe is roster-before-selection with graceful pre-world teardown and exit 0; normal play remains strict selector + `idle` policy. Release build green; Runtime 1,632/1,632 and Headless 141/141 on both Windows and Ubuntu/WSL | +| LA2 | review fix round complete; automated gates **GREEN**; narrow re-review pending | `c6019424`, `000ea979` + review fixes (this commit) | Opus dual-lens FIX FIRST; all 3 findings fixed, narrow re-review pending | ProbeComplete now requires a reported roster and is still before selection/EnterWorld; terminal status derives from the actual start outcome and matches failed-start process code 5; conditional config fields distinguish omission from explicit null without weakening strict JSON shape/type checks. Release build green on Windows and Ubuntu/WSL; Runtime 1,632/1,632 and Headless 149/149 on both | | LA3 | review FIX FIRST; fix round in flight | `37d74e44` + fixes pending | Opus 2026-08-14: 12 findings — 1 CRITICAL (`"paths": {}` breaks App loader), probe composition owed, Stop→SIGKILL hazard, 0600 temp window | Contract text now COMMITTED into LA1 section (review process note); cross-assembly loader test owed at LA1+LA3 merge; CI lane addition at merge | | LA4 | — | | | | | LA5 | — | | | | diff --git a/src/AcDream.Headless/Configuration/HeadlessConfiguration.cs b/src/AcDream.Headless/Configuration/HeadlessConfiguration.cs index 1ad3a913..fdae1225 100644 --- a/src/AcDream.Headless/Configuration/HeadlessConfiguration.cs +++ b/src/AcDream.Headless/Configuration/HeadlessConfiguration.cs @@ -47,8 +47,9 @@ internal sealed record HeadlessSessionDescriptor public string Account { get; init; } = string.Empty; /// - /// Campaign LA slice LA2: ABSENT () for normal play - /// sessions; for the LA2 probe + /// Campaign LA slice LA2: the JSON field is ABSENT for normal play + /// sessions (explicit JSON null is invalid); + /// for the LA2 probe /// (connect → characterList → graceful disconnect, never EnterWorld) — /// the pinned launch-contract schema's mode field /// (docs/plans/2026-08-14-launcher-campaign.md LA1/LA2). diff --git a/src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs b/src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs index 4c83699c..27686c1e 100644 --- a/src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs +++ b/src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs @@ -97,10 +97,15 @@ internal static class HeadlessConfigurationLoader string fullPath = Path.GetFullPath(path); using FileStream stream = File.OpenRead(fullPath); + using JsonDocument document = JsonDocument.Parse( + stream, + new JsonDocumentOptions + { + AllowTrailingCommas = false, + CommentHandling = JsonCommentHandling.Disallow, + }); HeadlessConfiguration? configuration = - JsonSerializer.Deserialize( - stream, - Options); + document.RootElement.Deserialize(Options); if (configuration is null) { @@ -123,9 +128,12 @@ internal static class HeadlessConfigurationLoader ValidateContent(configuration.Process?.Content); + JsonElement sessionsElement = + document.RootElement.GetProperty("sessions"); var sessionIds = new HashSet(StringComparer.Ordinal); var credentialReferences = new HashSet( StringComparer.Ordinal); + int sessionIndex = 0; foreach (HeadlessSessionDescriptor? session in configuration.Sessions) { if (session is null @@ -141,7 +149,7 @@ internal static class HeadlessConfigurationLoader $"Duplicate session id '{session.Id}'."); } - ValidateSession(session); + ValidateSession(session, sessionsElement[sessionIndex]); string credentialKey = $"{session.Credential.Provider}:{session.Credential.Reference}"; if (!credentialReferences.Add(credentialKey)) @@ -149,6 +157,7 @@ internal static class HeadlessConfigurationLoader throw new HeadlessConfigurationException( $"Credential reference for session '{session.Id}' is already in use."); } + sessionIndex++; } return configuration; @@ -166,7 +175,9 @@ internal static class HeadlessConfigurationLoader } } - private static void ValidateSession(HeadlessSessionDescriptor session) + private static void ValidateSession( + HeadlessSessionDescriptor session, + JsonElement sessionElement) { if (session.Endpoint is null || string.IsNullOrWhiteSpace(session.Endpoint.Host) @@ -182,7 +193,7 @@ internal static class HeadlessConfigurationLoader $"Session '{session.Id}' requires a non-empty account."); } - ValidateModeShape(session); + ValidateModeShape(session, sessionElement); if (session.Credential is null || string.IsNullOrWhiteSpace(session.Credential.Reference)) @@ -214,17 +225,37 @@ internal static class HeadlessConfigurationLoader /// that also declares a selector or a policy, rather than silently /// ignoring them. /// - private static void ValidateModeShape(HeadlessSessionDescriptor session) + private static void ValidateModeShape( + HeadlessSessionDescriptor session, + JsonElement sessionElement) { + bool hasMode = sessionElement.TryGetProperty( + "mode", + out JsonElement modeElement); + bool hasCharacter = sessionElement.TryGetProperty( + "character", + out JsonElement characterElement); + bool hasPolicy = sessionElement.TryGetProperty( + "policy", + out JsonElement policyElement); + + RejectExplicitNull(session.Id, "mode", hasMode, modeElement); + RejectExplicitNull( + session.Id, + "character", + hasCharacter, + characterElement); + RejectExplicitNull(session.Id, "policy", hasPolicy, policyElement); + if (session.Mode == HeadlessSessionMode.Probe) { - if (session.Character is not null) + if (hasCharacter) { throw new HeadlessConfigurationException( $"Session '{session.Id}' has mode \"probe\" and must omit " + "'character' — a probe never selects a character."); } - if (session.Policy is not null) + if (hasPolicy) { throw new HeadlessConfigurationException( $"Session '{session.Id}' has mode \"probe\" and must omit " @@ -233,6 +264,12 @@ internal static class HeadlessConfigurationLoader return; } + if (hasMode) + { + throw new HeadlessConfigurationException( + $"Session '{session.Id}' is normal play and must omit 'mode'."); + } + if (session.Character is null) { throw new HeadlessConfigurationException( @@ -259,6 +296,29 @@ internal static class HeadlessConfigurationLoader } } + /// + /// Campaign LA LA2 review fix: the pinned launch contract distinguishes + /// an omitted conditional field from a field explicitly authored as JSON + /// null. Nullable CLR properties cannot retain that distinction, so + /// validation also consumes the already-parsed strict JSON shape. The + /// typed serializer still owns unknown-member, enum, and value-type + /// enforcement; this check adds presence semantics without weakening any + /// of those gates. + /// + private static void RejectExplicitNull( + string sessionId, + string propertyName, + bool isPresent, + JsonElement value) + { + if (isPresent && value.ValueKind == JsonValueKind.Null) + { + throw new HeadlessConfigurationException( + $"Session '{sessionId}' field '{propertyName}' cannot be null; " + + "supply a value when allowed or omit the field."); + } + } + /// /// Campaign LA slice LA1: validates the four new optional per-session /// fields shared with the App session-config reader (see diff --git a/src/AcDream.Headless/Hosting/HeadlessSessionHost.cs b/src/AcDream.Headless/Hosting/HeadlessSessionHost.cs index 0a17e8f5..392d6166 100644 --- a/src/AcDream.Headless/Hosting/HeadlessSessionHost.cs +++ b/src/AcDream.Headless/Hosting/HeadlessSessionHost.cs @@ -120,6 +120,15 @@ internal sealed class HeadlessSessionHost : IDisposable /// rework of the shared-stdout diagnostics writer. /// private readonly SessionStatusWriter _statusWriter; + /// + /// Campaign LA LA2 review fix: the actual result returned by the process + /// start attempt. A configured probe mode is only intent; terminal status + /// may claim reason:"probe" after this records + /// . Any other + /// non-connected result maps to the same connection-error code returned by + /// . + /// + private RuntimeSessionStartStatus? _startOutcome; /// Guards 's disconnected status event /// so a Stop() on a session that never actually reached Connected (e.g. /// disposing a fresh, never-started host) does not report a spurious @@ -465,7 +474,10 @@ internal sealed class HeadlessSessionHost : IDisposable // Campaign LA slice LA1: "started" = session host start — the // earliest point this session actually attempts to connect. _statusWriter.Started(_descriptor.Id); - return Commands.Session.Start(Runtime.Generation); + RuntimeSessionStartResult result = + Commands.Session.Start(Runtime.Generation); + _startOutcome = result.Status; + return result; } internal RuntimeSessionStartResult Reconnect() => @@ -647,18 +659,16 @@ internal sealed class HeadlessSessionHost : IDisposable _stoppedGeneration); // Campaign LA slice LA1: "exited" = terminal — the sole // point every disposal path (graceful and post- - // quarantine) converges on. LA2: a probe session that - // never faulted reports reason "probe" here instead of - // "disposed" — the pinned contract's exit event for a - // successful probe. + // quarantine) converges on. LA2: only an actual + // ProbeComplete start outcome reports reason "probe"; + // configured probe intent cannot turn a failed start into + // a successful terminal event. + (int exitCode, string exitReason) = + ResolveTerminalStatus(); _statusWriter.Exited( _descriptor.Id, - _faulted ? 1 : 0, - _faulted - ? "fault" - : _descriptor.Mode == HeadlessSessionMode.Probe - ? "probe" - : "disposed"); + exitCode, + exitReason); _disposeStage++; _disposed = true; break; @@ -669,6 +679,33 @@ internal sealed class HeadlessSessionHost : IDisposable } } + /// + /// Produces the same terminal classification the owning process host uses. + /// Descriptor mode never participates: only an observed ProbeComplete may + /// report a successful probe. The surrounding disposal stage and LA1's + /// terminal/idempotent make this event + /// exact-once even when disposal is retried. + /// + private (int Code, string Reason) ResolveTerminalStatus() + { + if (_faulted) + { + return ( + (int)HeadlessExitCode.RuntimeError, + "runtime-fault"); + } + + return _startOutcome switch + { + RuntimeSessionStartStatus.ProbeComplete => + ((int)HeadlessExitCode.Success, "probe"), + null or RuntimeSessionStartStatus.Connected => + ((int)HeadlessExitCode.Success, "graceful"), + _ => + ((int)HeadlessExitCode.ConnectionError, "connection-error"), + }; + } + private RuntimeSessionStartResult StartCore( RuntimeGenerationToken expectedGeneration, bool reconnect) diff --git a/src/AcDream.Runtime/Session/LiveSessionController.cs b/src/AcDream.Runtime/Session/LiveSessionController.cs index ee4fdfbb..935de09d 100644 --- a/src/AcDream.Runtime/Session/LiveSessionController.cs +++ b/src/AcDream.Runtime/Session/LiveSessionController.cs @@ -656,16 +656,13 @@ public sealed class LiveSessionController } // Campaign LA slice LA2: the probe short-circuit lands here — - // right after the roster report, before TrySelectCharacter ever - // runs — so a probe session never reaches selection, - // ApplySelectedCharacter, or EnterWorld. This mirrors the - // NoCharacters early-exit immediately below (same StopCore - // teardown), the deliberate difference being the returned status - // is a SUCCESS, not a failure. Non-probe callers (options.Probe - // is false by default) fall straight through to the unchanged - // selection/enter path below — byte-identical to pre-LA2 - // behavior. - if (options.Probe) + // only after a real CharacterList was returned and its roster was + // reported, before TrySelectCharacter ever runs. A missing + // CharacterList falls through to the existing NoCharacters + // non-success path below; connectivity by itself is not a + // successful character-roster probe. Non-probe callers continue + // through the unchanged selection/enter path. + if (options.Probe && characters is not null) { Console.WriteLine( "live: probe complete — disconnecting before EnterWorld"); diff --git a/tests/AcDream.Headless.Tests/HeadlessConfigurationLoaderTests.cs b/tests/AcDream.Headless.Tests/HeadlessConfigurationLoaderTests.cs index 6902fccc..25636cb4 100644 --- a/tests/AcDream.Headless.Tests/HeadlessConfigurationLoaderTests.cs +++ b/tests/AcDream.Headless.Tests/HeadlessConfigurationLoaderTests.cs @@ -269,6 +269,105 @@ public sealed class HeadlessConfigurationLoaderTests () => HeadlessConfigurationLoader.Load(file.Path)); } + /// + /// Campaign LA LA2 review fix: the pinned contract is presence-aware. + /// Normal play omits mode and supplies character/policy; probe supplies + /// mode and omits character/policy. JSON null is not another spelling of + /// omission for any of those conditional fields. + /// + [Theory] + [InlineData(false, "mode")] + [InlineData(false, "character")] + [InlineData(false, "policy")] + [InlineData(true, "character")] + [InlineData(true, "policy")] + public void ConditionalSessionFieldsRejectExplicitJsonNull( + bool probe, + string nullField) + { + string mode = probe + ? "\"mode\":\"probe\"," + : nullField == "mode" + ? "\"mode\":null," + : string.Empty; + string character = nullField == "character" + ? "\"character\":null," + : probe + ? string.Empty + : "\"character\":{\"index\":0},"; + string policy = nullField == "policy" + ? "\"policy\":null," + : probe + ? string.Empty + : "\"policy\":{\"id\":\"idle\"},"; + using TemporaryConfiguration file = TemporaryConfiguration.Create( + $$""" + { + "version": 1, + "sessions": [ + { + "id": "explicit-null", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + {{mode}} + {{character}} + {{policy}} + "credential": { "provider": "environment", "reference": "PASSWORD" } + } + ] + } + """); + + HeadlessConfigurationException exception = Assert.Throws< + HeadlessConfigurationException>( + () => HeadlessConfigurationLoader.Load(file.Path)); + + Assert.Contains( + $"'{nullField}'", + exception.Message, + StringComparison.Ordinal); + Assert.Contains( + "cannot be null", + exception.Message, + StringComparison.Ordinal); + } + + [Fact] + public void PresenceAwareValidationKeepsUnmappedMemberRejection() + { + using TemporaryConfiguration file = TemporaryConfiguration.Create( + ConfigurationWith(Session( + "bot", + "PASSWORD", + "\"notAContractField\":true"))); + + Assert.Throws( + () => HeadlessConfigurationLoader.Load(file.Path)); + } + + [Fact] + public void PresenceAwareValidationKeepsTypedValueRejection() + { + using TemporaryConfiguration file = TemporaryConfiguration.Create( + """ + { + "version": 1, + "sessions": [ + { + "id": "wrong-type", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + "mode": { "value": "probe" }, + "credential": { "provider": "environment", "reference": "PASSWORD" } + } + ] + } + """); + + Assert.Throws( + () => HeadlessConfigurationLoader.Load(file.Path)); + } + [Fact] public void ProbeSessionDeclaringCharacterFailsLoadNamingTheField() { diff --git a/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs b/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs index f568feb2..8ec13904 100644 --- a/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs +++ b/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs @@ -262,6 +262,85 @@ public sealed class HeadlessSessionHostTests Assert.False(cancellation.IsCancellationRequested); } + /// + /// Campaign LA LA2 review fix: configured probe intent is not proof of a + /// completed probe. If the connected session produces no CharacterList, + /// Runtime returns NoCharacters, the process returns ConnectionError, and + /// the sole terminal status event reports that same non-success instead of + /// the former false code-0/reason-probe pair. + /// + [Fact] + public async Task ProbeWithoutRosterReportsTheProcessConnectionErrorExactlyOnce() + { + string statusPath = Path.Combine( + Path.GetTempPath(), + $"acdream-headless-probe-no-roster-{Guid.NewGuid():N}.jsonl"); + try + { + var configuration = new HeadlessConfiguration + { + Version = 1, + Sessions = + [ + ProbeDescriptor( + provider: HeadlessCredentialProviderKind.StandardInput, + credentialReference: "probe-password", + statusFile: statusPath), + ], + }; + var operations = new FixtureSessionOperations + { + Characters = null, + }; + using var diagnostics = new StringWriter(); + using var host = new HeadlessProcessHost( + configuration, + HeadlessPathSet.Resolve(new HeadlessPathOverrides()), + new System.IO.StringReader( + "probe-password" + Environment.NewLine), + diagnostics, + operations); + + HeadlessExitCode result = await host.RunAsync( + CancellationToken.None); + + Assert.Equal(HeadlessExitCode.ConnectionError, result); + Assert.Equal(0, operations.EnterWorldCallCount); + Assert.Equal(1, operations.DisposedSessionCount); + + host.Dispose(); + host.Dispose(); + + string[] lines = File.ReadAllLines(statusPath); + JsonElement[] events = lines + .Select(static line => + JsonDocument.Parse(line).RootElement.Clone()) + .ToArray(); + Assert.Equal( + ["started", "connected", "disconnected", "exited"], + events.Select(static item => + item.GetProperty("e").GetString())); + Assert.DoesNotContain( + events, + static item => + item.GetProperty("e").GetString() == "characterList"); + JsonElement exited = Assert.Single( + events, + static item => item.GetProperty("e").GetString() == "exited"); + Assert.Equal( + (int)result, + exited.GetProperty("code").GetInt32()); + Assert.Equal( + "connection-error", + exited.GetProperty("reason").GetString()); + } + finally + { + if (File.Exists(statusPath)) + File.Delete(statusPath); + } + } + /// /// Campaign LA slice LA2: a probe session completing must not tear down /// a sibling play session sharing the same process — the process exit @@ -3019,6 +3098,23 @@ public sealed class HeadlessSessionHostTests public int EnterWorldCallCount => Volatile.Read(ref _enterWorldCallCount); public int TickCallCount => Volatile.Read(ref _tickCallCount); + public CharacterList.Parsed? Characters { get; init; } = new( + 0u, + [ + new CharacterList.Character( + 0x50000001u, + "Other", + 0u), + new CharacterList.Character( + 0x50000002u, + "Headless", + 0u), + ], + [], + 11, + "account", + true, + true); public IPEndPoint ResolveEndpoint(string host, int port) => new(IPAddress.Loopback, port); @@ -3040,25 +3136,8 @@ public sealed class HeadlessSessionHostTests LastPassword = password; } - public CharacterList.Parsed GetCharacters( - WorldSession session) => - new( - 0u, - [ - new CharacterList.Character( - 0x50000001u, - "Other", - 0u), - new CharacterList.Character( - 0x50000002u, - "Headless", - 0u), - ], - [], - 11, - "account", - true, - true); + public CharacterList.Parsed? GetCharacters( + WorldSession session) => Characters; public void EnterWorld( WorldSession session, diff --git a/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs b/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs index b4ff6a25..10b4e18f 100644 --- a/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs @@ -441,13 +441,13 @@ public sealed class LiveSessionControllerTests } /// - /// The probe short-circuit fires even when the server never returns a - /// CharacterList at all (GetCharacters returns null) — a probe is a - /// connectivity check, not itself a character-selection operation, so it - /// must not fall through to the NoCharacters path. + /// Campaign LA LA2 review fix: ProbeComplete proves a real CharacterList + /// was received and reported, not merely that the socket connected. A + /// missing roster follows the existing NoCharacters non-success path and + /// still drains the exact pre-world teardown transaction. /// [Fact] - public void Start_ProbeWithoutCharacterListStillCompletesGracefully() + public void Start_ProbeWithoutCharacterListIsNonSuccessAndTearsDownGracefully() { var calls = new List(); var operations = new TestOperations(calls) { Characters = null }; @@ -458,10 +458,25 @@ public sealed class LiveSessionControllerTests LiveOptions(probe: true), host); - Assert.Equal(LiveSessionStartStatus.ProbeComplete, result.Status); + Assert.Equal(LiveSessionStartStatus.NoCharacters, result.Status); Assert.Empty(host.Rosters); + Assert.Equal( + [ + "reset", "resolve", "create", "bind", "report-connecting", + "connect", "report-connected", "deactivate", + "detach-events", "dispose-session", "detach-session", "reset", + ], + calls); Assert.Equal(0, operations.EnterWorldCount); Assert.False(controller.IsInWorld); + Assert.Null(controller.CurrentSession); + Assert.Equal(1, operations.DisposeCounts[operations.Sessions[0]]); + + LiveSessionOwnershipSnapshot ownership = controller.CaptureOwnership(); + Assert.Equal(RuntimeTeardownStage.Complete, ownership.LastTeardownStages); + Assert.False(ownership.HasActiveSession); + Assert.False(ownership.HasRetiredSession); + Assert.False(ownership.HasPendingOperation); } [Theory]