From 93f37dde0b6719231611a69472e9135e2a0e1637 Mon Sep 17 00:00:00 2001 From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:04:43 -0400 Subject: [PATCH] fix(upstream-triage): skip new upstream files inside a directory the fork removed (#656) * fix(upstream-triage): skip new upstream files inside a directory the fork removed A path absent from both HEAD and the merge base was always counted as new upstream work, so a file added inside a tree the fork deleted (a test added to a dropped portal) put the commit under "Worth reviewing" with a cherry-pick that can only conflict. The nearest existing ancestor directory now decides: present in HEAD means new work in a kept directory, present only at the base means the fork removed the tree. Co-Authored-By: Claude Opus 5.5 * changelog: place the #656 entry under [Unreleased] Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: Mads Lorentzen --- CHANGELOG.md | 9 +++++++++ tests/test_upstream_triage.py | 31 +++++++++++++++++++++++++++++++ tools/upstream_triage.py | 26 ++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff8391b..d23382c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -156,6 +156,15 @@ per-file diff commands. `administrativeRegion` filter (2,486 jobs for `Region Midtjylland`), and the docs map each `locations` category to its flag and give `--region` examples that exist. +- **`upstream_triage.py` skips a new upstream file inside a directory the fork removed** + (#656, follow-up to #585, `tools/upstream_triage.py`, `tests/test_upstream_triage.py`) - #585 counted + every path absent from both HEAD and the merge base as new upstream work, so a file added + inside a removed tree landed under "Worth reviewing": on a fork that dropped + `.agents/skills/jobnet-search/`, #586 (which adds a test file there and edits a source file + that is also gone) was listed with a ready-to-run cherry-pick that can only conflict. A new + path now counts as removed when its nearest ancestor directory exists at the base but not + in HEAD; a new file or subdirectory under a directory the fork kept is still worth reviewing. + ## [1.7.3] - 2026-10-19 ### Added diff --git a/tests/test_upstream_triage.py b/tests/test_upstream_triage.py index 198c9e82..c07faaa9 100644 --- a/tests/test_upstream_triage.py +++ b/tests/test_upstream_triage.py @@ -132,6 +132,37 @@ class RelevanceFilterTests(TriageRepoFixture): self.assertEqual(git(self.root, "status", "--porcelain"), before) self.assertFalse((self.root / "tools/new_workflow.py").exists()) + def test_new_file_in_removed_directory_is_skipped(self): + # #586's shape: upstream adds a test file to a portal the fork dropped. + self.write("portals/gone/cli/src/cli.ts", "run()\n") + self.commit("shared: add portal") + self.write("portals/gone/cli/tests/new.test.ts", "test()\n") + self.commit("upstream: add a test to the portal") + self.set_upstream_to_head() + git(self.root, "reset", "--hard", "HEAD~1") + git(self.root, "rm", "-r", "portals/gone") + self.commit("fork: remove demo portal") + + result = self.run_triage() + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("**0** worth reviewing", result.stdout) + self.assertIn("touches only files not in this fork", result.stdout) + + def test_new_directory_inside_kept_directory_is_worth_reviewing(self): + self.write("portals/kept/cli.ts", "run()\n") + self.commit("shared: add portal") + self.write("portals/kept/tests/new/case.test.ts", "test()\n") + added = self.commit("upstream: add nested tests to a kept portal") + self.set_upstream_to_head() + git(self.root, "reset", "--hard", "HEAD~1") + + result = self.run_triage() + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("**1** worth reviewing", result.stdout) + self.assertIn(f"git cherry-pick {added}", result.stdout) + def test_new_tool_with_changelog_is_not_changelog_only(self): self.write("CHANGELOG.md", "- base\n") self.commit("shared: add changelog") diff --git a/tools/upstream_triage.py b/tools/upstream_triage.py index 986de66e..7504ca28 100755 --- a/tools/upstream_triage.py +++ b/tools/upstream_triage.py @@ -24,7 +24,9 @@ Two signals drive the sort: 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 touched paths against HEAD and the shared base: a genuinely new upstream - path is worth reviewing, rather than a file the fork chose to remove. + path is worth reviewing, rather than a file the fork chose to remove. A new + file inside a directory the fork removed (a test added to a dropped portal) + counts as removed too. Usage: python tools/upstream_triage.py [--remote upstream] [--branch master] Exits 0 always (a report, not a gate). Prints a note to stderr and exits 0 if @@ -91,6 +93,24 @@ def path_exists(path: str, ref: str = "HEAD") -> bool: return r.returncode == 0 +def in_removed_dir(path: str, base: str) -> bool: + """Whether a path absent from HEAD and base sits in a directory the fork removed. + + The nearest ancestor directory decides: one HEAD still has means the path is + new in a directory the fork kept; one only the base has means the fork + deleted the tree the path would land in. Directories in neither are new + upstream too, so the walk goes on up to the root. + """ + parts = path.split("/")[:-1] + for depth in range(len(parts), 0, -1): + parent = "/".join(parts[:depth]) + if path_exists(parent): + return False + if path_exists(parent, base): + return True + return False + + def remote_slug(remote: str) -> str | None: """owner/repo for a GitHub remote, or None if it can't be parsed.""" try: @@ -177,7 +197,9 @@ def main() -> int: touched = files_touched(sha) relevant = [ f for f in touched - if path_exists(f) or common_base is None or not path_exists(f, common_base) + if path_exists(f) + or common_base is None + or not (path_exists(f, common_base) or in_removed_dir(f, common_base)) ] # A commit whose only surviving footprint is the changelog is one whose # real change lives in files this fork removed - the code doesn't apply,