Fix 500 on package retry/upgrade due to duplicate environment_packages rows

backend/migrations/versions/f1a2b3c4d5e6_unique_env_pkg.py

@@ -0,0 +1,48 @@
+"""add unique constraint on environment_packages(environment_id, package_name)
+
+Removes duplicate rows (keeping the highest-id record per pair) and then
+adds a unique constraint so the MultipleResultsFound 500 error cannot recur.
+
+Revision ID: f1a2b3c4d5e6
+Revises: b4d2e1a9c3f7
+Create Date: 2026-06-12 08:00:00.000000
+"""
+from typing import Sequence, Union
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = "f1a2b3c4d5e6"
+down_revision: Union[str, None] = "b4d2e1a9c3f7"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    conn = op.get_bind()
+
+    # Delete duplicate rows, keeping only the row with the highest id for each
+    # (environment_id, package_name) pair.  Works on both SQLite and PostgreSQL.
+    conn.execute(
+        sa.text(
+            """
+            DELETE FROM environment_packages
+            WHERE id NOT IN (
+                SELECT MAX(id)
+                FROM environment_packages
+                GROUP BY environment_id, package_name
+            )
+            """
+        )
+    )
+
+    with op.batch_alter_table("environment_packages") as batch_op:
+        batch_op.create_unique_constraint(
+            "uq_env_pkg", ["environment_id", "package_name"]
+        )
+
+
+def downgrade() -> None:
+    with op.batch_alter_table("environment_packages") as batch_op:
+        batch_op.drop_constraint("uq_env_pkg", type_="unique")

backend/src/environments/models.py

@@ -2,7 +2,7 @@
 
 from datetime import datetime
 
-from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, String, Text
+from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, String, Text, UniqueConstraint
 from sqlalchemy.orm import Mapped, mapped_column
 
 from src.database import Base, TimestampMixin
@@ -36,6 +36,7 @@ class EnvironmentPackage(Base):
     """Installed package in an environment."""
 
     __tablename__ = "environment_packages"
+    __table_args__ = (UniqueConstraint("environment_id", "package_name", name="uq_env_pkg"),)
 
     id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
     environment_id: Mapped[int] = mapped_column(ForeignKey("environments.id", ondelete="CASCADE"), index=True)

backend/src/environments/router.py

@@ -476,6 +476,10 @@ def install_package(
 
     pkg = add_package(db, env_id, data)
 
+    # Commit before dispatching so the background thread's separate session
+    # can see the new record (per CLAUDE.md critical pattern).
+    db.commit()
+
     # Trigger async installation
     try:
         from src.environments.tasks import install_package
@@ -504,20 +508,18 @@ def upgrade_package(
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Environment not found")
 
     # Find existing package record
-    result = db.execute(
+    pkg = db.execute(
         select(EnvironmentPackage).where(
             EnvironmentPackage.environment_id == env_id,
             EnvironmentPackage.package_name == package_name,
         )
-    )
-    pkg = result.scalar_one_or_none()
+    ).scalars().first()
     if pkg is None:
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Package not found")
 
     # Clear version constraint and trigger upgrade
     pkg.version = None
-    db.flush()
-    db.refresh(pkg)
+    db.commit()
 
     try:
         from src.environments.tasks import upgrade_package
@@ -550,7 +552,7 @@ def retry_package_install(
             EnvironmentPackage.environment_id == env_id,
             EnvironmentPackage.package_name == package_name,
         )
-    ).scalar_one_or_none()
+    ).scalars().first()
     if pkg is None:
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Package not found")
 

backend/src/environments/service.py

@@ -174,7 +174,29 @@ def list_packages(db: Session, env_id: int) -> list[EnvironmentPackage]:
 
 
 def add_package(db: Session, env_id: int, data: PackageCreate) -> EnvironmentPackage:
-    """Add a package to an environment."""
+    """Add a package to an environment.
+
+    If the package already exists (e.g. from a previous failed install),
+    the existing record is reset to 'pending' so it can be retried without
+    creating a duplicate row — which would cause MultipleResultsFound errors
+    in subsequent retry/upgrade queries.
+    """
+    existing = db.execute(
+        select(EnvironmentPackage).where(
+            EnvironmentPackage.environment_id == env_id,
+            EnvironmentPackage.package_name == data.package_name,
+        )
+    ).scalars().first()
+
+    if existing is not None:
+        existing.version = data.version
+        existing.install_status = "pending"
+        existing.install_error = None
+        existing.installed_version = None
+        db.flush()
+        db.refresh(existing)
+        return existing
+
     pkg = EnvironmentPackage(
         environment_id=env_id,
         package_name=data.package_name,
@@ -188,16 +210,17 @@ def add_package(db: Session, env_id: int, data: PackageCreate) -> EnvironmentPac
 
 def remove_package(db: Session, env_id: int, package_name: str) -> None:
     """Remove a package from an environment."""
-    result = db.execute(
+    # Use .all() defensively: the unique constraint guarantees at most one row,
+    # but pre-migration databases may still have duplicates that need cleaning up.
+    pkgs = db.execute(
         select(EnvironmentPackage).where(
             EnvironmentPackage.environment_id == env_id,
             EnvironmentPackage.package_name == package_name,
         )
-    )
-    pkg = result.scalar_one_or_none()
-    if pkg:
+    ).scalars().all()
+    for pkg in pkgs:
         db.delete(pkg)
-        db.flush()
+    db.flush()
 
 
 # --- Variables ---

backend/src/environments/tasks.py

@@ -408,7 +408,7 @@ def _install_package_inner(env_id: int, package_name: str, version: str | None =
                 EnvironmentPackage.environment_id == env_id,
                 EnvironmentPackage.package_name == package_name,
             )
-        ).scalar_one_or_none()
+        ).scalars().first()
 
         if pkg:
             pkg.install_status = "installing"
