fix(email): wire EmailSendRequest.provider fallback + exact pre-scan budget (#1603 review) (#1616)

Follow-up to #1614 (merged) — applies the claude.yml review suggestions
plus the **root-cause** enumeration fix that #1614 only worked around.

## Why this matters

#1614 shipped connector-derived mailbox selection, but two gaps
remained:

1. **`store.list_connections()` was hardcoded to `("google",)`** — so
*every generic* connection consumer stayed Microsoft-blind:
`api.list_connections()`, `api.get_connection("microsoft")` (returned
`None` even when Microsoft was connected), the `/v1/connections` REST
endpoint, the UI connectors list, and `tripwire_check()` (never swept
the Microsoft connection at startup). #1614's
`connected_mailbox_providers()` patched only the email agent's path.
This makes the primitive registry-driven (enumerate the registered
`oauth_pkce` providers via a function-local import — no layering
regression), so the whole system is consistent.
2. **`EmailSendRequest.provider` was a public REST field that did
nothing** — with two mailboxes connected and an unbound confirmation
token, a caller supplying `provider` to disambiguate still got a 400.
Now wired as the unbound-token fallback (the token's own binding still
wins when present).

Plus a latent-correctness fix to the pre-scan budget guard (counts
actual returned messages, not the per-backend cap).

## Test plan

- [x] `GAIA_MEMORY_DISABLED=1 python -m pytest tests/unit/connectors/
tests/unit/agents/email/ -x` — green; `util/lint.py --all` clean
- [x] New tests: `store.list_connections()` finds microsoft-only / both;
`api.list_connections()` + `get_connection("microsoft")` see Microsoft
(no secret leak); `TestSendRequestProviderFallback` (unbound→routes
there, bound-token wins, unbound+none+both→400); pre-scan budget counts
actual returned
- [x] **Real-world (Strix Halo, same persisted real Gmail + Outlook
connections), before/after:**

| Surface | BEFORE (`f768c2e`, merged #1614) | AFTER (`e694f00`, this
PR) |
  |---|---|---|
| `store.list_connections()` | `['google']` | `['google','microsoft']` |
  | `api.list_connections()` | `['google']` | `['google','microsoft']` |
| `api.get_connection('microsoft')` | `None` | full metadata (no
secrets) |
  | `GET /v1/connections` | `['google']` | `['google','microsoft']` |

Microsoft was connected the whole time — only the enumeration primitive
changed.

Deferred from the review (own follow-up): surfacing the chosen `from:`
mailbox in the `send_now` confirmation prompt — needs a base-agent
confirmation hook touching all agents.

Author: itomek

hub/agents/python/email/gaia_agent_email/agent.py

@@ -518,7 +518,15 @@ def _pre_scan_all_backends(self, *, max_messages: int) -> dict:
                 force_llm=force_llm,
                 debug=debug_flag,
             )
-            scanned += per_backend
+            # Count messages actually returned, not the cap — an under-filled
+            # backend would otherwise trip the budget guard and skip a later one.
+            backend_totals = out.get("totals", {})
+            scanned += (
+                int(backend_totals.get("urgent", 0))
+                + int(backend_totals.get("actionable", 0))
+                + int(backend_totals.get("suggested_archives", 0))
+                + int(out.get("informational_count", 0))
+            )
             merged_prefs_applied = out.get("preferences_applied", merged_prefs_applied)
             for item in out.get("urgent", []):
                 item["mailbox"] = provider

hub/agents/python/email/gaia_agent_email/api_routes.py

@@ -680,9 +680,11 @@ class EmailSendRequest(_Strict):
     provider: Optional[str] = Field(
         default=None,
         description=(
-            "Optional provider override ('google' or 'microsoft'). Ignored "
-            "when the confirmation token carries a provider binding — the "
-            "token's bound provider always wins."
+            "Optional provider ('google' or 'microsoft'), used ONLY as the "
+            "fallback when the confirmation token carries no provider binding. "
+            "A token's bound provider always wins; with two mailboxes connected "
+            "and neither a binding nor this field set, the send is rejected as "
+            "ambiguous (400)."
         ),
     )
 
