From 36ada7313da87c0ed885db0f6b1676950056d14f Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 28 Oct 2026 00:06:36 +0100 Subject: [PATCH 1/2] Show each trigger's subtype and data, and refuse a damaged trigger block Since snapshot schema seven a trigger's subtype and data were kept in the snapshot and in --json, but a person still saw only kinds and actions. bws show now prints one line per trigger with its subtype, and each data item on a line under it, aligned under the value column. The ten subtypes the SERVICE_TRIGGER documentation names are shown as words, only under the kind the documentation gives them, and every other one as its GUID. Bytes read in pairs as sc qtriggerinfo prints them. Only the triggers are split into lines: a line break inside any value from outside is still written out, so it cannot print a line that looks like a field of the report. A comparison shows a change of triggers as one line per trigger that only one side holds, with "nothing" on the other side, in the terminal and as rows of the window's table. The triggers on one side only are counted as a multiset and never paired. The machine document is unchanged: before and after still carry both whole lists. The window's details panel has a Triggers section after the fields, one full-width line per trigger, and a copy of the entry carries it. A prose line's label now spans the row, so a long trigger label no longer widens the label column of the whole panel. A trigger block too short for its header, pointing nowhere, or counting more records than it holds is now refused with ERROR_INVALID_DATA like a data item outside the block, instead of reading as no triggers or as the records that fitted. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 19 +++ src/Bws.Cli/DiffText.cs | 33 ++++- src/Bws.Cli/EntryReport.cs | 48 ++++-- src/Bws.Cli/Resources/cli.en.json | 17 +++ src/Bws.Cli/TriggerWords.cs | 72 +++++++++ src/Bws.Core/ManagerBlocks.Triggers.cs | 48 +++--- src/Bws.Core/Snapshots/EntryDocument.cs | 36 +++-- src/Bws.Core/Snapshots/SnapshotDiff.Values.cs | 41 ++++++ src/Bws.Core/Snapshots/SnapshotDiff.cs | 26 +++- src/Bws.Core/Snapshots/SnapshotJson.cs | 8 + src/Bws.Core/Snapshots/TriggerParts.cs | 127 ++++++++++++++++ src/Bws.Gui/DetailsView.xaml | 11 +- src/Bws.Gui/Resources/gui.en.json | 18 +++ src/Bws.Gui/ViewModels/CellFaces.cs | 2 +- src/Bws.Gui/ViewModels/Details.cs | 32 +++- src/Bws.Gui/ViewModels/DiffLines.cs | 30 +++- src/Bws.Gui/ViewModels/TriggerLines.cs | 91 ++++++++++++ tests/Bws.Cli.Tests/DiffOutputTests.cs | 48 ++++++ tests/Bws.Cli.Tests/EntryReportTests.cs | 73 +++++++++ tests/Bws.Cli.Tests/TerminalTextTests.cs | 18 ++- tests/Bws.Core.Tests/ManagerBlockTests.cs | 50 +++++-- tests/Bws.Core.Tests/SnapshotGoldenTests.cs | 3 +- tests/Bws.Core.Tests/TriggerChangeTests.cs | 138 +++++++++++++++++ tests/Bws.Core.Tests/TriggerPartsTests.cs | 81 ++++++++++ tests/Bws.Gui.Tests/DetailsGuards.cs | 68 +++++++++ tests/Bws.Gui.Tests/DetailsViewGuards.cs | 68 +++++++++ tests/Bws.Gui.Tests/DiffSheetTests.cs | 29 ++++ tests/mutation/entries.ps1 | 139 ++++++++++++++++-- 28 files changed, 1281 insertions(+), 93 deletions(-) create mode 100644 src/Bws.Core/Snapshots/TriggerParts.cs create mode 100644 src/Bws.Gui/ViewModels/TriggerLines.cs create mode 100644 tests/Bws.Core.Tests/TriggerChangeTests.cs create mode 100644 tests/Bws.Core.Tests/TriggerPartsTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f2fb7f..8c92c664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,9 +115,28 @@ is not part of this repository. and `bws show --json`, as `subtype` and a `data` list of items with a `type` and their `values`. In the JSON of a comparison the new caveat is `triggerDetailDiffers`, at the end of `caveats`. An older version of this tool refuses a version 7 snapshot by name. +- `bws show` prints each trigger on a line of its own with what it waits for, and each of its data + items on a line under it - for example `Firewall port/Start Port opened` and under it + `500, UDP, %windir%\system32\svchost.exe, IKEEXT`. The ten subtypes Microsoft documents are named in + words (RPC interface, named pipe, joined or left a domain, port opened or closed, machine or user + policy, first IP address arrived, last one removed), any other one is shown as its GUID, and bytes + are shown in pairs the way `sc qtriggerinfo` shows them. It used to print the kinds and actions only, + all on one line. +- The details panel of the window has a *Triggers* section after the fields, one line per trigger with + its subtype and data drawn at the full width of the panel, and a copy of the entry carries it too. + The *Triggers* line among the advanced fields still names the kinds, once each. +- A comparison shows a change of triggers as one line per trigger that only one side has, with + "nothing" on the other side - in the terminal and as rows of the window's table. It used to show the + two whole lists of triggers as JSON, which for an entry losing one of thirteen triggers was a line of + about three thousand characters. `--json` is unchanged and still carries both whole lists. ### Fixed +- A list of triggers that Windows hands back damaged - too short for its own header, or counting more + triggers than it holds - is now reported as a field that could not be read. It used to come back as + no triggers at all, or as the triggers that fitted, as if the list were complete. Nothing like this + has been seen on a real machine. + - A restart no longer starts, at its end, a service this run did not stop - one somebody else was already stopping when the step came, or one that was starting and then stopped by itself. The step asked for nothing and only watched the service get there, and its line in a terminal now says so - diff --git a/src/Bws.Cli/DiffText.cs b/src/Bws.Cli/DiffText.cs index a1512531..53ee0702 100644 --- a/src/Bws.Cli/DiffText.cs +++ b/src/Bws.Cli/DiffText.cs @@ -143,15 +143,9 @@ private static void Changed(StringBuilder text, string heading, IReadOnlyList + /// One difference as the lines it reads in: "field: before -> after", and for the triggers one such + /// line per trigger only one side holds, the other side "nothing" (backlog 625, owner's decision "a + /// line per trigger", 2026-10-27). Until then the triggers were the two whole lists as JSON - about + /// three thousand characters on one line for an entry losing one of thirteen. + /// + /// The whole lists are still the answer when there are no trigger lines to give - a field that + /// differs only as "none" against an empty list, which no real reading writes but a file can hold. + /// A difference reported with nothing under it would be the silence rule 8 forbids. + /// + private static IEnumerable Lines(FieldDifference difference) + { + var key = difference.Group == DifferenceGroup.RunningState ? "cli.diff.field.state" : "cli.diff.field.configuration"; + + if (difference.Triggers is not { } change || change.Removed.Count + change.Added.Count == 0) + { + return [Texts.Of(key, difference.Field, Value(difference.Before), Value(difference.After))]; + } + + return change.Removed.Select(trigger => Texts.Of(key, difference.Field, TriggerWords.Line(trigger), Value(null))) + .Concat(change.Added.Select(trigger => Texts.Of(key, difference.Field, Value(null), TriggerWords.Line(trigger)))); + } + /// /// A missing value said in words rather than left blank. /// diff --git a/src/Bws.Cli/EntryReport.cs b/src/Bws.Cli/EntryReport.cs index 96e3be84..11e0ae80 100644 --- a/src/Bws.Cli/EntryReport.cs +++ b/src/Bws.Cli/EntryReport.cs @@ -1,5 +1,6 @@ using System.Globalization; using Bws.Core; +using Bws.Core.Snapshots; namespace Bws.Cli; @@ -34,7 +35,12 @@ internal static class EntryReport /// private readonly record struct Told(ReadOutcome Outcome, string Text, int Code); - private sealed record Field(string Key, Func Reads); + /// + /// Whether the value is several lines of this tool's own making, joined by a line feed - the triggers, + /// since 2026-10-27. Every piece of such a value has been through + /// already, so the only line feeds left in it are the joins. + /// + private sealed record Field(string Key, Func Reads, bool Lines = false); private const string Identity = "cli.show.group.basics"; private const string About = "cli.show.group.about"; @@ -85,7 +91,7 @@ private static readonly (string Heading, Field[] Fields)[] Catalogue = new Field("cli.show.loadOrderGroup", entry => Say(entry.LoadOrderGroup, value => value)), new Field("cli.show.errorControl", entry => Say(entry.ErrorControl, value => value.ToString())), new Field("cli.show.dependsOn", entry => Say(entry.DependsOn, List)), - new Field("cli.show.triggers", entry => Say(entry.Triggers, Triggers)), + new Field("cli.show.triggers", entry => Say(entry.Triggers, Triggers), Lines: true), new Field("cli.show.requiredPrivileges", entry => Say(entry.RequiredPrivileges, List)), new Field("cli.show.securityDescriptor", entry => Say(entry.SecurityDescriptor, value => value)) ]) @@ -112,7 +118,7 @@ internal static string Render(ScmEntry entry, bool full) foreach (var (heading, fields) in Catalogue) { var shown = fields - .Select(field => (field.Key, Told: field.Reads(entry))) + .Select(field => (field.Key, Told: field.Reads(entry), field.Lines)) .Where(pair => full || pair.Told.Outcome != ReadOutcome.Absent) .ToList(); @@ -134,15 +140,32 @@ internal static string Render(ScmEntry entry, bool full) // what makes it scannable. var width = shown.Max(pair => Texts.Of(pair.Key).Length); - lines.AddRange(shown.Select(pair => Texts.Of( - "cli.show.line", - Texts.Of(pair.Key).PadRight(width), - Spell(pair.Told)))); + lines.AddRange(shown.SelectMany(pair => Aligned(Texts.Of(pair.Key).PadRight(width), Spell(pair.Told), pair.Lines))); } return string.Join(Environment.NewLine, lines); } + /// + /// One field as the lines it prints: the label and the value, and for a value of several lines of this + /// tool's own making - the triggers, since 2026-10-27 (backlog 625) - every further line under the + /// first one's value rather than at the left edge. + /// + /// ONLY those values are split, and that is the half that matters. A line break inside a value + /// that came from outside - a description, a path, a name out of a snapshot - is written out as + /// <U+000A> by since stability report C-4, because a + /// value free to break the line can print a line that looks like a field of this report. The first + /// version of this method split every value and TerminalTextTests went red on it. + /// + private static IEnumerable Aligned(string label, string value, bool lines) + { + var parts = lines ? value.Split('\n') : [value]; + var first = Texts.Of("cli.show.line", label, parts[0]); + var under = new string(' ', first.Length - Printable.Of(parts[0]).Length); + + return parts.Skip(1).Select(part => under + part).Prepend(first); + } + /// /// How one value reads once its state is taken into account. /// @@ -212,12 +235,13 @@ private static Told Publisher(ScmEntry entry) // - Unknown, Start and Stop - so its value name and the word for a person are the same string // and there is nothing to disagree about. Turning it into a lookup would buy one more file and // three more keys to say what it already says. + // + // A TRIGGER PER LINE WITH ITS SUBTYPE AND DATA SINCE 2026-10-27 (backlog 625), where it used to be + // the kinds and actions joined by commas - through the document form, so a live entry reads the + // way a comparison reads the same trigger. private static string Triggers(IReadOnlyList triggers) => string.Join( - ", ", - triggers.Select(trigger => Texts.Of( - "cli.show.trigger", - TriggerWords.Of(trigger.Kind), - trigger.Action.ToString()))); + '\n', + triggers.SelectMany(trigger => TriggerWords.Lines(TriggerDocument.Of(trigger)))); /// /// Megabytes with one decimal, the same as the listing, so the two agree about one entry. diff --git a/src/Bws.Cli/Resources/cli.en.json b/src/Bws.Cli/Resources/cli.en.json index 49823ed8..75f7e133 100644 --- a/src/Bws.Cli/Resources/cli.en.json +++ b/src/Bws.Cli/Resources/cli.en.json @@ -213,6 +213,23 @@ "cli.show.yes": "yes", "cli.show.no": "no", "cli.show.trigger": "{0}/{1}", + "cli.show.trigger.data": " {0}", + "cli.trigger.withSubtype": "{0} {1}", + "cli.trigger.withData": "{0} {1}", + "cli.trigger.level": "level {0}", + "cli.trigger.keywordAny": "any of {0}", + "cli.trigger.keywordAll": "all of {0}", + "cli.trigger.unknownType": "unknown type {0}", + "cli.trigger.subtype.rpcInterface": "RPC interface", + "cli.trigger.subtype.namedPipe": "Named pipe", + "cli.trigger.subtype.joinedDomain": "Joined a domain", + "cli.trigger.subtype.leftDomain": "Left a domain", + "cli.trigger.subtype.portOpened": "Port opened", + "cli.trigger.subtype.portClosed": "Port closed", + "cli.trigger.subtype.machinePolicy": "Machine policy", + "cli.trigger.subtype.userPolicy": "User policy", + "cli.trigger.subtype.firstAddressArrived": "First address arrived", + "cli.trigger.subtype.lastAddressRemoved": "Last address removed", "cli.show.serviceName": "Service name", "cli.show.displayName": "Display name", "cli.show.description": "Description", diff --git a/src/Bws.Cli/TriggerWords.cs b/src/Bws.Cli/TriggerWords.cs index 70181bb2..0d13a019 100644 --- a/src/Bws.Cli/TriggerWords.cs +++ b/src/Bws.Cli/TriggerWords.cs @@ -1,4 +1,5 @@ using Bws.Core; +using Bws.Core.Snapshots; namespace Bws.Cli; @@ -28,6 +29,77 @@ namespace Bws.Cli; /// internal static class TriggerWords { + /// + /// One trigger as the lines bws show prints under its label: the kind, the action and the + /// subtype on the first, and each data item on a line of its own under it, indented (backlog 625, + /// owner's decision "B - data under the trigger", 2026-10-27). + /// + /// One item, one line, because the items are what the documentation counts: a device trigger + /// fires on ANY of its hardware IDs, each an item, while a firewall trigger's one item holds four + /// values that must ALL match. Lines keep the first apart and the comma keeps the second together. + /// + internal static IEnumerable Lines(TriggerDocument trigger) => + Items(trigger).Select(item => Texts.Of("cli.show.trigger.data", item)).Prepend(Head(trigger)); + + /// + /// One trigger on one line, for a comparison, which says "before -> after" per line - the items joined + /// by a bar, the one character no value on the machine this was measured on holds. + /// + internal static string Line(TriggerDocument trigger) + { + var items = Items(trigger).ToList(); + + return items.Count == 0 ? Head(trigger) : Texts.Of("cli.trigger.withData", Head(trigger), string.Join(" | ", items)); + } + + /// + /// The kind and the action, and the subtype after them - in words for the ten the documentation names, + /// as the GUID otherwise. None at all for a trigger out of a file older than schema seven. + /// + private static string Head(TriggerDocument trigger) + { + var head = Texts.Of("cli.show.trigger", Kind(trigger.Kind), trigger.Action); + + return trigger.Subtype is { } subtype + ? Texts.Of("cli.trigger.withSubtype", head, TriggerParts.Subtype(trigger) is { } name ? Subtype(name) : subtype) + : head; + } + + /// + /// A kind as a document names it, in words - and a name this build does not know left as it is, since + /// a file written by a later build may hold one, and "unknown" would claim less than the file says. + /// + private static string Kind(string? kind) => + kind is not null && Enum.GetNames().Contains(kind, StringComparer.Ordinal) + ? Of(Enum.Parse(kind)) + : kind ?? Texts.Of("cli.cell.unknown"); + + /// Each data item as one piece of text, the type's word around its values. + private static IEnumerable Items(TriggerDocument trigger) => + (trigger.Data ?? []).Select(item => TriggerParts.TypeOf(item) switch + { + TriggerDataType.Level => Texts.Of("cli.trigger.level", TriggerParts.Values(item)), + TriggerDataType.KeywordAny => Texts.Of("cli.trigger.keywordAny", TriggerParts.Values(item)), + TriggerDataType.KeywordAll => Texts.Of("cli.trigger.keywordAll", TriggerParts.Values(item)), + TriggerDataType.Unknown => Texts.Of("cli.trigger.unknownType", TriggerParts.Values(item)), + _ => TriggerParts.Values(item) + }); + + private static string Subtype(TriggerSubtypeName name) => name switch + { + TriggerSubtypeName.RpcInterface => Texts.Of("cli.trigger.subtype.rpcInterface"), + TriggerSubtypeName.NamedPipe => Texts.Of("cli.trigger.subtype.namedPipe"), + TriggerSubtypeName.JoinedDomain => Texts.Of("cli.trigger.subtype.joinedDomain"), + TriggerSubtypeName.LeftDomain => Texts.Of("cli.trigger.subtype.leftDomain"), + TriggerSubtypeName.PortOpened => Texts.Of("cli.trigger.subtype.portOpened"), + TriggerSubtypeName.PortClosed => Texts.Of("cli.trigger.subtype.portClosed"), + TriggerSubtypeName.MachinePolicy => Texts.Of("cli.trigger.subtype.machinePolicy"), + TriggerSubtypeName.UserPolicy => Texts.Of("cli.trigger.subtype.userPolicy"), + TriggerSubtypeName.FirstAddressArrived => Texts.Of("cli.trigger.subtype.firstAddressArrived"), + TriggerSubtypeName.LastAddressRemoved => Texts.Of("cli.trigger.subtype.lastAddressRemoved"), + _ => name.ToString() + }; + internal static string Of(TriggerKind kind) => kind switch { TriggerKind.DeviceArrival => Texts.Of("cli.cell.trigger.deviceArrival"), diff --git a/src/Bws.Core/ManagerBlocks.Triggers.cs b/src/Bws.Core/ManagerBlocks.Triggers.cs index 1b4056c1..e0b4d5d7 100644 --- a/src/Bws.Core/ManagerBlocks.Triggers.cs +++ b/src/Bws.Core/ManagerBlocks.Triggers.cs @@ -41,17 +41,24 @@ internal static partial class ManagerBlocks /// very block the call filled, so an array that moved between that call and the fixed statement /// below would leave an offset nowhere near the buffer - refused here rather than followed. /// - /// A block that does not describe itself comes back as nothing read rather than as no - /// triggers. An empty list would be a claim about the service - that it declares none - - /// and that claim is exactly what could not be established. + /// ONE RULE FOR A BLOCK THAT DOES NOT DESCRIBE ITSELF, SINCE 2026-10-27 - the field is refused + /// with ERROR_INVALID_DATA (backlog 626, owner's decision "refuse everywhere"), the way a + /// configuration field outside its block is refused since R-4. A block too short for its header, a + /// record array that does not fit inside the block, a subtype or a data item outside it: each one. /// - /// What a record points at is refused rather than held down, unlike the record count - /// (2026-10-27). A count of records larger than the block keeps the records that fit, as it has - /// since 2026-08-26. A subtype or a data item outside the block refuses the whole field with - /// ERROR_INVALID_DATA, the way a configuration field outside its block is refused since R-4 - - /// because a trigger with one of its data items quietly dropped is a different trigger: a device - /// trigger fires on any of its hardware IDs and an ETW trigger on its filters, so one item fewer - /// widens or narrows what the entry waits for, and the reading would say so as fact. + /// Until that day there were three rules for one fault. A short block or a record pointer + /// to nowhere came back Absent - "declares no triggers", while this header claimed "nothing read" - + /// and a record count larger than the block was held down to the records that fit, from 2026-08-26. + /// Both were the silence rule 8 forbids: a list that looks complete or looks empty, about a block + /// nobody could read. And since schema seven a subtype or a data item outside the block refused the + /// whole field, because a trigger with one of its data items quietly dropped is a different trigger - + /// a device trigger fires on any of its hardware IDs and an ETW trigger on its filters. A record + /// quietly dropped is the same fault one level up, so it is answered the same way. + /// + /// The ordinary case is untouched, and that is measured rather than hoped: an entry with no + /// triggers comes back with a whole header holding a count of nought, which is still Absent, and + /// tools/scm-probe/probe-sizes.ps1 never saw a size of nought or a sizing call that succeeded + /// on any of 799 entries (2026-09-02). None of the refused shapes has been seen on a real machine. /// internal static unsafe Reading> ReadTriggerBuffer(byte[] buffer) { @@ -59,7 +66,7 @@ internal static unsafe Reading> ReadTriggerBuffer( if (buffer.Length < sizeof(SERVICE_TRIGGER_INFO)) { - return Reading>.Absent(); + return OutsideTheBlock>(); } fixed (byte* start = buffer) @@ -72,19 +79,14 @@ internal static unsafe Reading> ReadTriggerBuffer( return Reading>.Absent(); } - var offset = (byte*)info.pTriggers - start; + // The whole record array inside the block before any of it is read - a pointer to nowhere + // lands below the block, and a count the block cannot hold runs past its end. One count, + // checked once and walked once, so the two can never be different numbers again. + var count = info.cTriggers; - if (info.pTriggers is null || offset < 0 || offset >= buffer.Length) + if (Within(info.pTriggers, (long)count * sizeof(SERVICE_TRIGGER), start, buffer.Length) is not { } at) { - return Reading>.Absent(); - } - - var room = (uint)((buffer.Length - offset) / sizeof(SERVICE_TRIGGER)); - var count = Math.Min(info.cTriggers, room); - - if (count == 0) - { - return Reading>.Absent(); + return OutsideTheBlock>(); } var triggers = new List((int)count); @@ -95,7 +97,7 @@ internal static unsafe Reading> ReadTriggerBuffer( // so in the one case the check exists for, a pointer landing in range by accident, it // validated one address and dereferenced another. A guard that looks like a guard and // is not is worse than no guard, because the next reader stops looking. Backlog 297. - var records = (SERVICE_TRIGGER*)(start + offset); + var records = (SERVICE_TRIGGER*)(start + at); return Records(records, count, start, buffer.Length, triggers) ? Reading>.Present(triggers) diff --git a/src/Bws.Core/Snapshots/EntryDocument.cs b/src/Bws.Core/Snapshots/EntryDocument.cs index 56309db0..87461b37 100644 --- a/src/Bws.Core/Snapshots/EntryDocument.cs +++ b/src/Bws.Core/Snapshots/EntryDocument.cs @@ -25,7 +25,28 @@ public sealed record UnreadableDocument(int ErrorCode, string Message); /// numbers, never by this null (). /// public sealed record TriggerDocument( - string Kind, string Action, string? Subtype, IReadOnlyList? Data); + string Kind, string Action, string? Subtype, IReadOnlyList? Data) +{ + /// + /// One trigger as the document writes it. The GUID in lower case through the "D" form, the one + /// sc qtriggerinfo prints, so a person can put the two side by side. + /// + /// Public, and here rather than private to , since 2026-10-27 + /// (backlog 625): the words a person reads about a trigger are made from this form in both + /// interfaces, for bws show as much as for a comparison, so a trigger reads the same in a + /// live entry and in a snapshot - one mapping, not one per caller. + /// + public static TriggerDocument Of(ServiceTrigger trigger) + { + ArgumentNullException.ThrowIfNull(trigger); + + return new( + trigger.Kind.ToString(), + trigger.Action.ToString(), + trigger.Subtype?.ToString("D", CultureInfo.InvariantCulture), + [.. trigger.Data.Select(item => new TriggerDataDocument(item.Type.ToString(), item.Values))]); + } +} /// /// One data item of a trigger: its type in our words and its values as text - see @@ -357,7 +378,7 @@ public static EntryDocument From(ScmEntry entry) DependsOn = entry.DependsOn.IsPresent ? entry.DependsOn.Value : null, RequiredBy = entry.RequiredBy.IsPresent ? entry.RequiredBy.Value : null, - Triggers = entry.Triggers.IsPresent ? [.. entry.Triggers.Value!.Select(Trigger)] : null, + Triggers = entry.Triggers.IsPresent ? [.. entry.Triggers.Value!.Select(TriggerDocument.Of)] : null, BinaryPath = entry.BinaryPath.IsPresent ? entry.BinaryPath.Value : null, BinaryFile = entry.BinaryFile.IsPresent ? entry.BinaryFile.Value : null, @@ -392,17 +413,6 @@ public static EntryDocument From(ScmEntry entry) }; } - /// - /// One trigger as the document writes it. The GUID in lower case through the "D" form, the one - /// sc qtriggerinfo prints, so a person can put the two side by side. - /// - private static TriggerDocument Trigger(ServiceTrigger trigger) => - new( - trigger.Kind.ToString(), - trigger.Action.ToString(), - trigger.Subtype?.ToString("D", CultureInfo.InvariantCulture), - [.. trigger.Data.Select(item => new TriggerDataDocument(item.Type.ToString(), item.Values))]); - /// /// Takes the whole reading rather than its pieces. The pieces used to be handed over /// one by one, which needed a fallback for a reason that cannot be missing, and a diff --git a/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs b/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs index 0eb500a7..6c9ba376 100644 --- a/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs +++ b/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs @@ -103,6 +103,47 @@ private static void KindsAndActions(JsonObject document) ]); } + /// + /// The triggers each side holds and the other does not, for the triggers field - null for any other. + /// + /// Keyed by the very text sorts, so this agrees with the decision that the + /// field differs: exact case since schema seven, and the narrowed kind and action when one side is older + /// (the two documents arrive already narrowed by ). A quadratic match, said rather + /// than hidden: the most triggers one entry holds on the machine this was written on is thirteen. + /// + private static TriggerChange? Change(string field, JsonNode? was, JsonNode? now) + { + if (field != Triggers) + { + return null; + } + + var added = Listed(now).ToList(); + var removed = new List(); + + foreach (var (key, trigger) in Listed(was)) + { + var match = added.FindIndex(other => other.Key == key); + + if (match < 0) + { + removed.Add(trigger); + } + else + { + added.RemoveAt(match); + } + } + + return new TriggerChange(removed, [.. added.Select(item => item.Trigger)]); + } + + /// The triggers of one side with the text they are compared by. A side with none holds an empty list. + private static IEnumerable<(string Key, TriggerDocument Trigger)> Listed(JsonNode? node) => + node is JsonArray items + ? items.Select(item => (Text(item) ?? string.Empty, SnapshotJson.Trigger(item))) + : []; + /// /// Fields whose value names something Windows itself compares without case. /// diff --git a/src/Bws.Core/Snapshots/SnapshotDiff.cs b/src/Bws.Core/Snapshots/SnapshotDiff.cs index 317aae4d..028c7edd 100644 --- a/src/Bws.Core/Snapshots/SnapshotDiff.cs +++ b/src/Bws.Core/Snapshots/SnapshotDiff.cs @@ -21,7 +21,29 @@ public enum DifferenceGroup /// One field that says something different in the two snapshots. /// Null means the field was absent, which is itself a difference. -public sealed record FieldDifference(string Field, DifferenceGroup Group, string? Before, string? After); +public sealed record FieldDifference(string Field, DifferenceGroup Group, string? Before, string? After) +{ + /// + /// For the triggers field, the triggers each side holds and the other does not - null for every + /// other field (backlog 625, owner's decision "a line per trigger"). + /// + /// Beside and , not instead of them: those two are the + /// whole lists as JSON, which is what --json writes and a script reads - a frozen contract. + /// This is what a person reads instead of two lines of three thousand characters, and the machine + /// document maps its fields one by one, so it does not reach it. + /// + public TriggerChange? Triggers { get; init; } +} + +/// +/// The triggers on one side of a comparison only. Counted as a multiset, not a set: two identical +/// triggers on one side and one on the other leave one behind, where a set would say nothing changed. +/// Not paired - a trigger with one value changed is one removed and one added, because a trigger has +/// no identity of its own to pair them by. +/// +/// In the earlier snapshot only, in the order it holds them. +/// In the later snapshot only, in the order it holds them. +public sealed record TriggerChange(IReadOnlyList Removed, IReadOnlyList Added); /// An entry that is in one snapshot and not the other. public sealed record EntryPresence(string ServiceName, string DisplayName); @@ -456,7 +478,7 @@ private static List Kept(IReadOnlyList entries, ou field, field == State ? DifferenceGroup.RunningState : DifferenceGroup.Configuration, left, - right)); + right) { Triggers = Change(field, was[field], now[field]) }); } } diff --git a/src/Bws.Core/Snapshots/SnapshotJson.cs b/src/Bws.Core/Snapshots/SnapshotJson.cs index 79429986..9068a445 100644 --- a/src/Bws.Core/Snapshots/SnapshotJson.cs +++ b/src/Bws.Core/Snapshots/SnapshotJson.cs @@ -239,6 +239,14 @@ internal static JsonObject Document(EntryDocument entry) return tree; } + /// + /// One trigger of a document tree back as its record, in the shape the file is read with - for the + /// comparison, which works on trees and tells a person about triggers. A trigger a tree holds as + /// nothing comes back as one of no kind, so the person is told something rather than nothing. + /// + internal static TriggerDocument Trigger(JsonNode? node) => + node?.Deserialize(Shape) ?? new TriggerDocument(string.Empty, string.Empty, null, null); + private static void Drop(JsonObject entry, string property) { entry.Remove(property); diff --git a/src/Bws.Core/Snapshots/TriggerParts.cs b/src/Bws.Core/Snapshots/TriggerParts.cs new file mode 100644 index 00000000..fd087452 --- /dev/null +++ b/src/Bws.Core/Snapshots/TriggerParts.cs @@ -0,0 +1,127 @@ +namespace Bws.Core.Snapshots; + +/// +/// The trigger subtypes the documentation of SERVICE_TRIGGER gives a name - ten of them, read off +/// learn.microsoft.com on 2026-10-27 (backlog 625). Every other subtype is a GUID nobody names: a device +/// interface class, an ETW provider, or the one every system state change trigger on this machine carries. +/// +public enum TriggerSubtypeName +{ + /// RPC_INTERFACE_EVENT_GUID, under a network endpoint trigger. + RpcInterface, + + /// NAMED_PIPE_EVENT_GUID, under a network endpoint trigger. + NamedPipe, + + /// DOMAIN_JOIN_GUID, under a domain trigger. + JoinedDomain, + + /// DOMAIN_LEAVE_GUID, under a domain trigger. + LeftDomain, + + /// FIREWALL_PORT_OPEN_GUID, under a firewall port trigger. + PortOpened, + + /// FIREWALL_PORT_CLOSE_GUID, under a firewall port trigger. + PortClosed, + + /// MACHINE_POLICY_PRESENT_GUID, under a group policy trigger. + MachinePolicy, + + /// USER_POLICY_PRESENT_GUID, under a group policy trigger. + UserPolicy, + + /// NETWORK_MANAGER_FIRST_IP_ADDRESS_ARRIVAL_GUID, under an IP address trigger. + FirstAddressArrived, + + /// NETWORK_MANAGER_LAST_IP_ADDRESS_REMOVAL_GUID, under an IP address trigger. + LastAddressRemoved +} + +/// +/// The parts of a trigger a person reads that are the same in every language - which documented subtype +/// it is, and its data as text - so the two interfaces put their own words around one answer. +/// +/// Read from , the form a snapshot holds, because a comparison has +/// nothing else: its two sides are documents, and a side older than schema seven holds the kind and the +/// action alone. A live entry goes through first, so a trigger reads the +/// same in bws show and in a comparison. +/// +public static class TriggerParts +{ + /// + /// The documented subtypes by the kind they belong to. The kind is part of the key: the + /// documentation names each GUID for one kind only, so a port GUID under a group policy trigger is a GUID + /// nobody named, and calling it "port opened" there would be a claim the documentation does not make. + /// + private static readonly Dictionary<(string Kind, Guid Subtype), TriggerSubtypeName> Documented = new() + { + [(nameof(TriggerKind.NetworkEndpoint), new Guid("bc90d167-9470-4139-a9ba-be0bbbf5b74d"))] = TriggerSubtypeName.RpcInterface, + [(nameof(TriggerKind.NetworkEndpoint), new Guid("1f81d131-3fac-4537-9e0c-7e7b0c2f4b55"))] = TriggerSubtypeName.NamedPipe, + [(nameof(TriggerKind.DomainJoin), new Guid("1ce20aba-9851-4421-9430-1ddeb766e809"))] = TriggerSubtypeName.JoinedDomain, + [(nameof(TriggerKind.DomainJoin), new Guid("ddaf516e-58c2-4866-9574-c3b615d42ea1"))] = TriggerSubtypeName.LeftDomain, + [(nameof(TriggerKind.FirewallPort), new Guid("b7569e07-8421-4ee0-ad10-86915afdad09"))] = TriggerSubtypeName.PortOpened, + [(nameof(TriggerKind.FirewallPort), new Guid("a144ed38-8e12-4de4-9d96-e64740b1a524"))] = TriggerSubtypeName.PortClosed, + [(nameof(TriggerKind.GroupPolicy), new Guid("659fcae6-5bdb-4da9-b1ff-ca2a178d46e0"))] = TriggerSubtypeName.MachinePolicy, + [(nameof(TriggerKind.GroupPolicy), new Guid("54fb46c8-f089-464c-b1fd-59d1b62c3b50"))] = TriggerSubtypeName.UserPolicy, + [(nameof(TriggerKind.IpAddress), new Guid("4f27f2de-14e2-430b-a549-7cd48cbc8245"))] = TriggerSubtypeName.FirstAddressArrived, + [(nameof(TriggerKind.IpAddress), new Guid("cc4ba62a-162e-4648-847a-b6bdf993e335"))] = TriggerSubtypeName.LastAddressRemoved + }; + + /// + /// Which documented subtype the trigger points at, or null - for a GUID the documentation does not + /// name, for a named one under another kind, and for a trigger with no subtype at all, which every + /// trigger read from a file older than schema seven is. + /// + public static TriggerSubtypeName? Subtype(TriggerDocument trigger) + { + ArgumentNullException.ThrowIfNull(trigger); + + return trigger.Subtype is { } text + && Guid.TryParse(text, out var subtype) + && Documented.TryGetValue((trigger.Kind, subtype), out var name) + ? name + : null; + } + + /// + /// One data item's values as a person reads them, without the word for its type - the interfaces add + /// "level" or "any of" around it. + /// + /// Bytes as pairs with a space between them, the way sc qtriggerinfo prints them, for + /// the binary type and for a type nobody documented - both are written as unbroken hex in the + /// document, where nobody counts them. The values of one item joined by a comma, where + /// sc uses a semicolon: the house rule keeps semicolons out of what this program writes for a + /// person, and no value on the machine this was measured on holds a comma. An empty value is two + /// quotation marks rather than nothing, so an item of one empty string does not read as a missing + /// line. + /// + public static string Values(TriggerDataDocument item) + { + ArgumentNullException.ThrowIfNull(item); + + var bytes = item.Type is nameof(TriggerDataType.Binary) or nameof(TriggerDataType.Unknown); + + return string.Join(", ", item.Values.Select(value => bytes ? Pairs(value) : Shown(value))); + } + + private static string Shown(string value) => value.Length == 0 ? "\"\"" : value; + + private static string Pairs(string hex) => hex.Length == 0 + ? Shown(hex) + : string.Join(' ', hex.Chunk(2).Select(pair => new string(pair))); + + /// + /// A data item's type as the member it names, or for a word this + /// build does not know - which a file written by a later one could hold. + /// + public static TriggerDataType TypeOf(TriggerDataDocument item) + { + ArgumentNullException.ThrowIfNull(item); + + // By the names rather than Enum.TryParse, which would take "3" or "binary" as well. + return Enum.GetNames().Contains(item.Type, StringComparer.Ordinal) + ? Enum.Parse(item.Type) + : TriggerDataType.Unknown; + } +} diff --git a/src/Bws.Gui/DetailsView.xaml b/src/Bws.Gui/DetailsView.xaml index 57761fe7..43ff0791 100644 --- a/src/Bws.Gui/DetailsView.xaml +++ b/src/Bws.Gui/DetailsView.xaml @@ -88,7 +88,8 @@ - @@ -104,7 +105,15 @@ + + diff --git a/src/Bws.Gui/Resources/gui.en.json b/src/Bws.Gui/Resources/gui.en.json index d624ef4c..605121ff 100644 --- a/src/Bws.Gui/Resources/gui.en.json +++ b/src/Bws.Gui/Resources/gui.en.json @@ -83,6 +83,7 @@ "gui.details.none": "none", "gui.details.reading": "reading...", "gui.details.section.history": "History", + "gui.details.section.triggers": "Triggers", "gui.details.notRead.network": "Not read: the file is on another machine, and this window does not reach over the network.", "gui.details.readFailed": "The window could not read some of these fields: {0} Press F5 to read the machine again, and this panel tries once more.", "gui.columns.yielded": "{0} (in the details panel while it is open)", @@ -746,6 +747,23 @@ "gui.cell.trigger.networkEndpoint": "Network endpoint", "gui.cell.trigger.custom": "Custom", "gui.cell.trigger.systemState": "System state", + "gui.trigger.head": "{0}/{1}", + "gui.trigger.withSubtype": "{0} {1}", + "gui.trigger.withData": "{0} {1}", + "gui.trigger.level": "level {0}", + "gui.trigger.keywordAny": "any of {0}", + "gui.trigger.keywordAll": "all of {0}", + "gui.trigger.unknownType": "unknown type {0}", + "gui.trigger.subtype.rpcInterface": "RPC interface", + "gui.trigger.subtype.namedPipe": "Named pipe", + "gui.trigger.subtype.joinedDomain": "Joined a domain", + "gui.trigger.subtype.leftDomain": "Left a domain", + "gui.trigger.subtype.portOpened": "Port opened", + "gui.trigger.subtype.portClosed": "Port closed", + "gui.trigger.subtype.machinePolicy": "Machine policy", + "gui.trigger.subtype.userPolicy": "User policy", + "gui.trigger.subtype.firstAddressArrived": "First address arrived", + "gui.trigger.subtype.lastAddressRemoved": "Last address removed", "gui.cell.status.running": "Running", "gui.cell.status.stopped": "Stopped", diff --git a/src/Bws.Gui/ViewModels/CellFaces.cs b/src/Bws.Gui/ViewModels/CellFaces.cs index 7eee4ca4..ace524dc 100644 --- a/src/Bws.Gui/ViewModels/CellFaces.cs +++ b/src/Bws.Gui/ViewModels/CellFaces.cs @@ -453,7 +453,7 @@ public static string MemoryLabel(Reading reading) => Say(reading, public static string TriggerList(Reading> reading) => Say(reading, triggers => Join(triggers.Select(trigger => TriggerKindLabel(trigger.Kind)).Distinct(StringComparer.Ordinal))); - private static string TriggerKindLabel(TriggerKind kind) => kind switch + internal static string TriggerKindLabel(TriggerKind kind) => kind switch { TriggerKind.DeviceArrival => Texts.Of("gui.cell.trigger.deviceArrival"), TriggerKind.IpAddress => Texts.Of("gui.cell.trigger.ipAddress"), diff --git a/src/Bws.Gui/ViewModels/Details.cs b/src/Bws.Gui/ViewModels/Details.cs index 3284f783..7d574bba 100644 --- a/src/Bws.Gui/ViewModels/Details.cs +++ b/src/Bws.Gui/ViewModels/Details.cs @@ -1,5 +1,6 @@ using Bws.Core; using Bws.Core.Querying; +using Bws.Core.Snapshots; namespace Bws.Gui.ViewModels; @@ -20,8 +21,8 @@ public sealed class DetailLine /// time (J2-c, 2026-10-20). Plain text, read in full: nothing here is a column, so nothing has a /// mark, a reading or a cell to agree with. /// - internal static DetailLine Written(string label, string value) => - new(string.Empty, value, "text", ReadOutcome.Present) { _written = label }; + internal static DetailLine Written(string label, string value, string wears = "text") => + new(string.Empty, value, wears, ReadOutcome.Present) { _written = label }; internal DetailLine( string labelKey, string value, string wears, ReadOutcome outcome, string mark = "", string? reading = null) @@ -234,9 +235,34 @@ private static IReadOnlyList Build(ScmEntry entry, bool identity, sections.Add(new DetailSection(heading, [.. columns.Select(column => Line(column, entry, reading))], note)); } - return sections; + return Triggers(entry) is { } triggers ? [.. sections, triggers] : sections; } + /// + /// Every trigger with its subtype and its data, as a section of its own after the fields - backlog + /// 625, 2026-10-27. Not inside the "Triggers" line of the advanced section, which says the column's own + /// words like every line of this panel (the summary of this class): the kinds, once each. + /// + /// One line per trigger, the kind and the action as its label, the subtype and each data item + /// under it - drawn at the full width of the panel, because a GUID and the 73 characters of the longest + /// value on the owner's machine break mid-word in the value column. Only when the entry has + /// triggers: none is the ordinary case and says so in the advanced line, and so does a reading that + /// was refused - a section here would repeat the line above it. + /// + private static DetailSection? Triggers(ScmEntry entry) => + entry.Triggers is { IsPresent: true, Value: { Count: > 0 } triggers } + ? new DetailSection( + TriggersHeading, + [.. triggers.Select(TriggerDocument.Of).Select(trigger => DetailLine.Written( + TriggerLines.KindAndAction(trigger), TriggerLines.Detail(trigger), TriggerFace))]) + : null; + + /// The heading of the triggers section. A constant for the reason gives. + private const string TriggersHeading = "gui.details.section.triggers"; + + /// How a trigger's line is drawn - see . + private const string TriggerFace = "prose"; + /// /// Whether the entry's file is on another machine, which the window does not reach for - the /// inspector skips such a file (NetworkPaths.Skip, MainViewModel) and so does the path diff --git a/src/Bws.Gui/ViewModels/DiffLines.cs b/src/Bws.Gui/ViewModels/DiffLines.cs index 131aabcd..f0aa30f2 100644 --- a/src/Bws.Gui/ViewModels/DiffLines.cs +++ b/src/Bws.Gui/ViewModels/DiffLines.cs @@ -105,12 +105,12 @@ private static IEnumerable Fields(ChangedEntry entry) var name = Printable.Of(entry.ServiceName); var shown = Printable.Of(entry.DisplayName); - foreach (var difference in entry.Differences) + foreach (var row in entry.Differences.SelectMany(Rows)) { - yield return new DiffLine(name, shown, What(difference), Value(difference.Before), Value(difference.After)) + yield return new DiffLine(name, shown, What(row.Difference), row.Before, row.After) { Kind = DiffLineKind.Changed, - Field = difference.Field + Field = row.Difference.Field }; } @@ -136,6 +136,30 @@ private static string What(FieldDifference difference) => ? Texts.Of("gui.diff.row.state", Label(difference.Field)) : Label(difference.Field); + /// + /// The before and the after of each row one difference makes: one row, or for the triggers a row per + /// trigger only one side holds, the other side the word for nothing (backlog 625, owner's decision "a + /// line per trigger", 2026-10-27) - the command line's lines as rows, so the filter, the sort and the + /// export take them without a mechanism of their own. Until then a trigger row held the two whole lists + /// as JSON, about three thousand characters for an entry losing one of thirteen. + /// + /// The two whole lists are still the row when there is no trigger to name - nothing against an + /// empty list, which a file can hold. A difference with no row under it would be silence. + /// + private static IEnumerable Rows(FieldDifference difference) + { + if (difference.Triggers is not { } change || change.Removed.Count + change.Added.Count == 0) + { + return [new ChangedRow(difference, Value(difference.Before), Value(difference.After))]; + } + + return change.Removed.Select(trigger => new ChangedRow(difference, TriggerLines.Line(trigger), Value(null))) + .Concat(change.Added.Select(trigger => new ChangedRow(difference, Value(null), TriggerLines.Line(trigger)))); + } + + /// One row a difference makes, before it is a . + private readonly record struct ChangedRow(FieldDifference Difference, string Before, string After); + /// A missing value in a word rather than a blank cell, which reads as the window running out of room. private static string Value(string? value) => value is null ? Texts.Of("gui.diff.row.nothing") : Printable.Of(value); diff --git a/src/Bws.Gui/ViewModels/TriggerLines.cs b/src/Bws.Gui/ViewModels/TriggerLines.cs new file mode 100644 index 00000000..5dee0763 --- /dev/null +++ b/src/Bws.Gui/ViewModels/TriggerLines.cs @@ -0,0 +1,91 @@ +using Bws.Core; +using Bws.Core.Snapshots; + +namespace Bws.Gui.ViewModels; + +/// +/// One trigger in the words a person reads in this window: the kind and the action, the subtype, and the +/// data (backlog 625, 2026-10-27). The command line has the same in its own TriggerWords, in its own +/// language file, out of the same - so the two can differ in words and never in +/// which subtype a GUID is or how its bytes read. +/// +/// Every piece from outside goes through here, because this window's texts do +/// not escape what they are given and the command line's do: a kind, a subtype or a value out of a snapshot +/// file is whatever the file says, and the comparison table shows two files side by side. +/// +internal static class TriggerLines +{ + /// + /// One trigger on one line, for a row of the comparison table - the kind, the action, the subtype, and the + /// data items joined by a bar, the one character no value on the machine this was measured on holds. + /// + internal static string Line(TriggerDocument trigger) + { + var items = Items(trigger).ToList(); + + return items.Count == 0 ? Head(trigger) : Texts.Of("gui.trigger.withData", Head(trigger), string.Join(" | ", items)); + } + + /// + /// The kind and the action, and the subtype after them - in words for the ten the documentation names, as + /// the GUID otherwise, and none at all for a trigger out of a file older than schema seven. + /// + internal static string Head(TriggerDocument trigger) => + trigger.Subtype is null + ? KindAndAction(trigger) + : Texts.Of("gui.trigger.withSubtype", KindAndAction(trigger), Subtype(trigger)); + + /// The kind and the action - the label of a trigger's line in the details panel. + internal static string KindAndAction(TriggerDocument trigger) => + Texts.Of("gui.trigger.head", Kind(trigger.Kind), Printable.Of(trigger.Action ?? string.Empty)); + + /// + /// What the details panel says under a trigger's kind and action: the subtype on the first line and each + /// data item on a line of its own - the command line's layout (owner's decision "B"), as one value of + /// several lines. Empty for a trigger with neither, which the panel then says in its word for nothing. + /// + internal static string Detail(TriggerDocument trigger) => string.Join( + '\n', + trigger.Subtype is null ? Items(trigger) : Items(trigger).Prepend(Subtype(trigger))); + + /// The subtype in words for the ten the documentation names, as the GUID otherwise. + private static string Subtype(TriggerDocument trigger) => + TriggerParts.Subtype(trigger) is { } name ? Subtype(name) : Printable.Of(trigger.Subtype ?? string.Empty); + + /// Each data item as one piece of text, the type's word around its values. + internal static IEnumerable Items(TriggerDocument trigger) => + (trigger.Data ?? []).Select(item => TriggerParts.TypeOf(item) switch + { + TriggerDataType.Level => Texts.Of("gui.trigger.level", Values(item)), + TriggerDataType.KeywordAny => Texts.Of("gui.trigger.keywordAny", Values(item)), + TriggerDataType.KeywordAll => Texts.Of("gui.trigger.keywordAll", Values(item)), + TriggerDataType.Unknown => Texts.Of("gui.trigger.unknownType", Values(item)), + _ => Values(item) + }); + + private static string Values(TriggerDataDocument item) => Printable.Of(TriggerParts.Values(item)); + + /// + /// A kind as a document names it, in the list's own word - and a name this build does not know left as + /// it is, since a file written by a later build may hold one. + /// + private static string Kind(string? kind) => + kind is not null && Enum.GetNames().Contains(kind, StringComparer.Ordinal) + ? CellFaces.TriggerKindLabel(Enum.Parse(kind)) + : kind is null ? Texts.Of("gui.cell.unknown") : Printable.Of(kind); + + private static string Subtype(TriggerSubtypeName name) => name switch + { + TriggerSubtypeName.RpcInterface => Texts.Of("gui.trigger.subtype.rpcInterface"), + TriggerSubtypeName.NamedPipe => Texts.Of("gui.trigger.subtype.namedPipe"), + TriggerSubtypeName.JoinedDomain => Texts.Of("gui.trigger.subtype.joinedDomain"), + TriggerSubtypeName.LeftDomain => Texts.Of("gui.trigger.subtype.leftDomain"), + TriggerSubtypeName.PortOpened => Texts.Of("gui.trigger.subtype.portOpened"), + TriggerSubtypeName.PortClosed => Texts.Of("gui.trigger.subtype.portClosed"), + TriggerSubtypeName.MachinePolicy => Texts.Of("gui.trigger.subtype.machinePolicy"), + TriggerSubtypeName.UserPolicy => Texts.Of("gui.trigger.subtype.userPolicy"), + TriggerSubtypeName.FirstAddressArrived => Texts.Of("gui.trigger.subtype.firstAddressArrived"), + TriggerSubtypeName.LastAddressRemoved => Texts.Of("gui.trigger.subtype.lastAddressRemoved"), + _ => name.ToString() + }; +} diff --git a/tests/Bws.Cli.Tests/DiffOutputTests.cs b/tests/Bws.Cli.Tests/DiffOutputTests.cs index 975a8e3b..587a0978 100644 --- a/tests/Bws.Cli.Tests/DiffOutputTests.cs +++ b/tests/Bws.Cli.Tests/DiffOutputTests.cs @@ -202,6 +202,54 @@ public void A_field_refused_to_both_snapshots_is_written_last_with_its_entries() Assert.Empty(root.GetProperty("notFullyCompared").EnumerateArray()); } + /// + /// A line per trigger only one side holds, the other side "nothing" - backlog 625, the owner's + /// decision "a line per trigger" of 2026-10-27, in the words of the preview the owner chose. A value + /// changed in one trigger is one line out and one line in. And the machine document keeps the two + /// whole lists, which is the contract a script reads. + /// + [Fact] + public void A_trigger_change_is_a_line_per_trigger_and_the_json_keeps_the_two_lists() + { + var removed = new TriggerDocument( + "FirewallPort", "Start", "b7569e07-8421-4ee0-ad10-86915afdad09", [new TriggerDataDocument("String", ["500", "UDP"])]); + var added = removed with { Data = [new TriggerDataDocument("String", ["501", "UDP"])] }; + var difference = new FieldDifference("triggers", DifferenceGroup.Configuration, "[before]", "[after]") + { + Triggers = new TriggerChange([removed], [added]) + }; + var diff = Of(changed: [new ChangedEntry("IKEEXT", "IKE and AuthIP", [difference], [])]); + + var text = DiffText.Render(diff); + + Assert.Contains(" triggers: Firewall port/Start Port opened 500, UDP -> nothing" + Environment.NewLine, text, StringComparison.Ordinal); + Assert.Contains(" triggers: nothing -> Firewall port/Start Port opened 501, UDP" + Environment.NewLine, text, StringComparison.Ordinal); + Assert.DoesNotContain("[before]", text, StringComparison.Ordinal); + + var written = Document(diff).GetProperty("changed")[0].GetProperty("differences")[0]; + + Assert.Equal("[before]", written.GetProperty("before").GetString()); + Assert.Equal("[after]", written.GetProperty("after").GetString()); + Assert.False(written.TryGetProperty("triggers", out _)); + } + + /// + /// A triggers field that differs with no trigger on either side alone - nothing against an empty list, + /// which a file can hold - still says its two values. A difference with no line under it is silence. + /// + [Fact] + public void A_trigger_difference_with_no_line_to_give_says_its_two_values() + { + var difference = new FieldDifference("triggers", DifferenceGroup.Configuration, null, "[]") + { + Triggers = new TriggerChange([], []) + }; + + var text = DiffText.Render(Of(changed: [new ChangedEntry("Spooler", "Print Spooler", [difference], [])])); + + Assert.Contains(" triggers: nothing -> []" + Environment.NewLine, text, StringComparison.Ordinal); + } + private static SnapshotDiff Of(IReadOnlyList changed, InstancesLeftOut? leftOut = null) => new( [], [], changed, [], [], [], [], diff --git a/tests/Bws.Cli.Tests/EntryReportTests.cs b/tests/Bws.Cli.Tests/EntryReportTests.cs index 074c0d3c..ce144a44 100644 --- a/tests/Bws.Cli.Tests/EntryReportTests.cs +++ b/tests/Bws.Cli.Tests/EntryReportTests.cs @@ -146,5 +146,78 @@ [new ServiceTrigger(TriggerKind.DeviceArrival, TriggerAction.Start)]) Assert.Contains("Start", report, StringComparison.Ordinal); } + /// + /// A trigger per line with its subtype, and each data item on a line under it - backlog 625, the + /// owner's decision "B - data under the trigger" of 2026-10-27. Every line after the first starts where + /// the first line's value starts, and an item sits two further in. A documented subtype is a word, any + /// other the GUID. Shapes from the owner's machine: the firewall port's one item of four values, a + /// device trigger's two items, the system state's eight bytes, and a level. + /// + [Fact] + public void Each_trigger_is_a_line_with_its_subtype_and_each_data_item_a_line_under_it() + { + var entry = Entry with + { + Triggers = Reading>.Present( + [ + new ServiceTrigger( + TriggerKind.FirewallPort, TriggerAction.Start, new Guid("b7569e07-8421-4ee0-ad10-86915afdad09"), + [new TriggerData(TriggerDataType.String, ["500", "UDP", @"%windir%\system32\svchost.exe", "IKEEXT"])]), + new ServiceTrigger( + TriggerKind.DeviceArrival, TriggerAction.Start, new Guid("4d1e55b2-f16f-11cf-88cb-001111000030"), + [new TriggerData(TriggerDataType.String, ["HID_DEVICE_UP:000D_U:0001"]), new TriggerData(TriggerDataType.String, ["HID_DEVICE_UP:000D_U:0002"])]), + new ServiceTrigger( + TriggerKind.CustomSystemStateChange, TriggerAction.Start, new Guid("2d7a2816-0c5e-45fc-9ce7-570e5ecde9c9"), + [new TriggerData(TriggerDataType.Binary, ["7518bca328009213"])]), + new ServiceTrigger( + TriggerKind.Custom, TriggerAction.Stop, new Guid("a9f076c5-7e2b-48c4-8812-77bf7a616dfe"), + [new TriggerData(TriggerDataType.Level, ["4"])]) + ]) + }; + + var lines = EntryReport.Render(entry, full: false).Split(Environment.NewLine); + var at = Array.FindIndex(lines, line => line.TrimStart().StartsWith("Triggers ", StringComparison.Ordinal)); + var column = lines[at].IndexOf("Firewall port/Start", StringComparison.Ordinal); + var block = lines.Skip(at).Take(9).ToList(); + + Assert.All(block.Skip(1), line => Assert.True(string.IsNullOrWhiteSpace(line[..column]), line)); + Assert.Equal( + [ + "Firewall port/Start Port opened", + @" 500, UDP, %windir%\system32\svchost.exe, IKEEXT", + "Device arrival/Start 4d1e55b2-f16f-11cf-88cb-001111000030", + " HID_DEVICE_UP:000D_U:0001", + " HID_DEVICE_UP:000D_U:0002", + "System state/Start 2d7a2816-0c5e-45fc-9ce7-570e5ecde9c9", + " 75 18 bc a3 28 00 92 13", + "Custom/Stop a9f076c5-7e2b-48c4-8812-77bf7a616dfe", + " level 4" + ], + block.Select(line => line[column..])); + } + + /// + /// Only the triggers are several lines - a line break inside any other value stays written out, + /// on the line it belongs to (stability report C-4). The first version of the triggers' layout split + /// every value at its line breaks, which let a description print a line looking like a field of this + /// report, and TerminalTextTests caught it - this says the same next to the layout itself. The + /// trigger's own data takes the same door: a line break in it is written out too. + /// + [Fact] + public void A_line_break_inside_a_value_stays_written_out_and_only_the_triggers_are_several_lines() + { + var entry = Entry with + { + Description = Reading.Present("First line.\r\nAccount LocalSystem"), + Triggers = Reading>.Present( + [new ServiceTrigger(TriggerKind.Custom, TriggerAction.Start, Guid.Empty, [new TriggerData(TriggerDataType.String, ["a\nb"])])]) + }; + + var report = EntryReport.Render(entry, full: false); + + Assert.Contains("First line.Account LocalSystem", report, StringComparison.Ordinal); + Assert.Contains(" ab", report, StringComparison.Ordinal); + } + private static ScmEntry Entry => Entries.Full; } diff --git a/tests/Bws.Cli.Tests/TerminalTextTests.cs b/tests/Bws.Cli.Tests/TerminalTextTests.cs index e71912b2..65a90c0a 100644 --- a/tests/Bws.Cli.Tests/TerminalTextTests.cs +++ b/tests/Bws.Cli.Tests/TerminalTextTests.cs @@ -72,7 +72,12 @@ public void The_report_of_one_entry_moves_nothing() { DisplayName = Hostile, Description = Reading.Present(Hostile), - BinaryPath = Reading.Present(@"C:\Vendor\" + Hostile) + BinaryPath = Reading.Present(@"C:\Vendor\" + Hostile), + + // Since 2026-10-27 the triggers are several lines of the report, and their data is the + // manager's - an ETW trigger's strings are whatever the service registered. + Triggers = Reading>.Present( + [new ServiceTrigger(TriggerKind.Custom, TriggerAction.Start, Guid.Empty, [new TriggerData(TriggerDataType.String, [Hostile, Hostile])])]) }; var report = EntryReport.Render(entry, full: true); @@ -86,7 +91,16 @@ public void A_comparison_of_somebody_else_s_file_moves_nothing() { var diff = new SnapshotDiff( [new EntryPresence("Aaa", Hostile)], [], - [new ChangedEntry("Bbb", Hostile, [new FieldDifference("account", DifferenceGroup.Configuration, Hostile, "LocalSystem")], [])], + [new ChangedEntry("Bbb", Hostile, + [ + new FieldDifference("account", DifferenceGroup.Configuration, Hostile, "LocalSystem"), + + // A trigger line per trigger since 2026-10-27, made of a file's strings. + new FieldDifference("triggers", DifferenceGroup.Configuration, Hostile, null) + { + Triggers = new TriggerChange([new TriggerDocument(Hostile, Hostile, Hostile, [new TriggerDataDocument(Hostile, [Hostile])])], []) + } + ], [])], [], [Hostile], [new RefusedField(Hostile, [Hostile])], [new EntryPresence("Ccc", Hostile)], new ComparisonCaveats(false, false, false, false, false, false, false, false, []), new InstancesLeftOut(0, 0), diff --git a/tests/Bws.Core.Tests/ManagerBlockTests.cs b/tests/Bws.Core.Tests/ManagerBlockTests.cs index 15987476..3ad34268 100644 --- a/tests/Bws.Core.Tests/ManagerBlockTests.cs +++ b/tests/Bws.Core.Tests/ManagerBlockTests.cs @@ -13,8 +13,10 @@ namespace Bws.Core.Tests; /// What each of these is worth is worth saying, because it is not "the manager does this". /// Nothing below claims Windows ever writes a count of five into a block with room for one. The /// claim is narrower and it is the one that matters at three in the morning: if it did, this reads -/// one record and stops. Before 2026-08-26 it read five, out of a fifty six byte array, which on an -/// elevated process is a memory fault that ends the program with no report at all. +/// nothing past the block and says the field could not be read. Before 2026-08-26 it read five, out of +/// a fifty six byte array, which on an elevated process is a memory fault that ends the program with no +/// report at all - and from then until 2026-10-27 it read the one that fitted and called the list whole +/// (backlog 626). /// public sealed class ManagerBlockTests { @@ -113,10 +115,17 @@ public void A_dependent_name_running_to_the_end_of_the_block_stops_there() } } + /// + /// Refused rather than Absent since 2026-10-27 (backlog 626) - Absent says the entry declares no + /// triggers, which is exactly what a block this short cannot say. + /// [Fact] - public void A_block_too_short_to_hold_its_own_header_is_nothing_read() + public void A_block_too_short_to_hold_its_own_header_is_refused() { - Assert.False(ManagerBlocks.ReadTriggerBuffer(new byte[4]).IsPresent); + var reading = ManagerBlocks.ReadTriggerBuffer(new byte[4]); + + Assert.Equal(ReadOutcome.Denied, reading.Outcome); + Assert.Equal(13, reading.ErrorCode); } [Fact] @@ -133,20 +142,31 @@ public void A_block_declaring_no_triggers_is_a_fact_about_the_service() [Fact] public void A_count_with_a_pointer_to_nowhere_is_refused_rather_than_followed() { - // The block says there are a thousand records and does not say where. Following that - // dereferences nothing at address zero, which is where a program ends. - var buffer = new byte[HeaderLength]; + // The block says there is one record and does not say where - with room for that record right + // behind the header, so the pointer is the only thing wrong. Following it dereferences nothing + // at address zero, which is where a program ends. + var buffer = new byte[HeaderLength + RecordLength]; + + BitConverter.TryWriteBytes(buffer.AsSpan(TriggerCount), 1u); - BitConverter.TryWriteBytes(buffer.AsSpan(TriggerCount), 1000u); + // Refused, not Absent, since 2026-10-27 (backlog 626) - until then this came back as "no + // triggers", which is a claim about the service the block never made. + var reading = ManagerBlocks.ReadTriggerBuffer(buffer); - Assert.False(ManagerBlocks.ReadTriggerBuffer(buffer).IsPresent); + Assert.Equal(ReadOutcome.Denied, reading.Outcome); + Assert.Equal(13, reading.ErrorCode); } + /// + /// Held down to what fitted from 2026-08-26 to 2026-10-27, refused since (backlog 626, owner's + /// decision "refuse everywhere"). The one record that fitted read as the entry's whole list - four + /// triggers gone with nothing to say so, the silence a data item outside the block was already + /// refused for. The record that fits is a real one, so the refusal is about the count alone. + /// [Fact] - public void A_count_larger_than_the_block_is_held_down_to_what_the_block_holds() + public void A_count_larger_than_the_block_refuses_the_field_rather_than_keeping_what_fits() { - // Room for one record and a header that claims five. The one that fits is read and the - // four that do not are not gone looking for. + // Room for one record and a header that claims five. var buffer = new byte[HeaderLength + RecordLength]; var pinned = GCHandle.Alloc(buffer, GCHandleType.Pinned); @@ -163,6 +183,12 @@ public void A_count_larger_than_the_block_is_held_down_to_what_the_block_holds() BitConverter.TryWriteBytes(buffer.AsSpan(HeaderLength), 1u); BitConverter.TryWriteBytes(buffer.AsSpan(HeaderLength + 4), 1u); + Assert.Equal(ReadOutcome.Denied, ManagerBlocks.ReadTriggerBuffer(buffer).Outcome); + + // The canary: the same block telling the truth about its one record is read, so the + // refusal above is the count and nothing else about this block. + BitConverter.TryWriteBytes(buffer.AsSpan(TriggerCount), 1u); + var reading = ManagerBlocks.ReadTriggerBuffer(buffer); Assert.True(reading.IsPresent); diff --git a/tests/Bws.Core.Tests/SnapshotGoldenTests.cs b/tests/Bws.Core.Tests/SnapshotGoldenTests.cs index 239788ac..8d51cd0e 100644 --- a/tests/Bws.Core.Tests/SnapshotGoldenTests.cs +++ b/tests/Bws.Core.Tests/SnapshotGoldenTests.cs @@ -295,7 +295,8 @@ [.. Specimens.Inspected.Where(entry => entry.ServiceName != Specimens.CaseOnlyDi /// would get, so it has to fail here rather than be skipped over quietly. The line endings /// survive a checkout because .gitattributes marks the folder as not text. /// - private static string KeptText(string name) + /// A kept copy's text. Internal since 2026-10-27 so the trigger comparison tests read the same copies. + internal static string KeptText(string name) { using var stream = typeof(SnapshotGoldenTests).Assembly.GetManifestResourceStream(name) ?? throw new InvalidOperationException($"{name} is not embedded in the test assembly."); diff --git a/tests/Bws.Core.Tests/TriggerChangeTests.cs b/tests/Bws.Core.Tests/TriggerChangeTests.cs new file mode 100644 index 00000000..5e19018a --- /dev/null +++ b/tests/Bws.Core.Tests/TriggerChangeTests.cs @@ -0,0 +1,138 @@ +using System.Text.Json.Nodes; +using Bws.Core.Snapshots; + +namespace Bws.Core.Tests; + +/// +/// Which triggers a comparison says each side holds alone (backlog 625, owner's decision "a line per +/// trigger", 2026-10-27) - read off the kept copies of the snapshot, edited in one place each. +/// +public sealed class TriggerChangeTests +{ + private const string Seven = "snapshot-schema-7.json"; + private const string Six = "snapshot-schema-6.json"; + + /// The RPC interface the manager writes in capitals on this machine, which the edits below change. + private const string Interface = "0497B57D-2E66-424F-A0C6-157CD5D41700"; + + /// + /// One value changed is one trigger gone and one come - no pairing, a trigger has no identity to pair + /// by. And the whole lists stay where they were, as JSON, because that is what --json writes. + /// + [Fact] + public void A_trigger_whose_value_changed_is_one_removed_and_one_added() + { + var before = Tree(Seven); + Data(Trigger(before, "Appinfo", 4))[0] = "0497b57d-2e66-424f-a0c6-157cd5d41700"; + + var difference = Triggers(before, Tree(Seven)); + var change = difference.Triggers!; + + Assert.Equal("0497b57d-2e66-424f-a0c6-157cd5d41700", Assert.Single(Assert.Single(change.Removed).Data!).Values[0]); + Assert.Equal(Interface, Assert.Single(Assert.Single(change.Added).Data!).Values[0]); + Assert.StartsWith("[{", difference.Before, StringComparison.Ordinal); + Assert.StartsWith("[{", difference.After, StringComparison.Ordinal); + } + + /// A trigger dropped from the later side is said once, and nothing is said to have arrived. + [Fact] + public void A_trigger_taken_away_is_removed_and_nothing_is_added() + { + var after = Tree(Seven); + Entry(after, "Appinfo")["triggers"]!.AsArray().RemoveAt(2); + + var change = Triggers(Tree(Seven), after).Triggers!; + + Assert.Equal("fd7a0523-dc70-43dd-9b2e-9c5ed48225b1", Assert.Single(Assert.Single(change.Removed).Data!).Values[0]); + Assert.Empty(change.Added); + } + + /// + /// A multiset, not a set: the same trigger twice on one side and once on the other leaves one + /// behind. A set would see the same members on both sides and leave a reported difference with no + /// line under it. + /// + [Fact] + public void A_trigger_held_twice_on_one_side_and_once_on_the_other_leaves_one() + { + var before = Tree(Seven); + var triggers = Entry(before, "Appinfo")["triggers"]!.AsArray(); + triggers.Add(triggers[0]!.DeepClone()); + + var change = Triggers(before, Tree(Seven)).Triggers!; + + Assert.Equal("201ef99a-7fa0-444c-9399-19ba84f12a1a", Assert.Single(Assert.Single(change.Removed).Data!).Values[0]); + Assert.Empty(change.Added); + } + + /// An entry with no triggers on one side has all of the other side's arriving. + [Fact] + public void Triggers_where_there_were_none_have_all_arrived() + { + var before = Tree(Seven); + Entry(before, "Appinfo")["triggers"] = null; + + var change = Triggers(before, Tree(Seven)).Triggers!; + + Assert.Empty(change.Removed); + Assert.Equal(6, change.Added.Count); + Assert.All(change.Added, trigger => Assert.Equal("bc90d167-9470-4139-a9ba-be0bbbf5b74d", trigger.Subtype)); + } + + /// + /// Against a file older than seven the lines hold what was compared - the kind and the action - and no + /// subtype or data the older side never had. + /// + [Fact] + public void Against_an_older_file_a_line_holds_the_kind_and_the_action_alone() + { + var before = Tree(Six); + Entry(before, "sppsvc")["triggers"]![1]!["kind"] = "DeviceArrival"; + + var difference = Assert.Single(Diff(before, Tree(Seven)).Changed); + var change = Assert.Single(difference.Differences).Triggers!; + + Assert.Equal(new TriggerDocument("DeviceArrival", "Start", null, null), Assert.Single(change.Removed)); + Assert.Equal(new TriggerDocument("CustomSystemStateChange", "Start", null, null), Assert.Single(change.Added)); + } + + /// Only the triggers field carries the lines - every other difference is its two values. + [Fact] + public void Only_the_triggers_field_carries_a_trigger_change() + { + var before = Tree(Seven); + Entry(before, "Appinfo")["startType"] = "Disabled"; + + var difference = Assert.Single(Assert.Single(Diff(before, Tree(Seven)).Changed).Differences); + + Assert.Equal("startType", difference.Field); + Assert.Null(difference.Triggers); + } + + private static FieldDifference Triggers(JsonNode before, JsonNode after) + { + var changed = Assert.Single(Diff(before, after).Changed); + + Assert.Equal("Appinfo", changed.ServiceName); + + return Assert.Single(changed.Differences); + } + + private static SnapshotDiff Diff(JsonNode before, JsonNode after) + { + Assert.True(SnapshotJson.TryRead(before.ToJsonString(), out var earlier, out var failure), failure); + Assert.True(SnapshotJson.TryRead(after.ToJsonString(), out var later, out failure), failure); + Assert.True(SnapshotDiff.TryBetween(earlier!, later!, out var diff, out failure), failure); + + return diff; + } + + private static JsonNode Tree(string kept) => JsonNode.Parse(SnapshotGoldenTests.KeptText(kept))!; + + private static JsonNode Entry(JsonNode tree, string name) => + tree["entries"]!.AsArray().Single(entry => (string?)entry!["serviceName"] == name)!; + + private static JsonNode Trigger(JsonNode tree, string name, int index) => Entry(tree, name)["triggers"]![index]!; + + private static JsonArray Data(JsonNode trigger) => trigger["data"]![0]!["values"]!.AsArray(); +} diff --git a/tests/Bws.Core.Tests/TriggerPartsTests.cs b/tests/Bws.Core.Tests/TriggerPartsTests.cs new file mode 100644 index 00000000..9e7d2cf3 --- /dev/null +++ b/tests/Bws.Core.Tests/TriggerPartsTests.cs @@ -0,0 +1,81 @@ +using Bws.Core.Snapshots; + +namespace Bws.Core.Tests; + +/// +/// The parts of a trigger a person reads, before either interface puts its words around them (backlog +/// 625, 2026-10-27): which documented subtype a GUID is, and a data item's values as text. +/// +public sealed class TriggerPartsTests +{ + /// + /// Every GUID the documentation of SERVICE_TRIGGER names, under the kind it names it for. Ten, + /// read off learn.microsoft.com on 2026-10-27 - backlog 625 said twelve, which was a miscount. + /// + [Theory] + [InlineData("NetworkEndpoint", "bc90d167-9470-4139-a9ba-be0bbbf5b74d", TriggerSubtypeName.RpcInterface)] + [InlineData("NetworkEndpoint", "1f81d131-3fac-4537-9e0c-7e7b0c2f4b55", TriggerSubtypeName.NamedPipe)] + [InlineData("DomainJoin", "1ce20aba-9851-4421-9430-1ddeb766e809", TriggerSubtypeName.JoinedDomain)] + [InlineData("DomainJoin", "ddaf516e-58c2-4866-9574-c3b615d42ea1", TriggerSubtypeName.LeftDomain)] + [InlineData("FirewallPort", "b7569e07-8421-4ee0-ad10-86915afdad09", TriggerSubtypeName.PortOpened)] + [InlineData("FirewallPort", "a144ed38-8e12-4de4-9d96-e64740b1a524", TriggerSubtypeName.PortClosed)] + [InlineData("GroupPolicy", "659fcae6-5bdb-4da9-b1ff-ca2a178d46e0", TriggerSubtypeName.MachinePolicy)] + [InlineData("GroupPolicy", "54fb46c8-f089-464c-b1fd-59d1b62c3b50", TriggerSubtypeName.UserPolicy)] + [InlineData("IpAddress", "4f27f2de-14e2-430b-a549-7cd48cbc8245", TriggerSubtypeName.FirstAddressArrived)] + [InlineData("IpAddress", "cc4ba62a-162e-4648-847a-b6bdf993e335", TriggerSubtypeName.LastAddressRemoved)] + public void A_documented_subtype_under_its_own_kind_has_its_name(string kind, string subtype, TriggerSubtypeName name) + { + Assert.Equal(name, TriggerParts.Subtype(new TriggerDocument(kind, "Start", subtype, []))); + + // Read as a GUID rather than as text, so the capitals the documentation prints them in name it too. + Assert.Equal(name, TriggerParts.Subtype(new TriggerDocument(kind, "Start", subtype.ToUpperInvariant(), []))); + } + + /// + /// A named GUID under another kind is a GUID nobody named there, and a subtype the documentation does + /// not list, or no subtype at all - every trigger of a file older than seven - has no name either. + /// + [Fact] + public void A_subtype_is_named_only_where_the_documentation_names_it() + { + Assert.Null(TriggerParts.Subtype(new TriggerDocument("GroupPolicy", "Start", "b7569e07-8421-4ee0-ad10-86915afdad09", []))); + Assert.Null(TriggerParts.Subtype(new TriggerDocument("CustomSystemStateChange", "Start", "2d7a2816-0c5e-45fc-9ce7-570e5ecde9c9", []))); + Assert.Null(TriggerParts.Subtype(new TriggerDocument("FirewallPort", "Start", null, null))); + Assert.Null(TriggerParts.Subtype(new TriggerDocument("FirewallPort", "Start", "not a guid", []))); + } + + /// Bytes in pairs like sc qtriggerinfo prints them, for the binary type and for a type nobody documented. + [Fact] + public void Bytes_are_read_in_pairs() + { + Assert.Equal("75 18 bc a3 28 00 92 13", TriggerParts.Values(new TriggerDataDocument("Binary", ["7518bca328009213"]))); + Assert.Equal("0a ff", TriggerParts.Values(new TriggerDataDocument("Unknown", ["0aff"]))); + } + + /// + /// The values of one item joined by a comma - the four that a firewall port trigger must all match - + /// with an empty one shown as two quotation marks rather than as nothing. + /// + [Fact] + public void The_values_of_one_item_are_joined_and_an_empty_one_is_shown() + { + Assert.Equal( + @"500, UDP, %windir%\system32\svchost.exe, IKEEXT", + TriggerParts.Values(new TriggerDataDocument("String", ["500", "UDP", @"%windir%\system32\svchost.exe", "IKEEXT"]))); + Assert.Equal("first, \"\", third", TriggerParts.Values(new TriggerDataDocument("String", ["first", "", "third"]))); + Assert.Equal("\"\"", TriggerParts.Values(new TriggerDataDocument("String", [""]))); + Assert.Equal("4", TriggerParts.Values(new TriggerDataDocument("Level", ["4"]))); + Assert.Equal("0x0000000000000001", TriggerParts.Values(new TriggerDataDocument("KeywordAny", ["0x0000000000000001"]))); + } + + /// A type word this build does not know - a later build could write one - is the unknown type, with its values left as they are. + [Fact] + public void A_type_this_build_does_not_know_is_the_unknown_type() + { + Assert.Equal(TriggerDataType.KeywordAll, TriggerParts.TypeOf(new TriggerDataDocument("KeywordAll", ["0x1"]))); + Assert.Equal(TriggerDataType.Unknown, TriggerParts.TypeOf(new TriggerDataDocument("Filter", ["x"]))); + Assert.Equal(TriggerDataType.Unknown, TriggerParts.TypeOf(new TriggerDataDocument("binary", ["00"]))); + Assert.Equal(TriggerDataType.Unknown, TriggerParts.TypeOf(new TriggerDataDocument("3", ["00"]))); + Assert.Equal("x", TriggerParts.Values(new TriggerDataDocument("Filter", ["x"]))); + } +} diff --git a/tests/Bws.Gui.Tests/DetailsGuards.cs b/tests/Bws.Gui.Tests/DetailsGuards.cs index ff5e5c99..26172018 100644 --- a/tests/Bws.Gui.Tests/DetailsGuards.cs +++ b/tests/Bws.Gui.Tests/DetailsGuards.cs @@ -174,6 +174,74 @@ public void A_file_on_another_machine_is_the_one_unread_the_panel_explains() /// One grouping decided in one place - - so somebody who learned where a /// field lives while choosing columns finds it in the same place while reading about an entry. /// + /// + /// Every trigger is a line of a section of its own, after the four sections of fields - backlog + /// 625, the owner's decisions of 2026-10-27 ("V1 - full width", "after Advanced, before History"). The + /// label is the kind and the action, the value the subtype and each data item on a line under it, and + /// the line wears prose, so the panel draws it at full width. The advanced section's own line keeps the + /// cell's words - the kinds, once each - because that line is the column. + /// + [Fact] + public void Every_trigger_is_a_line_of_its_own_section_after_the_fields_drawn_at_full_width() + { + var sections = Details.Of(Triggered()); + var triggers = sections[^1]; + + Assert.Equal(5, sections.Count); + Assert.Equal(Texts.Of("gui.details.section.triggers"), triggers.Heading); + Assert.All(triggers.Lines, line => Assert.Equal("prose", line.Wears)); + Assert.Equal( + [ + ("Firewall port/Start", "Port opened\n500, UDP"), + ("Group policy/Start", "Machine policy"), + ("Custom/Stop", "a9f076c5-7e2b-48c4-8812-77bf7a616dfe\nlevel 4") + ], + triggers.Lines.Select(line => (line.Label, line.Value))); + + var advanced = sections.Single(section => section.Heading == Texts.Of(Columns.Advanced)); + + Assert.Equal( + "Firewall port, Group policy, Custom", + advanced.Lines.Single(line => line.Label == Texts.Of("gui.column.triggers")).Value); + } + + /// + /// No section for an entry with no triggers - the ordinary case - and none for a reading that was + /// refused either: the advanced line already says "no access", and a section would repeat it. + /// + [Fact] + public void An_entry_with_no_triggers_or_a_refused_reading_has_no_triggers_section() + { + var refused = Rows.Entry("Spooler") with { Triggers = Reading>.Denied(5, "Access is denied.") }; + + Assert.DoesNotContain(Details.Of(Rows.Entry("Spooler")), section => section.Heading == Texts.Of("gui.details.section.triggers")); + Assert.DoesNotContain(Details.Of(refused), section => section.Heading == Texts.Of("gui.details.section.triggers")); + } + + /// A copy carries the section, the owner's decision "yes" of 2026-10-27 - a copy is what the panel says. + [Fact] + public void A_copy_carries_the_triggers_section() + { + var copied = Details.AsText(Triggered()); + + Assert.Contains(Texts.Of("gui.details.section.triggers") + Environment.NewLine + "Firewall port/Start\tPort opened\n500, UDP", copied, StringComparison.Ordinal); + } + + /// A firewall port with its four values cut to two, a group policy with none, and a stop trigger with a level. + private static ScmEntry Triggered() => Rows.Entry("IKEEXT") with + { + Triggers = Reading>.Present( + [ + new ServiceTrigger( + TriggerKind.FirewallPort, TriggerAction.Start, new Guid("b7569e07-8421-4ee0-ad10-86915afdad09"), + [new TriggerData(TriggerDataType.String, ["500", "UDP"])]), + new ServiceTrigger(TriggerKind.GroupPolicy, TriggerAction.Start, new Guid("659fcae6-5bdb-4da9-b1ff-ca2a178d46e0"), []), + new ServiceTrigger( + TriggerKind.Custom, TriggerAction.Stop, new Guid("a9f076c5-7e2b-48c4-8812-77bf7a616dfe"), + [new TriggerData(TriggerDataType.Level, ["4"])]) + ]) + }; + [Fact] public void Every_line_sits_under_the_heading_its_column_belongs_to() { diff --git a/tests/Bws.Gui.Tests/DetailsViewGuards.cs b/tests/Bws.Gui.Tests/DetailsViewGuards.cs index 5ca7b746..3eacda2d 100644 --- a/tests/Bws.Gui.Tests/DetailsViewGuards.cs +++ b/tests/Bws.Gui.Tests/DetailsViewGuards.cs @@ -116,4 +116,72 @@ public void The_button_and_the_key_are_the_same_way_out() WpfHost.On(window.Close); } + + /// + /// A trigger's label does not move the column every value of the panel starts at - backlog 625, + /// 2026-10-27. The label column is one width for the whole panel, and a trigger line's label + /// ("Network endpoint/Start", the longest a trigger has) took part in it until the template let a prose + /// line's label span the row: measured on the owner's machine, every value of every section moved + /// 7 device pixels on an entry with such a trigger. Asked of the drawn panel, because the view model is + /// the same in both cases and only the template decides. + /// + [Fact] + public void A_trigger_label_does_not_move_the_column_the_values_start_at() + { + var window = WpfHost.Window(); + var model = WpfHost.On(() => (MainViewModel)window.DataContext); + + WpfHost.Settled(); + + var plain = ValueStart(window, model, Rows.Entry("Spooler")); + var triggered = ValueStart(window, model, Rows.Entry("Spooler") with + { + Triggers = Core.Reading>.Present( + [new Core.ServiceTrigger(Core.TriggerKind.NetworkEndpoint, Core.TriggerAction.Start)]) + }); + + WpfHost.On(window.Close); + + Assert.Equal(plain, triggered, precision: 1); + } + + /// Where the value of the status line starts, inside the panel, with the panel shown on this entry. + private static double ValueStart(MainWindow window, MainViewModel model, Core.ScmEntry entry) + { + WpfHost.On(() => model.Chosen.Row = EntryRow.Of(entry)); + WpfHost.On(() => model.Chosen.Show()); + WpfHost.Settled(); + + return WpfHost.On(() => + { + // Laid out by hand, the way CarryOutLabelGuards does it: the host's window is never drawn, so + // nothing realizes the panel's templates otherwise - a first run found no text in it at all. + window.DetailsPanel.Measure(new Size(400, 6000)); + window.DetailsPanel.Arrange(new Rect(0, 0, 400, 6000)); + window.DetailsPanel.UpdateLayout(); + + var label = Texts.Of(Columns.Of("status")!.LabelKey); + var line = Descendants(window.DetailsPanel).OfType() + .Single(block => block.Name == "Label" && block.Text == label); + var value = Descendants((DependencyObject)line.Parent).OfType() + .Single(block => block.Name == "Value"); + + return value.TranslatePoint(default, window.DetailsPanel).X; + }); + } + + private static IEnumerable Descendants(DependencyObject root) + { + for (var index = 0; index < System.Windows.Media.VisualTreeHelper.GetChildrenCount(root); index++) + { + var child = System.Windows.Media.VisualTreeHelper.GetChild(root, index); + + yield return child; + + foreach (var below in Descendants(child)) + { + yield return below; + } + } + } } diff --git a/tests/Bws.Gui.Tests/DiffSheetTests.cs b/tests/Bws.Gui.Tests/DiffSheetTests.cs index 171fac37..1f9c25aa 100644 --- a/tests/Bws.Gui.Tests/DiffSheetTests.cs +++ b/tests/Bws.Gui.Tests/DiffSheetTests.cs @@ -78,6 +78,35 @@ public void A_field_refused_to_both_sides_is_one_sentence_above_the_table_and_no sheet.Caveats); } + /// + /// A row per trigger only one side holds, the other side the word for nothing - backlog 625, the + /// owner's decision "a line per trigger" of 2026-10-27, the command line's lines as rows. A value + /// changed in one of two triggers is one row out and one row in, and the trigger both sides hold is no + /// row at all. The changed value carries a right to left override, written out like any value from a + /// file - the window's texts do not escape what they are given, so the trigger's words must. + /// + [Fact] + public void A_trigger_change_is_a_row_per_trigger_and_a_foreign_value_in_it_is_written_out() + { + var reversed = "50" + char.ConvertFromUtf32(0x202E) + "1"; + var nothing = Texts.Of("gui.diff.row.nothing"); + + var sheet = Sheet( + [Rows.Entry("IKEEXT") with { Triggers = Reading>.Present([Port("500"), Port("4500")]) }], + [Rows.Entry("IKEEXT") with { Triggers = Reading>.Present([Port(reversed), Port("4500")]) }]); + + Assert.Equal(2, sheet.Lines.Count); + Assert.All(sheet.Lines, line => Assert.Equal(Texts.Of(Columns.Of("triggers")!.LabelKey), line.What)); + Assert.Contains(sheet.Lines, line => line.Before == "Firewall port/Start Port opened 500, UDP" && line.After == nothing); + Assert.Contains(sheet.Lines, line => line.Before == nothing && line.After == "Firewall port/Start Port opened 501, UDP"); + } + + private static ServiceTrigger Port(string number) => new( + TriggerKind.FirewallPort, + TriggerAction.Start, + new Guid("b7569e07-8421-4ee0-ad10-86915afdad09"), + [new TriggerData(TriggerDataType.String, [number, "UDP"])]); + [Fact] public void A_character_in_a_foreign_file_cannot_reorder_the_text_around_it() { diff --git a/tests/mutation/entries.ps1 b/tests/mutation/entries.ps1 index 95539d58..41cac052 100644 --- a/tests/mutation/entries.ps1 +++ b/tests/mutation/entries.ps1 @@ -5935,20 +5935,42 @@ $mutations = @( # and a block claiming nothing at address zero is where the program ends. Name = 'blocks: a trigger pointer that lands nowhere is not followed' # MOVED 2026-10-27 with the whole trigger walk, into a file of its own (logic report C01). + # RE-ANCHORED the same day (backlog 626): the pointer and the count are one check now, so the + # mutation trusts the pointer by aiming at the first record after the header instead - the + # test's block has a real record there, and only the pointer is wrong. File = 'src\Bws.Core\ManagerBlocks.Triggers.cs' - From = 'if (info.pTriggers is null || offset < 0 || offset >= buffer.Length)' - To = 'if ((info.pTriggers is null || offset < 0 || offset >= buffer.Length) && false)' + From = 'if (Within(info.pTriggers, (long)count * sizeof(SERVICE_TRIGGER), start, buffer.Length) is not { } at)' + To = 'if (Within(start + sizeof(SERVICE_TRIGGER_INFO), (long)count * sizeof(SERVICE_TRIGGER), start, buffer.Length) is not { } at)' Test = 'ManagerBlockTests.A_count_with_a_pointer_to_nowhere_is_refused_rather_than_followed' }, @{ - # The count half. Max rather than a plain removal, so that `room` is still used and the - # tree still compiles - the "mutate a value, not a proof" advice from the header. - Name = 'blocks: a trigger count is held down to what the block holds' - # MOVED 2026-10-27 with the whole trigger walk, into a file of its own (logic report C01). + # The count half, and since 2026-10-27 (backlog 626, owner's decision "refuse everywhere") it + # guards the REFUSAL: the mutation puts back the clamp to what the block holds, which was the + # rule from 2026-08-26 until that day. A plain removal of the check would walk past the block + # in the test and pass or fail on whatever memory lies there - this one stays inside it. + Name = 'blocks: a trigger count larger than the block refuses the field' + File = 'src\Bws.Core\ManagerBlocks.Triggers.cs' + From = 'var count = info.cTriggers;' + To = 'var count = Math.Min(info.cTriggers, (uint)((buffer.Length - sizeof(SERVICE_TRIGGER_INFO)) / sizeof(SERVICE_TRIGGER)));' + Test = 'ManagerBlockTests.A_count_larger_than_the_block_refuses_the_field_rather_than_keeping_what_fits' + }, + @{ + # A block too short for its own header said "no triggers" until 2026-10-27 (backlog 626). + Name = 'blocks: a trigger block too short for its header is refused' File = 'src\Bws.Core\ManagerBlocks.Triggers.cs' - From = 'var count = Math.Min(info.cTriggers, room);' - To = 'var count = Math.Max(info.cTriggers, room);' - Test = 'ManagerBlockTests.A_count_larger_than_the_block_is_held_down_to_what_the_block_holds' + From = @' + if (buffer.Length < sizeof(SERVICE_TRIGGER_INFO)) + { + return OutsideTheBlock>(); + } +'@ + To = @' + if (buffer.Length < sizeof(SERVICE_TRIGGER_INFO)) + { + return Reading>.Absent(); + } +'@ + Test = 'ManagerBlockTests.A_block_too_short_to_hold_its_own_header_is_refused' }, @{ # Sends the list fields back through the plain text comparison, which is order for order @@ -6535,9 +6557,11 @@ $mutations = @( # deliberately NOT part of this - its three members are already single words, measured # 2026-09-01, and TriggerWords carries that note. Name = 'report: the trigger kind says a word for a person rather than the name of a value' - File = 'src\Bws.Cli\EntryReport.cs' - From = ' TriggerWords.Of(trigger.Kind),' - To = ' trigger.Kind.ToString(),' + # RE-ANCHORED 2026-10-27 (backlog 625): the report reads a trigger through its document form now, + # and the word for its kind is chosen in TriggerWords. + File = 'src\Bws.Cli\TriggerWords.cs' + From = '? Of(Enum.Parse(kind))' + To = '? kind' Test = 'EntryReportTests.A_trigger_kind_of_two_words_is_printed_as_two_words_and_its_action_is_left_alone' }, @{ @@ -14866,6 +14890,97 @@ internal sealed class SizeValue(NumberOperator operation, long low, long high) : To = 'Values.SequenceEqual(other.Values, StringComparer.OrdinalIgnoreCase)' Test = 'ServiceTriggerTests.A_trigger_whose_data_differs_only_in_case_is_a_different_trigger' } + @{ + # Backlog 625: the documentation names each GUID for one kind, so the kind is part of the key. + Name = 'triggers: a documented subtype is named only under its own kind' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = '&& Documented.TryGetValue((trigger.Kind, subtype), out var name)' + To = '&& Documented.TryGetValue(Documented.Keys.FirstOrDefault(key => key.Subtype == subtype), out var name)' + Test = 'TriggerPartsTests.A_subtype_is_named_only_where_the_documentation_names_it' + } + @{ + # Backlog 625: bytes read the way sc qtriggerinfo prints them. + Name = 'triggers: bytes are read in pairs' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = ": string.Join(' ', hex.Chunk(2).Select(pair => new string(pair)));" + To = ': hex;' + Test = 'TriggerPartsTests.Bytes_are_read_in_pairs' + } + @{ + # Backlog 625: a multiset - a set would report a difference with no line under it. + Name = 'diff: the triggers on one side only are counted as a multiset' + File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' + From = 'added.RemoveAt(match);' + To = '_ = match;' + Test = 'TriggerChangeTests.A_trigger_held_twice_on_one_side_and_once_on_the_other_leaves_one' + } + @{ + # Backlog 625, and the half that matters: only the triggers - lines of this tool's own making - + # are split. A value from outside split at its line breaks can print a line that looks like a + # field (stability report C-4). The first version of the layout did exactly this. + Name = 'show: a line break inside a value from outside is never a line of the report' + File = 'src\Bws.Cli\EntryReport.cs' + From = "var parts = lines ? value.Split('\n') : [value];" + To = "var parts = value.Split('\n');" + Test = 'EntryReportTests.A_line_break_inside_a_value_stays_written_out_and_only_the_triggers_are_several_lines' + } + @{ + # Backlog 625, owner's decision "B": the data items stand on lines under the trigger. + Name = 'show: a trigger and its data items are lines under the label' + File = 'src\Bws.Cli\EntryReport.cs' + From = "var parts = lines ? value.Split('\n') : [value];" + To = 'string[] parts = [value];' + Test = 'EntryReportTests.Each_trigger_is_a_line_with_its_subtype_and_each_data_item_a_line_under_it' + } + @{ + # Backlog 625, owner's decision "D2" in the command line. + Name = 'diff text: a trigger change is a line per trigger' + File = 'src\Bws.Cli\DiffText.cs' + From = 'change.Removed.Count + change.Added.Count == 0' + To = 'change.Removed.Count + change.Added.Count >= 0' + Test = 'DiffOutputTests.A_trigger_change_is_a_line_per_trigger_and_the_json_keeps_the_two_lists' + } + @{ + # Backlog 625, owner's decision "D2" in the window. + Name = 'diff window: a trigger change is a row per trigger' + File = 'src\Bws.Gui\ViewModels\DiffLines.cs' + From = 'change.Removed.Count + change.Added.Count == 0' + To = 'change.Removed.Count + change.Added.Count >= 0' + Test = 'DiffSheetTests.A_trigger_change_is_a_row_per_trigger_and_a_foreign_value_in_it_is_written_out' + } + @{ + # Backlog 625: the window's texts do not escape, so a trigger's values from a file must be. + Name = 'diff window: a trigger value from a file is written out' + File = 'src\Bws.Gui\ViewModels\TriggerLines.cs' + From = 'private static string Values(TriggerDataDocument item) => Printable.Of(TriggerParts.Values(item));' + To = 'private static string Values(TriggerDataDocument item) => TriggerParts.Values(item);' + Test = 'DiffSheetTests.A_trigger_change_is_a_row_per_trigger_and_a_foreign_value_in_it_is_written_out' + } + @{ + # Backlog 625, owner's decision "after Advanced, before History". + Name = 'details: the triggers are a section of their own' + File = 'src\Bws.Gui\ViewModels\Details.cs' + From = 'return Triggers(entry) is { } triggers ? [.. sections, triggers] : sections;' + To = 'return Triggers(entry) is { } triggers && false ? [.. sections, triggers] : sections;' + Test = 'DetailsGuards.Every_trigger_is_a_line_of_its_own_section_after_the_fields_drawn_at_full_width' + } + @{ + # Backlog 625, owner's decision "V1 - full width". + Name = 'details: a trigger line is drawn at full width' + File = 'src\Bws.Gui\ViewModels\Details.cs' + From = 'private const string TriggerFace = "prose";' + To = 'private const string TriggerFace = "text";' + Test = 'DetailsGuards.Every_trigger_is_a_line_of_its_own_section_after_the_fields_drawn_at_full_width' + } + @{ + # Backlog 625, found by measuring the drawn panel: a prose label in the shared label column + # widened it for the whole panel, 7 device pixels on the owner's machine. + Name = 'details: a prose label does not widen the label column' + File = 'src\Bws.Gui\DetailsView.xaml' + From = '' + To = '' + Test = 'DetailsViewGuards.A_trigger_label_does_not_move_the_column_the_values_start_at' + } ) $mutations From c3b2463fd54a41532d82bb8a83bb8d80b9877d44 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 28 Oct 2026 09:04:40 +0100 Subject: [PATCH 2/2] Answer the review of trigger lines: count, quote, and never fail on a hand-edited trigger Match the triggers each side holds alone by counting copies instead of searching the other list for every trigger. The search was quadratic, and a snapshot is a file anybody can hand over: with 40000 triggers in one entry, five runs after a first one not counted, a comparison took 12.8 to 20.4 s and now takes 1.78 to 1.82 s. The copies matched are still the first ones on the later side, so the lines come out in the same order. Give no trigger lines when either side holds a trigger that is not one to say - null as a whole, or a data item without its type or its values. The comparison then shows the two whole lists, as it did before. A null trigger used to come out as a line reading "/", and a data item of nulls in a schema 6 file - which is not asked for whole data when read - ended `bws snapshot diff` with an exception and code 1. Narrowing to kind and action now keeps a null trigger null. One rule for a whole data item, TriggerDataDocument.IsWhole, serves both the schema 7 reader and the comparison. Put a trigger value in quotation marks, its own doubled, when it is empty, holds a comma, a bar or a quotation mark, or starts or ends in white space. A firewall rule's program path may hold a comma, and one such value read exactly like two, so a real change printed two identical lines. Bytes are shown in pairs only for an even run of hex digits. Every trigger line on the machine this was measured on reads as before. Join the data items of one trigger in one place, TriggerParts.OneLine, beside the rule that quotes, instead of in both interfaces. Fix two stacked doc comments in tests, re-anchor two mutation entries and add ten for the behaviours above. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 +- src/Bws.Cli/DiffText.cs | 4 +- src/Bws.Cli/TriggerWords.cs | 6 +- src/Bws.Core/Snapshots/EntryDocument.cs | 15 ++- src/Bws.Core/Snapshots/Snapshot.cs | 6 +- src/Bws.Core/Snapshots/SnapshotDiff.Values.cs | 86 +++++++++++++---- src/Bws.Core/Snapshots/SnapshotDiff.cs | 4 +- src/Bws.Core/Snapshots/SnapshotJson.cs | 11 ++- src/Bws.Core/Snapshots/TriggerParts.cs | 60 ++++++++++-- src/Bws.Gui/ViewModels/DiffLines.cs | 4 +- src/Bws.Gui/ViewModels/TriggerLines.cs | 4 +- tests/Bws.Core.Tests/SnapshotGoldenTests.cs | 3 +- tests/Bws.Core.Tests/TriggerChangeTests.cs | 69 ++++++++++++++ tests/Bws.Core.Tests/TriggerPartsTests.cs | 41 ++++++++ tests/Bws.Gui.Tests/DetailsGuards.cs | 12 +-- tests/mutation/entries.ps1 | 94 ++++++++++++++++++- 16 files changed, 369 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c92c664..012d8c61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,15 +120,18 @@ is not part of this repository. `500, UDP, %windir%\system32\svchost.exe, IKEEXT`. The ten subtypes Microsoft documents are named in words (RPC interface, named pipe, joined or left a domain, port opened or closed, machine or user policy, first IP address arrived, last one removed), any other one is shown as its GUID, and bytes - are shown in pairs the way `sc qtriggerinfo` shows them. It used to print the kinds and actions only, - all on one line. + are shown in pairs the way `sc qtriggerinfo` shows them. A value holding a comma, a `|` or a quotation + mark, or starting or ending in a space, is shown in quotation marks, so one value never reads like + two. It used to print the kinds and actions only, all on one line. - The details panel of the window has a *Triggers* section after the fields, one line per trigger with its subtype and data drawn at the full width of the panel, and a copy of the entry carries it too. The *Triggers* line among the advanced fields still names the kinds, once each. - A comparison shows a change of triggers as one line per trigger that only one side has, with "nothing" on the other side - in the terminal and as rows of the window's table. It used to show the two whole lists of triggers as JSON, which for an entry losing one of thirteen triggers was a line of - about three thousand characters. `--json` is unchanged and still carries both whole lists. + about three thousand characters. `--json` is unchanged and still carries both whole lists. A snapshot + edited by hand into a trigger this program never writes - an empty one, or data without its type or + values - still shows the two whole lists for that entry. ### Fixed diff --git a/src/Bws.Cli/DiffText.cs b/src/Bws.Cli/DiffText.cs index 53ee0702..dbbb1183 100644 --- a/src/Bws.Cli/DiffText.cs +++ b/src/Bws.Cli/DiffText.cs @@ -215,7 +215,9 @@ private static void Summary(StringBuilder text, SnapshotDiff diff) /// /// The whole lists are still the answer when there are no trigger lines to give - a field that /// differs only as "none" against an empty list, which no real reading writes but a file can hold. - /// A difference reported with nothing under it would be the silence rule 8 forbids. + /// A difference reported with nothing under it would be the silence rule 8 forbids. And when a side + /// holds a trigger that is not one to say, which the core answers with no lines at all (since the + /// review of PR #98 - a null trigger read as "/", and a data item of nulls ended the run with code 1). /// private static IEnumerable Lines(FieldDifference difference) { diff --git a/src/Bws.Cli/TriggerWords.cs b/src/Bws.Cli/TriggerWords.cs index 0d13a019..231cd93b 100644 --- a/src/Bws.Cli/TriggerWords.cs +++ b/src/Bws.Cli/TriggerWords.cs @@ -42,14 +42,14 @@ internal static IEnumerable Lines(TriggerDocument trigger) => Items(trigger).Select(item => Texts.Of("cli.show.trigger.data", item)).Prepend(Head(trigger)); /// - /// One trigger on one line, for a comparison, which says "before -> after" per line - the items joined - /// by a bar, the one character no value on the machine this was measured on holds. + /// One trigger on one line, for a comparison, which says "before -> after" per line - the items on one + /// line as joins them, the same as the window. /// internal static string Line(TriggerDocument trigger) { var items = Items(trigger).ToList(); - return items.Count == 0 ? Head(trigger) : Texts.Of("cli.trigger.withData", Head(trigger), string.Join(" | ", items)); + return items.Count == 0 ? Head(trigger) : Texts.Of("cli.trigger.withData", Head(trigger), TriggerParts.OneLine(items)); } /// diff --git a/src/Bws.Core/Snapshots/EntryDocument.cs b/src/Bws.Core/Snapshots/EntryDocument.cs index 87461b37..6b527b29 100644 --- a/src/Bws.Core/Snapshots/EntryDocument.cs +++ b/src/Bws.Core/Snapshots/EntryDocument.cs @@ -52,7 +52,20 @@ public static TriggerDocument Of(ServiceTrigger trigger) /// One data item of a trigger: its type in our words and its values as text - see /// for which form each type is written in and why. /// -public sealed record TriggerDataDocument(string Type, IReadOnlyList Values); +public sealed record TriggerDataDocument(string Type, IReadOnlyList Values) +{ + /// + /// An item as every build writes it: present, with a type and at least one value, none of them missing. + /// + /// One answer for two questions, since the review of PR #98: whether a schema seven file holds + /// what its schema promises (Snapshot.BrokenEntries refuses it otherwise), and whether the + /// comparison can say a trigger as a line at all, in a file of any schema (SnapshotDiff.Change + /// shows the whole lists otherwise). The reader fills a missing type or list with null whatever the + /// record declares, which is why both are asked. + /// + internal static bool IsWhole(TriggerDataDocument? item) => + item is { Type: not null, Values.Count: > 0 } && item.Values.All(value => value is not null); +} /// /// What the system concluded about a file's signature. diff --git a/src/Bws.Core/Snapshots/Snapshot.cs b/src/Bws.Core/Snapshots/Snapshot.cs index 00fa0ef2..d0ac612b 100644 --- a/src/Bws.Core/Snapshots/Snapshot.cs +++ b/src/Bws.Core/Snapshots/Snapshot.cs @@ -453,8 +453,6 @@ internal bool BrokenEntries(out string? failure) : $"has trigger {gap + 1} without its whole data list, which a version {schema} snapshot always holds."; } - /// A data list, and every item in it with a type and at least one value, none of them missing. - private static bool Whole(TriggerDocument trigger) => - trigger.Data is { } data - && data.All(item => item is { Type: not null, Values.Count: > 0 } && item.Values.All(value => value is not null)); + /// A data list, and every item in it whole - see . + private static bool Whole(TriggerDocument trigger) => trigger.Data is { } data && data.All(TriggerDataDocument.IsWhole); } diff --git a/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs b/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs index 6c9ba376..a911ff97 100644 --- a/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs +++ b/src/Bws.Core/Snapshots/SnapshotDiff.Values.cs @@ -84,7 +84,9 @@ private static (JsonObject Was, JsonObject Now) Documents(EntryDocument before, /// /// Narrows every trigger in a document to its kind and action - what a file older than schema seven /// holds - so the two sides of compare the same - /// question. A document without triggers is left as it is. + /// question. A document without triggers is left as it is, and a trigger held as nothing stays nothing + /// rather than becoming one of no kind and no action (the review of PR #98), so + /// sees it for what it is. /// private static void KindsAndActions(JsonObject document) { @@ -95,11 +97,13 @@ private static void KindsAndActions(JsonObject document) document[Triggers] = new JsonArray( [ - .. triggers.Select(trigger => new JsonObject - { - ["kind"] = trigger?["kind"]?.DeepClone(), - ["action"] = trigger?["action"]?.DeepClone() - }) + .. triggers.Select(trigger => trigger is null + ? null + : new JsonObject + { + ["kind"] = trigger["kind"]?.DeepClone(), + ["action"] = trigger["action"]?.DeepClone() + }) ]); } @@ -108,8 +112,23 @@ private static void KindsAndActions(JsonObject document) /// /// Keyed by the very text sorts, so this agrees with the decision that the /// field differs: exact case since schema seven, and the narrowed kind and action when one side is older - /// (the two documents arrive already narrowed by ). A quadratic match, said rather - /// than hidden: the most triggers one entry holds on the machine this was written on is thirteen. + /// (the two documents arrive already narrowed by ). + /// + /// Counted rather than searched, since the review of PR #98. The first version looked for every + /// earlier trigger in the later list and took it out, which is quadratic - harmless at the thirteen one + /// entry holds on the machine this was written on, and a snapshot is a file anybody can hand over. + /// Measured with tools/snapshot-probe/trigger-shapes.ps1, forty thousand triggers in one entry, + /// five runs after a first one not counted: 12.8 to 20.4 s searching, 1.78 to 1.82 s counting, and 1.33 + /// to 1.56 s for the build before this list existed, which compared two texts and named no trigger. The + /// copies of one trigger that are matched are still its first ones on the later side, so the lines come + /// out as they did. + /// + /// Null as well when either side holds a trigger that is not one to say - null as a whole, or a + /// data item without its type or its values, which no build writes and a file older than schema seven is + /// not refused for ( asks it of a seven). Each interface then shows the two whole + /// lists, as it did before backlog 625, rather than a line it would have to make up: until that review a + /// null trigger came out as a line reading "/", and a data item of nulls ended the comparison with an + /// exception and code 1. /// private static TriggerChange? Change(string field, JsonNode? was, JsonNode? now) { @@ -118,32 +137,65 @@ private static void KindsAndActions(JsonObject document) return null; } - var added = Listed(now).ToList(); + var earlier = Listed(was).ToList(); + var later = Listed(now).ToList(); + + if (earlier.Concat(later).Any(item => item.Trigger is not { } trigger || !Sayable(trigger))) + { + return null; + } + + var copies = new Dictionary(StringComparer.Ordinal); + + foreach (var (key, _) in later) + { + copies[key] = copies.GetValueOrDefault(key) + 1; + } + + var matched = new Dictionary(StringComparer.Ordinal); var removed = new List(); - foreach (var (key, trigger) in Listed(was)) + foreach (var (key, trigger) in earlier) { - var match = added.FindIndex(other => other.Key == key); + if (matched.GetValueOrDefault(key) < copies.GetValueOrDefault(key)) + { + matched[key] = matched.GetValueOrDefault(key) + 1; + } + else + { + removed.Add(trigger!); + } + } - if (match < 0) + var added = new List(); + + foreach (var (key, trigger) in later) + { + if (matched.GetValueOrDefault(key) > 0) { - removed.Add(trigger); + matched[key]--; } else { - added.RemoveAt(match); + added.Add(trigger!); } } - return new TriggerChange(removed, [.. added.Select(item => item.Trigger)]); + return new TriggerChange(removed, added); } - /// The triggers of one side with the text they are compared by. A side with none holds an empty list. - private static IEnumerable<(string Key, TriggerDocument Trigger)> Listed(JsonNode? node) => + /// + /// The triggers of one side with the text they are compared by - a trigger the side holds as nothing + /// comes with none. A side with no list holds an empty one. + /// + private static IEnumerable<(string Key, TriggerDocument? Trigger)> Listed(JsonNode? node) => node is JsonArray items ? items.Select(item => (Text(item) ?? string.Empty, SnapshotJson.Trigger(item))) : []; + /// A trigger whose data, where it has any, is whole - see . + private static bool Sayable(TriggerDocument trigger) => trigger.Data?.All(TriggerDataDocument.IsWhole) ?? true; + /// /// Fields whose value names something Windows itself compares without case. /// diff --git a/src/Bws.Core/Snapshots/SnapshotDiff.cs b/src/Bws.Core/Snapshots/SnapshotDiff.cs index 028c7edd..59618706 100644 --- a/src/Bws.Core/Snapshots/SnapshotDiff.cs +++ b/src/Bws.Core/Snapshots/SnapshotDiff.cs @@ -25,7 +25,9 @@ public sealed record FieldDifference(string Field, DifferenceGroup Group, string { /// /// For the triggers field, the triggers each side holds and the other does not - null for every - /// other field (backlog 625, owner's decision "a line per trigger"). + /// other field (backlog 625, owner's decision "a line per trigger"). Null for the triggers field too + /// when either side holds a trigger that is not one to say - null as a whole, or with a data item + /// missing its type or values - and a person is then shown the two whole lists. /// /// Beside and , not instead of them: those two are the /// whole lists as JSON, which is what --json writes and a script reads - a frozen contract. diff --git a/src/Bws.Core/Snapshots/SnapshotJson.cs b/src/Bws.Core/Snapshots/SnapshotJson.cs index 9068a445..cdef121f 100644 --- a/src/Bws.Core/Snapshots/SnapshotJson.cs +++ b/src/Bws.Core/Snapshots/SnapshotJson.cs @@ -241,11 +241,14 @@ internal static JsonObject Document(EntryDocument entry) /// /// One trigger of a document tree back as its record, in the shape the file is read with - for the - /// comparison, which works on trees and tells a person about triggers. A trigger a tree holds as - /// nothing comes back as one of no kind, so the person is told something rather than nothing. + /// comparison, which works on trees and tells a person about triggers. + /// + /// A trigger the tree holds as nothing comes back as nothing, since the review of PR #98. Until + /// then it came back as one of no kind and no action, "so the person is told something" - and what the + /// person was told was a line reading "/", a trigger nobody wrote. The comparison now shows that field + /// as its two whole lists instead (SnapshotDiff.Change). /// - internal static TriggerDocument Trigger(JsonNode? node) => - node?.Deserialize(Shape) ?? new TriggerDocument(string.Empty, string.Empty, null, null); + internal static TriggerDocument? Trigger(JsonNode? node) => node?.Deserialize(Shape); private static void Drop(JsonObject entry, string property) { diff --git a/src/Bws.Core/Snapshots/TriggerParts.cs b/src/Bws.Core/Snapshots/TriggerParts.cs index fd087452..94be6669 100644 --- a/src/Bws.Core/Snapshots/TriggerParts.cs +++ b/src/Bws.Core/Snapshots/TriggerParts.cs @@ -1,3 +1,5 @@ +using System.Buffers; + namespace Bws.Core.Snapshots; /// @@ -68,6 +70,19 @@ public static class TriggerParts [(nameof(TriggerKind.IpAddress), new Guid("cc4ba62a-162e-4648-847a-b6bdf993e335"))] = TriggerSubtypeName.LastAddressRemoved }; + /// Between the values of one data item - see . + private const string ValueSeparator = ", "; + + /// Between the data items of one trigger on one line - see . + private const string ItemSeparator = " | "; + + /// + /// What puts a value in quotation marks: the two separators above, and the quotation mark itself. + /// Written out rather than built from the two constants, so a separator changed without this + /// changing with it stops the tests that pin a value holding each of them. + /// + private static readonly SearchValues Mistakable = SearchValues.Create([',', '|', '"']); + /// /// Which documented subtype the trigger points at, or null - for a GUID the documentation does not /// name, for a named one under another kind, and for a trigger with no subtype at all, which every @@ -90,11 +105,18 @@ public static class TriggerParts /// /// Bytes as pairs with a space between them, the way sc qtriggerinfo prints them, for /// the binary type and for a type nobody documented - both are written as unbroken hex in the - /// document, where nobody counts them. The values of one item joined by a comma, where - /// sc uses a semicolon: the house rule keeps semicolons out of what this program writes for a - /// person, and no value on the machine this was measured on holds a comma. An empty value is two - /// quotation marks rather than nothing, so an item of one empty string does not read as a missing - /// line. + /// document, where nobody counts them. A value of either that is not an even run of hex digits, which + /// only a file written by hand holds, is shown as text instead, so no pair is cut out of something that + /// is not a byte. The values of one item joined by a comma, where sc uses a semicolon: the + /// house rule keeps semicolons out of what this program writes for a person. + /// + /// A value that could be read as something else is in quotation marks, its own doubled - empty, + /// holding a comma, the bar joins items with or a quotation mark, or starting or + /// ending in white space. Since the review of PR #98: until then only the empty one was, on the argument + /// that no value on the machine this was measured on holds a comma. A firewall rule's program path may, + /// and C:\a, b.exe read exactly like the two values C:\a and b.exe - so a comparison + /// printed two identical lines for a change that was real (tools/snapshot-probe/trigger-shapes.ps1, + /// case comma-in-a-value). /// public static string Values(TriggerDataDocument item) { @@ -102,14 +124,32 @@ public static string Values(TriggerDataDocument item) var bytes = item.Type is nameof(TriggerDataType.Binary) or nameof(TriggerDataType.Unknown); - return string.Join(", ", item.Values.Select(value => bytes ? Pairs(value) : Shown(value))); + return string.Join(ValueSeparator, item.Values.Select(value => bytes && IsHex(value) ? Pairs(value) : Shown(value))); } - private static string Shown(string value) => value.Length == 0 ? "\"\"" : value; + /// + /// The data items of one trigger on one line, for a comparison, which says "before -> after" per line. + /// + /// Here, beside the rule that quotes, rather than in the two language files - the review of PR + /// #98 asked for those, and until it the bar was written in both interfaces. The bar and the comma are + /// safe only because quotes every value holding one, and a translation choosing + /// another separator would bring back, silently, two lines that read the same for two triggers. + /// + public static string OneLine(IEnumerable items) => string.Join(ItemSeparator, items); + + private static string Shown(string value) => NeedsQuotes(value) + ? "\"" + value.Replace("\"", "\"\"", StringComparison.Ordinal) + "\"" + : value; + + private static bool NeedsQuotes(string value) => + value.Length == 0 + || value.AsSpan().ContainsAny(Mistakable) + || char.IsWhiteSpace(value[0]) + || char.IsWhiteSpace(value[^1]); + + private static bool IsHex(string value) => value.Length > 0 && value.Length % 2 == 0 && value.All(char.IsAsciiHexDigit); - private static string Pairs(string hex) => hex.Length == 0 - ? Shown(hex) - : string.Join(' ', hex.Chunk(2).Select(pair => new string(pair))); + private static string Pairs(string hex) => string.Join(' ', hex.Chunk(2).Select(pair => new string(pair))); /// /// A data item's type as the member it names, or for a word this diff --git a/src/Bws.Gui/ViewModels/DiffLines.cs b/src/Bws.Gui/ViewModels/DiffLines.cs index f0aa30f2..27e4998b 100644 --- a/src/Bws.Gui/ViewModels/DiffLines.cs +++ b/src/Bws.Gui/ViewModels/DiffLines.cs @@ -144,7 +144,9 @@ private static string What(FieldDifference difference) => /// as JSON, about three thousand characters for an entry losing one of thirteen. /// /// The two whole lists are still the row when there is no trigger to name - nothing against an - /// empty list, which a file can hold. A difference with no row under it would be silence. + /// empty list, which a file can hold. A difference with no row under it would be silence. And when a + /// side holds a trigger that is not one to say, which the core answers with no rows at all (since the + /// review of PR #98). /// private static IEnumerable Rows(FieldDifference difference) { diff --git a/src/Bws.Gui/ViewModels/TriggerLines.cs b/src/Bws.Gui/ViewModels/TriggerLines.cs index 5dee0763..9ddabdfd 100644 --- a/src/Bws.Gui/ViewModels/TriggerLines.cs +++ b/src/Bws.Gui/ViewModels/TriggerLines.cs @@ -17,13 +17,13 @@ internal static class TriggerLines { /// /// One trigger on one line, for a row of the comparison table - the kind, the action, the subtype, and the - /// data items joined by a bar, the one character no value on the machine this was measured on holds. + /// data items on one line as joins them, the same as the command line. /// internal static string Line(TriggerDocument trigger) { var items = Items(trigger).ToList(); - return items.Count == 0 ? Head(trigger) : Texts.Of("gui.trigger.withData", Head(trigger), string.Join(" | ", items)); + return items.Count == 0 ? Head(trigger) : Texts.Of("gui.trigger.withData", Head(trigger), TriggerParts.OneLine(items)); } /// diff --git a/tests/Bws.Core.Tests/SnapshotGoldenTests.cs b/tests/Bws.Core.Tests/SnapshotGoldenTests.cs index 8d51cd0e..25263040 100644 --- a/tests/Bws.Core.Tests/SnapshotGoldenTests.cs +++ b/tests/Bws.Core.Tests/SnapshotGoldenTests.cs @@ -294,8 +294,9 @@ [.. Specimens.Inspected.Where(entry => entry.ServiceName != Specimens.CaseOnlyDi /// order mark: a mark that crept into the kept copy is a difference in the file a person /// would get, so it has to fail here rather than be skipped over quietly. The line endings /// survive a checkout because .gitattributes marks the folder as not text. + /// + /// Internal since 2026-10-27, so the trigger comparison tests read the same copies. /// - /// A kept copy's text. Internal since 2026-10-27 so the trigger comparison tests read the same copies. internal static string KeptText(string name) { using var stream = typeof(SnapshotGoldenTests).Assembly.GetManifestResourceStream(name) diff --git a/tests/Bws.Core.Tests/TriggerChangeTests.cs b/tests/Bws.Core.Tests/TriggerChangeTests.cs index 5e19018a..1f197bfe 100644 --- a/tests/Bws.Core.Tests/TriggerChangeTests.cs +++ b/tests/Bws.Core.Tests/TriggerChangeTests.cs @@ -65,6 +65,75 @@ public void A_trigger_held_twice_on_one_side_and_once_on_the_other_leaves_one() Assert.Empty(change.Added); } + /// + /// The copies matched are the first ones on the later side, so what is left over keeps its place - + /// the order the lines came out in while the match searched the list, kept when it started counting (the + /// review of PR #98). + /// + [Fact] + public void The_copies_matched_are_the_first_so_what_arrived_keeps_its_order() + { + var before = Tree(Seven); + var held = Entry(before, "Appinfo")["triggers"]!.AsArray(); + var first = held[0]!.DeepClone(); + var second = held[1]!.DeepClone(); + Entry(before, "Appinfo")["triggers"] = new JsonArray(first.DeepClone()); + + var after = Tree(Seven); + Entry(after, "Appinfo")["triggers"] = new JsonArray(first.DeepClone(), second, first.DeepClone()); + + var change = Triggers(before, after).Triggers!; + + Assert.Empty(change.Removed); + Assert.Equal( + ["5f54ce7d-5b79-4175-8584-cb65313a0e98", "201ef99a-7fa0-444c-9399-19ba84f12a1a"], + change.Added.Select(trigger => trigger.Data![0].Values[0])); + } + + /// + /// A trigger held as nothing gives no lines, and the person is shown the two whole lists instead - + /// the reader lets it through in every schema, and until the review of PR #98 it came out as a line + /// reading "/". The same against an older file, where the narrowing to kind and action once turned the + /// nothing into a trigger of no kind. + /// + [Theory] + [InlineData(Seven)] + [InlineData(Six)] + public void A_trigger_held_as_nothing_gives_no_lines_and_the_whole_lists_stay(string earlier) + { + var after = Tree(Seven); + Entry(after, "sppsvc")["triggers"]!.AsArray().Insert(0, null); + + var difference = Assert.Single(Assert.Single(Diff(Tree(earlier), after).Changed).Differences); + + Assert.Equal("triggers", difference.Field); + Assert.Null(difference.Triggers); + Assert.StartsWith("[null,", difference.After, StringComparison.Ordinal); + } + + /// + /// A data item that is not whole gives no lines either - a file older than seven is not refused + /// for one, because that schema never held data, and each of these ended the comparison with an + /// exception and code 1 before the review of PR #98 (tools/snapshot-probe/trigger-shapes.ps1). + /// + [Theory] + [InlineData("[null]")] + [InlineData("""[{"type":"String","values":null}]""")] + [InlineData("""[{"type":"String","values":["a",null]}]""")] + [InlineData("""[{"type":null,"values":["a"]}]""")] + [InlineData("""[{"type":"String","values":[]}]""")] + public void An_older_file_whose_data_item_is_not_whole_gives_no_lines(string data) + { + var after = Tree(Six); + Entry(after, "sppsvc")["triggers"]![1]!["data"] = JsonNode.Parse(data); + + var difference = Assert.Single(Assert.Single(Diff(Tree(Six), after).Changed).Differences); + + Assert.Equal("triggers", difference.Field); + Assert.Null(difference.Triggers); + Assert.Contains(data, difference.After, StringComparison.Ordinal); + } + /// An entry with no triggers on one side has all of the other side's arriving. [Fact] public void Triggers_where_there_were_none_have_all_arrived() diff --git a/tests/Bws.Core.Tests/TriggerPartsTests.cs b/tests/Bws.Core.Tests/TriggerPartsTests.cs index 9e7d2cf3..cf07e4cd 100644 --- a/tests/Bws.Core.Tests/TriggerPartsTests.cs +++ b/tests/Bws.Core.Tests/TriggerPartsTests.cs @@ -68,6 +68,47 @@ public void The_values_of_one_item_are_joined_and_an_empty_one_is_shown() Assert.Equal("0x0000000000000001", TriggerParts.Values(new TriggerDataDocument("KeywordAny", ["0x0000000000000001"]))); } + /// + /// A value that could be read as something else is in quotation marks (the review of PR #98): a + /// comma, the bar items are joined with, a quotation mark - doubled inside - or white space at either end. + /// One firewall path holding a comma no longer reads like two values, and a value holding the bar no + /// longer reads like two items. + /// + [Fact] + public void A_value_that_could_be_read_as_something_else_is_quoted() + { + var one = TriggerParts.Values(new TriggerDataDocument("String", [@"C:\a, b.exe"])); + var two = TriggerParts.Values(new TriggerDataDocument("String", [@"C:\a", "b.exe"])); + + Assert.Equal(@"""C:\a, b.exe""", one); + Assert.Equal(@"C:\a, b.exe", two); + Assert.Equal("\"x | y\"", TriggerParts.Values(new TriggerDataDocument("String", ["x | y"]))); + Assert.NotEqual( + TriggerParts.OneLine([TriggerParts.Values(new TriggerDataDocument("String", ["x | y"]))]), + TriggerParts.OneLine(["x", "y"])); + Assert.Equal("\"say \"\"hi\"\"\"", TriggerParts.Values(new TriggerDataDocument("String", ["say \"hi\""]))); + Assert.Equal("\" lead\", \"trail \"", TriggerParts.Values(new TriggerDataDocument("String", [" lead", "trail "]))); + Assert.Equal("in the middle", TriggerParts.Values(new TriggerDataDocument("String", ["in the middle"]))); + } + + /// + /// Bytes are pairs only where they are bytes - an even run of hex digits. Anything else a hand-written file + /// puts under the binary type is text, quoted by the same rule, so no pair is cut out of a comma. + /// + [Fact] + public void A_binary_value_that_is_not_bytes_is_shown_as_text() + { + Assert.Equal("abc", TriggerParts.Values(new TriggerDataDocument("Binary", ["abc"]))); + Assert.Equal("not hex", TriggerParts.Values(new TriggerDataDocument("Binary", ["not hex"]))); + Assert.Equal("\"ab,cd\"", TriggerParts.Values(new TriggerDataDocument("Binary", ["ab,cd"]))); + Assert.Equal("\"\"", TriggerParts.Values(new TriggerDataDocument("Binary", [""]))); + } + + /// The data items of one trigger on one line, joined by the bar, for a comparison. + [Fact] + public void The_items_of_one_trigger_are_joined_by_a_bar() => + Assert.Equal("level 4 | any of 0x1", TriggerParts.OneLine(["level 4", "any of 0x1"])); + /// A type word this build does not know - a later build could write one - is the unknown type, with its values left as they are. [Fact] public void A_type_this_build_does_not_know_is_the_unknown_type() diff --git a/tests/Bws.Gui.Tests/DetailsGuards.cs b/tests/Bws.Gui.Tests/DetailsGuards.cs index 26172018..082dbabb 100644 --- a/tests/Bws.Gui.Tests/DetailsGuards.cs +++ b/tests/Bws.Gui.Tests/DetailsGuards.cs @@ -168,12 +168,6 @@ public void A_file_on_another_machine_is_the_one_unread_the_panel_explains() Assert.Equal(Texts.Of("gui.details.notRead.network"), noted.Note); } - /// - /// Every line sits under the heading its own column belongs to in the picker. - /// - /// One grouping decided in one place - - so somebody who learned where a - /// field lives while choosing columns finds it in the same place while reading about an entry. - /// /// /// Every trigger is a line of a section of its own, after the four sections of fields - backlog /// 625, the owner's decisions of 2026-10-27 ("V1 - full width", "after Advanced, before History"). The @@ -242,6 +236,12 @@ [new TriggerData(TriggerDataType.Level, ["4"])]) ]) }; + /// + /// Every line sits under the heading its own column belongs to in the picker. + /// + /// One grouping decided in one place - - so somebody who learned where a + /// field lives while choosing columns finds it in the same place while reading about an entry. + /// [Fact] public void Every_line_sits_under_the_heading_its_column_belongs_to() { diff --git a/tests/mutation/entries.ps1 b/tests/mutation/entries.ps1 index 41cac052..fbd39853 100644 --- a/tests/mutation/entries.ps1 +++ b/tests/mutation/entries.ps1 @@ -14902,16 +14902,17 @@ internal sealed class SizeValue(NumberOperator operation, long low, long high) : # Backlog 625: bytes read the way sc qtriggerinfo prints them. Name = 'triggers: bytes are read in pairs' File = 'src\Bws.Core\Snapshots\TriggerParts.cs' - From = ": string.Join(' ', hex.Chunk(2).Select(pair => new string(pair)));" - To = ': hex;' + From = "private static string Pairs(string hex) => string.Join(' ', hex.Chunk(2).Select(pair => new string(pair)));" + To = 'private static string Pairs(string hex) => hex;' Test = 'TriggerPartsTests.Bytes_are_read_in_pairs' } @{ - # Backlog 625: a multiset - a set would report a difference with no line under it. + # Backlog 625: a multiset - a set would report a difference with no line under it. Re-anchored in + # the review of PR #98, when the search became a count: a set is "any copy matches". Name = 'diff: the triggers on one side only are counted as a multiset' File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' - From = 'added.RemoveAt(match);' - To = '_ = match;' + From = 'if (matched.GetValueOrDefault(key) < copies.GetValueOrDefault(key))' + To = 'if (copies.ContainsKey(key))' Test = 'TriggerChangeTests.A_trigger_held_twice_on_one_side_and_once_on_the_other_leaves_one' } @{ @@ -14981,6 +14982,89 @@ internal sealed class SizeValue(NumberOperator operation, long low, long high) : To = '' Test = 'DetailsViewGuards.A_trigger_label_does_not_move_the_column_the_values_start_at' } + @{ + # Review of PR #98: counting replaced a quadratic search, and the lines must come out in the order + # the search gave - the first copies on the later side matched, the rest in their places. + Name = 'diff: what arrived keeps the order of the later side' + File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' + From = 'added.Add(trigger!);' + To = 'added.Insert(0, trigger!);' + Test = 'TriggerChangeTests.The_copies_matched_are_the_first_so_what_arrived_keeps_its_order' + } + @{ + # Review of PR #98: a trigger held as nothing came out as a line reading "/". + Name = 'diff: a trigger held as nothing gives no lines' + File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' + From = 'if (earlier.Concat(later).Any(item => item.Trigger is not { } trigger || !Sayable(trigger)))' + To = 'if (earlier.Concat(later).Any(item => item.Trigger is { } trigger && !Sayable(trigger)))' + Test = 'TriggerChangeTests.A_trigger_held_as_nothing_gives_no_lines_and_the_whole_lists_stay' + } + @{ + # Review of PR #98: a data item without its type or values, in a file older than seven, ended the + # comparison with an exception and code 1. Weakened to "present", which is the half-check. + Name = 'diff: a data item that is not whole gives no lines' + File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' + From = 'private static bool Sayable(TriggerDocument trigger) => trigger.Data?.All(TriggerDataDocument.IsWhole) ?? true;' + To = 'private static bool Sayable(TriggerDocument trigger) => trigger.Data?.All(item => item is not null) ?? true;' + Test = 'TriggerChangeTests.An_older_file_whose_data_item_is_not_whole_gives_no_lines' + } + @{ + # Review of PR #98: narrowing to kind and action turned a trigger held as nothing into one of no + # kind, which the comparison then said as a line. The mutation is that old behaviour. + Name = 'diff: narrowing keeps a trigger held as nothing' + File = 'src\Bws.Core\Snapshots\SnapshotDiff.Values.cs' + From = '? null' + To = '? new JsonObject { ["kind"] = null, ["action"] = null }' + Test = 'TriggerChangeTests.A_trigger_held_as_nothing_gives_no_lines_and_the_whole_lists_stay' + } + @{ + # Review of PR #98: a value holding a comma read like two values, and one holding the bar like two items. + Name = 'triggers: a value holding a separator is quoted' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = '|| value.AsSpan().ContainsAny(Mistakable)' + To = '|| value.AsSpan().ContainsAny(Mistakable) && false' + Test = 'TriggerPartsTests.A_value_that_could_be_read_as_something_else_is_quoted' + } + @{ + # Review of PR #98: white space at the start of a value is invisible on a line. + Name = 'triggers: a value starting with white space is quoted' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = '|| char.IsWhiteSpace(value[0])' + To = '|| char.IsWhiteSpace(value[0]) && false' + Test = 'TriggerPartsTests.A_value_that_could_be_read_as_something_else_is_quoted' + } + @{ + # Review of PR #98: and at the end of one. + Name = 'triggers: a value ending in white space is quoted' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = '|| char.IsWhiteSpace(value[^1]);' + To = '|| char.IsWhiteSpace(value[^1]) && false;' + Test = 'TriggerPartsTests.A_value_that_could_be_read_as_something_else_is_quoted' + } + @{ + # Review of PR #98: a quotation mark inside a quoted value is doubled, or the quoting is ambiguous itself. + Name = 'triggers: a quotation mark inside a value is doubled' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = 'value.Replace("\"", "\"\"", StringComparison.Ordinal)' + To = 'value' + Test = 'TriggerPartsTests.A_value_that_could_be_read_as_something_else_is_quoted' + } + @{ + # Review of PR #98: pairs only where there are bytes - so no pair is cut out of a comma. + Name = 'triggers: a binary value that is not bytes is text' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = 'bytes && IsHex(value) ? Pairs(value) : Shown(value)' + To = 'bytes ? Pairs(value) : Shown(value)' + Test = 'TriggerPartsTests.A_binary_value_that_is_not_bytes_is_shown_as_text' + } + @{ + # Review of PR #98: the bar joins the items of one trigger, in one place for both interfaces. + Name = 'triggers: the items of one trigger are joined by a bar' + File = 'src\Bws.Core\Snapshots\TriggerParts.cs' + From = 'private const string ItemSeparator = " | ";' + To = 'private const string ItemSeparator = " / ";' + Test = 'TriggerPartsTests.The_items_of_one_trigger_are_joined_by_a_bar' + } ) $mutations