Add Taskbar collapse to running apps mod

[LarsGudm]

Adds a new mod: taskbar-collapse-to-running

Hides pinned taskbar icons whose app isn't running, so a taskbar full of pinned
shortcuts collapses down to just what's in use. Icons are only hidden and
unhidden in place, so pinned order is never changed.

Revealing is configurable: left-click the empty part of the taskbar (default),
hover, hover-once-the-cursor-slows ("rest"), a hotkey, or while the Start menu
is open. Windows' own icon reorder slide is stripped and replaced with an
optional spacing-only animation, including an eased gap-close when a running
app exits.

Tested on Windows 11 build 26200, left-aligned taskbar, single monitor.

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.

[LarsGudm]

/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, self-contained mod — the taskbar hooks are narrow, the settings all map to code, and the reveal/animation state machine is well commented. The findings below are mostly around unload safety and per-tick cost.

1. StopHotkeyThread can let the mod unload while the hotkey thread is still alive.

Two problems combine here:

• PostThreadMessage(g_hotkeyThreadId, WM_QUIT, ...) fails with ERROR_INVALID_THREAD_ID if the target thread has not created a message queue yet. HotkeyThreadProc only reaches GetMessage after winrt::init_apartment, an optional RegisterHotKey, and a CoCreateInstance(AppVisibility) that can take a while (it faults in the shell's UI stack). When the hotkey is empty and only RevealOnStart is on, nothing before GetMessage creates the queue at all, so WM_QUIT is silently dropped and the thread blocks forever.
• WaitForSingleObject(g_hotkeyThread, 2000) then gives up after 2 s, closes the handle and returns success. Windhawk unloads the mod right after Wh_ModBeforeUninit, so the thread is left blocked inside GetMessage with its return address pointing into an unmapped image — plus a still-registered system hotkey and a leaked IAppVisibility advise. Via Wh_ModSettingsChanged the same path leaks a second thread on every settings change once the first one is stuck.

A mod must be fully unloadable the moment its teardown returns, so the wait has to be unconditional. Fix both: create a ready event, force the queue with PeekMessage in the thread, and wait INFINITE. charging-sound.wh.cpp#L162-L165 and #L250-L258 are exactly this pattern:

// In HotkeyThreadProc, before anything slow:
MSG msg;
PeekMessageW(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE);
SetEvent(g_readyEvent);
...
// In StopHotkeyThread:
WaitForSingleObject(g_readyEvent, INFINITE);
PostThreadMessage(g_hotkeyThreadId, WM_QUIT, 0, 0);
WaitForSingleObject(g_hotkeyThread, INFINITE);

2. g_frames runs its destructor at process shutdown and releases XAML objects from the wrong thread.

std::unordered_map<void*, FrameContext> g_frames; (line 341) is a global whose elements hold strong, UI-thread-affine references: DispatcherTimer timer, CoreDispatcher dispatcher, std::vector<FrameworkElement> animButtons, and DeanimatedButton::originalTransitions / originalImplicit. Wh_ModBeforeUninit does not run when explorer.exe itself terminates (Explorer restart, sign-out, reboot) — the CRT still runs the global's destructor on the shutdown thread, after every other thread has been killed and the XAML core is gone. That is the "XAML and other UI-thread objects" / "containers of thread-affine elements" case from https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown (see #4 and #5).

Wrap it and keep releasing explicitly on the owning thread:

[[clang::no_destroy]] std::optional<std::unordered_map<void*, FrameContext>>
    g_frames{std::in_place};

Accesses become g_frames->find(key) etc.; the per-thread erase in CleanupOnThisThread stays as the real release (it already runs on the correct UI thread).

3. The leftover sweep in Wh_ModBeforeUninit destroys contexts without stopping their timer or rendering hook.

The final g_frames.clear() (line 2358) runs on Windhawk's thread. Any context that the two sweeps failed to reach still has a running DispatcherTimer whose Tick handler is OnTimerTick, and possibly a live CompositionTarget::Rendering handler — clearing the map removes the entry but not the registrations, so both keep calling into the mod image after it is unloaded. It also releases the XAML refs cross-thread (same issue as item 2).

FrameContext::dispatcher is already stored for exactly this kind of thing — use it as the fallback instead of clearing blind, e.g. dispatcher.RunAsync(...) that runs CleanupOnThisThread() and signals an event, then wait on the event with a timeout. If a context still cannot be cleaned, leaving the references alive is safer than dropping them (per the wiki page above), but the timer and rendering handler must be removed either way.

4. 20 Hz visual-tree polling on Explorer's UI thread is the default, and it rarely sleeps.

RefreshIntervalMs defaults to 50 ms, and each tick does a fair amount of work per taskbar: CollectTaskListButtons walks the repeater, then for every button FindRunningIndicator does a depth-6 recursive walk (with a std::function copy and an HSTRING Name() read per node — and no early exit for the common "pinned, not running" case), IsButtonRunning additionally allocates the automation name, and DeanimateButton issues several XAML/Composition property calls. That is hundreds of XAML interop calls, 20 times a second, forever.

The sleep mode doesn't help as much as it looks:

• TaskbarFrame_MeasureOverride_Hook resets g_lastActivityTick on every layout pass of the taskbar frame (line 1763), so any tray/clock/badge-driven remeasure inside the 2-minute window keeps it awake.
• With RevealTrigger: click, a reveal stays until it is clicked away, and !g_revealed is a precondition for sleeping (line 1226) — so a user who reveals and walks away polls at 50 ms indefinitely.

"Is this app running" changes at human speed; 250–500 ms would be plenty as a default (the grace-period countdown is the only thing that wants a fast tick, and that only matters while revealed). Suggest raising the default, and consider keeping the fast rate only while g_revealed || animActive || g_emptyHoverSinceTick != 0, plus caching the resolved RunningIndicator element per button instead of re-walking the subtree every tick.

5. ClearImplicitShowHide is applied to every button on every tick, and it cannot be undone.

DeanimateButton ends with ClearImplicitShowHide(button) (line 726), so SetImplicitShowAnimation/SetImplicitHideAnimation get nulled for all task buttons — including running ones the mod never hides — and, as your own comment notes, there is no getter, so ReanimateButtons can't restore them. Disabling the mod therefore leaves the taskbar permanently without those animations until Explorer restarts, which conflicts with Windhawk's "effects disappear when the mod is disabled" principle.

SetCollapseState already calls ClearImplicitShowHide right before it hides a button (line 840), which is the only place it's actually needed. Dropping the call from DeanimateButton limits the irreversible part to buttons the mod genuinely hides and removes two attached-property writes per button per tick. Whatever remains, please note the limitation in the README.

6. Remove the custom file logger.

DebugFileLog (lines 367-402) wraps Wh_Log and additionally writes to %TEMP%\taskbar-collapse-debug.log. Windhawk already provides per-mod logging with an enable/disable switch and mod-name prefixing, so the convention is to call Wh_Log directly and not to write log files — and the README's "flip kDebugLogging in the source and recompile" instruction isn't something users can act on. Please drop DebugFileLog, kDebugLogging, g_debugFileMutex, g_debugLinesWritten, the %TEMP% file writing and the README diagnostics bullet, and use Wh_Log at the handful of places that matter. <cstdarg>, <cstdio> and the _wfopen/_vsnwprintf/wcscat_s uses go away with it.

7. Add a screenshot or GIF to the README.

The whole point of the mod is a visible taskbar change with an animation, and there's currently no image. A short GIF of the collapse/reveal would help a lot on windhawk.net. Allowed hosts are i.imgur.com and raw.githubusercontent.com.

Optional improvements

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

• @compilerOptions (line 10) lists -loleaut32 and -lshcore, but I don't see anything in the mod that needs either. -luser32 is also linked by default.
• Wh_GetStringSetting never returns NULL — it returns L"" when unset or on error — so the if (trigger) / if (mode) / if (curve) / if (marker) / if (spec) checks in LoadSettings and StartHotkeyThread are dead. WindhawkUtils::StringSetting (RAII) is also nicer than the raw get + Wh_FreeStringSetting pairs:

  WindhawkUtils::StringSetting trigger = WindhawkUtils::StringSetting::make(L"RevealTrigger");
  if (wcscmp(trigger.get(), L"rest") == 0) { ... }

• GetModuleHandle(L"kernelbase.dll") at line 2292 isn't checked before GetProcAddress. It's always loaded in practice, but a nullptr there would silently install a hook on nullptr.
• sqrt is used at line 1252 without #include <cmath>; it currently comes in transitively.
• EnumChildElements takes std::function<bool(FrameworkElement)> by value (line 408), so every recursion level copies the functor. A const& parameter, or a template parameter, avoids that on a path that runs per node per tick.
• The RefreshIntervalMs description says it is "also the worst-case delay on the hotkey", but ToggleCollapse calls WakeAllFramesAsync(), so the hotkey is immediate now.
• g_settings' scalar fields are written by LoadSettings on Windhawk's thread and read from the taskbar UI thread without synchronization. Harmless for the ints, but animationCurve is four doubles — a settings change mid-animation can be read half-updated for a frame.
• TaskbarFrame_MeasureOverride_Hook does a QueryInterface plus a g_framesMutex lock on every layout pass even after the frame is registered. Cheap, but it's the layout hot path; a g_frames-is-non-empty fast path would skip most of it.

Functionality notes

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

• frame.UpdateLayout() is called from inside the CompositionTarget::Rendering handler (line 1134) and, indirectly via ApplyOnThisThreadNow, from inside the pointer-event hooks. Forcing a synchronous layout pass from the rendering callback is the classic way to get a layout/render feedback loop in XAML; it seems to work here because the state settles, but it's worth keeping an eye on if you ever see the taskbar pegging a core mid-animation.
• ElementPaddingPx is used in two different coordinate systems: DIPs in IsPointerClearOfButtons/ProbeStripIsClear (via TransformToVisual), and raw screen pixels in the WindowFromPoint neighbour test at lines 1578-1587. On a 150% display the second probe covers ~2/3 of the intended distance, so the "different window nearby" guard is weaker than the visual one. Scaling the screen-space offsets by the monitor DPI would make them agree.
• ProbeStripIsClear runs three FindElementsInHostCoordinates hit tests per qualifying pointer move. It's on the input path, so if hover feels sticky on a busy taskbar that's the first thing I'd measure.
• The reveal state is process-global, so hovering the primary taskbar also reveals the secondary ones. Probably what you want, but worth a line in the README since the mod is otherwise per-taskbar.
• The hotkey toggles g_collapseEnabled without persisting it, so the Collapsed checkbox in the settings UI goes stale, and any unrelated settings change snaps the state back to the checkbox. Wh_SetIntValue could persist it if you want the hotkey state to survive.
• The click trigger hooks TaskbarFrame::OnPointerReleased on empty taskbar space, which is the same real estate Click on empty taskbar space uses. Both call the original and don't swallow the event, so they should coexist, but users running both will get two actions from one click — maybe worth a README note.

---

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.

[LarsGudm]

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

---

Round 2. All seven items from the previous review are genuinely fixed — the hotkey thread now has a ready event and unconditional waits, g_frames is [[clang::no_destroy]] std::optional<...>, the leftover sweep goes through each context's own dispatcher, the default tick is 250 ms with a cached running indicator, ClearImplicitShowHide is confined to buttons actually being hidden, the file logger is gone, and the README has a GIF. The findings below are new ones, mostly from reading the parts the first pass didn't reach.

1. The hotkey thread and the Start-menu watcher start in every explorer.exe process, not just the one that owns the taskbar.

Wh_ModInit calls StartHotkeyThread() (line 2430) unconditionally, before anything has established that this process hosts a taskbar. @include explorer.exe matches every explorer.exe, and there is regularly more than one — most obviously with Launch folder windows in a separate process enabled, but also for transient shell-spawned instances.

The consequence is not just wasted work:

• RegisterHotKey fails if the combination is already registered by another thread or process. Whichever explorer.exe starts first wins. If that is a non-shell instance, the shell process's registration fails (silently — it is only Wh_Logged at line 2055) and the hotkey stops working, because ToggleCollapse only flips g_collapseEnabled in the process that received WM_HOTKEY, and that process has no entries in g_frames.
• Every such process also gets an extra thread, an MTA apartment, a CoCreateInstance(AppVisibility) and a live Advise sink, for a taskbar it will never touch.

Gate the thread on the taskbar actually being present. The cheapest hook is the flag you already maintain: start it where g_taskbarViewDllLoaded becomes true (the Wh_ModInit found-branch, the Wh_ModAfterInit retry, and LoadLibraryExW_Hook), and in Wh_ModSettingsChanged only restart it if it was running:

// Wh_ModInit: replace the unconditional StartHotkeyThread() with a call from
// each place that sets g_taskbarViewDllLoaded.
void Wh_ModSettingsChanged() {
    bool hadHotkeyThread = g_hotkeyThread != nullptr;
    StopHotkeyThread();
    LoadSettings();
    if (hadHotkeyThread || g_taskbarViewDllLoaded) {
        StartHotkeyThread();
    }
    g_instantApplyGen++;
}

If you'd rather key on the window than the DLL, FindCurrentProcessTaskbarWnd is the usual idiom — but note it can't be used from Wh_ModInit when the mod loads before Explorer creates its windows, which is why the DLL flag is the easier gate here.

2. Unload still has two five-second give-up paths that let the DLL unmap with mod code queued.

This is the same class of defect as the hotkey-thread wait from the last round, now in the dispatcher paths:

• Line 2527: if (!queued || WaitForSingleObject(done, 5000) == WAIT_OBJECT_0). On timeout the code deliberately leaks the event handle and moves on — but the RunAsync lambda is still sitting in that dispatcher's queue. When it eventually runs it executes CleanupOnThisThread(); SetEvent(done); inside an image that has already been FreeLibrary'd.
• Line 2535: for (int i = 0; i < 500 && g_pendingWakes.load() > 0; i++) Sleep(10); — after 5 s it proceeds with wakes still queued, same outcome.

Both waits can be unconditional, and by the time you reach them you have already earned that: StopHotkeyThread() returned, so no new wakes can be queued (WakeAllFramesAsync also early-returns on g_unloading), and the cleanup sweep just proved each taskbar thread is pumping. A thread that has stopped pumping entirely will hang the unload — but hanging is recoverable and unmapping under a live callback is not, which is the trade Windhawk's contract asks for.

if (!queued || WaitForSingleObject(done, INFINITE) == WAIT_OBJECT_0) {
    CloseHandle(done);
}
...
while (g_pendingWakes.load() > 0) {
    Sleep(10);
}

One residual note while you're in there: g_pendingWakes-- runs inside the lambda body (line 1446), so the count can reach zero while the dispatcher is still unwinding through the delegate's invoke thunk and destructor — both of which live in the mod image. It's a handful of instructions wide, but if you want it airtight, the structural fix is to stop queuing mod code at all: OnTimerTick already polls g_instantApplyGen, so WakeAllFramesAsync is purely a latency optimisation over the 250 ms tick.

3. Windows' task-button transitions stay stripped while the collapse feature is switched off.

ApplyToFrame deanimates every button on every tick regardless of state (lines 1046-1050):

if (!g_unloading) {
    for (auto& button : buttons) {
        DeanimateButton(ctx, button);
    }
}

desired is false both when the user unchecks Collapse taskbar and when the hotkey toggles collapse off, so in the "mod is on but doing nothing" state the taskbar still loses its reorder slide and implicit animations on every button. That's a visible change with the feature disabled, which is the opposite of what the checkbox implies. Restoring instead costs one line, and ReanimateButtons is a no-op once the ledger is empty:

if (g_unloading) {
    // leave as-is
} else if (!g_collapseEnabled && ctx.hiddenByUs.empty()) {
    ReanimateButtons(ctx);
} else {
    for (auto& button : buttons) {
        DeanimateButton(ctx, button);
    }
}

(Keep deanimating while g_collapseEnabled is true and the bar is merely revealed — a collapse can follow at any moment.)

Optional improvements

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

• HandleFrameMeasure's thread_local void* knownThis (line 1827) assumes one taskbar per thread, and the comment at lines 1822-1826 states that explicitly ("each taskbar measures only on its own UI thread, so it stays hot with any number of monitors"). In Explorer, the primary Shell_TrayWnd and every secondary Shell_SecondaryTrayWnd share one UI thread — which is exactly why your own RunOnAllTaskbarThreads dedupes by thread id. So on a multi-monitor setup the single slot thrashes between frames and every layout pass falls through to the QueryInterface + g_framesMutex lock. A thread_local two- or four-entry array, or a thread_local std::vector<void*>, restores the fast path. Worth fixing the comment either way.
• g_frames is never reset(). The per-context release in CleanupOnThisThread is the right thing and runs on the owning thread, but the unordered_map itself keeps its bucket array, which leaks on every enable/disable cycle. Once the sweeps have emptied it you can destroy it safely — the wiki calls this out under containers of resource-owning elements:

  std::lock_guard<std::mutex> guard(g_framesMutex);
  if (g_frames->empty()) {
      g_frames.reset();
  } else {
      Wh_Log(L"%u taskbar contexts could not be cleaned up", (unsigned)g_frames->size());
  }

• The scalar fields of g_settings (lines 233-249) are written by LoadSettings on Windhawk's thread and read from the taskbar UI thread with no synchronisation. animationCurve is now an enum, which removes the ugly case, but the remaining plain int/bool/enum reads are still formally a data race. std::atomic<int> on the handful that are read from the tick would close it at no cost.
• RunOnAllTaskbarThreads uses FindWindow(L"Shell_TrayWnd", nullptr) (line 2221), which returns the first match on the desktop regardless of owner; if it belongs to another process, consider() drops it and the primary taskbar is never visited directly. The leftover sweep does catch it, so nothing breaks — but the EnumWindows-with-process-filter form linked above is the convention and avoids relying on the fallback.
• Wh_Log at line 2372 reports OnPointerMoved, OnPointerReleased and MeasureOverride but not OnPointerExited, which is also optional.

Functionality notes

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

• frame.UpdateLayout() is still called from inside the CompositionTarget::Rendering handler (lines 962 and 1142). Forcing a synchronous layout pass from the rendering callback is the classic way to get a layout/render feedback loop in XAML; it evidently settles here, but it's the first thing to look at if you ever see the taskbar pegging a core mid-animation.
• Running detection rests on the element name RunningIndicator (with a Running-substring fallback) plus an English-by-default automation-name marker. Both are per-build/per-locale, and the failure mode is severe in a quiet way: a running app's button gets hidden and the user can't reach the window. A cheap guard is a per-pass sanity check — if no button on the taskbar resolved an indicator and the marker matched nothing either, treat detection as broken for that pass and hide nothing. That turns a "my apps vanished" report into "the mod does nothing on this build".
• The reveal path is mouse-only: PointerQualifiesForReveal (line 1589) and CursorOverAnyTaskbar both go through GetCursorPos/WindowFromPoint, which don't track a pen or touch contact. With a touch-first setup the hover/rest triggers won't arm, and the grace period can age out under a finger that's still on the bar. Using args.GetCurrentPoint(nullptr) for the position (you already have the args) would cover the qualification side; the tick-side aging is harder and may just be worth a README line.
• ProbeStripIsClear (line 1516) runs three FindElementsInHostCoordinates hit tests per qualifying pointer move, on the input path. Only reachable with the hover/rest triggers, so the default click setup never pays it — but if hover ever feels sticky on a busy taskbar, that's the first thing to measure.
• DeanimateButton does a GetElementVisual + Transitions() + ImplicitAnimations() read for every button on every tick, plus a linear scan of ctx.deanimated. At 250 ms with a dozen icons that's fine; the comment explains why strip-once isn't safe. If you ever want it cheaper, the ledger entry could cache "we've seen this button's animations settle" and re-check at a slower cadence than the running-state poll.

---

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.

[LarsGudm]

'nother round. Following things from last rounds review were not addressed on purpose. Heres what claude wrote (and where i interjected a little);

Two things I (Claude) did NOT do, on purpose
The "detection broken → hide nothing" guard (functionality note 2): the suggested heuristic is wrong as stated. A taskbar where zero apps are running also has zero resolvable indicators and zero marker matches — that's the legitimate all-idle state, where the correct action is to hide everything. Might change it later.

Touch/README note: showing you the line before writing it, per your rules. Proposed addition under Notes: * Reveal triggers follow the mouse pointer; touch and pen input are not tracked — it's a feature, not a fix. Also save for later if the need should arise!

[LarsGudm]

/ready-for-reviewer

[windhawk-reviewer]

@LarsGudm /ready-for-reviewer can't be applied here: the most recent AI review covers 3fa744b, but the current head of this pull request is f4c7621. Comment /ai-review to get a review of the current code.

[LarsGudm]

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

---

Round 3. The previous round's items are addressed: the hotkey thread is now gated on g_taskbarViewDllLoaded so non-taskbar explorer.exe instances stay out of it, and the ready-event + unconditional-wait teardown is correct. The DPI handling in the padding probe, the [[clang::no_destroy]] std::optional on g_frames, and the per-thread cleanup sweeps are all done right, and I found no existing mod that overlaps this one. Two items left.

1. The teardown path can escape as an exception, and both unbounded waits in Wh_ModBeforeUninit can then hang forever.

CleanupOnThisThread (line 2260) is the one mod entry point invoked from a foreign dispatch context with no catch-all, and the XAML calls it makes are unguarded. RestoreFrame (line 1193) → ClearSpacing (Translation), button.Visibility(...), and ReanimateButtons → button.Transitions(entry.originalTransitions) can all throw winrt::hresult_error — the Transitions reassignment especially, since re-parenting a DependencyObjectCollection that XAML considers owned elsewhere fails rather than no-ops. Everywhere else in the file this is guarded (ClearImplicitShowHide, FinalizePendingHides, MeasureGapOffsets, SortButtonsByPosition, the four hook wrappers), so this looks like an oversight rather than a decision. Two consequences:

• Via RunFromWindowThread, the throw unwinds out of the WH_CALLWNDPROC hook proc into user32 inside SendMessage on a taskbar thread — the exact thing the hook wrappers exist to prevent.
• Via the straggler loop (line 2542), the throw skips SetEvent(done). C++/WinRT's delegate wrapper swallows the exception and RunAsync just completes with a failure HRESULT, so WaitForSingleObject(done, INFINITE) at line 2561 never returns and the unload hangs permanently.

Separately, that same loop is reached precisely when a context's thread has no windows at all (EnumThreadWindows found none at line 2515) — the signature of a thread that has already exited. A CoreDispatcher whose thread is gone never runs the queued lambda, so the INFINITE wait hangs for that reason too, and while (g_pendingWakes.load() > 0) Sleep(10); (line 2570) has the same failure mode for a wake queued just before its thread died. You already record ctx.threadId, so this is cheap to rule out.

// CleanupOnThisThread: make the per-frame work exception-safe.
for (void* key : keys) {
    auto it = g_frames->find(key);
    if (it == g_frames->end()) continue;
    try {
        if (it->second.timer) {
            it->second.timer.Stop();
            it->second.timer.Tick(it->second.tickToken);
        }
        RestoreFrame(it->second);
    } catch (winrt::hresult_error const&) {
    }
    g_frames->erase(it);   // must still run, or the straggler sweep loops on it
}

// Straggler lambda: signal unconditionally.
dispatcher.RunAsync(High, [done]() {
    try { CleanupOnThisThread(); } catch (...) {}
    SetEvent(done);
});

// And skip the wait for a thread that is already gone:
if (HANDLE hThread = OpenThread(SYNCHRONIZE, FALSE, threadId)) {
    bool exited = WaitForSingleObject(hThread, 0) == WAIT_OBJECT_0;
    CloseHandle(hThread);
    if (exited) continue;  // nothing on that thread can ever run our lambda
}

2. Every pointer-move event over the taskbar runs three full XAML hit-tests plus four screen-space window lookups.

With RevealTrigger set to hover or rest, HandlePointerMoved calls PointerQualifiesForReveal (line 1570) on every OnPointerMoved while the bar is collapsed. That does, per event:

• IsPointerOverEmptySpace — up to 16 winrt::get_class_name calls (one HSTRING allocation each);
• IsPointerClearOfButtons — one TransformToVisual().TransformPoint() per pinned button;
• GetCursorPos + WindowFromPoint ×3 + GetDpiForWindow + GetWindowRect;
• ProbeStripIsClear (line 1526) — three FindElementsInHostCoordinates calls, each a full hit-test walk of the taskbar visual tree, plus a get_class_name for every returned hit.

Pointer moves arrive at 100–1000 Hz, so with ~15 pinned apps this is a few hundred tree hit-tests and a couple of thousand HSTRING allocations per second on the taskbar UI thread — enough to show up as taskbar input lag and dropped frames, which is the one thread where that is most visible. (The default click trigger is unaffected: there it only runs on release.)

The expensive half only decides whether the reveal is allowed to fire, not whether the dwell should be armed, so it can be deferred. Keep the cheap checks (IsPointerOverEmptySpace + IsPointerClearOfButtons) on the per-move path to arm/clear g_emptyHoverSinceTick, and run the WindowFromPoint probes and ProbeStripIsClear once, right before the flip:

ULONGLONG now = GetTickCount64();
ULONGLONG since = g_emptyHoverSinceTick;
if (since == 0) { g_emptyHoverSinceTick = now; return; }
if (now - since < (ULONGLONG)g_settings.revealDelayMs) return;

if (!PointerClearsNeighbouringSurfaces(key, args)) {   // the expensive half
    g_emptyHoverSinceTick = 0;
    return;
}
g_revealed = true;

OnTimerTick's dwell-completion branch (line 1319) needs the same treatment. A cheaper alternative if you'd rather not split the function: skip re-qualification entirely when the pointer has moved less than a few DIPs since the last qualified sample.

Optional improvements

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

• g_frames.reset() at line 2579 opens a small use-after-free window. Hooks are only removed after Wh_ModBeforeUninit returns, and IsFrameRegistered / GetFrameContext / WakeAllFramesAsync / CursorOverAnyTaskbar all dereference *g_frames after only an unlocked g_unloading check — a thread preempted just past that check resumes into a disengaged optional. The map is empty by then, so the reset only reclaims a bucket array; dropping the call removes the hazard for nothing.
• The hotkey-thread globals are unsynchronized across two callers. g_hotkeyThread / g_hotkeyThreadId / g_hotkeyThreadReady are written by Wh_ModSettingsChanged (Windhawk's thread) and by LoadLibraryExW_Hook → StartHotkeyThread (an arbitrary thread). If the taskbar DLL loads while a settings save is in flight you can end up with two threads or a StopHotkeyThread waiting on a replaced ready event — worst case an orphaned thread that outlives the unload. A small std::mutex around start/stop closes it.
• knownFrames (line 1837) caches raw pThis values that are never invalidated. If a TaskbarFrame is destroyed and a new one is allocated at the same address, MeasureOverride permanently stops registering it on that thread; recovery depends on a pointer move reaching HandlePointerMoved. Clearing the cache when a context is erased (or dropping it — IsFrameRegistered is a hash lookup under an uncontended mutex) avoids the stale-pointer semantics.
• g_speedSamplePrevMs / g_speedSamplePrevPos (lines 290-291) are plain non-atomic globals written from OnTimerTick. In practice all taskbars share one UI thread so there's no race, but the comment at line 1264 ("a second taskbar's tick in the same instant") suggests you don't rely on that — if so they should be atomic or moved into FrameContext.
• README says "Licensed MIT" but there's no @license field. Adding // @license MIT puts it in the mod catalog metadata rather than only in prose.
• LoadSettings falls back to RevealTrigger::Hover (line 2297) for an unrecognized value, while the documented and shipped default is click. Making the fallback Click keeps the two in sync.
• g_settings.collapsed is only ever read once, to seed g_collapseEnabled (line 2346) — it can be a local in LoadSettings instead of an atomic member.
• (*g_frames)[key] = ctx; (line 1389) default-constructs then copy-assigns a FrameContext with several WinRT refs and three vectors; g_frames->insert_or_assign(key, std::move(ctx)) avoids the copy.

Functionality notes

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

• Rest-speed is sampled at tick cadence, not from the pointer stream. g_cursorSpeedPxS is only updated inside OnTimerTick (line 1265), so with the default RefreshIntervalMs: 250 it is a 250 ms average that HandlePointerMoved then reads up to 250 ms stale. Two visible effects: a cursor that sweeps and then parks keeps reporting a high speed for up to a quarter second (delaying the reveal), and a cursor moving in a small circle with near-zero net displacement over the sample window reads as resting and reveals. PointerRoutedEventArgs already carries position and timestamp — deriving the speed from consecutive pointer events instead would be both cheaper and more accurate.
• A stalled CompositionTarget::Rendering can freeze a gap-close mid-flight. Rendering stops firing when the taskbar isn't being composed (auto-hide, occlusion by a fullscreen exclusive app, display off). If a gap close is at t < 0.5 when that happens, its pendingHide buttons sit at Opacity(0) while still occupying layout space, and OnTimerTick won't reconcile because animActive && desired == animTargetCollapse returns early (line 1341). It self-heals when rendering resumes — t will be ≥ 1 and StopAnimation finalizes — but until then the bar shows blank gaps. A watchdog in OnTimerTick covers it: if (ctx->animActive && NowMs() - ctx->animStartMs > duration * 3) StopAnimation(*ctx);.
• OnPointerReleased is registered as an optional symbol (line 2374) while click is the default trigger, so on a build where that symbol doesn't resolve the mod loads fine and the default reveal path silently does nothing. Worth at least an explicit Wh_Log warning distinguishing "hook missing" from "nothing to reveal" — the current combined log line at line 2396 makes it easy to miss.

---

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.

[LarsGudm]

/ai-review

[LarsGudm]

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

---

Round 4. The previous round's items are addressed: the per-move path is cheap again for the default click trigger, the expensive probes only run at the flip, the animation watchdog is in, LoadSettings falls back to Click, and the straggler sweep now checks the target thread before waiting. I also checked the catalog again — nothing existing hides pinned-but-idle taskbar buttons, so this isn't a duplicate. Three items, and the first two are really one theme: Taskbar.View.dll already exposes the state you're reverse-engineering.

1. Running detection should query the taskbar's own IsRunning, not a tree walk plus an English text marker.

IsButtonRunning (line 518) ORs two heuristics: a 6-level walk for a RunningIndicator child with a "any name containing Running" fallback (lines 422-450), and a substring match of the button's accessibility name against RunningNameMarker, which ships as the English string running window (line 135). Three problems:

• On any non-English Windows the name signal silently stops working. The $description tells the user to translate it themselves, but a mod's default behavior should be correct in every locale, not just when the user finds and edits a setting.
• The indicator resolved by FindRunningIndicator is cached in DeanimatedButton::runningIndicator (line 323, stored at line 767) and never invalidated. The taskbar's repeater recycles button containers; if a cached indicator element gets detached it can still report Visibility::Visible, Opacity > 0.01 and a non-zero ActualWidth from its last layout, so an idle app reads as running and never collapses.
• IndicatorSaysRunning also has to special-case its own hiding (the buttonLaidOut guard at line 506) because a collapsed button measures zero — a signal that the check is inferring state from geometry it is itself changing.

Taskbar.View.dll exports the property directly. taskbar-labels resolves it without hooking it (a SYMBOL_HOOK with a null hook function only retrieves the address) and calls it on the TaskListButton element — the exact element type CollectTaskListButtons produces — to make the same pinned-vs-running distinction you need (helper, call site):

using TaskListButton_get_IsRunning_t = HRESULT(WINAPI*)(void* pThis, bool* running);
TaskListButton_get_IsRunning_t TaskListButton_get_IsRunning_Original;

bool TaskListButton_IsRunning(FrameworkElement taskListButtonElement) {
    bool isRunning = false;
    TaskListButton_get_IsRunning_Original(
        winrt::get_abi(
            taskListButtonElement.as<winrt::Windows::Foundation::IUnknown>()),
        &isRunning);
    return isRunning;
}

// in the symbol hook array:
{
    {LR"(public: virtual int __cdecl winrt::impl::produce<struct winrt::Taskbar::implementation::TaskListButton,struct winrt::Taskbar::ITaskListButton>::get_IsRunning(bool *))"},
    &TaskListButton_get_IsRunning_Original,
},

That removes FindRunningIndicatorWalk, IndicatorSaysRunning, ContainsNoCase, the cached runningIndicator, the RunningNameMarker setting, g_settingsMutex / CopyRunningMarker, and the "Running detection" README section — and it is exact and locale-independent.

2. The permanent 250 ms / 1000 ms poll can be event-driven, and the poll rate is user-visible.

Every registered frame runs a DispatcherTimer forever (line 1335). Each tick calls ApplyToFrame, which re-walks the tree with CollectTaskListButtons, runs IsButtonRunning per button, and calls DeanimateButton for every button on every pass (line 1024) — a GetElementVisual + Transitions() + ImplicitAnimations() round trip each. That never stops; SleepEnabled only drops the rate from 250 ms to 1000 ms after two idle minutes.

The rate is also a correctness-visible number, not just a cost. The mod is what sets Visibility::Collapsed on a pinned button, so nothing but the mod's own tick can un-hide it. Launch a pinned app from the Start menu or a desktop shortcut — neither touches g_lastActivityTick — and after two idle minutes its taskbar button stays missing for up to a full second before the mod notices. That reads as the taskbar being broken.

Taskbar.View.dll raises this for you: TaskListButton::UpdateVisualStates is called when a button's visual state changes (running / active / indicator states), and it's the standard hook for exactly this — 15 mods in the catalog already use it, e.g. taskbar-labels and taskbar-on-top:

{
    {LR"(private: void __cdecl winrt::Taskbar::implementation::TaskListButton::UpdateVisualStates(void))"},
    &TaskListButton_UpdateVisualStates_Original,
    TaskListButton_UpdateVisualStates_Hook,
},

Apply from there (plus the pointer hooks you already have), and keep the DispatcherTimer only for the things that genuinely need a clock — the hover grace period, the dwell delay and the animation watchdog — starting it when one of those is armed and stopping it when timingSomething goes false. That makes app launches instant, removes the SleepEnabled / SleepIntervalMs / RefreshIntervalMs settings and the whole sleep/wake machinery, and leaves Explorer doing nothing at all while the taskbar is idle.

3. Two unbounded waits in Wh_ModBeforeUninit can hang Explorer permanently.

Both fallback paths block with INFINITE on something that may never be signalled:

• The straggler loop (lines 2538-2575) checks that the target thread is alive, closes the handle, then waits: WaitForSingleObject(done, INFINITE) at line 2572. If the thread exits in that window the wait never returns. Same outcome if RunAsync succeeds but the dispatcher is shutting down — it completes the IAsyncAction with a failure instead of throwing, so queued is true and the delegate is released without ever running. Keep the thread handle and wait on both:

  HANDLE hThread = OpenThread(SYNCHRONIZE, FALSE, threadId);
  if (!hThread) continue;
  if (WaitForSingleObject(hThread, 0) == WAIT_OBJECT_0) { CloseHandle(hThread); continue; }
  ...
  HANDLE waits[] = {done, hThread};
  WaitForMultipleObjects(2, waits, FALSE, INFINITE);  // thread gone => lambda can never run
  CloseHandle(done);
  CloseHandle(hThread);

• while (g_pendingWakes.load() > 0) Sleep(10); (line 2579) has the same failure mode: the decrement lives inside the RunAsync lambda (line 1407), so a delegate that is destroyed without being invoked leaks the count and the loop spins forever. Tie the decrement to the delegate's lifetime instead, so it runs whether the lambda is invoked or dropped:

  auto pending = std::shared_ptr<void>(nullptr, [](void*) { g_pendingWakes--; });
  g_pendingWakes++;
  dispatcher.RunAsync(..., [pending]() { if (!g_unloading) { try { ApplyOnThisThreadNow(); } catch (...) {} } });

Both are tail cases, but the failure is a hung explorer.exe plus a hung Windhawk UI, which is why they're worth closing rather than leaving to chance.

Optional improvements

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

• Discover frames from the TaskbarFrame constructor instead of MeasureOverride. HandleFrameMeasure (line 1839) runs on every layout pass and does a QueryInterface plus a g_framesMutex lock and hash lookup each time, just to answer "already registered?". taskbar-on-top hooks winrt::Taskbar::implementation::TaskbarFrame::TaskbarFrame(void) and attaches a one-shot Loaded handler — every frame is caught exactly once, at construction, with nothing on the hot path.
• g_hooksApplied is redundant and can theoretically swallow Wh_ApplyHookOperations. Wh_ModAfterInit sets it unconditionally at line 2463 without having won g_taskbarViewDllLoaded.exchange, so if LoadLibraryExW_Hook is mid-HookTaskbarViewDllSymbols at that moment, its own g_hooksApplied.exchange(true) (line 2418) returns true and the taskbar hooks are registered but never applied. g_taskbarViewDllLoaded.exchange already decides who hooks; drop the second flag and just call Wh_ApplyHookOperations() after a successful hook registration, the way taskbar-icon-size does.
• The !g_unloading checks inside ApplyToFrame (lines 1018 and 1039) are unreachable. OnTimerTick returns at line 1194 when g_unloading is set, and it's the only caller — so ApplyToFrame never runs during unload and both branches are dead.

Functionality notes

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

• Hover and Rest are mouse-only. CursorOverAnyTaskbar, the grace aging and the seam probes in PointerClearsSurfaces all go through GetCursorPos / WindowFromPoint, so a touch or pen contact never arms or holds a reveal. Click works (touch maps to LeftButtonReleased), so the feature degrades sensibly — but the README note you drafted for this in the 06:32 commit message didn't make it into the file; worth adding under Notes.
• The stripped show/hide animation is per-Explorer-session, and the README's exception is slightly narrower than the code. ClearImplicitShowHide (line 609) is also called from FinalizePendingHides (line 871), i.e. on the gap-close path, not only on direct hides — so "buttons the mod actually hid" is right, but it includes buttons hidden by an animation the user may not associate with the mod. Nothing to fix, just be aware the set is larger than it looks.
• Item 1 above also removes a smaller wart: SetCollapseState currently measures idleWidth from the same buttons whose visibility it is about to flip, and needs idleSetComplete (line 777) to reject a partial sample. With an authoritative IsRunning you can measure the idle set once, before any flip, and drop the guard.

---

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.

[LarsGudm]

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

---

Round 5. The previous round's items landed well: running state now comes from TaskListButton::get_IsRunning (exact and locale-independent), applies are driven by TaskListButton::UpdateVisualStates plus the pointer hooks, and the DispatcherTimer only runs while something genuinely needs a clock. Teardown reads clean too — the hotkey thread has a ready event and unconditional joins, the sink callbacks are drained, the per-thread sweeps run on the owning threads, and the [[clang::no_destroy]] std::optional<...> on g_frames with the per-element release in CleanupOnThisThread is exactly the right shape for a container of thread-affine XAML refs. One item left, and it's about the two fallback paths that are supposed to be bounded.

1. The two "give up and retry" paths never converge, so the 100 ms timer can stay running forever.

The mod's headline property is that an idle taskbar costs nothing (// The timer runs only while something needs a clock, line 1167). Both fallback paths can defeat that permanently, because neither of them is terminal.

a) The apply retry budget re-arms itself. Lines 1156-1164:

if (ApplyToFrame(*ctx, desired, instant)) {
    ctx->instantGenSeen = instantGen;
    ctx->applyRetries = 0;
} else if (ctx->applyRetries > 0) {
    ctx->applyRetries--;
} else if (instant || ctx->appliedCollapse < 0) {
    ctx->applyRetries = 20;
}

ApplyToFrame returns false whenever CollectTaskListButtons comes back empty, and in that case ctx->appliedCollapse stays at -1. So the budget drains to 0 over 20 ticks and the very next tick re-arms it to 20, forever — and applyRetries > 0 is one of the timing terms (line 1173), so the timer never stops. Each of those ticks runs the depth-8 CollectTaskListButtons(frame, 8, buttons) walk over the taskbar's visual tree on Explorer's UI thread.

A Taskbar.TaskbarFrame with no Taskbar.TaskListButton children is reachable (nothing pinned and nothing running, transiently during Explorer startup), but the case that matters is the one your own README calls out — "Other builds can name taskbar internals differently". If the buttons ever sit deeper than the depth limit, the failure mode isn't "the mod does nothing", it's "the mod polls Explorer's UI thread at 10 Hz forever". Arming the budget once per pending apply fixes it, and nothing is lost: UpdateVisualStates fires as soon as a button exists, which re-posts an apply on its own.

// FrameContext
uint64_t retriesArmedGen = UINT64_MAX;

// OnTimerTick
} else if (ctx->applyRetries > 0) {
    ctx->applyRetries--;
} else if (ctx->retriesArmedGen != instantGen &&
           (instant || ctx->appliedCollapse < 0)) {
    ctx->applyRetries = 20;
    ctx->retriesArmedGen = instantGen;   // one budget per generation
}

