From 05f23918f6d6e14eb92e5c2d3bb3afd6b5629479 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 10:07:16 -0500 Subject: [PATCH 1/3] Give each session its own spawn window, and carry the old ones across MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two connected characters whose triggers matched the same pattern shared one capture pane. `Workspace.SpawnWindowId` was `$"spawn:{target}"` — no owner in it — so the first session to match created the window *with its own session key on it* and every later session's lines were appended to a pane somebody else owned. That is worse than a mixed-up channel: the rail draws window rows for the active character only, so the second character's own channel was filed under the first and was invisible from the character it belonged to. The id now carries the owner, length-prefixed: `spawn:{owner.Length}:{owner}:{target}`, or `spawn:-:{target}` for a window nobody owns. The length is what makes it unambiguous rather than merely usually fine — a session key and a target are both user-controlled free text and either may hold a colon, so `spawn:{owner}:{target}` spells ("a", "b:c") and ("a:b", "c") identically, which is this very defect one shape rarer. Reading the digits, taking exactly that many characters and consuming one more colon is total and reversible, so distinct pairs cannot collide whatever is in them. It is an encoding and not a digest because this string is a dictionary key, a value in config.json and the stem of a RestoreLog file name, and all three want to stay readable. Windows with no owner keep sharing one bucket per target, deliberately: a live WorldSession always has a key (its world's name when it has no character), so an unowned spawn window cannot come from a connection at all, and the rail lists an unowned window under *every* character — two of them for one target would draw two identical rows everywhere with nothing able to tell them apart. Null is one identity, "nobody", not an unknown. Nothing on screen changes. Every surface that names a window reads `WorkspaceWindow.Title`; the one place that derived a display name from an id was a test helper, now asking the app for the title instead. Migration, both halves, because the two stores are joined only by this id: - `ConfigurationMigrator` v4 → v5 rewrites every saved spawn id in `lastSession.windows` and in every pane's `tabs`, taking the owner from the `sessionKey` that state has always persisted. Migrated rather than adopted or dropped: adoption leaves a pane no id the client produces can ever reach, and dropping throws away a pane the user had. The decision is made by schema version and never by looking at the id, because a target is free text and can be made to look like anything. - `SharpMUTermApp.CarryLegacySpawnLogsOver` moves the restore log's content onto the id its pane now has, at the launch that reads it, and drops the old file — so it happens once and cannot replay the same lines twice. A log no pane claims is left exactly where it was, as it always has been. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- .../Configuration/AppConfiguration.cs | 2 +- .../Configuration/ConfigurationMigrator.cs | 105 ++++++ src/SharpMUTerm.Core/Workspace/Workspace.cs | 106 +++++- src/SharpMUTerm.Tui/DemoScene.cs | 9 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 132 +++++++- .../Configuration/ConfigurationTests.cs | 3 +- .../SpawnWindowIdMigrationTests.cs | 229 +++++++++++++ .../Workspace/SpawnWindowIdTests.cs | 183 ++++++++++ .../Workspace/WorkspaceStateTests.cs | 9 +- .../Workspace/WorkspaceTests.cs | 4 +- .../FocusIndicationTests.cs | 2 +- .../HistorySearchEndToEndTests.cs | 6 +- .../InputAreaEndToEndTests.cs | 4 +- .../NawsPaneReportTests.cs | 2 +- .../PanePrefixEndToEndTests.cs | 4 +- .../PaneResizeEndToEndTests.cs | 2 +- .../PaneTabCloseTests.cs | 6 +- .../PrefixWhichKeyTests.cs | 2 +- tests/SharpMUTerm.Tui.Tests/RailClickTests.cs | 6 +- .../RailWindowRowTests.cs | 2 +- .../RestoreLogEndToEndTests.cs | 14 +- .../SessionStateAccountingTests.cs | 23 +- .../SpawnWindowIdUpgradeTests.cs | 279 ++++++++++++++++ .../SpawnWindowPerSessionTests.cs | 313 ++++++++++++++++++ 24 files changed, 1393 insertions(+), 54 deletions(-) create mode 100644 tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs create mode 100644 tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs diff --git a/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs b/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs index d83bcad..a93d1e3 100644 --- a/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs +++ b/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs @@ -8,7 +8,7 @@ namespace SharpMUTerm.Core.Configuration; public sealed class AppConfiguration { /// The current on-disk schema version. Older configs are upgraded by . - public const int CurrentVersion = 4; + public const int CurrentVersion = 5; /// Schema version, for future migrations. public int Version { get; set; } = CurrentVersion; diff --git a/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs b/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs index a166785..10da3ec 100644 --- a/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs +++ b/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs @@ -33,9 +33,114 @@ public static void Migrate(JsonObject root) MigrateV3ToV4(root); } + if (version < 5) + { + MigrateV4ToV5(root); + } + root["version"] = AppConfiguration.CurrentVersion; } + /// + /// v4's spawn window ids named only their target (spawn:Public), so a workspace held one + /// capture pane per target however many characters were capturing into it. v5 puts the owning + /// session in the id (), so two + /// connected characters running the same rule get a pane each. Every saved id is rewritten here — + /// in lastSession.windows and in every pane's tabs array that referenced it. + /// + /// It is a migration and not an adoption, and the reason is that a resumed workspace must not + /// come back holding a pane nothing writes to. Left alone, a saved spawn:Public would + /// match no id the running client can now produce: the pane would sit there for ever, empty, while + /// its channel filled a second pane beside it. Dropping it would have been the other way to avoid + /// that and it throws away a pane the user had, plus — through the window id the restore log is + /// keyed by — the scrollback in it. Rewriting keeps both: the pane stays where it was, and its log + /// file is carried across at startup by SharpMUTermApp.RestorePreviousSession. + /// + /// + /// What supplies the owner is the state itself. WorkspaceWindowState.SessionKey has + /// been persisted all along, so an old file already records which character each spawn window + /// belonged to and the rewrite is lossless rather than a guess. A window that recorded no owner + /// keeps that: it becomes the unowned form, which is exactly what the client produces for a spawn + /// window nobody owns, so it too stays reachable. + /// + /// + /// The decision is made by version, never by looking at the id. A target is free text and may + /// hold anything — including something that reads like a v5 id — so classifying ids by shape would + /// eventually mis-file one and produce the orphan pane this step exists to prevent. Everything under + /// a document that says version 4 is v4, by construction. + /// + /// + private static void MigrateV4ToV5(JsonObject root) + { + if (root["lastSession"] is not JsonObject session || session["windows"] is not JsonArray windows) + { + return; + } + + var rewritten = new Dictionary(StringComparer.Ordinal); + foreach (var window in windows.OfType()) + { + // Kind is written by the string enum converter, so it is the member name and not a number. + if (!string.Equals(Text(window["kind"]), nameof(Workspaces.WindowKind.Spawn), + StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (Text(window["id"]) is not { } id + || !id.StartsWith(Workspaces.Workspace.SpawnPrefix, StringComparison.Ordinal)) + { + continue; + } + + var target = id[Workspaces.Workspace.SpawnPrefix.Length..]; + if (target.Length == 0) + { + continue; + } + + var upgraded = Workspaces.Workspace.SpawnWindowId(Text(window["sessionKey"]), target); + window["id"] = upgraded; + rewritten[id] = upgraded; + } + + if (rewritten.Count > 0 && session["root"] is JsonObject layout) + { + RewriteTabs(layout, rewritten); + } + } + + /// Re-points every pane's tab list at the ids rewrote. + private static void RewriteTabs(JsonObject node, Dictionary rewritten) + { + if (node["tabs"] is JsonArray tabs) + { + for (var i = 0; i < tabs.Count; i++) + { + if (Text(tabs[i]) is { } tab && rewritten.TryGetValue(tab, out var upgraded)) + { + tabs[i] = upgraded; + } + } + } + + if (node["children"] is JsonArray children) + { + foreach (var child in children.OfType()) + { + RewriteTabs(child, rewritten); + } + } + } + + /// + /// A node's string value, or null when it is absent or is not a string. A hand-edited or + /// hand-truncated document must not stop the client starting, and GetValue<string> + /// throws on a number where this returns null. + /// + private static string? Text(JsonNode? node) => + node is JsonValue value && value.TryGetValue(out var text) ? text : null; + /// /// v3's autoLogin is gone: whether a character logs itself in is now derived from whether it /// has anything to send (, ). The diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index ac63129..dbcd900 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -1,3 +1,5 @@ +using System.Globalization; + namespace SharpMUTerm.Core.Workspaces; /// @@ -68,14 +70,23 @@ public WorkspaceWindow OpenWindow( } /// - /// Routes trigger-spawned output to a spawn window named , creating and - /// placing the window on first use, and counts the line as unread unless the window is currently - /// visible. Returns the destination window. + /// Routes trigger-spawned output to 's spawn window named + /// , creating and placing the window on first use, and counts the line as + /// unread unless the window is currently visible. Returns the destination window. + /// + /// The destination is per session, not per workspace. Two connected characters running the + /// same capture rule each get a window of their own; the id carries the owner, so the second + /// session to match cannot land in the first's window. It used to: the id was the target alone, so + /// whoever matched first created the window with their own session key on it and everybody + /// else's lines were appended to somebody else's pane. That was not merely a mixed-up channel — the + /// rail draws window rows for the active character only, so the second character's own channel was + /// filed under the first and was invisible from the character it belonged to. + /// /// public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) { ArgumentException.ThrowIfNullOrEmpty(target); - var id = SpawnWindowId(target); + var id = SpawnWindowId(sessionKey, target); if (!_windows.TryGetValue(id, out var window)) { window = new WorkspaceWindow(id, target, WindowKind.Spawn, sessionKey); @@ -87,8 +98,91 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) return window; } - /// The window id a spawn routes to. - public static string SpawnWindowId(string target) => $"spawn:{target}"; + /// Every spawn window id starts with this. + public const string SpawnPrefix = "spawn:"; + + /// + /// The owner field of a spawn window that belongs to nobody. A single -, which is not a + /// decimal length and so can never be mistaken for one — that is the whole reason it is not the + /// empty string. + /// + private const string Unowned = "-"; + + /// + /// The window id the spawn of the session + /// routes to. Unique per (owner, target) and stable for ever, so a reconnect or a restart + /// comes back to the pane it left. + /// + /// Why the length prefix. A session key and a target are both user-controlled strings that may + /// hold any character, colons included — a world or character can be called a:b and a + /// trigger's SpawnTarget is free text. Joining them with a separator is therefore not + /// injective: (a, b:c) and (a:b, c) would produce one id and collapse two characters' + /// panes into one, which is this defect again in a rarer shape. Writing the owner's length in front + /// of it makes the encoding total and reversible: the digits up to the first colon give the length, + /// exactly that many characters are the owner, one more colon is consumed, and everything left is + /// the target — so distinct pairs cannot produce equal ids, whatever is in them. + /// + /// + /// It is legible on purpose rather than hashed. This id is a dictionary key, a value in + /// config.json, and the stem of a RestoreLog file name; a digest would be unambiguous + /// too and would make every one of those unreadable to whoever has to look at them, for no property + /// a reversible encoding does not already have. (The file name's own collision handling is + /// unchanged and unaffected: RestoreLog stores the full id in each file's header and refuses + /// a file whose header names a different window, so a CRC-32 clash on the stem costs one window's + /// log rather than mixing two.) + /// + /// + /// The owning world.character session, or null for a window nobody owns. + /// The capture target, which is also the window's title. + public static string SpawnWindowId(string? sessionKey, string target) + { + ArgumentException.ThrowIfNullOrEmpty(target); + return sessionKey is null + ? $"{SpawnPrefix}{Unowned}:{target}" + : $"{SpawnPrefix}{sessionKey.Length}:{sessionKey}:{target}"; + } + + /// + /// Reads a spawn window id back into the pair that made it. False when is not + /// one this build writes — including a spawn id from before the owner was in it, which is what + /// ConfigurationMigrator's v4→v5 step upgrades. + /// + public static bool TryReadSpawnWindowId(string id, out string? sessionKey, out string target) + { + sessionKey = null; + target = string.Empty; + if (id is null || !id.StartsWith(SpawnPrefix, StringComparison.Ordinal)) + { + return false; + } + + var rest = id.AsSpan(SpawnPrefix.Length); + if (rest.StartsWith(Unowned + ":", StringComparison.Ordinal)) + { + target = rest[(Unowned.Length + 1)..].ToString(); + return target.Length > 0; + } + + // NumberStyles.None: bare digits only, so a sign or surrounding space is not silently accepted + // into a field whose whole job is to say how many characters to take. + var separator = rest.IndexOf(':'); + if (separator <= 0 + || !int.TryParse(rest[..separator], NumberStyles.None, CultureInfo.InvariantCulture, out var length)) + { + return false; + } + + // The owner's exact length, then the colon that closes it: anything shorter is not this encoding. + var owner = rest[(separator + 1)..]; + if (owner.Length < length + 1 || owner[length] != ':') + { + return false; + } + + sessionKey = owner[..length].ToString(); + target = owner[(length + 1)..].ToString(); + return target.Length > 0; + } /// /// Records a line arriving in a window: increments its unread badge unless the window is diff --git a/src/SharpMUTerm.Tui/DemoScene.cs b/src/SharpMUTerm.Tui/DemoScene.cs index 5781ba3..2585b5b 100644 --- a/src/SharpMUTerm.Tui/DemoScene.cs +++ b/src/SharpMUTerm.Tui/DemoScene.cs @@ -19,6 +19,13 @@ internal static class DemoScene /// The world.character the demo resumes as focused/connected. public const string ActiveSessionKey = "Aetherfall.Corvid"; + /// + /// The demo's Chat spawn window. Spelt once, here, because a spawn window's id names its + /// owner as well as its target — the saved workspace, the snapshot's spawn view and + /// the scene's own backlog all have to mean the same window, and three hand-built ids would not. + /// + public static string ChatWindowId => Workspace.SpawnWindowId(ActiveSessionKey, "Chat"); + public static AppConfiguration Build() { var config = new AppConfiguration(); @@ -243,7 +250,7 @@ private static void AddTriggerSets(AppConfiguration config) /// private static WorkspaceState BuildLastSession() { - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = ChatWindowId; return new WorkspaceState { Windows = diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 8acd555..1c9691d 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -706,7 +706,7 @@ public string RenderSnapshot(string? view = null) // Activate the Chat spawn window so its dim "⇱ capture …" header renders under the tab strip. if (string.Equals(view, "spawn", StringComparison.OrdinalIgnoreCase)) { - _workspace.ActivateWindow(Workspace.SpawnWindowId("Chat")); + _workspace.ActivateWindow(DemoScene.ChatWindowId); RebuildPaneArea(); } @@ -1205,7 +1205,7 @@ void Feed(string windowId, string ansiLine) // The Chat spawn window already exists (opened by the resumed session); feed its backlog and // leave it in the background with unread, as if lines arrived while another tab was focused. - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = DemoScene.ChatWindowId; PaneContentFor(chatId, "Chat"); var chatParser = new AnsiParser(); foreach (var text in new[] @@ -1316,14 +1316,31 @@ private void RestorePreviousSession() var restoredWindows = 0; var restoredLines = 0; - foreach (var window in _restore.Read()) - { + var logged = _restore.Read(); + var carriedOver = CarryLegacySpawnLogsOver(logged); + foreach (var window in logged) + { + // A log written before spawn window ids carried their owner has been re-filed under the id + // its pane now has; its own file is already gone, and replaying it under the old id would + // buffer the previous session's channel where nothing can ever see it. A null replacement + // is one whose pane already had a log of its own, so there is nothing left to replay. + var windowId = window.WindowId; + if (carriedOver.TryGetValue(window.WindowId, out var carriedTo)) + { + if (carriedTo is null) + { + continue; + } + + windowId = carriedTo; + } + // A character who opted out gets their content dropped rather than merely un-drawn: an // opt-out that left the last session's text lying in the config directory would be // answering a different question from the one it was asked. - if (!RestoreLogWanted(_workspace.FindWindow(window.WindowId)?.SessionKey)) + if (!RestoreLogWanted(_workspace.FindWindow(windowId)?.SessionKey)) { - _restore.Forget(window.WindowId); + _restore.Forget(windowId); continue; } @@ -1334,13 +1351,13 @@ private void RestorePreviousSession() foreach (var line in window.Lines) { - AppendWindowLine(window.WindowId, _formatter.ToMarkup(line.Line), line.Stamp); + AppendWindowLine(windowId, _formatter.ToMarkup(line.Line), line.Stamp); } // The boundary marker carries no stamp: it did not arrive, it was drawn, and a timestamp // gutter beside it would be claiming a time for a row the game never sent. AppendWindowLine( - window.WindowId, + windowId, RestoreBarRenderer.Bar(window.Lines.Count, window.LastWritten, FrozenAccentHex())); restoredWindows++; @@ -1358,6 +1375,93 @@ private void RestorePreviousSession() restoredWindows); } + /// + /// Moves any restore log written before a spawn window id carried its owner onto the id that pane + /// has now, and hands back the old-id → new-id map the replay reads. + /// + /// This is the content half of ConfigurationMigrator's v4→v5 step, and without it that + /// step would lose the user's scrollback. The log is keyed by window id and the saved workspace + /// has just had its spawn ids rewritten, so the file holding the previous session's Public + /// pane is now filed under an id nothing refers to: the pane would come back in the right place and + /// empty. It is done by copying the lines onto the new id and dropping the old file rather than by + /// remembering a mapping for ever — after this launch there is nothing left to map, so the next + /// launch does no work and cannot replay the same content twice. + /// + /// + /// What counts as an old id is decided by shape here, and that is safe in a way it would not be + /// in the configuration: an id is old only if it does not parse as a current one + /// () and names no window this workspace holds + /// and exactly one live spawn window claims its target. Anything short of all three is left + /// alone, which costs nothing — a log the workspace cannot place is buffered under its own id + /// exactly as it has always been, so its pane refills if that channel ever speaks again. + /// + /// + /// + /// Old id → the id to replay it under, or where the old file was dropped and + /// there is nothing left to replay. Ids absent from the map are not old and are replayed as they are. + /// + private Dictionary CarryLegacySpawnLogsOver(IReadOnlyList logged) + { + var carried = new Dictionary(StringComparer.Ordinal); + if (_restore is null) + { + return carried; + } + + // Which live spawn window would have been written under which pre-v5 id. A target claimed by + // more than one window is ambiguous and is left alone rather than guessed at. + var claims = new Dictionary(StringComparer.Ordinal); + foreach (var window in _workspace.Windows.Where(w => w.Kind == WindowKind.Spawn)) + { + if (!Workspace.TryReadSpawnWindowId(window.Id, out _, out var target)) + { + continue; + } + + var legacy = Workspace.SpawnPrefix + target; + claims[legacy] = claims.ContainsKey(legacy) ? null : window.Id; + } + + var known = logged.Select(w => w.WindowId).ToHashSet(StringComparer.Ordinal); + foreach (var window in logged) + { + if (!window.WindowId.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal) + || Workspace.TryReadSpawnWindowId(window.WindowId, out _, out _) + || _workspace.FindWindow(window.WindowId) is not null + || !claims.TryGetValue(window.WindowId, out var replacement) + || replacement is null) + { + continue; + } + + // A log already standing under the new id means an earlier launch did this and was + // interrupted before it could drop the old file. Take the new one and drop the old, which + // loses nothing that is not already there and cannot double a pane's history. + carried[window.WindowId] = null; + if (!known.Contains(replacement) && RestoreLogWanted(_workspace.FindWindow(replacement)?.SessionKey)) + { + var title = _workspace.FindWindow(replacement)?.Title ?? string.Empty; + foreach (var line in window.Lines) + { + _restore.Append(replacement, title, line.Line, line.Stamp); + } + + carried[window.WindowId] = replacement; + } + + _restore.Forget(window.WindowId); + } + + var moved = carried.Count(entry => entry.Value is not null); + if (moved > 0) + { + _diagnostics.Logger.LogInformation( + "Carried {Count} pane(s) of restored content onto the per-session spawn window ids", moved); + } + + return carried; + } + /// /// Whether a window owned by takes part in the restore log. An /// unowned window — the main window before any session adopts it, the web view — is allowed: no @@ -2143,10 +2247,18 @@ private void OnLine(WorldSession session, string windowId, StyledLine line) /// cosmetic: the rail lists a character's windows by it, and resolves /// which world a link clicked in a spawn window sends to by it. /// + /// + /// The same session key also picks the window (), + /// which is what gives two characters running one capture rule a pane each. While the id was the + /// target alone there was one window per workspace: the first session to match created it with its + /// own key on it and every other session's lines were appended to a pane somebody else owned — + /// invisible from the character whose channel it was, because the rail lists window rows for the + /// active character only. + /// /// private void OnSpawnLine(WorldSession session, string target, string pattern, StyledLine line) { - var existed = _workspace.FindWindow(Workspace.SpawnWindowId(target)) is not null; + var existed = _workspace.FindWindow(Workspace.SpawnWindowId(session.SessionKey, target)) is not null; var window = _workspace.RouteSpawn(target, session.SessionKey); // Label the pane with the rule that feeds it. The pattern comes with the line rather than being @@ -4361,7 +4473,7 @@ private IReadOnlyList BuildRail() /// windows, plus the ones that belong to nobody. /// /// The owner test is the point. Every window a session or a trigger opens carries its owner - /// (, and RouteSpawn(target, _active?.SessionKey)), so a + /// (, and RouteSpawn(target, session.SessionKey)), so a /// window with no owner is an auxiliary like the web view — global, reachable from wherever you /// are, and listed here because the rail is the only place you can click back to it. Without the /// test this returned every window in the workspace, so the rail drew one character's tabs diff --git a/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs b/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs index 7091eef..31a6cf1 100644 --- a/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs @@ -278,7 +278,8 @@ public async Task LastSession_RoundTripsThroughTheStore() await Assert.That(restored.LastSession).IsNotNull(); var workspace = restored.LastSession!.Restore(); - await Assert.That(workspace.FindWindow(Workspace.SpawnWindowId("Chat"))!.OwnerLabel).IsEqualTo("Corvid"); + await Assert.That(workspace.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.OwnerLabel) + .IsEqualTo("Corvid"); } [Test] diff --git a/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs new file mode 100644 index 0000000..8dd5704 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs @@ -0,0 +1,229 @@ +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Workspaces; + +namespace SharpMUTerm.Core.Tests.Configuration; + +/// +/// The v4 → v5 upgrade: spawn window ids gain their owner, so two characters capturing one target stop +/// sharing a pane. The whole risk of the change is on this seam — every existing installation's +/// config.json holds spawn:Public rows, and both the pane placement in lastSession +/// and the scrollback in the restore log are keyed by those exact strings. +/// +/// The decision, and why it is the one to make. An old id is migrated, not adopted and +/// not dropped. Adoption — leaving the id alone and letting whichever session claims the target keep it +/// — cannot work, because nothing the running client now produces equals spawn:Public: the pane +/// would come back and then sit there for ever with nothing writing to it while its channel filled a +/// second pane beside it. Dropping is honest and throws away a pane the user had and, through the id +/// the restore log is keyed by, the text in it. Migrating keeps the pane, its place in the split tree +/// and its content, and produces exactly the id the session that owned it will route to. +/// +/// +/// What supplies the owner is the saved state itself. WorkspaceWindowState.SessionKey has +/// been persisted since spawn windows existed, so an old document already records which character each +/// pane belonged to; the rewrite is a lookup and not a guess. +/// +/// +public class SpawnWindowIdMigrationTests +{ + /// A v4 document: one character, a main window and a Chat spawn beside it in one pane. + private const string V4 = """ + { + "version": 4, + "worlds": [ { "name": "Aetherfall", "host": "aetherfall.mux", "port": 4201, + "characters": [ { "name": "Corvid" } ] } ], + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid", + "ownerLabel": "Corvid", "capturePattern": "^\\[Chat\\]" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """; + + /// The id the running client will route Corvid's Chat capture to. + private static string CorvidsChat => Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"); + + /// + /// The headline: the saved pane keeps everything it had, under the id the session that owns it now + /// produces. Placement included — the rewrite has to reach the pane's tabs array as well as + /// the window row, or the workspace comes back with a window in no pane and a pane naming no window. + /// + [Test] + public async Task AV4SpawnWindowKeepsItsPaneItsPlaceAndItsMetadata() + { + var config = ConfigurationStore.Deserialize(V4); + + await Assert.That(config.Version).IsEqualTo(AppConfiguration.CurrentVersion); + + var window = config.LastSession!.Windows.Single(w => w.Kind == WindowKind.Spawn); + await Assert.That(window.Id).IsEqualTo(CorvidsChat); + await Assert.That(window.Title).IsEqualTo("Chat"); + await Assert.That(window.SessionKey).IsEqualTo("Aetherfall.Corvid"); + await Assert.That(window.OwnerLabel).IsEqualTo("Corvid"); + await Assert.That(window.CapturePattern).IsEqualTo(@"^\[Chat\]"); + + await Assert.That(config.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + } + + /// + /// The property the migration exists for. Every window the resumed workspace holds is one the + /// running client can route to: the id of each spawn pane is exactly what + /// produces for the owner and target that pane records. A pane + /// left under a v4 id would fail this — and that failure, on a live client, is a pane nobody ever + /// writes to again. + /// + [Test] + public async Task NoResumedPaneIsLeftWithAnIdNothingRoutesTo() + { + var workspace = ConfigurationStore.Deserialize(V4).LastSession!.Restore(); + + foreach (var window in workspace.Windows.Where(w => w.Kind == WindowKind.Spawn).ToList()) + { + var routed = workspace.RouteSpawn(window.Title, window.SessionKey); + await Assert.That(routed.Id).IsEqualTo(window.Id); + } + + // …and routing them did not open anything new beside them. + await Assert.That(workspace.Windows.Count).IsEqualTo(2); + await Assert.That(workspace.Layout.FindWindow(CorvidsChat)).IsNotNull(); + } + + /// + /// A v4 spawn window that recorded no owner becomes the unowned form, which is the id the client + /// produces for a spawn window nobody owns — so it stays reachable rather than becoming an orphan by + /// a different route. + /// + [Test] + public async Task AV4SpawnWindowWithNoOwnerBecomesTheUnownedId() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ { "id": "spawn:Notes", "title": "Notes", "kind": "Spawn" } ], + "root": { "type": "pane", "id": "p1", "tabs": [ "spawn:Notes" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); + + await Assert.That(config.LastSession!.Windows.Single().Id) + .IsEqualTo(Workspace.SpawnWindowId(null, "Notes")); + } + + /// The rewrite reaches panes nested in a split, not only the root one. + [Test] + public async Task TabsAreRewrittenInsideASplitToo() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid" } + ], + "root": { "type": "split", "direction": "Row", "sizes": [ 0.5, 0.5 ], "children": [ + { "type": "pane", "id": "p1", "tabs": [ "main" ], "activeIndex": 0 }, + { "type": "pane", "id": "p2", "tabs": [ "spawn:Chat" ], "activeIndex": 0 } + ] }, + "focusedPaneId": "p1" + } + } + """); + + var workspace = config.LastSession!.Restore(); + await Assert.That(workspace.Layout.FindWindow(CorvidsChat)!.Id).IsEqualTo("p2"); + } + + /// + /// Only spawn windows move. The main window and any auxiliary keep the ids they had, because those + /// were never derived from a target and nothing about them was ambiguous. + /// + [Test] + public async Task NonSpawnWindowsAreLeftAlone() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "web", "title": "Page", "kind": "Auxiliary" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "web" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); + + await Assert.That(config.LastSession!.Windows.Select(w => w.Id)).IsEquivalentTo(new[] { "main", "web" }); + await Assert.That(config.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", "web" }); + } + + /// + /// The step is idempotent in the way that matters: a document already at the current version is not + /// re-encoded. Running the rewrite twice would wrap one id inside another + /// (spawn:22:…:spawn:17:…:Chat) and lose the pane a second way, so this is the assertion that + /// keeps the version gate honest. + /// + [Test] + public async Task ADocumentAlreadyAtTheCurrentVersionIsNotRewrittenAgain() + { + var once = ConfigurationStore.Deserialize(V4); + var twice = ConfigurationStore.Deserialize(ConfigurationStore.Serialize(once)); + + await Assert.That(twice.LastSession!.Windows.Single(w => w.Kind == WindowKind.Spawn).Id) + .IsEqualTo(CorvidsChat); + await Assert.That(twice.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + } + + /// + /// A v4 document with no saved session at all — the overwhelmingly common shape of an old file — is + /// untouched apart from its version, and starting from it does not throw. + /// + [Test] + public async Task AV4DocumentWithNoSavedSessionIsFine() + { + var config = ConfigurationStore.Deserialize(""" + { "version": 4, "worlds": [ { "name": "Aetherfall", "host": "aetherfall.mux" } ] } + """); + + await Assert.That(config.LastSession).IsNull(); + await Assert.That(config.Version).IsEqualTo(AppConfiguration.CurrentVersion); + } + + /// + /// And a hand-edited document does not make the migrator throw. It is asserted at that + /// level rather than through because this step runs + /// before deserialization: a number where a window id belongs, a missing kind, a tabs array + /// with a number in it — all things GetValue<string> throws on and this reads as + /// "not a string, leave it alone". + /// + [Test] + public async Task AMangledSavedSessionDoesNotStopTheMigrator() + { + var root = (System.Text.Json.Nodes.JsonObject)System.Text.Json.Nodes.JsonNode.Parse(""" + { + "version": 4, + "lastSession": { + "windows": [ { "id": 7, "kind": 1 }, { "id": "spawn:Chat" }, { "kind": "Spawn" }, + { "id": "spawn:Chat", "kind": "Spawn" } ], + "root": { "type": "pane", "id": "p1", "tabs": [ 3, "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """)!; + + ConfigurationMigrator.Migrate(root); + + await Assert.That(root["version"]!.GetValue()).IsEqualTo(AppConfiguration.CurrentVersion); + + // The one row that was a spawn window with a readable id did move, and the tab moved with it. + var tabs = (System.Text.Json.Nodes.JsonArray)root["lastSession"]!["root"]!["tabs"]!; + await Assert.That(tabs[1]!.GetValue()).IsEqualTo(Workspace.SpawnWindowId(null, "Chat")); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs new file mode 100644 index 0000000..6b9c03d --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs @@ -0,0 +1,183 @@ +using SharpMUTerm.Core.Workspaces; + +namespace SharpMUTerm.Core.Tests.Workspaces; + +/// +/// The identity a capture pane is filed under. It has to be unique per (owner, target) — that is +/// the whole of the fix for two characters sharing one Public window — and stable for ever, since +/// it keys the workspace registry, a value in config.json, and a RestoreLog file name. +/// +/// The pressure this suite is really under is that both halves are user-controlled free text: a +/// world or a character may be called anything, and a trigger's SpawnTarget is whatever was typed +/// on F2. So "unique" cannot mean "unique for the names people usually pick" — the encoding has to be +/// injective over every pair, which is what the length prefix buys and what a plain owner:target +/// join does not. +/// +/// +public class SpawnWindowIdTests +{ + /// The ordinary case reads as what it is, which is why this is an encoding and not a digest. + [Test] + public async Task AnIdNamesItsOwnerAndItsTarget() + { + await Assert.That(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat")) + .IsEqualTo("spawn:17:Aetherfall.Corvid:Chat"); + await Assert.That(Workspace.SpawnWindowId(null, "Chat")).IsEqualTo("spawn:-:Chat"); + } + + /// Two characters, one capture rule, two windows. The report, at the level it is caused. + [Test] + public async Task TwoOwnersOfOneTargetGetTwoIds() + { + await Assert.That(Workspace.SpawnWindowId("Convergence.Ann", "Public")) + .IsNotEqualTo(Workspace.SpawnWindowId("Convergence.Bob", "Public")); + } + + /// + /// The pair that a separator-joined id would collapse. ("a", "b:c") and + /// ("a:b", "c") are two different windows — a character called a capturing a channel + /// called b:c, and a character called a:b capturing one called c — and + /// $"spawn:{owner}:{target}" spells both spawn:a:b:c. That is this very defect in a + /// rarer shape, and it is why the owner's length is written in front of it. + /// + [Test] + public async Task ColonsInEitherHalfDoNotCollapseTwoWindowsIntoOne() + { + await Assert.That(Workspace.SpawnWindowId("a", "b:c")).IsNotEqualTo(Workspace.SpawnWindowId("a:b", "c")); + } + + /// + /// And the property behind those examples, over a spread of names chosen to be awkward: distinct + /// pairs give distinct ids, every time. A table of cases can only ever say "not these"; this says + /// "none of them", which is the claim the design actually rests on. + /// + [Test] + public async Task DistinctPairsAlwaysGiveDistinctIds() + { + string?[] owners = [null, "", "a", "a:b", "1", "12:x", "-", "-:x", "World.Char", "spawn:", ":", "::"]; + string[] targets = ["Chat", "a", "b:c", "c", ":", "spawn:Chat", "-:Chat", "17:Aetherfall.Corvid:Chat", "1"]; + + var seen = new Dictionary(StringComparer.Ordinal); + foreach (var owner in owners) + { + foreach (var target in targets) + { + var id = Workspace.SpawnWindowId(owner, target); + await Assert.That(seen.TryAdd(id, (owner, target))) + .IsTrue() + .Because($"({owner ?? "null"}, {target}) collided with {seen.GetValueOrDefault(id)} on {id}"); + } + } + } + + /// + /// The id is readable back, which is what lets the restore log carry a pre-owner file onto the pane + /// that now holds its channel. Over the same awkward spread, so the reader is exercised on the + /// inputs that would break a naive split. + /// + [Test] + public async Task AnIdReadsBackAsThePairThatMadeIt() + { + string?[] owners = [null, "", "a:b", "-", "17:Aetherfall.Corvid", "World.Char"]; + string[] targets = ["Chat", "b:c", ":", "-:Chat", "spawn:Chat"]; + + foreach (var owner in owners) + { + foreach (var target in targets) + { + var id = Workspace.SpawnWindowId(owner, target); + await Assert.That(Workspace.TryReadSpawnWindowId(id, out var readOwner, out var readTarget)).IsTrue(); + await Assert.That(readOwner).IsEqualTo(owner); + await Assert.That(readTarget).IsEqualTo(target); + } + } + } + + /// + /// A window id from before the owner was in it does not read as one of these, which is what + /// lets the restore log tell an old file apart from a current one without being told. (The + /// configuration is not left to work it out by shape — it goes by the document's schema version — + /// because a target is free text and may be made to look like anything, including this.) + /// + [Test] + [Arguments("spawn:Chat")] + [Arguments("spawn:Public")] + [Arguments("spawn:")] + [Arguments("main")] + [Arguments("spawn:99:short:x")] + [Arguments("spawn:+3:abc:x")] + [Arguments("spawn: 3:abc:x")] + public async Task WhatIsNotOneOfTheseIsRefused(string id) + { + await Assert.That(Workspace.TryReadSpawnWindowId(id, out _, out _)).IsFalse(); + } + + /// A spawn window has to be called something; an empty target is not an id. + [Test] + public async Task AnEmptyTargetIsRefused() + { + await Assert.That(() => Workspace.SpawnWindowId("Aetherfall.Corvid", string.Empty)) + .Throws(); + } + + /// + /// The workspace routes by that identity: one target, two sessions, two windows, each owned by the + /// session whose rule fired, and neither holding the other's activity. + /// + [Test] + public async Task RouteSpawnGivesEachSessionItsOwnWindow() + { + var workspace = new Workspace(sessionKey: "Convergence.Ann"); + + var ann = workspace.RouteSpawn("Public", "Convergence.Ann"); + var bob = workspace.RouteSpawn("Public", "Convergence.Bob"); + + await Assert.That(ann).IsNotEqualTo(bob); + await Assert.That(ann.SessionKey).IsEqualTo("Convergence.Ann"); + await Assert.That(bob.SessionKey).IsEqualTo("Convergence.Bob"); + await Assert.That(ann.Title).IsEqualTo("Public"); + await Assert.That(bob.Title).IsEqualTo("Public"); + await Assert.That(ann.Unread).IsEqualTo(1); + await Assert.That(bob.Unread).IsEqualTo(1); + await Assert.That(workspace.Windows.Count).IsEqualTo(3); // main + one each + } + + /// + /// And the same session routing twice still lands in one window — the property that makes the id a + /// name rather than a fresh identity per line. + /// + [Test] + public async Task OneSessionRoutingTwiceKeepsOneWindow() + { + var workspace = new Workspace(sessionKey: "Convergence.Ann"); + + var first = workspace.RouteSpawn("Public", "Convergence.Ann"); + var second = workspace.RouteSpawn("Public", "Convergence.Ann"); + + await Assert.That(second).IsEqualTo(first); + await Assert.That(workspace.Windows.Count).IsEqualTo(2); + } + + /// + /// Windows nobody owns share one bucket, and that is right rather than an oversight. A live + /// WorldSession always has a key (its world's name when it has no character), so an unowned + /// spawn window cannot come from a connection at all. And the rail lists an unowned window under + /// every character precisely because it belongs to none — so two of them for one target + /// would draw two identical rows under everybody, with nothing in the client able to tell them apart + /// or route between them. Null is one identity, "nobody", and not an unknown owner. + /// + [Test] + public async Task WindowsWithNoOwnerAreOneWindowPerTarget() + { + var workspace = new Workspace(); + + var first = workspace.RouteSpawn("Public"); + var second = workspace.RouteSpawn("Public"); + + await Assert.That(second).IsEqualTo(first); + await Assert.That(first.SessionKey).IsNull(); + + // …and an unowned window is still nobody's, so it does not collide with an owned one. + await Assert.That(workspace.RouteSpawn("Public", "Convergence.Ann")).IsNotEqualTo(first); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs index 66647ea..0e09ffb 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs @@ -27,7 +27,7 @@ public async Task Capture_ThenRestore_PreservesWindowsAndTree() // Windows survive with their metadata. await Assert.That(restored.Windows.Count).IsEqualTo(2); - var chat = restored.FindWindow(Workspace.SpawnWindowId("Chat"))!; + var chat = restored.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!; await Assert.That(chat.Kind).IsEqualTo(WindowKind.Spawn); await Assert.That(chat.OwnerLabel).IsEqualTo("Corvid"); await Assert.That(chat.CapturePattern).IsEqualTo(@"^\[Chat\]"); @@ -36,9 +36,9 @@ public async Task Capture_ThenRestore_PreservesWindowsAndTree() // The split tree survives: two panes, main and Chat separated. await Assert.That(restored.Layout.Panes.Count).IsEqualTo(2); await Assert.That(restored.Layout.FindWindow("main")).IsNotNull(); - await Assert.That(restored.Layout.FindWindow(Workspace.SpawnWindowId("Chat"))).IsNotNull(); + await Assert.That(restored.Layout.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))).IsNotNull(); await Assert.That(restored.Layout.FindWindow("main")!.Id) - .IsNotEqualTo(restored.Layout.FindWindow(Workspace.SpawnWindowId("Chat"))!.Id); + .IsNotEqualTo(restored.Layout.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.Id); } [Test] @@ -80,6 +80,7 @@ public async Task State_RoundTripsThroughJson() await Assert.That(json).Contains("\"kind\": \"Spawn\""); await Assert.That(json).Contains("\"type\": \"split\""); await Assert.That(restored.Layout.Panes.Count).IsEqualTo(2); - await Assert.That(restored.FindWindow(Workspace.SpawnWindowId("Chat"))!.OwnerLabel).IsEqualTo("Corvid"); + await Assert.That(restored.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.OwnerLabel) + .IsEqualTo("Corvid"); } } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs index 6b75dba..16caea6 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs @@ -25,7 +25,7 @@ public async Task RouteSpawn_CreatesBackgroundWindow_AndAccruesUnread() var chat = w.RouteSpawn("Chat"); await Assert.That(chat.Kind).IsEqualTo(WindowKind.Spawn); - await Assert.That(chat.Id).IsEqualTo(Workspace.SpawnWindowId("Chat")); + await Assert.That(chat.Id).IsEqualTo(Workspace.SpawnWindowId(null, "Chat")); await Assert.That(w.IsVisible(chat.Id)).IsFalse(); // main stays the active tab await Assert.That(chat.Unread).IsEqualTo(1); @@ -93,7 +93,7 @@ public async Task CloseWindow_RemovesFromRegistryAndLayout() { var w = new Workspace(); w.RouteSpawn("Chat"); - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = Workspace.SpawnWindowId(null, "Chat"); var closed = w.CloseWindow(chatId); diff --git a/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs b/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs index 4d3f794..10b009c 100644 --- a/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs @@ -66,7 +66,7 @@ private static void Type(SharpMUTermApp app, string text) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; /// The truecolor background escape a colour is written as, e.g. 48;2;51;57;76. private static string Sgr(SharpConsoleUI.Color color) => $"48;2;{color.R};{color.G};{color.B}"; diff --git a/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs index 5816530..704f680 100644 --- a/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs @@ -594,11 +594,11 @@ public async Task CtrlWDoesNotKillTheWordBeforeTheCaret() public async Task CtrlWClosesTheWindow() { var (app, _) = await Demo(); - app.SimulateWindowChange("spawn:Chat"); - await Assert.That(app.WindowIds()).Contains("spawn:Chat"); + app.SimulateWindowChange(DemoScene.ChatWindowId); + await Assert.That(app.WindowIds()).Contains(DemoScene.ChatWindowId); app.SimulateKey(Chord(ConsoleKey.W)); - await Assert.That(app.WindowIds()).DoesNotContain("spawn:Chat"); + await Assert.That(app.WindowIds()).DoesNotContain(DemoScene.ChatWindowId); } } diff --git a/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs index 3046c3c..0c58b08 100644 --- a/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs @@ -28,7 +28,9 @@ public class InputAreaEndToEndTests private const int Width = 120; private const int Height = 34; private const string MainWindow = "main"; - private const string ChatWindow = "spawn:Chat"; + + /// The demo's Chat spawn window. Its id names its owner, so it is asked for rather than spelt. + private static string ChatWindow => DemoScene.ChatWindowId; private static readonly TerminalCapabilities Headless = new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); diff --git a/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs b/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs index a2fa015..1cf7fc5 100644 --- a/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs @@ -96,7 +96,7 @@ private static PaneRect OutputRectOf(SharpMUTermApp app, string windowId) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; // --- the bug --------------------------------------------------------------- diff --git a/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs index a621773..f4b1060 100644 --- a/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs @@ -41,9 +41,9 @@ private static SharpMUTermApp OneTab() return new SharpMUTermApp(new AppConfiguration(), Headless, new HeadlessConsoleDriver(Width, Height)); } - private static readonly string ChatId = Workspace.SpawnWindowId("Chat"); + private static readonly string ChatId = Workspace.SpawnWindowId(null, "Chat"); - private static readonly string OocId = Workspace.SpawnWindowId("OOC"); + private static readonly string OocId = Workspace.SpawnWindowId(null, "OOC"); /// /// One pane holding three tabs — main, a Chat spawn and an OOC spawn — with the tab at diff --git a/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs index eaa98c3..38fde54 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs @@ -70,7 +70,7 @@ private static void Type(SharpMUTermApp app, string text) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; /// Presses the chord and renders the frame it produces, the way the running app would. private static void Press(SharpMUTermApp app, ConsoleKey arrow) diff --git a/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs index 1d68d65..78ec30f 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs @@ -75,7 +75,7 @@ private static List Flatten(LayoutNodeState node) => public async Task TheActiveTabsCloseButtonClosesIt() { var harness = Rendered(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; await Assert.That(Tabs(harness.App)).Contains(chat); SweepRow(harness, 0); @@ -90,7 +90,7 @@ public async Task TheActiveTabsCloseButtonClosesIt() public async Task ClickingATabsLabelSelectsItWithoutClosing() { var harness = Rendered(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; // Column 1 is inside the first tab's label (the framework pads each tab with a space). harness.App.SimulateTabStripClick(harness.PaneId, 1, 0); @@ -121,6 +121,6 @@ public async Task TheMainWindowIsNeverClosable() SweepRow(harness, 0); await Assert.That(Tabs(harness.App)).Contains("main"); - await Assert.That(Tabs(harness.App)).Contains(Workspace.SpawnWindowId("Chat")); + await Assert.That(Tabs(harness.App)).Contains(DemoScene.ChatWindowId); } } diff --git a/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs b/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs index 89591fd..6103d93 100644 --- a/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs @@ -59,7 +59,7 @@ private static SharpMUTermApp Fresh() private static SharpMUTermApp ChatForward() { var app = App(); - app.SimulateWindowChange(Workspace.SpawnWindowId("Chat")); + app.SimulateWindowChange(DemoScene.ChatWindowId); return app; } diff --git a/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs b/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs index 5eb3b00..3391a0e 100644 --- a/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs @@ -151,9 +151,8 @@ public async Task ClickingAWindowRow_ActivatesThatWindow() app.RenderSnapshot(); // The demo's active character owns a spawn window as well as the main one. - var spawn = app.WindowIds().First(id => id.StartsWith("spawn:", StringComparison.Ordinal)); - var title = SpawnTitle(spawn); - ClickRailRow(app, title); + var spawn = app.WindowIds().First(id => id.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal)); + ClickRailRow(app, app.WindowTitleOf(spawn)!); await Assert.That(app.ActiveWindowId()).IsEqualTo(spawn); } @@ -417,5 +416,4 @@ private static void ClickRailRow(SharpMUTermApp app, string text) } } - private static string SpawnTitle(string spawnWindowId) => spawnWindowId["spawn:".Length..]; } diff --git a/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs b/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs index 3ea0197..868ddb5 100644 --- a/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs @@ -147,7 +147,7 @@ public async Task ClosingAWindowRemovesItsRowRatherThanMarkingItClosed() { var app = App(); app.RenderSnapshot(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; await Assert.That(string.Join("\n", Rail(app))).Contains("Chat"); await Assert.That(app.DispatchCommand("win:" + chat)).IsTrue(); // bring it up so ⌃W can close it diff --git a/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs index 6598a52..b4be291 100644 --- a/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs @@ -5,6 +5,7 @@ using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Session; using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; using SharpMUTerm.Graphics; namespace SharpMUTerm.Tui.Tests; @@ -32,7 +33,14 @@ public class RestoreLogEndToEndTests private const int Width = 160; private const int Height = 40; private const string MainWindow = "main"; - private const string ChatWindow = "spawn:Chat"; + + /// + /// The one character in this fixture, and the Chat window its capture rule routes to. A spawn + /// window's id names its owner, so the id is asked for rather than spelt out. + /// + private const string Character = "Aetherfall.Corvid"; + + private static readonly string ChatWindow = Workspace.SpawnWindowId(Character, "Chat"); /// The startup ceiling asserted below — see that test for the measured figure it guards. private const int Ceiling = 400; @@ -385,7 +393,9 @@ public async Task RestoringAFullLogCostsLittleEnoughToRunBeforeTheFirstFrame() { using var root = new TempRoot(); var config = Configuration(); - var windows = new[] { MainWindow, ChatWindow, "spawn:OOC", "spawn:Tells", "spawn:Guild", "spawn:Events" }; + var windows = new[] { MainWindow, ChatWindow } + .Concat(new[] { "OOC", "Tells", "Guild", "Events" }.Select(t => Workspace.SpawnWindowId(Character, t))) + .ToArray(); using (var seed = new RestoreLog(root.Path, config.RestoreLog)) { diff --git a/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs b/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs index a7c65da..8e19063 100644 --- a/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs @@ -68,7 +68,7 @@ public async Task ASetAssignedWhileConnected_OpensTheSpawnWindowOnTheNextMatchin var mannaz = wired.Config.Worlds[0].Characters[0]; wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsFalse(); // the state the bug lived in + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsFalse(); // the state the bug lived in // WorldsScreenRenderer.Assignment: a character opts into a set by list membership… mannaz.TriggerSets.Add("Comms"); @@ -77,8 +77,8 @@ public async Task ASetAssignedWhileConnected_OpensTheSpawnWindowOnTheNextMatchin wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); - await Assert.That(string.Join("\n", wired.App.PaneLines(Workspace.SpawnWindowId("Public")))) + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); + await Assert.That(string.Join("\n", wired.App.PaneLines(Workspace.SpawnWindowId("Convergence.Mannaz", "Public")))) .Contains("Lol"); } @@ -97,7 +97,7 @@ public async Task ARuleAddedToAnAssignedSetWhileConnected_TakesEffectOnTheNextLi wired.App.SaveConfiguration(); wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsFalse(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsFalse(); wired.Config.TriggerSets.Single(s => s.Name == "Comms").Triggers.Add(new Trigger { @@ -109,7 +109,7 @@ public async Task ARuleAddedToAnAssignedSetWhileConnected_TakesEffectOnTheNextLi wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); } /// @@ -129,7 +129,7 @@ public async Task ACommittedEditOnAnySettingsScreen_IsWhatMakesAnAutomationChang wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); } /// @@ -150,7 +150,7 @@ public async Task ASessionOpenedByTheCharacterSwitchPath_RoutesItsCaptures() wired.Receive(wired.RikoWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Grapevine.Riko", "Public")).IsTrue(); } /// @@ -293,8 +293,13 @@ public async Task ABackgroundConnectionDropping_ChangesTheCount() private const string PublicLine = " Starfall Empress Lucille Wolfsbane says, \"Lol\"\n"; - private static bool SpawnWindowExists(SharpMUTermApp app, string target) => - app.WindowIds().Contains(Workspace.SpawnWindowId(target), StringComparer.Ordinal); + /// + /// Whether has a spawn window for . The owner is + /// part of the question because it is part of the window id: two characters capturing one target + /// have a window each. + /// + private static bool SpawnWindowExists(SharpMUTermApp app, string owner, string target) => + app.WindowIds().Contains(Workspace.SpawnWindowId(owner, target), StringComparer.Ordinal); private static ConsoleKeyInfo CtrlQ() => new('\0', ConsoleKey.Q, false, false, true); diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs new file mode 100644 index 0000000..77b7fd4 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs @@ -0,0 +1,279 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The upgrade, driven the whole distance. A configuration written by the previous build — schema +/// v4, a saved workspace holding spawn:Chat in a pane — and a restore log written beside it under +/// that same id, opened by this build. The user must lose neither the pane nor the text in it, and the +/// pane must come back as something the client will still write to. +/// +/// The two stores are joined only by the window id, and this change moves it, so they have to be +/// upgraded together or the upgrade is worse than the bug: rewriting only config.json brings the +/// pane back empty for ever, and rewriting only the log leaves the content filed under an id no pane +/// has. ConfigurationMigrator's v4→v5 step does the first and +/// SharpMUTermApp.CarryLegacySpawnLogsOver does the second, at the launch that reads them. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class SpawnWindowIdUpgradeTests +{ + private const int Width = 160; + private const int Height = 40; + private const string Corvid = "Aetherfall.Corvid"; + private const string LegacyChat = "spawn:Chat"; + private const string Backlog = "Rivane: anyone up for the crypt run?"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static string CorvidsChat => Workspace.SpawnWindowId(Corvid, "Chat"); + + /// + /// The headline: the pane is still there, still in its pane, still called Chat, and still + /// holding the previous session's conversation — under the id its owner now routes to. The user sees + /// no difference at all, which is the point. + /// + [Test] + public async Task AnOldConfigKeepsItsPaneItsPlacementAndItsRestoredScrollback() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + // The pane, under the id this build routes to — and not under the one it was saved as. + await Assert.That(app.WindowIds()).Contains(CorvidsChat); + await Assert.That(app.WindowIds()).DoesNotContain(LegacyChat); + await Assert.That(app.WindowTitleOf(CorvidsChat)).IsEqualTo("Chat"); + await Assert.That(app.WindowOwnerOf(CorvidsChat)).IsEqualTo(Corvid); + + // Still beside the main window in the pane it was saved in. + await Assert.That(app.CaptureSession().Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + + // And holding what it held, closed off by the restore bar. + var lines = string.Join("\n", app.PaneLines(CorvidsChat)); + await Assert.That(lines).Contains(Backlog); + await Assert.That(lines).Contains(RestoreBarRenderer.Label); + } + + /// + /// No orphan. The upgraded pane is one the running client still writes to: the character + /// connects, its capture rule fires, and the line lands in the pane that came back rather than in a + /// second one beside it. This is the assertion that rules out the tempting cheaper answer of leaving + /// the old id alone. + /// + [Test] + public async Task TheUpgradedPaneIsTheOneTheLiveCaptureWritesTo() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + var telnet = new RecordingTelnetSession(); + app.TelnetFactory = _ => telnet; + app.DispatchCommand(CommandIds.Character(Corvid)); + await app.FindSession(Corvid)!.ConnectAsync(); + + telnet.Receive("[Chat] Bob: aye, meet me at the gate\n"); + app.RenderNextFrame(); + + // One Chat window, and the new line is under the restored history in it. + await Assert.That(app.WindowIds().Count(id => id.EndsWith(":Chat", StringComparison.Ordinal))).IsEqualTo(1); + var lines = app.PaneLines(CorvidsChat).ToList(); + await Assert.That(string.Join("\n", lines)).Contains(Backlog); + await Assert.That(lines[^1]).Contains("meet me at the gate"); + } + + /// + /// The carry-over happens once. The second launch reads a log that is already in the new shape, so + /// the pane comes back holding its history once rather than twice — which is what a mapping kept + /// for ever, or an old file left on disk beside the new one, would have cost on every launch after + /// the upgrade. + /// + [Test] + public async Task TheUpgradeHappensOnceAndDoesNotDoubleThePaneNextLaunch() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + await Assert.That(root.Files.Any(f => Path.GetFileName(f).StartsWith("spawnChat-", StringComparison.Ordinal))) + .IsTrue() + .Because("the previous build's file has to be there for its removal to mean anything"); + + using (var first = new RestoreLog(root.Path, config.RestoreLog)) + { + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: first); + config.LastSession = app.CaptureSession(); + } + + // The file the old id was in is gone, and the new id has one of its own. + await Assert.That(root.Files.Any(f => Path.GetFileName(f).StartsWith("spawnChat-", StringComparison.Ordinal))) + .IsFalse(); + + using var second = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var relaunched = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: second); + + var lines = relaunched.PaneLines(CorvidsChat).ToList(); + await Assert.That(lines.Count(l => l.Contains(Backlog, StringComparison.Ordinal))).IsEqualTo(1); + await Assert.That(lines.Count(l => l.Contains(RestoreBarRenderer.Label, StringComparison.Ordinal))) + .IsEqualTo(1); + } + + /// + /// A log under an old id that no pane claims is left exactly as it always was: buffered under its + /// own id, not carried anywhere, not deleted — so if that channel ever speaks again its pane opens + /// with its history already in it. The upgrade may not turn "the saved workspace forgot this pane" + /// into "the content is gone". + /// + [Test] + public async Task AnOldLogNoPaneClaimsIsLeftWhereItIs() + { + using var root = new TempRoot(); + using (var seed = new RestoreLog(root.Path)) + { + seed.Append("spawn:Tells", "Tells", StyledLine.FromText("Rivane pages: hello", TextStyle.Default), "09:24"); + } + + var config = OldConfiguration(); + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + await Assert.That(log.Read().Any(w => w.WindowId == "spawn:Tells")).IsTrue(); + await Assert.That(app.WindowIds().Any(id => id.EndsWith(":Tells", StringComparison.Ordinal))).IsFalse(); + } + + /// + /// And the fix survives the round trip it is most likely to be undone by. Two characters capture one + /// target, the workspace is saved and reopened, and each still has a pane of their own holding their + /// own conversation — the ids are stable across a restart, and the restore log's per-window keying + /// keeps the two apart on disk as well as in memory. + /// + [Test] + public async Task TwoCharactersPanesComeBackSeparatelyAfterARestart() + { + using var root = new TempRoot(); + var config = SpawnWindowPerSessionTests.Configuration(); + var ann = Workspace.SpawnWindowId("Convergence.Ann", "Public"); + var bob = Workspace.SpawnWindowId("Convergence.Bob", "Public"); + + using (var first = new RestoreLog(root.Path, config.RestoreLog)) + { + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: first); + + foreach (var (key, text) in new[] { ("Convergence.Ann", "first"), ("Convergence.Bob", "second") }) + { + var telnet = new RecordingTelnetSession(); + app.TelnetFactory = _ => telnet; + app.DispatchCommand(CommandIds.Character(key)); + await app.FindSession(key)!.ConnectAsync(); + telnet.Receive($" {key} says, \"{text}\"\n"); + app.RenderNextFrame(); + } + + config.LastSession = app.CaptureSession(); + } + + using var second = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var relaunched = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: second); + + await Assert.That(string.Join("\n", relaunched.PaneLines(ann))).Contains("first"); + await Assert.That(string.Join("\n", relaunched.PaneLines(ann))).DoesNotContain("second"); + await Assert.That(string.Join("\n", relaunched.PaneLines(bob))).Contains("second"); + await Assert.That(string.Join("\n", relaunched.PaneLines(bob))).DoesNotContain("first"); + } + + // ---- Harness ---------------------------------------------------------------------------- + + /// A throwaway restore-log root, removed however the test ends. + private sealed class TempRoot : IDisposable + { + public TempRoot() => + Path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"smuterm-upgrade-{Guid.NewGuid():N}"); + + public string Path { get; } + + public IReadOnlyList Files => + Directory.Exists(Path) ? Directory.GetFiles(Path) : Array.Empty(); + + public void Dispose() + { + try + { + if (Directory.Exists(Path)) + { + Directory.Delete(Path, recursive: true); + } + } + catch (Exception) + { + // Nothing a test should fail over. + } + } + } + + /// The previous build's restore log: one window, keyed by the id that build wrote. + private static void SeedLegacyLog(TempRoot root) + { + using var seed = new RestoreLog(root.Path); + seed.Append(LegacyChat, "Chat", StyledLine.FromText("[Chat] " + Backlog, TextStyle.Default), "09:24"); + } + + /// + /// A genuine v4 document, parsed the way the client parses one at startup — so the migration under + /// test is the one that actually runs, rather than a hand-built object graph that has already had + /// the answer written into it. + /// + private static AppConfiguration OldConfiguration() => ConfigurationStore.Deserialize(""" + { + "version": 4, + "worlds": [ { + "name": "Aetherfall", "host": "aetherfall.example.org", "port": 4201, + "characters": [ { "name": "Corvid", "triggerSets": [ "chat" ], "logging": { "format": "None" } } ] + } ], + "triggerSets": [ { + "name": "chat", + "triggers": [ { "name": "chat", "pattern": "^\\[Chat\\]", + "actions": { "spawnTarget": "Chat", "gag": true } } ] + } ], + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid", + "ownerLabel": "Corvid", "capturePattern": "^\\[Chat\\]" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); +} diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs new file mode 100644 index 0000000..223d480 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs @@ -0,0 +1,313 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The reported defect: two connected characters whose triggers match the same pattern shared one +/// capture pane. Whichever session matched first created the window — with its own +/// sessionKey on it — and every later session's lines were appended to that same window, which +/// somebody else owned. The second character's channel was therefore not merely mixed in with the +/// first's: it was filed under the wrong character, and draws +/// window rows for the active character only, so it was invisible from the character it +/// belonged to and reachable only from the one it did not. +/// +/// The cause was Workspace.SpawnWindowId, which was $"spawn:{target}" — no owner in it, so +/// one window per workspace per target rather than one per session per target. +/// +/// +/// The fixture is the smallest one that can show it: two characters on one world, both assigned the +/// same trigger set, both connected over their own recording transport. One character per target +/// passes either way, which is why this shipped. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class SpawnWindowPerSessionTests +{ + private const int Width = 160; + private const int Height = 40; + + private const string Ann = "Convergence.Ann"; + private const string Bob = "Convergence.Bob"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + // ---- The report ------------------------------------------------------------------------- + + /// + /// The headline. Both characters capture Public; there are two windows, one owned by each. + /// Before the fix there was one, owned by whoever matched first. + /// + [Test] + public async Task TwoCharactersCapturingOneTargetGetAWindowEach() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + var annWindow = Workspace.SpawnWindowId(Ann, "Public"); + var bobWindow = Workspace.SpawnWindowId(Bob, "Public"); + + await Assert.That(two.App.WindowIds()).Contains(annWindow); + await Assert.That(two.App.WindowIds()).Contains(bobWindow); + await Assert.That(annWindow).IsNotEqualTo(bobWindow); + + await Assert.That(two.App.WindowOwnerOf(annWindow)).IsEqualTo(Ann); + await Assert.That(two.App.WindowOwnerOf(bobWindow)).IsEqualTo(Bob); + } + + /// + /// And the lines land in their own pane and only there. This is the assertion the user's sentence is + /// about — "one wins and gets 2 redirects" — and it fails on the unfixed build with both lines in + /// Ann's pane and Bob's pane not existing at all. + /// + [Test] + public async Task EachCharactersCaptureLandsInItsOwnPaneAndNoOther() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + var ann = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Ann, "Public"))); + var bob = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Bob, "Public"))); + + await Assert.That(ann).Contains("first"); + await Assert.That(ann).DoesNotContain("second"); + await Assert.That(bob).Contains("second"); + await Assert.That(bob).DoesNotContain("first"); + } + + /// + /// The visible half of the same defect. The rail lists the active character's windows, so a + /// capture pane filed under the wrong owner cannot be seen or clicked from the character whose + /// channel it is. Each character's rail now carries exactly one Public row: their own. + /// + [Test] + public async Task EachCharactersRailListsItsOwnCapturePaneAndNotTheOthers() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + // Bob is the character switched to last, so the rail is drawn for Bob. + var bobsRail = two.App.RailLines; + await Assert.That(RailTargets(bobsRail)).Contains("win:" + Workspace.SpawnWindowId(Bob, "Public")); + await Assert.That(RailTargets(bobsRail)).DoesNotContain("win:" + Workspace.SpawnWindowId(Ann, "Public")); + + two.App.DispatchCommand(CommandIds.Character(Ann)); + var annsRail = two.App.RailLines; + await Assert.That(RailTargets(annsRail)).Contains("win:" + Workspace.SpawnWindowId(Ann, "Public")); + await Assert.That(RailTargets(annsRail)).DoesNotContain("win:" + Workspace.SpawnWindowId(Bob, "Public")); + } + + /// + /// The id changed and the name did not. Both panes are still called Public — on the tab + /// strip and in the sidebar — because every surface that names a window reads + /// and none derives a display name from the id. A pane labelled + /// Convergence.Ann:Public would be this fix leaking its own bookkeeping onto the screen. + /// + [Test] + public async Task BothPanesAreStillCalledPublic() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Ann, "Public"))).IsEqualTo("Public"); + await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Bob, "Public"))).IsEqualTo("Public"); + + // The tab strip and the sidebar say the same, and neither says the session key. + two.App.RenderNextFrame(); + var tabs = string.Join("\n", two.App.PaneTabTitles.Values.SelectMany(t => t)); + await Assert.That(tabs).Contains("Public"); + await Assert.That(tabs).DoesNotContain(Bob); + + // The rail's *visible* text, with the markup (and so the `win:` click payloads) stripped: + // the id may carry the owner, the drawn row may not. + var drawn = string.Join("\n", two.App.RailLines.Select(StripMarkup)); + await Assert.That(drawn).Contains("Public"); + await Assert.That(drawn).DoesNotContain(Bob); + } + + /// + /// The id is stable across a reconnect: the same character dropping and dialling back in keeps the + /// window it had, rather than accruing a second one beside it. + /// + [Test] + public async Task ReconnectingKeepsTheSameCapturePane() + { + var two = await Two(); + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + + var before = two.App.WindowIds().Count(id => id.Contains("Public", StringComparison.Ordinal)); + + await two.App.FindSession(Ann)!.DisconnectAsync(); + await two.App.FindSession(Ann)!.ConnectAsync(); + two.Receive(two.AnnWire, " Ann says, \"again\"\n"); + + await Assert.That(two.App.WindowIds().Count(id => id.Contains("Public", StringComparison.Ordinal))) + .IsEqualTo(before); + var pane = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Ann, "Public"))); + await Assert.That(pane).Contains("first"); + await Assert.That(pane).Contains("again"); + } + + // ---- Harness ---------------------------------------------------------------------------- + + /// + /// One rail row with its Spectre markup removed, so an assertion can read what the sidebar + /// draws rather than the [link=win:<id>] payload it carries underneath. + /// + private static string StripMarkup(string line) + { + var text = new System.Text.StringBuilder(line.Length); + for (var i = 0; i < line.Length; i++) + { + if (line[i] != '[') + { + text.Append(line[i]); + continue; + } + + if (i + 1 < line.Length && line[i + 1] == '[') + { + text.Append('['); + i++; + continue; + } + + var close = line.IndexOf(']', i); + i = close < 0 ? line.Length : close; + } + + return text.ToString(); + } + + /// The win:… click payloads the rail is currently drawing. + private static IReadOnlyList RailTargets(IReadOnlyList lines) + { + var targets = new List(); + foreach (var line in lines) + { + var at = 0; + while ((at = line.IndexOf("[link=", at, StringComparison.Ordinal)) >= 0) + { + at += "[link=".Length; + var end = line.IndexOf(']', at); + if (end < 0) + { + break; + } + + targets.Add(line[at..end]); + at = end; + } + } + + return targets; + } + + private static Task Two() => TwoCharacters.Build(); + + /// + /// One world, two characters, one shared trigger set routing ^<Public> to a spawn window + /// called Public — and both characters connected over a transport of their own, because a + /// session that was never connected never runs its receive path and would make any capture + /// assertion true whatever the code does. + /// + private sealed class TwoCharacters + { + private TwoCharacters( + SharpMUTermApp app, AppConfiguration config, RecordingTelnetSession ann, RecordingTelnetSession bob) + { + App = app; + Config = config; + AnnWire = ann; + BobWire = bob; + } + + internal SharpMUTermApp App { get; } + + internal AppConfiguration Config { get; } + + internal RecordingTelnetSession AnnWire { get; } + + internal RecordingTelnetSession BobWire { get; } + + internal static async Task Build() + { + Console.SetIn(TextReader.Null); + var config = Configuration(); + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + var two = new TwoCharacters(app, config, new RecordingTelnetSession(), new RecordingTelnetSession()); + + await two.Open(Ann, two.AnnWire); + await two.Open(Bob, two.BobWire); + app.RenderNextFrame(); + return two; + } + + internal void Receive(RecordingTelnetSession telnet, string text) + { + telnet.Receive(text); + App.RenderNextFrame(); + } + + internal async Task Open(string sessionKey, RecordingTelnetSession telnet) + { + App.TelnetFactory = _ => telnet; + if (!App.DispatchCommand(CommandIds.Character(sessionKey))) + { + throw new InvalidOperationException($"the app would not switch to {sessionKey}"); + } + + await App.FindSession(sessionKey)!.ConnectAsync(); + } + } + + internal static AppConfiguration Configuration() + { + var config = new AppConfiguration(); + config.TriggerSets.Add(new TriggerSet + { + Name = "Comms", + Triggers = + { + new Trigger + { + Name = "Public", + Pattern = "^", + Actions = new TriggerActions { SpawnTarget = "Public", Gag = true }, + }, + }, + }); + + config.Worlds.Add(new WorldDefinition + { + Name = "Convergence", + Host = "convergence.example.org", + Port = 4201, + Characters = + { + new CharacterDefinition { Name = "Ann", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + new CharacterDefinition { Name = "Bob", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + }, + }); + + return config; + } +} From 6774e90712c0f16826d12977bd887887d12ea206 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 10:09:08 -0500 Subject: [PATCH 2/3] Refuse a spawn id whose declared owner length would overflow the bounds check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `owner.Length < length + 1` passes for a declared length of int.MaxValue — the addition wraps negative — and the next line then indexes off the end of the string. `spawn:2147483647:abc:x` in a hand-edited config or a corrupt restore log file name threw IndexOutOfRangeException out of a reader whose whole contract is to answer false. Written as `owner.Length <= length`, which says the same thing and cannot wrap. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- src/SharpMUTerm.Core/Workspace/Workspace.cs | 6 ++++-- .../SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index dbcd900..7308c26 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -172,9 +172,11 @@ public static bool TryReadSpawnWindowId(string id, out string? sessionKey, out s return false; } - // The owner's exact length, then the colon that closes it: anything shorter is not this encoding. + // The owner's exact length, then the colon that closes it: anything shorter is not this + // encoding. Written as `<=` rather than `< length + 1` so a declared length of int.MaxValue + // cannot overflow the comparison into passing and then index off the end. var owner = rest[(separator + 1)..]; - if (owner.Length < length + 1 || owner[length] != ':') + if (owner.Length <= length || owner[length] != ':') { return false; } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs index 6b9c03d..374d726 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs @@ -107,6 +107,10 @@ public async Task AnIdReadsBackAsThePairThatMadeIt() [Arguments("spawn:99:short:x")] [Arguments("spawn:+3:abc:x")] [Arguments("spawn: 3:abc:x")] + // A declared length at the top of the range: the bounds check has to be written so this cannot + // overflow into passing and then index off the end of the string. + [Arguments("spawn:2147483647:abc:x")] + [Arguments("spawn:99999999999999999999:abc:x")] public async Task WhatIsNotOneOfTheseIsRefused(string id) { await Assert.That(Workspace.TryReadSpawnWindowId(id, out _, out _)).IsFalse(); From 52d9c2b88ce1ca101d4f5567f1e2c3de6080512e Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 11:49:39 -0500 Subject: [PATCH 3/3] Merge main, and scope the display-name assertions to their own windows #14 landed first, so this carries its rail rewrite, its window numbering and the removal of CapturePattern. The merge was textually clean and semantically not: SpawnWindowId takes an owner now, so #14's new tests called it with one argument, and its migration fixture asserted on a field #14 deleted. The review item on BothPanesAreStillCalledPublic was right that a frame-wide DoesNotContain is the wrong shape - it can pass for reasons unrelated to its claim - though not for the stated reason: the constant is the session key 'Convergence.Bob', not 'Bob', so the collections it warned about would not have matched it. Scoping the assertion found what the frame-wide one had been missing. A spawn tab reads 'Ann - Public (1)': owner label, name, unread count, in markup. So the test now reads that tab specifically, through StripMarkup, and pins the distinction that actually matters - the character's name is on the strip deliberately, so two characters capturing one target can be told apart, and the session key must never be. It still fails on the shared id: 4 of 5, including this one. --- .../SpawnWindowIdMigrationTests.cs | 6 +- .../Workspace/WindowNumberingTests.cs | 25 +++++---- .../SpawnWindowPerSessionTests.cs | 56 +++++++++++++++---- .../SharpMUTerm.Tui.Tests/WindowJumpTests.cs | 12 ++-- 4 files changed, 72 insertions(+), 27 deletions(-) diff --git a/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs index 8dd5704..a49d181 100644 --- a/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs @@ -63,7 +63,11 @@ public async Task AV4SpawnWindowKeepsItsPaneItsPlaceAndItsMetadata() await Assert.That(window.Title).IsEqualTo("Chat"); await Assert.That(window.SessionKey).IsEqualTo("Aetherfall.Corvid"); await Assert.That(window.OwnerLabel).IsEqualTo("Corvid"); - await Assert.That(window.CapturePattern).IsEqualTo(@"^\[Chat\]"); + + // There is no CapturePattern to assert any more: the capture header it fed was removed with the + // spawn pane's dim "⇱ capture …" row, and the field went with it. A v4 document may still carry + // the property — the fixture below does — and System.Text.Json drops what it cannot map, which is + // the behaviour that lets an old config load at all. await Assert.That(config.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs index 4cc9c03..3d107e1 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs @@ -37,7 +37,7 @@ public async Task WindowsAreNumberedInTheOrderTheyWereOpened() ws.RouteSpawn("Chat", "W.C"); ws.OpenWindow("web", "Web"); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Chat,web"); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Chat")},web"); } /// @@ -53,7 +53,7 @@ public async Task MovingAWindowToAnotherPaneDoesNotRenumberAnything() ws.RouteSpawn("Trade", "W.C"); var before = Order(ws); - ws.Layout.SplitWithWindow("spawn:Trade", ws.Layout.FocusedPaneId, Edge.Left); + ws.Layout.SplitWithWindow(Spawn("Trade"), ws.Layout.FocusedPaneId, Edge.Left); await Assert.That(Order(ws)).IsEqualTo(before); } @@ -69,7 +69,7 @@ public async Task ReorderingTabsDoesNotRenumberAnything() ws.RouteSpawn("Chat", "W.C"); var before = Order(ws); - ws.ActivateWindow("spawn:Chat"); + ws.ActivateWindow(Spawn("Chat")); await Assert.That(ws.Layout.ReorderActiveTab(-1)).IsTrue(); await Assert.That(Order(ws)).IsEqualTo(before); @@ -88,9 +88,9 @@ public async Task ClosingAWindowCompactsTheNumbering() ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.CloseWindow("spawn:Chat"); + ws.CloseWindow(Spawn("Chat")); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade"); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")}"); await Assert.That(ws.WindowsFor(Owner)[1].Sequence) .IsGreaterThan(2) .Because("the sequence keeps its hole; only the position closes up"); @@ -113,10 +113,10 @@ public async Task AWindowOpenedIntoAClosedOnesSlotStillTakesTheLastNumber() ws.RouteSpawn("Trade", "W.C"); ws.RouteSpawn("Newbie", "W.C"); - ws.CloseWindow("spawn:Chat"); // frees the second slot + ws.CloseWindow(Spawn("Chat")); // frees the second slot ws.RouteSpawn("Guild", "W.C"); // which the registry hands straight back out - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade,spawn:Newbie,spawn:Guild") + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")},{Spawn("Newbie")},{Spawn("Guild")}") .Because("the newcomer is ⌥4, and Trade and Newbie are still ⌥2 and ⌥3"); } @@ -133,10 +133,10 @@ public async Task AWindowNoPaneHoldsIsNotNumbered() ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.Layout.RemoveWindow("spawn:Chat"); // out of the tree, still in the registry + ws.Layout.RemoveWindow(Spawn("Chat")); // out of the tree, still in the registry - await Assert.That(ws.Windows.Select(w => w.Id)).Contains("spawn:Chat"); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade"); + await Assert.That(ws.Windows.Select(w => w.Id)).Contains(Spawn("Chat")); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")}"); } // --- across a restart --------------------------------------------------------------------------- @@ -151,7 +151,7 @@ public async Task TheNumberingComesBackAfterAResume() var ws = new Workspace(mainWindowId: "main", mainTitle: "Main", sessionKey: "W.C"); ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.CloseWindow("spawn:Chat"); + ws.CloseWindow(Spawn("Chat")); ws.RouteSpawn("Newbie", "W.C"); var before = Order(ws); @@ -234,6 +234,9 @@ public async Task AHalfMigratedWorkspaceGivesNoTwoWindowsOneNumber() /// The character every window in these fixtures belongs to. private const string Owner = "W.C"; + /// The window id produces for . + private static string Spawn(string target) => Workspace.SpawnWindowId(Owner, target); + private static string Order(Workspace workspace) => string.Join(",", workspace.WindowsFor(Owner).Select(w => w.Id)); } diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs index 223d480..c8b3a8c 100644 --- a/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs @@ -130,19 +130,55 @@ public async Task BothPanesAreStillCalledPublic() await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Ann, "Public"))).IsEqualTo("Public"); await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Bob, "Public"))).IsEqualTo("Public"); - // The tab strip and the sidebar say the same, and neither says the session key. + // The tab strip and the sidebar say the same — asserted about *those two windows* rather than + // over the whole frame, so the test cannot pass because some unrelated surface happened to + // satisfy it. + // + // What must not leak is the *session key*. The character's own name is a different thing and is + // on the strip deliberately: a spawn tab reads "Ann - Public" precisely so two characters each + // capturing Public can be told apart — which is the feature this PR exists for. So the drawn + // title carries "Ann" and must never carry "Convergence.Ann". two.App.RenderNextFrame(); - var tabs = string.Join("\n", two.App.PaneTabTitles.Values.SelectMany(t => t)); - await Assert.That(tabs).Contains("Public"); - await Assert.That(tabs).DoesNotContain(Bob); - - // The rail's *visible* text, with the markup (and so the `win:` click payloads) stripped: - // the id may carry the owner, the drawn row may not. - var drawn = string.Join("\n", two.App.RailLines.Select(StripMarkup)); - await Assert.That(drawn).Contains("Public"); - await Assert.That(drawn).DoesNotContain(Bob); + + foreach (var owner in new[] { Ann, Bob }) + { + var id = Workspace.SpawnWindowId(owner, "Public"); + + var pane = two.App.PaneIdOf(id); + await Assert.That(pane).IsNotNull().Because($"{owner}'s Public pane must be placed"); + + // Read through StripMarkup: the strip's labels are markup — #14 tints the tab a line + // arrived in — so the raw string carries colour tags around the name. + var tab = two.App.PaneTabTitles[pane!] + .Select(StripMarkup) + .SingleOrDefault(t => t.Contains("Public", StringComparison.Ordinal) && + t.Contains(CharacterOf(owner), StringComparison.Ordinal)); + + await Assert.That(tab).IsNotNull().Because($"{owner}'s Public pane must have a tab naming it"); + await Assert.That(tab!) + .DoesNotContain(owner) + .Because("the id carries the session key; the tab the reader sees must not"); + + // The rail row is found by its `win:` click payload — which does carry the owner — and + // then read for what it *draws*, which must not. Only the active character's window rows are + // drawn, so exactly one of the two is on screen. + var row = two.App.RailLines.SingleOrDefault(l => l.Contains("win:" + id, StringComparison.Ordinal)); + if (row is null) + { + continue; + } + + var drawn = StripMarkup(row); + await Assert.That(drawn).Contains("Public"); + await Assert.That(drawn) + .DoesNotContain(owner) + .Because("the id carries the session key; the row the reader sees must not"); + } } + /// The character half of a World.Character session key. + private static string CharacterOf(string sessionKey) => sessionKey[(sessionKey.IndexOf('.') + 1)..]; + /// /// The id is stable across a reconnect: the same character dropping and dialling back in keeps the /// window it had, rather than accruing a second one beside it. diff --git a/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs b/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs index ef2b20e..0181aed 100644 --- a/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs @@ -203,7 +203,7 @@ public async Task AnUnownedWindowIsNumberedUnderEveryCharacter() public async Task ADigitReachesACaptureWindowSittingBehindAnotherTabInItsPane() { var scene = await BuildScene(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; var chatPane = scene.App.PaneIdOf(chat)!; // Stand in Chat's own pane, on the other tab, so nothing but the tab has to move. @@ -397,11 +397,11 @@ public async Task ClosingAWindowCompactsTheNumberingOnTheChordAndInTheSidebar() await Assert.That(WindowRowChord(scene.App, "Web")).IsEqualTo("⌥3"); scene.App.SimulateKey(Alt(2)); // Chat - await Assert.That(scene.App.ActiveWindowId()).IsEqualTo(Workspace.SpawnWindowId("Chat")); + await Assert.That(scene.App.ActiveWindowId()).IsEqualTo(DemoScene.ChatWindowId); await Assert.That(scene.App.DispatchCommand("layout:close")).IsTrue(); scene.App.RenderNextFrame(); - await Assert.That(scene.App.NumberedWindowIds).DoesNotContain(Workspace.SpawnWindowId("Chat")); + await Assert.That(scene.App.NumberedWindowIds).DoesNotContain(DemoScene.ChatWindowId); await Assert.That(WindowRowChord(scene.App, "Web")) .IsEqualTo("⌥2") .Because("the windows on the screen must be numbered without a hole where the closed one was"); @@ -440,7 +440,9 @@ public async Task AWindowOpenedByTheWireTakesTheNextDigitAndMovesNobodyElses() scene.Transports[0].Receive(" Ann offers a lamp\n"); scene.App.RenderNextFrame(); - var arrival = Workspace.SpawnWindowId("Trade"); + // Transports[0] is Alfa.Ann's, and the line says so — the window belongs to the session whose + // wire it arrived on, which is the whole point of the per-session id. + var arrival = Workspace.SpawnWindowId("Alfa.Ann", "Trade"); await Assert.That(scene.App.NumberedWindowIds).Contains(arrival); foreach (var (what, chord) in before) @@ -877,7 +879,7 @@ private static async Task BuildScene() config.Worlds.Add(definition); } - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; var windows = new[] { "main", chat, "char:Bravo.Bob", "char:Cara.Cal" }; // Ann's two, in creation order — which is the whole of ⌥N while she is active. Bob's and Cal's