Skip to content

Conversation

@benhillis
Copy link
Member

@benhillis benhillis commented Dec 16, 2025

This pull request resolves an issue where the buttons on notifications were not working properly. This regressed with the introduction of the command line parsing helper class because this is difficult to test programmatically. There are three changes:

  1. Pass arguments to wslhost.exe correctly (fix double -- character insertion)
  2. Escape C:\Program Files\WSL\wslhost.exe path to resolve issue with space in the path name.
  3. Fix issue where the COM embedding variable was incorrectly changed from -Embedding to --Embedding
  4. NFC: Push logic to read disabled notification registry key into the TelemetryWorker method.

Resolves #13908

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in notification button functionality that occurred after introducing a command line parsing helper class. The core issue involved incorrect argument formatting when spawning wslhost.exe from notification actions.

Key Changes:

  • Fixed argument passing to wslhost.exe by removing double -- prefix and properly escaping the executable path
  • Corrected COM embedding flag from --Embedding to -Embedding to match COM conventions
  • Refactored telemetry worker to check notification registry settings lazily rather than at initialization

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/windows/wslhost/main.cpp Added proper path quoting when constructing command line for notification activation
src/windows/inc/wslhost.h Corrected COM embedding option from --Embedding to -Embedding
src/windows/common/notifications.cpp Removed -- prefix from notification action arguments to prevent double-dash issue
src/windows/service/exe/LxssUserSession.h Removed drvFsNotifications parameter from TelemetryWorker signature
src/windows/service/exe/LxssUserSession.cpp Moved registry check for DrvFs notifications into TelemetryWorker loop

@benhillis benhillis merged commit e24df7e into master Dec 16, 2025
6 checks passed
benhillis added a commit that referenced this pull request Dec 20, 2025
* cleanup: minor tweaks to path translation failed messages (#13892)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Localization change from build: 135812430 (#13905)

* Localization change from build: 135812430

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* remove unused string

---------

Co-authored-by: WSL localization <noreply@microsoft.com>
Co-authored-by: Ben Hillis <benhillis@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Resolve issue with config file writing sections outside of their expected header. (#13898)

* Resolve issue with config file writing sections outside of their expected header.

* add more writewslconfig variations

* formatting

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Resolve issue with buttons on notifications not working correctly (#13921)

* Resolve issue with buttons on notifications not working correctly

* pr feedback

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Fix minor typo in UserConfig.cmake.sample (.exe instead of .dll) (#13926)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* test: extend networking test coverage (#13914)

* test: extend coverage of virtioproxy networking mode

* test: add dns test variations to all networking classes

* remove bridged dns variations

* pr feedback

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* cleanup: remove duplicate AppId registration for wsldevicehost.dll (#13928)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Remove the explicit desktop field when creating a Windows process (#13930)

* [GH 13837] Remove trailing slash from $XDG_RUNTIME_DIR (#13929)

* [GH 13837] Remove trailing slash from $XDG_RUNTIME_DIR

* pr feedback

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* test: add more path translation variations (#13927)

* test: add more path translation variations

* Update test/windows/SimpleTests.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* virtiofs: enable additional test cases and skip one racey test (#13933)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* cleanup: add scsi disks during VM creation (#13939)

* cleanup: add scsi disks during VM creation

* pr feedback

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* cleanup: hcs logging improvements (#13942)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* cleanup: switch drvfs.cpp to use common MountUtil helper (#13890)

* cleanup: switch drvfs.cpp to use common MountUtil helper

* adjust mount.drvfs error logging

* adjust mount.drvfs error logging

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Blue <OneBlue@users.noreply.github.com>
Co-authored-by: WSL localization <noreply@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

I have hit don't show again button over a dozen times and it keeps reminding me. It's clearly broken.

3 participants