Claude finished @Yeraze's task —— View job

PR Review

• Gather context
• Read changed workflow files
• Analyze fix for correctness and best practices
• Post review feedback

The fix is well-reasoned and correctly addresses the root cause. The install-artifact-from-github vs prebuild-install behavioral difference (reading process.arch instead of npm_config_arch) is real and this approach is the right workaround. Here are my observations:

What's Good

Correct diagnosis and fix. Deleting the wrong-arch re2.node and rebuilding from source using Apple's native cross-compilation (-arch x86_64) is the right approach for packages that bypass the npm arch env vars. The --arch=x64 flag to node-gyp plus the CFLAGS/CXXFLAGS/LDFLAGS trio covers both the compiler and linker.

Verification step. The file desktop/resources/dist/node_modules/re2/build/Release/re2.node echo + file output at the end of the step is excellent for debugging CI logs — it makes it immediately visible if the binary is the wrong arch.

Consistent application. The fix is applied to both desktop-ci.yml (lines 348–353) and desktop-release.yml (lines 738–743), which is necessary for both jobs to produce correct x64 builds.

Missing env vars fixed. Adding npm_config_arch=x64, npm_config_target_arch=x64, npm_config_platform=darwin, and npm_config_target_platform=darwin to desktop-ci.yml's build-macos-x64 job (lines 337–342) closes the gap with desktop-release.yml that had these for other packages.

Minor Issues / Observations

node-gyp path assumption (low risk, worth noting). The rebuild uses ../.bin/node-gyp (i.e., node_modules/.bin/node-gyp relative to the re2 package directory). This assumes node-gyp is installed as a direct dev dependency of the root project, which is typically true since node-pre-gyp and similar tools bring it in. If the dependency tree changes such that it's hoisted differently, this path could break. A more robust alternative would be npx node-gyp or an explicit $(npm root)/.bin/node-gyp, but since this has worked in node_modules layout for years it's unlikely to be a practical problem.

Re2 rebuild happens inline in "Prepare desktop resources". This means the file output confirming x86_64 appears in the same large step, not in its own named step. For CI observability, a dedicated step like "Rebuild re2 for x64" would make it easier to spot at a glance. Not blocking, just a UX suggestion.

desktop-ci.yml build-macos-x64 uses macos-14 explicitly (line 257) while build-macos uses macos-latest (line 141). This is intentional (macos-14 = arm64 for cross-compilation) and correct — worth keeping as-is.

No retry or error handling if node-gyp rebuild fails. If the rebuild fails (e.g., missing Xcode SDK for x86_64), the step will fail with a somewhat cryptic error. A || (echo "re2 rebuild failed"; exit 1) with a clear message could improve diagnostics — though this is a very minor enhancement.

Security

No concerns. The change only affects build tooling and does not modify application code, introduce new dependencies, or expose any credentials.

Test Plan Assessment

The proposed test plan is appropriate: verifying the file output in the build log (confirming x86_64) and testing on an actual Intel Mac. The build log already emits this via the echo "re2.node architecture after rebuild:" + file command, so half the test plan is automated.

Summary

This is a clean, targeted fix for a real cross-compilation issue with a tricky root cause. The only substantive suggestion is the node-gyp path robustness concern, which is low risk in practice. The PR is ready to merge.