diff --git a/src/AcDream.App/Configuration/SessionConfigArgumentParsing.cs b/src/AcDream.App/Configuration/SessionConfigArgumentParsing.cs new file mode 100644 index 00000000..96d2f43c --- /dev/null +++ b/src/AcDream.App/Configuration/SessionConfigArgumentParsing.cs @@ -0,0 +1,73 @@ +namespace AcDream.App.Configuration; + +/// +/// Campaign LA slice LA1 review fix (F5): extracted from Program.cs's +/// top-level-statement local functions so the trailing-flag edge case is +/// unit testable — a top-level program's local functions are compiler- +/// synthesized private members of the generated Program class with +/// no stable surface a test assembly can reach. +/// +internal static class SessionConfigArgumentParsing +{ + /// + /// Finds in and + /// returns its value. Three distinct outcomes, distinguished by + /// and the return value together: + /// + /// flag absent: = , + /// returns — the caller's env-var/positional + /// fallback stays in effect, unchanged from before this flag + /// existed. + /// flag present with a following value: + /// = , returns that value. + /// flag present but is the LAST argument, with nothing after it: + /// = , returns + /// — the caller MUST treat this as a hard error + /// (the flag was typed but its value was not), never silently fall + /// through to the flag-absent path. + /// + /// + internal static string? ExtractFlagValue( + string[] arguments, + string flag, + out bool present) + { + ArgumentNullException.ThrowIfNull(arguments); + ArgumentException.ThrowIfNullOrWhiteSpace(flag); + + for (int i = 0; i < arguments.Length; i++) + { + if (!string.Equals(arguments[i], flag, StringComparison.Ordinal)) + continue; + + present = true; + return i == arguments.Length - 1 ? null : arguments[i + 1]; + } + + present = false; + return null; + } + + /// Returns with + /// and its following value (if any) removed. A trailing, valueless flag + /// is dropped on its own — this helper only strips arguments, it does + /// not decide whether a trailing flag is an error (see + /// 's present output for that). + internal static string[] WithoutFlagAndValue(string[] arguments, string flag) + { + ArgumentNullException.ThrowIfNull(arguments); + ArgumentException.ThrowIfNullOrWhiteSpace(flag); + + var result = new List(arguments.Length); + for (int i = 0; i < arguments.Length; i++) + { + if (string.Equals(arguments[i], flag, StringComparison.Ordinal)) + { + i++; // also skip the flag's value, if any + continue; + } + result.Add(arguments[i]); + } + return [.. result]; + } +} diff --git a/src/AcDream.App/Configuration/SessionConfiguration.cs b/src/AcDream.App/Configuration/SessionConfiguration.cs index 313f75ea..2a0359be 100644 --- a/src/AcDream.App/Configuration/SessionConfiguration.cs +++ b/src/AcDream.App/Configuration/SessionConfiguration.cs @@ -42,6 +42,26 @@ internal sealed class SessionConfiguration internal sealed class SessionProcessSettings { public SessionContentDescriptor? Content { get; init; } + + /// Campaign LA slice LA1 review fix (F2): accepted so the SAME + /// document also satisfies the Headless loader's own + /// process.paths member (HeadlessPathOverrides) — parsed + /// and ignored here, exactly like + /// and below. App has + /// no config/data/cache directory override concept of its own (those + /// come from ApplicationPathSet/env vars on this host); only the + /// Headless host consumes overrides composed under this key. + public SessionProcessPathOverrides? Paths { get; init; } +} + +/// Accepted-but-ignored mirror of Headless's +/// HeadlessPathOverrides shape — see +/// . +internal sealed class SessionProcessPathOverrides +{ + public string? ConfigDirectory { get; init; } + public string? DataDirectory { get; init; } + public string? CacheDirectory { get; init; } } internal sealed class SessionContentDescriptor @@ -75,6 +95,19 @@ internal sealed record SessionDescriptor /// App has no bot-policy concept. public SessionPolicyDescriptor? Policy { get; init; } + /// Campaign LA slice LA1 review fix (F2): pinned-contract + /// mode discriminator. ABSENT means today's ONLY App behavior — an + /// ordinary play session — so every document written before this field + /// existed keeps parsing unchanged. "probe" (LA2's connect + /// ▸ characterList ▸ graceful-disconnect flow, no EnterWorld) is + /// HEADLESS-ONLY; the App loader rejects it with an explicit message + /// naming the field rather than the caller ever seeing a raw unmapped- + /// member . Any other value + /// is a configuration error — the pinned contract defines no other + /// mode literal, so a document is either silent about mode (play) or + /// says "probe" exactly. + public string? Mode { get; init; } + [JsonRequired] public SessionCredentialDescriptor Credential { get; init; } = new(); diff --git a/src/AcDream.App/Configuration/SessionConfigurationLoader.cs b/src/AcDream.App/Configuration/SessionConfigurationLoader.cs index e26aa806..086cb11e 100644 --- a/src/AcDream.App/Configuration/SessionConfigurationLoader.cs +++ b/src/AcDream.App/Configuration/SessionConfigurationLoader.cs @@ -154,5 +154,31 @@ internal static class SessionConfigurationLoader throw new SessionConfigurationException( $"Session '{session.Id}' statusFile must be a non-empty path when present."); } + + ValidateMode(session); + } + + /// + /// Campaign LA slice LA1 review fix (F2): mode is Headless-only + /// on the App host — the graphical host has no probe concept (LA2 + /// builds the probe in Headless only). An absent field is today's ONLY + /// App behavior (play); "probe" gets a specific, actionable + /// message instead of a cryptic unmapped-member JSON error; anything + /// else is a plain configuration error. + /// + private static void ValidateMode(SessionDescriptor session) + { + if (session.Mode is null) + return; + + if (string.Equals(session.Mode, "probe", StringComparison.Ordinal)) + { + throw new SessionConfigurationException( + $"Session '{session.Id}' has mode 'probe'; probe sessions " + + "are headless-only and cannot run on the graphical host."); + } + + throw new SessionConfigurationException( + $"Session '{session.Id}' has unsupported mode '{session.Mode}'."); } } diff --git a/src/AcDream.App/Program.cs b/src/AcDream.App/Program.cs index 61ed733a..52e647be 100644 --- a/src/AcDream.App/Program.cs +++ b/src/AcDream.App/Program.cs @@ -38,8 +38,22 @@ Log.Information( // existing one positional dat-dir argument and every ACDREAM_* env var keep // working exactly as before when the flag is absent. See // docs/plans/2026-08-14-launcher-campaign.md LA1. -string? sessionConfigFlagPath = ExtractFlagValue(args, "--session-config"); -string[] positionalArgs = WithoutFlagAndValue(args, "--session-config"); +// +// Review fix F5 (LA1 review round): a trailing, valueless --session-config +// (the flag typed as the LAST argument, nothing after it) must be a hard +// error, never a silent fall-through to the env-var path — a launcher that +// mis-composed its argv would otherwise appear to work while quietly +// ignoring the session-config contract entirely. +string? sessionConfigFlagPath = SessionConfigArgumentParsing.ExtractFlagValue( + args, "--session-config", out bool sessionConfigFlagPresent); +if (sessionConfigFlagPath is null && sessionConfigFlagPresent) +{ + Log.Error( + "--session-config requires a value (a path to the session-config document)."); + return 2; +} +string[] positionalArgs = + SessionConfigArgumentParsing.WithoutFlagAndValue(args, "--session-config"); var datDirArg = positionalArgs.FirstOrDefault(); var envDatDir = Environment.GetEnvironmentVariable("ACDREAM_DAT_DIR"); @@ -240,34 +254,9 @@ finally return 0; -// Campaign LA slice LA1: --session-config parsing helpers. Kept -// local/minimal rather than a general-purpose CLI parser — App has exactly -// one optional flag-with-value today; the positional dat-dir argument must -// stay untouched by its presence (see the comment above the flag parse). -static string? ExtractFlagValue(string[] arguments, string flag) -{ - for (int i = 0; i < arguments.Length - 1; i++) - { - if (string.Equals(arguments[i], flag, StringComparison.Ordinal)) - return arguments[i + 1]; - } - return null; -} - -static string[] WithoutFlagAndValue(string[] arguments, string flag) -{ - var result = new List(arguments.Length); - for (int i = 0; i < arguments.Length; i++) - { - if (string.Equals(arguments[i], flag, StringComparison.Ordinal)) - { - i++; // also skip the flag's value - continue; - } - result.Add(arguments[i]); - } - return [.. result]; -} - +// Campaign LA slice LA1: --session-config value-presence helper. The +// flag/positional-argument extraction itself lives in +// AcDream.App.Configuration.SessionConfigArgumentParsing (review fix F5) so +// its trailing-flag edge case is unit testable. static string? NullIfEmpty(string? value) => string.IsNullOrWhiteSpace(value) ? null : value; diff --git a/src/AcDream.Runtime/Session/SessionStatusWriter.cs b/src/AcDream.Runtime/Session/SessionStatusWriter.cs index b2260453..6ccb27fe 100644 --- a/src/AcDream.Runtime/Session/SessionStatusWriter.cs +++ b/src/AcDream.Runtime/Session/SessionStatusWriter.cs @@ -33,6 +33,49 @@ namespace AcDream.Runtime.Session; /// event method below takes only identifiers, names, and counts — there is no /// parameter shape that could carry a password, by construction. /// +/// +/// +/// This writer can never fail or stall the session transaction it +/// observes (Campaign LA LA1 review fix F1). Every call site sits +/// inside a caller-owned try block that treats a throw as a real failure — +/// LiveSessionController.StartCore's connect/roster/enter-world +/// sequence, SessionStartCompositionPhase.Start (which calls +/// BEFORE Session.Start even runs), +/// GameWindow.CompleteShutdown (which calls +/// BEFORE PublishShutdownRoots, so a throw would skip graceful +/// teardown entirely), and HeadlessSessionHost.Dispose's stage machine +/// (a throw from stage 8's call leaves +/// _disposeStage unadvanced and _disposed unset forever — a +/// permanently un-disposable host). An observability sink that can fail the +/// transaction it is merely reporting on is a defect in the sink, not a +/// reason for every call site to defend itself — so every exception this +/// class's own I/O can raise (a missing parent directory on a fresh cache +/// dir, a path segment that collides with an existing file, a permissions +/// error, a network path some future caller supplies) is caught here, logged +/// once to stderr, and LATCHES the writer into a permanent no-op — the exact +/// same "cheap null-check forever after" shape a never-configured path +/// already gets. The parent directory is created lazily, once, on the first +/// write, inside the same protection, so a fresh +/// .../launcher/sessions/<id>/status.jsonl path (whose directory +/// does not exist yet) is the expected first-run case, not a failure. +/// +/// +/// +/// Latency posture: every write is a synchronous local-disk +/// file open + line append + flush + close on the calling thread — there is +/// no batching, no background writer, no async path. This is fine for the +/// low-frequency lifecycle events this class carries (at most a handful per +/// second even under LA5/LA6 plugin/login-command load) against a local +/// disk. A statusFile path that resolves to a network location (a +/// UNC share, a mapped network drive, a FUSE mount with high per-syscall +/// latency) is UNSUPPORTED BY DESIGN — every event write would block the +/// session transaction's calling thread for the round-trip, and a slow or +/// wedged network path would eventually get caught by the same catch clause +/// that handles a missing directory and latch off, silently dropping the +/// rest of that session's status stream. Callers that need a status stream +/// over the network should tail the local file with a separate process, +/// never point statusFile at a network path directly. +/// /// public sealed class SessionStatusWriter { @@ -46,6 +89,8 @@ public sealed class SessionStatusWriter private readonly string? _path; private readonly TimeProvider _timeProvider; private readonly object _gate = new(); + private bool _directoryEnsured; + private bool _latchedOff; public SessionStatusWriter(string? path, TimeProvider? timeProvider = null) { @@ -54,12 +99,13 @@ public sealed class SessionStatusWriter } /// - /// True when this writer has a configured path and will actually append - /// events. Lets a caller with an expensive report to build (e.g. the - /// roster projection) skip that work entirely when nobody configured a - /// status file for this session. + /// True when this writer has a configured path and has not latched + /// itself off after a failed write. Lets a caller with an expensive + /// report to build (e.g. the roster projection) skip that work entirely + /// when nobody configured a status file for this session, or when this + /// writer already gave up after an I/O failure. /// - public bool IsEnabled => _path is not null; + public bool IsEnabled => _path is not null && !_latchedOff; public void Started(string sessionId) => Write(new @@ -143,20 +189,71 @@ public sealed class SessionStatusWriter private void Write(T value) { - if (_path is not { } path) + if (_path is not { } path || _latchedOff) return; - string line = JsonSerializer.Serialize(value, JsonOptions); lock (_gate) { - using FileStream stream = new( - path, - FileMode.Append, - FileAccess.Write, - FileShare.Read); - using var writer = new StreamWriter(stream); - writer.WriteLine(line); - writer.Flush(); + // 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) + 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); + } } } + + private void EnsureDirectory(string path) + { + if (_directoryEnsured) + return; + + string? directory = Path.GetDirectoryName(path); + if (!string.IsNullOrEmpty(directory)) + Directory.CreateDirectory(directory); + _directoryEnsured = true; + } + + 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."); + } + + /// + /// The set of exceptions this class's own file I/O can plausibly raise + /// — a missing parent directory, a path segment colliding with an + /// existing file, permission failures, an unsupported path shape, or a + /// platform security restriction. Anything outside this set (e.g. an + /// ) is deliberately NOT caught — + /// this class only promises to survive ITS OWN recoverable I/O + /// failures, never to become a blanket exception sink. + /// + private static bool IsRecoverableIoFailure(Exception error) => + error is IOException + or UnauthorizedAccessException + or NotSupportedException + or ArgumentException + or System.Security.SecurityException + or DirectoryNotFoundException; } diff --git a/tests/AcDream.App.Tests/Configuration/SessionConfigArgumentParsingTests.cs b/tests/AcDream.App.Tests/Configuration/SessionConfigArgumentParsingTests.cs new file mode 100644 index 00000000..1aed83f0 --- /dev/null +++ b/tests/AcDream.App.Tests/Configuration/SessionConfigArgumentParsingTests.cs @@ -0,0 +1,97 @@ +using AcDream.App.Configuration; + +namespace AcDream.App.Tests.Configuration; + +/// +/// Campaign LA slice LA1 review fix (F5): pins +/// 's trailing-flag edge case — +/// --session-config present as the LAST argument with nothing after +/// it must be distinguishable from the flag being entirely absent, so +/// Program.cs can turn it into a hard error instead of a silent +/// fall-through to the env-var/positional dat-dir path. +/// +public sealed class SessionConfigArgumentParsingTests +{ + private const string Flag = "--session-config"; + + [Fact] + public void FlagWithAFollowingValueReturnsThatValueAndIsPresent() + { + string? value = SessionConfigArgumentParsing.ExtractFlagValue( + ["D:\\dats", Flag, "session.json"], + Flag, + out bool present); + + Assert.True(present); + Assert.Equal("session.json", value); + } + + [Fact] + public void FlagAbsentReturnsNullAndIsNotPresent() + { + string? value = SessionConfigArgumentParsing.ExtractFlagValue( + ["D:\\dats"], + Flag, + out bool present); + + Assert.False(present); + Assert.Null(value); + } + + [Fact] + public void TrailingFlagWithNoValueIsPresentWithANullValue() + { + string? value = SessionConfigArgumentParsing.ExtractFlagValue( + ["D:\\dats", Flag], + Flag, + out bool present); + + // This is the case Program.cs must turn into exit code 2 — present + // but no value is categorically different from "not present at + // all", even though both currently yield a null return value. + Assert.True(present); + Assert.Null(value); + } + + [Fact] + public void FlagAloneAsTheOnlyArgumentIsPresentWithANullValue() + { + string? value = SessionConfigArgumentParsing.ExtractFlagValue( + [Flag], + Flag, + out bool present); + + Assert.True(present); + Assert.Null(value); + } + + [Fact] + public void WithoutFlagAndValueDropsTheFlagAndItsValue() + { + string[] positional = SessionConfigArgumentParsing.WithoutFlagAndValue( + ["D:\\dats", Flag, "session.json", "extra"], + Flag); + + Assert.Equal(["D:\\dats", "extra"], positional); + } + + [Fact] + public void WithoutFlagAndValueTrailingFlagDropsOnlyTheFlagItself() + { + string[] positional = SessionConfigArgumentParsing.WithoutFlagAndValue( + ["D:\\dats", Flag], + Flag); + + Assert.Equal(["D:\\dats"], positional); + } + + [Fact] + public void WithoutFlagAndValueLeavesArgumentsUnchangedWhenFlagIsAbsent() + { + string[] positional = SessionConfigArgumentParsing.WithoutFlagAndValue( + ["D:\\dats"], + Flag); + + Assert.Equal(["D:\\dats"], positional); + } +} diff --git a/tests/AcDream.App.Tests/Configuration/SessionConfigurationLoaderTests.cs b/tests/AcDream.App.Tests/Configuration/SessionConfigurationLoaderTests.cs new file mode 100644 index 00000000..54e64408 --- /dev/null +++ b/tests/AcDream.App.Tests/Configuration/SessionConfigurationLoaderTests.cs @@ -0,0 +1,140 @@ +using AcDream.App.Configuration; + +namespace AcDream.App.Tests.Configuration; + +/// +/// Campaign LA slice LA1 review fix (F2): the App session-config reader +/// must TOLERATE the two document shapes only the Headless side of the +/// pinned contract currently defines meaning for — process.paths +/// (HeadlessPathOverrides) and the per-session mode +/// discriminator (LA2's probe flow) — so a launcher-composed document does +/// not throw a raw unmapped-member +/// on the App host. See docs/plans/2026-08-14-launcher-campaign.md +/// LA1's pinned contract. +/// +public sealed class SessionConfigurationLoaderTests +{ + [Fact] + public void ProcessPathsAreAcceptedButIgnored() + { + using TemporaryFile file = TemporaryFile.Create( + """ + { + "version": 1, + "process": { + "paths": { + "configDirectory": "/config", + "dataDirectory": "/data", + "cacheDirectory": "/cache" + } + }, + "sessions": [ + { + "id": "paths-tolerant", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + "credential": { "provider": "environment", "reference": "X" } + } + ] + } + """); + + (SessionConfiguration configuration, SessionDescriptor session) = + SessionConfigurationLoader.Load(file.Path); + + Assert.Equal("paths-tolerant", session.Id); + Assert.Equal("/config", configuration.Process?.Paths?.ConfigDirectory); + Assert.Equal("/data", configuration.Process?.Paths?.DataDirectory); + Assert.Equal("/cache", configuration.Process?.Paths?.CacheDirectory); + } + + [Fact] + public void AbsentModeIsTreatedAsPlay() + { + using TemporaryFile file = TemporaryFile.Create( + """ + { + "version": 1, + "sessions": [ + { + "id": "no-mode", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + "credential": { "provider": "environment", "reference": "X" } + } + ] + } + """); + + (_, SessionDescriptor session) = SessionConfigurationLoader.Load(file.Path); + + Assert.Null(session.Mode); + } + + [Fact] + public void ProbeModeFailsLoadWithAnExplicitHeadlessOnlyMessage() + { + using TemporaryFile file = TemporaryFile.Create( + """ + { + "version": 1, + "sessions": [ + { + "id": "probe-session", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + "credential": { "provider": "environment", "reference": "X" }, + "mode": "probe" + } + ] + } + """); + + SessionConfigurationException error = Assert.Throws( + () => SessionConfigurationLoader.Load(file.Path)); + Assert.Contains("mode", error.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("probe", error.Message, StringComparison.Ordinal); + Assert.Contains("headless-only", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void UnrecognizedModeFailsLoad() + { + using TemporaryFile file = TemporaryFile.Create( + """ + { + "version": 1, + "sessions": [ + { + "id": "bad-mode", + "endpoint": { "host": "127.0.0.1", "port": 9000 }, + "account": "account", + "credential": { "provider": "environment", "reference": "X" }, + "mode": "bogus" + } + ] + } + """); + + Assert.Throws( + () => SessionConfigurationLoader.Load(file.Path)); + } + + private sealed class TemporaryFile : IDisposable + { + private TemporaryFile(string path) => Path = path; + + internal string Path { get; } + + internal static TemporaryFile Create(string json) + { + string path = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + $"acdream-app-la1-loader-{Guid.NewGuid():N}.json"); + File.WriteAllText(path, json); + return new TemporaryFile(path); + } + + public void Dispose() => File.Delete(Path); + } +} diff --git a/tests/AcDream.Runtime.Tests/Session/SessionStatusWriterTests.cs b/tests/AcDream.Runtime.Tests/Session/SessionStatusWriterTests.cs index 5b9e7133..c1bfc020 100644 --- a/tests/AcDream.Runtime.Tests/Session/SessionStatusWriterTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/SessionStatusWriterTests.cs @@ -97,8 +97,22 @@ public sealed class SessionStatusWriterTests writer.Started("s1"); } + /// + /// F7 (Campaign LA LA1 review fix round): replaces the earlier + /// "DoesNotContain 'hunter2'/'password'" assertion, which could never + /// actually fail — no writer method below accepts a credential-shaped + /// parameter in the first place, so the absence of those literal strings + /// proved nothing about the SHAPE of what gets serialized. This test + /// asserts the structural claim that actually backs the "never write + /// credential material into this stream" contract: each event kind + /// serializes EXACTLY its pinned property set — the shared envelope + /// (v/e/t/sessionId) plus that event's own + /// named fields, nothing else. An extra property (a smuggled password, + /// or any other accidental field) fails this test by construction, + /// regardless of what value it carries. + /// [Fact] - public void PasswordNeverAppearsInTheStatusStream() + public void EachEventSerializesExactlyItsPinnedPropertySetAndNothingElse() { using TemporaryFile file = TemporaryFile.Create(); var writer = new SessionStatusWriter(file.Path); @@ -115,9 +129,110 @@ public sealed class SessionStatusWriterTests writer.Disconnected("bot", "stopped"); writer.Exited("bot", 0, "disposed"); - string contents = File.ReadAllText(file.Path); - Assert.DoesNotContain("hunter2", contents, StringComparison.Ordinal); - Assert.DoesNotContain("password", contents, StringComparison.OrdinalIgnoreCase); + string[] lines = File.ReadAllLines(file.Path); + Assert.Equal(6, lines.Length); + + AssertExactProperties(lines[0], "v", "e", "t", "sessionId"); + AssertExactProperties(lines[1], "v", "e", "t", "sessionId"); + AssertExactProperties( + lines[2], + "v", "e", "t", "sessionId", "accountName", "slotCount", "characters"); + AssertExactProperties( + lines[3], "v", "e", "t", "sessionId", "characterId", "characterName"); + AssertExactProperties(lines[4], "v", "e", "t", "sessionId", "reason"); + AssertExactProperties(lines[5], "v", "e", "t", "sessionId", "code", "reason"); + + // The nested characters[] entries are exact too — the exact shape a + // password could otherwise be smuggled through. + JsonElement character = Parse(lines[2]).GetProperty("characters")[0]; + AssertExactProperties(character, "id", "name", "secondsGreyedOut"); + } + + private static void AssertExactProperties(string line, params string[] expected) => + AssertExactProperties(Parse(line), expected); + + private static void AssertExactProperties(JsonElement element, params string[] expected) + { + string[] actual = element.EnumerateObject() + .Select(static property => property.Name) + .OrderBy(static name => name, StringComparer.Ordinal) + .ToArray(); + string[] sortedExpected = expected + .OrderBy(static name => name, StringComparer.Ordinal) + .ToArray(); + Assert.Equal(sortedExpected, actual); + } + + /// + /// F1 (Campaign LA LA1 review fix round): a status file whose parent + /// directory does not exist yet — the expected first-run shape of + /// .../launcher/sessions/<id>/status.jsonl on a fresh cache + /// dir — must be created lazily rather than throwing + /// out of the transaction the + /// writer is merely observing. + /// + [Fact] + public void MissingParentDirectoryIsCreatedAndEventsFlow() + { + string root = Path.Combine( + Path.GetTempPath(), + $"acdream-status-root-{Guid.NewGuid():N}"); + string path = Path.Combine(root, "nested", "sessions", "s1", "status.jsonl"); + try + { + Assert.False(Directory.Exists(Path.GetDirectoryName(path))); + var writer = new SessionStatusWriter(path); + + writer.Started("s1"); + writer.Connected("s1"); + + Assert.True(writer.IsEnabled); + string[] lines = File.ReadAllLines(path); + Assert.Equal(2, lines.Length); + Assert.Contains("\"started\"", lines[0]); + Assert.Contains("\"connected\"", lines[1]); + } + finally + { + if (Directory.Exists(root)) + Directory.Delete(root, recursive: true); + } + } + + /// + /// F1: a path whose PARENT SEGMENT already exists as an ordinary file + /// (so cannot turn it into a + /// directory) is exactly the "unwritable path" case the review asked + /// for — the writer must latch itself off instead of throwing, and every + /// subsequent call must stay a cheap no-op. + /// + [Fact] + public void ParentSegmentIsAFileLatchesTheWriterInsteadOfThrowing() + { + string blocker = Path.Combine( + Path.GetTempPath(), + $"acdream-status-blocker-{Guid.NewGuid():N}"); + File.WriteAllText(blocker, "not a directory"); + string path = Path.Combine(blocker, "status.jsonl"); + try + { + var writer = new SessionStatusWriter(path); + Assert.True(writer.IsEnabled); + + // Must not throw — the writer swallows its own I/O failure and + // latches off instead of failing the caller's transaction. + writer.Started("s1"); + Assert.False(writer.IsEnabled); + + // Latched-off calls stay cheap no-ops — no exception, no retry. + writer.Connected("s1"); + writer.Exited("s1", 0, "disposed"); + } + finally + { + if (File.Exists(blocker)) + File.Delete(blocker); + } } [Fact] diff --git a/tests/Fixtures/campaign-la/session-config-shared-fixture.json b/tests/Fixtures/campaign-la/session-config-shared-fixture.json index 822921a6..24ec55a1 100644 --- a/tests/Fixtures/campaign-la/session-config-shared-fixture.json +++ b/tests/Fixtures/campaign-la/session-config-shared-fixture.json @@ -1,5 +1,11 @@ { "version": 1, + "process": { + "content": { + "datDirectory": "shared-fixture-dats", + "preparedAssetPath": "shared-fixture-dats/acdream.pak" + } + }, "sessions": [ { "id": "shared-fixture", @@ -8,8 +14,8 @@ "character": { "name": "SharedToon" }, "policy": { "id": "idle" }, "credential": { - "provider": "environment", - "reference": "SHARED_FIXTURE_PASSWORD" + "provider": "standardInput", + "reference": "session" }, "plugins": ["ExamplePlugin", "AnotherPlugin"], "loginCommands": ["/tell someone, hi", "/vt start"],