[Detail Bug] KNX GUI: Confirm Reset dialog can silently retarget to a different device while open (via Ctrl+Z undo)

Start with apps/knx-gui/src/knx_gui/plugins/project/ui/components/restart_section.py and trace how the destructive popup is opened and rendered. Run apps/knx-gui/src/knx_gui/testing/tests/test_popup_device_race.py; done means the confirmation continues to target device A after the simulated selection change, with the selected mode also fixed for the dialog lifetime.

Detail Bug Report

https://app.detail.dev/org_62aa40f5-2c23-4914-a665-3bb2068af20e/bugs/bug_e0cbc079-e1d0-4f02-b851-749a2ae31afd

Introduced in #24 by @kewde on Sep 6, 2026

Summary

• Context: RestartSection opens a modal "Confirm Reset" popup for a destructive KNX reset mode in render(device), but the popup body re-reads the per-frame device argument every frame instead of a snapshot taken at open_popup time. While the popup is open, an out-of-band ProjectService.undo() (via the app-level Ctrl+Z shortcut handler in main.py:_handle_shortcuts) can cause ConfigurePanel.render's device is None -> devices[0] fallback to swap the live selected device from A to a different device B.
• Bug: The still-open popup then re-renders its body text with B's name and re-targets its "Reset" button to B; a user who confirms the popup (still reads the static title "Confirm Reset", same button) dispatches a destructive master-reset against B's individual address — the device the application silently re-targeted to while the popup was open, not the one the user opened it for.
• Actual vs. expected: Actual: confirming the destructive reset can operate on a different device than the one used to open the confirmation popup, if selection changes while the modal is open. Expected: the confirm dialog’s target device (and reset mode) should be fixed for the lifetime of the dialog.
• Impact: The destructive modes (Factory Reset, Reset IA, Reset Application Program, Reset Parameters, Reset Links, Erase Persistently Stored Application Data) issue Erase Code 0x02..0x07 master-reset requests against device.individual_address on the KNX bus. This is a confirm-dialog target-mutation bug: the dialog can silently re-target mid-flight and dispatch a destructive reset to the wrong physical device.

Code with Bug

apps/knx-gui/src/knx_gui/plugins/project/ui/components/restart_section.py:

def render(self, device: Device) -> None:
    ...
    if imgui.button(S.BTN_RESET, button_size):
        if selected.destructive:
            imgui.open_popup(S.POPUP_CONFIRM_RESET_TITLE)   # <-- BUG 🔴 no snapshot of `device` taken at open_popup time
        else:
            self._do_restart(device, selected)
    ...
    self._render_reset_confirm_popup(device, selected)      # <-- BUG 🔴 uses per-frame `device`, re-read every frame

def _render_reset_confirm_popup(self, device: Device, selected: _ResetMode) -> None:
    if imgui.begin_popup_modal(S.POPUP_CONFIRM_RESET_TITLE, ...)[0]:
        imgui.text(S.POPUP_CONFIRM_RESET_TEXT.format(
            mode=selected.label, device=device.name
        ))
        imgui.separator()
        if imgui.button(S.BTN_RESET, imgui.ImVec2(75, 0)):
            self._do_restart(device, selected)              # <-- BUG 🔴 confirm dispatches against current device, not open-time device
            imgui.close_current_popup()
        ...

apps/knx-gui/src/knx_gui/plugins/project/ui/configure.py:

device = self._get_selected_device()
if device is None:
    device = devices[0]
    self._set_selected_device(device)

Explanation

• RestartSection does not persist the device (or mode) used to open the destructive confirmation popup; it reuses whatever device is passed to render() each subsequent frame while the modal remains open.
• While the modal is open, KnxGuiApp._handle_shortcuts can still fire Ctrl+Z because it reads raw key state every frame and is not blocked by ImGui’s modal input grab.
• An undo that removes/invalidates the selected device can leave ProjectService.selected_device as None, and ConfigurePanel.render then falls back to selecting devices[0] (potentially a different device). On confirm, _do_restart is called with this newly selected device, sending a master-reset request to the wrong individual address.
• This is empirically confirmed by a harness test that opens the popup on device A, flips the per-frame device to B before confirming, and observes the restart call firing on B.

Failing Test

apps/knx-gui/src/knx_gui/testing/tests/test_popup_device_race.py:

"""Device-A opens the destructive 'Confirm Reset' popup; device-B is what
`render()` sees on the confirm frame (simulating an undo that swapped the
selected device). Expected (snapshotted): on_restart_device fires with A.
Actual (per-frame): on_restart_device fires with B."""
from __future__ import annotations
from types import SimpleNamespace
from typing import cast
from imgui_bundle import imgui
from imgui_bundle.immapp import testing as imgui_testing
import knx_gui.main  # noqa: F401
from knx_gui.device import Device
from knx_gui.plugins.project.ui.components import RestartRequest, RestartSection
from knx_gui.testing.harness import TestContext as _TestContext

_WINDOW_SIZE = (400, 300)

def _device(name: str, ia: str = "1.1.1") -> Device:
    return cast(Device, SimpleNamespace(individual_address=ia, name=name))

def test_popup_uses_per_frame_device_not_snapshot() -> None:
    device_a = _device("Device A", ia="1.1.2")
    device_b = _device("Device B", ia="1.1.3")
    state: dict[str, object] = {"current": device_a}
    calls: list[tuple[Device, RestartRequest]] = []
    def on_restart(d: Device, req: RestartRequest) -> None: calls.append((d, req))
    section = RestartSection(on_restart)
    section._reset_mode_index = 2  # pyright: ignore[reportPrivateUsage]  # Factory Reset - destructive

    def gui_function() -> None:
        imgui.begin("TestPanel"); section.render(state["current"])  # type: ignore[arg-type]
        imgui.end()

    def test_function(ctx: _TestContext) -> None:
        ctx.set_ref("//TestPanel")
        ctx.item_click("Reset"); ctx.yield_()
        assert calls == [], "destructive mode must not restart on first click"
        ctx.set_ref("//Confirm Reset"); ctx.yield_()
        state["current"] = device_b; ctx.yield_()    # simulate the per-frame re-resolution flipping A -> B
        ctx.item_click("Reset"); ctx.yield_()        # confirm inside the still-open popup

    imgui_testing.run(gui_function, test_function, window_size=_WINDOW_SIZE)
    assert len(calls) == 1
    fired_device, req = calls[0]
    assert fired_device is device_a

Test currently fails because fired_device is device_b, demonstrating the confirm dialog dispatches against the post-flip device.

Recommended Fix

Snapshot the device (and selected mode) at open_popup time and have the popup body and confirm action use that snapshot, clearing it on confirm/cancel.

History

This bug was introduced in commit fe3f58e. Later refactor 7158478 extracted the same logic into restart_section.py without changing the no-snapshot behavior.