cleanup: collapse duplicated reporting, and make two guards sound

Quality pass over the preceding six commits. No behaviour change except where
noted; two of these close gaps the first versions left open.

**Two guards were unsound.**

`scanForSymlinkedComponent` tested `Type() == 0` for "a regular file where a
directory belongs", which is a blocklist of one: a FIFO, socket or device node
at `.claude` came back clean and doctor printed nothing, while os.Root and every
hook install fail on it. Now an allowlist of the traversable shapes
(`traversableComponent`), which is what the .entire scan's doc argues for at
length — an allowlist a rejected type can enter by setting an extra bit is not
an allowlist. fs.ModeIrregular stays tolerated: a Windows junction arrives as
bare ModeIrregular and a cloud placeholder directory as ModeDir|ModeIrregular,
and both are traversable.

`RemoveDir` refused an `AllProtectedDirs()` entry, which is a blocklist and
misses the case that matters: that list holds `.opencode` and `.github/hooks`
but not `.opencode/plugins`, `.pi/extensions` or `.github`, so a future agent
whose config sits one level below its root still got
`RemoveAll(".opencode/plugins")` — the user's other plugins. Stated positively
instead: the directory has to be one Entire named. Every agent root and shared
intermediate fails that; pi's `.pi/extensions/entire` passes because Entire
created it.

**Duplication.**

`printCappedList` replaces four inline copies of the same capped-list loop in
doctor.go (three added by 761b87d, one pre-existing in
checkEntireDirSymlinks), so the off-by-one truncation contract is written once.
`testutil.SkipWithoutSymlinks` replaces six copies of the Windows symlink skip
in three different wordings. `agentHelpSkillTemplate` is back to one switch on
agentName, so a fourth agent cannot get a path with no body. The three
worktreeFileName tests are one table, which also makes them agree visibly with
worktreedir's four cases one layer down — and writing it out caught a wrong
expectation of mine: a RELATIVE in-repo link never reaches the resolve, because
os.Root follows it, so the fast path returns the original name.

**Waste and dead state.**

readCapped asked `utf8.ValidString` over the whole 6KB prefix up to four times
to find a rune boundary; `utf8.RuneStart` at the cut answers the same question
by looking at one byte (2081ns -> 2ns measured) and removes the whole-prefix
fallback with it. A new test pins the floor, since a file of nothing but
continuation bytes must still keep its content.

The `vercelJSONName == ""` back-fill was dead — with either value the branch it
guarded was skipped — so the name is now the sole presence signal and
`loadVercelConfigIfPresent` says out loud that no vercel.json means nothing to
read. worktreeFileName's third not-exist exit is gone: after a successful
EvalSymlinks the re-stat can only fail on a race, which is not a state worth
reading as absent. The guard test's helper returns the sorted slice both callers
built by hand, dropping `sortedPackageDirs`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1GW5MCC47ZC1M29PTWJZ9Z2

Author: Soph

cmd/entire/cli/agent/hook_config_file.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"slices"
 
 	"github.com/entireio/cli/cmd/entire/cli/jsonutil"
 	"github.com/entireio/cli/cmd/entire/cli/osroot"
