gcal/gmail: update MCP servers + SKALD.md

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/<name>/`, 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/<session>/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")