Make repo reconciliation review-first

Stop automatically stashing, pulling, and popping dirty repos during reconciliation. Clean repos still pull, dirty repos open Magit for review, and results now include structured statuses, skip reasons, pruning, and a summary.

1 files changed, 22 insertions, 83 deletions

diff --git a/tests/test-reconcile--pull-dirty.el b/tests/test-reconcile--pull-dirty.el
index 2ba1f5d1..c26c8548 100644
--- a/tests/test-reconcile--pull-dirty.el
+++ b/tests/test-reconcile--pull-dirty.el
@@ -1,7 +1,7 @@
-;;; test-reconcile--pull-dirty.el --- Tests for cj/reconcile--pull-dirty -*- lexical-binding: t; -*-
+;;; test-reconcile--pull-dirty.el --- Tests for dirty repo review handling -*- lexical-binding: t; -*-
 
 ;;; Commentary:
-;; Tests for the dirty-repo reconciliation: stash, pull, pop, magit.
+;; Dirty repositories should be review-first: no stash, pull, or stash-pop.
 
 ;;; Code:
 
@@ -9,104 +9,43 @@
 (require 'testutil-reconcile-open-repos)
 (require 'reconcile-open-repos)
 
-;;; Normal Cases
-
-(ert-deftest test-pull-dirty-normal-stash-pull-pop-success ()
-  "When stash, pull, and pop all succeed, magit is still opened."
+(ert-deftest test-pull-dirty-normal-opens-magit-for-review ()
+  "Dirty repo handling opens Magit and returns a needs-review result."
   (reconcile-test-with-temp-dirs
    ("repo/.git/")
    (let ((dir (expand-file-name "repo" test-root)))
      (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (_cmd) 0)
-           (lambda (_cmd) "")
-         (cj/reconcile--pull-dirty dir))
-       (should (member dir reconcile-test-magit-calls))))))
-
-(ert-deftest test-pull-dirty-normal-stash-fails-opens-magit ()
-  "When stash fails, magit is opened and warning emitted."
-  (reconcile-test-with-temp-dirs
-   ("repo/.git/")
-   (let ((dir (expand-file-name "repo" test-root))
-         (messages nil))
-     (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (cmd)
-             (if (string-match-p "stash --quiet\\'" cmd) 1 0))
-           (lambda (_cmd) "")
-         (cl-letf (((symbol-function 'message)
-                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
-           (cj/reconcile--pull-dirty dir)))
-       (should (member dir reconcile-test-magit-calls))
-       (should (cl-some (lambda (m) (string-match-p "stash failed" m)) messages))))))
+       (let ((result (cj/reconcile--pull-dirty dir)))
+         (should (member dir reconcile-test-magit-calls))
+         (should (eq (plist-get result :status) 'needs-review))
+         (should (equal (plist-get result :directory) dir)))))))
 
-(ert-deftest test-pull-dirty-normal-pull-fails-warns ()
-  "When stash succeeds but pull fails, warning mentions pull failure."
-  (reconcile-test-with-temp-dirs
-   ("repo/.git/")
-   (let ((dir (expand-file-name "repo" test-root))
-         (messages nil))
-     (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (cmd)
-             (cond ((string-match-p "stash --quiet\\'" cmd) 0)
-                   ((string-match-p "pull" cmd) 1)
-                   (t 0)))
-           (lambda (_cmd) "")
-         (cl-letf (((symbol-function 'message)
-                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
-           (cj/reconcile--pull-dirty dir)))
-       (should (cl-some (lambda (m) (string-match-p "git pull failed" m)) messages))))))
-
-(ert-deftest test-pull-dirty-normal-stash-pop-fails-warns ()
-  "When stash and pull succeed but pop fails, warning mentions stash pop."
-  (reconcile-test-with-temp-dirs
-   ("repo/.git/")
-   (let ((dir (expand-file-name "repo" test-root))
-         (messages nil))
-     (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (cmd)
-             (cond ((string-match-p "stash pop" cmd) 1)
-                   ((string-match-p "stash" cmd) 0)
-                   (t 0)))
-           (lambda (_cmd) "")
-         (cl-letf (((symbol-function 'message)
-                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
-           (cj/reconcile--pull-dirty dir)))
-       (should (cl-some (lambda (m) (string-match-p "stash pop failed" m)) messages))))))
-
-;;; Boundary Cases
-
-(ert-deftest test-pull-dirty-boundary-always-opens-magit ()
-  "Magit is opened regardless of whether pull succeeds or fails."
+(ert-deftest test-pull-dirty-normal-does-not-run-git-commands ()
+  "Dirty repo handling must not mutate the worktree with git commands."
   (reconcile-test-with-temp-dirs
    ("repo/.git/")
    (let ((dir (expand-file-name "repo" test-root)))
-     ;; Test with pull failure
      (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (cmd)
-             (if (string-match-p "pull" cmd) 1 0))
-           (lambda (_cmd) "")
-         (cl-letf (((symbol-function 'message) (lambda (_fmt &rest _args))))
-           (cj/reconcile--pull-dirty dir)))
+       (reconcile-test-with-git-mock
+           (lambda (_args)
+             (ert-fail "Dirty repo handler should not run git commands"))
+         (cj/reconcile--pull-dirty dir))
        (should (member dir reconcile-test-magit-calls))))))
 
 (ert-deftest test-pull-dirty-boundary-uncommitted-work-message ()
-  "Always emits 'contains uncommitted work' message."
+  "Dirty repo handling announces review instead of auto-reconciling."
   (reconcile-test-with-temp-dirs
    ("repo/.git/")
    (let ((dir (expand-file-name "repo" test-root))
          (messages nil))
      (reconcile-test-with-magit-mock
-       (reconcile-test-with-shell-mocks
-           (lambda (_cmd) 0)
-           (lambda (_cmd) "")
-         (cl-letf (((symbol-function 'message)
-                    (lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
-           (cj/reconcile--pull-dirty dir)))
-       (should (cl-some (lambda (m) (string-match-p "uncommitted work" m)) messages))))))
+       (cl-letf (((symbol-function 'message)
+                  (lambda (fmt &rest args)
+                    (push (apply #'format fmt args) messages))))
+         (cj/reconcile--pull-dirty dir)))
+     (should (cl-some (lambda (m)
+                        (string-match-p "opening Magit for review" m))
+                      messages)))))
 
 (provide 'test-reconcile--pull-dirty)
 ;;; test-reconcile--pull-dirty.el ends here