DEV Community

Blake Yang
Blake Yang

Posted on

Clean Merge, Wrong Patch: Re-Validating an OSS Contribution After Upstream Moves

A contributor opens a parser fix, waits eleven days for review, and watches upstream refactor the very function the patch touches. The rebase produces no conflict, the test suite stays green, and the second review round takes four minutes. The merged code is still wrong, because the assertion was pinned to a string literal that the refactor had moved into a different module.

That scenario is a composite of a well-known failure mode rather than a report from a specific repository, and every path below is a placeholder. The interesting part is not the bug; it is the review evidence that silently expired between the first approval and the merge. A patch can be textually current and semantically stale at the same time, and Git will not tell the difference.

Why a clean rebase proves nothing

git rebase answers a question about text, not about intent. When two changes touch disjoint lines, the tool applies both and reports success, even if the second change assumed a contract the first one removed. Reviewers then re-read the diff, recognise the shape they approved last week, and skim.

The practical consequence is that review effort must be re-allocated after upstream moves. Instead of re-reading the patch, re-capture the evidence the patch was supposed to produce: the failing reproduction, the passing assertion, and the reason the assertion still targets the original defect. Only the third item is expensive, and it is the only one a rebase can quietly invalidate.

Store the base commit before the first review

Evidence expires because nobody recorded what it was measured against. A short brief file, committed on the contributor branch, fixes that with three fields and no tooling investment.

# .pr-brief.yml — committed on the contributor branch
issue: "#4121 parser drops trailing empty field"
base_commit: 9f3c1ab          # origin/main at the time evidence was captured
repro: "python -m pytest tests/test_parser.py::test_trailing_empty -q"
assertion_target: "parse() return value, not the log line"
Enter fullscreen mode Exit fullscreen mode

The base_commit field is the whole trick. It converts "the tests pass" into "the tests pass against this tree", which is a claim that can be re-checked later. Keep the file small; a brief that needs maintenance gets deleted in the first review round.

Re-apply the patch and re-capture evidence

The script below rebases onto the current upstream head using the recorded base, then re-runs exactly the command from the brief. It refuses to continue when the rebase conflicts, because a conflict in the touched file means the reproduction must be re-derived first.

#!/usr/bin/env bash
# rebase-evidence.sh — re-apply a reviewed patch onto a moved upstream
set -euo pipefail

BRANCH="${BRANCH:?set BRANCH}"        # e.g. review/parser-trailing-field
TEST_CMD="${TEST_CMD:?set TEST_CMD}"

git fetch origin main --quiet
NEW_BASE="$(git rev-parse origin/main)"
OLD_BASE="$(sed -n 's/^base_commit: *//p' .pr-brief.yml)"

echo "old base: $OLD_BASE"
echo "new base: $NEW_BASE"

git switch -c drift/retest "$BRANCH"
if ! git rebase --onto "$NEW_BASE" "$OLD_BASE" drift/retest; then
  echo "CONFLICT: re-derive the reproduction before re-running tests" >&2
  exit 2
fi

$TEST_CMD 2>&1 | tee "drift-after-$(git rev-parse --short "$NEW_BASE").log"
Enter fullscreen mode Exit fullscreen mode

A green log from this script means less than it appears to. The next step decides whether the evidence is still about the same defect.

Classify the drift instead of re-reading the diff

Four outcomes cover almost every re-application, and each one maps to a different amount of human attention. The table is deliberately ordered by risk, not by frequency.

Signal after re-application Interpretation Action
Conflict inside a patch hunk Upstream rewrote the same lines Re-derive the reproduction; do not resolve mechanically
Clean rebase, test now fails Test rot or a genuine upstream regression Bisect before touching the patch
Clean rebase, upstream hunks overlap patch hunks, test passes Highest risk: intent may have moved Read the upstream hunk, not the patch
Clean rebase, no hunk overlap, coverage on touched lines unchanged Low risk Re-approve with the new log attached

