From 25b6d9cc88aaadd243e604de8d68c7faa18a75cb Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 19:47:17 -0500 Subject: [PATCH 1/7] Drop the MSSP crawler, and keep what the client needs from it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maintainer's call: "let's drop the crawler aspect. instead, focus on the mssp info and build that in. kill the crawler." Deleted src/SharpMUTerm.Crawler and tests/SharpMUTerm.Crawler.Tests, their solution entries, and the CI step. The 'if: !cancelled()' on the remaining steps stays — it is independent of the crawler and earned its place. Three files in Core/Telnet/Mssp and three fates: MsspData kept. It is the report the INFO screen renders, and it was already a projection of the library's reader rather than a parser. Referrals and CrawlDelay go with MsspHost. MsspVariables kept. Its names are what MsspData's domain accessors read by; REFERRAL and CRAWL DELAY drop out with their accessors. MsspHost deleted. Referral parsing, scope classification and IsCrawlable are crawler shape end to end; nothing else reads them, and REFERRAL's raw values are still one indexer away for anything that wants to audit them. MsspParsingTests moved to the Core suite rather than dying with the crawler. Its subject was never the crawler — it pins what TelnetNegotiationCore's MSSP reader does with arrays, booleans, spaced names and unknown variables, which is exactly the supply the INFO screen is built on. It now drives a real TelnetSession instead of the probe; MsspWire and ScriptedTransport came with it. TelnetSessionOptions.RequestOptions/MsspOption would otherwise have become dead, and deleting them would have been the wrong reading: the client wants what the crawler wanted. WorldSession's session factory now sends IAC DO MSSP. The library opens with IAC WILL NAWS and nothing else, so a server that supports MSSP but waits to be asked is never asked — and an INFO screen reporting that as "publishes no MSSP" would be making a claim about the server out of our own silence. Also swept: CLAUDE.md (six suites to five, the stale 2.6.5 pin, the two upstream MSSP defects that 2.7.0 fixed), the design doc's crawler boundary, and Directory.Packages.props' console sink — declared for one project, which is gone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- .github/workflows/ci.yml | 10 +- CLAUDE.md | 41 +- Directory.Packages.props | 7 - SharpMUTerm.slnx | 2 - ...026-07-30-structured-server-data-design.md | 32 +- src/SharpMUTerm.Core/Session/WorldSession.cs | 8 + src/SharpMUTerm.Core/Telnet/Mssp/MsspData.cs | 58 +-- src/SharpMUTerm.Core/Telnet/Mssp/MsspHost.cs | 240 ---------- .../Telnet/Mssp/MsspVariables.cs | 2 - src/SharpMUTerm.Core/Telnet/TelnetSession.cs | 8 +- src/SharpMUTerm.Crawler/CommandLine.cs | 400 ---------------- src/SharpMUTerm.Crawler/CrawlOptions.cs | 214 --------- src/SharpMUTerm.Crawler/Model/CrawlModel.cs | 111 ----- src/SharpMUTerm.Crawler/MsspCrawler.cs | 323 ------------- src/SharpMUTerm.Crawler/Output/CrawlReport.cs | 297 ------------ .../Output/ObservationLog.cs | 132 ------ .../Probing/TelnetMsspProbe.cs | 178 -------- src/SharpMUTerm.Crawler/Program.cs | 115 ----- .../Scheduling/CrawlFrontier.cs | 275 ----------- .../Scheduling/CrawlRateLimiter.cs | 101 ---- src/SharpMUTerm.Crawler/SeedList.cs | 137 ------ .../SharpMUTerm.Crawler.csproj | 23 - src/SharpMUTerm.Crawler/Storage/CrawlStore.cs | 197 -------- .../Telnet}/MsspParsingTests.cs | 84 ++-- .../Telnet}/MsspWire.cs | 8 +- .../Telnet}/ScriptedTransport.cs | 4 +- .../SharpMUTerm.Crawler.Tests/BackoffTests.cs | 179 -------- .../CrawlLoopTests.cs | 360 --------------- .../PersistenceTests.cs | 431 ------------------ tests/SharpMUTerm.Crawler.Tests/ProbeTests.cs | 249 ---------- .../RateLimitTests.cs | 148 ------ .../ReferralTests.cs | 254 ----------- tests/SharpMUTerm.Crawler.Tests/SeedTests.cs | 191 -------- .../SharpMUTerm.Crawler.Tests.csproj | 19 - .../Support/FakeProbe.cs | 115 ----- .../Support/ManualTimeProvider.cs | 175 ------- 36 files changed, 115 insertions(+), 5013 deletions(-) delete mode 100644 src/SharpMUTerm.Core/Telnet/Mssp/MsspHost.cs delete mode 100644 src/SharpMUTerm.Crawler/CommandLine.cs delete mode 100644 src/SharpMUTerm.Crawler/CrawlOptions.cs delete mode 100644 src/SharpMUTerm.Crawler/Model/CrawlModel.cs delete mode 100644 src/SharpMUTerm.Crawler/MsspCrawler.cs delete mode 100644 src/SharpMUTerm.Crawler/Output/CrawlReport.cs delete mode 100644 src/SharpMUTerm.Crawler/Output/ObservationLog.cs delete mode 100644 src/SharpMUTerm.Crawler/Probing/TelnetMsspProbe.cs delete mode 100644 src/SharpMUTerm.Crawler/Program.cs delete mode 100644 src/SharpMUTerm.Crawler/Scheduling/CrawlFrontier.cs delete mode 100644 src/SharpMUTerm.Crawler/Scheduling/CrawlRateLimiter.cs delete mode 100644 src/SharpMUTerm.Crawler/SeedList.cs delete mode 100644 src/SharpMUTerm.Crawler/SharpMUTerm.Crawler.csproj delete mode 100644 src/SharpMUTerm.Crawler/Storage/CrawlStore.cs rename tests/{SharpMUTerm.Crawler.Tests => SharpMUTerm.Core.Tests/Telnet}/MsspParsingTests.cs (83%) rename tests/{SharpMUTerm.Crawler.Tests/Support => SharpMUTerm.Core.Tests/Telnet}/MsspWire.cs (91%) rename tests/{SharpMUTerm.Crawler.Tests/Support => SharpMUTerm.Core.Tests/Telnet}/ScriptedTransport.cs (98%) delete mode 100644 tests/SharpMUTerm.Crawler.Tests/BackoffTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/CrawlLoopTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/PersistenceTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/ProbeTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/RateLimitTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/ReferralTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/SeedTests.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/SharpMUTerm.Crawler.Tests.csproj delete mode 100644 tests/SharpMUTerm.Crawler.Tests/Support/FakeProbe.cs delete mode 100644 tests/SharpMUTerm.Crawler.Tests/Support/ManualTimeProvider.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index acd9795..4fe101a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,7 @@ jobs: # (the classic `dotnet test`/VSTest path is not used by MTP on .NET 10). # # Every suite carries `if: ${{ !cancelled() }}` so one failure does not hide the rest. Without it - # the job stops at the first red step, and a Core failure means Graphics through Crawler simply do + # the job stops at the first red step, and a Core failure means Graphics through Tui simply do # not run — which is how a Windows file-sharing defect in Core kept two Tui failures invisible for # days: nobody knew they were there until Core went green. A cancelled run still stops. - name: Test — Core @@ -64,14 +64,6 @@ jobs: shell: bash run: dotnet run -c Release --no-build --project tests/SharpMUTerm.Tui.Tests/SharpMUTerm.Tui.Tests.csproj - # The Crawler suite was not listed here at all — on either platform, not merely on Windows. Its 102 - # tests have never run in CI, which is why a FileShare omission in ObservationLog survived the - # sweep that fixed the same pattern in the spill, the restore log and both transcript sinks. - - name: Test — Crawler - if: ${{ !cancelled() }} - shell: bash - run: dotnet run -c Release --no-build --project tests/SharpMUTerm.Crawler.Tests/SharpMUTerm.Crawler.Tests.csproj - # End-to-end guard: the headless snapshot must render the workspace (rail, worlds, command # surface) and exit — catches both UI-render regressions and any return of the stdin-block hang. - name: Smoke — headless snapshot diff --git a/CLAUDE.md b/CLAUDE.md index a0cb78b..e0419e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,8 +131,8 @@ fallbacks) for inline images/maps. ```bash dotnet run -c Release --project tests/SharpMUTerm.Core.Tests - - - - - - - diff --git a/src/SharpMUTerm.Crawler/Storage/CrawlStore.cs b/src/SharpMUTerm.Crawler/Storage/CrawlStore.cs deleted file mode 100644 index 781dd46..0000000 --- a/src/SharpMUTerm.Crawler/Storage/CrawlStore.cs +++ /dev/null @@ -1,197 +0,0 @@ -using System.Text.Json; -using System.Text.Json.Serialization; -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; - -namespace SharpMUTerm.Crawler.Storage; - -/// -/// The crawl's memory between runs: every host it has ever heard of, how each attempt went, and when -/// each may next be attempted. -/// -/// Without this a second run is a first run: it would re-dial every host it had just visited, the -/// revisit interval would describe nothing, and a host that has refused a hundred times would be -/// refused a hundred and one. The whole of the politeness story depends on the crawler remembering, -/// so the store is written after every observation rather than at the end of a run — a crawl killed -/// half way through must not forget the half it did. -/// -/// -/// It is a plain JSON file under the run's own output directory. Never the user's configuration -/// directory: this tool reads no configuration of the client's and writes none. -/// -/// -public sealed class CrawlStore(string path) -{ - /// Bumped when the on-disk shape changes in a way an older file cannot be read as. - public const int CurrentVersion = 1; - - private static readonly JsonSerializerOptions Json = new() - { - WriteIndented = true, - // camelCase, matched case-insensitively on the way in, so a file written by an older build or - // edited by hand still loads. - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - PropertyNameCaseInsensitive = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - Converters = { new JsonStringEnumConverter() }, - }; - - public string Path { get; } = path; - - /// - /// Reads the previous run's state, or returns nothing when there is none. - /// - /// A file that cannot be parsed — a truncated write, a version from the future — is reported and - /// treated as absent rather than throwing. Losing the memory of a crawl costs one round of extra - /// politeness; refusing to start because of it costs the run. - /// - /// - public IReadOnlyList Load(out string? problem) - { - problem = null; - if (!File.Exists(Path)) - { - return []; - } - - try - { - var document = JsonSerializer.Deserialize(File.ReadAllText(Path), Json); - if (document is null) - { - problem = "the state file was empty"; - return []; - } - - if (document.Version > CurrentVersion) - { - problem = $"the state file is version {document.Version}, newer than this build understands"; - return []; - } - - return document.Hosts.Select(FromEntry).OfType().ToList(); - } - catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) - { - problem = $"the state file could not be read ({ex.GetType().Name})"; - return []; - } - } - - /// - /// Writes the state, atomically: to a temporary file beside the real one, then replaced over it. - /// A crawl that is killed mid-write must not leave a state file that cannot be read, because the - /// consequence of losing it is re-visiting everyone. - /// - public void Save(IEnumerable records) - { - var document = new StateDocument - { - Version = CurrentVersion, - SavedAt = DateTimeOffset.UtcNow, - Hosts = records.Select(ToEntry).OrderBy(entry => entry.Host, StringComparer.Ordinal).ToList(), - }; - - var directory = System.IO.Path.GetDirectoryName(System.IO.Path.GetFullPath(Path)); - if (!string.IsNullOrEmpty(directory)) - { - Directory.CreateDirectory(directory); - } - - var temporary = Path + ".tmp"; - File.WriteAllText(temporary, JsonSerializer.Serialize(document, Json)); - File.Move(temporary, Path, overwrite: true); - } - - private static HostEntry ToEntry(HostRecord record) => new() - { - Host = record.Host.Host, - Port = record.Host.Port, - Depth = record.Depth, - DiscoveredFrom = record.DiscoveredFrom?.ToReferralString(), - FirstSeen = record.FirstSeen, - LastAttempt = record.LastAttempt, - LastSuccess = record.LastSuccess, - LastOutcome = record.LastOutcome, - LastError = record.LastError, - Name = record.Name, - CrawlDelayHours = record.CrawlDelayHours, - ConsecutiveFailures = record.ConsecutiveFailures, - Attempts = record.Attempts, - NotBefore = record.NotBefore, - Retired = record.Retired, - }; - - private static HostRecord? FromEntry(HostEntry entry) - { - // Re-normalising through MsspHost.Create rather than trusting the file: a state file written by - // an older build (or edited by hand) may hold a spelling that is no longer canonical, and two - // spellings of one host in the frontier is exactly the bug the normalisation exists to prevent. - if (MsspHost.Create(entry.Host, entry.Port) is not { } host) - { - return null; - } - - MsspHost.TryParse(entry.DiscoveredFrom, out var from); - - return new HostRecord - { - Host = host, - Depth = entry.Depth, - DiscoveredFrom = from, - FirstSeen = entry.FirstSeen, - LastAttempt = entry.LastAttempt, - LastSuccess = entry.LastSuccess, - LastOutcome = entry.LastOutcome, - LastError = entry.LastError, - Name = entry.Name, - CrawlDelayHours = entry.CrawlDelayHours, - ConsecutiveFailures = entry.ConsecutiveFailures, - Attempts = entry.Attempts, - NotBefore = entry.NotBefore, - Retired = entry.Retired, - }; - } - - private sealed class StateDocument - { - public int Version { get; set; } - - public DateTimeOffset SavedAt { get; set; } - - public List Hosts { get; set; } = []; - } - - private sealed class HostEntry - { - public string Host { get; set; } = string.Empty; - - public int Port { get; set; } - - public int Depth { get; set; } - - public string? DiscoveredFrom { get; set; } - - public DateTimeOffset FirstSeen { get; set; } - - public DateTimeOffset? LastAttempt { get; set; } - - public DateTimeOffset? LastSuccess { get; set; } - - public CrawlOutcome LastOutcome { get; set; } - - public string? LastError { get; set; } - - public string? Name { get; set; } - - public double? CrawlDelayHours { get; set; } - - public int ConsecutiveFailures { get; set; } - - public int Attempts { get; set; } - - public DateTimeOffset? NotBefore { get; set; } - - public bool Retired { get; set; } - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/MsspParsingTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs similarity index 83% rename from tests/SharpMUTerm.Crawler.Tests/MsspParsingTests.cs rename to tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs index 94dc989..fad51ce 100644 --- a/tests/SharpMUTerm.Crawler.Tests/MsspParsingTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs @@ -1,36 +1,35 @@ using System.Text; +using SharpMUTerm.Core.Telnet; using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Probing; -using SharpMUTerm.Crawler.Tests.Support; using TelnetNegotiationCore.Models; -namespace SharpMUTerm.Crawler.Tests; +namespace SharpMUTerm.Core.Tests.Telnet; /// /// What a server's MSSP report turns into, against payloads built byte by byte from the -/// specification's own format and fed through a real telnet session. +/// specification's own format and fed through a real . /// -/// There is no MSSP parser in this repository any more. These cases used to pin one, written -/// because TelnetNegotiationCore's reader destroyed arrays, booleans and unknown variables before any +/// There is no MSSP parser in this repository. These cases used to pin one, written because +/// TelnetNegotiationCore's reader destroyed arrays, booleans and unknown variables before any /// consumer saw them. That is fixed upstream (2.6.5), so the same cases now run against the library's /// own MSSPConfig.Variables by way of , and prove the replacement keeps -/// what the workaround kept. The two that diverge are named as such and say why. +/// what the workaround kept. The two that diverge from the old behaviour are named as such +/// and say why. +/// +/// +/// The session is built the way a world's is — +/// carrying , so the client asks rather than waits — and +/// the scripted server answers only a client that asked. These are therefore the pins for the INFO +/// screen's supply as much as for the model. /// /// public class MsspParsingTests { - private static MsspHost Host => MsspHost.Create("server.example.org", 4201)!; - - private static CrawlOptions Options => new() - { - ConnectTimeout = TimeSpan.FromSeconds(5), - MsspTimeout = TimeSpan.FromSeconds(5), - }; + private static readonly TimeSpan Patience = TimeSpan.FromSeconds(5); /// - /// One server, one report: a scripted server offers MSSP, answers the crawler's DO with - /// , and the report the probe came back with is returned. + /// One server, one report: a scripted server offers MSSP, answers the client's DO with + /// , and the report the session raised is returned. /// private static Task Read(params (string Variable, string[] Values)[] entries) => ReadRaw(MsspWire.Subnegotiation(entries)); @@ -40,11 +39,28 @@ private static async Task ReadRaw(byte[] payload, bool fragmented = fa var transport = new ScriptedTransport { Greeting = MsspWire.Offer(), Fragmented = fragmented } .RespondingToDo(MsspWire.Mssp, payload); - var result = await new TelnetMsspProbe(Options, _ => transport).ProbeAsync(Host, CancellationToken.None); + return await ReadFrom(transport) + ?? throw new InvalidOperationException("The session raised no MSSP report."); + } - return result.Outcome == CrawlOutcome.MsspReceived && result.Data is { } data - ? data - : throw new InvalidOperationException($"No MSSP report: {result.Outcome} ({result.Error})."); + /// + /// Connects a real session to and returns the first MSSP report it + /// raises, or null when none arrives inside . Null is a result here + /// rather than a failure: "this server publishes no MSSP" is one of the three states the INFO + /// screen has to tell apart, and one case below has that as its whole subject. + /// + private static async Task ReadFrom(ScriptedTransport transport, TimeSpan? patience = null) + { + var received = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var session = new TelnetSession( + transport, + options: new TelnetSessionOptions { RequestOptions = [TelnetSessionOptions.MsspOption] }); + session.MsspReceived += (_, e) => received.TrySetResult(e.Data); + + await session.ConnectAsync(); + + var completed = await Task.WhenAny(received.Task, Task.Delay(patience ?? Patience)); + return completed == received.Task ? await received.Task : null; } [Test] @@ -77,11 +93,11 @@ public async Task ArrayNotationKeepsEveryValueInOrderWithTheDefaultLast() [Test] public async Task TheReferralListArrivesAsAList() { - // REFERRAL is array-only and is what a crawler follows; 2.6.0 delivered it as null. + // REFERRAL is array-only; 2.6.0 delivered it as null. Nothing reads it by name any more — the + // INFO screen renders it among the rest — so what must survive is the array, whole and in order. var data = await Read(("REFERRAL", ["a.example.org 4000", "b.example.net 23", "2001:db8::5 4201"])); - await Assert.That(data["REFERRAL"]).Count().IsEqualTo(3); - await Assert.That(data.Referrals.Select(r => r.ToReferralString())) + await Assert.That(data["REFERRAL"]) .IsEquivalentTo(new[] { "a.example.org 4000", "b.example.net 23", "2001:db8::5 4201" }); } @@ -184,16 +200,7 @@ public async Task VariableNamesAreMatchedWithoutRegardToCaseOrStrayWhitespace() var data = await Read((" crawl delay ", ["11"])); await Assert.That(data.ContainsKey("CRAWL DELAY")).IsTrue(); - await Assert.That(data.CrawlDelay).IsEqualTo(TimeSpan.FromHours(11)); - } - - [Test] - public async Task ACrawlDelayOfMinusOneMeansNoPreferenceRatherThanANegativeInterval() - { - // "Send -1 to use the crawler's default." The library's own Integer() hands -1 back as-is, - // deliberately; the reading that a scheduler can use is this projection's. - await Assert.That((await Read(("CRAWL DELAY", ["-1"]))).CrawlDelay).IsNull(); - await Assert.That((await Read(("CRAWL DELAY", ["23"]))).CrawlDelay).IsEqualTo(TimeSpan.FromHours(23)); + await Assert.That(data.Default("crawl_delay")).IsEqualTo("11"); } [Test] @@ -246,7 +253,7 @@ public async Task APayloadSplitAcrossReadsParsesTheSameAsAWholeOne() await Assert.That(data.Name).IsEqualTo("Corvid Nest"); await Assert.That(data.Ports).IsEquivalentTo(new[] { 80, 23, 4201 }); - await Assert.That(data.Referrals.Single().ToReferralString()).IsEqualTo("a.example.org 4000"); + await Assert.That(data["REFERRAL"]).IsEquivalentTo(new[] { "a.example.org 4000" }); } [Test] @@ -290,12 +297,7 @@ public async Task AnUnterminatedPayloadYieldsNothingRatherThanAPartialReport() var transport = new ScriptedTransport { Greeting = MsspWire.Offer() } .RespondingToDo(MsspWire.Mssp, whole.AsSpan(0, whole.Length - 2).ToArray()); - var result = await new TelnetMsspProbe( - Options with { MsspTimeout = TimeSpan.FromMilliseconds(400) }, _ => transport) - .ProbeAsync(Host, CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.NoMssp); - await Assert.That(result.Data).IsNull(); + await Assert.That(await ReadFrom(transport, TimeSpan.FromMilliseconds(400))).IsNull(); } [Test] diff --git a/tests/SharpMUTerm.Crawler.Tests/Support/MsspWire.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspWire.cs similarity index 91% rename from tests/SharpMUTerm.Crawler.Tests/Support/MsspWire.cs rename to tests/SharpMUTerm.Core.Tests/Telnet/MsspWire.cs index 046764a..af81d01 100644 --- a/tests/SharpMUTerm.Crawler.Tests/Support/MsspWire.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspWire.cs @@ -1,7 +1,7 @@ using System.Text; using SharpMUTerm.Core.Telnet.Mssp; -namespace SharpMUTerm.Crawler.Tests.Support; +namespace SharpMUTerm.Core.Tests.Telnet; /// /// Builds MSSP subnegotiations byte by byte, exactly as the specification spells them, so a test @@ -46,9 +46,9 @@ public static byte[] Subnegotiation(params (string Variable, string[] Values)[] /// /// The same entries as an , without a socket. For the tests whose subject is - /// what the crawler does with a report — scheduling, referral following, persistence — - /// rather than how the report was read. Reading it is the telnet layer's job and is pinned, once, - /// by MsspParsingTests driving a real session. + /// what something does with a report — caching it, rendering it — rather than how it was + /// read. Reading it is the telnet layer's job and is pinned, once, by MsspParsingTests + /// driving a real session. /// public static MsspData Report(params (string Variable, string[] Values)[] entries) => MsspData.From(entries.Select(entry => diff --git a/tests/SharpMUTerm.Crawler.Tests/Support/ScriptedTransport.cs b/tests/SharpMUTerm.Core.Tests/Telnet/ScriptedTransport.cs similarity index 98% rename from tests/SharpMUTerm.Crawler.Tests/Support/ScriptedTransport.cs rename to tests/SharpMUTerm.Core.Tests/Telnet/ScriptedTransport.cs index a6d4876..2629a6c 100644 --- a/tests/SharpMUTerm.Crawler.Tests/Support/ScriptedTransport.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/ScriptedTransport.cs @@ -1,7 +1,7 @@ using System.Threading.Channels; using SharpMUTerm.Core.Transport; -namespace SharpMUTerm.Crawler.Tests.Support; +namespace SharpMUTerm.Core.Tests.Telnet; /// /// An in-memory transport standing in for a server: queue bytes to be delivered, capture everything @@ -10,7 +10,7 @@ namespace SharpMUTerm.Crawler.Tests.Support; /// This is a *server*, not just a pipe. The MSSP handshake is a conversation — the server says /// IAC WILL MSSP and only sends its report once the client has answered IAC DO MSSP — so /// a transport that replayed a fixed script would test the telnet layer's reader against bytes no real -/// server would have sent yet, and would never exercise the negotiation the crawler depends on. +/// server would have sent yet, and would never exercise the negotiation the client depends on. /// /// internal sealed class ScriptedTransport : ITransport diff --git a/tests/SharpMUTerm.Crawler.Tests/BackoffTests.cs b/tests/SharpMUTerm.Crawler.Tests/BackoffTests.cs deleted file mode 100644 index 1ad12eb..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/BackoffTests.cs +++ /dev/null @@ -1,179 +0,0 @@ -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Scheduling; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// What the crawler remembers about a host that failed, and when it is willing to try again. -/// -public class BackoffTests -{ - private static readonly DateTimeOffset Now = new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); - - private static MsspHost Host(string name = "a.example.org", int port = 4201) => MsspHost.Create(name, port)!; - - private static ProbeResult Result(MsspHost host, CrawlOutcome outcome, MsspData? data = null, DateTimeOffset? at = null) => - new() - { - Host = host, - Outcome = outcome, - ObservedAt = at ?? Now, - Data = data, - Error = outcome is CrawlOutcome.ConnectFailed ? "ConnectionRefused" : null, - }; - - private static (CrawlFrontier Frontier, MsspHost Host) Seeded(CrawlOptions? options = null) - { - var frontier = new CrawlFrontier(options ?? new CrawlOptions()); - var host = Host(); - frontier.AddSeed(host, Now); - return (frontier, host); - } - - [Test] - public async Task ARefusedConnectionIsRecordedAndNotRetriedImmediately() - { - var (frontier, host) = Seeded(); - - frontier.Record(Result(host, CrawlOutcome.ConnectFailed)); - var record = frontier.Records.Single(); - - await Assert.That(record.ConsecutiveFailures).IsEqualTo(1); - await Assert.That(record.LastOutcome).IsEqualTo(CrawlOutcome.ConnectFailed); - await Assert.That(record.LastError).IsEqualTo("ConnectionRefused"); - await Assert.That(record.NotBefore).IsEqualTo(Now + TimeSpan.FromHours(1)); - await Assert.That(record.IsDue(Now)).IsFalse(); - await Assert.That(record.IsDue(Now + TimeSpan.FromHours(1))).IsTrue(); - - // And the frontier will not hand it out again while it is not due. - await Assert.That(frontier.TakeNext(Now)).IsNull(); - } - - [Test] - public async Task EachFurtherFailureWaitsLonger() - { - var (frontier, host) = Seeded(); - var expected = new[] { 1d, 6d, 24d, 72d }; - - var at = Now; - for (var attempt = 0; attempt < expected.Length; attempt++) - { - frontier.Record(Result(host, CrawlOutcome.ConnectFailed, at: at)); - var record = frontier.Records.Single(); - await Assert.That(record.NotBefore).IsEqualTo(at + TimeSpan.FromHours(expected[attempt])); - - at = record.NotBefore!.Value; - // Re-claim it, the way the crawl loop does, so the next Record has something to release. - frontier.TakeNext(at); - } - } - - [Test] - public async Task RepeatedFailuresRetireAHostEntirely() - { - var (frontier, host) = Seeded(new CrawlOptions { RetireAfterFailures = 3 }); - - for (var i = 0; i < 3; i++) - { - frontier.Record(Result(host, CrawlOutcome.ConnectFailed)); - frontier.TakeNext(Now + TimeSpan.FromDays(30)); - } - - var record = frontier.Records.Single(); - await Assert.That(record.Retired).IsTrue(); - await Assert.That(record.IsDue(Now + TimeSpan.FromDays(365))).IsFalse(); - - // Retired means retired: the frontier never offers it again, however long a run waits. - await Assert.That(frontier.TakeNext(Now + TimeSpan.FromDays(365))).IsNull(); - } - - [Test] - public async Task OneSuccessClearsTheFailureCount() - { - var (frontier, host) = Seeded(); - - frontier.Record(Result(host, CrawlOutcome.ConnectFailed)); - frontier.TakeNext(Now + TimeSpan.FromHours(2)); - frontier.Record(Result(host, CrawlOutcome.MsspReceived, MsspData.Empty, Now + TimeSpan.FromHours(2))); - - var record = frontier.Records.Single(); - await Assert.That(record.ConsecutiveFailures).IsEqualTo(0); - await Assert.That(record.LastSuccess).IsEqualTo(Now + TimeSpan.FromHours(2)); - } - - [Test] - public async Task AServerWithNoMsspIsNotAFailureButWaitsMuchLonger() - { - // Most MU* servers do not implement MSSP. That is not a fault of theirs and must not retire - // them, but asking again tomorrow would be pure cost to both sides. - var (frontier, host) = Seeded(); - - frontier.Record(Result(host, CrawlOutcome.NoMssp)); - var record = frontier.Records.Single(); - - await Assert.That(record.ConsecutiveFailures).IsEqualTo(0); - await Assert.That(record.Retired).IsFalse(); - await Assert.That(record.NotBefore).IsEqualTo(Now + TimeSpan.FromDays(7)); - } - - [Test] - public async Task ASuccessfulCrawlSetsTheRevisitInterval() - { - var (frontier, host) = Seeded(); - - frontier.Record(Result(host, CrawlOutcome.MsspReceived, MsspData.Empty)); - var record = frontier.Records.Single(); - - await Assert.That(record.NotBefore).IsEqualTo(Now + TimeSpan.FromHours(24)); - await Assert.That(record.IsDue(Now + TimeSpan.FromHours(23))).IsFalse(); - await Assert.That(record.IsDue(Now + TimeSpan.FromHours(24))).IsTrue(); - } - - [Test] - public async Task AServerAskingForALongerCrawlDelayGetsIt() - { - // "CRAWL DELAY — Preferred minimum number of hours between crawls." A server asking for more - // than our default is asking politely and is obeyed. - var options = new CrawlOptions { RevisitInterval = TimeSpan.FromHours(6) }; - var (frontier, host) = Seeded(options); - - var data = MsspWire.Report(("CRAWL DELAY", ["23"])); - - frontier.Record(Result(host, CrawlOutcome.MsspReceived, data)); - - await Assert.That(frontier.Records.Single().NotBefore).IsEqualTo(Now + TimeSpan.FromHours(23)); - await Assert.That(frontier.Records.Single().CrawlDelayHours).IsEqualTo(23); - } - - [Test] - public async Task AServerAskingForAShorterCrawlDelayDoesNotGetVisitedMoreOften() - { - // It is a minimum a server asks for, not a permission it grants. Our own politeness setting is - // not theirs to lower. - var options = new CrawlOptions { RevisitInterval = TimeSpan.FromHours(24) }; - var (frontier, host) = Seeded(options); - - var data = MsspWire.Report(("CRAWL DELAY", ["1"])); - - frontier.Record(Result(host, CrawlOutcome.MsspReceived, data)); - - await Assert.That(frontier.Records.Single().NotBefore).IsEqualTo(Now + TimeSpan.FromHours(24)); - } - - [Test] - public async Task ADryRunDoesNotMoveTheSchedule() - { - // Checking a configuration must not silently postpone the crawl it was checking. - var (frontier, host) = Seeded(); - - frontier.Record(Result(host, CrawlOutcome.Skipped)); - var record = frontier.Records.Single(); - - await Assert.That(record.Attempts).IsEqualTo(0); - await Assert.That(record.LastAttempt).IsNull(); - await Assert.That(record.NotBefore).IsNull(); - await Assert.That(record.IsDue(Now)).IsTrue(); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/CrawlLoopTests.cs b/tests/SharpMUTerm.Crawler.Tests/CrawlLoopTests.cs deleted file mode 100644 index 187ff02..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/CrawlLoopTests.cs +++ /dev/null @@ -1,360 +0,0 @@ -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Output; -using SharpMUTerm.Crawler.Scheduling; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// The crawl loop, against a scripted probe. Time is virtual throughout — the loop really does wait on -/// its rate limiter, and here waiting moves the clock instead of the thread. -/// -public class CrawlLoopTests -{ - private static MsspHost Host(string name, int port = 4201) => MsspHost.Create(name, port)!; - - /// A configuration with the politeness intervals at zero, for tests that are not about them. - private static CrawlOptions Fast(CrawlOptions? from = null) => (from ?? new CrawlOptions()) with - { - GlobalInterval = TimeSpan.Zero, - PerHostInterval = TimeSpan.Zero, - MaxConcurrency = 1, - }; - - [Test] - public async Task ACrawlFollowsReferralsOutwardsFromItsSeed() - { - var time = new VirtualTimeProvider(); - var a = Host("a.example.org"); - var b = Host("b.example.org"); - var c = Host("c.example.org"); - - var probe = new FakeProbe(time).Referring(a, b).Referring(b, c); - var options = Fast(); - var frontier = new CrawlFrontier(options); - frontier.AddSeed(a, time.GetUtcNow()); - - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(probe.Visited).IsEquivalentTo(new[] { a, b, c }); - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.Exhausted); - await Assert.That(frontier.Records.Count).IsEqualTo(3); - } - - [Test] - public async Task ACycleOfReferralsVisitsEachServerExactlyOnce() - { - // A ↔ B and both pointing at C, which points back at A. Every arrow is followed, no host is - // dialled twice, and the run ends. - var time = new VirtualTimeProvider(); - var a = Host("a.example.org"); - var b = Host("b.example.org"); - var c = Host("c.example.org"); - - var probe = new FakeProbe(time) - .Referring(a, b, c) - .Referring(b, a, c) - .Referring(c, a, b); - - var options = Fast(); - var frontier = new CrawlFrontier(options); - frontier.AddSeed(a, time.GetUtcNow()); - - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(probe.Visited.Count).IsEqualTo(3); - await Assert.That(probe.Visited.Distinct().Count()).IsEqualTo(3); - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.Exhausted); - await Assert.That(summary.Verdicts[DiscoveryVerdict.AlreadyKnown]).IsGreaterThan(0); - } - - [Test] - public async Task TheHostCapStopsTheRunEvenWithReferralsLeftToFollow() - { - // Each server refers to two more, so the frontier grows faster than it is consumed and only the - // cap can end this. - var time = new VirtualTimeProvider(); - var probe = new FakeProbe(time); - for (var i = 0; i < 200; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i * 2 + 1}.example.org"), Host($"h{i * 2 + 2}.example.org")); - } - - var options = Fast() with { MaxHosts = 7, MaxDepth = 99 }; - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("h0.example.org"), time.GetUtcNow()); - - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.HostCap); - await Assert.That(probe.Visited.Count).IsEqualTo(7); - await Assert.That(summary.Contacted).IsEqualTo(7); - - // The frontier still knows about the ones it did not reach, so the next run picks them up. - await Assert.That(frontier.Records.Count).IsGreaterThan(7); - } - - [Test] - public async Task TheHostCapIsNeverExceededEvenWhenSeveralWorkersRace() - { - var time = new VirtualTimeProvider(); - var probe = new FakeProbe(time); - for (var i = 0; i < 200; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i * 2 + 1}.example.org"), Host($"h{i * 2 + 2}.example.org")); - } - - var options = Fast() with { MaxHosts = 10, MaxDepth = 99, MaxConcurrency = 8 }; - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("h0.example.org"), time.GetUtcNow()); - - await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(probe.Visited.Count).IsEqualTo(10); - } - - [Test] - public async Task ACapDoesNotAbortAConnectionAlreadyInFlight() - { - // Found by watching a real run: the cap used to cancel the crawl's token, which killed the probes - // already talking to a server. The data was thrown away, the server was left with a connection - // dropped mid-handshake, and the host was recorded as never attempted — so the next run dialled - // it again. A cap must stop the crawl starting anything new and nothing else. - var time = new VirtualTimeProvider(); - var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var probe = new FakeProbe(time) { Gate = gate }; - for (var i = 0; i < 50; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i * 2 + 1}.example.org"), Host($"h{i * 2 + 2}.example.org")); - } - - var options = Fast() with { MaxHosts = 3, MaxDepth = 99, MaxConcurrency = 3 }; - var frontier = new CrawlFrontier(options); - for (var i = 0; i < 3; i++) - { - frontier.AddSeed(Host($"h{i}.example.org"), time.GetUtcNow()); - } - - var run = new MsspCrawler(options, probe, frontier, time).RunAsync(); - - // All three workers are inside a probe when the cap is reached. - await Task.Delay(150); - gate.SetResult(); - var summary = await run; - - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.HostCap); - await Assert.That(probe.Visited.Count).IsEqualTo(3); - - // Every probe that started was allowed to finish and was recorded as a real observation. - await Assert.That(summary.Results.Count).IsEqualTo(3); - await Assert.That(summary.Results.All(r => r.Outcome == CrawlOutcome.MsspReceived)).IsTrue(); - await Assert.That(summary.Contacted).IsEqualTo(3); - } - - [Test] - public async Task TheDepthCapStopsAChainOfReferrals() - { - var time = new VirtualTimeProvider(); - var probe = new FakeProbe(time); - for (var i = 0; i < 20; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i + 1}.example.org")); - } - - var options = Fast() with { MaxDepth = 2, MaxHosts = 1000 }; - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("h0.example.org"), time.GetUtcNow()); - - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - // The seed plus two hops. - await Assert.That(probe.Visited.Count).IsEqualTo(3); - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.Exhausted); - await Assert.That(summary.Verdicts[DiscoveryVerdict.TooDeep]).IsEqualTo(1); - } - - [Test] - public async Task TheTimeCapStopsTheRun() - { - // With a one-second global interval and a ten-second budget, the rate limiter alone runs the - // clock out. Virtual time makes that instant and exact. - var time = new VirtualTimeProvider(); - var probe = new FakeProbe(time); - for (var i = 0; i < 500; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i * 2 + 1}.example.org"), Host($"h{i * 2 + 2}.example.org")); - } - - var options = new CrawlOptions - { - GlobalInterval = TimeSpan.FromSeconds(1), - PerHostInterval = TimeSpan.Zero, - MaxConcurrency = 1, - MaxDuration = TimeSpan.FromSeconds(10), - MaxHosts = 10_000, - MaxDepth = 99, - }; - - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("h0.example.org"), time.GetUtcNow()); - - var started = time.GetUtcNow(); - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.TimeCap); - await Assert.That(time.GetUtcNow() - started).IsGreaterThanOrEqualTo(TimeSpan.FromSeconds(10)); - - // Roughly one host per second of budget, which is the rate limiter doing its job inside the loop - // rather than the loop merely being told about it. - await Assert.That(probe.Visited.Count).IsLessThanOrEqualTo(12); - } - - [Test] - public async Task TheConcurrencyCapIsRespected() - { - var time = new VirtualTimeProvider(); - var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var probe = new FakeProbe(time) { Gate = gate }; - - var seedOptions = Fast() with { MaxConcurrency = 3, MaxHosts = 100 }; - var frontier = new CrawlFrontier(seedOptions); - for (var i = 0; i < 20; i++) - { - frontier.AddSeed(Host($"h{i}.example.org"), time.GetUtcNow()); - } - - var run = new MsspCrawler(seedOptions, probe, frontier, time).RunAsync(); - - // Let the workers pile up against the gate, then release them. - await Task.Delay(150); - var peakWhileBlocked = probe.PeakConcurrency; - gate.SetResult(); - await run; - - await Assert.That(peakWhileBlocked).IsEqualTo(3); - await Assert.That(probe.PeakConcurrency).IsLessThanOrEqualTo(3); - await Assert.That(probe.Visited.Count).IsEqualTo(20); - } - - [Test] - public async Task ARunResumesWhereThePreviousOneStopped() - { - var time = new VirtualTimeProvider(); - var options = Fast() with { MaxHosts = 2, MaxDepth = 99 }; - - var probe = new FakeProbe(time); - for (var i = 0; i < 20; i++) - { - probe.Referring(Host($"h{i}.example.org"), Host($"h{i + 1}.example.org")); - } - - // First run: stopped by the host cap after two. - var first = new CrawlFrontier(options); - first.AddSeed(Host("h0.example.org"), time.GetUtcNow()); - IReadOnlyCollection saved = []; - var firstSummary = await new MsspCrawler(options, probe, first, time, persist: records => saved = records) - .RunAsync(); - - await Assert.That(firstSummary.StopReason).IsEqualTo(CrawlStopReason.HostCap); - await Assert.That(probe.Visited).IsEquivalentTo(new[] { Host("h0.example.org"), Host("h1.example.org") }); - - // Second run, from the saved state and with no seeds at all: it must continue rather than start - // over, and must not re-dial the two it has just visited. - var second = new CrawlFrontier(options, saved); - var resumed = new FakeProbe(time); - for (var i = 0; i < 20; i++) - { - resumed.Referring(Host($"h{i}.example.org"), Host($"h{i + 1}.example.org")); - } - - await new MsspCrawler(options, resumed, second, time).RunAsync(); - - await Assert.That(resumed.Visited).IsEquivalentTo(new[] { Host("h2.example.org"), Host("h3.example.org") }); - } - - [Test] - public async Task ARunWithNothingDueContactsNobody() - { - // The whole point of the revisit interval: a second run started straight after the first must be - // a no-op, not a second pass over the same servers. - var time = new VirtualTimeProvider(); - var options = Fast() with { MaxHosts = 100 }; - - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("a.example.org"), time.GetUtcNow()); - var probe = new FakeProbe(time).Referring(Host("a.example.org")); - - IReadOnlyCollection saved = []; - await new MsspCrawler(options, probe, frontier, time, persist: records => saved = records).RunAsync(); - await Assert.That(probe.Visited.Count).IsEqualTo(1); - - var again = new FakeProbe(time); - var summary = await new MsspCrawler(options, again, new CrawlFrontier(options, saved), time).RunAsync(); - - await Assert.That(again.Visited).IsEmpty(); - await Assert.That(summary.Contacted).IsEqualTo(0); - - // …and once the interval has passed, it is due again. - time.Advance(TimeSpan.FromHours(25)); - var later = new FakeProbe(time); - await new MsspCrawler(options, later, new CrawlFrontier(options, saved), time).RunAsync(); - await Assert.That(later.Visited.Count).IsEqualTo(1); - } - - [Test] - public async Task ADryRunContactsNobodyAndLeavesTheScheduleAlone() - { - var time = new VirtualTimeProvider(); - var options = Fast() with { DryRun = true }; - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("a.example.org"), time.GetUtcNow()); - - var probe = new FakeProbe(time); - var summary = await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(probe.Visited).IsEmpty(); - await Assert.That(summary.Contacted).IsEqualTo(0); - await Assert.That(frontier.Records.Single().IsDue(time.GetUtcNow())).IsTrue(); - } - - [Test] - public async Task AFailingHostBacksOffAndIsNotRetriedWithinTheSameRun() - { - var time = new VirtualTimeProvider(); - var options = Fast() with { MaxHosts = 100 }; - var host = Host("dead.example.org"); - - var frontier = new CrawlFrontier(options); - frontier.AddSeed(host, time.GetUtcNow()); - var probe = new FakeProbe(time).Answering(host, CrawlOutcome.ConnectFailed, "ConnectionRefused"); - - await new MsspCrawler(options, probe, frontier, time).RunAsync(); - - await Assert.That(probe.Visited.Count).IsEqualTo(1); - var record = frontier.Records.Single(); - await Assert.That(record.ConsecutiveFailures).IsEqualTo(1); - await Assert.That(record.LastError).IsEqualTo("ConnectionRefused"); - await Assert.That(record.NotBefore).IsNotNull(); - } - - [Test] - public async Task AProbeThatThrowsIsRecordedRatherThanTakingTheRunDown() - { - var time = new VirtualTimeProvider(); - var options = Fast(); - var frontier = new CrawlFrontier(options); - frontier.AddSeed(Host("a.example.org"), time.GetUtcNow()); - - var summary = await new MsspCrawler(options, new ThrowingProbe(), frontier, time).RunAsync(); - - await Assert.That(summary.Results.Single().Outcome).IsEqualTo(CrawlOutcome.Error); - await Assert.That(summary.StopReason).IsEqualTo(CrawlStopReason.Exhausted); - } - - private sealed class ThrowingProbe : SharpMUTerm.Crawler.Probing.IMsspProbe - { - public Task ProbeAsync(MsspHost host, CancellationToken cancellationToken) => - throw new InvalidOperationException("a bug in the probe"); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/PersistenceTests.cs b/tests/SharpMUTerm.Crawler.Tests/PersistenceTests.cs deleted file mode 100644 index 20f0579..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/PersistenceTests.cs +++ /dev/null @@ -1,431 +0,0 @@ -using System.Text.Json; -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Output; -using SharpMUTerm.Crawler.Scheduling; -using SharpMUTerm.Crawler.Storage; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// The state file and the two outputs. Everything here writes into a temporary directory of its own -/// and deletes it: this tool must never write anywhere near the user's configuration. -/// -public class PersistenceTests -{ - private static readonly DateTimeOffset Now = new(2026, 1, 1, 12, 0, 0, TimeSpan.Zero); - - private static MsspHost Host(string name, int port = 4201) => MsspHost.Create(name, port)!; - - private static string Scratch() - { - var path = Path.Combine(Path.GetTempPath(), "sharpmuterm-crawler-tests", Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(path); - return path; - } - - [Test] - public async Task StateSurvivesARoundTripSoASecondRunResumes() - { - var directory = Scratch(); - try - { - var store = new CrawlStore(Path.Combine(directory, "state.json")); - var records = new[] - { - new HostRecord - { - Host = Host("a.example.org"), - Depth = 0, - FirstSeen = Now, - LastAttempt = Now, - LastSuccess = Now, - LastOutcome = CrawlOutcome.MsspReceived, - Name = "Corvid Nest", - CrawlDelayHours = 5, - Attempts = 3, - NotBefore = Now + TimeSpan.FromHours(24), - }, - new HostRecord - { - Host = Host("b.example.org", 4000), - Depth = 2, - DiscoveredFrom = Host("a.example.org"), - FirstSeen = Now, - LastOutcome = CrawlOutcome.ConnectFailed, - LastError = "ConnectionRefused", - ConsecutiveFailures = 2, - NotBefore = Now + TimeSpan.FromHours(6), - }, - new HostRecord - { - Host = Host("dead.example.org"), - FirstSeen = Now, - Retired = true, - ConsecutiveFailures = 5, - }, - }; - - store.Save(records); - var loaded = store.Load(out var problem); - - await Assert.That(problem).IsNull(); - await Assert.That(loaded.Count).IsEqualTo(3); - - var a = loaded.Single(r => r.Host == Host("a.example.org")); - await Assert.That(a.Name).IsEqualTo("Corvid Nest"); - await Assert.That(a.CrawlDelayHours).IsEqualTo(5); - await Assert.That(a.NotBefore).IsEqualTo(Now + TimeSpan.FromHours(24)); - await Assert.That(a.IsDue(Now)).IsFalse(); - await Assert.That(a.IsDue(Now + TimeSpan.FromHours(25))).IsTrue(); - - var b = loaded.Single(r => r.Host == Host("b.example.org", 4000)); - await Assert.That(b.DiscoveredFrom).IsEqualTo(Host("a.example.org")); - await Assert.That(b.ConsecutiveFailures).IsEqualTo(2); - - var retired = loaded.Single(r => r.Host == Host("dead.example.org")); - await Assert.That(retired.Retired).IsTrue(); - await Assert.That(retired.IsDue(Now + TimeSpan.FromDays(365))).IsFalse(); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task AHostSpelledDifferentlyInTheFileIsNormalisedOnTheWayBackIn() - { - // A state file written by an older build, or edited by hand. Two spellings of one host in the - // frontier is exactly the duplicate the normalisation exists to prevent. - var directory = Scratch(); - try - { - var path = Path.Combine(directory, "state.json"); - File.WriteAllText(path, """ - { - "version": 1, - "savedAt": "2026-01-01T00:00:00+00:00", - "hosts": [ - { "host": "A.Example.ORG.", "port": 4201, "firstSeen": "2026-01-01T00:00:00+00:00" }, - { "host": "2001:0DB8:0000::0001", "port": 4201, "firstSeen": "2026-01-01T00:00:00+00:00" }, - { "host": "", "port": 4201, "firstSeen": "2026-01-01T00:00:00+00:00" } - ] - } - """); - - var loaded = new CrawlStore(path).Load(out var problem); - - await Assert.That(problem).IsNull(); - - // The unusable entry is dropped rather than carried as a host that can never be dialled. - await Assert.That(loaded.Count).IsEqualTo(2); - await Assert.That(loaded.Select(r => r.Host.Host)) - .IsEquivalentTo(new[] { "a.example.org", "2001:db8::1" }); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task AnUnreadableStateFileIsReportedAndTreatedAsAbsent() - { - var directory = Scratch(); - try - { - var path = Path.Combine(directory, "state.json"); - File.WriteAllText(path, "{ this is not json"); - - var loaded = new CrawlStore(path).Load(out var problem); - - await Assert.That(loaded).IsEmpty(); - await Assert.That(problem).IsNotNull(); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task AStateFileFromAFutureVersionIsRefusedRatherThanMisread() - { - var directory = Scratch(); - try - { - var path = Path.Combine(directory, "state.json"); - File.WriteAllText(path, """{ "version": 99, "hosts": [] }"""); - - new CrawlStore(path).Load(out var problem); - await Assert.That(problem).Contains("newer than this build"); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task TheObservationLogRecordsWhenEachRecordWasSeenAndKeepsEveryArray() - { - var directory = Scratch(); - try - { - var path = Path.Combine(directory, "observations.jsonl"); - var data = MsspWire.Report(MsspWire.RepresentativeReport("peer.example.net 4000")); - - using (var log = new ObservationLog(path)) - { - log.Append(new ProbeResult - { - Host = Host("a.example.org"), - Outcome = CrawlOutcome.MsspReceived, - ObservedAt = Now, - Duration = TimeSpan.FromMilliseconds(412), - Data = data, - }); - - log.Append(new ProbeResult - { - Host = Host("b.example.org"), - Outcome = CrawlOutcome.ConnectFailed, - ObservedAt = Now + TimeSpan.FromSeconds(2), - Error = "ConnectionRefused", - }); - } - - var lines = File.ReadAllLines(path); - await Assert.That(lines.Length).IsEqualTo(2); - - using var first = JsonDocument.Parse(lines[0]); - var root = first.RootElement; - - // MSSP goes stale; a record without a timestamp is data of unknown age. - await Assert.That(root.GetProperty("observedAt").GetDateTimeOffset()).IsEqualTo(Now); - await Assert.That(root.GetProperty("host").GetString()).IsEqualTo("a.example.org"); - await Assert.That(root.GetProperty("players").GetInt32()).IsEqualTo(17); - - // Arrays survive to the file, which is the whole reason the model holds lists. - var ports = root.GetProperty("variables").GetProperty("PORT").EnumerateArray() - .Select(v => v.GetString() ?? string.Empty).ToArray(); - await Assert.That(ports).IsEquivalentTo(new[] { "80", "23", "4201" }); - - // Including the variables no model knows about. - await Assert.That(root.GetProperty("variables").GetProperty("CORVID SPECIFIC")[0].GetString()) - .IsEqualTo("nevermore"); - - using var second = JsonDocument.Parse(lines[1]); - await Assert.That(second.RootElement.GetProperty("outcome").GetString()).IsEqualTo("ConnectFailed"); - await Assert.That(second.RootElement.GetProperty("error").GetString()).IsEqualTo("ConnectionRefused"); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task NeitherOutputStartsWithAByteOrderMark() - { - // Found by actually reading the files a run produced: Encoding.UTF8 emits a BOM, and a BOM at the - // head of a JSON-lines file breaks the first record for every consumer that does not know to - // strip it — Python's own json module among them. The Markdown grows a stray glyph before its - // first heading. Both are only visible if somebody opens the file, so they get a test. - var directory = Scratch(); - try - { - var observations = Path.Combine(directory, "observations.jsonl"); - using (var log = new ObservationLog(observations)) - { - log.Append(new ProbeResult - { - Host = Host("a.example.org"), - Outcome = CrawlOutcome.NoMssp, - ObservedAt = Now, - }); - } - - var report = Path.Combine(directory, "report.md"); - CrawlReport.Write(report, new CrawlSummary - { - StartedAt = Now, - FinishedAt = Now, - StopReason = CrawlStopReason.Exhausted, - Results = [], - Hosts = [], - Verdicts = new Dictionary(), - }, new CrawlOptions()); - - byte[] bom = [0xEF, 0xBB, 0xBF]; - await Assert.That(File.ReadAllBytes(observations).Take(3)).IsNotEquivalentTo(bom); - await Assert.That(File.ReadAllBytes(report).Take(3)).IsNotEquivalentTo(bom); - - // And the record really does parse as JSON when read straight off the first byte. - using var parsed = JsonDocument.Parse(File.ReadAllText(observations)); - await Assert.That(parsed.RootElement.GetProperty("host").GetString()).IsEqualTo("a.example.org"); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task TheObservationLogIsAppendedToRatherThanReplaced() - { - var directory = Scratch(); - try - { - var path = Path.Combine(directory, "observations.jsonl"); - var result = new ProbeResult - { - Host = Host("a.example.org"), - Outcome = CrawlOutcome.NoMssp, - ObservedAt = Now, - }; - - using (var first = new ObservationLog(path)) - { - first.Append(result); - } - - using (var second = new ObservationLog(path)) - { - second.Append(result); - } - - await Assert.That(File.ReadAllLines(path).Length).IsEqualTo(2); - } - finally - { - Directory.Delete(directory, recursive: true); - } - } - - [Test] - public async Task TheReportSaysWhatHappenedAndWhy() - { - var data = MsspWire.Report(MsspWire.RepresentativeReport("peer.example.net 4000")); - - var summary = new CrawlSummary - { - StartedAt = Now, - FinishedAt = Now + TimeSpan.FromMinutes(3), - StopReason = CrawlStopReason.HostCap, - Results = - [ - new ProbeResult - { - Host = Host("a.example.org"), - Outcome = CrawlOutcome.MsspReceived, - ObservedAt = Now, - Data = data, - }, - new ProbeResult - { - Host = Host("b.example.org"), - Outcome = CrawlOutcome.ConnectFailed, - ObservedAt = Now, - Error = "ConnectionRefused", - }, - ], - Hosts = - [ - new HostRecord { Host = Host("a.example.org"), FirstSeen = Now, NotBefore = Now + TimeSpan.FromHours(24) }, - new HostRecord - { - Host = Host("peer.example.net", 4000), - Depth = 1, - DiscoveredFrom = Host("a.example.org"), - FirstSeen = Now, - }, - ], - Verdicts = new Dictionary - { - [DiscoveryVerdict.Added] = 1, - [DiscoveryVerdict.AlreadyKnown] = 4, - }, - }; - - var report = CrawlReport.Render(summary, new CrawlOptions()); - - await Assert.That(report).Contains("the host cap was reached"); - await Assert.That(report).Contains("Corvid Nest"); - await Assert.That(report).Contains("peer.example.net:4000"); - await Assert.That(report).Contains("ConnectionRefused"); - await Assert.That(report).Contains("a cycle, or two servers naming the same peer"); - await Assert.That(report).Contains("2026-01-01 12:00:00Z"); - } - - [Test] - public async Task AServerNameCannotBreakTheReportsTable() - { - // Every string in that table came off the wire from a stranger. A MUD called "Pipe|Dream" must - // not be able to add a column to somebody's report. - var data = MsspWire.Report(("NAME", ["Pipe|Dream\nSecond line"])); - - var summary = new CrawlSummary - { - StartedAt = Now, - FinishedAt = Now, - StopReason = CrawlStopReason.Exhausted, - Results = [new ProbeResult { Host = Host("a.example.org"), Outcome = CrawlOutcome.MsspReceived, ObservedAt = Now, Data = data }], - Hosts = [], - Verdicts = new Dictionary(), - }; - - var row = CrawlReport.Render(summary, new CrawlOptions()) - .Split('\n') - .Single(line => line.Contains("Pipe")); - - await Assert.That(row).Contains("Pipe\\|Dream"); - await Assert.That(row).DoesNotContain("\r"); - - // Seven columns, so eight column separators — the one inside the name is escaped and does not - // count, which is the whole point. - var separators = row.Where((c, i) => c == '|' && (i == 0 || row[i - 1] != '\\')).Count(); - await Assert.That(separators).IsEqualTo(8); - } - - /// - /// A report is the same bytes wherever it was produced. StringBuilder.AppendLine emits - /// , so the renderer used to write CRLF on Windows and LF on Linux — - /// the same crawl, two different files. A report gets committed, diffed and pasted, and one whose - /// shape follows the machine that made it is useless as a baseline. - /// - /// This assertion only bites on Windows, where Environment.NewLine is CRLF; on Linux it - /// is LF and the test passes whether or not the normalisation is there. It is the Windows CI job that - /// guards this, which is why the Crawler suite now runs there. - /// - /// - [Test] - public async Task AReportIsTheSameBytesOnEveryPlatform() - { - var summary = new CrawlSummary - { - StartedAt = Now, - FinishedAt = Now, - StopReason = CrawlStopReason.Exhausted, - Results = - [ - new ProbeResult - { - Host = Host("a.example.org"), - Outcome = CrawlOutcome.MsspReceived, - ObservedAt = Now, - Data = MsspWire.Report(("NAME", ["Corvid Nest"])), - }, - ], - Hosts = [], - Verdicts = new Dictionary(), - }; - - var report = CrawlReport.Render(summary, new CrawlOptions()); - - await Assert.That(report).DoesNotContain("\r"); - await Assert.That(report).Contains("\n").Because("the report is still line-delimited, just with LF"); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/ProbeTests.cs b/tests/SharpMUTerm.Crawler.Tests/ProbeTests.cs deleted file mode 100644 index 476c4d9..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/ProbeTests.cs +++ /dev/null @@ -1,249 +0,0 @@ -using System.Text; -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Core.Transport; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Probing; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// The probe end to end: a real TelnetSession over a scripted server, negotiating for real. -/// -public class ProbeTests -{ - private static MsspHost Host(string name = "server.example.org", int port = 4201) => - MsspHost.Create(name, port)!; - - private static CrawlOptions Options => new() - { - ConnectTimeout = TimeSpan.FromSeconds(5), - MsspTimeout = TimeSpan.FromSeconds(5), - }; - - private static ScriptedTransport MsspServer(params string[] referrals) => - new ScriptedTransport { Greeting = MsspWire.Offer() } - .RespondingToDo(MsspWire.Mssp, MsspWire.Subnegotiation(MsspWire.RepresentativeReport(referrals))); - - [Test] - public async Task AServerThatOffersMsspIsReadCompletely() - { - var transport = MsspServer("peer.example.net 4000", "2001:db8::5 4201"); - var probe = new TelnetMsspProbe(Options, _ => transport); - - var result = await probe.ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.MsspReceived); - await Assert.That(result.Data).IsNotNull(); - - var data = result.Data!; - await Assert.That(data.Name).IsEqualTo("Corvid Nest"); - await Assert.That(data.Players).IsEqualTo(17); - - // The array-valued variables: the case a crawler exists for. - await Assert.That(data.Ports).IsEquivalentTo(new[] { 80, 23, 4201 }); - await Assert.That(data.Referrals.Select(r => r.ToReferralString())) - .IsEquivalentTo(new[] { "peer.example.net 4000", "2001:db8::5 4201" }); - - // The booleans and the unknown variables, likewise. - await Assert.That(data.Flag("ANSI")).IsTrue(); - await Assert.That(data.Default("CORVID SPECIFIC")).IsEqualTo("nevermore"); - } - - [Test] - public async Task AServerThatNegotiatesNoMsspAtAllIsRecordedAsSuchRatherThanAsAFailure() - { - // Most MU* servers. It offers ECHO and SUPPRESS-GO-AHEAD, prints a login banner, and never - // mentions MSSP. - var banner = new List(); - banner.AddRange([MsspWire.Iac, MsspWire.Will, 1]); // WILL ECHO - banner.AddRange([MsspWire.Iac, MsspWire.Will, 3]); // WILL SUPPRESS-GO-AHEAD - banner.AddRange(Encoding.ASCII.GetBytes("\r\nWelcome to Some MUD.\r\nBy what name do you wish to be known? ")); - - var transport = new ScriptedTransport { Greeting = [.. banner] }; - var options = Options with { MsspTimeout = TimeSpan.FromMilliseconds(300) }; - - var result = await new TelnetMsspProbe(options, _ => transport).ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.NoMssp); - await Assert.That(result.Data).IsNull(); - await Assert.That(result.IsFailure).IsFalse(); - } - - [Test] - public async Task AServerThatOnlyAnswersWhenAskedIsStillRead() - { - // The case that made a live server look as if it had no MSSP at all. The specification says a - // server "should" send IAC WILL MSSP on connect; plenty do not, and answer IAC DO MSSP instead. - // This server volunteers nothing — no greeting whatsoever — so it is only reachable by asking. - var transport = new ScriptedTransport() - .RespondingToDo(MsspWire.Mssp, MsspWire.Subnegotiation(("NAME", ["Silent Until Asked"]))); - - var result = await new TelnetMsspProbe(Options, _ => transport).ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.MsspReceived); - await Assert.That(result.Data!.Name).IsEqualTo("Silent Until Asked"); - } - - [Test] - public async Task TheProbeAsksForMsspRatherThanWaitingToBeOffered() - { - var transport = new ScriptedTransport(); - var probing = new TelnetMsspProbe( - Options with { MsspTimeout = TimeSpan.FromMilliseconds(400) }, _ => transport) - .ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(await transport.WaitForSentAsync([MsspWire.Iac, MsspWire.Do, MsspWire.Mssp])) - .IsTrue() - .Because("a crawler that only listens misses every server that waits to be asked"); - - transport.Close(); - await probing; - } - - [Test] - public async Task AServerThatHangsUpWithoutMsspIsRecordedAsHavingNone() - { - var transport = new ScriptedTransport(); - var probe = new TelnetMsspProbe(Options, _ => transport); - - var probing = probe.ProbeAsync(Host(), CancellationToken.None); - transport.Close(); - - await Assert.That((await probing).Outcome).IsEqualTo(CrawlOutcome.NoMssp); - } - - [Test] - public async Task NothingButTelnetNegotiationIsEverSentToTheServer() - { - // The politeness requirement, asserted against the bytes rather than against a reading of the - // code: a crawler must never log in and never send a command. Every byte the probe puts on the - // wire has to belong to an IAC sequence. - var transport = MsspServer("peer.example.net 4000"); - var result = await new TelnetMsspProbe(Options, _ => transport).ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.MsspReceived); - - var stray = StrayDataBytes(transport.Sent); - await Assert.That(stray) - .IsEmpty() - .Because($"the crawler sent application data: \"{Encoding.ASCII.GetString([.. stray])}\""); - } - - [Test] - public async Task NothingIsSentEvenToAServerThatPromptsForALogin() - { - // The case that would tempt a client: a login banner arrives and the connection sits there. It - // must still be answered with silence. - var banner = Encoding.ASCII.GetBytes("\r\nEnter your name: "); - var transport = new ScriptedTransport { Greeting = banner }; - var options = Options with { MsspTimeout = TimeSpan.FromMilliseconds(300) }; - - await new TelnetMsspProbe(options, _ => transport).ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(StrayDataBytes(transport.Sent)).IsEmpty(); - } - - [Test] - public async Task TheCrawlerNamesItselfWhenTheServerAsksWhatConnected() - { - // MTTS: the server sends IAC SB TTYPE SEND IAC SE and the client answers IAC SB TTYPE IS . - // A crawler that did not answer with its own name would be logged as whatever the telnet layer - // calls itself, which tells a server operator nothing. - const byte ttype = 24; - const byte send = 1; - - var transport = new ScriptedTransport { Greeting = [MsspWire.Iac, MsspWire.Do, ttype] }; - - var probing = new TelnetMsspProbe( - Options with { MsspTimeout = TimeSpan.FromSeconds(3) }, - _ => transport).ProbeAsync(Host(), CancellationToken.None); - - // Wait for the client to agree to TTYPE, then ask it for one. - await Assert.That(await transport.WaitForSentAsync([MsspWire.Iac, MsspWire.Will, ttype])).IsTrue(); - transport.SendToClient(MsspWire.Iac, MsspWire.Sb, ttype, send, MsspWire.Iac, MsspWire.Se); - - await Assert.That(await transport.WaitForSentAsync(Encoding.ASCII.GetBytes("SHARPMUTERM-MSSPCRAWLER"))) - .IsTrue(); - - transport.Close(); - await probing; - - var text = Encoding.ASCII.GetString(transport.Sent); - await Assert.That(text).Contains("SHARPMUTERM-MSSPCRAWLER"); - await Assert.That(text).DoesNotContain("TNC"); - } - - [Test] - public async Task AConnectionThatIsRefusedIsRecordedAsAFailure() - { - var result = await new TelnetMsspProbe(Options, _ => new RefusingTransport()) - .ProbeAsync(Host(), CancellationToken.None); - - await Assert.That(result.Outcome).IsEqualTo(CrawlOutcome.ConnectFailed); - await Assert.That(result.IsFailure).IsTrue(); - await Assert.That(result.Error).IsEqualTo("ConnectionRefused"); - } - - /// - /// Every byte written that is not part of an IAC sequence — i.e. everything that would be - /// application data reaching the game. - /// - private static List StrayDataBytes(byte[] sent) - { - var stray = new List(); - var i = 0; - while (i < sent.Length) - { - if (sent[i] != MsspWire.Iac) - { - stray.Add(sent[i]); - i++; - continue; - } - - if (i + 1 >= sent.Length) - { - break; - } - - var command = sent[i + 1]; - if (command == MsspWire.Sb) - { - // Skip to IAC SE, honouring IAC IAC inside the payload. - i += 2; - while (i + 1 < sent.Length && !(sent[i] == MsspWire.Iac && sent[i + 1] == MsspWire.Se)) - { - i += sent[i] == MsspWire.Iac && sent[i + 1] == MsspWire.Iac ? 2 : 1; - } - - i += 2; - continue; - } - - i += command is >= MsspWire.Will and <= MsspWire.Dont ? 3 : 2; - } - - return stray; - } - - private sealed class RefusingTransport : ITransport - { - public bool IsConnected => false; - - public string? RemoteDescription => null; - - public Task ConnectAsync(CancellationToken cancellationToken = default) => - throw new System.Net.Sockets.SocketException((int)System.Net.Sockets.SocketError.ConnectionRefused); - - public ValueTask SendAsync(ReadOnlyMemory data, CancellationToken cancellationToken = default) => - ValueTask.CompletedTask; - - public ValueTask ReceiveAsync(Memory buffer, CancellationToken cancellationToken = default) => - ValueTask.FromResult(0); - - public Task CloseAsync() => Task.CompletedTask; - - public ValueTask DisposeAsync() => ValueTask.CompletedTask; - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/RateLimitTests.cs b/tests/SharpMUTerm.Crawler.Tests/RateLimitTests.cs deleted file mode 100644 index 26cb00e..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/RateLimitTests.cs +++ /dev/null @@ -1,148 +0,0 @@ -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Scheduling; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// The rate limiter, driven by a clock the test moves by hand. Nothing here sleeps: a limiter tested -/// by waiting for its own interval proves only that the machine was not too busy. -/// -public class RateLimitTests -{ - private static MsspHost Host(string name, int port = 4201) => MsspHost.Create(name, port)!; - - private static readonly CrawlOptions Options = new() - { - GlobalInterval = TimeSpan.FromSeconds(2), - PerHostInterval = TimeSpan.FromMinutes(5), - }; - - [Test] - public async Task TheFirstConnectionIsAllowedImmediately() - { - var limiter = new CrawlRateLimiter(Options, new ManualTimeProvider()); - - await Assert.That(limiter.DelayBefore(Host("a.example.org"))).IsEqualTo(TimeSpan.Zero); - } - - [Test] - public async Task TheGlobalIntervalHoldsBackTheNextConnectionToADifferentHost() - { - var time = new ManualTimeProvider(); - var limiter = new CrawlRateLimiter(Options, time); - - limiter.RecordStart(Host("a.example.org")); - - // A different host, so the per-host limit has nothing to say; the global one does. - await Assert.That(limiter.DelayBefore(Host("b.example.org"))).IsEqualTo(TimeSpan.FromSeconds(2)); - - time.Advance(TimeSpan.FromSeconds(1)); - await Assert.That(limiter.DelayBefore(Host("b.example.org"))).IsEqualTo(TimeSpan.FromSeconds(1)); - - time.Advance(TimeSpan.FromSeconds(1)); - await Assert.That(limiter.DelayBefore(Host("b.example.org"))).IsEqualTo(TimeSpan.Zero); - } - - [Test] - public async Task TheSameHostWaitsTheLongerPerHostInterval() - { - var time = new ManualTimeProvider(); - var limiter = new CrawlRateLimiter(Options, time); - - var host = Host("a.example.org"); - limiter.RecordStart(host); - - time.Advance(TimeSpan.FromSeconds(10)); - - // The global interval is long since satisfied; the per-host one is not, and the longer of the - // two is what is owed. - await Assert.That(limiter.DelayBefore(Host("b.example.org"))).IsEqualTo(TimeSpan.Zero); - await Assert.That(limiter.DelayBefore(host)).IsEqualTo(TimeSpan.FromSeconds(290)); - - time.Advance(TimeSpan.FromSeconds(290)); - await Assert.That(limiter.DelayBefore(host)).IsEqualTo(TimeSpan.Zero); - } - - [Test] - public async Task PortsOnOneMachineAreSeparateHostsForRateLimitingButTheGlobalLimitStillApplies() - { - // Two ports on one server are two entries; the per-host limit is keyed on both. The global limit - // is what stops a server with six advertised ports being dialled six times in a second. - var time = new ManualTimeProvider(); - var limiter = new CrawlRateLimiter(Options, time); - - limiter.RecordStart(Host("a.example.org", 4201)); - await Assert.That(limiter.DelayBefore(Host("a.example.org", 4202))).IsEqualTo(TimeSpan.FromSeconds(2)); - } - - [Test] - public async Task AStreamOfConnectionsIsSpacedByTheGlobalInterval() - { - // The property that matters: over a run, connections start no closer together than the interval. - var time = new ManualTimeProvider(); - var limiter = new CrawlRateLimiter(Options, time); - var starts = new List(); - - for (var i = 0; i < 5; i++) - { - var host = Host($"host{i}.example.org"); - var wait = limiter.DelayBefore(host); - time.Advance(wait); - starts.Add(time.GetUtcNow()); - limiter.RecordStart(host); - } - - var gaps = starts.Zip(starts.Skip(1), (first, second) => second - first).ToList(); - await Assert.That(gaps).IsNotEmpty(); - foreach (var gap in gaps) - { - await Assert.That(gap).IsGreaterThanOrEqualTo(TimeSpan.FromSeconds(2)); - } - } - - [Test] - public async Task WaitingForATurnStampsTheStartSoTheNextCallerIsHeldBack() - { - var time = new ManualTimeProvider(); - var limiter = new CrawlRateLimiter(Options, time); - - // Nothing to wait for, so this completes without the clock moving at all. - await limiter.WaitForTurnAsync(Host("a.example.org"), CancellationToken.None); - - await Assert.That(limiter.DelayBefore(Host("b.example.org"))).IsEqualTo(TimeSpan.FromSeconds(2)); - } - - [Test] - public async Task AZeroIntervalMeansNoWaitingAtAll() - { - // The configuration a test harness uses, and the one a careless operator might. It must work - // rather than divide by something. - var limiter = new CrawlRateLimiter( - new CrawlOptions { GlobalInterval = TimeSpan.Zero, PerHostInterval = TimeSpan.Zero }, - new ManualTimeProvider()); - - var host = Host("a.example.org"); - limiter.RecordStart(host); - - await Assert.That(limiter.DelayBefore(host)).IsEqualTo(TimeSpan.Zero); - } - - [Test] - public async Task TheDefaultsAreConservative() - { - // A pin on the politeness settings themselves. These are the numbers that reach other people's - // servers, and lowering one should be a deliberate act with a test to change. - var defaults = new CrawlOptions(); - - await Assert.That(defaults.MaxConcurrency).IsEqualTo(4); - await Assert.That(defaults.GlobalInterval).IsEqualTo(TimeSpan.FromSeconds(1)); - await Assert.That(defaults.PerHostInterval).IsEqualTo(TimeSpan.FromMinutes(5)); - await Assert.That(defaults.RevisitInterval).IsEqualTo(TimeSpan.FromHours(24)); - await Assert.That(defaults.NoMsspRevisitInterval).IsEqualTo(TimeSpan.FromDays(7)); - await Assert.That(defaults.MaxHosts).IsEqualTo(500); - await Assert.That(defaults.MaxDuration).IsEqualTo(TimeSpan.FromHours(1)); - await Assert.That(defaults.MaxDepth).IsEqualTo(4); - await Assert.That(defaults.FollowPrivateAddresses).IsFalse(); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/ReferralTests.cs b/tests/SharpMUTerm.Crawler.Tests/ReferralTests.cs deleted file mode 100644 index 7f45376..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/ReferralTests.cs +++ /dev/null @@ -1,254 +0,0 @@ -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Scheduling; -using SharpMUTerm.Crawler.Tests.Support; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// REFERRAL: the format the specification defines, and what a crawler does with what it finds. -/// -public class ReferralTests -{ - private static readonly DateTimeOffset Now = new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); - - private static MsspHost Host(string value) - { - MsspHost.TryParse(value, out var host); - return host!; - } - - [Test] - public async Task TheSpecifiedFormatIsHostSpacePort() - { - // "using the host port format … Make sure to separate the host and port with a space rather - // than : because IPv6 addresses contain colons." - MsspHost.TryParse("mud.example.org 4000", out var host); - - await Assert.That(host).IsNotNull(); - await Assert.That(host!.Host).IsEqualTo("mud.example.org"); - await Assert.That(host.Port).IsEqualTo(4000); - await Assert.That(host.ToReferralString()).IsEqualTo("mud.example.org 4000"); - } - - [Test] - public async Task AnIpV6ReferralIsParsedAndCanonicalised() - { - // The exact reason the specification chose a space. Two spellings of one address must become one - // host, or a crawl visits it once per spelling its peers happen to use. - MsspHost.TryParse("2001:0DB8:0000:0000:0000:0000:0000:0001 4201", out var verbose); - MsspHost.TryParse("2001:db8::1 4201", out var compact); - - await Assert.That(verbose).IsNotNull(); - await Assert.That(verbose!.Host).IsEqualTo("2001:db8::1"); - await Assert.That(verbose).IsEqualTo(compact!); - await Assert.That(verbose.ToString()).IsEqualTo("[2001:db8::1]:4201"); - } - - [Test] - public async Task ABracketedIpV6LiteralIsAccepted() - { - MsspHost.TryParse("[2001:db8::1] 4201", out var host); - - await Assert.That(host).IsNotNull(); - await Assert.That(host!.Host).IsEqualTo("2001:db8::1"); - } - - [Test] - public async Task TheColonFormIsToleratedOnlyWhereItCannotBeAnIpV6Address() - { - // Real servers emit host:port despite the specification. Accepting it recovers referrals that - // would otherwise be lost… - MsspHost.TryParse("mud.example.org:4000", out var name); - await Assert.That(name).IsNotNull(); - await Assert.That(name!.Port).IsEqualTo(4000); - - // …but a string with more than one colon is far more likely to be a bare IPv6 address, and - // splitting it at the last colon would silently rewrite it into a different address. - await Assert.That(MsspHost.TryParse("2001:db8::1", out _)).IsFalse(); - await Assert.That(MsspHost.TryParse("2001:db8::1:4201", out _)).IsFalse(); - } - - [Test] - [Arguments("")] - [Arguments(" ")] - [Arguments("mud.example.org")] - [Arguments("mud.example.org 0")] - [Arguments("mud.example.org 70000")] - [Arguments("mud.example.org -1")] - [Arguments("mud.example.org notaport")] - [Arguments("http://mud.example.org/ 4000")] - [Arguments("a stale line someone typed")] - public async Task AMalformedReferralIsRejectedRatherThanGuessedAt(string value) => - await Assert.That(MsspHost.TryParse(value, out _)).IsFalse(); - - [Test] - public async Task HostNamesAreNormalisedSoOneServerIsOneEntry() - { - var spellings = new[] - { - "MUD.Example.ORG 4201", - "mud.example.org 4201", - "mud.example.org. 4201", - "mud.example.org:4201", - }; - - var hosts = spellings.Select(Host).ToHashSet(); - await Assert.That(hosts.Count).IsEqualTo(1); - } - - [Test] - public async Task TheReferralListIsReadFromTheArrayAndDeduplicated() - { - var data = MsspWire.Report( - ("REFERRAL", - [ - "a.example.org 4000", - "b.example.org 4000", - "A.EXAMPLE.ORG 4000", // the same host, spelled differently - "not a referral", // a stale line; dropped, not fatal - "2001:db8::9 4201", - ])); - - await Assert.That(data.Referrals.Select(r => r.ToReferralString())) - .IsEquivalentTo(new[] { "a.example.org 4000", "b.example.org 4000", "2001:db8::9 4201" }); - - // The raw strings stay readable, so a report can show what a server actually said. - await Assert.That(data["REFERRAL"].Count).IsEqualTo(5); - } - - [Test] - public async Task PrivateAndLoopbackAddressesAreClassifiedAsUncrawlable() - { - await Assert.That(Host("127.0.0.1 4201").Scope).IsEqualTo(MsspHostScope.Loopback); - await Assert.That(Host("::1 4201").Scope).IsEqualTo(MsspHostScope.Loopback); - await Assert.That(Host("10.1.2.3 4201").Scope).IsEqualTo(MsspHostScope.Private); - await Assert.That(Host("192.168.1.1 4201").Scope).IsEqualTo(MsspHostScope.Private); - await Assert.That(Host("172.16.0.1 4201").Scope).IsEqualTo(MsspHostScope.Private); - await Assert.That(Host("fd00::1 4201").Scope).IsEqualTo(MsspHostScope.Private); - - // The cloud metadata address, which is the one that matters. - await Assert.That(Host("169.254.169.254 80").Scope).IsEqualTo(MsspHostScope.LinkLocal); - - await Assert.That(Host("198.51.100.7 4201").Scope).IsEqualTo(MsspHostScope.Global); - await Assert.That(Host("mud.example.org 4201").Scope).IsEqualTo(MsspHostScope.Unresolved); - } - - // ---- What the frontier does with them ---- - - private static CrawlFrontier Frontier(CrawlOptions? options = null) => - new(options ?? new CrawlOptions()); - - [Test] - public async Task AReferralBackToTheReferrerIsRecognisedAndIgnored() - { - // The simplest cycle, and the commonest: two servers list each other. Neither is a fault. - var frontier = Frontier(); - var a = Host("a.example.org 4000"); - var b = Host("b.example.org 4000"); - - frontier.AddSeed(a, Now); - await Assert.That(frontier.Discover(b, a, 0, Now)).IsEqualTo(DiscoveryVerdict.Added); - await Assert.That(frontier.Discover(a, b, 1, Now)).IsEqualTo(DiscoveryVerdict.AlreadyKnown); - - await Assert.That(frontier.Records.Count).IsEqualTo(2); - } - - [Test] - public async Task AServerThatRefersToItselfIsCountedAndNotFollowed() - { - var frontier = Frontier(); - var a = Host("a.example.org 4000"); - frontier.AddSeed(a, Now); - - await Assert.That(frontier.Discover(a, a, 0, Now)).IsEqualTo(DiscoveryVerdict.SelfReferral); - await Assert.That(frontier.Records.Count).IsEqualTo(1); - } - - [Test] - public async Task ALongerCycleTerminatesBecauseIdentityIsNormalised() - { - // A → B → C → A, with each hop spelling the next host differently. Identity is over the - // normalised host, so the loop closes on the third hop instead of running for ever. - var frontier = Frontier(); - var a = Host("a.example.org 4000"); - var b = Host("b.example.org 4000"); - var c = Host("c.example.org 4000"); - - frontier.AddSeed(a, Now); - frontier.Discover(b, a, 0, Now); - frontier.Discover(c, b, 1, Now); - - await Assert.That(frontier.Discover(Host("A.Example.ORG. 4000"), c, 2, Now)) - .IsEqualTo(DiscoveryVerdict.AlreadyKnown); - await Assert.That(frontier.Records.Count).IsEqualTo(3); - } - - [Test] - public async Task AHostReachedAgainByAShorterPathKeepsTheShorterDepth() - { - // Depth is what the cap is measured against, so a host first met at the limit would otherwise - // never have its own referrals followed even after a shorter route to it turned up. - var frontier = Frontier(); - var seed = Host("seed.example.org 4000"); - var far = Host("far.example.org 4000"); - - frontier.AddSeed(seed, Now); - frontier.Discover(far, seed, 3, Now); - await Assert.That(frontier.Records.Single(r => r.Host == far).Depth).IsEqualTo(4); - - frontier.Discover(far, seed, 0, Now); - await Assert.That(frontier.Records.Single(r => r.Host == far).Depth).IsEqualTo(1); - } - - [Test] - public async Task AReferralBeyondTheDepthLimitIsRefused() - { - var frontier = Frontier(new CrawlOptions { MaxDepth = 2 }); - frontier.AddSeed(Host("seed.example.org 4000"), Now); - - var referrer = Host("seed.example.org 4000"); - await Assert.That(frontier.Discover(Host("a 4000"), referrer, 1, Now)).IsEqualTo(DiscoveryVerdict.Added); - await Assert.That(frontier.Discover(Host("b 4000"), referrer, 2, Now)).IsEqualTo(DiscoveryVerdict.TooDeep); - } - - [Test] - public async Task AReferralIntoPrivateSpaceIsRefusedUnlessTheOperatorAsksForIt() - { - var referrer = Host("stranger.example.org 4000"); - - var guarded = Frontier(); - guarded.AddSeed(referrer, Now); - await Assert.That(guarded.Discover(Host("169.254.169.254 80"), referrer, 0, Now)) - .IsEqualTo(DiscoveryVerdict.NotRoutable); - await Assert.That(guarded.Discover(Host("127.0.0.1 4201"), referrer, 0, Now)) - .IsEqualTo(DiscoveryVerdict.NotRoutable); - - var permissive = new CrawlFrontier(new CrawlOptions { FollowPrivateAddresses = true }); - permissive.AddSeed(referrer, Now); - await Assert.That(permissive.Discover(Host("127.0.0.1 4201"), referrer, 0, Now)) - .IsEqualTo(DiscoveryVerdict.Added); - } - - [Test] - public async Task ASeedIntoPrivateSpaceIsAcceptedBecauseTheOperatorSaidSo() - { - // The operator pointing this at their own test server is not the same thing as a stranger's - // referral aiming it at a network it could not otherwise reach. - var frontier = Frontier(); - await Assert.That(frontier.AddSeed(Host("127.0.0.1 4201"), Now)).IsEqualTo(DiscoveryVerdict.Added); - } - - [Test] - public async Task ReferralsAreNotFollowedAtAllWhenTheRunSaysNotTo() - { - var frontier = Frontier(new CrawlOptions { FollowReferrals = false }); - var a = Host("a.example.org 4000"); - frontier.AddSeed(a, Now); - - await Assert.That(frontier.Discover(Host("b.example.org 4000"), a, 0, Now)) - .IsEqualTo(DiscoveryVerdict.ReferralsDisabled); - await Assert.That(frontier.Records.Count).IsEqualTo(1); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/SeedTests.cs b/tests/SharpMUTerm.Crawler.Tests/SeedTests.cs deleted file mode 100644 index 6abb15c..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/SeedTests.cs +++ /dev/null @@ -1,191 +0,0 @@ -using SharpMUTerm.Crawler; - -namespace SharpMUTerm.Crawler.Tests; - -/// -/// Where a run's starting hosts come from, and — more importantly — what it refuses to read. -/// -public class SeedTests -{ - private static string WriteTemp(string name, string content) - { - var directory = Path.Combine(Path.GetTempPath(), "sharpmuterm-crawler-tests", Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(directory); - var path = Path.Combine(directory, name); - File.WriteAllText(path, content); - return path; - } - - [Test] - public async Task ASeedFileIsHostSpacePortWithCommentsIgnored() - { - var path = WriteTemp("seeds.txt", """ - # The hosts this run starts from. - mud.example.org 4201 - other.example.net 4000 # trailing comment - - 2001:db8::1 4201 - legacy.example.com:23 - """); - - try - { - var seeds = Seeds.FromFile(path); - - await Assert.That(seeds.Rejected).IsEmpty(); - await Assert.That(seeds.Hosts.Select(h => h.ToReferralString())).IsEquivalentTo(new[] - { - "mud.example.org 4201", - "other.example.net 4000", - "2001:db8::1 4201", - "legacy.example.com 23", - }); - } - finally - { - Directory.Delete(Path.GetDirectoryName(path)!, recursive: true); - } - } - - [Test] - public async Task ABadSeedLineIsReportedRatherThanStoppingTheRun() - { - var path = WriteTemp("seeds.txt", """ - good.example.org 4201 - this is not a host - good.example.org 4201 - """); - - try - { - var seeds = Seeds.FromFile(path); - - await Assert.That(seeds.Hosts.Count).IsEqualTo(1); // the duplicate collapses - await Assert.That(seeds.Rejected).IsEquivalentTo(new[] { "this is not a host" }); - } - finally - { - Directory.Delete(Path.GetDirectoryName(path)!, recursive: true); - } - } - - [Test] - public async Task AWorldsFileYieldsOnlyHostsAndPortsAndNothingElse() - { - // A realistic SharpMUTerm configuration, including the fields this tool must never look at. - var path = WriteTemp("config.json", """ - { - "version": 3, - "worlds": [ - { - "name": "Corvid", - "host": "corvid.example.org", - "port": 4201, - "useTls": false, - "characters": [ - { "name": "Ann", "login": "connect Ann hunter2", "passwordRef": "9f2a-…" } - ] - }, - { - "name": "Second", - "host": "second.example.net", - "port": 2000, - "characters": [] - }, - { - "name": "No port", - "host": "broken.example.org" - } - ] - } - """); - - try - { - var seeds = Seeds.FromWorldsFile(path); - - await Assert.That(seeds.Hosts.Select(h => h.ToReferralString())) - .IsEquivalentTo(new[] { "corvid.example.org 4201", "second.example.net 2000" }); - - // The world with no port is reported by name; nothing about its characters is read at all. - await Assert.That(seeds.Rejected).IsEquivalentTo(new[] { "No port: no host or port" }); - await Assert.That(string.Join("|", seeds.Rejected)).DoesNotContain("hunter2"); - await Assert.That(string.Join("|", seeds.Rejected)).DoesNotContain("Ann"); - } - finally - { - Directory.Delete(Path.GetDirectoryName(path)!, recursive: true); - } - } - - [Test] - [Arguments("GetFolderPath")] - [Arguments("SpecialFolder")] - [Arguments("XDG_CONFIG_HOME")] - [Arguments("secrets.json")] - public async Task TheCrawlerCannotReachForTheUsersConfigurationDirectory(string forbidden) - { - // The guarantee this tool makes is that it reads nothing it was not explicitly pointed at, and - // that guarantee is worth a test that cannot be satisfied by a careful reading. Every method a - // program calls leaves its name in the assembly's metadata, and every string literal it holds is - // in there too — so if a future change ever resolves the configuration directory, or names the - // secrets file, the name is in this file and this fails. - var bytes = await File.ReadAllBytesAsync(typeof(Seeds).Assembly.Location); - var needle = System.Text.Encoding.UTF8.GetBytes(forbidden); - - var found = false; - for (var i = 0; i + needle.Length <= bytes.Length && !found; i++) - { - found = bytes.AsSpan(i, needle.Length).SequenceEqual(needle); - } - - await Assert.That(found).IsFalse() - .Because($"the crawler assembly refers to \"{forbidden}\"; it must not know where the user's " - + "configuration lives, only what it was handed on the command line"); - } - - // ---- The command line ---- - - [Test] - public async Task ASeedIsTakenFromTheCommandLine() - { - var parsed = CommandLine.Parse(["--seed", "mud.example.org:4201", "--seed", "other.example.net 23"]); - - await Assert.That(parsed.Error).IsNull(); - await Assert.That(parsed.Options!.Seeds.Select(h => h.ToReferralString())) - .IsEquivalentTo(new[] { "mud.example.org 4201", "other.example.net 23" }); - } - - [Test] - public async Task PolitenessSettingsCanBeLoosenedOnlyDeliberately() - { - var defaults = CommandLine.Parse(["--seed", "a.example.org:1"]).Options!; - await Assert.That(defaults.MaxConcurrency).IsEqualTo(4); - await Assert.That(defaults.RevisitInterval).IsEqualTo(TimeSpan.FromHours(24)); - - var loosened = CommandLine.Parse( - ["--seed", "a.example.org:1", "--concurrency", "2", "--revisit", "48", "--max-hosts", "10"]).Options!; - await Assert.That(loosened.MaxConcurrency).IsEqualTo(2); - await Assert.That(loosened.RevisitInterval).IsEqualTo(TimeSpan.FromHours(48)); - await Assert.That(loosened.MaxHosts).IsEqualTo(10); - } - - [Test] - public async Task ASettingThatCouldOnlyBeATypoIsRefusedBeforeAnythingReachesTheNetwork() - { - await Assert.That(CommandLine.Parse(["--seed", "a.example.org:1", "--concurrency", "0"]).Error).IsNotNull(); - await Assert.That(CommandLine.Parse(["--seed", "a.example.org:1", "--max-hosts", "0"]).Error).IsNotNull(); - await Assert.That(CommandLine.Parse(["--nonsense"]).Error).IsNotNull(); - await Assert.That(CommandLine.Parse(["--seed", "not a host at all"]).Error).IsNotNull(); - } - - [Test] - public async Task TheHelpTextSaysWhatTheToolWillAndWillNotDo() - { - // A server operator who finds this in their logs and searches for the name should reach a tool - // that states plainly that it never logs in. - await Assert.That(CommandLine.Parse(["--help"]).WantsHelp).IsTrue(); - await Assert.That(CommandLine.Usage).Contains("never logs in and never sends a command"); - await Assert.That(CommandLine.Usage).Contains("SHARPMUTERM-MSSPCRAWLER"); - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/SharpMUTerm.Crawler.Tests.csproj b/tests/SharpMUTerm.Crawler.Tests/SharpMUTerm.Crawler.Tests.csproj deleted file mode 100644 index 087ff68..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/SharpMUTerm.Crawler.Tests.csproj +++ /dev/null @@ -1,19 +0,0 @@ - - - - Exe - SharpMUTerm.Crawler.Tests - SharpMUTerm.Crawler.Tests - false - true - - - - - - - - - - - diff --git a/tests/SharpMUTerm.Crawler.Tests/Support/FakeProbe.cs b/tests/SharpMUTerm.Crawler.Tests/Support/FakeProbe.cs deleted file mode 100644 index abba4e6..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/Support/FakeProbe.cs +++ /dev/null @@ -1,115 +0,0 @@ -using SharpMUTerm.Core.Telnet.Mssp; -using SharpMUTerm.Crawler.Model; -using SharpMUTerm.Crawler.Probing; - -namespace SharpMUTerm.Crawler.Tests.Support; - -/// -/// A probe that answers from a script instead of a socket, so the crawl loop can be tested without a -/// network. Records every host it was asked about, in order, and how many were in flight at once. -/// -internal sealed class FakeProbe(TimeProvider time) : IMsspProbe -{ - private readonly Lock _gate = new(); - private readonly List _visited = []; - private readonly Dictionary> _answers = []; - - private int _inFlight; - - /// Every host probed, in the order the loop reached them. - public IReadOnlyList Visited - { - get - { - lock (_gate) - { - return [.. _visited]; - } - } - } - - /// The most connections that were open at the same moment. - public int PeakConcurrency { get; private set; } - - /// What an unscripted host answers with. Defaults to a server that has no MSSP. - public CrawlOutcome DefaultOutcome { get; set; } = CrawlOutcome.NoMssp; - - /// Held open until released, so a test can observe several probes in flight at once. - public TaskCompletionSource? Gate { get; set; } - - /// Answers with a report referring to . - public FakeProbe Referring(MsspHost host, params MsspHost[] referrals) - { - var data = MsspWire.Report( - ("NAME", [$"Server at {host.Host}"]), - ("REFERRAL", referrals.Select(r => r.ToReferralString()).ToArray())); - - lock (_gate) - { - _answers[host] = probed => new ProbeResult - { - Host = probed, - Outcome = CrawlOutcome.MsspReceived, - ObservedAt = time.GetUtcNow(), - Data = data, - }; - } - - return this; - } - - /// Answers with a specific outcome and no data. - public FakeProbe Answering(MsspHost host, CrawlOutcome outcome, string? error = null) - { - lock (_gate) - { - _answers[host] = probed => new ProbeResult - { - Host = probed, - Outcome = outcome, - ObservedAt = time.GetUtcNow(), - Error = error, - }; - } - - return this; - } - - public async Task ProbeAsync(MsspHost host, CancellationToken cancellationToken) - { - Func? answer; - lock (_gate) - { - _visited.Add(host); - _inFlight++; - PeakConcurrency = Math.Max(PeakConcurrency, _inFlight); - _answers.TryGetValue(host, out answer); - } - - try - { - if (Gate is { } gate) - { - await gate.Task.WaitAsync(cancellationToken).ConfigureAwait(false); - } - else - { - await Task.Yield(); - } - - return answer?.Invoke(host) ?? new ProbeResult - { - Host = host, - Outcome = DefaultOutcome, - ObservedAt = time.GetUtcNow(), - }; - } - finally - { - lock (_gate) - { - _inFlight--; - } - } - } -} diff --git a/tests/SharpMUTerm.Crawler.Tests/Support/ManualTimeProvider.cs b/tests/SharpMUTerm.Crawler.Tests/Support/ManualTimeProvider.cs deleted file mode 100644 index 597993a..0000000 --- a/tests/SharpMUTerm.Crawler.Tests/Support/ManualTimeProvider.cs +++ /dev/null @@ -1,175 +0,0 @@ -namespace SharpMUTerm.Crawler.Tests.Support; - -/// -/// A clock and timer source a test moves by hand. -/// -/// It exists so a rate limit can be asserted rather than waited for. A limiter tested with -/// real time is a test that sleeps for its own interval and then races the machine it runs on: slow -/// enough to be annoying, flaky enough to be disabled, and it proves nothing about the interval it did -/// not sleep for. moves the clock and fires every timer it passes, on the calling -/// thread, so the effect of a wait lands before the next line of the test. -/// -/// -internal sealed class ManualTimeProvider : TimeProvider -{ - private readonly List _timers = []; - private readonly Lock _gate = new(); - - private DateTimeOffset _now = new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); - - public override DateTimeOffset GetUtcNow() - { - lock (_gate) - { - return _now; - } - } - - public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) - { - var timer = new ManualTimer(this, callback, state); - lock (_gate) - { - _timers.Add(timer); - } - - timer.Change(dueTime, period); - return timer; - } - - /// Moves the clock on by , firing every timer that comes due. - public void Advance(TimeSpan by) - { - ManualTimer[] due; - lock (_gate) - { - _now += by; - // A firing callback may arm another timer, so fire from a snapshot and let anything new - // wait for the next Advance. - due = [.. _timers]; - } - - var now = GetUtcNow(); - foreach (var timer in due) - { - timer.FireIfDue(now); - } - } - - private void Forget(ManualTimer timer) - { - lock (_gate) - { - _timers.Remove(timer); - } - } - - private sealed class ManualTimer(ManualTimeProvider owner, TimerCallback callback, object? state) : ITimer - { - private DateTimeOffset? _dueAt; - - public bool Change(TimeSpan dueTime, TimeSpan period) - { - _dueAt = dueTime == Timeout.InfiniteTimeSpan ? null : owner.GetUtcNow() + dueTime; - return true; - } - - public void FireIfDue(DateTimeOffset now) - { - if (_dueAt is { } due && now >= due) - { - _dueAt = null; - callback(state); - } - } - - public void Dispose() - { - _dueAt = null; - owner.Forget(this); - } - - public ValueTask DisposeAsync() - { - Dispose(); - return ValueTask.CompletedTask; - } - } -} - -/// -/// A clock that runs on demand: a delay does not wait, it moves the clock forward by the amount asked -/// for and returns. -/// -/// This is what lets the crawl loop — which really does wait on its rate limiter — be tested -/// without sleeping. cannot serve here: the loop decides how long to -/// wait from inside itself, so a test cannot know how far to advance the clock without reimplementing -/// the thing it is testing. Here time passes exactly as the code under test asks it to, and -/// afterwards is the true elapsed total — which is what the time-cap test -/// asserts against. -/// -/// -internal sealed class VirtualTimeProvider : TimeProvider -{ - private readonly Lock _gate = new(); - - private DateTimeOffset _now = new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); - - public override DateTimeOffset GetUtcNow() - { - lock (_gate) - { - return _now; - } - } - - /// Moves the clock without any timer being involved — how a test sets up "a day later". - public void Advance(TimeSpan by) - { - lock (_gate) - { - _now += by; - } - } - - public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) - => new InstantTimer(this, callback, state, dueTime); - - private sealed class InstantTimer : ITimer - { - private readonly CancellationTokenSource _cancel = new(); - - public InstantTimer(VirtualTimeProvider owner, TimerCallback callback, object? state, TimeSpan dueTime) - { - if (dueTime == Timeout.InfiniteTimeSpan) - { - return; - } - - // Queued rather than run inline: Task.Delay builds its timer inside its own constructor, and - // completing it before that returns deadlocks. The clock still moves by the full amount, so - // the code under test observes exactly the interval it asked to wait. - _ = Task.Run(async () => - { - await Task.Yield(); - if (_cancel.IsCancellationRequested) - { - return; - } - - owner.Advance(dueTime); - callback(state); - }); - } - - public bool Change(TimeSpan dueTime, TimeSpan period) => true; - - public void Dispose() => _cancel.Cancel(); - - public ValueTask DisposeAsync() - { - Dispose(); - return ValueTask.CompletedTask; - } - } -} From cd22fd85d4ab09ee94bb8f0e87dc9cb16295e21f Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:14:20 -0500 Subject: [PATCH 2/7] =?UTF-8?q?Surface=20MSSP=20in=20the=20client:=20F5=20?= =?UTF-8?q?=E2=96=B8=20i=20opens=20a=20world's=20server=20information?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "someone who has created a World area to look at the MSSP information in a nice UI. So an INFO button that goes to an MSSP read screen." MSSP was parsed and thrown away — WorldSession.MsspReceived was raised and nothing in the Tui subscribed. It is now captured per server, kept between launches, and read from the F5 Worlds & Characters screen. Three decisions, argued in the code: Where it lives. Not config.json. That file is what the user asked for and is hand-edited; a write per connect has no business landing there. It is a sibling cache, mssp.json, versioned on its own so nothing here can force an AppConfiguration migration — the shape SecretsStore and the restore log already have. Fourth of the save:/logRoot:/restore: family with one difference: the field is never null. A MsspCache with no path is memory-only *by construction*, so "a snapshot writes nothing" is a property of the object rather than a check at each use site, and the screen needs no "is there a cache" branch. Per host:port, not per world. MSSP describes a server; a world name is a user-editable label two entries may share, and a rename must not lose a report. Two worlds pointed at one server read one report. The one case that reads oddly — editing a world's port makes its report "vanish" — is right: those are two endpoints, and one of them is usually the TLS one. A second report replaces the first. MSSP is not a delta protocol; a server sends its whole table once per connection. A merge would keep variables it has stopped publishing for ever — accumulating room exits in a different costume — and would leave a report that is a snapshot of no moment that ever existed, which is exactly what a dated report exists not to be. Three states, not two. ConnectedAt is written on the Connected transition and ObservedAt only when a report arrives, so "never dialled", "dialled and this server publishes none" and "here is the report, as of 3 days ago" are three different screens. The middle one is the ordinary case on a MUSH and is worded as optional-and-normally-absent, with the world's own host, port and transport beside it; saying "no data" there is what makes a client look broken. Everything the server sent is shown, official and unofficial, told apart by a reserved mark column with a legend. Multi-valued variables (PORT, CODEBASE, REFERRAL) are drawn as the lists they are — one value per row, the name printed once — because "least to most relevant" is meaning and the first value is a server's *least* preferred. The values are a stranger's. Control characters are replaced, the raw text is truncated before it is escaped (truncating escaped markup splits a [[), and the column widths are functions of the terminal and never of the data — at 80 columns a fixed 58-cell value column overruns by three. Capture is bounded at the door as well as at the renderer: a value only the screen trimmed would still be full size on disk and in memory on every later launch. The key is a key, not a chip. ScreenButtonKind gains Detail, Stops() trims it with Remove, and the WORLDS pane reads list → [+ world] → i info … → Del removes …. An INFO chip reached with ↑↓ would drag the selection to the last world and could only ever have reported on that one — the documented "only the last world can be deleted" bug, one feature later. `i` runs outside ScreenEdits: opening a screen is navigation, and routing it through the edit log would write config.json and re-periodise every running timer each time somebody looked at a world. SettingsOverlay swaps content over a one-deep stack rather than stacking a second modal, so Esc goes back to the world you were on and the screen stays drivable headlessly. Views mssp / mssp-none / mssp-never drive the real `i` into a real F5, and the demo report goes in through SharpMUTermApp.CaptureMssp — the same writer the wire uses — so the frame and the connection cannot file under different endpoints. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- CLAUDE.md | 26 +- README.md | 7 + src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs | 423 +++++++++++++++ .../Telnet/Mssp/MsspObservation.cs | 46 ++ src/SharpMUTerm.Tui/DemoScene.cs | 48 ++ src/SharpMUTerm.Tui/MsspScreenRenderer.cs | 439 +++++++++++++++ src/SharpMUTerm.Tui/MsspScreenView.cs | 60 +++ src/SharpMUTerm.Tui/Program.cs | 24 +- src/SharpMUTerm.Tui/ScreenChrome.cs | 46 +- src/SharpMUTerm.Tui/ScreenModel.cs | 107 +++- src/SharpMUTerm.Tui/SettingsOverlay.cs | 49 +- src/SharpMUTerm.Tui/SettingsSession.cs | 37 ++ src/SharpMUTerm.Tui/SharpMUTermApp.cs | 124 ++++- src/SharpMUTerm.Tui/WorldsScreenRenderer.cs | 54 +- src/SharpMUTerm.Tui/WorldsScreenView.cs | 9 +- .../Telnet/MsspCacheTests.cs | 314 +++++++++++ .../SharpMUTerm.Tui.Tests/MsspScreenTests.cs | 502 ++++++++++++++++++ 17 files changed, 2285 insertions(+), 30 deletions(-) create mode 100644 src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs create mode 100644 src/SharpMUTerm.Core/Telnet/Mssp/MsspObservation.cs create mode 100644 src/SharpMUTerm.Tui/MsspScreenRenderer.cs create mode 100644 src/SharpMUTerm.Tui/MsspScreenView.cs create mode 100644 tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index e0419e9..9064624 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,26 @@ fallbacks) for inline images/maps. Restored content is closed off by one `RestoreBarRenderer` row and the lines themselves are left alone. Restoring 3,000 lines costs ~18 ms before the first frame. `restore:` is the third member of the `save:`/`logRoot:` family — **null by default, so no test and no snapshot owns one**. +- **Every server's MSSP report is kept, and the INFO screen reads it** (`MsspCache`, Core; `mssp.json` + beside `config.json`, keyed by `host:port`; F5 ▸ `i`). Fourth of the `save:`/`logRoot:`/`restore:` + family with **one deliberate difference**: the constructor parameter is null by default like the + others, but the *field* never is — a `MsspCache` with no path is memory-only **by construction**, so + the "a snapshot writes nothing" guarantee is a property of the object rather than a null check at + each use site, and the screen needs no "is there a cache" branch. Three decisions worth not + relitigating. **Keyed by endpoint, not world**: MSSP describes a *server*, a world name is a + user-editable label two entries may share, and a rename must not lose a report. **A second report + replaces the first**: MSSP is not a delta protocol — a server sends its whole table once per + connection — so a merge would keep variables it has stopped publishing and would leave a report that + is a snapshot of no moment that existed. **Two timestamps**, because there are *three* states and two + would only separate two: `ConnectedAt` is written on the `Connected` transition and `ObservedAt` only + when a report arrives, so "never dialled", "dialled and publishes nothing" and "here is the report, + as of…" are three different screens. Report capture is bounded at the door + (`MaxVariables`/`MaxValuesPerVariable`/`MaxValueLength`), not only at the renderer — a value only the + screen trimmed would still be full size on disk and in memory on every later launch. +- **`IAC DO MSSP` is sent on connect** (`TelnetSessionOptions.RequestOptions`, set by `WorldSession`'s + session factory). The library opens with `IAC WILL NAWS` and nothing else, so a server that supports + MSSP but waits to be asked is never asked — and the INFO screen would then report it as publishing + none, which is a claim about the server made out of our own silence. - **A launch connects nothing unless it is told to** (`StartupConnections.Resolve`, Core). A host on the command line wins outright; otherwise it is every character with `ConnectAtStartup` (F5's `at start`), in configuration order; otherwise none, and the client says which of the two empty states it is in. @@ -163,7 +183,11 @@ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg `messages`, `quit`, `connections` (**two connections on one world** — the one view where the header's fraction, the rail's dots and the quit prompt's count are all visible together and all have to agree; every other view has at most one character connected per world, which is what hid a header dividing - connections by *worlds* and a quit prompt reducing them to distinct world names), `deletions`, `web`, + connections by *worlds* and a quit prompt reducing them to distinct world names), `deletions`, + `mssp`/`mssp-none`/`mssp-never` (the **three** states of the F5 ▸ `i` server-information report — + a report, a server that answered and publishes none, and a world nothing has dialled; all three + reached by driving the real `i` into a real F5, and all three needed because the two empty ones are + the pair it is easy to conflate), `web`, `rail-long`, `scrollback`, `scrollback-up`, `freeze-scrollback`, `prefix-panel` (the ⌃B which-key panel — the state `prefix` becomes a few hundred milliseconds later, if no key has arrived), `focus`/`focus-moved` (a split *and* a second command line — the one geometry showing a focused pane diff --git a/README.md b/README.md index ca182f0..93dd0b4 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,13 @@ Multi-world tabs · regex triggers · aliases · macros/keybinds · spawns · ma inline image viewer · puppets · multiple input windows · scripting (Lua) · TLS + IPv6 · HTML logging · GMCP / MSDP / MSSP / MCCP · MXP + Pueblo · Unicode/emoji. +**MSSP is in and readable.** Every server's own description of itself — name, players, uptime, +codebase, contact, website, and everything else it chose to publish — is captured on connect, kept +between launches, and read from **F5 ▸ `i`** on the selected world. It says when it was captured, +because a player count from last week is not a player count; and it distinguishes a world you have +never dialled from a server that answered and publishes no MSSP, which is the ordinary case on a MUSH +and not a fault. + ## Solution layout (planned) | Project | Responsibility | diff --git a/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs new file mode 100644 index 0000000..9a0dcfc --- /dev/null +++ b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs @@ -0,0 +1,423 @@ +using System.Globalization; +using System.Text.Json; +using System.Text.Json.Nodes; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace SharpMUTerm.Core.Telnet.Mssp; + +/// +/// What every server we have connected to said about itself, kept so the INFO screen can answer while +/// nothing is connected — which is when the question is asked. +/// +/// Not in config.json. That file holds what the user asked for and is one people hand-edit; +/// this holds what a stranger's server said, and a write per connect would make opening a socket dirty +/// somebody's settings. It is a sibling cache file — the shape SecretsStore and the restore log +/// already have — versioned on its own, so nothing here can force an AppConfiguration migration. +/// +/// +/// Keyed by host:port, not by world. MSSP describes a server; a world name is a +/// user-editable label that two entries may share and that a rename changes out from under a report that +/// has not. Keying by endpoint means two worlds pointed at one server read one report, renaming a world +/// keeps it, and a world repointed at a different host correctly stops seeing the old server's answer. +/// The cost is the one case where it reads oddly — a world whose port is edited from 4000 to 4001 looks +/// as though its report vanished — and that reading is right: those are two endpoints, they may run +/// different games, and one of them is the plaintext port and the other the TLS one. +/// +/// +/// A second report replaces the first outright. MSSP is not a delta protocol: a server sends its +/// whole table once per connection, so merging would keep variables it has stopped publishing for ever — +/// the same failure as a merged room-exit list, in a different costume. Worse, a merged report is a +/// snapshot of no moment that ever existed, which is precisely what a dated report exists not to be. So +/// the newest whole report wins, and moves with it. +/// +/// +/// Persistence is optional and the default is memory-only. A cache built with no path reads +/// nothing and writes nothing, which is what every test and every --snapshot gets: the guarantee +/// is structural rather than a null check at each use site, so there is no third call site to forget. +/// Only Program hands one a path. +/// +/// +/// Nothing here throws at a caller. A cache file that cannot be read starts empty and says so through +/// ; one that cannot be written degrades to memory-only for the session. +/// +/// +public sealed class MsspCache +{ + /// The file, beside config.json. + public const string FileName = "mssp.json"; + + /// This file's own schema version, independent of the configuration's. + public const int CurrentVersion = 1; + + /// + /// How many endpoints are kept. A client that has connected to two hundred distinct servers is not + /// a client whose two-hundred-and-first lookup is worth an unbounded file; the least recently + /// reached entry is dropped. The bound is in entries and not bytes because the per-entry + /// bounds below already make an entry finite. + /// + public const int MaxEndpoints = 200; + + /// + /// How many variables of one report are kept. The specification defines about forty-five and a + /// generous server invents a handful more; this is several times that, and it is here because MSSP + /// has no payload cap upstream (SubnegotiationBuffer guards GMCP, MSDP and CHARSET's TTABLE + /// and not this), so the number of variables a hostile server can put in one report is its choice. + /// + public const int MaxVariables = 128; + + /// How many values of one variable are kept. PORT and REFERRAL use a handful. + public const int MaxValuesPerVariable = 16; + + /// + /// How long one value may be. Bounding at the door rather than only at the renderer matters because + /// this file is written, read back and rendered: a megabyte NAME that only the screen trimmed + /// would still be a megabyte on disk and a megabyte in memory for every later launch. + /// + public const int MaxValueLength = 512; + + private readonly string? _path; + private readonly Dictionary _observations = new(StringComparer.Ordinal); + private readonly Lock _gate = new(); + + private bool _writable = true; + + /// + /// Builds a cache. null — the default — is memory-only: nothing is read and + /// nothing is ever written, which is the state a snapshot and every test runs in. + /// + public MsspCache(string? path = null) + { + _path = string.IsNullOrWhiteSpace(path) ? null : Path.GetFullPath(path); + if (_path is not null) + { + Problem = Load(_path, _observations); + } + } + + /// The cache file beside a configuration file, the way SecretsStore.PathFor resolves. + public static string PathFor(string configurationPath) + { + ArgumentException.ThrowIfNullOrEmpty(configurationPath); + var directory = Path.GetDirectoryName(Path.GetFullPath(configurationPath)); + return Path.Combine(directory ?? string.Empty, FileName); + } + + /// Diagnostics for anything this cache does on disk. + public ILogger Logger { get; set; } = NullLogger.Instance; + + /// Why the file on disk could not be read, or null when there was nothing wrong with it. + public string? Problem { get; } + + /// Whether this cache has a file at all; false means memory-only, by construction. + public bool IsPersistent => _path is not null; + + /// Every endpoint on record, most recently reached first. + public IReadOnlyList All + { + get + { + lock (_gate) + { + return [.. _observations.Values.OrderByDescending(o => o.ConnectedAt)]; + } + } + } + + /// + /// The identity a report is filed under: the host lower-cased with any trailing root dot removed, + /// a colon, and the port. Normalising means MUD.Example.ORG. and mud.example.org are + /// one server rather than two half-filled entries — the same folding a hostname comparison would do, + /// done once, here, so no caller has to remember to. + /// + public static string Key(string? host, int port) => + string.Concat( + (host ?? string.Empty).Trim().TrimEnd('.').ToLowerInvariant(), + ":", + port.ToString(CultureInfo.InvariantCulture)); + + /// What this endpoint last said, or null when nothing here has ever reached it. + public MsspObservation? Find(string? host, int port) + { + var key = Key(host, port); + lock (_gate) + { + return _observations.GetValueOrDefault(key); + } + } + + /// + /// Records that a session reached this endpoint, without a report. This is what separates "connected + /// and the server publishes nothing" from "never connected", and it is the reason the screen can say + /// the first out loud instead of showing the same emptiness for both. + /// + /// It never clears an existing report. A server that published MSSP once and has stopped is still a + /// server whose report we hold, dated when it arrived; blanking it on the next connect would throw + /// away the only answer we have in order to be freshly ignorant. + /// + /// + public void RecordConnection(string? host, int port, DateTimeOffset at) + { + var key = Key(host, port); + lock (_gate) + { + var existing = _observations.GetValueOrDefault(key); + _observations[key] = existing is null + ? new MsspObservation(key, at, null, null) + : existing with { ConnectedAt = at }; + Persist(); + } + } + + /// + /// Records a report from this endpoint, replacing whatever was there. See the type summary for why + /// replacement rather than a merge, and for what is kept of a hostile one. + /// + public void RecordReport(string? host, int port, MsspData report, DateTimeOffset at) + { + ArgumentNullException.ThrowIfNull(report); + + var key = Key(host, port); + var clamped = Clamp(report); + lock (_gate) + { + var connected = _observations.GetValueOrDefault(key)?.ConnectedAt ?? at; + _observations[key] = new MsspObservation(key, connected, clamped, at); + Persist(); + } + } + + /// + /// Cuts a report down to what this cache will hold: variables, each with + /// at most values of at most + /// characters. Order is preserved, so what survives is the head of what the server sent rather than + /// an arbitrary subset. + /// + /// Truncation happens here, before anything is stored or written, and not only in the + /// renderer. A value the screen trimmed would still have been read off the wire, held in memory, + /// written to disk and read back on every later launch at full size; bounding at the door is the + /// only place that costs a hostile server anything. + /// + /// + private static MsspData Clamp(MsspData report) + { + var kept = new List>>(); + foreach (var (name, values) in report) + { + if (kept.Count >= MaxVariables) + { + break; + } + + var trimmed = new List(Math.Min(values.Count, MaxValuesPerVariable)); + foreach (var value in values.Take(MaxValuesPerVariable)) + { + trimmed.Add(value.Length > MaxValueLength ? value[..MaxValueLength] : value); + } + + kept.Add(new KeyValuePair>(name, trimmed)); + } + + return MsspData.From(kept); + } + + // ---- Disk ---- + + private void Persist() + { + // Bounded first, and outside the early return below, because a memory-only cache is bounded + // too: the file is one reason for the cap and the process's own memory is the other, and a + // long-running client that dialled a thousand servers should not hold a thousand reports just + // because it happens to own no file. + Evict(); + + if (_path is null || !_writable) + { + return; + } + + try + { + var document = new JsonObject + { + ["version"] = CurrentVersion, + ["servers"] = Servers(), + }; + + Directory.CreateDirectory(Path.GetDirectoryName(_path) ?? string.Empty); + + // Temp file plus rename: a launch that is interrupted mid-write finds either the old file or + // the new one, never half of one. This is derived data and a truncated file would only cost a + // reset — but the same is true of the restore log, which does this, and a cache that can + // corrupt itself is a cache somebody eventually debugs. + var temporary = _path + ".tmp"; + File.WriteAllText(temporary, document.ToJsonString(new JsonSerializerOptions { WriteIndented = true })); + File.Move(temporary, _path, overwrite: true); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException or NotSupportedException) + { + // Memory-only from here. Retrying per connect would log the same failure for ever. + _writable = false; + Logger.LogWarning(e, "MSSP cache is not writable at {Path}; keeping reports in memory only.", _path); + } + } + + private JsonObject Servers() + { + var servers = new JsonObject(); + foreach (var observation in _observations.Values.OrderBy(o => o.Endpoint, StringComparer.Ordinal)) + { + var entry = new JsonObject + { + ["connectedAt"] = observation.ConnectedAt.ToString("O", CultureInfo.InvariantCulture), + }; + + if (observation is { Report: { } report, ObservedAt: { } observed }) + { + entry["observedAt"] = observed.ToString("O", CultureInfo.InvariantCulture); + + // An *array* of name/value pairs rather than an object, because MSSP's order is meaning: + // "multiple values should be ordered from least to most relevant", and a JSON object's + // member order is not something a reader is obliged to preserve. + var variables = new JsonArray(); + foreach (var (name, values) in report) + { + var list = new JsonArray(); + foreach (var value in values) + { + list.Add(value); + } + + variables.Add(new JsonObject { ["name"] = name, ["values"] = list }); + } + + entry["variables"] = variables; + } + + servers[observation.Endpoint] = entry; + } + + return servers; + } + + /// Drops the least recently reached endpoints until the file is back within its bound. + private void Evict() + { + if (_observations.Count <= MaxEndpoints) + { + return; + } + + foreach (var stale in _observations.Values + .OrderByDescending(o => o.ConnectedAt) + .Skip(MaxEndpoints) + .Select(o => o.Endpoint) + .ToList()) + { + _observations.Remove(stale); + } + } + + /// + /// Reads the file into , returning what was wrong with it or null. A missing + /// file is not wrong. Neither is a single malformed entry: it is skipped and counted, because one + /// bad line in a cache of two hundred servers is not a reason to forget the other hundred and + /// ninety-nine. + /// + private static string? Load(string path, Dictionary into) + { + try + { + if (!File.Exists(path)) + { + return null; + } + + if (JsonNode.Parse(File.ReadAllText(path)) is not JsonObject root) + { + return $"{FileName} is not a JSON object; starting with no cached server information."; + } + + var version = root["version"]?.GetValue() ?? 0; + if (version > CurrentVersion) + { + return $"{FileName} was written by a newer version of SharpMUTerm " + + $"(schema {version.ToString(CultureInfo.InvariantCulture)}); ignoring it."; + } + + if (root["servers"] is not JsonObject servers) + { + return null; + } + + var skipped = 0; + foreach (var (endpoint, node) in servers) + { + if (node is not JsonObject entry || Read(endpoint, entry) is not { } observation) + { + skipped++; + continue; + } + + into[endpoint] = observation; + } + + return skipped == 0 + ? null + : $"{FileName}: {skipped.ToString(CultureInfo.InvariantCulture)} unreadable entries skipped."; + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException or JsonException or FormatException) + { + return $"{FileName} could not be read ({e.GetType().Name}); starting with no cached server information."; + } + } + + private static MsspObservation? Read(string endpoint, JsonObject entry) + { + if (endpoint.Length == 0 + || entry["connectedAt"]?.GetValue() is not { } connectedText + || !DateTimeOffset.TryParse( + connectedText, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var connectedAt)) + { + return null; + } + + if (entry["observedAt"]?.GetValue() is not { } observedText + || !DateTimeOffset.TryParse( + observedText, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var observedAt) + || entry["variables"] is not JsonArray variables) + { + return new MsspObservation(endpoint, connectedAt, null, null); + } + + var pairs = new List>>(); + foreach (var variable in variables) + { + if (variable is not JsonObject pair + || pair["name"]?.GetValue() is not { Length: > 0 } name) + { + continue; + } + + var values = new List(); + if (pair["values"] is JsonArray list) + { + foreach (var value in list) + { + if (value?.GetValue() is { } text) + { + values.Add(text.Length > MaxValueLength ? text[..MaxValueLength] : text); + } + } + } + + pairs.Add(new KeyValuePair>(name, values)); + if (pairs.Count >= MaxVariables) + { + break; + } + } + + return new MsspObservation(endpoint, connectedAt, MsspData.From(pairs), observedAt); + } +} diff --git a/src/SharpMUTerm.Core/Telnet/Mssp/MsspObservation.cs b/src/SharpMUTerm.Core/Telnet/Mssp/MsspObservation.cs new file mode 100644 index 0000000..41866ab --- /dev/null +++ b/src/SharpMUTerm.Core/Telnet/Mssp/MsspObservation.cs @@ -0,0 +1,46 @@ +namespace SharpMUTerm.Core.Telnet.Mssp; + +/// +/// What one server endpoint has told us about itself, and when — the unit the INFO screen renders and +/// the unit stores. +/// +/// Two timestamps, because there are three states and two would only distinguish two. +/// is the last time a session reached this endpoint at all; +/// is when the report in arrived, and is null exactly +/// when no report ever has. So: +/// +/// +/// no observation for an endpoint — never connected; +/// an observation with a null — connected, and the server published no +/// MSSP, which on a MUSH is the ordinary case and not a fault; +/// an observation with a report — here is what it said, as of . +/// +/// +/// Collapsing the first two into "no data" is the easy mistake and it makes the screen untrustworthy: +/// a client that says the same thing about a server it has never spoken to and a server that answered +/// is a client whose screen you have to go and verify elsewhere. +/// +/// +/// The two are kept apart rather than one being derived from the other because they drift on purpose. +/// A server that published MSSP in January and stopped in March leaves a March +/// beside a January , and the screen dates the +/// report — anything else would present a stale player count as current, which is the whole +/// reason a capture time is recorded at all. +/// +/// +/// The normalised host:port this describes — see . +/// When a session last reached this endpoint. +/// The last MSSP report it published, or null when it has never published one. +/// When arrived; null exactly when it is null. +public sealed record MsspObservation( + string Endpoint, + DateTimeOffset ConnectedAt, + MsspData? Report, + DateTimeOffset? ObservedAt) +{ + /// + /// True when this endpoint has been reached and published nothing — the state the screen has to + /// word as "optional, and normally absent" rather than as an error. + /// + public bool PublishesNothing => Report is null; +} diff --git a/src/SharpMUTerm.Tui/DemoScene.cs b/src/SharpMUTerm.Tui/DemoScene.cs index ca1dcca..5781ba3 100644 --- a/src/SharpMUTerm.Tui/DemoScene.cs +++ b/src/SharpMUTerm.Tui/DemoScene.cs @@ -1,6 +1,7 @@ using SharpMUTerm.Core.Automation; using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Telnet; +using SharpMUTerm.Core.Telnet.Mssp; using SharpMUTerm.Core.Text; using SharpMUTerm.Core.Workspaces; @@ -80,6 +81,53 @@ private static void AddWorlds(AppConfiguration config) }); } + /// + /// The MSSP report the demo's Aetherfall is pretended to have published, for the mssp + /// snapshot view. + /// + /// It is written through SharpMUTermApp.CaptureMssp — the same method the live + /// subscription calls — and not poked into a cache here. The demo has no session, so anything a + /// session writes has to be written by hand; three separate bugs have hidden in exactly that gap, + /// and the rule this file already carries for the main window's title is the rule here: pin the + /// faked state against the live writer rather than against a second copy of what it does. + /// + /// + /// The contents are chosen to make the screen's own hard cases visible in one frame: a multi-valued + /// PORT and CODEBASE (drawn as the lists they are, not as their first value), a + /// -1 world count (drawn as unknown), an UPTIME that is a Unix timestamp + /// (drawn as a duration), an official variable with no strongly typed reading (CHARSET), an + /// unofficial one that looks standard (PUEBLO), and one nothing anywhere has heard of. + /// + /// + public static MsspData MsspReport() => MsspData.From( + [ + Variable("NAME", "Aetherfall"), + Variable("PLAYERS", "37"), + Variable("UPTIME", "1735689600"), + Variable("CODEBASE", "PennMUSH 1.8.8", "SharpMUSH 1.0"), + Variable("FAMILY", "TinyMUD"), + Variable("HOSTNAME", "aetherfall.mux"), + Variable("PORT", "4200", "4201"), + Variable("SSL", "4201"), + Variable("CHARSET", "UTF-8"), + Variable("CONTACT", "wizards@aetherfall.mux"), + Variable("WEBSITE", "https://aetherfall.mux/"), + Variable("GENRE", "Fantasy"), + Variable("STATUS", "Live"), + Variable("MINIMUM AGE", "16"), + Variable("LANGUAGE", "English"), + Variable("LOCATION", "United Kingdom"), + Variable("ROOMS", "-1"), + Variable("ANSI", "1"), + Variable("UTF-8", "1"), + Variable("PUEBLO", "1"), + Variable("DISCORD", "https://discord.gg/aetherfall"), + Variable("CORVID SPECIFIC", "nevermore"), + ]); + + private static KeyValuePair> Variable(string name, params string[] values) => + new(name, values); + private static void AddTriggerSets(AppConfiguration config) { var teal = TerminalColor.FromRgb(0x00, 0xf5, 0xb7); diff --git a/src/SharpMUTerm.Tui/MsspScreenRenderer.cs b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs new file mode 100644 index 0000000..9f36e5f --- /dev/null +++ b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs @@ -0,0 +1,439 @@ +using System.Globalization; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Telnet.Mssp; +using static SharpMUTerm.Tui.MarkupText; +using static SharpMUTerm.Tui.ScreenPalette; + +namespace SharpMUTerm.Tui; + +/// +/// The read-only MSSP report for one world — what the server said about itself, and when it said it. +/// Reached from F5 with i on the selected world. +/// +/// Read-only is the shape, not an omission. Every other settings screen exists to change +/// something, and this project's affordance rule is that a field well means "the keyboard can change +/// this here" and its absence means it cannot (). A whole screen of +/// well-less rows is therefore exactly right, and it has to be deliberately well-less so it does +/// not read as a form whose wells failed to render. Nothing here calls . +/// +/// +/// Three states, and the two empty ones must not look the same. A world we have never connected +/// to, a server that answered and publishes no MSSP, and a report. Conflating the last two is the easy +/// mistake and it is the one that makes a client look broken: on a MUSH, "this server publishes no MSSP" +/// is the ordinary answer, and saying so — with what we do know from the world's own configuration +/// beside it — is the same information as "no data" and the opposite impression. +/// +/// +/// Every value here came off the wire from a stranger. Three things follow, and all three +/// happen in Row in this order: control characters are replaced (a raw newline in a value would +/// otherwise end the row and shift everything below it), the raw text is truncated before +/// escaping (truncating escaped markup can split a [[ pair into an unbalanced tag), and only +/// then is it escaped. Column widths are functions of the terminal and never of the data, +/// because this repository has twice paid for chrome whose width was a function of something arriving +/// from the wire — RailRenderer.UnsentFieldWidth, and the status row's scrollback distance that +/// wrapped every pane's height at 99 → 100. +/// +/// +internal static class MsspScreenRenderer +{ + /// The screen's title, and what the ⌃P entry and the snapshot view are named after. + internal const string Title = "Server information"; + + /// The snapshot view name (--view mssp). + internal const string View = "mssp"; + + /// + /// How wide the variable-name column runs. Official MSSP names top out at XTERM 256 COLORS + /// (17) and HIRING BUILDERS (15); an invented one can be any length at all, so the column is + /// a constant and a longer name is elided into it rather than being allowed to push the values + /// right. See the type summary for why that is not fussiness. + /// + internal const int NameWidth = 20; + + /// + /// The most of one value that is ever drawn. A server may legitimately send a long WEBSITE or + /// a wordy DESCRIPTION; it may also send half a megabyte. + /// already bounds what is stored — this bounds what one row spends, which is the layout half of the + /// same rule. + /// + internal const int ValueWidth = 58; + + /// The fewest cells a value is still worth drawing in, on a terminal too narrow for more. + private const int MinValueWidth = 16; + + /// What a row spends before the value: the mark column, the name column, and their gaps. + private const int RowChrome = 1 + 1 + 1 + NameWidth + 2; + + /// + /// How much of a value this frame can afford — where there is room for it, + /// and what is left of the terminal where there is not. + /// + /// A function of the terminal, never of the data. That is the whole discipline: this + /// repository has twice shipped chrome whose width was a function of something arriving from the + /// wire, and both times the symptom was a row growing past its box and taking a row off everything + /// below it. A value at the full 58 cells needs 83, so at 80 columns the constant alone would have + /// overrun by three — visible in a rendered frame and in nothing else. + /// + /// + private static int ValueCells(int width) => + width <= 0 ? ValueWidth : Math.Clamp(width - RowChrome - 1, MinValueWidth, ValueWidth); + + /// + /// How many values of one variable are listed before the rest are summarised. Multi-valued variables + /// are real and are the reason the model is a name → list map at all (PORT, + /// REFERRAL, CODEBASE), so they are drawn as the list they are — one value per row, + /// the name printed once — rather than as their first or last value. This caps the rows one variable + /// can spend. + /// + internal const int MaxValueRows = 8; + + /// What a row shows where the server said nothing at all. + internal const string Unreported = "—"; + + /// What a numeric world variable of -1 reads as: the specification's "not available". + internal const string Unavailable = "unknown"; + + /// The words the never-connected state is put in. + internal const string NeverConnected = "No server information yet — connect once and this fills in."; + + /// + /// The words the connected-and-silent state is put in. It says the absence is normal because it is: + /// MSSP is optional, most MUSHes do not implement it, and a client that reported the ordinary case + /// as a failure would be teaching people to distrust the screen. + /// + internal const string NoMssp = "This server does not publish MSSP. It is optional, and most MUSHes do not."; + + /// + /// The first-class rows, in the order somebody browsing a world list wants them. Everything not on + /// this list is still shown — under — because a protocol whose entire + /// purpose is servers describing themselves is a protocol whose unofficial half is where the + /// interesting things are. + /// + private static readonly (string Label, string Variable)[] Headline = + [ + ("name", MsspVariables.Name), + ("players", MsspVariables.Players), + ("uptime", MsspVariables.Uptime), + ("codebase", MsspVariables.Codebase), + ("family", MsspVariables.Family), + ("hostname", MsspVariables.Hostname), + ("port", MsspVariables.Port), + ("ssl", MsspVariables.Ssl), + ("charset", MsspVariables.Charset), + ("contact", MsspVariables.Contact), + ("website", MsspVariables.Website), + ("genre", MsspVariables.Genre), + ("status", MsspVariables.Status), + ("minimum age", MsspVariables.MinimumAge), + ]; + + /// The heading over the variables that did not earn a headline row. + internal const string EverythingElse = "EVERYTHING ELSE"; + + /// The heading over the headline rows. + internal const string SummaryHeading = "SERVER"; + + /// + /// How an unofficial variable is marked. Official and unofficial must both be visible and must be + /// told apart: a name the specification defines is a claim a crawler and a client read the same way, + /// and a name somebody's codebase invented is not — and the reader cannot tell which is which from + /// the name alone (DISCORD is official as of 2.7.0; PUEBLO, which looks every bit as + /// standard, is not). + /// + internal const string UnofficialMark = "·"; + + /// The legend that says what means, so the mark is readable. + internal const string UnofficialLegend = "not in the MSSP specification"; + + /// + /// The screen's navigable shape: one stop per drawn body row, so ↑↓ scroll a report longer than the + /// screen through . + /// + /// There is nothing to edit, nothing to toggle and nothing to remove, and the shape says so: the + /// header offers no ⏎ edit, no Space toggle and no Del remove, because every one + /// of those hints is derived from this model rather than written by the screen. + /// + /// + internal static ScreenModel Model( + WorldDefinition? world, MsspObservation? observation, DateTimeOffset now, int width = 0) => + new(ScreenModel.Stops(Body(world, observation, now, width).Count)); + + /// + /// The screen's header band: its title, and the one key that leaves it. It does not go through + /// because that composes a settings screen's contract — + /// F5/Esc close — and this screen's Esc does something else: it goes back to the + /// screen that opened it, with its selection intact. A header offering "close" would name the right + /// key for the wrong outcome. + /// + internal static string HeaderLine(int width) + { + var hints = $"[{Label}]↑↓ scroll · [/][{Accent}]Esc[/][{Label}] back [/]"; + return SpreadLR($" [bold {Value}] {Escape(Title)}[/]", hints, width); + } + + /// The screen's action bar: where the cursor is in the report, and the key that leaves. + internal static string FooterLine( + WorldDefinition? world, MsspObservation? observation, ScreenFocus? focus, int width, DateTimeOffset now) + { + var rows = Body(world, observation, now, width).Count; + var context = ScreenChrome.Context( + world is null ? null : Escape(world.Name), + observation is null ? null : Escape(observation.Endpoint), + focus is { } cursor && cursor.Index >= 0 && rows > 0 + ? ScreenChrome.Position("row", cursor.Index, rows) + : null); + + return SpreadLR( + " " + context, + $"[{Label}] [[Esc]] Back [/]", + width); + } + + /// + /// The whole report as markup rows, with the cursor band on the focused one and the block windowed + /// to . is the same rows without either, which is what + /// counts — so the number of cursor stops and the number of drawn rows are one + /// number by construction rather than by two functions agreeing. + /// + internal static List Render( + WorldDefinition? world, + MsspObservation? observation, + DateTimeOffset now, + ScreenFocus? focus = null, + int height = 0, + int width = 0) + { + var cursor = focus ?? ScreenFocus.None; + var body = Body(world, observation, now, width); + for (var i = 0; i < body.Count; i++) + { + // The bar spans the terminal, not the columns: this screen is one pane filling the window, + // so a bar that stopped where the value column stops would read as a second, invisible + // column edge. On F5 the same call pads to the pane's width, which is the same rule. + body[i] = ScreenChrome.Cursor( + body[i], cursor.IsOn(0, i), width > 0 ? width : RowChrome + ValueWidth); + } + + return ScreenChrome.Window(body, height); + } + + /// + /// The report's rows, unfocused and unwindowed. Deterministic in its arguments, because + /// counts these rows and draws them: two functions that + /// disagreed about how many there are would give the screen a cursor stop nobody ever drew, which is + /// the failure exists one level up to stop. + /// + internal static List Body( + WorldDefinition? world, MsspObservation? observation, DateTimeOffset now, int width = 0) + { + var cells = ValueCells(width); + var rows = new List(); + AddConfigured(rows, world, cells); + + if (observation is null) + { + rows.Add(string.Empty); + rows.Add($" [{Muted}]{Escape(NeverConnected)}[/]"); + return rows; + } + + if (observation is { Report: null } or { ObservedAt: null }) + { + rows.Add(string.Empty); + rows.Add($" [{Muted}]{Escape(NoMssp)}[/]"); + rows.Add(string.Empty); + rows.Add(Row("last seen", Since(observation.ConnectedAt, now), cells)); + return rows; + } + + var report = observation.Report!; + rows.Add(string.Empty); + rows.Add(Row("captured", Since(observation.ObservedAt!.Value, now), cells)); + rows.Add(string.Empty); + rows.Add($" [{Label}]{SummaryHeading}[/]"); + + var drawn = new HashSet(StringComparer.Ordinal); + foreach (var (label, variable) in Headline) + { + drawn.Add(variable); + AddVariable(rows, label, report[variable], variable, report, now, cells); + } + + var remaining = report.Keys.Where(name => !drawn.Contains(name)).ToList(); + rows.Add(string.Empty); + rows.Add($" [{Label}]{EverythingElse}[/]"); + if (remaining.Count == 0) + { + rows.Add($" [{Muted}]nothing else was sent[/]"); + return rows; + } + + foreach (var name in remaining) + { + AddVariable(rows, name, report[name], name, report, now, cells); + } + + rows.Add(string.Empty); + rows.Add($" [{Muted}]{UnofficialMark} {Escape(UnofficialLegend)}[/]"); + return rows; + } + + /// + /// What the client knows without asking anybody: the world's own name, host, port and whether the + /// connection is encrypted. It heads every state, including both empty ones — which is the point. + /// A screen that had nothing at all to show for a world it has never reached would be a screen you + /// stop opening; one that shows the configuration it does hold answers half the question. + /// + private static void AddConfigured(List rows, WorldDefinition? world, int cells) + { + if (world is null) + { + rows.Add($" [{Muted}]no world selected[/]"); + return; + } + + rows.Add(Row("world", world.Name, cells)); + rows.Add(Row( + "address", + $"{world.Host}:{world.Port.ToString(CultureInfo.InvariantCulture)}", + cells)); + rows.Add(Row("transport", world.UseTls ? "TLS" : "plain", cells)); + } + + /// + /// One variable, as one row per value. The name is printed on the first row only, so a three-port + /// server reads as one variable with three values rather than as three variables — and the values + /// are drawn least-to-most-relevant, in wire order, which is the order the specification gives them + /// meaning in ("the last reported value should be used as the default value"). + /// + private static void AddVariable( + List rows, + string label, + IReadOnlyList values, + string variable, + MsspData report, + DateTimeOffset now, + int cells) + { + var mark = report.ContainsKey(variable) && report.UnofficialNames.Contains(variable) + ? UnofficialMark + : " "; + + if (values.Count == 0) + { + // Two different absences, and the row says which. A variable the server sent with no value + // is a fact about the server; one it never mentioned is a fact about the report. + rows.Add(Row(label, report.ContainsKey(variable) ? Unavailable : Unreported, cells, mark)); + return; + } + + var shown = Math.Min(values.Count, MaxValueRows); + for (var i = 0; i < shown; i++) + { + rows.Add(Row( + i == 0 ? label : string.Empty, + Reading(variable, values[i], now), + cells, + i == 0 ? mark : " ")); + } + + if (values.Count > shown) + { + rows.Add(Row( + string.Empty, + $"… {(values.Count - shown).ToString(CultureInfo.InvariantCulture)} more", + cells)); + } + } + + /// + /// How one value reads. Almost all of them read as themselves; the two exceptions are the ones where + /// the raw string is actively misleading — UPTIME is a Unix timestamp, which is a number + /// nobody can read as a duration, and -1 is the specification's marker for "this server + /// cannot count that" rather than a count of minus one. + /// + private static string Reading(string variable, string value, DateTimeOffset now) + { + if (string.Equals(variable, MsspVariables.Uptime, StringComparison.Ordinal) + && long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var unix) + && unix > 0) + { + var booted = DateTimeOffset.FromUnixTimeSeconds(unix); + return $"{Duration(now - booted)} (since {booted.UtcDateTime:yyyy-MM-dd HH:mm} UTC)"; + } + + return string.Equals(value, "-1", StringComparison.Ordinal) ? Unavailable : value; + } + + /// + /// A label value row. The label is padded to a constant, the value is sanitised, truncated + /// and escaped, and the whole thing carries a one-cell mark column — reserved, blank when the + /// variable is official, so a report of unofficial variables and a report of official ones put their + /// values in the same column. + /// + private static string Row(string label, string value, int cells, string mark = " ") => + $" [{Muted}]{mark}[/] [{Label}]{Escape(Fit(label, NameWidth)).PadRight(NameWidth)}[/] " + + ScreenChrome.ReadOnly(Fit(Sanitize(value), cells)); + + /// + /// Replaces every control character with a space. This is the first thing done to any value and it + /// is not cosmetic: a markup block is a list of rows, so a raw \n inside one value would end + /// that row early and push a fragment of a stranger's text onto a line of its own, below the row it + /// belongs to and outside the column it was measured for. An ESC would be worse — the frame + /// is ANSI, and the compositor is not the only thing that reads it. + /// + private static string Sanitize(string value) + { + if (!value.Any(char.IsControl)) + { + return value; + } + + return string.Create(value.Length, value, static (span, source) => + { + for (var i = 0; i < source.Length; i++) + { + span[i] = char.IsControl(source[i]) ? ' ' : source[i]; + } + }); + } + + /// + /// Truncates raw text to cells, ellipsis included in the count. It runs + /// before at every call site: escaping doubles every bracket, + /// so truncating escaped markup can cut a [[ in half and leave an unbalanced tag the parser + /// then eats the rest of the row with. + /// + private static string Fit(string text, int width) => + text.Length <= width ? text : text[..Math.Max(1, width - 1)] + "…"; + + /// + /// How long ago something was, in words, with the exact instant beside it. Both halves earn their + /// place: "3 days ago" is what tells a reader the player count in front of them is not current, and + /// the timestamp is what lets them decide whether it matters. A screen that presented a week-old + /// snapshot with no date at all would be reporting stale data as fact. + /// + private static string Since(DateTimeOffset at, DateTimeOffset now) + { + var ago = now - at; + var words = ago < TimeSpan.Zero ? "just now" : $"{Duration(ago)} ago"; + return $"{words} ({at.UtcDateTime:yyyy-MM-dd HH:mm} UTC)"; + } + + /// A coarse duration — the largest unit that is not zero, which is all any of this needs. + private static string Duration(TimeSpan span) + { + if (span < TimeSpan.Zero) + { + span = TimeSpan.Zero; + } + + return span.TotalDays >= 1 ? Plural((int)span.TotalDays, "day") + : span.TotalHours >= 1 ? Plural((int)span.TotalHours, "hour") + : span.TotalMinutes >= 1 ? Plural((int)span.TotalMinutes, "minute") + : "moments"; + } + + private static string Plural(int count, string noun) => + $"{count.ToString(CultureInfo.InvariantCulture)} {noun}{(count == 1 ? string.Empty : "s")}"; +} diff --git a/src/SharpMUTerm.Tui/MsspScreenView.cs b/src/SharpMUTerm.Tui/MsspScreenView.cs new file mode 100644 index 0000000..d8a6ba2 --- /dev/null +++ b/src/SharpMUTerm.Tui/MsspScreenView.cs @@ -0,0 +1,60 @@ +using SharpConsoleUI.Builders; +using SharpConsoleUI.Controls; +using SharpConsoleUI.Layout; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Telnet.Mssp; + +namespace SharpMUTerm.Tui; + +/// +/// Composes 's blocks into the control tree the settings overlay hosts. +/// One column, not two: this screen is a report rather than a list beside an editor, so there is no +/// second pane to divide, and the hairline every other screen carries would be dividing nothing. +/// +internal static class MsspScreenView +{ + internal static IWindowControl Build( + WorldDefinition? world, + MsspObservation? observation, + DateTimeOffset now, + int width, + ScreenFocus? focus = null, + int height = 0) + { + var header = ScreenChrome.Band(MsspScreenRenderer.HeaderLine(width), ScreenPalette.HeaderBg); + var footer = ScreenChrome.Band( + MsspScreenRenderer.FooterLine(world, observation, focus, width, now), ScreenPalette.FooterBg); + + var rows = ScreenChrome.Rows(height); + var body = ScreenChrome.Stretch(new MarkupControl( + MsspScreenRenderer.Render(world, observation, now, focus, rows, width))); + + var root = Controls.Grid() + .WithAlignment(HorizontalAlignment.Stretch) + .WithVerticalAlignment(VerticalAlignment.Fill); + + if (rows <= 0) + { + root.Rows(GridLength.Cells(1), GridLength.Star(1), GridLength.Cells(1)).Columns(GridLength.Star(1)); + root.Place(header, 0, 0, 1, 1); + root.Place(body, 1, 0, 1, 1); + root.Place(footer, 2, 0, 1, 1); + return root.Build(); + } + + // The body is sized to its content and the slack below it belongs to the backdrop, the same call + // ScreenChrome.Split makes: a report of six rows should not be drawn as a thirty-row empty panel + // whose emptiness reads as missing data on a screen whose whole subject is what is missing. + root.Rows( + GridLength.Cells(1), + GridLength.Cells(Math.Clamp( + MsspScreenRenderer.Render(world, observation, now, focus, rows, width).Count, 1, rows)), + GridLength.Star(1), + GridLength.Cells(1)) + .Columns(GridLength.Star(1)); + root.Place(header, 0, 0, 1, 1); + root.Place(body, 1, 0, 1, 1); + root.Place(footer, 3, 0, 1, 1); + return root.Build(); + } +} diff --git a/src/SharpMUTerm.Tui/Program.cs b/src/SharpMUTerm.Tui/Program.cs index 5d1bf82..5e0d17e 100644 --- a/src/SharpMUTerm.Tui/Program.cs +++ b/src/SharpMUTerm.Tui/Program.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Logging; using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Telnet.Mssp; using SharpMUTerm.Core.Text; using SharpMUTerm.Graphics; using SharpConsoleUI.Drivers; @@ -106,13 +107,28 @@ private static int Main(string[] args) Logger = diagnostics.For("SharpMUTerm.RestoreLog"), }; + // What every server has said about itself, beside the configuration and deliberately not in it: + // config.json is what the user asked for and is hand-edited, and a write per connect has no + // business landing there. Resolved here for the same reason logRoot and the restore log are — + // only this code knows it is the live client. Anything else gets a memory-only cache and so + // provably writes nothing. + var mssp = new MsspCache(MsspCache.PathFor(ConfigurationStore.DefaultPath)) + { + Logger = diagnostics.For("SharpMUTerm.Mssp"), + }; + if (mssp.Problem is { } msspProblem) + { + loadLogger.LogWarning("{Notice}", msspProblem); + } + var liveApp = new SharpMUTermApp( config, capabilities, diagnostics: diagnostics, save: saved => ConfigurationStore.Save(ConfigurationStore.DefaultPath, saved), logRoot: logRoot, - restore: restore); + restore: restore, + mssp: mssp); var exitCode = liveApp.Run(startup); // blocks on the SharpConsoleUI main loop until exit // Persist the workspace so the next launch resumes where this one left off. @@ -254,6 +270,12 @@ private static void WriteUsage(TextWriter usage) // password go" should be answerable without reading the source. Config is safe to share; this is not. usage.WriteLine($"Secrets: {SecretsStore.PathFor(ConfigurationStore.DefaultPath)}" + " — character passwords, plain text, owner-only. Not the file to paste."); + + // Named for the same reason: it is a file this client creates in the user's own directory, and + // "what is this and can I delete it" should be answerable from the help page. It can: it is a + // cache of what servers published, and deleting it costs only the next connection's report. + usage.WriteLine($"Servers: {MsspCache.PathFor(ConfigurationStore.DefaultPath)}" + + " — each server's last MSSP report (F5 ▸ i). A cache; safe to delete."); // "why does it connect to *that*?" is the question this setting answers, so the answer belongs // on the page a user reaches for when they ask it. Both halves are stated: what connects with no // host, and that a host overrides it. diff --git a/src/SharpMUTerm.Tui/ScreenChrome.cs b/src/SharpMUTerm.Tui/ScreenChrome.cs index 593a816..71b4e59 100644 --- a/src/SharpMUTerm.Tui/ScreenChrome.cs +++ b/src/SharpMUTerm.Tui/ScreenChrome.cs @@ -26,7 +26,12 @@ internal static class ScreenChrome /// /// internal static string Hints( - string verbs, string fkey, bool editable = false, ScreenFocus? focus = null, bool removable = false) + string verbs, + string fkey, + bool editable = false, + ScreenFocus? focus = null, + bool removable = false, + bool detailed = false) { if (focus?.Edit is { } edit) { @@ -46,7 +51,14 @@ internal static string Hints( + $"[{ScreenPalette.Label}] close [/]"; } - var all = (editable ? verbs + EditHint : verbs) + (removable ? DeleteHint : string.Empty); + // Order is a budget, not taste: a header narrower than its hints loses the *tail* of this string + // (see the 80-column frames), so whatever is appended last is what a narrow terminal does not + // get. `i info` goes after `Del remove` because Delete is the destructive key and must be the + // one that survives — and because the INFO key's drawn row (`i info on Aetherfall`) is on the + // screen either way, while a key that removes a world has only this line and its own row. + var all = (editable ? verbs + EditHint : verbs) + + (removable ? DeleteHint : string.Empty) + + (detailed ? DetailHint : string.Empty); return $"[{ScreenPalette.Label}]{all} · [/][{ScreenPalette.Accent}]{fkey}[/][{ScreenPalette.Label}]/[/]" + $"[{ScreenPalette.Accent}]Esc[/][{ScreenPalette.Label}] close [/]"; } @@ -81,6 +93,15 @@ internal static string Hints( /// internal const string DeleteHint = " · Del remove"; + /// + /// What a screen adds to its hints when — and only when — a pane offers a read-only report on the + /// selected row. It matters more than that this is derived rather than + /// written: i is an ordinary letter, so a screen that answered it without saying so would be + /// a hidden feature, and one that said so without answering it would look broken on the one pane + /// where the key does nothing. + /// + internal const string DetailHint = " · i info"; + /// The hints that replace a screen's own while a field edit is open. internal const string EditingHints = "⏎ commit · Esc revert"; @@ -697,9 +718,12 @@ internal static List Buttons( continue; } - lines.Add(button.Kind == ScreenButtonKind.Remove - ? RemovalRow(button) - : Cursor(AddRow(button), cursor.IsOn(pane, firstIndex + i), width)); + lines.Add(button.Kind switch + { + ScreenButtonKind.Remove => KeyHintRow(button, RemovesWord), + ScreenButtonKind.Detail => KeyHintRow(button, InfoWords), + _ => Cursor(AddRow(button), cursor.IsOn(pane, firstIndex + i), width), + }); } return lines; @@ -715,17 +739,21 @@ private static string AddRow(ScreenButton button) } /// - /// What Delete would take, as a row. Never drawn with a cursor bar, because the cursor cannot get - /// there — that is the whole fix for "only the last world can be deleted". + /// What a targeted key would act on, as a row: the key, the verb, the victim or subject. Never drawn + /// with a cursor bar, because the cursor cannot get there — that is the whole fix for "only the last + /// world can be deleted", and the reason the INFO key is drawn this way too rather than as a chip. /// - private static string RemovalRow(ScreenButton button) => + private static string KeyHintRow(ScreenButton button, string verb) => $"[{ScreenPalette.Accent}]{MarkupText.Escape(button.Label)}[/]" - + $" [{ScreenPalette.Label}]{RemovesWord}[/] " + + $" [{ScreenPalette.Label}]{verb}[/] " + $"[{ScreenPalette.Value}]{MarkupText.Escape(button.Target ?? string.Empty)}[/]"; /// The verb on a removal row, between the key and what it would take. internal const string RemovesWord = "removes"; + /// The verb on a report row, between the key and what it would report on. + internal const string InfoWords = "info on"; + /// /// Where the cursor is within one of a screen's lists — trigger 1/4, world 2/2. Every /// footer's context line opens with one of these, so the eight screens answer the same question in diff --git a/src/SharpMUTerm.Tui/ScreenModel.cs b/src/SharpMUTerm.Tui/ScreenModel.cs index 2f7b5c7..8bd2986 100644 --- a/src/SharpMUTerm.Tui/ScreenModel.cs +++ b/src/SharpMUTerm.Tui/ScreenModel.cs @@ -134,6 +134,38 @@ internal static ScreenButton Add( /// ; it defaults to , which is the honest /// answer for every row that takes nothing else with it. /// + /// + /// What a read-only detail screen's key is drawn as, for the same reason + /// is: the row naming it is not a cursor stop, so a chip would be an + /// affordance for something the keyboard cannot land on. + /// + internal const string DetailKeyLabel = "i"; + + /// + /// Opens a read-only report about the selected row. It is a so that the + /// row the screen draws and the key that runs it come from one place, exactly as a removal does — + /// but it changes nothing, so it returns no undo and moves no cursor, and + /// runs it outside . Navigation is + /// not an edit: routing it through the edit log would persist the configuration and re-periodise + /// every running timer every time somebody looked at a world. + /// + /// The row this would report on, named on the drawn key-hint row. + /// Puts the report on the screen. + internal static ScreenButton Detail(string target, Action open) + { + ArgumentNullException.ThrowIfNull(open); + + return new ScreenButton( + DetailKeyLabel, + () => + { + open(); + return new ScreenPress(null); + }, + ScreenButtonKind.Detail, + target); + } + internal static ScreenButton Remove( IList list, int index, int offset = 0, string? target = null, Func? describe = null) { @@ -173,6 +205,16 @@ internal enum ScreenButtonKind /// Aetherfall) rather than as a chip, because it is not a cursor stop — see . /// Remove, + + /// + /// Opens a read-only report about the selected row (i info on Aetherfall), changing nothing. + /// It is drawn and navigated exactly as a removal is, and for the identical reason: the action has a + /// target, and an action with a target must not steal the cursor from the thing it acts on + /// (). A chip walked to with ↑↓ would drag the selection to the last + /// row of the list, so an INFO chip could only ever report on the last world — the same defect as + /// "only the last world can be deleted", one feature later. + /// + Detail, } /// @@ -270,16 +312,22 @@ internal ScreenModel(params IReadOnlyList[] panes) internal IReadOnlyList Sizes { get; } /// - /// How many of a pane's rows the cursor may occupy: all of them but the destructive buttons at the - /// end. Removals are appended last on every screen (a pane reads list → add → duplicate → remove), so - /// this is a count and not a set of holes — the cursor never has to skip a row in the middle, and - /// stays a plain clamp. pins that - /// the shape really is that way on all eight screens. + /// How many of a pane's rows the cursor may occupy: all of them but the targeted buttons at + /// the end. Those are appended last on every screen (a pane reads list → add → duplicate → info → + /// remove), so this is a count and not a set of holes — the cursor never has to skip a row in the + /// middle, and stays a plain clamp. ScreenModelTests pins the + /// count against the drawn rows, and MsspScreenTests.TheInfoRowIsDrawnAndIsNotSomewhereTheCursorCanGo + /// pins it for the half. + /// + /// Both non-stop kinds must stay trailing. A row put + /// anywhere but among them gives the cursor a hole, and the clamp becomes a skip list. + /// /// private static int Stops(IReadOnlyList rows) { var count = rows.Count; - while (count > 0 && rows[count - 1].Button is { Kind: ScreenButtonKind.Remove }) + while (count > 0 + && rows[count - 1].Button is { Kind: ScreenButtonKind.Remove or ScreenButtonKind.Detail }) { count--; } @@ -446,6 +494,53 @@ internal bool HasRemovableRow return null; } + /// + /// Whether any pane offers a read-only report on the selected row. The i info hint is derived + /// from this exactly as Del remove is derived from : a screen + /// physically cannot advertise a letter key it does not answer, which matters more here than for + /// Delete because i is a letter and a screen that swallowed it silently would be indis- + /// tinguishable from one where the key was simply not wired. + /// + internal bool HasDetailRow + { + get + { + for (var pane = 0; pane < _panes.Length; pane++) + { + if (DetailIn(pane) is not null) + { + return true; + } + } + + return false; + } + } + + /// + /// A pane's read-only report button, or null when it has none — what i runs while the cursor + /// is on one of that pane's list rows. The same shape as , and scoped the same + /// way: a pane that does not offer the button does not answer the key, which is what keeps i + /// from meaning anything in a pane that has editable fields. + /// + internal ScreenButton? DetailIn(int pane) + { + if (pane < 0 || pane >= _panes.Length) + { + return null; + } + + foreach (var row in _panes[pane]) + { + if (row.Button is { Kind: ScreenButtonKind.Detail } button) + { + return button; + } + } + + return null; + } + /// /// Whether a pane position is one of its *list* rows rather than one of the buttons appended after /// them. Delete asks this before acting: on a button row the cursor already has ⏎, and deleting the diff --git a/src/SharpMUTerm.Tui/SettingsOverlay.cs b/src/SharpMUTerm.Tui/SettingsOverlay.cs index d3ecafa..7139d94 100644 --- a/src/SharpMUTerm.Tui/SettingsOverlay.cs +++ b/src/SharpMUTerm.Tui/SettingsOverlay.cs @@ -34,6 +34,21 @@ internal sealed class SettingsOverlay private ConsoleKey _openKey; private ScreenBinding? _binding; + /// + /// What Esc goes back to. A read-only report opened from a screen (F5's INFO) pushes the screen it + /// came from here and replaces the content of the same window; Esc pops it and the screen + /// is rebuilt from its own untouched , so the cursor is on the world it + /// was on. It is a stack rather than a single slot only because a stack cannot be got wrong by a + /// later report that opens a report. + /// + /// Content-swapping, not window-stacking, and that is the same call + /// made for the same reason: two modal windows with two + /// PreviewKeyPressed handlers cannot be driven headlessly, so a second window here would put + /// this screen outside every snapshot and every test in the suite. + /// + /// + private readonly Stack _behind = new(); + /// /// The overlay no longer takes a save action. Persistence moved to the point of change: each screen's /// writes the configuration out as it accepts one, so there is no moment on @@ -84,6 +99,26 @@ public void Toggle(ConsoleKey key, Func binding) /// Renders a screen into a headless frame (used by snapshots). public void OpenForSnapshot(ConsoleKey key, ScreenBinding binding) => Open(key, binding); + /// + /// Puts a read-only report over the screen that asked for it, keeping that screen alive behind it so + /// Esc comes back to the row it was on. Does nothing when no screen is open — a report about a + /// selection that is not on the screen has nothing to be about. + /// + public void OpenDetail(ScreenBinding binding) + { + if (_window is null || _binding is not { } current) + { + return; + } + + _behind.Push(current); + _binding = binding; + Refresh(); + } + + /// Whether a report is open over a screen — which is what makes Esc mean back. + internal bool IsShowingDetail => _behind.Count > 0; + /// /// Feeds one key to the open screen through the very handler PreviewKeyPressed raises, so a /// snapshot can show a screen in a state only the keyboard can reach (a field mid-edit) without @@ -188,6 +223,14 @@ private bool Apply(ScreenBinding binding, ScreenAction action) { switch (action) { + case ScreenAction.Close when _behind.Count > 0: + // Esc out of a report goes back to the screen it was opened from, not out of the + // settings altogether. The screen behind is its own live session, so it comes back with + // its cursor, its open selection and its edit log intact — the report never touched them. + _binding = _behind.Pop(); + Refresh(); + return true; + case ScreenAction.Close: CloseAndReview(); return true; @@ -251,7 +294,10 @@ private void Refresh() /// private void CloseAndReview() { - var edits = _binding?.Session.Edits; + // The edits to review are the *screen's*, which is at the bottom of the stack when a report is + // over it: a report has an empty edit log of its own, so reading the top would silently drop a + // deletion made just before someone pressed i. + var edits = (_behind.Count > 0 ? _behind.Last() : _binding)?.Session.Edits; Close(); if (edits is { HasDeletions: true }) @@ -276,5 +322,6 @@ private void Reset() _system.ConsoleDriver.Paste -= OnPaste; _window = null; _binding = null; + _behind.Clear(); } } diff --git a/src/SharpMUTerm.Tui/SettingsSession.cs b/src/SharpMUTerm.Tui/SettingsSession.cs index 3d2e41e..fdd4e8e 100644 --- a/src/SharpMUTerm.Tui/SettingsSession.cs +++ b/src/SharpMUTerm.Tui/SettingsSession.cs @@ -232,6 +232,9 @@ private ScreenAction Interpret(ConsoleKeyInfo key) case ConsoleKey.Delete: return Remove(model); + case ConsoleKey.I when key.Modifiers == 0: + return Detail(model); + case ConsoleKey.Spacebar: return Toggle(model); @@ -273,6 +276,40 @@ private ScreenAction Remove(ScreenModel model) return ScreenAction.Redraw; } + /// + /// i on one of a pane's list rows opens that pane's read-only report on the selected row — + /// F5's INFO. It is the shape with two deliberate differences. + /// + /// It does not go through . Every other button press is an edit and is + /// persisted the moment it is accepted; this one changes nothing, and routing it through the edit + /// log would write config.json and re-periodise every running timer each time somebody looked + /// at a world. Opening a screen is navigation. + /// + /// + /// It returns , not . The + /// report has already replaced what is on screen by the time this returns; a redraw here would + /// rebuild the screen we have just navigated away from, over the one we navigated to. + /// + /// + /// A plain letter is only safe as a command because it is scoped by + /// to a pane that offers one — and because has already handed the whole + /// keyboard to an open field edit several branches above this. Both halves are load-bearing: without + /// the first, i would be swallowed in the CHARACTERS pane where nothing answers it; without + /// the second it could not be typed into a world's name. + /// + /// + private ScreenAction Detail(ScreenModel model) + { + if (!model.IsListRow(Selection.Pane, Selection.Index) + || model.DetailIn(Selection.Pane) is not { } button) + { + return ScreenAction.None; + } + + button.Run(); + return ScreenAction.Consumed; + } + private ScreenAction Toggle(ScreenModel model) { if (model.ToggleAt(Selection.Pane, Selection.Index) is not { } toggle) diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 1cd74f5..8acd555 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -8,6 +8,7 @@ using SharpMUTerm.Core.Logging; using SharpMUTerm.Core.Session; using SharpMUTerm.Core.Telnet; +using SharpMUTerm.Core.Telnet.Mssp; using SharpMUTerm.Core.Text; using SharpMUTerm.Core.Transport; using SharpMUTerm.Core.Theming; @@ -223,6 +224,13 @@ private sealed class SizeReport private readonly CommandPalette _palette; private readonly SettingsOverlay _settings; + /// + /// The settings overlay, so a headless test can drive a key into an open screen and ask what + /// happened. It is the same seam SimulateKey exists for and for the same reason: the + /// framework only pumps input inside Run(), which no test enters. + /// + internal SettingsOverlay Settings => _settings; + /// The ⌃P ▸ Show client messages viewer over the diagnostics log. private readonly MessageLogOverlay _messageLog; @@ -326,6 +334,16 @@ private sealed class SizeReport /// private readonly RestoreLog? _restore; + /// + /// Every server's last MSSP report, which is what the F5 ▸ i INFO screen reads. Never null, + /// and that is the difference from the three above rather than an inconsistency with them: a cache + /// built with no path is memory-only by construction — it reads nothing and writes nothing — + /// so the guarantee those three get from a null check, this one gets from the object it is handed. + /// A structural guarantee beats a check at every use site, and there is exactly one use site here + /// that a check could be forgotten at anyway. Only Program hands one a path. + /// + private readonly MsspCache _mssp; + /// The pane the live mouse drag is hovering, and the edge it would split — null when idle. private string? _dragTargetPaneId; private Edge? _dragEdge; @@ -388,6 +406,14 @@ private sealed class SizeReport /// has no business creating those. It is also what keeps the demo scene honest — a /// --demo-config snapshot would otherwise restore your panes into the demo's. /// + /// + /// Where each server's MSSP report is kept between launches, or null for a memory-only cache — which + /// is the default, and is what every test and every snapshot gets. It differs from the three + /// parameters above in one way that matters: the *field* is never null, because a memory-only + /// is a working cache that happens to own no file. The INFO screen therefore + /// needs no "is there a cache" branch, and a snapshot can seed a report through the same writer a + /// live session uses without any of it reaching disk. + /// public SharpMUTermApp( AppConfiguration config, TerminalCapabilities capabilities, @@ -396,12 +422,14 @@ public SharpMUTermApp( ClientDiagnostics? diagnostics = null, Action? save = null, string? logRoot = null, - RestoreLog? restore = null) + RestoreLog? restore = null, + MsspCache? mssp = null) { _config = config; _save = save; _logRoot = string.IsNullOrWhiteSpace(logRoot) ? null : logRoot; _restore = restore; + _mssp = mssp ?? new MsspCache(); _capabilities = capabilities; _time = time ?? TimeProvider.System; _diagnostics = diagnostics ?? ClientDiagnostics.InMemory(); @@ -966,6 +994,32 @@ public string RenderSnapshot(string? view = null) _settings.SimulateKey(Stroke('\0', ConsoleKey.Escape)); } + // The MSSP report, reached the only way a user can reach it: open F5 and press `i` on the + // selected world. Nothing about the screen is faked here — the key runs the real button, which + // opens the real binding over the real overlay. + // + // Three views because the screen has three states and two of them are empty ones that must not + // look alike. `mssp` seeds a report through the live writer; `mssp-none` only records a + // connection, which is the "connected and this server publishes no MSSP" arm; `mssp-never` + // seeds nothing at all, which is the arm a world you have not dialled is in. + if (view is not null && view.StartsWith(MsspScreenRenderer.View, StringComparison.OrdinalIgnoreCase)) + { + var world = _config.Worlds.Count > 0 ? _config.Worlds[0] : null; + if (world is not null && !view.EndsWith("-never", StringComparison.OrdinalIgnoreCase)) + { + _mssp.RecordConnection(world.Host, world.Port, _time.GetUtcNow()); + if (!view.EndsWith("-none", StringComparison.OrdinalIgnoreCase)) + { + CaptureMssp(world, DemoScene.MsspReport()); + } + } + + _settings.OpenForSnapshot(ConsoleKey.F5, WorldsScreen()); + _settings.SimulateKey(Stroke('i', ConsoleKey.I)); + SyncInputWidth(); + return RenderFrame(); + } + // Settings screens (composed-control or markup — SettingsView hands back a control factory // either way) open over the workspace for their --view name. A "-edit" view opens the // same screen and then drives real keys into it, so the frame shows a field genuinely mid-edit @@ -1633,10 +1687,24 @@ private void BindSession(WorldSession session, string? windowId = null) if (e.State == ConnectionState.Connected) { ReportAutomation(session); + + // Recorded on the *connection*, not on the report — because "we reached this server and + // it published nothing" is a fact the INFO screen has to be able to state, and it is + // only ever knowable from the connection having happened. MSSP arrives on the read loop + // after this transition, so a server that does publish overwrites nothing: it adds a + // report beside a connection time that is already here. + _mssp.RecordConnection(session.World.Host, session.World.Port, _time.GetUtcNow()); } UpdateStatus(); }); + + // MSSP is captured per world and kept, so the report is readable while nothing is connected — + // which is when it is wanted, since the question the screen answers is "what is this world" + // asked before or between sessions. It goes through CaptureMssp rather than straight into the + // cache so the snapshot's demo report is written by the same code the wire is (see DemoScene's + // remarks on state a live session writes). + session.MsspReceived += (_, e) => OnUi(() => CaptureMssp(session.World, e.Data)); session.GmcpReceived += (_, e) => OnUi(() => { if (_stats.Update(e.Package, e.Json)) @@ -3812,6 +3880,54 @@ private LoggingSettings ActiveLogging() /// character's trigger sets → the selected world's security checkboxes), seeded on whatever is /// connected so the screen opens where the user already is. /// + /// + /// Files one server's MSSP report under the world it came from. The one writer: the live + /// subscription in and the snapshot's demo report both go through it, so + /// the endpoint a frame is rendered from and the endpoint a connection files under cannot be + /// different strings — the gap that has hidden three separate bugs in the demo scene already. + /// + internal void CaptureMssp(WorldDefinition world, MsspData report) + { + ArgumentNullException.ThrowIfNull(world); + _mssp.RecordReport(world.Host, world.Port, report, _time.GetUtcNow()); + } + + /// + /// Opens the read-only MSSP report for a world, over the screen that asked for it. Bounds-checked + /// against the live list rather than trusted, because the index was captured when the WORLDS pane's + /// buttons were built and a keystroke between then and now could have deleted the row. + /// + private void OpenMsspScreen(int world) + { + if (world < 0 || world >= _config.Worlds.Count) + { + return; + } + + _settings.OpenDetail(MsspScreen(_config.Worlds[world])); + } + + /// + /// The MSSP report screen for one world. A full like every other screen + /// — the same session, the same key table, the same overlay — because that is what gives it + /// scrolling, a cursor and headless key simulation for nothing. Its model offers no fields, no + /// toggles and no removals, so the header derives none of those hints. + /// + private ScreenBinding MsspScreen(WorldDefinition world) + { + var observation = _mssp.Find(world.Host, world.Port); + var session = new SettingsSession(_ => MsspScreenRenderer.Model( + world, observation, _time.GetUtcNow(), _system.DesktopDimensions.Width)); + + return new ScreenBinding(session, () => MsspScreenView.Build( + world, + observation, + _time.GetUtcNow(), + _system.DesktopDimensions.Width, + session.Focus(), + _system.DesktopDimensions.Height)); + } + private ScreenBinding WorldsScreen() => WorldsScreen(WorldsScreenRenderer.FKey, onCharacters: false); /// @@ -3838,7 +3954,8 @@ private ScreenBinding WorldsScreen(string fkey, bool onCharacters) _config.TriggerSets, selection.SelectionIn(WorldsScreenRenderer.WorldsPane), selection.SelectionIn(WorldsScreenRenderer.CharactersPane), - selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane)), + selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane), + OpenMsspScreen), SaveConfiguration); session.Selection.Seed(WorldsScreenRenderer.WorldsPane, ActiveWorldIndex()); session.Selection.Seed(WorldsScreenRenderer.CharactersPane, ActiveCharacterIndex()); @@ -3856,7 +3973,8 @@ private ScreenBinding WorldsScreen(string fkey, bool onCharacters) session.Focus(), fkey, session.Selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane), - _system.DesktopDimensions.Height)); + _system.DesktopDimensions.Height, + info: true)); } /// diff --git a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs index 68cca39..1ee314c 100644 --- a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs @@ -237,7 +237,12 @@ internal static string HeaderLine( { var title = $"[bold {Value}] Worlds & Characters[/]"; var hints = ScreenChrome.Hints( - ScreenChrome.ListHints, fkey, model?.HasEditableRow ?? false, focus, model?.HasRemovableRow ?? false); + ScreenChrome.ListHints, + fkey, + model?.HasEditableRow ?? false, + focus, + model?.HasRemovableRow ?? false, + model?.HasDetailRow ?? false); return SpreadLR(" " + title, hints, width); } @@ -363,12 +368,20 @@ internal static string HeaderLine( /// the first, the way every other pane's cursor starts on its first row, so a caller that only wants /// the navigable shape still gets the pane's real buttons. /// + /// + /// Opens the read-only MSSP report for the world at the given index, or null when this projection has + /// nowhere to open one — which is every caller but the live app: the renderer is pure and a screen is + /// not something a markup block can put on the screen by itself. Null means the WORLDS pane grows no + /// i row, so the header hint (derived from ) does not + /// advertise a key that would do nothing. + /// internal static ScreenModel Model( IReadOnlyList worlds, IReadOnlyList triggerSets, int selectedWorld, int selectedCharacter, - int selectedSet = 0) + int selectedSet = 0, + Action? info = null) { ArgumentNullException.ThrowIfNull(worlds); ArgumentNullException.ThrowIfNull(triggerSets); @@ -381,7 +394,7 @@ internal static ScreenModel Model( ScreenField.Integer("port", () => w.Port, v => w.Port = v, 1, 65535), ScreenField.Choice("encoding", () => w.Encoding, v => w.Encoding = v, Encodings), ScreenField.Integer("keepalive", () => w.KeepaliveSeconds, v => w.KeepaliveSeconds = v, 0, 86400))) - .Concat(WorldButtons(worlds, selectedWorld)) + .Concat(WorldButtons(worlds, selectedWorld, info)) .ToArray(); var world = selectedWorld >= 0 && selectedWorld < worlds.Count ? worlds[selectedWorld] : null; @@ -638,7 +651,16 @@ private static ScreenRow[] SecurityRows(WorldDefinition world) => new[] /// delete; a brand-new world is a blank template, because a world's whole identity is its host and /// a "helpfully" prefilled one would be a guess the user then has to notice and undo. /// - private static List WorldButtons(IReadOnlyList worlds, int selectedWorld) + /// + /// The WORLDS pane's buttons, in the order they are drawn and — decisively — in the order + /// needs them: the cursor stop first, then the two targeted key + /// hints. [+ world] has no target and so needs somewhere to be pressed from; i and + /// Del both act on the selected row and must not steal the cursor from it, so they trail and + /// are trimmed out of the pane's stop count. Put either of them above [+ world] and the + /// cursor gains a hole. + /// + private static List WorldButtons( + IReadOnlyList worlds, int selectedWorld, Action? info = null) { var rows = new List(); // Arrays report IsReadOnly through IList, and a renderer handed one (the unit tests, any @@ -652,6 +674,14 @@ private static List WorldButtons(IReadOnlyList world if (selectedWorld >= 0 && selectedWorld < list.Count) { var world = list[selectedWorld]; + if (info is not null) + { + // The index is captured, not the world: the report is opened against whatever the WORLDS + // list holds at that position when the key is pressed, which is the row the cursor is on. + var at = selectedWorld; + rows.Add(ScreenRow.Of(ScreenButton.Detail(world.Name, () => info(at)))); + } + rows.Add(ScreenRow.Of(ScreenButton.Remove( list, selectedWorld, target: world.Name, describe: () => DescribeWorld(world)))); } @@ -726,7 +756,11 @@ internal static string FooterLine( /// was never drawn, which is precisely the failure exists to stop. /// internal static List WorldsColumn( - IReadOnlyList worlds, int selectedWorld, ScreenFocus? focus = null, int height = 0) + IReadOnlyList worlds, + int selectedWorld, + ScreenFocus? focus = null, + int height = 0, + bool info = false) { var cursor = focus ?? ScreenFocus.None; selectedWorld = Selected(worlds.Count, selectedWorld); @@ -756,8 +790,16 @@ internal static List WorldsColumn( } left.Add(string.Empty); + // The drawn rows come from the same WorldButtons the model navigates by, so the row saying what + // `i` acts on and the button the key runs cannot name different worlds. The action itself is not + // needed to draw it — only whether there is one — so the column takes a bool rather than the + // delegate, which keeps the renderer pure. left.AddRange(ScreenChrome.Buttons( - WorldButtons(worlds, selectedWorld), cursor, WorldsPane, worlds.Count, LeftColumnWidth)); + WorldButtons(worlds, selectedWorld, info ? _ => { } : null), + cursor, + WorldsPane, + worlds.Count, + LeftColumnWidth)); // Compacted, then windowed — the same two steps the detail column takes, and it must be both: // dropping the blank separators buys back a row per world, and the window is what guarantees the diff --git a/src/SharpMUTerm.Tui/WorldsScreenView.cs b/src/SharpMUTerm.Tui/WorldsScreenView.cs index 6efd489..508c382 100644 --- a/src/SharpMUTerm.Tui/WorldsScreenView.cs +++ b/src/SharpMUTerm.Tui/WorldsScreenView.cs @@ -25,7 +25,8 @@ public static IWindowControl Build( ScreenFocus? focus = null, string fkey = WorldsScreenRenderer.FKey, int selectedSet = 0, - int height = 0) + int height = 0, + bool info = false) { // Both panes end in button rows, so a raw cursor can point past its list; resolving once here // keeps every block of the screen agreeing on which world and character are selected. @@ -33,8 +34,10 @@ public static IWindowControl Build( WorldsScreenRenderer.Resolve(worlds, selectedWorld, selectedCharacter); var accent = WorldsScreenRenderer.AccentFor(worlds, selectedWorld); + // The model is rebuilt here only to derive the header's hints, so the INFO row's presence is all + // it needs of the action — a no-op stands in for it, and the delegate stays with the session. var model = WorldsScreenRenderer.Model( - worlds, triggerSets, selectedWorld, selectedCharacter, selectedSet); + worlds, triggerSets, selectedWorld, selectedCharacter, selectedSet, info ? _ => { } : null); var header = ScreenChrome.Band( WorldsScreenRenderer.HeaderLine(width, model, focus, fkey), ScreenPalette.HeaderBg); var footer = ScreenChrome.Band( @@ -60,7 +63,7 @@ public static IWindowControl Build( // Body: WORLDS list │ detail, as two real columns. var worldsCol = ScreenChrome.Stretch(new MarkupControl( - WorldsScreenRenderer.WorldsColumn(worlds, selectedWorld, focus, rows).ToList())); + WorldsScreenRenderer.WorldsColumn(worlds, selectedWorld, focus, rows, info).ToList())); var detailCol = ScreenChrome.Stretch(new MarkupControl( WorldsScreenRenderer .DetailColumn(worlds, triggerSets, selectedWorld, selectedCharacter, accent, focus, rows) diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs new file mode 100644 index 0000000..3f62677 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs @@ -0,0 +1,314 @@ +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Telnet.Mssp; + +namespace SharpMUTerm.Core.Tests.Telnet; + +/// +/// What the INFO screen reads: which endpoint a report is filed under, how the three states are told +/// apart, what a second connection does to the first one's report, and what a hostile server gets to +/// put on disk. +/// +/// Nothing here touches ConfigurationStore.DefaultPath. Every case either uses a memory-only +/// cache or a temporary directory of its own — the developer's own configuration is not a fixture. +/// +/// +public class MsspCacheTests +{ + private static readonly DateTimeOffset Noon = new(2026, 7, 30, 12, 0, 0, TimeSpan.Zero); + + private static MsspData Report(params (string Variable, string[] Values)[] entries) => + MsspWire.Report(entries); + + /// A directory of its own, deleted afterwards; never created until something writes. + private sealed class TempRoot : IDisposable + { + public TempRoot() => + Root = Path.Combine(Path.GetTempPath(), $"smuterm-mssp-{Guid.NewGuid():N}"); + + public string Root { get; } + + /// One level deeper, so the store has a folder to create the way the real one does. + public string ConfigPath => Path.Combine(Root, "SharpMUTerm", "config.json"); + + public string CachePath => MsspCache.PathFor(ConfigPath); + + public void Dispose() + { + try + { + if (Directory.Exists(Root)) + { + Directory.Delete(Root, recursive: true); + } + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + // A temp directory that will not go is not this test's business. + } + } + } + + // ---- Identity ---- + + [Test] + public async Task ACacheFileSitsBesideTheConfigurationAndItsSecrets() + { + var config = Path.Combine(Path.GetTempPath(), "nowhere", "SharpMUTerm", "config.json"); + + await Assert.That(MsspCache.PathFor(config)) + .IsEqualTo(Path.Combine(Path.GetTempPath(), "nowhere", "SharpMUTerm", MsspCache.FileName)); + await Assert.That(Path.GetDirectoryName(MsspCache.PathFor(config))) + .IsEqualTo(Path.GetDirectoryName(SecretsStore.PathFor(config))); + } + + [Test] + public async Task OneServerIsOneEntryHoweverItsHostIsSpelled() + { + // The folding is what stops a server accumulating a half-filled entry per spelling its worlds + // happen to use. A trailing root dot and a capital are the same host to DNS and must be here. + var cache = new MsspCache(); + cache.RecordReport("MUD.Example.ORG.", 4201, Report(("NAME", ["One"])), Noon); + cache.RecordReport("mud.example.org", 4201, Report(("NAME", ["Two"])), Noon); + + await Assert.That(cache.All).HasCount().EqualTo(1); + await Assert.That(cache.Find("MUD.EXAMPLE.ORG", 4201)!.Report!.Name).IsEqualTo("Two"); + } + + [Test] + public async Task TwoPortsOnOneHostAreTwoServers() + { + // The plaintext port and the TLS port are two endpoints and may run two games. Keying by host + // alone would have one report standing for both, which is the one reading a user cannot correct. + var cache = new MsspCache(); + cache.RecordReport("mud.example.org", 4000, Report(("NAME", ["Plain"])), Noon); + cache.RecordReport("mud.example.org", 4001, Report(("NAME", ["Secure"])), Noon); + + await Assert.That(cache.Find("mud.example.org", 4000)!.Report!.Name).IsEqualTo("Plain"); + await Assert.That(cache.Find("mud.example.org", 4001)!.Report!.Name).IsEqualTo("Secure"); + } + + // ---- The three states ---- + + [Test] + public async Task AnEndpointNothingHasReachedHasNoEntryAtAll() + { + await Assert.That(new MsspCache().Find("never.example.org", 4000)).IsNull(); + } + + [Test] + public async Task AConnectionWithNoReportIsRecordedAsExactlyThat() + { + // The state the whole two-timestamp design exists for: we spoke to this server and it published + // nothing. It must be distinguishable from never having spoken to it, and it is — there is an + // observation, and its report is null. + var cache = new MsspCache(); + cache.RecordConnection("quiet.example.org", 4000, Noon); + + var observation = cache.Find("quiet.example.org", 4000); + await Assert.That(observation).IsNotNull(); + await Assert.That(observation!.PublishesNothing).IsTrue(); + await Assert.That(observation.ObservedAt).IsNull(); + await Assert.That(observation.ConnectedAt).IsEqualTo(Noon); + } + + [Test] + public async Task AReportKeepsTheConnectionTimeItArrivedUnder() + { + var cache = new MsspCache(); + cache.RecordConnection("mud.example.org", 4000, Noon); + cache.RecordReport("mud.example.org", 4000, Report(("NAME", ["Corvid"])), Noon.AddSeconds(2)); + + var observation = cache.Find("mud.example.org", 4000)!; + await Assert.That(observation.ConnectedAt).IsEqualTo(Noon); + await Assert.That(observation.ObservedAt).IsEqualTo(Noon.AddSeconds(2)); + await Assert.That(observation.PublishesNothing).IsFalse(); + } + + [Test] + public async Task AServerThatStopsPublishingKeepsTheReportItLastGave() + { + // Its date moves on and the report's does not, which is the point: the screen dates the report, + // so a stale player count is labelled stale rather than blanked or re-dated. Clearing it on the + // next connect would throw away the only answer we have in order to be freshly ignorant. + var cache = new MsspCache(); + cache.RecordReport("mud.example.org", 4000, Report(("PLAYERS", ["17"])), Noon); + cache.RecordConnection("mud.example.org", 4000, Noon.AddDays(30)); + + var observation = cache.Find("mud.example.org", 4000)!; + await Assert.That(observation.Report!.Players).IsEqualTo(17); + await Assert.That(observation.ObservedAt).IsEqualTo(Noon); + await Assert.That(observation.ConnectedAt).IsEqualTo(Noon.AddDays(30)); + } + + // ---- A second report ---- + + [Test] + public async Task ASecondReportReplacesTheFirstRatherThanMergingWithIt() + { + // MSSP is not a delta protocol: a server sends its whole table once per connection. A merge + // would keep a variable it has stopped publishing for ever — the accumulating-room-exits failure + // in a different costume — and would leave a report that is a snapshot of no moment that existed. + var cache = new MsspCache(); + cache.RecordReport( + "mud.example.org", 4000, Report(("NAME", ["Old"]), ("DISCORD", ["https://old"])), Noon); + cache.RecordReport("mud.example.org", 4000, Report(("NAME", ["New"])), Noon.AddDays(1)); + + var report = cache.Find("mud.example.org", 4000)!.Report!; + await Assert.That(report.Name).IsEqualTo("New"); + await Assert.That(report.ContainsKey("DISCORD")).IsFalse(); + await Assert.That(cache.Find("mud.example.org", 4000)!.ObservedAt).IsEqualTo(Noon.AddDays(1)); + } + + [Test] + public async Task AnEmptyReportIsStillAReportAndStillReplaces() + { + // A server that negotiates MSSP and sends no variables has said something: it publishes, and it + // publishes nothing in particular. That is not the same as never having answered. + var cache = new MsspCache(); + cache.RecordReport("mud.example.org", 4000, Report(("NAME", ["Was"])), Noon); + cache.RecordReport("mud.example.org", 4000, MsspData.Empty, Noon.AddDays(1)); + + var observation = cache.Find("mud.example.org", 4000)!; + await Assert.That(observation.PublishesNothing).IsFalse(); + await Assert.That(observation.Report!.Count).IsEqualTo(0); + } + + // ---- Bounds ---- + + [Test] + public async Task AHostileReportIsCutDownBeforeItIsStored() + { + var cache = new MsspCache(); + var flood = Enumerable.Range(0, MsspCache.MaxVariables + 50) + .Select(i => ($"VAR{i}", new[] { new string('x', MsspCache.MaxValueLength * 4) })) + .Append(("PORT", Enumerable.Range(0, MsspCache.MaxValuesPerVariable + 20) + .Select(i => i.ToString(System.Globalization.CultureInfo.InvariantCulture)) + .ToArray())) + .ToArray(); + + cache.RecordReport("hostile.example.org", 4000, Report(flood), Noon); + var report = cache.Find("hostile.example.org", 4000)!.Report!; + + await Assert.That(report.Count).IsEqualTo(MsspCache.MaxVariables); + await Assert.That(report["VAR0"][0].Length).IsEqualTo(MsspCache.MaxValueLength); + // PORT was appended past the variable cap, so it is not kept at all — the head of what the + // server sent survives, which is a subset a reader can reason about. + await Assert.That(report.ContainsKey("PORT")).IsFalse(); + } + + [Test] + public async Task AMultiValuedVariableInsideTheCapKeepsEveryValueInOrder() + { + var cache = new MsspCache(); + cache.RecordReport("mud.example.org", 4000, Report(("PORT", ["80", "23", "4201"])), Noon); + + await Assert.That(cache.Find("mud.example.org", 4000)!.Report!["PORT"]) + .IsEquivalentTo(new[] { "80", "23", "4201" }); + } + + [Test] + public async Task TheOldestEndpointsAreDroppedOnceTheFileIsFull() + { + var cache = new MsspCache(); + for (var i = 0; i < MsspCache.MaxEndpoints + 10; i++) + { + cache.RecordConnection($"host{i}.example.org", 4000, Noon.AddMinutes(i)); + } + + await Assert.That(cache.All).HasCount().EqualTo(MsspCache.MaxEndpoints); + await Assert.That(cache.Find("host0.example.org", 4000)).IsNull(); + await Assert.That(cache.Find($"host{MsspCache.MaxEndpoints + 9}.example.org", 4000)).IsNotNull(); + } + + // ---- Disk ---- + + [Test] + public async Task ACacheWithNoPathWritesNothingAnywhere() + { + // This is the guarantee a snapshot and every test in the suite runs on, and it is structural: + // there is no file to write to, rather than a check at each use site that could be forgotten. + using var temp = new TempRoot(); + var cache = new MsspCache(); + cache.RecordReport("mud.example.org", 4000, Report(("NAME", ["Corvid"])), Noon); + + await Assert.That(cache.IsPersistent).IsFalse(); + await Assert.That(Directory.Exists(temp.Root)).IsFalse(); + } + + [Test] + public async Task AReportSurvivesARestartWithItsValuesAndTheirOrderIntact() + { + using var temp = new TempRoot(); + var writer = new MsspCache(temp.CachePath); + writer.RecordConnection("mud.example.org", 4201, Noon); + writer.RecordReport( + "mud.example.org", + 4201, + Report(("NAME", ["Corvid Nest"]), ("PORT", ["80", "23", "4201"]), ("VANITY", ["least", "most"])), + Noon.AddSeconds(1)); + + var reread = new MsspCache(temp.CachePath); + var observation = reread.Find("mud.example.org", 4201)!; + + await Assert.That(reread.Problem).IsNull(); + await Assert.That(observation.ConnectedAt).IsEqualTo(Noon); + await Assert.That(observation.ObservedAt).IsEqualTo(Noon.AddSeconds(1)); + await Assert.That(observation.Report!.Name).IsEqualTo("Corvid Nest"); + await Assert.That(observation.Report["PORT"]).IsEquivalentTo(new[] { "80", "23", "4201" }); + + // Order is meaning in MSSP — "least to most relevant" — so the on-disk form is an array and the + // wire order has to come back the way it went in, not however a JSON object happened to keep it. + await Assert.That(observation.Report.Keys).IsEquivalentTo(new[] { "NAME", "PORT", "VANITY" }); + } + + [Test] + public async Task AConnectionWithNoReportSurvivesARestartAsThatStateAndNotAsNothing() + { + using var temp = new TempRoot(); + new MsspCache(temp.CachePath).RecordConnection("quiet.example.org", 4000, Noon); + + var observation = new MsspCache(temp.CachePath).Find("quiet.example.org", 4000); + await Assert.That(observation).IsNotNull(); + await Assert.That(observation!.PublishesNothing).IsTrue(); + } + + [Test] + public async Task AnUnreadableCacheStartsEmptyAndSaysSoInsteadOfThrowing() + { + using var temp = new TempRoot(); + Directory.CreateDirectory(Path.GetDirectoryName(temp.CachePath)!); + File.WriteAllText(temp.CachePath, "{ this is not json"); + + var cache = new MsspCache(temp.CachePath); + await Assert.That(cache.All).IsEmpty(); + await Assert.That(cache.Problem).IsNotNull(); + } + + [Test] + public async Task ACacheFromANewerSchemaIsIgnoredRatherThanMisread() + { + using var temp = new TempRoot(); + Directory.CreateDirectory(Path.GetDirectoryName(temp.CachePath)!); + File.WriteAllText(temp.CachePath, """{ "version": 99, "servers": { "a:1": {} } }"""); + + var cache = new MsspCache(temp.CachePath); + await Assert.That(cache.All).IsEmpty(); + await Assert.That(cache.Problem).IsNotNull(); + } + + [Test] + public async Task OneUnreadableEntryDoesNotCostTheRest() + { + using var temp = new TempRoot(); + var writer = new MsspCache(temp.CachePath); + writer.RecordConnection("good.example.org", 4000, Noon); + + var text = File.ReadAllText(temp.CachePath) + .Replace("\"servers\": {", "\"servers\": {\n \"bad:1\": { \"connectedAt\": \"not a date\" },"); + File.WriteAllText(temp.CachePath, text); + + var cache = new MsspCache(temp.CachePath); + await Assert.That(cache.Find("good.example.org", 4000)).IsNotNull(); + await Assert.That(cache.Problem).IsNotNull(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs new file mode 100644 index 0000000..b3686d0 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs @@ -0,0 +1,502 @@ +using System.Text.RegularExpressions; +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Telnet.Mssp; +using SharpMUTerm.Graphics; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The F5 ▸ i MSSP report: that the key reaches it, that the three states are distinguishable +/// on a rendered frame, that a report is drawn as the lists it is, and that no value a stranger can +/// send widens or breaks a row. +/// +/// +/// Serialised where a frame is rendered, for the same reason every snapshot test in this suite is: +/// capturing a frame redirects Console.Out, and that is process-global. +/// +[NotInParallel] +public class MsspScreenTests +{ + private const int Width = 120; + private const int Height = 34; + + private static readonly DateTimeOffset Noon = new(2026, 7, 30, 12, 0, 0, TimeSpan.Zero); + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + /// + /// A rendered frame, decoded to the cells the driver was handed. Assertions go through the grid and + /// never through the raw ANSI: one drawn phrase is several SGR runs, so Contains on the frame + /// string can miss text that is plainly on the screen — and, worse, can pass on text that is not. + /// + private static IReadOnlyList Frame(string view, int width = Width, int height = Height) + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(width, height)); + return FrameGrid.Decode(app.RenderSnapshot(view), width, height); + } + + /// The whole frame as one string, for the assertions that only ask whether a phrase is on it. + private static string Screen(string view, int width = Width, int height = Height) => + string.Join("\n", Frame(view, width, height)); + + private static WorldDefinition World() => + new() { Name = "Aetherfall", Host = "aetherfall.mux", Port = 4201, UseTls = true }; + + private static MsspObservation Observed(MsspData report, DateTimeOffset? at = null) => + new("aetherfall.mux:4201", Noon, report, at ?? Noon); + + private static MsspData Report(params (string Name, string[] Values)[] entries) => + MsspData.From(entries.Select(e => + new KeyValuePair>(e.Name, e.Values))); + + /// + /// A body block as plain text: markup tags removed, escaped brackets put back. The inverse of what + /// counts, written the same way — the escapes are guarded + /// with two characters no value can carry (control characters never survive the renderer) before + /// the tag pattern runs. + /// + private static List Plain(IEnumerable lines) => + lines.Select(StripMarkup).ToList(); + + private static string StripMarkup(string markup) + { + const char openMark = '\u0001'; + const char closeMark = '\u0002'; + var guarded = markup.Replace("[[", openMark.ToString()).Replace("]]", closeMark.ToString()); + var stripped = Regex.Replace(guarded, @"\[[^\[\]]*\]", string.Empty); + return stripped.Replace(openMark, '[').Replace(closeMark, ']'); + } + + /// + /// The label column of a plain row — three cells of mark and gap, then the fixed-width name field. + /// Rows are read by column because a substring match crosses fields: transport contains + /// port, and a test that matched the wrong row would be asserting about the wrong thing while + /// passing. + /// + private static string LabelOf(string row) + { + const int labelAt = 3; + return row.Length <= labelAt + ? string.Empty + : row[labelAt..Math.Min(row.Length, labelAt + MsspScreenRenderer.NameWidth)].Trim(); + } + + // ---- Reaching it ---- + + [Test] + public async Task TheWorldsScreenAdvertisesTheInfoKeyAndSaysWhichWorldItActsOn() + { + var frame = Screen("worlds"); + + // Derived from the model, never written: the hint and the drawn row both come from the same + // ScreenButton, so a screen cannot advertise a key it does not answer. + await Assert.That(frame).Contains("i info"); + await Assert.That(frame).Contains($"{ScreenChrome.InfoWords} Aetherfall"); + } + + [Test] + public async Task PressingIOnTheSelectedWorldOpensItsReport() + { + // The snapshot view drives the real key through the real button; nothing about the route is faked. + var frame = Screen("mssp"); + + await Assert.That(frame).Contains(MsspScreenRenderer.Title); + await Assert.That(frame).Contains("Aetherfall"); + await Assert.That(frame).Contains("Esc back"); + + // And it is a report, not a form: the world list it came from is gone from the screen. + await Assert.That(frame).DoesNotContain("Worlds & Characters"); + } + + [Test] + public async Task EscapeFromTheReportGoesBackToTheWorldItWasOpenedFrom() + { + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height)); + app.RenderSnapshot("mssp"); + + // One Esc pops the report; the screen behind comes back with its own cursor and hints. A second + // Esc leaves the settings altogether, which is the layering Esc has everywhere else in the app. + await Assert.That(app.Settings.IsShowingDetail).IsTrue(); + app.Settings.SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.Escape, false, false, false)); + + await Assert.That(app.Settings.IsShowingDetail).IsFalse(); + await Assert.That(app.Settings.IsOpen).IsTrue(); + await Assert.That(app.RenderWholeFrame()).Contains("Worlds & Characters"); + } + + // ---- The key's shape and scope ---- + + /// + /// Both targeted keys trail the pane's one cursor stop and neither is one. This is the invariant + /// is built on — an action with a target must not steal the cursor + /// from the thing it acts on — and the reason an INFO chip would have been wrong: reaching + /// it with ↑↓ walks the selection to the last world, so it could only ever have reported on that one. + /// + [Test] + public async Task TheInfoRowIsDrawnAndIsNotSomewhereTheCursorCanGo() + { + var worlds = new List { World(), new() { Name = "Grapevine" } }; + var model = WorldsScreenRenderer.Model(worlds, [], 0, 0, 0, _ => { }); + + // Two worlds, then [+ world]; the `i` and `Del` rows are drawn past the end of the stops. + await Assert.That(model.RowCount(WorldsScreenRenderer.WorldsPane)).IsEqualTo(5); + await Assert.That(model.Sizes[WorldsScreenRenderer.WorldsPane]).IsEqualTo(3); + await Assert.That(model.HasDetailRow).IsTrue(); + + // And the drawn column says what the key would act on, which is why nothing is lost by it not + // being a chip. + var column = WorldsScreenRenderer.WorldsColumn(worlds, 0, info: true); + await Assert.That(Plain(column).Any(l => l.Contains($"{ScreenChrome.InfoWords} Aetherfall"))).IsTrue(); + } + + [Test] + public async Task AProjectionWithNowhereToOpenAReportOffersNeitherTheRowNorTheHint() + { + // The renderer is pure and cannot put a screen on the screen, so a caller that supplies no action + // gets no `i` row — and therefore no `i info` hint, because the hint is derived from the row. + var model = WorldsScreenRenderer.Model(new List { World() }, [], 0, 0); + + await Assert.That(model.HasDetailRow).IsFalse(); + await Assert.That(WorldsScreenRenderer.HeaderLine(Width, model)).DoesNotContain(ScreenChrome.DetailHint); + } + + [Test] + public async Task TheKeyRunsOnAWorldRowAndIsDeclinedEverywhereElse() + { + var opened = new List(); + var worlds = new List { World(), new() { Name = "Grapevine" } }; + var session = new SettingsSession(selection => WorldsScreenRenderer.Model( + worlds, + [], + selection.SelectionIn(WorldsScreenRenderer.WorldsPane), + selection.SelectionIn(WorldsScreenRenderer.CharactersPane), + selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane), + opened.Add)); + + // On the second world's row: opens that world, not the selected-at-seed one. + session.Selection.Seed(WorldsScreenRenderer.WorldsPane, 1); + await Assert.That(session.Handle(Key(ConsoleKey.I))).IsEqualTo(ScreenAction.Consumed); + await Assert.That(opened).IsEquivalentTo(new[] { 1 }); + + // On [+ world] — a button row, not a list row — it declines, exactly as Delete does there. + session.Selection.Seed(WorldsScreenRenderer.WorldsPane, 2); + await Assert.That(session.Handle(Key(ConsoleKey.I))).IsEqualTo(ScreenAction.None); + + // In the CHARACTERS pane, which offers no report, the letter is not ours at all — it must fall + // through rather than be swallowed, or a pane that does nothing with `i` would eat it silently. + session.Selection.FocusPane(WorldsScreenRenderer.CharactersPane); + await Assert.That(session.Handle(Key(ConsoleKey.I))).IsEqualTo(ScreenAction.None); + await Assert.That(opened).IsEquivalentTo(new[] { 1 }); + } + + [Test] + public async Task TheLetterIsStillATypedCharacterWhileAFieldIsOpen() + { + // `i` is an ordinary letter, and the only thing that makes it safe as a command is that an open + // field edit takes the whole keyboard several branches earlier. Without that, a world could not + // be named `Riverside`. + var opened = new List(); + var worlds = new List { new() { Name = "Old", Host = "h", Port = 1 } }; + var session = new SettingsSession(selection => WorldsScreenRenderer.Model( + worlds, + [], + selection.SelectionIn(WorldsScreenRenderer.WorldsPane), + selection.SelectionIn(WorldsScreenRenderer.CharactersPane), + selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane), + opened.Add)); + + session.Handle(Key(ConsoleKey.Enter)); + foreach (var ch in "Riverside") + { + session.Handle(new ConsoleKeyInfo(ch, ConsoleKey.None, false, false, false)); + } + + session.Handle(Key(ConsoleKey.Enter)); + + // Contains rather than equals: ⏎ opens the field on its existing text, and where the caret lands + // in it is the edit buffer's business and not this test's. What matters is that all nine letters + // — the `i` among them — went into the name, and that none of them opened a report. + await Assert.That(worlds[0].Name).Contains("Riverside"); + await Assert.That(opened).IsEmpty(); + } + + [Test] + public async Task OpeningAReportPersistsNothing() + { + // Every other button on these screens is an edit and is written to disk the moment it is + // accepted. This one is navigation: routing it through ScreenEdits would write config.json and + // re-periodise every running timer each time somebody looked at a world. + var saves = 0; + var worlds = new List { World() }; + var session = new SettingsSession( + selection => WorldsScreenRenderer.Model( + worlds, + [], + selection.SelectionIn(WorldsScreenRenderer.WorldsPane), + selection.SelectionIn(WorldsScreenRenderer.CharactersPane), + selection.SelectionIn(WorldsScreenRenderer.TriggerSetsPane), + _ => { }), + () => saves++); + + session.Handle(Key(ConsoleKey.I)); + + await Assert.That(saves).IsEqualTo(0); + await Assert.That(session.Edits.HasDeletions).IsFalse(); + } + + private static ConsoleKeyInfo Key(ConsoleKey key) => new('\0', key, false, false, false); + + // ---- The three states ---- + + [Test] + public async Task AWorldNothingHasConnectedToSaysSoRatherThanShowingNothing() + { + var frame = Screen("mssp-never"); + + await Assert.That(frame).Contains("connect once and this fills in"); + await Assert.That(frame).DoesNotContain("does not publish MSSP"); + + // And it still shows what the client knows without asking anybody, which is what stops the + // screen reading as broken. + await Assert.That(frame).Contains("aetherfall.mux:4201"); + } + + [Test] + public async Task AServerThatAnsweredAndPublishesNothingIsNotTheSameEmptyScreen() + { + var frame = Screen("mssp-none"); + + await Assert.That(frame).Contains("does not publish MSSP"); + await Assert.That(frame).Contains("It is optional"); + await Assert.That(frame).DoesNotContain("connect once and this fills in"); + + // It says when we last reached the server, because "we asked and it said nothing" is only a + // claim worth making if the asking is dated. + await Assert.That(frame).Contains("last seen"); + } + + [Test] + public async Task AReportIsDatedSoAStalePlayerCountIsNotPresentedAsCurrent() + { + var week = Plain(MsspScreenRenderer.Body( + World(), + Observed(Report(("PLAYERS", ["37"])), Noon.AddDays(-7)), + Noon, + Width)); + + await Assert.That(week.Any(l => l.Contains("captured") && l.Contains("7 days ago"))).IsTrue(); + await Assert.That(week.Any(l => l.Contains("2026-07-23"))).IsTrue(); + } + + // ---- What it shows ---- + + [Test] + public async Task AMultiValuedVariableIsDrawnAsTheListItIsAndNotAsOneOfItsValues() + { + var rows = Plain(MsspScreenRenderer.Body( + World(), Observed(Report(("PORT", ["80", "23", "4201"]))), Noon, Width)); + + // All three, in wire order, on three rows — least to most relevant, which is the order the + // specification gives them meaning in. A model keeping one value per variable would print a + // server's *least* preferred port and call it the port; one joining them would lose the ordering. + // Matched on the label *column*, not on a substring: `transport` contains `port`, and a test + // that found the wrong row would have been asserting about the world's TLS setting. + var at = rows.FindIndex(l => LabelOf(l) == "port"); + await Assert.That(at).IsGreaterThanOrEqualTo(0); + await Assert.That(rows[at].TrimEnd()).EndsWith("80", StringComparison.Ordinal); + await Assert.That(rows[at + 1].TrimEnd()).EndsWith("23", StringComparison.Ordinal); + await Assert.That(rows[at + 2].TrimEnd()).EndsWith("4201", StringComparison.Ordinal); + + // The name is printed once, so three values read as one variable rather than as three. + await Assert.That(LabelOf(rows[at + 1])).IsEmpty(); + } + + [Test] + public async Task EverythingTheServerSentIsShownAndTheUnofficialHalfIsMarked() + { + var rows = Plain(MsspScreenRenderer.Body( + World(), + Observed(Report( + ("NAME", ["Corvid"]), + ("ANSI", ["1"]), + ("PUEBLO", ["1"]), + ("CORVID SPECIFIC", ["nevermore"]))), + Noon, + Width)); + + var text = string.Join("\n", rows); + await Assert.That(text).Contains(MsspScreenRenderer.EverythingElse); + await Assert.That(text).Contains("ANSI"); + await Assert.That(text).Contains("PUEBLO"); + await Assert.That(text).Contains("CORVID SPECIFIC"); + await Assert.That(text).Contains("nevermore"); + + // Official and unofficial are both visible and are told apart. ANSI is in the specification's + // tables; PUEBLO looks every bit as standard and is not, which is exactly why the reader cannot + // be left to tell from the name. + await Assert.That(rows.Single(l => l.Contains("PUEBLO")).TrimStart()) + .StartsWith(MsspScreenRenderer.UnofficialMark, StringComparison.Ordinal); + await Assert.That(rows.Single(l => l.Contains(" ANSI")).TrimStart()) + .DoesNotStartWith(MsspScreenRenderer.UnofficialMark); + await Assert.That(text).Contains(MsspScreenRenderer.UnofficialLegend); + } + + [Test] + public async Task AMinusOneWorldCountReadsAsUnknownRatherThanAsMinusOne() + { + var rows = Plain(MsspScreenRenderer.Body( + World(), Observed(Report(("ROOMS", ["-1"]))), Noon, Width)); + + await Assert.That(rows.Any(l => l.Contains("ROOMS") && l.Contains(MsspScreenRenderer.Unavailable))) + .IsTrue(); + await Assert.That(rows.Any(l => l.Contains("-1"))).IsFalse(); + } + + [Test] + public async Task AVariableTheServerNeverMentionedReadsDifferentlyFromOneItSentEmpty() + { + // Two different absences and the screen distinguishes them: "this server cannot tell you" is a + // fact about the server, "it never came up" is a fact about the report. + var rows = Plain(MsspScreenRenderer.Body( + World(), Observed(Report(("CONTACT", []))), Noon, Width)); + + await Assert.That(rows.Any(l => l.Contains("contact") && l.Contains(MsspScreenRenderer.Unavailable))) + .IsTrue(); + await Assert.That(rows.Any(l => l.Contains("website") && l.Contains(MsspScreenRenderer.Unreported))) + .IsTrue(); + } + + // ---- Hostile values ---- + + [Test] + public async Task NoValueAStrangerCanSendMakesARowWiderThanTheScreen() + { + var hostile = Report( + ("NAME", [new string('W', 5000)]), + ("WEBSITE", ["https://" + new string('x', 900)]), + ("CONTACT", [new string('あ', 400)])); + + foreach (var width in new[] { 80, 100, 120, 160 }) + { + var rows = MsspScreenRenderer.Body(World(), Observed(hostile), Noon, width); + foreach (var row in rows) + { + await Assert.That(MarkupText.VisibleLength(row)).IsLessThanOrEqualTo(width) + .Because($"a row must fit {width} columns, and this one is off the wire"); + } + } + } + + [Test] + public async Task AValueCarryingMarkupCannotOpenATagOfItsOwn() + { + // A world's value is escaped, so `[bold red]` is text rather than a colour — and, more to the + // point, an unbalanced `[` cannot eat the rest of the row. + var rows = MsspScreenRenderer.Body( + World(), + Observed(Report(("NAME", ["[bold #ff0000 on #ff0000]owned"]), ("STATUS", ["[/]["]))), + Noon, + Width); + + var text = string.Join("\n", rows); + await Assert.That(text).Contains("[[bold #ff0000 on #ff0000]]owned"); + await Assert.That(Plain(rows).Any(l => l.Contains("[/]["))).IsTrue(); + } + + [Test] + public async Task AValueCarryingControlCharactersCannotAddOrShiftARow() + { + // A raw newline inside one value would end its row early and put a fragment of a stranger's text + // on a line of its own, below the row it belongs to and outside the column it was measured for. + // An ESC would be worse: the frame is ANSI. + var clean = MsspScreenRenderer.Body( + World(), Observed(Report(("NAME", ["ordinary"]))), Noon, Width); + var nasty = MsspScreenRenderer.Body( + World(), Observed(Report(("NAME", ["a\nb\rcd\te"]))), Noon, Width); + + await Assert.That(nasty).HasCount().EqualTo(clean.Count); + foreach (var row in nasty) + { + await Assert.That(row.Any(char.IsControl)).IsFalse(); + } + } + + [Test] + public async Task AVariableWithAThousandValuesSpendsABoundedNumberOfRows() + { + var flood = Report(("REFERRAL", Enumerable.Range(0, 1000).Select(i => $"h{i}.example.org 4000").ToArray())); + var rows = Plain(MsspScreenRenderer.Body(World(), Observed(flood), Noon, Width)); + + await Assert.That(rows.Count(l => l.Contains("example.org"))).IsEqualTo(MsspScreenRenderer.MaxValueRows); + await Assert.That(rows.Any(l => l.Contains("more"))).IsTrue(); + } + + // ---- The shape of the screen ---- + + [Test] + public async Task EveryStopTheCursorHasIsARowTheScreenDraws() + { + // Model counts Body's rows and Render draws them, so a cursor stop that was never drawn is + // structurally impossible rather than two functions agreeing by inspection. + var observation = Observed(Report(("NAME", ["Corvid"]), ("PORT", ["23", "4201"]))); + var model = MsspScreenRenderer.Model(World(), observation, Noon, Width); + + await Assert.That(model.PaneCount).IsEqualTo(1); + await Assert.That(model.Sizes[0]) + .IsEqualTo(MsspScreenRenderer.Body(World(), observation, Noon, Width).Count); + } + + [Test] + public async Task TheReportOffersNothingToEditToggleOrRemove() + { + // Read-only is the shape. Every hint on these screens is derived from the model, so a screen + // that offered none of the three physically cannot advertise them. + var model = MsspScreenRenderer.Model(World(), Observed(Report(("NAME", ["Corvid"]))), Noon, Width); + + await Assert.That(model.HasEditableRow).IsFalse(); + await Assert.That(model.HasRemovableRow).IsFalse(); + await Assert.That(model.HasDetailRow).IsFalse(); + } + + [Test] + public async Task ALongReportScrollsRatherThanLosingItsTail() + { + var long_ = Report(Enumerable.Range(0, 60) + .Select(i => ($"VAR{i}", new[] { $"value {i}" })) + .ToArray()); + var observation = Observed(long_); + + var top = Plain(MsspScreenRenderer.Render( + World(), observation, Noon, new ScreenFocus(0, 0), 20, Width)); + var down = Plain(MsspScreenRenderer.Render( + World(), observation, Noon, new ScreenFocus(0, 60), 20, Width)); + + await Assert.That(top).HasCount().EqualTo(20); + await Assert.That(down).HasCount().EqualTo(20); + await Assert.That(string.Join("\n", top)).IsNotEqualTo(string.Join("\n", down)); + + // The edges say what they are hiding rather than silently ending. + await Assert.That(top[^1]).Contains("more"); + await Assert.That(down[0]).Contains("more"); + } + + [Test] + public async Task TheReportFitsEveryTerminalItIsRenderedIn() + { + // Read off the frame the driver was handed, not off the markup: a settings screen is composed + // into real controls, and this repository has been bitten by text overrunning a narrow panel. + foreach (var (width, height) in new[] { (80, 24), (100, 30), (120, 32), (160, 48) }) + { + foreach (var row in Frame("mssp", width, height)) + { + await Assert.That(row.TrimEnd().Length).IsLessThanOrEqualTo(width) + .Because($"the report must fit {width}x{height}"); + } + } + } +} From 8fac7f7d428b32584110c72fde7ea2979d109018 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:17:01 -0500 Subject: [PATCH 3/7] Build the MSSP screen warning-free, and stop rebuilding a set per row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things a --no-incremental build turned up that an incremental one hid. TUnit's HasCount() is [Obsolete] in favour of Count(); five of the new assertions used it, and this repository's primary signal is a build with no warnings in it. An incremental build does not recompile a test project whose sources have not changed since, which is exactly how they went unseen. MsspData.UnofficialNames allocates and filters on every read, and the mark column asked it once per drawn row — a hundred-variable report walked it a hundred times. Built once per render instead. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- src/SharpMUTerm.Tui/MsspScreenRenderer.cs | 13 ++++++++----- .../SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs | 4 ++-- tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs | 6 +++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/SharpMUTerm.Tui/MsspScreenRenderer.cs b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs index 9f36e5f..63800dd 100644 --- a/src/SharpMUTerm.Tui/MsspScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs @@ -247,6 +247,10 @@ internal static List Body( } var report = observation.Report!; + + // Built once. MsspData.UnofficialNames allocates and filters on every read, and the mark column + // asks per row — a hundred-variable report would have walked it a hundred times. + var unofficial = new HashSet(report.UnofficialNames, StringComparer.Ordinal); rows.Add(string.Empty); rows.Add(Row("captured", Since(observation.ObservedAt!.Value, now), cells)); rows.Add(string.Empty); @@ -256,7 +260,7 @@ internal static List Body( foreach (var (label, variable) in Headline) { drawn.Add(variable); - AddVariable(rows, label, report[variable], variable, report, now, cells); + AddVariable(rows, label, report[variable], variable, report, unofficial, now, cells); } var remaining = report.Keys.Where(name => !drawn.Contains(name)).ToList(); @@ -270,7 +274,7 @@ internal static List Body( foreach (var name in remaining) { - AddVariable(rows, name, report[name], name, report, now, cells); + AddVariable(rows, name, report[name], name, report, unofficial, now, cells); } rows.Add(string.Empty); @@ -312,12 +316,11 @@ private static void AddVariable( IReadOnlyList values, string variable, MsspData report, + IReadOnlySet unofficial, DateTimeOffset now, int cells) { - var mark = report.ContainsKey(variable) && report.UnofficialNames.Contains(variable) - ? UnofficialMark - : " "; + var mark = unofficial.Contains(variable) ? UnofficialMark : " "; if (values.Count == 0) { diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs index 3f62677..35433bf 100644 --- a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs @@ -70,7 +70,7 @@ public async Task OneServerIsOneEntryHoweverItsHostIsSpelled() cache.RecordReport("MUD.Example.ORG.", 4201, Report(("NAME", ["One"])), Noon); cache.RecordReport("mud.example.org", 4201, Report(("NAME", ["Two"])), Noon); - await Assert.That(cache.All).HasCount().EqualTo(1); + await Assert.That(cache.All).Count().IsEqualTo(1); await Assert.That(cache.Find("MUD.EXAMPLE.ORG", 4201)!.Report!.Name).IsEqualTo("Two"); } @@ -215,7 +215,7 @@ public async Task TheOldestEndpointsAreDroppedOnceTheFileIsFull() cache.RecordConnection($"host{i}.example.org", 4000, Noon.AddMinutes(i)); } - await Assert.That(cache.All).HasCount().EqualTo(MsspCache.MaxEndpoints); + await Assert.That(cache.All).Count().IsEqualTo(MsspCache.MaxEndpoints); await Assert.That(cache.Find("host0.example.org", 4000)).IsNull(); await Assert.That(cache.Find($"host{MsspCache.MaxEndpoints + 9}.example.org", 4000)).IsNotNull(); } diff --git a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs index b3686d0..6e1d5af 100644 --- a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs @@ -419,7 +419,7 @@ public async Task AValueCarryingControlCharactersCannotAddOrShiftARow() var nasty = MsspScreenRenderer.Body( World(), Observed(Report(("NAME", ["a\nb\rcd\te"]))), Noon, Width); - await Assert.That(nasty).HasCount().EqualTo(clean.Count); + await Assert.That(nasty).Count().IsEqualTo(clean.Count); foreach (var row in nasty) { await Assert.That(row.Any(char.IsControl)).IsFalse(); @@ -476,8 +476,8 @@ public async Task ALongReportScrollsRatherThanLosingItsTail() var down = Plain(MsspScreenRenderer.Render( World(), observation, Noon, new ScreenFocus(0, 60), 20, Width)); - await Assert.That(top).HasCount().EqualTo(20); - await Assert.That(down).HasCount().EqualTo(20); + await Assert.That(top).Count().IsEqualTo(20); + await Assert.That(down).Count().IsEqualTo(20); await Assert.That(string.Join("\n", top)).IsNotEqualTo(string.Join("\n", down)); // The edges say what they are hiding rather than silently ending. From 2dcb2d621935bf8c31d5c42dd4b1412353aeb2d0 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:19:01 -0500 Subject: [PATCH 4/7] Review a deletion made before an INFO screen was opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing from over a report has to review the *screen's* deletions. A report carries an edit log of its own and it is always empty, so reading the top of the stack silently dropped a world someone had just taken out — which is the one class of edit this project asks about precisely because its subject cannot be retyped. The line was already right; nothing pinned it. Now something does, and breaking it fails ADeletionMadeBeforeOpeningAReportIsStillReviewedOnTheWayOut rather than being noticed by a user. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- .../SharpMUTerm.Tui.Tests/MsspScreenTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs index 6e1d5af..be85cab 100644 --- a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs @@ -129,6 +129,30 @@ public async Task EscapeFromTheReportGoesBackToTheWorldItWasOpenedFrom() await Assert.That(app.RenderWholeFrame()).Contains("Worlds & Characters"); } + [Test] + public async Task ADeletionMadeBeforeOpeningAReportIsStillReviewedOnTheWayOut() + { + // Closing from *over* a report has to review the screen's deletions, not the report's. A report + // carries an edit log of its own and it is always empty, so reading the top of the stack would + // silently drop a world someone had just taken out — the one class of edit this project asks + // about precisely because it cannot be retyped. + Console.SetIn(TextReader.Null); + var app = new SharpMUTermApp(DemoScene.Build(), Headless, new HeadlessConsoleDriver(Width, Height)); + app.RenderSnapshot("worlds"); + + app.Settings.SimulateKey(Key(ConsoleKey.Delete)); + app.Settings.SimulateKey(new ConsoleKeyInfo('i', ConsoleKey.I, false, false, false)); + await Assert.That(app.Settings.IsShowingDetail).IsTrue(); + + // F5 is a global shortcut, not one of the screen's own keys, so it is pressed the way the + // framework would deliver it — through the app rather than into the overlay. + app.SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.F5, false, false, false)); + + await Assert.That(app.Settings.IsOpen).IsFalse(); + await Assert.That(app.Settings.Review.IsOpen).IsTrue(); + await Assert.That(string.Join("\n", app.Settings.Review.Lines)).Contains("Aetherfall"); + } + // ---- The key's shape and scope ---- /// From ecd1c6aec1ded4465d0d259c5c2f6249abd1e244 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:20:24 -0500 Subject: [PATCH 5/7] Record the INFO screen's shape against the decision that deferred it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The design doc left "its own full-screen overlay or an overlay over F5" open. It is an overlay over F5 — one window, content swapped over a one-deep stack — for a reason the doc could not have known: two modal windows with two PreviewKeyPressed handlers cannot be driven headlessly, which would have put this screen outside every snapshot and every test in the suite. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- .../specs/2026-07-30-structured-server-data-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-07-30-structured-server-data-design.md b/docs/superpowers/specs/2026-07-30-structured-server-data-design.md index ebfb766..9c939bf 100644 --- a/docs/superpowers/specs/2026-07-30-structured-server-data-design.md +++ b/docs/superpowers/specs/2026-07-30-structured-server-data-design.md @@ -1240,7 +1240,7 @@ promise. `docs/PLAN.md:46`'s `GmcpRouter` should be renamed to `SessionData` in | Per-world control of whether we volunteer `IAC DO GMCP` | A server that misbehaves on an unsolicited `DO`. None is known. | | `Core.Supports.Add` mid-session vs. requiring a reconnect | Testing against real worlds; the spec permits Add but adoption is uneven. | | Live Lua table vs. snapshot copy for `gmcp.get` | Whether the script bridge marshals callbacks onto the UI thread. If it does, a live table is safe and free. | -| INFO as its own full-screen overlay vs. an overlay over F5 | Whether anything else wants a read-only report. If the client message viewer and this converge, make them one mechanism — which argues for its own screen. | +| ~~INFO as its own full-screen overlay vs. an overlay over F5~~ | **Settled: over F5, one window, content swapped over a one-deep stack.** It keeps the world you were looking at and Esc goes *back* to it; it costs no global F-key; and it stays snapshot- and test-reachable, which a second modal window would not be — `EditReviewOverlay` already documents why two `PreviewKeyPressed` modals cannot be driven headlessly. The convergence argument still stands and is now cheaper to act on: anything else wanting a read-only report reuses `SettingsOverlay.OpenDetail` and a `ScreenButtonKind.Detail` row. | | Upstream PR vs. reflection for `TerminalTypes` | Not either/or: reflection unblocks stage 2, the PR removes it. The question is only whether stage 2 waits, and it should not. | | Whether the 8 KiB GMCP truncation (§1.1) is a bug or a deliberate limit | Ask upstream. It is silent either way, which is the part that is certainly wrong: a message that loses its tail should say so. This is the highest-value upstream report of the three named in this document, because it makes a *correct* server look like a broken one, and no amount of care on our side can recover the dropped bytes. | | Whether MSSP should also be shown at connect, in the output pane | Nobody has asked for it, and it is the server's stream. Deliberately not designed here. | From e72f0cc558bb90e60739ff9ef6fc4378e2d9190e Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:22:28 -0500 Subject: [PATCH 6/7] Read the MSSP cache without trusting the JSON kinds in it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JsonNode.GetValue() throws on a number or a bool, and mssp.json sits beside config.json — a file people hand-edit. A mistyped entry would have thrown out of MsspCache's constructor, on the startup path, over a cache whose entire failure mode is supposed to be 'start empty and say so'. Both readers are total now, and a file whose every field is the wrong kind comes back as skipped rows rather than as a crash. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs | 22 +++++++++++---- .../Telnet/MsspCacheTests.cs | 27 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs index 9a0dcfc..fa0df76 100644 --- a/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs +++ b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs @@ -338,7 +338,7 @@ private void Evict() return $"{FileName} is not a JSON object; starting with no cached server information."; } - var version = root["version"]?.GetValue() ?? 0; + var version = Number(root["version"]) ?? 0; if (version > CurrentVersion) { return $"{FileName} was written by a newer version of SharpMUTerm " @@ -372,17 +372,29 @@ private void Evict() } } + /// + /// A node as a string, or null when it is absent or is any other JSON kind. GetValue<string>() + /// throws on a number or a bool, and this file is on a path a user can hand-edit — so a + /// mistyped entry would have taken the client's whole startup with it rather than being one skipped + /// row. Same reasoning for . + /// + private static string? Text(JsonNode? node) => + node is JsonValue value && value.TryGetValue(out var text) ? text : null; + + private static int? Number(JsonNode? node) => + node is JsonValue value && value.TryGetValue(out var number) ? number : null; + private static MsspObservation? Read(string endpoint, JsonObject entry) { if (endpoint.Length == 0 - || entry["connectedAt"]?.GetValue() is not { } connectedText + || Text(entry["connectedAt"]) is not { } connectedText || !DateTimeOffset.TryParse( connectedText, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var connectedAt)) { return null; } - if (entry["observedAt"]?.GetValue() is not { } observedText + if (Text(entry["observedAt"]) is not { } observedText || !DateTimeOffset.TryParse( observedText, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var observedAt) || entry["variables"] is not JsonArray variables) @@ -394,7 +406,7 @@ private void Evict() foreach (var variable in variables) { if (variable is not JsonObject pair - || pair["name"]?.GetValue() is not { Length: > 0 } name) + || Text(pair["name"]) is not { Length: > 0 } name) { continue; } @@ -404,7 +416,7 @@ private void Evict() { foreach (var value in list) { - if (value?.GetValue() is { } text) + if (Text(value) is { } text) { values.Add(text.Length > MaxValueLength ? text[..MaxValueLength] : text); } diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs index 35433bf..844cc9f 100644 --- a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs @@ -296,6 +296,33 @@ public async Task ACacheFromANewerSchemaIsIgnoredRatherThanMisread() await Assert.That(cache.Problem).IsNotNull(); } + [Test] + public async Task AnEntryOfTheWrongJsonKindIsSkippedRatherThanThrown() + { + // This file sits beside config.json, which people hand-edit, so a number where a string belongs + // is a thing that will happen. JsonNode.GetValue() *throws* on one — and a throw here is + // a throw out of the constructor, on the startup path, over a cache. + using var temp = new TempRoot(); + Directory.CreateDirectory(Path.GetDirectoryName(temp.CachePath)!); + File.WriteAllText(temp.CachePath, """ + { + "version": "one", + "servers": { + "a:1": { "connectedAt": 12345 }, + "b:2": { "connectedAt": "2026-07-30T12:00:00+00:00", "observedAt": true, + "variables": [ { "name": 7, "values": [1, "ok"] } ] } + } + } + """); + + var cache = new MsspCache(temp.CachePath); + + await Assert.That(cache.Find("a", 1)).IsNull(); + var b = cache.Find("b", 2); + await Assert.That(b).IsNotNull(); + await Assert.That(b!.PublishesNothing).IsTrue(); + } + [Test] public async Task OneUnreadableEntryDoesNotCostTheRest() { From fd708452d121b6de4727f20ead36558bc506bb5f Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Thu, 30 Jul 2026 20:47:39 -0500 Subject: [PATCH 7/7] Act on the PR review: two real bugs, three doc regressions of mine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit found eight things worth acting on. Verified each against the code rather than applied on sight; two were genuine defects, three were doc damage this branch had done, one was already fixed, and the rest were text. **The label was padded after it was escaped.** Under EVERYTHING ELSE the label *is* the variable name the server sent, so it is exactly as hostile as a value — and Escape doubles every bracket, so a name containing `[` padded to NameWidth *characters* is short of NameWidth *visible cells* and the value column steps left on that row alone. The hostile-value tests only ever put brackets in a value; they now put them in a name too. Sanitised as well, for the same reason: MSSP says names are upper-case letters and spaces, and a server is not obliged to be truthful. **The cache trusted the endpoint keys in its own file.** Persist writes normalised keys so a self-written file was always fine, but mssp.json sits beside config.json: a hand-written `MUD.Example.ORG:4201` was filed under a key Key() can never produce — unreachable through Find for ever while still spending the endpoint budget. Re-keyed through Key() on the way in. And MaxEndpoints was enforced in Persist alone, so a bloated file was fully materialised at startup and trimmed only if something later wrote, which is exactly the launch where the bound was wanted. Three doc regressions, all introduced by this branch: ScreenModel Remove's summary and its `describe` param were left above the inserted Detail members, so they documented DetailKeyLabel. Rehomed. WorldsScreenRenderer A second was added above the existing one on WorldButtons. Folded into one. CLAUDE.md The architecture line still said the session wraps 2.6.5, and Verification still said six suites — the sweep caught the dependency note and the Building section and missed these two. MsspScreenView rendered the body twice, the second time only to read .Count for the row budget — wasted work per layout pass, and a way for the content and the height it was sized for to disagree the moment the renderer stopped being pure. Declined: nothing. The wrong-JSON-kind finding was already fixed in e72f0cc; the README and design-doc wording is qualified here rather than as suggested, because "a bounded, sanitized copy" buries what the feature does under how it is defended. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM --- CLAUDE.md | 4 +- README.md | 6 ++- ...026-07-30-structured-server-data-design.md | 3 +- src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs | 38 ++++++++++++++- src/SharpMUTerm.Tui/MsspScreenRenderer.cs | 17 +++++-- src/SharpMUTerm.Tui/MsspScreenView.cs | 12 +++-- src/SharpMUTerm.Tui/ScreenModel.cs | 32 ++++++------- src/SharpMUTerm.Tui/WorldsScreenRenderer.cs | 10 ++-- .../Telnet/MsspCacheTests.cs | 47 +++++++++++++++++++ .../SharpMUTerm.Tui.Tests/MsspScreenTests.cs | 24 ++++++++++ 10 files changed, 156 insertions(+), 37 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9064624..62fc099 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,7 +49,7 @@ fallbacks) for inline images/maps. paged off an ephemeral per-session cache under `$XDG_CACHE_HOME`; absolute line indices, ranged reads capped at `MaxRangeLines`, and any disk failure degrades to memory-only. Emphatically **not** the session log — that stays `PlainTextLogSink`/`HtmlLogSink`, opt-in and kept), - `TcpTransport` (TLS + IPv6), `TelnetSession` (wraps TelnetNegotiationCore **2.6.5**), + `TcpTransport` (TLS + IPv6), `TelnetSession` (wraps TelnetNegotiationCore **2.7.0**), trigger/alias/macro engines + `IntervalScheduler`, plain-text + HTML logging, versioned JSON config (worlds → characters + shared trigger sets, with migration), `Theme`/`ThemeLibrary`, and `WorldSession`/`SessionManager` orchestration. @@ -671,7 +671,7 @@ Kept for context; **M1 is done** (see *Repository state* above). As originally s ## Verification -- Primary signal: `dotnet build SharpMUTerm.slnx` plus all six suites (see *Building and testing*). +- Primary signal: `dotnet build SharpMUTerm.slnx` plus all five suites (see *Building and testing*). Keep coverage in `SharpMUTerm.Core.Tests` — ANSI/SGR parser, telnet round-trips, engines. - **The TUI is verifiable headlessly** via the snapshot pipeline above; a claim about layout or chrome should be backed by a rendered frame you actually looked at, not by reading the markup. diff --git a/README.md b/README.md index 93dd0b4..f777c96 100644 --- a/README.md +++ b/README.md @@ -41,8 +41,10 @@ inline image viewer · puppets · multiple input windows · scripting (Lua) · T HTML logging · GMCP / MSDP / MSSP / MCCP · MXP + Pueblo · Unicode/emoji. **MSSP is in and readable.** Every server's own description of itself — name, players, uptime, -codebase, contact, website, and everything else it chose to publish — is captured on connect, kept -between launches, and read from **F5 ▸ `i`** on the selected world. It says when it was captured, +codebase, contact, website, and the rest of what it chose to publish, official variables and invented +ones alike — is captured on connect, kept between launches, and read from **F5 ▸ `i`** on the selected +world. (Capture is bounded and stripped of control characters, because the report comes off the wire +from a stranger; the limits are far above anything a real server sends.) It says when it was captured, because a player count from last week is not a player count; and it distinguishes a world you have never dialled from a server that answered and publishes no MSSP, which is the ordinary case on a MUSH and not a fault. diff --git a/docs/superpowers/specs/2026-07-30-structured-server-data-design.md b/docs/superpowers/specs/2026-07-30-structured-server-data-design.md index 9c939bf..b6c91e9 100644 --- a/docs/superpowers/specs/2026-07-30-structured-server-data-design.md +++ b/docs/superpowers/specs/2026-07-30-structured-server-data-design.md @@ -1202,7 +1202,8 @@ answer when nothing is connected, which is exactly when someone is asking. `/gmc **Stage 3 — MSSP.** -- `MsspReport` + `MsspVariables` in Core; upstream fix or own parse (§8.3). +- ~~`MsspReport` + `MsspVariables` in Core; upstream fix or own parse (§8.3).~~ The upstream fix + landed, so there is no own parse: `MsspData` projects `MSSPConfig.Variables` and parses nothing. - **The TLS-upgrade notice (§8.2)** — first, because it is the smallest piece and the only one with a security payoff. - The `mssp.json` cache. diff --git a/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs index fa0df76..29a5fa0 100644 --- a/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs +++ b/src/SharpMUTerm.Core/Telnet/Mssp/MsspCache.cs @@ -351,9 +351,24 @@ private void Evict() } var skipped = 0; - foreach (var (endpoint, node) in servers) + foreach (var (property, node) in servers) { - if (node is not JsonObject entry || Read(endpoint, entry) is not { } observation) + // Bounded on the way *in*, not only on the way out. MaxEndpoints was enforced in + // Persist, so a bloated or hand-grown file was fully materialised at startup and only + // trimmed if something later wrote — which is the launch where the bound was needed. + if (into.Count >= MaxEndpoints) + { + skipped++; + continue; + } + + // Re-keyed through Key(), never trusted as written. The property name is a string in a + // file a user can edit and a future writer may spell differently, and an entry filed + // under `MUD.Example.ORG:4201` is a key Key() can never produce — permanently + // unreachable through Find while still spending the endpoint budget. + if (node is not JsonObject entry + || Endpoint(property) is not { } endpoint + || Read(endpoint, entry) is not { } observation) { skipped++; continue; @@ -372,6 +387,25 @@ private void Evict() } } + /// + /// A written-down host:port re-normalised through , or null when it is not + /// one. The port is split at the last colon so an IPv6 literal keeps its own. + /// + private static string? Endpoint(string property) + { + var colon = property.LastIndexOf(':'); + if (colon <= 0 + || !int.TryParse( + property[(colon + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out var port) + || port is < 1 or > 65535) + { + return null; + } + + var key = Key(property[..colon], port); + return key.StartsWith(':') ? null : key; + } + /// /// A node as a string, or null when it is absent or is any other JSON kind. GetValue<string>() /// throws on a number or a bool, and this file is on a path a user can hand-edit — so a diff --git a/src/SharpMUTerm.Tui/MsspScreenRenderer.cs b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs index 63800dd..21be61b 100644 --- a/src/SharpMUTerm.Tui/MsspScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/MsspScreenRenderer.cs @@ -369,13 +369,20 @@ private static string Reading(string variable, string value, DateTimeOffset now) } /// - /// A label value row. The label is padded to a constant, the value is sanitised, truncated - /// and escaped, and the whole thing carries a one-cell mark column — reserved, blank when the - /// variable is official, so a report of unofficial variables and a report of official ones put their - /// values in the same column. + /// A label value row. Both halves are sanitised, truncated and escaped, and the whole thing + /// carries a one-cell mark column — reserved, blank when the variable is official, so a report of + /// unofficial variables and one of official variables put their values in the same column. + /// + /// The label is padded before it is escaped, and it is a label off the wire. Under + /// EVERYTHING ELSE the label is the variable name the server sent, so it is exactly as + /// hostile as a value: Escape doubles every bracket, so padding the escaped string pads a + /// name containing [ to fewer visible cells than and the + /// value column steps left on that row alone. Sanitising it matters for the same reason — MSSP + /// says names are upper-case letters and spaces, and a server is not obliged to be truthful. + /// /// private static string Row(string label, string value, int cells, string mark = " ") => - $" [{Muted}]{mark}[/] [{Label}]{Escape(Fit(label, NameWidth)).PadRight(NameWidth)}[/] " + $" [{Muted}]{mark}[/] [{Label}]{Escape(Fit(Sanitize(label), NameWidth).PadRight(NameWidth))}[/] " + ScreenChrome.ReadOnly(Fit(Sanitize(value), cells)); /// diff --git a/src/SharpMUTerm.Tui/MsspScreenView.cs b/src/SharpMUTerm.Tui/MsspScreenView.cs index d8a6ba2..5249b93 100644 --- a/src/SharpMUTerm.Tui/MsspScreenView.cs +++ b/src/SharpMUTerm.Tui/MsspScreenView.cs @@ -26,8 +26,13 @@ internal static IWindowControl Build( MsspScreenRenderer.FooterLine(world, observation, focus, width, now), ScreenPalette.FooterBg); var rows = ScreenChrome.Rows(height); - var body = ScreenChrome.Stretch(new MarkupControl( - MsspScreenRenderer.Render(world, observation, now, focus, rows, width))); + + // Rendered once and both used and measured. It was rendered twice — the second call only to + // read `.Count` for the row budget — which was wasted work on every layout pass and, worse, a + // way for the control's content and the height it was sized for to disagree the moment the + // renderer stopped being a pure function of its arguments. + var lines = MsspScreenRenderer.Render(world, observation, now, focus, rows, width); + var body = ScreenChrome.Stretch(new MarkupControl(lines)); var root = Controls.Grid() .WithAlignment(HorizontalAlignment.Stretch) @@ -47,8 +52,7 @@ internal static IWindowControl Build( // whose emptiness reads as missing data on a screen whose whole subject is what is missing. root.Rows( GridLength.Cells(1), - GridLength.Cells(Math.Clamp( - MsspScreenRenderer.Render(world, observation, now, focus, rows, width).Count, 1, rows)), + GridLength.Cells(Math.Clamp(lines.Count, 1, rows)), GridLength.Star(1), GridLength.Cells(1)) .Columns(GridLength.Star(1)); diff --git a/src/SharpMUTerm.Tui/ScreenModel.cs b/src/SharpMUTerm.Tui/ScreenModel.cs index 8bd2986..163dc5a 100644 --- a/src/SharpMUTerm.Tui/ScreenModel.cs +++ b/src/SharpMUTerm.Tui/ScreenModel.cs @@ -118,22 +118,6 @@ internal static ScreenButton Add( target); } - /// - /// Removes the item at , restoring it *at that index* on undo. The cursor - /// stays on the same ordinal, which is now whatever followed the deleted row — the same place the - /// eye is. means what it does on . - /// - /// It takes no label. A removal is drawn as the key that runs it - /// () rather than as a chip, because its row is no longer - /// somewhere the cursor can go — see for why, and - /// for what that row now reads as. - /// - /// - /// - /// What this removal would destroy, in words, for the closing review to name. See - /// ; it defaults to , which is the honest - /// answer for every row that takes nothing else with it. - /// /// /// What a read-only detail screen's key is drawn as, for the same reason /// is: the row naming it is not a cursor stop, so a chip would be an @@ -166,6 +150,22 @@ internal static ScreenButton Detail(string target, Action open) target); } + /// + /// Removes the item at , restoring it *at that index* on undo. The cursor + /// stays on the same ordinal, which is now whatever followed the deleted row — the same place the + /// eye is. means what it does on . + /// + /// It takes no label. A removal is drawn as the key that runs it + /// () rather than as a chip, because its row is no longer + /// somewhere the cursor can go — see for why, and + /// for what that row now reads as. + /// + /// + /// + /// What this removal would destroy, in words, for the closing review to name. See + /// ; it defaults to , which is the honest + /// answer for every row that takes nothing else with it. + /// internal static ScreenButton Remove( IList list, int index, int offset = 0, string? target = null, Func? describe = null) { diff --git a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs index 1ee314c..f32100b 100644 --- a/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/WorldsScreenRenderer.cs @@ -646,11 +646,6 @@ private static ScreenRow[] SecurityRows(WorldDefinition world) => new[] () => world.AllowInvalidCertificates, v => world.AllowInvalidCertificates = v)), }; - /// - /// The WORLDS list's buttons. Deleting is offered only when there is a world under the cursor to - /// delete; a brand-new world is a blank template, because a world's whole identity is its host and - /// a "helpfully" prefilled one would be a guess the user then has to notice and undo. - /// /// /// The WORLDS pane's buttons, in the order they are drawn and — decisively — in the order /// needs them: the cursor stop first, then the two targeted key @@ -658,6 +653,11 @@ private static ScreenRow[] SecurityRows(WorldDefinition world) => new[] /// Del both act on the selected row and must not steal the cursor from it, so they trail and /// are trimmed out of the pane's stop count. Put either of them above [+ world] and the /// cursor gains a hole. + /// + /// Both targeted keys are offered only when there is a world under the cursor for them to act on. A + /// brand-new world is a blank template, because a world's whole identity is its host and a + /// "helpfully" prefilled one would be a guess the user then has to notice and undo. + /// /// private static List WorldButtons( IReadOnlyList worlds, int selectedWorld, Action? info = null) diff --git a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs index 844cc9f..ef6edf6 100644 --- a/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Telnet/MsspCacheTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Telnet.Mssp; @@ -296,6 +297,52 @@ public async Task ACacheFromANewerSchemaIsIgnoredRatherThanMisread() await Assert.That(cache.Problem).IsNotNull(); } + [Test] + public async Task AnEntryWrittenUnderAnUnnormalisedKeyIsStillFindable() + { + // Persist writes normalised keys, so a self-written file is always fine — but this file sits + // beside config.json and a hand-written (or future, or foreign) entry spelled `MUD.Example.ORG:4201` + // would be filed under a key Key() can never produce: permanently unreachable through Find while + // still spending the endpoint budget. Re-key on the way in, never trust the property name. + using var temp = new TempRoot(); + Directory.CreateDirectory(Path.GetDirectoryName(temp.CachePath)!); + File.WriteAllText(temp.CachePath, """ + { + "version": 1, + "servers": { + "MUD.Example.ORG.:4201": { "connectedAt": "2026-07-30T12:00:00+00:00" }, + "not-an-endpoint": { "connectedAt": "2026-07-30T12:00:00+00:00" } + } + } + """); + + var cache = new MsspCache(temp.CachePath); + + await Assert.That(cache.Find("mud.example.org", 4201)).IsNotNull(); + await Assert.That(cache.All).Count().IsEqualTo(1); + await Assert.That(cache.Problem).IsNotNull().Because("the portless entry was skipped"); + } + + [Test] + public async Task ABloatedFileIsBoundedOnTheWayInAndNotOnlyOnTheNextWrite() + { + // MaxEndpoints was enforced in Persist alone, so a file grown past it was fully materialised at + // startup and trimmed only if something later wrote — which is precisely the launch where the + // bound was wanted. + using var temp = new TempRoot(); + Directory.CreateDirectory(Path.GetDirectoryName(temp.CachePath)!); + var entries = string.Join( + ",\n", + Enumerable.Range(0, MaxEndpointsOverflow).Select(i => + " \"host" + i.ToString(CultureInfo.InvariantCulture) + + ".example.org:4000\": { \"connectedAt\": \"2026-07-30T12:00:00+00:00\" }")); + File.WriteAllText(temp.CachePath, "{ \"version\": 1, \"servers\": {\n" + entries + "\n} }"); + + await Assert.That(new MsspCache(temp.CachePath).All).Count().IsEqualTo(MsspCache.MaxEndpoints); + } + + private const int MaxEndpointsOverflow = MsspCache.MaxEndpoints + 25; + [Test] public async Task AnEntryOfTheWrongJsonKindIsSkippedRatherThanThrown() { diff --git a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs index be85cab..d92770b 100644 --- a/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/MsspScreenTests.cs @@ -450,6 +450,30 @@ public async Task AValueCarryingControlCharactersCannotAddOrShiftARow() } } + [Test] + public async Task AVariableNameCarryingMarkupKeepsTheValueColumnWhereItIs() + { + // Under EVERYTHING ELSE the *label* is the variable name the server sent, so it is exactly as + // hostile as a value — and padding it after escaping is the subtle half: Escape doubles every + // bracket, so a name containing `[` padded to NameWidth *characters* is short of NameWidth + // *visible cells*, and the value column steps left on that one row. + var rows = MsspScreenRenderer.Body( + World(), + Observed(Report(("PLAIN", ["a"]), ("A[B]C", ["b"]), ("D[[E", ["c"]))), + Noon, + Width); + + var values = Plain(rows) + .Where(l => l.TrimEnd().EndsWith("a", StringComparison.Ordinal) + || l.TrimEnd().EndsWith("b", StringComparison.Ordinal) + || l.TrimEnd().EndsWith("c", StringComparison.Ordinal)) + .ToList(); + + await Assert.That(values).Count().IsEqualTo(3); + await Assert.That(values.Select(l => l.Length).Distinct()).Count().IsEqualTo(1) + .Because("a bracket in a variable name must not move the value column"); + } + [Test] public async Task AVariableWithAThousandValuesSpendsABoundedNumberOfRows() {