diff --git a/.gitignore b/.gitignore index 5754911..a7f2d0f 100644 --- a/.gitignore +++ b/.gitignore @@ -56,3 +56,7 @@ Krasivoe*.ps1 tools/proxmox/stand.config*.json !tools/proxmox/stand.config.example.json tools/proxmox/results/ + +# Mutation-testing scratch backups. Added after a runner's backup and one of its +# mutants were swept into a commit by `git add -A`: *.bak did not match .mutbak. +*.mutbak diff --git a/CHANGELOG.md b/CHANGELOG.md index c13266a..860425e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,93 @@ Windows Update driver listing, run-to-run delta and HTML report. See CLAUDE.md. --- +## [2.22] - 2026-07-23 + +A release about knowing when work is finished, and about the run surviving things that +have nothing to do with maintenance. Two of the three defects here are the same mistake in +different places: treating "I could not observe it" as an answer. Disk Cleanup was +declared incomplete because it had not exited, though it had demonstrably stopped working; +the installer accepted a PowerShell whose version it had failed to read. + +### Fixed + +- **A Disk Cleanup that had stopped doing anything was reported as still deleting, and + cost the full timeout.** Measured on a live workstation: `cleanmgr /sagerun` did its + work in about ten seconds, closed its window, and then stayed resident - no CPU, no I/O + on any of the three counters, all six threads waiting. Because the wait was written + against process exit, the run sat there for the remaining ~890 seconds and then set + `DiskCleanupPending` and warned that the figures were partial, having observed nothing + happening for most of that time. The wait now also ends on total stillness: no CPU and + no I/O across twelve consecutive checks, after activity was seen at least once. That is + reported as what it is - an observation, not proof of completion - so the new status is + called `idle-resident` and the log says what was measured. Not being able to measure + activity resets the counter rather than advancing it, so a machine where WMI is broken + keeps the old, patient behaviour instead of cutting cleanups short. +- **An unusable `-LogPath` killed the run before any maintenance happened.** The log + header was written with two bare `Out-File` calls placed before the block that + guarantees the result JSON and the summary. Six of seven bad paths make `Out-File` throw + a terminating error even at the default `ErrorActionPreference` (missing directory, path + is a directory, invalid characters, a colon in the name, an over-long path, an + unreachable UNC share), and the exception escaped the whole function. A log that cannot + be written is now a degraded run, as it always was everywhere else - never a failed one. +- **`install.ps1` accepted a PowerShell 7 whose version it could not read.** The check was + `if ($pwshVersion -and $pwshVersion -lt '7.1')`, so an unreadable version left the + comparison unevaluated and the installer carried on to pin an elevated desktop shortcut + to a binary nobody had established was suitable. This is the same fail-open shape as the + SHA256 verification that hid inside `if ($hashAsset)` until v2.17. +- **The banner's frame depended on the version being four characters long.** It was a + single literal block whose padding had been counted by hand; the title row measured 70 + columns only because "2.21" happens to be four characters, so a version like 2.5 or + 2.100 would have pushed its border out of line. The banner is now composed. + +### Changed + +- **Console output follows the window.** The desktop shortcut opens conhost at its + 120-column default while winget's upgrade table needs about 140 on a localised system, + so the table wrapped and every wrapped row landed across WinClean's own output. The + window is now widened best-effort at startup (never beyond what the screen can show, + never failing the run if the host refuses - Windows Terminal does), and the frames are + derived from the actual console width instead of a literal 70 repeated in four places. + Bounded at both ends: never narrower than the historical 70, never wider than 90. +- **Every run ends through one code path.** A successful self-update used to call `exit` + itself, bypassing the block that writes the result JSON, shows the summary and releases + the log handle - so it hand-copied the parts someone remembered at the time. That is how + v2.21 came to ship two separate fixes to the same few lines. The list now exists once. + Exit codes are unchanged. +- **Storage Sense: the log now says which of two things happened** when it returns success + having freed nothing measurable - it is switched off, or it ran and there was simply + nothing left. On a regularly maintained machine the second happens every time and looked + like a malfunction. Deliberately reported rather than acted on: skipping Disk Cleanup on + that basis would silently stop cleaning Update Cleanup, memory dumps, Language Pack, old + ChkDsk files and Windows Error Reporting, none of which Storage Sense touches. + +### Added + +- `DiskCleanupStatus` in the result JSON - twelve values covering every way that step can + end, replacing a boolean that covered two different truths. `idle-resident` is named + after what is measured (a process seen working, then completely still) rather than after + the conclusion drawn from it, because that conclusion can be wrong and nothing downstream + is built to depend on it. `running`, `skipped-report-only` and `skipped-cleanup-group` + exist because "not-run" is documented as "the run aborted before reaching it", which is + untrue of a healthy preview, a deliberate skip, or a cleanup still in flight. The old + `failed` was split into `not-armed`, `start-failed` and `exit-nonzero` - only the last + means the machine may be partially cleaned. +- `SUPPORT.md`, a release-notes template in `docs/release-process.md`, and a section in + `CONTRIBUTING.md` describing what gets accepted and on what criteria. + +### Tests + +- 573 -> 702. Four mutation runs over the new logic (79 mutations); every one is caught. + Eight of them survived at first and each exposed a real gap rather than a wrong fix - + among them that `Should -Invoke -Times N` means *at least* N and cannot see a duplicate, + that the "unreadable activity" branch was never exercised, and that nothing pinned the + logo's alignment or the title's centring. +- Frame geometry is now checked at 70, 74, 80 and 90 columns. The smoke test runs with + output redirected, where the width is unknown and the frame falls back to 70 - so on its + own it could only ever have verified the width that existed before this release. + +--- + ## [2.21] - 2026-07-23 A release about telling the truth on the way out. The self-update could change a file diff --git a/CLAUDE.md b/CLAUDE.md index 7052544..fb80854 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,7 +1,7 @@ # WinClean - Инструкции для Claude > Последнее обновление: 2026-07-23 -> Текущая версия скрипта: 2.21 (ВЫПУЩЕНА 23.07.2026: GitHub Release с обоими ассетами + PSGallery 2.21 + e2e через опубликованный one-liner) +> Текущая версия скрипта: 2.22 (В РАБОТЕ, ветка `feature/v2.22`; последняя ВЫПУЩЕННАЯ - 2.21 от 23.07.2026) --- @@ -11,7 +11,7 @@ | Параметр | Значение | |----------|----------| -| **Версия** | 2.21 | +| **Версия** | 2.22 | | **Язык** | PowerShell 7.1+ | | **Платформа** | Windows 11 (23H2/24H2/25H2) | | **Лицензия** | MIT | @@ -43,12 +43,12 @@ CleanScript/ │ └── logo.svg # Логотип проекта ├── get.ps1 # Bootstrap: разовый запуск одной командой (irm | iex) ├── install.ps1 # Bootstrap: установка/обновление + ярлык (RunAs admin) -├── tests/ # Pester тесты (573 всего; счётчик - прогоном, не грепом) -│ ├── Helpers.Tests.ps1 # Unit-тесты helper-функций (280, дот-сорсят продукт - нужны права админа) -│ ├── Fixes.Tests.ps1 # Валидационные тесты исправлений (183) +├── tests/ # Pester тесты (702 всего; счётчик - прогоном, не грепом) +│ ├── Helpers.Tests.ps1 # Unit-тесты helper-функций (378, дот-сорсят продукт - нужны права админа) +│ ├── Fixes.Tests.ps1 # Валидационные тесты исправлений (206) │ ├── Integration.Tests.ps1 # Интеграционные тесты в песочнице ФС (67, требуют admin) -│ ├── StandHelpers.Tests.ps1 # Чистые хелперы стенда: dead-man + версионный гейт ассертов (17, без admin) -│ └── Docs.Tests.ps1 # Гварды документации: нет тире + нет битых внутренних ссылок (26, без admin) +│ ├── StandHelpers.Tests.ps1 # Чистые хелперы стенда: dead-man + версионный гейт + таблица истинности report-гварда (23, без admin) +│ └── Docs.Tests.ps1 # Гварды документации: нет тире + нет битых ссылок (28, без admin; файлы ПЕРЕЧИСЛЯЮТСЯ, а не задаются списком) ├── tools/ # Тестовая инфраструктура (не публикуется в PSGallery) │ ├── Invoke-ReleaseCheck.ps1 # 🔴 Единая проверка перед релизом (fail-closed) │ ├── Invoke-SmokeTest.ps1 # Смоук: ReportOnly + JSON + геометрия рамок @@ -198,7 +198,7 @@ Publish-PSResource -Path .\WinClean.ps1 -Repository PSGallery -ApiKey $env:PSGAL **Проверки (4 job'а, счёт сверять gh run view --json jobs, а не этой строкой):** 1. **lint** - PSScriptAnalyzer (Error+Warning) через общий ools/Invoke-Lint.ps1 - тот же, что зовёт релиз-гейт 2. **syntax** - Проверка синтаксиса PowerShell -3. **test** - Pester тесты (573; интеграционные требуют admin - на GitHub runners это выполняется) +3. **test** - Pester тесты (702; интеграционные требуют admin - на GitHub runners это выполняется) 4. **smoke** - прогон -ReportOnly + геометрия рамок + result JSON **Исключения PSScriptAnalyzer** (допустимые для CLI): @@ -208,7 +208,7 @@ Publish-PSResource -Path .\WinClean.ps1 -Repository PSGallery -ApiKey $env:PSGAL ### Pester тесты (v2.13+) -- `tests/Helpers.Tests.ps1` - 280 unit-тестов (дот-сорсят WinClean.ps1), `tests/Fixes.Tests.ps1` - 183, `tests/Integration.Tests.ps1` - 67 (песочница ФС, требуют admin), `tests/StandHelpers.Tests.ps1` - 17 (без admin), `tests/Docs.Tests.ps1` - 26 (гварды доков, без admin) +- `tests/Helpers.Tests.ps1` - 378 unit-тестов (дот-сорсят WinClean.ps1), `tests/Fixes.Tests.ps1` - 206, `tests/Integration.Tests.ps1` - 67 (песочница ФС, требуют admin), `tests/StandHelpers.Tests.ps1` - 23 (без admin), `tests/Docs.Tests.ps1` - 28 (гварды доков, без admin) - Особенности: функции в BeforeAll (не AST), regex для locale-независимости, отдельные It блоки --- @@ -218,6 +218,41 @@ Publish-PSResource -Path .\WinClean.ps1 -Repository PSGallery -ApiKey $env:PSGAL ### В работе - [ ] Публикация статьи на Хабре (`docs/_habr/habr-article.md` - текст готов, ждёт скриншоты). ⚠️ Текст писался под старую версию - сверить с v2.21 (появились get.ps1/install.ps1, стенд, ночные прогоны, очистка Driver Store, пофазное выполнение, публичный docs/, `-SkipDiskCleanup`) +### v2.22 - В РАБОТЕ (ветка `feature/v2.22`) + +Эпик **`MyAI-bt2h`**. Свои задачи: `MyAI-zfwv` (P1), `MyAI-r6cd` (P2), `MyAI-1qtn` (P3). +Внешние замечания двух команд разобраны критически, подтверждённые заведены как +`MyAI-bt2h.1` (лог), `.2` (install.ps1), `.3` (единый выход), `.4` (доки). + +🔴 **Два замечания ОТКЛОНЕНЫ проверкой, а не мнением - не переоткрывать:** + +| Замечание | Чем опровергнуто | +|---|---| +| Приоритет `-in`/`-and` в `Invoke-StandTest.ps1` якобы пропускает `ReportNoCleanup` | AST показал группировку `($Mode -in @(...)) -and (-not ...)`, таблица истинности верна. Скобки добавлены только для читаемости, поведение прежнее | +| Публичный README на GitHub якобы отстаёт (6 браузеров, v2.16) | Запрошен `README.md` с `main` через GitHub API: 7 браузеров, `-SkipDiskCleanup`, диаграмма v2.21. Аудитор смотрел устаревший снимок. Вдобавок покрыто гейтом транзитивно (чистое дерево + синк с origin) | + +🔴 **Отклонено и предложение `MyAI-1qtn`** (доверять включённому Storage Sense и не +запускать cleanmgr): проверка показала, что вооружаемые категории включают **Update +Cleanup, дампы памяти, Language Pack, старые ChkDsk, WER** - ничего из этого Storage Sense +не делает. Регулярно обслуживаемая машина молча перестала бы их чистить. Реализована +наблюдаемость (лог различает «выключен» и «включён, но чистить нечего»), решение не +тронуто; на это есть тест-страж в `Fixes.Tests.ps1`. + +🔴 **Durable-уроки этого релиза:** +1. **Мутационная проверка нашла то, чего не нашли 600+ тестов.** Три прогона, 30 мутаций; + восемь сначала ВЫЖИЛИ, и каждая вскрыла реальный пробел, а не требовала подгонки: + `Should -Invoke -Times N` при N>0 означает **«не менее N»** и не видит дубля (`-Times 0` + при этом строгий - проверено экспериментом, поэтому тесты v2.21 править не пришлось); + ветка `catch` (сломанный WMI) не была покрыта вовсе; ничто не пиновало выравнивание + логотипа и центрирование заголовка. Одна мутация оказалась **эквивалентной** - записана + как таковая, а не «починена» подгонкой теста. +2. **Мои гипотезы опровергались замером трижды.** Ожидал, что `Out-File` даст + non-terminating error - **шесть из семи** плохих путей бросают terminating. Ожидал, что + `-Times 0` вакуумен - он строгий. Ожидал, что `Show-Banner` использует общую ширину - он + рисовал жёсткие 70. +3. **Греп-тест дважды поймал паттерн в собственном комментарии.** Оба раза чинилось + фильтрацией строк-комментариев, а не переписыванием комментария. + ### v2.21 - ВЫПУЩЕНА 23.07.2026 Задачи `MyAI-3y5i` (P1), `MyAI-iodp`, `MyAI-7iej`. Релиз про честность на выходе: авто-обновление меняло не тот файл, который выполняется, и печатало «Update complete», а код возврата объявлял провалом успешный прогон, если на машине не было winget или сети. Каналы синхронны: `main`, GitHub Release v2.21 с обоими ассетами, **PSGallery 2.21**, CHANGELOG. Гейт **14/14** с `-VerifyPublished`, стенд Full на обеих VM (RU 1219 МБ, EN 2724 МБ), e2e через опубликованный one-liner (режим подтверждён из result JSON). 452 -> 573 теста, 21 мутация - все пойманы. @@ -399,7 +434,7 @@ pwsh tools/Invoke-ReleaseCheck.ps1 # версия во всех pwsh tools/Invoke-ReleaseCheck.ps1 -IncludeStand # + боевой прогон на VM (минуты) pwsh tools/Invoke-ReleaseCheck.ps1 -VerifyPublished # ПОСЛЕ выпуска: ассеты релиза и SHA256 -Invoke-Pester ./tests -Output Detailed # 573 Pester тестов (считать прогоном, не грепом) +Invoke-Pester ./tests -Output Detailed # 702 Pester тестов (считать прогоном, не грепом) pwsh tools/Invoke-SmokeTest.ps1 # Смоук: ReportOnly + геометрия UI pwsh tools/proxmox/Invoke-StandTest.ps1 -Mode Report # Стенд на Proxmox (RU=VM 190, EN: -ConfigPath ...en.json = VM 191) # Ночная матрица: cron 03:30 на proxmos (/opt/winclean-stand, /etc/cron.d/winclean-stand), отчёт в Telegram diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6a015f5..7c40ffc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -128,7 +128,7 @@ Invoke-Pester ./tests/Integration.Tests.ps1 All PRs automatically run: - PSScriptAnalyzer (linting) - Syntax check -- Pester tests (573 tests) +- Pester tests (702 tests) ### Release-impacting changes @@ -145,6 +145,39 @@ then before opening the PR: The full release runbook is in [docs/release-process.md](docs/release-process.md). +### What gets accepted + +WinClean runs elevated on other people's working machines, so the bar is about +consequences rather than about size. Before proposing a new cleanup target or behaviour, +it helps to have answers to these: + +- **Safety.** What exactly is deleted, and what could a user lose if the assumption behind + it is wrong on their machine? Anything that deletes must respect `$script:ProtectedPaths`. +- **Preview.** Does `-ReportOnly` describe it accurately without changing anything? +- **Honesty of the report.** Can the step tell "it worked" from "it could not tell"? An + operation that cannot verify itself should say so rather than report success - several + past releases were spent removing exactly that. +- **Tests.** Behavioural tests, not just a grep for the new string. A test that passes + when the feature is deleted is worse than no test. +- **Result JSON.** Does anything change in the schema that automation reads? +- **Verification.** Does it need a real Windows VM to be believed? Anything touching + Windows Update, drivers, Storage Sense, Docker/WSL or locale-dependent output does, and + is checked on the ru-RU and en-US stands before release. + +Ideas that need discussion before code are welcome as a +[Discussion](https://github.com/bivlked/WinClean/discussions) - it is much cheaper to +agree on the acceptance criteria there than after an implementation exists. + +Deliberately out of scope: registry "optimisation", service disabling, telemetry blocking +beyond the documented Group Policy switch, and anything whose benefit cannot be measured +on a stand. + +### Conduct + +Be straightforward and civil - review comments are about the code, not the person. There +is no separate code of conduct document for a project this size; if that ever stops being +enough, one will be added. + ### Commit Messages Follow [Conventional Commits](https://www.conventionalcommits.org/): @@ -251,7 +284,7 @@ Invoke-Pester ./tests/Integration.Tests.ps1 Все PR автоматически проходят: - PSScriptAnalyzer (линтинг) - Проверка синтаксиса -- Pester тесты (573 тестов) +- Pester тесты (702 тестов) ### Изменения, влияющие на релиз diff --git a/README.md b/README.md index 60b5850..3d528c2 100644 --- a/README.md +++ b/README.md @@ -321,7 +321,7 @@ WinClean is built to be safe to run on a working machine. The short version: ``` ┌────────────────────────────────────────────────────────────────┐ -│ WinClean v2.21 │ +│ WinClean v2.22 │ ├────────────────────────────────────────────────────────────────┤ │ PREPARATION │ │ ├─ ✓ Check Administrator Rights │ diff --git a/README_RU.md b/README_RU.md index 186589e..e1bfd5a 100644 --- a/README_RU.md +++ b/README_RU.md @@ -321,7 +321,7 @@ WinClean создан так, чтобы его можно было безопа ``` ┌────────────────────────────────────────────────────────────────┐ -│ WinClean v2.21 │ +│ WinClean v2.22 │ ├────────────────────────────────────────────────────────────────┤ │ ПОДГОТОВКА │ │ ├─ ✓ Проверка прав администратора │ diff --git a/SUPPORT.md b/SUPPORT.md new file mode 100644 index 0000000..d91acf2 --- /dev/null +++ b/SUPPORT.md @@ -0,0 +1,61 @@ +# Support + +WinClean runs elevated and performs system maintenance, so a good report is worth more +here than in most projects: it is often the only way to tell "WinClean did not clean X" +from "there was nothing left of X to clean". + +## Start here + +| Question | Where to look | +|:---------|:--------------| +| Something failed, or a step did nothing | [docs/troubleshooting.md](docs/troubleshooting.md) | +| Is this safe? What exactly gets deleted? | [docs/safety.md](docs/safety.md), [docs/what-is-cleaned.md](docs/what-is-cleaned.md) | +| General questions (Windows 10, how often to run it, rollback) | [docs/faq.md](docs/faq.md) | +| What the run summary fields mean | [docs/result-json.md](docs/result-json.md) | + +## Where to go next + +**A reproducible defect** - open a [bug report](https://github.com/bivlked/WinClean/issues/new?template=bug_report.md). +Something behaved differently from what the documentation says, and you can describe the +steps that produced it. + +**A question, an idea, or a story about how you use it** - +open a [Discussion](https://github.com/bivlked/WinClean/discussions). Questions like "why +does winget keep offering the same package" or "which profile should I use" are answered +there, and feature ideas are easier to shape in a conversation than in an issue. + +**A security vulnerability** - do **not** open a public issue. Use private reporting as +described in [SECURITY.md](SECURITY.md). + +## What to include + +Most of this comes straight from the run, and it is what turns "it did not work" into +something that can actually be diagnosed: + +- **WinClean version** - the banner prints it, and it is also `Version` in the result JSON +- **How you installed it** - `get.ps1` one-liner, `install.ps1` + shortcut, PowerShell + Gallery, or a manual download +- **Windows version** - `winver`, e.g. Windows 11 24H2 (build 26100) +- **PowerShell version** - `$PSVersionTable.PSVersion` +- **Whether the shell was elevated**, and whether you passed `-ReportOnly` +- **The log** from `%TEMP%\WinClean_.log` - the relevant part is enough; it contains + file paths from your machine, so read it before pasting +- **The result JSON**, if you ran with `-ResultJsonPath`. This is the single most useful + artefact: it records which phases ran, which were skipped, which failed, and the + warning and error counts + +If a cleanup step freed less than you expected, `-ReportOnly` output from the same machine +is very helpful too: it shows what WinClean believed was there to clean. + +## What is not supported + +- Windows versions before 10, and PowerShell 5.1 (WinClean requires PowerShell 7.1+) +- Running without administrator rights - most maintenance operations simply cannot work +- Modified copies. If you changed `WinClean.ps1`, please reproduce the problem with an + unmodified release first, so it is clear whose behaviour is being discussed. + +## Response times + +This is a personal open-source project maintained in spare time. Issues and discussions +are read, but there is no service level attached to them. Security reports are looked at +first. diff --git a/WinClean.ps1 b/WinClean.ps1 index 84925f1..415cb28 100644 --- a/WinClean.ps1 +++ b/WinClean.ps1 @@ -1,5 +1,5 @@ <#PSScriptInfo -.VERSION 2.21 +.VERSION 2.22 .GUID 8f7c3b2a-1d4e-5f6a-9b8c-0d1e2f3a4b5c .AUTHOR bivlked .COMPANYNAME @@ -12,6 +12,7 @@ .REQUIREDSCRIPTS .EXTERNALSCRIPTDEPENDENCIES .RELEASENOTES + v2.22: Honest completion and adaptive output - a finished Disk Cleanup is no longer reported as partial, a bad log path can no longer kill a run, every run now ends through one path, and the console output follows the window width v2.21: Self-update targeting and honest exit codes - the update could change a file that was not the one running and still report success; a missing winget or no connectivity no longer fails the run v2.20: Correctness and honesty round - a junction could bypass protected-path checks, four operations reported success while doing nothing, Storage Sense was unreachable so on machines where it works the slow Disk Cleanup no longer runs; where it fails, the new -SkipDiskCleanup is what removes the wait v2.19: Contract and documentation round - -SkipCleanup now skips ALL cleanup categories, result JSON gains a tri-state PhasesSkipped, AppUpdatesCount renamed to AppUpdatesOffered (offered, not installed), full docs overhaul @@ -30,7 +31,7 @@ <# .SYNOPSIS - WinClean - Ultimate Windows 11 Maintenance Script v2.21 + WinClean - Ultimate Windows 11 Maintenance Script v2.22 .DESCRIPTION Комплексный скрипт для обновления и очистки Windows 11: - Обновление Windows (включая драйверы) @@ -44,8 +45,21 @@ - Подробный цветной вывод + лог-файл .NOTES Author: biv - Version: 2.21 + Version: 2.22 Requires: PowerShell 7.1+, Windows 11, Administrator rights + Changes in 2.22: + - A Disk Cleanup that stops doing anything but never exits is no longer waited out + for the full 15-minute timeout, nor reported as still deleting: the wait now also + ends when the process has shown no CPU or I/O activity for two minutes + - An unusable -LogPath can no longer kill the run before it starts - the log header + is written through the same fault-tolerant path as every other log line + - Every run now ends through one code path, so the result JSON, the summary and the + log handle release cannot drift apart between normal and self-update exits + - Console output adapts to the window width: the window is widened best-effort at + startup and the frames follow it, so winget's table stops wrapping into them + - install.ps1 refuses a PowerShell 7 whose version it cannot read, instead of + treating "could not check" as "good enough" + Changes in 2.21: - The self-update could update a DIFFERENT copy than the one running and report success - it asked whether a Gallery copy existed anywhere, not whether the @@ -370,9 +384,21 @@ function New-RunStats { # 'unknown' must not be mistaken for a verified state by consumers of the JSON ControlledFolderAccess = 'unknown' Aborted = $null # v2.17: set when the run stops before finishing - # v2.20: cleanmgr outlived its timeout and is still deleting in the background. The - # totals reported by this run are partial, and a consumer must not read them as final. + # v2.20: the wait expired without cleanmgr either exiting or being seen to go idle, + # so it was left running. The totals may be partial and must not be read as final. + # Note this does NOT assert it was observed working - the same branch catches a + # machine where activity could not be measured at all (raised in review). DiskCleanupPending = $false + # v2.22: how the Storage Sense / Disk Cleanup step actually ended. DiskCleanupPending + # alone could not tell "still visibly deleting" from "resident but doing nothing", and + # reported both as pending - so a cleanup nothing had been observed to be doing was + # published as partial. + # Same shape and reasoning as AppUpdatesStatus (v2.21): when a boolean starts covering + # two different truths, the fix is a status, not a cleverer boolean. + # 'not-run' | 'skipped-parameter' | 'skipped-cleanup-group' | 'skipped-report-only' | + # 'storage-sense' | 'running' | 'completed' | 'idle-resident' | 'timeout' | + # 'not-armed' | 'start-failed' | 'exit-nonzero' + DiskCleanupStatus = 'not-run' # v2.17 (p.11 of the audit): which top-level phases ran to completion vs threw. # Before this, one exception anywhere in the run silently skipped every phase # after it - Developer Cleanup, Docker/WSL, Visual Studio, Deep System Cleanup, @@ -403,6 +429,11 @@ $script:InternetConnectionCache = $null # once instead of on every call - and surfaces in the result JSON as LoggingDegraded $script:LogWriteFailed = $false +# Inner width of every framed section. v2.22: was the literal 70 repeated in four places; +# Start-WinClean now derives it once from the actual console. 70 stays the default so a +# host whose width cannot be read looks exactly as it always did. +$script:BoxWidth = 70 + # Initialize log path (script scope for access in functions) if (-not $LogPath) { $script:LogPath = Join-Path $env:TEMP "WinClean_$((Get-Date).ToString('yyyyMMdd_HHmmss')).log" @@ -416,7 +447,7 @@ if (-not $LogPath) { } # Script version (single source of truth for version checking) -$script:Version = "2.21" +$script:Version = "2.22" # Protected paths that should never be deleted $script:ProtectedPaths = @( @@ -436,6 +467,81 @@ $script:ProtectedPaths = @( # LOGGING FUNCTIONS #═══════════════════════════════════════════════════════════════════════════════ +function Write-LogFileLine { + <# + .SYNOPSIS + Appends one line to the log file, degrading instead of throwing + .DESCRIPTION + v2.22, raised in external review: extracted from Write-Log so that EVERY write to + the log file - including the header, which Start-WinClean emits before its main + try/finally exists - degrades the same way instead of each caller inventing its + own fault tolerance. + + The header used to be two bare Out-File calls. Measured, not assumed: six of seven + bad log paths make Out-File throw a TERMINATING error even though the script leaves + ErrorActionPreference at Continue (missing directory, path is a directory, invalid + characters, colon in the name, over-long path, unreachable UNC; only a reserved + device name did not). Thrown there, before the safety net, the exception escaped + Start-WinClean entirely: no result JSON, no final summary, no exit-code accounting, + and none of the maintenance the run was started for - all because of the log. + + A log that cannot be written is a degraded run, never a failed one. + #> + param( + [Parameter(Mandatory)] + [AllowEmptyString()] + [string]$Line, + + # The header line starts a fresh file. The Out-File call this replaced had no + # -Append, so it truncated on every run; preserved deliberately rather than lost + # in the refactor, or a custom -LogPath reused across runs would accumulate them + # all into one file and the log would no longer describe a single run. + [switch]$StartNewFile + ) + + # v2.17 (p.7 of the audit): Out-File used to open, seek to end, write and close the + # file on every single call - Write-Log fires hundreds of times per run. A StreamWriter + # kept open for the run avoids that, with AutoFlush so each line still lands on disk + # immediately (same durability as before, just cheaper). FileShare.Delete matters for + # tests: they Remove-Item the log path in AfterAll while this writer may still be the + # last one that touched it. + try { + if ($StartNewFile -or -not $script:LogWriter -or $script:LogWriterPath -ne $script:LogPath) { + if ($script:LogWriter) { $script:LogWriter.Dispose() } + $mode = if ($StartNewFile) { [System.IO.FileMode]::Create } else { [System.IO.FileMode]::Append } + $fileStream = [System.IO.File]::Open( + $script:LogPath, $mode, [System.IO.FileAccess]::Write, + ([System.IO.FileShare]::ReadWrite -bor [System.IO.FileShare]::Delete)) + $script:LogWriter = [System.IO.StreamWriter]::new($fileStream, [System.Text.Encoding]::UTF8) + $script:LogWriter.AutoFlush = $true + $script:LogWriterPath = $script:LogPath + } + $script:LogWriter.WriteLine($Line) + } catch { + # v2.20: this used to be an empty catch, so a log that stopped being written + # (full volume, revoked permissions, the v2.14 case where cleanup deleted the + # log out from under us) was invisible: destructive work carried on, the final + # JSON said ErrorsCount=0, and LogPath pointed at a truncated file. + # + # Latched: one console line, not one per call - Write-Log fires hundreds of + # times per run. Deliberately Write-Host and not Write-Log, which would + # recurse straight back into this catch. + # v2.20, corrected in review: drop the writer so the NEXT call reopens it. + # Without this the guard above stays satisfied by a dead writer object and + # every later line is silently discarded for the rest of the run - the empty + # catch would simply have moved from the first failure to all the others. + try { if ($script:LogWriter) { $script:LogWriter.Dispose() } } catch { } + $script:LogWriter = $null + $script:LogWriterPath = $null + + if (-not $script:LogWriteFailed) { + $script:LogWriteFailed = $true + $script:Stats.WarningsCount++ + Write-Host " [WARN] Log file could not be written ($($_.Exception.Message)) - the run continues, but $($script:LogPath) may be incomplete" -ForegroundColor Yellow + } + } +} + function Write-Log { <# .SYNOPSIS @@ -456,52 +562,13 @@ function Write-Log { # Consistent left indent for all output (matches banner style) $indent = " " - $boxWidth = 70 # Inner width for framed sections + $boxWidth = $script:BoxWidth # Inner width for framed sections (v2.22: adaptive) $timestamp = (Get-Date).ToString('HH:mm:ss') $logMessage = "[$timestamp] [$Level] $Message" - # Write to log file. v2.17 (p.7 of the audit): Out-File used to open, seek to end, - # write and close the file on every single call - Write-Log fires hundreds of times - # per run. A StreamWriter kept open for the run avoids that, with AutoFlush so each - # line still lands on disk immediately (same durability as before, just cheaper). - # FileShare.Delete matters for tests: they Remove-Item the log path in AfterAll while - # this writer may still be the last one that touched it. if (-not $NoLog) { - try { - if (-not $script:LogWriter -or $script:LogWriterPath -ne $script:LogPath) { - if ($script:LogWriter) { $script:LogWriter.Dispose() } - $fileStream = [System.IO.File]::Open( - $script:LogPath, [System.IO.FileMode]::Append, [System.IO.FileAccess]::Write, - ([System.IO.FileShare]::ReadWrite -bor [System.IO.FileShare]::Delete)) - $script:LogWriter = [System.IO.StreamWriter]::new($fileStream, [System.Text.Encoding]::UTF8) - $script:LogWriter.AutoFlush = $true - $script:LogWriterPath = $script:LogPath - } - $script:LogWriter.WriteLine($logMessage) - } catch { - # v2.20: this used to be an empty catch, so a log that stopped being written - # (full volume, revoked permissions, the v2.14 case where cleanup deleted the - # log out from under us) was invisible: destructive work carried on, the final - # JSON said ErrorsCount=0, and LogPath pointed at a truncated file. - # - # Latched: one console line, not one per call - Write-Log fires hundreds of - # times per run. Deliberately Write-Host and not Write-Log, which would - # recurse straight back into this catch. - # v2.20, corrected in review: drop the writer so the NEXT call reopens it. - # Without this the guard above stays satisfied by a dead writer object and - # every later line is silently discarded for the rest of the run - the empty - # catch would simply have moved from the first failure to all the others. - try { if ($script:LogWriter) { $script:LogWriter.Dispose() } } catch { } - $script:LogWriter = $null - $script:LogWriterPath = $null - - if (-not $script:LogWriteFailed) { - $script:LogWriteFailed = $true - $script:Stats.WarningsCount++ - Write-Host " [WARN] Log file could not be written ($($_.Exception.Message)) - the run continues, but $($script:LogPath) may be incomplete" -ForegroundColor Yellow - } - } + Write-LogFileLine -Line $logMessage } # Console output with colors @@ -536,7 +603,7 @@ function Write-Log { Write-Host "" Write-Host "$indent┌─ " -NoNewline -ForegroundColor DarkGray Write-Host $Message -ForegroundColor $tagColors.Message - Write-Host "$indent└$("─" * 70)" -ForegroundColor DarkGray + Write-Host "$indent└$("─" * $boxWidth)" -ForegroundColor DarkGray } 'DETAIL' { # Detail line with vertical bar @@ -768,6 +835,96 @@ function Test-InteractiveConsole { } } +function Get-ConsoleWidth { + <# + .SYNOPSIS + The console width in columns, or 0 when it cannot be determined + .DESCRIPTION + v2.22. Returns 0 when there is no console to measure - a scheduled task, a service, + or any host without one - and the caller then falls back to the historical fixed + width rather than laying out against a guess. + + Measured, because the obvious assumption is wrong: REDIRECTING output does NOT make + this return 0. $Host.UI.RawUI reads the attached console, not stdout, so + `pwsh -File script.ps1 *> out.txt` from a terminal still reports that terminal's + width. The smoke test therefore exercises the adaptive width, not the 70 fallback. + #> + try { + $width = $Host.UI.RawUI.WindowSize.Width + if ($width -gt 0) { return [int]$width } + } catch { } + return 0 +} + +function Get-BoxWidth { + <# + .SYNOPSIS + Inner width for framed sections, derived from the console width + .DESCRIPTION + v2.22, pure so the bounds are testable. A box is printed as two spaces of indent, + a border character, the inner width, and a closing border character - so it needs + ConsoleWidth-4 at the very most, and 6 is subtracted to keep a margin away from + the wrap column. + + Bounded both ways on purpose: + - never below 70, the width every previous version used. A narrow console cannot + be laid out well either way, and shrinking below 70 would change how WinClean + looks for everyone who has been reading it at 70 for ten releases. + - never above 90. Raising the ceiling further makes the frames span the screen on + a wide monitor, which reads worse rather than better - the user asked for + "somewhat wider", not "as wide as it goes". + #> + param([int]$ConsoleWidth) + + if ($ConsoleWidth -le 0) { return 70 } + return [math]::Max(70, [math]::Min(90, $ConsoleWidth - 6)) +} + +function Expand-ConsoleWindow { + <# + .SYNOPSIS + Best-effort widening of the console window at startup + .DESCRIPTION + v2.22. The desktop shortcut opens conhost at its 120-column default, and winget's + upgrade table needs about 140 on a localised system - so it wrapped, and every + wrapped row landed across the script's own output. Measured on the reporting + machine: window 120x30, buffer 120x9001, maximum physical window 3824 columns. + There was room; nobody had asked for it. + + Deliberately done here rather than by writing console properties into the .lnk: + the shortcut route only fixes the shortcut (and NT_CONSOLE_PROPS is a binary blob + WScript.Shell will not write), while this helps every way of starting the script. + + Best-effort by design. Windows Terminal ignores or refuses programmatic resizing, + redirected hosts throw - none of that is worth a warning, let alone a failed run. + The buffer is grown before the window because a window may never exceed its + buffer; the reverse order fails. + #> + param([int]$DesiredWidth = 140) + + try { + $raw = $Host.UI.RawUI + $current = $raw.WindowSize + if ($current.Width -le 0 -or $current.Width -ge $DesiredWidth) { return } + + # Never ask for more than the screen can physically show + $target = [math]::Min($DesiredWidth, $raw.MaxPhysicalWindowSize.Width) + if ($target -le $current.Width) { return } + + $buffer = $raw.BufferSize + if ($buffer.Width -lt $target) { + $buffer.Width = $target + $raw.BufferSize = $buffer + } + + $window = $raw.WindowSize + $window.Width = $target + $raw.WindowSize = $window + } catch { + # Host refused; the run continues at whatever width it already had + } +} + function Test-InternetConnection { <# .SYNOPSIS @@ -1450,7 +1607,7 @@ function Invoke-ScriptUpdate { # Dynamically centered title in a 70-char box (matches the rest of the UI; v2.14 # fixes a misaligned right border caused by hardcoded padding) - $boxWidth = 70 + $boxWidth = $script:BoxWidth $updateTitle = "UPDATE AVAILABLE" $titlePadding = [math]::Max(0, $boxWidth - $updateTitle.Length) $titleLeftPad = [math]::Floor($titlePadding / 2) @@ -1486,7 +1643,7 @@ function Invoke-ScriptUpdate { Write-Host $line -ForegroundColor Gray } Write-Host "" - return + return $false } # Check if interactive console is available @@ -1500,7 +1657,7 @@ function Invoke-ScriptUpdate { Write-Host $line -ForegroundColor Gray } Write-Host "" - return + return $false } if ($UpdateInfo.Channel -ne 'gallery') { @@ -1525,7 +1682,7 @@ function Invoke-ScriptUpdate { Write-Host " Press any key to continue with current version..." -ForegroundColor DarkGray Wait-ForKeyPress Write-Host "" - return + return $false } # The running file is the Gallery copy, so updating it updates what runs next @@ -1538,7 +1695,7 @@ function Invoke-ScriptUpdate { Write-Log "Update skipped by user" -Level INFO Write-Host " Update skipped. Continuing with current version..." -ForegroundColor DarkGray Write-Host "" - return + return $false } Write-Host "" @@ -1547,7 +1704,12 @@ function Invoke-ScriptUpdate { try { # PowerShellGet ships with PowerShell today and PSResourceGet is its replacement; # either can be the one present, and the one that answered discovery goes first - switch (Select-UpdateCommand -Provider $UpdateInfo.Provider) { + # $null = ... deliberately: v2.22 made this function's return value meaningful + # ("the run is over"), and a bare switch emits whatever the update provider writes + # to the pipeline. A provider that returned an object would turn $true into an + # array, and `if (Invoke-ScriptUpdate ...)` would then be deciding on the array's + # truthiness rather than on the answer this function meant to give. + $null = switch (Select-UpdateCommand -Provider $UpdateInfo.Provider) { 'Update-Script' { Update-Script -Name WinClean -Force -ErrorAction Stop } 'Update-PSResource' { Update-PSResource -Name WinClean -Force -TrustRepository -ErrorAction Stop } default { throw "no update command available (neither Update-Script nor Update-PSResource)" } @@ -1562,7 +1724,7 @@ function Invoke-ScriptUpdate { Write-Host " ✗ Update failed: $_" -ForegroundColor Red Write-Host " Continuing with current version..." -ForegroundColor Yellow Write-Host "" - return + return $false } # v2.21: verify against the file being executed. A provider that reports success @@ -1587,7 +1749,7 @@ function Invoke-ScriptUpdate { Write-Host $line -ForegroundColor Gray } Write-Host "" - return + return $false } Write-Log "Update successful" -Level SUCCESS @@ -1595,22 +1757,21 @@ function Invoke-ScriptUpdate { Write-Host " ✓ Update complete!" -ForegroundColor Green Write-Host " Please run WinClean again to use the new version." -ForegroundColor Gray Write-Host "" - Write-Host " Press any key to exit..." -ForegroundColor DarkGray - # This exit bypasses the finally in Start-WinClean, so the result JSON has to be - # written here (raised in review). Start-WinClean deletes any previous file before the - # update check, so without this a consumer saw exit code 0 and NO artefact at all - - # indistinguishable from a crash, and identical to a run that did nothing. Same shape - # as the PendingRebootDeclined path, which already does this. + # v2.22: this used to call exit here, which bypassed the finally in Start-WinClean and + # therefore had to hand-copy the result JSON write and the exit-code rule alongside it. + # The caller now owns ending the run, through the one path that does it (raised in + # external review). The exit code is unchanged: the entry point already derives it from + # ErrorsCount, which is what the copied lines here were re-deriving. + # + # The "press any key" pause deliberately does NOT happen here (raised in the second + # review pass): it blocks indefinitely, and in v2.21 the result JSON was already on + # disk before it. Pausing here would put an unbounded wait between the update and the + # artefacts, so a window closed or interrupted at that prompt would leave the run with + # no result JSON and an unreleased log handle - a regression introduced by this very + # refactor. The caller pauses after the run is complete. $script:Stats.Aborted = 'UpdatedAndExited' - Write-ResultJson -Path $ResultJsonPath - - Wait-ForKeyPress - # Not an unconditional 0 (raised in review): Write-ResultJson reports its own failure by - # counting an error rather than throwing, and exiting 0 anyway would rebuild the exact - # "success with no artefact" state the JSON write was added here to prevent. - if ($script:Stats.ErrorsCount -gt 0) { exit 1 } - exit 0 + return $true } function Install-ModuleWithTimeout { @@ -5175,6 +5336,210 @@ function Invoke-DISMCleanup { } } +function Get-DiskCleanupActivityFingerprint { + <# + .SYNOPSIS + Activity fingerprint of the whole cleanmgr family, not just the process we started + .DESCRIPTION + v2.22, raised in review, then narrowed in the round after that. + + The concern is real: the handle we hold is not necessarily the only process doing + the work - cleanmgr was started with -WindowStyle Hidden and a Disk Cleanup window + appeared anyway. If a worker does the deleting while the process we started merely + waits, fingerprinting our PID alone would show a frozen parent and call a live + cleanup finished. + + The first attempt aggregated over every cleanmgr.exe on the machine, and that was + WORSE: a Disk Cleanup the user opens by hand is also called cleanmgr.exe, so an + unrelated process could both satisfy "activity was observed" for our run and, by + staying busy, keep our run waiting the full fifteen minutes after our own cleanup + had finished. It made the verdict depend on a process we know nothing about. + + So: our process and its descendants, and nothing else. Same protection against a + worker doing the deleting, no dependence on strangers. Children are matched by + parent PID regardless of name, since a worker need not be called cleanmgr.exe. + + Counters are summed as [long] rather than through Measure-Object, which returns a + Double and would silently lose integer precision past 2^53 (raised in review). + + $null when nothing can be read; the caller must treat that as "cannot tell". + + ACCEPTED LIMITS, stated rather than implied (raised in review). Process identity on + Windows is not exact: a process whose parent died and whose parent's PID was later + reused by our cleanmgr would be counted as family, and a worker started through a + service or COM rather than as a child would be missed. No process-tree scheme + avoids both. This is why the outcome is reported as observed idleness rather than + as proven completion, and why the registry sweep still refuses to touch a cleanmgr + that has not exited: every consumer of this signal is built to tolerate it being + wrong, which is the property that actually matters. + #> + param([int]$ProcessId) + + try { + $all = @(Get-CimInstance -ClassName Win32_Process ` + -Property ProcessId, ParentProcessId, KernelModeTime, UserModeTime, + ReadOperationCount, WriteOperationCount, OtherOperationCount ` + -ErrorAction Stop) + if (-not $all) { return $null } + + # Walk down from our PID. The seen-set both prevents rework and makes a cyclic + # parent chain (possible after PID reuse) terminate instead of spinning. + $byParent = @{} + foreach ($p in $all) { + $parent = [int]$p.ParentProcessId + if (-not $byParent.ContainsKey($parent)) { $byParent[$parent] = [System.Collections.Generic.List[object]]::new() } + $byParent[$parent].Add($p) + } + + $family = [System.Collections.Generic.List[object]]::new() + $seen = [System.Collections.Generic.HashSet[int]]::new() + $pending = [System.Collections.Generic.Queue[int]]::new() + $pending.Enqueue($ProcessId) + + while ($pending.Count -gt 0) { + $current = $pending.Dequeue() + if (-not $seen.Add($current)) { continue } + + $proc = $all | Where-Object { [int]$_.ProcessId -eq $current } | Select-Object -First 1 + if ($proc) { $family.Add($proc) } + + if ($byParent.ContainsKey($current)) { + foreach ($child in $byParent[$current]) { $pending.Enqueue([int]$child.ProcessId) } + } + } + + if ($family.Count -eq 0) { + # Our process is gone. The caller detects the exit on its own; "cannot tell" + # is the honest answer here, never "idle". + return $null + } + + [long]$kernel = 0; [long]$user = 0; [long]$read = 0; [long]$write = 0; [long]$other = 0 + foreach ($p in $family) { + $kernel += [long]$p.KernelModeTime + $user += [long]$p.UserModeTime + $read += [long]$p.ReadOperationCount + $write += [long]$p.WriteOperationCount + $other += [long]$p.OtherOperationCount + } + $pids = (($family | ForEach-Object { [int]$_.ProcessId } | Sort-Object) -join ',') + + # Two segments separated by '#': the ACTIVITY part (counters) and the IDENTITY + # part (how many processes, and which). Raised in review: identity belongs in the + # fingerprint - a child appearing or exiting is a state change and must reset the + # idle streak - but it is NOT work. Letting it satisfy "activity was observed" + # would qualify a cleanmgr that merely spawned a helper and then blocked forever. + # The caller compares the whole string for stillness, the activity part alone for + # "did it ever do anything". + return '{0}|{1}|{2}|{3}|{4}#{5}|{6}' -f $kernel, $user, $read, $write, $other, $family.Count, $pids + } catch { + return $null + } +} + +function Update-IdleStreak { + <# + .SYNOPSIS + Counts consecutive checks in which a process did nothing at all + .DESCRIPTION + v2.22, pure so the rule is testable without a process. Returns the new streak + length: one longer when the two fingerprints match, zero otherwise. + + An unreadable fingerprint on either side resets the streak. That is the whole + safety property: "I could not measure it" must never accumulate towards "it has + finished", or a machine with broken WMI would cut every Disk Cleanup short. + #> + param( + [AllowNull()][string]$Previous, + [AllowNull()][string]$Current, + [int]$Streak + ) + + if ([string]::IsNullOrEmpty($Previous) -or [string]::IsNullOrEmpty($Current)) { return 0 } + if ($Current -ceq $Previous) { return $Streak + 1 } + return 0 +} + +function Wait-CleanmgrCompletion { + <# + .SYNOPSIS + Waits for Disk Cleanup to finish its work, which is not the same as exiting + .DESCRIPTION + v2.22, split out in the style of Wait-StorageSenseTask so the wait can be tested + without a process and without waiting fifteen minutes: the caller injects how to + tell whether the process exited, how to read its activity, and how to wait. + + Two signals for "is there still something to wait for", because HasExited alone + was the wrong model of it. Note the distinction that matters throughout this + function: the MEASURED case below is described as what it was on that machine, + while what this code can PROVE in general is only the absence of observable + activity. The first is history, the second is the contract. + Measured on a live workstation: cleanmgr /sagerun did its work in about ten + seconds, closed its window, then stayed resident with CPU and all three I/O + counters frozen and every thread in Wait. The run sat out the remaining ~890 + seconds and then published the finished cleanup as partial. + + Returns Outcome ('exited' | 'idle-resident' | 'timeout') and Elapsed seconds. + 'idle-resident' names what was OBSERVED - the process was seen working and then + did nothing at all for long enough - not the conclusion drawn from it. That the + work is therefore over is an inference, and a process blocked on something + external would look identical, so nothing is built to depend on it being right. + 'timeout' means it was still visibly working when time ran out. + + 'idle-resident' additionally requires that activity was OBSERVED at least once + (raised in review). Without that, "it has finished" and "it has not started yet" + are the same observation - a process that is suspended, or waiting on something + before it begins, would be declared complete after two minutes of a stillness that + never followed any work. Since the first fingerprint is taken the moment cleanmgr + starts, a run that does anything at all changes it within the first interval. + #> + param( + [scriptblock]$HasExited, + [scriptblock]$GetFingerprint, + [int]$MaxWaitSeconds = 900, + [int]$CheckInterval = 10, + [int]$IdleChecksRequired = 12, + [scriptblock]$OnProgress = { param($seconds) }, + [scriptblock]$Wait = { param($seconds) Start-Sleep -Seconds $seconds } + ) + + $elapsed = 0 + $idleStreak = 0 + $sawActivity = $false + $fingerprint = & $GetFingerprint + + while (-not (& $HasExited) -and $elapsed -lt $MaxWaitSeconds) { + & $Wait $CheckInterval + $elapsed += $CheckInterval + + $previousFingerprint = $fingerprint + $fingerprint = & $GetFingerprint + $idleStreak = Update-IdleStreak -Previous $previousFingerprint -Current $fingerprint -Streak $idleStreak + + # Real work, and only real work, opens the gate (raised in review). Two readable + # fingerprints differing is not enough: they also differ when a child process + # appears or exits, which is a state change but not work. Compare the activity + # segment (before '#') alone, so a cleanmgr that spawns a helper and then blocks + # forever cannot qualify for the early verdict. An unreadable sample never counts. + if ($previousFingerprint -and $fingerprint) { + $previousWork = ($previousFingerprint -split '#')[0] + $currentWork = ($fingerprint -split '#')[0] + if ($currentWork -cne $previousWork) { $sawActivity = $true } + } + + if ($sawActivity -and $idleStreak -ge $IdleChecksRequired) { + return @{ Outcome = 'idle-resident'; Elapsed = $elapsed } + } + + if ($elapsed % 60 -eq 0) { & $OnProgress $elapsed } + } + + # Re-read rather than assume: the process may have exited during the last interval, + # and that is a cleaner answer than "timeout" for the same instant. + if (& $HasExited) { return @{ Outcome = 'exited'; Elapsed = $elapsed } } + return @{ Outcome = 'timeout'; Elapsed = $elapsed } +} + function Select-StorageSenseTask { <# .SYNOPSIS @@ -5194,6 +5559,31 @@ function Select-StorageSenseTask { return @{ Task = $found[0]; Reason = 'ok' } } +function Test-StorageSenseEnabled { + <# + .SYNOPSIS + Whether Storage Sense is switched on in Settings + .DESCRIPTION + v2.22. Tri-state on purpose: $true / $false / $null for "cannot tell". Used ONLY + to explain what happened, never to decide whether Disk Cleanup runs - see the note + in Invoke-StorageSense for why that distinction matters. + + Verified on a live Windows 11 25H2 (build 26200): the value is still + HKCU\...\StorageSense\Parameters\StoragePolicy\01, and it was not renamed. + + HKCU is the user's hive, so a run under SYSTEM (a scheduled task) reads a + different one and gets nothing - which is exactly why the answer must be allowed + to be $null instead of defaulting to "off". + #> + try { + $policy = Get-ItemProperty -Path 'HKCU:\Software\Microsoft\Windows\CurrentVersion\StorageSense\Parameters\StoragePolicy' ` + -Name '01' -ErrorAction Stop + return [bool][int]$policy.'01' + } catch { + return $null + } +} + function Get-StorageSenseVerdict { <# .SYNOPSIS @@ -5336,6 +5726,11 @@ function Invoke-StorageSense { # Docker/WSL, Visual Studio and the driver store - everything, to avoid one step. if ($ReportOnly) { Write-Log "Would run: Storage Sense" -Level DETAIL + # Its own value, not the 'not-run' default (raised in review): a preview is the + # most common way to reach this function - every smoke run and two of the stand + # modes use it - and 'not-run' is documented as "the step never executed, for + # example the run aborted earlier", which describes something else entirely. + $script:Stats.DiskCleanupStatus = 'skipped-report-only' return } @@ -5367,6 +5762,7 @@ function Invoke-StorageSense { # was added for. if ($SkipDiskCleanup) { Write-Log "Storage Sense / Disk Cleanup skipped (parameter)" -Level INFO + $script:Stats.DiskCleanupStatus = 'skipped-parameter' return } @@ -5478,7 +5874,25 @@ function Invoke-StorageSense { 'unreadable' { Write-Log "Storage Sense ran but its result could not be read - using Disk Cleanup as well" -Level INFO } 'failed' { Write-Log ("Storage Sense failed (task result 0x{0:X8}) - using Disk Cleanup instead" -f $taskResult) -Level INFO } 'not-measured' { Write-Log "Storage Sense reported success but free space could not be measured - running Disk Cleanup as well" -Level INFO } - 'nothing-freed' { Write-Log "Storage Sense reported success but freed nothing measurable - running Disk Cleanup as well" -Level INFO } + 'nothing-freed' { + # v2.22 (MyAI-1qtn): say WHICH of the two states this is. "Returned + # 0 and freed nothing" covers both "switched off, so it did nothing" + # and "switched on, ran, and there was nothing left to free" - and + # on a regularly maintained machine the second happens every time, + # which looked like a malfunction to the person reading the log. + # + # Reported, NOT acted on. The proposal was to trust an enabled + # Storage Sense and skip Disk Cleanup, and checking what that would + # cost settled it: the categories armed below include Update + # Cleanup, memory dumps, Language Pack, old ChkDsk files and Windows + # Error Reporting - none of which Storage Sense touches at all. A + # maintained machine would silently stop having them cleaned. + switch (Test-StorageSenseEnabled) { + $true { Write-Log "Storage Sense is enabled and ran, but there was nothing left for it to free - running Disk Cleanup as well for the categories it does not cover" -Level INFO } + $false { Write-Log "Storage Sense is switched off in Settings, so its run freed nothing - using Disk Cleanup instead" -Level INFO } + default { Write-Log "Storage Sense reported success but freed nothing measurable, and its setting could not be read - running Disk Cleanup as well" -Level INFO } + } + } default { Write-Log "Storage Sense verdict '$($verdict.Reason)' not recognised - running Disk Cleanup as well" -Level INFO } } } elseif ($waitResult.Outcome -eq 'vanished') { @@ -5521,6 +5935,13 @@ function Invoke-StorageSense { } } + if ($storageSenseDone) { + # Storage Sense demonstrably did the work, so cleanmgr is not run at all. Recorded + # so the JSON distinguishes this from "Disk Cleanup ran and completed" - they free + # different things, and a consumer comparing runs needs to know which one happened. + $script:Stats.DiskCleanupStatus = 'storage-sense' + } + if (-not $storageSenseDone) { # Fallback to cleanmgr. Every other reason for landing here (ambiguous lookup, # start failed, task failed, timed out, vanished, unverifiable) has already said @@ -5598,6 +6019,9 @@ function Invoke-StorageSense { if ($armed -eq 0) { Write-Log "No Disk Cleanup handlers could be armed - skipping cleanmgr" -Level WARNING $script:Stats.WarningsCount++ + # Distinct from a cleanmgr that started and failed (raised in review): + # nothing was attempted here, so nothing is half-done on the machine. + $script:Stats.DiskCleanupStatus = 'not-armed' return } @@ -5620,43 +6044,100 @@ function Invoke-StorageSense { if (-not $cleanmgr) { Write-Log "Disk Cleanup could not be started (cleanmgr.exe is missing or blocked) - it cleaned nothing" -Level WARNING $script:Stats.WarningsCount++ + $script:Stats.DiskCleanupStatus = 'start-failed' return } + # Recorded BEFORE the wait (raised in review): the status was only assigned + # after Wait-CleanmgrCompletion returned, leaving a window of up to fifteen + # minutes in which the JSON still said 'not-run'. If the run was interrupted + # there - Ctrl+C, or a throw anywhere in this block - a consumer read "the step + # never executed" while an elevated cleanmgr was actively deleting. The wrong + # answer on an abnormal exit is now "started, and we cannot account for it". + $script:Stats.DiskCleanupStatus = 'running' + # v2.16: raised from 420s. cleanmgr regularly needs longer on a workstation # with a large component store, and killing it produced a warning on every # single run while cleanmgr kept working in the background anyway. + # + # v2.22: waiting on HasExited alone was the wrong model of "there is still + # something to wait for". Measured on a live workstation: cleanmgr /sagerun did + # its work in about ten seconds, closed its window and then simply stayed + # resident - CPU, all three I/O counters and its six threads frozen, every + # thread in Wait. The run sat here for the remaining ~890 seconds and then + # declared the cleanup partial. Both halves of that are wrong, and the second + # is the worse one: it is the same class of dishonest report v2.20 and v2.21 + # were spent removing, only inverted - not success that never happened, but + # incompleteness that was never observed either. + # + # So a second, independent signal to stop waiting: total stillness. If the + # process has done no CPU work and no I/O at all across $idleChecksRequired + # consecutive checks, there is nothing left to wait for that we can observe. + # That is weaker than "it has finished", and is reported as such. $maxWait = 900 # 15 minutes - $elapsed = 0 $checkInterval = 10 - - while (-not $cleanmgr.HasExited -and $elapsed -lt $maxWait) { - Start-Sleep -Seconds $checkInterval - $elapsed += $checkInterval - - # Log progress every minute - if ($elapsed % 60 -eq 0) { - Write-Log "Disk Cleanup still running... ($elapsed seconds)" -Level INFO - } - } - - if (-not $cleanmgr.HasExited) { - # Still killing it would be worse - cleanmgr keeps working after a kill and - # the deletion is mid-flight (v2.16). But this is not an informational - # event either: everything measured after this point is partial, the run - # is about to print a total and write its JSON while an elevated process - # is still deleting, and the freed bytes it goes on to reclaim are counted - # by nobody. - $script:Stats.DiskCleanupPending = $true - Write-Log "Disk Cleanup exceeded $maxWait seconds and is still running - it continues in the background, so the freed figures below are partial" -Level WARNING - $script:Stats.WarningsCount++ - } elseif ($cleanmgr.ExitCode -ne 0) { + # Two full minutes of absolute stillness. Deliberately far longer than needed + # to observe the measured case: a process mid-delete moves at least the "other + # operations" counter, so this is not a race with slow work - it is a margin + # against a pause nobody has observed yet. The cost of being wrong is bounded + # anyway: the registry sweep below still refuses to touch a process that has + # not exited, so a premature verdict cannot pull configuration out from under + # a cleanmgr that turns out to be working after all. + $idleChecksRequired = 12 + + $waitOutcome = Wait-CleanmgrCompletion ` + -HasExited { $cleanmgr.HasExited } ` + -GetFingerprint { Get-DiskCleanupActivityFingerprint -ProcessId $cleanmgr.Id } ` + -MaxWaitSeconds $maxWait -CheckInterval $checkInterval -IdleChecksRequired $idleChecksRequired ` + -OnProgress { param($seconds) Write-Log "Disk Cleanup still running... ($seconds seconds)" -Level INFO } + + $elapsed = $waitOutcome.Elapsed + $wentIdleWhileResident = $waitOutcome.Outcome -eq 'idle-resident' + + if ($cleanmgr.HasExited -and $cleanmgr.ExitCode -ne 0) { # v2.16: the exit code used to be ignored entirely, so a crash one second # in was still logged as a success Write-Log "Disk Cleanup exited with code $($cleanmgr.ExitCode) - results unverified" -Level WARNING $script:Stats.WarningsCount++ - } else { + # Unlike the two above, this one RAN: the machine may be partially + # cleaned, which is a different thing for a consumer to know. + $script:Stats.DiskCleanupStatus = 'exit-nonzero' + } elseif ($cleanmgr.HasExited) { Write-Log "Disk Cleanup completed ($armed categories)" -Level SUCCESS + $script:Stats.DiskCleanupStatus = 'completed' + } elseif ($wentIdleWhileResident) { + # Deliberately worded as an OBSERVATION, not a conclusion (raised in the + # third review pass). What was measured is that cleanmgr and its children + # did nothing at all for two minutes after having been seen working; that + # they FINISHED is the inference, and the earlier wording ("completed") + # stated the inference as fact. This project has spent three releases + # removing exactly that habit, so the status is 'idle-resident' and the log + # says what was seen. + # + # DiskCleanupPending stays false, which is the best available estimate: a + # process performing no CPU work and no I/O is not deleting anything. It is + # an estimate rather than a certainty, and that is why it is not called + # completion. Not a warning either - on the machine where this was measured + # it is the normal ending, and warning on every run would be noise. + Write-Log "Disk Cleanup ($armed categories): worked, then stopped doing anything for $($idleChecksRequired * $checkInterval) seconds" -Level SUCCESS + Write-Log "cleanmgr.exe is still in the process list but shows no CPU or I/O activity - treating its work as over and not waiting out the remaining $($maxWait - $elapsed) seconds" -Level DETAIL + $script:Stats.DiskCleanupStatus = 'idle-resident' + } else { + # Genuinely still working when the timeout expired. Killing it would be + # worse - cleanmgr keeps working after a kill and the deletion is + # mid-flight (v2.16). But this is not an informational event either: + # everything measured after this point is partial, the run is about to + # print a total and write its JSON while an elevated process is still + # deleting, and the freed bytes it goes on to reclaim are counted by nobody. + # Worded for what is actually known (raised in review): this branch is the + # fall-through for "neither signal fired", which also catches the cases + # where activity could NEVER be measured (broken WMI) or was never observed + # at all. Claiming it "is still working" would be the same inference stated + # as observation that the idle branch above was reworded to avoid. + $script:Stats.DiskCleanupPending = $true + Write-Log "Disk Cleanup did not finish within $maxWait seconds and was not observed to stop - it is left running, so the freed figures below may be incomplete" -Level WARNING + $script:Stats.WarningsCount++ + $script:Stats.DiskCleanupStatus = 'timeout' } } finally { # Remove StateFlags to avoid leaving traces in the registry. @@ -5668,10 +6149,18 @@ function Invoke-StorageSense { # deliberately leaves it working in the background, and this sweep then pulled # its configuration out from under it. Whether cleanmgr re-reads the flags per # handler or only once at startup is not something to guess at while it holds - # an elevated deletion loop. The flags are swept by the next run's own sweep, + # an elevated deletion loop. The flags are swept by the first later run that # which is exactly the leftover case v2.16 added it for. if ($cleanmgr -and -not $cleanmgr.HasExited) { - Write-Log "Disk Cleanup is still running - its registry configuration will be swept by the next run" -Level DETAIL + # No promise about WHEN (raised in review). The next run's sweep is gated on + # no cleanmgr being present, and an idle-resident one typically lives until + # logoff or reboot - so on exactly the machines where that ending is normal, + # the next run would skip the sweep too. Sweeping here instead was rejected: + # if the idleness inference is wrong, pulling the flags could truncate a live + # cleanup, and v2.20 deliberately refused to guess whether cleanmgr re-reads + # them. Leaving stale flags is the cheaper of the two errors, but it must not + # be described as a cleanup that is going to happen. + Write-Log "cleanmgr.exe is still present, so its registry flags are left in place - they are swept by the first run that starts with no cleanmgr running" -Level DETAIL } else { Get-ChildItem -Path $regPath -ErrorAction SilentlyContinue | ForEach-Object { Remove-ItemProperty -Path $_.PSPath -Name "StateFlags$sageset" -Force -ErrorAction SilentlyContinue @@ -5690,24 +6179,49 @@ function Invoke-StorageSense { function Show-Banner { try { Clear-Host } catch { } - $banner = @" - - ╔══════════════════════════════════════════════════════════════════════╗ - ║ ║ - ║ ██████╗██╗ ███████╗ █████╗ ███╗ ██╗ ║ - ║ ██╔════╝██║ ██╔════╝██╔══██╗████╗ ██║ ║ - ║ ██║ ██║ █████╗ ███████║██╔██╗ ██║ ║ - ║ ██║ ██║ ██╔══╝ ██╔══██║██║╚██╗██║ ║ - ║ ╚██████╗███████╗███████╗██║ ██║██║ ╚████║ ║ - ║ ╚═════╝╚══════╝╚══════╝╚═╝ ╚═╝╚═╝ ╚═══╝ ║ - ║ ║ - ║ Ultimate Windows 11 Maintenance Script v$($script:Version) ║ - ║ ║ - ╚══════════════════════════════════════════════════════════════════════╝ + # v2.22: composed instead of pasted. As one literal here-string its padding was + # hand-counted for a four-character version - the title row measured 70 columns only + # because "2.21" happens to be four characters, and a version like 2.5 or 2.100 would + # have pushed that row's right border out of line with the rest of the box. Composing + # it fixes that and lets the frame follow the console width like every other box. + $inner = $script:BoxWidth + + # Centred as a BLOCK, never line by line: the rows have different lengths by design + # and centring each one would shear the letters apart. 70 is the block's historical + # width, so at the default width the banner is drawn exactly where it always was. + $artBlockWidth = 70 + $art = @( + '' + ' ██████╗██╗ ███████╗ █████╗ ███╗ ██╗' + ' ██╔════╝██║ ██╔════╝██╔══██╗████╗ ██║' + ' ██║ ██║ █████╗ ███████║██╔██╗ ██║' + ' ██║ ██║ ██╔══╝ ██╔══██║██║╚██╗██║' + ' ╚██████╗███████╗███████╗██║ ██║██║ ╚████║' + ' ╚═════╝╚══════╝╚══════╝╚═╝ ╚═╝╚═╝ ╚═══╝' + '' + ) + $artPad = [math]::Max(0, [math]::Floor(($inner - $artBlockWidth) / 2)) -"@ + $title = "Ultimate Windows 11 Maintenance Script v$($script:Version)" + $titlePad = [math]::Max(0, [math]::Floor(($inner - $title.Length) / 2)) - Write-Host $banner -ForegroundColor Cyan + $rows = foreach ($line in $art) { (' ' * $artPad) + $line.PadRight($artBlockWidth) } + $rows = @($rows) + @((' ' * $titlePad) + $title) + @('') + + $bannerLines = @(" ╔$('═' * $inner)╗") + foreach ($row in $rows) { + # Pad, then truncate: a row must fill the frame exactly. Anything longer would + # push the right border out, which is the defect this rewrite removes - so a row + # that cannot fit is cut rather than allowed to break the box. + $cell = $row.PadRight($inner) + if ($cell.Length -gt $inner) { $cell = $cell.Substring(0, $inner) } + $bannerLines += " ║$cell║" + } + $bannerLines += " ╚$('═' * $inner)╝" + + Write-Host "" + Write-Host ($bannerLines -join [Environment]::NewLine) -ForegroundColor Cyan + Write-Host "" # System info $os = Get-CimInstance -ClassName Win32_OperatingSystem @@ -5759,7 +6273,7 @@ function Show-FinalStatisticsBody { Clear-AllProgress # Box dimensions - $boxWidth = 70 # Inner width (matches banner) + $boxWidth = $script:BoxWidth # Inner width (matches banner) $labelWidth = 18 # Width for label column (e.g., "Space freed:") # Determine overall status @@ -5955,6 +6469,11 @@ function Write-ResultJson { # v2.20: true when Disk Cleanup outlived its timeout and was left running. # TotalFreedBytes is then a lower bound, not the final figure. DiskCleanupPending = [bool]$script:Stats.DiskCleanupPending + # v2.22: how that step ended, because the boolean above conflated two states. + # 'idle-resident' is the measured case: cleanmgr was seen working, then went + # completely still without exiting. 'timeout' is the genuine overrun the + # boolean was added for - still visibly working when time ran out. + DiskCleanupStatus = [string]$script:Stats.DiskCleanupStatus # 'enabled' means cleanup figures are understated (Defender blocked some # deletions without reporting an error); 'unknown' means the check itself # failed, so the figures are unverified rather than confirmed good @@ -5990,6 +6509,52 @@ function Write-ResultJson { } } +function Complete-WinCleanRun { + <# + .SYNOPSIS + The single end-of-run path: result JSON, final summary, log handle release + .DESCRIPTION + v2.22, raised in external review. Three paths ended a run - the normal finally, a + successful self-update, and a declined pending-reboot prompt - and only the first + released the log handle or showed a summary. The other two hand-copied whichever + parts someone had remembered at the time, which is exactly how v2.21 came to ship + two separate fixes to the same few lines (first a missing result JSON, then an + unconditional exit 0 over a run that had errors). The defect was never any one + omission; it was that the list existed in three places. Anything added here from + now on reaches every exit. + + Latched, so a path that completes the run explicitly and then unwinds through a + finally does not write the artefacts twice. + #> + param([string]$ResultPath) + + if ($script:RunCompleted) { return } + $script:RunCompleted = $true + + # JSON goes first: Show-FinalStatistics may block on a keypress in interactive + # mode, and automated runs must get the result regardless. + Write-ResultJson -Path $ResultPath + + # An aborted run has no maintenance to summarise, and the summary header would + # announce "COMPLETED SUCCESSFULLY" over a run that deliberately did nothing. + # Preserved behaviour: neither abort path ever showed it. + if (-not $script:Stats.Aborted) { + Show-FinalStatistics + } + + # Release the log file handle (v2.17, p.7): a stand or the user may want to move or + # zip the log right after the run finishes. + # v2.20: guarded. Dispose on a writer whose stream already failed throws, and this + # used to be the last statement of the outer finally - the exception escaped + # Start-WinClean and the entry point never reached its exit-code check, so a run with + # errors could still exit 0 (raised in review). + if ($script:LogWriter) { + try { $script:LogWriter.Dispose() } catch { } + $script:LogWriter = $null + $script:LogWriterPath = $null + } +} + function Invoke-Phase { <# .SYNOPSIS @@ -6047,10 +6612,25 @@ function Start-WinClean { $script:ProgressActivities = @() $script:InternetConnectionCache = $null $script:LogWriteFailed = $false - - # Initialize log - "WinClean v$($script:Version) - Started at $(Get-Date)" | Out-File -FilePath $script:LogPath -Encoding utf8 - "=" * 70 | Out-File -FilePath $script:LogPath -Append -Encoding utf8 + # v2.22: the latch on Complete-WinCleanRun, reset with everything else - a second + # call in the same session must produce its own artefacts, not silently skip them + # because the first run already completed. + $script:RunCompleted = $false + + # v2.22: ask for a wider window, then lay out against whatever the host actually + # gave us. In this order deliberately - measuring first would size every frame for + # the old width. Both steps are best-effort and silent: a host that refuses to + # resize, or whose width cannot be read, simply keeps the historical 70. + Expand-ConsoleWindow + $script:BoxWidth = Get-BoxWidth -ConsoleWidth (Get-ConsoleWidth) + + # Initialize log. v2.22 (raised in external review): these two lines were bare Out-File + # calls, and this runs BEFORE the main try/finally below. A log path that cannot be + # opened throws there - measured on six of seven bad paths - and the exception escaped + # Start-WinClean before any safety net existed: no result JSON, no summary, and none of + # the maintenance, because of the log. Now they degrade like every other log write. + Write-LogFileLine -Line "WinClean v$($script:Version) - Started at $(Get-Date)" -StartNewFile + Write-LogFileLine -Line ("=" * $script:BoxWidth) # v2.17 (p.13 of the audit): recover from a hard-killed previous run before doing # anything else - not in ReportOnly, which promises no changes @@ -6101,8 +6681,11 @@ function Start-WinClean { Write-Host " Operation cancelled. Please reboot and run again." -ForegroundColor Yellow Write-Host "" # Record the abort so automation does not mistake it for a completed run + # v2.22: through the shared end-of-run path. This branch used to write the + # JSON by hand and return, so it released no log handle - one of the three + # divergent endings that made the same fix necessary twice (external review). $script:Stats.Aborted = 'PendingRebootDeclined' - Write-ResultJson -Path $ResultJsonPath + Complete-WinCleanRun -ResultPath $ResultJsonPath return } } else { @@ -6122,7 +6705,20 @@ function Start-WinClean { try { $updateInfo = Test-ScriptUpdate if ($updateInfo) { - Invoke-ScriptUpdate -UpdateInfo $updateInfo + # v2.22: Invoke-ScriptUpdate reports whether the run is over instead of + # calling exit itself. A successful self-update replaced the running file, + # so continuing would perform maintenance with the old code still loaded - + # the run ends here, but it ends the same way every other run does. + if (Invoke-ScriptUpdate -UpdateInfo $updateInfo) { + # Artefacts first, then the pause. The prompt blocks until a key is + # pressed and a double-clicked shortcut may simply be closed there; + # writing the JSON afterwards would mean an abandoned window produced + # no result file at all (raised in the second review pass). + Complete-WinCleanRun -ResultPath $ResultJsonPath + Write-Host " Press any key to exit..." -ForegroundColor DarkGray + Wait-ForKeyPress + return + } } } catch { Write-Log "Update check could not be completed: $_" -Level WARNING @@ -6172,6 +6768,11 @@ function Start-WinClean { # stops the phase from dispatching at all, so the branch that sets this inside the # function is unreachable in production and the status stayed 'not-run'. if ($SkipUpdates) { $script:Stats.AppUpdatesStatus = 'skipped-parameter' } + # Same reason, for the same reason (raised in review): -SkipCleanup suppresses + # the whole DeepSystemCleanup phase, so Invoke-StorageSense never runs and never + # assigns a status - leaving the 'not-run' default, which the schema documents as + # "the run aborted before reaching it". A deliberate skip is not an abort. + if ($SkipCleanup) { $script:Stats.DiskCleanupStatus = 'skipped-cleanup-group' } Invoke-Phase -Name 'Updates' -Skip:$SkipUpdates -Action { Update-WindowsSystem @@ -6241,21 +6842,10 @@ function Start-WinClean { Write-Log "Critical error outside any phase: $_" -Level ERROR $script:Stats.ErrorsCount++ } finally { - # JSON goes first: Show-FinalStatistics may block on a keypress in - # interactive mode, and automated runs must get the result regardless - Write-ResultJson -Path $ResultJsonPath - Show-FinalStatistics - # Release the log file handle (v2.17, p.7): a stand or the user may want to - # move/zip the log right after the run finishes. - # v2.20: guarded. Dispose on a writer whose stream already failed throws, and this - # is the last statement of the outer finally - the exception escaped Start-WinClean - # and the entry point never reached its exit-code check, so a run with errors could - # still exit 0 (raised in review). - if ($script:LogWriter) { - try { $script:LogWriter.Dispose() } catch { } - $script:LogWriter = $null - $script:LogWriterPath = $null - } + # v2.22: the whole end-of-run sequence now lives in one function, shared with the + # two abort paths that used to hand-copy parts of it. Ordering, the aborted-run + # rule and the guarded Dispose are documented there. + Complete-WinCleanRun -ResultPath $ResultJsonPath } } diff --git a/docs/release-process.md b/docs/release-process.md index 0d3ab45..1bd4d75 100644 --- a/docs/release-process.md +++ b/docs/release-process.md @@ -65,6 +65,40 @@ $hash = (Get-FileHash .\WinClean.ps1 -Algorithm SHA256).Hash gh release upload vX.Y ".\WinClean.ps1#WinClean.ps1" "$env:TEMP\WinClean.ps1.sha256#WinClean.ps1.sha256" --clobber ``` +### Release notes template + +For a tool that runs elevated, predictability is worth more than a long feature list, so +every release uses the same headings. Keep them even when a section is short - a reader +looking for "did this change what gets deleted?" should find the answer in the same place +every time. + +```markdown +## What changed +Grouped as Fixed / Changed / Added, one line each, in user terms rather than internal +task numbers. Say what was wrong, not only what was done. + +## Safety notes +Anything that alters what is deleted, what is reported, or how the script decides it is +finished. Say "nothing in this release changes what gets deleted" when that is the case - +its absence is what people check for. + +## Verification +Release gate, Pester count, PSScriptAnalyzer, smoke, and the RU/EN stand runs, plus the +end-to-end run through the published one-liner. State what was actually run for THIS +release, not what the process says in general. + +## Assets +`WinClean.ps1` and `WinClean.ps1.sha256`. Both are required: the bootstrap scripts verify +the hash fail-closed and refuse to run without it. + +## Upgrade +Which of the four installation methods needs an action, and which update themselves. + +## Known limitations +Anything deliberately not fixed in this release, with the reason. This section is the one +most worth writing honestly - it is where trust is earned or lost. +``` + ## Publishing to PSGallery ```powershell diff --git a/docs/result-json.md b/docs/result-json.md index ac22a2d..d99e92b 100644 --- a/docs/result-json.md +++ b/docs/result-json.md @@ -14,7 +14,7 @@ This page documents every field, gives a full sample, and explains how to consum | Field | Type | Meaning | |-------|------|---------| -| `Version` | string | Script version that produced this file, e.g. `"2.19"`. | +| `Version` | string | Script version that produced this file, e.g. `"2.22"`. | | `Timestamp` | string | Run time in ISO-8601 round-trip format (`"o"`, e.g. `2026-07-21T03:15:42.1234567+00:00`). Use it to confirm the file belongs to the run you started, not a leftover. | | `DurationSeconds` | number | Wall-clock duration of the run, rounded to one decimal. | | `ReportOnly` | bool | `true` when the run was a preview (`-ReportOnly`): no cleanup or updates were applied (the log and this result file are still written). | @@ -28,7 +28,8 @@ This page documents every field, gives a full sample, and explains how to consum | `ErrorsCount` | number | Count of errors raised during the run. A healthy run reports `0`. | | `RebootRequired` | bool | `true` when a change (a Windows update, an app update finishing on reboot) needs a restart to take effect. | | `LoggingDegraded` | bool | v2.20. `true` when writing the log file failed at some point during the run. The run itself still completed, but `LogPath` points at an incomplete file: do not read that log as the full record of what happened. | -| `DiskCleanupPending` | bool | v2.20. `true` when Disk Cleanup outlived its timeout and was left running in the background. `TotalFreedBytes` is then a lower bound rather than the final figure, because deletion continued after this file was written. | +| `DiskCleanupPending` | bool | v2.20. `true` when Disk Cleanup was still **visibly working** when its timeout expired and was left running in the background; `TotalFreedBytes` is then a lower bound. Note `false` is not a proof that nothing more will ever be deleted - see `DiskCleanupStatus`, which is the field to read when that distinction matters. | +| `DiskCleanupStatus` | string | v2.22. How the Storage Sense / Disk Cleanup step actually ended - twelve values, listed below. The boolean above could not tell a cleanup that had stopped doing anything from one still deleting, and reported both as pending. | | `ControlledFolderAccess` | string | Tri-state, see below. Reflects whether Defender's Controlled Folder Access may have silently blocked deletions. | | `Aborted` | string or null | `null` unless the run stopped early for a known reason: `"PendingRebootDeclined"` (the user declined to continue with a reboot pending) or `"UpdatedAndExited"` (v2.21 - the script updated itself and exited so the new version runs next time). When set, the phase arrays below are incomplete by design. Note `null` does not by itself prove every phase ran - see the invariant note below. | | `PhasesCompleted` | array of string | Phases whose action ran to completion without an uncaught exception. | @@ -78,6 +79,45 @@ Consequently: Treat any value other than `checked` as "the count means nothing", not as "there was nothing to update". +### `DiskCleanupStatus` (added in v2.22) + +`DiskCleanupPending` alone could not tell two different situations apart, and reported +both as pending. Measured on a live workstation: `cleanmgr /sagerun` did its work in about +ten seconds, closed its window, and then stayed in the process list doing nothing at all - +no CPU, no I/O, every thread waiting. Treating process exit as the only sign that there is +still something to wait for therefore burned the remaining fifteen-minute timeout, and +then published as partial a cleanup that had not been observed doing anything for most of +that time. + +Since v2.22 a second, independent signal is used to stop waiting: total stillness. If the +process was seen working and then performs no CPU work and no I/O across twelve +consecutive ten-second checks, there is nothing observable left to wait for. That is +deliberately weaker than "it has finished", and the field names the observation rather +than the conclusion. + +| Value | Meaning | +|-------|---------| +| `completed` | cleanmgr ran and exited with code 0. | +| `idle-resident` | cleanmgr was seen working, then did nothing at all for two minutes and never exited. Reported as what was **observed**, not as proven completion: a process performing no CPU work and no I/O is not deleting anything, so the figures are treated as final and `DiskCleanupPending` stays `false`. | +| `timeout` | The wait expired without either signal firing. That includes the case where activity could never be measured at all (WMI unavailable) or was never observed, so it does not by itself mean cleanmgr was seen working. `DiskCleanupPending` is `true` and `TotalFreedBytes` is a lower bound. | +| `running` | Set as soon as cleanmgr starts, and replaced by one of the values above when the wait ends. Seeing it in a result file means the run was interrupted while an elevated cleanmgr was in flight - the totals are not final and the process may still be deleting. | +| `storage-sense` | Storage Sense demonstrably did the work, so cleanmgr was not run. Note this covers a different, smaller set of things than cleanmgr handlers - it does not touch Update Cleanup, memory dumps, Language Pack, old ChkDsk files or Windows Error Reporting. | +| `not-armed` | No Disk Cleanup handler could be armed, so cleanmgr was never started. Nothing was attempted and nothing is half-done. | +| `start-failed` | cleanmgr.exe could not be started at all (missing, or blocked by AppLocker/WDAC). Nothing was attempted. | +| `exit-nonzero` | cleanmgr started and exited with a non-zero code. Unlike the two above, it **ran**: the machine may be partially cleaned. | +| `skipped-parameter` | `-SkipDiskCleanup` was passed. | +| `skipped-cleanup-group` | `-SkipCleanup` was passed, which suppresses the whole cleanup group before this step is dispatched. | +| `skipped-report-only` | `-ReportOnly` was passed. This is what every preview run, the smoke test and the `Report`/`ReportNoCleanup` stand modes produce. | +| `not-run` | The step never executed - the run aborted before reaching it. | + +`idle-resident` is not an error, and it is named after the observation rather than after +the conclusion on purpose. What is measured is stillness; "it finished" is an inference +from it, and the inference can be wrong - a cleanmgr blocked on something external would +look identical. Nothing downstream is built to depend on it being right: the registry +configuration of a process that has not exited is still left alone, and the log states +what was actually seen. A `cleanmgr.exe` still visible in Task Manager after the run is +the expected consequence, which is why it is reported rather than left to be discovered. + ### `ControlledFolderAccess` (tri-state string) | Value | Meaning | @@ -109,7 +149,7 @@ VisualStudioCleanup, DeepSystemCleanup, DiskSpaceReport, Telemetry ```json { - "Version": "2.20", + "Version": "2.22", "Timestamp": "2026-07-21T03:15:42.1234567+00:00", "DurationSeconds": 196.4, "ReportOnly": false, @@ -140,6 +180,7 @@ VisualStudioCleanup, DeepSystemCleanup, DiskSpaceReport, Telemetry "ControlledFolderAccess": "disabled", "LoggingDegraded": false, "DiskCleanupPending": false, + "DiskCleanupStatus": "completed", "Aborted": null, "PhasesCompleted": [ "Preparation", diff --git a/install.ps1 b/install.ps1 index 8b626cf..69c89f8 100644 --- a/install.ps1 +++ b/install.ps1 @@ -77,8 +77,23 @@ if (-not (Test-Path $pwshPath)) { return } +# The version has to be PROVEN, not merely "not disproven" (raised in external review). +# The previous form was `if ($pwshVersion -and $pwshVersion -lt '7.1')`, so a version that +# could not be read left $pwshVersion $null, skipped the comparison and continued - the +# installer then pinned an elevated desktop shortcut to a binary whose suitability nobody +# had established. That is the same fail-open shape as the SHA256 verification that hid +# inside `if ($hashAsset)` until v2.17: a check that cannot run becomes a check that +# passes. Absence of evidence is not evidence of compatibility. $pwshVersion = try { [version](((Get-Item $pwshPath).VersionInfo.ProductVersion -split '-')[0]) } catch { $null } -if ($pwshVersion -and $pwshVersion -lt [version]'7.1') { +if (-not $pwshVersion) { + # Deliberately a different message from "your version is too old": the user needs to + # know the file is there but unreadable, which points at a damaged or substituted + # install rather than at an outdated one. + Stop-Install "PowerShell 7 was found at $pwshPath, but its version could not be read - WinClean requires 7.1+ and will not assume it." ` + "Repair or reinstall PowerShell 7: winget install --id Microsoft.PowerShell --force" + return +} +if ($pwshVersion -lt [version]'7.1') { Stop-Install "PowerShell $pwshVersion found at $pwshPath, but WinClean requires 7.1+." ` "Update it with: winget upgrade --id Microsoft.PowerShell" return diff --git a/tests/Docs.Tests.ps1 b/tests/Docs.Tests.ps1 index 97a82d8..614c02b 100644 --- a/tests/Docs.Tests.ps1 +++ b/tests/Docs.Tests.ps1 @@ -12,7 +12,15 @@ # Computed at discovery time so -ForEach can enumerate one test per file. $RepoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path -$DocFiles = @('README.md', 'README_RU.md', 'SECURITY.md', 'CONTRIBUTING.md', 'CHANGELOG.md') + +# v2.22: enumerated instead of listed by hand. SUPPORT.md was added in this release and +# silently fell outside the guards - a new user-facing document with links in it was the +# one file nothing checked. A hand-written list only covers the files someone remembered, +# and the next addition would have been forgotten the same way. +# CLAUDE.md is excluded deliberately: it is the maintainer/agent runbook, not user +# documentation, and it is not part of the published contract. +$DocFiles = @(Get-ChildItem $RepoRoot -Filter *.md -File | + Where-Object { $_.Name -ne 'CLAUDE.md' } | + ForEach-Object { $_.Name }) + (Get-ChildItem (Join-Path $RepoRoot 'docs') -Filter *.md -File -ErrorAction SilentlyContinue | ForEach-Object { Join-Path 'docs' $_.Name }) $DocCases = $DocFiles | ForEach-Object { @{ File = $_; RepoRoot = $RepoRoot } } diff --git a/tests/Fixes.Tests.ps1 b/tests/Fixes.Tests.ps1 index 69f9981..c50e901 100644 --- a/tests/Fixes.Tests.ps1 +++ b/tests/Fixes.Tests.ps1 @@ -656,9 +656,16 @@ Describe "v2.16: Disk Cleanup timeout" -Tag "Fix", "V216" { # after this point is partial, and the run prints its total and writes its JSON # while an elevated process is still deleting. Silence made a partial result look # like a final one. + # v2.22 reworded the message and this test with it. The old text asserted cleanmgr + # "is still running", which the code cannot know: this branch is the fall-through + # for "neither signal fired", and broken WMI or activity that was never observed + # land here too. What must survive is the CONSEQUENCE - the figures may be + # incomplete - and the machine-readable flag that carries it. $body = Get-FunctionBody -Name 'Invoke-StorageSense' - $body | Should -Match 'still running - it continues in the background' + $body | Should -Match 'was not observed to stop' + $body | Should -Match 'may be incomplete' $body | Should -Match '\$script:Stats\.DiskCleanupPending = \$true' + $body | Should -Not -Match 'still running - it continues in the background' } It "cleanmgr is not killed on timeout" { @@ -904,6 +911,20 @@ Describe "v2.17: bootstrap verification is mandatory" -Tag "Fix", "V217" { $installScript | Should -Match "\[Environment\]::GetFolderPath\(\[Environment\+SpecialFolder\]::ProgramFiles\)" $installScript | Should -Not -Match "Join-Path \`$env:ProgramFiles" } + + It "install.ps1 refuses a pwsh whose version it could not read (v2.22)" { + # External review: `if ($pwshVersion -and $pwshVersion -lt '7.1')` let an unreadable + # version through, because $null fails the first operand and the comparison never + # runs. Same fail-open shape as the SHA256 check that hid inside `if ($hashAsset)` + # until v2.17. The unreadable case must be its own stop, before the comparison. + # Code lines only: the comment above the fix quotes the old fail-open expression + # verbatim, and a naive match found it there. Second time this exact trap fired in + # this release, which is why both checks now strip comments first. + $code = ($installScript -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match '(?m)^if \(-not \$pwshVersion\) \{' + $code | Should -Match 'version could not be read' + $code | Should -Not -Match '\$pwshVersion -and \$pwshVersion -lt' + } } Describe "v2.18: bootstrap host allowlist is exact, not a broad suffix" -Tag "Fix", "V218" -ForEach @( @@ -1167,12 +1188,293 @@ Describe "v2.20: an operation that did nothing does not report success" -Tag "Fi } } + Context "Storage Sense setting explains, it does not decide (v2.22)" { + It "The skip verdict stays a function of what happened, not of a setting" { + # MyAI-1qtn proposed trusting an enabled Storage Sense and skipping Disk + # Cleanup when it freed nothing measurable. Checking what that would cost + # settled it: the armed cleanmgr categories include Update Cleanup, memory + # dumps, Language Pack, old ChkDsk files and Windows Error Reporting - none of + # which Storage Sense touches. A maintained machine would silently stop having + # them cleaned. The setting is therefore read for the log only, and this test + # exists so that decision cannot be reversed by accident. + # The decision lives in Invoke-StorageSense ($storageSenseDone), NOT in + # Get-StorageSenseVerdict. Guarding only the verdict function was theatre: a + # reviewer inserted `if (Test-StorageSenseEnabled) { $storageSenseDone = $true }` + # into Invoke-StorageSense - exactly the reversal this test forbids - and all + # 679 tests stayed green. Guard the variable that actually decides. + $verdict = Get-FunctionBody -Name 'Get-StorageSenseVerdict' + $verdict | Should -Not -Match 'Test-StorageSenseEnabled' + $verdict | Should -Not -Match 'StoragePolicy' + + $sense = Get-FunctionBody -Name 'Invoke-StorageSense' + $senseCode = ($sense -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $assignments = @($senseCode -split "`n" | Where-Object { $_ -match '\$storageSenseDone\s*=' }) + $assignments.Count | Should -Be 2 -Because 'only the initialiser and the verdict may set it' + ($assignments -join "`n") | Should -Not -Match 'Test-StorageSenseEnabled' + ($assignments -join "`n") | Should -Match '\$verdict\.Done' + } + } + + Context "Console widening is bounded and cannot fail a run (v2.22)" { + # Structural, and deliberately so: $Host.UI.RawUI is not reachable through any + # cmdlet, so there is nothing to mock and no way to drive these branches from a + # test. Both properties matter enough to pin somehow rather than not at all. + BeforeAll { + $script:expandBody = Get-FunctionBody -Name 'Expand-ConsoleWindow' + } + + It "never asks for a window wider than the screen can show" { + # Asking for more than the physical maximum throws; the catch would swallow it + # and the window would silently stay narrow - the defect, unfixed and unreported. + $script:expandBody | Should -Match 'MaxPhysicalWindowSize' + } + + It "grows the buffer before the window, which is the only order that works" { + $bufferAt = $script:expandBody.IndexOf('$raw.BufferSize =') + $windowAt = $script:expandBody.IndexOf('$raw.WindowSize =') + $bufferAt | Should -BeGreaterThan 0 + $windowAt | Should -BeGreaterThan $bufferAt -Because 'a window may never exceed its buffer' + } + + It "swallows a refusing host instead of failing the run" { + # Windows Terminal refuses programmatic resizing. This runs at the start of + # every run: cosmetics must never be able to stop maintenance. + $script:expandBody | Should -Match '(?s)\} catch \{' + $script:expandBody | Should -Not -Match '(?s)catch \{[^}]*throw' + } + } + + Context "Disk Cleanup that finished but never exited (v2.22)" { + # Wait-CleanmgrCompletion is covered behaviourally in Helpers.Tests.ps1. What is + # not reachable without a live cleanmgr is how Invoke-StorageSense REPORTS the + # 'idle-resident' outcome - and misreporting it is the entire defect: a finished + # cleanup was published as partial. Structural, and labelled as such. + BeforeAll { + $script:senseBody = Get-FunctionBody -Name 'Invoke-StorageSense' + $script:senseCode = ($script:senseBody -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + } + + It "A cleanup that went idle while resident is not reported as still deleting" { + # DiskCleanupPending means "an elevated process was still VISIBLY deleting, the figures + # below are partial". The measured case is the opposite: nothing was happening. + $residentBranch = [regex]::Match($script:senseCode, + '(?s)\}\s*elseif\s*\(\$wentIdleWhileResident\)\s*\{(.*?)\}\s*else\s*\{').Groups[1].Value + $residentBranch | Should -Not -BeNullOrEmpty -Because 'the branch must exist to be checked' + $residentBranch | Should -Not -Match 'DiskCleanupPending' + $residentBranch | Should -Match "DiskCleanupStatus = 'idle-resident'" + } + + It "The genuine overrun still sets DiskCleanupPending" { + # The v2.20 guarantee must survive: a cleanmgr that really is still working + # has to keep marking the run's totals as a lower bound. + $script:senseCode | Should -Match '\$script:Stats\.DiskCleanupPending = \$true' + $script:senseCode | Should -Match "DiskCleanupStatus = 'timeout'" + } + + It "The registry sweep still refuses to touch a process that has not exited" { + # This is what bounds the cost of a wrong idle verdict: even if the wait ends + # early, the StateFlags of a still-running cleanmgr are left alone. + $script:senseCode | Should -Match 'if \(\$cleanmgr -and -not \$cleanmgr\.HasExited\)' + } + } + + Context "Single end-of-run path (v2.22)" { + # The behaviour of each piece is tested in Helpers.Tests.ps1. What cannot be reached + # behaviourally without running the whole script as admin is the WIRING: that all + # three endings go through the one function. That is precisely what drifted before - + # each ending kept its own hand-copied subset of the list. + + It "Every ending goes through Complete-WinCleanRun" { + # Comment lines are excluded deliberately: counting raw matches also counted the + # comment that names the function, which is the documented way these grep-style + # tests go wrong (a match in prose passing for a match in code). + $body = Get-FunctionBody -Name 'Start-WinClean' + $calls = @($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' -and $_ -match 'Complete-WinCleanRun' }) + $calls.Count | + Should -Be 3 -Because 'the finally, the self-update path and the declined-reboot path must all use it' + } + + It "Start-WinClean no longer hand-copies the end-of-run steps" { + $body = Get-FunctionBody -Name 'Start-WinClean' + $body | Should -Not -Match 'Show-FinalStatistics' + $body | Should -Not -Match 'Write-ResultJson' + } + + It "A verified self-update ends the run through the caller, not with exit" { + $body = Get-FunctionBody -Name 'Invoke-ScriptUpdate' + $body | Should -Not -Match '(?m)^\s*exit\s' + $body | Should -Not -Match 'Write-ResultJson' + $body | Should -Match 'return \$true' + } + + It "The self-update pause happens after the artefacts, never before them" { + # Raised in the second review pass. Wait-ForKeyPress blocks indefinitely; in + # v2.21 the result JSON was written before it. Moving the pause into + # Invoke-ScriptUpdate put an unbounded wait between the update and the + # artefacts, so an abandoned window produced no result file at all. + # Scoped to the self-update branch itself (raised in review): comparing the + # FIRST Complete-WinCleanRun in the whole function found the pending-reboot + # call instead, so the ordering held even if the self-update branch lost its + # completion call entirely. + $body = Get-FunctionBody -Name 'Start-WinClean' + $branch = [regex]::Match($body, + '(?s)if \(Invoke-ScriptUpdate -UpdateInfo \$updateInfo\) \{(.*?)\n\s*\}').Groups[1].Value + $branch | Should -Not -BeNullOrEmpty -Because 'the branch must exist to be checked' + + $completeAt = $branch.IndexOf('Complete-WinCleanRun') + $pauseAt = $branch.IndexOf('Wait-ForKeyPress') + $completeAt | Should -BeGreaterOrEqual 0 -Because 'the self-update branch must complete the run itself' + $pauseAt | Should -BeGreaterThan $completeAt + + # Exactly one prompt on this path, so the pause is neither lost nor doubled + ([regex]::Matches($branch, 'Wait-ForKeyPress')).Count | Should -Be 1 + ([regex]::Matches($branch, 'Press any key to exit')).Count | Should -Be 1 + + # And it must not have crept back into the updater + $update = Get-FunctionBody -Name 'Invoke-ScriptUpdate' + $tail = $update.Substring($update.IndexOf('Update complete')) + $tail | Should -Not -Match 'Wait-ForKeyPress' + } + + It "The log header call site passes -StartNewFile" { + # A mutation removing this switch from the only production caller survived the + # whole suite AND reached a commit. The behavioural test proves the function + # honours the switch; nothing proved the caller sends it. Without it a fixed + # -LogPath accumulates every run into one file. + $body = Get-FunctionBody -Name 'Start-WinClean' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match 'Write-LogFileLine -Line "WinClean v[^"]*" -StartNewFile' + } + + It "The adaptive width is actually wired into the run" { + # Deleting both lines that compute the width survived the entire suite: + # Get-BoxWidth had ten unit tests and no caller assertion, so the feature could + # be disconnected while every test about it stayed green. + $body = Get-FunctionBody -Name 'Start-WinClean' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match 'Expand-ConsoleWindow' + $code | Should -Match '\$script:BoxWidth\s*=\s*Get-BoxWidth -ConsoleWidth \(Get-ConsoleWidth\)' + } + + It "Every frame consumer reads the shared width, none re-hardcodes 70" { + # Four consumers were individually revertible to the literal 70 without any + # test noticing, because the geometry checker validates each box internally + # and never compares widths BETWEEN boxes. + foreach ($fn in 'Write-Log', 'Invoke-ScriptUpdate', 'Show-FinalStatisticsBody', 'Show-Banner') { + $b = Get-FunctionBody -Name $fn + $c = ($b -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $c | Should -Not -Match '\$boxWidth\s*=\s*70' -Because "$fn must not pin the width again" + $c | Should -Not -Match '"[-─═]"\s*\*\s*70' -Because "$fn must not pin a rule width again" + } + } + + It "The abort marker is set BEFORE the run is completed, on both abort paths" { + # Swapping the two lines survived: the result JSON would then record + # Aborted: null and the summary would print "COMPLETED SUCCESSFULLY" over a run + # that deliberately did nothing. Neither branch has a finally behind it - both + # return before the outer try - so the order at the call site is the only guard. + # Only PendingRebootDeclined is set at the call site; UpdatedAndExited is set + # inside Invoke-ScriptUpdate before it returns $true, so it is already in place + # by the time Start-WinClean completes the run. Both are checked, each where it + # actually lives - the first draft of this test looked for both in Start-WinClean + # and failed, which is the test doing its job. + $body = Get-FunctionBody -Name 'Start-WinClean' + $at = $body.IndexOf("Aborted = 'PendingRebootDeclined'") + $at | Should -BeGreaterThan 0 + $body.IndexOf('Complete-WinCleanRun', $at) | Should -BeGreaterThan $at + + $update = Get-FunctionBody -Name 'Invoke-ScriptUpdate' + $setAt = $update.IndexOf("Aborted = 'UpdatedAndExited'") + $retAt = $update.IndexOf('return $true') + $setAt | Should -BeGreaterThan 0 + $retAt | Should -BeGreaterThan $setAt -Because 'the caller must see the marker already set' + } + + It "A deliberate skip is not reported as an abort" { + # -SkipCleanup suppresses the whole DeepSystemCleanup phase, so Invoke-StorageSense + # never runs and never assigns a status - leaving the 'not-run' default, which the + # schema documents as "the run aborted before reaching it". Raised in review; the + # round-trip test cannot see this because it assigns values by hand. + $body = Get-FunctionBody -Name 'Start-WinClean' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match "if \(\`$SkipCleanup\) \{ \`$script:Stats\.DiskCleanupStatus = 'skipped-cleanup-group' \}" + } + + It "The completion latch is reset for every run" { + # Deleting the reset survived: a second Start-WinClean in one session would + # then silently produce no result JSON, no summary and no handle release, + # because the latch from the first run is still set. + $body = Get-FunctionBody -Name 'Start-WinClean' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match '\$script:RunCompleted\s*=\s*\$false' + } + + It "The result JSON is written before anything that can block" { + # Complete-WinCleanRun documents "JSON goes first: Show-FinalStatistics may + # block on a keypress, and automated runs must get the result regardless". + # Swapping them survived - the three latch tests use -Exactly but never compare + # order. The equivalent ordering IS pinned for the self-update path; this makes + # the pair symmetrical. + # Comment-stripped: the docblock above the calls names Show-FinalStatistics + # while explaining why the JSON goes first, so a raw IndexOf found the prose + # and reported the wrong order. Third time this trap fired in this release. + $body = Get-FunctionBody -Name 'Complete-WinCleanRun' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $jsonAt = $code.IndexOf('Write-ResultJson') + $statsAt = $code.IndexOf('Show-FinalStatistics') + $jsonAt | Should -BeGreaterThan 0 + $statsAt | Should -BeGreaterThan $jsonAt + } + + It "The idle threshold used in production is the documented two minutes" { + # Every Wait-CleanmgrCompletion test passes -IdleChecksRequired explicitly, so + # the production value was invisible to them: setting it to 1 survived, which + # would declare a live cleanup finished after a single quiet check. + $body = Get-FunctionBody -Name 'Invoke-StorageSense' + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match '\$idleChecksRequired = 12' + $code | Should -Match '\$checkInterval = 10' + } + + It "Start-WinClean acts on the answer instead of ignoring it" { + # A caller that dropped the `if` would carry on running maintenance with the + # script file already replaced underneath it. + $body = Get-FunctionBody -Name 'Start-WinClean' + $body | Should -Match 'if \(Invoke-ScriptUpdate -UpdateInfo \$updateInfo\)' + } + } + Context "Logging failure is visible" { It "Latches the first log write failure instead of swallowing every one" { - $body = Get-FunctionBody -Name 'Write-Log' + # v2.22: the degradation moved into Write-LogFileLine, so that the log header - + # written before Start-WinClean's try/finally exists - goes through it too. + $body = Get-FunctionBody -Name 'Write-LogFileLine' $body | Should -Match '\$script:LogWriteFailed' - # Write-Log must not report its own failure through Write-Log - $body | Should -Match 'Write-Host' + # Comment-stripped and matched on the MESSAGE, not the cmdlet name (raised in + # review, proved by mutation): the catch block contains the comment + # "Deliberately Write-Host and not Write-Log", so a bare 'Write-Host' match + # survived deleting the actual warning call - the run's only visible sign that + # logging failed could vanish with this test still green. + $code = ($body -split "`n" | Where-Object { $_ -notmatch '^\s*#' }) -join "`n" + $code | Should -Match 'Write-Host\s+"\s*\[WARN\]\s+Log file could not be written' + } + + It "Write-Log still routes file writes through the one degrading primitive" { + # Guards the seam the refactor created: a Write-Log that opened the file itself + # again would leave two implementations of the same fault tolerance, which is + # the state this change removed. + $body = Get-FunctionBody -Name 'Write-Log' + $body | Should -Match 'Write-LogFileLine' + $body | Should -Not -Match '\[System\.IO\.File\]::Open' + } + + It "The log header cannot kill the run - it is not written with a bare Out-File" { + # The defect itself (external review, v2.22): two Out-File calls before any + # safety net. Measured: six of seven bad log paths throw a terminating error, + # which escaped Start-WinClean and cost the run its JSON and its maintenance. + $body = Get-FunctionBody -Name 'Start-WinClean' + $body | Should -Not -Match 'Out-File\s+-FilePath\s+\$script:LogPath' + $body | Should -Match 'Write-LogFileLine -Line "WinClean v' } It "Surfaces it in the result JSON so a consumer knows the log is incomplete" { diff --git a/tests/Helpers.Tests.ps1 b/tests/Helpers.Tests.ps1 index b5c5822..a6122cb 100644 --- a/tests/Helpers.Tests.ps1 +++ b/tests/Helpers.Tests.ps1 @@ -962,6 +962,125 @@ Describe "Write-Log" -Tag "Unit", "Helper" { #endregion +#region Write-LogFileLine Tests (v2.22 - log init must never kill the run) + +Describe "Write-LogFileLine" -Tag "Unit", "Helper", "V222" { + + BeforeEach { + $script:prevLogPath = $script:LogPath + $script:prevWarnings = $script:Stats.WarningsCount + $script:prevFailed = $script:LogWriteFailed + if ($script:LogWriter) { $script:LogWriter.Dispose(); $script:LogWriter = $null; $script:LogWriterPath = $null } + $script:LogWriteFailed = $false + $script:testLog = Join-Path ([System.IO.Path]::GetTempPath()) "WinCleanLogLine_$(Get-Random).log" + $script:LogPath = $script:testLog + } + + AfterEach { + if ($script:LogWriter) { $script:LogWriter.Dispose(); $script:LogWriter = $null; $script:LogWriterPath = $null } + Remove-Item -LiteralPath $script:testLog -Force -ErrorAction SilentlyContinue + $script:LogPath = $script:prevLogPath + $script:Stats.WarningsCount = $script:prevWarnings + $script:LogWriteFailed = $script:prevFailed + } + + It "Writes the line verbatim, without adding a timestamp or level" { + # The header is not a log entry: it must land exactly as composed. Write-Log's + # "[HH:mm:ss] [LEVEL] " prefix belongs to Write-Log, not to this primitive. + Write-LogFileLine -Line 'WinClean v9.99 - Started at whenever' -StartNewFile + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Match ([regex]::Escape('WinClean v9.99 - Started at whenever')) + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Not -Match '^\[' + } + + It "Appends by default and truncates only with -StartNewFile" { + # -StartNewFile preserves what the replaced Out-File (no -Append) did. Losing it + # would silently merge every run sharing a custom -LogPath into one file. + Write-LogFileLine -Line 'first run' -StartNewFile + Write-LogFileLine -Line 'same run' + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Match 'first run' + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Match 'same run' + + Write-LogFileLine -Line 'second run' -StartNewFile + $after = Get-Content -LiteralPath $script:testLog -Raw + $after | Should -Match 'second run' + $after | Should -Not -Match 'first run' -Because '-StartNewFile must truncate, matching the Out-File it replaced' + } + + # The defect this whole function exists for. Measured in review: six of seven bad log + # paths make Out-File throw a TERMINATING error even at ErrorActionPreference=Continue, + # and the header is written before Start-WinClean's try/finally exists - so the run + # died with no result JSON and no maintenance because of the log file. + It "Never throws on a log path that cannot be opened - " -ForEach @( + @{ Case = 'missing directory'; Path = { Join-Path ([System.IO.Path]::GetTempPath()) "nope_$(Get-Random)\sub\a.log" } } + @{ Case = 'path is a directory'; Path = { [System.IO.Path]::GetTempPath() } } + @{ Case = 'invalid characters'; Path = { Join-Path ([System.IO.Path]::GetTempPath()) 'a:b:c.log' } } + @{ Case = 'over-long path'; Path = { Join-Path ([System.IO.Path]::GetTempPath()) (('x' * 300) + '.log') } } + # An unreachable UNC path was measured too (IOException, same as the two above) but + # is deliberately not a case here: it is the only one that depends on network + # resolution, took 8.3s locally, and would be this suite's one flake-prone test. + ) { + $script:LogPath = & $Path + { Write-LogFileLine -Line 'header' -StartNewFile } | Should -Not -Throw -Because 'a log that cannot be written is a degraded run, never a failed one' + } + + It "Reports the failure instead of swallowing it - LoggingDegraded reaches the result JSON" { + # Not throwing must not become not telling. LogWriteFailed is what surfaces as + # LoggingDegraded in the result JSON, so an automated consumer can see the run's + # log is incomplete rather than trusting a silent success. + $script:LogPath = Join-Path ([System.IO.Path]::GetTempPath()) "nope_$(Get-Random)\sub\a.log" + $before = $script:Stats.WarningsCount + + Write-LogFileLine -Line 'header' -StartNewFile 3>$null 6>$null + + $script:LogWriteFailed | Should -BeTrue -Because 'LoggingDegraded in the result JSON is driven by this flag' + $script:Stats.WarningsCount | Should -Be ($before + 1) + } + + It "Latches the warning - a failing log costs one warning, not one per line" { + $script:LogPath = Join-Path ([System.IO.Path]::GetTempPath()) "nope_$(Get-Random)\sub\a.log" + $before = $script:Stats.WarningsCount + + 1..5 | ForEach-Object { Write-LogFileLine -Line "line $_" 3>$null 6>$null } + + $script:Stats.WarningsCount | Should -Be ($before + 1) -Because 'Write-Log fires hundreds of times per run' + } + + It "Recovers when the path becomes writable again" { + # The v2.20 lesson kept: drop the dead writer so a later call reopens it, instead + # of leaving the guard satisfied by a broken object and discarding the rest. + $script:LogPath = Join-Path ([System.IO.Path]::GetTempPath()) "nope_$(Get-Random)\sub\a.log" + Write-LogFileLine -Line 'lost' -StartNewFile 3>$null 6>$null + $script:LogWriter | Should -BeNullOrEmpty -Because 'a failed writer must not be reused' + + $script:LogPath = $script:testLog + Write-LogFileLine -Line 'recovered' -StartNewFile + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Match 'recovered' + } + + It "Drops a writer whose stream broke mid-run, so later lines are not discarded" { + # Added after a mutation survived: the test above only covers a writer that never + # opened, and in that case the field is already $null - so deleting the reset would + # not have failed anything. This covers the case the reset actually exists for: an + # open writer whose stream dies later (the v2.14 shape, where the run's own temp + # cleanup deleted the log out from under it). Without the reset, the guard stays + # satisfied by a dead object and every remaining line is silently thrown away. + Write-LogFileLine -Line 'opened fine' -StartNewFile + $script:LogWriter | Should -Not -BeNullOrEmpty + + # Break the stream underneath the writer without touching the script's bookkeeping + $script:LogWriter.BaseStream.Dispose() + + Write-LogFileLine -Line 'this write fails' 3>$null 6>$null + $script:LogWriter | Should -BeNullOrEmpty -Because 'a broken writer must be dropped, not kept and reused' + + # And the very next line must actually reach the file again + Write-LogFileLine -Line 'back in business' + (Get-Content -LiteralPath $script:testLog -Raw) | Should -Match 'back in business' + } +} + +#endregion + #region Get-RecycleBinSize Tests Describe "Get-RecycleBinSize" -Tag "Unit", "Helper" { @@ -2014,7 +2133,10 @@ Describe "Test-ScriptUpdate" -Tag "Unit", "Helper", "V221" { Describe "Invoke-ScriptUpdate branches" -Tag "Unit", "Helper", "V221" { # These branches were unreachable by the helper tests, which only proved that the # instruction TEXT exists - not that Invoke-ScriptUpdate ever prints it (raised in - # review). Every branch here returns instead of reaching `exit 0`, so the suite is safe. + # review). Every branch here returns rather than completing an update. + # v2.22: the function no longer calls exit at all, so the successful path is testable + # too - it is covered in the V222 region below, which is why this note no longer says + # the suite is only safe because that path is never reached. BeforeEach { $script:Stats.WarningsCount = 0 @@ -2103,24 +2225,23 @@ Describe "Invoke-ScriptUpdate branches" -Tag "Unit", "Helper", "V221" { $script:askedPath | Should -Be $script:WinCleanPath } - It "writes the result JSON before exiting on a verified update" { - # The success path ends in `exit 0`, which bypasses the finally in Start-WinClean - - # and Start-WinClean has already deleted any previous result file by then, so - # without this a consumer saw exit code 0 and no artefact, indistinguishable from a - # crash. Wait-ForKeyPress is the last statement before the exit, so throwing from it - # proves the end of the path was reached without letting the suite exit + It "does not block on a keypress before the run's artefacts exist" { + # Rewritten twice. It first asserted that this function wrote the result JSON, + # which was only true because its own `exit` bypassed the finally that should + # have. Then it asserted the pause happened here - and the second review pass + # showed that was the regression: the pause blocks indefinitely, so in v2.21 the + # JSON was already on disk before it, while pausing here puts an unbounded wait + # BEFORE the artefacts. A window closed at that prompt would leave no result file. + # The pause now belongs to the caller, after Complete-WinCleanRun. Mock Test-InteractiveConsole { $true } Mock Read-Host { 'y' } Mock Update-Script { } Mock Get-ScriptFileVersion { '2.21' } - Mock Write-ResultJson { } - Mock Wait-ForKeyPress { throw 'REACHED_EXIT' } - { Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' - Channel = 'gallery'; Provider = 'PowerShellGet' } } | - Should -Throw -ExpectedMessage 'REACHED_EXIT' + $null = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } - Should -Invoke Write-ResultJson -Times 1 + Should -Invoke Wait-ForKeyPress -Exactly -Times 0 -Because 'nothing may block between the update and the result JSON' $script:Stats.Aborted | Should -Be 'UpdatedAndExited' ($script:printed -join "`n").Contains('Update complete') | Should -BeTrue } @@ -2295,6 +2416,820 @@ Describe "Invoke-ScriptUpdate branches" -Tag "Unit", "Helper", "V221" { } } +#region v2.22 Storage Sense setting is reported, not acted on + +Describe "Test-StorageSenseEnabled" -Tag "Unit", "Helper", "V222" { + # Explains which of two states produced "returned 0 and freed nothing". Deliberately + # NOT wired into the skip decision - see the comment at its call site. + + It "reports enabled when the policy value says so" { + Mock Get-ItemProperty { [pscustomobject]@{ '01' = 1 } } + Test-StorageSenseEnabled | Should -BeTrue + } + + It "reports disabled when the policy value says so" { + Mock Get-ItemProperty { [pscustomobject]@{ '01' = 0 } } + Test-StorageSenseEnabled | Should -BeFalse + } + + It "answers null - not false - when the setting cannot be read" { + # Under SYSTEM (a scheduled task) HKCU is a different hive and the key is absent. + # Reporting "switched off" there would state a fact nobody established. + Mock Get-ItemProperty { throw 'no such key' } + Test-StorageSenseEnabled | Should -BeNullOrEmpty + } + +} + +#endregion + +#region v2.22 DiskCleanupStatus reaches the result JSON + +Describe "DiskCleanupStatus is actually produced and published" -Tag "Unit", "Helper", "V222" { + # Added after a mutation run: the headline field of this release had NO behavioural + # coverage at all. Deleting five of its seven assignments, changing the default, or + # removing the field from the result JSON entirely all left 679 tests green. Only two + # comment-stripped greps existed, and a grep cannot see a value that is never set. + + BeforeEach { + $script:Stats = New-RunStats + $script:resultFile = Join-Path ([System.IO.Path]::GetTempPath()) "WinCleanStatus_$(Get-Random).json" + } + + AfterEach { + Remove-Item -LiteralPath $script:resultFile -Force -ErrorAction SilentlyContinue + } + + It "starts as 'not-run' so an aborted run cannot look like a completed cleanup" { + (New-RunStats).DiskCleanupStatus | Should -Be 'not-run' + } + + It "is written to the result JSON, with the value the run actually reached" { + # The mutation that deleted the field from Write-ResultJson survived; this is the + # assertion that kills it. A consumer reading the documented schema must find it. + $script:Stats.DiskCleanupStatus = 'idle-resident' + + Write-ResultJson -Path $script:resultFile + + $json = Get-Content -LiteralPath $script:resultFile -Raw | ConvertFrom-Json + $json.PSObject.Properties.Name | Should -Contain 'DiskCleanupStatus' + $json.DiskCleanupStatus | Should -Be 'idle-resident' + } + + It "survives the JSON round trip as a string for every documented value - " -ForEach @( + @{ Value = 'not-run' }, @{ Value = 'skipped-parameter' }, @{ Value = 'skipped-report-only' } + @{ Value = 'storage-sense' }, @{ Value = 'running' }, @{ Value = 'completed' } + @{ Value = 'idle-resident' }, @{ Value = 'timeout' } + @{ Value = 'not-armed' }, @{ Value = 'start-failed' }, @{ Value = 'exit-nonzero' } + @{ Value = 'skipped-cleanup-group' } + ) { + # Pins the vocabulary itself: docs/result-json.md documents exactly these, and a + # value that only exists in the code is a contract nobody can consume. + $script:Stats.DiskCleanupStatus = $Value + Write-ResultJson -Path $script:resultFile + (Get-Content -LiteralPath $script:resultFile -Raw | ConvertFrom-Json).DiskCleanupStatus | Should -Be $Value + } + + It "records the preview as its own value, not as 'not-run'" { + # -ReportOnly is the most common way into this function: the smoke test and two + # stand modes all use it. Reporting the documented "the step never executed (for + # example the run aborted earlier)" for a healthy preview is simply untrue. + # Safe to call for real: the ReportOnly branch returns before touching anything. + $ReportOnly = $true + Mock Write-Log { } + + Invoke-StorageSense + + $script:Stats.DiskCleanupStatus | Should -Be 'skipped-report-only' + } +} + +#endregion + +#region v2.22 adaptive console width + +Describe "Get-BoxWidth" -Tag "Unit", "Helper", "V222" { + # The frames were a literal 70 in four places. Widening them is only safe if the + # result is bounded at BOTH ends: too wide and the box wraps, which is the very + # defect being fixed (winget's table wrapping into WinClean's output). + + It "keeps the historical width when the console width is unknown - " -ForEach @( + @{ Case = 'zero'; Width = 0 } + @{ Case = 'negative'; Width = -1 } + ) { + # Redirected output and scheduled tasks report no usable width. Laying out + # against a guess there would corrupt exactly the runs nobody is watching. + Get-BoxWidth -ConsoleWidth $Width | Should -Be 70 + } + + It "fits the console whenever a 70-column box can fit at all - columns" -ForEach @( + @{ Width = 80 }, @{ Width = 100 }, @{ Width = 120 }, @{ Width = 200 }, @{ Width = 3824 } + ) { + # A box costs 2 indent + 1 border + inner + 1 border. Anything wider than the + # console wraps, and a wrapped frame is worse than a narrow one. + # Scoped honestly (raised in review): this property holds for consoles of 74 + # columns and up. Below that no box fits, and the case is covered separately. + $inner = Get-BoxWidth -ConsoleWidth $Width + ($inner + 4) | Should -BeLessOrEqual $Width + } + + It "keeps the historical 70 on a console too narrow for any box - columns" -ForEach @( + @{ Width = 60 }, @{ Width = 40 } + ) { + # Stated as the accepted limitation it is, not hidden. Below 74 columns a framed + # box cannot fit, and it never could - every previous version drew 70 there too. + # Narrowing the frame would change how WinClean looks for everyone in order to + # improve a console nobody has reported using, so the old behaviour is kept and + # written down instead of quietly implied by a bound. + Get-BoxWidth -ConsoleWidth $Width | Should -Be 70 + (Get-BoxWidth -ConsoleWidth $Width) + 4 | Should -BeGreaterThan $Width -Because 'this is the known, accepted limit - the assertion records it rather than pretending it does not exist' + } + + It "does not grow past 90 however wide the console is" { + # The user asked for "somewhat wider", not "as wide as it goes": full-width + # frames on a 3824-column window read worse than 90. + Get-BoxWidth -ConsoleWidth 3824 | Should -Be 90 + Get-BoxWidth -ConsoleWidth 200 | Should -Be 90 + } + + It "actually widens at the reported default of 120 columns" { + # The measured configuration on the machine that reported this: conhost 120x30. + # If this returned 70 the change would have achieved nothing there. + Get-BoxWidth -ConsoleWidth 120 | Should -BeGreaterThan 70 + } + + It "is monotonic - a wider console never yields a narrower box" { + $widths = 40, 60, 76, 80, 96, 120, 200 + $previous = 0 + foreach ($w in $widths) { + $inner = Get-BoxWidth -ConsoleWidth $w + $inner | Should -BeGreaterOrEqual $previous + $previous = $inner + } + } +} + +Describe "Get-ConsoleWidth" -Tag "Unit", "Helper", "V222" { + + It "always answers with an int and never throws" { + # No mock here on purpose: $Host.UI.RawUI is not resolved through any cmdlet, so + # a Mock would be theatre - it would intercept nothing and the test would pass + # for the wrong reason. The no-console path is instead covered where it actually + # matters, by Get-BoxWidth's contract for 0 and negative widths. + { Get-ConsoleWidth } | Should -Not -Throw + Get-ConsoleWidth | Should -BeOfType [int] + } + + It "reports a positive width in this console session" { + # Pester runs in a real console here; if this ever returns 0 locally the + # adaptive path is silently inert and the box stays 70 forever. + Get-ConsoleWidth | Should -BeGreaterThan 0 + } +} + +Describe "Frames stay geometrically consistent at every adaptive width" -Tag "Unit", "Helper", "V222" { + # The smoke test validates geometry too, but it runs with output redirected, where + # the console width is unknown and the box falls back to 70 - so it can only ever + # check the width that existed before this change. Widening is the risky part, and + # this is where it gets checked. + + BeforeAll { + . (Join-Path $PSScriptRoot '..' 'tools' 'BoxGeometry.ps1') + $script:savedBoxWidth = $script:BoxWidth + } + + AfterAll { + $script:BoxWidth = $script:savedBoxWidth + } + + It "renders a consistent banner and section frames at width " -ForEach @( + @{ Width = 70 }, @{ Width = 74 }, @{ Width = 80 }, @{ Width = 90 } + ) { + $script:BoxWidth = $Width + + # Write-Host goes to the information stream in PowerShell 7, so the frames can be + # captured and measured rather than eyeballed. + $out = & { + Show-Banner + Write-Log -Message "A title that must stay centred" -Level TITLE -NoLog + Write-Log -Message "A section header" -Level SECTION -NoLog + Write-Log -Message "An ordinary line" -Level INFO -NoLog + } 6>&1 | ForEach-Object { [string]$_ } + + $lines = ($out -join "`n") -split "`r?`n" | ForEach-Object { $_.TrimEnd() } + $issues = Test-BoxGeometry -Lines $lines + $issues | Should -BeNullOrEmpty -Because "frames must line up at $Width columns, not only at the historical 70" + + # And the frames must actually follow the setting, or this test proves nothing. + # @() around the filter deliberately: a single match indexed with [0] yields the + # first CHARACTER of the string, which quietly measured 0 while looking correct. + $tops = @($lines | Where-Object { $_ -match '^\s*╔═+╗$' }) + $tops | Should -Not -BeNullOrEmpty + [regex]::Match($tops[0], '═+').Value.Length | Should -Be $Width + } + + It "moves the logo as one block, so the letters do not shear apart" { + # Added after a mutation survived: centring each art row independently keeps the + # frame perfectly consistent (every row is padded to the border) while wrecking + # the logo, so the geometry check above cannot see it. The invariant is that + # widening shifts every art row by the SAME amount. + $indentsAt = @{} + foreach ($width in 70, 90) { + $script:BoxWidth = $width + $out = & { Show-Banner } 6>&1 | ForEach-Object { [string]$_ } + $lines = ($out -join "`n") -split "`r?`n" + # Art rows: framed rows whose CONTENT carries logo glyphs. Matching on '█' + # alone finds only five - the last row of the wordmark is drawn entirely from + # '╚═╝' and has no block character in it at all. + $art = @($lines | + ForEach-Object { [regex]::Match($_, '^\s*║(.*)║$').Groups[1].Value } | + Where-Object { $_ -match '[█╚]' }) + $art.Count | Should -Be 6 -Because 'the logo is six rows' + $indentsAt[$width] = $art | ForEach-Object { $_.Length - $_.TrimStart().Length } + } + + $shifts = 0..5 | ForEach-Object { $indentsAt[90][$_] - $indentsAt[70][$_] } + ($shifts | Select-Object -Unique).Count | + Should -Be 1 -Because 'every row of the logo must shift by the same amount, or the letters no longer line up' + $shifts[0] | Should -BeGreaterThan 0 -Because 'a wider box must actually move the block' + + # And at the default width the banner must sit exactly where ten releases of users + # have seen it. Added after a mutation survived the check above: centring each row + # independently keeps the rows in step with each other (they are all the same + # length) while sliding the whole logo sideways, so only an absolute anchor sees it. + $indentsAt[70][0] | Should -Be 6 + $indentsAt[70][1] | Should -Be 5 + $indentsAt[70][5] | Should -Be 6 + } + + It "centres the title rather than padding it by a fixed amount" { + # Added after a mutation survived: a hardcoded left pad still produces a valid box + # (the row is padded out to the border), so only the symmetry can catch it. + $script:BoxWidth = 90 + $out = & { Show-Banner } 6>&1 | ForEach-Object { [string]$_ } + $lines = ($out -join "`n") -split "`r?`n" + + $titleRow = @($lines | Where-Object { $_ -match 'Ultimate Windows 11 Maintenance Script' }) + $titleRow | Should -Not -BeNullOrEmpty + $inner = [regex]::Match($titleRow[0], '^\s*║(.*)║$').Groups[1].Value + $left = $inner.Length - $inner.TrimStart().Length + $right = $inner.Length - $inner.TrimEnd().Length + + [math]::Abs($left - $right) | Should -BeLessOrEqual 1 -Because 'the title must sit in the middle of the frame' + } + + It "keeps the banner square for a version string of any length - ''" -ForEach @( + @{ Version = '2.5' }, @{ Version = '2.21' }, @{ Version = '2.100' }, @{ Version = '10.0' } + ) { + # The banner used to be a literal here-string whose title row was padded by hand + # for exactly four characters of version. It measured 70 columns only because + # "2.21" is four characters; 2.5 or 2.100 would have shifted that row's border + # out of line with the rest of the box - and the next release bumps the version. + $script:BoxWidth = 70 + $savedVersion = $script:Version + try { + $script:Version = $Version + $out = & { Show-Banner } 6>&1 | ForEach-Object { [string]$_ } + $lines = ($out -join "`n") -split "`r?`n" | ForEach-Object { $_.TrimEnd() } + + Test-BoxGeometry -Lines $lines | Should -BeNullOrEmpty + ($lines -join "`n") | Should -Match ([regex]::Escape("v$Version")) + } finally { + $script:Version = $savedVersion + } + } +} + +Describe "Expand-ConsoleWindow" -Tag "Unit", "Helper", "V222" { + + It "never throws, whatever the host does" { + # Windows Terminal refuses programmatic resizing and redirected hosts throw. + # This runs during startup of every single run: it must not be able to fail one. + { Expand-ConsoleWindow -DesiredWidth 140 } | Should -Not -Throw + } + + It "does not shrink a console that is already wider than asked" { + # Asking for less than the current width must be a no-op, not a resize down - + # a user who widened the window deliberately keeps their width. + $before = Get-ConsoleWidth + Expand-ConsoleWindow -DesiredWidth 10 + Get-ConsoleWidth | Should -Be $before + } +} + +#endregion + +#region v2.22 Disk Cleanup idle detection + +Describe "Update-IdleStreak" -Tag "Unit", "Helper", "V222" { + # The rule that decides when a resident cleanmgr is declared finished. Pure, so the + # decision can be exercised without a process - the measured case (finished in ~10s, + # then frozen for the remaining ~890) is otherwise only reproducible on a live machine. + + It "counts consecutive identical fingerprints" { + $s = 0 + $s = Update-IdleStreak -Previous 'a' -Current 'a' -Streak $s + $s | Should -Be 1 + $s = Update-IdleStreak -Previous 'a' -Current 'a' -Streak $s + $s | Should -Be 2 + } + + It "resets the moment the process does anything" { + # One counter moving is enough: a process mid-delete is not idle. + Update-IdleStreak -Previous 'cpu|1|2|3|4' -Current 'cpu|1|2|3|5' -Streak 11 | Should -Be 0 + } + + It "treats an unreadable fingerprint as 'cannot tell', never as idle - " -ForEach @( + @{ Case = 'previous unreadable'; Prev = $null; Curr = 'a' } + @{ Case = 'current unreadable'; Prev = 'a'; Curr = $null } + @{ Case = 'both unreadable'; Prev = $null; Curr = $null } + @{ Case = 'previous empty'; Prev = ''; Curr = 'a' } + @{ Case = 'current empty'; Prev = 'a'; Curr = '' } + ) { + # The safety property. Get-DiskCleanupActivityFingerprint returns $null when WMI cannot + # answer, and if that accumulated towards the threshold a machine with broken WMI + # would cut every Disk Cleanup short after two minutes and call it complete. + Update-IdleStreak -Previous $Prev -Current $Curr -Streak 11 | + Should -Be 0 -Because 'not being able to measure work is not evidence that work finished' + } + + It "compares case-sensitively, so counters differing only in case still count as work" { + # Fingerprints are numeric today, but -eq in PowerShell is case-INSENSITIVE by + # default and this comparison decides whether to stop waiting. Pinned deliberately. + Update-IdleStreak -Previous 'A|1' -Current 'a|1' -Streak 5 | Should -Be 0 + } +} + +Describe "Get-DiskCleanupActivityFingerprint" -Tag "Unit", "Helper", "V222" { + + It "returns a comparable fingerprint for a live process" { + $fp = Get-DiskCleanupActivityFingerprint -ProcessId $PID + $fp | Should -Not -BeNullOrEmpty + # CPU kernel + CPU user + three I/O counters + # Five counters, then '#', then the identity segment (process count, PID list) + ($fp -split '[|#]').Count | Should -Be 7 + $fp | Should -Match '#' -Because 'the activity and identity parts must stay separable' + } + + It "changes after the process does measurable work" { + # Proves the fingerprint actually tracks activity. If it were built from cached or + # constant values, everything would look idle and every cleanmgr would be cut short + # at two minutes - the failure mode that matters most here. + $before = Get-DiskCleanupActivityFingerprint -ProcessId $PID + $sink = 0 + 1..200000 | ForEach-Object { $sink += $_ } + $null = Get-ChildItem -Path ([System.IO.Path]::GetTempPath()) -ErrorAction SilentlyContinue | Select-Object -First 50 + $after = Get-DiskCleanupActivityFingerprint -ProcessId $PID + + $after | Should -Not -Be $before + } + + It "returns null for a process id that does not exist, rather than throwing" { + # cleanmgr can exit between two checks; the caller must get "cannot tell", and + # Update-IdleStreak turns that into a reset rather than a false completion. + Get-DiskCleanupActivityFingerprint -ProcessId 999999 | Should -BeNullOrEmpty + } + + It "the Disk Cleanup fingerprint covers our own process tree, not every cleanmgr" { + # Raised in review, after the first attempt aggregated by process NAME. A Disk + # Cleanup the user opens by hand is also cleanmgr.exe, and including it made the + # verdict depend on a stranger: its activity could satisfy "work was observed" for + # our run, and its busyness could keep us waiting the full fifteen minutes after + # our own cleanup had finished. Descendants of our PID, and nothing else. + Mock Get-CimInstance { + @( + [pscustomobject]@{ ProcessId = 100; ParentProcessId = 1; KernelModeTime = 10; UserModeTime = 1; ReadOperationCount = 1; WriteOperationCount = 1; OtherOperationCount = 1 } + [pscustomobject]@{ ProcessId = 200; ParentProcessId = 100; KernelModeTime = 20; UserModeTime = 2; ReadOperationCount = 2; WriteOperationCount = 2; OtherOperationCount = 2 } + # An unrelated cleanmgr the user started: must not appear in the fingerprint + [pscustomobject]@{ ProcessId = 900; ParentProcessId = 1; KernelModeTime = 99999; UserModeTime = 99999; ReadOperationCount = 99999; WriteOperationCount = 99999; OtherOperationCount = 99999 } + ) + } + + $fp = Get-DiskCleanupActivityFingerprint -ProcessId 100 + + # our process + its child: kernel 10+20, and the PID list names exactly those two + $fp | Should -Be '30|3|3|3|3#2|100,200' + $fp | Should -Not -Match '900' -Because 'an unrelated cleanmgr must not influence our verdict' + } + + It "the Disk Cleanup fingerprint is null when our process is gone" { + Mock Get-CimInstance { @([pscustomobject]@{ ProcessId = 900; ParentProcessId = 1; KernelModeTime = 1; UserModeTime = 1; ReadOperationCount = 1; WriteOperationCount = 1; OtherOperationCount = 1 }) } + Get-DiskCleanupActivityFingerprint -ProcessId 100 | Should -BeNullOrEmpty + } + + It "the Disk Cleanup fingerprint terminates on a cyclic parent chain" { + # PID reuse can produce a cycle; the walk must end rather than spin forever inside + # a wait that is already holding up the run. + Mock Get-CimInstance { + @( + [pscustomobject]@{ ProcessId = 100; ParentProcessId = 200; KernelModeTime = 1; UserModeTime = 0; ReadOperationCount = 0; WriteOperationCount = 0; OtherOperationCount = 0 } + [pscustomobject]@{ ProcessId = 200; ParentProcessId = 100; KernelModeTime = 1; UserModeTime = 0; ReadOperationCount = 0; WriteOperationCount = 0; OtherOperationCount = 0 } + ) + } + $fp = Get-DiskCleanupActivityFingerprint -ProcessId 100 + $fp | Should -Be '2|0|0|0|0#2|100,200' + } + + It "sums counters as integers, without the precision loss of a Double" { + # Measure-Object -Sum returns a Double and silently loses integer precision past + # 2^53, so a small counter change could vanish at large totals (raised in review). + $big = 9007199254740993 # 2^53 + 1, not representable as a Double + Mock Get-CimInstance { + @([pscustomobject]@{ ProcessId = 100; ParentProcessId = 1; KernelModeTime = $big; UserModeTime = 0; ReadOperationCount = 0; WriteOperationCount = 0; OtherOperationCount = 0 }) + } + (Get-DiskCleanupActivityFingerprint -ProcessId 100).Split('|')[0] | Should -Be "$big" + } + + It "returns null - never a constant - when the counters cannot be read at all" { + # Added after a mutation survived. The test above exercises the "no such process" + # branch, where Get-CimInstance returns nothing; it never reached the catch, which + # is where a broken WMI lands. Returning any FIXED value there would be the worst + # possible answer: two consecutive unreadable checks would compare equal, the idle + # streak would build on pure ignorance, and a machine with broken WMI would cut + # every Disk Cleanup short and call it finished. + Mock Get-CimInstance { throw 'WMI is unavailable' } + + $first = Get-DiskCleanupActivityFingerprint -ProcessId $PID + $second = Get-DiskCleanupActivityFingerprint -ProcessId $PID + + $first | Should -BeNullOrEmpty + Update-IdleStreak -Previous $first -Current $second -Streak 11 | + Should -Be 0 -Because 'two unreadable samples must not look like two identical ones' + + # The function actually used by the Disk Cleanup wait must obey the same rule. + # Raised in review: this guard used to exercise only the single-process helper, + # so a fixed value returned from the family function's catch would have survived it. + Get-DiskCleanupActivityFingerprint -ProcessId $PID | Should -BeNullOrEmpty + } +} + +Describe "Wait-CleanmgrCompletion" -Tag "Unit", "Helper", "V222" { + # The whole wait, exercised without a process and without waiting fifteen minutes - + # the caller injects exit state, activity and the sleep. This is the part that could + # previously only be observed on a live machine, which is why the defect it fixes + # survived into a release: on the stand cleanmgr exits normally. + + BeforeEach { + $script:progressAt = [System.Collections.Generic.List[int]]::new() + $script:noWait = { param($seconds) } + $script:onProgress = { param($seconds) $script:progressAt.Add($seconds) } + } + + It "returns 'exited' as soon as the process leaves" { + $script:calls = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $script:calls++; $script:calls -gt 3 } ` + -GetFingerprint { "moving-$($script:calls)" } ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'exited' + $r.Elapsed | Should -BeLessThan 900 + } + + It "stops waiting once the process has been completely still for the threshold" { + # The measured case: works briefly, then never exits and never moves again. The verdict + # is 'nothing left to observe', not 'proven finished'. One interval of real work + # first, because stillness only counts after something was seen happening. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { $script:n++; if ($script:n -le 2) { "working-$($script:n)" } else { 'frozen' } } ` + -CheckInterval 10 -IdleChecksRequired 12 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'idle-resident' + # 20s of observed movement (working-1 -> working-2 -> frozen, two changes), then + # 12 identical checks of 10s each. + $r.Elapsed | Should -Be 140 -Because 'the streak only starts once the fingerprint stops changing' + } + + It "waits the full timeout when the process keeps working" { + # Every check shows movement, so the idle streak never builds - this must still + # behave exactly as it did before v2.22. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { $script:n++; "busy-$($script:n)" } ` + -MaxWaitSeconds 900 -CheckInterval 10 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' + $r.Elapsed | Should -Be 900 + } + + It "does not call a busy process idle just because it pauses briefly" { + # Stillness must be CONSECUTIVE. A process that goes quiet for a while and then + # resumes is working, and cutting it short would truncate a real cleanup. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { + $script:n++ + # quiet for 8 checks, then a burst of work, repeatedly + if ($script:n % 10 -lt 8) { 'quiet-block-' + [math]::Floor($script:n / 10) } else { "work-$($script:n)" } + } ` + -MaxWaitSeconds 900 -CheckInterval 10 -IdleChecksRequired 12 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' -Because 'the streak never reached 12 consecutive still checks' + } + + It "never declares idle when activity cannot be measured at all" { + # Broken WMI must not look like a finished cleanup. Without this, such a machine + # would silently cut every Disk Cleanup off after two minutes. + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { $null } ` + -MaxWaitSeconds 900 -CheckInterval 10 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' + $r.Elapsed | Should -Be 900 + } + + It "reports progress once a minute, not on every check" { + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { $script:n++; "busy-$($script:n)" } ` + -MaxWaitSeconds 300 -CheckInterval 10 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' + $script:progressAt | Should -Be @(60, 120, 180, 240, 300) + } + + It "does not call a process finished if it was never seen doing anything" { + # Raised in review: without this, "it has finished" and "it has not started yet" + # are the same observation. A cleanmgr that is suspended, or waiting on something + # before it begins, would otherwise be declared complete after two minutes of a + # stillness that never followed any work at all. + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { 'frozen-from-the-start' } ` + -MaxWaitSeconds 900 -CheckInterval 10 -IdleChecksRequired 12 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' -Because 'stillness is only taken as "nothing left to wait for" if work was observed first' + $r.Elapsed | Should -Be 900 + } + + It "does not count an unreadable sample as the activity that qualifies an early verdict" { + # A reset caused by "cannot tell" must not stand in for observed work: otherwise + # a machine with flaky WMI could satisfy the sawActivity condition without any + # process ever having done anything. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { + $script:n++ + # one unreadable blip, then frozen for the rest of the run + if ($script:n -eq 2) { $null } else { 'frozen' } + } ` + -MaxWaitSeconds 900 -CheckInterval 10 -IdleChecksRequired 12 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'timeout' + } + + It "still ends early when real work was observed and then stopped" { + # The measured case, now with the stricter rule in place: it must still work. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $false } ` + -GetFingerprint { + $script:n++ + if ($script:n -le 3) { "working-$($script:n)" } else { 'done-and-frozen' } + } ` + -MaxWaitSeconds 900 -CheckInterval 10 -IdleChecksRequired 12 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'idle-resident' + $r.Elapsed | Should -BeLessThan 900 + } + + It "prefers 'exited' over 'timeout' when the process leaves during the final interval" { + # Same instant, two possible labels; the accurate one is that it exited. + $script:n = 0 + $r = Wait-CleanmgrCompletion ` + -HasExited { $script:n -ge 30 } ` + -GetFingerprint { $script:n++; "busy-$($script:n)" } ` + -MaxWaitSeconds 300 -CheckInterval 10 ` + -Wait $script:noWait -OnProgress $script:onProgress + + $r.Outcome | Should -Be 'exited' + } +} + +#endregion + +#region v2.22 single end-of-run path + +Describe "Invoke-ScriptUpdate stop/continue contract" -Tag "Unit", "Helper", "V222" { + # v2.22: the function used to end the process itself, so its most important branch - + # the successful update - could not be tested at all without killing the suite. It now + # answers a question ("is the run over?") and the caller ends the run. That answer is + # the whole contract: get it wrong in the false direction and a run continues doing + # maintenance with a replaced script file underneath it; wrong in the true direction + # and an ordinary run stops before doing any work. + + BeforeEach { + $script:Stats.WarningsCount = 0 + $script:Stats.ErrorsCount = 0 + $script:Stats.Aborted = $null + $script:printed = [System.Collections.Generic.List[string]]::new() + Mock Write-Host { if ($Object) { $script:printed.Add([string]$Object) } } + Mock Wait-ForKeyPress { } + } + + It "returns true after an update it verified, so the caller ends the run" { + Mock Test-InteractiveConsole { $true } + Mock Read-Host { 'y' } + Mock Select-UpdateCommand { 'Update-Script' } + Mock Update-Script { } + Mock Get-ScriptFileVersion { '2.21' } + + $stop = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } + + $stop | Should -BeOfType [bool] -Because 'a leaked pipeline object here would make the caller decide on an array' + $stop | Should -BeTrue + $script:Stats.Aborted | Should -Be 'UpdatedAndExited' + ($script:printed -join "`n").Contains('Update complete') | Should -BeTrue + } + + It "stays a plain boolean even when the update provider writes to the pipeline" { + # Added after a mutation survived: dropping the `$null =` in front of the provider + # switch changed nothing, because every mock returns nothing. A real provider that + # emitted an object would make this function return an array, and the caller's + # `if (...)` would then be judging the array rather than the answer. Update-Script + # and Update-PSResource are documented as returning nothing, but "documented as" + # is not "guaranteed to", and the cost of the guard is one assignment. + Mock Test-InteractiveConsole { $true } + Mock Read-Host { 'y' } + Mock Select-UpdateCommand { 'Update-Script' } + Mock Update-Script { [pscustomobject]@{ Name = 'WinClean'; Version = '2.21' } } + Mock Get-ScriptFileVersion { '2.21' } + + $stop = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } + + @($stop).Count | Should -Be 1 -Because 'provider output must not be part of the answer' + $stop | Should -BeOfType [bool] + $stop | Should -BeTrue + } + + It "does not write the result JSON itself - that belongs to the shared end-of-run path" { + # The v2.21 shape: this function hand-copied the JSON write because its own exit + # bypassed the finally. Copying it back would recreate two owners of one artefact. + Mock Test-InteractiveConsole { $true } + Mock Read-Host { 'y' } + Mock Select-UpdateCommand { 'Update-Script' } + Mock Update-Script { } + Mock Get-ScriptFileVersion { '2.21' } + Mock Write-ResultJson { } + + $null = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } + + Should -Invoke Write-ResultJson -Times 0 + } + + It "returns false in a preview, without ever attempting an update" { + # Rewritten (raised in review): this case used to live in the -ForEach below, whose + # body sets a LOCAL $ReportOnly = $false before running each Setup. PowerShell + # resolves variables through the dynamic scope chain, so the product saw that local + # and not the $script: value the Setup wrote - the case never entered the ReportOnly + # branch at all and merely duplicated 'non-interactive'. Proved by mutation: making + # the ReportOnly branch return $true survived this Describe entirely. + # The local assignment here is what the function actually reads. + $ReportOnly = $true + Mock Test-InteractiveConsole { $true } + Mock Read-Host { 'y' } + Mock Update-Script { } + Mock Update-PSResource { } + + $stop = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } + + $stop | Should -BeFalse + Should -Invoke Update-Script -Exactly -Times 0 -Because 'a preview must change nothing' + Should -Invoke Update-PSResource -Exactly -Times 0 + $script:Stats.Aborted | Should -BeNullOrEmpty + } + + It "returns false so the run continues - " -ForEach @( + @{ Case = 'non-interactive'; Setup = { Mock Test-InteractiveConsole { $false } } } + @{ Case = 'user declines'; Setup = { Mock Test-InteractiveConsole { $true }; Mock Read-Host { 'n' } } } + @{ Case = 'update command failed'; Setup = { + Mock Test-InteractiveConsole { $true }; Mock Read-Host { 'y' } + Mock Select-UpdateCommand { 'Update-Script' } + Mock Update-Script { throw 'gallery exploded' } } } + @{ Case = 'version did not change'; Setup = { + Mock Test-InteractiveConsole { $true }; Mock Read-Host { 'y' } + Mock Select-UpdateCommand { 'Update-Script' } + Mock Update-Script { } + Mock Get-ScriptFileVersion { '2.20' } } } + ) { + $ReportOnly = $false + & $Setup + + $stop = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = 'gallery'; Provider = 'PowerShellGet' } + + $stop | Should -BeFalse -Because 'the maintenance the user asked for must still run' + $script:Stats.Aborted | Should -BeNullOrEmpty + } + + It "returns false for a copy it cannot aim an update at - " -ForEach @( + @{ Channel = 'installer' } + @{ Channel = 'oneliner' } + @{ Channel = 'manual' } + @{ Channel = 'unknown' } + @{ Channel = 'gallery-ambiguous' } + ) { + $ReportOnly = $false + Mock Test-InteractiveConsole { $true } + + $stop = Invoke-ScriptUpdate -UpdateInfo @{ CurrentVersion = '2.20'; LatestVersion = '2.21' + Channel = $Channel; Provider = 'PowerShellGet' } + + $stop | Should -BeFalse + } +} + +Describe "Complete-WinCleanRun" -Tag "Unit", "Helper", "V222" { + # One end-of-run path for the normal finally and for both abort branches. Before this + # the list of things a run must do on the way out existed in three places, and v2.21 + # shipped two separate fixes to the copies rather than one fix to the list. + + BeforeEach { + $script:Stats.Aborted = $null + $script:RunCompleted = $false + Mock Write-ResultJson { } + Mock Show-FinalStatistics { } + } + + It "writes the result JSON and shows the summary for an ordinary run" { + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + + # -Exactly throughout this block, established by experiment: Pester treats + # `-Times 0` as "never" but `-Times N` (N>0) as "AT LEAST N", so the plain form + # cannot see a duplicate. That is exactly what the latch below has to prevent, and + # a mutation run proved the non-exact assertion could not catch its removal. + Should -Invoke Write-ResultJson -Exactly -Times 1 + Should -Invoke Show-FinalStatistics -Exactly -Times 1 + } + + It "still writes the JSON for an aborted run but shows no summary - " -ForEach @( + @{ Aborted = 'UpdatedAndExited' } + @{ Aborted = 'PendingRebootDeclined' } + ) { + # Automation must always get the artefact; a human must not be told + # "COMPLETED SUCCESSFULLY" about a run that deliberately did nothing. + $script:Stats.Aborted = $Aborted + + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + + Should -Invoke Write-ResultJson -Exactly -Times 1 + Should -Invoke Show-FinalStatistics -Exactly -Times 0 + } + + It "is latched - an abort path that also unwinds through the finally writes once" { + # Both abort paths call this explicitly and then return; the finally calls it again + # for every other run. Without the latch the JSON would be written twice and the + # summary shown twice for the ordinary case. + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + + Should -Invoke Write-ResultJson -Exactly -Times 1 + Should -Invoke Show-FinalStatistics -Exactly -Times 1 + } + + It "releases the log handle so the log can be moved right after the run" { + $logFile = Join-Path ([System.IO.Path]::GetTempPath()) "WinCleanComplete_$(Get-Random).log" + $prevPath = $script:LogPath + $script:LogPath = $logFile + try { + Write-LogFileLine -Line 'held open' -StartNewFile + $script:LogWriter | Should -Not -BeNullOrEmpty -Because 'the test needs a real open handle to prove it gets released' + + Complete-WinCleanRun -ResultPath 'C:\some\result.json' + + $script:LogWriter | Should -BeNullOrEmpty + # The operational point of releasing it, asserted as behaviour rather than as + # a null check: a still-open handle would make this fail on Windows. + { Remove-Item -LiteralPath $logFile -Force -ErrorAction Stop } | Should -Not -Throw + } finally { + if ($script:LogWriter) { $script:LogWriter.Dispose(); $script:LogWriter = $null; $script:LogWriterPath = $null } + Remove-Item -LiteralPath $logFile -Force -ErrorAction SilentlyContinue + $script:LogPath = $prevPath + } + } +} + +#endregion + Describe "Update-Applications when winget is absent" -Tag "Unit", "Helper", "V221" { BeforeEach { @@ -2342,11 +3277,17 @@ Describe "Update-Applications when winget is absent" -Tag "Unit", "Helper", "V22 # -Skip stops the dispatch, so the in-function branch is unreachable there and the # status would stay 'not-run'. This pins the assignment that Start-WinClean makes # before the phase, which no behavioural test in this suite can reach. + # Ordering, not proximity (v2.22): this used to look inside a 400-character window + # before the dispatch, and adding one more pre-phase assignment next to it pushed + # the line out of that window and failed a test about something else entirely. + # What matters is that the assignment happens BEFORE the phase is dispatched. $source = Get-Content $script:WinCleanPath -Raw $dispatch = $source.IndexOf("Invoke-Phase -Name 'Updates'") + $assign = $source.IndexOf("AppUpdatesStatus = 'skipped-parameter'`n", [StringComparison]::Ordinal) + if ($assign -lt 0) { $assign = $source.IndexOf("AppUpdatesStatus = 'skipped-parameter' }", [StringComparison]::Ordinal) } $dispatch | Should -BeGreaterThan 0 - $preamble = $source.Substring([math]::Max(0, $dispatch - 400), [math]::Min(400, $dispatch)) - $preamble.Contains("AppUpdatesStatus = 'skipped-parameter'") | Should -BeTrue + $assign | Should -BeGreaterThan 0 + $assign | Should -BeLessThan $dispatch -Because 'the status must be set before the phase is dispatched, not merely somewhere near it' } It "keeps a present-but-failing winget an ERROR, and does not call that check 'checked'" { diff --git a/tests/StandHelpers.Tests.ps1 b/tests/StandHelpers.Tests.ps1 index 352594c..046a668 100644 --- a/tests/StandHelpers.Tests.ps1 +++ b/tests/StandHelpers.Tests.ps1 @@ -115,3 +115,49 @@ Describe "Test-ResultSupportsPhaseBuckets (version gate for stand assertions)" - Test-ResultSupportsPhaseBuckets ([string]$r.Version) | Should -BeTrue } } + +Describe "Stand report-mode guard truth table" -Tag "Unit", "Stand", "V222" { + <# + An external reviewer read the guard in Invoke-StandTest.ps1 + + if ($Mode -in 'Report', 'ReportNoCleanup' -and -not $result.ReportOnly) + + as ambiguous, and suspected ReportNoCleanup might fall outside it - which would + let a report-mode run that silently performed real changes pass as correct (the + 18.07 incident). It was a false alarm: PowerShell binds -in tighter than -and, so + the grouping was already ($Mode -in @(...)) -and (-not ...), confirmed by the AST. + + The expression is parenthesised now for readability, and pinned here so the + question is settled by a test rather than re-argued from precedence rules. + #> + BeforeAll { + # Pester 5 runs a Describe body during discovery only, so a bare `function` + # statement there is gone by the time the tests execute (the same trap is + # documented in Helpers.Tests.ps1). + function Test-ReportModeGuard { + param($Mode, $ReportOnly) + if (($Mode -in 'Report', 'ReportNoCleanup') -and (-not $ReportOnly)) { 'flagged' } else { 'accepted' } + } + } + + It "flags when the result JSON does not confirm ReportOnly" -ForEach @( + @{ Mode = 'Report' } + @{ Mode = 'ReportNoCleanup' } + ) { + Test-ReportModeGuard -Mode $Mode -ReportOnly $false | Should -Be 'flagged' + } + + It "accepts when ReportOnly is confirmed" -ForEach @( + @{ Mode = 'Report' } + @{ Mode = 'ReportNoCleanup' } + ) { + Test-ReportModeGuard -Mode $Mode -ReportOnly $true | Should -Be 'accepted' + } + + It "does not apply to , which is expected to change things" -ForEach @( + @{ Mode = 'Full' } + @{ Mode = 'FullWithUpdates' } + ) { + Test-ReportModeGuard -Mode $Mode -ReportOnly $false | Should -Be 'accepted' + } +} diff --git a/tools/Invoke-ReleaseCheck.ps1 b/tools/Invoke-ReleaseCheck.ps1 index dbe585f..ae853f2 100644 --- a/tools/Invoke-ReleaseCheck.ps1 +++ b/tools/Invoke-ReleaseCheck.ps1 @@ -186,9 +186,15 @@ if ($pester) { elseif ($failedContainers) { "$failedContainers тест-файл(ов) не загрузились - их тесты не выполнялись" } else { '' }) + # v2.22: CHANGELOG.md added (raised in review). It was the one document a user reads + # and quotes, it states the count in the form "A -> B", and nothing verified it - so it + # shipped stale while the two files that WERE checked had already been corrected. $countClaims = @( @{ File = 'CLAUDE.md'; Pattern = "$pesterCount Pester" } @{ File = 'CONTRIBUTING.md'; Pattern = "$pesterCount tests" } + # Literal, not a regex: the matcher below runs every pattern through + # [regex]::Escape, so a hand-written "\." would be searched for as a backslash. + @{ File = 'CHANGELOG.md'; Pattern = "-> $pesterCount." } ) $wrongCounts = foreach ($claim in $countClaims) { $full = Join-Path $repoRoot $claim.File diff --git a/tools/proxmox/Invoke-StandTest.ps1 b/tools/proxmox/Invoke-StandTest.ps1 index 4ab951a..8c2c8bd 100644 --- a/tools/proxmox/Invoke-StandTest.ps1 +++ b/tools/proxmox/Invoke-StandTest.ps1 @@ -195,7 +195,13 @@ if (-not $jsonRaw) { # A report mode that silently ran for real is the 18.07 incident: guard it explicitly # before trusting the ReportOnly-driven branch below, or a run that dropped ReportOnly # would fall through to the "Full freed enough" check and pass while destroying data. - if ($Mode -in 'Report', 'ReportNoCleanup' -and -not $result.ReportOnly) { + # Parenthesised in v2.22 for readability only. An external reviewer read the + # unparenthesised form as ambiguous and suspected ReportNoCleanup fell outside the + # guard; it did not - PowerShell binds -in tighter than -and, and both the AST and a + # truth table confirmed the grouping was already ($Mode -in @(...)) -and (-not ...). + # The behaviour is unchanged; the parentheses just stop the next reader from having to + # verify that again. StandHelpers.Tests.ps1 pins the truth table. + if (($Mode -in 'Report', 'ReportNoCleanup') -and (-not $result.ReportOnly)) { $failures += "ReportOnly not confirmed in result JSON for mode $Mode" } if ($result.ReportOnly) {