Skip to content

fix(orphan): allow orphaning branches whose parent link is invalid#123

Open
mvanhorn wants to merge 1 commit into
boneskull:mainfrom
mvanhorn:osc/116-orphan-handles-invalid-parents
Open

fix(orphan): allow orphaning branches whose parent link is invalid#123
mvanhorn wants to merge 1 commit into
boneskull:mainfrom
mvanhorn:osc/116-orphan-handles-invalid-parents

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Summary

Fix #116: gh stack orphan now succeeds when the current branch's recorded parent is missing or itself untracked, instead of reporting the branch as "not tracked".

Why this matters

The reporter asked the contributor to evaluate why the orphan check was misfiring, and what the consequences of removing it would be. Tracing through the path:

tree.Build (internal/tree/tree.go:46-54) silently continues when a tracked branch's recorded parent is not itself in the tree map:

parentNode, ok := nodes[parent]
if !ok {
    // Broken parent link - parent doesn't exist
    continue
}

The branch's own node is in nodes[branch], but it is never wired into any parent's Children slice and its own Parent pointer stays nil. tree.FindNode(root, branchName) (internal/tree/tree.go:76-87) walks root.Children recursively, so the disconnected node is unreachable - the function returns nil. runOrphan (cmd/orphan.go:60-63) then translates nil to branch %q is not tracked, which contradicts the git config that still records branch.<name>.stackParent.

The branch IS tracked. The link is just dangling. Treating that as "not tracked" leaves the user with an orphaned config entry the CLI cannot remove, which is the exact symptom #116 reports.

The fix preserves the existing safety check (a branch that has no stackParent config key at all still hits the "not tracked" error) but falls back to cfg.ListTrackedBranches() when FindNode returns nil and the user is allowed to clean up the dangling entry. The children check is safe to skip on this branch: the Build step that disconnected this branch from root also kept it out of any descendant's Parent slot, so a disconnected node never has the kind of children that would silently take an orphan along for the ride.

Testing

  • go test ./... is green (the new TestOrphanDisconnectedBranchIsAllowed e2e case lands the regression net).
  • golangci-lint run ./cmd/... reports 0 issues.

The new e2e test mirrors the exact scenario from #116: create feat-a off main, then manually rewrite branch.feat-a.stackParent to point at a non-existent branch, then run gh stack orphan feat-a and assert the config entry is cleared. Without the patch the same test fails with branch "feat-a" is not tracked.

Closes #116

Per boneskull#116, if a tracked branch's recorded parent is missing or is itself
untracked, tree.Build silently drops the parent-child wiring (the
`continue` at internal/tree/tree.go:50). The branch's node stays in
nodes[] but isn't connected to root, so tree.FindNode walking root's
Children never reaches it and runOrphan reports the disconnected
branch as "not tracked" - exactly the symptom in the issue.

A disconnected branch IS tracked - the stackParent config entry is
still there - so refusing to orphan it leaves the user stuck with a
config entry they can't clean up through the CLI. When FindNode
returns nil, fall back to ListTrackedBranches and treat the branch as
orphan-able when its name appears there. The cleanup path is the same
RemoveParent/RemovePR/RemoveForkPoint/RemovePRBase sequence the
normal flow runs.

The children check is safe to skip on this branch: tree.Build wires
children up from parent links pointing AT this branch, and Build's
parent-link miss already kept this branch out of any parent's
Children list. So a disconnected branch with descendants would be
caught the same way by Build - none of them appear as children.

A new e2e test in e2e/orphan_test.go simulates the scenario by
setting a stackParent that points at a missing branch, then runs
orphan and asserts both that the command succeeds and that the
stackParent config entry is cleared.

Closes boneskull#116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orphan: handle invalid parent branches

1 participant