Re-land the os.Root coverage follow-ups dropped from #2220 - #2290
Merged
Conversation
Two changes to Name's containment answer. 2ddbae3 paired VolumeName with IsAbs to keep "C:foo" off the name branch. Windows has a second form of the same shape: "\foo", where volumeNameLen is 0 because a single leading backslash is neither a volume nor a UNC prefix, so IsAbs is false and VolumeName is empty and the guard passed. filepath.IsLocal is the single primitive that covers both, and is what os.Root itself uses one layer down — which is also why neither was a reachable escape. Closed for the reason the first one was: Name exists to answer "is this inside the worktree?" independently of the caller. NameFollowingLinks is new, for the callers whose file is the USER's rather than Entire's. os.Root refuses an ABSOLUTE symlink target unconditionally, including one resolving inside the root, so a root alone cannot express "follow a link that stays in the worktree, refuse one that leaves". It resolves the link, then answers for the target. The base is resolved too, against the same rules: without that, every repository below a symlinked component judges its own files from a path that no longer matches them — on macOS /var is a link to /private/var, so a worktree under /var/folders/... reported each of its own files as outside itself. The name returned is worktree-relative and symlink-free, so the caller's read still goes through the root and a link repointed in between cannot escape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1GPKVR0RXKEVYMMTY64E5WR
708f781 moved Vercel detection onto the worktree root and said in two places that an in-repo symlink is still followed, because pointing vercel.json at a monorepo's shared config is a real setup and the file is the user's rather than Entire's. That holds only for RELATIVE links: os.Root refuses an absolute target unconditionally, even one landing inside the root, and the error is not os.ErrNotExist. root.Stat("vercel.json") err=statat vercel.json: path escapes from parent root.Stat("vercel-rel.json") err=<nil> So `entire enable` took the error branch, printed "could not check vercel.json: ... path escapes from parent", and returned false — dropping the feature for a repo where it worked before the anchor went in. CLAUDE.md already records this os.Root property as the reason checkpoint/configloader.go keeps a symlink-following billy.Basic. worktreeFileName tries the root first and, on a refusal that is not "absent", resolves through worktreedir.NameFollowingLinks and retries at the resolved name. That delivers what the comments promised — follow a link that stays inside, refuse one that leaves — with the retried read still going through the root. A dangling link reads as absent, which is what os.Stat gave before. LoadIn's doc now says following an in-repo link is the caller's job, since it cannot be the root's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1GPM7287QNEQ1CH466S1DEQ
Four things in checkAgentDirSymlinks, all of them cases where the check went
quiet or said the wrong thing.
A regular file at .claude made Lstat(".claude/settings.json") return ENOTDIR,
which is not ENOENT, so it landed in componentScanUnreadable and doctor answered
"check the ownership and permissions" for a condition only replacing the path
fixes. That is the else-branch pattern CLAUDE.md's .entire section separates
ErrEntireDirNotDirectory from ErrEntireDirUnreadable to avoid, and for the same
reason: the two remedies are different things. componentScanWrongType reports it
under BROKEN with the replace remedy. Identified from the parent's own mode —
Type() == 0 is positively a regular file — rather than by matching an errno,
and fs.ModeIrregular is left out of that test the way the .entire scan leaves it
out, since Windows lands junctions and cloud placeholders on the same bit.
.claude/agents/ was in no candidate. removeLegacySearchSubagent deletes
.claude/agents/entire-search.md through osroot.LstatNoSymlinks, which refuses a
symlinked parent — its own doc says .claude/agents/ arrives with a checkout — so
a link there was refused at enable with a best-effort warning and doctor said
nothing. .codex/agents and .gemini/agents were covered already, but only as a
side effect of the agent-help template living under them; legacySearchSubagentPath
is now a source in its own right.
A worktree root that would not open returned bare, printing nothing at all on a
repo where every hook install also fails. It reports NOT CHECKED, which is the
stance the per-component path twelve lines down already takes.
searchSkillTemplatePath and agentHelpSkillTemplatePath split the location out of
the template builders. The candidate scan wants nineteen paths and was trimming
and copying twelve multi-KB bodies to get them, then discarding the bytes.
Also records why the list is not gated on the agent being configured: .github
and .agents are shared user-owned trees, so a monorepo symlinking either is told
about it even when those agents are off. Gating would have to ask whether hooks
are installed, and that question is answered by reading through the very config
a symlink hides, so the check would fall silent exactly where it is needed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1GPMPEQE84YNPDGCCE7NKBM
RemoveDir's doc says every other agent must not call it, because "taking the
parent would delete the user's own config with it". The only guard was
dir == ".", and path.Dir is .claude for .claude/settings.json, .cursor for
.cursor/hooks.json, .codex for .codex/hooks.json — so a second caller got
RemoveAll(".claude"), taking hand-written settings, subagents and skills. Pi is
the only caller today; the hazard is a copy-paste during the next agent
integration. It now refuses a directory that is an AllProtectedDirs() entry,
which is the same argument the HookConfigLocator build guard makes: a
precondition this load-bearing belongs in the code.
Write's error message used path.Dir on f.path, an OS-separated absolute path. On
Windows that has no forward slashes, so the message named "." instead of the
directory. filepath.Dir, as the two arms beside it already use.
The guard test compared len(declared) against len(callers). Counts passed
whenever an added omission was offset by a removal in the same change — the
exact failure it exists to catch, since the agent still works and only doctor's
diagnosis goes quiet — and would fail on an agent whose call sits in a
sub-package, which is no defect. It now compares the set of packages calling
OpenHookConfig against the set implementing HookConfigRelPath (eight each), plus
a registry count so a locator that never reaches AllHookConfigRelPaths is its
own failure.
Pi's extension directory went 0755 -> 0750 in 6a02bde, silently: the deleted
line carried a note saying pi reads the directory. Keeping 0750, and saying so —
the eight other agents have been on 0750 all along and pi is not structurally
different, since it runs as whoever ran `entire enable`. A setup where those
differ breaks all nine equally and is not pi's to fix here.
GeneratedState's doc pointed at GeneratedHookFileState, which this branch
deletes, so the only explanation of marker and render was unreachable. Inlined.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1GPN52BK5QQMBPF9V5HBW66
99cef4a bounded the read and re-documented the cap as bytes, but the callers' constants still say chars (tuneDocCap = 6000 // max chars) and the cut is still s[:maxLen], a byte offset. A CLAUDE.md whose 6000th byte falls inside a multi-byte rune put an invalid UTF-8 sequence into the prompt the runner sends to a model — invisible to the caller, since nothing downstream validates. Backs up to the last valid boundary at or below the cap. First tests for readCapped, covering that, a short file, and a missing one. Also: osroot.RemoveAllNoSymlinks wrapped its error with the name while its one caller wrapped again with the absolute path, so a symlinked .pi read "remove /repo/.pi/extensions/entire: remove .pi/extensions/entire: .pi: path is a symlink". Returns unwrapped, like RemoveNoSymlinks directly above it, which leaves the naming to the caller for exactly this reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1GPNGV06MDF6FKS6Q214WK1
323a53b backed up to the last valid UTF-8 boundary at or below the cap, with no floor. On a doc that is not UTF-8 at all — a latin-1 README, where every prefix is invalid and the empty string is not — that walks cut to 0, so the caller got the truncation marker and none of the content: readCapped(latin-1 CLAUDE.md, cap=20) = "\n…(truncated)…" Worse than the pre-existing behaviour, which passed the bytes through, and O(maxLen²) in the process. The backup is now bounded to UTFMax-1 bytes, which is the most a partial trailing rune can be short; anything still invalid past that is the file's own encoding, not our cut, and the under-cap path passes those bytes through too. Test covers it. worktreeFileName discarded resolveErr entirely, so a resolve that failed for its own reason (a permission denied part-way down a link chain) was invisible. The root's refusal stays the wrapped cause, since that is what the user acts on, and resolveErr rides along. Also drops an osroot.ResetShared cleanup from a t.Parallel test added in 761b87d. The registry is process-global, so closing it mid-run breaks whatever is running alongside — here readCapped's tests, which open a root of their own, failed only when the two ran together. The neighbouring tests that call it are serial (t.Chdir), where it is safe; a unique temp dir needs no cleanup at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1GS51Y1479R9VGTRQ9TFPJJ
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
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
readCapped’s new rune-boundary backoff loop can underflow and panic for small caps with certain byte sequences unless cut is guarded.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR re-lands a set of follow-ups to the repo’s os.Root/anchored-filesystem work, focusing on correctly handling in-worktree symlinks for user-owned files (notably vercel.json), tightening uninstall safety for agent hook configs, and improving entire doctor diagnostics for blocked/broken agent directories.
Changes:
- Add worktree-path helpers to correctly resolve and validate symlink targets that remain inside the worktree (including absolute symlink targets that
os.Rootrefuses). - Harden agent hook uninstall behavior (directory removal guards) and improve doctor reporting for symlinked / unreadable / wrong-typed path components.
- Improve runner doc gathering truncation to avoid cutting UTF-8 runes mid-sequence, plus add shared test utilities for symlink-dependent tests.
File summaries
| File | Description |
|---|---|
| cmd/entire/cli/worktreedir/worktreedir.go | Switch to filepath.IsLocal for containment checks and add NameFollowingLinks to support safe symlink resolution inside the worktree. |
| cmd/entire/cli/worktreedir/worktreedir_test.go | Add tests covering NameFollowingLinks behavior for in-repo absolute/relative symlinks and escapes. |
| cmd/entire/cli/vercelconfig/vercelconfig.go | Clarify LoadIn contract: caller resolves symlinked names when needed (esp. absolute targets). |
| cmd/entire/cli/vercelconfig/vercelconfig_test.go | Document/lock in os.Root behavior for absolute symlink targets and the expected caller workaround. |
| cmd/entire/cli/testutil/testutil.go | Add SkipWithoutSymlinks helper for consistent cross-package symlink test skipping. |
| cmd/entire/cli/setup.go | Introduce worktreeFileName and wire Vercel detection to handle absolute in-repo symlinks safely. |
| cmd/entire/cli/setup_test.go | Add table-driven coverage for worktreeFileName symlink/absence/escape cases. |
| cmd/entire/cli/setup_search_skill.go | Factor out searchSkillTemplatePath and centralize .claude directory constant usage. |
| cmd/entire/cli/setup_agent_help_skill.go | Factor out agentHelpSkillTemplatePath while keeping path/body consistency in one switch. |
| cmd/entire/cli/runner_gather.go | Truncate gathered docs on UTF-8 rune boundaries to avoid emitting invalid UTF-8 in prompts. |
| cmd/entire/cli/runner_gather_test.go | Add tests for rune-boundary truncation, non-UTF8 handling, and bounded backoff behavior. |
| cmd/entire/cli/osroot/osroot.go | Adjust RemoveAllNoSymlinks wrapping behavior to avoid duplicate context in errors. |
| cmd/entire/cli/doctor.go | Add capped-list printing helper; improve agent-dir scanning (symlinks, wrong types, unreadable roots). |
| cmd/entire/cli/doctor_test.go | Expand coverage for wrong-typed and non-traversable components and ensure correct remedies. |
| cmd/entire/cli/agent/pi/hooks.go | Align Pi directory permissions with other agents and update explanatory comment. |
| cmd/entire/cli/agent/hook_config_file.go | Fix Windows path formatting in errors; enforce RemoveDir safety by requiring an entire-named directory. |
| cmd/entire/cli/agent/hook_config_file_test.go | Add tests ensuring RemoveDir refuses agent-owned dirs and only deletes Entire-owned directories. |
| cmd/entire/cli/agent_hook_config_guard_test.go | Make the guard compare sets of agent packages rather than counts; extract helper for git-grep matching. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The backoff ran a fixed UTFMax-1 passes with no lower bound, so at maxLen=1 over
a file of continuation bytes the third pass indexed s[-1]:
readCapped(dir, "go.mod", 1) over "\x80\x80..."
panic: runtime error: index out of range [-1]
Introduced by 36daaea, which swapped a whole-prefix utf8.ValidString for
utf8.RuneStart at the cut. The count-based bound was safe with ValidString —
s[:0] is a legal slice — and became an out-of-range index once it was used to
subscript. Found by Copilot and by the trail's own review, both on the same
line. No caller passes a cap this small (the smallest is go.mod's 400), so it
was latent, which is the reason to pin it rather than to shrug.
The floor is now explicit (`max(0, maxLen-(utf8.UTFMax-1))`) instead of implied
by an iteration count, and a rune start that never turns up leaves the cut at
maxLen — so the file's own bytes are kept rather than dropped, which is what the
preceding commit was about.
Behaviour at every cap 0..7, over an all-continuation body, a
continuation-prefixed body and a valid two-byte-rune body: no panic; the cut
lands on a rune boundary or keeps the file's bytes; an empty result only below
one rune's width, where no non-empty valid prefix exists. The test asserts that
disjunction rather than "not empty", because "" is the correct answer at
maxLen=1 over a 2-byte rune — an earlier draft of it asserted non-empty and was
wrong about the contract.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1X8562W7XKYF5Z7Z76YMBQS
test-windows failed to BUILD on a8f9075: cmd\entire\cli\doctor_test.go:1779:20: undefined: syscall.Mkfifo syscall.Mkfifo does not exist on Windows, so guarding it with `if runtime.GOOS == windowsGOOS { t.Skip(...) }` was the wrong tool: a runtime skip still has to compile. Moved to doctor_fifo_unix_test.go behind `//go:build unix`, which is the guard that actually applies, and the now-redundant runtime skip is gone with it. `GOOS=windows go vet ./cmd/entire/cli/` reproduces the failure in a second and would have caught it before the push; `mise run test:ci` cannot, because it compiles for the host only. Worth reaching for on any change that touches syscall, build tags or platform-specific paths — the whole tree now type-checks clean for windows, linux and darwin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1X8T65FGPEZTK7W58F1PJVR
Nine findings from the trail's review of the re-land. The substantive one first.
**A FIFO at the leaf was reported clean.** The type check added in this branch
was gated on `prefix != name`, so it covered intermediate components only, and
the doc sentence claiming "the leaf is refused too" became false for types while
staying true for symlinks. Both positions are now checked, with the expectation
depending on where the component sits: a directory above, a regular file at the
leaf (`componentHasExpectedShape`).
That matters more than the parent case, because a FIFO there does not fail the
read, it blocks it. Verified with this branch's binary, in a repo where Entire is
not enabled:
mkdir -p .claude && mkfifo .claude/settings.json && entire doctor
# prints "Metadata branches: OK", then blocks in openat indefinitely
os.(*Root).Open root.go:103
osroot.OpenNoFollow osroot/osroot.go:80 <- parent.Open, no O_NONBLOCK
agent.(*HookConfigFile).Read agent/hook_config_file.go:112
claudecode.loadClaudeSettings claudecode/hooks.go:433
The hang is PRE-EXISTING — `OpenNoFollow` is untouched by this branch — and every
agent shares the shape, so refusing to open a non-regular leaf belongs there
rather than here and is left for a separate change. This commit only makes the
condition nameable, which is all a scan can do.
The item line now names what it found, via `paths.DescribeMode` exported for the
purpose: the two scans describe the same conditions in the same words instead of
growing a second vocabulary for them.
**The rest.** A 22-line doc block documenting the agent→directory mapping was
left heading `const claudeDirName` when that constant was inserted above it,
leaving `searchSkillTemplate` undocumented; it moves onto
`searchSkillTemplatePath`, which owns the mapping now. `worktreeFileName`'s bool
was the same fact as its name on every return path, so it is gone and both
callers test the name — the argument its own call-site comment already made.
`skipWithoutSymlinks` was a seventh copy of the skip in the change that removed
six; inlined at its three call sites. Pi's eight new comment lines explained
#2220's directory-mode change, in the wrong file, splitting a sentence in half:
the two-liner is restored and the 0750 rationale now sits on the
MkdirAllNoSymlink that sets it, stated once for all nine agents.
The guard test's second assertion is a count sitting directly under the argument
against counts. It stays a count — mapping a package directory to the rel path it
declares is not derivable, since `geminicli` declares `.gemini/settings.json` and
`copilotcli` declares `.github/hooks/entire.json` — and now says so, so the next
reader does not have to work out whether it was an oversight.
Plus a duplicated comment line, two wrong counts ("four shapes" over a six-row
table), `claudeDirName` half-applied across five sites in doctor_test.go, and
three comments narrating this branch's own revisions rather than the reasons
behind them.
Verified: lint 0 issues, test:ci exit 0, and `GOOS=windows|linux|darwin go vet
./...` all clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1XM81MKG3QHJVXTQQZ89B8W
The byte cap cutting a multi-byte rune in prompt text has nothing to do with os.Root coverage, and it is the one piece of this branch that stands alone, so it goes to #2307 rather than riding along here. Raised in review of the re-land. runner_gather.go and runner_gather_test.go are now identical to main on this branch, so the two PRs do not overlap and either can land first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1XMSQWVJ8XN0J5JPV6N7MX6
gtrrz-victor
approved these changes
Sep 7, 2026
timothybrush
pushed a commit
to timothybrush/cli-2
that referenced
this pull request
Sep 7, 2026
readCapped's cap is a byte budget, applied as `s[:maxLen]`, while both callers'
constants describe it in characters:
tuneDocCap = 6000 // max chars embedded per doc (CLAUDE.md etc.)
tuneReadmeCap = 2000
So a CLAUDE.md whose 6000th byte falls inside a multi-byte rune put an invalid
UTF-8 sequence into the prompt `entire runner tune` sends to a model. Nothing
downstream validates, so it was invisible from the call site.
A continuation byte at the cut is exactly what "we cut mid-rune" means, so the
fix backs off the continuation bytes — at most UTFMax-1 of them, which is the
furthest a rune's start can be. `utf8.RuneStart` answers that from one byte,
where validating the prefix walks the whole 6KB and answers a different question:
"is this valid?" is false for a doc that is not UTF-8 at all, and chasing it back
drops the file's content in favour of a bare truncation marker. Invalidity our
cut did not cause is the file's own, and the under-cap path passes those bytes
through too.
Two traps, both pinned by tests rather than described:
- A latin-1 README came back as just the truncation marker when the backoff had
no bound, which is worse than the mid-rune cut it was fixing.
- The floor has to be explicit rather than implied by an iteration count. At
maxLen=1 over a file of continuation bytes, a count-based bound indexes s[-1]
and panics — latent, since the smallest cap any caller passes is go.mod's 400.
First tests for readCapped: the boundary itself, a short file, a missing one, a
non-UTF-8 doc keeping its content, and every cap from 0 to 7 over three body
shapes asserting the contract as a disjunction — the cut lands on a rune
boundary, or it keeps the file's own bytes. An empty result is correct below one
rune's width, where no non-empty valid prefix exists.
Split out of the os.Root coverage follow-ups (entireio#2290): a byte cap cutting a rune
in prompt text is unrelated to filesystem anchoring, and this is the piece of
that branch that stands alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1XMH1AYKXWZHY2VHXNK3MR6
MuskanPaliwal
pushed a commit
to MuskanPaliwal/cli
that referenced
this pull request
Sep 7, 2026
A named pipe at a path Entire reads hung the process instead of failing it.
open(2) on a FIFO with no writer blocks until one arrives, and none of the
NoFollow helpers passes O_NONBLOCK, so in a repo with a FIFO at
.claude/settings.json:
mkdir -p .claude && mkfifo .claude/settings.json && entire doctor
# prints "Metadata branches: OK", then blocks in openat. SIGINT only.
os.(*Root).Open root.go:103
osroot.OpenNoFollow osroot/osroot.go:80 <- parent.Open, no O_NONBLOCK
agent.(*HookConfigFile).Read agent/hook_config_file.go:112
claudecode.loadClaudeSettings claudecode/hooks.go:433
claudecode.CheckHookConfig claudecode/hooks.go:499
OpenNoFollow already Lstats the leaf to reject a symlink, so the type check goes
in the same place and costs nothing. Being BEFORE the open is the whole point: a
post-open check cannot help, because the open is what blocks.
Directories are refused too. io.ReadAll of one already failed a step later with
a platform-dependent errno, and a caller asking for a file wants the refusal
rather than an EISDIR from the middle of its read.
Scope, measured rather than assumed:
- `entire doctor` hangs. `entire status` and `entire agent list` do NOT — they
reach the config through Exists (an Lstat), not Read. The exposure is narrower
than "every command that reads an agent config", which is what I first wrote.
- `git status --porcelain` does not list a FIFO at all, so the checkpoint path
that reads working files from status output was never exposed.
- Every read funnels through here, so the fix covers all nine agents plus
permissions.go, the plugin manifest, doctor's log readers and the settings
loader. Verified no caller wants a non-regular file: the one that reads from a
directory listing (review/manifest.go:737) already skips entry.IsDir().
ErrNotRegularFile is deliberately not classified as os.ErrNotExist. Several
callers reach these helpers to decide "is there a config here?", and answering
"absent" for an occupied path would have Entire write a fresh file over whatever
is there. A missing file still classifies as before, which a test pins.
fs.ModeIrregular is masked out rather than matched, the same tolerance the
.entire entry scan applies: Windows maps uncategorised reparse tags onto that
bit, which lands OneDrive Files On-Demand placeholders there, and a placeholder
is a readable file.
The FIFO test races the open against a timer instead of asserting on its error,
because a regression does not fail here — it hangs, and a plain assertion would
take the package's timeout with it.
Follow-up to entireio#2290, which made this condition reportable by checking the leaf's
type in doctor's scan but could not stop the hang. Independent of it: no shared
files, either can land first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M1XP8G6Y4CN4CR752AM0M9FD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/1240
Follow-up to #2220. These seven commits were pushed to
paulo-goroot-ognibbefore it merged, but a force-push to that branch dropped them, so none of it reachedmain— every issue below is still live there. Rebased onto currentmainand re-proposed unchanged, apart from three merge resolutions noted at the end.The first six came out of reviewing #2220; the seventh is a quality pass over the other six.
The one that changes behaviour for users today
entire enablesilently drops Vercel deployment blocking in a repo whosevercel.jsonis an absolute symlink into a monorepo's shared config.708f78136moved the detection onto the worktree root and said in two places that in-repo links are still followed. That holds only for relative links —os.Rootrefuses an absolute target unconditionally, even one landing inside the root, and the error is notos.ErrNotExist:So the error branch printed
could not check vercel.json: ... path escapes from parentand returned false.worktreeFileNametries the root first and, on a refusal that is not "absent", resolves through the newworktreedir.NameFollowingLinksand retries at the resolved name — following a link that stays inside, refusing one that leaves, with the retried read still going through the root. CLAUDE.md already records thisos.Rootproperty as whycheckpoint/configloader.gokeeps a symlink-followingbilly.Basic.Two guards that were not sound
RemoveDirguarded only the worktree root, sopath.Dir(".claude/settings.json")is.claudeand the next agent integration to copy pi's uninstall getsRemoveAll(".claude")— the user's hand-written settings, subagents and skills. A blocklist ofAllProtectedDirs()was the obvious fix and is also unsound: that list holds.opencodeand.github/hooksbut not.opencode/plugins,.pi/extensionsor.github, so an agent whose config sits one level below its root still reachesRemoveAll(".opencode/plugins"). Stated positively instead — the directory has to be one Entire named. Every agent root and shared intermediate fails that; pi's.pi/extensions/entirepasses because Entire created it.scanForSymlinkedComponenttestedType() == 0, a blocklist of one, so a FIFO, socket or device node where a directory belongs came back clean and doctor printed nothing — whileos.Rootand every hook install fail on it. Now an allowlist of the traversable shapes, which is what the.entirescan's doc argues for at length.fs.ModeIrregularstays tolerated: a Windows junction arrives as bareModeIrregularand a cloud placeholder directory asModeDir|ModeIrregular, and both are traversable.Diagnosis that named the wrong thing, or nothing
A regular file at
.claudewas reported asNOT READABLEwithFix: check the ownership and permissions— a remedy that cannot fix it. That is the else-branch pattern CLAUDE.md's.entiresection separates two error values to avoid; it now gets its ownBROKENheading and the replace remedy..claude/agents/was in no candidate, thoughremoveLegacySearchSubagentdeletes through it withosroot.LstatNoSymlinks, which refuses a symlinked parent. A link there was refused at enable with a best-effort warning and doctor said nothing..codex/agentsand.gemini/agentswere covered only as a side effect of the agent-help template living under them.A worktree root that would not open returned bare, printing nothing on a repo where every hook install also fails.
Smaller correctness
Write's error message passedf.pathtopath.Dir, which finds no forward slashes on Windows and yielded".".The build guard compared
len(declared) == len(callers), so an added omission offset by a removal in the same change passed — the exact failure it exists to catch, since the agent still works and only doctor's diagnosis goes quiet. It compares sets now (eight packages each side).Quality pass
printCappedListreplaces four inline copies of the same capped-list loop;testutil.SkipWithoutSymlinksreplaces six copies of the Windows symlink skip in three wordings;agentHelpSkillTemplateis back to one switch so a fourth agent cannot get a path with no body; theworktreeFileNametests are one table. Writing that table caught a wrong expectation of mine — a relative in-repo link never reaches the resolve, becauseos.Rootfollows it, so the fast path returns the original name.Merge resolutions worth a look
Three conflicts came from
mainmoving under these commits, and all three keepmain's side:agent_hook_config_guard_test.go—mainaddedgrep.Env = gitrepo.EnvWithoutRepoOverrides()to the git-grep this commit extracts into a helper. Carried into the helper rather than dropped; it is the CLAUDE.md rule for any git subprocess naming its target withcmd.Dir.runner_gather_test.go—maincreated this file forTestGatherTrailsUsesNativeRepoBaseForFindings. Both sets of tests kept, imports unioned.setup_test.go— append-vs-append withTestConfigureCmd_SummarizeProvider_ExternalLocalTarget_GrantSurvives. Both kept.Split out
readCapped's rune-boundary fix moved to #2307 on review feedback — a byte cap cutting a multi-byte rune in prompt text is unrelated to filesystem anchoring, and it was the one piece of this branch that stood alone.runner_gather.goandrunner_gather_test.goare now identical tomainhere, so the two PRs do not overlap and either can land first. The three commits that developed it plus the revert are still in this branch's history and net to zero; the diff is what matters.Still open, deliberately not in here
A FIFO at an agent config leaf hangs every command that reads it,
entire doctorincluded. Reproduced on this branch in a repo where Entire is not enabled:mkdir -p .claude && mkfifo .claude/settings.json && entire doctorblocks inopenatuntil interrupted, becauseosroot.OpenNoFollowopens withoutO_NONBLOCK. Pre-existing and shared by all seven worktree-config agents, so the fix belongs inOpenNoFollowrather than here; this PR makes the condition reportable (the leaf's type is now checked) but cannot stop the hang. Filed separately.readCappedopens withosroot.OpenNoFollow, and this repo's ownAGENTS.mdis a symlink toCLAUDE.md— soentire runner tunedrops it. Harmless here becauseCLAUDE.mdis read anyway, but a repo with only a symlinkedAGENTS.mdloses it. That is a product call (should the runner read symlinked docs at all?) rather than a fix to slip into a follow-up, and it predates these commits.Two structural follow-ups also left out: making
OpenHookConfigtake aHookConfigLocator, which turns the guard test into a compile error but touches nine agent packages; and giving the registry aManagedWorktreePaths()capability so "what does Entire write for this agent?" is the registry's question rather than threeswitch agentNameblocks incli.Verification
mise run lint0 issues andmise run test:ciexit 0 on the rebased tree. The touched tests also run 5x together, because one of these commits fixed at.Paralleltest that calledosroot.ResetSharedand broke a different test — a single clean run does not catch that class.🤖 Generated with Claude Code
Note
Medium Risk
Touches enable/setup (Vercel), recursive delete guards, and doctor path scanning—behavior changes for symlinked configs and mis-typed paths, with broad test coverage but real install/diagnostic surface area.
Overview
Re-lands follow-ups from the
os.Root/ worktree safety work: safer uninstalls, clearerentire doctoroutput, and a user-visible fix for Vercel setup whenvercel.jsonis symlinked.Vercel / monorepo symlinks: Setup no longer treats an in-repo absolute
vercel.jsonsymlink as a hard failure (os.Rootreports “path escapes”).worktreeFileNameandworktreedir.NameFollowingLinksresolve links that stay inside the worktree, refuse escapes, and pass the resolved name intovercelconfig.LoadIn.Hook config safety:
HookConfigFile.RemoveDirnow refuses unless the parent directory basename isentire(pi’s.pi/extensions/entire), so copying that uninstall path cannotRemoveAll.claudeor similar agent-owned trees.Writeerror text usesfilepath.Diron Windows.Doctor / agent paths: Agent-directory scanning adds a BROKEN section when a path component is a non-directory (files, FIFOs, etc.) instead of “NOT READABLE”; expands candidates (e.g.
.claude/agents/, legacy subagent paths) viasearchSkillTemplatePath/agentHelpSkillTemplatePath; reports when the worktree root cannot be opened; dedupes long lists withprintCappedList. TheOpenHookConfig↔HookConfigRelPathguard compares package sets, not counts.Smaller fixes:
readCappedtruncates on UTF-8 rune boundaries; Pi extension dirs use0750like other agents; sharedtestutil.SkipWithoutSymlinks;worktreedir.Nameusesfilepath.IsLocal.Reviewed by Cursor Bugbot for commit edc15a4. Configure here.