Fix: rewrite relative .path deps when copying local libs (#129) by apotema · Pull Request #130 · labelle-toolkit/labelle-cli

apotema · 2026-04-13T13:34:53Z

Summary

Mirror of labelle-toolkit/labelle-assembler PR — same fix applied to the CLI's bundled generator/ until the CLI migrates to use labelle-assembler as a dependency
When the assembler copies local libs into .labelle/deps/, it now rewrites relative .path dependencies in build.zig.zon so they resolve correctly from the new location
Only local plugins are affected — remote plugins, framework packages, backends, and ECS are untouched

Closes #129

Test plan

7 new unit tests: computeRelativePath, normalizePath, rewriteZonPaths (rewrite + skip)
All existing tests pass
Verified flying-platform-labelle game runs correctly with the change

🤖 Generated with Claude Code

cursor · 2026-04-13T13:35:01Z

PR Summary

Medium Risk
Touches dependency packaging/linking and mutates build.zig.zon contents, so mistakes could break builds for local plugins/GUI deps. Scope is limited to rewriting relative .path entries and is covered by new unit tests.

Overview
Fixes local dependency resolution after copying packages into .labelle/deps/ by rewriting relative ZON .path dependencies to be correct from the new hardlinked destination.

Adds rewriteZonPaths (temp-file + rename to avoid modifying the original hardlinked file), computeRelativePath (cross-platform relative path computation + Windows slash normalization), and hooks this rewrite step for local plugins plus the optional GUI plugin/bridge. Includes unit tests covering relative path computation and ZON rewriting/skip behavior.

^{Reviewed by Cursor Bugbot for commit 0b1e70b. Bugbot is set up for automated code reviews on this repo. Configure here.}

gemini-code-assist

Code Review

This pull request introduces logic to rewrite relative .path dependencies in build.zig.zon files after they have been hardlinked to the .labelle/deps/ directory, ensuring local plugins maintain correct references. The review feedback highlights several significant issues: a potential risk of truncating original source files due to unsafe handling of hardlink deletion, the use of POSIX-specific path logic that breaks Windows compatibility, and silent error suppression. It is recommended to use standard library functions like std.fs.path.relative for cross-platform path manipulation and to ensure errors are properly propagated.

gemini-code-assist · 2026-04-13T13:37:42Z

+        cwd.deleteFile(zon_path) catch {};
+        const file = try cwd.createFile(zon_path, .{});


Ignoring errors from deleteFile is dangerous. Because zon_path is a hardlink to the original source file, if deleteFile fails (e.g., due to a sharing violation on Windows) and the code proceeds to createFile, it will truncate and overwrite the original source file. You should ensure the hardlink is successfully removed before creating a new file, while still allowing for the case where the file doesn't exist.

cwd.deleteFile(zon_path) catch |err| switch (err) { error.FileNotFound => {}, else => return err, }; const file = try cwd.createFile(zon_path, .{});

gemini-code-assist · 2026-04-13T13:37:42Z

+fn computeRelativePath(allocator: std.mem.Allocator, from_dir: []const u8, to_path: []const u8) ![]u8 {
+    // Normalize both paths by resolving . and ..
+    const from_norm = try normalizePath(allocator, from_dir);
+    defer allocator.free(from_norm);
+    const to_norm = try normalizePath(allocator, to_path);
+    defer allocator.free(to_norm);
+
+    // Split into components
+    var from_parts = std.ArrayList([]const u8){};
+    defer from_parts.deinit(allocator);
+    var to_parts = std.ArrayList([]const u8){};
+    defer to_parts.deinit(allocator);
+
+    var from_iter = std.mem.splitScalar(u8, from_norm, '/');
+    while (from_iter.next()) |part| {
+        if (part.len > 0) try from_parts.append(allocator, part);
+    }
+    var to_iter = std.mem.splitScalar(u8, to_norm, '/');
+    while (to_iter.next()) |part| {
+        if (part.len > 0) try to_parts.append(allocator, part);
+    }
+
+    // Find common prefix length
+    var common: usize = 0;
+    while (common < from_parts.items.len and common < to_parts.items.len) {
+        if (!std.mem.eql(u8, from_parts.items[common], to_parts.items[common])) break;
+        common += 1;
+    }
+
+    // Build relative path: go up from `from` then down to `to`
+    var rel = std.ArrayList(u8){};
+    const ups = from_parts.items.len - common;
+    for (0..ups) |j| {
+        if (j > 0) try rel.append(allocator, '/');
+        try rel.appendSlice(allocator, "..");
+    }
+    for (common..to_parts.items.len) |j| {
+        if (rel.items.len > 0) try rel.append(allocator, '/');
+        try rel.appendSlice(allocator, to_parts.items[j]);
+    }
+
+    if (rel.items.len == 0) {
+        try rel.append(allocator, '.');
+    }
+
+    return rel.toOwnedSlice(allocator);
+}


This manual implementation of relative path computation is POSIX-centric (hardcoding / as a separator and assuming absolute paths start with /) and will fail on Windows. Since the project requires Zig 0.15.2, you should use the standard library's std.fs.path.relative, which is cross-platform and handles edge cases correctly.

fn computeRelativePath(allocator: std.mem.Allocator, from_dir: []const u8, to_path: []const u8) ![]u8 { return std.fs.path.relative(allocator, from_dir, to_path); }

gemini-code-assist · 2026-04-13T13:37:42Z

+    const zon_path = try std.fs.path.join(allocator, &.{ dest_dir, "build.zig.zon" });
+    defer allocator.free(zon_path);
+
+    const content = std.fs.cwd().readFileAlloc(allocator, zon_path, 256 * 1024) catch return;


Returning silently when readFileAlloc fails can make troubleshooting difficult. Since this file was just hardlinked in the previous step, an error here indicates a significant issue (e.g., I/O error or permission change) that should be propagated to the caller.

const content = try std.fs.cwd().readFileAlloc(allocator, zon_path, 256 * 1024);

gemini-code-assist · 2026-04-13T13:37:42Z

+                    try result.appendSlice(allocator, content[prefix_start..][0..8]); // .path =
+                    // re-append any whitespace between = and "
+                    try result.append(allocator, '"');
+                    try result.appendSlice(allocator, new_rel);
+                    try result.append(allocator, '"');


The code does not preserve the whitespace between the assignment operator and the opening quote, which contradicts the comment on line 268. This results in lost formatting in the build.zig.zon file if it uses non-standard spacing (e.g., multiple spaces or tabs).

gemini-code-assist · 2026-04-13T13:37:42Z

+fn normalizePath(allocator: std.mem.Allocator, path: []const u8) ![]u8 {
+    var parts = std.ArrayList([]const u8){};
+    defer parts.deinit(allocator);
+
+    var iter = std.mem.splitScalar(u8, path, '/');
+    while (iter.next()) |part| {
+        if (part.len == 0 or std.mem.eql(u8, part, ".")) continue;
+        if (std.mem.eql(u8, part, "..")) {
+            if (parts.items.len > 0) {
+                _ = parts.pop();
+            }
+        } else {
+            try parts.append(allocator, part);
+        }
+    }
+
+    var result = std.ArrayList(u8){};
+    if (path.len > 0 and path[0] == '/') {
+        try result.append(allocator, '/');
+    }
+    for (parts.items, 0..) |part, j| {
+        if (j > 0) try result.append(allocator, '/');
+        try result.appendSlice(allocator, part);
+    }
+
+    return result.toOwnedSlice(allocator);
+}


This function is redundant and contains POSIX-specific logic (e.g., line 365) that breaks on Windows. It can be removed entirely if computeRelativePath is updated to use std.fs.path.relative.

…deps/ (#129) Local plugins with relative .path dependencies in build.zig.zon would break after being hardlinked into .labelle/deps/, because the paths still resolved relative to the original location. Now the assembler rewrites those paths to resolve correctly from the new location. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

apotema · 2026-04-13T13:50:11Z

Superseded by #132 — the bundled generator fallback is being removed, so the fix only needs to live in labelle-toolkit/labelle-assembler#1.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0b1e70b. Configure here.}

cursor · 2026-04-13T13:56:30Z

+        try rewriteLocalDep(allocator, cwd, gui.plugin_dir, deps_dir, "labelle-gui");
+        if (gui.bridge_dir) |bd|
+            try rewriteLocalDep(allocator, cwd, bd, deps_dir, "gui-bridge");
+    }


GUI rewriting lacks locality check unlike plugin loop

Medium Severity

The plugin loop correctly gates rewriting with if (!plugin.isLocal()) continue;, but the GUI block at lines 134–138 calls rewriteLocalDep unconditionally for all resolved GUIs. When a GUI references a remote plugin via .plugin = "name" (supported by resolvePluginDir in gui_resolve.zig), gui.plugin_dir is the cache path. If that remote package has .path entries, rewriteZonPaths resolves them against the cache directory and computes relative paths from .labelle/deps/ back to the cache — producing incorrect, fragile paths instead of deps-relative ones. The PR description states "only local plugins are affected" but this isn't enforced for GUI.

Additional Locations (1)

generator/src/deps_linker.zig#L109-L111

^{Reviewed by Cursor Bugbot for commit 0b1e70b. Configure here.}

apotema force-pushed the fix/129-rewrite-local-dep-paths branch from d3b76fc to 032e9b1 Compare April 13, 2026 13:35

gemini-code-assist Bot reviewed Apr 13, 2026

View reviewed changes

apotema force-pushed the fix/129-rewrite-local-dep-paths branch from 032e9b1 to 0b1e70b Compare April 13, 2026 13:46

apotema closed this Apr 13, 2026

apotema deleted the fix/129-rewrite-local-dep-paths branch April 13, 2026 13:50

cursor Bot reviewed Apr 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix: rewrite relative .path deps when copying local libs (#129) - #130

Fix: rewrite relative .path deps when copying local libs (#129)#130
apotema wants to merge 1 commit into
mainfrom
fix/129-rewrite-local-dep-paths

apotema commented Apr 13, 2026

Uh oh!

cursor Bot commented Apr 13, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

apotema commented Apr 13, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		cwd.deleteFile(zon_path) catch {};
		const file = try cwd.createFile(zon_path, .{});

Uh oh!

Conversation

apotema commented Apr 13, 2026

Summary

Test plan

Uh oh!

cursor Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

apotema commented Apr 13, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Apr 13, 2026

Choose a reason for hiding this comment

GUI rewriting lacks locality check unlike plugin loop

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cursor Bot commented Apr 13, 2026 •

edited

Loading