Create win-vista-and-7-welcome-center.wh.cpp

[Cips35]

Changelog

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

• Changelog item 1...
• Changelog item 2...

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.

[Cips35]

/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.

---

This is an impressive amount of work, and there's no existing mod that overlaps with it — nothing in mods/ recreates the Vista Welcome Center. The blocking problems are all about how it integrates with Windhawk rather than about the recreation itself: it ships a prebuilt binary, it leaves files and shortcuts behind after it's disabled, and it hosts its whole UI inside explorer.exe even though it installs no hooks there.

1. The embedded prebuilt executable has to go (kShowExeB64, line 23614).

The mod carries a compiled x64 PE as base64 and writes it to %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe (EnsureShowLauncher, line 23677), then uses that path as the target of the Start Menu shortcut, the desktop shortcut, and the HKCU CLSID\...\Shell\Open\Command value. That binary is not buildable from this PR and nobody reviewing it can verify what it does — the imports visible in the blob (FindWindowW, PostMessageW, RegisterWindowMessageW) suggest it's harmless, but the review can only go on the author's word. Windhawk mods are distributed as reviewable source; an opaque binary payload that gets dropped on disk and wired into the Start Menu and Control Panel isn't something that can be accepted.

You already have the mechanism to replace it. Wh_ModInit handles a second windhawk.exe -tool-mod "<id>" instance by forwarding -show to the running host and exiting (line 25078), so the shortcut and the Control Panel command can simply be:

"<path to windhawk.exe>" -tool-mod "vista-welcome-center" -show

That costs one short-lived process instead of a shipped binary. Please also drop the launcher.exe / WelcomeCenter.exe user-override paths (FindUserAsset(L"launcher", ...), FileStartsWithMz) along with it — letting a shortcut target be swapped for an arbitrary user-supplied PE isn't something the mod needs.

2. The mod isn't reversible — it leaves state behind when disabled.

A core Windhawk principle is that a mod's effects disappear when it is disabled. WhTool_ModUninit only calls UnregisterControlPanelItem(). Everything else survives:

• Welcome Center.lnk / Getting Started.lnk in FOLDERID_Programs (created by default — createStartMenuShortcut: true)
• the desktop shortcut, if it was ever enabled
• %LOCALAPPDATA%\WelcomeCenter\ and its contents (WelcomeCenter.exe, app-vista.ico, app-windows7.ico)
• <mod storage>\resources\README.txt and the resources folder (EnsureResourceGuide, line 23309)

On top of that, Wh_ModUninit returns early for g_isToolModProcessLauncher (line 25229), so the process that created the shortcuts and the Control Panel keys in Wh_ModAfterInit never cleans up any of it — today that's masked only because the Explorer host happens to also run UnregisterControlPanelItem().

Teardown should remove every file, folder, shortcut and registry key the mod created, on every path that can create them.

3. Drop @include explorer.exe and make this a plain tool mod.

The mod installs no function hooks in Explorer. In the Explorer branch it just spawns a thread, registers two window classes, creates its own windows, adds a tray icon and registers a hotkey — all of which work from any process. That's precisely the case the "Mods as tools" page describes, and it lists exactly the symptoms this mod has: multiple explorer.exe processes (each one starts a competing UI thread), shell stability risk, and unnecessary coupling. Concretely:

• 84% of this file (1.80 MB of 2.14 MB) is embedded base64 artwork. Hosting in Explorer means that image, plus GDI+ and COM, plus ~25k lines of drawing, PE parsing and image-container scanning, is loaded into the shell — and any fault in it takes down the desktop.
• You had to write a second single-instance mutex (Windhawk.VistaWelcomeCenter.UI in UiThread, line 24717) on top of the framework's windhawk-tool-mod_ mutex, and which process ends up owning the UI is a startup race between Explorer and the tool process. The wiki snippet's mutex already solves this when there's only one host.
• PublishPresence() is called from Wh_ModInit in the Explorer branch (line 25034). Wh_ModInit runs before the target process starts executing, and PublishPresence does CoInitializeEx + CoCreateInstance(CLSID_ShellLink) + file and registry writes. Doing shell COM work in a process that hasn't run its entry point yet is asking for trouble, and it's redundant with the PublishPresence() the UI thread already does.

Tray icons and TaskbarCreated work fine from a dedicated tool process — theme-toggler-tray, mic-tray-control, internet-status-indicator and ultimate-custom-tray are all @include windhawk.exe only. Removing the Explorer host also lets g_isExplorerHost, IsExplorerProcess(), the extra UI mutex and most of the boilerplate divergence go away.

4. WhTool_ModUninit can return while the UI thread is still running.

const DWORD wr = WaitForSingleObject(g.uiThread, 4000);
if (wr == WAIT_TIMEOUT) {
    Wh_Log(L"UI thread did not exit in time; continuing unload");
}

When Wh_ModUninit returns, Windhawk FreeLibrarys the mod image. A thread still executing mod code at that point crashes the host even if it never touches mod state, because its instruction pointer and return addresses live in the unmapped image. In the Explorer host that's an Explorer crash.

This isn't theoretical — UiThread calls WaitForDesktop(15000) before entering its message loop (line 24827), so with runAtStartup enabled the posted WM_CLOSE isn't processed for up to 15 seconds. Disable the mod during logon and the 4-second wait is guaranteed to expire. The same applies while a ShellExecuteExW (SEE_MASK_FLAG_DDEWAIT | SEE_MASK_NOASYNC) is in flight or a modal MessageBox is open.

The wait has to be unconditional. Signal properly and then block indefinitely, e.g.:

g_quitting = true;                                   // std::atomic<bool>, checked by WaitForDesktop's loop
PostThreadMessageW(g.uiThreadId, WM_QUIT, 0, 0);     // in addition to the WM_CLOSE post
WaitForSingleObject(g.uiThread, INFINITE);
CloseHandle(g.uiThread);

Note the timeout path has a second consequence: UiAssetsGuard never runs, so kHostClass and kMainClass stay registered with an lpfnWndProc pointing into the unmapped image. The next enable then fails RegisterClassExW with ERROR_CLASS_ALREADY_EXISTS and the UI silently never initializes.

5. static App g has a non-trivial destructor that runs at process shutdown.

App holds a MemDc, a UniqueIcon and three UniqueGdi fonts, so ~App calls SelectObject / DeleteDC / DeleteObject / DestroyIcon. Wh_ModUninit does not run when the host process terminates (Explorer restart, sign-out, reboot) — only the CRT destructors of globals do, on the shutdown thread, after every other thread has already been killed. The memory DC and its bitmap were created by the UI thread, which is gone by then, so the destructor operates on handles the system may already have reclaimed.

ReleaseUiResources() in UiAssetsGuard already performs the real cleanup on the owning thread, so the automatic destructor only needs to be suppressed:

[[clang::no_destroy]] static App g;

See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown for the two teardown paths and the case-by-case rules.

6. OpenSelected holds a reference into the item vectors across a call that pumps messages.

const Item& it = items[static_cast<size_t>(g.selectedIndex)];
...
LaunchTask(hwnd, it.settingsUri.c_str(), it.classicFile.c_str(),
           it.classicParams.c_str(), g.settings.preferClassicCpl);

LaunchShell → ShellExecuteExW with SEE_MASK_FLAG_DDEWAIT runs a modal pump internally. If WM_APPLY_SETTINGS is dispatched during it (the user saved the mod settings while a task was launching), ApplySettingsOnUiThread calls RebuildItems(), which replaces g.gettingStarted / g.offers / g.oem and frees the strings those const wchar_t* arguments point at — and LaunchTask re-uses settingsUri after the first LaunchShell fails. Copy the three strings into locals before launching:

const std::wstring uri = it.settingsUri, file = it.classicFile, params = it.classicParams;
const bool oem = it.oem;
...

7. Add a screenshot to the README.

The mod is a pixel-accurate visual recreation with two skins, and the README has no image at all. Please add at least one screenshot of the Vista appearance and one of the Windows 7 Getting Started skin (i.imgur.com or raw.githubusercontent.com are the allowed hosts).

Optional improvements

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

