Add Hide Empty Optical Drives mod

[Solomag]

Adds a Windhawk mod that hides empty CD/DVD/BD drives from This PC while keeping the drive fully available to Windows.

The drive automatically reappears when media is inserted and is hidden again when the media is removed.

Tested on Windows 10 22H2 (19045).

Changelog

If this pull request updates an existing mod, describe the changes below:

Mod authorship

If this pull request introduces a new mod, please complete the section below.

This mod was created by:

• The submitter, without AI assistance
• The submitter, with AI assistance
• Claude
• ChatGPT
• Gemini
• Another AI (please specify):
• Other (please specify):

Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.

[windhawk-reviewer]

Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review.

To get started, comment /ai-review. Once you're happy with the result, comment /ready-for-reviewer to hand it over to a human reviewer.

See the pull request review process for the full details.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Nice idea, and the mod is careful about not touching the device or the drive letter. The main problems are around the shell integration approach and the unload path — several of them can crash or hang Explorer.

1. Explorer window subclasses are never removed → crash after unload. TrySubclassExplorerWindow subclasses every CabinetWClass window, but Wh_ModUninit only unhooks the WinEvent hook and stops the thread — the subclasses stay installed. Once the mod image is unmapped, the next message delivered to a still-open Explorer window jumps into freed memory. Track the subclassed windows and remove them on unload, e.g. as in explorer-navigation-pane-tweaks.wh.cpp#L1038-L1049 (note the copy-out-of-the-lock pattern there — RemoveWindowSubclassFromAnyThread is a cross-thread SendMessage, so it must not be called while holding a lock the subclass proc also takes).

The same applies to the early return FALSE when CreateThread fails: Wh_ModUninit is not called after Wh_ModInit returns FALSE, so the subclasses installed by EnumWindows and the SetWinEventHook registration leak with no cleanup at all. Clean up explicitly before returning FALSE.

2. SetWindowSubclass is used across threads. Every Explorer browser window (CabinetWClass) runs on its own thread, while TrySubclassExplorerWindow is called from Wh_ModInit's thread (via EnumWindows) and from the WinEvent callback. Raw SetWindowSubclass can't subclass a window across threads, so in practice the removal-interception feature silently does nothing for most windows. Use WindhawkUtils::SetWindowSubclassFromAnyThread / RemoveWindowSubclassFromAnyThread (Development tips). Note the callback signature differs — WH_SUBCLASSPROC is (HWND, UINT, WPARAM, LPARAM, DWORD_PTR dwRefData), with no uIdSubclass parameter.

3. The Explorer subclass swallows WM_DEVICECHANGE.

if (GetManagedOpticalLetterFromVolume(lParam, &letter)) {
    ...
    return TRUE;   // never reaches DefSubclassProc
}

Returning without calling DefSubclassProc breaks the subclass chain: Explorer's own DBT_DEVICEREMOVECOMPLETE handling never runs (e.g. views browsing the ejected disc aren't navigated away), and any other mod subclassing the same window stops receiving the message too. Do the mod's work and then fall through to DefSubclassProc.

4. SetWinEventHook is installed on a thread with no message loop. WINEVENT_OUTOFCONTEXT events are dispatched from the message loop of the thread that called SetWinEventHook. When the mod is enabled mid-session, Wh_ModInit runs on the Windhawk engine thread, which doesn't pump messages for the mod — so WindowCreateWinEventProc never fires and newly opened Explorer windows are never subclassed. UnhookWinEvent in Wh_ModUninit likewise runs on an arbitrary thread rather than the installing one. Install and unhook it on the notification thread, which already has a message loop; see taskbar-auto-hide-when-maximized.wh.cpp#L1076 and its teardown at #L1614-L1618.

5. The mod hands the shell a COM object whose vtable lives in the mod image. FilteredEnumIDList is returned from the EnumObjects hook and its lifetime is entirely up to the shell. If Explorer still holds a reference when the mod is disabled/updated, the next Next/Release call dereferences a vtable in an unmapped image → crash. There is no way for the mod to wait for those references to drop, so this can't be fixed by adding cleanup.

The much simpler fix is to not create an enumerator at all and hook the shell's own "should this item be shown" callback instead. CDrivesViewCallback::ShouldShow in shell32.dll is exactly the This PC filter point — return S_FALSE to hide an item. classic-this-pc-sort-order.wh.cpp#L141-L160 does this to hide Control Panel from This PC, with the symbol hook at #L263-L272. That also removes several other liabilities in one go: the hardcoded vtable[4] index, the CoInitializeEx + SHGetDesktopFolder + BindToObject sequence in Wh_ModInit (which runs before the process has started executing when the mod is loaded at Explorer startup — doing shell COM work at that point is risky, and if it fails the mod refuses to load), and the per-enumeration QueryInterface/GetCurFolder probe in IsThisPcFolder.

6. The fake SHCNE_DRIVEREMOVED / SHCNE_MEDIAINSERTED broadcasts are system-wide. SHChangeNotify reaches every shell-notification listener in every process, so NotifyMediaRemoved tells the whole system that drive X: was removed when it wasn't. Other applications may drop the drive from their views, and nothing ever re-adds it — the mod never sends SHCNE_DRIVEADDED, not even on unload. The mod's stated scope is "only the This PC view in Explorer", so the notification should be limited to what RefreshThisPc() already does (SHCNE_UPDATEDIR on the This PC PIDL); please drop the SHCNE_DRIVEREMOVED/SHCNE_MEDIAINSERTED broadcasts.

7. SHCNF_FLUSH from inside a window procedure. NotifyMediaRemoved uses SHCNF_FLUSH, which blocks until all shell listeners have processed the notification — and it's called from ExplorerWindowSubclassProc while handling WM_DEVICECHANGE, i.e. on the Explorer UI thread inside a system broadcast. That's a good way to stall or hang the window. Use SHCNF_FLUSHNOWAIT (as the other call sites already do).

8. The unload path can leave the notification thread running.

WaitForSingleObject(g_notificationThread, 3000);
CloseHandle(g_notificationThread);

If the wait times out, Wh_ModUninit returns with the thread still executing code in the mod image, which Windhawk is about to FreeLibrary → crash. Wait INFINITE. There's also a lost-wakeup window: if Wh_ModUninit runs before the thread has created its window, g_notificationWindow is still nullptr and PostThreadMessageW(WM_QUIT) fails when the thread hasn't created a message queue yet — the thread then blocks in GetMessageW forever. Either signal readiness with an event before waiting, or retry the post:

