fix(apps): refuse to reinstall an app from a different branch

docs/commands.md

@@ -27,7 +27,7 @@ Some runtime commands support all benches when invoked with the CLI option for a
 ## App Commands
 
 - `pilot new-app APP`: scaffold a new Frappe app under `apps/` and install it. Prompts for title, description, publisher, email, license, GitHub workflow, and branch; pass any of `--title/--description/--publisher/--email/--license/--branch/--github-workflow` to skip prompts (branch defaults to `develop`).
-- `pilot get-app REPO_OR_NAME`: clone and install an app into the bench.
+- `pilot get-app REPO_OR_NAME [--branch BRANCH_OR_COMMIT]`: clone and install an app into the bench. Without `--branch`, a fresh clone uses the remote default branch and an existing install is reused at its current revision. An explicit branch or commit is a revision constraint, so a mismatch with an existing checkout fails instead of reusing it.
 - `pilot list-apps`: list apps present in the bench.
 - `pilot install-app APP --site SITE`: install apps on a site. An app the site only has disabled is enabled instead, bringing back anything it requires first.
 - `pilot uninstall-app APP --site SITE`: uninstall apps from a site, dropping their data.

pilot/commands/apps/download.py

@@ -21,7 +21,7 @@ class GetAppCommand(Command):
     def __post_init__(self) -> None:
         from pilot.core.app import App
 
-        self.app = App.from_repo(self.bench, self.repo, self.branch or "main")
+        self.app = App.from_repo(self.bench, self.repo, self.branch)
         self.installed_dependencies: list[App] = []
 
     def run(self) -> None:

pilot/core/app/__init__.py

@@ -245,6 +245,40 @@ def build_assets(self) -> None:
             return
         run_command(["yarn", "--cwd", str(self.path), "build"])
 
+    def _reject_revision_mismatch(self, commit: str = "") -> None:
+        from pilot.integrations.git.base import repo_host, same_repository
+
+        requested = self.config.branch
+        installed = self.bench.app(self.module_name)
+        if (
+            repo_host(self.config.repo)
+            and repo_host(installed.config.repo)
+            and not same_repository(self.config.repo, installed.config.repo)
+        ):
+            raise BenchError(
+                f"'{self.config.name}' is already installed from a different repository. "
+                "Remove the app and add it again to change its repository."
+            )
+        requested_commit = commit or (requested if self.is_commit_hash(requested) else "")
+        if requested_commit:
+            if not installed.is_on_revision(RevisionPin(kind="commit", ref=requested_commit)):
+                raise BenchError(
+                    f"'{self.config.name}' is already installed at a different commit, "
+                    f"so this install cannot deliver '{requested_commit}'. Remove the app and "
+                    f"add it again to change its revision."
+                )
+            return
+        if not requested:
+            return
+        current = installed.current_branch
+        if requested != current:
+            source = f"branch '{current}'" if current else "a detached commit"
+            raise BenchError(
+                f"'{self.config.name}' is already installed from {source}, "
+                f"so this install cannot deliver '{requested}'. Remove the app and "
+                f"add it again to change its branch."
+            )
+
     def _skip_already_installed(
         self, on_progress: Callable[[str], None], install_dependencies: bool = False
     ) -> AppInstallResult:
@@ -262,6 +296,7 @@ def install(
     ) -> AppInstallResult:
         """Pinned commit based Clone, validate, install, register, and build app assets."""
         if self.bench.is_app_installed(self.config.name):
+            self._reject_revision_mismatch(commit)
             return self._skip_already_installed(on_progress, install_dependencies)
 
         existing_clone = self.existing_clone_path

tests/e2e/flows/admin.py

@@ -7,8 +7,8 @@
 
 
 def login(page: Page, base_url: str, password: str) -> None:
-    # The wizard's own sign-in carries over in this shared browser context, so without
-    # clearing it the login form would never render. Drop it to exercise real login.
+    # Let the setup page finish its restart redirect before starting login.
+    page.wait_for_url(f"{base_url}/sites", timeout=30_000)
     page.context.clear_cookies()
     page.goto(f"{base_url}/")
     page.get_by_placeholder("Password").fill(password)

tests/pilot/commands/test_get_app.py

@@ -2,15 +2,55 @@
 
 from __future__ import annotations
 
+import subprocess
 from pathlib import Path
 from unittest.mock import patch
 
+import pytest
+
 from pilot.commands.apps.download import GetAppCommand
 from pilot.core.app import App
+from pilot.exceptions import BenchError
 from pilot.integrations.marketplace import Marketplace, Resolver
 from tests.pilot.commands.test_commands import make_bench
 
 