• QueryUserName (line 1855) uses LoadLibraryW(L"secur32.dll"). secur32.dll isn't a KnownDLL, so the default search order includes the host executable's directory. Both target processes live in protected directories, so this is hardening rather than a real vector, but LoadLibraryExW(L"secur32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) costs nothing. While you're there: -lsecur32 in @compilerOptions is unused, since GetUserNameExW is resolved dynamically.
• SystemControlPanelExists() (line 24937) is ~90 lines of DOS/PE header parsing plus a file read on every mod load, and it aborts Wh_ModInit entirely if it fails. control.exe is only used for the optional classic-CPL fallback (preferClassicCpl, off by default) — every task also has an ms-settings: / shell: target, and the Control Panel item is a namespace CLSID that doesn't involve control.exe at all. Refusing to start the whole mod over this seems disproportionate; a GetFileAttributesW check at the point of use would do.
• IsDangerousFileName / HasPathMeta (lines 2535–2573) allow-list the OEM link targets, but those come from the mod's own settings, which only the user can edit — someone who wants to run cmd.exe can already do so directly. It mostly just blocks legitimate targets. Related: line 2604 has an if whose body is a comment only, which is dead code:

  if (wcschr(expanded, L'"') || wcschr(expanded, L' ')) {
      // Quoted paths with args are rejected; ...
  }

• SetWindowLongPtrW(g.vscroll, GWLP_WNDPROC, ...) (line 24107) swaps the scrollbar's window procedure directly. It's your own child control so it won't break another mod, but SetWindowSubclass is the convention and handles the chain correctly.
• The runAtStartup handling has three sources of truth — the Windhawk setting, the runAtStartup.override.v2 value, and the runAtStartup.settingSnapshot.v2 reconciliation value — for a boolean that already exists as a Windhawk setting. Dropping the in-window checkbox (or making it just open the mod settings) would remove StoreRunAtStartupPreference, SyncRunAtStartupFromWindhawk and ResolveRunAtStartupPreference entirely.
• kModVersion = L"1.0.64" (line 262) doesn't match @version 1.0.0, so the About dialog shows the wrong version. WH_MOD_VERSION is available as a wide-string literal; kModAuthor can come from the metadata the same way.
• g.trayIcon is only ever reset(), never assigned, and g.userTile is commented // unused. Both can go.
• The tool-mod boilerplate diverges from the wiki snippet in several places (try/catch wrappers, the CreateProcessW fallback, ExitProcess(0) instead of ExitProcess(1), the g_isExplorerHost branches). Keeping it verbatim makes it much easier to pick up future fixes — explorer-folder-hover-menu is a byte-for-byte copy. If the -show forwarding has to stay, keeping it as the only deviation would be ideal.
• Undoc::SHGetUserPicturePath is called with flag 0x80000000, which copies the account picture to a temp path; that file is never deleted.

Functionality notes

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

• SetForegroundWindow(g.main) in ShowWelcome (line 24534) will usually fail when the call doesn't originate from a foreground-input event — the Control Panel and shortcut paths post WM_SHOW_WELCOME to a host window in a process that doesn't hold foreground rights, so the window can come up behind whatever is in front. The usual workaround is a brief SetWindowPos(HWND_TOPMOST) / HWND_NOTOPMOST flip, or attaching to the foreground thread's input queue.
• PositionMainWindow uses SPI_GETWORKAREA, which is the primary monitor's work area, while the DPI comes from GetWndDpi(hwnd). On a multi-monitor setup the window always lands centered on the primary display even when the tray icon was clicked on a secondary one, and on a mixed-DPI setup the size and the placement can be computed against different monitors. MonitorFromPoint(cursorPos, MONITOR_DEFAULTTONEAREST) + GetMonitorInfo would keep them consistent.
• WaitForDesktop polls FindWindowW(L"Shell_TrayWnd") every 150 ms for up to 15 s, and kTrayTimerId re-polls for the taskbar every 2 s. Since HostProcInner already handles the TaskbarCreated broadcast, both are mostly redundant — the broadcast arrives when the shell is actually ready.
• FindLargestEmbeddedImage (line 21300) is O(n²) in the worst case: for every FF D8 FF occurrence with no matching FF D9 after it, the inner loop scans to the end of the buffer. Files are capped at 8 MB, so a pathological .accountpicture-ms could stall the UI thread for a long time. Recording the last-seen candidate start instead of restarting the scan would make it linear.
• The 1.80 MB of embedded base64 makes the mod slow to compile and large in memory. Even after moving to a dedicated process, it may be worth checking whether some of the artwork can be drawn (the chevron, the green arrow, the user frame) rather than embedded as PNG.

---

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.

[Cips35]

/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.

---

This is an impressive amount of work — the Vista/Win7 recreation itself is detailed and the artwork pipeline (user files override embedded Base64) follows the Win7 Network Flyout precedent well. The problems are almost entirely in how the mod integrates with Windhawk and the system, not in the recreation.

1. The mod ships a prebuilt executable as a Base64 blob and writes it to disk.

kShowExeB64 (line 23616) is a ~2.5 KB PE image that EnsureShowLauncher decodes and writes to %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe, and that binary is then registered as the target of the Start Menu shortcut, the Desktop shortcut, and the HKCU\...\CLSID\{8F3C2A91-...}\Shell\Open\Command Control Panel entry.

Windhawk mods are distributed and reviewed as source. An opaque compiled binary can't be read, diffed, or rebuilt by a reviewer or by a user — the import table (FindWindowW, PostMessageW, RegisterWindowMessageW, ShowWindow, SetForegroundWindow, Sleep, ExitProcess) is consistent with the stated purpose, but that is not the same as being auditable, and every future update to the mod would carry an equally unreviewable blob. This needs to go.

The good news is that the mod already contains the plumbing to do without it. Wh_ModInit handles -tool-mod <id> plus -show (CommandLineHasShow(), line 25087), forwarding the request to the already-running instance via WM_SHOW_WELCOME. So GetToolLaunch can return the real windhawk.exe path with -tool-mod "<mod id>" -show as arguments instead of dropping a helper. If that turns out not to be viable, the fallback is to drop the shortcut/Control Panel integration — the tray icon and the hotkey already cover opening the window, and neither needs an executable on disk.

2. The mod leaves persistent files behind when it is disabled.

Reversibility is a core Windhawk principle: everything the mod does should disappear when it's turned off. WhTool_ModUninit (line 24888) only calls UnregisterControlPanelItem(). Not removed:

• %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe
• %LOCALAPPDATA%\WelcomeCenter\app-vista.ico / app-windows7.ico
• the Start Menu .lnk (FOLDERID_Programs)
• the Desktop .lnk (FOLDERID_Desktop)
• <mod storage>\resources\README.txt

After uninstalling the mod the user is left with Start Menu and Desktop shortcuts pointing at a stale helper executable that nothing maintains any more. DeleteOwnedWelcomeShortcut already exists and correctly verifies ownership before deleting — call it (plus DeleteStaleKnownLinks for both known folders) and delete the generated files from Wh_ModUninit.

3. Wh_ModUninit lets the mod unload while its UI thread is still running.

const DWORD wr = WaitForSingleObject(g.uiThread, 4000);
if (wr == WAIT_TIMEOUT) {
    Wh_Log(L"UI thread did not exit in time; continuing unload");
}

When Wh_ModUninit returns, Windhawk unloads the mod image with a single FreeLibrary. If the wait timed out, the mod image is unmapped while the UI thread is still executing mod code, and while kMainClass/kHostClass are still registered with lpfnWndProc pointing into the unmapped image (the UiAssetsGuard destructor that unregisters them only runs when the message loop exits). That's a crash in explorer.exe.

This isn't a theoretical timeout. The teardown relies on PostMessageW(g.host, WM_CLOSE, ...) being dispatched, but the UI thread is not pumping messages while it is:

• inside WaitForDesktop(15000) (line 24834) — a Sleep(150) poll loop that can run for 15 seconds at logon before the message loop even starts;
• inside TrackPopupMenu in ShowTrayMenu;
• inside a modal UiMessageBox (About dialog, "online offers are disabled", the Media Player prompt).

Fix: make the thread interruptible and then wait unconditionally. Set a stop flag checked by WaitForDesktop's loop, PostThreadMessageW(g.uiThreadId, WM_QUIT, 0, 0) in addition to the WM_CLOSE, and use WaitForSingleObject(g.uiThread, INFINITE). win7-network-flyout-recreation.wh.cpp joins its threads with INFINITE for exactly this reason.

4. This should be a pure tool mod — drop @include explorer.exe.

The mod installs no function hooks in Explorer. Everything it does — Shell_NotifyIcon, RegisterHotKey, RegisterWindowMessage, CreateWindowEx, SHGetStockIconInfo, the registry and shortcut writes — works from any process, and the mod even carries its own single-instance mutex (Windhawk.VistaWelcomeCenter.UI). That is the exact signature of the "mods as tools" pattern, which the mod already half-implements.

Running the UI in Explorer as well costs you:

• ~7 000 lines of GDI+/COM/shell UI code running inside the shell — any bug there takes down explorer.exe instead of a disposable helper process;
• per-process cost in every explorer.exe (separate-process folder windows included): SystemControlPanelExists() opens and reads control.exe, LoadSettings() runs, PublishPresence() runs, and a UI thread is created only to exit on the mutex;
• non-deterministic behavior: which host actually owns the UI is decided by a race on that mutex between Explorer and the windhawk.exe tool process, so "the tool process is a fallback" isn't really true — it's whoever wins.

Recommend removing @include explorer.exe, deleting the g_isExplorerHost branch and IsExplorerProcess(), and keeping only @include windhawk.exe with the wiki boilerplate verbatim. The current boilerplate has diverged from the snippet in several places (the Explorer branch, SystemControlPanelExists, the CreateProcessW fallback, PublishPresence in Wh_ModAfterInit/Wh_ModSettingsChanged); the maintainer prefers an unmodified copy so it stays easy to diff. explorer-folder-hover-menu.wh.cpp is a good reference — its launcher block at the bottom of the file is a verbatim copy of the wiki snippet.

5. Heavy COM / shell / filesystem work runs inside Wh_ModInit in explorer.exe.

Wh_ModInit runs before the target process starts executing. In the Explorer branch (lines 25022–25040) it currently does:

• SystemControlPanelExists() — CreateFileW + two ReadFiles on %SystemRoot%\System32\control.exe;
• LoadSettings();
• PublishPresence() — CoInitializeEx, CoCreateInstance(CLSID_ShellLink), IPersistFile::Save of two .lnk files, Base64-decode + write of WelcomeCenter.exe, six RegCreateKeyEx/RegSetValueEx calls, and SHChangeNotify(SHCNE_ASSOCCHANGED, ...).

Initializing COM and creating shell objects that early in Explorer's lifetime is a startup hang/crash risk, and broadcasting SHCNE_ASSOCCHANGED forces a system-wide association refresh on every mod load, every settings change, and every unload. If item 4 is addressed most of this goes away; otherwise move all of it onto the UI thread (or at minimum to Wh_ModAfterInit), and only fire SHChangeNotify when the registered values actually changed.

6. PublishPresence() races with itself across processes, and rewrites the helper .exe on every call.

PublishPresence() is invoked from Wh_ModInit (per Explorer process), from UiThread, from Wh_ModAfterInit (launcher), from Wh_ModSettingsChanged (launcher), and from ApplySettingsOnUiThread — with no coordination. Multiple processes therefore write the same two .lnk files and the same registry keys concurrently.

Worse, each PublishPresence() calls GetToolLaunch three times (Start Menu link, Desktop link, Control Panel command), and EnsureShowLauncher unconditionally re-writes the executable each time:

UniqueHandle f(CreateFileW(exePath, GENERIC_WRITE, 0, nullptr,
                           CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr));

dwShareMode == 0 and CREATE_ALWAYS, so a concurrent writer (or the helper being run by the user at that moment) makes this fail, GetToolLaunch returns false, and the shortcut / Control Panel entry is silently skipped. EnsureAppIconFile already gets this right with a FileEqualsBytes check plus a temp-file-and-MoveFileEx swap — if the helper survives item 1, it needs the same treatment; either way PublishPresence should have a single owner.

7. The README has no screenshot.

The whole point of the mod is a pixel-accurate window, in two skins. Please add at least one screenshot (ideally one per appearance) — i.imgur.com and raw.githubusercontent.com are the allowed image hosts.

Optional improvements

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

• Dead embedded payloads (~1 600 lines / ~120 KB of Base64). kDetailsPngB64 (lines 10663–11235) is never used as any TaskDef::embedPng; its only reference is the b64 == kDetailsPngB64 pointer comparison in LogicalFromEmbed, which can therefore never be true. kAppIconIcoB64 (lines 8072–9142) exists only so FindUserAppIconFile can byte-compare against an app.ico produced by "builds through 1.0.51" — but this is the mod's first version in the catalog (@version 1.0.1), so no such file can exist on any user's machine. The same goes for the show.ps1 cleanup in EnsureShowLauncher and the .v2 suffixes on runAtStartup.override.v2 / runAtStartup.settingSnapshot.v2. All of it can be deleted for the initial submission.

• Dead code. The Guarded() template (line 402) is never called. App::userTile (line 13809) is unused and says so in its own comment. ComLifetime::ok() is never checked by any caller.

• LoadLibraryW(L"secur32.dll") (line 1856) loads a non-KnownDLL by bare name, which searches the executable's own directory first. Both hosts live in protected directories so this is hardening rather than a live vector, but LoadLibraryExW(L"secur32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) is free.

• -lsecur32 is unused — GetUserNameExW is resolved with GetProcAddress, so nothing links against the import library. Worth double-checking -loleaut32 too.

• WhString reimplements WindhawkUtils::StringSetting. Add #include <windhawk_utils.h> and use WindhawkUtils::StringSetting (it supports the L"oemLinks[%d]", i form as well). Note that Wh_GetStringSetting never returns NULL — it returns L"" — so the null handling in WhString::c_str()/empty() is unnecessary.

• kModVersion[] = L"1.0.1" duplicates @version and has to be kept in sync by hand. WH_MOD_VERSION is available as a wide-string literal.

• g.mainMenu can leak. It's created by BuildMenu() and destroyed only implicitly by DestroyWindow(g.main). If the window is destroyed while full screen, SetFullScreen has already done SetMenu(hwnd, nullptr), so the menu is detached and never freed; the UiAssetsGuard destructor just sets g.mainMenu = nullptr. One HMENU per mod reload, in explorer.exe. DestroyMenu it explicitly in the guard when it isn't attached.

• IsDangerousFileName is a leaky blocklist. LaunchUserTarget allows .lnk targets, and a shortcut can carry arbitrary arguments and point at any of the blocked executables — so the "no arguments (prevents /c and similar)" guarantee doesn't hold. .cpl has the same issue (it's a DLL run through the Control Panel host). Since the target comes from the user's own Windhawk settings, this whole layer mostly adds surface area while implying a guarantee it can't keep; consider dropping it, or at least resolving .lnk targets before checking.