@@ -542,7 +542,7 @@ def _upgrade_package_inner(env_id: int, package_name: str) -> dict:
                 EnvironmentPackage.environment_id == env_id,
                 EnvironmentPackage.package_name == package_name,
             )
-        ).scalar_one_or_none()
+        ).scalars().first()
 
         if pkg:
             pkg.install_status = "installing"

backend/tests/environments/test_router.py

@@ -1,6 +1,7 @@
 """Tests for environment management API endpoints."""
 
 import pytest
+from sqlalchemy import select, text
 from unittest.mock import patch, MagicMock
 
 from src.environments.models import Environment, EnvironmentPackage, EnvironmentVariable
@@ -746,6 +747,102 @@ def test_retry_endpoint_package_not_found(self, mock_dispatch, client, db_sessio
         )
         assert response.status_code == 404
 
+    @patch("src.environments.router.dispatch_task")
+    def test_install_package_twice_reuses_existing_record(self, mock_dispatch, client, db_session, admin_user):
+        """Installing the same package a second time must not create a duplicate row.
+
+        Previously a second POST would create a new EnvironmentPackage row; then
+        retry / upgrade queries using scalar_one_or_none() would raise
+        MultipleResultsFound → 500 Internal Server Error (GitHub #42).
+        """
+        mock_dispatch.return_value = MagicMock(id="fake-task-id")
+
+        env = Environment(
+            name="dedup-env",
+            python_version="3.12",
+            created_by=admin_user.id,
+        )
+        db_session.add(env)
+        db_session.flush()
+        db_session.refresh(env)
+
+        # First install — creates the row
+        resp1 = client.post(
+            f"{URL}/{env.id}/packages",
+            json={"package_name": "requests", "version": "2.31.0"},
+            headers=auth_header(admin_user),
+        )
+        assert resp1.status_code == 201
+        pkg_id_first = resp1.json()["id"]
+
+        # Simulate a failed install so the user would try again
+        db_session.execute(
+            text("UPDATE environment_packages SET install_status='failed' WHERE id=:id"),
+            {"id": pkg_id_first},
+        )
+        db_session.commit()
+
+        # Second install of the same package — must reuse the existing row
+        resp2 = client.post(
+            f"{URL}/{env.id}/packages",
+            json={"package_name": "requests", "version": "2.32.0"},
+            headers=auth_header(admin_user),
+        )
+        assert resp2.status_code == 201
+        pkg_id_second = resp2.json()["id"]
+
+        # Same DB row, not a new one
+        assert pkg_id_first == pkg_id_second
+
+        # Status reset to pending, version updated
+        assert resp2.json()["install_status"] == "pending"
+        assert resp2.json()["version"] == "2.32.0"
+
+        # Exactly one row in DB for this package
+        rows = db_session.execute(
+            select(EnvironmentPackage).where(
+                EnvironmentPackage.environment_id == env.id,
+                EnvironmentPackage.package_name == "requests",
+            )
+        ).scalars().all()
+        assert len(rows) == 1
+
+    @patch("src.environments.router.dispatch_task")
+    def test_retry_after_failed_install_does_not_500(self, mock_dispatch, client, db_session, admin_user):
+        """Retry on a failed package (the scenario from GitHub #42) must succeed.
+
+        The root cause was scalar_one_or_none() raising MultipleResultsFound when
+        duplicate rows existed.  With the upsert fix + unique constraint this can no
+        longer happen, but the endpoint should work correctly in all cases.
+        """
+        mock_dispatch.return_value = MagicMock(id="fake-task-id")
+
+        env = Environment(
+            name="retry-ok-env",
+            python_version="3.12",
+            created_by=admin_user.id,
+        )
+        db_session.add(env)
+        db_session.flush()
+        db_session.refresh(env)
+
+        pkg = EnvironmentPackage(
+            environment_id=env.id,
+            package_name="robotframework",
+            install_status="failed",
+            install_error="some error",
+        )
+        db_session.add(pkg)
+        db_session.flush()
+
+        response = client.post(
+            f"{URL}/{env.id}/packages/robotframework/retry",
+            headers=auth_header(admin_user),
+        )
+        assert response.status_code == 200
+        assert response.json()["install_status"] == "pending"
+        assert response.json()["install_error"] is None
+
 
 class TestRfLibraries:
     """Tests for the RF keyword library scan endpoint."""