while (!PostThreadMessageW(g_notificationThreadId, WM_QUIT, 0, 0) &&
       WaitForSingleObject(g_notificationThread, 50) == WAIT_TIMEOUT) {
}
WaitForSingleObject(g_notificationThread, INFINITE);

9. Blocking GetVolumeInformationW inside the enumerator. IsEmptyOpticalDrive probes the drive synchronously on every single enumeration of This PC, with no caching. On a disc that's spinning up, or a scratched/marginal disc, this call can block for seconds on whatever shell thread is enumerating, freezing the view. Since the mod already tracks media state via WM_DEVICECHANGE, cache the per-drive state and have the filter read the cached value instead of probing. Also worth wrapping the probe in SetThreadErrorMode(SEM_FAILCRITICALERRORS, ...) so a drive in a bad state can't raise the "There is no disk in the drive" system dialog.

10. DBT_DEVNODES_CHANGED triggers a probe + notification storm. That message is broadcast for any device-node change in the system (USB plug, Bluetooth, audio device, …). Each time, RecheckManagedOpticalDrives blocking-probes every optical drive and then unconditionally calls NotifyMediaInserted + RefreshThisPc for every drive that has media — even when nothing changed. Track the last known state per drive and only notify on an actual transition.

11. Shared state is raced between threads. g_lastArrivalTick / g_lastRemovalTick are read and written by IsDuplicateEvent from both the notification thread (HandleMediaArrival) and Explorer window threads (ExplorerWindowSubclassProc), with no synchronization. g_decisionLogState is likewise touched from arbitrary shell enumeration threads. Guard them with a mutex or make the entries atomic.

12. Effects don't disappear immediately on unload. Wh_ModUninit frees g_thisPcPidl without refreshing, so hidden drives stay hidden in open This PC views until the user refreshes manually. Call RefreshThisPc() before freeing the PIDL, and also at the end of Wh_ModSettingsChanged so a changed driveLetters value takes effect right away.