• Exception wrapping. MainProc/MainProcInner, HostProc/HostProcInner, and the try/catch(...) blocks in ShowWelcome, ApplySettingsOnUiThread, DrawHeader, PaintMain, Wh_ModInit, WhTool_ModInit and WhTool_ModUninit add a lot of structure for paths that shouldn't throw in the first place (the only real thrower is GDI+ allocation, already handled locally). Swallowing an unknown exception in a window proc and returning DefWindowProcW mostly converts a diagnosable crash into a silently broken window. Worth trimming to the couple of places that genuinely need it.

Functionality notes

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

• Hit rects aren't clipped to the task area. DrawTasks pushes every Hit with its raw rect (irc, computed from y - g.scrollY) while the actual drawing happens under gr.SetClip(clip). Once the list is scrolled, tiles that have scrolled up behind the header still have hit rects overlapping the header region, and HitTest walks the vector backwards so those items win over the HitKind::ShowMore rect that was pushed first. Result: clicking in the header area can select an invisible tile instead of "More about this PC". Intersect each rect with L.tasks before pushing (and skip it if the intersection is empty).

• "More about this PC" fires up to three times on a double-click. Both WM_LBUTTONUP and WM_LBUTTONDBLCLK call OpenSelected/OpenShowMore for HitKind::ShowMore, and the class has CS_DBLCLKS, so a double-click delivers WM_LBUTTONDOWN → WM_LBUTTONUP → WM_LBUTTONDBLCLK → WM_LBUTTONUP, i.e. three launches. Handle it in one message only.

