Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions .github/workflows/lua.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ jobs:
uses: cachix/install-nix-action@v31
with:
github_access_token: ${{ secrets.GITHUB_TOKEN }}
- name: Install Neovim
run: nix profile install nixpkgs#neovim
- name: Validate Neovim Configuration
run: make lua-check-neovim
- name: Validate Neovim Configuration (Dev Shell)
run: nix develop .# --command make lua-check-neovim

Copilot AI Nov 26, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The .# syntax should be .#default for clarity. While .# implicitly refers to the default development shell, being explicit improves readability and avoids potential confusion.

Copilot uses AI. Check for mistakes.
lua-hammerspoon:
runs-on: macos-latest
timeout-minutes: 300
Expand All @@ -38,8 +36,8 @@ jobs:
uses: cachix/install-nix-action@v31
with:
github_access_token: ${{ secrets.GITHUB_TOKEN }}
- name: Validate Hammerspoon Configuration
run: make lua-check-hammerspoon
- name: Validate Hammerspoon Configuration (Dev Shell)
run: nix develop .# --command make lua-check-hammerspoon

Copilot AI Nov 26, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The .# syntax should be .#default for clarity. While .# implicitly refers to the default development shell, being explicit improves readability and avoids potential confusion.

Suggested change
run: nix develop .# --command make lua-check-hammerspoon
run: nix develop .#default --command make lua-check-hammerspoon

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40

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 Badge Keep macOS CI on supported nix systems

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 👍 / 👎.

lua-check:
if: always()
needs:
Expand Down
22 changes: 17 additions & 5 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,19 @@
};

perSystem =
{ system, pkgs, ... }:
{ system, ... }:
let
overlays = import ./overlays { inherit inputs; };
nixpkgsConfig = import ./lib/nixpkgs-config.nix {
nixpkgsLib = inputs.nixpkgs.lib;
};
devPkgs = import inputs.nixpkgs {
inherit system overlays;
config = nixpkgsConfig;
};
in
{
_module.args.pkgs = devPkgs;
treefmt = {
projectRootFile = "flake.nix";
programs = {
Expand All @@ -133,13 +144,14 @@
};
};

devShells.default = pkgs.mkShell {
devShells.default = devPkgs.mkShell {
packages = [
pkgs.nodejs_20
pkgs.bun
devPkgs.nodejs_20
devPkgs.bun
devPkgs.neovim
];
shellHook = ''
echo "Dev shell ready: Node.js + bun available."
echo "Dev shell ready: Node.js, bun, and Neovim available."
'';
};
};
Expand Down
4 changes: 2 additions & 2 deletions home-manager/services/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ let
codeSyncer = import ./code-syncer { inherit pkgs; };
dotfilesUpdater = import ./dotfiles-updater { inherit pkgs; };
neversslKeepalive = import ./neverssl-keepalive { inherit pkgs; };
# ollama = import ./ollama { inherit pkgs; }; # FIXME: ollama 0.12.11 has build issues with UI assets, uncomment when fixed
ollama = import ./ollama { inherit pkgs; };
brewUpgrader = import ./brew-upgrader { inherit pkgs; };
in
[
brewUpgrader
codeSyncer
dotfilesUpdater
neversslKeepalive
# ollama
ollama
]
21 changes: 19 additions & 2 deletions home-manager/services/ollama/default.nix
Original file line number Diff line number Diff line change
@@ -1,13 +1,30 @@
{ pkgs }:
let
inherit (pkgs) lib;
inherit (pkgs) lib writeShellApplication;

# launchd wrapper so we can reuse the Homebrew-installed ollama binary

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

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

ollamaHomebrew = writeShellApplication {
name = "ollama-homebrew";
text = ''
set -euo pipefail

if [ -x /opt/homebrew/bin/ollama ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

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

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
Comment on lines +11 to +18

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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

'';
};
in
lib.mkIf pkgs.stdenv.isDarwin {
launchd.agents.ollama = {
enable = true;
config = {
ProgramArguments = [
"${pkgs.ollama}/bin/ollama"
(lib.getExe ollamaHomebrew)
"serve"
];
KeepAlive = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

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

Expand Down
3 changes: 2 additions & 1 deletion nix-darwin/config/homebrew.nix
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"kurtosis-cli"
"mas"
"opencode"
"ollama"

Copilot AI Nov 26, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
"ollama"

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

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

"pandoc"
"pinentry-mac"
"pnpm"
Expand Down Expand Up @@ -71,7 +72,7 @@
"linear-linear"
"lm-studio"
"notion"
# "ollama-app" # FIXME: conflicts_with formula is disabled, uncomment when Homebrew fixes conflict
"ollama-app"
"raycast"
"rescuetime"
"screen-studio"
Expand Down
Loading