chore: keep eslint out of agent worktrees

[thecodedrift]

A worktree under .claude/worktrees/ is a complete second checkout nested inside the repo, so a root eslint . walked straight into it. With two agent worktrees live, that was 2,983 extra files — linting whatever an agent had half-written and failing on code the developer never touched. Local-only (CI gets a fresh checkout), but confusing precisely when you are least expecting it.

The ignore is scoped to .claude/worktrees/ rather than all of .claude/, so any JS we put elsewhere under .claude/ or .agents/ stays checked.

I checked the rest of the toolchain rather than assuming eslint was representative: prettier's globs do not descend into dot-directories, and tsc runs per-package through turbo instead of from the repo root. Neither needs an ignore. That is recorded in the skill so the next person does not re-test it — and so anyone adding a repo-root tool that walks the tree knows to give it the same treatment.

Then we went further and moved the worktrees out of the repo entirely. Placement turns out to be configurable: a WorktreeCreate hook replaces the default logic and returns the path it created, with WorktreeRemove as its counterpart. Agent and hand-made worktrees now both land in ../skills-worktrees/<id>, so no repo-root tool can reach them — the class of problem is gone rather than patched per tool. It also makes "what worktrees do I have" a single ls.

The ignore added above stays as a backstop for anything that bypasses the hooks; the cost of a stale ignore line is a smell, the cost of being wrong is silent lint failures.

Two notes for whoever touches this next. The settings schema requires the nested hooks form ([{ "hooks": [{ type, command }] }]) — the flat shape shown on one docs page does not validate, which is worth knowing before hand-editing. And the remove hook deliberately refuses to delete a worktree with uncommitted changes: WorktreeRemove failures are logged in debug mode only and never surface, so silently discarding work would be invisible, whereas a stale worktree is visible in git worktree list.

Both hooks were verified end to end against synthetic stdin payloads — created, returned the sibling path, removed, cleaned up the branch and parent directory — before being wired into settings. The two live agent worktrees were migrated with git worktree move, which rewrites the .git pointers and carries node_modules along, so no reinstall was needed.

The worktrees-pnpm skill also had two things wrong, both now fixed:

• Its placement guidance is now the sibling directory above, with the one-worktree-per-branch constraint that bites when the primary tree already has the branch you want, and a note that relocating an existing worktree is git worktree move rather than mv.
• It described enableGlobalVirtualStore as an open question to evaluate. It is now recorded as rejected: experimental, documented by pnpm as not working with ESM under hoisted dependencies, and this repo is ESM throughout. The skill says not to re-propose it, and explains that disk is not the reason to avoid a worktree anyway — the pnpm store shares package content by copy-on-write on the same volume, so du's ~370 MB is apparent rather than incremental.

Fixes nothing tracked; found while running two agents in parallel worktrees.

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 3m 5s —— View job

---

Review of PR #75

• Gather context (diff, changed files)
• Review .claude/hooks/worktree-create.sh
• Review .claude/hooks/worktree-remove.sh
• Review .claude/settings.json changes
• Review eslint.config.js change
• Review worktrees-pnpm SKILL.md changes
• Post overall assessment

Overall assessment

Solid, well-scoped infra change. The diff matches what the PR description claims, and the reasoning holds up under inspection:

• eslint.config.js: the new .claude/worktrees/ ignore is correctly scoped (not all of .claude/), and "lint": "eslint" in root package.json confirms this was a real problem (bare eslint walks the tree from cwd).
• Hooks (worktree-create.sh / worktree-remove.sh): both use set -euo pipefail, quote all variable expansions, and fail closed in sensible directions — creation errors propagate (non-zero exit / empty stdout fails creation, per the documented contract), while removal errs toward not deleting: uncommitted changes block removal, an unmerged branch is left alone (branch -d, not -D), and a failed git worktree remove leaves the directory for manual cleanup rather than forcing it. That asymmetry is the right call given the PR's own observation that WorktreeRemove failures aren't surfaced to the user.
• settings.json: uses the nested [{ "hooks": [...] }] form for both WorktreeCreate and WorktreeRemove, matching what the PR says is the schema-required shape (the flatter form on the docs page doesn't validate). JSON is well-formed.
• SKILL.md: cross-checked its claims against the repo — .gitignore does still list .claude/worktrees/ (line 19) as described, and the "prettier/tsc are unaffected by nesting" claim is consistent with tsc running per-package via turbo run typecheck rather than from the root. The enableGlobalVirtualStore rejection is now recorded with a concrete reason (ESM + hoisted deps) rather than left as an open question, which is a real improvement for anyone tempted to re-propose it later.

I left two inline comments on worktree-create.sh, both minor/non-blocking:

1. Line 33–37: the idempotency check only verifies the target directory exists, not that it's a real git worktree. A stray directory at that path would be returned as a success. Cheap fix would be checking git worktree list --porcelain instead of [ -d ... ].
2. Line 31: worktree_id is used unvalidated in both the directory path and the branch name. It's quoted everywhere so there's no injection risk, but an unexpected value (e.g. containing ../) could escape the intended sibling directory. Given the value originates from the trusted Claude Code harness rather than external input, this is low severity — flagged as a defense-in-depth suggestion, not a blocker.

No correctness, security, or test-adequacy issues found that would block merging. The WorktreeRemove branch-delete line (>&2 2>/dev/null) is a little cryptic (it routes success messages to stderr and swallows failure messages entirely) but does what the comment above it implies, so I didn't flag it as an issue — just noting it for whoever next has to read that line.

[thecodedrift]

Re: @claude[bot] — "Claude finished @thecodedrift's task in 3m 5s — Review of PR #75…"
https://github.com/taskless/skills/actions/runs/30417133805

Both inline findings fixed in 4a70b71, and the third observation acted on too.

Idempotency check now asks git worktree list --porcelain instead of testing for a directory, so a stray path is no longer reported as a successful creation. A path that exists without being a registered worktree fails loudly rather than falling through.

worktree_id validation — constrained to ^[A-Za-z0-9][A-Za-z0-9._-]*$ with .. rejected. You rated this low severity because the value is harness-supplied, which is fair, but an implicit trust assumption tends to stop being true quietly; enforcing it costs one line. Rejection cases verified individually (../escape, -force, a..b, has space), each exiting 1 with a named reason.

The cryptic >&2 2>/dev/null you noted but did not flag: you were right that it did what the comment implied, and right that it read badly — it sent the "Deleted branch" notice to stderr and swallowed refusals entirely. Now plain >&2 || true, so both outcomes reach debug logs and neither is fatal.

— AI Coding Agent