Tighten Trail Worktree Checkout Guards

Using pfleidi:review to run the branch review exactly against origin/main. Crawler has summoned the audit bureau; the paperwork has teeth.

I’m reading the branch shape first: diff stat, exact file list, commits, and PR metadata if GitHub has one. The review base is origin/main, because local main is where certainty goes to die.

Changed files against origin/main: 4 files, 1031 insertions, 3 deletions.

Spawning the review panel now. Five lenses, because this is Go and apparently the branch wanted a boss fight.

The review skill delegates de-slop and Go-cleanliness criteria to local skill docs, so I’m loading those before the specialist agents get their marching orders. No inventing standards mid-review; the dungeon already has bylaws.

Launching the first-pass reviewers. They are read-only, source-backed, and instructed to return nothing if nothing earns it. Bold policy, I know.

The first spawn call bounced because full-history forks can’t override agent type. Retrying without the fork; same instructions, less ceremony.

While they run, I’m doing the local pass over the changed implementation and tests. Subagents catch breadth; I still verify anything before it earns table space.

The implementation centers on trail_checkout_worktree.go: it creates a managed worktree, fetches missing branches, updates .gitignore, and copies ignored files matched by .worktreeinclude. I’m tracing the filesystem boundary now; that’s where this kind of code usually hides the knives.

One local note so far: the code is deliberately using os.Root and same-file checks around .worktreeinclude copies, which is the right neighborhood for symlink escape resistance. I’m checking whether any edge around branch/ref handling or stale worktree registration slips past that.

One probe looked for the remote fetch helper at the wrong filename; the repo answered by pointing at checkpoint/remote/git.go. Annoying, useful, and now I’m reading the actual helper.

I found a likely filesystem-boundary concern locally: paths under .entire/worktrees and .gitignore are created/updated with ordinary OS calls, so repo-controlled symlinks may matter. I’m waiting for the specialist pass before I promote that from “suspicious stain” to a finding.

Readability reported clean. I’m still waiting on security, correctness, de-slop, and clean-Go; meanwhile I’m checking the suspicious symlink/write path against tests and existing osroot helpers.

Security confirmed the .gitignore symlink write. I’m also checking the sibling issue: the managed worktree directory itself is created with os.MkdirAll, so a repo-owned .entire symlink may redirect the supposedly managed checkout. That one is extra spicy if true.

Clean Go reported clean. The remaining reviewers are correctness and de-slop; I’m using the time to verify whether the .entire/worktrees root is actually protected or just wearing a tiny paper helmet.

De-slop is clean too. One first-pass agent remains, then I’ll run the independent coverage sweep the skill requires and verify any new claim against source.

First pass is complete: two Required findings emerged, both eligible on paper. I’m launching the independent coverage sweep now, then I’ll verify and deduplicate before touching code.

I’m validating the stale-worktree finding against Git’s porcelain shape and the current parser. The bug is the classic “directory exists therefore universe is fine” decision. Bold.

The test file reuses a package-level runGit helper from another test file, so the checkout tests are compiling through shared package test helpers. Minor footnote, not a finding.

The stale-worktree parser is shared with resume worktree clash detection, so any fix there has to keep existing resume behavior intact. Translation: one local helper, two traps, no broad refactor.

Coverage is still running, so I’m checking the fix logistics: whether there’s a project-local temp artifact dir for the required ledger, and which focused package tests will cover this.

./tmp/ exists and is ignored, so if fixes are applied I can write the review ledger under tmp/pfleidi-review/. Verification will be focused: gofmt on touched files, go test ./cmd/entire/cli -run ..., go test ./cmd/entire -run TestBuild -count=1 if needed? Need find build compile. More likely go test ./cmd/entire/cli -run 'Test...' -count=1 compiles package. Lint mise run lint maybe project lint not scoped. Need read lint task.

There is no explicit lint task in visible mise.toml, but the repo docs mention mise run lint; I’ll use that for lint verification if fixes happen. Compile can be covered by go build ./cmd/entire/, and focused tests by go test ./cmd/entire/cli.

For the .gitignore fix, I’m checking the os.Root API instead of hand-rolling no-follow behavior. If the standard library gives us a constrained primitive, we use it; if not, we keep the failure mode explicit.

os.Root follows symlinks inside the root but blocks escape outside the root, and it has Lstat. That gives a clean fix for .gitignore: reject a symlink before reading/writing, then perform access through the repo root handle.

The coverage agent still hasn’t returned. I’m not blocking idle: I’m pinning the two likely fixes to focused tests so the eventual report can move straight into code if coverage doesn’t add anything new.

The stale-worktree fix does not need to teach the shared parser about every porcelain annotation. The smaller fix is local: when reusing a managed path, verify it is still a real worktree for that branch before printing success.

I have enough for the two fixes, but the review contract still requires the second-pass result before synthesis. Waiting once more; if it adds nothing, the report will be two Required findings and no Improvements.

