_get_connection hard-codes connect_timeout=DEFAULT_MARIADB_TIMEOUT_S (5s) while the config’s timeout_seconds default is 10s and is used for read/write timeouts. This makes connect behavior inconsistent with the configured timeout and with the PR’s “timeouts enforced” claim. Consider deriving connect_timeout from config.timeout_seconds (or exposing a separate connect_timeout_seconds field) so all timeouts are consistently configurable.

When config.ssl is true, the SSL options explicitly disable hostname verification (check_hostname: False). That weakens TLS protections and can allow MITM if certificates are otherwise accepted. Prefer leaving hostname verification enabled by default and only disabling it via an explicit opt-out setting (or require a CA bundle / verify mode configuration).

validate_mariadb_config only checks that host is set, but the rest of the integration (tools’ is_available and MariaDBConfig.is_configured) requires both host and database. This can lead to opensre integrations verify mariadb reporting success even when the DB name is missing and the tools won’t be available. Align validation with tool requirements by also requiring database (and likely username) to be non-empty for a “passed” result.

get_replication_status executes SHOW ALL SLAVES STATUS but only reads a single row via fetchone(). On multi-source replication setups this will silently drop additional channels, contradicting the stated multi-source support. Consider fetching all rows and returning a list of channel statuses (keyed by Connection_name) or aggregating them into a structured result.

detect_sources adds a mariadb source when only host is present, but the MariaDB tools’ availability/config checks require both host and database. This can yield a partially-populated sources['mariadb'] with connection_verified=True while the tools still won’t show/run. Consider requiring a non-empty database here (or aligning mariadb_is_available/MariaDBConfig.is_configured to only require host if that’s intended).

In the env-var fallback branch, resolve_effective_integrations treats MARIADB_HOST alone as sufficient to include MariaDB in the effective integrations, but the rest of the integration requires a non-empty database (and typically username). This can make verify/display flows report MariaDB as configured even when the tools will be unavailable. Consider only adding the env-backed MariaDB entry when the required fields are present, or make verification fail with a clear message when they’re missing.

The repo has integration-level E2E coverage for other database integrations (e.g., tests/e2e/mongodb/test_mongodb_e2e.py covers resolution + verify + detect_sources + tools availability), but this PR only adds unit/contract tests for MariaDB. Consider adding a similar tests/e2e/mariadb/ suite to validate the full wiring (env/store resolution → verify → detect_sources → tool availability) and prevent regressions in the end-to-end investigation flow.

Multi-source replication silently returns only the first replica

SHOW ALL SLAVES STATUS returns one row per master connection in a multi-source setup, but fetchone() discards every row after the first. The PR description explicitly claims multi-source support, so this is a present functional defect — any replica beyond the first is silently dropped.

After this change, iterate rows to build a list of replica dicts, return "replicas": [...] instead of "replication": {}.

This is a comment left during a code review.
Path: app/integrations/mariadb.py
Line: 366-378

Comment:
**Multi-source replication silently returns only the first replica**

`SHOW ALL SLAVES STATUS` returns one row per master connection in a multi-source setup, but `fetchone()` discards every row after the first. The PR description explicitly claims multi-source support, so this is a present functional defect — any replica beyond the first is silently dropped.

```suggestion
                rows = cur.fetchall() or []
                if cur.description:
                    columns = [d[0] for d in cur.description]
                break
```

After this change, iterate `rows` to build a list of replica dicts, return `"replicas": [...]` instead of `"replication": {}`.

How can I resolve this? If you propose a fix, please make it concise.

check_hostname is not a recognized pymysql SSL dict key

pymysql's ssl parameter accepts ca, cert, key, capath, and cipher. The key check_hostname is silently ignored, so hostname-verification behavior is undefined. Passing an empty dict {} or None is clearer and achieves the same "connect with TLS, no cert verification" effect that appears to be the intent here.

This is a comment left during a code review.
Path: app/integrations/mariadb.py
Line: 96-99

