-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Bundler improvements, add ISO installer support, and Modelfile folder #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds ISO packaging and CI support plus a refactored interactive installer. New scripts: Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Just as just (task runner)
participant MakeISO as scripts/make_iso.sh
participant Bundler as scripts/bundler.sh
participant Installer as scripts/install.sh
participant ISOtool as xorriso|genisoimage|mkisofs
participant GH as GitHub Actions Artifact
Dev->>Just: just iso (or CI triggers)
Just->>MakeISO: execute
MakeISO->>MakeISO: read Cargo.toml, determine VERSION, OS/ARCH
MakeISO->>Bundler: ./scripts/bundler.sh (create bundle)
Bundler-->>MakeISO: dist/tiles-bundle.tar
MakeISO->>MakeISO: create .tiles-installer, copy bundle + install script (+ ASCII art)
alt darwin
MakeISO->>MakeISO: generate Install Tiles.command and Install Tiles.app
end
MakeISO->>ISOtool: build ISO with label tiles-VERSION
ISOtool-->>MakeISO: tiles-installer-VERSION.iso
MakeISO->>GH: upload tiles-installer-*.iso artifact
GH-->>Dev: artifact available
sequenceDiagram
participant User as End user
participant Installer as scripts/install.sh
participant Env as Host (python/uv/tooling)
participant Bundle as tiles-bundle.tar
User->>Installer: run install.sh (from ISO or bundle)
Installer->>Env: check dependencies (python, uv, etc.)
alt missing
Installer->>Env: print instructions / attempt install (brew/apt/uv installer)
end
Installer->>Bundle: locate bundle (local / repo / GitHub release)
alt found
Installer->>Installer: extract bundle, install CLI, setup Python server (uv sync)
Installer->>User: print PATH/setup guidance and completion
else not found
Installer->>User: error and abort
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{rs,toml}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)src/commands/mod.rs (2)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| create_iso() { | ||
| local src_dir="$1" | ||
| local out_path="$2" | ||
| local label="$3" | ||
|
|
||
| if command -v xorriso >/dev/null 2>&1; then | ||
| xorriso -as mkisofs -quiet -o "${out_path}" -V "${label}" "${src_dir}" | ||
| elif command -v genisoimage >/dev/null 2>&1; then | ||
| genisoimage -quiet -V "${label}" -o "${out_path}" "${src_dir}" | ||
| elif command -v mkisofs >/dev/null 2>&1; then | ||
| mkisofs -quiet -V "${label}" -o "${out_path}" "${src_dir}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve long file names when generating installer ISO
The ISO is created with xorriso/genisoimage/mkisofs using only -V and -o. Without Rock Ridge or Joliet extensions the tools fall back to ISO9660 level 1, truncating file and directory names to 8.3 uppercase. The macOS bundle laid out earlier in this script (Install Tiles.app/Contents/Info.plist, .../Resources/...) relies on those exact long paths; after truncation Finder cannot treat the directory as an application and the auto‑launch installer fails to run. Add options such as -J -R (or other platform‑appropriate flags) so that long file names are preserved whenever a macOS ISO is produced.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
scripts/make_iso.sh (2)
143-221: AppleScript launcher logic is comprehensive but has multiple search paths.The fallback mechanism that searches mounted volumes (lines 180-206) could theoretically match an incorrect volume if multiple
/Volumes/tiles-*directories exist. Consider adding a more specific identifier (e.g., version number in volume name) or prompt user to select if multiple matches are found. However, this is a defensive fallback and unlikely to cause issues in normal use.
273-290: Add validation that ISO creation succeeded.While the tool fallback logic is solid, consider verifying that the output ISO file was created before reporting success at line 292. Current code will report success even if
xorriso/genisoimage/mkisofsfailed silently.Apply this diff to validate ISO creation:
mkdir -p "${DIST_DIR}" create_iso "${TMPDIR}" "${ISO_PATH}" "${ISO_LABEL}" + +if [[ ! -f "${ISO_PATH}" ]]; then + err "ISO creation failed: ${ISO_PATH} not found" +fi log "ISO created: ${ISO_PATH}"scripts/install.sh (3)
122-125: Add defensive check forsudoavailability on Linux systems.Line 124 invokes
sudowithout verifying the user has sudo access. If sudo is unavailable or user lacks permissions, the installer will fail with a permission error. Consider checking sudo availability or handling the error more gracefully.elif [[ -f /etc/debian_version ]]; then + if ! sudo -n true 2>/dev/null; then + err "This system requires 'sudo' to install Python. Please configure sudo access." + fi print_info "Installing Python via apt..." sudo apt-get update -y && sudo apt-get install -y python3 python3-venv
137-141: Verify uv installation succeeded.While the official uv installation method is used, there's no verification that
uvis actually available after line 138 completes. If the installer script fails silently, line 140's success message would be misleading. Consider checking foruvcommand availability after PATH update.print_info "Installing uv..." curl -LsSf https://astral.sh/uv/install.sh | sh export PATH="$HOME/.local/bin:$PATH" + if ! command -v uv >/dev/null 2>&1; then + err "uv installation failed. Please visit https://docs.astral.sh/uv/getting-started/installation/" + fi print_success "uv installed"
1-3: Add cleanup trap for main script TMPDIR.While
rm -rf "${TMPDIR}"is called at line 213, it's not in a trap handler, so TMPDIR won't be cleaned up if the script exits early due to an error. Consider adding an early cleanup trap to ensure temp directory is always removed.#!/usr/bin/env bash set -euo pipefail + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "${TMPDIR}"' EXIT # Terminal setupThen remove the manual cleanup at line 213. This ensures cleanup happens regardless of how the script exits.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (10)
.github/workflows/rust.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)ascii-art.txt(1 hunks)justfile(1 hunks)memgpt.modelfile(1 hunks)scripts/bundler.sh(2 hunks)scripts/install.sh(1 hunks)scripts/make_iso.sh(1 hunks)src/main.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{rs,toml}
⚙️ CodeRabbit configuration file
Review the Rust code for conformity with best practices in Rust, Systems programming. Highlight any deviations.
Files:
src/main.rs
🧬 Code graph analysis (2)
scripts/make_iso.sh (1)
scripts/install.sh (2)
log(87-89)err(91-95)
scripts/install.sh (1)
scripts/make_iso.sh (1)
err(16-16)
🪛 LanguageTool
ascii-art.txt
[duplication] ~1-~1: Ebla mistajpaĵo: vi ripetis vorton
Context: ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~2-~2: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~4-~4: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~6-~6: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~8-~8: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~43-~43: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~45-~45: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~47-~47: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~49-~49: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~51-~51: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░...
(WORD_REPEAT_RULE)
[duplication] ~53-~53: Ebla mistajpaĵo: vi ripetis vorton
Context: ...░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░
(WORD_REPEAT_RULE)
🔇 Additional comments (15)
.gitignore (1)
3-3: LGTM!The addition of the
dist/directory to gitignore appropriately excludes build artifacts generated by the new ISO packaging workflow.justfile (1)
23-24: LGTM!The new
isotarget follows the existing pattern of shell script delegation and integrates cleanly with the packaging workflow.ascii-art.txt (1)
1-55: LGTM!The ASCII art asset will be included in the ISO installer as a visual element. The static analysis warnings about character repetition are false positives—the repeated block elements are intentional parts of the ASCII art design.
README.md (1)
18-21: LGTM!The new documentation section clearly explains the packaging workflow, providing users with straightforward instructions for creating bundles and bootable ISO images.
.github/workflows/rust.yml (1)
41-54: LGTM!The new CI steps install ISO tooling, build the installer, and upload artifacts with appropriate error handling. The use of
if-no-files-found: errorensures the workflow fails if the ISO generation doesn't produce expected outputs.scripts/bundler.sh (1)
14-29: LGTM!The removal of emoji characters from output messages improves compatibility with various terminal environments and automated tooling while maintaining clear status reporting.
src/main.rs (1)
7-7: LGTM!The CLI description update provides users with a clearer understanding of the tool's purpose and functionality, specifically highlighting privacy, on-device processing, and Obsidian integration.
memgpt.modelfile (1)
1-1: Based on my verification, the original review comment was based on incorrect assumptions about the file format and requires correction.The review comment is incorrect.
The FROM instruction in an Ollama Modelfile identifies the base model and is mandatory for defining the foundation. You can use any model available in the Ollama library or even a local file path to a GGUF model file.
The file
memgpt.modelfileis an Ollama Modelfile, not a Docker container definition. The original review comment incorrectly treated it as a Docker image reference by suggesting Docker Hub image verification. However, the GitHub repository references "mem-agent-mlx-4bit" and "mem-agent-mlx-8bit" as model names for use with LM Studio, and the MLX version of the mem-agent model has been quantized to 8-bit and 4-bit precisions, with the 4-bit model showing little to no loss in performance.The change from
driaforall/mem-agenttodriaforall/mem-agent-mlx-4bitappears intentionally designed to reference a quantized model variant. This is a valid model configuration update, not a Docker image dependency issue.Likely an incorrect or invalid review comment.
scripts/make_iso.sh (2)
18-23: Bundle validation and error handling look solid.Proper file existence check with clear error message on failure.
85-99: Exit code handling is correct.Proper use of
${PIPESTATUS[0]}to capture the exit code frominstall.shbeforetee, with appropriate success/failure handling.scripts/install.sh (5)
8-25: Terminal setup and ASCII art detection are well-designed.Defensive terminal width detection and dual-path ASCII art lookup handle both local and mounted volume scenarios appropriately.
119-127: Verify macOS Python installation doesn't fail silently.Line 121 assumes
brewis available on macOS but doesn't verify it exists first. Ifbrewisn't installed, thebrew install pythoncommand fails, but because it's piped toerr()via the||operator, it should exit correctly. However, clarify the intent: is this a fatal error ifbrewis unavailable, or should there be a fallback?Consider checking for
brewavailability explicitly:if [[ "$OS" == "darwin" ]]; then + if ! command -v brew >/dev/null 2>&1; then + err "Homebrew not found. Please install from https://brew.sh" + fi print_info "Installing Python via Homebrew..." brew install python || err "Could not install Python. Please install manually from https://www.python.org" elif [[ -f /etc/debian_version ]]; then
149-181: Bundle resolution and extraction logic is well-structured.Clear priority order (local → repo → GitHub) with appropriate error handling and explicit success messaging for each path.
187-209: Installation steps use appropriate commands with proper error handling.Binary permissions (755), recursive server copy, and reproducible
uv sync --frozenwith failure handling are all appropriate.
228-252: PATH setup detection and help display are comprehensive.The parameter expansion check for PATH membership (line 228) is correct, and the fallback chain for displaying help (lines 243-247) handles cases where the
tilescommand isn't yet available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 16
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| .canonicalize() | ||
| .map(|p| p.to_string_lossy().to_string()) | ||
| .map_err(|e| format!("Failed to canonicalize path '{}': {}", input, e)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Path-conflict: prioritize model lookup over local files
The resolve_modelfile_path function incorrectly prioritizes file existence over model name resolution. If a user runs tiles run memgpt and there happens to be a file or directory named memgpt in the current directory, it will use that local file instead of looking for the model in the registry. The function should check if the input is explicitly a path (contains /, \, or starts with . or ~) before checking existence, or at minimum check if it's a relative/absolute path pattern before treating it as a file path.
|
@cursoragent fix this error in build step: https://github.com/tilesprivacy/tilekit/actions/runs/19133405090/job/54679797502#step:6:1 2025-11-06T11:03:15.8986303Z ##[debug]Evaluating condition for step: 'Run check' |
|
Perfect! I've fixed all the formatting issues: Changes MadeFixed formatting errors in two files:
The Learn more about Cursor Agents |
Co-authored-by: hey <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Note
Introduces ISO packaging with CI artifact, a revamped installer, registry-based model resolution in CLI, and updates default model to
mem-agent-mlx-4bit.scripts/make_iso.shto build a bootable installer ISO (macOS auto-launch app, logging, README, ASCII art).scripts/install.shwith UI, dependency checks, bundle discovery (local/release), and installation steps.scripts/bundler.sh; addjust isoand ignoredist/.ascii-art.txtfor installer branding.tiles runnow accepts a model name by resolving to~/.local/share/tiles/registry/<name>/Modelfile(addsget_registry_dir).registry/memgpt/Modelfile.MODEL_IDtodriaforall/mem-agent-mlx-4bitinserver/config.py.Written by Cursor Bugbot for commit 92e56eb. This will update automatically on new commits. Configure here.