fix: adopt --remote fails when branches aren't ordered parent-first#679
fix: adopt --remote fails when branches aren't ordered parent-first#679
Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
🔃 FlexReview StatusCommon Owner:
Review SLO: |
This comment was marked as outdated.
This comment was marked as outdated.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
simsinght
force-pushed
the
fix/adopt-remote-batch-ordering
branch
from
d9fadd6 to
675168c
Compare
The remote PR walker builds the branch list leaf-first (starting from the given branch and walking up to trunk). initAdoption iterated chosenTargets from the tree selector, which has no guaranteed order, and adopted each branch sequentially. When a child was processed before its parent, ValidateNoCycle failed with "ancestor branch is missing from av metadata". Fix by iterating prs in reverse (parent-first) filtered to chosen targets, so parents are always adopted before their children.
simsinght
force-pushed
the
fix/adopt-remote-batch-ordering
branch
from
675168c to
2caa729
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses a bug in av adopt --remote that caused adoption to fail when branches were not processed in parent-first order. The fix correctly iterates through pull requests in reverse to establish the right adoption sequence and also improves performance by using a map for lookups. While reviewing, I identified a separate potential bug concerning parent branch resolution when a parent PR is closed, which I've detailed in a specific comment.
| ab := actions.AdoptingBranch{ | ||
| Name: target.Short(), | ||
| Name: pr.Name, | ||
| Parent: pr.Parent, | ||
| PullRequest: &pr.PullRequest, | ||
| } |
There was a problem hiding this comment.
There appears to be a bug in how the parent branch is determined. The ab variable is initialized with pr.Parent on line 391. However, the subsequent loop (lines 395-401) modifies pr.Parent to find the correct ancestor if the direct parent's PR is merged or closed. The ab variable that is appended to the branches slice on line 402 does not reflect this change, as it was created before its parent was updated in the loop. This could lead to incorrect parent metadata for the adopted branches.
To resolve this, the initialization of ab should be moved to after the loop that finalizes the parent branch (i.e., after line 401).
1 participant
Summary
av adopt --remotefails with "ancestor branch is missing from av metadata" when adopting stacksinitAdoptionbuilt the branches slice by iteratingchosenTargetsfrom the tree selector, which has no guaranteed order. The remote PR walker returns PRs leaf-first, butchosenTargetsdoesn't preserve that order. When a child branch was adopted before its parent,ValidateNoCyclefailed because the parent wasn't in metadata yet.prsin reverse (parent-first) filtered to chosen targets, instead of iteratingchosenTargetsdirectly. Only the--remotecodepath is affected; local adopt is unchanged.Test plan
TestAdopte2e tests passpre-commit run --all-filespassesav adopt --remote inbox/suspense) that previously failed