Comment:
**`check_hostname` is not a recognized pymysql SSL dict key**

pymysql's `ssl` parameter accepts `ca`, `cert`, `key`, `capath`, and `cipher`. The key `check_hostname` is silently ignored, so hostname-verification behavior is undefined. Passing an empty dict `{}` or `None` is clearer and achieves the same "connect with TLS, no cert verification" effect that appears to be the intent here.

```suggestion
        ssl_arg: dict[str, Any] | None = {} if config.ssl else None
```

How can I resolve this? If you propose a fix, please make it concise.

integration_id not propagated for MariaDB

Every other integration in _classify_integrations (Grafana, Datadog, Honeycomb, Coralogix, Sentry, Vercel, OpsGenie) passes "integration_id": integration.get("id", "") when building its config object. The MariaDB block omits this, so resolved["mariadb"]["integration_id"] is always "" for store-backed configurations.

This is a comment left during a code review.
Path: app/nodes/resolve_integrations/node.py
Line: 229-242

Comment:
**`integration_id` not propagated for MariaDB**

Every other integration in `_classify_integrations` (Grafana, Datadog, Honeycomb, Coralogix, Sentry, Vercel, OpsGenie) passes `"integration_id": integration.get("id", "")` when building its config object. The MariaDB block omits this, so `resolved["mariadb"]["integration_id"]` is always `""` for store-backed configurations.

```suggestion
            try:
                mariadb_config = build_mariadb_config({
                    "host": credentials.get("host", ""),
                    "port": credentials.get("port", 3306),
                    "database": credentials.get("database", ""),
                    "username": credentials.get("username", ""),
                    "password": credentials.get("password", ""),
                    "ssl": credentials.get("ssl", True),
                    "integration_id": integration.get("id", ""),
                })
```

How can I resolve this? If you propose a fix, please make it concise.

OR SCHEMA_NAME IS NULL may surface unrelated system queries

Rows with SCHEMA_NAME IS NULL in events_statements_summary_by_digest represent cross-schema or administrative statements (SHOW STATUS, SET NAMES, SELECT @@version, etc.) that are unrelated to the target application database. If these statements have a high AVG_TIMER_WAIT they can displace genuinely slow application queries in the results.

Consider removing the OR SCHEMA_NAME IS NULL clause unless including system-level statements is intentional:

This is a comment left during a code review.
Path: app/integrations/mariadb.py
Line: 323-326

Comment:
**`OR SCHEMA_NAME IS NULL` may surface unrelated system queries**

Rows with `SCHEMA_NAME IS NULL` in `events_statements_summary_by_digest` represent cross-schema or administrative statements (`SHOW STATUS`, `SET NAMES`, `SELECT @@version`, etc.) that are unrelated to the target application database. If these statements have a high `AVG_TIMER_WAIT` they can displace genuinely slow application queries in the results.

Consider removing the `OR SCHEMA_NAME IS NULL` clause unless including system-level statements is intentional:

```suggestion
                    WHERE SCHEMA_NAME = %s
```

How can I resolve this? If you propose a fix, please make it concise.

_parse_port does not exist — entire test file fails with ImportError

app.integrations.cli has no _parse_port function. Every test in TestParsePort will raise ImportError at collection time. The PR description claims "all MariaDB tests pass (57 new tests)", but this file cannot even be imported.

The actual port handling in _setup_mariadb uses int(port) if port.isdigit() else 3306, which also doesn't match the tests: "0".isdigit() and "65536".isdigit() both return True, so out-of-range ports are stored rather than falling back to the default.

_parse_port needs to be defined in app/integrations/cli.py and wired into _setup_mariadb:

def _parse_port(value: str, default: int = 3306) -> int:
    try:
        port = int(value)
    except (ValueError, TypeError):
        return default
    return port if 1 <= port <= 65535 else default

This is a comment left during a code review.
Path: tests/cli/test_mariadb_cli.py
Line: 7