13. README has no screenshot. The mod has a visible effect, so a before/after screenshot of This PC would help a lot. Only i.imgur.com and raw.githubusercontent.com are allowed image hosts.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• Drop the verboseLogging setting. Windhawk already has a per-mod logging toggle, so a custom verbosity switch is redundant — just call Wh_Log unconditionally. That also lets you delete the whole DecisionLogState dedupe array, which exists only to avoid repeating log lines.
• Use WindhawkUtils::StringSetting instead of Wh_GetStringSetting + manual Wh_FreeStringSetting in LoadSettings. Also, Wh_GetStringSetting never returns NULL (it returns L"" when unset), so the if (letters) check is dead code.
• Use WindhawkUtils::SetFunctionHook rather than raw Wh_SetFunctionHook with void* casts — moot if you move to the symbol hook suggested above.
• Wh_Log(L"Initializing Hide Empty Optical Drives v1.0.0") hardcodes the version, which will drift on the next bump. WH_MOD_VERSION is available as a wide-string literal, or just drop the version from the message.
• SHGetFolderLocation(CSIDL_DRIVES) is deprecated; SHGetKnownFolderIDList(FOLDERID_ComputerFolder, 0, nullptr, &pidl) is the modern equivalent.
• g_settings (including the driveLetters array) is rewritten in place by Wh_ModSettingsChanged while other threads read it. Only reachable on a settings change, so low impact, but a swap-under-lock or a double-buffer would make it clean.
• FilteredEnumIDList::Next turns a genuine failure HRESULT from the inner enumerator into S_FALSE, hiding real errors from the caller.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• The README scope claim is a bit narrower than the code. Hooking EnumObjects on the This PC folder filters every in-process consumer of that enumeration (navigation pane, breadcrumb dropdowns, …), not just the This PC folder view. The CDrivesViewCallback::ShouldShow approach suggested above matches the documented scope exactly. Either way, worth a README line that the mod only affects explorer.exe — file dialogs and third-party file managers in other processes still show the empty drive.
• Consider whether the notification machinery is needed at all. Explorer already refreshes This PC on media insert/remove — that's how the drive's icon and label update today. It's worth testing whether the hook alone gives the right result, in which case the notification thread, the window, the retry timer and the SHChangeNotify calls could all go away. A lot of the complexity here (the 250 ms duplicate-event window, the 20×500 ms retry ladder, the DBT_DEVNODES_CHANGED fallback) reads like defensive scaffolding rather than something the feature needs; since the PR mentions AI assistance, it's worth double-checking which parts are actually load-bearing.
• DBTF_MEDIA isn't checked. GetManagedOpticalLetterFromVolume accepts any DBT_DEVTYP_VOLUME event, so unplugging a USB optical drive is treated identically to ejecting a disc. ((DEV_BROADCAST_VOLUME*)lParam)->dbcv_flags & DBTF_MEDIA distinguishes the two.
• Only the first drive in dbcv_unitmask is handled. The loop returns on the first matching letter, so if two optical drives change in one notification the second is ignored. Returning a mask (or invoking the callback per bit) would cover it.
• The 250 ms duplicate window can swallow real events — a quick eject-then-reinsert within that window is dropped. Comparing against the last known state rather than the last timestamp would be more robust.
• Relation to the built-in option. Windows has a "Hide empty drives" option in Folder Options → View; a short README note on how this mod differs (presumably that the built-in option doesn't cover optical drives) would help users decide whether they need it.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Overall the design is sound — hooking CDrivesViewCallback::ShouldShow is the right injection point (same one classic-this-pc-sort-order.wh.cpp uses to hide the Control Panel item), the mod makes no persistent changes, @include is correctly narrowed to explorer.exe, and the notification thread is properly signalled, joined and closed in Wh_ModUninit. A few things to fix:

1. The initial scan and refresh run before the hook is installed, so enabling the mod with This PC open often does nothing.

Windhawk only installs the mod's hooks after Wh_ModInit returns. But NotificationThreadProc posts kMsgInitialScan as soon as the window exists (line 718), and the notification thread starts pumping immediately — so InitialScan → RefreshThisPc (line 379) fires while CDrivesViewCallback_ShouldShow_Hook is not yet active. The same applies to the kMsgRefreshThisPc posted at the end of Wh_ModInit (line 931): it's a post to another thread that is already running, so it can be processed before Wh_ModInit returns.

Result: when the mod is enabled while a This PC window is open (the normal case — that's how a user tries the mod out), the view re-enumerates unfiltered and the empty drive stays visible until the user presses F5 or media state changes.

Move the scan/refresh trigger to Wh_ModAfterInit, which is documented to run after the engine has finished setting up hooks:

void Wh_ModAfterInit() {
    HWND hwnd = g_notificationWindow.load(std::memory_order_acquire);
    if (hwnd) {
        PostMessageW(hwnd, kMsgInitialScan, 0, 0);
        PostMessageW(hwnd, kMsgRefreshThisPc, 0, 0);
    }
}

and drop the PostMessageW(hwnd, kMsgInitialScan, ...) from NotificationThreadProc and the kMsgRefreshThisPc post from Wh_ModInit.

2. Blocking device I/O runs on the notification window's message loop.

HandleVolumeDeviceChange calls ProbeOpticalMediaState (i.e. GetVolumeInformationW on the optical root) synchronously from the WM_DEVICECHANGE handler, and ProcessPendingInsertions does the same from WM_TIMER on that same thread.

DBT_DEVICEARRIVAL / DBT_DEVICEREMOVECOMPLETE for volumes are sent (not posted) as a broadcast to every top-level window — the DEV_BROADCAST_VOLUME* in lParam is only valid for the duration of the send. Your window is a top-level window, so whatever time you spend in that handler is time the system's device-notification broadcast is stalled for every window after yours. GetVolumeInformationW against optical media is real device I/O: spin-up, TOC read, a dirty/scratched or multi-session disc — it can take seconds. The mod's own 10-second retry loop exists precisely because the drive isn't ready right away. The WM_TIMER path has the same effect: while the loop is blocked in a probe, any incoming broadcast send to that window blocks too.

It also makes unload slow: Wh_ModUninit → StopNotificationThread waits INFINITE for the thread, so a disable/update/reload can hang for as long as a probe takes.

Keep the message-loop thread purely for dispatch and move the probing off it — e.g. a second worker thread that waits on an event with a timeout (replacing SetTimer), does the GetVolumeInformationW probes, and posts a result message back to the notification window. In the DBT_DEVICEARRIVAL branch, just record the pending letter and signal the worker (StartInsertionRetry) instead of calling ProbeOpticalMediaState inline.

3. Add a screenshot to the README.

The mod has a visible effect, and the README currently has no image. A before/after pair of This PC (drive visible with no disc → drive gone) makes the mod's purpose immediately clear in the Windhawk catalog. Images must be hosted on i.imgur.com or raw.githubusercontent.com.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• g_enabled is dead state. The only place that sets it to false is Wh_ModUninit, but Windhawk removes the mod's hooks before calling Wh_ModUninit — by then CDrivesViewCallback_ShouldShow_Hook can no longer fire. The atomic, its check at line 303 and the comment at line 971 can all go; the RefreshThisPc posted via kMsgShutdown is what actually restores the view. See the mod lifetime flow chart.

• GetModuleHandleW instead of LoadLibraryExW for shell32. shell32.dll is always loaded in explorer.exe, and HookDrivesViewShouldShow never calls FreeLibrary, so every mod load/reload leaks a reference on it. GetModuleHandleW(L"shell32.dll") avoids both. (The current call is at least safe — LOAD_LIBRARY_SEARCH_SYSTEM32 rules out DLL hijacking.)

• Short-circuit the hook when no optical drive is tracked. CDrivesViewCallback_ShouldShow_Hook calls GetDisplayNameOf + StrRetToBufW for every item in This PC on every enumeration, even on machines with no optical drive at all. A cheap early-out before the COM call:

  if (g_opticalMask.load(std::memory_order_acquire) == 0) {
      return hr;
  }

• Don't swallow WM_DEVICECHANGE subtypes you don't handle. The handler returns TRUE for every WM_DEVICECHANGE, including query events like DBT_DEVICEQUERYREMOVE. Only intercept DBT_DEVICEARRIVAL / DBT_DEVICEREMOVECOMPLETE and let everything else fall through to DefSubclassProc.

• Use a private message for the "quit without restoring" path too. StopNotificationThread(false) posts WM_CLOSE, and the subclass proc destroys the window on it. Your window is a top-level window in explorer.exe, so a stray broadcast WM_CLOSE (some "close all windows" utilities do this) would silently tear down the notification thread for the rest of the session, leaving the mod loaded but inert. You already have kMsgShutdown; a second WM_APP-based message (or a wParam flag on kMsgShutdown) avoids relying on WM_CLOSE.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Other places the drive appears. ShouldShow filters the defview enumeration of the Computer folder. Worth checking whether the empty drive is also hidden in the navigation pane tree, the address-bar dropdown, and the "This PC" node in file dialogs hosted by Explorer — if any of those still list it, mention the limitation in the README so users know what to expect.

• The initial probe wakes the drive. InitialScan runs GetVolumeInformationW on every optical drive at mod load and on every settings change, which spins up a physical drive (audible on most desktops). That's unavoidable for the first state determination, but it's worth being aware that toggling settings re-triggers it.

• COM initialization on the notification thread. The thread calls SHGetKnownFolderIDList without ever calling CoInitializeEx. It evidently works on your test machine, but if it ever returns a failure HRESULT, the thread bails out and Wh_ModInit returns FALSE — the mod silently does nothing. If you want to be defensive, either initialize an apartment on that thread or fall back to SHGetSpecialFolderLocation(nullptr, CSIDL_DRIVES, &pidl).

• Retry window length. 20 attempts × 500 ms = 10 s after DBT_DEVICEARRIVAL. Slow or older drives (and dual-layer / multi-session discs) can take longer than that to report a readable volume; after the window expires the state is latched to whatever the last probe said, and nothing re-probes until the next device event. Consider a longer window with a backoff (e.g. 500 ms → 2 s) rather than a fixed 10 s.

• Unreadable-but-present media. A blank CD-R/DVD-R makes GetVolumeInformationW fail with something other than ERROR_NOT_READY/ERROR_NO_MEDIA_IN_DRIVE, so it lands in MediaState::Unknown and the drive is shown. That's the correct fail-open behaviour for this mod, just confirming it's intentional — a user who inserts a blank disc to burn it will see the drive reappear.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

The Windhawk integration side is in good shape: threads are signalled and joined inside Wh_ModUninit, the window subclass is removed on WM_NCDESTROY, all globals are trivially destructible, the symbol array name matches the module it's resolved against, HookSymbols is called once, the @include is narrow and unloading restores the view. The main thing to fix is the media-presence detection itself.

1. A drive with a disc in it can end up hidden (blank / unmountable media).

ProbeOpticalMediaState decides "no media" purely from GetVolumeInformationW:

if (error == ERROR_NOT_READY || error == ERROR_NO_MEDIA_IN_DRIVE) {
    return MediaState::Empty;
}

GetVolumeInformationW answers "is there a mountable filesystem", not "is there a disc". A blank CD-R/DVD-R/BD-R waiting to be burned, a disc written with a filesystem Windows can't mount, and a disc that is still being recognized all commonly fail with ERROR_NOT_READY — the same error as an empty tray. ProcessRetryMask then commits that after the retry window expires:

if (++retryAttempts[index] >= kMaxRetryAttempts) {
    ...
    changed |= SetCachedMediaState(letter, state);   // state == Empty

so after ~10 s the drive is hidden with a disc physically inside it. That contradicts the README's "unknown/error state -> show (fail open)" contract, and it means a blank disc can't be found in This PC to burn to.

Use a media-presence check that doesn't depend on a filesystem, and keep GetVolumeInformationW only for what it's actually good at:

// Media presence, independent of whether a filesystem can be mounted.
// dwDesiredAccess == 0 -> no admin rights needed, and
// IOCTL_STORAGE_CHECK_VERIFY2 doesn't require read access either.
WCHAR devicePath[] = L"\\\\.\\X:";
devicePath[4] = letter;

HANDLE drive = CreateFileW(devicePath, 0,
                           FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
                           OPEN_EXISTING, 0, nullptr);
if (drive == INVALID_HANDLE_VALUE) {
    return MediaState::Unknown;  // fail open
}

DWORD returned = 0;
BOOL present = DeviceIoControl(drive, IOCTL_STORAGE_CHECK_VERIFY2, nullptr, 0,
                              nullptr, 0, &returned, nullptr);
DWORD error = present ? ERROR_SUCCESS : GetLastError();
CloseHandle(drive);

if (present) {
    return MediaState::Present;
}
if (error == ERROR_NOT_READY || error == ERROR_NO_MEDIA_IN_DRIVE) {
    return MediaState::Empty;
}
return MediaState::Unknown;  // fail open

(IOCTL_STORAGE_CHECK_VERIFY2 is in <winioctl.h>.) This also makes the retry loop cheaper — no volume mount is attempted on each attempt — and keeps the SetThreadErrorMode(SEM_FAILCRITICALERRORS, ...) guard useful for the same reason it is now.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• Formatting. The file is 1200 lines for what is roughly 400 lines of code, because almost every call is wrapped at ~40 columns with one argument per line (SetCachedMediaState(\n letter,\n MediaState::Unknown);). The repo ships a .clang-format (Chromium, 4-space indent, 80 columns) — running it would cut the file roughly in half and make it much easier to review. This reads like an AI formatting artifact rather than an intentional style.

• GetDriveLetterFromItem matches any parsing name that starts with X:, not just a drive root:

  if (candidate < L'A' || candidate > L'Z' || parsingName[1] != L':') {
      return false;
  }

  If the This PC view ever contains a non-root item that happens to live under an optical drive letter (a redirected known folder, a namespace extension item), it gets hidden along with the drive. One-line tightening:

  if (candidate < L'A' || candidate > L'Z' || parsingName[1] != L':' ||
      parsingName[2] != L'\\' || parsingName[3] != L'\0') {
      return false;
  }

• Invalid driveLetters input silently means "all drives". LoadManagedMask returns kAllDriveBits whenever the parsed mask is empty, so a typo like driveLetters: 1 manages every optical drive instead of none. A Wh_Log when the setting is non-empty but yielded no letters would make that debuggable. The $description is also a bit cryptic (for example G or DE) — the parser happily accepts D, E, so it's worth saying separators are allowed.

• The extra RequestThisPcRefresh() calls are redundant. Both Wh_ModAfterInit and Wh_ModSettingsChanged queue a scan and then immediately request a refresh, but the worker already calls RequestThisPcRefresh() whenever the scan actually changed something (if (refresh) RequestThisPcRefresh();). The immediate call fires a system-wide SHChangeNotify before any state is known, i.e. one guaranteed no-op This PC re-enumeration per Explorer start and per settings change.

• TranslateMessage in the notification loop does nothing useful — that window never sees keyboard input.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Scope of the hook. CDrivesViewCallback::ShouldShow filters the This PC view only. The navigation-pane tree, the address-bar dropdown and the Desktop's This PC expansion enumerate the Computer folder directly and don't go through the view callback, so the empty drive still shows up there. The README currently only mentions file dialogs and third-party managers — worth stating this limitation too, since it's the first thing users will notice.

• Worth investigating: reuse the shell's own media state instead of probing. CDrivesFolder already knows whether a drive has media (that's how it decides between "DVD RW Drive (D:)" and a volume label + capacity bar), and it keeps that cached. If that state is reachable from ShouldShow — e.g. IShellFolder2::GetDetailsEx with PKEY_Capacity on the item, or one of shell32's internal drive-info helpers found via the Windhawk Symbol Helper — the worker thread, the 500 ms × 20 retry state machine and the cache-staleness window all disappear, and only the WM_DEVICECHANGE-driven refresh would remain. Only worth it if the lookup is genuinely cheap/cached, since ShouldShow runs on the Explorer UI thread; the current split (probe on a worker, refresh from the message loop) is the safe design otherwise.

• Refresh cost. SHChangeNotify(SHCNE_UPDATEDIR, ...) on This PC is a machine-wide notification: every shell client on the system re-enumerates This PC on each media insert/eject, and open This PC windows lose selection/scroll position. The shell already refreshes on media change, so this is mostly duplicate work. There isn't a clean process-scoped alternative short of walking IShellWindows and calling IShellView::Refresh(), so this is an FYI rather than something to change.

• Transient visibility on eject. Explorer handles the media-removal notification on its own before the worker has updated the cache, so the drive briefly renders as an empty drive and then disappears on the mod's refresh. Cosmetic, but noticeable on a slow machine.

• Eject affordance. Once an empty drive is hidden there's no Explorer context menu to open the tray — users have to use the physical button. Probably worth a line in the README.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ai-review

[windhawk-reviewer]

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-09-06 21:15 UTC (in 22 hours) to get another one.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

The hook target and hooking style are right (GetModuleHandleW instead of a bare LoadLibrary, a single HookSymbols call, a stock STATIC window + subclass instead of a home-grown window class, both threads joined in Wh_ModUninit, no persistent system changes, English UI, demo GIF present). The issues below are all in the media-detection state machine.

1. A generic DBT_DEVNODES_CHANGED can permanently hide a drive that has a disc in it.

ProcessInitialScan (line 466) settles ProbeResult::NotReady as MediaState::Empty on the non-grace path, with the retry bits explicitly cleared just above it:

} else if (result == ProbeResult::Empty ||
           result == ProbeResult::NotReady) {
    changed |= SetCachedMediaState(letter, MediaState::Empty);
}

An optical drive that has spun down after idling — or that is busy — answers IOCTL_STORAGE_CHECK_VERIFY2 with ERROR_NOT_READY even though media is present. So: user inserts a disc → drive shows (Present) → drive spins down a few minutes later → user plugs in any USB/Bluetooth device → DBT_DEVNODES_CHANGED → one probe, no retry → NOT_READY → the drive is hidden despite containing a disc. Nothing recovers it: the only trigger for a re-probe is another device event, which again gets a single shot against a still-spun-down drive, and the drive can't spin up because nothing can reach it while it's hidden.

The rest of the file already treats NOT_READY as transient — that is exactly what the arrival/startup grace window exists for — so the two paths contradict each other. Only ERROR_NO_MEDIA_IN_DRIVE is a conclusive "empty" answer. Suggested fix: never let a single non-grace NotReady downgrade the cached state; schedule the bounded retry instead.

} else if (result == ProbeResult::Empty) {
    changed |= SetCachedMediaState(letter, MediaState::Empty);
} else if (result == ProbeResult::NotReady) {
    // Inconclusive - let the bounded grace window resolve it instead of
    // settling on Empty after a single probe.
    *retryMask |= bit;
    *graceRetryMask |= bit;
} else {
    changed |= SetCachedMediaState(letter, MediaState::Unknown);
}

