From 4116e7edbd3c86743a3597550295f28e65a88e24 Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Wed, 10 Jun 2026 13:06:03 +0200
Subject: [PATCH 1/7] aggregate by

---
 .../actions/admin_action_list.py              | 109 ++++++++++++++----
 src/django_smartbase_admin/mcp/mcp.py         |  28 ++++-
 .../mcp/tests/test_aggregate.py               |  85 +++++++++++++-
 3 files changed, 192 insertions(+), 30 deletions(-)

diff --git a/src/django_smartbase_admin/actions/admin_action_list.py b/src/django_smartbase_admin/actions/admin_action_list.py
index 0967e10c..022ea22a 100644
--- a/src/django_smartbase_admin/actions/admin_action_list.py
+++ b/src/django_smartbase_admin/actions/admin_action_list.py
@@ -5,7 +5,7 @@
 
 from django.contrib.admin.utils import lookup_spawns_duplicates
 from django.core.exceptions import FieldError
-from django.db.models import Avg, Count, Field, Max, Min, Q, Sum
+from django.db.models import Avg, Count, Field, Max, Min, Q, Sum, Value
 from django.utils import timezone
 from django.utils.html import escape
 from django.utils.safestring import SafeString
@@ -783,14 +783,49 @@ def validate_aggregate(self, aggregate, field_map=None) -> list[dict]:
             )
         return specs
 
-    def get_aggregates(self, aggregate, field_map=None) -> dict:
-        """Compute ``{alias: value}`` for ``aggregate`` over the filtered set.
+    def validate_group_by(self, group_by, field_map=None) -> list:
+        """Resolve ``group_by`` names to declared SBAdminFields to GROUP BY on.
 
-        Reuses the list's filtered, annotated queryset. Non-multiplying
-        metrics batch into one query; a ``count`` over a multi-valued
-        relation runs separately and is merged.
+        A multi-valued (relation) field is rejected — grouping on it fans
+        the rows out. ``None`` / ``[]`` → no grouping; raises ``ValueError``
+        / ``LookupError`` on a bad entry.
+        """
+        if not group_by:
+            return []
+        if not isinstance(group_by, list):
+            raise ValueError("group_by must be a list of declared field names.")
+        if field_map is None:
+            field_map = self.view.get_field_map(self.threadsafe_request)
+
+        fields = []
+        for name in group_by:
+            sbfield = field_map.get(name)
+            if sbfield is None:
+                raise LookupError(
+                    f"Unknown field {name!r}; declared fields: {sorted(field_map)}."
+                )
+            if sbfield.is_multivalued():
+                raise ValueError(f"Cannot group by multi-valued field {name!r}.")
+            fields.append(sbfield)
+        return fields
+
+    def get_aggregates(self, aggregate, field_map=None, group_by=None):
+        """Compute ``aggregate`` totals over the filtered set.
+
+        Reuses the list's filtered, annotated queryset and runs a single
+        ``values(*group).annotate(**aggs)``. No ``group_by`` collapses to
+        one constant group (``Value(1)``), so the scalar rollup is just the
+        degenerate case of the same query.
+
+        Without ``group_by`` returns a scalar ``{alias: value}`` dict; with
+        ``group_by`` returns a list of ``{**group_fields, **aliases}`` rows,
+        ordered by the group columns. Each metric runs in its own query and
+        is merged by group key, so a metric that traverses a to-many
+        relation (a multi-valued ``count``, or an annotated field that
+        aggregates a related set) can never fan-out-inflate a sibling.
         """
         specs = self.validate_aggregate(aggregate, field_map=field_map)
+        group_fields = self.validate_group_by(group_by, field_map=field_map)
         # Full base so every aggregable field's annotation is present.
         queryset = self.get_data_queryset()
         pk_name = self.get_pk_field().name
@@ -812,6 +847,23 @@ def get_aggregates(self, aggregate, field_map=None) -> dict:
             if field.supporting_annotates:
                 extra.update(field.supporting_annotates)
             extra[field.field] = field.annotate
+
+        # Resolve group targets the same way; a computed (annotated) group
+        # column needs its expression present before ``.values()``.
+        group_targets: list[str] = []
+        for field in group_fields:
+            target = (
+                field.model_field.name if field.model_field is not None else field.field
+            )
+            group_targets.append(target)
+            if (
+                field.model_field is None
+                and target not in queryset.query.annotations
+                and field.annotate is not None
+            ):
+                if field.supporting_annotates:
+                    extra.update(field.supporting_annotates)
+                extra[target] = field.annotate
         if extra:
             queryset = queryset.annotate(**extra)
 
@@ -822,23 +874,34 @@ def get_aggregates(self, aggregate, field_map=None) -> dict:
         for plugin in request.request_data.configuration.plugins:
             queryset = plugin.modify_count_queryset(self, request=request, qs=queryset)
 
-        batch: dict = {}
-        isolated: list[tuple[str, Any]] = []
-        for spec in specs:
-            agg = LIST_AGGREGATE_FUNCTIONS[spec["fn"]](spec["target"])
-            if spec["multiplying"]:
-                isolated.append((spec["alias"], agg))
-            else:
-                batch[spec["alias"]] = agg
-
-        # Undetermined-type expressions are only rejected once Django
-        # resolves them against the query.
+        # GROUP BY: no group_by → a single constant group, so the scalar
+        # case falls out of the same path.
+        if group_targets:
+            base = queryset.values(*group_targets)
+        else:
+            base = queryset.annotate(_sb_group=Value(1)).values("_sb_group")
+
+        # One query per metric, merged by group key — a metric that
+        # traverses a to-many relation never shares a query with (and so
+        # never inflates) another. Undetermined-type expressions are only
+        # rejected once Django resolves them against the query.
+        merged: dict = {}
+        order: list = []
         try:
-            result: dict = {}
-            if batch:
-                result.update(queryset.aggregate(**batch))
-            for alias, agg in isolated:
-                result[alias] = queryset.aggregate(**{alias: agg})[alias]
+            for spec in specs:
+                alias = spec["alias"]
+                agg = LIST_AGGREGATE_FUNCTIONS[spec["fn"]](spec["target"])
+                for row in base.annotate(**{alias: agg}).order_by(*group_targets):
+                    key = tuple(row[t] for t in group_targets)
+                    slot = merged.get(key)
+                    if slot is None:
+                        slot = {t: row[t] for t in group_targets}
+                        merged[key] = slot
+                        order.append(key)
+                    slot[alias] = row[alias]
         except FieldError as exc:
             raise ValueError(f"Cannot aggregate the requested field(s): {exc}") from exc
-        return result
+
+        if not group_targets:
+            return merged[order[0]] if order else {}
+        return [merged[key] for key in order]
diff --git a/src/django_smartbase_admin/mcp/mcp.py b/src/django_smartbase_admin/mcp/mcp.py
index 58d9a3ad..00b83067 100644
--- a/src/django_smartbase_admin/mcp/mcp.py
+++ b/src/django_smartbase_admin/mcp/mcp.py
@@ -500,6 +500,7 @@ def list_rows(
         full_text_search: str | None = None,
         include_inlines: list | None = None,
         aggregate: list | None = None,
+        group_by: list | None = None,
     ) -> dict:
         """List rows for one admin — same data the UI list shows.
 