b) The animation watchdog aborts without landing the target state. Lines 1146-1150:

if (ctx->animActive &&
    NowMs() - ctx->animStartMs >
        (double)g_settings.animationDurationMs * 3.0 + 1000.0) {
    StopAnimation(*ctx);
}

The watchdog exists for the case where CompositionTarget::Rendering isn't firing (bar not being composed — auto-hidden, occluded by a fullscreen exclusive app, display off). In that case the accordion never reaches t >= 0.5, so SetCollapseState never ran and ctx->appliedCollapse still holds the old value. StopAnimation only clears the spacing, so the apply that follows in the same tick sees (collapse ? 1 : 0) != ctx.appliedCollapse and calls StartAnimation again (line 893) — and Rendering is still silent, so ~1.5 s later the watchdog fires again. That's an unbounded start/abort cycle with animActive pinning timing true the whole time.

Make the watchdog terminal by landing the state it was animating toward:

if (ctx->animActive && NowMs() - ctx->animStartMs > ...) {
    // Land the target, or the next apply starts the same animation again.
    if (ctx->animKind == AnimKind::Accordion && !ctx->animSwapped) {
        SetCollapseState(*ctx, ctx->animButtons, ctx->animTargetCollapse);
    }
    StopAnimation(*ctx);
}

Optional improvements

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

