fix(pstack): make the upstream merge lever refuse drift and report unsafe rows

scripts/upstream-merge-probe.py

@@ -0,0 +1,136 @@
+#!/usr/bin/env python3
+"""Prove scripts/upstream-merge.py refuses unsafe runs and applies safe ones.
+
+Builds throwaway worktrees at the audited port commit, feeds the real audit
+and relabeled variants through the merge, and asserts what changed on disk.
+
+    python3 scripts/upstream-audit.py --port <sha> --upstream <sha> > audit.json
+    python3 scripts/upstream-merge-probe.py audit.json
+"""
+import copy
+import json
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+
+ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+MERGE = os.path.join(ROOT, "scripts", "upstream-merge.py")
+
+
+def sh(*args, cwd, check=True):
+    return subprocess.run(args, cwd=cwd, capture_output=True, text=True, check=check)
+
+
+def worktree(commit):
+    path = tempfile.mkdtemp(prefix="merge-probe-")
+    os.rmdir(path)
+    sh("git", "worktree", "add", "--detach", "-q", path, commit, cwd=ROOT)
+    return path
+
+
+def run(audit, tree):
+    audit_path = os.path.join(tree, "probe-audit.json")
+    json.dump(audit, open(audit_path, "w"))
+    result = sh("python3", MERGE, audit_path, cwd=tree, check=False)
+    os.remove(audit_path)
+    return result.returncode, result.stdout + result.stderr
+
+
+def changed(tree):
+    return sh("git", "status", "--porcelain", "--untracked-files=all", cwd=tree).stdout.strip()
+
+
+def check(name, condition, detail=""):
+    print(f"{'ok' if condition else 'FAIL'}: {name}{'' if condition else ' ' + detail}")
+    return condition
+
+
+def main(audit_path):
+    audit = json.load(open(audit_path))
+    port = audit["port_commit"]
+    results = []
+    trees = []
+    try:
+        tree = worktree(port); trees.append(tree)
+        code, out = run(audit, tree)
+        results.append(check("real range merges (exit 1 for hand review)", code == 1 and "verbatim 24, clean merge 32, needs review 32, removed 2" in out, out.splitlines()[0] if out else ""))
+
+        tree = worktree(f"{port}~1"); trees.append(tree)
+        code, out = run(audit, tree)
+        results.append(check("stale HEAD refused, nothing written", code == 2 and changed(tree) == "", out))
+
+        tree = worktree(port); trees.append(tree)
+        dirty = next(c["port_path"] for c in audit["changes"] if c["comparison"] == "unchanged-since-base")
+        open(os.path.join(tree, dirty), "a").write("\nlocal edit\n")
+        code, out = run(audit, tree)
+        results.append(check("dirty mapped path refused, edit kept", code == 2 and "local edit" in open(os.path.join(tree, dirty)).read(), out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        variant["changes"] = [c for c in variant["changes"] if c["change"] != "modify"]
+        for c in variant["changes"]:
+            c["comparison"] = "port-diverged-review"
+        code, out = run(variant, tree)
+        results.append(check("diverged add/delete reported, nothing written", code == 1 and changed(tree) == "" and out.count("review upstream") == len(variant["changes"]), out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        row = copy.deepcopy(next(c for c in variant["changes"] if c["change"] == "modify" and c["port_path"]))
+        row["comparison"] = "absent-from-port-review-exclusion"
+        row["port_path"] = "plugins/pstack/skills/make-bot-ui/SKILL.md"
+        variant["changes"] = [row]
+        code, out = run(variant, tree)
+        results.append(check("excluded path reported without crash or directory", code == 1 and "excluded path" in out and not os.path.exists(os.path.join(tree, "plugins/pstack/skills/make-bot-ui")), out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        conflicted = next(c for c in variant["changes"] if c["comparison"] == "port-diverged-review" and c["change"] == "modify")
+        variant["changes"] = [conflicted]
+        sibling = os.path.join(tree, conflicted["port_path"] + ".upstream-base")
+        open(sibling, "w").write("keep me")
+        code, out = run(variant, tree)
+        results.append(check("sibling temp-name file untouched", os.path.exists(sibling) and open(sibling).read() == "keep me", out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        row = copy.deepcopy(next(c for c in variant["changes"] if c["comparison"] == "unchanged-since-base"))
+        row["target"]["mode"] = "100755"
+        variant["changes"] = [row]
+        code, out = run(variant, tree)
+        results.append(check("executable bit applied from target mode", code == 0 and os.access(os.path.join(tree, row["port_path"]), os.X_OK), out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        row = copy.deepcopy(next(c for c in variant["changes"] if c["change"] == "add"))
+        row["comparison"] = "already-matches-target"
+        variant["changes"] = [row]
+        code, out = run(variant, tree)
+        results.append(check("already-matching addition is a no-op", code == 0 and changed(tree) == "", out))
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        variant["changes"] = [c for c in variant["changes"] if c["port_path"] is None]
+        open(os.path.join(tree, "audit.json"), "w").write("{}")
+        code, out = run(variant, tree)
+        results.append(check("all-unmapped audit is not refused by the untracked audit file", code == 0 and "unmapped" in out, out))
+
+        tree = worktree(port); trees.append(tree)
+        variant = copy.deepcopy(audit)
+        row = copy.deepcopy(next(c for c in variant["changes"] if c["comparison"] == "port-diverged-review" and c["change"] == "modify"))
+        row["target"]["mode"] = "100755"
+        variant["changes"] = [row]
+        binary = os.path.join(tree, row["port_path"])
+        open(binary, "wb").write(b"\x00\x01 binary local copy\n")
+        sh("git", "update-index", "--assume-unchanged", row["port_path"], cwd=tree)
+        code, out = run(variant, tree)
+        results.append(check("binary merge-file failure reported, no mode change", "merge-file failed" in out and not os.access(binary, os.X_OK), out))
+    finally:
+        for tree in trees:
+            sh("git", "worktree", "remove", "--force", tree, cwd=ROOT, check=False)
+            shutil.rmtree(tree, ignore_errors=True)
+    return 0 if all(results) else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1]))

scripts/upstream-merge.py

@@ -1,11 +1,12 @@
 #!/usr/bin/env python3
 """Apply an upstream pstack range to the port tree mechanically.
 
-Reads the JSON that scripts/upstream-audit.py prints. For each mapped
-modification it either checks out the upstream blob (the port still matches
-the old upstream blob) or runs a three-way `git merge-file` in place, leaving
-conflict markers for the hand pass. Additions are copied; deletions are
-removed. Unmapped paths are listed and left alone.
+Reads the JSON that scripts/upstream-audit.py prints. For each mapped path it
+either checks out the upstream blob (the port still matches the old upstream
+blob), runs a three-way `git merge-file` in place and leaves conflict markers
+for the hand pass, or reports the row for review without touching the tree.
+Refuses to run unless HEAD is the audited port commit and the mapped paths are
+clean, so an audit never overwrites work done after it was taken.
 
     python3 scripts/upstream-audit.py --port <sha> --upstream <sha> > audit.json
     python3 scripts/upstream-merge.py audit.json
@@ -14,55 +15,102 @@
 import os
 import subprocess
 import sys
+import tempfile
+
+REGULAR_MODES = {"100644", "100755"}
+
+
+def git(*args, **kwargs):
+    return subprocess.run(["git", *args], capture_output=True, check=True, **kwargs).stdout
 
 
 def blob(rev, path):
-    return subprocess.run(["git", "show", f"{rev}:{path}"], capture_output=True, check=True).stdout
+    return git("show", f"{rev}:{path}")
+
+
+def apply_mode(path, mode):
+    executable = mode == "100755"
+    current = os.stat(path).st_mode
+    os.chmod(path, (current | 0o111) if executable else (current & ~0o111))
+
+
+def refuse_drift(audit, ports):
+    head = git("rev-parse", "HEAD").decode().strip()
+    if head != audit["port_commit"]:
+        return f"HEAD {head[:12]} is not the audited port commit {audit['port_commit'][:12]}"
+    if not ports:
+        return None
+    dirty = git("status", "--porcelain", "--untracked-files=all", "--", *ports).decode().strip()
+    if dirty:
+        return "mapped paths have local changes:\n" + dirty
+    return None
+
+
+def merge_three_way(port, old, new):
+    with tempfile.TemporaryDirectory(prefix="upstream-merge-") as tmp:
+        base_path, new_path = os.path.join(tmp, "base"), os.path.join(tmp, "target")
+        open(base_path, "wb").write(old)
+        open(new_path, "wb").write(new)
+        return subprocess.run(
+            ["git", "merge-file", "-L", "port", "-L", "upstream-base", "-L", "upstream-target", port, base_path, new_path]
+        ).returncode
 
 
 def main(audit_path):
     audit = json.load(open(audit_path))
     base, target = audit["upstream_base"], audit["upstream_target"]
-    verbatim, clean, conflicted, removed, skipped = [], [], [], [], []
+    mapped = [c for c in audit["changes"] if c["port_path"] is not None]
+    drift = refuse_drift(audit, [c["port_path"] for c in mapped])
+    if drift:
+        print(f"refusing to run: {drift}")
+        return 2
+    verbatim, clean, review, removed, skipped = [], [], [], [], []
     for change in audit["changes"]:
-        up, port = change["upstream_path"], change["port_path"]
+        up, port, comparison = change["upstream_path"], change["port_path"], change["comparison"]
         if port is None:
             skipped.append(up)
             continue
-        comparison = change["comparison"]
+        if comparison == "already-matches-target":
+            continue
+        if comparison == "absent-from-port-review-exclusion":
+            review.append((port, f"upstream {change['change']}d an excluded path"))
+            continue
         if change["change"] == "delete":
-            if comparison not in ("unchanged-since-base", "already-matches-target"):
-                conflicted.append((port, f"upstream deleted a port-edited file ({comparison})"))
+            if comparison != "unchanged-since-base":
+                review.append((port, f"upstream deleted a port-edited file ({comparison})"))
                 continue
-            if os.path.exists(port):
-                os.remove(port)
+            os.remove(port)
             removed.append(port)
             continue
+        mode = change["target"]["mode"]
+        if mode not in REGULAR_MODES:
+            review.append((port, f"upstream entry mode {mode} is not a regular file"))
+            continue
         new = blob(target, up)
         if change["change"] == "add" and comparison != "upstream-addition":
-            conflicted.append((port, f"upstream added a path the port already has ({comparison})"))
+            review.append((port, f"upstream added a path the port already has ({comparison})"))
             continue
         if change["change"] == "add" or comparison == "unchanged-since-base":
             os.makedirs(os.path.dirname(port) or ".", exist_ok=True)
             open(port, "wb").write(new)
+            apply_mode(port, mode)
             verbatim.append(port)
             continue
-        old = blob(base, up)
-        tmp_base, tmp_new = port + ".upstream-base", port + ".upstream-target"
-        open(tmp_base, "wb").write(old)
-        open(tmp_new, "wb").write(new)
-        result = subprocess.run(
-            ["git", "merge-file", "-L", "port", "-L", "upstream-base", "-L", "upstream-target", port, tmp_base, tmp_new]
-        )
-        os.remove(tmp_base)
-        os.remove(tmp_new)
-        (clean if result.returncode == 0 else conflicted).append((port, result.returncode))
-    print(f"verbatim {len(verbatim)}, clean merge {len(clean)}, conflicted {len(conflicted)}, removed {len(removed)}, unmapped {len(skipped)}")
-    for port, why in conflicted:
-        print(f"conflict {why} {port}")
+        hunks = merge_three_way(port, blob(base, up), new)
+        if hunks < 0 or hunks > 127:
+            review.append((port, f"git merge-file failed with status {hunks}"))
+            continue
+        apply_mode(port, mode)
+        if hunks:
+            review.append((port, f"{hunks} conflict hunks"))
+        else:
+            clean.append(port)
+    print(f"verbatim {len(verbatim)}, clean merge {len(clean)}, needs review {len(review)}, removed {len(removed)}, unmapped {len(skipped)}")
+    for port, why in review:
+        print(f"review {why}: {port}")
     for path in skipped:
         print(f"unmapped {path}")
-    return 1 if conflicted else 0
+    return 1 if review else 0
 
 
 if __name__ == "__main__":