@@ -564,12 +565,23 @@ def list_rows(
             ``fn`` is one of ``sum / avg / min / max / count``; ``field``
             must be a declared column from ``list_admins`` —
             ``sum/avg/min/max`` need a numeric one, ``count`` may omit
-            ``field`` for a row count. Results land under ``aggregates``,
-            keyed ``f"{fn}_{field}"`` (or ``"count"``).
+            ``field`` for a row count. Without ``group_by`` the results
+            land under ``aggregates``, keyed ``f"{fn}_{field}"`` (or
+            ``"count"``).
+          group_by: optional list of declared column names to break the
+            ``aggregate`` totals down by (SQL ``GROUP BY``) — e.g.
+            ``group_by=["queue"]`` with ``aggregate=[{"fn": "count"}]`` for
+            tickets-per-queue in one call instead of one call per queue.
+            Multi-valued (relation) columns are rejected. Requires
+            ``aggregate``. Results land under ``groups`` as a list of
+            ``{**group_columns, **aggregate_aliases}`` rows ordered by the
+            group columns; a column grouped on a foreign key comes back as
+            the bare pk (resolve names with ``autocomplete``).
 
         Returns ``{"data": [...], "last_page": int, "last_row": int}``
-        plus any pagination metadata the list view emits, and ``aggregates``
-        when the ``aggregate`` argument is supplied.
+        plus any pagination metadata the list view emits, ``aggregates``
+        when ``aggregate`` is supplied without ``group_by``, and ``groups``
+        when ``group_by`` is supplied.
         """
         request = self.request
         ensure_sbadmin_request_data(request)
@@ -613,9 +625,13 @@ def list_rows(
 
         # Totals over the whole filtered set, validated before the page fetch.
         aggregates = None
+        if group_by and aggregate is None:
+            raise ValueError("group_by requires aggregate.")
         if aggregate is not None:
             action = admin.sbadmin_list_action_class(admin, request)
-            aggregates = action.get_aggregates(aggregate, field_map=field_map)
+            aggregates = action.get_aggregates(
+                aggregate, field_map=field_map, group_by=group_by
+            )
 
         # Route through the same action dispatch as the browser so
         # ``has_permission_for_action`` stays the single gate, then unwrap
@@ -642,7 +658,7 @@ def list_rows(
         if include_inlines:
             attach_inlines(admin, request, rows, include_inlines)
         if aggregates is not None:
-            result["aggregates"] = aggregates
+            result["groups" if group_by else "aggregates"] = aggregates
         return result
 
     @_guarded_tool_call
diff --git a/src/django_smartbase_admin/mcp/tests/test_aggregate.py b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
index 3f3e9d17..88a63eda 100644
--- a/src/django_smartbase_admin/mcp/tests/test_aggregate.py
+++ b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
@@ -23,7 +23,9 @@
 class _Admin(SBAdmin):
     model = Folder
     search_fields = ("name",)
-    sbadmin_list_display = ("id", "name", "id_alias")
+    # ``children`` is the reverse of the self-FK ``parent`` → a one-to-many
+    # (multi-valued) field, used to exercise the join fan-out path.
+    sbadmin_list_display = ("id", "name", "id_alias", "parent", "children")
 
     # A method field with ``admin_order_field`` → backed by the ``id``
     # column via an annotation, so its ORM identifier is suffixed
@@ -104,6 +106,87 @@ def test_field_name_resolves_to_orm_identifier(self):
         # raw name would raise FieldError.
         self.assertEqual(result["aggregates"], {"sum_id_alias": sum(ids)})
 
+    def test_group_by_breaks_totals_down_per_group(self):
+        # Two parents (themselves parent=None), children nested under them →
+        # group the count/sum by the parent FK so each parent surfaces once
+        # with its own totals.
+        a = Folder.objects.create(name="a")
+        b = Folder.objects.create(name="b")
+        a_ids = [Folder.objects.create(name=n, parent=a).pk for n in ("a1", "a2")]
+        b_ids = [Folder.objects.create(name=n, parent=b).pk for n in ("b1",)]
+
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            group_by=["parent"],
+            aggregate=[{"fn": "count"}, {"fn": "sum", "field": "id"}],
+        )
+
+        # No top-level scalar aggregates; a "groups" list instead, one row
+        # per parent, ordered by the group column (NULLs first in sqlite).
+        self.assertNotIn("aggregates", result)
+        self.assertEqual(
+            result["groups"],
+            [
+                {"parent": None, "count": 2, "sum_id": a.pk + b.pk},
+                {"parent": a.pk, "count": 2, "sum_id": sum(a_ids)},
+                {"parent": b.pk, "count": 1, "sum_id": sum(b_ids)},
+            ],
+        )
+
+    def test_aggregate_without_group_by_still_returns_scalar_dict(self):
+        # Back-compat: the degenerate single-constant-group path keeps the
+        # original scalar shape under "aggregates".
+        ids = [Folder.objects.create(name=n).pk for n in ("a", "b")]
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            aggregate=[{"fn": "count"}, {"fn": "sum", "field": "id"}],
+        )
+        self.assertNotIn("groups", result)
+        self.assertEqual(result["aggregates"], {"count": 2, "sum_id": sum(ids)})
+
+    def test_count_over_relation_does_not_inflate_sibling_sum(self):
+        # A parent with 3 children: counting the multi-valued ``children``
+        # relation adds a row-multiplying join. Run in one shared query, that
+        # join would triple the parent's id in ``sum_id``. Each metric runs
+        # in its own query, so the sum stays correct alongside the count.
+        p = Folder.objects.create(name="p")
+        child_ids = [
+            Folder.objects.create(name=n, parent=p).pk for n in ("c1", "c2", "c3")
+        ]
+        all_ids = [p.pk, *child_ids]
+
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            aggregate=[
+                {"fn": "count", "field": "children"},
+                {"fn": "sum", "field": "id"},
+            ],
+        )
+
+        # 3 children joined, but the sum is over the 4 real rows — not inflated
+        # by the fan-out (a shared-query bug would give 3*p.pk + child sum).
+        self.assertEqual(
+            result["aggregates"],
+            {"count_children": 3, "sum_id": sum(all_ids)},
+        )
+
+    def test_invalid_group_by_specs_are_rejected(self):
+        tools = self._tools()
+        base = dict(view_id="filer_folder", fields=["id", "name"])
+
+        # Undeclared group field.
+        with self.assertRaises(LookupError):
+            tools.list_rows(**base, aggregate=[{"fn": "count"}], group_by=["nope"])
+        # group_by without aggregate is meaningless.
+        with self.assertRaises(ValueError):
+            tools.list_rows(**base, group_by=["name"])
+        # Grouping on a multi-valued relation fans the rows out → rejected.
+        with self.assertRaises(ValueError):
+            tools.list_rows(**base, aggregate=[{"fn": "count"}], group_by=["children"])
+
     def test_invalid_aggregate_specs_are_rejected(self):
         tools = self._tools()
         base = dict(view_id="filer_folder", fields=["id", "name"])

From 17860795deb39c453d6bdd3a71cb11e9c0cc569f Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Thu, 11 Jun 2026 13:58:04 +0200
Subject: [PATCH 2/7] allow loopback url

---
 .../mcp/oauth/validators.py                   | 36 ++++++++++++++++-
 .../mcp/tests/test_oauth_redirect_loopback.py | 40 +++++++++++++++++++
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/src/django_smartbase_admin/mcp/oauth/validators.py b/src/django_smartbase_admin/mcp/oauth/validators.py
index 4a17b87b..1a8fc448 100644
--- a/src/django_smartbase_admin/mcp/oauth/validators.py
+++ b/src/django_smartbase_admin/mcp/oauth/validators.py
@@ -6,6 +6,11 @@
 host. This validator narrows http to loopback hosts (RFC 8252 §7.3); https and
 native custom schemes (``cursor://``, ``claude://``) are unaffected.
 
+It also relaxes DOT's exact-match for loopback redirects: native MCP clients
+(Claude Code, Cursor) bind an ephemeral localhost port per auth run, so the
+port registered at DCR time never matches later runs. RFC 8252 §7.3 requires
+the AS to allow any port for loopback redirect URIs.
+
 Wire it in as ``OAUTH2_PROVIDER["OAUTH2_VALIDATOR_CLASS"]``, or subclass it if
 the deployment needs further customization.
 """
@@ -25,10 +30,37 @@ def is_non_loopback_http(redirect_uri: str) -> bool:
     return parts.scheme == "http" and parts.hostname not in LOOPBACK_HOSTS
 
 
+def loopback_redirect_allowed(redirect_uri: str, allowed_uris) -> bool:
+    """RFC 8252 §7.3 — match a loopback redirect URI ignoring its port.
+
+    Loopback hosts are treated as interchangeable (a client registered with
+    ``127.0.0.1`` may come back as ``localhost``); scheme, path and query must
+    still match a registered loopback URI exactly.
+    """
+    requested = urlsplit(redirect_uri)
+    if requested.hostname not in LOOPBACK_HOSTS:
+        return False
+    for allowed in allowed_uris:
+        candidate = urlsplit(allowed)
+        if (
+            candidate.hostname in LOOPBACK_HOSTS
+            and candidate.scheme == requested.scheme
+            and candidate.path == requested.path
+            and candidate.query == requested.query
+        ):
+            return True
+    return False
+
+
 class SBAdminMCPOAuth2Validator(OAuth2Validator):
     def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs):
         if is_non_loopback_http(redirect_uri):
             return False