2. DBT_DEVNODES_CHANGED shouldn't drive media probing at all.

NotificationWindowSubclassProc (line 902) calls QueueInitialScan() for every DBT_DEVNODES_CHANGED. That message is broadcast on essentially any device-node change in the system — USB plug/unplug, Bluetooth pairing, docking, audio device changes — and each one issues CreateFile + IOCTL_STORAGE_CHECK_VERIFY2 against every managed optical drive. That's a TEST UNIT READY to the physical drive, which on many drives means an audible spin-up/seek. The README's "there is no permanent polling" is true in the strict sense, but in practice this is device-change-triggered polling of the drive.

DBT_DEVNODES_CHANGED only needs to answer "did an optical drive appear or disappear" — GetLogicalDrives() + GetDriveTypeW() answer that with no device I/O. Media state is already covered by the DBT_DEVICEARRIVAL / DBT_DEVICEREMOVECOMPLETE volume broadcasts (which carry DBTF_MEDIA for disc insert/eject) plus the startup and resume scans. Splitting the two would fix this and item 1 at the same time, and would remove most of the retry machinery that item 3 is about.

3. The mod is roughly 1,300 lines for "hide a drive when it has no disc", and the complexity is where the bugs are.

The media state is spread across three parallel bitmasks (retryMask, graceRetryMask, arrivalRetryMask), a MediaState cache, an int retryAttempts[26], and four near-duplicate 26-letter loops (ProcessInitialScan / ProcessRemovalMask / ProcessArrivalMask / ProcessRetryMask) that each re-implement the same "clear the bits, probe, classify, maybe retry" sequence with subtly different rules. Items 1 and 2 are direct consequences of that: the same probe result means different things depending on which of the four functions observed it, and which combination of masks happens to be set.

