fix(ai): resolve sidecar spawn path and auto-install the llama.cpp engine (I73) by scotej · Pull Request #88 · scotej/studyvis

scotej · 2026-07-26T06:43:44Z

Summary

Root cause (I73, Sev1): the in-app llama-server spawn has never worked in any build. shell().sidecar("binaries/llama-server") resolves <exe_dir>/binaries/llama-server, but tauri-build (dev) and the bundler (release) strip the directory prefix and place the binary at <exe_dir>/llama-server — verified in both target/debug/ and the installed StudyVis.app/Contents/MacOS/. Every sidecar_start failed with spawn llama-server: No such file or directory, shown as "AI failed to start" / "AI model crashed". Day-one bug (tauri-plugin-shell pinned at 2.3.5 since V1-P1), masked until I72 (v1.7.1) let model downloads complete; the 2026-07-26 attempt left a 0-byte llama-server.log — the child never ran.

Fix + hardening:

Spawn resolution: binaries resolve to absolute paths and spawn via shell().command() (same piped stdio + CREATE_NO_WINDOW, minus the broken exe-relative join). Preference order: bundled <exe_dir>/llama-server(.exe) (4 MB size gate filters the dev placeholder), then managed data_dir/engine/<tag>-<triple>/. Spawn failures fall through candidates; the crash-restart watcher re-resolves but never downloads.
Engine auto-install (src-tauri/src/commands/engine.rs): when no candidate resolves, sidecar_start downloads the pinned llama.cpp b9095 release asset for the current triple — SHA-256-verified, streaming with throttled engine:progress events — extracts llama-server + companion libs (entries flattened to file names: traversal-proof by construction; other CLI tools skipped), and installs atomically (staging dir + rename). Concurrent installs coalesce behind a gate with a re-check (no "already in flight" errors). Pins are lockstep-tested against scripts/fetch-llama-server.sh in cargo test.
Settings → AI → "AI engine": status row (bundled/downloaded/missing/unsupported, live download progress as text with role="status"), Install/Reinstall button (stops the sidecar first — Windows can't swap loaded DLLs; locked during sessions), and an Install engine automatically toggle (engine_auto_install, default ON; absent-key hydration is default-safe for existing users).
Calm failure modes: auto-install off + engine missing → exact engine_not_installed sentinel → "The AI engine isn't installed. Install it in Settings → AI." toast with an Open-settings action, not a raw error. Windows spawn failure with vcruntime140.dll absent → error names the VC++ redistributable with the aka.ms link.
Zero-setup dev builds: build.rs writes a debug-profile-only placeholder sidecar so fresh checkouts compile/run without the fetch script (release-profile builds still hard-fail; CI/release fetch real binaries first, unchanged).
Docs: ISSUES.md I73 row, CHANGELOG Unreleased, ARCHITECTURE §8 engine-resolution paragraph, README dev-setup note.

All four supported triples (mac-arm64, mac-x64, win-x64, linux-x64) have pinned assets + checksums; unsupported triples surface a calm "No prebuilt engine for this computer yet." state, never an error.

Test plan

Machine-verified (this Mac, Apple Silicon):

cargo test (87: incl. pin-lockstep vs fetch script, tar/zip extraction fixtures w/ traversal + filtering + exec bit, asset matrix), cargo clippy (correctness), cargo fmt
npm run build, lint, test (832), check-tokens, check-strings, check-contrast, check-a11y (axe over Storybook)
Live: all four pinned archives downloaded from GitHub and SHA-256s match the fetch-script pins byte-for-byte; extracted mac-arm64 llama-server --version runs (Metal init, @loader_path rpath confirmed → flat managed dir needs no env); the installed bundle's binary spawns via the exact new resolution (Contents/MacOS/llama-server + DYLD_FALLBACK_LIBRARY_PATH, exit 0) while the old Contents/MacOS/binaries/llama-server path provably doesn't exist; fresh-checkout placeholder build compiles and launches tauri dev (one-time watcher re-trigger noted in build.rs).

User-walked (dev binary's keychain prompt blocks machine-driving the GUI):

Settings → AI shows the "AI engine" row ("llama.cpp b9095 — included with this build." with the fetched binary present)
With src-tauri/binaries/llama-server-* stashed: row shows "Not installed yet…", Install now downloads with live progress and lands on "downloaded automatically"
Start AI in a session → sidecar starts (first time ever through the UI); with auto-install off + engine missing → calm settings toast

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added automatic AI engine installation when the engine is missing or unusable.
- Added AI engine status, download progress, and Install/Reinstall controls in Settings → AI.
- Added an “Install engine automatically” setting, enabled by default.
- Added checksum verification and clearer Windows runtime error guidance.
Bug Fixes
- Fixed AI startup failures caused by incorrect engine binary path resolution.
- Added fallback handling when the bundled engine cannot start.
Documentation
- Updated development instructions and architecture documentation for the new engine setup.

…gine (I73) The in-app llama-server spawn never worked: shell().sidecar("binaries/ llama-server") resolves <exe_dir>/binaries/llama-server, but tauri-build and the bundler place the file at <exe_dir>/llama-server — every sidecar_start since V2-P1 failed with ENOENT, surfaced as "AI failed to start" / "AI model crashed". Day-one bug (plugin pinned at 2.3.5 since V1-P1), masked until I72 let model downloads complete. Fix + hardening, treated as one production story: - Spawn via shell().command() on absolute paths: bundled binary at <exe_dir>/llama-server(.exe) (size-gated), then a managed install at data_dir/engine/<tag>-<triple>/; spawn failures fall through candidates. - New commands/engine.rs: when nothing resolves, sidecar_start downloads the pinned llama.cpp b9095 asset for the current triple (SHA-256 verified, pins lockstep-tested against scripts/fetch-llama-server.sh), extracts llama-server + companion libs (flattened, filtered, traversal- proof), installs atomically, and spawns it. Concurrent installs coalesce; failures persist in engine_info.last_error. - engine_auto_install setting (default ON, absent-key-safe) + Settings → AI "AI engine" row: status, live download progress (role=status), and Install/Reinstall (stops the sidecar first; locked during sessions). - engine_not_installed sentinel (auto-install off) maps to a calm "Install it in Settings → AI" toast, not a crash string. - build.rs writes a debug-profile-only placeholder so fresh checkouts compile and run with zero manual steps; release-profile builds still hard-fail without real binaries. - Windows spawn failures name the missing VC++ redistributable (vcruntime140.dll probe) with the aka.ms install link. Verified live on macOS: installed-bundle binary spawns via the exact new resolution (--version, exit 0); placeholder build compiles + launches; pinned archives download, hash-match, extract, and run. GUI walkthrough is user-walked (dev-binary keychain prompt blocks machine-driving). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-07-26T06:44:08Z

Warning

Review limit reached

@scotej, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0af81875-a2ef-4b5a-a283-c082a6e7b0bd

📥 Commits

Reviewing files that changed from the base of the PR and between 4cbaa71 and 417046f.

📒 Files selected for processing (9)

CHANGELOG.md
README.md
src-tauri/build.rs
src-tauri/src/commands/engine.rs
src-tauri/src/commands/sidecar.rs
src/features/ai/ModelPickerContainer.tsx
src/strings.ts
tests/unit/ai-sidecar.test.ts
tests/unit/settings-migration.test.ts

📝 Walkthrough

Walkthrough

The PR adds a pinned, checksum-verified llama-server auto-installer, absolute-path sidecar resolution with managed fallback, frontend engine controls and progress reporting, persisted auto-install settings, debug placeholders, and related documentation and tests.

Changes

AI engine lifecycle

Layer / File(s)	Summary
Pinned engine packaging and installation `src-tauri/Cargo.toml`, `src-tauri/src/commands/engine.rs`, `src-tauri/build.rs`, `src-tauri/src/commands/mod.rs`, `src-tauri/src/commands/models.rs`	Adds pinned per-target assets, checksum verification, archive extraction, atomic managed installation, progress state, cleanup, debug placeholders, and extraction tests.
Sidecar startup and fallback `src-tauri/src/commands/sidecar.rs`, `src/features/ai/sidecar.ts`, `tests/unit/ai-sidecar.test.ts`	Resolves bundled and managed binaries by absolute path, optionally installs before startup, falls back after spawn failure, and reuses the candidate chain during restarts.
Engine settings and frontend bridge `src-tauri/src/lib.rs`, `src/features/ai/engine.ts`, `src/features/ai/index.ts`, `src/stores/settingsStore.ts`, `src/features/settings/categories/AiCategory.tsx`, `src/features/session/SessionView.tsx`, `src/strings.ts`, `tests/unit/settings-migration.test.ts`	Exposes engine commands and progress events, persists the default-on auto-install setting, adds Settings installation controls, and shows a dedicated missing-engine error.
Development and workflow documentation `ARCHITECTURE.md`, `CHANGELOG.md`, `ISSUES.md`, `README.md`	Documents engine resolution, automatic installation, debug placeholders, release setup requirements, and the recorded startup issue.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SessionView
  participant SidecarRuntime
  participant sidecar_start
  participant EngineCommands
  SessionView->>SidecarRuntime: start engineAutoInstall
  SidecarRuntime->>sidecar_start: invoke startup command
  sidecar_start->>EngineCommands: resolve or install engine
  EngineCommands-->>sidecar_start: return executable path
  sidecar_start-->>SidecarRuntime: return port or engine_not_installed

Suggested reviewers: copilot, claude

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing AI sidecar spawn path resolution and adding automatic llama.cpp engine installation.
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.

✨ Finishing Touches

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch feat/engine-auto-install

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (4)

tests/unit/ai-sidecar.test.ts (2)
176-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the auto-install-off path too.

Every stub pins getEngineAutoInstall to true, so nothing exercises engineAutoInstall: false forwarding or the ERR_ENGINE_NOT_INSTALLED sentinel surfacing through lastError — the two behaviors this cohort actually adds on the JS side.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/ai-sidecar.test.ts` around lines 176 - 178, Extend the ai-sidecar
tests around start() to cover getEngineAutoInstall returning false, asserting
engineAutoInstall: false is forwarded to sidecar_start and
ERR_ENGINE_NOT_INSTALLED is surfaced through lastError. Keep the existing
true-path coverage unchanged.
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

startCalls's element type is now stale.

It omits engineAutoInstall even though push(params) carries it, so a future drift in the forwarded payload wouldn't be caught at type level.
♻️ Widen the recorded type
   const startCalls: Array<{
     modelPath: string
     mmprojPath: string | null
     ctxSize: number
+    engineAutoInstall: boolean
   }> = []
Also applies to: 98-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/ai-sidecar.test.ts` around lines 27 - 31, Update the startCalls
element type and its recorded entries to include the engineAutoInstall field
forwarded by push(params), matching the parameter shape used by the startup call
so future payload drift is caught by TypeScript.
tests/unit/settings-migration.test.ts (1)
160-171: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer the exported key constant over the string literal.

fakeStore({ engine_auto_install: false }) hardcodes the key; using SETTINGS_KEY_ENGINE_AUTO_INSTALL keeps the test pinned to the same contract hydrateValuesFromStore reads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/settings-migration.test.ts` around lines 160 - 171, Update the
fakeStore input in the “defaults engineAutoInstall on when missing and preserves
an explicit off” test to use the exported SETTINGS_KEY_ENGINE_AUTO_INSTALL
constant instead of the hardcoded engine_auto_install string, preserving the
existing false-value assertion.
src-tauri/src/commands/engine.rs (1)
305-326: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Staging dir and partial archive leak on failure paths.

If extract_archive fails, the ?? returns before fs::remove_file(&archive_path), leaving both .download-* and a partial .tmp-* behind. Same for the staged_binary metadata error at Line 317. They're swept by the next cleanup_stale, but a failed install then leaves a full archive (~100 MB) on disk indefinitely if the user never retries.
♻️ Remove the archive before propagating extraction errors
     let staging = root.join(format!(".tmp-{ENGINE_TAG}-{TARGET_TRIPLE}"));
-    {
+    let extract_result = {
         let archive_path = archive_path.clone();
         let staging = staging.clone();
         tauri::async_runtime::spawn_blocking(move || {
             extract_archive(&archive_path, &staging, binary_name())
         })
         .await
-        .map_err(|e| format!("extract task: {e}"))??;
-    }
-    let _ = fs::remove_file(&archive_path);
+        .map_err(|e| format!("extract task: {e}"))
+        .and_then(|r| r)
+    };
+    let _ = fs::remove_file(&archive_path);
+    if let Err(e) = extract_result {
+        let _ = fs::remove_dir_all(&staging);
+        return Err(e);
+    }
 
     let staged_binary = staging.join(binary_name());
-    let staged_len = fs::metadata(&staged_binary)
-        .map_err(|e| format!("{} missing from archive: {e}", binary_name()))?
-        .len();
+    let staged_len = match fs::metadata(&staged_binary) {
+        Ok(m) => m.len(),
+        Err(e) => {
+            let _ = fs::remove_dir_all(&staging);
+            return Err(format!("{} missing from archive: {e}", binary_name()));
+        }
+    };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/commands/engine.rs` around lines 305 - 326, Update the archive
extraction flow around extract_archive so cleanup runs before propagating
extraction errors: remove archive_path and staging on any extract failure,
including partial archives. Also clean up both archive_path and staging when
fs::metadata(&staged_binary) fails, while preserving the existing validation and
success path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 40-52: Update the changelog section describing the AI engine
installation to say installation is attempted when the default-enabled setting
permits it and may fail, replacing unconditional “automatically” and “No error,
no manual step” claims. Also update README.md lines 330-334 to state that debug
builds auto-install only while the setting remains enabled; when disabled, users
must provide or manually install the engine.

In `@src-tauri/src/commands/sidecar.rs`:
- Around line 185-195: Update the sidecar_start handler to acquire the shared
EngineState.gate using its existing engine state before resolve_candidates and
keep the guard through the spawn operation. This must serialize presence checks
and process startup with ensure_installed/forced reinstall, while preserving the
current installation and error-handling behavior.

In `@src/strings.ts`:
- Around line 1218-1245: Update the AI privacy messaging to limit “Nothing
leaves your computer” to camera and screen data, and explicitly disclose that
the llama.cpp engine may be downloaded from GitHub. Keep the disclosure
consistent with the engine.auto.help text and the default auto-install flow in
the engine strings.

---

Nitpick comments:
In `@src-tauri/src/commands/engine.rs`:
- Around line 305-326: Update the archive extraction flow around extract_archive
so cleanup runs before propagating extraction errors: remove archive_path and
staging on any extract failure, including partial archives. Also clean up both
archive_path and staging when fs::metadata(&staged_binary) fails, while
preserving the existing validation and success path.

In `@tests/unit/ai-sidecar.test.ts`:
- Around line 176-178: Extend the ai-sidecar tests around start() to cover
getEngineAutoInstall returning false, asserting engineAutoInstall: false is
forwarded to sidecar_start and ERR_ENGINE_NOT_INSTALLED is surfaced through
lastError. Keep the existing true-path coverage unchanged.
- Around line 27-31: Update the startCalls element type and its recorded entries
to include the engineAutoInstall field forwarded by push(params), matching the
parameter shape used by the startup call so future payload drift is caught by
TypeScript.

In `@tests/unit/settings-migration.test.ts`:
- Around line 160-171: Update the fakeStore input in the “defaults
engineAutoInstall on when missing and preserves an explicit off” test to use the
exported SETTINGS_KEY_ENGINE_AUTO_INSTALL constant instead of the hardcoded
engine_auto_install string, preserving the existing false-value assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: edd09be6-9944-452d-951a-1be161441250

📥 Commits

Reviewing files that changed from the base of the PR and between 0573738 and 4cbaa71.

⛔ Files ignored due to path filters (1)

src-tauri/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (20)

ARCHITECTURE.md
CHANGELOG.md
ISSUES.md
README.md
src-tauri/Cargo.toml
src-tauri/build.rs
src-tauri/src/commands/engine.rs
src-tauri/src/commands/mod.rs
src-tauri/src/commands/models.rs
src-tauri/src/commands/sidecar.rs
src-tauri/src/lib.rs
src/features/ai/engine.ts
src/features/ai/index.ts
src/features/ai/sidecar.ts
src/features/session/SessionView.tsx
src/features/settings/categories/AiCategory.tsx
src/stores/settingsStore.ts
src/strings.ts
tests/unit/ai-sidecar.test.ts
tests/unit/settings-migration.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (3)

GitHub Check: Rust (Windows)
GitHub Check: Rust (macOS)
GitHub Check: Frontend

🧰 Additional context used

📓 Path-based instructions (9)

tests/**/*.{ts,tsx}