Comment:
**`_parse_port` does not exist — entire test file fails with `ImportError`**

`app.integrations.cli` has no `_parse_port` function. Every test in `TestParsePort` will raise `ImportError` at collection time. The PR description claims "all MariaDB tests pass (57 new tests)", but this file cannot even be imported.

The actual port handling in `_setup_mariadb` uses `int(port) if port.isdigit() else 3306`, which also doesn't match the tests: `"0".isdigit()` and `"65536".isdigit()` both return `True`, so out-of-range ports are stored rather than falling back to the default.

`_parse_port` needs to be defined in `app/integrations/cli.py` and wired into `_setup_mariadb`:

```python
def _parse_port(value: str, default: int = 3306) -> int:
    try:
        port = int(value)
    except (ValueError, TypeError):
        return default
    return port if 1 <= port <= 65535 else default
```

How can I resolve this? If you propose a fix, please make it concise.

Passing an empty dict to ssl= in pymysql encrypts the channel but skips certificate verification. An attacker on the network can MITM the connection. For a tool that transmits database credentials, this is a meaningful risk.

Fix: pass {"ssl_verify_cert": True, "ssl_ca": "/etc/ssl/certs/ca-certificates.crt"}, or at minimum expose a ssl_verify_cert config option and document the default behavior.

Good point, you're right that an empty dict skips verification. Changed it to pass ssl_verify_cert: True so pymysql actually validates the server certificate against system CAs.

Correct for new files. However, a store file created by an older version (e.g. 0o644) stays world-readable until the next write. Consider adding a read-time check: if the file exists and its mode is not 0o600, fix it immediately before returning data.

A bare except Exception here will silently swallow timeouts and connection-lost errors on SHOW ALL SLAVES STATUS and attempt SHOW SLAVE STATUS on a dead cursor. Should catch only the syntax/unsupported error:
except pymysql.err.ProgrammingError:

performance_schema timers count in picoseconds (10⁻¹²). Dividing by 10¹² gives seconds, not milliseconds. The column name avg_time_ms is wrong by 1000×.
Fix: divide by 1000000000 (10⁹) for ms, or rename the columns to avg_time_s.
eg
ROUND(AVG_TIMER_WAIT / 1000000000, 4) AS avg_time_ms,
ROUND(SUM_TIMER_WAIT / 1000000000, 4) AS total_time_ms,

Bare int() crashes on invalid MARIADB_PORT

If MARIADB_HOST is set and MARIADB_PORT contains a non-numeric value (e.g. "localhost:3306", "abc"), int(...) raises ValueError with no surrounding try/except, crashing resolve_effective_integrations() entirely — breaking verification for all integrations, not just MariaDB. The parallel path in node.py's _load_env_integrations avoids this by routing through build_mariadb_config, which handles coercion gracefully via _normalize_port.

Reuse the existing helper:

from app.integrations.cli import _parse_port
...
"port": _parse_port(os.getenv("MARIADB_PORT", "3306").strip()),

This is a comment left during a code review.
Path: app/integrations/verify.py
Line: 307

Comment:
**Bare `int()` crashes on invalid `MARIADB_PORT`**

If `MARIADB_HOST` is set and `MARIADB_PORT` contains a non-numeric value (e.g. `"localhost:3306"`, `"abc"`), `int(...)` raises `ValueError` with no surrounding try/except, crashing `resolve_effective_integrations()` entirely — breaking verification for all integrations, not just MariaDB. The parallel path in `node.py`'s `_load_env_integrations` avoids this by routing through `build_mariadb_config`, which handles coercion gracefully via `_normalize_port`.

Reuse the existing helper:
```python
from app.integrations.cli import _parse_port
...
"port": _parse_port(os.getenv("MARIADB_PORT", "3306").strip()),
```

How can I resolve this? If you propose a fix, please make it concise.

Fixed — using _parse_port now which handles invalid values gracefully instead of bare int().

Fixed — removed the circular import. Using inline safe parsing now instead of importing from cli.py.