@@ -767,9 +769,10 @@ async def send_email(request: EmailSendRequest) -> EmailSendResponse:
     # send takes a single 'to' header string. Run off the event loop so
     # get_access_token_sync (used inside the token resolvers) does not hit
     # the "called from a thread with a running event loop" guard (#1594).
-    # The token's bound provider (D5) overrides the count-based D2 logic;
-    # if no binding, fall back to the single-mailbox resolver.
-    backend = _resolve_backend_for_provider(bound_provider)
+    # Provider precedence: the token's bound provider (D5) always wins; only an
+    # unbound token falls back to request.provider; with neither, the
+    # count-based resolver decides (and 400s when 2+ are connected).
+    backend = _resolve_backend_for_provider(bound_provider or request.provider)
     to_header = ", ".join(_format_address(a) for a in request.to)
     result = await asyncio.to_thread(
         backend.send_message, to=to_header, subject=request.subject, body=request.body

src/gaia/connectors/store.py

@@ -367,10 +367,21 @@ def list_connections() -> List[str]:
     Best-effort enumeration of stored providers.
 
     The ``keyring`` API does not expose a portable "list all entries for
-    service" call. v1 returns the providers we know about (currently
-    just ``google``); future providers extend this.
+    service" call, so we probe each registered OAuth provider for a stored
+    connection blob. The provider set is registry-driven (every ``oauth_pkce``
+    spec), so a newly added provider is enumerated without editing this
+    function — and generic consumers (``api.list_connections`` /
+    ``get_connection`` / the REST endpoint / ``tripwire_check``) see every
+    connected mailbox, not just Google.
     """
-    known = ("google",)
+    # Function-local import: keep this module free of a module-level registry
+    # dependency (mirrors ``api._require_mcp_server_for_activation``). Importing
+    # the catalog registers the built-in specs; the module cache makes repeats a
+    # no-op.
+    import gaia.connectors.catalog  # noqa: F401  # pylint: disable=unused-import
+    from gaia.connectors.registry import REGISTRY
+
+    known = tuple(spec.id for spec in REGISTRY.all() if spec.type == "oauth_pkce")
     found: list[str] = []
     for provider in known:
         username = _connection_username(provider, DEFAULT_ACCOUNT)

tests/unit/agents/email/test_draft_token_provider_binding.py

@@ -255,3 +255,125 @@ def test_confirmation_store_none_provider_on_unbound_token(self):
         ok, bound_provider = store.consume_with_provider(token, fp)
         assert ok is True
         assert bound_provider is None
+
+
+class TestSendRequestProviderFallback:
+    """``EmailSendRequest.provider`` is the unbound-token fallback.
+
+    Precedence: the token's bound provider always wins; ``request.provider`` is
+    consulted ONLY when the token carries no binding. Both None + multiple
+    connected → still 400 ambiguous.
+    """
+
+    def _draft_token(self, client, *, provider=None):
+        body = {
+            "to": [{"email": "bob@example.com"}],
+            "subject": "Hello",
+            "body": "World",
+        }
+        if provider is not None:
+            body["provider"] = provider
+        resp = client.post("/v1/email/draft", json=body)
+        assert resp.status_code == 200, resp.text
+        return resp.json()["confirmation_token"]
+
+    def _spy_resolver(self, monkeypatch, calls):
+        """Patch _resolve_backend_for_provider to record the provider it sees."""
+
+        class _Fake:
+            def __init__(self, name):
+                self.name = name
+
+            def send_message(self, *, to, subject, body, **_kw):
+                calls.append(self.name)
+                if self.name == "microsoft":
+                    return {"id": "", "sent": True, "to": to, "subject": subject}
+                return {"id": f"{self.name}-sent", "to": to, "subject": subject}
+
+        def _resolve(provider=None):
+            # Mirror production: None + 2 connected → 400 ambiguous.
+            if provider is None:
+                from fastapi import HTTPException
+
+                connected = email_routes.connected_mailbox_providers()
+                if len(connected) != 1:
+                    raise HTTPException(400, f"ambiguous: {connected}")
+                provider = connected[0]
+            return _Fake(provider)
+
+        monkeypatch.setattr(email_routes, "_resolve_backend_for_provider", _resolve)
+        return calls
+
+    def test_unbound_token_uses_request_provider(self, monkeypatch):
+        """Unbound token + request.provider='microsoft' + both connected → Outlook."""
+        if app is None:
+            pytest.skip("gaia.api.openai_server not available")
+        monkeypatch.setattr(
+            email_routes,
+            "connected_mailbox_providers",
+            lambda: ["google", "microsoft"],
+        )
+        calls = self._spy_resolver(monkeypatch, [])
+        client = TestClient(app)
+        token = self._draft_token(client)  # no binding
+        resp = client.post(
+            "/v1/email/send",
+            json={
+                "to": [{"email": "bob@example.com"}],
+                "subject": "Hello",
+                "body": "World",
+                "confirmation_token": token,
+                "provider": "microsoft",
+            },
+        )
+        assert resp.status_code == 200, resp.text
+        assert calls == ["microsoft"], f"request.provider ignored: {calls}"
+
+    def test_token_binding_wins_over_request_provider(self, monkeypatch):
+        """Token bound to google + request.provider='microsoft' → google wins."""
+        if app is None:
+            pytest.skip("gaia.api.openai_server not available")
+        monkeypatch.setattr(
+            email_routes,
+            "connected_mailbox_providers",
+            lambda: ["google", "microsoft"],
+        )
+        calls = self._spy_resolver(monkeypatch, [])
+        client = TestClient(app)
+        token = self._draft_token(client, provider="google")  # bound to google
+        resp = client.post(
+            "/v1/email/send",
+            json={
+                "to": [{"email": "bob@example.com"}],
+                "subject": "Hello",
+                "body": "World",
+                "confirmation_token": token,
+                "provider": "microsoft",  # must be ignored
+            },
+        )
+        assert resp.status_code == 200, resp.text
+        assert calls == ["google"], f"token binding did not win: {calls}"
+
+    def test_unbound_token_no_provider_both_connected_still_400(self, monkeypatch):
+        """Unbound token + no request.provider + both connected → 400 ambiguous."""
+        if app is None:
+            pytest.skip("gaia.api.openai_server not available")
+        monkeypatch.setattr(
+            email_routes,
+            "connected_mailbox_providers",
+            lambda: ["google", "microsoft"],
+        )
+        calls = self._spy_resolver(monkeypatch, [])
+        client = TestClient(app)
+        token = self._draft_token(client)  # no binding, no provider on send
+        resp = client.post(
+            "/v1/email/send",
+            json={
+                "to": [{"email": "bob@example.com"}],
+                "subject": "Hello",
+                "body": "World",
+                "confirmation_token": token,
+            },
+        )
+        assert resp.status_code == 400, resp.text
+        assert calls == [], f"a send leaked through the ambiguity guard: {calls}"

tests/unit/agents/email/test_multi_inbox_triage.py

@@ -241,6 +241,33 @@ def test_pre_scan_tags_every_section_item(self, tmp_path, monkeypatch):
         finally:
             agent.close_db()
 
+    def test_pre_scan_under_budget_backend_not_skipped_when_earlier_backend_underfills(
+        self, tmp_path, monkeypatch
+    ):
+        # The budget guard must count messages ACTUALLY returned, not the
+        # per-backend cap. Fails-first scenario (max_messages < n_backends):
+        #   - google inbox EMPTY, microsoft has 1 message
+        #   - pre_scan(max_messages=1) → per_backend = max(1, 1//2) = 1
+        #   - OLD `scanned += per_backend`: empty google bumps scanned to 1 →
+        #     guard `scanned >= 1` trips → microsoft is SKIPPED (the bug)
+        #   - NEW `scanned += actual`: empty google returns 0 → scanned stays 0
+        #     → microsoft is scanned and contributes.
+        # google is first in registry order, so it is the under-filling backend.
+        agent, _, _ = _agent_two_backends(
+            tmp_path, monkeypatch, google_ids=[], microsoft_ids=["m1"]
+        )
+        try:
+            envelope = json.loads(_registered_tool("pre_scan_inbox")(1))
+            data = envelope["data"]
+            items = data["urgent"] + data["actionable"] + data["suggested_archives"]
+            mailboxes = {it["mailbox"] for it in items}
+            # microsoft was reached despite the empty google scanned first.
+            assert "microsoft" in mailboxes, mailboxes
+            # And its message was tagged for downstream routing.
+            assert agent._message_mailbox.get("m1") == "microsoft"
+        finally:
+            agent.close_db()
+
 
 # ---------------------------------------------------------------------------
 # D4 — per-message backend routing

tests/unit/connectors/test_api.py

@@ -26,6 +26,7 @@
 from gaia.connectors import (
     AuthRequiredError,
     get_access_token,
+    get_connection,
     grant_agent,
     list_agent_grants,
     list_connections,
@@ -174,3 +175,48 @@ def test_list_connections_via_public_api(self, seeded):
     def test_revoke_connection_via_public_api(self, seeded):
         revoke_connection("google")
         assert list_connections() == []
+
+    def test_microsoft_connection_visible_to_generic_api(self, monkeypatch, tmp_path):
+        # Root-cause fix (#1603): a stored Microsoft connection with no google
+        # must be seen by the GENERIC api surface — list_connections() includes
+        # it and get_connection("microsoft") returns metadata (not None). The
+        # old store.list_connections() hardcoded ("google",), so every generic
+        # consumer was Microsoft-blind.
+        monkeypatch.setenv("GAIA_MICROSOFT_CLIENT_ID", "test-ms-client")
+        monkeypatch.setattr("gaia.connectors.grants.Path.home", lambda: tmp_path)
+        _registry.clear()
+        from gaia.connectors.providers import get as get_provider
+
+        ms = get_provider("microsoft")
+        save_connection(
+            provider="microsoft",
+            account_email="user@outlook.com",
+            refresh_token="ms-rt",
+            scopes=["Mail.ReadWrite"],
+            client_id_hash=ms.client_id_hash,
+        )
+
+        rows = list_connections()
+        providers = {row["provider"] for row in rows}
+        assert "microsoft" in providers
+
+        conn = get_connection("microsoft")
+        assert conn is not None, "get_connection('microsoft') must not be None"
+        assert conn["account_email"] == "user@outlook.com"
+        assert "refresh_token" not in conn
+
+    def test_both_providers_visible_to_generic_api(self, monkeypatch, tmp_path, seeded):
+        # google seeded by the fixture; add microsoft and assert both surface.
+        monkeypatch.setenv("GAIA_MICROSOFT_CLIENT_ID", "test-ms-client")
+        from gaia.connectors.providers import get as get_provider
+
+        ms = get_provider("microsoft")
+        save_connection(
+            provider="microsoft",
+            account_email="user@outlook.com",
+            refresh_token="ms-rt",
+            scopes=["Mail.ReadWrite"],
+            client_id_hash=ms.client_id_hash,
+        )
+        providers = {row["provider"] for row in list_connections()}
+        assert {"google", "microsoft"} <= providers

tests/unit/connectors/test_store.py

@@ -90,6 +90,41 @@ def test_list_connections(self):
         ids = list_connections()
         assert "google" in ids
 
+    def test_list_connections_finds_microsoft_only(self):
+        # Registry-driven enumeration (#1603): a stored Microsoft connection
+        # with NO google must surface — the old hardcoded ("google",) tuple
+        # made every generic consumer Microsoft-blind.
+        save_connection(
+            provider="microsoft",
+            account_email="m@example.com",
+            refresh_token=SENTINEL_REFRESH_TOKEN,
+            scopes=["s1"],
+            client_id_hash="h",
+        )
+        ids = list_connections()
+        assert ids == ["microsoft"]
+
+    def test_list_connections_finds_both_providers(self):
+        save_connection(
+            provider="google",
+            account_email="g@example.com",
+            refresh_token=SENTINEL_REFRESH_TOKEN,
+            scopes=["s1"],
+            client_id_hash="h",
+        )
+        save_connection(
+            provider="microsoft",
+            account_email="m@example.com",
+            refresh_token=SENTINEL_REFRESH_TOKEN,
+            scopes=["s1"],
+            client_id_hash="h",
+        )
+        ids = list_connections()
+        assert set(ids) == {"google", "microsoft"}
+        # Only oauth_pkce providers are enumerated — MCP-server connectors have
+        # no keyring connection and must not appear.
+        assert "mcp-git" not in ids
+
 
 class TestSingleBlobAtomicity:
     def test_one_keyring_slot_per_connection(self):