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 <noreply@anthropic.com>

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

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com>
This commit is contained in:
authored and GitHub committed 2026-10-19 20:04:43 +02:00
1 parent 09a8c31b24
commit 93f37dde0b
3 files changed
+64 -2

No files matched your search

+9
View File
@@ -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
+31
View File
@@ -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")
+24 -2
View File
@@ -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,