• IsFrameRegistered can't tell "registered" from "stale", which makes RegisterFrame's dead-frame branch unreachable. IsFrameRegistered (line 1263) only checks key presence, and both call sites (HandlePointerMoved line 1534, HandleFrameMeasure line 1745) skip RegisterFrame when it returns true. FrameKeyFromThis keys on the raw FrameworkElement ABI pointer, so a TaskbarFrame recreated at a recycled address hits the stale entry and never registers — the careful if (it->second.frame.get()) return; ... erase path at lines 1221-1232 can only be reached from RegisterFrame's own callers, which never get there. It does self-heal (any later ApplyOnThisThreadNow ticks the stale context, which erases itself at line 1063, and the next MeasureOverride then registers), so this is just closing the window:

  bool IsFrameRegistered(void* key) {
      std::lock_guard<std::mutex> guard(g_framesMutex);
      auto it = g_frames->find(key);
      return it != g_frames->end() && it->second.frame.get() != nullptr;
  }

• The touch/pen note you drafted in the 06:32 commit message still isn't in the README. Hover and Rest go through GetCursorPos/WindowFromPoint and PointerQualifiesForReveal, so a finger or pen contact never arms or holds a reveal (Click works). One line under Notes (lines 43-54) would set expectations.
• #include <winrt/Windows.UI.Xaml.Controls.h> (line 135) looks unused — nothing from the Controls namespace appears outside the symbol-hook strings (DispatcherTimer comes from Windows.UI.Xaml.h, VisualTreeHelper/TransitionCollection from the Media headers).

