Add comprehensive roadmap for pc-powershelltools project - #1
Conversation
Documents the current state of the four scripts, the structural issues blocking growth (duplicated logic across three UIs, blocking UI thread, no repo infrastructure, unpinned irm|iex distribution), and a phased plan: repo foundation, PCTools module extraction, promoting the GUI framework to the app shell, safety/trust work, the existing feature roadmap, and distribution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ugbD2a2kVkzXGagAw9zzf
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe43d28032
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Every mutating function gets `[CmdletBinding(SupportsShouldProcess)]`.** | ||
| This delivers two roadmap items at once: `-WhatIf` is the dry-run mode, and | ||
| `-Confirm` with `ConfirmImpact = 'High'` is the safe-confirmation prompt. |
There was a problem hiding this comment.
SupportsShouldProcess only adds the common -WhatIf/-Confirm parameters; it does not suppress the function body. In particular, the planned native dism, sfc, netsh, and winget calls will still execute unless every mutating action explicitly gates them with $PSCmdlet.ShouldProcess(...). Without that requirement, the Phase 3 preflight can apply destructive changes while being presented as a dry run.
Useful? React with 👍 / 👎.
| - **Pin the install command.** Point the README at a release tag, never `main`: | ||
| `irm https://raw.githubusercontent.com/likeBloodMoon/pc-powershelltools/v0.3/pc-cleanuptool.ps1 | iex`, | ||
| and publish a `SHA256SUMS` file with a verify-then-run snippet next to the | ||
| one-liner. | ||
| - **Authenticode-sign** release scripts in the CI pipeline. |
There was a problem hiding this comment.
The displayed primary command still pipes remote text directly to iex, so neither the proposed SHA256 file nor the Authenticode signature is checked by users following it; Authenticode verification is not performed for text passed to Invoke-Expression. A moved/compromised release tag or release object would therefore still execute with Administrator privileges. Make the install command download and verify the checksum/signature before execution (or remove the direct irm | iex path) rather than treating adjacent verification instructions as protection.
Useful? React with 👍 / 👎.
| - `Complete-AsyncTask` calls `Invoke-UI { & $Task.OnSuccess $result }`. The | ||
| scriptblock is queued with `BeginInvoke` and runs later, resolving `$Task` and | ||
| `$result` from the enclosing scope at *execution* time. When two tasks complete | ||
| in the same 120 ms timer tick, both queued callbacks are likely to see the last | ||
| task's values. Pass them as explicit `BeginInvoke` arguments and add a |
There was a problem hiding this comment.
Complete-AsyncTask is invoked by the WinForms timer tick (gui-framework.ps1:618-625), which runs on the UI thread. On that thread Invoke-UI takes its inline & $Script branch rather than BeginInvoke (gui-framework.ps1:109-115), so each success callback finishes before the loop advances to the next completed task. The described late-binding race and the proposed BeginInvoke-argument regression test target behavior that cannot occur in the current framework.
Useful? React with 👍 / 👎.
Adds the infrastructure the project had none of: - MIT LICENSE and .gitignore - CHANGELOG.md with the existing v0.1/v0.2 history - build/Invoke-Build.ps1: one Test/Analyze/Release entry point that CI and local development both call - GitHub Actions CI running PSScriptAnalyzer and Pester on both Windows PowerShell 5.1 (the runtime the WinForms GUIs target) and PowerShell 7 - Pester suite asserting every script parses, guarding the failure mode that matters most here: a syntax break reaching users who pipe the script straight into iex as Administrator The encoding test caught a live bug. Both GUIs stored non-ASCII characters without a UTF-8 BOM, so Windows PowerShell 5.1 decoded them as ANSI: Net Diag's minimize button rendered as a mojibake sequence rather than a dash, and the Cleanup Tool's elevation prompt showed a mangled arrow. Replaced with ASCII equivalents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ugbD2a2kVkzXGagAw9zzf
Moves every action out of the WinForms click handlers into a real module, so the same code is usable from the GUI, a console session and a scheduled task - and is testable without a UI. Three rules shape the extraction: - Every action returns a PCTools.ActionResult (Action/Status/Detail/ BytesFreed/Duration/RebootRequired) instead of writing a line to a log box. One shape, consumed by the GUI, the console and Export-PCReport. - Every mutating action declares SupportsShouldProcess, so -WhatIf is the dry run and -Confirm is the safety prompt. - Approved verbs and a PC noun prefix throughout, replacing Flush-, Run-, Create-, Load- and Require-. 28 public functions across Cleanup, Repair, Network, Preferences, Software, plus orchestration (Invoke-PCMaintenance, maintenance profiles) and reporting (Export-PCReport). Behaviour fixed while extracting: - Every external command now runs through Invoke-PCProcess, the timeout wrapper that previously guarded only Net Diag's full scan. A stuck DISM no longer hangs the caller forever. - DISM, SFC, CHKDSK and winget exit codes are interpreted rather than piped to Out-Null; a failed repair no longer looks identical to a successful one. SFC's four real outcomes are distinguished. - Set-PCNetworkAddress removes the existing address and route first. The original failed with an "instance already exists" error on any adapter that already had an address. - Set-PCDhcp also resets DNS, which the original left static. - Clear-PCWindowsUpdateCache restarts wuauserv in a finally block, so a mid-run failure no longer leaves Windows Update stopped. - New-PCRestorePoint detects Windows silently throttling the 24-hour limit instead of reporting success when nothing was created. - Preferences are data, not code, so they can be read (Get-PCPreference) and reverted - the original had six one-way setters and no way to tell what was already applied. - Folder cleanup enumerates once instead of recursing twice, uses -LiteralPath so paths containing brackets are not skipped, and reports bytes reclaimed. - ConvertTo-PCPrefixLength rejects non-contiguous masks, and handles a /0 mask without tripping StrictMode on a null pipeline. Tests: manifest/export contract, approved-verb and help conventions, the ShouldProcess contract for all 18 mutating actions, and unit coverage of the byte formatter, subnet conversion, action wrapper, connectivity verdict engine, profiles, preference definitions and log sinks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ugbD2a2kVkzXGagAw9zzf
1 participant
Summary
This PR adds a detailed roadmap document that outlines a strategic plan for consolidating four standalone PowerShell scripts into a unified, maintainable toolkit. The roadmap provides clear visibility into the project's current state, identifies architectural and operational gaps, and proposes a phased approach to address them.
Key Changes
pc-cleanuptool.ps1,pc-netdiag.ps1,gui-framework.ps1,quickspeedboost.ps1), their sizes, roles, and architectural maturity levelsirm | iexwithout pinning/signing).gitignore, CI/CD, documentation fixes)PCToolsmodule with standardized verbs, structured return objects, andSupportsShouldProcessfor-WhatIfdry-run supportgui-framework.ps1to unified application shell with proper async/runspace architecturequickspeedboost.ps1consolidationNotable Details
SupportsShouldProcessusage)irm | iexdistribution model as a trust and safety concern requiring pinning and checksumshttps://claude.ai/code/session_011ugbD2a2kVkzXGagAw9zzf