diff --git a/product/skills/land-stack/SKILL.md b/product/skills/land-stack/SKILL.md index d757d88c..938dcb44 100644 --- a/product/skills/land-stack/SKILL.md +++ b/product/skills/land-stack/SKILL.md @@ -109,8 +109,16 @@ guard before any write (label, thread-resolve, queue, merge). - Do not bypass the guard by hand-adding a bypass label or merging directly to skip a broken check — if the queue is unhealthy, that's a different, riskier operation that needs its own explicit authorization, not this skill. -- Do not resolve review threads to unblock a merge unless the user has decided - to defer those findings; record the deferral on the PR. +- Do not resolve review threads to unblock a merge by default. Decision tree: + - **Automated review thread under an explicit babysit-until-merged job** + (for example CodeRabbit): after the current head addresses the thread + (or the thread is outdated), resolve it yourself via the GitHub + review-thread API. Do not bounce that to the user. + - **Deferral required for human reviewer threads:** resolve only when the + user has decided to defer those findings; record the deferral on the PR. + This rule alone never authorizes resolving a human thread. + - **Otherwise leave the thread open:** when there is no babysit-until-merged + job, or the head does not address the thread, leave it open. - Do not act on a PR whose head SHA is not in your local clone. ## Prove state before reporting it diff --git a/product/skills/land-stack/tests/test_review_thread_decision_tree.py b/product/skills/land-stack/tests/test_review_thread_decision_tree.py new file mode 100644 index 00000000..6c44267b --- /dev/null +++ b/product/skills/land-stack/tests/test_review_thread_decision_tree.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import unittest +from pathlib import Path + +SKILL = (Path(__file__).resolve().parents[1] / "SKILL.md").read_text(encoding="utf-8") + + +class TestReviewThreadDecisionTree(unittest.TestCase): + def test_do_not_defaults_to_decision_tree(self): + self.assertIn( + "Do not resolve review threads to unblock a merge by default. Decision tree:", + SKILL, + ) + + def test_babysit_bot_thread_resolves_after_head_addresses(self): + start = SKILL.index( + "**Automated review thread under an explicit babysit-until-merged job**" + ) + end = SKILL.index("**Deferral required for human reviewer threads:**") + bot = SKILL[start:end] + self.assertIn("current head addresses the thread", bot) + self.assertIn("or the thread is outdated", bot) + self.assertIn("Do not bounce that to the user", bot) + + def test_human_thread_needs_recorded_deferral(self): + start = SKILL.index("**Deferral required for human reviewer threads:**") + end = SKILL.index("**Otherwise leave the thread open:**") + human = SKILL[start:end] + self.assertIn("record the deferral on the PR", human) + self.assertIn("This rule alone never", human) + self.assertIn("authorizes resolving a human thread", human) + + def test_no_babysit_leaves_thread_open(self): + start = SKILL.index("**Otherwise leave the thread open:**") + otherwise = SKILL[start : start + 200] + self.assertIn("no babysit-until-merged", otherwise) + self.assertIn("leave it open", otherwise) + + +if __name__ == "__main__": + unittest.main()