diff --git a/SKALD.md b/SKALD.md index c9a0889..1c7e668 100644 --- a/SKALD.md +++ b/SKALD.md @@ -1,6 +1,12 @@ # Skald Connectors Marketplace +### 2026-07-23 — gmaps: fix env var injection +- Aggiunto `mcp_config.env` in `connector.json` per iniettare `GOOGLE_MAPS_API_KEY` nel processo MCP +- Il connector era dichiarato come `delivery: env` ma senza `mcp_config.env` Skald non sapeva passare la variabile al processo Python +- Version bump: fragment 4→5, connector 1→2 +- Indice rigenerato con compile.py ✅ + ### 2026-07-23 — Context7 icon update (PNG) - Sostituite icone Context7 da SVG a PNG (icona nuova fornita dall'utente): - `icon_sm.png` — 48×48 (2.7 KB) diff --git a/connectors/gcal/gcal_mcp_server.py b/connectors/gcal/gcal_mcp_server.py index a84bd59..5913cff 100755 --- a/connectors/gcal/gcal_mcp_server.py +++ b/connectors/gcal/gcal_mcp_server.py @@ -36,6 +36,24 @@ def log(msg: str) -> None: # Protects all stdout writes (main thread + poll thread). _stdout_lock = threading.Lock() +# Serializes every Calendar API call and the service/credential build. +# +# googleapiclient is built on httplib2, which is NOT thread-safe: a service object +# owns a single Http holding one reused TLS connection. The poll thread and the +# request thread sharing that object means two threads can drive the same OpenSSL +# socket at once — which does not raise, it segfaults (this exact race killed the +# Gmail connector in production with SIGSEGV, no traceback). `_creds` has the same +# problem: google.auth refreshes it in place, so two concurrent refreshes corrupt +# the token. +# +# Every API call in this file goes through `_call`, and the service and creds are +# built in `_get_service`, so serializing those two entry points covers the whole +# surface — including any call site added later. Reentrant because the two can nest. +# +# Held per-call, never per-batch: correctness needs non-concurrency on the socket, +# not atomicity, and a poll tick must not wait for a long listing to finish. +_api_lock = threading.RLock() + # ── Push notifications ───────────────────────────────────────────────────────── def _emit_notification(method: str, params: dict) -> None: @@ -215,9 +233,12 @@ def _build_service() -> Any: def _get_service() -> Any: global _service - if _service is None: - _service = _build_service() - return _service + # Locked: two threads racing here would build (and refresh) two services, and + # the loser's would be dropped while its connection was still in flight. + with _api_lock: + if _service is None: + _service = _build_service() + return _service # ── Error mapping & refresh-on-auth-error ────────────────────────────────────── @@ -245,21 +266,25 @@ def _call(fn: Callable[[], Any], api_label: str) -> Any: or a RefreshError. We refresh once, persist the new token, and retry the call. Anything else (or a second failure) is re-raised so the caller can format it via _format_google_error. + + Holds `_api_lock` for the whole body — see the lock's comment. This is the one + choke point every `.execute()` in this file passes through. """ - try: - return fn() - except Exception as e: - if not _is_auth_error(e) or _creds is None or not getattr(_creds, "refresh_token", None): - raise + with _api_lock: try: - from google.auth.transport.requests import Request - _creds.refresh(Request()) - _persist_creds() - log("Access token refreshed mid-session after auth error; retrying the call.") - except Exception as refresh_err: - log(f"Mid-session token refresh failed: {refresh_err}") - raise - return fn() + return fn() + except Exception as e: + if not _is_auth_error(e) or _creds is None or not getattr(_creds, "refresh_token", None): + raise + try: + from google.auth.transport.requests import Request + _creds.refresh(Request()) + _persist_creds() + log("Access token refreshed mid-session after auth error; retrying the call.") + except Exception as refresh_err: + log(f"Mid-session token refresh failed: {refresh_err}") + raise + return fn() def _http_error_reason(e: Exception) -> str: diff --git a/connectors/gmail/gmail_mcp_server.py b/connectors/gmail/gmail_mcp_server.py index aa77a94..f15f080 100644 --- a/connectors/gmail/gmail_mcp_server.py +++ b/connectors/gmail/gmail_mcp_server.py @@ -40,6 +40,28 @@ def log(msg: str) -> None: # Protects all stdout writes (main request-handling thread + poll thread). _stdout_lock = threading.Lock() +# Serializes every Gmail API call and the service/credential build. +# +# googleapiclient is built on httplib2, which is NOT thread-safe: a service object +# owns a single Http holding one reused TLS connection. The poll thread and the +# request thread sharing that object means two threads can drive the same OpenSSL +# socket at once — which does not raise, it segfaults. Observed in production: a +# `list_messages` (max_results=40 ⇒ 41 sequential requests, several seconds of +# traffic) overlapped the 60s poll tick and the process died with SIGSEGV (exit +# 139), no traceback, no log line. `_creds` has the same problem: google.auth +# refreshes it in place, so two concurrent refreshes corrupt the token. +# +# Every API call in this file goes through `_call`, and the service and creds are +# built in `_get_service`, so serializing those two entry points covers the whole +# surface — including any call site added later. Reentrant because `_get_service` +# and `_call` can nest. +# +# Deliberately held per-call, never per-batch: `list_messages` issues 41 requests +# and a poll tick may interleave between any two of them. Correctness needs +# non-concurrency on the socket, not atomicity of the batch — and a poll must not +# wait for a long listing to finish. +_api_lock = threading.RLock() + # ── Push notifications ───────────────────────────────────────────────────────── def _emit_notification(method: str, params: dict) -> None: @@ -109,9 +131,31 @@ def _poll_once() -> None: _fetch_and_emit_email(svc, msg_id) except Exception as e: + # Gmail expires history records after about a week and answers 404 for a + # startHistoryId older than that. Without a re-sync the cursor stays stale + # forever and polling is silently dead for the rest of the process's life, + # so re-anchor on the current historyId: we lose the notifications for the + # gap (unknowable by definition) but the watch resumes. + if _is_history_expired(e): + log("History cursor expired (404); re-anchoring on the current historyId.") + try: + profile = _call(lambda: svc.users().getProfile(userId="me").execute(), "Gmail") + _last_history_id = str(profile.get("historyId", "")) or _last_history_id + except Exception as resync_err: + log(f"History re-anchor failed: {_format_google_error(resync_err, 'Gmail')}") + return log(f"Gmail history poll error: {_format_google_error(e, 'Gmail')}") +def _is_history_expired(e: Exception) -> bool: + """True for the 404 Gmail returns when startHistoryId is too old to serve.""" + try: + from googleapiclient.errors import HttpError + except ImportError: + return False + return isinstance(e, HttpError) and getattr(e, "status_code", None) == 404 + + def _fetch_and_emit_email(svc: Any, msg_id: str) -> None: """Fetch metadata for a message and emit an event/new_email notification.""" try: @@ -241,9 +285,12 @@ def _build_service() -> Any: def _get_service() -> Any: global _service - if _service is None: - _service = _build_service() - return _service + # Locked: two threads racing here would build (and refresh) two services, and + # the loser's would be dropped while its connection was still in flight. + with _api_lock: + if _service is None: + _service = _build_service() + return _service # ── Error mapping & refresh-on-auth-error ────────────────────────────────────── @@ -271,21 +318,25 @@ def _call(fn: Callable[[], Any], api_label: str) -> Any: or a RefreshError. We refresh once, persist the new token, and retry the call. Anything else (or a second failure) is re-raised so the caller can format it via _format_google_error. + + Holds `_api_lock` for the whole body — see the lock's comment. This is the one + choke point every `.execute()` in this file passes through. """ - try: - return fn() - except Exception as e: - if not _is_auth_error(e) or _creds is None or not getattr(_creds, "refresh_token", None): - raise + with _api_lock: try: - from google.auth.transport.requests import Request - _creds.refresh(Request()) - _persist_creds() - log("Access token refreshed mid-session after auth error; retrying the call.") - except Exception as refresh_err: - log(f"Mid-session token refresh failed: {refresh_err}") - raise - return fn() + return fn() + except Exception as e: + if not _is_auth_error(e) or _creds is None or not getattr(_creds, "refresh_token", None): + raise + try: + from google.auth.transport.requests import Request + _creds.refresh(Request()) + _persist_creds() + log("Access token refreshed mid-session after auth error; retrying the call.") + except Exception as refresh_err: + log(f"Mid-session token refresh failed: {refresh_err}") + raise + return fn() def _http_error_reason(e: Exception) -> str: @@ -353,6 +404,26 @@ def _status_report(icon: str, label: str, kind: str, description: str, steps: li # ── Helpers ──────────────────────────────────────────────────────────────────── +def _home() -> str: + """The user's home — the root of the namespace the agent actually sees. + + Every filesystem path this server accepts or produces anchors here. It must + NOT anchor at the connector's own directory: Skald installs a per-user + connector under `~/.skald/mcp//`, so the former + `dirname(dirname(__file__))` resolved to `~/.skald/mcp` — a hidden internal + directory the agent never lists and cannot name. An attachment passed as + `uploads//cv.pdf` (the path the agent is handed for an upload) + resolved under it and simply did not exist. + """ + return os.path.expanduser("~") + + +def _resolve_user_path(raw: str) -> str: + """Turn an agent-supplied path into an absolute one, anchored at the home.""" + expanded = os.path.expanduser(raw) + return expanded if os.path.isabs(expanded) else os.path.join(_home(), expanded) + + def _decode_body(parts: Any, mime_type: str = "text/plain") -> str: """Recursively extract the body of the given MIME type from MIME parts.""" if isinstance(parts, list): @@ -471,8 +542,9 @@ def _gmail_status(args: dict | None = None) -> str: if svc is None: return _status_report("❌", "NOT_CONFIGURED", "action needed", f"The Gmail service could not be built: {_init_error or 'unknown error'}.", - ["Run scripts/gmail_oauth_setup.py to authenticate (standalone).", - "For Skald mode, ensure GMAIL_CREDS_JSON env var is set."]) + ["Sign in to Google from the Gmail connector page in Skald.", + "If the sign-in was already done, an admin should check that the Google " + "sign-in provider is configured (client id/secret and redirect URI)."]) # Step 2: live probe — refresh-on-auth-error is handled inside _call. try: @@ -747,14 +819,13 @@ def _gmail_send_message(args: dict) -> str: if isinstance(attachments, str): attachments = [attachments] - # Resolve every attachment path (relative paths are anchored at the project - # root, like _build_service / download_attachments) and fail early if any file - # is missing — the email is NOT sent unless every attachment is present. - project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + # Resolve every attachment path (relative paths are anchored at the home — see + # _home) and fail early if any file is missing: the email is NOT sent unless + # every attachment is present. resolved: list[str] = [] total_bytes = 0 for raw_path in attachments: - path = raw_path if os.path.isabs(raw_path) else os.path.join(project_root, raw_path) + path = _resolve_user_path(raw_path) if not os.path.isfile(path): return f"Error: attachment not found: {raw_path}" total_bytes += os.path.getsize(path) @@ -867,13 +938,11 @@ def _gmail_download_attachments(args: dict) -> str: if not msg_id: return "Error: Missing required parameter 'message_id'." - # Default to data/gmail_attachments/ (served via /data/... in the frontend, - # consistent with whatsapp_media). Allow override. - default_folder = os.path.join( - os.path.dirname(os.path.dirname(os.path.abspath(__file__))), - "data", "gmail_attachments", - ) - folder = args.get("folder") or default_folder + # Default into the home so the saved files are reachable by the agent's own + # file tools (and by execute_cmd) under `gmail_attachments/`. The old default + # wrote into the connector's private install dir, where nothing could read them. + folder = args.get("folder") + folder = _resolve_user_path(folder) if folder else os.path.join(_home(), "gmail_attachments") try: msg = _call(lambda: svc.users().messages().get(userId="me", id=msg_id, format="full").execute(), "Gmail")