Functionality notes

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

• A gap-close in flight suppresses an app that launches during it. The re-assert loop at lines 939-947 (and the repair pass in MeasureGapOffsets, lines 782-794) re-collapses everything in ctx->hiddenByUs on every rendering frame, without re-checking running state. If a pinned app launches mid-run, Windows shows its button and the mod hides it again each frame until StopAnimation; the launch itself also bumps g_lastButtonChurnMs, which extends the settle hold, so the icon can be held back for close to kGapHoldMaxMs + the close. It self-corrects on the next apply, but a ~1 s delay before a launched app's icon appears is the kind of thing users notice. Guarding the re-assert with TaskListButton_IsRunning(button) would keep the "Windows re-showed what we hid" repair while letting genuine launches through.
• frame.UpdateLayout() is still called from inside the CompositionTarget::Rendering handler (lines 950-952). It evidently settles, but forcing a synchronous layout pass from the rendering callback is the classic way to get a layout/render feedback loop in XAML — worth remembering if the taskbar ever pegs a core mid-animation.
• The dead-frame cleanup in OnTimerTick depends on the timer still ticking. When a context's frame dies while nothing is being timed, the entry (and its DispatcherTimer) sit in g_frames until some other apply pass on that thread happens to tick it. In practice UpdateVisualStates fires constantly so it's cleaned within seconds; just noting the entry's lifetime isn't tied to the frame's.