• FindLargestEmbeddedImage is quadratic. For every byte offset it scans forward looking for a terminator, and gives up only at end-of-buffer when there is none. On an input up to the 8 MB cap (LoadAccountPictureContainer) with repeated FF D8 FF / \x89PNG sequences and no matching terminator, that's O(n²) on the UI thread during RefreshSysInfo. It's also reached for any file GDI+ fails to decode, not just .accountpicture-ms, via the fallback in LoadUserPictureFile. Parsing the container format (or at least bounding the outer scan) would be more predictable.

• The window always opens on the primary monitor. PositionMainWindow centers using SystemParametersInfoW(SPI_GETWORKAREA, ...), which returns the primary display's work area only. On a multi-monitor setup, opening via the tray icon or the hotkey puts the window on the primary display regardless of where the user is working — MonitorFromPoint(cursorPos, MONITOR_DEFAULTTONEAREST) + GetMonitorInfo would follow the user. It also means the initial g.dpi is sampled before the window lands on its final monitor, so on a mixed-DPI setup the first frame can be scaled for the wrong display until WM_DPICHANGED arrives.

• LoadStockIcon can grow the cache without bound. When ExtractSizedIcon fails for both sizes it still pushes a CachedIcon with icon == nullptr, and the lookup loop skips null entries — so every call for that stock ID appends another dead entry. GetStockIconBitmap doesn't have this problem (it returns early on failure); worth aligning the two.

• Two SetTimer cadences worth a look. The header cross-fade runs a 16 ms timer (kHeaderAnimTimerId) that invalidates the whole client area each tick — it's correctly killed when the animation finishes, but a full-window InvalidateRect at 60 fps for a header-only fade is more repainting than needed; invalidating L.header would be enough. The kTrayTimerId 2 s retry loop is fine, but it's only killed once the icon is added — if showTrayIcon is on and the taskbar never appears it polls forever.

---

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.

[Cips35]

/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.

---

Impressive amount of work, and the artwork/localization effort shows. The blocking problems are all about how the mod integrates with the system rather than about the recreation itself: it ships a prebuilt binary, leaves files behind after it is disabled, injects a full UI into explorer.exe it doesn't need, and can let Windhawk unload the module while its UI thread is still running.

1. The mod embeds a prebuilt .exe and writes it to disk — this can't be accepted as-is

kShowExeB64 (line 23616) is a base64-encoded 2,560-byte PE32+ executable, and EnsureShowLauncher writes it to %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe, then points the Start Menu shortcut, the desktop shortcut and the Control Panel Shell\Open\Command at it.

Windhawk mods are distributed as source that Windhawk compiles on the user's machine — that is what makes them auditable. An opaque machine-code blob bypasses that entirely: nobody reviewing this PR (or the mod page) can tell what those 2,560 bytes do. On top of that, a shell process writing an executable into %LOCALAPPDATA% and registering a Start Menu/Control Panel entry that launches it is exactly the behaviour AV/EDR products flag as a dropper, and the blob is x64-only so it would run under emulation on ARM64.

Related, FindUserAsset(L"launcher", ...) (via kAssetNames, line 19230) will pick up any file named WelcomeCenter.exe or launcher.exe sitting in the mod storage folder, its resources subfolder, or the user-configured Custom resource folder, and silently wire that arbitrary executable into the Start Menu shortcut and the Control Panel open command. That's a surprising amount of trust to hand to a filename.

Please drop the launcher entirely. The tray icon and the hotkey already cover opening the window, and both work from any process. If you want to keep the Start Menu / Control Panel entries, they need a target that isn't a binary the mod carries — otherwise the honest simplification is to remove those two integrations.

2. The mod isn't fully reversible

A core Windhawk principle is that a mod's effects disappear when it is disabled. Right now WhTool_ModUninit only calls UnregisterControlPanelItem(). Everything else survives:

• %LOCALAPPDATA%\WelcomeCenter\ — WelcomeCenter.exe, app-vista.ico, app-windows7.ico
• Welcome Center.lnk / Getting Started.lnk in the Start Menu (FOLDERID_Programs) and, if enabled, on the desktop
• resources\README.txt in the mod storage folder

So after disabling the mod the user is left with a Start Menu entry that launches a leftover executable that does nothing. Delete every file and shortcut the mod created in Wh_ModUninit (you already have DeleteOwnedWelcomeShortcut and DeleteStaleKnownLinks — call them unconditionally on unload), or drop the shortcut/CPL features per item 1.

3. Hosting the UI inside explorer.exe — this should be a pure tool mod

The mod declares @include explorer.exe and @include windhawk.exe, and both processes run WhTool_ModInit → UiThread. It installs no function hooks in Explorer at all; it only creates windows, a tray icon and a hotkey — all of which work from any process in the session. That is the textbook "mods as tools" case: https://github.com/ramensoftware/windhawk/wiki/Mods-as-tools:-Running-mods-in-a-dedicated-process

Two concrete consequences of the current design:

• Which process hosts the UI is a race. Explorer's copy and the dedicated windhawk.exe -tool-mod process both start a UI thread and then fight over the Windhawk.VistaWelcomeCenter.UI mutex (line 24718); the loser exits. Nothing makes Explorer win, so the README's "UI is hosted in explorer.exe … a dedicated windhawk.exe tool process is still used as a fallback" doesn't describe what the code does. The hand-rolled mutex is also exactly what the framework's launcher already handles.
• Any bug in ~10k lines of GDI+ painting, hit-testing and shell-launching code can take down the shell. The try/catch (...) wrappers around MainProc/HostProc don't help here — Clang/mingw won't turn an access violation into a C++ exception.

The README's stated reason ("UI is hosted in explorer.exe so the tray icon and Control Panel item actually appear") doesn't hold: a tool mod gets both. See mods/theme-toggler-tray.wh.cpp — @include windhawk.exe only, tray icon plus TaskbarCreated re-registration — and mods/caffeine.wh.cpp.

Please drop @include explorer.exe, delete g_isExplorerHost / IsExplorerProcess() and the Explorer branches in Wh_ModInit / Wh_ModSettingsChanged / Wh_ModUninit, and restore the launcher boilerplate to the wiki snippet verbatim — the current copy has diverged (the extra Explorer branch, the -show forwarding, the CreateProcessW fallback, ExitProcess(0) instead of ExitProcess(1)). Compare against the bottom of mods/explorer-folder-hover-menu.wh.cpp, which is a verbatim copy.

4. WhTool_ModUninit lets the module unload while the UI thread may still be running

const DWORD wr = WaitForSingleObject(g.uiThread, 4000);
if (wr == WAIT_TIMEOUT) {
    Wh_Log(L"UI thread did not exit in time; continuing unload");
}

When Wh_ModUninit returns, Windhawk calls FreeLibrary on the mod. If the UI thread is still alive at that point, its instruction pointer and return addresses are in an image that has just been unmapped — the host process crashes, even though the thread never touches mod state. It also leaves kMainClass / kHostClass registered with an lpfnWndProc pointing into the unmapped image, so the next load hits ERROR_CLASS_ALREADY_EXISTS and the UI silently never comes up.

4 seconds is not a safe margin, because several UI-thread paths block for longer:

• WaitForDesktop(15000) runs before the message loop is entered (line 24834), so a posted WM_CLOSE just sits in the queue for up to 15 s.
• modal MessageBoxW via UiMessageBox — About, "online offers are disabled", the Media Player prompt — and TrackPopupMenu in ShowTrayMenu.
• LaunchShell uses SEE_MASK_NOASYNC | SEE_MASK_FLAG_DDEWAIT, so ShellExecuteExW blocks until the target is up.

