NativeForms modal loop has no reentrancy guard; timer-driven UpdateOfferDialog can nest inside an open form

[coneilen]

Summary

NativeForms.show()'s modal message loop pumps every message on the thread, so App's WM_TIMER handler continues to fire while a native form is open. That handler can synchronously present UpdateOfferDialog — a second, unrelated nested modal with its own global state — with no guard checking whether a NativeForms dialog is already active.

This is pre-existing, not introduced by any recent PR.

Mechanism (verified on main @ 4e88dfa)

• NativeForms.zig:708 — const code = c.GetMessageW(&message, null, 0, 0); The null HWND filter means it pumps all thread messages, not just the dialog's.
• App.zig:4855 — c.WM_TIMER => if (wparam == MainWindow.timer_id) { ... }
• App.zig:4860 — calls app.finishUpdateCheck()
• App.zig:1589 / :1619 — finishUpdateCheck() can synchronously call UpdateOfferDialog.show(...)

Nothing on that path tests whether a NativeForms modal is currently on the stack.

Why this is worth fixing

The codebase already establishes the correct pattern — NativeForms is the outlier:

• WindowsCodespaceDialog.zig:227 — var dialog_active = false;, with :252 if (dialog_active) return error.CodespaceDialogAlreadyOpen; and :262-263 set/defer reset.
• WindowsRepositoryDialogs.zig:610 — var repository_dialog_active = false;, same guard at :619.
• WindowsRepositoryDialogs.zig:1049 — var operation_dialog_active = false;, same shape.

Both guarded modules also short-circuit their window procs when inactive (:343, :699, :1146). NativeForms.zig has no equivalent state.

Exposure has widened

The race is old, but the prompt-attachments work (#415) added the first NativeForms path whose modal loop can stay open for an arbitrarily long, real wall-clock duration — a live native IFileOpenDialog picker waiting on a human. Previously these dialogs closed in seconds. That materially widens the window in which a timer-driven update offer can nest itself inside an open form.

Impact and current reachability

No known user-facing failure yet, and not currently reachable in CI — update checks need real network/version data that CI doesn't provide. Investigation of the unexplained 64-minute windows-spikes hang found no evidence this was the cause (see below); it is flagged as a latent defect found during that work, not as a diagnosed root cause.

Suggested fix

Give NativeForms the same dialog_active guard its sibling dialog modules already use, and have finishUpdateCheck() defer presenting the update offer while any native modal is active rather than presenting it nested.

Provenance

Found while investigating the 64-minute windows-spikes hang on the prompt-attachments slice. That investigation ruled out the attachments code specifically, with citations: Forms.generateDraftId is O(1) (timestamp ^ fetchAdd(sequence), no retry loop); DraftAttachments.discardAll is a single deleteTree ... catch {} with no loop; attachment control ids 4/5/6 don't collide with the 9100/9200/9600 ranges; and no CI script (uia-live-gate.ps1, WindowsShell.Tests.ps1, TrayDaemon.Tests.ps1, windows-shell.ps1) references "Attach" or any attachment control, so nothing in CI exercises that path. The hang remains unexplained and is most consistent with the runner-saturation contention seen fleet-wide.