WindowsRepositoryDialogs: showOperationDialog has no reentrancy guard and can overwrite in-flight operation state

[coneilen]

Summary

WindowsRepositoryDialogs.showOperationDialog() has a module-level operation_dialog_active flag that looks like a reentrancy guard but never functions as one. Unlike its two siblings in the same file, it is never tested before acquisition, so a reentrant call proceeds and overwrites shared global state belonging to the in-flight operation.

This is the residue of #419. That issue fixed NativeForms, which was the obvious outlier. While confirming the remaining modals were sound, this path turned out to be guarded only in appearance.

Evidence (verified on main)

Every reference to the flag in graphcode-windows/src/WindowsRepositoryDialogs.zig:

1049:  var operation_dialog_active = false;      // declaration
1073:      operation_dialog_active = true;       // set, with no preceding check
1092:          operation_dialog_active = false;  // release
1115:      operation_dialog_active = false;      // release
1146:      if (!operation_dialog_active) return c.DefWindowProcW(...)   // wndproc filter

The only read is at :1146, inside the window procedure. It filters stray messages; it does not gate entry. There is no if (operation_dialog_active) return error.... anywhere.

Contrast the correctly guarded sibling in the same file, openRepositoryDialog():

610:  var repository_dialog_active = false;
619:      if (repository_dialog_active) return error.RepositoryDialogAlreadyOpen;

and WindowsCodespaceDialog.zig:252, which returns error.CodespaceDialogAlreadyOpen. Both check before setting. showOperationDialog does not.

The concrete failure

operation_dialog_state at :1050 is a single module-level value, written by both entry points before entering the modal:

1053:  operation_dialog_state = .{ .allocator = allocator, .parent = parent, .clone = operation };      // showCloneProgress
1065:  operation_dialog_state = .{ .allocator = allocator, .parent = parent, .remote = operation };    // showRemoteValidation

If either is entered while an operation dialog is already running, operation_dialog_state is overwritten in place. The first operation's modal loop is still live and its wndproc continues reading that same global, so it resumes against a different allocator, parent, and operation union. :1074-1075 additionally reset closed/cancelled, so a cancel or completion signalled for the first operation can be lost.

Severity is bounded by reachability, not by damage: this is a progress dialog for clone and SSH validation, and the owner window is disabled while it runs, so ordinary duplicate user activation is unlikely. The failure needs an asynchronous or programmatic entry. Note this is exactly the shape of #419, where a WM_TIMER-driven update check reentered a modal with no user action involved.

Expected

showOperationDialog() checks the flag before setting it and returns a distinct error on reentry, matching the established pattern at :619 and WindowsCodespaceDialog.zig:252. Release should be defer-based on all exits rather than the current two separate assignments at :1092 and :1115.

Explicitly out of scope

WindowsCodespaceDialog.zig:252 and WindowsRepositoryDialogs.zig:619 both fail fast and drop the second request. That was reviewed and is correct as-is: both are synchronous user-initiated opens with a disabled owner window, and there is no durable event that must eventually be presented. The deferral machinery added for update offers in #420 is not needed here and should not be copied over. Only the missing acquisition check in showOperationDialog is in scope.

Suggested test

A focused executable test asserting the second acquisition returns the new error, plus a mutation check that removing the guard makes that named test fail. Note WindowsRepositoryDialogs.zig (13 tests) is currently one of the orphaned files in #424 and is not executed by any harness, so wire it or coordinate with that work, otherwise a test added here will not run in CI.