fix(connectors): repair Email Triage Google grant + actionable token errors (#1592)

[itomek]

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 #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 #1592

Test plan

• pytest tests/unit/connectors/ — 534 passed, 5 skipped (incl. new grant-migration, token-401, secret-validation, and email-tool-envelope regression tests)
• python util/lint.py --all
• 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.

[github-actions]

Review — fix(connectors): repair Email Triage Google grant + actionable token errors (#1592)

Approve with suggestions. This is a well-structured, well-tested fix that correctly attacks all four root causes behind the broken Email Triage grant flow. The changes are scope-clean, the error paths fail loudly with actionable messages, and the new test coverage is genuinely strong (migration idempotency, the 401 secret-present/absent split, the envelope-prefix contract, and an explicit "this is the bug" regression test). The one thing worth confirming before merge: the startup migration is wired into the UI server only, so CLI-only users with an orphaned builtin:email grant won't auto-migrate.

Issues

🟡 Legacy-grant migration runs on UI startup only, not CLI (src/gaia/ui/server.py:437)
migrate_legacy_agent_grants() is invoked in create_app() init, but the per-agent grant check fires for every email tool call regardless of whether the agent was launched via the UI or gaia email. A user who granted under builtin:email (pre-#1520) and runs the agent purely from the CLI will keep hitting AGENT_NOT_GRANTED until they happen to start the UI once. The migration's own docstring says "Call once at startup (e.g. from the CLI or the UI server init)" — but the CLI half isn't wired. #1592 is UI-scoped so this may be acceptable, but please confirm intent; if CLI is in scope, wiring the same call into the gaia email entry path would close the gap. Migration is idempotent, so double-invocation is safe.

🟢 The actionable 401 reauth message is discarded when surfaced through the email CTA (src/gaia/connectors/tokens.py:226)
The REAUTH_REQUIRED AuthRequiredError carries a carefully-built message ("Token endpoint returned 401 for google … Reconnect from Settings → … See docs/runbooks/google-oauth-client.md"), but format_connector_error ignores the custom message for REAUTH_REQUIRED and returns the canned NOT_CONNECTED: google is not currently connected… string (formatting.py:71-78). So in the email-tool path the user never sees the 401-specific wording or the runbook link — they see a generic "click Connect." It still surfaces verbatim on direct str(exc)/CLI paths, and your test_reauth_required_has_prefix confirms the prefix is intentional, so this isn't a bug — just flagging that the rich message is effectively dead in the CTA path. Consider whether REAUTH_REQUIRED deserves its own prefix that preserves the message.

🟢 PR description says the 401 error "names the account"; the code names only the provider (tokens.py:229-235)
_refresh_token(provider, refresh_token) has no account_email in scope, so neither the ConfigurationError nor the AuthRequiredError includes the account — they name the provider. The tests only assert on the provider name, which matches the code. Minor: tighten the PR description ("names the account, distinguishes reconnect vs. server-side client-secret") to match what's actually emitted, or thread the account through if naming it is a real AC.

🟢 Broad except Exception around the migration (src/gaia/ui/server.py:436)
The noqa: BLE001 — defence in depth swallow is reasonable here — a migration failure shouldn't take down UI startup, and it logs an actionable warning rather than substituting behavior, so it reads as boundary tolerance rather than a silent fallback. No change required; noted only because CLAUDE.md's "fail loudly" rule makes broad catches worth a second look.

Strengths

• Root-cause tests, not just happy-path. test_str_exc_does_not_have_prefix documents the exact pre-fix bug, and the summarize split has dedicated regression tests proving EmailSummarizeError now yields a plain string while ConnectorsError keeps the AGENT_NOT_GRANTED: prefix — this is precisely the contract that broke the CTA, locked down on both the Python and the vitest side.
• The ConnectorsError/EmailSummarizeError handler split (read_tools.py:811, summarize_tools.py:250) is the right call. The old combined except (ConnectorsError, EmailSummarizeError) would have routed summarize failures through format_connector_error and mislabeled them UNEXPECTED_ERROR: — good catch separating them.
• Connect-time secret validation fails loudly at the right seam. _validate_provider_secret runs after credential save but before start_authorization, so a Google account that would 401 on first refresh is rejected at connect time with an actionable message — exactly the "loud error the user can fix over a quiet wrong answer" the project asks for. Restricting it to known-secret-requiring providers and passing unknowns through is a sensible conservative default.
• Migration concurrency is correct — it reuses _write_lock + load_grants/_save_grants_locked, mutates only the nested agent_grants values (not the outer dict under iteration), and the "new key already present → drop stale legacy key, preserve existing value" branch is the safe choice.

Verdict

Approve with suggestions. No blocking issues. Please confirm the UI-only migration wiring is intentional (🟡 above) — that's the only item that could leave a class of users still broken. The two 🟢 message-fidelity notes are polish; the description tweak is trivial.

[github-actions]

🟡 src/gaia/connectors/tokens.py:215 — broad except Exception substituting a placeholder

try:
    err_payload = response.json()
except Exception:
    err_payload = {}

This violates CLAUDE.md's explicit prohibition on except Exception handlers that discard errors and return a placeholder. If response.json() raises something unexpected (e.g. AttributeError on a mock or a memory error), the bug silently collapses to {} and the err_payload.get('error', 'invalid_client') in the message will always say "invalid_client", hiding the real problem.

        try:
            err_payload = response.json()
        except (ValueError, json.JSONDecodeError):
            err_payload = {}

httpx's .json() only raises json.JSONDecodeError (a subclass of ValueError) on malformed bodies — narrowing to those two is sufficient and lets any genuine programming error surface.

[kovtcharov-amd]

Approve. Thorough fix with two correctly-diagnosed root causes and strong regression coverage.

Strengths:

• str(exc) -> format_connector_error(exc) is the right fix; guarantees the AGENT_NOT_GRANTED:/NOT_CONNECTED:/AUTH_REQUIRED: prefix the frontend CTA keys on, applied consistently across tool modules.
• Good catch splitting EmailSummarizeError out of the ConnectorsError except branch — otherwise it would have been mangled into 'UNEXPECTED_ERROR: EmailSummarizeError: …'. Guarded by tests.
• Migration is idempotent and conservative (never clobbers an existing installed:email grant), held under _write_lock, with a no-op fast path.
• 401 handling is correctly placed between the existing 400 branch and the generic !=200 branch, and distinguishes 'secret missing -> ConfigurationError' from 'secret present but rejected -> reauth'.

Non-blocking points to consider:

1. Migration is only wired into ui/server.py; the docstring mentions the CLI too but the CLI path isn't wired. Acceptable since #1592 is UI-scoped — consider wiring the CLI entrypoint or tightening the docstring.
2. Wording drift: new messages say 'Settings -> Connections' while other parts (and CTA fixtures) say 'Connectors'. The fuzzy matcher accepts both, but pick one for consistency.
3. _validate_provider_secret hardcodes the google-only early return; a comment or capability flag would age better when Microsoft needs a secret.

[itomek]

This PR merged before these two review items were addressed, so they're tracked as follow-ups:

• Legacy-grant migration on UI startup only (not CLI) → fix(connectors): wire builtin:email→installed:email grant migration into the CLI email path (UI-only today) #1610 — wire migrate_legacy_agent_grants() into the CLI gaia email path so CLI-only users with a pre-refactor(agents): migrate email to hub (#1102) #1520 builtin:email grant auto-migrate too. ([Bug]: Agent UI — connected Google account can't be used by Email Triage agent (per-agent grant missing in UI, then token 401) #1592 was intentionally UI-scoped, so this was correctly out of this PR.)
• Broad except Exception in tokens.py 401-handling → fix(connectors): narrow broad except in tokens.py 401-handling (No-Silent-Fallbacks) #1612 — narrow to (ValueError, json.JSONDecodeError) per the No-Silent-Fallbacks rule.