Skip to content

fix(embed): extract macOS ONNX Runtime tarballs (./ prefix)#24

Merged
raymondj99 merged 1 commit into
mainfrom
fix/macos-ort-extraction
May 20, 2026
Merged

fix(embed): extract macOS ONNX Runtime tarballs (./ prefix)#24
raymondj99 merged 1 commit into
mainfrom
fix/macos-ort-extraction

Conversation

@raymondj99
Copy link
Copy Markdown
Owner

@raymondj99 raymondj99 commented May 20, 2026

Summary

  • Primary fix: extract_lib_subtree now strips a leading ./ from each tar entry path before matching the prefix. macOS ONNX Runtime tarballs (onnxruntime-osx-*-1.20.0.tgz) prefix every entry with ./, Linux tarballs do not — so the original matcher dropped every macOS entry, left the install dir empty, and failed with extraction completed but ...libonnxruntime.1.20.0.dylib is missing.
  • Secondary fix: ensure_dylib_symlink now uses Path::symlink_metadata instead of Path::exists. The old check followed the symlink, so a broken symlink left by a prior failed run looked absent; the retry then crashed with creating symlink ...: File exists (os error 17).
  • Tests: three new tests in crates/openmemory-embed/src/runtime.rs build synthetic tarballs mirroring the upstream Linux and macOS layouts and assert correct extraction. A third test pre-seeds a broken symlink and confirms ensure_dylib_symlink is a no-op.

Repro (before this PR)

$ openmemory --home /tmp/home model download
Installing ONNX Runtime 1.20.0...
INFO openmemory_embed::runtime: Downloading ONNX Runtime 1.20.0 from
     https://github.com/microsoft/onnxruntime/releases/download/v1.20.0/onnxruntime-osx-arm64-1.20.0.tgz
error: installing ONNX Runtime
  caused by: configuration error: extraction completed but
  /tmp/home/runtime/onnxruntime-osx-arm64-1.20.0/lib/libonnxruntime.1.20.0.dylib is missing

After this PR

$ openmemory --home /tmp/home model download
Installing ONNX Runtime 1.20.0...
INFO openmemory_embed::runtime: ONNX Runtime installed at
     /tmp/home/runtime/onnxruntime-osx-arm64-1.20.0/lib/libonnxruntime.1.20.0.dylib
ONNX Runtime 1.20.0 ready.
Downloading 'nomic-embed-text-v1.5'...
Model 'nomic-embed-text-v1.5' ready.

End-to-end openmemory_search mode=vector round-trip works: ort loads the dylib (Loaded ONNX Runtime dylib with version '1.20.0'), embeddings active, semantic recall returns a 0.61-similarity match between "fast canine over slothful canid" and "the quick brown fox jumps over the lazy dog".

Test plan

  • cargo test -p openmemory-embed --lib runtime:: — 7/7 pass (including 3 new tests)
  • cargo test --workspace — all suites green
  • cargo clippy --workspace --all-targets -- -D warnings (default + --no-default-features)
  • cargo fmt --all -- --check
  • cargo doc --workspace --no-deps (default + --all-features)
  • macOS clean-room: rm -rf /tmp/home && openmemory --home /tmp/home model download → 25 MB versioned dylib + dSYM + symlink extracted; subsequent openmemory_search mode=vector works
  • Linux unaffected: new matcher is a strict superset of the old behavior (strip_prefix("./").unwrap_or(s)); the extracts_linux_style_tarball_without_dot_slash_prefix test guards against regression and CI's Ubuntu job exercises the full integration

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Resolved macOS ONNX Runtime extraction issues with tarball path formats.
    • Enhanced symlink handling to recover from broken symlinks on retry attempts.

Review Change Stack

`openmemory model download` shipped in v0.3.2 with bundled
ONNX Runtime install. The tar extractor matched entries by string
prefix against `<archive_root>/lib/`, but macOS ONNX Runtime
tarballs prefix every entry with `./` while Linux ones do not, so
on macOS the matcher dropped every entry. The install dir ended up
empty, `ensure_dylib_symlink` then wrote a broken
`libonnxruntime.dylib` symlink, and the post-check fired with
`extraction completed but ...libonnxruntime.1.20.0.dylib is
missing`.

`extract_lib_subtree` now strips a leading `./` from each entry
before comparing the prefix. `ensure_dylib_symlink` switches its
"already installed?" check from `Path::exists` (which follows
symlinks and reports false for a broken one) to
`Path::symlink_metadata`, so a broken symlink left over from a
prior failed run no longer poisons retries with EEXIST.

Two new tests build synthetic tarballs that mirror the upstream
Linux and macOS layouts (real versioned dylib + unversioned
symlink in `lib/`, unrelated entry in `include/`) and assert that
both extract correctly and the include subtree is skipped. A third
test pre-seeds a broken symlink and confirms `ensure_dylib_symlink`
is a no-op rather than a hard error.

Verified end-to-end on macOS by running `openmemory model download`
into a clean home directory: 25 MB versioned dylib, dSYM, and
unversioned symlink all land under
`<home>/runtime/onnxruntime-osx-arm64-1.20.0/lib/`. Linux remains
unaffected because the new matcher is a strict superset of the old
(strip `./` if present, otherwise leave the path unchanged); the
new Linux-style test guards against regression.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: String must contain at most 250 character(s) at "tone_instructions"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a489a57-eec9-4229-b404-2df69cb027e1

📥 Commits

Reviewing files that changed from the base of the PR and between 5039247 and 3b21e25.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • crates/openmemory-embed/src/runtime.rs

📝 Walkthrough

Walkthrough

This PR fixes ONNX Runtime tarball extraction on macOS by normalizing tar entry paths to handle leading ./ prefixes, improves symlink presence detection to tolerate broken symlinks, and adds regression tests validating both fixes across macOS and Linux tarball layouts.

Changes

Tar Extraction and Symlink Robustness

Layer / File(s) Summary
Tar extraction and symlink robustness improvements
crates/openmemory-embed/src/runtime.rs
extract_lib_subtree strips leading ./ from tar entry paths before prefix matching; ensure_dylib_symlink uses symlink_metadata() to detect symlink presence and permit retries with broken symlinks.
Regression tests and test infrastructure
crates/openmemory-embed/src/runtime.rs
build_fake_tarball helper constructs synthetic ONNX Runtime tarballs with configurable prefixes; new tests cover macOS ./-prefixed extraction, Linux extraction, and symlink recovery from missing targets.
Changelog documentation
CHANGELOG.md
Fixed section documents the macOS path normalization, symlink robustness change, and synthetic tarball test coverage.

Possibly Related PRs

  • raymondj99/openmemory#20: Introduces the ONNX Runtime extraction and symlink installation logic that this PR refines with macOS ./ prefix handling and symlink metadata robustness.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A tarball comes from macOS bright,
With ./ prefixes, oh what a sight!
Symlinks once broken now pass the test,
Extract and retry—no more unrest.
The rabbit hops on, fixes complete!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the primary fix: enabling extraction of macOS ONNX Runtime tarballs by handling the ./ prefix issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/macos-ort-extraction

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 20, 2026

Merging this PR will not alter performance

✅ 19 untouched benchmarks


Comparing fix/macos-ort-extraction (3b21e25) with main (5039247)

Open in CodSpeed

@raymondj99 raymondj99 merged commit f042342 into main May 20, 2026
14 checks passed
@raymondj99 raymondj99 deleted the fix/macos-ort-extraction branch May 20, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant