Restore Ollama service and update Homebrew configuration - #368
Conversation
…ations in CI workflow fix(flake): enhance devShell configuration to include Neovim and improve shell readiness message
|
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. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR introduces a dev shell-based approach for Lua validation, refactors the flake.nix perSystem function to use a custom package set overlay, enables ollama in home-manager services with Homebrew integration, and adds ollama to the macOS homebrew configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (11).github/workflows/*.{yml,yaml}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
.github/workflows/*.yml📄 CodeRabbit inference engine (.cursor/rules/github-workflows.mdc)
Files:
{.github/workflows/*.yml,renovate.json}📄 CodeRabbit inference engine (.cursor/rules/github-workflows.mdc)
Files:
**/*.nix📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/default.nix📄 CodeRabbit inference engine (CLAUDE.md)
Files:
home-manager/services/**/default.nix📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Files:
home-manager/**/*.nix📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Files:
home-manager/services/**/*.nix📄 CodeRabbit inference engine (.cursor/rules/nix.mdc)
Files:
home-manager/services/*/default.nix📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Files:
nix-darwin/**/*.nix📄 CodeRabbit inference engine (CLAUDE.md)
Files:
flake.nix📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧠 Learnings (22)📓 Common learnings📚 Learning: 2025-11-25T09:34:47.810ZApplied to files:
📚 Learning: 2025-11-25T09:34:23.209ZApplied to files:
📚 Learning: 2025-11-25T09:35:01.056ZApplied to files:
📚 Learning: 2025-11-25T09:34:47.810ZApplied to files:
📚 Learning: 2025-11-25T09:34:23.209ZApplied to files:
📚 Learning: 2025-11-25T09:34:47.810ZApplied to files:
📚 Learning: 2025-11-25T09:35:01.056ZApplied to files:
📚 Learning: 2025-11-25T09:34:40.052ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.006ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.005ZApplied to files:
📚 Learning: 2025-11-25T09:35:01.056ZApplied to files:
📚 Learning: 2025-11-25T09:35:01.056ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.006ZApplied to files:
📚 Learning: 2025-11-25T09:34:23.210ZApplied to files:
📚 Learning: 2025-11-25T09:34:40.052ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.006ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.006ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.005ZApplied to files:
📚 Learning: 2025-11-25T09:34:55.005ZApplied to files:
📚 Learning: 2025-11-25T09:34:40.052ZApplied to files:
📚 Learning: 2025-11-25T09:35:01.056ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (3)
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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on restoring and improving the integration of the Ollama service within the development environment, specifically by aligning it with Homebrew installations. Additionally, it enhances the default development shell by incorporating Neovim and refactors the Nix flake to better manage development packages. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively restores the Ollama service by integrating it with Homebrew for more reliable binary management, which is a smart workaround for the previous build issues with the Nix package. The changes are well-contained and logical. You've correctly updated the Homebrew configuration to install ollama and ollama-app, and the launchd service is now correctly pointing to the Homebrew-managed binary via a wrapper script. Additionally, the development shell is enhanced to include Neovim, which aligns with the goal of improving the CI workflow. I have one suggestion to improve the shell script in the ollama service for better maintainability.
| if [ -x /opt/homebrew/bin/ollama ]; then | ||
| exec /opt/homebrew/bin/ollama "$@" | ||
| elif [ -x /usr/local/bin/ollama ]; then | ||
| exec /usr/local/bin/ollama "$@" | ||
| else | ||
| echo "ollama binary not found; install it with \"brew install ollama\"" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
For better readability and future extensibility, you could simplify this shell script. Using a for loop to iterate over a list of possible paths for the ollama binary would make the script cleaner and easier to update if more search paths are needed in the future. The logic becomes more straightforward: iterate through paths, and if none are found, then exit with an error.
for OLLAMA_PATH in /opt/homebrew/bin/ollama /usr/local/bin/ollama; do
if [ -x "$OLLAMA_PATH" ]; then
exec "$OLLAMA_PATH" "$@"
fi
done
echo "ollama binary not found; install it with \"brew install ollama\"" >&2
exit 1
There was a problem hiding this comment.
Pull request overview
This PR restores the Ollama service by switching from a nixpkgs-based installation to a Homebrew-based approach, resolving previous build conflicts. It also modernizes the CI workflow to use the development shell for Lua validation tasks and adds Neovim to the development environment.
Key Changes:
- Restored Ollama service with Homebrew wrapper to avoid nixpkgs build issues
- Updated CI workflow to use
nix developfor consistent Neovim and Hammerspoon validation - Enhanced development shell with Neovim and custom pkgs configuration
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| nix-darwin/config/homebrew.nix | Added ollama formula and ollama-app cask, removing the FIXME comment about conflicts |
| home-manager/services/ollama/default.nix | Introduced Homebrew wrapper script to use system-installed Ollama binary instead of nixpkgs |
| home-manager/services/default.nix | Re-enabled the Ollama service by uncommenting its import |
| flake.nix | Restructured perSystem to use custom pkgs with overlays, added Neovim to devShell |
| .github/workflows/lua.yml | Updated CI to use nix develop for Neovim and Hammerspoon validation steps |
Comments suppressed due to low confidence (1)
home-manager/services/ollama/default.nix:32
- Binding Ollama to
0.0.0.0exposes the service to all network interfaces, potentially allowing external access. This is a security risk on shared or public networks. Consider using127.0.0.1for localhost-only access, or document why external access is required and add firewall/network restrictions.
EnvironmentVariables.OLLAMA_HOST = "0.0.0.0";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "kurtosis-cli" | ||
| "mas" | ||
| "opencode" | ||
| "ollama" |
There was a problem hiding this comment.
Installing both the ollama formula (CLI) and ollama-app cask (GUI) creates potential conflicts. The Ollama app includes its own CLI binary at the same path, which could cause version mismatches or confusion. Consider documenting why both are needed or removing one if redundant.
| "ollama" |
| - name: Validate Neovim Configuration | ||
| run: make lua-check-neovim | ||
| - name: Validate Neovim Configuration (Dev Shell) | ||
| run: nix develop .# --command make lua-check-neovim |
There was a problem hiding this comment.
[nitpick] The .# syntax should be .#default for clarity. While .# implicitly refers to the default development shell, being explicit improves readability and avoids potential confusion.
| - name: Validate Hammerspoon Configuration | ||
| run: make lua-check-hammerspoon | ||
| - name: Validate Hammerspoon Configuration (Dev Shell) | ||
| run: nix develop .# --command make lua-check-hammerspoon |
There was a problem hiding this comment.
[nitpick] The .# syntax should be .#default for clarity. While .# implicitly refers to the default development shell, being explicit improves readability and avoids potential confusion.
| run: nix develop .# --command make lua-check-hammerspoon | |
| run: nix develop .#default --command make lua-check-hammerspoon |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 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".
| - name: Validate Hammerspoon Configuration (Dev Shell) | ||
| run: nix develop .# --command make lua-check-hammerspoon |
There was a problem hiding this comment.
The macOS job now enters the flake dev shell via nix develop .# --command make lua-check-hammerspoon, but the flake only declares systems = [ "aarch64-darwin" "aarch64-linux" "x86_64-linux" ]. On the default GitHub macos-latest runner (which is still x86_64-darwin), nix develop aborts with “system not supported” before the Lua check runs, breaking the job. Consider switching the runner to an arm64 image or adding x86_64-darwin to the flake systems so the dev shell can be instantiated on macOS CI.
Useful? React with 👍 / 👎.
Mesa DescriptionTL;DRRestore the Ollama service on macOS, integrating it with Homebrew, and update CI Lua checks to run within a Nix development shell that now includes Neovim. What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of f932859...a61400d
Analysis
-
Broken Reproducibility - Homebrew-installed Ollama isn't captured in flake.lock, creating unversioned dependencies outside of Nix's declarative model that could lead to version incompatibilities across different systems.
-
Two-Phase Installation Process - System now requires both
darwin-rebuild switchAND manualbrew install ollamato function properly, breaking the single-command deployment model of Nix. -
Service Reliability Issues - The launchd service with
KeepAlive=truewill continuously restart if the Homebrew binary is missing or incompatible, potentially consuming system resources without clear error reporting. -
Hidden State Dependencies - The Nix configuration now has implicit runtime dependencies on Homebrew state that aren't declared in flake.nix, making the system less predictable and self-documenting.
-
Lack of Version Validation - No mechanism exists to ensure the Homebrew-installed Ollama version is compatible with expected service interfaces, risking runtime incompatibilities after Homebrew updates.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
5 files reviewed | 4 comments | Edit Agent Settings • Read Docs
| "kurtosis-cli" | ||
| "mas" | ||
| "opencode" | ||
| "ollama" |
There was a problem hiding this comment.
This formula is now a required runtime dependency for the home-manager Ollama service, not just an optional installation. Consider documenting this dependency relationship with a comment, and note that since Homebrew formulae aren't version-pinned here, brew upgrade could introduce breaking changes without corresponding updates to the Nix service configuration. You may want to add a comment about version compatibility expectations.
Agent: 🏛 Architecture
| inherit (pkgs) lib; | ||
| inherit (pkgs) lib writeShellApplication; | ||
|
|
||
| # launchd wrapper so we can reuse the Homebrew-installed ollama binary |
There was a problem hiding this comment.
This wrapper pattern creates a runtime dependency on Homebrew state that isn't captured in flake.lock, breaking Nix's reproducibility guarantees. Consider adding: (1) a comment with a TODO and tracking issue for reverting to Nix-packaged ollama when upstream build issues are fixed, (2) version validation to ensure compatibility between the Nix service configuration and Homebrew binary, (3) documentation explaining why this workaround exists.
Agent: 🏛 Architecture
| (lib.getExe ollamaHomebrew) | ||
| "serve" | ||
| ]; | ||
| KeepAlive = true; |
There was a problem hiding this comment.
With KeepAlive=true, if the Homebrew binary is missing or fails immediately, launchd will continuously restart the service, potentially consuming system resources. Consider adding ThrottleInterval or StartInterval to prevent rapid restart loops, or implement a more sophisticated health check that allows graceful degradation when Homebrew dependencies are missing.
Agent: 🏛 Architecture
| text = '' | ||
| set -euo pipefail | ||
|
|
||
| if [ -x /opt/homebrew/bin/ollama ]; then |
There was a problem hiding this comment.
The hard-coded Homebrew paths assume standard installation locations. Consider using command -v ollama or checking additional paths to handle non-standard Homebrew installations (e.g., different architectures or custom prefixes). This would make the wrapper more robust across different macOS configurations.
Agent: 🏛 Architecture
Restore the Ollama service and ensure it is correctly integrated with Homebrew. Update CI workflow validation steps for Neovim and Hammerspoon configurations, enhancing the development shell to include Neovim and improve readiness messaging.
Summary by cubic
Restore the Ollama service on macOS and integrate it with Homebrew for reliable binary management. CI Lua checks now run via the dev shell, which includes Neovim.
New Features
nix develop; dev shell now includes Neovim.Migration
nix-darwin switchto apply changes; Homebrew will install the required packages.Written for commit 0f566a9. Summary will update automatically on new commits.