Skip to content

fix(aft-bridge): align PATH defaults to plugin#127

Open
tobwen wants to merge 1 commit into
cortexkit:mainfrom
tobwen:fix/standalone-storage-defaults
Open

fix(aft-bridge): align PATH defaults to plugin#127
tobwen wants to merge 1 commit into
cortexkit:mainfrom
tobwen:fix/standalone-storage-defaults

Conversation

@tobwen

@tobwen tobwen commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #128

What is this about?

Three functions defaulted to ~/.cache/aft instead of the plugin's ~/.local/share/cortexkit/aft, causing standalone aft warmup to write to a different path than the plugin. The fallback now mirrors resolveCortexKitStorageRoot() from the plugin.

warmup.rs also never set ORT_DYLIB_PATH, so semantic index warmup failed with dlopen('libonnxruntime.so'). Now resolves the cached library from <storage_dir>/onnxruntime/1.24.4/ (and lib/ subdir) if ORT_DYLIB_PATH is not already set.

Hint

Currently, the version of onnxruntime was hardcoded, so I didn't change this.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Aligns standalone AFT storage defaults with the plugin so both use the same cortexkit/aft path, and fixes aft warmup by auto-setting ORT_DYLIB_PATH from the cached ONNX Runtime to prevent dlopen errors.

  • Bug Fixes
    • Unified storage fallbacks to match the plugin’s resolveCortexKitStorageRoot: prefer XDG_DATA_HOME; on Windows use LOCALAPPDATA/APPDATA; else ~/.local/share/cortexkit/aft (Windows: AppData/Local/cortexkit/aft). Applied in bash_background::storage_dir, warmup_storage_dir, and search_index::resolve_cache_dir.
    • aft warmup --semantic now auto-sets ORT_DYLIB_PATH when unset by locating the cached library under <storage_dir>/onnxruntime/1.24.4/ (also checks lib/). Prevents dlopen('libonnxruntime.*')/missing DLL errors when running standalone.

Written for commit dc0dae5. Summary will update on new commits.

Review in cubic

Greptile Summary

This PR aligns the standalone aft binary's storage path defaults with the TypeScript plugin's resolveCortexKitStorageRoot(), replacing ~/.cache/aft with ~/.local/share/cortexkit/aft (XDG-aware, with proper Windows equivalents) across bash_background::storage_dir, warmup_storage_dir, and search_index::resolve_cache_dir. It also adds try_set_ort_dylib_path to auto-resolve ORT_DYLIB_PATH from the plugin-managed ONNX Runtime cache before running semantic warmup.

  • The path-fallback logic is triplicated across three functions with identical branching; the new try_set_ort_dylib_path helper silently no-ops when the library is absent, which preserves the original dlopen failure rather than surfacing a clear diagnostic.
  • The ONNX Runtime version 1.24.4 is hardcoded in the path probe, which the PR author acknowledges; a version mismatch will cause try_set_ort_dylib_path to silently return without setting the variable, leaving the failure mode unchanged from before this fix.

Confidence Score: 4/5

The path-alignment changes are correct and consistent across all three files; the only rough edge is the new ORT dylib probe, which silently no-ops when the library is absent.

The storage-path unification is mechanical and well-reasoned. The try_set_ort_dylib_path helper improves the common case but leaves an undiagnosed failure mode when the ONNX Runtime library hasn't been downloaded yet — the hardcoded version string compounds this because any plugin-side upgrade will silently break the probe. Neither issue corrupts data or causes a regression in existing behaviour, but users hitting aft warmup --semantic on a fresh machine will still see an opaque dlopen error.

crates/aft/src/cli/warmup.rs — specifically the try_set_ort_dylib_path function and its silent-failure path.

Important Files Changed

