docs(skills): add nixpkgs review guidance

dotfiles/agents/skills/disk-space-cleanup/references/ignore-paths.md

@@ -5,6 +5,9 @@ Use this file to track mountpoints or directories that should be excluded from `
 ## Known Ignores
 
 - `$HOME/keybase`
+- `$HOME/.cache/keybase`
+- `$HOME/.local/share/keybase`
+- `$HOME/.config/keybase`
 - `/keybase`
 - `/var/lib/railbird`
 - `/run/user/*/doc` (FUSE portal mount; machine-specific example observed: `/run/user/1004/doc`)

dotfiles/agents/skills/nixpkgs-review/SKILL.md

@@ -0,0 +1,77 @@
+---
+name: nixpkgs-review
+description: Review or prepare nixpkgs package changes and PRs using a checklist distilled from review feedback on Ivan Malison's own NixOS/nixpkgs pull requests. Use when working in nixpkgs on package inits, updates, packaging fixes, or before opening or reviewing a nixpkgs PR.
+---
+
+# Nixpkgs Review
+
+Use this skill when the task is specifically about reviewing or tightening a change in `NixOS/nixpkgs`.
+
+The goal is not generic style review. The goal is to catch the kinds of issues that repeatedly came up in real nixpkgs feedback on Ivan's PRs: derivation structure, builder choice, metadata, PR hygiene, and JS packaging details.
+
+## Workflow
+
+1. Read the scope first.
+   Open the changed `package.nix` files, related metadata, and the PR title/body if there is one.
+
+2. Run the historical checklist below.
+   Bias toward concrete review findings and actionable edits, not abstract style commentary.
+
+3. Validate the package path.
+   Use the narrowest reasonable validation for the task: targeted build, package eval, or `nixpkgs-review` when appropriate.
+
+4. If you are writing a review:
+   Lead with findings ordered by severity, include file references, and tie each point to a nixpkgs expectation.
+
+5. If you are preparing a PR:
+   Fix the checklist items before opening it, then confirm title/body/commit hygiene.
+
+## Historical Checklist
+
+### Derivation structure
+
+- Prefer `finalAttrs` over `rec` for derivations and nested derivations when self-references matter.
+- Prefer `tag = "v${...}"` over `rev` when fetching a tagged upstream release.
+- Check whether `strictDeps = true;` should be enabled.
+- Use the narrowest builder/stdenv that matches the package. If no compiler is needed, consider `stdenvNoCC`.
+- Put source modifications in `postPatch` or another appropriate hook, not inside `buildPhase`.
+- Prefer `makeBinaryWrapper` over `makeWrapper` when a compiled wrapper is sufficient.
+- Keep wrappers aligned with `meta.mainProgram` so overrides remain clean.
+- Avoid `with lib;` in package expressions; prefer explicit `lib.*` references.
+
+### Metadata and platform expectations
+
+- For new packages, ensure maintainers are present and include the submitter when appropriate.
+- Check whether platform restrictions are justified. Do not mark packages Linux-only or broken without evidence.
+- If a package is only workable through patch accumulation and has no maintainer, call that out directly.
+
+### JS, Bun, Electron, and wrapper-heavy packages
+
+- Separate runtime deps from build-only deps. Large closures attract review attention.
+- Remove redundant env vars and duplicated configuration if build hooks already cover them.
+- Check bundled tool/runtime version alignment, especially browser/runtime pairs.
+- Install completions, desktop files, or icons when upstream clearly ships them and the package already exposes the feature.
+- Be careful with wrappers that hardcode env vars users may want to override.
+
+### PR hygiene
+
+- PR title should match nixpkgs naming and the package version.
+- Keep the PR template intact unless there is a strong reason not to.
+- Avoid unrelated commits in the PR branch.
+- Watch for duplicate or overlapping PRs before investing in deeper review.
+- If asked, squash fixup history before merge.
+
+## Review Output
+
+When producing a review, prefer this shape:
+
+- Finding: what is wrong or risky.
+- Why it matters in nixpkgs terms.
+- Concrete fix, ideally with the exact attr/hook/builder to use.
+
+If there are no findings, say so explicitly and mention remaining validation gaps.
+
+## References
+
+- Read [references/review-patterns.md](references/review-patterns.md) for the curated list of recurring review themes and concrete PR examples.
+- Run `scripts/mine_pr_feedback.py --repo NixOS/nixpkgs --author colonelpanic8 --limit 20 --format markdown` to refresh the source material from newer PRs.

dotfiles/agents/skills/nixpkgs-review/agents/openai.yaml

@@ -0,0 +1,4 @@
+interface:
+  display_name: "Nixpkgs Review"
+  short_description: "Review nixpkgs changes with historical guidance"
+  default_prompt: "Use $nixpkgs-review to review this nixpkgs package change before I open the PR."

dotfiles/agents/skills/nixpkgs-review/references/review-patterns.md

@@ -0,0 +1,105 @@
+# Nixpkgs Review Patterns
+
+This reference is a curated summary of recurring feedback from Ivan Malison's `NixOS/nixpkgs` PRs. Use it to ground reviews in patterns that have already come up from nixpkgs reviewers.
+
+## Most Repeated Themes
+
+### 1. Prefer `finalAttrs` over `rec`
+
+This came up repeatedly on both package init and update PRs.
+
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) `playwright-cli`: reviewer asked for `buildNpmPackage (finalAttrs: { ... })` instead of `rec`.
+- [PR #490033](https://github.com/NixOS/nixpkgs/pull/490033) `rumno`: same feedback for `rustPlatform.buildRustPackage`.
+
+Practical rule:
+- If the derivation self-references `version`, `src`, `pname`, `meta.mainProgram`, or nested outputs, default to `finalAttrs`.
+
+### 2. Prefer `tag` when upstream release is a tag
+
+This also repeated across multiple PRs.
+
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) `playwright-cli`
+- [PR #490033](https://github.com/NixOS/nixpkgs/pull/490033) `rumno`
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`
+
+Practical rule:
+- If upstream publishes a named release tag, prefer `tag = "v${finalAttrs.version}";` or the exact tag format instead of a raw `rev`.
+
+### 3. Use the right hook and builder
+
+Reviewers often push on hook placement and builder/stdenv choice.
+
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`: feedback to move work from `buildPhase` into `postPatch`.
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`: feedback to consider `stdenvNoCC`.
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) `playwright-cli`: prefer `makeBinaryWrapper` for a simple wrapper.
+
+Practical rule:
+- Check whether each mutation belongs in `postPatch`, `preConfigure`, `buildPhase`, or `installPhase`.
+- Check whether the package genuinely needs a compiler toolchain.
+- For simple env/arg wrappers, prefer `makeBinaryWrapper`.
+
+### 4. Enable `strictDeps` unless there is a reason not to
+
+This was called out explicitly on [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465).
+
+Practical rule:
+- For new derivations, ask whether `strictDeps = true;` should be present.
+- If not, be ready to justify why the builder or package layout makes it unnecessary.
+
+### 5. Keep metadata explicit and override-friendly
+
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) `playwright-cli`: reviewer asked to avoid `with lib;`.
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`: reviewer suggested deriving wrapper executable name from `finalAttrs.meta.mainProgram`.
+
+Practical rule:
+- Prefer `lib.licenses.mit` over `with lib;`.
+- Keep `meta.mainProgram` authoritative and have wrappers follow it when practical.
+
+### 6. Maintainers matter for new packages
+
+- [PR #496806](https://github.com/NixOS/nixpkgs/pull/496806) `gws`: reviewer would not merge until the submitter appeared in maintainers.
+
+Practical rule:
+- For package inits, check maintainers early rather than waiting for review feedback.
+
+### 7. PR title and template hygiene are review targets
+
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`: asked to fix the PR title to match the version.
+- [PR #490033](https://github.com/NixOS/nixpkgs/pull/490033) `rumno`: reviewer asked what happened to the PR template.
+
+Practical rule:
+- Before opening or updating a PR, verify the title, template, and branch scope.
+
+### 8. Duplicate or overlapping PRs get noticed quickly
+
+- [PR #490227](https://github.com/NixOS/nixpkgs/pull/490227) was replaced by [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230).
+- [PR #490053](https://github.com/NixOS/nixpkgs/pull/490053) overlapped with [PR #490033](https://github.com/NixOS/nixpkgs/pull/490033).
+- [PR #488606](https://github.com/NixOS/nixpkgs/pull/488606), [PR #488602](https://github.com/NixOS/nixpkgs/pull/488602), and [PR #488603](https://github.com/NixOS/nixpkgs/pull/488603) were closed after reviewers pointed to existing work.
+
+Practical rule:
+- Search for existing PRs on the package before spending time polishing a review.
+- If a branch contains unrelated commits, fix that before asking for review.
+
+### 9. JS/Bun/Electron packages draw runtime-layout scrutiny
+
+This came up heavily on `t3code` and `playwright-cli`.
+
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) `t3code`: reviewers proposed trimming the runtime closure, removing unnecessary env vars, and adding shell completions and desktop integration.
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) `playwright-cli`: reviewers called out mismatched bundled `playwright-core` and browser binaries, and wrapper behavior that prevented user overrides.
+
+Practical rule:
+- For JS-heavy packages, inspect closure size, runtime vs build-only deps, wrapper env vars, and version alignment between bundled libraries and external binaries.
+
+### 10. Cross-platform evidence helps
+
+- [PR #490230](https://github.com/NixOS/nixpkgs/pull/490230) received an approval explicitly noting Darwin success.
+- [PR #497465](https://github.com/NixOS/nixpkgs/pull/497465) got feedback questioning platform restrictions and build behavior.
+
+Practical rule:
+- If the package plausibly supports Darwin, avoid premature Linux-only restrictions and mention what was or was not tested.
+
+## How To Use This Reference
+
+- Use these patterns as a focused checklist before submitting or reviewing nixpkgs changes.
+- Do not blindly apply every point. Check whether the builder, language ecosystem, and upstream release model actually match.
+- When in doubt, prefer concrete evidence from the current package diff over generic convention.

dotfiles/agents/skills/nixpkgs-review/scripts/.gitignore

@@ -0,0 +1,2 @@
+__pycache__/
+*.pyc

dotfiles/agents/skills/nixpkgs-review/scripts/mine_pr_feedback.py

@@ -0,0 +1,208 @@
+#!/usr/bin/env python3
+"""
+Mine external feedback from recent GitHub PRs.
+
+Examples:
+  python scripts/mine_pr_feedback.py --repo NixOS/nixpkgs --author colonelpanic8
+  python scripts/mine_pr_feedback.py --repo NixOS/nixpkgs --author colonelpanic8 --limit 30 --format json
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import subprocess
+import sys
+from collections import Counter
+from concurrent.futures import ThreadPoolExecutor, as_completed
+
+
+def run(cmd: list[str]) -> str:
+    proc = subprocess.run(cmd, capture_output=True, text=True)
+    if proc.returncode != 0:
+        raise RuntimeError(proc.stderr.strip() or f"command failed: {' '.join(cmd)}")
+    return proc.stdout
+
+
+def gh_json(args: list[str]) -> object:
+    return json.loads(run(["gh", *args]))
+
+
+def fetch_prs(repo: str, author: str, limit: int) -> list[dict]:
+    prs: dict[int, dict] = {}
+    for state in ("open", "closed"):
+        data = gh_json(
+            [
+                "search",
+                "prs",
+                "--repo",
+                repo,
+                "--author",
+                author,
+                "--limit",
+                str(max(limit, 30)),
+                "--state",
+                state,
+                "--json",
+                "number,title,state,closedAt,updatedAt,url",
+            ]
+        )
+        for pr in data:
+            prs[pr["number"]] = pr
+    return sorted(
+        prs.values(),
+        key=lambda pr: (pr["updatedAt"], pr["number"]),
+        reverse=True,
+    )[:limit]
+
+
+def fetch_feedback(repo: str, author: str, pr: dict) -> dict:
+    owner, name = repo.split("/", 1)
+    number = pr["number"]
+
+    def api(path: str) -> list[dict]:
+        return gh_json(["api", f"repos/{owner}/{name}/{path}", "--paginate"])
+
+    issue_comments = api(f"issues/{number}/comments")
+    review_comments = api(f"pulls/{number}/comments")
+    reviews = api(f"pulls/{number}/reviews")
+
+    comments = []
+    for comment in issue_comments:
+        login = comment["user"]["login"]
+        body = (comment.get("body") or "").strip()
+        if login != author and body:
+            comments.append({"kind": "issue", "user": login, "body": body})
+    for comment in review_comments:
+        login = comment["user"]["login"]
+        body = (comment.get("body") or "").strip()
+        if login != author and body:
+            comments.append(
+                {
+                    "kind": "review_comment",
+                    "user": login,
+                    "body": body,
+                    "path": comment.get("path"),
+                    "line": comment.get("line"),
+                }
+            )
+    for review in reviews:
+        login = review["user"]["login"]
+        body = (review.get("body") or "").strip()
+        if login != author and body:
+            comments.append(
+                {
+                    "kind": "review",
+                    "user": login,
+                    "body": body,
+                    "state": review.get("state"),
+                }
+            )
+
+    return {**pr, "comments": comments}
+
+
+def is_bot(login: str) -> bool:
+    return login.endswith("[bot]") or login in {"github-actions", "app/dependabot"}
+
+
+def render_markdown(results: list[dict], include_bots: bool) -> str:
+    commenters = Counter()
+    kept = []
+    for pr in results:
+        comments = [
+            comment
+            for comment in pr["comments"]
+            if include_bots or not is_bot(comment["user"])
+        ]
+        if comments:
+            kept.append({**pr, "comments": comments})
+            commenters.update(comment["user"] for comment in comments)
+
+    lines = [
+        "# PR Feedback Summary",
+        "",
+        f"- PRs scanned: {len(results)}",
+        f"- PRs with external feedback: {len(kept)}",
+        "",
+        "## Top commenters",
+        "",
+    ]
+    for user, count in commenters.most_common(10):
+        lines.append(f"- `{user}`: {count}")
+
+    for pr in kept:
+        lines.extend(
+            [
+                "",
+                f"## PR #{pr['number']}: {pr['title']}",
+                "",
+                f"- URL: {pr['url']}",
+                f"- State: {pr['state']}",
+                "",
+            ]
+        )
+        for comment in pr["comments"]:
+            body = comment["body"].replace("\r", " ").replace("\n", " ").strip()
+            snippet = body[:280] + ("..." if len(body) > 280 else "")
+            lines.append(f"- `{comment['user']}` `{comment['kind']}`: {snippet}")
+
+    return "\n".join(lines) + "\n"
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description="Collect review feedback from recent GitHub PRs.")
+    parser.add_argument("--repo", required=True, help="GitHub repo in owner/name form")
+    parser.add_argument("--author", required=True, help="PR author to inspect")
+    parser.add_argument("--limit", type=int, default=20, help="How many recent PRs to inspect")
+    parser.add_argument(
+        "--format",
+        choices=("markdown", "json"),
+        default="markdown",
+        help="Output format",
+    )
+    parser.add_argument(
+        "--include-bots",
+        action="store_true",
+        help="Keep bot comments in the output",
+    )
+    parser.add_argument(
+        "--workers",
+        type=int,
+        default=6,
+        help="Maximum concurrent GitHub API workers",
+    )
+    args = parser.parse_args()
+
+    try:
+        run(["gh", "auth", "status"])
+    except RuntimeError as err:
+        print(err, file=sys.stderr)
+        return 1
+
+    prs = fetch_prs(args.repo, args.author, args.limit)
+
+    results = []
+    with ThreadPoolExecutor(max_workers=args.workers) as pool:
+        futures = [pool.submit(fetch_feedback, args.repo, args.author, pr) for pr in prs]
+        for future in as_completed(futures):
+            results.append(future.result())
+
+    results.sort(key=lambda pr: (pr["updatedAt"], pr["number"]), reverse=True)
+
+    if args.format == "json":
+        if not args.include_bots:
+            for pr in results:
+                pr["comments"] = [
+                    comment for comment in pr["comments"] if not is_bot(comment["user"])
+                ]
+        json.dump(results, sys.stdout, indent=2)
+        sys.stdout.write("\n")
+    else:
+        sys.stdout.write(render_markdown(results, args.include_bots))
+
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())