-        return super().validate_redirect_uri(
+        if super().validate_redirect_uri(
             client_id, redirect_uri, request, *args, **kwargs
-        )
+        ):
+            return True
+        client = getattr(request, "client", None)
+        if client is None:
+            return False
+        return loopback_redirect_allowed(redirect_uri, client.redirect_uris.split())
diff --git a/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py b/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
index be58d080..061aa954 100644
--- a/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
+++ b/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
@@ -17,6 +17,7 @@
 from django_smartbase_admin.mcp.oauth.validators import (
     SBAdminMCPOAuth2Validator,
     is_non_loopback_http,
+    loopback_redirect_allowed,
 )
 from django_smartbase_admin.mcp.oauth.views import register
 
@@ -59,6 +60,45 @@ def test_validator_rejects_non_loopback_http_only(self):
             )
             parent.assert_called_once()
 
+    def test_validator_ignores_port_for_loopback_redirects(self):
+        """RFC 8252 §7.3: native clients bind a fresh loopback port per run."""
+        registered = ["http://localhost:33418/callback"]
+        # Port differs, loopback hosts are interchangeable -> allowed.
+        self.assertTrue(
+            loopback_redirect_allowed("http://localhost:52756/callback", registered)
+        )
+        self.assertTrue(
+            loopback_redirect_allowed("http://127.0.0.1:52756/callback", registered)
+        )
+        # Scheme, path or host mismatch -> refused.
+        self.assertFalse(
+            loopback_redirect_allowed("https://localhost:52756/callback", registered)
+        )
+        self.assertFalse(
+            loopback_redirect_allowed("http://localhost:52756/other", registered)
+        )
+        self.assertFalse(
+            loopback_redirect_allowed("http://evil.example:52756/callback", registered)
+        )
+
+        # End-to-end through the validator: DOT's exact match fails on the
+        # port, the loopback fallback recovers it; no client -> no fallback.
+        validator = SBAdminMCPOAuth2Validator()
+        request = SimpleNamespace(
+            client=SimpleNamespace(redirect_uris="http://localhost:33418/callback")
+        )
+        with patch.object(OAuth2Validator, "validate_redirect_uri", return_value=False):
+            self.assertTrue(
+                validator.validate_redirect_uri(
+                    "c", "http://localhost:52756/callback", request
+                )
+            )
+            self.assertFalse(
+                validator.validate_redirect_uri(
+                    "c", "http://localhost:52756/callback", SimpleNamespace()
+                )
+            )
+
     def test_dcr_register_enforces_loopback_for_http(self):
         rf = RequestFactory()
 