Second-pass coverage returned clean. Final verified findings are the two first-pass Required issues: .gitignore symlink escape and bogus reuse of a prunable/stale worktree path. I’m closing the review agents and then applying only those autofix-eligible fixes.

Required

Improvements (follow-up)

No improvement findings survived source verification.

Summary: 2 Required, 0 Improvements. Merge-readiness is blocked until these are fixed; both are narrow and autofix-eligible.

Creating the review ledger under ignored tmp/pfleidi-review/, then I’ll add the failing regressions first. The tests should fail against current code; that’s the satisfying little scream before the fix.

I’m adding the ledger plus two focused regression tests now: one for a symlinked .gitignore, one for a stale worktree registration whose directory has been recreated but is not a valid worktree.

Running the two new focused tests against the current implementation to confirm they fail for the reasons found. No fixing until the red path is real.

The red checks failed as expected:

Now I’m applying the narrow production fixes: symlink rejection for .gitignore, and a real-worktree branch check before reuse.

Production code is patched. I’m formatting the touched Go files before rerunning the focused tests so the verification state matches what would be reviewed.

Rerunning the same two focused tests that were red. If they pass, I’ll expand to the related checkout test package scope, then build and lint.

The focused regressions are green. I’m reviewing the exact diff now before broader verification, because fixes like this can accidentally grow tentacles.

One refinement before broader tests: the reuse check should confirm the path belongs to the same repository, not merely any Git repo with the same branch name. Otherwise a stale slot could be replaced by an unrelated repo wearing a fake mustache.

Formatting after the reuse refinement, then I’ll run the focused tests again.

Running the two red-to-green tests again after the same-repo check.

The targeted fixes are green. I’m running boundary verification now: focused package tests, a CLI build, and the documented lint task. Same tree, no edits while they run.

Verification passed. I’m updating the review ledger and checking the final working-tree shape so the handoff is exact.

Only the two intended source files are tracked as modified; the ledger is ignored under tmp/. I’m marking both findings fixed in the ledger with the verification evidence.

Ledger updated. Final check is just a status read; no more code changes.

SYSTEM NOTICE: review fixes landed. No commits made.

Ledger: review-cli-20260710-125136.md

Tracked working tree now has only the two intended modified files.

Using receiving-code-review because you’re challenging the review fix and giving narrower product feedback. The correction here is to separate “protect against hostile repo contents” from “don’t mutate .gitignore at all.”

I’m checking the current ignore-rule flow before changing it. Requirement I’m taking from this: don’t create .gitignore just to support worktree checkout; keep the managed worktree behavior working without surprising repo-file mutation.

I agree with the narrower model: the ignore rule was for convenience, to keep managed worktrees out of git status, not for correctness. If .gitignore doesn’t already exist, creating it is unnecessary mutation; I’m changing that to a no-op, and removing the symlink rejection because it only mattered once we chose to auto-write that file.

The tests now express the revised behavior and will fail until the helper returns “no append” on a missing .gitignore. Running the focused ones red before changing implementation.

The red check is at compile time because the test now expects appendIgnoreRule to report whether it changed the file. That’s the right shape for suppressing the “Added…” message on no-op. Applying that now.

Formatting, then rerunning the revised ignore-rule tests.

Running the revised focused tests now.

Ignore-rule behavior is green. Since the production helper changed again, I’m rerunning the package tests, build, and lint from the current tree.

Verification passed after the revised behavior. I’m updating the ignored ledger and checking the final diff.

Updating the ledger so it doesn’t memorialize the rejected symlink fix like some cursed audit fossil.

Final diff check now.

The .gitignore check was only for convenience: keep .entire/worktrees/ out of git status. It is not required for checkout correctness, so auto-creating .gitignore was too much.

I changed it:

• Missing .gitignore is now a silent no-op.
• Existing .gitignore still gets .entire/worktrees/ appended when needed.
• The symlink rejection is gone.
• The stale worktree reuse fix remains.

Touched files:

• trail_checkout_worktree.go
• trail_checkout_worktree_test.go

Verification, all exit 0:

• go test ./cmd/entire/cli -run TestAppendIgnoreRule -count=1
• go test ./cmd/entire/cli -run TestEnsureTrailWorktreeIgnoreRule -count=1
• go test ./cmd/entire/cli -count=1
• go build ./cmd/entire/
• mise run lint

No commits made.

Committing the two tracked files only. I’ll reuse the verification that passed after the last edit; no code changed since then, so rerunning would just feed the meter.

Status is scoped to the intended two files. Creating one focused commit now.

Commit created. I’m doing the final status read so there’s no mystery debris.

Committed:

c3c14108f4 tighten trail worktree checkout guards

Working tree is clean. Verification from before commit still applies: focused tests, go test ./cmd/entire/cli -count=1, go build ./cmd/entire/, and mise run lint all passed.