---

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.

[LarsGudm]

/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 mod is unusually careful for its size — weak refs throughout, [[clang::no_destroy]] std::optional<> on the map that owns XAML references, a three-stage teardown, the symbol-hook module comment, and every declared setting is read, clamped, and matched to the right API. Nothing found in the settings-vs-code cross-check, the symbol-hook module declaration, or the get_IsRunning call shape (it matches taskbar-labels). The findings below are about load-time discovery, the irreversible animation strip, and the unload path.

1. Nothing discovers taskbars that already exist when the mod loads.

A FrameContext is only created from HandlePointerMoved (line 1544) or HandleFrameMeasure (line 1758). Both are hooks that fire on future activity. Wh_ModAfterInit (line 2404) only handles the late-Taskbar.View.dll case, and Wh_ModSettingsChanged calls WakeAllFramesAsync(), which iterates g_frames — empty if nothing has registered yet.

So when the mod is enabled mid-session (the normal case — a user installs it and the taskbar is already up and idle), nothing collapses until the taskbar happens to re-measure or the cursor moves over it. Same for the first settings change after that. Please verify by enabling the mod and not touching the mouse.

The established pattern is an explicit apply on the taskbar thread at Wh_ModAfterInit. taskbar-tray-show-on-hover does it via GetTaskbarXamlRoot / GetSecondaryTaskbarXamlRoot (L345-L392); taskbar-count-badges runs an InitializeExistingButtonsOnTaskbarThread pass the same way. A lighter option, if you'd rather not add the CTaskBand::GetTaskbarHost plumbing, is to force a layout pass so your existing MeasureOverride hook fires — taskbar-labels nudges the taskbar with WM_SETTINGCHANGE for exactly this reason. You already have RunOnAllTaskbarThreads to carry it.

