feat: add /make and /lookup skills, unified package registry#44
feat: add /make and /lookup skills, unified package registry#44paulnsorensen wants to merge 8 commits intomainfrom
Conversation
Consolidate .brew, .apt, and brew/packages.yaml into a single packages.yaml with packages/sync.sh. Adds cargo support (lspmux) and SHA-256 hash cache so repeat syncs skip package checks entirely. Closes #31 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Prevents reading file contents from other branches via git plumbing commands, which bypass the Read tool and file access controls. Adds hookify rule and skill with correct alternatives (git diff, checkout). Also adds weak-tests reference doc and copilot-setup review category for tests that prove nothing (tautological assertions, mock echoes, happy-path-only coverage, etc.). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Forked subagent skill that auto-detects build systems (just, cargo, npm, go, make, cmake, gradle, maven, uv/python), runs the build, parses errors into structured file:line:col format, and returns only actionable results. Protects the main context window from verbose compiler output. Includes CLAUDE.md fallback for projects with custom build commands, sandbox-aware error reporting, and multi-step build tracking. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR adds new Claude Code skills (notably /make) to run builds in an isolated subagent and expands the dotfiles sync system with a unified, cached package registry (packages.yaml) plus a new dots sync refresh path to bypass the cache.
Changes:
- Add
claude/skills/make(forked subagent) and related Claude docs/skills. - Consolidate brew/apt/cargo package lists into repo-root
packages.yamland addpackages/sync.shto install them with a hash cache. - Wire package syncing + refresh flag through
.sync-with-rollback, docs, and YAML validation tests.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/config-validation.bats | Validates new packages.yaml location. |
| packages/sync.sh | New unified package sync script (brew/apt/cargo) with hash cache. |
| packages.yaml | Centralized package registry for macOS (brew), Linux (apt), and cargo. |
| claude/skills/make/SKILL.md | New /make skill spec for context-safe builds/tests/fmt/clean. |
| claude/skills/git-hygiene/SKILL.md | New skill doc to avoid git show ref:path file reads. |
| claude/reference/weak-tests.md | New reference doc describing weak test patterns. |
| claude/commands/copilot-setup.md | Adds “weak tests” as a review focus area. |
| bin/dots | Help text updated to mention dots s refresh. |
| CLAUDE.md | Docs updated for new package flow and repo structure. |
| .sync-with-rollback | Adds refresh flag handling and calls packages/sync.sh. |
| brew/packages.yaml | Removed (replaced by unified packages.yaml). |
| apt/packages.yaml | Removed (replaced by unified packages.yaml). |
| .brew | Removed (replaced by packages/sync.sh). |
| .apt | Removed (replaced by packages/sync.sh). |
| @@ -250,15 +254,16 @@ run_sync() { | |||
| esac | |||
| done | |||
|
|
|||
| # install platform packages | |||
| if [ "$(uname)" == "Darwin" ]; then | |||
| log_info "Running homebrew script" | |||
| sh "$dir/.brew" | |||
| elif [ "$(uname)" == "Linux" ]; then | |||
| log_info "Checking apt packages" | |||
| bash "$dir/.apt" | |||
| # Bootstrap yq (needed by packages/sync.sh to parse YAML) | |||
| if [[ "$(uname)" == "Darwin" ]] && ! command -v yq &>/dev/null; then | |||
| log_info "Bootstrapping yq..." | |||
| brew install yq | |||
| fi | |||
|
|
|||
| # Install platform packages | |||
| log_info "Syncing packages" | |||
| bash "$dir/packages/sync.sh" | |||
There was a problem hiding this comment.
New behavior (refresh|r → FORCE_PACKAGES=true and the package sync invocation) isn’t covered by the existing bats CLI tests. Since tests/dots.bats already validates shorthands like dots s, consider adding a small test that dots s refresh is accepted and forwarded (e.g., by stubbing packages/sync.sh in a temp DOTFILES_DIR and asserting it was invoked with FORCE_PACKAGES=true).
| - `dots sync` - Sync dotfiles (symlinks, packages, fonts) with rollback support | ||
| - `dots sync refresh` - Force re-check all packages (bypass cache) |
There was a problem hiding this comment.
The PR title/description focus on adding the /make skill, but this change also documents and introduces a unified packages.yaml + new package sync flow (and removes the old .brew/.apt approach). Consider updating the PR description/title to reflect the additional scope, or splitting the packaging work into a separate PR to keep review/rollback risk smaller.
| # Bootstrap yq (needed by packages/sync.sh to parse YAML) | ||
| if [[ "$(uname)" == "Darwin" ]] && ! command -v yq &>/dev/null; then | ||
| log_info "Bootstrapping yq..." | ||
| brew install yq | ||
| fi |
There was a problem hiding this comment.
brew install yq is invoked without first ensuring Homebrew is installed. On a fresh macOS machine (no brew yet), this will fail and abort the sync before packages/sync.sh can install Homebrew. Consider either installing Homebrew here when missing, or moving the yq bootstrap into packages/sync.sh (install brew → install yq → parse YAML).
| check_cache() { | ||
| if [[ "${FORCE_PACKAGES:-false}" == "true" ]]; then | ||
| log_info "FORCE_PACKAGES set, skipping cache" | ||
| return 1 | ||
| fi | ||
|
|
||
| if [[ ! -f "$CACHE_FILE" ]]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| local current_hash stored_hash | ||
| current_hash=$(shasum -a 256 "$PACKAGES_FILE" | cut -d' ' -f1) | ||
| stored_hash=$(cat "$CACHE_FILE") | ||
|
|
||
| if [[ "$current_hash" == "$stored_hash" ]]; then | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
The package cache key only hashes packages.yaml, but the actual work also depends on runtime flags like DOTFILES_DEV. If a user runs a normal sync (cache saved) and later runs dots sync dev, the cache will incorrectly skip installing dev packages unless they also use refresh. Consider including relevant env flags (e.g., DOTFILES_DEV) in the cached hash/key or bypassing cache when DOTFILES_DEV=true.
| local current_hash stored_hash | ||
| current_hash=$(shasum -a 256 "$PACKAGES_FILE" | cut -d' ' -f1) | ||
| stored_hash=$(cat "$CACHE_FILE") | ||
|
|
||
| if [[ "$current_hash" == "$stored_hash" ]]; then | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } | ||
|
|
||
| save_cache() { | ||
| mkdir -p "$CACHE_DIR" | ||
| shasum -a 256 "$PACKAGES_FILE" | cut -d' ' -f1 > "$CACHE_FILE" | ||
| } |
There was a problem hiding this comment.
check_cache/save_cache rely on shasum, which may not be present on all Linux distros by default. With set -e, a missing shasum will abort the entire sync. Consider falling back to sha256sum when available (or skipping cache when no hashing tool exists).
| sync_cargo() { | ||
| if ! yq -e '.cargo' "$PACKAGES_FILE" > /dev/null 2>&1; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
sync_cargo calls yq unconditionally to detect the .cargo section. If yq is not installed yet (common on Linux before the user installs it via snap), the script will exit due to set -euo pipefail instead of gracefully skipping cargo. Consider checking command -v yq first and either skipping cargo with a clear warning or providing a minimal fallback parser.
| sync_apt() { | ||
| if ! command -v apt-get &>/dev/null; then | ||
| return 0 | ||
| fi | ||
|
|
||
| log_info "Checking apt packages" |
There was a problem hiding this comment.
sync_apt is ~50+ lines long, exceeding the repo's complexity budget (max 40 lines per function). Consider extracting the YAML parsing and the “collect missing packages” loop into small helper functions to keep this function under the limit and make it easier to evolve.
Adds a code intelligence router skill that directs the LLM to the right tool (LSP, Serena, ast-grep, Context7, octocode) instead of brute-force patterns like cargo doc + grep or grepping dependency caches. The hook blocks common anti-patterns at the PreToolUse level: - cargo doc + grep chains for signature lookup - grep in ~/.cargo/registry/ or node_modules/ - grep in target/doc/ (generated docs) - find + xargs/exec grep chains for type discovery Each blocked pattern suggests the correct code intelligence tool. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ision Hook now catches dependency cache grepping across all ecosystems: Rust (.cargo/registry), JS (node_modules, .pnpm-store), Python (site-packages, .venv), Go (go/pkg/mod), Java (.m2), Ruby (.gem). Also catches go doc, pydoc, python help(), and ri + grep chains. Lookup skill adds anti-pattern examples for all languages and explains why it runs inline (LSP/Serena need foreground context) rather than forked. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…routes Hook now catches the original anti-pattern: piping build commands through tail/head/grep (e.g., cargo check 2>&1 | tail -30). Covers 15+ build commands across all ecosystems. Redirects to /make skill. Lookup skill now routes file-finding queries (Glob, fd, rg) and explicitly delegates structural queries to /trace skill. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The block-brute-lookup.js hook was doing double duty: blocking brute-force code lookups (dep cache grepping, doc+grep chains) AND blocking build output piping. Based on analysis of 55 real occurrences in tern transcripts, the build-pipe pattern is better served by hookify rules that provide softer, LLM-readable guidance toward the /make skill. Removed: BUILD_CMD_PATTERNS array and all build-pipe matching logic Kept: dep cache grepping, doc-gen+grep chains, find+grep chains (always wrong) Companion hookify rules (local-only, .claude/ gitignored): - block-build-pipe-tail: blocks build piping, redirects to /make - block-head-source-read: blocks head -N on source files, redirects to Read tool - warn-cat-source-read: warns on cat for source files, suggests Read tool Co-Authored-By: Claude Opus 4.6 <[email protected]>
# Conflicts: # .sync-with-rollback
Summary
/makeskill: Forked subagent (Haiku) that auto-detects build systems, runs builds, and returns structuredfile:line:colerrors. Zero context pollution. Supports check/test/fmt/clean subcommands with CLAUDE.md fallback for custom projects./lookupskill: Code intelligence router that prevents brute-force lookups. Routes to LSP, Serena, ast-grep, Context7, or octocode based on local-vs-external and what-you-need. Inline (not forked) because LSP/Serena need foreground context.block-brute-lookup.jshook: Hard blocks dependency cache grepping (6 ecosystems), doc-gen+grep chains, and find+grep chains. Build output piping delegated to hookify rules.packages.yaml+packages/sync.sh: Unified package registry replacing separate.brew/.apt/cargo/files. Hash-based cache skips reinstall when registry unchanged.git-hygieneskill: Guardrail for safe git operationstdd-assertionsrefactor: Moved framework-specific references toclaude/reference/weak-tests.md, removed pre-commit hook (too noisy)Hookify rules (local-only, not in diff)
Created from analysis of 55+ real anti-pattern occurrences in tern conversation transcripts:
block-build-pipe-tail: Blocks build output piping, redirects to/makeblock-head-source-read: Blockshead -Non source files, redirects to Read toolwarn-cat-source-read: Warns oncatfor source files, suggests Read toolTest plan
dots syncsucceeds with new packages.yaml/makedetects build system in Rust, JS, and Python projects/lookuproutes correctly for local symbols vs external depsblock-brute-lookup.jsblocks grepping dependency caches and doc+grep chains🤖 Generated with Claude Code