diff --git a/generator/src/tend/templates/mention.yaml.j2 b/generator/src/tend/templates/mention.yaml.j2 index 0cf53499..eb2ba12d 100644 --- a/generator/src/tend/templates/mention.yaml.j2 +++ b/generator/src/tend/templates/mention.yaml.j2 @@ -289,6 +289,23 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" exit 0 fi + + # Somebody else's content-free approval is terminal too. The gate + # above is author-keyed, so a human's bare APPROVED falls through to + # the PR-author short-circuit (on a bot-authored PR) or to + # BOT_REVIEWS (on one the bot has reviewed) and starts a session + # whose only possible outcome is a silent exit: an approval with no + # body and no inline comments asks for nothing, and the bot cannot + # merge on its own. Unlike the bot-authored gates this one requires + # `$INLINE` to be empty rather than reply-only — an approval whose + # nits live inline is a request to the PR's author, which on a + # bot-authored PR is a role the bot has to act in. + if [ "$REVIEW_STATE" = "approved" ] \ + && [ -z "$COMMENT_BODY" ] \ + && [ -z "$INLINE" ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi # Non-mention: check bot engagement diff --git a/generator/tests/_regtest_outputs/test_generate.test_sandbox_levers_regtest.out b/generator/tests/_regtest_outputs/test_generate.test_sandbox_levers_regtest.out index 8014ac6c..60c9f1e4 100644 --- a/generator/tests/_regtest_outputs/test_generate.test_sandbox_levers_regtest.out +++ b/generator/tests/_regtest_outputs/test_generate.test_sandbox_levers_regtest.out @@ -277,6 +277,23 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" exit 0 fi + + # Somebody else's content-free approval is terminal too. The gate + # above is author-keyed, so a human's bare APPROVED falls through to + # the PR-author short-circuit (on a bot-authored PR) or to + # BOT_REVIEWS (on one the bot has reviewed) and starts a session + # whose only possible outcome is a silent exit: an approval with no + # body and no inline comments asks for nothing, and the bot cannot + # merge on its own. Unlike the bot-authored gates this one requires + # `$INLINE` to be empty rather than reply-only ? an approval whose + # nits live inline is a request to the PR's author, which on a + # bot-authored PR is a role the bot has to act in. + if [ "$REVIEW_STATE" = "approved" ] \ + && [ -z "$COMMENT_BODY" ] \ + && [ -z "$INLINE" ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi # Non-mention: check bot engagement diff --git a/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_codex_regtest[mention].out b/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_codex_regtest[mention].out index 6ad9e983..cd664a03 100644 --- a/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_codex_regtest[mention].out +++ b/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_codex_regtest[mention].out @@ -277,6 +277,23 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" exit 0 fi + + # Somebody else's content-free approval is terminal too. The gate + # above is author-keyed, so a human's bare APPROVED falls through to + # the PR-author short-circuit (on a bot-authored PR) or to + # BOT_REVIEWS (on one the bot has reviewed) and starts a session + # whose only possible outcome is a silent exit: an approval with no + # body and no inline comments asks for nothing, and the bot cannot + # merge on its own. Unlike the bot-authored gates this one requires + # `$INLINE` to be empty rather than reply-only ? an approval whose + # nits live inline is a request to the PR's author, which on a + # bot-authored PR is a role the bot has to act in. + if [ "$REVIEW_STATE" = "approved" ] \ + && [ -z "$COMMENT_BODY" ] \ + && [ -z "$INLINE" ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi # Non-mention: check bot engagement diff --git a/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_regtest[mention].out b/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_regtest[mention].out index 60ef5043..32bf9795 100644 --- a/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_regtest[mention].out +++ b/generator/tests/_regtest_outputs/test_generate.test_workflow_minimal_regtest[mention].out @@ -277,6 +277,23 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" exit 0 fi + + # Somebody else's content-free approval is terminal too. The gate + # above is author-keyed, so a human's bare APPROVED falls through to + # the PR-author short-circuit (on a bot-authored PR) or to + # BOT_REVIEWS (on one the bot has reviewed) and starts a session + # whose only possible outcome is a silent exit: an approval with no + # body and no inline comments asks for nothing, and the bot cannot + # merge on its own. Unlike the bot-authored gates this one requires + # `$INLINE` to be empty rather than reply-only ? an approval whose + # nits live inline is a request to the PR's author, which on a + # bot-authored PR is a role the bot has to act in. + if [ "$REVIEW_STATE" = "approved" ] \ + && [ -z "$COMMENT_BODY" ] \ + && [ -z "$INLINE" ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi # Non-mention: check bot engagement diff --git a/generator/tests/_regtest_outputs/test_generate.test_workflow_with_setup_regtest[mention].out b/generator/tests/_regtest_outputs/test_generate.test_workflow_with_setup_regtest[mention].out index 1f0aaf78..fe84045c 100644 --- a/generator/tests/_regtest_outputs/test_generate.test_workflow_with_setup_regtest[mention].out +++ b/generator/tests/_regtest_outputs/test_generate.test_workflow_with_setup_regtest[mention].out @@ -277,6 +277,23 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" exit 0 fi + + # Somebody else's content-free approval is terminal too. The gate + # above is author-keyed, so a human's bare APPROVED falls through to + # the PR-author short-circuit (on a bot-authored PR) or to + # BOT_REVIEWS (on one the bot has reviewed) and starts a session + # whose only possible outcome is a silent exit: an approval with no + # body and no inline comments asks for nothing, and the bot cannot + # merge on its own. Unlike the bot-authored gates this one requires + # `$INLINE` to be empty rather than reply-only ? an approval whose + # nits live inline is a request to the PR's author, which on a + # bot-authored PR is a role the bot has to act in. + if [ "$REVIEW_STATE" = "approved" ] \ + && [ -z "$COMMENT_BODY" ] \ + && [ -z "$INLINE" ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi # Non-mention: check bot engagement diff --git a/generator/tests/test_generate.py b/generator/tests/test_generate.py index da76f693..55ee7142 100644 --- a/generator/tests/test_generate.py +++ b/generator/tests/test_generate.py @@ -1160,6 +1160,61 @@ def test_mention_skips_bot_reply_container(tmp_path: Path) -> None: assert run.index(count) < run.index("BOT_REVIEWS=$(") +def test_mention_skips_third_party_bare_approval(tmp_path: Path) -> None: + """A human's APPROVED review with no body and no inline comments is + terminal for the bot as well, and the two gates above are author-keyed, so + it falls through to the PR-author short-circuit (on a bot-authored PR) or + to BOT_REVIEWS (on one the bot has reviewed) and starts a session that can + only exit silently — the approval asks for nothing and the bot cannot + merge. + + The narrowing is load-bearing in both directions: + + - `approved` — a bare CHANGES_REQUESTED or COMMENTED review is not + terminal, and a human's synthetic reply container arrives as COMMENTED, + so widening the state would silence the bot on replies to its own review + threads (the same trap `test_mention_skips_bot_reply_container` pins from + the author side). + - `$INLINE` empty, not reply-only — an approval whose nits live inline is a + request addressed to the PR's author, a role the bot has to act in on its + own PRs. + """ + cfg = Config.load(_minimal_config(tmp_path)) + wf = generate_mention(cfg) + data = yaml.safe_load(wf.content) + check_step = next( + s for s in data["jobs"]["verify"]["steps"] if s.get("id") == "check" + ) + run = check_step["run"] + + # The two author-keyed gates both open on `$KIND` / `$REVIEW_AUTHOR`, so + # this opening belongs to the third-party gate alone. Compare the whole + # condition with whitespace normalized: an author clause added here, or the + # `$INLINE` clause dropped, fails the equality rather than sliding past an + # `in` check. + gate = run[run.index('if [ "$REVIEW_STATE" = "approved" ]') :] + condition = " ".join(gate.split("; then")[0].split()) + assert condition == ( + 'if [ "$REVIEW_STATE" = "approved" ] \\ ' + '&& [ -z "$COMMENT_BODY" ] \\ ' + '&& [ -z "$INLINE" ]' + ), "the third-party skip must key on state, empty body, and zero inline comments" + + # `[ -z "$INLINE" ]` only means "zero inline comments" below the fetch that + # populates it, and that property is positional rather than textual, so the + # equality above does not pin it. Hoisting the gate above the fetch — the + # shape the bot-authored approval gate deliberately takes, one API call + # cheaper — leaves `$INLINE` unset, which reads as empty and silently skips + # exactly the approvals-with-inline-nits this gate must let through. + assert run.index("INLINE=$(") < run.index('[ -z "$INLINE" ]'), ( + "the third-party skip must sit below the inline-comment fetch" + ) + + # And it precedes the engagement heuristic that would otherwise count the + # triggering review as prior participation. + assert run.index('[ -z "$INLINE" ]') < run.index("BOT_REVIEWS=$(") + + # --------------------------------------------------------------------------- # Fork guard # ---------------------------------------------------------------------------