Fix Windows node ids and text dialog initial focus (beta17 Dev Box findings)

graphcode-windows/src/DaemonClient.zig

@@ -125,6 +125,25 @@ test "sketch promotion test client has a fixed synthetic endpoint and inactive t
     try std.testing.expectEqual(@as(usize, 0), client.inbound_count);
 }
 
+test "client-generated node ids on the create-node wire are random version-4 UUIDs" {
+    const allocator = std.testing.allocator;
+    var client = try DaemonClient.initUnstartedForTest(allocator);
+    defer client.deinit();
+    client.sendCreateNodeConfigured("C:\\work\\graph", "Loop", "claudeCode", null);
+    client.sendCreateNodeDraft("C:\\work\\graph", .{ .title = "" });
+    try std.testing.expectEqual(@as(usize, 2), client.outbound_count);
+    var ids: [2][]const u8 = undefined;
+    for (client.outbound[0..2], &ids) |command, *id| {
+        const marker = "\"createNode\":{\"_0\":{\"id\":\"";
+        const start = (std.mem.indexOf(u8, command, marker) orelse return error.MissingNodeId) + marker.len;
+        id.* = command[start .. start + 36];
+        try std.testing.expect(Forms.isUuid(id.*));
+        try std.testing.expectEqual(@as(u8, '4'), id.*[14]);
+        try std.testing.expect(!std.mem.startsWith(u8, id.*, "00000000-0000-4000-8000-"));
+    }
+    try std.testing.expect(!std.mem.eql(u8, ids[0], ids[1]));
+}
+
 pub const EventCallback = *const fn (
     context: ?*anyopaque,
     frame: [*]const u8,
@@ -167,7 +186,6 @@ pub const DaemonClient = struct {
     last_error: []const u8 = "",
     resume_from: u64 = 0,
     next_request: u64 = 1,
-    next_draft: u64 = 1,
     pending_request_ids: [64][36]u8 = undefined,
     pending_request_count: usize = 0,
     v1_pending_count: usize = 0,
@@ -485,11 +503,7 @@ pub const DaemonClient = struct {
         model_tier: ?[]const u8,
     ) void {
         var node_id: [36]u8 = undefined;
-        self.mutex.lock();
-        const sequence = self.next_draft;
-        self.next_draft +%= 1;
-        self.mutex.unlock();
-        makeRequestID(&node_id, sequence);
+        Forms.generateDraftId(&node_id);
         const command = Wire.commandGraphCreateNodeConfigured(
             self.allocator,
             project_path,
@@ -513,11 +527,7 @@ pub const DaemonClient = struct {
         // creates has to carry the same one. Everything else keeps the historical
         // generate-at-send-time id.
         const node_id: []const u8 = if (draft.node_id.len != 0) draft.node_id else blk: {
-            self.mutex.lock();
-            const sequence = self.next_draft;
-            self.next_draft +%= 1;
-            self.mutex.unlock();
-            makeRequestID(&generated_id, sequence);
+            Forms.generateDraftId(&generated_id);
             break :blk &generated_id;
         };
         const command = Wire.commandGraphCreateNodeFull(self.allocator, project_path, node_id, draft) catch {

graphcode-windows/src/Forms.zig

@@ -65,27 +65,28 @@ pub const NodeDraft = struct {
     }
 };
 
-/// A `[[0-9a-f]{8}-...]` version-4-shaped id, generated the same way
-/// `DaemonClient.zig`'s own `makeRequestID` does — a nanosecond timestamp rather than a
-/// cryptographic random source, because these ids only ever need to be unique within one
-/// running client, never unguessable. Exposed here (rather than kept private to
-/// `DaemonClient.zig`) so a draft's id can be chosen before its dialog opens, which is
-/// what lets an attachment picked mid-dialog be filed under the id the node will
-/// actually carry.
-///
-/// Mixed with a process-lifetime counter, not the timestamp alone: two calls close
-/// enough together can land on the same nanosecond reading on lower-resolution clocks,
-/// which would hand two different attachment directories the same name.
-var draft_id_sequence = std.atomic.Value(u64).init(0);
-
+/// A random RFC 4122 version-4 id, formatted like Swift `UUID().uuidString` (uppercase),
+/// which is also how the daemon echoes node ids back. Every node id the client chooses
+/// comes from here — `DaemonClient` uses it for create-node commands too — so Windows
+/// ids carry the same 122 random bits as macOS ones instead of a timestamp behind a
+/// zero-filled prefix. Exposed here (rather than kept private to `DaemonClient.zig`) so a
+/// draft's id can be chosen before its dialog opens, which is what lets an attachment
+/// picked mid-dialog be filed under the id the node will actually carry.
 pub fn generateDraftId(buffer: *[36]u8) void {
-    const timestamp: u64 = @intCast(std.time.nanoTimestamp());
-    const sequence = draft_id_sequence.fetchAdd(1, .monotonic);
-    _ = std.fmt.bufPrint(
-        buffer,
-        "00000000-0000-4000-8000-{x:0>12}",
-        .{(timestamp ^ sequence) & 0xffffffffffff},
-    ) catch unreachable;
+    var bytes: [16]u8 = undefined;
+    std.crypto.random.bytes(&bytes);
+    bytes[6] = (bytes[6] & 0x0f) | 0x40;
+    bytes[8] = (bytes[8] & 0x3f) | 0x80;
+    const hex = std.fmt.bytesToHex(bytes, .upper);
+    @memcpy(buffer[0..8], hex[0..8]);
+    buffer[8] = '-';
+    @memcpy(buffer[9..13], hex[8..12]);
+    buffer[13] = '-';
+    @memcpy(buffer[14..18], hex[12..16]);
+    buffer[18] = '-';
+    @memcpy(buffer[19..23], hex[16..20]);
+    buffer[23] = '-';
+    @memcpy(buffer[24..36], hex[20..32]);
 }
 
 pub const EdgeDraft = struct {
@@ -867,6 +868,27 @@ test "generateDraftId produces a version-4-shaped, distinct id each call" {
     try std.testing.expect(!std.mem.eql(u8, &first, &second));
 }
 
+test "generateDraftId produces random RFC 4122 version-4 ids, not a zero-filled prefix" {
+    // Dev Box beta17 created a loop whose id was `00000000-0000-4000-8000-AA395289111D`:
+    // 74 of 122 random bits were constant, leaving only a timestamp to tell ids apart.
+    const samples = 32;
+    var ids: [samples][36]u8 = undefined;
+    for (&ids) |*id| generateDraftId(id);
+    for (ids, 0..) |id, index| {
+        try std.testing.expect(isUuid(&id));
+        try std.testing.expectEqual(@as(u8, '4'), id[14]);
+        try std.testing.expect(std.mem.indexOfScalar(u8, "89ABab", id[19]) != null);
+        // Same case as Swift `UUID().uuidString`, which is what the daemon echoes back.
+        for (id) |byte| try std.testing.expect(!std.ascii.isLower(byte));
+        try std.testing.expect(!std.mem.startsWith(u8, &id, "00000000-0000-4000-8000-"));
+        for (ids[0..index]) |earlier| {
+            // The leading 32 bits alone must already differ between ids; a collision
+            // among 32 random samples has probability below 2^-22.
+            try std.testing.expect(!std.mem.eql(u8, earlier[0..8], id[0..8]));
+        }
+    }
+}
+
 test "node updates preserve unchanged fields and allow stall clear sentinel" {
     try validateNodeUpdate(.{ .stall_after_seconds = 0 });
     try std.testing.expectError(error.InvalidGoal, validateNodeUpdate(.{ .poll_interval_seconds = 0 }));

graphcode-windows/src/WindowsNativeDialogs.zig

@@ -29,6 +29,9 @@ const State = struct {
     closed: bool = false,
     failure: ?anyerror = null,
     button_y: i32 = 565,
+    /// The control that owns keyboard focus whenever the dialog is active: the first
+    /// field on open, then whichever child the user left focused when it deactivated.
+    focus: c.HWND = null,
 };
 
 const class_name = std.unicode.utf8ToUtf16LeStringLiteral("GraphCodeWindowsDialog");
@@ -89,27 +92,13 @@ pub fn textWithDescription(
     active_state.closed = false;
     active_state.accepted = false;
     active = true;
-    const hwnd = c.CreateWindowExW(
-        c.WS_EX_DLGMODALFRAME | c.WS_EX_CONTROLPARENT,
-        class_name.ptr,
-        wide_title.ptr,
-        c.WS_OVERLAPPED | c.WS_CAPTION | c.WS_SYSMENU | c.WS_VSCROLL,
-        c.CW_USEDEFAULT,
-        c.CW_USEDEFAULT,
-        600,
-        window_height,
-        parent,
-        null,
-        c.GetModuleHandleW(null),
-        null,
-    ) orelse {
+    const hwnd = createDialogWindow(parent, wide_title, window_height) orelse {
         freeStateValues(&active_state);
         active = false;
         return error.DialogCreationFailed;
     };
     _ = c.EnableWindow(parent, 0);
-    _ = c.ShowWindow(hwnd, c.SW_SHOW);
-    _ = c.SetForegroundWindow(hwnd);
+    presentDialog(hwnd);
     var message: c.MSG = undefined;
     while (!active_state.closed) {
         const code = c.GetMessageW(&message, null, 0, 0);
@@ -126,6 +115,29 @@ pub fn textWithDescription(
     return finishText(&active_state);
 }
 
+fn createDialogWindow(parent: c.HWND, wide_title: []const u16, window_height: i32) c.HWND {
+    return c.CreateWindowExW(
+        c.WS_EX_DLGMODALFRAME | c.WS_EX_CONTROLPARENT,
+        class_name.ptr,
+        wide_title.ptr,
+        c.WS_OVERLAPPED | c.WS_CAPTION | c.WS_SYSMENU | c.WS_VSCROLL,
+        c.CW_USEDEFAULT,
+        c.CW_USEDEFAULT,
+        600,
+        window_height,
+        parent,
+        null,
+        c.GetModuleHandleW(null),
+        null,
+    );
+}
+
+fn presentDialog(hwnd: c.HWND) void {
+    _ = c.ShowWindow(hwnd, c.SW_SHOW);
+    _ = c.SetForegroundWindow(hwnd);
+    if (active_state.focus) |target| _ = c.SetFocus(target);
+}
+
 fn finishText(state: *State) !?Result {
     if (state.failure) |err| {
         freeStateValues(state);
@@ -184,6 +196,26 @@ fn windowProc(hwnd: c.HWND, message: c.UINT, wparam: c.WPARAM, lparam: c.LPARAM)
             }
             createButton(hwnd, "OK", ok_id, 490, active_state.button_y);
             createButton(hwnd, "Cancel", cancel_id, 400, active_state.button_y);
+            if (active_state.edits[0]) |first| {
+                _ = c.SendMessageW(first, c.EM_SETSEL, 0, -1);
+                active_state.focus = first;
+            }
+            return 0;
+        },
+        // This is a plain window, not a dialog-manager dialog, so nothing hands focus to
+        // a control on its own: DefWindowProc's activation leaves it on the frame, where
+        // typing goes nowhere. Route it to the remembered control instead.
+        c.WM_ACTIVATE => {
+            if ((wparam & 0xffff) == c.WA_INACTIVE) {
+                const current = c.GetFocus();
+                if (current != null and c.IsChild(hwnd, current) != 0) active_state.focus = current;
+            } else if (active_state.focus) |target| {
+                _ = c.SetFocus(target);
+            }
+            return 0;
+        },
+        c.WM_SETFOCUS => {
+            if (active_state.focus) |target| _ = c.SetFocus(target);
             return 0;
         },
         c.WM_VSCROLL => {
@@ -449,3 +481,41 @@ test "workspace text capture and transfer release every partial allocation" {
     };
     try std.testing.checkAllAllocationFailures(std.testing.allocator, Probe.run, .{edit});
 }
+
+test "text dialog opens with keyboard focus in its first field, text selected, and keeps it across activation" {
+    // Dev Box beta17: Rename Loop opened with focus on the dialog frame, so typing did
+    // nothing until the Title edit was clicked.
+    const allocator = std.testing.allocator;
+    const previous_active = active;
+    active_state = State{ .allocator = allocator, .parent = null, .count = 1, .description = "Shown on the loop card." };
+    active_state.labels[0] = "Title";
+    active_state.values[0] = try allocator.dupe(u8, "GCQCrud17");
+    active = true;
+    defer {
+        freeStateValues(&active_state);
+        active = previous_active;
+    }
+    try registerClass();
+    const wide_title = try wideZ(allocator, "Rename Loop");
+    defer allocator.free(wide_title);
+    const hwnd = createDialogWindow(null, wide_title, 220) orelse return error.TestWindowCreationFailed;
+    defer _ = c.DestroyWindow(hwnd);
+    const title_edit = active_state.edits[0] orelse return error.TestWindowCreationFailed;
+    try std.testing.expectEqual(@as(isize, 9904), c.GetDlgCtrlID(title_edit));
+
+    presentDialog(hwnd);
+    try std.testing.expectEqual(title_edit, c.GetFocus());
+    var start: c.DWORD = 0;
+    var end: c.DWORD = 0;
+    _ = c.SendMessageW(title_edit, c.EM_GETSEL, @intFromPtr(&start), @bitCast(@intFromPtr(&end)));
+    try std.testing.expectEqual(@as(c.DWORD, 0), start);
+    try std.testing.expectEqual(@as(c.DWORD, "GCQCrud17".len), end);
+
+    // Activation hands focus to the frame (DefWindowProc's WM_ACTIVATE does exactly
+    // that); the frame must pass it straight back to the field the user was in.
+    _ = c.SendMessageW(hwnd, c.WM_ACTIVATE, c.WA_INACTIVE, 0);
+    _ = c.SetFocus(hwnd);
+    try std.testing.expectEqual(title_edit, c.GetFocus());
+    _ = c.SendMessageW(hwnd, c.WM_ACTIVATE, c.WA_ACTIVE, 0);
+    try std.testing.expectEqual(title_edit, c.GetFocus());
+}