From 8599167617b890b7444e86717db8ff03ab142a7d Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Thu, 11 Jun 2026 16:59:59 +0200
Subject: [PATCH 3/7] add test

---
 .../mcp/tests/test_aggregate.py                       | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/django_smartbase_admin/mcp/tests/test_aggregate.py b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
index 88a63eda..ec721dbe 100644
--- a/src/django_smartbase_admin/mcp/tests/test_aggregate.py
+++ b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
@@ -146,6 +146,17 @@ def test_aggregate_without_group_by_still_returns_scalar_dict(self):
         self.assertNotIn("groups", result)
         self.assertEqual(result["aggregates"], {"count": 2, "sum_id": sum(ids)})
 
+        # An empty filtered set still yields every requested alias (Django
+        # drops the constant group from GROUP BY, so the rollup query always
+        # returns one row) — not an empty dict.
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            full_text_search="no-match",
+            aggregate=[{"fn": "count"}, {"fn": "sum", "field": "id"}],
+        )
+        self.assertEqual(result["aggregates"], {"count": 0, "sum_id": None})
+
     def test_count_over_relation_does_not_inflate_sibling_sum(self):
         # A parent with 3 children: counting the multi-valued ``children``
         # relation adds a row-multiplying join. Run in one shared query, that

From f065e81fefd999a1d7330535aadb3a7efabd8440 Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Fri, 19 Jun 2026 14:31:07 +0200
Subject: [PATCH 4/7] fixes

