fix(upstream): distinguish new paths from fork deletions (#585)

* fix(upstream): distinguish new paths from fork deletions

* docs(changelog): record fix for #585

CHANGELOG.md

@@ -194,6 +194,8 @@ per-file diff commands.
   the Jobtype item and a description paragraph (fails on master), and an entity-encoded
   `<div>` in a description that stays in the text.
 
+- **Upstream checks distinguish new files from fork deletions** (#585) - a path absent from the base is reported as newly added upstream instead of incorrectly treated as a deletion in the fork.
+
 - **`/add-portal` includes the `.agents` prefix in generated `allowed-tools`** (#545,
   `.claude/commands/add-portal.md`, `tests/test_add_portal_command.py`) - the specification template
   for the new portal's `SKILL.md` frontmatter wrote `allowed-tools: Bash(bun run skills/<name>/cli/src/cli.ts *)`

tests/test_upstream_triage.py

@@ -71,13 +71,15 @@ class UpToDateTests(TriageRepoFixture):
 
 class RelevanceFilterTests(TriageRepoFixture):
     def test_commit_touching_only_removed_files_is_skipped(self):
-        # Upstream edits a file this fork never had -> not relevant.
+        # Both start with the portal; upstream edits it while the fork drops it.
         self.write("portals/removed_portal.py", "x = 1\n")
-        self.commit("upstream: add removed_portal")
+        self.commit("shared: add portal")
+        self.write("portals/removed_portal.py", "x = 2\n")
+        self.commit("upstream: update portal")
         self.set_upstream_to_head()
-        # Fork drops back to before that commit and deletes nothing extra;
-        # the file simply is not in fork HEAD.
         git(self.root, "reset", "--hard", "HEAD~1")
+        git(self.root, "rm", "portals/removed_portal.py")
+        self.commit("fork: remove demo portal")
 
         result = self.run_triage()
         self.assertEqual(result.returncode, 0, result.stderr)
@@ -98,19 +100,54 @@ class RelevanceFilterTests(TriageRepoFixture):
         self.assertIn("git cherry-pick", result.stdout)
 
     def test_changelog_only_footprint_is_skipped(self):
+        self.write("portals/gone.py", "y = 1\n")
+        self.write("CHANGELOG.md", "- base\n")
+        self.commit("shared: add portal and changelog")
         self.write("portals/gone.py", "y = 2\n")
         self.write("CHANGELOG.md", "- did a thing\n")
         self.commit("upstream: feature living in removed area + changelog")
         self.set_upstream_to_head()
         # Fork ships CHANGELOG.md but not the removed portal file.
         git(self.root, "reset", "--hard", "HEAD~1")
-        self.write("CHANGELOG.md", "- fork changelog\n")
-        self.commit("fork changelog")
+        git(self.root, "rm", "portals/gone.py")
+        self.commit("fork: remove demo portal")
 
         result = self.run_triage()
         self.assertEqual(result.returncode, 0, result.stderr)
         self.assertIn("changelog-only footprint in this fork", result.stdout)
 
+    def test_new_upstream_tool_is_worth_reviewing(self):
+        self.write("tools/new_workflow.py", "print('new workflow')\n")
+        added = self.commit("upstream: add a new workflow tool")
+        self.set_upstream_to_head()
+        git(self.root, "reset", "--hard", "HEAD~1")
+        before = git(self.root, "status", "--porcelain")
+
+        result = self.run_triage()
+
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("**1** worth reviewing", result.stdout)
+        self.assertIn("tools/new_workflow.py", result.stdout)
+        self.assertIn(f"git cherry-pick {added}", result.stdout)
+        self.assertEqual(git(self.root, "status", "--porcelain"), before)
+        self.assertFalse((self.root / "tools/new_workflow.py").exists())
+
+    def test_new_tool_with_changelog_is_not_changelog_only(self):
+        self.write("CHANGELOG.md", "- base\n")
+        self.commit("shared: add changelog")
+        self.write("tools/new_workflow.py", "print('new workflow')\n")
+        self.write("CHANGELOG.md", "- new workflow tool\n")
+        added = self.commit("upstream: ship a new tool and changelog")
+        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)
+        self.assertNotIn("changelog-only footprint in this fork", result.stdout)
+
 
 class AlreadyAppliedTests(TriageRepoFixture):
     def test_cherry_picked_commit_drops_off_via_patch_id(self):

tools/upstream_triage.py

@@ -23,7 +23,8 @@ 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 the working tree and flag accordingly.
+   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.
 
 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
@@ -81,10 +82,10 @@ def files_touched(sha: str) -> list[str]:
     return [f for f in out.splitlines() if f]
 
 
-def path_exists(path: str) -> bool:
-    # ls-tree against HEAD is authoritative for "does this fork still ship it".
+def path_exists(path: str, ref: str = "HEAD") -> bool:
+    # Read committed trees, rather than uncommitted files in the working tree.
     r = subprocess.run(
-        ["git", "cat-file", "-e", f"HEAD:{path}"], capture_output=True
+        ["git", "cat-file", "-e", f"{ref}:{path}"], capture_output=True
     )
     return r.returncode == 0
 
@@ -154,6 +155,11 @@ def main() -> int:
     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_subjects = {subject(s) for s in fork_only}
+    try:
+        common_base = git("merge-base", "HEAD", ref).strip()
+    except subprocess.CalledProcessError:
+        # Without shared history, absence cannot establish a deliberate deletion.
+        common_base = None
 
     review: list[tuple[str, str, str, list[str]]] = []
     skip: list[tuple[str, str, str, str]] = []
@@ -168,14 +174,17 @@ def main() -> int:
             skip.append((short, sha, subj, "on the fork's won't-port list"))
             continue
         touched = files_touched(sha)
-        present = [f for f in touched if path_exists(f)]
+        relevant = [
+            f for f in touched
+            if path_exists(f) or common_base is None or not path_exists(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,
         # only a doc line would. Low signal; demote it.
-        substantive = [f for f in present if f != "CHANGELOG.md"]
-        if touched and not present:
+        substantive = [f for f in relevant if f != "CHANGELOG.md"]
+        if touched and not relevant:
             skip.append((short, sha, subj, "touches only files not in this fork"))
-        elif present and not substantive:
+        elif relevant and not substantive:
             skip.append((short, sha, subj, "changelog-only footprint in this fork"))
         else:
             review.append((short, sha, subj, substantive))
@@ -190,7 +199,7 @@ def main() -> int:
     lines.append("### Worth reviewing")
     if review:
         lines.append("")
-        lines.append("| Commit | Subject | Fork files it touches |")
+        lines.append("| Commit | Subject | Relevant files it touches |")
         lines.append("|---|---|---|")
         for short, sha, subj, present in review:
             shown = ", ".join(f"`{p}`" for p in present[:4]) or "_(new/shared paths)_"