Repository navigation
Report git output and conflicting hunks on backport failure - #72
Draft
aiolibsbot wants to merge 2 commits into
Draft
aiolibsbot wants to merge 2 commits into
aiolibsbot wants to merge 2 commits into
Conversation
The bot spawned every git invocation through `check_call()`, which lets the child inherit the parent's stdout and stderr. Nothing was captured, so `CalledProcessError.stdout` was always `None` and the push-failure report rendered an empty `[OUTPUT]` block — the one place the template promised details. A cherry-pick conflict was even thinner: a bare `Failed to cleanly apply ... on top of ...`, with the conflicting paths and hunks only discoverable by replaying the backport by hand. Capture the output instead — stderr merged into stdout, decoded leniently because git happily emits non-UTF-8 — and carry it into every exception the backport raises: the fetch and checkout lookups, the conflict, and the push. The conflict additionally reports `git diff --diff-filter=U` in a collapsed section, which is where the actual collision lives. Everything is clipped to stay inside GitHub's 65535-character cap on check-run output and comment bodies, and passed through the token mask first. The plumbing moves to `patchback.backport`, which imports nothing but the stdlib. That keeps it reachable without a web framework in the way, and drops `event_handlers` from 505 to 351 lines. Refs sanitizers#18, sanitizers#54
The repo had no tests and no CI job running them, for a bot whose core operation force-pushes branches into other people's repositories. Cover `patchback.backport` against real local repositories — a bare remote, a clean target branch and a diverging one — so the cherry-pick, the merge-commit mainline handling and each failure report are exercised for real rather than mocked. No network and no third-party imports beyond pytest are involved, which is what makes the split in the previous commit worth having. Test dependencies are installed unpinned in CI on purpose: the suite needs only pytest, and threading it through the hash-pinned pip-tools lock buys nothing until it grows real dependencies.
This was referenced Sep 16, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Capture the output of every git invocation the backport runs, and report it —
conflicting hunks included — in the check run and the progress comment.
Why
spawn_procwascheck_call(), so no output was ever captured. Two visibleconsequences:
CalledProcessError.stdout/.stderrinto
CMD_RUN_OUT_TMPL, which were alwaysNone— the[OUTPUT]blockusers were shown was always blank. That is the traceability gap Improve logging GitHub push errors #18 asks to
close.
Failed to cleanly apply <sha> on top of <branch>. Finding out what conflicted meant replaying the wholebackport locally, per Expose the cherry-pick application conflict in the status updates #54.
How
patchback/backport.py, importing stdlib only.event_handlers.pygoes 505 → 351 lines; the plumbing becomes reachablewithout octomachinery in the picture, which is what makes it testable.
run_proc()replacesspawn_proc:check_outputwith stderr merged in anderrors='replace', since git emits non-UTF-8 freely. Failures land inCalledProcessError.output.sanitized command log. The conflict also reports
git diff --diff-filter=Uin a<details>block; git already names eachconflicting path in its own output, so no extra call is needed for the file
list.
output.*fields and commentbodies both cap at 65535 characters, so each captured block is clipped at
20000 with an explicit marker. Both interfaces get the same detail — the
reporter feeds one payload to both.
sanitize_secret()is module-level and applied to everything rendered, sothe installation token cannot reach a report through the new paths.
Testing
tests/is new — the repo had none. 14 tests drivebackport_pr_syncagainstreal local repositories (bare remote, clean target branch, diverging one), so
the clean cherry-pick, the merge-commit
--mainline 1path, and each failurereport are exercised for real. No network, no third-party imports beyond
pytest.
.github/workflows/tests.ymlruns them on 3.9/3.13/3.14.Left out deliberately: #48's rulesets/branch-protection wording in the
privilege message (separate issue, separate PR), and #18's retry/
--forcediscussion — this only closes its reporting half.
Quality Report
Changes: 5 files changed, 498 insertions(+), 154 deletions(-)
Code scan: 1 issue(s) found
patchback/backport.py:22— TODO commentTests: failed (command not found)
Branch hygiene: clean
Generated by Kōan