Please make the wait unconditional (WaitForSingleObject(g.uiThread, INFINITE) / join) and make those paths abortable so the wait always terminates: cancel WaitForDesktop on a stop event instead of Sleep-polling for 15 s, and close any modal dialog before waiting (or move the blocking ShellExecuteExW off the UI thread). Giving up and unloading anyway is the one outcome that must not happen.

5. Wh_ModInit does COM, shell and file I/O before Explorer has executed a single instruction

In the Explorer branch, Wh_ModInit calls PublishPresence() synchronously (line 25035), which does CoInitializeEx → CoCreateInstance(CLSID_ShellLink) → SHGetKnownFolderPath → HKCU registry writes → writes WelcomeCenter.exe and an .ico → SHChangeNotify. Wh_ModInit runs before the target process starts executing, so this is COM and shell-namespace work inside explorer.exe before Explorer itself has initialized, and it delays every Explorer start.

Additionally, SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, nullptr, nullptr) (lines 23944 and 24000) is a system-wide shell refresh — it invalidates icon caches and makes every Explorer window in the session re-query associations. It currently fires on init, on unload, and on every settings change, from every host process. Fire it only when the Control Panel registration actually changed, and move all of PublishPresence() onto the UI thread. (Item 3 removes the Explorer copy of this problem entirely.)

6. static App g; runs GDI teardown at process shutdown

App holds MemDc buffer (DeleteDC + DeleteObject), UniqueIcon trayIcon (DestroyIcon) and three UniqueGdi fonts (DeleteObject). Wh_ModUninit is not called when the host process exits (Explorer restart, sign-out, reboot), but the destructor of g still runs there — alone on the exiting thread, under the loader lock, after every other thread has been terminated. RAII wrappers that call teardown APIs are exactly the case the wiki says to suppress:

[[clang::no_destroy]] std::optional<App> g_app;  // engage in UiThread, reset in ReleaseUiResources/uninit

ReleaseUiResources() already does the real cleanup on the controlled path, so keep calling it and make sure it fully releases. See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown (§ Needs the fix → "RAII wrappers calling teardown APIs").

7. Double-clicking "More about this PC" launches the target three times

MainProcInner handles the ShowMore hit in both WM_LBUTTONUP (line 24236) and WM_LBUTTONDBLCLK (line 24227), and the class has CS_DBLCLKS. A double-click produces DOWN, UP, DBLCLK, UP, so OpenShowMore / OpenSelected runs three times and Settings (or the selected task) is opened three times. Handle it in one message only — drop the HitKind::ShowMore branch from WM_LBUTTONDBLCLK.

8. Add a screenshot to the README

This is a purely visual mod with two skins and 14 languages, and the README has no image at all. Users pick mods from the catalog largely on the screenshot. Please add at least one screenshot (ideally one per skin); only i.imgur.com and raw.githubusercontent.com are allowed as image hosts.

Optional improvements

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

• Bare-name LoadLibraryW(L"secur32.dll") (line 1856). secur32.dll is not a KnownDLL, so the default search order includes the executable's directory. Both hosts live in protected directories today, so this is hardening rather than a live hole, but the one-line fix is LoadLibraryExW(L"secur32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32). See issue 2063.
• -lsecur32 in @compilerOptions is unused — GetUserNameExW is only ever resolved with GetProcAddress, so nothing links against the import library.
• WhString reimplements WindhawkUtils::StringSetting. #include <windhawk_utils.h> and use WindhawkUtils::StringSetting instead. Note that Wh_GetStringSetting never returns NULL (it returns L""), so the null checks inside WhString are dead too.
• Dead code:
  • Guarded() (line 402) is defined and never called.
  • App::userTile (line 13809) is declared with a // unused comment.
  • Lines 2609-2612 are an if with an empty body containing only a comment.
  • GetToolLaunch's args output is always empty, so the args[0] branch in RegisterControlPanelItem and sl->SetArguments(args) are no-ops.
• IsDangerousFileName() is security theatre. It only gates targets the user typed into the mod's own oemLinks setting — someone who can edit the setting can already point it at a .lnk or a batch-wrapping .exe. It adds a maintenance burden (the list will always be incomplete) without a threat it actually stops. Same for the HasPathMeta .. check, which also rejects legitimate paths that happen to contain two dots.
• Reentrancy on settings change: OpenSelected holds const Item& it = items[...] across LaunchShell, which runs ShellExecuteExW with SEE_MASK_FLAG_DDEWAIT — that can pump messages, dispatching a queued WM_APPLY_SETTINGS → ApplySettingsOnUiThread → RebuildItems(), which reallocates the vector and dangles it. Copy the strings you need into locals before launching.
• shell32 ordinal 261 (Undoc::SHGetUserPicturePath, line 1661). Ordinals aren't a stable contract; if it ever shifts you'll call an unrelated function with a guessed signature. You already try six documented locations first — consider just dropping the last-resort call.
• SetMenu(hwnd, nullptr) in SetFullScreen detaches the menu bar, so if the window is destroyed while full screen, DestroyWindow no longer frees it and UiAssetsGuard only sets g.mainMenu = nullptr. Call DestroyMenu there.
• The runAtStartup three-way state (Windhawk setting + runAtStartup.override.v2 + runAtStartup.settingSnapshot.v2) is a lot of machinery for a checkbox that duplicates a Windhawk setting. Dropping the in-window checkbox and the tray-menu toggle and letting the Windhawk setting be authoritative would remove StoreRunAtStartupPreference / SyncRunAtStartupFromWindhawk / ResolveRunAtStartupPreference entirely.
• The catch (...) wrappers around MainProc, HostProc, UiThread, Wh_ModInit, ApplySettingsOnUiThread etc. only catch C++ exceptions, which this code doesn't throw (GDI+ returns status codes). They mostly hide errors; consider removing them.

Functionality notes

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

• High-resolution mouse wheels don't scroll. WM_MOUSEWHEEL does const int steps = delta / WHEEL_DELTA; (line 24250), so any device that reports deltas smaller than 120 (precision touchpads, many modern mice) yields steps == 0 and nothing moves. Accumulate the remainder in a static/global instead of truncating.
• The header animation repaints the whole window at ~60 fps. SetTimer(g.main, kHeaderAnimTimerId, 16, ...) plus InvalidateRect(hwnd, nullptr, FALSE) re-runs the full GDI+ pass (SmoothingModeHighQuality, InterpolationModeHighQualityBicubic, ClearType) including all task tiles, for the whole 2100 ms intro. Invalidating only L.header during the animation would cut most of that.
• The launcher is never refreshed. EnsureShowLauncher returns the file found by FindUserAsset before it ever writes, so once WelcomeCenter.exe exists it is used forever — a copy written by an older mod version is never updated. (Moot if you drop it per item 1.)
• Scroll range: whenever the content is scrollable, MaxScrollY() adds ScalePx(8, g.dpi) past the measured content height, so the list can be scrolled 8 px past the last tile. Probably intentional bottom padding — just noting it in case it isn't.
• SectionTitle returns a pointer to a function-local static wchar_t buf[192], so two calls in one expression would clobber each other. It's fine today (each call is consumed immediately), just fragile.

---

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.

[Cips35]

/ai-review

[Cips35]

/ai-review

[windhawk-reviewer]

@Cips35 /ai-review can't be applied here: an AI review was already requested, please wait for it to be posted.