This is worth restructuring before merge — it's the part a future maintainer (or you, in six months) has to reason about. A per-drive struct plus one update routine expresses the whole thing:

struct DriveState {
    MediaState media = MediaState::Unknown;
    bool isOptical = false;
    int retriesLeft = 0;   // 0 = settled
    bool grace = false;    // NOT_READY/Empty are transient while true
};
DriveState g_drives[26];   // worker-thread-owned; publish media via the atomics

enum class Trigger { Startup, Resume, SettingsChanged, VolumeArrival,
                     VolumeRemoval, Retry };
static bool UpdateDrive(WCHAR letter, Trigger trigger);

Only the MediaState (and the optical mask) has to be atomic, because only those are read from the Explorer UI thread in the ShouldShow hook; the retry bookkeeping never leaves the worker thread, so it doesn't need to be bit-packed or atomic at all.

4. A failed SHGetKnownFolderIDList silently disables the mod for the rest of the Explorer session.

NotificationThreadProc (line 967) resolves the This PC PIDL before creating the window, and bails out on failure:

PIDLIST_ABSOLUTE thisPcPidl = AcquireThisPcPidl();

if (!thisPcPidl) {
    return 1;
}

The thread then never creates the notification window, so no WM_DEVICECHANGE / WM_POWERBROADCAST ever arrives and no refresh is ever posted — the cache freezes at whatever the Wh_ModAfterInit scan produced, and the hook keeps applying it. The user sees a mod that "sometimes stops working" with only a log line to show for it.