---
 .../actions/admin_action_list.py              | 14 ++++++++++++-
 .../mcp/oauth/validators.py                   | 19 +++++++++++------
 .../mcp/tests/test_aggregate.py               | 21 +++++++++++++++++++
 .../mcp/tests/test_oauth_redirect_loopback.py | 17 +++++++++++++++
 4 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/src/django_smartbase_admin/actions/admin_action_list.py b/src/django_smartbase_admin/actions/admin_action_list.py
index ee2b9eaf..1ed1cec6 100644
--- a/src/django_smartbase_admin/actions/admin_action_list.py
+++ b/src/django_smartbase_admin/actions/admin_action_list.py
@@ -864,11 +864,20 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
 
         # Resolve group targets the same way; a computed (annotated) group
         # column needs its expression present before ``.values()``.
+        # Keep the caller-facing group key (the declared name they grouped by)
+        # separate from the ORM target. They diverge when a declared field's
+        # public name differs from its DB column — e.g. an ``id_alias`` method
+        # field backed by ``@admin.display(ordering="id")`` resolves to ``id``.
+        # ``group_targets`` drives ``.values()`` / ``order_by`` / dedup; the
+        # response rows are keyed by ``group_keys`` so ``group_by=["id_alias"]``
+        # comes back under ``id_alias``, not ``id``.
+        group_keys: list[str] = []
         group_targets: list[str] = []
         for field in group_fields:
             target = (
                 field.model_field.name if field.model_field is not None else field.field
             )
