Repository navigation
km 1.10.1: hold the bump branch only while its head is the workflow's own - #47
Merged
Merged
Conversation
… own bump-mounted-pins leaves the open pull request as it is when a mount fails that the branch head moves (--hold-if-moved), so the proposed move is not lost over a passing failure. The hold applied to any head, including one --branch-prs rejects: a commit pushed by hand onto the branch then survived under the workflow's pull request on every day a mount failed. The hold now also needs the head that --branch-prs accepted (seen: one commit on main changing gitlinks only), the result the step already has. Any other head is regenerated and the failing mount stays at main's pin until it resolves again; a move of it the workflow had proposed comes back then. The script documents that --hold-if-moved takes such a head, and SHIPPED lists both templates' new hashes next to the old ones. The smoke test's stand-in script no longer moves a mount that fails, as the real one does not, so the hold cases run with a second mount that does move.
A team brain's bump-mounted-pins workflow holds its open pull request over a failing mount only while the branch head is the workflow's own commit; a commit pushed onto the branch by hand is regenerated away instead of staying under the workflow's pull request. km upgrade replaces an unchanged 1.10.0 copy of the workflow and its script. Without the bump no brain or plugin install would see the fix.
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
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.
What
A security fix for the team template
bump-mounted-pins, and the release that ships it.When a mount fails to resolve and the open pull request's head moves that mount,
--hold-if-movedleaves the bump branch untouched, so the workflow's proposed move is not lost over a failure that may pass. In 1.10.0 the hold applied to any head, including one that--branch-prsrejects (not exactly one commit ahead of main, or touching files other than gitlinks). So a commit pushed onto the bump branch by hand survived under the workflow's pull request whenever a mount failed at the same time.Fix
The workflow now passes
--hold-if-movedonly whenopenandseenare both true.seenis the result--branch-prsalready returns in the same step: a pull request had this head,ahead_by == 1against main, and the compare lists a non-empty set of files that are all submodule paths. No second compare call is made. Any other head is regenerated, and the failing mount stays at main's pin until it resolves again.moves()keeps its fail-safe (True when the gitlink comparison raises). Its input is now only a head that passed that validation, and the step fetches the branch with--depth 2, so the parent is always present. The fail-safe only fires on something unexpected, and it then preserves nothing but the workflow's own one-commit proposal.The script's docstring documents that
--hold-if-movedtakes a head--branch-prsaccepted.SHIPPEDlists the new hashes of both templates next to the 1.10.0 ones, so a brain with an unchanged 1.10.0 copy is still recognised and gets replaced bykm upgrade.The second commit bumps the version to 1.10.1 (
src/km/__init__.py,.claude-plugin/plugin.json, the README examples), as earlier releases did.Tests
changed=true problems=1). Without theseengate it readschanged=false problems=1.changed=false problems=1). With the hold removed entirely it readschanged=true problems=1.resolve(). Without that, the new case would have passed for the wrong reason. The hold cases now use a second mount that does move.tests/*_smoke.shpass. The resolve step needs bash 4, somounted_pins_smoke.shwas also run under bash 5 in apython:3.12-slimcontainer, with both mutations checked there.fileswithahead_by: 1, soseenis reachable for the workflow's own commits.Review
/simplify(reuse, simplification, efficiency, altitude): applied the loop form ofbranch_withand shortened the duplicated comments and the help text. Efficiency and altitude found nothing. Altitude agreed the workflow is the right place for the check: only that step has both the pull request lookup and the compare result, and a local check in the script (REF^ == main) would disable the hold whenever main moves after the bump commit was built./code-review:seenalways false.changed=falseand the hand-pushed commit stays under the pull request. That was already so in 1.10.0, independently of the hold. Regenerating then would mean closing the pull request or deleting the branch, which is a larger behaviour change. Setup step 5 in the workflow header ("Require approval of the most recent reviewable push") keeps such a commit from being merged without someone else's review.Also left as is: a single hand-pushed commit that moves only gitlinks and is one commit ahead of main passes
--branch-prs, as it already did for the "already proposes these pins" check. Setup step 5 is the mitigation there too.