From 7f6d71fd9f25344b38f0b8a206914269e9d7748a Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Mon, 5 Oct 2026 16:31:31 -0700 Subject: [PATCH] Accept the daemon's canonical reply for the shell's own folder open The native folder picker returns a backslash path such as C:\a. The production daemon canonicalizes it and answers the openProject request with C:/a, echoing the request ID in uppercase. The shell compared both the path and the request ID byte-for-byte, so it discarded the reply and kept the pending open forever: the project registered in the daemon but never appeared in the shell (beta7 ProductionCoreProjectVisibility). The same uppercase echo meant v2 open rejections were never correlated either. Adopt the daemon's canonical path when the reply is provably for the shell's own in-flight open (case-insensitive request ID on v2, separator/case path equivalence on v1), compare request IDs case-insensitively, assert the daemon contract in the real-daemon round trip, and run that round trip in Windows shell validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- Tools/windows/Tests/WindowsShell.Tests.ps1 | 5 + Tools/windows/validate.ps1 | 8 ++ graphcode-windows/src/App.zig | 106 +++++++++++++++++- .../src/DaemonRoundTripTests.zig | 23 +++- graphcode-windows/src/Wire.zig | 19 ++++ 5 files changed, 158 insertions(+), 3 deletions(-) diff --git a/Tools/windows/Tests/WindowsShell.Tests.ps1 b/Tools/windows/Tests/WindowsShell.Tests.ps1 index 1cadb0c3..5a6fe1e8 100644 --- a/Tools/windows/Tests/WindowsShell.Tests.ps1 +++ b/Tools/windows/Tests/WindowsShell.Tests.ps1 @@ -257,6 +257,11 @@ Assert-Contract ($scrubbedStartupSource -match 'Invoke-Case "registered-project" $daemonRoundTripSource -notmatch 'modelCompatibleFrame' -and $daemonRoundTripSource -match 'self\.model\.updateFromFrame\(frame\)') ` "production daemon graph frames must reach the shell model unmodified and render a registered project" +Assert-Contract ($validationRunnerSource -match + '(?s)Scrubbed production shell startup.*?Real daemon wire round trip.*?& pwsh -NoProfile -File.*?DaemonRoundTrip\.Live\.Tests\.ps1.*?Native UI Automation live gate' -and + $daemonRoundTripSource -match 'DAEMON_OPEN_CANONICAL' -and + $daemonRoundTripSource -match 'eqlIgnoreCase\(&token') ` + "Windows shell validation must exercise the production daemon wire contract, including canonical open replies" $menuTimerBlock = [regex]::Match( $appSource, '(?s)else if \(wparam == MainWindow\.timer_id\) \{.*?const updated_connection_state' diff --git a/Tools/windows/validate.ps1 b/Tools/windows/validate.ps1 index 3a250488..5432272d 100644 --- a/Tools/windows/validate.ps1 +++ b/Tools/windows/validate.ps1 @@ -1209,6 +1209,14 @@ function Invoke-Task([string] $name) { -Cli (Join-Path $daemonRuntime "graphcode.exe") ` -ScratchRoot (Join-Path $env:TEMP "scrubbed-shell-startup") } + Invoke-Native "Real daemon wire round trip" { + & pwsh -NoProfile -File ` + (Join-Path $repoRoot "Tools\windows\Tests\DaemonRoundTrip.Live.Tests.ps1") ` + -DaemonExecutable (Join-Path $daemonRuntime "graphcoded.exe") ` + -ZigExecutable $zig0152 ` + -WinghosttyInclude (Join-Path $winghosttyRoot "include") ` + -ScratchRoot (Join-Path $env:TEMP "daemon-roundtrip") + } & (Join-Path $repoRoot "Tools\windows\Tests\TrayDaemon.Tests.ps1") ` -Executable (Join-Path $repoRoot "graphcode-windows\zig-out\bin\graphcode-windows.exe") if ($LASTEXITCODE -ne 0) { diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index 27ad3380..36c68a42 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -1201,6 +1201,38 @@ pub const App = struct { return app; } + /// The daemon answers an open with its canonical spelling of the project path + /// (for example `C:/a` for a picker's `C:\a`). When the reply is provably for the + /// shell's own in-flight open, adopt that spelling so the reply is not discarded. + fn adoptCanonicalOpenPath(self: *App, frame: []const u8, canonical: []const u8) void { + if (!self.pending_open_sent or self.pending_sent_path.len == 0) return; + if (std.mem.eql(u8, canonical, self.pending_sent_path)) return; + const owned = switch (self.client.protocolMode()) { + .v2 => blk: { + const request_id = self.pending_open_request_id orelse break :blk false; + const response_id = Wire.responseRequestID(frame) orelse break :blk false; + break :blk std.ascii.eqlIgnoreCase(response_id, &request_id); + }, + .v1 => Wire.sameLocalProjectPath(canonical, self.pending_sent_path), + }; + if (!owned) return; + const rebind_is_sent = std.mem.eql(u8, self.pending_rebind_path, self.pending_sent_path); + const sent = self.allocator.dupe(u8, canonical) catch return; + const rebind: ?[]u8 = if (rebind_is_sent) + (self.allocator.dupe(u8, canonical) catch { + self.allocator.free(sent); + return; + }) + else + null; + self.allocator.free(self.pending_sent_path); + self.pending_sent_path = sent; + if (rebind) |value| { + self.allocator.free(self.pending_rebind_path); + self.pending_rebind_path = value; + } + } + fn sendPendingOpen(self: *App) void { if (self.pending_rebind_path.len == 0 or self.client.connectionState() != .connected or @@ -1485,6 +1517,7 @@ pub const App = struct { const path = Wire.copyGraphChangedProjectPath(self.allocator, frame) catch null; if (path) |value| { incoming_project_path = value; + self.adoptCanonicalOpenPath(frame, value); if (self.client.protocolMode() == .v1 and self.pending_open_sent) { if (!std.mem.eql(u8, value, self.pending_sent_path) and !std.mem.eql(u8, value, self.accepted_subscription)) return; @@ -1611,7 +1644,8 @@ pub const App = struct { if (self.client.protocolMode() == .v2) { const request_id = self.pending_open_request_id orelse return; const response_id = Wire.responseRequestID(frame) orelse return; - if (!std.mem.eql(u8, response_id, &request_id)) return; + // The daemon echoes the UUID in uppercase. + if (!std.ascii.eqlIgnoreCase(response_id, &request_id)) return; } else if (!self.pending_open_sent) { return; } @@ -10350,6 +10384,76 @@ test "graph publication v1 queued opens accept the sent owner before the newest try F.expectPublished(); } +test "graph publication adopts the daemon canonical path for the shell's own folder open" { + const F = GraphPublicationTest; + for ([_]Wire.ProtocolMode{ .v1, .v2 }) |mode| { + var app = try F.init(mode); + defer F.deinit(&app); + try F.seed(&app); + // The native folder picker returns a backslash path; the production daemon + // replies with its canonical forward-slash spelling of the same folder. + app.openProjectWithLayout("C:\\Fixtures\\Core", F.layout); + app.client.state = .connected; + app.sendPendingOpen(); + try std.testing.expect(app.pending_open_sent); + const request = app.pending_open_request_id; + try F.takeOpen(&app, "C:\\Fixtures\\Core"); + if (mode == .v2) { + // An uncorrelated publication for the canonical spelling is not adopted. + app.onFrameWithEffects( + \\{"version":2,"kind":"event","sequence":4,"event":{"graphChanged":{"_0":{"project":{"path":"C:/Fixtures/Core","name":"Core"},"nodes":[],"edges":[]}}}} + , F.rebind, F.refresh, F.publish); + try std.testing.expect(app.model.graphFor("C:/Fixtures/Core") == null); + try std.testing.expectEqualStrings("C:\\Fixtures\\Core", app.pending_rebind_path); + } + const reply = if (mode == .v2) + try std.fmt.allocPrint(app.allocator, "{{\"version\":2,\"kind\":\"response\",\"requestID\":\"{s}\",\"event\":{{\"graphChanged\":{{\"_0\":{{\"project\":{{\"path\":\"C:/Fixtures/Core\",\"name\":\"Core\"}},\"nodes\":[],\"edges\":[]}}}}}}}}", .{&upperRequestID(request.?)}) + else + try app.allocator.dupe(u8, + \\{"version":1,"kind":"event","event":{"graphChanged":{"_0":{"project":{"path":"C:/Fixtures/Core","name":"Core"},"nodes":[],"edges":[]}}}} + ); + defer app.allocator.free(reply); + app.onFrameWithEffects(reply, F.rebind, F.refresh, F.publish); + const current = app.model.currentGraph() orelse return error.FolderOpenGraphDropped; + try std.testing.expectEqualStrings("C:/Fixtures/Core", current.project.path); + try std.testing.expectEqualStrings("Core", current.project.name); + try std.testing.expectEqualStrings("C:/Fixtures/Core", app.accepted_subscription); + try std.testing.expectEqualStrings("C:/Fixtures/Core", app.client.subscription_path); + try std.testing.expectEqualStrings("C:/Fixtures/Core", app.last_project_opened); + try std.testing.expectEqual(@as(usize, 0), app.pending_rebind_path.len); + try std.testing.expectEqual(@as(usize, 0), app.pending_sent_path.len); + try std.testing.expect(!app.pending_open_sent and !app.open_project_pending); + try std.testing.expectEqual(@as(usize, 0), app.ingress_error.len); + } +} + +/// Swift's `UUID.uuidString` echoes request IDs in uppercase. +fn upperRequestID(request: [36]u8) [36]u8 { + var upper: [36]u8 = undefined; + for (request, 0..) |byte, index| upper[index] = std.ascii.toUpper(byte); + return upper; +} + +test "graph publication v2 open rejection correlates the daemon's uppercase request ID" { + const F = GraphPublicationTest; + var app = try F.init(.v2); + defer F.deinit(&app); + try F.seed(&app); + app.openProjectWithLayout("B", F.layout); + app.client.state = .connected; + app.sendPendingOpen(); + const request = app.pending_open_request_id.?; + try F.takeOpen(&app, "B"); + try std.testing.expect(!std.mem.eql(u8, &request, &upperRequestID(request))); + const rejection = try std.fmt.allocPrint(app.allocator, "{{\"version\":2,\"kind\":\"response\",\"requestID\":\"{s}\",\"event\":{{\"errorOccurred\":\"B rejected\"}}}}", .{&upperRequestID(request)}); + defer app.allocator.free(rejection); + app.onFrameWithEffects(rejection, F.rebind, F.refresh, F.publish); + try std.testing.expectEqual(@as(usize, 0), app.pending_rebind_path.len); + try std.testing.expect(!app.pending_open_sent and !app.open_project_pending); + try std.testing.expectEqualStrings("A", app.accepted_subscription); + try std.testing.expectEqualStrings("B rejected", app.ingress_error); +} + test "graph publication v2 superseded opens reject stale graphs and errors without losing latest intent" { const F = GraphPublicationTest; var app = try F.init(.v2); diff --git a/graphcode-windows/src/DaemonRoundTripTests.zig b/graphcode-windows/src/DaemonRoundTripTests.zig index 3d328165..8af41ca1 100644 --- a/graphcode-windows/src/DaemonRoundTripTests.zig +++ b/graphcode-windows/src/DaemonRoundTripTests.zig @@ -56,6 +56,10 @@ const Probe = struct { last_kind: Wire.EventKind = .unknown, invalid_response: bool = false, model_error: bool = false, + last_request_id: [36]u8 = undefined, + last_request_id_len: usize = 0, + last_response_path: [512]u8 = undefined, + last_response_path_len: usize = 0, fn init() Probe { return .{ .model = GraphModel.Model.init(allocator) }; @@ -64,9 +68,17 @@ const Probe = struct { fn receive(context: ?*anyopaque, frame_ptr: [*]const u8, length: usize) callconv(.c) void { const self: *Probe = @ptrCast(@alignCast(context orelse return)); const frame = frame_ptr[0..length]; - if (Wire.responseRequestID(frame) != null) { + if (Wire.responseRequestID(frame)) |request_id| { self.response_count += 1; self.last_kind = Wire.eventKind(frame); + self.last_request_id_len = @min(request_id.len, self.last_request_id.len); + @memcpy(self.last_request_id[0..self.last_request_id_len], request_id[0..self.last_request_id_len]); + self.last_response_path_len = 0; + if (Wire.copyGraphChangedProjectPath(std.heap.page_allocator, frame) catch null) |path| { + defer std.heap.page_allocator.free(path); + self.last_response_path_len = @min(path.len, self.last_response_path.len); + @memcpy(self.last_response_path[0..self.last_response_path_len], path[0..self.last_response_path_len]); + } const parsed = std.json.parseFromSlice( std.json.Value, std.heap.page_allocator, @@ -141,11 +153,18 @@ fn connectProject(client: *DaemonClient, probe: *Probe, path: []const u8) !void } const before = probe.response_count; - if (client.sendOpenProject(path) == null) return error.OpenProjectQueueRejected; + const token = client.sendOpenProject(path) orelse return error.OpenProjectQueueRejected; try waitForAcceptedResponse(client, probe, before); try std.testing.expect(!probe.model_error); const canonical_path = try std.mem.replaceOwned(u8, allocator, path, "\\", "/"); defer allocator.free(canonical_path); + // The shell's folder open sends the picker's backslash spelling; the daemon + // answers that exact request with its canonical forward-slash spelling. + try std.testing.expect(std.mem.indexOfScalar(u8, path, '\\') != null); + try std.testing.expect(std.ascii.eqlIgnoreCase(&token, probe.last_request_id[0..probe.last_request_id_len])); + try std.testing.expectEqualStrings(canonical_path, probe.last_response_path[0..probe.last_response_path_len]); + try std.testing.expect(!std.mem.eql(u8, path, canonical_path)); + std.debug.print("DAEMON_OPEN_CANONICAL: request correlated; picker spelling differs from daemon canonical path\n", .{}); try std.testing.expectEqualStrings(canonical_path, probe.model.currentGraph().?.project.path); const expected_name = std.fs.path.basename(canonical_path); try std.testing.expect(expected_name.len != 0); diff --git a/graphcode-windows/src/Wire.zig b/graphcode-windows/src/Wire.zig index 7ab8e0f3..e66b3f0d 100644 --- a/graphcode-windows/src/Wire.zig +++ b/graphcode-windows/src/Wire.zig @@ -1120,6 +1120,25 @@ pub fn isCurrentGraphPath(pending: []const u8, accepted: []const u8, path: []con return pending.len == 0 or std.mem.eql(u8, path, pending) or std.mem.eql(u8, path, accepted); } +/// Whether two local Windows project paths name the same folder spelling apart from +/// separator choice and ASCII case, as the daemon canonicalizes `C:\a` to `C:/a`. +pub fn sameLocalProjectPath(a: []const u8, b: []const u8) bool { + if (a.len != b.len) return false; + for (a, b) |left, right| { + const l = if (left == '\\') '/' else std.ascii.toLower(left); + const r = if (right == '\\') '/' else std.ascii.toLower(right); + if (l != r) return false; + } + return true; +} + +test "local project path equivalence ignores separators and ASCII case only" { + try std.testing.expect(sameLocalProjectPath("C:\\GraphCode-Fixtures\\Core", "C:/GraphCode-Fixtures/Core")); + try std.testing.expect(sameLocalProjectPath("c:\\fixtures\\core", "C:/Fixtures/Core")); + try std.testing.expect(!sameLocalProjectPath("C:\\Fixtures\\Core", "C:/Fixtures/Core2")); + try std.testing.expect(!sameLocalProjectPath("C:\\Fixtures\\Core", "D:/Fixtures/Core")); +} + pub fn jsonNumber(data: []const u8, key: []const u8) ?u64 { var needle_buffer: [128]u8 = undefined; if (key.len + 3 > needle_buffer.len) return null;