fix: retrospective review fixes for merged PR #106#117
Merged
Conversation
PR #106 wired linkWorktreeClaudeMd into `bough create` only, mirroring `linkWorktreeArtifacts` at the time. That mirrored pattern was itself the bug issue #61 reported and #112 fixed for .claude/{skills,agents,commands}: a per-worktree symlink helper wired only into `create` never runs for a worktree that predates the feature, was hand-created, or was registered by an earlier `bough backfill` — so it silently keeps missing the linked content with no non-destructive repair path. `runBackfill` picked up the artifacts fix but never got the equivalent call for CLAUDE.md, so any worktree bootstrapped before this symlink feature shipped (or created outside `bough create`) still starts every Claude session with no root guidance, and `bough backfill` — the documented non-destructive repair command — does nothing to fix it. Call linkWorktreeClaudeMd for every discovered worktree dir in runBackfill, same as linkWorktreeArtifacts. ensureSymlink is idempotent so relinking an already-correct worktree is a no-op.
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
1 participant
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.
Retrospective
/code-reviewsweep of merged PR #106 (symlink rootCLAUDE.mdinto each worktree on create), which shipped without pre-merge review.Symlink mechanics (
ensureSymlink,linkWorktreeClaudeMd) checked out fine — target is absolute (filepath.Abs'd before use), a stale/incorrect symlink is repointed idempotently, a real (non-symlink)CLAUDE.mdalready in the worktree is refused/left untouched, and a missing monorepo-rootCLAUDE.mdis a silent no-op rather than a dangling link. No bug there.What was still live:
bough backfillnever relinks CLAUDE.md. #106 wiredlinkWorktreeClaudeMdintobough createonly — the exact same shape as issue #61, which PR #112 fixed forlinkWorktreeArtifacts(.claude/{skills,agents,commands}) by relinking every discovered worktree dir inrunBackfill. That fix reused the pattern for artifacts but didn't also add the equivalent call for CLAUDE.md, since #61 predates #106 and was scoped to skills only.Net effect: any worktree bootstrapped before the CLAUDE.md-symlink feature shipped, or created outside
bough create(hand-created dir, or registered by an earlierbough backfillrun), never gets the rootCLAUDE.mdsymlink — andbough backfill, the documented non-destructive repair command for exactly this class of gap, does nothing about it. Those worktree sessions keep starting with zero project guidance indefinitely; the only "fix" is the destructivebough remove+bough createcycle #61 called out as unacceptable for the artifacts case.Fix: call
linkWorktreeClaudeMd(stderr, monorepoRoot, wtRoot)for every discovered worktree dir inrunBackfill, alongside the existinglinkWorktreeArtifactscall —ensureSymlinkis already idempotent so relinking an already-correct worktree is a no-op.New tests in
internal/cli/backfill_test.go:TestRunBackfill_RelinksClaudeMd— backfill creates the worktreeCLAUDE.mdsymlink for a pre-existing dir and is idempotent on a second run (verified this fails without the fix, passes with it).TestRunBackfill_ClaudeMdRealFileGuardAndMissingRoot— a hand-authored realCLAUDE.mdin the worktree survives backfill untouched, and a monorepo root with noCLAUDE.mddoes not error.Test plan