2. ClearImplicitShowHide is irreversible, and it runs even when the mod isn't animating.

SetCollapseState calls it on every button it hides (line 626), unconditionally — including when AnimationMode is none. Since there is no getter for these properties, the effect survives disabling the mod until Explorer restarts, which is a (small) violation of "a mod's effects disappear when it's disabled". You document it in the README, which is the right thing to do, but it should at least be narrowed to the cases that actually need it:

• With AnimationMode: none the mod isn't running its own animation, so letting Windows play its normal appear/disappear animation is a reasonable — arguably better — behaviour, and costs nothing irreversible.
• Consider whether the accordion path needs it either, or only the GapClose path (which is the one that genuinely fights Windows' own reorder slide).

Ideally this becomes something the user opts into rather than a side effect of installing the mod.

3. Unload path: a stray timer stays armed, and the final drain is unbounded.

Two separate points, both in the teardown:

• OnTimerTick (lines 1080-1083) handles g_unloading by calling RestoreFrame and returning — it leaves the DispatcherTimer running and the context in g_frames. That's the one state you don't want to be in during unload: a live Tick handler pointing into an image that is about to be unmapped. It should tear itself down instead (stop the timer, timer.Tick(tickToken), StopAnimation, erase the key) — the same thing the dead-frame branch just above already does. That also gives you a fourth fallback for a frame the three sweeps in Wh_ModBeforeUninit miss, which is exactly the case you log at line 2532.
• The while (g_pendingWakes.load() > 0) Sleep(10); spin (line 2524) only terminates if every dispatcher a wake was posted to eventually runs or destroys the delegate. If a taskbar thread has already exited with delegates still queued, Wh_ModBeforeUninit never returns and disabling/updating the mod hangs Windhawk. The reasoning in the comment is sound (giving up would run the lambda in a freed image), so I'm not asking you to just add a timeout — but please confirm that a CoreDispatcher whose thread is gone does release its queued delegates, since that's the assumption the loop rests on. If it doesn't, the straggler loop above is the place to fix it: it already detects dead threads with WaitForSingleObject(hThread, 0), so it could account for their wakes there.

Optional improvements

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

• Hand-rolled easing where Composition has it built in. BezierAxis / CubicBezierEase (lines 541-566) plus the per-frame CompositionTarget::Rendering tick reimplement what Compositor::CreateCubicBezierEasingFunction + a Vector3KeyFrameAnimation on the element visual already do, with CompositionScopedBatch::Completed for the mid-point swap. See taskbar-elastic-pill. It would also remove the "Rendering stops when the bar is not being composed" watchdog for that path. The GapClose path has a real reason to stay CPU-driven (it has to re-assert visibility against Windows' churn), so this only applies to the accordion.
• CleanupOnThisThread holds g_framesMutex across the XAML teardown (line 2213). Everywhere else the lock is held only for map access — OnTimerTick calls RestoreFrame without it. Moving the contexts into a local vector under the lock and restoring them after releasing it would make the locking discipline uniform and remove any chance of re-entering the non-recursive mutex from a XAML callback.
• TaskListButton_IsRunning does a get_class_name + QI per call, and the GapClose repair loops in OnRenderingTick (lines 940, 954) call it for every hidden button on every composition frame. Caching the "is really a Taskbar.TaskListButton" answer next to the weak ref would take that out of the per-frame path.
• StartHotkeyThread is called from inside the LoadLibraryExW hook (line 2364), i.e. under the loader lock, while StopHotkeyThread joins that thread under g_hotkeyThreadMutex from Windhawk's thread. The interleaving needed for an actual cycle is contrived, but deferring the thread start out of the loader-lock path (e.g. from the first frame registration) removes the question entirely.
• HandlePointerReleased's comment says "dragging off and letting go must not count" (line 1698), but the handler only inspects where the release landed — a drag that starts on a button and ends on empty space will still toggle. If that's intended, the comment is misleading; if not, you'd need to track the press.
• Windows API macro forms. GetClassName, FindWindowEx, SetWindowsHookEx, RegisterWindowMessage, CreateEvent, GetModuleHandle, SendMessage are used in macro form. Most recent taskbar mods spell the W suffix explicitly.

Functionality notes

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

• The accordion only spreads to the right. ApplySpacing (line 571) pins the leftmost button and translates the rest rightward, and Translation is render-only, so nothing clamps the stretched row to the free strip. On a left-aligned taskbar (what you tested) there's slack to the right and it looks fine. On the Windows 11 default centred layout the group renders visibly off-centre for the duration of the animation, and at high AnimationAmplitudePct (up to 200) with many pinned icons the rightmost buttons can render over the tray. Splitting the extra spacing around the group's centre, or clamping animAmplitude to the space actually available, would cover both layouts. Worth a pass on a centred and a multi-monitor setup before this ships, since the README already flags them as untested.
• IAppVisibility is the only Start-menu signal and has no fallback. No other mod in the catalog uses it, so its behaviour across Windows 11 builds isn't well established here. If LauncherVisibilityChange stops firing on a future build, RevealOnStart silently does nothing — a Wh_Log on the first event received would at least make that diagnosable.
• Rest-speed sampling reads GetCursorPos inside the handler (line 1574) rather than the pointer event's own position/timestamp, so the measured speed includes handler-dispatch jitter. Also, the first move event after a >150 ms gap returns early (line 1583) before StartTimersOnThisThread, so a fast flick into empty space followed by an immediate stop can leave the dwell unarmed until the next move event.
• Settings descriptions could say which trigger they apply to. RevealDelayMs and HoverGraceMs only affect hover / rest; RestSpeedPxPerSec already spells this out and the other two would benefit from the same.
• With Click, a reveal never ages out. Clicking empty space to reveal, then clicking an icon to launch something, leaves the taskbar expanded until the next empty-space click. That matches the README's "toggle" wording, but an optional auto-collapse after N seconds might be nicer than it sounds.
• The default hotkey grabs Ctrl+Alt+T process-wide from Explorer, which starts early enough to win the registration against most apps. Defaults with hotkeys are normal in the catalog, so this is just something to be aware of.

---

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.

[LarsGudm]

/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 plumbing is in good shape — settings match the code exactly, HookSymbols is called once per module, LoadLibraryExW is hooked in kernelbase (not via the kernel32 import), the get_IsRunning and MeasureOverride hook shapes match taskbar-labels and taskbar-vertical, the DPI-nudge discovery trick follows taskbar-icon-size, [[clang::no_destroy]] std::optional<...> on g_frames is the right form, and window enumeration filters by process ID. The items below are what's left.

Teardown gives up instead of guaranteeing it finished. Wh_ModBeforeUninit has three escalating cleanup passes, but the last one is still best-effort: if dispatcher.RunAsync throws, queued stays false, the loop moves on, and the function ends by logging "%u taskbar contexts could not be cleaned up" and returning. A context left in g_frames still owns a running DispatcherTimer whose Tick handler is a lambda in the mod image (and possibly a live CompositionTarget::Rendering token). Windhawk FreeLibrarys the mod immediately after Wh_ModBeforeUninit/Wh_ModUninit return, so the next tick executes unmapped code and takes explorer.exe with it. Since you've already established the thread is alive (WaitForSingleObject(hThread, 0) a few lines up), the post has to be retried rather than abandoned — loop on RunAsync (re-checking the thread handle each round) until it either succeeds or the thread dies, and make the final g_frames check a hard invariant rather than a log line.

The unload drain can spin forever. The last wait is unbounded:

while (g_pendingWakes.load() > 0) {
    Sleep(10);
}

g_pendingWakes only decrements when the CoreDispatcher destroys the queued delegate. But the comment right below deliberately keeps the leftover contexts — and therefore their CoreDispatcher references — alive forever. So a wake that was queued to a dispatcher whose thread has since exited is never invoked and never released, and the loop never terminates: the mod unload hangs, which hangs Windhawk's mod management and any explorer.exe restart that waits on it. Before draining, release the dispatcher reference for every context whose thread is already gone (you have the OpenThread/WaitForSingleObject(hThread, 0) check for exactly that), or give the drain a bounded timeout with a documented fallback. Same reasoning applies to the INFINITE waits in StopHotkeyThread — worth convincing yourself each one has a guaranteed signaller.

ClearImplicitShowHide is a permanent change to the taskbar that survives disabling the mod. As the comment says, ElementCompositionPreview::SetImplicitShowAnimation/SetImplicitHideAnimation have no getter, so nulling them can't be undone. Every button the mod hides gets stripped, and after the mod is disabled those buttons keep popping in and out with no fade until explorer.exe restarts. Windhawk's core principle is that a mod's effects disappear when it's disabled, so this needs to shrink. You already have the reversible half working: DeanimateButton/ReanimateButtons save and restore both FrameworkElement::Transitions and Visual::ImplicitAnimations. Please check whether that alone is enough to kill the slide, and if so drop ClearImplicitShowHide entirely. If the implicit show/hide really is a separate mechanism you can't suppress any other way, say so explicitly in the README ("restart Explorer to restore the taskbar's built-in icon fade") so users aren't surprised.

The mod is ~3,000 lines for "hide pinned icons whose app isn't running", and roughly two thirds of that is a bespoke animation engine. BuildAnimPlan's measure-flip-measure-unflip pass, the per-phase fold tables, the neighbour-interpolated coordinate synthesis, animChildCount/visiblePhase1 divergence detection, the per-render-frame re-show repair loop with an inline frame.UpdateLayout(), the duration * 3 + 1000 watchdog, FinalizeCollapseRun's "belt" sweep plus a verification PostApply, and the three-tier unload fallback are all scaffolding that exists because the animation is fighting the taskbar's own layout animations rather than working with them. Every one of those mechanisms runs on explorer.exe's UI thread, and each is a place a future Windows build can break in a way that's hard to diagnose. This isn't a correctness finding, but it is a real cost to the catalog: consider shipping the core feature (running-state detection, the reveal triggers, AnimationMode: none) plus at most one simple animation, and cutting the accordion machinery — or landing the mod in two steps so the animation work can be reviewed on its own. Note that AnimationMode: none is already a supported code path, so this is mostly a question of what to delete rather than what to write.

Optional improvements

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

• The taskbar UI thread can block on g_hotkeyThreadMutex from inside a layout pass. RegisterFrame is called from TaskbarFrame_MeasureOverride_Hook and ends with StartHotkeyThread(), which takes g_hotkeyThreadMutex. Wh_ModSettingsChanged holds that same mutex across WaitForSingleObject(g_hotkeyThread, INFINITE), so a settings save that lands during a measure pass stalls the taskbar's layout for as long as the hotkey thread takes to exit. Normally instant, but it's a UI-thread block behind an unbounded wait. A try_lock in StartHotkeyThread (it's opportunistic and gets called again on the next registration / pointer move) or moving the call into the posted apply would remove the coupling.

• Hotkey defaults to Ctrl+Alt+T. That's a process-wide RegisterHotKey, so a fresh install silently takes that combination away from every other app on the system. Defaulting the setting to empty (the parser and StartHotkeyThread already handle it) would make the hotkey opt-in, which fits better with RevealTrigger: click being the default reveal.

• CursorOverAnyTaskbar's fallback is broader than "taskbar". After the Shell_TrayWnd/Shell_SecondaryTrayWnd class check fails, it treats any window in this process owned by a thread that has a registered frame as taskbar. That's meant to catch jump lists and flyouts, but it also catches anything else that happens to share that thread. Matching the popup's owner chain back to a taskbar window (or checking against a known set of popup classes) would be tighter than a thread-ID match.

• Empty-space left-click overlaps with an existing mod. taskbar-empty-space-clicks lets users bind actions to left-clicks on empty taskbar space. Your hook calls the original first and doesn't mark the event handled, so both should fire rather than conflict — but since click is your default trigger, a one-line note in the README that a left-click reveal will also trigger whatever that mod has bound would save some confusion.

• ApplyToFrame clears ctx.lastButtons before it knows it found anything. When a taskbar rebuild makes CollectTaskListButtons come back empty, lastButtons is left empty and the function returns false — until the retry succeeds, IsPointerClearOfButtons has nothing to test against and the ElementPaddingPx guard trivially passes. Rebuilding into a local and only swapping it in on success would keep the last known-good set.

• HandlePointerMoved calls ApplyOnThisThreadNow() and then WakeAllFramesAsync(), and the latter posts to this thread's dispatcher too, so the flip applies twice on the local taskbar. Harmless, but skipping the current thread's dispatcher in WakeAllFramesAsync (or dropping the direct call) would avoid the redundant pass.

Functionality notes

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

• Element classification is substring-based. IsPointerOverEmptySpace and ProbeStripIsClear decide "is this empty space" by looking for Button / SystemTray / TaskItemThumbnail inside winrt::get_class_name. That's a reasonable heuristic today, but any future taskbar element whose class name contains none of those substrings reads as empty space and becomes a reveal trigger, while anything that happens to contain Button blocks a reveal. If you can enumerate the classes that actually appear as strip children on the builds you support, an explicit list would fail more predictably.

• Start detection depends on IAppVisibility still reporting the Win11 Start menu. You've already hedged with the one-shot "Start visibility events active" log and the "AppVisibility unavailable" path, which is the right call. Worth knowing that if this ever goes silent on a build, RevealOnStart just quietly does nothing — the log is the only signal, so a README line pointing at it would help with bug reports.

• TaskbarIsCentered() reads TaskbarAl from the registry on every StartAnimation. That's correct for stock Windows, but mods that change taskbar alignment without writing that value (or that reposition the strip themselves) will make the accordion pick the wrong branch. Measuring the strip's actual offset within the frame would be self-correcting, if that's cheap enough at animation start.

• Reveal state is global across monitors (g_revealed is a single atomic), so hovering one taskbar reveals all of them. That reads as deliberate from the comments — just calling it out since the README describes reveal in per-taskbar terms ("rest the cursor on empty taskbar").

• The animation only moves along X (Translation(float3{x, 0, 0})). Fine for the Windows 11 taskbar, but it means the mod's animation won't do anything sensible if combined with a vertical-taskbar mod. A note in the README's Notes section would set expectations.

---

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.