[codex] promote DesktopManager automation core and thin wrappers#164
Conversation
![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: 6945df6bd5
ℹ️ 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".
| if (uint.TryParse(trimmed, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out uint hexValue)) { | ||
| return hexValue; |
There was a problem hiding this comment.
ParseRequiredColor currently tries hexadecimal parsing for every input, so plain decimal values like "255" are interpreted as hex (0x255 => 597) instead of decimal. This breaks the documented decimal|0xRRGGBB|#RRGGBB behavior for set-background-color/set_desktop_background_color and yields incorrect colors whenever users pass decimal strings without a prefix.
Useful? React with 👍 / 👎.
| @@ -107,7 +523,7 @@ public bool ActiveWindowMatches(WindowQueryOptions options) { | |||
| } | |||
|
|
|||
| WindowInfo window = ResolveWindows(options, all: false)[0]; | |||
There was a problem hiding this comment.
GetFocusedControlObservation is nullable by contract, but this direct indexing path throws when no window currently matches the selector. In practice, wait workflows that poll for focused controls will fail immediately on the first miss instead of waiting for a matching window/control to appear.
Useful? React with 👍 / 👎.
| DesktopTextObservationOptions settings = observationOptions ?? new DesktopTextObservationOptions(); | ||
| ValidateTextObservationOptions(settings); | ||
|
|
||
| WindowInfo window = ResolveSingleWindow(options); |
There was a problem hiding this comment.
ObserveWindowText advertises a nullable result, but this call path throws if no window is currently present. That causes wait_for_observed_text polling to terminate early with an exception whenever the target window appears later, instead of continuing until timeout or success.
Useful? React with 👍 / 👎.
| "stop_window_keep_alive" => ReadBool(arguments, "allSessions") | ||
| ? DesktopOperations.StopAllWindowKeepAlive() | ||
| : DesktopOperations.StopWindowKeepAlive(ReadWindowCriteria(arguments, true)), |
There was a problem hiding this comment.
When allSessions is true, MCP unconditionally calls StopAllWindowKeepAlive() and ignores any provided selectors. If a caller sends both (for example via generic argument builders), it can unintentionally stop keep-alive sessions for unrelated apps; CLI already guards against this combination, so MCP should apply the same validation.
Useful? React with 👍 / 👎.
1 participant
What changed
DesktopAutomationServiceas the canonical C# automation coreWhy
DesktopManager is increasingly being used as the foundation for MCP, plugins, PowerShell, and EasyControlX. This change moves the real desktop automation behavior into the C# core so the wrapper layers stay thin and consistent.
Impact
WindowManagerorMonitorsValidation
dotnet test Sources/DesktopManager.Tests/DesktopManager.Tests.csproj -c Debug -f net8.0-windows --filter "DesktopAutomationCoreTests|PowerShellCoreCmdletSurfaceTests|PowerShellControlMutationTests"dotnet test Sources/DesktopManager.Tests/DesktopManager.Tests.csproj -c Debug -f net8.0-windows