The third row is the one reviewers miss, and it is mechanical to detect. Extract the hunk ranges the patch changed and the hunk ranges upstream changed since the recorded base, then compare them numerically.

# hunks the patch owns
 git diff -U0 "$OLD_BASE"..."$BRANCH" -- src/parser.py \
  | grep -E '^@@' | sed -E 's/^@@ -([0-9]+)(,([0-9]+))? .*/\1 \3/' > patch.hunks

# hunks upstream changed since the last review
 git diff -U0 "$OLD_BASE".."$NEW_BASE" -- src/parser.py \
  | grep -E '^@@' | sed -E 's/^@@ -([0-9]+)(,([0-9]+))? .*/\1 \3/' > upstream.hunks

# report any upstream hunk whose range intersects a patch hunk
awk 'NR==FNR{s[NR]=$1; c[NR]=($2==""?1:$2); n=NR; next}
     {for(i=1;i<=n;i++){e=s[i]+c[i]-1;
       if($1<=e && $1+($2==""?1:$2)-1>=s[i]){print "overlap:",$1,$2; break}}}' \
    patch.hunks upstream.hunks
Enter fullscreen mode Exit fullscreen mode

The output is not a verdict; it is a pointer to the two or three files where a human must read the upstream change before re-approving. Everything else can be re-approved with the fresh log attached to the brief.

Where narrow free-model tasks fit in the loop

Re-validation is repetitive and low-judgement for most files, which makes it a reasonable place to use free model access rather than paid capacity. The operator of MonkeyCode describes both free model access and a free server option in the product; only the first is relevant to this section.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

Three tasks survive contact with real review, and each one keeps the context window small on purpose:

  1. Summarise a single upstream hunk. Paste the hunk and the issue text only, and ask what behaviour changed. A model that cannot see the rest of the repository cannot invent call sites, which is the failure mode that makes whole-repo prompts unreliable.
  2. Check whether the assertion still names the defect. Paste the test function and the issue description, then ask which observable behaviour is asserted. The answer is cheap to verify by reading two paragraphs.
  3. Propose three edge cases the test omits. Treat the output as a list of candidates for a human to convert into tests, never as tests to append directly.

Two rules keep this honest. Every model claim must quote text that was pasted into the prompt, and every proposal must be labelled as a proposal until a human has run it. A model is useful here as a fast reader of a fifty-line hunk; it is not a substitute for reading the upstream change yourself.

Run the matrix in a disposable environment

Re-validation is cheap to describe and annoying to run, because it needs a clean checkout, the recorded base commit, and a test run per scenario. Running it on a laptop leaves evidence scattered across shell history and stale branches.

The disposable alternative is to script the whole re-application and run it in a fresh environment per patch, so the log files become the artefact rather than a memory of what happened. A free server option is enough for this class of job, because the workload is a clone, a rebase, and one targeted test command per patch. Do not set expectations about runtime or concurrency for a shared free tier; size the matrix to a handful of patches and check the logs in afterwards.

# one environment per patch, same script, log archived with the base commit
for pr in review/*; do
  [ -d "$pr" ] || continue
  BRANCH="$pr" TEST_CMD="python -m pytest tests/test_parser.py -q" \
    ./rebase-evidence.sh || echo "manual: $pr"
done
Enter fullscreen mode Exit fullscreen mode

Limitations and who should skip this

The loop assumes a patch with a stable, recorded base commit and a test command that finishes quickly. It does not catch intent drift in untouched callers, so a refactor that changes a shared helper will still slip through unless the touched-file check is widened deliberately.

Anyone merging within hours of a review can ignore most of this, because the window for drift is too small to justify the bookkeeping. The same applies to patches over generated code, where hunk ranges carry little meaning. Finally, a free tier is a convenience, not a contract: if re-validation becomes a release gate, give it an owner and a paid fallback rather than depending on a shared free resource.

If you already keep a brief per patch, the only new habit is re-running rebase-evidence.sh after any upstream fetch and attaching the new log before re-approving. That single step converts a skimmed re-approval into a checkable claim about a specific tree.

Top comments (0)