From a830d6f00a8711b144de3c5979dd708d49584cc5 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Tue, 29 Sep 2026 08:30:32 +0800 Subject: [PATCH] Teach land-stack when babysit jobs may resolve automated review threads. Under babysit-until-merged, resolve automated threads after head addresses them; human threads still need a recorded deferral. Co-authored-by: Cursor Change-Id: I3fb2c9ca1fbf590468da5ae0dfce6d417be1432e --- product/skills/land-stack/SKILL.md | 12 +++++- .../tests/test_review_thread_decision_tree.py | 43 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 product/skills/land-stack/tests/test_review_thread_decision_tree.py 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()