From be0db09237166e175ca40b6a26e3537420bf94b3 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 30 Sep 2026 10:46:59 +0200 Subject: [PATCH] Give back only what a run took down, and plan a restart of a stopped service as its start A step putting a service back was carried out whatever had happened before it. Stop pressed before the first step of a restart started a service that was not running, an interrupted restart of a selection started services it never stopped, and a dependent stopped by somebody else after the preview was started again. Such a step is now skipped with the new reason nothingToPutBack unless the run itself stopped the entry or ended the process it lived in, counted the same way the way back counts it. A restart of a service that is not running is planned as a start, the way Restart-Service treats it, with the new warning restartOnlyStarts. A disabled service that is not running now gets the warning a start of it gets, where the restart used to be refused with a sentence about stopping it. The warnings about the start in a plan moved to a switch of their own in both interfaces, so the warning switches grew thinner rather than reaching their complexity ceiling. Stability report W-5, package D. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 ++ src/Bws.Cli/PlanText.Starting.cs | 33 +++ src/Bws.Cli/PlanText.Warnings.cs | 14 +- src/Bws.Cli/Resources/cli.en.json | 2 + src/Bws.Core/Planning/NetEffect.cs | 30 ++- src/Bws.Core/Planning/PlanBuilder.cs | 52 ++++- src/Bws.Core/Planning/PlanRun.cs | 18 +- src/Bws.Core/Planning/PlanRunner.cs | 36 ++- src/Bws.Core/Planning/PlanWarnings.cs | 14 +- src/Bws.Core/Planning/RefusedStartWarnings.cs | 7 +- src/Bws.Gui/Resources/gui.en.json | 1 + src/Bws.Gui/ViewModels/PlanWords.Starting.cs | 28 +++ src/Bws.Gui/ViewModels/PlanWords.Warnings.cs | 11 +- tests/Bws.Cli.Tests/PutBackOutputTests.cs | 59 +++++ tests/Bws.Core.Tests/PlanRunnerTests.cs | 18 +- tests/Bws.Core.Tests/PutBackTests.cs | 210 ++++++++++++++++++ tests/Bws.Core.Tests/RestartOfStoppedTests.cs | 93 ++++++++ 17 files changed, 590 insertions(+), 49 deletions(-) create mode 100644 src/Bws.Cli/PlanText.Starting.cs create mode 100644 src/Bws.Gui/ViewModels/PlanWords.Starting.cs create mode 100644 tests/Bws.Cli.Tests/PutBackOutputTests.cs create mode 100644 tests/Bws.Core.Tests/PutBackTests.cs create mode 100644 tests/Bws.Core.Tests/RestartOfStoppedTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index f8f1b26..4a14904 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,6 +120,19 @@ is not part of this repository. - Just before the process is ended, a force stop looks at it once more. If a service has started inside it since the preview, or a running service outside it has started to depend on something inside it, the process is not ended and the step says why. +- Stop pressed before the first step of a restart, or Ctrl+C, no longer starts a service that was + not running. A run now starts again only what it stopped itself: after an interruption, after a + stop that was refused, and for a dependent somebody else stopped between the preview and the + run, the step says "not started, this run never stopped it" and `skippedBecause` in the JSON of + a run is `nothingToPutBack`. The same goes for a restart of a whole selection interrupted half + way. A service stopped by somebody else after the preview of its own restart is left stopped, + and the run says it did not end where the plan wanted it. +- Restarting a service that is not running only starts it, from the window and with + `bws restart`, and the preview says so - one step and the warning "is not running, so + restarting it only starts it" (`restartOnlyStarts` in the JSON). Until now the preview showed a + stop and a start. A disabled service that is not running is no longer refused with a sentence + about stopping it - the plan warns that Windows will refuse to start it, the same as a plan to + start it. ## [0.3.0] - 2026-09-25 diff --git a/src/Bws.Cli/PlanText.Starting.cs b/src/Bws.Cli/PlanText.Starting.cs new file mode 100644 index 0000000..ccaa85c --- /dev/null +++ b/src/Bws.Cli/PlanText.Starting.cs @@ -0,0 +1,33 @@ +using Bws.Core.Planning; + +namespace Bws.Cli; + +/// +/// What a plan says about the start in it - a start the manager is known to refuse, and a restart that is +/// only a start because the entry is not running. +/// +/// Its own file since 2026-09-30, reached from the discard arm of the warning switch beside it +/// (stability report W-5, package D). The window's twin of that switch stood one fork under the complexity +/// ceiling, and the third warning about a start would have taken it there - so the two that were already +/// in it moved here with the new one, and both switches grew thinner rather than one of them fatter. The +/// terminal is cut the same way so the two keep one shape. What this switch does not know goes on to +/// , which keeps the refusal every kind without a sentence meets. +/// +internal static partial class PlanText +{ + private static string Starting(PlanWarning warning) => warning.Kind switch + { + // The line that makes the start possible, built the way the offer to stop a disabled entry builds + // its own - the window has a startup type action for this and a terminal has the command. + PlanWarningKind.DisabledCannotStart => Texts.Of( + "cli.plan.warning.disabledCannotStart", warning.ServiceName, + EquivalentCommand.For(new ServiceAction( + ActionKind.SetStartType, warning.ServiceName, To: StartSetting.Manual))), + + PlanWarningKind.PausedCannotStart => Texts.Of("cli.plan.warning.pausedCannotStart", warning.ServiceName), + + PlanWarningKind.RestartOnlyStarts => Texts.Of("cli.plan.warning.restartOnlyStarts", warning.ServiceName), + + _ => Aftermath(warning) + }; +} diff --git a/src/Bws.Cli/PlanText.Warnings.cs b/src/Bws.Cli/PlanText.Warnings.cs index 3f3cb58..5a95265 100644 --- a/src/Bws.Cli/PlanText.Warnings.cs +++ b/src/Bws.Cli/PlanText.Warnings.cs @@ -77,21 +77,13 @@ internal static partial class PlanText PlanWarningKind.StartsAtNextBoot => Texts.Of("cli.plan.warning.startsAtNextBoot", warning.ServiceName), - // The line that makes the start possible, built the way the offer above builds its own - the - // window has a startup type action for this and a terminal has the command. - PlanWarningKind.DisabledCannotStart => Texts.Of( - "cli.plan.warning.disabledCannotStart", warning.ServiceName, - EquivalentCommand.For(new ServiceAction( - ActionKind.SetStartType, warning.ServiceName, To: StartSetting.Manual))), - - PlanWarningKind.PausedCannotStart => Texts.Of("cli.plan.warning.pausedCannotStart", warning.ServiceName), - // NAMED ARMS AND A REFUSAL, SINCE 2026-09-06, AND THE WILDCARD THAT WAS HERE IS WHY. Every // kind but one used to fall through to "is already in that state, so nothing would change" - // so a warning added without a sentence would not have been silent, which is survivable, but // would have said something confident and wrong about a machine, which is not. The window's // own switch had the same shape and was changed the same day. Since 2026-09-30 the refusal - // stands at the end of the next switch along, which names what an ending sets off. - _ => Aftermath(warning) + // stands at the end of a chain of switches along - the start in the plan first, then what an + // ending sets off. + _ => Starting(warning) }; } diff --git a/src/Bws.Cli/Resources/cli.en.json b/src/Bws.Cli/Resources/cli.en.json index 6e97561..fba5db2 100644 --- a/src/Bws.Cli/Resources/cli.en.json +++ b/src/Bws.Cli/Resources/cli.en.json @@ -89,6 +89,7 @@ "cli.plan.warning.startsAtNextBoot": "{0} is stopped, and a start type does not start it. It starts at the next restart of the machine.", "cli.plan.warning.disabledCannotStart": "{0} is disabled, and Windows refuses to start a disabled entry. To make it startable first: {1}", "cli.plan.warning.pausedCannotStart": "{0} is paused, and a start does not resume a paused service - Windows will refuse it. To resume it instead: sc.exe continue {0}", + "cli.plan.warning.restartOnlyStarts": "{0} is not running, so restarting it only starts it.", "cli.plan.warning.recoveryRestarts.one": "Once the process behind {0} is ended, Windows starts {2} again by itself - its recovery actions say so. The stop may not last. See them with sc.exe qfailure {2}", "cli.plan.warning.recoveryRestarts.many": "Once the process behind {0} is ended, Windows starts {1} entries again by itself - their recovery actions say so: {2}. The stop may not last. See them with sc.exe qfailure", "cli.plan.warning.recoveryRunsProgram.one": "Once the process behind {0} is ended, Windows runs the program named in the recovery actions of {2}. See it with sc.exe qfailure {2}", @@ -115,6 +116,7 @@ "cli.run.outcome.earlierStepFailed": "not tried, an earlier step did not work", "cli.run.outcome.cancelled": "not tried, the run was interrupted", "cli.run.outcome.processStays": "not needed, the process it lives in is not being ended", + "cli.run.outcome.nothingToPutBack": "not started, this run never stopped it", "cli.run.took.milliseconds": "{0} ms", "cli.run.took.seconds": "{0} s", diff --git a/src/Bws.Core/Planning/NetEffect.cs b/src/Bws.Core/Planning/NetEffect.cs index f01df68..a9a9ab1 100644 --- a/src/Bws.Core/Planning/NetEffect.cs +++ b/src/Bws.Core/Planning/NetEffect.cs @@ -123,6 +123,26 @@ public static IReadOnlyList Of(IEnumerable results) .Select(one => one.Step)]; } + /// + /// Whether these results took the entry down - asked by a step that puts the entry back, before it is + /// tried. + /// + /// HERE RATHER THAN IN THE RUNNER BECAUSE IT IS THE WAY BACK'S OWN QUESTION, and two answers to it + /// would be two things that have to agree - the argument this file was made for. On the owner's + /// decision of 2026-09-30 (stability report W-5) a step putting something back gives back only what the + /// run took: a stop that arrived or timed out, or an ending that took the entry with its process. Until + /// then every such step ran, and an entry that was stopped all along was started by a restart nobody + /// let begin. + /// + /// One case counts here and not in the way back: an ending after which Windows started the entry + /// again at once (). The process was ended, so the entry WAS taken + /// down - the step putting it back reads it, finds it running and says so. For the way back the same + /// entry ended where it began, which is why does not count it. + /// + internal static bool TookDown(IEnumerable results, string serviceName) => + Tally(results, endingCounts: true).Moves.TryGetValue(serviceName, out var moved) + && moved.Last is StepOperation.Stop or StepOperation.Terminate; + /// /// Where each entry was first and last moved, and what each startup setting was before and after - /// the two tallies turns into lines. @@ -131,10 +151,14 @@ public static IReadOnlyList Of(IEnumerable results) /// (stability report W-10) and Of went past the length the shape guard calls close to its ceiling. /// The seam is the one the method already had: counting what happened, then saying what undoes it. /// + /// + /// Count an ending that Windows answered by starting the entry again at once - + /// asks with it, without. + /// private static ( Dictionary Moves, Dictionary Settings) - Tally(IEnumerable results) + Tally(IEnumerable results, bool endingCounts = false) { var moves = new Dictionary( StringComparer.OrdinalIgnoreCase); @@ -166,7 +190,9 @@ private static ( foundStopped.Add(result.Step.ServiceName); } - if (result.Outcome != StepOutcome.Succeeded && result.Outcome != StepOutcome.TimedOut) + if (result.Outcome != StepOutcome.Succeeded + && result.Outcome != StepOutcome.TimedOut + && !(endingCounts && result.StartedAgain)) { continue; } diff --git a/src/Bws.Core/Planning/PlanBuilder.cs b/src/Bws.Core/Planning/PlanBuilder.cs index 75f8b3c..8dd90ed 100644 --- a/src/Bws.Core/Planning/PlanBuilder.cs +++ b/src/Bws.Core/Planning/PlanBuilder.cs @@ -52,6 +52,9 @@ public OperationPlan Build(ServiceAction action) return refused; } + // What is worked out below, which is not always what was asked for - AsPlanned says when. The + // plan still carries the ask itself, so the command it hands back is the one somebody typed. + var asked = AsPlanned(action, target); var warnings = new List(); var steps = new List(); @@ -60,15 +63,15 @@ public OperationPlan Build(ServiceAction action) // down - setting a start type does not move the service at all, so the question does not // arise. Both end up with an empty list and they get there for different reasons. A setting // carrying a stop is the exception, and it asks exactly what a plain stop asks. - var blocking = action.Kind is ActionKind.Start - || (action.Kind == ActionKind.SetStartType && !StopsAlong(action, target)) + var blocking = asked.Kind is ActionKind.Start + || (asked.Kind == ActionKind.SetStartType && !StopsAlong(asked, target)) ? [] : StoppingOrder(target, warnings); // Asking to stop one service is not asking to stop seven. Without the word, the // ones in the way are named and left alone, and the plan says plainly that the // manager will refuse the stop while they run. - var cascade = action.IncludeDependents ? blocking : []; + var cascade = asked.IncludeDependents ? blocking : []; // Found by looking at a real plan rather than by reasoning: stopping BFE on this // machine drags in WdNisDrv and wtd, both kernel drivers. Refusing a driver as the @@ -103,12 +106,12 @@ public OperationPlan Build(ServiceAction action) ending = decided; } - if (StuckDown(action.Kind, target, [.. cascade, .. ending?.Sharing ?? []]) is { Count: > 0 } cannotComeBack) + if (StuckDown(asked.Kind, target, [.. cascade, .. ending?.Sharing ?? []]) is { Count: > 0 } cannotComeBack) { return Refuse(action, PlanProblemKind.CannotComeBack, cannotComeBack); } - if (!action.IncludeDependents && blocking.Count > 0) + if (!asked.IncludeDependents && blocking.Count > 0) { warnings.Add(new PlanWarning( PlanWarningKind.DependentsInTheWay, @@ -116,9 +119,15 @@ public OperationPlan Build(ServiceAction action) [.. blocking.Select(entry => entry.ServiceName)])); } - AddSteps(steps, action, target, cascade, ending); + AddSteps(steps, asked, target, cascade, ending); - AddWarnings(warnings, target, action, cascade, ending); + AddWarnings(warnings, target, asked, cascade, ending); + + if (asked != action) + { + // First, because it says what the whole plan is - every other sentence is about a step in it. + warnings.Insert(0, new PlanWarning(PlanWarningKind.RestartOnlyStarts, target.ServiceName)); + } return new OperationPlan { @@ -258,6 +267,10 @@ private static void AddSteps( // machine on 2026-08-01 by pressing Ctrl+C during a restart, which left the // service stopped - the plan had taken it down and then classified putting // it back as forward progress to be abandoned. + // + // An entry read as stopped no longer reaches this arm since 2026-09-30 - it is + // planned as a start (AsPlanned) - and the runner gives back only what the run + // took down, so a restore after an interrupted or refused stop starts nothing. AddStops(steps, cascade, target); steps.Add(PlanSteps.Made(target, StepOperation.Start, StepReason.Restore)); @@ -315,6 +328,31 @@ kind is ActionKind.Restart or ActionKind.ForceRestart .Select(entry => entry.ServiceName)] : []; + /// + /// The ask as it is worked out: a restart of an entry that is not running becomes a start, and every + /// other ask stays what it was. + /// + /// The owner's decision of 2026-09-30 (stability report W-5), and it is the answer + /// Restart-Service gives - Microsoft's page for the cmdlet says a service already stopped is + /// started. Until then the plan was a stop the runner would find already done and a start that put the + /// entry back, and putting back is carried out even after an interruption - so Stop pressed before the + /// first step started a service that had been stopped all along. Planned as a start, it is a step + /// forward like any other, and Stop pressed before it starts nothing. + /// + /// No dependants come down. They stand in the way of stopping an entry, and this one is not + /// going to be stopped. The rules of a start apply whole: a disabled entry gets the warning a start of + /// it gets, where the restart used to refuse with a sentence about stopping it - false of an entry that + /// is not running. + /// + /// Only Stopped as read for the preview. An entry on its way down is still stopped by the plan, + /// the step waiting for it, and one whose state could not be read is read again by the runner before + /// anything is asked. + /// + private static ServiceAction AsPlanned(ServiceAction action, ScmEntry target) => + action.Kind == ActionKind.Restart && target.Status == EntryStatus.Stopped + ? action with { Kind = ActionKind.Start } + : action; + /// /// Whether a startup setting carries a stop of its own entry - somebody took the offer, and the /// entry is not already stopped. An entry whose state could not be read gets the step, because diff --git a/src/Bws.Core/Planning/PlanRun.cs b/src/Bws.Core/Planning/PlanRun.cs index d722d9e..b54916e 100644 --- a/src/Bws.Core/Planning/PlanRun.cs +++ b/src/Bws.Core/Planning/PlanRun.cs @@ -25,7 +25,7 @@ public enum StepOutcome /// /// Why a step was never attempted. /// -/// Four quite different stories, and folding them into one word would be the empty-value +/// Five quite different stories, and folding them into one word would be the empty-value /// mistake part 3 of 06-STRUKTURA-I-KONWENCJE is about: "skipped" alone cannot tell /// somebody whether the machine is where they wanted it or half-way to somewhere else. /// @@ -51,7 +51,21 @@ public enum SkipReason /// the reason, not a failure. The neighbours are asked AFTER the entry itself since that day /// (stability report W-2), so that a polite stop which works leaves them running. /// - ProcessStays + ProcessStays, + + /// + /// A step putting an entry back, not tried because this run never took that entry down - its stop + /// was interrupted, held back, refused, or found it already stopped. + /// + /// A fifth story, added on the owner's decision of 2026-09-30 (stability report W-5). Until + /// that day every step putting something back ran whatever had happened before it, on the belief + /// that an entry never taken down would be found already in place. That held for an entry left + /// running and failed for one that was stopped all along: pressing Stop before the first step of a + /// restart started it, and so did an interrupted bulk restart and a dependant somebody else had + /// stopped in the meantime. "Already there" would be false here - the entry may well be stopped - + /// and "interrupted" would not say why nothing was started when the run was not interrupted. + /// + NothingToPutBack } /// diff --git a/src/Bws.Core/Planning/PlanRunner.cs b/src/Bws.Core/Planning/PlanRunner.cs index 30722e7..78d675a 100644 --- a/src/Bws.Core/Planning/PlanRunner.cs +++ b/src/Bws.Core/Planning/PlanRunner.cs @@ -86,12 +86,8 @@ public PlanRun Run( cancelled |= cancellation.IsCancellationRequested; abandoned |= abandonment.IsCancellationRequested; - var because = Held(step.Reason, abandoned, cancelled, forwardFailed, cascadeFailed); - - if (because is null && step.Reason == StepReason.SharesTheProcess && Stays(plan)) - { - because = SkipReason.ProcessStays; - } + var because = Held(step.Reason, abandoned, cancelled, forwardFailed, cascadeFailed) + ?? Unneeded(step, plan, results); if (because is { } skipped) { @@ -140,9 +136,10 @@ public PlanRun Run( /// /// Putting things back is not part of the forward path and does not stop when the forward path /// does. Those steps exist to give back what earlier steps took, and abandoning them would - /// leave the machine trimmed by a plan that failed - the one outcome nobody asked for. Anything - /// that was never taken down is found already in place and reported as such, so this costs - /// nothing when it is not needed. + /// leave the machine trimmed by a plan that failed - the one outcome nobody asked for. What + /// was never taken down is not given back at all, since 2026-09-30 - + /// says why. This comment used to say such an entry would be found already in place, which was + /// true of an entry left running and false of one stopped all along (stability report W-5). /// /// THE STEPS STANDING BEHIND THE ENTRY'S OWN STOP NEED THE OPPOSITE TREATMENT. The ending /// of a process and, since 2026-09-29, the neighbours asked on the way to it exist for the case @@ -173,6 +170,27 @@ public PlanRun Run( : null; } + /// + /// Why a step nothing held back is still not worth trying - or nothing when it is. + /// + /// Two answers, one question: the plan wanted this step only on the way to something that is not + /// going to happen. A neighbour is asked to stop only so its process can be ended, and + /// says when it will not be. A step putting an entry back exists only to give + /// back what the run took, and says when it took nothing. + /// + /// The second arrived on the owner's decision of 2026-09-30 (stability report W-5). Until then + /// every step putting something back was tried, so Stop pressed before the first step of a restart + /// started a service that had been stopped all along - and an interrupted bulk restart and a + /// dependant somebody else had stopped in the meantime did the same. Asked from what the run itself + /// recorded, so it costs no reading of the machine. + /// + private SkipReason? Unneeded(PlanStep step, OperationPlan plan, List results) => step.Reason switch + { + StepReason.SharesTheProcess when Stays(plan) => SkipReason.ProcessStays, + StepReason.Restore when !NetEffect.TookDown(results, step.ServiceName) => SkipReason.NothingToPutBack, + _ => null + }; + /// /// Whether the process a plan ends is going to stay, asked just before a neighbour would be told /// to stop on the way to ending it. diff --git a/src/Bws.Core/Planning/PlanWarnings.cs b/src/Bws.Core/Planning/PlanWarnings.cs index 7b6215c..1f84b03 100644 --- a/src/Bws.Core/Planning/PlanWarnings.cs +++ b/src/Bws.Core/Planning/PlanWarnings.cs @@ -197,7 +197,19 @@ public enum PlanWarningKind /// four and Schedule carries a fifth. Said rather than guessed at, and never left out: the /// owner's decision of 2026-09-30. /// - RecoveryUnnamed + RecoveryUnnamed, + + /// + /// A restart of an entry read as Stopped, planned as a start - there is nothing to stop, so all it + /// does is start it. + /// + /// The owner's decision of 2026-09-30 (stability report W-5), the answer Restart-Service + /// gives as well. Said because the plan no longer shows the stop somebody expects of a restart, and a + /// plan shorter than expected with no word about it reads as a plan that forgot something. Measured + /// before the change: bws restart AxInstSV --dry-run on a stopped entry of this machine showed a + /// stop and a start "put back" and said nothing about the entry not running. + /// + RestartOnlyStarts } /// diff --git a/src/Bws.Core/Planning/RefusedStartWarnings.cs b/src/Bws.Core/Planning/RefusedStartWarnings.cs index 6fb3a63..9533694 100644 --- a/src/Bws.Core/Planning/RefusedStartWarnings.cs +++ b/src/Bws.Core/Planning/RefusedStartWarnings.cs @@ -12,9 +12,10 @@ namespace Bws.Core.Planning; /// warning method stands near its ceilings, and the subject is its own - everything there is about /// what a plan does, and this is about a start the machine is known to turn down. /// -/// Only a plain start. A restart of a disabled entry is refused outright before any warning -/// is worked out (PlanProblemKind.CannotComeBack), and a restart of a paused one stops it first, -/// which a paused service accepts. +/// Only a start - which since 2026-09-30 includes a restart of an entry read as stopped, planned as +/// a start (PlanBuilder.AsPlanned). A restart of a running disabled entry is refused outright before +/// any warning is worked out (PlanProblemKind.CannotComeBack), and a restart of a paused one stops +/// it first, which a paused service accepts. /// internal static class RefusedStartWarnings { diff --git a/src/Bws.Gui/Resources/gui.en.json b/src/Bws.Gui/Resources/gui.en.json index 703e0c4..8f78441 100644 --- a/src/Bws.Gui/Resources/gui.en.json +++ b/src/Bws.Gui/Resources/gui.en.json @@ -276,6 +276,7 @@ "gui.plan.warning.startsAtNextBoot": "{0} is stopped, and a startup type does not start it. It starts at the next restart of the machine.", "gui.plan.warning.disabledCannotStart": "{0} is disabled, and Windows refuses to start a disabled entry. Change its startup type first.", "gui.plan.warning.pausedCannotStart": "{0} is paused, and a start does not resume a paused service - Windows will refuse it.", + "gui.plan.warning.restartOnlyStarts": "{0} is not running, so restarting it only starts it.", "gui.plan.warning.recoveryRestarts.one": "Once the process behind {0} is ended, Windows starts {2} again by itself - its recovery actions say so. The stop may not last.", "gui.plan.warning.recoveryRestarts.many": "Once the process behind {0} is ended, Windows starts {1} entries again by itself - their recovery actions say so: {2}. The stop may not last.", "gui.plan.warning.recoveryRunsProgram.one": "Once the process behind {0} is ended, Windows runs the program named in the recovery actions of {2}.", diff --git a/src/Bws.Gui/ViewModels/PlanWords.Starting.cs b/src/Bws.Gui/ViewModels/PlanWords.Starting.cs new file mode 100644 index 0000000..583499f --- /dev/null +++ b/src/Bws.Gui/ViewModels/PlanWords.Starting.cs @@ -0,0 +1,28 @@ +using Bws.Core.Planning; + +namespace Bws.Gui.ViewModels; + +/// +/// What a plan says about the start in it - a start the manager is known to refuse, and a restart that is +/// only a start because the entry is not running. +/// +/// Its own file since 2026-09-30, reached from the discard arm of the warning switch beside it +/// (stability report W-5, package D). That switch stood one fork under the complexity ceiling, and the +/// third warning about a start would have taken it there - so the two that were already in it moved here +/// with the new one, and the switch grew thinner rather than fatter. The terminal's PlanText is cut the +/// same way, so the two keep one shape. What this switch does not know goes on to +/// , which keeps the refusal every kind without a sentence meets. +/// +internal static partial class PlanWords +{ + private static string Starting(PlanWarning warning) => warning.Kind switch + { + PlanWarningKind.DisabledCannotStart => Texts.Of("gui.plan.warning.disabledCannotStart", warning.ServiceName), + + PlanWarningKind.PausedCannotStart => Texts.Of("gui.plan.warning.pausedCannotStart", warning.ServiceName), + + PlanWarningKind.RestartOnlyStarts => Texts.Of("gui.plan.warning.restartOnlyStarts", warning.ServiceName), + + _ => Aftermath(warning) + }; +} diff --git a/src/Bws.Gui/ViewModels/PlanWords.Warnings.cs b/src/Bws.Gui/ViewModels/PlanWords.Warnings.cs index 2cdd23a..ad69792 100644 --- a/src/Bws.Gui/ViewModels/PlanWords.Warnings.cs +++ b/src/Bws.Gui/ViewModels/PlanWords.Warnings.cs @@ -97,16 +97,13 @@ internal static partial class PlanWords PlanWarningKind.StartsAtNextBoot => Texts.Of("gui.plan.warning.startsAtNextBoot", warning.ServiceName), - PlanWarningKind.DisabledCannotStart => Texts.Of("gui.plan.warning.disabledCannotStart", warning.ServiceName), - - PlanWarningKind.PausedCannotStart => Texts.Of("gui.plan.warning.pausedCannotStart", warning.ServiceName), - // NAMED ARMS AND A REFUSAL, SINCE 2026-09-06, AND THE WILDCARD THAT WAS HERE IS WHY. Every // kind but one used to fall through to "is already in that state, so nothing would change" - // a warning added without a sentence would have said something confident and wrong about a // machine rather than nothing at all. The terminal's own switch had the same shape and was - // changed the same day. Since 2026-09-30 the refusal stands at the end of the next switch - // along, which names what an ending sets off - this one is one fork under the ceiling. - _ => Aftermath(warning) + // changed the same day. Since 2026-09-30 the refusal stands at the end of a chain of switches + // along - the start in the plan first, then what an ending sets off - and this one gave its + // two arms about a start to the first of them rather than growing to the ceiling. + _ => Starting(warning) }; } diff --git a/tests/Bws.Cli.Tests/PutBackOutputTests.cs b/tests/Bws.Cli.Tests/PutBackOutputTests.cs new file mode 100644 index 0000000..cbf0961 --- /dev/null +++ b/tests/Bws.Cli.Tests/PutBackOutputTests.cs @@ -0,0 +1,59 @@ +using System.Text.Json; +using Bws.Core; +using Bws.Core.Planning; + +namespace Bws.Cli.Tests; + +/// +/// What the terminal and the machine readable output say about a step that had nothing to put back and a +/// restart that only starts - stability report W-5, the owner's decision of 2026-09-30. +/// +/// The sentence is found by a key built from the value's name, so a reason added without one would +/// print its own key rather than fail - which is why the first test asks every reason, not only the new one. +/// +public sealed class PutBackOutputTests +{ + [Fact] + public void Every_reason_a_step_was_skipped_has_a_sentence_of_its_own() + { + var said = Enum.GetValues().Select(reason => PlanText.Describe(Skipped(reason))).ToList(); + + Assert.All(said, sentence => Assert.DoesNotContain("cli.run.outcome", sentence, StringComparison.Ordinal)); + Assert.Equal(said.Count, said.Distinct(StringComparer.Ordinal).Count()); + } + + [Fact] + public void The_document_names_the_reason_and_the_warning() + { + var run = new PlanRun + { + Plan = new OperationPlan + { + Action = new ServiceAction(ActionKind.Restart, "AxInstSV"), + Steps = [Skipped(SkipReason.NothingToPutBack).Step], + Warnings = [new PlanWarning(PlanWarningKind.RestartOnlyStarts, "AxInstSV", [])], + Problems = [] + }, + Results = [Skipped(SkipReason.NothingToPutBack)], + Cancelled = false, + Ceiling = TimeSpan.FromMinutes(1) + }; + + var document = JsonDocument.Parse(PlanJson.Render(run)).RootElement; + + Assert.Equal("nothingToPutBack", document.GetProperty("results")[0].GetProperty("skippedBecause").GetString()); + Assert.Equal("restartOnlyStarts", document.GetProperty("warnings")[0].GetProperty("kind").GetString()); + } + + private static StepResult Skipped(SkipReason reason) => new() + { + Step = new PlanStep("AxInstSV", "ActiveX Installer", StepOperation.Start, StepReason.Restore), + Outcome = StepOutcome.Skipped, + SkippedBecause = reason, + Status = EntryStatus.Unknown, + ProcessId = Reading.NotRead(), + ErrorCode = 0, + Error = null, + Milliseconds = 0 + }; +} diff --git a/tests/Bws.Core.Tests/PlanRunnerTests.cs b/tests/Bws.Core.Tests/PlanRunnerTests.cs index 9427114..9136667 100644 --- a/tests/Bws.Core.Tests/PlanRunnerTests.cs +++ b/tests/Bws.Core.Tests/PlanRunnerTests.cs @@ -225,9 +225,10 @@ public void A_restart_that_fails_half_way_still_puts_back_what_it_took_down() Assert.False(run.Completed); Assert.Equal(StepOutcome.Failed, Outcome(run, "MRxSmb20", StepOperation.Stop)); - // Its own start gives it back, so it is tried rather than abandoned - and finds - // nothing to do, because the stop that failed left it running. - Assert.Equal(SkipReason.AlreadyThere, Result(run, "MRxSmb20", StepOperation.Start).SkippedBecause); + // Its own start is not tried, because the stop that failed never took it down. Until + // 2026-09-30 it was tried and found the entry running - the same machine, and a + // reason that only held because the entry happened to be running (stability report W-5). + Assert.Equal(SkipReason.NothingToPutBack, Result(run, "MRxSmb20", StepOperation.Start).SkippedBecause); foreach (var name in (string[])["LanmanWorkstation", "Netlogon", "SessionEnv"]) { @@ -273,8 +274,9 @@ public void A_restore_that_fails_does_not_stop_the_other_restores() Assert.Equal(StepOutcome.Failed, Outcome(run, "LanmanWorkstation", StepOperation.Stop)); - // Nothing after it went down, so there is nothing to put back and the restores find - // their work done. What matters is that they were reached at all. + // Nothing after it went down, so the restores of those have nothing to put back - and + // say so - while the two dependants that did go down come back. What matters is that + // no restore was held back by the failure itself. Assert.All( run.Results.Where(result => result.Step.Reason == StepReason.Restore), result => Assert.NotEqual(SkipReason.EarlierStepFailed, result.SkippedBecause)); @@ -333,8 +335,10 @@ public void A_step_announces_its_place_in_the_plan_rather_than_its_place_in_the_ starting: (_, number) => announced.Add(number)); // Three and four are missing, because the forward path stopped at the refusal, and - // the numbers that follow do not close the gap. That is the whole point. - Assert.Equal([1, 2, 5, 6, 7, 8], announced); + // five to seven since 2026-09-30, because what they would put back was never taken + // down - Netlogon refused and the two behind it were not tried. The one number left + // after the gap does not close it. That is the whole point. + Assert.Equal([1, 2, 8], announced); } [Fact] diff --git a/tests/Bws.Core.Tests/PutBackTests.cs b/tests/Bws.Core.Tests/PutBackTests.cs new file mode 100644 index 0000000..44787e4 --- /dev/null +++ b/tests/Bws.Core.Tests/PutBackTests.cs @@ -0,0 +1,210 @@ +using Bws.Core.Planning; +using Bws.Core.Tests.Fakes; + +namespace Bws.Core.Tests; + +/// +/// A run gives back only what it took down - the external stability report of 2026-09-29 (W-5) and the +/// owner's decision on it of 2026-09-30. +/// +/// Three shapes of one mistake, and each started a service the run had never touched. Stop pressed +/// before the first step of a restart, a restart of a whole selection interrupted half way, and a +/// dependant somebody else stopped between the preview and the run. Every step putting something back used +/// to be tried whatever had happened before it. +/// +/// The assertions are about what the manager was ASKED, because that is the half a reported outcome +/// can hide - and each is red on the code before that day for exactly that reason. +/// +public sealed class PutBackTests +{ + private static readonly TimeSpan Minute = TimeSpan.FromMinutes(1); + + private const int Held = 4812; + + [Fact] + public void Stop_pressed_before_a_restart_of_a_stopped_entry_starts_nothing() + { + using var interruption = new CancellationTokenSource(); + interruption.Cancel(); + + var control = new FakeScmControl().At("AxInstSV", EntryStatus.Stopped); + + var run = new PlanRunner(control, new FakeClock()).Run( + Planned(ActionKind.Restart, Entry("AxInstSV", EntryStatus.Stopped)), Minute, interruption.Token); + + Assert.Equal(SkipReason.Cancelled, Assert.Single(run.Results).SkippedBecause); + Assert.Empty(control.Requested); + } + + [Fact] + public void Stop_pressed_before_a_restart_gives_nothing_back() + { + using var interruption = new CancellationTokenSource(); + interruption.Cancel(); + + // Running for the preview and stopped by somebody else since - the one shape in which the step + // putting it back used to do harm rather than find nothing to do. + var control = new FakeScmControl().At("Spooler", EntryStatus.Stopped); + + var run = new PlanRunner(control, new FakeClock()).Run( + Planned(ActionKind.Restart, Entry("Spooler", EntryStatus.Running)), Minute, interruption.Token); + + Assert.Equal( + [SkipReason.Cancelled, SkipReason.NothingToPutBack], + run.Results.Select(result => result.SkippedBecause)); + + Assert.Empty(control.Requested); + } + + [Fact] + public void A_dependant_somebody_else_stopped_before_the_run_is_left_stopped() + { + var control = new FakeScmControl() + .At("Lanman", EntryStatus.Running) + .At("Dependant", EntryStatus.Stopped); + + var run = Run(control, Planned( + ActionKind.Restart, Entry("Lanman", EntryStatus.Running), Entry("Dependant", EntryStatus.Running))); + + Assert.Equal(SkipReason.AlreadyThere, Result(run, "Dependant", StepOperation.Stop).SkippedBecause); + Assert.Equal(SkipReason.NothingToPutBack, Result(run, "Dependant", StepOperation.Start).SkippedBecause); + Assert.Equal(["Lanman", "Lanman"], control.Requested); + + // Not where the plan wanted it at the end, and said so rather than counted as done. + Assert.False(run.Completed); + } + + [Fact] + public void A_neighbour_found_stopped_is_not_started_after_the_process_ends() + { + var control = new FakeScmControl() + .Reaching("Spooler", new ServiceProgress(EntryStatus.StopPending, 0, TimeSpan.Zero, Held)) + .At("Housemate", EntryStatus.Stopped); + + var run = Run(control, ForcedRestart( + Step("Spooler", StepOperation.Stop, StepReason.Requested), + Step("Housemate", StepOperation.Stop, StepReason.SharesTheProcess), + Ending("Housemate"), + Step("Spooler", StepOperation.Start, StepReason.Restore), + Step("Housemate", StepOperation.Start, StepReason.Restore))); + + // Its own stop found it stopped, so it was not in the process the ending took down. + Assert.Equal(SkipReason.NothingToPutBack, Result(run, "Housemate", StepOperation.Start).SkippedBecause); + Assert.Equal(StepOutcome.Succeeded, Result(run, "Spooler", StepOperation.Start).Outcome); + Assert.DoesNotContain("Housemate", control.Requested); + } + + [Fact] + public void An_entry_its_ending_took_down_is_given_back_although_its_own_stop_was_refused() + { + // The ending is what took it down, so that is what the step putting it back has to count. + var control = new FakeScmControl().RefusingRequests("Spooler", 1052); + + var run = Run(control, ForcedRestart( + Step("Spooler", StepOperation.Stop, StepReason.Requested), + Ending(), + Step("Spooler", StepOperation.Start, StepReason.Restore))); + + Assert.Equal(Held, Assert.Single(control.Ended)); + + // Asked twice - the stop, and the start that gives it back. The double refuses both, which is + // beside the point: what is tested is that the second was asked at all. + Assert.Equal(["Spooler", "Spooler"], control.Requested); + } + + [Fact] + public void An_ending_Windows_answered_by_starting_the_entry_again_still_took_it_down() + { + // The process was ended and the manager started the entry again at once. The step putting it back + // reads it and finds it running - which is a different sentence from "this run never stopped it". + var control = new FakeScmControl() + .ComingBack("Spooler", new ServiceProgress(EntryStatus.Running, 0, TimeSpan.Zero, 5555)) + .At("Housemate", EntryStatus.Running); + + var run = Run(control, ForcedRestart( + Ending("Housemate") with { Reason = StepReason.Requested }, + Step("Spooler", StepOperation.Start, StepReason.Restore), + Step("Housemate", StepOperation.Start, StepReason.Restore))); + + Assert.True(run.Results[0].StartedAgain); + Assert.Equal(SkipReason.AlreadyThere, Result(run, "Spooler", StepOperation.Start).SkippedBecause); + + // The neighbour died with the process and nothing brought it back, so it is given back. + Assert.Equal(StepOutcome.Succeeded, Result(run, "Housemate", StepOperation.Start).Outcome); + } + + [Fact] + public void An_interrupted_restart_of_a_selection_starts_nothing_it_did_not_stop() + { + using var interruption = new CancellationTokenSource(); + + var catalog = new FakeScmCatalog([Entry("First", EntryStatus.Running), Entry("Second", EntryStatus.Running)]); + + var bulk = new BulkPlanBuilder(catalog.ReadAll(), catalog) + .Build(new BulkAction(ActionKind.Restart, ["First", "Second"], false)); + + // The second was stopped by somebody else after the preview. + var control = new FakeScmControl() + .At("First", EntryStatus.Running) + .At("Second", EntryStatus.Stopped); + + var run = new BulkRunner(new PlanRunner(control, new FakeClock())).Run( + bulk, Minute, interruption.Token, starting: (_, _) => interruption.Cancel()); + + Assert.True(run.Cancelled); + Assert.DoesNotContain("Second", control.Requested); + + Assert.Equal( + SkipReason.NothingToPutBack, + run.Runs.SelectMany(one => one.Results) + .Single(result => result.Step is { ServiceName: "Second", Operation: StepOperation.Start }) + .SkippedBecause); + } + + // -- fixtures -------------------------------------------------------------------------- + + /// A plan from the builder, the first entry the target and every other one depending on it. + private static OperationPlan Planned(ActionKind kind, ScmEntry target, params ScmEntry[] dependants) + { + var catalog = new FakeScmCatalog([target, .. dependants]); + + if (dependants.Length > 0) + { + catalog.DependedOnBy(target.ServiceName, [.. dependants.Select(entry => entry.ServiceName)]); + } + + return new PlanBuilder(catalog.ReadAll(), catalog) + .Build(new ServiceAction(kind, target.ServiceName, IncludeDependents: dependants.Length > 0)); + } + + private static ScmEntry Entry(string serviceName, EntryStatus status) => + Entries.Named(serviceName, serviceName) with + { + Status = status, + StartType = Reading.Present(Core.StartType.Manual), + DelayedAuto = Reading.Absent(), + ProcessId = status == EntryStatus.Stopped ? Reading.Absent() : Reading.Present(4444) + }; + + private static PlanStep Step(string name, StepOperation operation, StepReason reason) => + new(name, name, operation, reason); + + private static PlanStep Ending(params string[] housemates) => + new("Spooler", "Spooler", StepOperation.Terminate, StepReason.Escalation, ProcessId: Held, TakesWithIt: housemates); + + private static OperationPlan ForcedRestart(params PlanStep[] steps) => new() + { + Action = new ServiceAction(ActionKind.ForceRestart, "Spooler"), + Steps = steps, + Warnings = [], + Problems = [] + }; + + private static PlanRun Run(FakeScmControl control, OperationPlan plan) => + new PlanRunner(control, new FakeClock()).Run(plan, TimeSpan.FromSeconds(30)); + + private static StepResult Result(PlanRun run, string serviceName, StepOperation operation) => + Assert.Single( + run.Results, + result => result.Step.ServiceName == serviceName && result.Step.Operation == operation); +} diff --git a/tests/Bws.Core.Tests/RestartOfStoppedTests.cs b/tests/Bws.Core.Tests/RestartOfStoppedTests.cs new file mode 100644 index 0000000..a42f6f4 --- /dev/null +++ b/tests/Bws.Core.Tests/RestartOfStoppedTests.cs @@ -0,0 +1,93 @@ +using Bws.Core.Planning; +using Bws.Core.Tests.Fakes; + +namespace Bws.Core.Tests; + +/// +/// A restart of an entry that is not running is planned as its start - the external stability report of +/// 2026-09-29 (W-5) and the owner's decision on it of 2026-09-30, which is also what Restart-Service does. +/// +/// Measured before the change on this machine: bws restart AxInstSV --dry-run on a stopped +/// entry showed a stop and a start "put back", and the start was carried out even when Stop was pressed +/// before the first step. And a restart of a stopped disabled entry was refused with a sentence about +/// stopping it, which it is not. +/// +public sealed class RestartOfStoppedTests +{ + [Fact] + public void A_restart_of_a_stopped_entry_is_one_start_asked_for() + { + var plan = Build(new ServiceAction(ActionKind.Restart, "AxInstSV"), Entry("AxInstSV", EntryStatus.Stopped)); + + var step = Assert.Single(plan.Steps); + + Assert.Equal(StepOperation.Start, step.Operation); + Assert.Equal(StepReason.Requested, step.Reason); + + // The ask is kept, so the command handed back is the one somebody typed. + Assert.Equal(ActionKind.Restart, plan.Action.Kind); + Assert.Equal(PlanWarningKind.RestartOnlyStarts, plan.Warnings[0].Kind); + } + + [Fact] + public void A_restart_of_a_running_entry_is_still_a_stop_and_a_start_put_back() + { + var plan = Build(new ServiceAction(ActionKind.Restart, "Spooler"), Entry("Spooler", EntryStatus.Running)); + + Assert.Equal( + [(StepOperation.Stop, StepReason.Requested), (StepOperation.Start, StepReason.Restore)], + plan.Steps.Select(step => (step.Operation, step.Reason))); + + Assert.DoesNotContain(plan.Warnings, warning => warning.Kind == PlanWarningKind.RestartOnlyStarts); + } + + [Fact] + public void A_restart_of_a_stopped_disabled_entry_warns_the_way_its_start_does() + { + var disabled = Entry("AmdCrash", EntryStatus.Stopped) with + { + StartType = Reading.Present(Core.StartType.Disabled) + }; + + var plan = Build(new ServiceAction(ActionKind.Restart, "AmdCrash"), disabled); + + // Refused until 2026-09-30 with "restarting it would stop it and could not start it again" - false + // of an entry that is not running. Now what a start of it gets, on the owner's decision of package C. + Assert.True(plan.IsRunnable); + + Assert.Equal( + [PlanWarningKind.RestartOnlyStarts, PlanWarningKind.DisabledCannotStart], + plan.Warnings.Select(warning => warning.Kind)); + } + + [Fact] + public void A_restart_of_a_stopped_entry_takes_no_dependant_down() + { + // A dependant still running on a stopped entry is rare and real - the entry died under it. It is not + // in the way of anything, because nothing here is going to be stopped. + var catalog = new FakeScmCatalog([Entry("Lanman", EntryStatus.Stopped), Entry("Dependant", EntryStatus.Running)]) + .DependedOnBy("Lanman", "Dependant"); + + var plan = new PlanBuilder(catalog.ReadAll(), catalog) + .Build(new ServiceAction(ActionKind.Restart, "Lanman", IncludeDependents: true)); + + Assert.Equal("Lanman", Assert.Single(plan.Steps).ServiceName); + Assert.DoesNotContain(plan.Warnings, warning => warning.Kind == PlanWarningKind.Cascade); + } + + private static OperationPlan Build(ServiceAction action, ScmEntry entry) + { + var catalog = new FakeScmCatalog([entry]); + + return new PlanBuilder(catalog.ReadAll(), catalog).Build(action); + } + + private static ScmEntry Entry(string serviceName, EntryStatus status) => + Entries.Named(serviceName, serviceName) with + { + Status = status, + StartType = Reading.Present(Core.StartType.Manual), + DelayedAuto = Reading.Absent(), + ProcessId = status == EntryStatus.Stopped ? Reading.Absent() : Reading.Present(4444) + }; +}