Filename Overview
crates/aft/src/cli/warmup.rs Adds try_set_ort_dylib_path with hardcoded version "1.24.4" and silent no-op on library-not-found; warmup_storage_dir path logic duplicated from bash_background.
crates/aft/src/bash_background/mod.rs storage_dir fallback updated to XDG/Windows-aware cortexkit/aft paths, mirroring the plugin; logic is correct and well-commented.
crates/aft/src/search_index.rs resolve_cache_dir fallback updated to cortexkit/aft path with identical pattern to the other two functions; straightforward path-construction change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["aft warmup --semantic"] --> B["warmup_storage_dir()"]
    B --> B1{"AFT_STORAGE_DIR set?"}
    B1 -->|yes| B2["use AFT_STORAGE_DIR"]
    B1 -->|no| B3{"XDG_DATA_HOME set?"}
    B3 -->|yes| B4["XDG_DATA_HOME/cortexkit/aft"]
    B3 -->|no| B5{"Windows?"}
    B5 -->|yes| B6{"LOCALAPPDATA / APPDATA set?"}
    B6 -->|yes| B7["LOCALAPPDATA/cortexkit/aft"]
    B6 -->|no| B8["USERPROFILE/AppData/Local/cortexkit/aft"]
    B5 -->|no| B9["HOME/.local/share/cortexkit/aft"]
    B2 & B4 & B7 & B8 & B9 --> C["storage_dir resolved"]
    C --> D["try_set_ort_dylib_path(storage_dir)"]
    D --> D1{"ORT_DYLIB_PATH already set?"}
    D1 -->|yes| D2["skip — env already set"]
    D1 -->|no| D3["probe storage_dir/onnxruntime/1.24.4/libonnxruntime.*"]
    D3 --> D4{"lib file exists?"}
    D4 -->|yes| D5["set ORT_DYLIB_PATH"]
    D4 -->|no| D6["⚠️ silent no-op — dlopen may fail later"]
    D2 & D5 & D6 --> E["configure() → semantic warmup"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["aft warmup --semantic"] --> B["warmup_storage_dir()"]
    B --> B1{"AFT_STORAGE_DIR set?"}
    B1 -->|yes| B2["use AFT_STORAGE_DIR"]
    B1 -->|no| B3{"XDG_DATA_HOME set?"}
    B3 -->|yes| B4["XDG_DATA_HOME/cortexkit/aft"]
    B3 -->|no| B5{"Windows?"}
    B5 -->|yes| B6{"LOCALAPPDATA / APPDATA set?"}
    B6 -->|yes| B7["LOCALAPPDATA/cortexkit/aft"]
    B6 -->|no| B8["USERPROFILE/AppData/Local/cortexkit/aft"]
    B5 -->|no| B9["HOME/.local/share/cortexkit/aft"]
    B2 & B4 & B7 & B8 & B9 --> C["storage_dir resolved"]
    C --> D["try_set_ort_dylib_path(storage_dir)"]
    D --> D1{"ORT_DYLIB_PATH already set?"}
    D1 -->|yes| D2["skip — env already set"]
    D1 -->|no| D3["probe storage_dir/onnxruntime/1.24.4/libonnxruntime.*"]
    D3 --> D4{"lib file exists?"}
    D4 -->|yes| D5["set ORT_DYLIB_PATH"]
    D4 -->|no| D6["⚠️ silent no-op — dlopen may fail later"]
    D2 & D5 & D6 --> E["configure() → semantic warmup"]
Loading

Reviews (1): Last reviewed commit: "fix: align standalone storage dir fallba..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/aft/src/cli/warmup.rs">

<violation number="1" location="crates/aft/src/cli/warmup.rs:294">
P1: Hardcoded ONNX Runtime version string can drift from the actual cached ORT version and break semantic warmup.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

} else {
"libonnxruntime.so"
};
let ort_dir = storage_dir.join("onnxruntime").join("1.24.4");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Hardcoded ONNX Runtime version string can drift from the actual cached ORT version and break semantic warmup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/cli/warmup.rs, line 294:

<comment>Hardcoded ONNX Runtime version string can drift from the actual cached ORT version and break semantic warmup.</comment>

<file context>
@@ -248,11 +254,50 @@ fn warmup_storage_dir() -> PathBuf {
+    } else {
+        "libonnxruntime.so"
+    };
+    let ort_dir = storage_dir.join("onnxruntime").join("1.24.4");
+    for candidate in [ort_dir.join(lib_name), ort_dir.join("lib").join(lib_name)] {
+        if candidate.is_file() {
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing pattern! The plugin side already pins 1.24.4 in two separate files (onnx-runtime.ts, onnx.ts). The Rust string is a third copy. Cross-language constant sync would require build-time codegen, which is out of scope for this fix.

Comment thread crates/aft/src/bash_background/mod.rs
…_DYLIB_PATH in warmup

Three functions defaulted to ~/.cache/aft instead of the plugin's
~/.local/share/cortexkit/aft, causing standalone `aft warmup` to
write to a different path than the plugin. Fallback now mirrors
resolveCortexKitStorageRoot() from the TS plugin (XDG_DATA_HOME,
Windows LOCALAPPDATA/APPDATA, ~/.local/share/cortexkit/aft).

warmup.rs also never set ORT_DYLIB_PATH, so semantic index warmup
failed with dlopen('libonnxruntime.so'). Now resolves the cached
library from <storage_dir>/onnxruntime/1.24.4/ (and lib/ subdir)
if ORT_DYLIB_PATH is not already set.
@tobwen tobwen force-pushed the fix/standalone-storage-defaults branch from a3b71fb to dc0dae5 Compare June 24, 2026 05:13
Comment on lines +294 to +300
let ort_dir = storage_dir.join("onnxruntime").join("1.24.4");
for candidate in [ort_dir.join(lib_name), ort_dir.join("lib").join(lib_name)] {
if candidate.is_file() {
std::env::set_var("ORT_DYLIB_PATH", &candidate);
break;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent no-op when library not cached

try_set_ort_dylib_path returns without setting ORT_DYLIB_PATH (and without any diagnostic) if neither candidate path exists. The subsequent configure call will then hit the same dlopen failure this fix is meant to prevent, with no indication of why. A user who hasn't yet run the plugin (so the library was never downloaded) will see the exact same opaque error as before. At minimum, an eprintln! warning pointing to the expected path would help with diagnosis.

} else {
"libonnxruntime.so"
};
let ort_dir = storage_dir.join("onnxruntime").join("1.24.4");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The version string "1.24.4" is hardcoded in the library probe path. When the plugin upgrades its ONNX Runtime cache (e.g. onnxruntime/1.25.0/), try_set_ort_dylib_path will silently find nothing and ORT_DYLIB_PATH will remain unset. Given the PR description already acknowledges this, consider at least making it a named constant so there is a single place to update.

Suggested change
let ort_dir = storage_dir.join("onnxruntime").join("1.24.4");
const ORT_VERSION: &str = "1.24.4";
let ort_dir = storage_dir.join("onnxruntime").join(ORT_VERSION);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

Standalone aft warmup uses wrong storage default and fails to load ONNX Runtime

1 participant