feat(window-manager): ✨ Enhance window management capabilities#153
Merged
PrzemyslawKlys merged 14 commits intov2-speedygonzalesfrom Jan 23, 2026
Merged
feat(window-manager): ✨ Enhance window management capabilities#153PrzemyslawKlys merged 14 commits intov2-speedygonzalesfrom
PrzemyslawKlys merged 14 commits intov2-speedygonzalesfrom
Conversation
* Added methods to minimize and restore multiple windows at once. * Introduced `WindowQueryOptions` for flexible window enumeration. * Implemented process information retrieval for windows, including elevation status. * Added validation for window handles to prevent errors. * Enhanced `GetWindows` method to support additional filters such as `includeCloaked` and `includeOwned`. * Improved window positioning logic to ensure windows are visible on monitors. * Introduced `WindowProcessInfo` to encapsulate process-related data for windows.
* Introduced `SupportedOSPlatform("windows")` attribute to specify compatibility.
* This change enhances platform targeting for the application.
* Added new parameters to `CmdletSetDesktopWindowText` for improved input handling. * Introduced `CmdletGetDesktopWindowProcessInfo` to retrieve process metadata for desktop windows. * Updated documentation for clarity and added support for filtering options in `CmdletGetDesktopWindow`. * Added support for Windows platform in `AssemblyInfo.Platform.cs`.
* Introduced new tests for window child enumeration, process info retrieval, and state helpers. * Enhanced existing tests for window activation, positioning, and visibility with better process handling. * Improved test reliability by ensuring Notepad instances are properly managed and cleaned up. * Added assertions to verify expected behaviors in window state changes and layout management. * Ensured compatibility with Windows platform checks and interactive session requirements.
* Added support for interactive UI tests based on environment variables. * Improved cleanup logic for processes in test cases to ensure proper resource management. * Updated test cases to validate against multiple editable control classes.
Contributor
|
Claude finished @PrzemyslawKlys's task —— View job PR Review: Window Management Capabilities EnhancementI've completed a thorough review of this significant PR that enhances window management capabilities. Here are my findings: Todo List:
🟢 StrengthsArchitecture & Design:
Security Implementation:
Test Coverage:
🟡 Areas for ImprovementCode Quality:
Performance Concerns: using var process = Process.GetProcessById((int)processId); // Potential performance impactConsider caching or batch processing for multiple windows from the same process.
🟠 Security ConsiderationsHandle Leaks Prevention:
Access Rights:
🔵 Minor IssuesPowerShell Cmdlets:
🏆 Overall AssessmentThis is a high-quality enhancement that significantly improves the window management capabilities. The code demonstrates:
Recommendation: ✅ Approve with minor suggestions The identified issues are minor and don't impact the core functionality. The performance concerns are worth monitoring in production but aren't blockers. The security implementation is solid with proper handle management and access rights. |
Contributor
|
Claude finished @PrzemyslawKlys's task —— View job Comprehensive PR Review ✅I've completed a thorough review of this window management enhancement PR. Here's my detailed analysis: Todo List:
🎯 SummaryThis PR significantly enhances the DesktopManager's window management capabilities with well-architected features including process information retrieval, improved filtering, and comprehensive test coverage. ✅ Code Quality & Best PracticesExcellent Architecture
Strong PowerShell Integration
🛡️ Security AnalysisRobust Security Implementation
Native Method Safety
🚀 Performance ConsiderationsEfficient Implementation
Resource Management
🧪 Test Coverage AssessmentComprehensive Testing
Test Quality
🔍 Potential Issues & RecommendationsMinor Concerns
Suggestions
🎯 Overall AssessmentStrengths
Final VerdictThis is a high-quality PR that significantly enhances the window management capabilities while maintaining security, performance, and code quality standards. The implementation demonstrates mature understanding of Windows APIs and follows defensive programming practices. Recommendation: ✅ Approve - This PR is ready for merge with only minor suggestions for future enhancement. |
![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: 825d2ea138
ℹ️ 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".
Comment on lines
+115
to
+116
| var options = new WindowInputOptions { | ||
| ActivateWindow = !NoActivate, |
There was a problem hiding this comment.
When -NoActivate (or -SafeMode) is used, ActivateWindow is set to false here, but the default typing path still uses WindowInputService.SendInputText (SendInput), which sends keystrokes to the current foreground window rather than the target handle. In the common case where the target window is not already focused, this means Set-DesktopWindowText -NoActivate will type into the wrong window. Consider forcing UseSendInput = false (WM_CHAR) or throwing when activation is disabled but -UseMessage isn’t set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Enhances DesktopManager’s window management, input, and wallpaper capabilities while making UI-impacting tests opt-in for safer local/CI execution.
Changes:
- Adds richer window querying (options-based filtering, z-order/topmost/visibility/cloaking/ownership metadata) and new process info APIs.
- Improves window input robustness (retryable clipboard/activation/typing options) and control click behavior.
- Refactors tests to be opt-in for interactive/desktop-changing scenarios and updates target frameworks/platform annotations.
Reviewed changes
Copilot reviewed 89 out of 89 changed files in this pull request and generated 87 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/InvokeDesktopWindowScreenshot.Tests.ps1 | Makes screenshot UI tests opt-in and safer process cleanup. |
| Tests/GetDesktopWindowControl.Tests.ps1 | Makes UI control enumeration tests opt-in and more resilient to Notepad variants. |
| TODO.md | Adds roadmap/tasks for future window/monitor features. |
| Sources/WindowTextHelper32/WindowTextHelper32.csproj | Updates net8 target to Windows-specific TFM and publish settings. |
| Sources/WindowTextHelper32/AssemblyInfo.Platform.cs | Adds Windows platform annotation for newer TFMs. |
| Sources/DesktopManager/WindowQueryOptions.cs | Introduces options object for richer window enumeration filtering. |
| Sources/DesktopManager/WindowProcessInfo.cs | Adds DTO for window-associated process metadata. |
| Sources/DesktopManager/WindowManager.cs | Extends window enumeration, adds child enumeration, enriches WindowInfo population. |
| Sources/DesktopManager/WindowManager.Visibility.cs | Centralizes window validation in ShowWindow. |
| Sources/DesktopManager/WindowManager.Style.cs | Centralizes window validation in style getters/setters. |
| Sources/DesktopManager/WindowManager.Process.cs | Adds window process utilities (path/elevation/WOW64/thread/owner info). |
| Sources/DesktopManager/WindowManager.Layout.cs | Adds overloads for input methods using options. |
| Sources/DesktopManager/WindowManager.Actions.cs | Adds bulk minimize/restore and on-screen enforcement helpers. |
| Sources/DesktopManager/WindowKeepAlive.cs | Adds argument validation for handle-based keep-alive. |
| Sources/DesktopManager/WindowInputService.cs | Adds retryable/preserved clipboard + activation/focus restore input flows. |
| Sources/DesktopManager/WindowInputOptions.cs | Adds configurable options for window input behaviors. |
| Sources/DesktopManager/WindowInfo.cs | Adds ownership/parent/cloaking/topmost/z-order/thread metadata + defaults. |
| Sources/DesktopManager/WindowControlService.cs | Refactors click to use client coords + message-based click fallback. |
| Sources/DesktopManager/WindowControlInfo.cs | Adds non-null defaults for control metadata. |
| Sources/DesktopManager/ScreenshotService.cs | Tightens nullability for monitor capture filters. |
| Sources/DesktopManager/Monitors.cs | Adds nullability tweaks + new all-users wallpaper API forwarding. |
| Sources/DesktopManager/MonitorWatcher.cs | Improves nullability + safer device enumeration and lParam guarding. |
| Sources/DesktopManager/MonitorService.Wallpaper.cs | Adds wallpaper caching + internalized wallpaper-set flows and validation. |
| Sources/DesktopManager/MonitorService.Theme.cs | Fixes registry key nullability handling. |
| Sources/DesktopManager/MonitorService.LogonWallpaper.cs | Adds validation and net8 WinRT fast path + safer reflection fallback. |
| Sources/DesktopManager/MonitorService.FileUtilities.cs | Preserves stream position when writing temp files. |
| Sources/DesktopManager/MonitorService.Display.cs | Adds validation, improves error messages, safer COM object handling. |
| Sources/DesktopManager/MonitorService.Core.cs | Defers Enable call; adds one-time enable helper; warms wallpaper cache. |
| Sources/DesktopManager/MonitorService.AllUsersWallpaper.cs | Adds elevated “set wallpaper for all users” registry/hive management. |
| Sources/DesktopManager/MonitorNativeMethods.Window.cs | Adds parent/owner APIs, cloaking detection, client rect, mouse msg constants. |
| Sources/DesktopManager/MonitorNativeMethods.Taskbar.cs | Fixes nullability for FindWindow signature. |
| Sources/DesktopManager/MonitorNativeMethods.Process.cs | Adds process/token P/Invokes for path/elevation querying. |
| Sources/DesktopManager/MonitorNativeMethods.Display.cs | Fixes nullability for display enumeration P/Invokes. |
| Sources/DesktopManager/Monitor.cs | Adds non-null defaults for monitor metadata. |
| Sources/DesktopManager/IDesktopManager.cs | Updates COM interface: nullable monitorId + Enable(bool) signature. |
| Sources/DesktopManager/DesktopManager.csproj | Moves net8 to Windows TFM; enables nullable; platform attribute wiring. |
| Sources/DesktopManager/ClipboardHelper.cs | Adds clipboard open retries + TryGetText support. |
| Sources/DesktopManager/AssemblyInfo.Platform.cs | Adds assembly-level SupportedOSPlatform("windows"). |
| Sources/DesktopManager.Tests/WindowVisibilityTests.cs | Makes test deterministic using Notepad + opt-in interactive guard. |
| Sources/DesktopManager.Tests/WindowTransparencyTests.cs | Uses Notepad fixture + interactive guard + safer cleanup. |
| Sources/DesktopManager.Tests/WindowTopMostActivationTests.cs | Uses Notepad fixture + interactive guard; reduces focus disruption. |
| Sources/DesktopManager.Tests/WindowTextFallbackTests.cs | Hardens deployment directory resolution for helper binary. |
| Sources/DesktopManager.Tests/WindowStyleModificationTests.cs | Uses Notepad fixture + interactive guard + cleanup. |
| Sources/DesktopManager.Tests/WindowStateHelpersTests.cs | Adds tests for bulk minimize/restore + ensure-on-screen. |
| Sources/DesktopManager.Tests/WindowProcessInfoTests.cs | Adds tests for new process info APIs. |
| Sources/DesktopManager.Tests/WindowPositionTests.cs | Makes position tests deterministic and opt-in interactive. |
| Sources/DesktopManager.Tests/WindowManagerWildcardTests.cs | Simplifies wildcard test setup and adds reflection null checks. |
| Sources/DesktopManager.Tests/WindowManagerFilterTests.cs | Adds tests for new WindowQueryOptions filters and z-order bounds. |
| Sources/DesktopManager.Tests/WindowLayoutTests.cs | Refactors layout tests to use filtered layout + Notepad fixture. |
| Sources/DesktopManager.Tests/WindowControlCheckTests.cs | Adds interactive gating for UI control tests. |
| Sources/DesktopManager.Tests/WindowChildEnumerationTests.cs | Adds test coverage for child window enumeration. |
| Sources/DesktopManager.Tests/WindowActivationPositioningTests.cs | Uses Notepad fixture + interactive gating for resizing tests. |
| Sources/DesktopManager.Tests/TestHelper.cs | Adds opt-in env var gating, Notepad launch helper, process tracking/cleanup. |
| Sources/DesktopManager.Tests/TestAssemblyHooks.cs | Adds assembly-level Notepad cleanup hooks. |
| Sources/DesktopManager.Tests/ScreenshotServiceTests.cs | Adds interactive gating for screenshot tests. |
| Sources/DesktopManager.Tests/RegistryDisposalTests.cs | Updates mocks for new Enable(bool) signature. |
| Sources/DesktopManager.Tests/MonitorWatcherTests.cs | Adjusts platform attributes and nullability. |
| Sources/DesktopManager.Tests/MonitorWatcherStateFailureTests.cs | Adjusts platform attributes and nullability. |
| Sources/DesktopManager.Tests/MonitorWatcherPowerTests.cs | Adjusts platform attributes and nullability. |
| Sources/DesktopManager.Tests/MonitorWatcherDeviceChangeTests.cs | Adjusts platform attributes and nullability. |
| Sources/DesktopManager.Tests/MonitorServiceTests.cs | Updates expectations for deferred Enable call + stream null validation. |
| Sources/DesktopManager.Tests/MonitorServiceInitializationTests.cs | Updates mocks and test expectations for deferred Enable. |
| Sources/DesktopManager.Tests/MonitorServiceCleanupTests.cs | Updates mocks for new Enable(bool) signature. |
| Sources/DesktopManager.Tests/MonitorResolutionOrientationTests.cs | Adds destructive-test gating for display-changing tests. |
| Sources/DesktopManager.Tests/MonitorFallbackTests.cs | Updates mocks and adds destructive gating where desktop changes occur. |
| Sources/DesktopManager.Tests/MonitorEnumerationTests.cs | Makes monitor enumeration tests environment-tolerant. |
| Sources/DesktopManager.Tests/MonitorBrightnessTests.cs | Adds destructive-test gating for brightness changes. |
| Sources/DesktopManager.Tests/LogonWallpaperTests.cs | Adds platform attribute + destructive gating. |
| Sources/DesktopManager.Tests/HotkeyServiceTests.cs | Adjusts platform attributes for newer TFMs. |
| Sources/DesktopManager.Tests/FakeDesktopManager.cs | Updates fake for new Enable(bool) signature. |
| Sources/DesktopManager.Tests/DesktopManager.Tests.csproj | Updates net8 target to Windows TFM; removes redundant platform attribute polyfill include. |
| Sources/DesktopManager.Tests/ComInitializationTests.cs | Adds platform annotation for logon wallpaper COM tests. |
| Sources/DesktopManager.Tests/BackgroundColorTests.cs | Adds destructive-test gating for background color changes. |
| Sources/DesktopManager.Tests/AssemblyInfo.Platform.cs | Adds conditional assembly-level platform annotation. |
| Sources/DesktopManager.PowerShell/DesktopManager.PowerShell.csproj | Updates net8 target to Windows TFM. |
| Sources/DesktopManager.PowerShell/CmdletSetLogonWallpaper.cs | Adds OS version platform annotation. |
| Sources/DesktopManager.PowerShell/CmdletSetDesktopWindowText.cs | Adds input reliability options (retries, focus/clipboard preservation, SendInput toggle). |
| Sources/DesktopManager.PowerShell/CmdletSetDesktopWallpaper.cs | Adds AllUsers registry/hive wallpaper application support. |
| Sources/DesktopManager.PowerShell/CmdletGetLogonWallpaper.cs | Adds OS version platform annotation. |
| Sources/DesktopManager.PowerShell/CmdletGetDesktopWindowProcessInfo.cs | Adds cmdlet to expose new window process info API. |
| Sources/DesktopManager.PowerShell/CmdletGetDesktopWindow.cs | Extends window filtering parameters backed by WindowQueryOptions. |
| Sources/DesktopManager.PowerShell/AssemblyInfo.Platform.cs | Adds conditional assembly-level platform annotation. |
| Sources/DesktopManager.Example/DesktopManager.Example.csproj | Updates net8 target to Windows TFM. |
| Sources/DesktopManager.Example/AssemblyInfo.Platform.cs | Adds assembly-level platform annotation. |
| README.MD | Documents opt-in env vars for UI/destructive tests. |
| Directory.Build.props | Enables TreatWarningsAsErrors globally. |
| DesktopManager.psm1 | Makes development binary loading opt-in via env var; adjusts assembly folder enumeration. |
| DesktopManager.Tests.ps1 | Excludes interactive Pester tests by default unless enabled via env var. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+140
to
+144
| } | ||
|
|
||
| var windowInfo = new WindowInfo { | ||
| Title = title, | ||
| Handle = handle, | ||
| ProcessId = windowProcessId, | ||
| IsVisible = MonitorNativeMethods.IsWindowVisible(handle) | ||
| }; | ||
|
|
||
| // Get window position and state | ||
| RECT rect = new RECT(); | ||
| if (MonitorNativeMethods.GetWindowRect(handle, out rect)) { | ||
| windowInfo.Left = rect.Left; | ||
| windowInfo.Top = rect.Top; | ||
| windowInfo.Right = rect.Right; | ||
| windowInfo.Bottom = rect.Bottom; | ||
|
|
||
| // Get window state using the IntPtr wrapper to work on x86 and x64 | ||
| IntPtr stylePtr = MonitorNativeMethods.GetWindowLongPtr(handle, MonitorNativeMethods.GWL_STYLE); | ||
| long style = stylePtr.ToInt64(); | ||
| if ((style & MonitorNativeMethods.WS_MINIMIZE) != 0) { | ||
| windowInfo.State = WindowState.Minimize; | ||
| } else if ((style & MonitorNativeMethods.WS_MAXIMIZE) != 0) { | ||
| windowInfo.State = WindowState.Maximize; | ||
| } else { | ||
| windowInfo.State = WindowState.Normal; | ||
| } | ||
|
|
||
| // Find which monitor this window is primarily on | ||
| var monitors = _monitors.GetMonitors(); | ||
| foreach (var monitor in monitors) { | ||
| var monitorRect = monitor.GetMonitorBounds(); | ||
| // Check if window center point is within monitor bounds | ||
| int windowCenterX = (rect.Left + rect.Right) / 2; | ||
| int windowCenterY = (rect.Top + rect.Bottom) / 2; | ||
|
|
||
| if (windowCenterX >= monitorRect.Left && windowCenterX < monitorRect.Right && | ||
| windowCenterY >= monitorRect.Top && windowCenterY < monitorRect.Bottom) { | ||
| windowInfo.MonitorIndex = monitor.Index; | ||
| windowInfo.MonitorDeviceId = monitor.DeviceId; | ||
| windowInfo.MonitorDeviceName = monitor.DeviceName; | ||
| windowInfo.IsOnPrimaryMonitor = monitor.IsPrimary; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| if (options.ProcessId > 0 && windowProcessId != options.ProcessId) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
WindowQueryOptions.ProcessId is an int, but windowProcessId is a uint, and the comparison windowProcessId != options.ProcessId will not compile (operator '!=' cannot be applied to 'uint' and 'int'). Consider aligning the types (e.g., make WindowQueryOptions.ProcessId a uint/uint? to match WindowInfo.ProcessId, or cast safely with range checks) and update the comparisons accordingly.
| @@ -0,0 +1,46 @@ | |||
| using System.Management.Automation; | |||
There was a problem hiding this comment.
This new cmdlet references WindowInfo, WindowManager, and WindowProcessInfo but does not import the DesktopManager namespace (unlike other cmdlets in this project). As written, the file will not compile because these types will be unresolved. Add using DesktopManager; (or fully qualify the types).
Suggested change
| using System.Management.Automation; | |
| using System.Management.Automation; | |
| using DesktopManager; |
| @@ -0,0 +1,5 @@ | |||
| <Project> | |||
| <PropertyGroup> | |||
There was a problem hiding this comment.
Setting TreatWarningsAsErrors=true repo-wide can break the build if there are any existing warnings. For example, Sources/DesktopManager/WindowManager.cs currently has an unused using System.IO;, which would raise CS8019 and become an error. Either clean up the current warnings before enabling this globally, or scope it to specific projects/configurations or suppress known/acceptable warnings (e.g., NoWarn for CS8019) so CI/build remains green.
Suggested change
| <PropertyGroup> | |
| <PropertyGroup Condition="'$(Configuration)' == 'Release'"> |
| $Development = $DevelopmentEnv.ToString().ToLowerInvariant() -in @('1', 'true', 'yes', 'on') | ||
| } | ||
| $DevelopmentPath = "$PSScriptRoot\Sources\DesktopManager.PowerShell\bin\Debug" | ||
| $DevelopmentFolderCore = "net8.0" |
There was a problem hiding this comment.
$DevelopmentFolderCore is still set to net8.0, but the projects were updated to target net8.0-windows10.0.19041.0, so development mode will likely fail to resolve binaries. Consider updating this value (or discovering the net8 output folder dynamically) to match the new TFM output path.
Suggested change
| $DevelopmentFolderCore = "net8.0" | |
| $DevelopmentFolderCore = "net8.0" | |
| $net8Folders = Get-ChildItem -Path $DevelopmentPath -Directory -Filter 'net8.*' -ErrorAction SilentlyContinue | Select-Object -First 1 | |
| if ($net8Folders) { | |
| $DevelopmentFolderCore = $net8Folders.Name | |
| } else { | |
| # Fallback to the known TFM used by the updated projects | |
| $DevelopmentFolderCore = "net8.0-windows10.0.19041.0" | |
| } |
Comment on lines
+409
to
+416
| foreach (var monitor in monitors) { | ||
| var bounds = monitor.GetMonitorBounds(); | ||
| if (rect.Right <= bounds.Left || rect.Left >= bounds.Right || | ||
| rect.Bottom <= bounds.Top || rect.Top >= bounds.Bottom) { | ||
| continue; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| } | ||
|
|
||
| [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] | ||
| private static extern int RegLoadKey(UIntPtr hKey, string lpSubKey, string lpFile); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
Comment on lines
+359
to
+364
| if (_wallpaperCache.TryGetValue(monitorId, out var entry)) { | ||
| if (allowStale || DateTimeOffset.UtcNow - entry.Updated <= WallpaperCacheTtl) { | ||
| wallpaperPath = entry.Path; | ||
| return !string.IsNullOrWhiteSpace(wallpaperPath); | ||
| } | ||
| } |
There was a problem hiding this comment.
These 'if' statements can be combined.
| // Skip title matching if title is empty and we're doing process-based search | ||
| if (!string.IsNullOrEmpty(title) || options.ProcessId <= 0) { | ||
| bool titleMatches = options.TitleRegex != null ? | ||
| (string.IsNullOrEmpty(title) ? false : options.TitleRegex.IsMatch(title)) : |
There was a problem hiding this comment.
The expression 'A ? false : B' can be simplified to '!A && B'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.