+            group_keys.append(field.name)
             group_targets.append(target)
             if (
                 field.model_field is None
@@ -909,7 +918,10 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
                     key = tuple(row[t] for t in group_targets)
                     slot = merged.get(key)
                     if slot is None:
-                        slot = {t: row[t] for t in group_targets}
+                        slot = {
+                            key_name: row[target]
+                            for key_name, target in zip(group_keys, group_targets)
+                        }
                         merged[key] = slot
                         order.append(key)
                     slot[alias] = row[alias]
diff --git a/src/django_smartbase_admin/mcp/oauth/validators.py b/src/django_smartbase_admin/mcp/oauth/validators.py
index 1a8fc448..96a508a5 100644
--- a/src/django_smartbase_admin/mcp/oauth/validators.py
+++ b/src/django_smartbase_admin/mcp/oauth/validators.py
@@ -31,20 +31,27 @@ def is_non_loopback_http(redirect_uri: str) -> bool:
 
 
 def loopback_redirect_allowed(redirect_uri: str, allowed_uris) -> bool:
-    """RFC 8252 §7.3 — match a loopback redirect URI ignoring its port.
+    """RFC 8252 §7.3 — match a loopback ``http`` redirect URI ignoring its port.
 
     Loopback hosts are treated as interchangeable (a client registered with
-    ``127.0.0.1`` may come back as ``localhost``); scheme, path and query must
-    still match a registered loopback URI exactly.
+    ``127.0.0.1`` may come back as ``localhost``); path and query must still
+    match a registered loopback URI exactly.
+
+    Port relaxation is confined to ``http`` — the only RFC 8252 §7.3 case and
+    the only scheme this validator deliberately permits for loopback hosts.
+    Custom/native schemes (``cursor://``, ``claude://``) stay on DOT's exact
+    match, which enforces ``ALLOWED_REDIRECT_URI_SCHEMES``; without this gate a
+    dynamically registered URI like ``javascript://localhost:1/callback`` could
+    slip past the scheme allowlist on a mere port mismatch.
     """
     requested = urlsplit(redirect_uri)
-    if requested.hostname not in LOOPBACK_HOSTS:
+    if requested.scheme != "http" or requested.hostname not in LOOPBACK_HOSTS:
         return False
     for allowed in allowed_uris:
         candidate = urlsplit(allowed)
         if (
-            candidate.hostname in LOOPBACK_HOSTS
-            and candidate.scheme == requested.scheme
+            candidate.scheme == "http"
+            and candidate.hostname in LOOPBACK_HOSTS
             and candidate.path == requested.path
             and candidate.query == requested.query
         ):
diff --git a/src/django_smartbase_admin/mcp/tests/test_aggregate.py b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
index ec721dbe..7dffd2f7 100644
--- a/src/django_smartbase_admin/mcp/tests/test_aggregate.py
+++ b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
@@ -134,6 +134,27 @@ def test_group_by_breaks_totals_down_per_group(self):
             ],
         )
 
+    def test_group_by_keys_rows_by_requested_name_not_orm_target(self):
+        # ``id_alias`` is a method field whose ORM identifier is the annotation
+        # alias ``id_alias_annt``. Grouping by it must still surface rows under
+        # the agent-facing name ``id_alias`` (what the caller asked for), not
+        # the internal target — otherwise the group key the caller reads back
+        # doesn't match the field they grouped on.
+        ids = sorted(Folder.objects.create(name=n).pk for n in ("a", "b", "c"))
+
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            group_by=["id_alias"],
+            aggregate=[{"fn": "count"}],
+        )
+
+        self.assertNotIn("aggregates", result)
+        self.assertEqual(
+            result["groups"],
+            [{"id_alias": pk, "count": 1} for pk in ids],
+        )
+
     def test_aggregate_without_group_by_still_returns_scalar_dict(self):
         # Back-compat: the degenerate single-constant-group path keeps the
         # original scalar shape under "aggregates".
diff --git a/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py b/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
index 061aa954..9e654044 100644
--- a/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
+++ b/src/django_smartbase_admin/mcp/tests/test_oauth_redirect_loopback.py
@@ -81,6 +81,23 @@ def test_validator_ignores_port_for_loopback_redirects(self):
             loopback_redirect_allowed("http://evil.example:52756/callback", registered)
         )
 
+    def test_loopback_fallback_is_http_only(self):
+        """Port relaxation must not smuggle a non-http scheme past the allowlist.
+
+        DOT rejects a scheme outside ``ALLOWED_REDIRECT_URI_SCHEMES``; the
+        loopback fallback must not re-admit it just because a registered URI
+        shares the (non-http) scheme, loopback host, path and query but a
+        different port.
+        """
+        registered = ["javascript://localhost:1/callback"]
+        self.assertFalse(
+            loopback_redirect_allowed("javascript://localhost:9/callback", registered)
+        )
+        # Even an exact non-http loopback match goes through DOT, never here.
+        self.assertFalse(
+            loopback_redirect_allowed("javascript://localhost:1/callback", registered)
+        )
+
         # End-to-end through the validator: DOT's exact match fails on the
         # port, the loopback fallback recovers it; no client -> no fallback.
         validator = SBAdminMCPOAuth2Validator()

From 5ad0f5720d959c4a1a1c5ef64ea1f2f6f2ac70be Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Fri, 19 Jun 2026 15:24:16 +0200
Subject: [PATCH 5/7] fixes

---
 .../actions/admin_action_list.py              | 32 ++++++++-----
 src/django_smartbase_admin/mcp/mcp.py         |  9 ++--
 .../mcp/tests/test_aggregate.py               | 46 +++++++++++++++++--
 3 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/src/django_smartbase_admin/actions/admin_action_list.py b/src/django_smartbase_admin/actions/admin_action_list.py
index 1ed1cec6..63bda1a4 100644
--- a/src/django_smartbase_admin/actions/admin_action_list.py
+++ b/src/django_smartbase_admin/actions/admin_action_list.py
@@ -832,11 +832,16 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
         degenerate case of the same query.
 
         Without ``group_by`` returns a scalar ``{alias: value}`` dict; with
-        ``group_by`` returns a list of ``{**group_fields, **aliases}`` rows,
-        ordered by the group columns. Each metric runs in its own query and
-        is merged by group key, so a metric that traverses a to-many
-        relation (a multi-valued ``count``, or an annotated field that
-        aggregates a related set) can never fan-out-inflate a sibling.
+        ``group_by`` returns a list of
+        ``{"group": {name: value, ...}, "aggregates": {alias: value, ...}}``
+        rows, ordered by the group columns. The two namespaces are nested
+        rather than merged into one flat dict so a group column whose public
+        name equals a derived aggregate alias (e.g. a ``count`` field grouped
+        on while also aggregating ``count``) can't clobber the other — they
+        live under separate keys. Each metric runs in its own query and is
+        merged by group key, so a metric that traverses a to-many relation (a
+        multi-valued ``count``, or an annotated field that aggregates a related
+        set) can never fan-out-inflate a sibling.
         """
         specs = self.validate_aggregate(aggregate, field_map=field_map)
         group_fields = self.validate_group_by(group_by, field_map=field_map)
@@ -869,8 +874,8 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
         # public name differs from its DB column — e.g. an ``id_alias`` method
         # field backed by ``@admin.display(ordering="id")`` resolves to ``id``.
         # ``group_targets`` drives ``.values()`` / ``order_by`` / dedup; the
-        # response rows are keyed by ``group_keys`` so ``group_by=["id_alias"]``
-        # comes back under ``id_alias``, not ``id``.
+        # response ``group`` dict is keyed by ``group_keys`` so
+        # ``group_by=["id_alias"]`` comes back under ``id_alias``, not ``id``.
         group_keys: list[str] = []
         group_targets: list[str] = []
         for field in group_fields:
@@ -887,6 +892,7 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
                 if field.supporting_annotates:
                     extra.update(field.supporting_annotates)
                 extra[target] = field.annotate
+
         if extra:
             queryset = queryset.annotate(**extra)
 
@@ -919,15 +925,19 @@ def get_aggregates(self, aggregate, field_map=None, group_by=None):
                     slot = merged.get(key)
                     if slot is None:
                         slot = {
-                            key_name: row[target]
-                            for key_name, target in zip(group_keys, group_targets)
+                            "group": {
+                                key_name: row[target]
+                                for key_name, target in zip(group_keys, group_targets)
+                            },
+                            "aggregates": {},
                         }
                         merged[key] = slot
                         order.append(key)
-                    slot[alias] = row[alias]
+                    slot["aggregates"][alias] = row[alias]
         except FieldError as exc:
             raise ValueError(f"Cannot aggregate the requested field(s): {exc}") from exc
 
+        # No group dimension → unwrap to the bare scalar aggregate dict.
         if not group_targets:
-            return merged[order[0]] if order else {}
+            return merged[order[0]]["aggregates"] if order else {}
         return [merged[key] for key in order]
diff --git a/src/django_smartbase_admin/mcp/mcp.py b/src/django_smartbase_admin/mcp/mcp.py
index 00b83067..487456c5 100644
--- a/src/django_smartbase_admin/mcp/mcp.py
+++ b/src/django_smartbase_admin/mcp/mcp.py
@@ -574,9 +574,12 @@ def list_rows(
             tickets-per-queue in one call instead of one call per queue.
             Multi-valued (relation) columns are rejected. Requires
             ``aggregate``. Results land under ``groups`` as a list of
-            ``{**group_columns, **aggregate_aliases}`` rows ordered by the
-            group columns; a column grouped on a foreign key comes back as
-            the bare pk (resolve names with ``autocomplete``).
+            ``{"group": {column: value, ...}, "aggregates": {alias: value,
+            ...}}`` rows ordered by the group columns — group columns and
+            aggregate aliases are nested under separate keys so a column whose
+            name equals an aggregate alias can't collide. A column grouped on a
+            foreign key comes back as the bare pk (resolve names with
+            ``autocomplete``).
 
         Returns ``{"data": [...], "last_page": int, "last_row": int}``
         plus any pagination metadata the list view emits, ``aggregates``
diff --git a/src/django_smartbase_admin/mcp/tests/test_aggregate.py b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
index 7dffd2f7..3116b332 100644
--- a/src/django_smartbase_admin/mcp/tests/test_aggregate.py
+++ b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
@@ -25,7 +25,7 @@ class _Admin(SBAdmin):
     search_fields = ("name",)
     # ``children`` is the reverse of the self-FK ``parent`` → a one-to-many
     # (multi-valued) field, used to exercise the join fan-out path.
-    sbadmin_list_display = ("id", "name", "id_alias", "parent", "children")
+    sbadmin_list_display = ("id", "name", "id_alias", "count", "parent", "children")
 
     # A method field with ``admin_order_field`` → backed by the ``id``
     # column via an annotation, so its ORM identifier is suffixed
@@ -34,6 +34,12 @@ class _Admin(SBAdmin):
     def id_alias(self, obj):
         return obj.id
 
+    # A method field whose public name collides with the derived ``count``
+    # aggregate alias — used to exercise the group-key/alias collision guard.
+    @admin.display(ordering="id")
+    def count(self, obj):
+        return obj.id
+
 
 @override_settings(
     ROOT_URLCONF=__name__,
@@ -124,13 +130,23 @@ def test_group_by_breaks_totals_down_per_group(self):
 
         # No top-level scalar aggregates; a "groups" list instead, one row
         # per parent, ordered by the group column (NULLs first in sqlite).
+        # Group columns and aggregates are nested under separate keys.
         self.assertNotIn("aggregates", result)
         self.assertEqual(
             result["groups"],
             [
-                {"parent": None, "count": 2, "sum_id": a.pk + b.pk},
-                {"parent": a.pk, "count": 2, "sum_id": sum(a_ids)},
-                {"parent": b.pk, "count": 1, "sum_id": sum(b_ids)},
+                {
+                    "group": {"parent": None},
+                    "aggregates": {"count": 2, "sum_id": a.pk + b.pk},
+                },
+                {
+                    "group": {"parent": a.pk},
+                    "aggregates": {"count": 2, "sum_id": sum(a_ids)},
+                },
+                {
+                    "group": {"parent": b.pk},
+                    "aggregates": {"count": 1, "sum_id": sum(b_ids)},
+                },
             ],
         )
 
@@ -152,7 +168,27 @@ def test_group_by_keys_rows_by_requested_name_not_orm_target(self):
         self.assertNotIn("aggregates", result)
         self.assertEqual(
             result["groups"],
-            [{"id_alias": pk, "count": 1} for pk in ids],
+            [{"group": {"id_alias": pk}, "aggregates": {"count": 1}} for pk in ids],
+        )
+
+    def test_group_key_equal_to_aggregate_alias_does_not_collide(self):
+        # ``count`` is a method field whose public name equals the derived
+        # ``count`` aggregate alias. Nesting the group column and the aggregate
+        # under separate keys means they no longer share a dict, so both
+        # survive — the group value under ``group.count`` and the metric under
+        # ``aggregates.count``.
+        ids = sorted(Folder.objects.create(name=n).pk for n in ("a", "b", "c"))
+
+        result = self._tools().list_rows(
+            "filer_folder",
+            fields=["id", "name"],
+            group_by=["count"],
+            aggregate=[{"fn": "count"}],
+        )
+
+        self.assertEqual(
+            result["groups"],
+            [{"group": {"count": pk}, "aggregates": {"count": 1}} for pk in ids],
         )
 
     def test_aggregate_without_group_by_still_returns_scalar_dict(self):

From 7d5ace636b5d4f8f63a7e0dfee7307525ff2cd7c Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Fri, 19 Jun 2026 15:25:29 +0200
Subject: [PATCH 6/7] fixes

---
 src/django_smartbase_admin/mcp/tests/test_aggregate.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/django_smartbase_admin/mcp/tests/test_aggregate.py b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
index 3116b332..21773471 100644
--- a/src/django_smartbase_admin/mcp/tests/test_aggregate.py
+++ b/src/django_smartbase_admin/mcp/tests/test_aggregate.py
@@ -34,8 +34,9 @@ class _Admin(SBAdmin):
     def id_alias(self, obj):
         return obj.id
 
-    # A method field whose public name collides with the derived ``count``
-    # aggregate alias — used to exercise the group-key/alias collision guard.
+    # A method field whose public name equals the derived ``count`` aggregate
+    # alias — used to exercise that nesting keeps a group column and an
+    # equally-named aggregate from colliding.
     @admin.display(ordering="id")
     def count(self, obj):
         return obj.id

From c042ec95e7533e611a82cbc0ba407f71d6cdf2f7 Mon Sep 17 00:00:00 2001
From: oko-vac <oko-vac@users.noreply.github.com>
Date: Fri, 19 Jun 2026 15:28:23 +0200
Subject: [PATCH 7/7] fixes

---
 src/django_smartbase_admin/mcp/mcp.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/django_smartbase_admin/mcp/mcp.py b/src/django_smartbase_admin/mcp/mcp.py
index 487456c5..0f414d7e 100644
--- a/src/django_smartbase_admin/mcp/mcp.py
+++ b/src/django_smartbase_admin/mcp/mcp.py
@@ -565,9 +565,10 @@ def list_rows(
             ``fn`` is one of ``sum / avg / min / max / count``; ``field``
             must be a declared column from ``list_admins`` —
             ``sum/avg/min/max`` need a numeric one, ``count`` may omit
-            ``field`` for a row count. Without ``group_by`` the results
-            land under ``aggregates``, keyed ``f"{fn}_{field}"`` (or
-            ``"count"``).
+            ``field`` for a row count. Each result is keyed by a derived
+            alias — ``f"{fn}_{field}"`` (or ``"count"`` for a bare count);
+            aliases can't be overridden. Without ``group_by`` the results
+            land under ``aggregates`` as a flat ``{alias: value}`` dict.
           group_by: optional list of declared column names to break the
             ``aggregate`` totals down by (SQL ``GROUP BY``) — e.g.
             ``group_by=["queue"]`` with ``aggregate=[{"fn": "count"}]`` for