[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 work overall — the tool-mod boilerplate is a verbatim copy of the wiki snippet, the [[clang::no_destroy]] std::optional<App> global with an explicit g_app.reset() in WhTool_ModUninit is exactly the right pattern, window classes are unregistered, TaskbarCreated is handled, and there's no overlapping mod in the catalog. The items below are what I'd want fixed before merge.

1. Unloading the mod can hang forever. WhTool_ModUninit does WaitForSingleObject(g.uiThread, INFINITE) (line 24669), but RequestUiShutdown() (line 24389) only posts WM_CLOSE to g.main/g.host. There are at least two ways the UI thread never gets that message:

• Startup race. g.host is not created until line 24498, after LoadSettings(), RebuildItems() and RefreshSysInfo() — and RefreshSysInfo does registry reads, EnumDisplayDevicesW, several FindFirstFileW directory scans and (last resort) SHGetUserPicturePath, which copies a file. If the mod is disabled during that window, RequestUiShutdown posts nothing, the thread proceeds into GetMessageW and never exits.
• Modal loops. EndMenu() on line 24393 runs on the uninit thread; per its docs it "ends the calling thread's active menu", so it does nothing for a popup owned by the UI thread. And CloseThreadWindowProc posts WM_CLOSE + WM_COMMAND IDCANCEL to #32770 windows, which an MB_YESNO box ignores — so if the "Windows Media Player Legacy is unavailable" prompt (line 2723) is open when the mod is disabled, the wait never returns.

The robust fix is to post WM_QUIT to the thread instead of WM_CLOSE to windows: a WM_QUIT breaks out of the main loop and out of MessageBox/TrackPopupMenu modal loops. Force the queue to exist at the top of UiThread so PostThreadMessageW can't fail, and signal readiness:

static HANDLE g_uiReadyEvent;  // created in WhTool_ModInit

static DWORD WINAPI UiThread(LPVOID) {
    g.uiThreadId = GetCurrentThreadId();
    MSG dummy;
    PeekMessageW(&dummy, nullptr, WM_USER, WM_USER, PM_NOREMOVE);  // create the queue
    SetEvent(g_uiReadyEvent);
    ...
}

static void RequestUiShutdown() {
    if (g_stopEvent) SetEvent(g_stopEvent);
    if (g_uiReadyEvent) WaitForSingleObject(g_uiReadyEvent, 5000);
    if (g.uiThreadId) PostThreadMessageW(g.uiThreadId, WM_QUIT, 0, 0);
}

Also worth checking the stop event once more right before while (GetMessageW(...)), so a stop requested during window creation is honoured. (Minor, same area: RequestUiShutdown reads g.uiThreadId / g.main / g.host while the UI thread writes them, with no synchronization.)

2. ~2,100 lines of dead embedded assets — EnsureAppIconFile is never called. Nothing in the file calls EnsureAppIconFile (line 21103), which is the only consumer of FindUserAppIconFile, GetAppIconFilePath, SelectedAppIconIcoBase64 and FileEqualsBytes — and, transitively, of the two embedded ICO blobs kAppIconIcoB64 (lines 8057–9127) and kVistaAppIconIcoB64 (lines 9696–10647). That's roughly 190 KB of base64 in a 2.1 MB source file that can never be decoded, plus the file-writing machinery for an .ico the mod no longer needs (the window/tray icon comes from GetAppHicon() → kVistaAppIconPngB64). Please delete the whole chain. It also means the app.ico override documented in the readme has no effect (FindUserAsset(L"app-ico", ...) is only reachable from the dead path).

3. CleanupLegacyPresence() deletes user files and registry keys on every unload, for a mod that has never shipped. On each WhTool_ModUninit the mod initializes COM and then (lines 23673–23711) SHDeleteKeyWs two HKEY_CURRENT_USER subtrees, walks all 15 languages × 2 product names looking for .lnk files to delete from the user's Start Menu and Desktop, deletes %LOCALAPPDATA%\WelcomeCenter\{WelcomeCenter.exe, app-vista.ico, app-windows7.ico, show.ps1}, removes that directory, and deletes README.txt from the mod storage folder. This is a first submission (v1.0.2) — no catalog version ever created any of this, and since EnsureAppIconFile is dead nothing writes to %LOCALAPPDATA%\WelcomeCenter either. Migration code for private pre-release builds shouldn't ship in the catalog: please drop CleanupLegacyPresence, UnregisterControlPanelItem, DeleteStaleKnownLinks, DeleteOwnedWelcomeShortcut, ShortcutTargetsWelcomeLauncher, GetKnownLinkPathForName and GetWelcomeCenterDir. A mod deleting shortcut files out of the user's Start Menu is the kind of thing that should never be in the unload path at all. (FindUserAppIconFile's comment referring to "builds through 1.0.51" is the same problem.)

4. The readme describes behaviour the code doesn't implement. These will confuse users who go looking for the features:

• "A README.txt is created there listing the names" — nothing creates a README.txt; the only code that touches one deletes it.
• app.png / app.ico in the artwork table — the .ico half is dead (see item 2).
• "No artwork is loaded from %SystemRoot%" — QueryOemInfo (line 2144) reads HKLM\...\OEMInformation\Logo, which on most OEM machines points at %WINDIR%\System32\oemlogo.bmp, and GetOemLogoBitmap (line 21771) loads it directly. Task icons also come out of system DLLs via SHDefExtractIconW. Either reword the claim or scope it to the mod's own artwork.
• Related: kModVersion is L"1.0.1" (line 251) while @version is 1.0.2, so the About dialog shows the wrong version. Use the built-in WH_MOD_VERSION instead of a hand-maintained constant, and consider WH_MOD_ID-style constants elsewhere too.

5. The window won't follow per-monitor DPI. The mod handles WM_DPICHANGED and scales everything through ScalePx/GetDpiForWindow, but never sets a DPI context for its UI thread, so all of that depends on whatever windhawk.exe declares process-wide. Set it explicitly at the top of UiThread, before any window is created — this is what the other tool mod with its own UI does: mods/quick-launch-media-panel.wh.cpp#L1714.

SetThreadDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);

6. A custom OEM link with a query string is silently dropped. LaunchUserTarget (line 2565) calls HasPathMeta(target) before the protocol dispatch, and HasPathMeta (line 2521) rejects any string containing &, |, <, >, ^ or ... & is a normal URL query separator, and it isn't a shell metacharacter here — ShellExecuteExW takes lpFile as a single string, not a command line. So My link|https://example.com/p?a=1&b=2 just does nothing, with no error shown. Move the metacharacter check into the local-file branch, where it's actually about a path.

Optional improvements

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

