fix(cli): restore Intel macOS background removal - #2480

Reviewed at 09a5ce74.

onnxruntime-node is pinned from ^1.20.0 to exactly 1.23.2 — the last release that ships a darwin/x64 native binding (1.26.0 dropped it). Regression test locks the version and verifies the Intel macOS binary is present after install.

Blockers

(none)

Concerns

(none)

Nits

🟡 Exact pin blocks patch-level security updates. "onnxruntime-node": "1.23.2" (no caret / tilde) freezes the version until someone manually bumps. A hypothetical 1.23.3 with a security fix wouldn't auto-flow. Understandable given the 1.26.0 darwin/x64 drop bug, but worth documenting the pin's semantics in the package.json comment or a SECURITY.md note so a future dependency-bump automation doesn't accidentally range-relax it back to ^1.23.2 without checking Intel macOS support first. The dependency-compat.test.ts catches the regression at CI time either way — the note would just add the upstream context.

Green notes

🟢 Regression test locks both surfaces. expect(packageJson.dependencies?.["onnxruntime-node"]).toBe("1.23.2") pins the manifest spelling, and existsSync(join(packageRoot, "bin/napi-v6/darwin/x64/onnxruntime_binding.node")) pins the installed artifact. If a future PR bumps the version to a release that drops the Intel binary again, this fails at test time before any Intel macOS user is affected. ✓

🟢 Transitive-dep downgrade is contained. The [email protected] → 3.0.0 cascade brings in [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected] — all well-known Gajus/npm micro-libs, no suspicious supply-chain shapes. matcher@4→3 + serialize-error@8→7 + [email protected]→0.13.1 follow global-agent's peer requirements; not surprising.

🟢 Dynamic import site (inference.ts:71-72) unchanged. The () => import("onnxruntime-node") deferred load keeps CLI startup unaffected on non-BG-removal invocations, regardless of version pin. ✓

🟢 CI signal. Focused background-removal tests (10/10), Linux native import, and lockfile validation all pass per the PR body — the Linux CI runner's ability to resolve darwin/x64/onnxruntime_binding.node (checked by the new test) confirms the binaries ship together in the npm tarball, not via optionalDependencies. ✓

Verdict framing

Right kind of pin for the failure mode. Regression test surface is exactly the two invariants that need locking (version string + Intel binary presence). LGTM from my side.

— Review by Rames D Jusso

Reviewed at 09a5ce74. Additive to @james-russo-rames-d-jusso's review — Rames covered the pin diagnosis, transitive-dep containment, dynamic-import site (inference.ts:71-72), and the exact-pin/security-patch nit. Not repeating those; adding empirical verification + adversarial angles.

Position: RIGHT. The PR thesis — "1.26.0 dropped darwin/x64 binaries, pin to 1.23.2" — is empirically verifiable from the upstream npm tarballs. Confirmed below.

Strengths (empirical verification of the diagnosis)

• packages/cli/package.json:41 — exact pin "onnxruntime-node": "1.23.2" (not ^1.23.2 / ~1.23.2) is the right shape here. npm tarballs are immutable, so the pin is a stronger contract than the on-disk test alone.
• Upstream tarball contents, verified by tar -tzf:
  • [email protected] → bin/napi-v6/{darwin/x64, darwin/arm64, linux/x64, linux/arm64, win32/x64, win32/arm64}/onnxruntime_binding.node — six arches, complete.
  • [email protected] → same file list minus darwin/x64 (no libonnxruntime.1.26.0.dylib for x64 either). Diagnosis matches upstream reality.
• packages/cli/src/background-removal/dependency-compat.test.ts:11-22 — require.resolve("onnxruntime-node") on main: dist/index.js gives <pkg>/dist/index.js; join(dirname(...), "..") walks back to <pkg>/, then bin/napi-v6/darwin/x64/... resolves correctly. Test math is right and runs on every host (Linux CI included) because the tarball ships all arches regardless of the installing host's platform.

Findings (additive to Rames)

Nits

• Regression test is Intel-macOS-only. dependency-compat.test.ts:19-21 asserts bin/napi-v6/darwin/x64/... exists. The other three arches (darwin/arm64, linux/x64, linux/arm64) are protected only by the exact version pin + npm tarball immutability. Not a coverage gap in practice — a rebuilt-under-same-version tarball is essentially impossible on npm — but if you wanted symmetric coverage, iterating over [["darwin","x64"], ["darwin","arm64"], ["linux","x64"], ["linux","arm64"]] and asserting each binding is one loop away. Optional.
• No postinstall silent-fail path. onnxruntime-node's script/install.js runs on install, but the CPU / CoreML native bindings ship inside the tarball (they're the bin/napi-v6/... files, not fetched at install time — install.js is only for opt-in GPU/DirectML packages). So a network-blocked install can't silently produce a broken darwin/x64 layout. Worth noting as a *non-*concern reviewers might otherwise raise. ✓

Adversarial pass — the questions asked and their answers

• (a) Opset compat. U2Net (from rembg v0.0.0, manager.ts:13) uses opset 11 ops — Conv, Sigmoid, Upsample, MaxPool, BatchNorm. ORT 1.23.2 supports opset ≥ 11 fully. Downgrading from 1.26 to 1.23 does not reduce model op coverage for u2net_human_seg.onnx. ✓
• (b) darwin/arm64 preservation. Confirmed in the 1.23.2 tarball listing above — Apple Silicon was and remains supported. manager.ts:64-72 (hasCoreML && arch === "arm64") path untouched. ✓
• (c) Linux paths. Both linux/x64 and linux/arm64 bindings ship in 1.23.2. CI Test, CLI smoke (required), and Producer: * all green on Ubuntu.

Extrapolation — other native modules with the same gap?

• sharp@^0.34.5 (packages/cli/package.json:47): full @img/sharp-* optionalDependency matrix at 0.34.5 includes @img/sharp-darwin-x64 and @img/sharp-libvips-darwin-x64. No darwin/x64 gap. ✓
• @google/genai@^1.50.1 — pure JS, no native binding.
• puppeteer-core, esbuild, fontkit — either downloaded browsers or well-maintained multi-arch native. No pattern match.

No sibling darwin/x64 gap found in the CLI package.

Verdict: APPROVE
Reasoning: Diagnosis is empirically correct against upstream tarballs; fix mechanism (exact pin + on-disk regression test) locks the right invariants; no arch regressed; transitive downgrades contained inside onnxruntime-node's subtree; CI green including macos-latest + windows-latest CLI shims. LGTM.

— Via