From 548fba65c06fea9ae5ec0126c3ffd824207b2ed4 Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Tue, 22 Sep 2026 17:26:12 -0700 Subject: [PATCH] Guard native form modal reentrancy Reject nested NativeForms presentations, expose the active modal state to the app, and retain completed update offers until the current native form closes. Add focused decision and guard tests plus a mutation-sensitive assertion in the Windows shell contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- Tools/windows/Tests/WindowsShell.Tests.ps1 | 9 ++++ graphcode-windows/src/App.zig | 45 ++++++++++++------- graphcode-windows/src/NativeForms.zig | 33 ++++++++++++-- .../src/UpdateOfferPresentation.zig | 20 +++++++++ 4 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 graphcode-windows/src/UpdateOfferPresentation.zig diff --git a/Tools/windows/Tests/WindowsShell.Tests.ps1 b/Tools/windows/Tests/WindowsShell.Tests.ps1 index 6d4093b2..bf92d852 100644 --- a/Tools/windows/Tests/WindowsShell.Tests.ps1 +++ b/Tools/windows/Tests/WindowsShell.Tests.ps1 @@ -62,6 +62,10 @@ Assert-Contract ($appSource -match $appSource -match 'if \(!envFlag\("GRAPHCODE_UIA_UPDATE_AVAILABLE"\)\) self\.requestUpdateCheck\(false\)' -and $appSource -match 'shouldPresentOffer\(self\.update_user_initiated\)') ` "explicit and background update checks must preserve their presentation intent" +Assert-Contract ($nativeFormsSource -match 'if \(active_state\) return error\.FormAlreadyOpen;' -and + $nativeFormsSource -match 'pub fn isModalActive\(\) bool' -and + $appSource -match 'UpdateOfferPresentation\.decide\(completed_offer, self\.update_offer_pending, NativeForms\.isModalActive\(\)\)') ` + "native forms must reject reentrancy and completed update offers must wait for the active modal" Assert-Contract ($appSource -match 'const uia_gate_hook = envFlag\("GRAPHCODE_UIA_GATE"\);' -and $appSource -match 'if \(!daemon_supervisor_test_hook and !uia_gate_hook\) GdiplusAA\.init\(\);') ` @@ -191,6 +195,7 @@ foreach ($path in @( "src\InputRouter.zig", "src\Forms.zig", "src\NativeForms.zig", + "src\UpdateOfferPresentation.zig", "src\WindowsOnboarding.zig", "src\WindowsProductSettings.zig", "src\Accessibility.zig", @@ -370,6 +375,10 @@ Invoke-Native "Native dialog message-loop executable tests" { & $zig test src\NativeForms.zig -target x86_64-windows-msvc -lc -luser32 "-I$include" } finally { Pop-Location } } +Invoke-Native "Update offer modal deferral executable tests" { + Push-Location $shellRoot + try { & $zig test src\UpdateOfferPresentation.zig } finally { Pop-Location } +} Invoke-Native "Jump palette executable tests" { $depotRoot = Split-Path (Split-Path $repoRoot -Parent) -Parent $winghosttyRoot = [Environment]::GetEnvironmentVariable("GRAPHCODE_WINGHOSTTY_ROOT") diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index b41f7b1a..4fdfcea0 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -30,6 +30,7 @@ const Codespaces = @import("Codespaces.zig"); const Onboarding = @import("WindowsOnboarding.zig"); const WindowsUpdates = @import("WindowsUpdates.zig"); const UpdateOfferDialog = @import("UpdateOfferDialog.zig"); +const UpdateOfferPresentation = @import("UpdateOfferPresentation.zig"); const WorktreeDialog = @import("WorktreeDialog.zig"); const Accessibility = @import("Accessibility.zig"); const Navigation = @import("Navigation.zig"); @@ -269,6 +270,7 @@ pub const App = struct { update_cancel: std.atomic.Value(bool) = std.atomic.Value(bool).init(false), update_generation: u64 = 0, update_pending: bool = false, + update_offer_pending: bool = false, update_user_initiated: bool = false, update_version: []u8 = &.{}, update_release_url: []u8 = &.{}, @@ -1524,6 +1526,7 @@ pub const App = struct { self.update_generation += 1; self.update_user_initiated = user_initiated; self.update_pending = true; + self.update_offer_pending = false; if (self.update_thread != null) { self.update_cancel.store(true, .release); self.update_lock.unlock(); @@ -1590,25 +1593,33 @@ pub const App = struct { self.update_lock.lock(); const done = self.update_done; self.update_lock.unlock(); - if (!done) return; - if (self.update_thread) |thread| { - thread.join(); - self.update_thread = null; - self.update_lock.lock(); - const pending = self.update_pending; - self.update_pending = false; - const label = self.update_state.label(); - const present_offer = self.update_state.shouldPresentOffer(self.update_user_initiated); - const version = self.update_version; - const release_url = self.update_release_url; - self.update_lock.unlock(); - if (pending) { - self.launchUpdateCheck(); - } else { - self.setStatus(label); - if (present_offer) self.showAvailableUpdate(version, release_url); + var completed_offer = false; + if (done) { + if (self.update_thread) |thread| { + thread.join(); + self.update_thread = null; + self.update_lock.lock(); + const pending = self.update_pending; + self.update_pending = false; + const label = self.update_state.label(); + const present_offer = self.update_state.shouldPresentOffer(self.update_user_initiated); + self.update_lock.unlock(); + if (pending) { + self.launchUpdateCheck(); + } else { + self.setStatus(label); + completed_offer = present_offer; + } } } + switch (UpdateOfferPresentation.decide(completed_offer, self.update_offer_pending, NativeForms.isModalActive())) { + .none => {}, + .defer_until_modal_closes => self.update_offer_pending = true, + .present => { + self.update_offer_pending = false; + self.showAvailableUpdate(self.update_version, self.update_release_url); + }, + } } fn showAvailableUpdate(self: *App, version: []const u8, release_url: []const u8) void { diff --git a/graphcode-windows/src/NativeForms.zig b/graphcode-windows/src/NativeForms.zig index 9730dad8..b7304ee8 100644 --- a/graphcode-windows/src/NativeForms.zig +++ b/graphcode-windows/src/NativeForms.zig @@ -86,6 +86,19 @@ var active_state_storage: DialogState = undefined; const ModalCommand = enum { submit, cancel, close, destroy }; +pub fn isModalActive() bool { + return active_state; +} + +fn acquireModal() !void { + if (active_state) return error.FormAlreadyOpen; + active_state = true; +} + +fn releaseModal() void { + active_state = false; +} + /// Loop-type teaching-tile accents, converted from the exact RGB values macOS /// uses for the same four types (LoopTypeAppearance.swift's `accent`), so the /// Windows tiles read as the same visual language rather than a new palette. @@ -673,13 +686,14 @@ pub fn worktreeSweep( fn show(state: *DialogState, title: []const u8, labels: []const []const u8) !bool { _ = labels; + try acquireModal(); + defer releaseModal(); registerClass() catch return error.FormClassRegistrationFailed; const wide_title = try utf8ToWideZ(state.allocator, title); defer state.allocator.free(wide_title); active_state_storage = state.*; active_state_storage.closed = false; active_state_storage.result = false; - active_state = true; const screen_height = c.GetSystemMetrics(c.SM_CYSCREEN); const dialog_height: i32 = if (state.kind == .worktree_policy) 430 else @max(320, @min(700, screen_height - 96)); const hwnd = c.CreateWindowExW( @@ -696,7 +710,6 @@ fn show(state: *DialogState, title: []const u8, labels: []const []const u8) !boo c.GetModuleHandleW(null), @ptrCast(state), ) orelse { - active_state = false; return error.FormCreationFailed; }; _ = c.EnableWindow(state.parent, 0); @@ -722,7 +735,6 @@ fn show(state: *DialogState, title: []const u8, labels: []const []const u8) !boo _ = c.EnableWindow(state.parent, 1); _ = c.SetActiveWindow(state.parent); state.* = active_state_storage; - active_state = false; if (quit_code) |value| c.PostQuitMessage(@intCast(value)); return state.result; } @@ -2104,6 +2116,21 @@ test "modal submit and cancel transitions always terminate the loop" { try std.testing.expect(state.result); } +test "native forms reject reentrant modal acquisition and allow sequential dialogs" { + active_state = false; + defer active_state = false; + + try acquireModal(); + try std.testing.expect(isModalActive()); + try std.testing.expectError(error.FormAlreadyOpen, acquireModal()); + + releaseModal(); + try std.testing.expect(!isModalActive()); + try acquireModal(); + try std.testing.expect(isModalActive()); + releaseModal(); +} + test "jump modal result uses production query validation" { var state = DialogState{ .allocator = undefined, .kind = .jump, .parent = null }; state.values[0] = @constCast(" \t\r\n"); diff --git a/graphcode-windows/src/UpdateOfferPresentation.zig b/graphcode-windows/src/UpdateOfferPresentation.zig new file mode 100644 index 00000000..25cb4cf0 --- /dev/null +++ b/graphcode-windows/src/UpdateOfferPresentation.zig @@ -0,0 +1,20 @@ +const std = @import("std"); + +pub const Decision = enum { + none, + defer_until_modal_closes, + present, +}; + +pub fn decide(completed_offer: bool, deferred_offer: bool, modal_active: bool) Decision { + if (!completed_offer and !deferred_offer) return .none; + if (modal_active) return .defer_until_modal_closes; + return .present; +} + +test "update offer remains deferred until the native modal closes" { + try std.testing.expectEqual(Decision.none, decide(false, false, false)); + try std.testing.expectEqual(Decision.defer_until_modal_closes, decide(true, false, true)); + try std.testing.expectEqual(Decision.defer_until_modal_closes, decide(false, true, true)); + try std.testing.expectEqual(Decision.present, decide(false, true, false)); +}