diff --git a/.agents/skills/worktrees-pnpm/SKILL.md b/.agents/skills/worktrees-pnpm/SKILL.md index 606d1131..ba2d05b1 100644 --- a/.agents/skills/worktrees-pnpm/SKILL.md +++ b/.agents/skills/worktrees-pnpm/SKILL.md @@ -1,6 +1,6 @@ --- name: worktrees-pnpm -description: Create and work in git worktrees in this pnpm workspace. Use when creating a worktree, delegating to a background agent with worktree isolation, or when a worktree fails at commit time with a missing prettier/eslint/tsx binary. Covers why a worktree needs its own pnpm install and how to make that install cheap. +description: Create and work in git worktrees in this pnpm workspace. Use when creating a worktree, delegating to a background agent with worktree isolation, or when a worktree fails at commit time with a missing prettier/eslint/tsx binary. Covers why a worktree needs its own pnpm install, and why worktrees live beside the repo rather than inside it. --- # Worktrees in a pnpm workspace @@ -14,11 +14,17 @@ problem in this repo. **`git worktree add` is not finished until `pnpm install` has run inside the new worktree.** ```bash -git worktree add ../skills- -b -cd ../skills- +git worktree add ../skills-worktrees/ # or -b for a new one +cd ../skills-worktrees/ pnpm install # ← not optional ``` +Worktrees go **beside the repo**, in `../skills-worktrees/`, never inside it. Agent worktrees land there too: the `WorktreeCreate` / `WorktreeRemove` hooks in `.claude/settings.json` (scripts under `.claude/hooks/`) replace the default placement, which would otherwise nest them at `.claude/worktrees/`. + +That is not a matter of taste. A worktree is a complete second checkout, so nesting it inside the repo means every tool that walks the tree from the root walks into it. We hit exactly that: a root `eslint .` traversed 2983 files across two agent worktrees and failed on code an agent had half-written. An ignore rule patches one tool; a sibling directory makes the whole class of problem impossible. One directory also answers "what worktrees do I have?" at a glance — `ls ../skills-worktrees/`. + +A branch can only be checked out in one worktree at a time. If the branch you want is checked out in the primary tree, move that tree to another branch first. + Skipping the install leaves a checkout that looks fine and fails the moment you try to accomplish anything: @@ -38,26 +44,31 @@ pnpm does not provide worktree subcommands. Use `git worktree` — it is the cor (pnpm's own repository has a `worktree:new` helper script, but that is a script in _their_ repo, not a pnpm feature. Do not go looking for it here.) -What pnpm contributes is making the required install cheap; see below. +pnpm's only contribution here is the install you owe the new worktree. -## Making the install cheap +## What the install actually costs -`enableGlobalVirtualStore: true` in `pnpm-workspace.yaml` makes `node_modules` a tree of -symlinks into one content-addressable store shared across worktrees, so a second worktree's -`pnpm install` is close to instant instead of a full materialization. +`du` reports `node_modules` at roughly 370 MB, but that is apparent size. The pnpm store lives +on the same APFS volume, so package content is shared by copy-on-write rather than duplicated; +the real incremental cost of another worktree is far smaller than the number suggests. Disk is +not the reason to avoid a worktree. -It is **not currently enabled in this repo**, deliberately. Before turning it on, know: +**`enableGlobalVirtualStore` is not used here, and that is settled.** It would make installs a +tree of symlinks into one shared store, but it is experimental, pnpm documents it as not working +with ESM under hoisted dependencies, and this repo is ESM throughout. We gain nothing worth that +risk. Do not re-propose it. -- Requires pnpm ≥ **10.12.1** (this repo runs 10.12.4, so the version is fine). -- Defaults to `false`, and pnpm disables it automatically in detected CI. -- **It does not work with ESM when hoisted dependencies are used**, because Node no longer - honours `NODE_PATH` in ESM. This repo is ESM (`packages/cli` is `"type": "module"`) and - currently sets no hoisting configuration, so it is likely fine — but "likely" is why it is - off. Enable it as its own change, with the full test suite as the check. -- The store assumes mutually trusting users and processes. Do not share one writable store - across untrusted agents. +So a worktree install is a normal install. Budget for it; do not skip it. -Until it is enabled, a worktree install is a normal install. Budget for it; do not skip it. +## The in-repo ignore is a backstop, not the mechanism + +`eslint.config.js` still ignores `.claude/worktrees/`, and `.gitignore` still lists it. With the +hooks in place nothing should land there — the ignores exist so that a worktree created by hand +in the old location, or by a tool that bypasses the hooks, cannot silently break a root lint. +Cheap insurance against a failure that is otherwise invisible. + +Verified as unaffected by nesting either way: prettier (its globs do not descend into +dot-directories) and `tsc` (typecheck runs per-package through turbo, not from the root). ## Delegating to a background agent with worktree isolation @@ -77,12 +88,16 @@ Until it is enabled, a worktree install is a normal install. Budget for it; do n ## Cleaning up ```bash -git worktree remove ../skills- # add --force if it has uncommitted changes -git worktree list # confirm +git worktree remove ../skills-worktrees/ # add --force if it has uncommitted changes +git worktree list # confirm ``` Removing the directory by hand leaves a stale registration; `git worktree prune` clears it. +To relocate an existing worktree, use `git worktree move ` — it rewrites the `.git` +pointers, and `node_modules` comes along, so no reinstall is needed. Moving the directory +yourself leaves the worktree pointing at a path that no longer exists. + ## Recovery: the main checkout got switched onto an agent's branch Your work is safe as long as it was pushed — confirm `origin/` and the PR head SHA @@ -96,7 +111,7 @@ git checkout ## When a worktree is worth it -Worktrees cost a full `node_modules` (until the global virtual store is enabled). They earn it +Worktrees cost a `pnpm install` and a little wall-clock. They earn it when you need two branches checked out at once — running a long test suite on one branch while editing another, or letting a background agent work without disturbing your tree. diff --git a/.claude/hooks/worktree-create.sh b/.claude/hooks/worktree-create.sh new file mode 100755 index 00000000..994b9eda --- /dev/null +++ b/.claude/hooks/worktree-create.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# +# WorktreeCreate hook — place worktrees BESIDE the repo, not inside it. +# +# Default Claude Code behavior creates worktrees at /.claude/worktrees/. +# A worktree is a complete second checkout, so nesting it inside the repo means +# every tool that walks the tree from the root walks into it. We hit that: a root +# `eslint .` traversed 2983 files across two agent worktrees and failed on code an +# agent had half-written. An ignore rule patches one tool; placing worktrees +# outside the repo makes the whole class of problem impossible. +# +# Layout: /path/to/ -> /path/to/-worktrees/ +# +# Contract (docs: code.claude.com/docs/en/hooks): +# stdin - JSON with .worktree_id (and .base_path, .cwd, .session_id, ...) +# stdout - the absolute path of the created worktree, plain text, REQUIRED +# exit - non-zero, or zero with empty stdout, fails worktree creation +# This hook replaces the default logic entirely, so it must run `git worktree add`. +set -euo pipefail + +payload=$(cat) +worktree_id=$(printf '%s' "$payload" | jq -r '.worktree_id // empty') +if [ -z "$worktree_id" ]; then + echo "WorktreeCreate: no .worktree_id on stdin" >&2 + exit 1 +fi + +# The id reaches both a filesystem path and a git ref, so constrain it rather +# than trusting its origin: `..` would escape the sibling directory, a leading +# `-` could be read as a flag, and ref-invalid characters would fail the +# `worktree add` further down with a much less obvious error. The harness only +# ever sends simple slugs; this makes that assumption explicit and cheap to keep. +if ! [[ "$worktree_id" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] || [[ "$worktree_id" == *..* ]]; then + echo "WorktreeCreate: refusing unsafe worktree_id '$worktree_id'" >&2 + exit 1 +fi + +# Resolve against the repo this hook was invoked for, not $PWD. +cwd=$(printf '%s' "$payload" | jq -r '.cwd // empty') +repo_root=$(git -C "${cwd:-$PWD}" rev-parse --show-toplevel) +worktree_dir="$(dirname "$repo_root")/$(basename "$repo_root")-worktrees/$worktree_id" + +# Idempotent, but only for a real worktree. A bare directory test would hand back +# a path that git knows nothing about — left by a partial cleanup, an interrupted +# run, or a stray mkdir — and the caller would treat creation as successful. +if git -C "$repo_root" worktree list --porcelain | grep -qxF "worktree $worktree_dir"; then + echo "$worktree_dir" + exit 0 +fi + +# A directory that is NOT a registered worktree is an unsafe place to add one: +# `git worktree add` would fail on a non-empty path anyway, so say why plainly. +if [ -e "$worktree_dir" ]; then + echo "WorktreeCreate: $worktree_dir exists but is not a registered worktree" >&2 + exit 1 +fi + +mkdir -p "$(dirname "$worktree_dir")" + +# Mirror the default: a fresh branch per worktree, based on current HEAD. The +# agent checks out whatever branch it actually needs once inside. -B rather than +# -b so a leftover branch from a removed worktree does not wedge creation. +# All git chatter goes to stderr; stdout carries the path and nothing else. +git -C "$repo_root" worktree add -B "worktree-$worktree_id" "$worktree_dir" >&2 + +echo "$worktree_dir" diff --git a/.claude/hooks/worktree-remove.sh b/.claude/hooks/worktree-remove.sh new file mode 100755 index 00000000..74b2703f --- /dev/null +++ b/.claude/hooks/worktree-remove.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# +# WorktreeRemove hook — counterpart to worktree-create.sh. +# +# Fires when a session ends, a subagent finishes, or a background session is +# deleted. Because WorktreeCreate replaced the default creation logic, this must +# do the corresponding removal. +# +# Contract (docs: code.claude.com/docs/en/hooks): +# stdin - JSON with .worktree_path +# stdout - ignored; this event is side-effect only +# exit - failures are logged in debug mode only and never surface to the user +# +# Because failures are invisible, this errs toward leaving things in place rather +# than deleting aggressively: uncommitted work in a worktree is real work, and a +# stale worktree is a cheap, visible problem (`git worktree list`) whereas +# silently discarded changes are not. +set -euo pipefail + +payload=$(cat) +worktree_path=$(printf '%s' "$payload" | jq -r '.worktree_path // empty') +[ -n "$worktree_path" ] || exit 0 +[ -d "$worktree_path" ] || exit 0 + +repo_root=$(git -C "$worktree_path" rev-parse --path-format=absolute --git-common-dir 2>/dev/null | xargs dirname) || exit 0 + +# Refuse to discard uncommitted changes. `git worktree remove` without --force +# already refuses, but check explicitly so the reason is greppable in debug logs. +if [ -n "$(git -C "$worktree_path" status --porcelain 2>/dev/null)" ]; then + echo "WorktreeRemove: $worktree_path has uncommitted changes; leaving it in place" >&2 + exit 0 +fi + +git -C "$repo_root" worktree remove "$worktree_path" >&2 || { + echo "WorktreeRemove: could not remove $worktree_path; leaving it for manual cleanup" >&2 + exit 0 +} + +# Drop the per-worktree branch this hook's counterpart created, but only if it is +# fully merged — `-d` (not `-D`) so an unmerged branch, which may be the only +# reference to real work, is left alone. Best-effort: both the "Deleted branch" +# notice and any refusal go to stderr for debug logs, and neither is fatal. +branch="worktree-$(basename "$worktree_path")" +git -C "$repo_root" branch -d "$branch" >&2 || true + +# Remove the parent directory only when it is empty, so the sibling directory +# does not linger once the last worktree is gone. +rmdir "$(dirname "$worktree_path")" 2>/dev/null || true diff --git a/.claude/settings.json b/.claude/settings.json index 62c502ca..8ca4ed73 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -51,5 +51,28 @@ "**/credentials/**", "**/secrets/**" ] + }, + "hooks": { + "WorktreeCreate": [ + { + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/worktree-create.sh\"", + "statusMessage": "Creating worktree beside the repo" + } + ] + } + ], + "WorktreeRemove": [ + { + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/worktree-remove.sh\"" + } + ] + } + ] } } diff --git a/eslint.config.js b/eslint.config.js index 112c58c7..16c2dd44 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -17,6 +17,12 @@ export default tseslint.config( "openspec/", "**/test/fixtures/", "tmp/", + // Agent worktrees are full checkouts nested inside the repo. Without + // this, a root `eslint .` lints every worktree's copy of the tree — + // slow, and it fails on whatever an agent has mid-edit. Scoped to + // `worktrees` rather than all of `.claude/` so anything else we put + // there is still checked. + ".claude/worktrees/", // Zero-dependency CommonJS workflow scripts (covered by their own // node:test suite); the app's TS/ESM-oriented rules don't apply. ".github/scripts/",