+def register_app_on_branch(bench, name: str, branch: str) -> str:
+    app_dir = bench.apps_path / name
+    app_dir.mkdir(parents=True)
+    subprocess.run(["git", "init", "-b", branch, str(app_dir)], check=True, capture_output=True)
+    subprocess.run(
+        ["git", "-C", str(app_dir), "remote", "add", "origin", f"https://github.com/frappe/{name}"],
+        check=True,
+        capture_output=True,
+    )
+    (app_dir / "hooks.py").write_text("")
+    subprocess.run(["git", "-C", str(app_dir), "add", "hooks.py"], check=True, capture_output=True)
+    subprocess.run(
+        [
+            "git",
+            "-C",
+            str(app_dir),
+            "-c",
+            "user.name=Pilot Tests",
+            "-c",
+            "user.email=pilot@example.com",
+            "commit",
+            "-m",
+            "Initial commit",
+        ],
+        check=True,
+        capture_output=True,
+    )
+    (bench.sites_path / "apps.txt").write_text(f"frappe\n{name}\n")
+    return subprocess.run(
+        ["git", "-C", str(app_dir), "rev-parse", "HEAD"],
+        check=True,
+        capture_output=True,
+        text=True,
+    ).stdout.strip()
+
+
 def make_resolver(name: str, deps: dict[str, str] | None = None) -> Resolver:
     return Resolver(
         app=name,
@@ -71,6 +111,103 @@ def test_run_short_circuits_when_app_already_registered(tmp_path: Path) -> None:
     mock_build.assert_not_called()
 
 
+def test_reinstall_with_a_different_branch_fails_loudly(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    register_app_on_branch(bench, "myapp", "version-16")
+
+    cmd = GetAppCommand(bench, repo="https://github.com/frappe/myapp", branch="version-16-hotfix")
+
+    with pytest.raises(BenchError, match="already installed from branch 'version-16'"):
+        cmd.run()
+
+
+def test_reinstall_from_a_different_repository_fails_loudly(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    register_app_on_branch(bench, "myapp", "version-16")
+
+    cmd = GetAppCommand(bench, repo="https://github.com/acme/myapp", branch="version-16")
+
+    with pytest.raises(BenchError, match="already installed from a different repository"):
+        cmd.run()
+
+
+def test_reinstall_with_the_same_branch_still_short_circuits(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    register_app_on_branch(bench, "myapp", "version-16")
+
+    cmd = GetAppCommand(bench, repo="https://github.com/frappe/myapp", branch="version-16")
+
+    with patch.object(App, "clone") as mock_clone:
+        cmd.run()
+
+    mock_clone.assert_not_called()
+
+
+def test_reinstall_with_a_branch_rejects_a_detached_checkout(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    current = register_app_on_branch(bench, "myapp", "version-16")
+    subprocess.run(
+        ["git", "-C", str(bench.apps_path / "myapp"), "checkout", "--detach", current],
+        check=True,
+        capture_output=True,
+    )
+
+    cmd = GetAppCommand(bench, repo="https://github.com/frappe/myapp", branch="version-16")
+
+    with pytest.raises(BenchError, match="already installed from a detached commit"):
+        cmd.run()
+
+
+def test_reinstall_with_a_different_commit_fails_loudly(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    register_app_on_branch(bench, "myapp", "version-16")
+
+    requested = "0" * 40
+    cmd = GetAppCommand(bench, repo="https://github.com/frappe/myapp", branch=requested)
+
+    with pytest.raises(BenchError, match="already installed at a different commit"):
+        cmd.run()
+
+
+def test_reinstall_with_the_same_commit_still_short_circuits(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    current = register_app_on_branch(bench, "myapp", "version-16")
+
+    cmd = GetAppCommand(bench, repo="https://github.com/frappe/myapp", branch=current[:8])
+
+    with patch.object(App, "clone") as mock_clone:
+        cmd.run()
+
+    mock_clone.assert_not_called()
+
+
+def test_reinstall_with_a_different_marketplace_commit_fails_loudly(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    register_app_on_branch(bench, "myapp", "version-16")
+    app = App.from_repo(bench, "https://github.com/frappe/myapp", branch="version-16")
+
+    with pytest.raises(BenchError, match="already installed at a different commit"):
+        app.install(commit="0" * 40)
+
+
+def test_reinstall_with_the_same_marketplace_commit_still_short_circuits(tmp_path: Path) -> None:
+    bench = make_bench(tmp_path)
+    bench.create_directories()
+    current = register_app_on_branch(bench, "myapp", "version-16")
+    app = App.from_repo(bench, "https://github.com/frappe/myapp", branch="version-16")
+
+    result = app.install(commit=current[:8])
+
+    assert result.already_installed is True
+
+
 def test_short_circuit_adopts_real_on_disk_app_path(tmp_path: Path) -> None:
     """Regression: short-circuit uses the normalized on-disk app path."""
     bench = make_bench(tmp_path)