fix(upstream-triage): distinguish subject hints from patch matches (#569)

* fix(upstream-triage): require patch identity before skipping commits

* fix(upstream-triage): retain subject-only ports as verification hints

* changelog: place the #569 entry under [Unreleased]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-10-21 07:21:40 +02:00
1 parent b67eb9e05a
commit 358d167cff
3 files changed
+70 -4

No files matched your search

+2
View File
@@ -426,6 +426,8 @@ per-file diff commands.
most cards and `height:0` alone is a spacer. Three new `Bypasses` cases, seven subtests most cards and `height:0` alone is a spacer. Three new `Bypasses` cases, seven subtests
failing on master, plus a calibration case pinning that either half alone stays visible. failing on master, plus a calibration case pinning that either half alone stays visible.
- **Upstream triage distinguishes subject-only matches from verified patch matches** (#569) - matching subjects with different patches now name the fork commit and request verification instead of claiming the change was already cherry-picked.
### Security ### Security
- **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a - **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a
+55
View File
@@ -205,6 +205,61 @@ class AlreadyAppliedTests(TriageRepoFixture):
self.assertIn("**1** worth reviewing", result.stdout) self.assertIn("**1** worth reviewing", result.stdout)
class SubjectCollisionTests(TriageRepoFixture):
def test_different_patch_with_same_subject_requests_verification(self):
self.write("kept.py", "print('upstream fix')\n")
upstream_sha = self.commit("Fix configuration")
self.set_upstream_to_head()
git(self.root, "reset", "--hard", "HEAD~1")
self.write("shared.txt", "fork configuration\n")
fork_sha = self.commit("Fix configuration")
result = self.run_triage()
self.assertEqual(result.returncode, 0, result.stderr)
self.assertIn("**0** worth reviewing", result.stdout)
self.assertIn(f"subject matches fork commit {fork_sha[:9]}, patch differs: verify", result.stdout)
self.assertNotIn(f"git cherry-pick {upstream_sha}", result.stdout)
self.assertNotIn("already applied (cherry-picked)", result.stdout)
def test_conflict_resolved_personalized_port_has_distinct_verify_reason(self):
profile = ".claude/skills/job-application-assistant/01-candidate-profile.md"
self.write(profile, "Name: template\nGuidance: old\n")
self.commit("Add template profile")
self.write(profile, "Name: template\nGuidance: improved\n")
upstream_sha = self.commit("Improve profile guidance")
self.set_upstream_to_head()
git(self.root, "reset", "--hard", "HEAD~1")
self.write(profile, "Name: personal\nGuidance: old\n")
self.commit("Personalize profile")
picked = subprocess.run(["git", "cherry-pick", upstream_sha], cwd=self.root,
capture_output=True, text=True)
self.assertNotEqual(picked.returncode, 0, "fixture must exercise a real conflict")
self.write(profile, "Name: personal\nGuidance: improved\n")
git(self.root, "add", profile)
git(self.root, "cherry-pick", "--continue")
fork_sha = git(self.root, "rev-parse", "HEAD").strip()
result = self.run_triage()
self.assertEqual(result.returncode, 0, result.stderr)
self.assertIn("**0** worth reviewing", result.stdout)
self.assertIn(f"subject matches fork commit {fork_sha[:9]}, patch differs: verify", result.stdout)
self.assertNotIn("already applied (cherry-picked)", result.stdout)
def test_same_patch_with_renamed_subject_is_still_already_applied(self):
self.write("kept.py", "print('upstream fix')\n")
upstream_sha = self.commit("Fix configuration")
self.set_upstream_to_head()
git(self.root, "reset", "--hard", "HEAD~1")
self.write("fork_only.txt", "fork\n")
self.commit("Fork configuration")
git(self.root, "cherry-pick", upstream_sha)
git(self.root, "commit", "--amend", "-m", "Port upstream configuration fix")
result = self.run_triage()
self.assertEqual(result.returncode, 0, result.stderr)
self.assertIn("**0** worth reviewing", result.stdout)
self.assertIn("already applied (cherry-picked)", result.stdout)
class SuggestedCherryPickOrderTests(TriageRepoFixture): class SuggestedCherryPickOrderTests(TriageRepoFixture):
def test_suggested_commands_apply_dependent_changes_in_ancestry_order(self): def test_suggested_commands_apply_dependent_changes_in_ancestry_order(self):
self.write("kept.py", "print('first upstream change')\n") self.write("kept.py", "print('first upstream change')\n")
+13 -4
View File
@@ -18,8 +18,10 @@ Two signals drive the sort:
1. Already applied? A cherry-pick lands with a NEW sha but the same patch, so a 1. Already applied? A cherry-pick lands with a NEW sha but the same patch, so a
raw sha comparison misreports it as missing. We compute git patch-ids for the raw sha comparison misreports it as missing. We compute git patch-ids for the
fork-only commits and treat any upstream commit whose patch-id (or exact fork-only commits and treat any upstream commit whose patch-id
subject) matches as already applied. matches as already applied. A matching subject with a different patch may be a
conflict-resolved port or an unrelated change: keep it as a distinct verify
reason in the advisory skip list, never as proof of a cherry-pick.
2. Relevant to this fork? A commit that only touches files this fork deleted 2. Relevant to this fork? A commit that only touches files this fork deleted
(e.g. removed demo portals) is almost certainly N/A. We check each commit's (e.g. removed demo portals) is almost certainly N/A. We check each commit's
@@ -175,7 +177,9 @@ def main() -> int:
fork_only = rev_list(f"{ref}..HEAD") fork_only = rev_list(f"{ref}..HEAD")
fork_patch_ids = {p for p in (patch_id(s) for s in fork_only) if p} fork_patch_ids = {p for p in (patch_id(s) for s in fork_only) if p}
fork_subjects = {subject(s) for s in fork_only} fork_subjects = {}
for fork_sha in fork_only:
fork_subjects.setdefault(subject(fork_sha), fork_sha)
try: try:
common_base = git("merge-base", "HEAD", ref).strip() common_base = git("merge-base", "HEAD", ref).strip()
except subprocess.CalledProcessError: except subprocess.CalledProcessError:
@@ -188,9 +192,14 @@ def main() -> int:
for sha in behind: for sha in behind:
subj = subject(sha) subj = subject(sha)
short = sha[:9] short = sha[:9]
if patch_id(sha) in fork_patch_ids or subj in fork_subjects: if patch_id(sha) in fork_patch_ids:
skip.append((short, sha, subj, "already applied (cherry-picked)")) skip.append((short, sha, subj, "already applied (cherry-picked)"))
continue continue
if subj in fork_subjects:
match = fork_subjects[subj][:9]
skip.append((short, sha, subj,
f"subject matches fork commit {match}, patch differs: verify"))
continue
if any(sha.startswith(e) for e in wontport): if any(sha.startswith(e) for e in wontport):
skip.append((short, sha, subj, "on the fork's won't-port list")) skip.append((short, sha, subj, "on the fork's won't-port list"))
continue continue