Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions engine/skills/make-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<number>-<short-sha>`, say where it went,
then `git push --force-with-lease=<branch>:<old-sha>` 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
Expand Down
42 changes: 42 additions & 0 deletions engine/skills/make-pr/tests/test_stack_or_overwrite.py
Original file line number Diff line number Diff line change
@@ -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<number>-<short-sha>", 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()
Loading