From a56c15e6120d9ab7ef0d41163057489ea453696e Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 18:05:16 -0700 Subject: [PATCH] make-pr: stack on top, or overwrite the branch, decided from the diff A PR that already exists gets one of two treatments, read off its current diff: stack another commit when its claim still holds and the addition is small, or overwrite the branch when the diff stopped matching the claim -- it carries work already on the base, it mixes review units, or it ships a second claim. The record that gets read later is the final diff and body, so such a branch is rewritten rather than explained in Slice Rationale. Overwriting requires a backup branch and --force-with-lease; a lease failure means another session moved the branch, so re-read it instead of forcing. The choice is mechanical and is not a question for the user. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Gnua5cuAAF1ey1b7zRqJCC Change-Id: I3839f20ff1f27f9375ec7cf84816e23747986b0a --- engine/skills/make-pr/SKILL.md | 26 ++++++++++++ .../make-pr/tests/test_stack_or_overwrite.py | 42 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 engine/skills/make-pr/tests/test_stack_or_overwrite.py diff --git a/engine/skills/make-pr/SKILL.md b/engine/skills/make-pr/SKILL.md index 1cf0f62..42246f6 100644 --- a/engine/skills/make-pr/SKILL.md +++ b/engine/skills/make-pr/SKILL.md @@ -27,6 +27,32 @@ along. See [docs/ecosystem.md](../../../docs/ecosystem.md). A change whose code reads another unit's output is two stacked PRs, producer first -- not one PR with the coupling explained in Slice Rationale. +## Stack on top, or overwrite the branch + +A PR that already exists gets one of two treatments, decided from its current +diff against its base -- never from how much work went into it, and never by +asking the user which they would prefer: + +- **Stack on top** when the claim still holds and what is being added is + small: a fix to code the PR already ships, the base merged in, or docs for + the same claim. Push another commit to the same branch, or open a new PR + based on it. +- **Overwrite the branch** when the diff no longer matches the claim. Any one + of these is enough: + - it carries work already on the base + - it mixes review units + - it ships a second claim + + Save the old head as `backup/pr-`, say where it went, + then `git push --force-with-lease=:` and rewrite the title + and body to the slice that remains. + +The record that gets read later is the PR's final diff and body, so a branch +whose diff stopped matching its title is rewritten, not explained in Slice +Rationale. `--force-with-lease` and the backup branch are the floor, not a +precaution to skip: a lease failure means another session moved that branch, +so re-read it and decide again instead of forcing past it. + ## Preflight (run first) ```sh diff --git a/engine/skills/make-pr/tests/test_stack_or_overwrite.py b/engine/skills/make-pr/tests/test_stack_or_overwrite.py new file mode 100644 index 0000000..8cba408 --- /dev/null +++ b/engine/skills/make-pr/tests/test_stack_or_overwrite.py @@ -0,0 +1,42 @@ +import os +import unittest + +SKILL = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "SKILL.md") +HEADING = "## Stack on top, or overwrite the branch" + + +class TestStackOrOverwrite(unittest.TestCase): + @classmethod + def setUpClass(cls): + with open(SKILL, encoding="utf-8") as handle: + cls.text = handle.read() + cls.section = cls.text.split(HEADING, 1)[-1].split("\n## ", 1)[0] + + def test_the_skill_has_the_section(self): + self.assertIn(HEADING, self.text) + + def test_the_section_names_both_treatments(self): + self.assertIn("**Stack on top**", self.section) + self.assertIn("**Overwrite the branch**", self.section) + + def test_stacking_is_for_a_claim_that_still_holds(self): + self.assertIn("the claim still holds", self.section) + + def test_overwriting_is_for_a_diff_that_stopped_matching_the_claim(self): + for trigger in ("work already on the base", "mixes review units", "ships a second claim"): + with self.subTest(trigger=trigger): + self.assertIn(trigger, self.section) + + def test_overwriting_requires_a_lease_and_a_backup_branch(self): + self.assertIn("--force-with-lease", self.section) + self.assertIn("backup/pr-", self.section) + + def test_a_lease_failure_stops_the_overwrite(self): + self.assertIn("re-read it and decide again", self.section) + + def test_the_choice_is_not_a_question_for_the_user(self): + self.assertIn("never by\nasking the user", self.section) + + +if __name__ == "__main__": + unittest.main()