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
17 changes: 17 additions & 0 deletions generator/src/tend/templates/mention.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 55 additions & 0 deletions generator/tests/test_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=$(")
Comment thread
tend-agent marked this conversation as resolved.


# ---------------------------------------------------------------------------
# Fork guard
# ---------------------------------------------------------------------------
Expand Down
Loading