docs/plans/2026-04-15-nixpkgs-cleanup-and-improvements.md

@@ -0,0 +1,637 @@
+# m3ta-nixpkgs: Cleanup & Improvements Plan
+
+> **For Hermes:** Use subagent-driven-development skill to implement this plan task-by-task.
+
+**Goal:** Address 10 issues identified in codebase review — reduce duplication, improve naming consistency, extract inline scripts, add testing, and update documentation.
+
+**Architecture:** Incremental improvements across lib/, modules/, overlays/, docs/, and CI. Each change is self-contained and can be merged independently. No breaking changes to public API (backward-compat aliases preserved where needed).
+
+**Repo:** `gitea@code.m3ta.dev:m3tam3re/nixpkgs.git` (master branch)
+
+---
+
+## Phase 1: Deduplication & Naming (Low Risk)
+
+### Task 1: Remove duplicate opencode-rules.nix file
+
+**Objective:** Eliminate the duplicate file import. The `coding-rules.nix` is the canonical source; `opencode-rules.nix` is an identical copy. Make the alias a one-liner in `lib/default.nix`.
+
+**Files:**
+- Delete: `lib/opencode-rules.nix`
+- Modify: `lib/default.nix`
+
+**Step 1: Update lib/default.nix to alias directly**
+
+```nix
+{lib}: {
+  ports = import ./ports.nix {inherit lib;};
+
+  coding-rules = import ./coding-rules.nix {inherit lib;};
+
+  # Backward-compat alias: opencode-rules → coding-rules
+  opencode-rules = import ./coding-rules.nix {inherit lib;};
+  opencode = import ./coding-rules.nix {inherit lib;};
+}
+```
+
+**Step 2: Delete the duplicate file**
+
+```bash
+git rm lib/opencode-rules.nix
+```
+
+**Step 3: Verify nothing breaks**
+
+```bash
+nix flake check
+```
+
+**Step 4: Commit**
+
+```bash
+git commit -m "refactor: remove duplicate opencode-rules.nix, use alias in default.nix"
+```
+
+---
+
+### Task 2: Tool-agnostic naming in coding-rules.nix internals
+
+**Objective:** Rename internal variables and output artifacts in `coding-rules.nix` from opencode-specific names to generic names, while keeping the backward-compat alias `mkOpencodeRules`.
+
+**Files:**
+- Modify: `lib/coding-rules.nix`
+
+**Step 1: Rename internal symbols**
+
+In `lib/coding-rules.nix`, rename:
+- `rulesDir` stays `.opencode-rules` (this is a filesystem path used by existing projects, changing it would break)
+- `opencodeConfig` → `rulesConfig`
+- `opencode.json` output → `coding-rules.json` (add a comment noting it was renamed)
+- Add `rulesDir` option to function signature with default `.opencode-rules`
+
+Updated function:
+
+```nix
+{lib}: let
+  mkCodingRules = {
+    agents,
+    languages ? [],
+    concerns ? [
+      "coding-style"
+      "naming"
+      "documentation"
+      "testing"
+      "git-workflow"
+      "project-structure"
+    ],
+    frameworks ? [],
+    extraInstructions ? [],
+    rulesDir ? ".opencode-rules",
+  }: let
+    instructions =
+      (map (c: "${rulesDir}/concerns/${c}.md") concerns)
+      ++ (map (l: "${rulesDir}/languages/${l}.md") languages)
+      ++ (map (f: "${rulesDir}/frameworks/${f}.md") frameworks)
+      ++ extraInstructions;
+
+    rulesConfig = {
+      "$schema" = "https://opencode.ai/config.json";
+      inherit instructions;
+    };
+  in {
+    inherit instructions;
+
+    shellHook = ''
+      # Create/update symlink to AGENTS rules directory
+      ln -sfn ${agents}/rules ${rulesDir}
+
+      # Generate coding-rules configuration file
+      cat > coding-rules.json <<'RULES_EOF'
+      ${builtins.toJSON rulesConfig}
+      RULES_EOF
+    '';
+  };
+
+  # Backward-compat alias
+  mkOpencodeRules = mkCodingRules;
+in {
+  inherit mkCodingRules mkOpencodeRules;
+};
+```
+
+**Step 2: Update shellHook comment in AGENTS.md**
+
+In `AGENTS.md`, update the coding-rules section to mention the new `rulesDir` parameter and the `coding-rules.json` output file.
+
+**Step 3: Verify**
+
+```bash
+nix flake check
+```
+
+**Step 4: Commit**
+
+```bash
+git commit -m "refactor: tool-agnostic naming in coding-rules.nix internals"
+```
+
+---
+
+### Task 3: Remove redundant overlays entry in flake.nix
+
+**Objective:** The `default` and `additions` overlays in `flake.nix` produce identical output. Remove `additions` if not referenced elsewhere, or document why both exist.
+
+**Files:**
+- Modify: `flake.nix`
+- Check: all consumer repos for references to `overlays.additions`
+
+**Step 1: Search for consumers of overlays.additions**
+
+```bash
+# Check nixos-config and other repos
+grep -r "overlays.additions" /data/.hermes/repos/nixos-config/
+grep -r "additions" /data/.hermes/repos/nixos-config/ --include="*.nix" | grep overlay
+```
+
+**Step 2: If no consumers found, remove additions**
+
+In `flake.nix`, simplify overlays to:
+
+```nix
+overlays = {
+  default = final: prev:
+    import ./pkgs {
+      pkgs = final;
+      inputs = inputs;
+    };
+
+  modifications = final: prev: import ./overlays/mods {inherit prev;};
+};
+```
+
+**Step 3: Verify**
+
+```bash
+nix flake check
+```
+
+**Step 4: Commit**
+
+```bash
+git commit -m "refactor: remove redundant 'additions' overlay (identical to 'default')"
+```
+
+**Note:** If `additions` IS used elsewhere, add a comment explaining the convention and skip this task.
+
+---
+
+## Phase 2: Extract Inline Scripts (Medium Risk)
+
+### Task 4: Extract pi-agent runner script to standalone file
+
+**Objective:** Move the ~200-line inline bash script in `modules/nixos/pi-agent.nix` (the `runner` variable) to a separate file `modules/nixos/pi-agent-runner.sh` that gets imported via `builtins.readFile` + `pkgs.writeShellApplication`.
+
+**Files:**
+- Create: `modules/nixos/pi-agent-runner.sh`
+- Modify: `modules/nixos/pi-agent.nix`
+
+**Step 1: Create the runner script file**
+
+Extract the body of the `runner` script (everything inside the `pkgs.writeShellScriptBin cfg.wrapper.runnerName '' ... ''`) into `modules/nixos/pi-agent-runner.sh`.
+
+The script uses Nix-style variable interpolation (`${...}`). We need to keep Nix template variables as `${...}` and convert runtime bash variables to use `$` prefix. Since the script already uses Nix `escapeShellArg` and `escapeShellArg` calls, the cleanest approach is:
+
+Create `modules/nixos/pi-agent-runner.sh` as a template that `pkgs.substituteAll` or `builtins.readFile` + string replacement can process. However, given the heavy Nix interpolation, the pragmatic approach is to use `pkgs.writeShellApplication` with the script body inline but extracted to a `let` binding:
+
+```nix
+# In pi-agent.nix, replace the inline runner with:
+let
+  runnerScript = builtins.readFile ./pi-agent-runner.sh;
+  # ... or keep as let binding but move the body to a separate derivation
+```
+
+**Important caveat:** The script has ~30 Nix variable interpolations (`${cfg.user}`, `${escapeShellArg ...}`, etc.). Full extraction to a .sh file would require either:
+- (a) `substituteAll` with `--replace` for each variable — unwieldy at 30+ substitutions
+- (b) Converting to env vars passed at runtime — cleaner but changes security posture
+- (c) Keeping the Nix interpolation but extracting to a `let` block in a separate `.nix` file
+
+**Recommended approach: Option (c)** — Create `modules/nixos/pi-agent-runner.nix` as a function that takes `cfg` and returns the script:
+
+```nix
+# modules/nixos/pi-agent-runner.nix
+{cfg, pkgs, lib, ...}:
+with lib; let
+  # ... all the helper variables from pi-agent.nix ...
+in
+  pkgs.writeShellScriptBin cfg.wrapper.runnerName ''
+    # ... the script body ...
+  '';
+```
+
+Then in `pi-agent.nix`:
+```nix
+runner = import ./pi-agent-runner.nix {inherit cfg pkgs lib;};
+```
+
+**Step 2: Similarly extract the wrapper script**
+
+Create `modules/nixos/pi-agent-wrapper.nix` for the `wrapper` variable.
+
+**Step 3: Verify**
+
+```bash
+nix flake check
+# Also test in a nixos-rebuild if possible
+```
+
+**Step 4: Commit**
+
+```bash
+git add modules/nixos/pi-agent-runner.nix modules/nixos/pi-agent-wrapper.nix
+git commit -m "refactor: extract pi-agent runner and wrapper to separate files"
+```
+
+---
+
+## Phase 3: Testing (Higher Value)
+
+### Task 5: Add basic lib function tests
+
+**Objective:** Add `nix eval`-based tests for `lib/agents.nix` parseRule logic and `lib/coding-rules.nix` instruction generation.
+
+**Files:**
+- Create: `tests/lib/agents-test.nix`
+- Create: `tests/lib/coding-rules-test.nix`
+- Modify: `flake.nix` (add checks)
+
+**Step 1: Create test infrastructure**
+
+```nix
+# tests/lib/default.nix
+{
+  agents = import ./agents-test.nix;
+  coding-rules = import ./coding-rules-test.nix;
+}
+```
+
+**Step 2: Write agents.nix parseRule test**
+
+```nix
+# tests/lib/agents-test.nix
+let
+  lib = import <nixpkgs/lib>;
+  agentsLib = (import ../../lib {inherit lib;}).agents;
+
+  # Test parseRule helper
+  test1 = let
+    result = builtins.tryEval (
+      let
+        # We can't directly test parseRule since it's internal.
+        # Instead, test the renderer with minimal input.
+        canonical = {
+          test-agent = {
+            description = "Test agent";
+            mode = "primary";
+            systemPrompt = "You are a test.";
+            permissions = {
+              bash = { intent = "allow"; };
+              edit = { intent = "ask"; rules = ["rm -rf *:deny"]; };
+            };
+          };
+        };
+        pkgs = import <nixpkgs> { system = "x86_64-linux"; };
+        rendered = agentsLib.renderForOpencode {
+          inherit pkgs canonical;
+        };
+      in
+        # Verify the derivation builds
+        builtins.pathExists "${rendered}/test-agent.md"
+    );
+  in assert result.value == true; true;
+
+in {
+  parseRule-basic = test1;
+}
+```
+
+**Step 3: Write coding-rules test**
+
+```nix
+# tests/lib/coding-rules-test.nix
+let
+  lib = import <nixpkgs/lib>;
+  codingRulesLib = (import ../../lib {inherit lib;}).coding-rules;
+
+  rules = codingRulesLib.mkCodingRules {
+    agents = "/tmp/fake-agents";
+    languages = ["python"];
+    concerns = ["naming"];
+    rulesDir = ".coding-rules";
+  };
+
+  # Verify instructions are generated correctly
+  test1 = assert rules.instructions == [
+    ".coding-rules/concerns/naming.md"
+    ".coding-rules/languages/python.md"
+  ]; true;
+
+  # Verify backward-compat alias exists
+  test2 = assert codingRulesLib.mkOpencodeRules == codingRulesLib.mkCodingRules; true;
+
+in {
+  instructions-correct = test1;
+  backward-compat = test2;
+}
+```
+
+**Step 4: Add to flake.nix checks**
+
+In `flake.nix`, extend the `checks` attribute:
+
+```nix
+checks = forAllSystems (system: let
+  pkgs = pkgsFor system;
+  packages = import ./pkgs {inherit pkgs inputs;};
+in
+  builtins.mapAttrs (name: pkg: pkgs.lib.hydraJob pkg) packages
+  // {
+    formatting = pkgs.runCommand "check-formatting" {} ''
+      ${pkgs.alejandra}/bin/alejandra --check ${./.}
+      touch $out
+    '';
+    lib-tests = pkgs.runCommand "lib-tests" {} ''
+      ${pkgs.nix}/bin/nix-instantiate --eval ${./tests/lib/default.nix}
+      touch $out
+    '';
+  });
+```
+
+**Step 5: Verify**
+
+```bash
+nix flake check
+```
+
+**Step 6: Commit**
+
+```bash
+git add tests/
+git commit -m "test: add basic lib function tests for agents and coding-rules"
+```
+
+---
+
+### Task 6: Add NixOS VM test for pi-agent module
+
+**Objective:** Add a basic NixOS VM test that verifies the pi-agent module can be evaluated and the wrapper/runner scripts exist.
+
+**Files:**
+- Create: `tests/nixos/pi-agent-test.nix`
+- Modify: `flake.nix` (add to checks)
+
+**Step 1: Write the VM test**
+
+```nix
+# tests/nixos/pi-agent-test.nix
+{pkgs, ...}: {
+  name = "pi-agent";
+
+  nodes.machine = {config, ...}: {
+    imports = [
+      ${(pkgs.path + "/nixos/modules/module-list.nix")}
+    ];
+
+    # Minimal pi-agent config
+    m3ta.pi-agent = {
+      enable = true;
+      package = pkgs.writeScriptBin "pi-agent" ''
+        #!/bin/sh
+        echo "pi-agent mock"
+      '';
+      createUser = true;
+      hostUsers = {
+        testuser = {
+          projectRoots = ["/tmp/test-project"];
+        };
+      };
+    };
+
+    users.users.testuser = {
+      isNormalUser = true;
+    };
+  };
+
+  testScript = ''
+    machine.start()
+    machine.wait_for_unit("multi-user.target")
+
+    # Verify user was created
+    machine.succeed("id pi-agent")
+
+    # Verify wrapper exists
+    machine.succeed("which pi")
+
+    # Verify state directory
+    machine.succeed("test -d /var/lib/pi-agent")
+    machine.succeed("test -d /var/lib/pi-agent/.pi")
+  '';
+}
+```
+
+**Step 2: Add to flake.nix checks**
+
+```nix
+# In the checks attrset:
+pi-agent-vm-test = pkgs.nixosTest (import ./tests/nixos/pi-agent-test.nix {inherit pkgs;});
+```
+
+**Step 3: Verify**
+
+```bash
+nix build .#checks.x86_64-linux.pi-agent-vm-test
+```
+
+**Step 4: Commit**
+
+```bash
+git add tests/nixos/
+git commit -m "test: add NixOS VM test for pi-agent module"
+```
+
+---
+
+## Phase 4: Documentation (Low Risk, High Value)
+
+### Task 7: Update AGENTS.md to reflect current state
+
+**Objective:** Remove outdated migration sections, update function signatures, and align with current code.
+
+**Files:**
+- Modify: `AGENTS.md`
+
+**Step 1: Update the AGENTS REWORK migration section**
+
+The section starting with `## MIGRATION: Agent System (OpenCode → Canonical TOML)` describes a completed migration. Convert it to a brief "Architecture" section that describes the current state, not the migration path.
+
+**Step 2: Update lib.agents function table**
+
+Verify that the function signatures and descriptions in the AGENTS.md table match the actual functions in `lib/agents.nix`. Specifically:
+- `loadCanonical` takes `{agentsInput}` — confirm docs match
+- `renderForPi` now has `primaryAgent` parameter — confirm documented
+- `shellHookForTool` exists — confirm documented
+
+**Step 3: Update coding-rules documentation**
+
+Replace references to `mkOpencodeRules` with `mkCodingRules` as primary, `mkOpencodeRules` as backward-compat alias. Document the new `rulesDir` parameter.
+
+**Step 4: Update overlay documentation**
+
+Remove or annotate the `additions` overlay depending on Task 3 outcome.
+
+**Step 5: Commit**
+
+```bash
+git commit -m "docs: update AGENTS.md to reflect current codebase state"
+```
+
+---
+
+### Task 8: Add CHANGELOG.md
+
+**Objective:** Create a changelog that captures recent work (from git log) so consumers can track changes.
+
+**Files:**
+- Create: `CHANGELOG.md`
+
+**Step 1: Generate changelog from git history**
+
+```bash
+cd /data/.hermes/repos/nixpkgs-review
+git log --oneline --no-merges master | head -30
+```
+
+**Step 2: Write CHANGELOG.md**
+
+Structure as Keep a Changelog format:
+
+```markdown
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/).
+
+## [Unreleased]
+
+## [0.4.0] - 2026-04-15
+
+### Added
+- Pi agent wrapper with per-host-user policy enforcement (`m3ta.pi-agent` NixOS module)
+- `coding.agents.pi` Home Manager module with settings, MCP, and skills support
+- `coding.agents.claude-code` Home Manager module with MCP integration
+- Automated package updates via Gitea Actions (`nix-update` workflow)
+- `lib.agents.renderForPi` with primaryAgent selection and pi-subagents format
+- `pkgs/td` - Task management CLI for AI coding sessions
+
+### Changed
+- Renamed `lib.opencode-rules` → `lib.coding-rules` (backward-compat alias preserved)
+- Agent system migrated to harness-agnostic canonical format
+- Pi settings sync now merges host and Nix-managed values via deep_merge
+
+### Fixed
+- Pi settings sync race condition on first run
+```
+
+**Step 3: Commit**
+
+```bash
+git commit -m "docs: add CHANGELOG.md"
+```
+
+---
+
+## Phase 5: Minor Cleanups (Low Risk)
+
+### Task 9: Clean up pkgs/default.nix unused `system` binding
+
+**Objective:** The `system = pkgs.stdenv.hostPlatform.system;` binding in `pkgs/default.nix` is only used for the two input-pass-throughs. If those are the only consumers, it's fine, but add a clarifying comment.
+
+**Files:**
+- Modify: `pkgs/default.nix`
+
+**Step 1: Add clarifying comment**
+
+```nix
+{
+  pkgs,
+  inputs,
+  ...
+}: let
+  # Only used for flake input pass-throughs below
+  system = pkgs.stdenv.hostPlatform.system;
+in {
+  ...
+```
+
+**Step 2: Commit**
+
+```bash
+git commit -m "docs: clarify system binding in pkgs/default.nix"
+```
+
+---
+
+### Task 10: Remove commented-out overlay entries in overlays/default.nix
+
+**Objective:** Clean up the large block of commented-out code in `overlays/default.nix` (nodejs_24, paperless-ngx, anytype-heart, hyprpanel, etc.). These belong in git history, not in active code.
+
+**Files:**
+- Modify: `overlays/default.nix`
+
+**Step 1: Remove commented-out blocks**
+
+Remove:
+- The `rose-pine-hyprcursor` addition from `additions` (if it's unused — check with grep)
+- The commented-out `nodejs_24`, `paperless-ngx`, `anytype-heart`, `trezord`, `mesa`, `hyprpanel` blocks from `modifications`
+- The commented-out overlay inputs (`temp-packages`, `stable-packages`, `pinned-packages`, `locked-packages`, `master-packages`) if they reference inputs not in `flake.nix`
+
+Actually, `nixpkgs-stable`, `nixpkgs-9e9486b`, `nixpkgs-9472de4`, `nixpkgs-locked`, `nixpkgs-master` are NOT in the current `flake.nix` inputs. These overlays will fail if referenced. They should either be removed or the inputs should be added.
+
+**Action:**
+- Keep `master-packages` IF `nixpkgs-master` is in flake.nix inputs (it IS — good)
+- Remove `temp-packages`, `pinned-packages`, `locked-packages` (inputs don't exist)
+- Keep `stable-packages` IF `nixpkgs-stable` exists in inputs (check — it does NOT currently exist)
+- Keep `additions` with `rose-pine-hyprcursor` IF `rose-pine-hyprcursor` input exists (check)
+
+**Step 2: Verify**
+
+```bash
+nix flake check
+```
+
+**Step 3: Commit**
+
+```bash
+git commit -m "chore: remove dead overlay entries for non-existent flake inputs"
+```
+
+---
+
+## Execution Order & Priority
+
+| Task | Risk | Effort | Impact | Dependencies |
+|------|------|--------|--------|-------------|
+| T1: Remove opencode-rules.nix | Low | 5min | Clean | None |
+| T2: Tool-agnostic naming | Low | 15min | Consistency | None |
+| T3: Remove redundant overlay | Low | 10min | Clean | Check consumers |
+| T9: Clarify system binding | Low | 2min | Docs | None |
+| T10: Remove dead overlays | Low | 10min | Clean | None |
+| T7: Update AGENTS.md | Low | 20min | Docs | After T1, T2 |
+| T8: Add CHANGELOG.md | Low | 15min | Docs | None |
+| T4: Extract pi-agent scripts | Medium | 45min | Maintainability | None |
+| T5: Lib function tests | Medium | 30min | Quality | None |
+| T6: NixOS VM test | Medium | 45min | Quality | None |
+
+**Recommended order:** T1 → T9 → T10 → T3 → T2 → T7 → T8 → T5 → T4 → T6
+
+**Branching strategy:** Create a feature branch `chore/cleanup-review` from master, implement all tasks, open PR for review before merging.