This matters because Wh_ModInit runs before the target process starts executing, so this thread makes a shell call at the very beginning of explorer.exe's life (and on a thread with no COM apartment — worth confirming SHGetKnownFolderIDList is happy there). The window is the part that must exist; the PIDL isn't needed until the first refresh. Creating the window first and resolving the PIDL lazily in the kMsgRefreshThisPc handler (caching it once it succeeds, retrying on the next refresh if it doesn't) keeps the startup path non-blocking and removes the permanent-failure mode.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• The explicit std::memory_order_* arguments are noise, and in two places they don't give what the comments assume. RequestThisPcRefresh (line 273) and the g_notificationStopRequested / g_notificationWindow handshake in StopNotificationThread (line 1138) are both store-then-load-the-other-variable patterns. release/acquire do not order a store against a later load of a different atomic, so in principle both sides can miss each other (x86 buffers stores). Plain .load() / .store() (i.e. seq_cst, the default) is both simpler to read and actually gives the ordering the comments describe, and there's no measurable cost at these frequencies.
• AcquireThisPcPidl's CSIDL_DRIVES fallback (line 867) is effectively dead code. SHGetKnownFolderIDList(FOLDERID_ComputerFolder) doesn't fail for a well-known static folder, and SHGetSpecialFolderLocation is deprecated. Dropping it removes a branch that can never be tested.
• ProbeOpticalMediaState re-checks GetDriveTypeW (line 207) although all four callers already established the drive is DRIVE_CDROM immediately before calling it.
• arrivalRetryMask exists only to decide whether to reset retryAttempts on a repeated arrival (line 632). Folding it into the per-drive struct from item 3 would drop a whole 26-bit mask threaded through five function signatures.
• RequestThisPcRefresh has exactly one producer (the worker thread), so the "multiple state changes can be coalesced" exchange/re-store dance is heavier than the situation needs — a plain flag plus the post would do.
• The two "still stopping after N ms" warning loops (lines 1123-1129 and 1177-1183) are identical diagnostics duplicated in StopWorkerThread and StopNotificationThread; worth factoring or dropping.
• MediaState and ProbeResult overlap heavily — one enum with an explicit NotReady member would remove a translation step in every classify block.
• README/PR mismatch: the README's Compatibility section lists Windows 11 25H2, while the PR description says Windows 10 22H2 (19045) only. Worth aligning so users know what's actually been verified.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Fixed 500 ms × 20 retry cadence. During a grace window the drive is probed 20 times over 10 seconds. For a drive that is genuinely empty but reports NOT_READY (some drives do), that's 20 TEST UNIT READY commands to settle on the answer the first one already implied. A backoff (500 ms → 1 s → 2 s, same 10 s budget) would get the same responsiveness for a real insert with far fewer commands.
• Up to 10 s before an inserted disc makes the drive appear. That's the deliberate consequence of the grace window and it's the right trade-off, but it's worth stating in the README's "Detection behavior" list so users don't read a slow appearance as a bug.
• CDrivesViewCallback_ShouldShow_Hook calls GetDisplayNameOf + StrRetToBufW for every item in This PC whenever any optical drive is known (line 342). It's not a hot path, but if you want to trim it, the item's SFGAO_FILESYSTEM/SFGAO_STORAGE attributes are a cheaper pre-filter than a parsing-name round trip for folders, Control Panel items, etc.
• Loss of the context-menu Eject command while a drive is hidden is documented, which is good. If it turns out to bite people, a per-drive "always show" escape hatch in the settings would be a natural addition — no need to do it now.
• mods/classic-this-pc-sort-order.wh.cpp hooks the same CDrivesViewCallback::ShouldShow symbol (line 260) and its hook returns S_FALSE to hide, matching your semantics — useful as a cross-check that the contract is right. The two mods chain fine; no conflict.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Overall this is a well-structured mod: it targets only explorer.exe, keeps all state in atomics, never writes to the registry, joins both of its threads in Wh_ModUninit, avoids RegisterClass (so there's no window-class-lifecycle problem across reloads), and all globals are trivially destructible so process shutdown is safe. It also doesn't overlap any existing mod — classic-this-pc-sort-order hooks the same CDrivesViewCallback::ShouldShow but for a different purpose, and Windhawk chains those hooks fine. Two things are worth fixing:

1. A failed This PC PIDL lookup permanently disables all device handling for the session

NotificationThreadProc resolves the PIDL before creating the window, and gives up entirely if that fails:

PIDLIST_ABSOLUTE thisPcPidl = AcquireThisPcPidl();

if (!thisPcPidl) {
    return 1;
}

If this ever fails, there is no notification window, so the mod receives no WM_DEVICECHANGE and no WM_POWERBROADCAST for the rest of the process's life. The worker still runs the initial scan and the hook still hides drives — so the user ends up with a drive that is hidden and never reappears when a disc is inserted, with nothing but a log line to explain it.

This isn't hypothetical. Wh_ModInit runs on Explorer's main thread, before the process starts executing (Mod lifetime), so this thread starts extremely early in Explorer's startup, and it never calls CoInitializeEx — SHGetKnownFolderIDList goes through the COM-based Known Folder manager, so it can fail with CO_E_NOTINITIALIZED on an uninitialized thread. The SHGetSpecialFolderLocation fallback probably saves you today, but the mod shouldn't depend on that.

Suggested fix — create the window unconditionally and resolve the PIDL lazily on first use:

static DWORD WINAPI NotificationThreadProc(void*) {
    MSG msg = {};
    PeekMessageW(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE);

    HWND hwnd = CreateWindowExW(...);   // no PIDL dependency
    ...
}

// In the subclass proc:
case kMsgRefreshThisPc: {
    PIDLIST_ABSOLUTE pidl = g_thisPcPidl.load(std::memory_order_acquire);
    if (!pidl) {
        pidl = AcquireThisPcPidl();     // retried on every refresh
        if (pidl) {
            g_thisPcPidl.store(pidl, std::memory_order_release);
        }
    }
    NotifyThisPcUpdated(pidl);
    return 0;
}

This also removes the need to pass the PIDL through the subclass refData, which currently duplicates g_thisPcPidl. Additionally, consider CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED) / CoUninitialize around the message loop, since the thread hosts a window and calls shell APIs — see win7-classic-autoplay-restorer.wh.cpp#L6395 for the same pattern.

2. Every DBT_DEVNODES_CHANGED re-probes every managed optical drive

case WM_DEVICECHANGE:
    if (wParam == DBT_DEVNODES_CHANGED) {
        QueueInitialScan();
        break;
    }

DBT_DEVNODES_CHANGED is broadcast for any device-tree change — plugging in a USB stick, connecting a Bluetooth device, docking, a driver load/unload — and Windows typically sends several per hardware event. Each one runs ProcessInitialScan, which does CreateFileW(L"\\\\.\\X:") + IOCTL_STORAGE_CHECK_VERIFY2 on every managed optical drive. That's a TEST UNIT READY against the drive, which wakes/spins it up. The result is an audible optical drive spinning up every time an unrelated device is plugged in — the comment at line 456 shows you were already thinking about exactly this cost, but the one-probe-per-event limit doesn't avoid it.

Since a hot-plugged or removed external optical drive always changes the set of logical drives, you can gate the probes on that:

static DWORD g_lastLogicalDrives = 0;  // worker-thread-local state
...
DWORD logicalDrives = GetLogicalDrives();
bool driveSetChanged = (logicalDrives != g_lastLogicalDrives);
g_lastLogicalDrives = logicalDrives;

and in the !allowGrace path, skip ProbeOpticalMediaState (just refresh g_opticalMask via GetDriveTypeW) when neither the logical-drive set nor the optical mask changed. Media insertion/removal already arrives separately as DBT_DEVICEARRIVAL/DBT_DEVICEREMOVECOMPLETE with DBT_DEVTYP_VOLUME, so nothing is lost.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• IsWorkerStopRequested() issues a WaitForSingleObject syscall on every call, and it's called once per drive letter in each of the four scan loops (~26+ syscalls per pass). A companion std::atomic<bool> g_workerStop set alongside SetEvent(g_workerStopEvent) would make the polling checks free; keep the event only for the actual wait.

• RequestThisPcRefresh() is only ever called from the worker thread, so the exchange-based coalescing is doing less than the comment suggests — the only real interaction is with the one-shot drain in NotificationThreadProc. Worth simplifying the comment, or the code.

• GetDriveLetterFromItem runs GetDisplayNameOf(SHGDN_FORPARSING) on every visible This PC item (whenever any optical drive exists), including third-party namespace extensions and WPD/phone entries. SHGDN_FORPARSING | SHGDN_INFOLDER gives you the same X:\ for drives while asking the folder for a cheaper, non-fully-qualified name.

• SetThreadErrorMode(SEM_FAILCRITICALERRORS, nullptr) at the top of WorkerThreadProc is cheap insurance against a hard-error dialog ("There is no disk in the drive…") appearing on a background thread that Explorer's user can't see, which would also wedge the join in StopWorkerThread.

• @architecture x86-64 excludes 32-bit Windows 10, where explorer.exe is x86. If you want to cover it, old-this-pc-commands carries both symbol spellings for the same function — see old-this-pc-commands.wh.cpp#L264 — and you can then drop @architecture entirely.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Inconsistent handling of NotReady between the arrival path and the grace scan. ProcessArrivalMask deliberately keeps the previous cached state on NotReady to avoid premature hiding, but ProcessInitialScan's grace branch marks it Empty right away:

  } else if (result == ProbeResult::Empty ||
             result == ProbeResult::NotReady) {
      *retryMask |= bit;
      *graceRetryMask |= bit;
      retryAttempts[index] = 0;
      changed |= SetCachedMediaState(letter, MediaState::Empty);   // hides now
  }

  At startup that's invisible (no view is open yet), but the same path runs on PBT_APMRESUMEAUTOMATIC and on Wh_ModSettingsChanged. Resuming from sleep with a spun-down disc in the drive will hide the drive and then un-hide it a second or two later. Treating NotReady like the arrival path (keep the previous state, let the 10 s window settle it) would make the two consistent.