@@ -165,32 +164,40 @@ func (f *HookConfigFile) Remove() error {
 // correct uninstall and taking the parent would delete the user's own config
 // with it.
 //
-// Two refusals, because "every other agent must not call this" was a comment
-// and nothing else. A file directly at the worktree root would make the
-// directory to delete the repository; a file one level down makes it the
-// agent's own top-level directory, so `.claude/settings.json` would take
-// `.claude` — the user's hand-written settings, subagents and skills with it.
-// The second is the one a copy-paste during the next agent integration reaches,
-// which is the same argument the HookConfigLocator build guard makes: a
-// precondition this load-bearing belongs in the code, not in the doc comment
-// above it.
+// Enforced rather than described, because "every other agent must not call
+// this" was a comment and nothing else, and the call it guards is a recursive
+// delete. The precondition is stated positively: the directory has to be one
+// ENTIRE named, which is the only kind it created to hold a generated file.
+//
+// A blocklist of the agents' own directories was the obvious alternative and is
+// not sound. AllProtectedDirs() holds `.opencode` and `.github/hooks` but not
+// `.opencode/plugins`, `.pi/extensions` or `.github`, so deriving the target
+// with path.Dir and checking it against that list still permits
+// RemoveAll(".opencode/plugins") — the user's other OpenCode plugins — for any
+// future agent whose config sits one level deeper than its root. Every agent
+// root and every shared intermediate fails the name test instead, and pi's
+// `.pi/extensions/entire` passes it because Entire is what created it.
 func (f *HookConfigFile) RemoveDir() error {
 	dir := path.Dir(f.name)
 	if dir == "." {
 		return fmt.Errorf("remove %s: refusing to remove the worktree root", filepath.Dir(f.path))
 	}
-	// Compared slash-to-slash: ProtectedDirs entries are repo-relative git-style
-	// paths, and dir came from path.Dir of one.
-	if slices.Contains(AllProtectedDirs(), dir) {
-		return fmt.Errorf("remove %s: refusing to remove an agent's own directory; "+
-			"RemoveDir is for a directory Entire created to hold one generated file", filepath.Dir(f.path))
+	if path.Base(dir) != entireOwnedDirName {
+		return fmt.Errorf("remove %s: refusing to remove %q, which Entire did not create; "+
+			"RemoveDir is only for a directory named %q that holds one generated file",
+			filepath.Dir(f.path), path.Base(dir), entireOwnedDirName)
 	}
 	if err := osroot.RemoveAllNoSymlinks(f.root, dir); err != nil {
 		return fmt.Errorf("remove %s: %w", filepath.Dir(f.path), err)
 	}
 	return nil
 }
 
+// entireOwnedDirName is the directory name Entire uses when it has to create a
+// directory of its own inside a tree an agent owns (`.pi/extensions/entire`).
+// RemoveDir keys its refusal on it.
+const entireOwnedDirName = "entire"
+
 // Root exposes the underlying root and the file's name inside it, for the
 // callers that need a descriptor rather than the bytes. Both are Codex, which
 // bounds .codex/hooks.json on its stat size before reading any of it: Read is

cmd/entire/cli/agent/hook_config_file_test.go

@@ -3,21 +3,20 @@ package agent_test
 import (
 	"os"
 	"path/filepath"
-	"runtime"
 	"testing"
 
 	"github.com/entireio/cli/cmd/entire/cli/agent"
 	"github.com/entireio/cli/cmd/entire/cli/osroot"
+	"github.com/entireio/cli/cmd/entire/cli/testutil"
 	"github.com/stretchr/testify/require"
 )
 
-// skipWithoutSymlinks skips a test that needs to create one. On Windows that
-// takes elevation or developer mode, neither of which CI has.
+// skipWithoutSymlinks skips a test that needs to create one. Kept as a
+// package-local name for its callers; the condition lives in testutil so every
+// package that needs it says the same thing.
 func skipWithoutSymlinks(t *testing.T) {
 	t.Helper()
-	if runtime.GOOS == "windows" {
-		t.Skip("symlink creation needs elevation on Windows")
-	}
+	testutil.SkipWithoutSymlinks(t)
 }
 
 func TestHookConfig_ReadWriteRemoveRoundTrip(t *testing.T) {

cmd/entire/cli/agent_hook_config_guard_test.go

@@ -45,17 +45,15 @@ func TestAllHookConfigRelPaths_CoversEveryWorktreeConfigAgent(t *testing.T) {
 
 	dir := strings.TrimSpace(string(repoRoot))
 	callers := agentPackagesMatching(t, dir, "agent.OpenHookConfig(")
-	require.NotEmpty(t, callers, "the detection pattern has gone stale and must be re-pointed")
 	locators := agentPackagesMatching(t, dir, ") HookConfigRelPath() string {")
-	require.NotEmpty(t, locators, "the detection pattern has gone stale and must be re-pointed")
 
 	// Sets, not counts. len(declared) == len(callers) passed whenever an added
 	// omission was offset by a removal in the same change — the failure this
 	// test exists to catch, since the agent still works and only doctor's
 	// diagnosis goes quiet — and failed on an agent whose call happens to sit in
 	// a sub-package, which is no defect at all. Both sides are package
 	// directories, so they are directly comparable.
-	require.Equal(t, sortedPackageDirs(locators), sortedPackageDirs(callers),
+	require.Equal(t, locators, callers,
 		"the agent packages calling agent.OpenHookConfig and those implementing\n"+
 			"agent.HookConfigRelPath must be the same set. An agent that opens its\n"+
 			"hook config without declaring where it lives leaves the directories\n"+
@@ -70,9 +68,11 @@ func TestAllHookConfigRelPaths_CoversEveryWorktreeConfigAgent(t *testing.T) {
 		len(locators), len(agent.AllHookConfigRelPaths()), strings.Join(agent.AllHookConfigRelPaths(), ", "))
 }
 
-// agentPackagesMatching returns the agent package directories whose non-test
-// sources contain needle.
-func agentPackagesMatching(t *testing.T, repoRoot, needle string) map[string]struct{} {
+// agentPackagesMatching returns the sorted, deduplicated agent package
+// directories whose non-test sources contain needle, asserting that the pattern
+// still matches something — a re-worded signature would otherwise turn this
+// guard into a comparison of two empty sets.
+func agentPackagesMatching(t *testing.T, repoRoot, needle string) []string {
 	t.Helper()
 	grep := exec.Command("git", "grep", "-l", "--fixed-strings", "--", //nolint:noctx // guard test, no cancellation needed
 		needle, "--", ":(glob)cmd/entire/cli/agent/**/*.go")
@@ -85,21 +85,16 @@ func agentPackagesMatching(t *testing.T, repoRoot, needle string) map[string]str
 	out, err := grep.Output()
 	require.NoError(t, err, "no agent source matches %q, which cannot be right", needle)
 
-	pkgs := make(map[string]struct{})
+	var pkgs []string
 	for line := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") {
 		if line == "" || strings.HasSuffix(line, "_test.go") {
 			continue
 		}
-		pkgs[path.Dir(line)] = struct{}{}
+		if dir := path.Dir(line); !slices.Contains(pkgs, dir) {
+			pkgs = append(pkgs, dir)
+		}
 	}
+	slices.Sort(pkgs)
+	require.NotEmpty(t, pkgs, "the detection pattern %q has gone stale and must be re-pointed", needle)
 	return pkgs
 }
-
-func sortedPackageDirs(m map[string]struct{}) []string {
-	out := make([]string, 0, len(m))
-	for k := range m {
-		out = append(out, k)
-	}
-	slices.Sort(out)
-	return out
-}

cmd/entire/cli/doctor.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"log/slog"
 	"os"
 	"path"
@@ -729,19 +730,29 @@ func checkEntireDirSymlinks(cmd *cobra.Command) {
 	}
 
 	fmt.Fprintf(w, "%s contents: SYMLINKS PRESENT\n", paths.EntireDir)
-	for i, name := range links {
-		if i == symlinkReportLimit {
-			fmt.Fprintf(w, "  ... and %d more\n", len(links)-symlinkReportLimit)
-			break
-		}
-		fmt.Fprintf(w, "  %s -> %s\n", path.Join(paths.EntireDir, name), readlinkOrUnknownIn(root, name))
-	}
+	printCappedList(w, links, func(name string) string {
+		return path.Join(paths.EntireDir, name) + " -> " + readlinkOrUnknownIn(root, name)
+	})
 	fmt.Fprintln(w, "  Entire will not create or write through a symlinked directory here, so")
 	fmt.Fprintln(w, "  anything that belongs under one of these paths is not being captured.")
 	fmt.Fprintln(w, "  Fix: replace each path above with a real directory. If it is tracked in git,")
 	fmt.Fprintln(w, "  `git rm --cached` it first, and add it to .gitignore so it does not come back.")
 }
 
+// printCappedList prints one indented line per name via render, replacing the
+// tail past symlinkReportLimit with a count. Four call sites had this loop
+// inline, differing only in the item line, so the off-by-one truncation
+// contract was written out four times.
+func printCappedList(w io.Writer, names []string, render func(string) string) {
+	for i, name := range names {
+		if i == symlinkReportLimit {
+			fmt.Fprintf(w, "  ... and %d more\n", len(names)-symlinkReportLimit)
+			return
+		}
+		fmt.Fprintf(w, "  %s\n", render(name))
+	}
+}
+
 // checkAgentDirSymlinks reports a symlink at any directory component Entire
 // creates or writes through for an agent: the agents' own config directories
 // (.claude, .codex, .cursor, .gemini, .factory, .opencode, .pi, .github/hooks)
@@ -813,13 +824,9 @@ func checkAgentDirSymlinks(cmd *cobra.Command) {
 
 	if len(links) > 0 {
 		fmt.Fprintln(w, "Agent config directories: SYMLINKS PRESENT")
-		for i, name := range links {
-			if i == symlinkReportLimit {
-				fmt.Fprintf(w, "  ... and %d more\n", len(links)-symlinkReportLimit)
-				break
-			}
-			fmt.Fprintf(w, "  %s -> %s\n", name, readlinkOrUnknownIn(root, name))
-		}
+		printCappedList(w, links, func(name string) string {
+			return name + " -> " + readlinkOrUnknownIn(root, name)
+		})
 		fmt.Fprintln(w, "  Entire will not create or write through a symlinked path here, so the")
 		fmt.Fprintln(w, "  hooks and skills that belong under these paths are not installed, and")
 		fmt.Fprintln(w, "  `entire status` reports them as absent rather than as blocked.")
@@ -834,13 +841,9 @@ func checkAgentDirSymlinks(cmd *cobra.Command) {
 	// ErrEntireDirUnreadable, for the same reason.
 	if len(wrongType) > 0 {
 		fmt.Fprintln(w, "Agent config directories: BROKEN")
-		for i, name := range wrongType {
-			if i == symlinkReportLimit {
-				fmt.Fprintf(w, "  ... and %d more\n", len(wrongType)-symlinkReportLimit)
-				break
-			}
-			fmt.Fprintf(w, "  %s is a file, but Entire needs a directory there\n", name)
-		}
+		printCappedList(w, wrongType, func(name string) string {
+			return name + " is not a directory, but Entire needs one there"
+		})
 		fmt.Fprintln(w, "  Entire cannot create the hooks and skills that belong under these paths,")
 		fmt.Fprintln(w, "  so `entire status` reports them as absent rather than as blocked.")
 		fmt.Fprintln(w, "  Fix: replace each path above with a real directory. If it is tracked in")
@@ -853,13 +856,7 @@ func checkAgentDirSymlinks(cmd *cobra.Command) {
 	// install and a doctor that says nothing.
 	if len(unreadable) > 0 {
 		fmt.Fprintln(w, "Agent config directories: NOT READABLE")
-		for i, name := range unreadable {
-			if i == symlinkReportLimit {
-				fmt.Fprintf(w, "  ... and %d more\n", len(unreadable)-symlinkReportLimit)
-				break
-			}
-			fmt.Fprintf(w, "  %s\n", name)
-		}
+		printCappedList(w, unreadable, func(name string) string { return name })
 		fmt.Fprintln(w, "  Entire could not tell whether these paths are real directories, so it")
 		fmt.Fprintln(w, "  cannot say whether hooks and skills can be installed under them.")
 		fmt.Fprintln(w, "  Fix: check the ownership and permissions of each path above.")
@@ -979,21 +976,36 @@ func scanForSymlinkedComponent(root *os.Root, name string) (string, componentSca
 		if info.Mode()&os.ModeSymlink != 0 {
 			return prefix, componentScanLinked
 		}
-		// A regular file with components still to go is a different fault from
-		// an unreadable one, and it is identified here from the mode rather than
-		// from the ENOTDIR the next Lstat would return: Type() == 0 is a regular
-		// file positively, where matching an errno would also have to be right
-		// about which errno each platform picks. fs.ModeIrregular is left out of
-		// this test on purpose, the same way the .entire scan tolerates it —
-		// Windows lands directory junctions and cloud placeholders on that bit,
-		// and both are traversable.
-		if info.Mode().Type() == 0 && i < len(components)-1 {
+		// A component with more path still to go has to be a directory. Stated
+		// as an allowlist of the traversable shapes, not as a test for a regular
+		// file: the .entire scan's doc spends a paragraph on why an allowlist a
+		// rejected type can enter by setting an extra bit is not an allowlist,
+		// and the narrower version here missed a FIFO, socket or device node at
+		// `.claude` entirely — os.Root and every hook install fail on one, and
+		// doctor printed nothing.
+		//
+		// Identified from the mode rather than from the ENOTDIR the next Lstat
+		// would return, which would mean being right about which errno each
+		// platform picks. fs.ModeIrregular is tolerated the way the .entire scan
+		// tolerates it: Windows maps directory junctions and cloud placeholders
+		// onto that bit and both are traversable, and a junction arrives as bare
+		// ModeIrregular (a name-surrogate reparse tag withholds ModeDir) while a
+		// placeholder directory arrives as ModeDir|ModeIrregular.
+		if prefix != name && !traversableComponent(info.Mode()) {
 			return prefix, componentScanWrongType
 		}
 	}
 	return "", componentScanClean
 }
 
+// traversableComponent reports whether mode can hold a path below it: a real
+// directory, or one of the two shapes Windows expresses with fs.ModeIrregular.
+// See scanForSymlinkedComponent for why that bit is tolerated.
+func traversableComponent(mode fs.FileMode) bool {
+	t := mode.Type()
+	return t == fs.ModeDir || t == fs.ModeIrregular || t == fs.ModeDir|fs.ModeIrregular
+}
+
 // readlinkOrUnknown renders a symlink's target for a diagnostic, never failing:
 // an unreadable link is still worth naming.
 func readlinkOrUnknown(name string) string {