Skip to content

test(ci): pin metacharacter PR titles in the bot auto-merge re-arm (issue #3348) #3389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luoye-cpu wants to merge 2 commits into
base: master
Choose a base branch
from
Open

test(ci): pin metacharacter PR titles in the bot auto-merge re-arm (issue #3348) #3389

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc6f0ab
test(ci): pin metacharacter PR titles in the bot auto-merge re-arm
luoye-cpu Nov 10, 2026
c96bc5f
Merge remote-tracking branch 'origin/master' into test/issue-3348-aut…
luoye-cpu Nov 10, 2026
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
34 changes: 34 additions & 0 deletions scripts/ci/tests/test_fix_bot_automerge_body.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,40 @@ def test_success_returns_body(self) -> None:
assert rc == 0
assert body == signoff

def test_metacharacter_headline_reaches_the_mutation_unchanged(self) -> None:
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
Resolved
"""Issue #3348: a title with quoting or shell metacharacters reaches the
mutation verbatim. The headline travels as one gh argv element, never
through a shell, and a trailing newline survives.
"""
mod = _load_module()
title = 'chore(deps): update "lock" `pin` $(echo pwned) \\ end\n'
signoff = (
"Signed-off-by: renovate[bot] " "<29139614+renovate[bot]@users.noreply.github.com>"

@lusoris lusoris Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the two adjacent string literals here join implicitly, which reads like a missing comma in a list. Ruff's ISC rules flag this shape, although the repository does not enable them yet. A single literal fits the 100-column limit at this indentation, and Black 26 leaves it as is:

signoff = "Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>"

)
mock_result = MagicMock()
mock_result.returncode = 0
mock_result.stdout = self._make_response(signoff)
mock_result.stderr = ""
bot_env = {
"PR_NUMBER": "3047",
"PR_NODE_ID": "PR_fakenode123",
"PR_TITLE": title,
"PR_AUTHOR": "renovate[bot]",
"PR_AUTHOR_TYPE": "Bot",
"PR_AUTHOR_ID": "29139614",
"AUTO_MERGE_BODY": "",
}
with (
patch.object(subprocess, "run", return_value=mock_result) as run,
patch.dict(os.environ, bot_env),
):
rc = mod.main()
assert rc == 0
argv = run.call_args.args[0]
assert isinstance(argv, list), "gh must run from an argv list, not a shell string"
assert not run.call_args.kwargs.get("shell", False)
assert f"headline={title} (#3047)" in argv

@sourcery-ai sourcery-ai Bot Nov 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Unbound headline passes the test

When the -f immediately before the headline argument is removed, test_metacharacter_headline_reaches_the_mutation_unchanged still passes at line 162 because it checks only that the headline string appears somewhere in argv; the mocked success bypasses gh option parsing, so the string can be an unbound argument and never reach the mutation.

Assert that -f and the exact headline field occur together in the expected argument sequence.

Prompt for AI agents 
In `scripts/ci/tests/test_fix_bot_automerge_body.py` at line 162:

**Unbound headline passes the test**

When the `-f` immediately before the headline argument is removed, `test_metacharacter_headline_reaches_the_mutation_unchanged` still passes at line 162 because it checks only that the headline string appears somewhere in `argv`; the mocked success bypasses `gh` option parsing, so the string can be an unbound argument and never reach the mutation.

Assert that `-f` and the exact headline field occur together in the expected argument sequence.

@lusoris lusoris Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. With the -f before headline= removed from scripts/ci/fix-bot-automerge-body.py, this test still passes (1 passed), because the membership check does not bind the field to its flag. Pinning the pair closes it:

field = f"headline={title} (#3047)"
assert field in argv
assert argv[argv.index(field) - 1] == "-f"

@luoye-cpu, could you add this? I approved before running that mutant, so I am moving my review to changes requested until the assertion is in.


def test_gh_nonzero_exit(self) -> None:
mod = _load_module()
mock_result = MagicMock()
Expand Down
Loading