• Related: in ProcessRetryMask, a non-grace retry that gets ProbeResult::Unknown sets MediaState::Unknown (shows the drive) and a subsequent probe may set Empty (hides it again) — another possible blink. Holding the previous state for the duration of a retry series would avoid it.

• SHChangeNotify(SHCNE_UPDATEDIR, …) on the This PC PIDL makes Explorer re-enumerate the whole folder, so a media change can reset selection/scroll in an open This PC window. There isn't an obviously better option for "an item that didn't exist should now exist", so this is just an FYI.

• The stop loops in StopWorkerThread/StopNotificationThread are unbounded. Joining is the correct thing to do, and CancelSynchronousIo is the right tool, but device I/O in the storage stack isn't always cancellable — if a drive wedges, disabling/updating the mod will appear to hang (on the Windhawk engine thread, so Explorer's UI stays responsive). Issuing the IOCTL with FILE_FLAG_OVERLAPPED and waiting on {stop event, I/O event} would make the stop deterministic, if you think it's worth the complexity.

• Media detection depends entirely on Windows broadcasting DBT_DEVICEARRIVAL/DBTF_MEDIA. Drives that don't report media-change notifications won't un-hide until some other device event or a resume happens. Probably fine in practice, but worth a line in the README's "Detection behavior" list.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ai-review

[windhawk-reviewer]

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-09-10 23:49 UTC (in 6 hours) to get another one.

[Solomag]

/ai-review

[windhawk-reviewer]

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-09-10 23:49 UTC (in 10 minutes) to get another one.

[Solomag]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Good foundation overall: no overlap with an existing mod (nothing in mods/ targets optical drives), the CDrivesViewCallback::ShouldShow symbol matches the x64 form already used by classic-this-pc-sort-order.wh.cpp, HookSymbols is called once per module with a matching shell32DllHooks name, both threads are joined in Wh_ModUninit, no global has a dangerous destructor, and no persistent system state is touched. A few things to fix:

1. Resume / settings rescan hides a drive that actually has media. In the grace path of ProcessInitialScan (lines 558-563), ProbeResult::NotReady is collapsed into MediaState::Empty immediately:

} else if (result == ProbeResult::Empty ||
           result == ProbeResult::NotReady) {
    *retryMask |= bit;
    *graceRetryMask |= bit;
    retryAttempts[index] = 0;
    changed |= SetCachedMediaState(letter, MediaState::Empty);
}

An optical drive that has spun down (which is exactly what happens across PBT_APMRESUMEAUTOMATIC, and often at Explorer startup) answers ERROR_NOT_READY even with a disc inserted. So the mod marks it Empty, fires a refresh, the disc disappears from This PC, and it only comes back when the retry loop later sees Present — a visible flicker of up to the full 10 s grace window. That also contradicts the README's stated "other inconclusive probe failures -> show (fail open)".

