From 78cc37a977aac2d9004d53b9a689ca61b992a9f2 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sun, 12 Jul 2026 02:35:47 -0400 Subject: [PATCH] feat(observability): optional self-hosted Sentry instrumentation for MCP workflow failures (Closes #606) Add an env-var-gated, off-by-default Sentry SDK integration so MCP runtime errors, fail-closed workflow blockers, lease/terminal-lock/stale-runtime collisions, and recurring watchdog check-ins are visible in a self-hosted Sentry at https://sentry.prgs.cc/. Gitea stays the source of truth; Sentry is observe-only. New module `sentry_observability.py` mirrors the `gitea_audit` conventions (env-gated, best-effort, redacting): - Config from MCP_SENTRY_ENABLED / SENTRY_DSN / SENTRY_ENVIRONMENT / SENTRY_RELEASE / MCP_SENTRY_TRACES_SAMPLE_RATE / MCP_SENTRY_ENABLE_LOGS. Active only when enabled AND a DSN is present; otherwise a hard no-op. - Fail OPEN for observability (never blocks a tool success path) and fail CLOSED for redaction (drop a field rather than risk leaking it). - `scrub_event` before_send/before_send_log hook + allowlisted tags: no tokens/passwords/keychain ids/DSNs/cookies, no raw session-state or full prompt bodies (session_id -> 12-char hash), no full filesystem paths (worktree path -> coarse category). Reuses incident_bridge + gitea_audit scrubbers. - capture_exception, capture_workflow_blocker (with canonical next action), and monitor_checkin with six stable cron slugs (stale lease scan, terminal lock scan, allocator health, namespace health, dashboard freshness, reconciler cleanup). - `sentry_sdk` is a lazily-imported optional dependency; the module imports and no-ops cleanly when the package is absent. Wiring in gitea_mcp_server.py (additive, guarded, best-effort): - init_sentry() in __main__ before mcp.run. - capture_exception in the `_audited` failure path; capture_workflow_blocker in `_audit_pr_result` BLOCKED/FAILED path. - allocator + namespace-health watchdog check-ins at their MCP tool sites (domain modules left pure). Also: pin `sentry-sdk==2.20.0` (optional), document the six env vars in `.env.example`, and add `docs/observability/sentry-integration.md` covering project creation in https://sentry.prgs.cc/, DSN handling, local/dev/prod config, redaction guarantees, and coexistence with the #612 incident bridge. Tests: tests/test_sentry_observability.py (36 cases) cover disabled / enabled / missing-DSN / missing-SDK, redaction, exception capture, workflow-blocker capture, and cron check-in behaviour. Full suite: 2632 passed, 6 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --- .env.example | 21 + docs/observability/sentry-integration.md | 138 ++++++ gitea_mcp_server.py | 50 +++ requirements.txt | 1 + sentry_observability.py | 535 +++++++++++++++++++++++ tests/test_sentry_observability.py | 412 +++++++++++++++++ 6 files changed, 1157 insertions(+) create mode 100644 docs/observability/sentry-integration.md create mode 100644 sentry_observability.py create mode 100644 tests/test_sentry_observability.py diff --git a/.env.example b/.env.example index 4777fac..13391f5 100644 --- a/.env.example +++ b/.env.example @@ -39,6 +39,27 @@ GITEA_AUDIT_LOG=/path/to/gitea-mcp-audit.log # only — never the token value. Surfaced by gitea_get_profile. GITEA_TOKEN_SOURCE=GITEA_TOKEN +# ── Optional self-hosted Sentry observability (#606) ──────────────────────── +# Emits runtime errors, fail-closed workflow blockers, lease/terminal-lock/ +# stale-runtime collisions, and watchdog cron check-ins to a SELF-HOSTED Sentry +# (https://sentry.prgs.cc/) — never Sentry Cloud. Gitea stays the source of +# truth; Sentry is observe-only. OFF by default: with MCP_SENTRY_ENABLED unset +# or SENTRY_DSN empty, nothing is initialised and no events are sent. +# +# Master gate. Truthy = 1/true/yes/on. Both this AND SENTRY_DSN are required. +MCP_SENTRY_ENABLED=0 +# DSN for the self-hosted project (create a `gitea-tools-mcp` project in +# https://sentry.prgs.cc/ and copy its DSN). Never commit a real DSN. +SENTRY_DSN= +# Deployment environment tag (local/dev/prod). Defaults to "development". +SENTRY_ENVIRONMENT=development +# Optional release identifier (e.g. a git SHA or version string). +SENTRY_RELEASE= +# Performance-trace sample rate, 0.0–1.0 (clamped). Default 0.0 (traces off). +MCP_SENTRY_TRACES_SAMPLE_RATE=0.0 +# Set to 1 to forward Python logs to Sentry as structured logs. Default off. +MCP_SENTRY_ENABLE_LOGS=0 + # Optional canonical runtime-profile config (#19). Instead of the fields above, # point every LLM launcher at ONE JSON file of named profiles and select one. # Secrets are referenced (keychain id / env var name), never inlined. See diff --git a/docs/observability/sentry-integration.md b/docs/observability/sentry-integration.md new file mode 100644 index 0000000..e06de0a --- /dev/null +++ b/docs/observability/sentry-integration.md @@ -0,0 +1,138 @@ +# Self-hosted Sentry observability for the Gitea MCP server (#606) + +Optional, **off-by-default** instrumentation that reports MCP runtime errors, +fail-closed workflow blockers, lease / terminal-lock / stale-runtime +collisions, and recurring watchdog check-ins to a **self-hosted** Sentry at +`https://sentry.prgs.cc/`. + +> **Gitea remains the source of truth.** Sentry is observe-only. It never +> approves, merges, closes, or otherwise mutates Gitea workflow state, and it +> never bypasses leases, #332, workflow roles, or the MCP gates. Sentry alerts +> may only feed the *sanctioned* Gitea issue/comment path via the #612 incident +> bridge — never a direct write. + +Implemented by [`sentry_observability.py`](../../sentry_observability.py). + +--- + +## 1. Create the Sentry project + +1. Sign in to the self-hosted Sentry at **`https://sentry.prgs.cc/`** (this is + **not** Sentry Cloud — do not use `*.ingest.sentry.io`). +2. Create a new **Python** project named **`gitea-tools-mcp`**. +3. Open **Settings → Projects → gitea-tools-mcp → Client Keys (DSN)** and copy + the DSN. It looks like `https://@sentry.prgs.cc/`. +4. **Never commit the DSN.** It is a runtime secret supplied via env var only. + +## 2. Configure the environment + +All configuration is env-var driven (see [`.env.example`](../../.env.example)): + +| Variable | Purpose | Default | +|----------|---------|---------| +| `MCP_SENTRY_ENABLED` | Master gate (`1/true/yes/on`). Required. | off | +| `SENTRY_DSN` | Self-hosted DSN. Required. | *(empty)* | +| `SENTRY_ENVIRONMENT` | `local` / `dev` / `prod` tag. | `development` | +| `SENTRY_RELEASE` | Release id (git SHA or version). | *(none)* | +| `MCP_SENTRY_TRACES_SAMPLE_RATE` | Perf-trace sample rate `0.0–1.0` (clamped). | `0.0` | +| `MCP_SENTRY_ENABLE_LOGS` | Forward Python logs as structured logs. | off | + +**The feature stays completely off unless `MCP_SENTRY_ENABLED` is truthy *and* +`SENTRY_DSN` is non-empty.** With either missing, `init_sentry()` is a no-op, +the SDK is never initialised, and no events are sent — existing tool behaviour +and API-call patterns are unchanged. + +### Per-environment examples + +```bash +# local (quiet: capture errors/blockers, no traces) +export MCP_SENTRY_ENABLED=1 +export SENTRY_DSN="https://@sentry.prgs.cc/" +export SENTRY_ENVIRONMENT=local + +# dev (light tracing + logs) +export MCP_SENTRY_ENABLED=1 +export SENTRY_DSN="https://@sentry.prgs.cc/" +export SENTRY_ENVIRONMENT=dev +export MCP_SENTRY_TRACES_SAMPLE_RATE=0.2 +export MCP_SENTRY_ENABLE_LOGS=1 + +# prod (errors/blockers + low-rate tracing, release-tagged) +export MCP_SENTRY_ENABLED=1 +export SENTRY_DSN="https://@sentry.prgs.cc/" +export SENTRY_ENVIRONMENT=prod +export SENTRY_RELEASE="$(git rev-parse --short HEAD)" +export MCP_SENTRY_TRACES_SAMPLE_RATE=0.05 +``` + +The optional SDK is pinned in [`requirements.txt`](../../requirements.txt) +(`sentry-sdk==2.20.0`). It is imported lazily: if the package is absent, the +module still imports and every entry point is a safe no-op. + +## 3. What is instrumented + +| Signal | Where | Notes | +|--------|-------|-------| +| Startup init | `gitea_mcp_server.py` `__main__`, before `mcp.run` | Prints a redaction-safe status line to stderr. | +| Failing mutations (exceptions) | `_audited(...)` context manager | `capture_exception` with scrubbed tags. | +| Fail-closed blockers / failed mutations | `_audit_pr_result(...)` (BLOCKED/FAILED) | Structured `capture_workflow_blocker` event incl. the canonical next action when available (criterion 7). | +| Allocator watchdog check-ins | `gitea_allocate_next_work` tool | `allocator_health`, `stale_lease_scan`, `terminal_lock_scan`. | +| Namespace-health check-in | `gitea_assess_mcp_namespace_health` tool | `namespace_health`. | + +All capture paths are **best-effort / fail open**: a Sentry outage or capture +error never breaks an MCP tool success path. + +## 4. Cron / watchdog monitors + +`sentry_observability.MONITOR_SLUGS` defines stable check-in slugs: + +| Registry key | Sentry monitor slug | Wired at | +|--------------|--------------------|----------| +| `stale_lease_scan` | `gitea-mcp-stale-lease-scan` | allocator run (global lease expiry) | +| `terminal_lock_scan` | `gitea-mcp-terminal-lock-scan` | allocator run (terminal-lock lookup) | +| `allocator_health` | `gitea-mcp-allocator-health` | allocator run | +| `namespace_health` | `gitea-mcp-namespace-health` | namespace-health probe | +| `dashboard_freshness` | `gitea-mcp-dashboard-freshness` | call `monitor_checkin("dashboard_freshness", ...)` from the dashboard refresh job (#605) | +| `reconciler_cleanup` | `gitea-mcp-reconciler-cleanup` | call `monitor_checkin("reconciler_cleanup", ...)` from the reconciler cleanup entrypoint | + +Create matching Cron monitors in Sentry with those slugs. Emit an +`in_progress` check-in at job start and `ok`/`error` at completion via +`sentry_observability.monitor_checkin(slug_key, status)`. + +## 5. Redaction guarantees (fail closed) + +Redaction fails *closed*: if a field cannot be proven safe it is dropped rather +than sent. The `before_send` (and `before_send_log`) hook `scrub_event` +recursively redacts every outgoing event; on any error it drops the event +entirely. Guarantees, proven by `tests/test_sentry_observability.py`: + +- **No** tokens, passwords, keychain IDs, DSNs, cookies, or `user:pass@host`. +- **No** raw session-state or full prompt/comment bodies — `session_id` is only + ever surfaced as a 12-char `session_id_hash`. +- **No** private config contents or raw credential headers. +- **No** full local filesystem paths — a worktree path collapses to a coarse + `worktree_category` (`author` / `reviewer` / `merger` / `reconciler` / + `branches` / `root` / `other`). +- Only the allowlisted tag keys in `ALLOWED_TAG_KEYS` are ever attached. + +## 6. Coexistence with GlitchTip / the #612 incident bridge + +This is the **outbound** path (MCP → Sentry SDK). It complements — it does not +replace — the **inbound** [`incident_bridge.py`](../../incident_bridge.py) +(#612), which turns Sentry/GlitchTip *observations* into durable Gitea issues +and `incident_links` rows. + +- Prefer **one** observability path per environment. Point the MCP server's + `SENTRY_DSN` at the same self-hosted `gitea-tools-mcp` project that the #612 + bridge reconciles from, so an MCP-reported error and its Gitea issue line up. +- GlitchTip is Sentry-protocol compatible; if an existing GlitchTip DSN is in + use, either migrate it to `https://sentry.prgs.cc/` or document the split + (MCP → Sentry, legacy → GlitchTip) explicitly for operators. +- The bridge remains the **only** sanctioned route from an alert back into + Gitea workflow state. + +## 7. Non-goals + +- Sentry must **not** become the workflow source of truth. +- Sentry must **not** approve, merge, close, or mutate Gitea workflow state. +- Sentry must **not** bypass leases, #332, workflow roles, or the MCP gates. diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 669e9aa..efdf045 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -918,6 +918,7 @@ import allocator_service # noqa: E402 import control_plane_db # noqa: E402 import lease_lifecycle # noqa: E402 import incident_bridge # noqa: E402 +import sentry_observability # noqa: E402 (#606 optional Sentry observability) import agent_temp_artifacts import issue_lock_worktree # noqa: E402 import issue_lock_provenance # noqa: E402 @@ -1777,6 +1778,18 @@ def _audited(action: str, *, host, remote, org=None, repo=None, result=gitea_audit.FAILED, reason=_redact(str(exc)), request_metadata=request_metadata, issue_number=issue_number, pr_number=pr_number, target_branch=target_branch) + # #606: best-effort Sentry capture of the failing mutation (fail open). + sentry_observability.capture_exception( + exc, + tags={ + "mutation_tool": action, + "remote": remote, + "repo": repo, + "org": org, + "issue_number": issue_number, + "pr_number": pr_number, + }, + ) raise _audit(action, host=host, remote=remote, org=org, repo=repo, result=gitea_audit.SUCCEEDED, request_metadata=request_metadata, @@ -1823,6 +1836,20 @@ def _audit_pr_result(action: str): "merge_method": result.get("merge_method"), }, ) + # #606: surface fail-closed blockers / failed mutations to + # Sentry as structured events (best-effort, fail open). + if status in (gitea_audit.BLOCKED, gitea_audit.FAILED): + sentry_observability.capture_workflow_blocker( + action, + message="; ".join(reasons) or action, + next_action=result.get("safe_next_action"), + level="error" if status == gitea_audit.FAILED else "warning", + tags={ + "mutation_tool": action, + "pr_number": result.get("pr_number"), + "current_head_sha": result.get("head_sha"), + }, + ) except Exception: pass # best-effort; never break the tool return result @@ -9954,6 +9981,11 @@ def gitea_assess_mcp_namespace_health( probe_source=probe_source, ) _record_live_namespace_health(result) + # #606: namespace-health watchdog check-in (best-effort, fail open). + sentry_observability.monitor_checkin( + "namespace_health", + "ok" if result.get("healthy", result.get("callable", True)) else "error", + ) return result @@ -11881,6 +11913,18 @@ def gitea_allocate_next_work( result["inventory_source"] = ( "candidates_json" if candidates_json else "gitea_live" ) + # #606: watchdog check-ins for the recurring jobs this allocator run + # performs — global stale-lease expiry, terminal-lock lookup, and the + # allocator itself. Best-effort; a failed selection reports "error". + _alloc_ok = bool(result.get("success")) + sentry_observability.monitor_checkin( + "allocator_health", "ok" if _alloc_ok else "error" + ) + if _alloc_ok: + # These two scans complete inside allocate_next_work before selection; + # a successful result proves both ran. + sentry_observability.monitor_checkin("stale_lease_scan", "ok") + sentry_observability.monitor_checkin("terminal_lock_scan", "ok") return result @@ -12205,4 +12249,10 @@ if __name__ == "__main__": # processes (e.g. review_pr.py) can detect and refuse profile # side-channel overrides (#199). _export_session_profile_lock() + # #606: optional self-hosted Sentry observability. No-op unless + # MCP_SENTRY_ENABLED is truthy and SENTRY_DSN is set; never blocks startup. + _sentry_status = sentry_observability.init_sentry() + sys.stderr.write( + f"--- Sentry observability: {_sentry_status.get('reason')} ---\n" + ) mcp.run(transport="stdio") diff --git a/requirements.txt b/requirements.txt index bbe0703..f022c56 100644 --- a/requirements.txt +++ b/requirements.txt @@ -31,6 +31,7 @@ python-multipart==0.0.32 referencing==0.37.0 rich==15.0.0 rpds-py==2026.5.1 +sentry-sdk==2.20.0 shellingham==1.5.4 sse-starlette==3.4.5 starlette==1.3.1 diff --git a/sentry_observability.py b/sentry_observability.py new file mode 100644 index 0000000..292af7a --- /dev/null +++ b/sentry_observability.py @@ -0,0 +1,535 @@ +"""Optional self-hosted Sentry observability for the Gitea MCP server (#606). + +Adds env-var-gated Sentry SDK instrumentation so runtime errors, fail-closed +workflow blockers, lease/terminal-lock/stale-runtime collisions, and recurring +watchdog check-ins are visible in a *self-hosted* Sentry at +``https://sentry.prgs.cc/`` — never Sentry Cloud, and never as the workflow +source of truth (Gitea stays canonical). + +Design constraints (mirror ``gitea_audit`` and the #612 incident bridge): + +- **Off by default.** With ``MCP_SENTRY_ENABLED`` false/unset *or* ``SENTRY_DSN`` + empty, ``init_sentry`` is a no-op and no events are ever sent — existing tool + behaviour and API-call patterns are unchanged (acceptance criterion 1). +- **Fail *open* for observability.** A Sentry outage, a missing ``sentry_sdk`` + package, or any capture error must never break an MCP tool success path. Every + public entry point swallows its own exceptions. +- **Fail *closed* for redaction.** If a field cannot be proven safe it is dropped + rather than sent. Tokens, passwords, keychain IDs, DSNs, private config, raw + session-state, full prompt bodies, and full filesystem paths never leave here. +- **No hard dependency.** ``sentry_sdk`` is imported lazily; the module is fully + importable and testable without it installed. + +Sentry is observe-only: it must not approve, merge, close, or otherwise mutate +Gitea workflow state, nor bypass leases, #332, or MCP gates. Alerts may only feed +the sanctioned Gitea issue/comment path via the #612 incident bridge. +""" + +from __future__ import annotations + +import hashlib +import os +import re +from dataclasses import dataclass +from typing import Any + +# Reuse the most comprehensive existing scrubber so redaction stays consistent +# with the #612 incident bridge (tokens, DSNs, cookies, bearer/basic, keychain +# ids, session ids, user:pass@host). +from incident_bridge import redact_text as _redact_text + +# Second, complementary scrubber: catches bare ``token `` / +# ``Bearer `` / ``Basic `` prefixes and raw URLs that the +# incident-bridge delimiter patterns miss. +from gitea_audit import _redact_str as _redact_prefixes + +# ── Optional SDK (lazy, never a hard dependency) ──────────────────────────── +try: # pragma: no cover - trivial import guard + import sentry_sdk # type: ignore +except Exception: # pragma: no cover - absence is a supported state + sentry_sdk = None # type: ignore + + +# ── Env var names (single source of truth) ────────────────────────────────── +ENV_ENABLED = "MCP_SENTRY_ENABLED" +ENV_DSN = "SENTRY_DSN" +ENV_ENVIRONMENT = "SENTRY_ENVIRONMENT" +ENV_RELEASE = "SENTRY_RELEASE" +ENV_TRACES_SAMPLE_RATE = "MCP_SENTRY_TRACES_SAMPLE_RATE" +ENV_ENABLE_LOGS = "MCP_SENTRY_ENABLE_LOGS" + +_TRUTHY = frozenset({"1", "true", "yes", "on"}) + +REDACTED = "[REDACTED]" +REDACTED_PATH = "[REDACTED_PATH]" + + +# ── Cron / watchdog monitor slugs (acceptance criterion 6) ────────────────── +# Stable slugs for the recurring/watchdog jobs #606 wants check-ins for. The +# slug is the durable monitor identity in Sentry; the wiring call sites pass one +# of these keys (or an explicit slug) to ``monitor_checkin``. +MONITOR_SLUGS: dict[str, str] = { + "stale_lease_scan": "gitea-mcp-stale-lease-scan", + "terminal_lock_scan": "gitea-mcp-terminal-lock-scan", + "allocator_health": "gitea-mcp-allocator-health", + "namespace_health": "gitea-mcp-namespace-health", + "dashboard_freshness": "gitea-mcp-dashboard-freshness", + "reconciler_cleanup": "gitea-mcp-reconciler-cleanup", +} + +_CHECKIN_STATUSES = frozenset({"in_progress", "ok", "error"}) + + +# ── Tag allowlist (issue "Suggested Sentry tags/context") ─────────────────── +# Only these keys are ever attached as Sentry tags. Anything else is dropped so +# a caller cannot accidentally leak a sensitive value through a tag. +ALLOWED_TAG_KEYS = frozenset({ + "role", + "profile", + "namespace", + "repo", + "org", + "issue_number", + "pr_number", + "blocker_type", + "workflow_hash", + "session_id_hash", # hash only — never the raw session id + "pid", + "worktree_category", # category, never the full sensitive path + "lease_comment_id", + "expected_head_sha", + "current_head_sha", + "terminal_lock_state", + "capability", + "mutation_tool", +}) + +# Absolute-path shapes that must never be sent verbatim (macOS/Linux + temp). +_PATH_RE = re.compile(r"(?:/private)?/(?:Users|home|tmp|var|opt|Volumes)/[^\s\"']*") + +# ``extra`` keys whose *full* contents are forbidden by the redaction rules +# (raw session-state, full prompt/comment bodies, private config blobs, raw +# headers). +_FORBIDDEN_EXTRA_KEYS = frozenset({ + "prompt", + "prompt_body", + "next_prompt", + "body", + "raw_body", + "session_state", + "session_state_contents", + "config", + "config_contents", + "private_config", + "headers", + "authorization", +}) + + +# ── Configuration ─────────────────────────────────────────────────────────── +@dataclass(frozen=True) +class SentryConfig: + """Immutable snapshot of the Sentry env configuration.""" + + enabled: bool = False + dsn: str | None = None + environment: str = "development" + release: str | None = None + traces_sample_rate: float = 0.0 + enable_logs: bool = False + + @property + def active(self) -> bool: + """True only when the operator both opted in *and* supplied a DSN. + + This is the single gate that keeps the feature off by default: enabling + the flag without a DSN (or vice versa) sends nothing. + """ + return bool(self.enabled and self.dsn) + + def safe_summary(self) -> dict[str, Any]: + """Operator-facing status with **no** DSN value (only presence).""" + return { + "enabled": self.enabled, + "dsn_present": bool(self.dsn), + "environment": self.environment, + "release": self.release, + "traces_sample_rate": self.traces_sample_rate, + "enable_logs": self.enable_logs, + "active": self.active, + } + + +def _env_bool(name: str, env: dict[str, str]) -> bool: + return (env.get(name) or "").strip().lower() in _TRUTHY + + +def _env_float(name: str, default: float, env: dict[str, str]) -> float: + raw = (env.get(name) or "").strip() + if not raw: + return default + try: + val = float(raw) + except (TypeError, ValueError): + return default + # Clamp to Sentry's valid [0.0, 1.0] sample-rate range. + if val < 0.0: + return 0.0 + if val > 1.0: + return 1.0 + return val + + +def load_config(env: dict[str, str] | None = None) -> SentryConfig: + """Build a :class:`SentryConfig` from the environment (read at call time).""" + env = dict(os.environ if env is None else env) + dsn = (env.get(ENV_DSN) or "").strip() or None + return SentryConfig( + enabled=_env_bool(ENV_ENABLED, env), + dsn=dsn, + environment=(env.get(ENV_ENVIRONMENT) or "").strip() or "development", + release=(env.get(ENV_RELEASE) or "").strip() or None, + traces_sample_rate=_env_float(ENV_TRACES_SAMPLE_RATE, 0.0, env), + enable_logs=_env_bool(ENV_ENABLE_LOGS, env), + ) + + +def sdk_available() -> bool: + """True when the optional ``sentry_sdk`` package is importable.""" + return sentry_sdk is not None + + +# ── Redaction (fail closed) ───────────────────────────────────────────────── +def sanitize_path(value: Any) -> str: + """Reduce a filesystem path to a non-sensitive *category* token. + + Full local paths must never be sent. We keep only a coarse worktree + category derived from the path shape (author/reviewer/merger/reconciler/ + branches/root/other). + """ + text = "" if value is None else str(value) + low = text.lower() + if not text: + return "unknown" + # Order matters: more specific role markers before the generic "branches". + if "reconcile" in low: + return "reconciler" + if "review" in low: + return "reviewer" + if "merge" in low or "merger" in low: + return "merger" + if "author" in low or re.search(r"/branches/(?:feat|fix|docs|chore|issue)", low): + return "author" + if "/branches/" in low: + return "branches" + if low.rstrip("/").endswith("gitea-tools"): + return "root" + return "other" + + +def redact_value(value: Any) -> Any: + """Recursively redact a JSON-able value: secret text, absolute paths, and + known-sensitive dict keys are removed. Fail closed — any error drops the + value entirely rather than risk leaking it.""" + try: + if isinstance(value, dict): + out: dict[str, Any] = {} + for k, v in value.items(): + key = str(k) + low = key.lower() + if low in _FORBIDDEN_EXTRA_KEYS or any( + s in low + for s in ("token", "secret", "password", "cookie", "auth", "dsn", "keychain") + ): + out[key] = REDACTED + continue + out[key] = redact_value(v) + return out + if isinstance(value, (list, tuple)): + return [redact_value(v) for v in value] + if isinstance(value, str): + scrubbed = _redact_text(value) + scrubbed = _redact_prefixes(scrubbed) + scrubbed = _PATH_RE.sub(REDACTED_PATH, scrubbed) + return scrubbed + return value + except Exception: + return REDACTED + + +def hash_session_id(session_id: Any) -> str: + """Short, stable, non-reversible fingerprint of a session id.""" + digest = hashlib.sha256(str(session_id).encode("utf-8", "replace")).hexdigest() + return digest[:12] + + +def build_tags(**kwargs: Any) -> dict[str, str]: + """Return a scrubbed, allowlisted tag dict. + + ``session_id`` is accepted but only ever surfaced as ``session_id_hash``. + ``worktree_path`` collapses to ``worktree_category``. Any non-allowlisted + key, or a value that still contains redacted material after scrubbing, is + dropped. + """ + raw: dict[str, Any] = dict(kwargs) + + # Hash the session id — never emit it raw. + session_id = raw.pop("session_id", None) + if session_id and "session_id_hash" not in raw: + raw["session_id_hash"] = hash_session_id(session_id) + + # A full worktree path collapses to a category tag. + wt = raw.pop("worktree_path", None) + if wt and "worktree_category" not in raw: + raw["worktree_category"] = sanitize_path(wt) + + out: dict[str, str] = {} + for key, val in raw.items(): + if key not in ALLOWED_TAG_KEYS: + continue + if val is None: + continue + scrubbed = redact_value(val) + text = str(scrubbed) + if not text or REDACTED in text or REDACTED_PATH in text: + continue + if len(text) > 200: + text = text[:200] + "…" + out[key] = text + return out + + +def scrub_event(event: Any, hint: Any = None) -> dict[str, Any] | None: + """Sentry ``before_send`` / ``before_send_log`` hook. + + Recursively redacts the outgoing event. On *any* failure it returns ``None`` + so the event is dropped rather than sent unscrubbed (fail closed for + redaction). + """ + try: + if not isinstance(event, dict): + return None + scrubbed = redact_value(event) + # Drop server_name if it leaked a hostname/path; PID is kept via tags. + scrubbed.pop("server_name", None) + return scrubbed + except Exception: + return None + + +# ── Event builders (pure, independently testable) ─────────────────────────── +def build_blocker_event( + blocker_type: str, + *, + message: str | None = None, + next_action: str | None = None, + level: str = "warning", + tags: dict[str, Any] | None = None, + extra: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Build a redacted, structured Sentry event for a workflow blocker. + + ``next_action`` maps the issue's "canonical next action when available" + requirement (acceptance criterion 7). + """ + merged_tags = dict(tags or {}) + merged_tags.setdefault("blocker_type", blocker_type) + safe_tags = build_tags(**merged_tags) + + safe_extra = redact_value(dict(extra or {})) + if next_action: + # A short canonical next action is allowed (it is not a full prompt). + safe_extra["canonical_next_action"] = redact_value(str(next_action)[:500]) + + event: dict[str, Any] = { + "message": redact_value(message or blocker_type), + "level": level if level in ("debug", "info", "warning", "error", "fatal") else "warning", + "logger": "gitea-mcp.workflow", + "tags": safe_tags, + "extra": safe_extra, + "fingerprint": ["workflow-blocker", blocker_type], + } + return event + + +def build_checkin_payload( + monitor: str, + status: str, + *, + check_in_id: str | None = None, + duration: float | None = None, +) -> dict[str, Any]: + """Build a Sentry cron check-in payload for one of :data:`MONITOR_SLUGS`. + + ``monitor`` may be a registry key (e.g. ``"stale_lease_scan"``) or an + explicit slug. Raises ``ValueError`` on an unknown status so callers cannot + silently send a malformed check-in. + """ + if status not in _CHECKIN_STATUSES: + raise ValueError( + f"invalid check-in status {status!r}; expected one of {sorted(_CHECKIN_STATUSES)}" + ) + slug = MONITOR_SLUGS.get(monitor, monitor) + payload: dict[str, Any] = {"monitor_slug": slug, "status": status} + if check_in_id: + payload["check_in_id"] = str(check_in_id) + if duration is not None: + try: + payload["duration"] = float(duration) + except (TypeError, ValueError): + pass + return payload + + +# ── Runtime init + capture (fail open) ────────────────────────────────────── +_STATE: dict[str, Any] = {"initialized": False, "config": None} + + +def is_initialized() -> bool: + return bool(_STATE.get("initialized")) + + +def active_config() -> SentryConfig | None: + return _STATE.get("config") + + +def reset_for_tests() -> None: + """Clear module init state. Test-only helper (never called in production).""" + _STATE["initialized"] = False + _STATE["config"] = None + + +def init_sentry(config: SentryConfig | None = None) -> dict[str, Any]: + """Initialise the Sentry SDK if (and only if) enabled + DSN + SDK present. + + Idempotent and never raises. Returns an operator-safe status dict (no DSN + value). Behaviour is unchanged when the feature is off. + """ + cfg = config or load_config() + status: dict[str, Any] = {"initialized": False, **cfg.safe_summary()} + try: + if not cfg.active: + status["reason"] = "disabled (MCP_SENTRY_ENABLED false or SENTRY_DSN empty)" + _STATE["config"] = cfg + return status + if not sdk_available(): + status["reason"] = "sentry_sdk not installed" + _STATE["config"] = cfg + return status + + init_kwargs: dict[str, Any] = { + "dsn": cfg.dsn, + "environment": cfg.environment, + "release": cfg.release, + "traces_sample_rate": cfg.traces_sample_rate, + "before_send": scrub_event, + "send_default_pii": False, + } + if cfg.enable_logs: + # sentry-sdk 2.x captures Python logs as structured logs when the + # experimental logs feature is enabled; scrub those too. + init_kwargs["_experiments"] = { + "enable_logs": True, + "before_send_log": scrub_event, + } + sentry_sdk.init(**init_kwargs) # type: ignore[union-attr] + _STATE["initialized"] = True + _STATE["config"] = cfg + status["initialized"] = True + status["reason"] = "sentry initialised" + except Exception as exc: # fail open: observability must not block startup + status["reason"] = f"init failed (ignored): {type(exc).__name__}" + _STATE["initialized"] = False + return status + + +def _set_scope_tags(scope: Any, tags: dict[str, str]) -> None: + for key, val in tags.items(): + try: + scope.set_tag(key, val) + except Exception: + pass + + +def capture_workflow_blocker( + blocker_type: str, + *, + message: str | None = None, + next_action: str | None = None, + level: str = "warning", + tags: dict[str, Any] | None = None, + extra: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Capture a fail-closed workflow blocker as a structured Sentry event. + + Always returns the redacted event dict (so callers/tests can inspect it), + and sends it to Sentry only when initialised. Fail open. + """ + event = build_blocker_event( + blocker_type, + message=message, + next_action=next_action, + level=level, + tags=tags, + extra=extra, + ) + try: + if is_initialized() and sdk_available(): + sentry_sdk.capture_event(event) # type: ignore[union-attr] + except Exception: + pass + return event + + +def capture_exception( + exc: BaseException, + *, + tags: dict[str, Any] | None = None, + extra: dict[str, Any] | None = None, +) -> bool: + """Capture a runtime exception with scrubbed tags. Fail open. + + Returns True only when the event was handed to an initialised SDK. + """ + try: + if not (is_initialized() and sdk_available()): + return False + safe_tags = build_tags(**(tags or {})) + safe_extra = redact_value(dict(extra or {})) + with sentry_sdk.push_scope() as scope: # type: ignore[union-attr] + _set_scope_tags(scope, safe_tags) + for key, val in safe_extra.items(): + try: + scope.set_extra(key, val) + except Exception: + pass + sentry_sdk.capture_exception(exc) # type: ignore[union-attr] + return True + except Exception: + return False + + +def monitor_checkin( + monitor: str, + status: str, + *, + check_in_id: str | None = None, + duration: float | None = None, +) -> dict[str, Any] | None: + """Send a Sentry cron check-in for a watchdog job. Fail open. + + Returns the payload (for inspection/tests), or ``None`` if the status was + invalid. Only transmits when initialised. + """ + try: + payload = build_checkin_payload( + monitor, status, check_in_id=check_in_id, duration=duration + ) + except ValueError: + return None + try: + if is_initialized() and sdk_available() and hasattr(sentry_sdk, "capture_checkin"): + sentry_sdk.capture_checkin(**payload) # type: ignore[union-attr] + except Exception: + pass + return payload diff --git a/tests/test_sentry_observability.py b/tests/test_sentry_observability.py new file mode 100644 index 0000000..a5461b2 --- /dev/null +++ b/tests/test_sentry_observability.py @@ -0,0 +1,412 @@ +"""Tests for optional self-hosted Sentry observability (#606). + +Covers the pure module (config, redaction, event/check-in builders) and the +runtime capture paths using a fake ``sentry_sdk``, so nothing ever touches the +network. Critically proves the feature is a no-op when disabled or DSN-less +(acceptance criterion 1) and that secrets/paths/session-state are never sent +(criterion 5). +""" + +import sys +import contextlib +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import sentry_observability as so # noqa: E402 + + +# ── Fake SDK ──────────────────────────────────────────────────────────────── +class _FakeScope: + def __init__(self): + self.tags = {} + self.extras = {} + + def set_tag(self, k, v): + self.tags[k] = v + + def set_extra(self, k, v): + self.extras[k] = v + + +class FakeSentrySDK: + """Minimal stand-in exposing the SDK surface sentry_observability uses.""" + + def __init__(self): + self.init_kwargs = None + self.events = [] + self.exceptions = [] + self.checkins = [] + self.last_scope = None + + def init(self, **kwargs): + self.init_kwargs = kwargs + + def capture_event(self, event): + self.events.append(event) + + def capture_exception(self, exc): + self.exceptions.append(exc) + + def capture_checkin(self, **payload): + self.checkins.append(payload) + + @contextlib.contextmanager + def push_scope(self): + self.last_scope = _FakeScope() + yield self.last_scope + + +@pytest.fixture(autouse=True) +def _reset_state(): + """Isolate module init state between tests.""" + so.reset_for_tests() + yield + so.reset_for_tests() + + +@pytest.fixture +def fake_sdk(monkeypatch): + sdk = FakeSentrySDK() + monkeypatch.setattr(so, "sentry_sdk", sdk) + return sdk + + +# ── Config / gating ───────────────────────────────────────────────────────── +def test_disabled_by_default_empty_env(): + cfg = so.load_config(env={}) + assert cfg.enabled is False + assert cfg.active is False + + +def test_enabled_flag_without_dsn_is_not_active(): + cfg = so.load_config(env={"MCP_SENTRY_ENABLED": "1"}) + assert cfg.enabled is True + assert cfg.dsn is None + assert cfg.active is False # DSN required + + +def test_dsn_without_enabled_flag_is_not_active(): + cfg = so.load_config(env={"SENTRY_DSN": "https://k@sentry.prgs.cc/1"}) + assert cfg.active is False + + +def test_active_requires_enabled_and_dsn(): + cfg = so.load_config( + env={"MCP_SENTRY_ENABLED": "true", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + assert cfg.active is True + + +def test_truthy_variants(): + for val in ("1", "true", "YES", "On"): + cfg = so.load_config(env={"MCP_SENTRY_ENABLED": val}) + assert cfg.enabled is True + for val in ("0", "false", "no", "", "off"): + cfg = so.load_config(env={"MCP_SENTRY_ENABLED": val}) + assert cfg.enabled is False + + +def test_traces_sample_rate_parsed_and_clamped(): + assert so.load_config(env={"MCP_SENTRY_TRACES_SAMPLE_RATE": "0.25"}).traces_sample_rate == 0.25 + assert so.load_config(env={"MCP_SENTRY_TRACES_SAMPLE_RATE": "5"}).traces_sample_rate == 1.0 + assert so.load_config(env={"MCP_SENTRY_TRACES_SAMPLE_RATE": "-1"}).traces_sample_rate == 0.0 + assert so.load_config(env={"MCP_SENTRY_TRACES_SAMPLE_RATE": "junk"}).traces_sample_rate == 0.0 + + +def test_safe_summary_has_no_dsn_value(): + cfg = so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://secret@sentry.prgs.cc/1"} + ) + summary = cfg.safe_summary() + assert summary["dsn_present"] is True + assert "secret" not in repr(summary) + assert "dsn" not in summary # only presence, never the value + + +# ── init_sentry ───────────────────────────────────────────────────────────── +def test_init_noop_when_disabled(fake_sdk): + status = so.init_sentry(so.load_config(env={})) + assert status["initialized"] is False + assert fake_sdk.init_kwargs is None # no SDK init + assert so.is_initialized() is False + + +def test_init_noop_when_enabled_but_missing_dsn(fake_sdk): + status = so.init_sentry(so.load_config(env={"MCP_SENTRY_ENABLED": "1"})) + assert status["initialized"] is False + assert fake_sdk.init_kwargs is None + + +def test_init_reports_missing_sdk(monkeypatch): + monkeypatch.setattr(so, "sentry_sdk", None) + status = so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + assert status["initialized"] is False + assert "not installed" in status["reason"] + + +def test_init_configures_sdk_with_scrubber(fake_sdk): + status = so.init_sentry( + so.load_config( + env={ + "MCP_SENTRY_ENABLED": "1", + "SENTRY_DSN": "https://k@sentry.prgs.cc/1", + "SENTRY_ENVIRONMENT": "prod", + "MCP_SENTRY_TRACES_SAMPLE_RATE": "0.1", + } + ) + ) + assert status["initialized"] is True + assert so.is_initialized() is True + kw = fake_sdk.init_kwargs + assert kw["dsn"] == "https://k@sentry.prgs.cc/1" + assert kw["environment"] == "prod" + assert kw["traces_sample_rate"] == 0.1 + assert kw["before_send"] is so.scrub_event + assert kw["send_default_pii"] is False + + +def test_init_enable_logs_wires_log_scrubber(fake_sdk): + so.init_sentry( + so.load_config( + env={ + "MCP_SENTRY_ENABLED": "1", + "SENTRY_DSN": "https://k@sentry.prgs.cc/1", + "MCP_SENTRY_ENABLE_LOGS": "1", + } + ) + ) + exp = fake_sdk.init_kwargs["_experiments"] + assert exp["enable_logs"] is True + assert exp["before_send_log"] is so.scrub_event + + +def test_init_never_raises_on_sdk_failure(monkeypatch): + class Boom: + def init(self, **kwargs): + raise RuntimeError("sentry down") + + monkeypatch.setattr(so, "sentry_sdk", Boom()) + status = so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + assert status["initialized"] is False + assert "init failed" in status["reason"] + + +# ── Redaction (fail closed) ───────────────────────────────────────────────── +def test_build_tags_allowlist_only(): + tags = so.build_tags(role="author", secret_thing="leak", pid=123) + assert tags["role"] == "author" + assert tags["pid"] == "123" + assert "secret_thing" not in tags + + +def test_build_tags_hashes_session_id(): + tags = so.build_tags(session_id="prgs-author-20479-cf9ac178") + assert "session_id" not in tags + assert "session_id_hash" in tags + assert tags["session_id_hash"] != "prgs-author-20479-cf9ac178" + assert len(tags["session_id_hash"]) == 12 + + +def test_build_tags_collapses_worktree_path(): + tags = so.build_tags( + worktree_path="/Users/x/Development/Gitea-Tools/branches/issue-606-sentry-observability" + ) + assert "worktree_path" not in tags + assert tags["worktree_category"] == "author" + + +def test_build_tags_drops_value_that_scrubs_to_redacted(): + # A tag value that is itself a token gets scrubbed then dropped. + tags = so.build_tags(capability="token abcdef1234567890") + assert "capability" not in tags + + +def test_sanitize_path_categories(): + assert so.sanitize_path("/repo/branches/review-pr-654") == "reviewer" + assert so.sanitize_path("/repo/branches/merge-pr-1") == "merger" + assert so.sanitize_path("/repo/branches/reconcile-pr-1") == "reconciler" + assert so.sanitize_path("/repo/branches/feat-issue-606") == "author" + assert so.sanitize_path("/x/y/Gitea-Tools") == "root" + + +def test_redact_value_scrubs_secrets_and_paths(): + out = so.redact_value( + { + "token": "abc123", + "note": "Authorization: Bearer sk_live_abcdefgh12345", + "path": "/Users/jasonwalker/Development/Gitea-Tools/secret", + "dsn": "https://k@sentry.prgs.cc/1", + "safe": "hello", + } + ) + assert out["token"] == so.REDACTED + assert out["dsn"] == so.REDACTED + assert "sk_live" not in out["note"] + assert so.REDACTED_PATH in out["path"] + assert "/Users/" not in out["path"] + assert out["safe"] == "hello" + + +def test_redact_value_forbidden_prompt_and_session_state(): + out = so.redact_value( + {"prompt": "full body", "session_state": "{...}", "keep": "ok"} + ) + assert out["prompt"] == so.REDACTED + assert out["session_state"] == so.REDACTED + assert out["keep"] == "ok" + + +def test_scrub_event_redacts_nested_and_drops_server_name(): + event = { + "server_name": "some-host", + "message": "boom", + "extra": {"token": "leak", "ok": "1"}, + } + scrubbed = so.scrub_event(event) + assert "server_name" not in scrubbed + assert scrubbed["extra"]["token"] == so.REDACTED + assert scrubbed["extra"]["ok"] == "1" + + +def test_scrub_event_drops_non_dict(): + assert so.scrub_event("not a dict") is None + assert so.scrub_event(None) is None + + +# ── Event builders ────────────────────────────────────────────────────────── +def test_build_blocker_event_structure_and_next_action(): + event = so.build_blocker_event( + "active_foreign_lease", + message="blocked by foreign lease", + next_action="wait or adopt via allocator", + level="warning", + tags={"pr_number": 606, "session_id": "s-123"}, + ) + assert event["tags"]["blocker_type"] == "active_foreign_lease" + assert event["tags"]["pr_number"] == "606" + assert "session_id" not in event["tags"] + assert event["tags"]["session_id_hash"] + assert event["extra"]["canonical_next_action"] == "wait or adopt via allocator" + assert event["fingerprint"] == ["workflow-blocker", "active_foreign_lease"] + + +def test_build_blocker_event_invalid_level_defaults_warning(): + event = so.build_blocker_event("x", level="nonsense") + assert event["level"] == "warning" + + +def test_build_checkin_payload_maps_slugs(): + for key, slug in so.MONITOR_SLUGS.items(): + payload = so.build_checkin_payload(key, "ok") + assert payload["monitor_slug"] == slug + assert payload["status"] == "ok" + + +def test_build_checkin_payload_explicit_slug_passthrough(): + payload = so.build_checkin_payload("custom-slug", "in_progress", duration=1.5) + assert payload["monitor_slug"] == "custom-slug" + assert payload["duration"] == 1.5 + + +def test_build_checkin_payload_rejects_bad_status(): + with pytest.raises(ValueError): + so.build_checkin_payload("allocator_health", "bogus") + + +def test_all_six_monitors_registered(): + assert set(so.MONITOR_SLUGS) == { + "stale_lease_scan", + "terminal_lock_scan", + "allocator_health", + "namespace_health", + "dashboard_freshness", + "reconciler_cleanup", + } + + +# ── Capture paths (fail open) ─────────────────────────────────────────────── +def test_capture_workflow_blocker_noop_when_disabled(fake_sdk): + # not initialised + event = so.capture_workflow_blocker("some_blocker", message="x") + assert isinstance(event, dict) # still returns redacted event + assert fake_sdk.events == [] # but nothing sent + + +def test_capture_workflow_blocker_sends_when_initialised(fake_sdk): + so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + so.capture_workflow_blocker("terminal_lock_occupied", message="held") + assert len(fake_sdk.events) == 1 + assert fake_sdk.events[0]["tags"]["blocker_type"] == "terminal_lock_occupied" + + +def test_capture_exception_noop_when_disabled(fake_sdk): + assert so.capture_exception(ValueError("x")) is False + assert fake_sdk.exceptions == [] + + +def test_capture_exception_sends_scrubbed_tags(fake_sdk): + so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + ok = so.capture_exception( + RuntimeError("bad"), tags={"mutation_tool": "gitea_merge_pr", "leaky": "x"} + ) + assert ok is True + assert len(fake_sdk.exceptions) == 1 + assert fake_sdk.last_scope.tags["mutation_tool"] == "gitea_merge_pr" + assert "leaky" not in fake_sdk.last_scope.tags + + +def test_capture_exception_never_raises(monkeypatch, fake_sdk): + so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + + def boom(exc): + raise RuntimeError("sdk exploded") + + monkeypatch.setattr(fake_sdk, "capture_exception", boom) + # Must swallow the SDK failure (fail open). + assert so.capture_exception(ValueError("y")) is False + + +def test_monitor_checkin_noop_when_disabled(fake_sdk): + payload = so.monitor_checkin("allocator_health", "ok") + assert payload["monitor_slug"] == "gitea-mcp-allocator-health" + assert fake_sdk.checkins == [] # not sent while disabled + + +def test_monitor_checkin_sends_when_initialised(fake_sdk): + so.init_sentry( + so.load_config( + env={"MCP_SENTRY_ENABLED": "1", "SENTRY_DSN": "https://k@sentry.prgs.cc/1"} + ) + ) + so.monitor_checkin("stale_lease_scan", "ok") + assert fake_sdk.checkins == [ + {"monitor_slug": "gitea-mcp-stale-lease-scan", "status": "ok"} + ] + + +def test_monitor_checkin_invalid_status_returns_none(fake_sdk): + assert so.monitor_checkin("allocator_health", "bogus") is None + assert fake_sdk.checkins == [] -- 2.43.7