Opus review of LA3 returned FIX FIRST; this addresses every finding in
scope (F1-F5, F7-F12; F6 CI-lane addition excluded per instructions):
- F1 (CRITICAL): SessionProcessSettings.Paths is now nullable and left
null by SessionConfigComposer unless a caller supplies overrides, so
the JSON key is entirely absent instead of "paths":{} — the App-side
loader's strict UnmappedMemberHandling.Disallow would otherwise reject
every gui/guiSelect session-config document at load.
- F2: added SessionConfigComposer.ComposeProbe and a nullable
SessionDescriptor.Mode field ("probe", omitted for normal play) per
the pinned contract — no character/policy/plugins/loginCommands.
- F3: LauncherProcessSupervisor.Stop now tries
ILauncherChildProcess.TryRequestGracefulStop (Linux: libc SIGINT via
LibraryImport, K4-proven graceful headless logout) before
CloseMainWindow. Windows has no reliable no-window-console equivalent
today; filed docs/ISSUES.md #397 with the CREATE_NEW_PROCESS_GROUP +
CTRL_BREAK fix direction. Stop()'s blocking-timeout contract is now
documented for LA4.
- F4: LauncherProfileStore.Save chmods the Linux temp file to 0600
immediately after creation, before any credential is serialized;
failure paths and Load() clean up a stale .tmp.
- F5: added LauncherCoreDependencyBoundaryTests asserting Launcher.Core
references exactly AcDream.Platform and no packages.
- F7: StatusEventParser.Parse no longer throws on a whitespace/null
line; StatusFileTailer.ReadNewEvents swallows the File.Exists/open
TOCTOU window (FileNotFoundException/DirectoryNotFoundException/
IOException) instead of throwing.
- F8: Start() now kills (entire process tree) and disposes a child that
started successfully but failed while being fed its stdin password,
instead of orphaning it.
- F9: SetState is monotonic — once Exited, no later transition applies
or fires StateChanged, closing a Start()-path race where a
synchronously-exiting child could be "resurrected" to Running.
- F10: CharacterIdFormat.TryParse now requires the "0x" prefix (an
unprefixed hand-typed decimal id is also valid hex and was silently
misread); a parsed id of 0 is treated as unusable and falls back to
the name selector; LauncherProfileStore.MergeRoster normalizes both
sides through TryParse/ToHexString instead of raw string equality, so
a legacy unprefixed-hex row self-heals via name match instead of
duplicating.
- F11: StatusCharacterEntry.SecondsGreyedOut is now uint, matching
CharacterRosterEntry and the host writer.
- F12: added MalformedStatusEvent, returned for a recognized `e` whose
payload doesn't match its shape, distinguished from UnknownStatusEvent
(an unrecognized `e`).
AllowUnsafeBlocks was added to AcDream.Launcher.Core.csproj — required
by the LibraryImport source generator's function-pointer marshalling
stub for F3's Linux SIGINT P/Invoke.
Verification: dotnet build AcDream.slnx -c Release green (0 errors);
dotnet test tests/AcDream.Launcher.Core.Tests -c Release green at 94/94
on native Windows and under WSL (Ubuntu, verified across multiple runs
for the timing-sensitive SIGINT/sharing-violation tests, no flakes
observed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
172 lines
6.4 KiB
C#
172 lines
6.4 KiB
C#
using AcDream.Launcher.Core.Profiles;
|
|
|
|
namespace AcDream.Launcher.Core.Tests.Profiles;
|
|
|
|
/// <summary>
|
|
/// The roster-merge matrix (Campaign LA plan §LA3 acceptance): a new
|
|
/// character, an existing character keeping its user settings, and a
|
|
/// character absent from a later roster snapshot being retained
|
|
/// (possibly pending-delete).
|
|
/// </summary>
|
|
public sealed class RosterMergeTests
|
|
{
|
|
private static LauncherProfileStore NewStoreWithServerAndAccount()
|
|
{
|
|
var store = new LauncherProfileStore(
|
|
Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".json"));
|
|
store.Load();
|
|
store.AddServer("Local ACE", "127.0.0.1", 9000);
|
|
store.AddAccount("Local ACE", "testaccount", "testpassword");
|
|
return store;
|
|
}
|
|
|
|
[Fact]
|
|
public void FirstMergeAddsNewCharactersWithDefaultLaunchModeGuiSelect()
|
|
{
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[
|
|
new CharacterRosterEntry(0x5000000A, "+Acdream", 0),
|
|
new CharacterRosterEntry(0x5000000B, "+Second", 0),
|
|
]);
|
|
|
|
List<CharacterProfile> characters =
|
|
store.Document.Servers.Single().Accounts.Single().Characters;
|
|
Assert.Equal(2, characters.Count);
|
|
|
|
CharacterProfile first = characters.Single(c => c.Name == "+Acdream");
|
|
Assert.Equal("0x5000000A", first.Id);
|
|
Assert.Equal(LaunchMode.GuiSelect, first.LaunchMode);
|
|
Assert.Empty(first.Plugins);
|
|
Assert.Empty(first.LoginCommands);
|
|
|
|
CharacterProfile second = characters.Single(c => c.Name == "+Second");
|
|
Assert.Equal("0x5000000B", second.Id);
|
|
Assert.Equal(LaunchMode.GuiSelect, second.LaunchMode);
|
|
}
|
|
|
|
[Fact]
|
|
public void SecondMergePreservesUserSettingsOnAnExistingCharacter()
|
|
{
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+Acdream", 0)]);
|
|
store.EditCharacter(
|
|
"Local ACE",
|
|
"testaccount",
|
|
"+Acdream",
|
|
launchMode: LaunchMode.Headless,
|
|
plugins: ["ExamplePlugin"],
|
|
loginCommands: ["/vt start"]);
|
|
|
|
// A later probe reports the same character again (same id), with
|
|
// a renamed display — settings must survive untouched.
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+Acdream", 0)]);
|
|
|
|
CharacterProfile character = Assert.Single(
|
|
store.Document.Servers.Single().Accounts.Single().Characters);
|
|
Assert.Equal(LaunchMode.Headless, character.LaunchMode);
|
|
Assert.Equal(["ExamplePlugin"], character.Plugins);
|
|
Assert.Equal(["/vt start"], character.LoginCommands);
|
|
}
|
|
|
|
[Fact]
|
|
public void MergeUpdatesNameWhenIdMatchesButDisplayNameChanged()
|
|
{
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+OldName", 0)]);
|
|
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+NewName", 0)]);
|
|
|
|
CharacterProfile character = Assert.Single(
|
|
store.Document.Servers.Single().Accounts.Single().Characters);
|
|
Assert.Equal("+NewName", character.Name);
|
|
Assert.Equal("0x5000000A", character.Id);
|
|
}
|
|
|
|
[Fact]
|
|
public void CharacterAbsentFromALaterRosterSnapshotIsRetained()
|
|
{
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[
|
|
new CharacterRosterEntry(0x5000000A, "+Acdream", 0),
|
|
new CharacterRosterEntry(0x5000000B, "+PendingDelete", 1),
|
|
]);
|
|
|
|
// A later probe's roster only reports one of the two — e.g. the
|
|
// other was deleted and is now in ACE's grace window / a
|
|
// partial snapshot. The store never removes rows on the
|
|
// caller's behalf.
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+Acdream", 0)]);
|
|
|
|
List<CharacterProfile> characters =
|
|
store.Document.Servers.Single().Accounts.Single().Characters;
|
|
Assert.Equal(2, characters.Count);
|
|
Assert.Contains(characters, c => c.Name == "+Acdream");
|
|
Assert.Contains(characters, c => c.Name == "+PendingDelete");
|
|
}
|
|
|
|
[Fact]
|
|
public void MergeNormalizesAnUnprefixedHexIdInsteadOfCreatingADuplicateRow()
|
|
{
|
|
// Review finding F10: a hand-edited row can carry an id without
|
|
// the "0x" prefix (e.g. copy-pasted from somewhere that dropped
|
|
// it). CharacterIdFormat.TryParse now REJECTS that string
|
|
// outright (it no longer guesses hex-without-a-prefix), so the
|
|
// old raw string-equality comparison against the roster's
|
|
// canonical "0x..." form would never match and would add a
|
|
// second row forever. The name-fallback match must still
|
|
// recognize this as the SAME character and self-heal its id.
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
store.Document.Servers.Single().Accounts.Single().Characters.Add(
|
|
new CharacterProfile { Id = "5000000A", Name = "+Acdream" });
|
|
|
|
store.MergeRoster(
|
|
"Local ACE",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(0x5000000A, "+Acdream", 0)]);
|
|
|
|
CharacterProfile character = Assert.Single(
|
|
store.Document.Servers.Single().Accounts.Single().Characters);
|
|
Assert.Equal("0x5000000A", character.Id);
|
|
Assert.Equal("+Acdream", character.Name);
|
|
}
|
|
|
|
[Fact]
|
|
public void MergeThrowsForUnknownServerOrAccount()
|
|
{
|
|
LauncherProfileStore store = NewStoreWithServerAndAccount();
|
|
|
|
Assert.Throws<LauncherProfileException>(
|
|
() => store.MergeRoster(
|
|
"Nope",
|
|
"testaccount",
|
|
[new CharacterRosterEntry(1, "x", 0)]));
|
|
|
|
Assert.Throws<LauncherProfileException>(
|
|
() => store.MergeRoster(
|
|
"Local ACE",
|
|
"nope",
|
|
[new CharacterRosterEntry(1, "x", 0)]));
|
|
}
|
|
}
|