From 19e501514de05b7edeaa073e5e11fd3211d4661f Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Wed, 25 Mar 2026 11:36:16 -0400 Subject: [PATCH] fix: resolve BranchOnto target from git ref, not stored state BranchOnto resolved the target hash from the stored state (LookupBranch.Head) for non-trunk branches, but used PeelToCommit (current git ref) for trunk. This asymmetry could cause stale target hashes when a prior operation in the same transaction modified the target branch's git ref without updating the stored state. Use PeelToCommit consistently for all target branches. --- internal/spice/onto.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/internal/spice/onto.go b/internal/spice/onto.go index 2ff998dab..41f0987ca 100644 --- a/internal/spice/onto.go +++ b/internal/spice/onto.go @@ -43,19 +43,22 @@ func (s *Service) BranchOnto(ctx context.Context, req *BranchOntoRequest) error return fmt.Errorf("lookup branch: %w", err) } - var ontoHash git.Hash - if req.Onto == s.store.Trunk() { - ontoHash, err = s.repo.PeelToCommit(ctx, req.Onto) - if err != nil { - return fmt.Errorf("resolve trunk: %w", err) - } - } else { - // Non-trunk branches must be tracked. - onto, err := s.LookupBranch(ctx, req.Onto) - if err != nil { + // Verify non-trunk target is tracked. + if req.Onto != s.store.Trunk() { + if _, err := s.LookupBranch(ctx, req.Onto); err != nil { return fmt.Errorf("lookup onto: %w", err) } - ontoHash = onto.Head + } + + // Always resolve the target from the git ref + // rather than the stored state. + // The stored head may be stale + // if a prior operation in the same transaction + // (e.g. branch onto moving upstack branches) + // modified the target branch's git ref. + ontoHash, err := s.repo.PeelToCommit(ctx, req.Onto) + if err != nil { + return fmt.Errorf("resolve %v: %w", req.Onto, err) } // The recorded base hash may be stale if the old base branch