refactor(agents): migrate email to hub (#1102)#1520
Conversation
SummaryClean, well-executed migration that moves The one thing to fix before merge: six test files still import Issues🟡 Important — broken
|
|
🟡
from gaia.api import email_routes # ← ImportError: module no longer exists
monkeypatch.setattr(email_routes, "resolve_send_backend", lambda: backend)Because the CI job installs Suggested fix for all four sites: The monkeypatch call ( |
8520039 to
b11299e
Compare
|
🟡
from gaia.api import email_routes
monkeypatch.setattr(email_routes, "resolve_send_backend", lambda: backend)
The alias Apply that substitution to all four call sites (lines 205, 223, 253, 366). |
Brings the migration up to date with main (31 commits). Resolves the two structural conflicts: agent.py keeps the gaia_agent_email tool-mixin imports and picks up main's PhishingToolsMixin; main's new phishing_tools.py is relocated into the hub package.
Make the migrated email agent pass CI against current main: - Guard tests/unit/test_synthetic_mbox.py with importorskip(gaia_agent_email) so the generic unit-test job (no hub install) stops aborting collection. - Rewrite main's phishing tests to the gaia_agent_email namespace + guard. - Drop duplicate 'import pytest' (F811) in 10 migrated email tests. - Registry creatability test uses reg.discover() — email is an installed wheel agent, registered via entry point, not a built-in. - Align the email agent identity to installed:email everywhere (scopes.py, the AGENT_NOT_GRANTED message map, connectors docs, and the connectors/ email tests). The registry enforces installed:<id> for wheel agents, so a stale builtin:email left the forwarded-grant scope-shortfall check unable to resolve the agent's required scopes (returned 201 instead of 403).
|
🔴 Fix — swap the import in each affected location: That one-liner replacement makes all downstream attribute accesses ( |
test_scope_shortfall_fails_loudly relied on the email agent being present in the live registry via create_app()'s discover() — but that only registers the agent when the gaia-agent-email wheel is pip-installed (entry-point metadata). The connector test jobs install gaia core but not the wheel, so discover() found no email agent, the router resolved no required scopes, and the forward returned 201 instead of 403. Inject the email agent's registry entry directly (the pattern TestRouterDrivenScopeResolution already uses) so the router's scope-shortfall check is deterministic and independent of which agent wheels are installed in the test env.
|
🔴
# line 205, 223, 253, 366 — all broken
from gaia.api import email_routes # module no longer exists
monkeypatch.setattr(email_routes, "resolve_send_backend", ...)The Apply that substitution at lines 205, 223, 253, and 366. |
Address review feedback on the #1102 email→hub migration: - Repoint 6 stale 'from gaia.api import email_routes' imports to 'gaia_agent_email.api_routes' (the module moved into the wheel) in tests/integration/test_never_auto_send.py and tests/test_api.py. These would ModuleNotFoundError in any wheel-installed job. - test_email_agent.yml: install .[dev,api] so fastapi is present for the REST spec tests (test_spec_html), and run tests/integration/test_never_auto_send.py so the never-auto-send confirmation gate has a wheel-installed CI home. - Drop 'email' from test_reserved_id_rejected — email is no longer a reserved builtin id; it ships as the gaia-agent-email wheel and registers via the gaia.agent entry point (#1102). - Update docs/spec/email-contract.mdx to the gaia_agent_email.contract path.
|
Thanks for the thorough review — all addressed in 49b3cdc (pushed as a fast-forward). 🔴/🟡 Stale 🟡 Never-auto-send CI coverage — 🟢 Doc spec — updated 🟢 generate_mbox.py diff noise — left as-is per your note that it's harmless (the generator's line endings don't affect fixture output). Also fixed two migration tails the review didn't cover but CI surfaced: |
|
🟡 The old code imported from from gaia_agent_email.tools.triage_heuristics import ...
The comment still reads:
That claim no longer holds. Either update the comment to say installation is now required, or add _HUB_EMAIL = _REPO_ROOT / "hub" / "agents" / "python" / "email"
if str(_HUB_EMAIL) not in sys.path:
sys.path.insert(0, str(_HUB_EMAIL))CI isn't affected (the email workflow installs the wheel; |
- isort the repointed email_routes imports in tests/test_api.py and tests/integration/test_never_auto_send.py — CI's isort groups the function-level gaia_agent_email imports (my pinned isort version didn't). - generate_mbox.py: add the hub email package to sys.path so the fixture generator stays runnable standalone — the triage taxonomy moved into the gaia-agent-email wheel (#1102) and isn't reachable from the repo root alone. The importorskip guard in test_synthetic_mbox still skips in wheel-less jobs (it runs before generate_mbox is imported).
|
Both addressed: 🔴 Stale 🟡 |
|
@claude review the latest and greatest! |
|
All prior review items are resolved, and the migration is clean. One residual coverage gap remains around the relocated REST surface — details below. Verified fixed ✅
🟡 Residual gap — the relocated REST surface still runs in no wheel-installed jobThe never-auto-send gate is covered now, but the other half of the original suggestion ("…and the REST e2e") slipped through. The relocated
Net: 🟢 Nits (non-blocking)
Verdict: approve once the REST e2e tests have a wheel-installed CI home — everything else is solid and consistent with the analyst/browser hub-wheel precedent. |
|
Reviewed the current state ( Decisions worth your review1. Email identity moved 2. REST email-test coverage gap. I gave the never-auto-send confirmation gate a wheel-installed home (added 3. Standalone fixture generator. Added SolidFail-loud preserved throughout, security send-gate covered, lint clean (matched to CI's LGTM once you're comfortable with the identity call in (1). |
Address the residual review gap on the #1102 email→hub migration: - test_email_agent.yml now runs tests/integration/test_email_rest_api_e2e.py and the email REST classes from tests/test_api.py (TestEmailTriageEndpoint, TestEmailSendConfirmationGate). The /v1/email/* endpoints relocated into the wheel but were importorskip'd in every job (API Tests has no wheel; the email job didn't run them), so the moved surface had zero e2e coverage. Verified green with the wheel installed (33 passed). - openai_server.py: gate the optional email-router mount on an 'import gaia_agent_email' presence check, then import api_routes OUTSIDE the try/except — a genuine broken import inside an installed wheel now fails loudly instead of being swallowed as 'not installed' (no-silent-fallback). - test_api.py: fix the stale 'added by src/gaia/api/email_routes.py' comment.
|
All three addressed in b84ebd7: 🟡 REST surface had no wheel-installed CI home — 🟢 Stale comment (test_api.py:1199) — updated; it now points at 🟢 Silent-swallow in the optional mount (openai_server.py) — good catch, fixed to honor no-silent-fallback. The mount now gates on a lightweight |
The 'import gaia_agent_email' presence check tripped pylint W0611 (unused-import) — '# noqa: F401' only silences flake8, not pylint. Switch to importlib.util.find_spec, which probes the wheel's presence without an import side effect (no unused import), while keeping the fail-loud behavior: the real api_routes import stays outside the gate, so a broken import in an installed wheel still raises loudly.
…md#1539) (amd#1590) The `engine=llm` path added for amd#1452 was written against `src/gaia/api/email_routes.py`, which the hub migration (amd#1520) deleted — leaving the feature stranded on a conflicting branch. Body text was silently truncated to 4 000 characters before reaching the local model, cutting signal from long emails and threads (amd#1539 observation). This PR ports both fixes to the hub's canonical location. **After:** `POST /v1/email/triage?engine=llm` works end-to-end — low-confidence results are escalated to the local Lemonade model (Gemma-4-E4B) with the full email body and a 4 096-token output cap so chain-of-thought reasoning fits. Thread bodies are now joined newest-first. `EmailTriageResult.message_id` echoes the request ID back to the caller. ## Test plan - [x] `uv run python -m pytest hub/agents/python/email/tests/ -x` — 11 tests pass (including body-not-clipped and newest-first ordering assertions) - [x] Live smoke: `curl -s localhost:4200/v1/email/triage?engine=llm -d '...'` returns 200 with `result.category` from LLM (not 502) - [ ] `curl ... ?engine=openai` returns 422 Closes amd#1452 Closes amd#1539
…errors (amd#1592) (amd#1599) Connecting a Google account in the Agent UI left the Email Triage agent unusable: triage failed with a vague "no permissions" message that could only be cleared from the CLI, and after the CLI grant, token refresh failed with a generic "technical issue." Two root causes — the amd#1520 hub migration renamed the per-agent grant key `builtin:email`→`installed:email` (orphaning every existing grant), and the in-chat "grant access" CTA never fired because the email tools emitted `str(exc)` instead of the `AGENT_NOT_GRANTED:`-prefixed `format_connector_error`. After this change: legacy grants migrate automatically on startup; the in-chat CTA fires and points the user at the existing in-UI grant control (no CLI step); token-refresh 401s become actionable (names the account, distinguishes reconnect vs. server-side client-secret); and the OAuth client secret is validated at connect time so a "connected" account that can't refresh is caught immediately. Closes amd#1592 ## Test plan - [x] `pytest tests/unit/connectors/` — 534 passed, 5 skipped (incl. new grant-migration, token-401, secret-validation, and email-tool-envelope regression tests) - [x] `python util/lint.py --all` - [x] `cd src/gaia/apps/webui && npm run build` + vitest (`EmailConnectCta` fires on the `AGENT_NOT_GRANTED:` prefix) - [ ] Real-world (t-nx-strx-halo): connect Google → Email Triage → "triage my last N emails" runs with **no** CLI grant; a stale `builtin:email` grant migrates to `installed:email` on restart — **pending live OAuth** Note: auto-confer-on-connect was intentionally NOT added (consent scope) — the in-UI one-click grant + migration satisfy the AC without blanket-granting every agent.
Why this matters
EmailTriageAgent was the last registry builtin still living in the core source tree. It now ships as the standalone
gaia-agent-emailwheel underhub/agents/python/email/, discovered via thegaia.agententry point — so the core framework wheel no longer hardcodes it and it versions independently. This completes the chat-family/email wave of the #1102 framework-only-core restructure.Unlike the analyst/browser migration (#1446), email owned a core API/MCP surface (
email_routes.py,email_mcp.py). Per the restructure spec these move into the wheel (gaia_agent_email/api_routes.py,mcp_server.py);openai_server.pymounts the email router conditionally and logs an actionablepip install gaia-agent-emailhint when the wheel is absent (no silent fallback).gaia emailresolves through the registry and fails loudly with the same hint.Test plan
python util/lint.py --black --isort— cleangaia.agents.email/gaia.api.email_routes/email_mcpreferences insrc/Email Agent Unit Testsworkflow installs the wheel and runs its suiteNote: API/MCP surface relocation is the highest-risk area — review the guarded import in
openai_server.pyand the testimportorskipguards.Do not merge until CI is green and reviewed.