From b2208277c40f614277599d19cb01cfde42e0fbb3 Mon Sep 17 00:00:00 2001 From: Chiron Date: Wed, 15 Apr 2026 18:22:10 +0000 Subject: [PATCH] docs: add cleanup and improvements plan --- ...-04-15-nixpkgs-cleanup-and-improvements.md | 637 ++++++++++++++++++ 1 file changed, 637 insertions(+) create mode 100644 docs/plans/2026-04-15-nixpkgs-cleanup-and-improvements.md diff --git a/docs/plans/2026-04-15-nixpkgs-cleanup-and-improvements.md b/docs/plans/2026-04-15-nixpkgs-cleanup-and-improvements.md new file mode 100644 index 0000000..0be64a7 --- /dev/null +++ b/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 ; + 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 { 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 ; + 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.