• More dead code to prune: FileStartsWithMz (line 19294) is never called; no entry in kAssetNames sets allowExe, so that parameter, the .exe branch of LooksLikeAssetName and kMaxLauncherBytes are all unreachable; WM_SHOW_WELCOME (line 257) is handled but never posted; the RegisteredShowMessage() message (line 24287) is handled but never sent by anything; App::trayIcon (line 13793) is only ever .reset(), never assigned, and App::userTile (line 13794) is marked unused in its own comment. Line 2593 has an if whose body is a comment only — presumably a leftover.
• The target allow-listing looks like scaffolding for a threat that doesn't exist. IsDangerousFileName (line 2539), the plain-http:// refusal, and LaunchDefaultBrowser checking the user's own default browser against that blocklist (line 2620) all guard against strings the local user typed into the mod's own settings. There's no attacker in that path, and the effect is that legitimate configurations fail with a confusing message box. Since the PR discloses AI assistance — is this intentional, or an artifact? I'd drop the blocklist and keep only the "must be an existing file with an allowed extension" check.
• SystemControlPanelExists() (line 24555) refuses to start the entire mod — 80 lines of DOS/PE header parsing — if %SystemRoot%\System32\control.exe isn't a valid PE. But control.exe is only a fallback target (preferClassicCpl is off by default, and the ms-settings: URIs work regardless), so a stripped-down install loses the whole mod instead of just the classic-CPL fallback. Degrading (hide/skip the classic targets) would be friendlier than failing closed, and a plain existence check would do.
• LoadLibraryW(L"secur32.dll") (line 1847) loads by bare name; secur32.dll isn't a KnownDLL, so the default search order includes the host executable's directory. The host here is windhawk.exe in a protected directory, so this is hardening rather than a real hole — but LoadLibraryExW(L"secur32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) costs nothing. Same idea for LaunchShell(owner, L"control.exe", ...) (lines 12835, 12859, 12869, …): ShellExecuteExW resolves a bare name through the current directory first, and you already compute the absolute %SystemRoot%\System32\control.exe path in SystemControlPanelExists — pass that instead. (The LoadLibraryW(L"shell32.dll") fallback on line 1646 is fine — shell32 is a KnownDLL — but it's unreachable, since shell32 is always loaded in this process.)
• Unused build inputs: -lsecur32 in @compilerOptions isn't needed (GetUserNameExW is resolved dynamically), and #include <lmcons.h> is unused (UNLEN never appears).
• WhString (line 406) reimplements WindhawkUtils::StringSetting from windhawk_utils.h — same RAII shape, including the format-string constructor. Using the built-in one drops ~20 lines. (Also, Wh_GetStringSetting never returns NULL — it returns L"" — so the p_ ? p_ : L"" guard is belt-and-braces.)
• #define g (*g_app) (line 13830) is a one-character object-like macro in a 25 k-line file; it will silently rewrite any future local named g (including the very common Gdiplus::Graphics g). A three-letter #define app (*g_app) or an inline accessor would be much safer.
• Stale comment: line 13886 says "The UI runs inside explorer.exe, so its process-level AppUserModelID belongs to File Explorer" — no longer true now that it's a tool mod in windhawk.exe.
• Class icon can dangle across a settings change. mc.hIcon = GetAppHicon() is baked into the class at registration (line 24484), but InvalidateModAssets() (line 23721) DestroyIcons g.appIcon on every settings change, and RefreshSkinLabels only re-sets GCLP_HICON when g.main exists. If the window has never been opened, the class keeps a destroyed handle. Easiest fix: leave hIcon/hIconSm out of the WNDCLASSEXW and set the icon on the window with WM_SETICON only.
• Undoc::SHGetUserPicturePath (shell32 ordinal 261) sits at the end of a six-step documented fallback chain, and per your own comment it copies the picture to a temp file that's never deleted. Given how thorough the documented chain already is, dropping this one removes an undocumented-ordinal dependency and a stray temp file for almost no loss.

Functionality notes

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

• Precision wheels won't scroll. WM_MOUSEWHEEL computes const int steps = delta / WHEEL_DELTA; (line 23935) and discards the remainder, so devices that send deltas smaller than 120 (precision touchpads, many high-resolution mice) scroll by zero. Accumulate the residual in a static/App member and consume it in WHEEL_DELTA chunks.
• The intro animation repaints the whole window at 60 fps. kHeaderAnimTimerId fires every 16 ms (lines 21924/21930) for the 2100 ms intro and each 240 ms hover transition, and the handler does InvalidateRect(hwnd, nullptr, FALSE) — a full GDI+ re-render of the header, every task tile and the footer. Since only the header animates, invalidating just L.header would cut that cost dramatically.
• A failed embed decode isn't cached. GetEmbedBitmap (line 20969) only pushes into g.embedBitmaps when the bitmap loads. If LoadPngFromBase64 fails (or a user file is present but corrupt), the ~100 KB base64 blob is re-decoded on every single paint. Caching the nullptr would avoid that.
• The readme has good screenshots for both skins on an allowed host — nice.

---

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.

[Cips35]

/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.

---

A lot of care clearly went into this — the localization, the artwork pipeline and the layout work are impressive. The problems below are mostly about how the mod integrates with Windhawk (where it runs, what it drops on disk, and how it unloads) rather than the recreation itself.

1. Drop @include explorer.exe — this should be a pure tool mod.

The mod already carries the full tool-mod boilerplate and @include windhawk.exe, but it also injects into explorer.exe and hosts the same UI there, with whichever host wins the Windhawk.VistaWelcomeCenter.UI mutex becoming the real instance. That is exactly the arrangement the wiki page you cite argues against (Mods as tools):

• The mod installs no function hooks in Explorer — it needs nothing from injection there. Shell_NotifyIcon, RegisterHotKey, HKCU registry writes, IShellLink and its own top-level window all work from any process.
• Every explorer.exe gets injected, and there can be several (the "launch folder windows in a separate process" option, desktop restarts, etc.). Each one runs Wh_ModInit, reads and PE-validates control.exe, calls PublishPresence() (which writes shortcuts, the .ico and the registry) and spins a UI thread — which then loses the mutex and exits. That is a lot of wasted work per Explorer process, and three processes racing to write the same .lnk/.ico/registry values.
• A fault anywhere in ~24k lines of GDI+/COM UI code takes down the shell. In a dedicated windhawk.exe you can also fall back to ExitProcess on a stuck teardown, which is what explorer-folder-hover-menu.wh.cpp#L3819-L3833 does — in Explorer you have no such escape (see item 4).

The README's justification ("UI is hosted in explorer.exe so the tray icon and Control Panel item actually appear") doesn't hold: mods/mic-tray-control.wh.cpp is @include windhawk.exe only and has a working tray icon with TaskbarCreated re-add handling. Please remove @include explorer.exe, IsExplorerProcess(), g_isExplorerHost and the home-grown Windhawk.VistaWelcomeCenter.UI mutex — the tool-mod launcher's own windhawk-tool-mod_<id> mutex already guarantees a single instance.

2. Remove the embedded prebuilt WelcomeCenter.exe.

kShowExeB64 is a base64-encoded x64 PE image, and EnsureShowLauncher() writes it to %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe, which is then registered as the Control Panel Shell\Open\Command target and as the Start Menu / desktop shortcut target. This isn't acceptable in the catalog:

• Windhawk mods are distributed as source that Windhawk compiles. A compiled binary with no source in the PR cannot be reviewed, audited or rebuilt by anyone — not the maintainer, not a user reading the mod.
• It drops an executable into a user-writable directory and then makes the shell launch it. Anything running as the user can replace it and get executed via the Control Panel item / Start Menu shortcut.
• FindUserAsset(L"launcher") (line ~22949) makes it worse: any WelcomeCenter.exe or launcher.exe a user drops into the mod storage folder, its resources subfolder, or the custom resource folder wins over the embedded copy and gets wired into the Control Panel command. The FileStartsWithMz + IsDangerousFileName checks only confirm it's a PE that isn't named cmd.exe.

If the Start Menu shortcut and Control Panel entry can't be built without a helper executable, they should be dropped — the tray icon and the hotkey cover the same need and require no on-disk binary.

3. The mod is not fully reversible — it leaves files behind when disabled.

WhTool_ModUninit() only calls UnregisterControlPanelItem(). Everything else the mod creates survives a disable/uninstall:

• %LOCALAPPDATA%\WelcomeCenter\WelcomeCenter.exe (item 2)
• %LOCALAPPDATA%\WelcomeCenter\app-vista.ico / app-windows7.ico (EnsureAppIconFile)
• <mod storage>\resources\README.txt (EnsureResourceGuide)
• The Start Menu and desktop .lnk files (EnsureStartMenuShortcut / EnsureDesktopShortcut)

A mod's effects must disappear when it's disabled. Please delete all of these in WhTool_ModUninit — you already have DeleteOwnedWelcomeShortcut() for the links, so it's mostly a matter of calling it for both known folders plus DeleteFileW on the generated files and removing the created directory if empty.

Related: Wh_ModUninit returns early for g_isToolModProcessLauncher, so the process that wrote the shortcuts and registry keys in Wh_ModAfterInit (the main windhawk.exe) cleans up nothing at all. Once item 1 is done and PublishPresence() only runs on the tool process's UI thread, this resolves itself.

4. WhTool_ModUninit lets the mod unload while its UI thread may still be running.

const DWORD wr = WaitForSingleObject(g.uiThread, 4000);
if (wr == WAIT_TIMEOUT) {
    Wh_Log(L"UI thread did not exit in time; continuing unload");
}
CloseHandle(g.uiThread);

When Wh_ModUninit returns, Windhawk unloads the mod with a single FreeLibrary. If that 4-second wait timed out, the UI thread is still executing code in — and has return addresses pointing into — an image that is about to be unmapped, which crashes the host. This is not hypothetical: the UI thread can legitimately be blocked well past 4 s inside ShellExecuteExW (you pass SEE_MASK_NOASYNC | SEE_MASK_FLAG_DDEWAIT, so it is synchronous), inside a modal MessageBox, or inside TrackPopupMenu.

The secondary damage in that path is just as bad: UiAssetsGuard never runs, so the tray icon stays, g.host/g.main are never destroyed, and kHostClass/kMainClass stay registered with an lpfnWndProc pointing into the unmapped image — any message to those leftover windows executes freed memory, and the next load's RegisterClassExW fails with ERROR_CLASS_ALREADY_EXISTS so the UI never comes back.

The wait must not be bounded in a way that lets the unload proceed. In a dedicated tool process the accepted fallback is ExitProcess (see explorer-folder-hover-menu.wh.cpp#L3819-L3833) — which is another reason item 1 matters, because you can't do that inside explorer.exe. The same applies to the WaitForSingleObject(g_uiReadyEvent, 5000) above it: if that times out, g.uiThreadId may still be 0, no WM_QUIT is ever posted, and the thread wait is then guaranteed to time out.

5. Double-clicking "More about this PC" fires the action three times.

The ShowMore hit region is handled in both WM_LBUTTONUP and WM_LBUTTONDBLCLK (lines ~23478–23503). With CS_DBLCLKS set on the class, a double-click delivers WM_LBUTTONDOWN, WM_LBUTTONUP, WM_LBUTTONDBLCLK, WM_LBUTTONUP — so OpenShowMore()/OpenSelected() runs three times and three copies of ms-settings:about (or sysdm.cpl) are launched. Since the on-screen hint explicitly tells users to double-click to open, this is the common path. Handle ShowMore in exactly one of the two messages (WM_LBUTTONUP alone matches the single-click behaviour the rest of the UI uses).

6. The host window exists but pumps no messages for up to 15 seconds at logon.

In UiThread, g.host is created and the tray icon added, and only afterwards does it call WaitForDesktop(15000) and ShowWelcome(true) — the GetMessageW loop starts after that. During that window g.host is a live top-level window that dispatches nothing, so any SendMessage-based broadcast that reaches it (theme/settings changes, etc.) blocks on it until its timeout. Start the message loop first and drive the startup show from a message you post to g.host (or a one-shot SetTimer), so the window is always responsive.

7. The tool-mod boilerplate has been modified in several places.

The convention is to paste the wiki snippet verbatim so it stays maintainable. Current deviations, beyond the Explorer host: the ERROR_ALREADY_EXISTS branch (ExitProcess(0) plus -show forwarding instead of ExitProcess(1)), the added CreateProcessW fallback (the snippet deliberately uses only CreateProcessInternalW), SystemControlPanelExists() gating Wh_ModInit, and LoadSettings()/PublishPresence()/SyncRunAtStartupFromWindhawk() running inside Wh_ModAfterInit / Wh_ModSettingsChanged of the launcher process. Please restore the snippet as-is and keep all mod logic in the WhTool_* callbacks.

8. g_uiReadyEvent and g_uiStopEvent are never closed.

They're created in WhTool_ModInit and left open in WhTool_ModUninit, so two handles leak in the host process on every disable/enable, settings reload and mod update. Close them (and null them) at the end of WhTool_ModUninit.

9. The README has no screenshot.

The whole mod is a visual recreation with two distinct skins, so a screenshot (or GIF) of the Vista appearance and the Windows 7 "Getting Started" appearance would help a lot. i.imgur.com and raw.githubusercontent.com are the allowed image hosts.

Optional improvements

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

• QueryUserName() (line ~1863) does LoadLibraryW(L"secur32.dll"). secur32.dll isn't a KnownDLL, so this uses the default search order, which includes the executable's directory. The blast radius is negligible here (Explorer and Windhawk both run from protected directories), but the hardened form costs nothing: LoadLibraryExW(L"secur32.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32). While you're there, -lsecur32 in @compilerOptions is unused — GetUserNameExW is resolved via GetProcAddress.
• Undoc::SHGetUserPicturePath (line ~1660) falls back to LoadLibraryW(L"shell32.dll") if GetModuleHandleW fails. The mod statically imports shell32 (-lshell32), so the handle is always present and the fallback is dead code.
• LaunchUserTarget has an empty if body with only a comment (lines ~2623–2626) — either implement the check or drop the block.
• PublishPresence() calls CoInitializeEx and only calls CoUninitialize when the result is S_OK. S_FALSE (apartment already initialized) still requires a matching CoUninitialize, so calling it from an already-initialized thread permanently bumps the init count. ComLifetime has the same needUninit_ = (hr == S_OK) pattern.
• RegisterClassExW uses g.instance = GetModuleHandleW(nullptr), i.e. the host executable's HINSTANCE, for classes whose WndProc lives in the mod image. It works because you unregister symmetrically, but the mod's own module handle is the conventional choice (GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, ...) — the UNCHANGED_REFCOUNT flag matters, otherwise you'd pin an extra reference on the mod).
• ApplySettingsOnUiThread() calls InvalidateModAssets() (which DestroyIcons g.appIcon) well before RefreshSkinLabels() re-publishes the new icon to GCLP_HICON/WM_SETICON. In between, AddOrUpdateTray() → Shell_NotifyIconW does a cross-process send and pumps messages, so a frame repaint in that window can use the destroyed handle. Refreshing the window/class icons before the tray update would close the gap.
• SetFullScreen(hwnd, true) detaches the menu with SetMenu(hwnd, nullptr) while g.mainMenu still owns it. If the mod is unloaded while full screen, DestroyWindow(g.main) won't free the detached menu and UiAssetsGuard just sets g.mainMenu = nullptr — a small HMENU leak. An explicit DestroyMenu when the menu isn't attached would cover it.
• SystemControlPanelExists() opens and PE-validates %SystemRoot%\System32\control.exe in Wh_ModInit of every injected process, and refuses to load the mod entirely if it fails. The main window doesn't need control.exe — only the optional classic-CPL fallbacks do — so gating the whole mod on it (and paying the file I/O at every process start) seems heavier than the problem warrants.
• Wh_ModInit's duplicate-instance branch and UiThread's duplicate branch both PostMessageW(host, WM_SHOW_WELCOME, ...) cross-process, where WM_SHOW_WELCOME is WM_APP + 3. You already have RegisteredShowMessage() (RegisterWindowMessageW) for exactly this purpose — using it consistently for anything crossing a process boundary is safer and self-documenting.
• IsDangerousFileName() is a blocklist of ~18 executable names. It reads like a security boundary but isn't one (a renamed copy of cmd.exe, wusa.exe, ftp.exe, … all pass). Since the targets come from the user's own settings, that's fine — but it may be worth a comment saying so, rather than implying it's a sandbox.

Functionality notes

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

• Two sources of truth for "Run at startup". The Windhawk runAtStartup setting and the in-window checkbox are reconciled through three Wh_*Value keys (runAtStartup.override.v2, runAtStartup.settingSnapshot.v2) plus SyncRunAtStartupFromWindhawk() on every settings change, in every process. It works, but it's a lot of machinery, and the snapshot/override dance is hard to reason about when several processes write it concurrently. Making the checkbox a live view of the Windhawk setting (write it via Wh_SetIntValue in one place, or drop the persistence and let the checkbox only affect the current session) would remove most of it.
• LaunchShell blocks the UI thread. SEE_MASK_NOASYNC | SEE_MASK_FLAG_DDEWAIT makes ShellExecuteExW synchronous, and activating an ms-settings: URI or a .cpl can take a second or more on a cold system — during which the window is frozen. Since the launch happens from a message handler on a thread you own, dropping SEE_MASK_NOASYNC (the thread isn't about to exit) would keep the UI responsive.
• Hotkey parsing ignores the shift state from VkKeyScanW. ParseHotkey takes LOBYTE(scan) and discards the high byte, so a single-character key that requires Shift/AltGr on the current layout will register a different physical combination than the string suggests. Also, +, - and space are all treated as separators, so a key literally named - can't be expressed.
• File size. At ~24.5k lines / 2 MB, the vast majority of which is base64 artwork, this is by far the largest mod in the catalog. Compile time and repo weight are real costs. Worth considering whether every embedded tile is necessary, or whether some could fall back to SHGetStockIconInfo (which you already support) instead of shipping a 256px PNG.
• Cosmetic "Windows version text" options. Offering Windows 95/98/XP labels for the PC-information line is harmless, but they're pure fiction on a Windows 10/11 box and users may report them as bugs. A shorter list (or a free-text field) might age better.

---

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.