ProcessArrivalMask already handles this correctly (lines 700-711: keep the previous cached state and let the retry window settle it). Do the same here — leave NotReady alone in the grace path and let the expiry branch in ProcessRetryMask (lines 787-802) demote it to Empty once the window is exhausted:

} else if (result == ProbeResult::Empty) {
    *retryMask |= bit;
    *graceRetryMask |= bit;
    retryAttempts[index] = 0;
    changed |= SetCachedMediaState(letter, MediaState::Empty);
} else {
    // NotReady / Unknown are inconclusive: keep the cached state and let the
    // bounded retry window settle it.
    *retryMask |= bit;
    *graceRetryMask |= bit;
    retryAttempts[index] = 0;
}

2. Don't CancelSynchronousIo the notification thread. StopNotificationThread calls it at line 1245 and then again every 250 ms at line 1268. That thread is either parked in GetMessageW (nothing to cancel) or executing shell code — SHGetKnownFolderIDList / SHChangeNotify — whose internal registry and file I/O you'd be aborting with ERROR_OPERATION_ABORTED in paths that don't expect it, inside a shared process. The shutdown path is already deterministic without it (g_notificationStopRequested checks, kMsgStop → DestroyWindow → PostQuitMessage, plus PostThreadMessageW(WM_QUIT) for the pre-window case), so just wait on the handle. Cancelling I/O the mod didn't issue is only safe on the worker thread, where every blocking call is the mod's own CreateFileW/DeviceIoControl.

3. Dead code and a duplicated scan path. QueueInitialScan is only ever called with true (lines 1022, 1399, 1417), so the allowGrace == false half of ProcessInitialScan — the guard at line 513 and the whole block at lines 523-548 — is unreachable, as is the default argument at line 372. StopNotificationThread(bool restoreView) (line 1229) has the same problem: its single call site (line 1428) passes true. And ProcessTopologyScan (lines 386-447) is a near-verbatim copy of the first half of ProcessInitialScan (lines 463-494). Given the PR is AI-assisted, this reads as leftover scaffolding. Deleting the unreachable branches and factoring the shared drive-enumeration loop into one helper would cut a meaningful chunk out of a 1430-line mod and make the remaining state machine (retryMask / graceRetryMask / arrivalRetryMask / retryAttempts + five request flags) much easier to follow and to trust.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• Suppress hard-error dialogs around the probe. Touching removable media can raise the "There is no disk in the drive. Please insert a disk into drive X:" hard error. Explorer very likely already sets SEM_FAILCRITICALERRORS process-wide, so this is belt-and-braces, but it's one line at the top of WorkerThreadProc:

  SetThreadErrorMode(SEM_FAILCRITICALERRORS, nullptr);

  Use SetThreadErrorMode, not SetErrorMode — the latter would change process-wide state that Explorer and every other mod share.

• Bounded unload instead of cancel-and-retry. StopWorkerThread (lines 1202-1223) loops CancelSynchronousIo + a 100 ms wait indefinitely. CancelSynchronousIo can't abort an IOCTL the class driver has already accepted, so a flaky disc can stall the unload for tens of seconds (harmless to Explorer, but the mod can't be disabled or updated meanwhile). Opening the device with FILE_FLAG_OVERLAPPED and issuing DeviceIoControl asynchronously would let the worker wait on {overlappedEvent, g_workerStopEvent} and bail out immediately via CancelIoEx.

• IsWorkerStopRequested() is a WaitForSingleObject(..., 0) syscall executed up to 26 times per scan pass and again inside every probe. A plain std::atomic<bool> g_workerStopping set alongside SetEvent(g_workerStopEvent) would be cheaper and reads better; the event is still needed for the WaitForMultipleObjects.

• If CreateWindowExW or SetWindowSubclass fails in NotificationThreadProc (lines 1077-1086), the mod stays loaded and permanently does nothing — no device notifications, no refreshes — with only a log line. Since the drives are never hidden in that state it's not harmful, but returning FALSE from init (Windhawk retries on the next settings change) would be more honest than a silent no-op session.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Empty drives get 21 probes over 10 s on every startup / resume / settings save. In the grace path, a definitive ProbeResult::Empty (ERROR_NO_MEDIA_IN_DRIVE) still sets *graceRetryMask |= bit, and ProcessRetryMask deliberately doesn't clear a grace retry on Empty (line 775), so the full 20-attempt budget runs against a drive that already gave an unambiguous answer. On drives that seek or spin briefly on IOCTL_STORAGE_CHECK_VERIFY2 this is audible. NO_MEDIA is conclusive in a way NOT_READY isn't — consider clearing the grace window on Empty and keeping the retry budget for NotReady only. (This pairs naturally with item 1 above.)

• Per-explorer.exe multiplication. With "Launch folder windows in a separate process" enabled, each explorer.exe gets its own worker thread and its own probe cadence, so the same drive is polled N times during a grace window and N SHChangeNotify broadcasts are emitted per transition. Nothing breaks, and a tool mod isn't an option here since the ShouldShow hook genuinely needs to live inside Explorer — just something to be aware of.

• Navigation pane / address-bar dropdown still list the drive. The README calls this out, which is the right thing to do, but users will likely read it as an inconsistency. Worth a sentence on whether covering those is out of scope by design (broader blast radius) rather than just unimplemented.

• SHChangeNotify(SHCNE_UPDATEDIR, ...) on the Computer folder is a system-wide shell event, so every process with a This PC view open re-enumerates, not just the Explorer instance that sent it. That's the correct mechanism and the refresh only fires on an actual state transition, so the rate is fine — noting it only so the global reach is a conscious choice.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[Solomag]

/ready-for-reviewer

Hi,

I went through the AI review findings and addressed the issues that appeared to be actual user-facing bugs.

The remaining findings are mostly edge cases or architectural suggestions rather than functional problems:

• The suggested DriveState/state-machine rewrite would be a larger refactor of the whole media detection logic. The current implementation already separates startup, arrival, removal and retry handling, and changing the model at this stage would significantly increase the risk of introducing new regressions.
• The remaining NotReady discussion concerns rare timing cases around startup/resume. The mod already uses bounded retries and does not permanently cache an incorrect state.
• The notification thread shutdown concern is a theoretical robustness issue during unload rather than a normal runtime problem.

The mod has gone through several review iterations and the remaining suggestions would mostly be improvements for a future refactor rather than blockers for this PR.

I would appreciate a human review to determine whether any of these remaining points should actually block submission.

Thanks.