From 3ea43adcfd53c215113839579b4d69ea1b28836f Mon Sep 17 00:00:00 2001 From: dtoro Date: Wed, 8 Jul 2026 12:27:25 +0200 Subject: [PATCH] fix: repair 5 MCP analysis tools with SQL errors + seed entity_status rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - get_event_timeline: event_type/actor/message → type/source/data::text - get_blast_radius: add JOIN entities for slug column - get_state_snapshot: remove nonexistent disk_usage_pct, drift_count - get_change_history: timestamp/actor_label/details → ts/actor_id::text/detail - seed: upsert entity_status rows during inventory ingest (was empty, causing INNER JOIN on get_health_summary to return nothing) --- internal/db/seed.go | 12 + internal/mcp/server.go | 16 +- .../done/2026-07-08-fix-mcp-analysis-tools.md | 228 ++++++++++++++++++ plans/index.md | 1 + 4 files changed, 248 insertions(+), 9 deletions(-) create mode 100644 plans/done/2026-07-08-fix-mcp-analysis-tools.md diff --git a/internal/db/seed.go b/internal/db/seed.go index 8c1a352..5aead93 100644 --- a/internal/db/seed.go +++ b/internal/db/seed.go @@ -131,6 +131,18 @@ func IngestInventorySeed(ctx context.Context, tx pgx.Tx, data map[string]any) (* if err != nil { return nil, fmt.Errorf("entity %s: %w", slug, err) } + + // Seed initial entity_status row so health queries return + // results even before the scheduler populates check results. + _, err = tx.Exec(ctx, + `INSERT INTO entity_status (entity_id, health, updated_at) + VALUES ($1, 'unknown', now()) + ON CONFLICT (entity_id) DO NOTHING`, + entityID) + if err != nil { + return nil, fmt.Errorf("entity_status %s: %w", slug, err) + } + r.Entities++ } diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 39983fc..37fd271 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -113,7 +113,7 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { slug, _ := args["entity_id"].(string) depth := int(getFloat(args, "depth", 3)) return queryRows(ctx, pool, - "SELECT slug, CAST(depth AS int) FROM blast_radius((SELECT id FROM entities WHERE slug = $1), $2)", + "SELECT e.slug, CAST(b.depth AS int) FROM blast_radius((SELECT id FROM entities WHERE slug = $1), $2) b JOIN entities e ON e.id = b.entity_id", slug, depth), nil }) @@ -430,8 +430,8 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { args := argsMap(req) limit := int(getFloat(args, "limit", 50)) return queryRows(ctx, pool, ` - SELECT ev.ts, ev.event_type, ev.severity, ev.actor, e.slug AS entity_slug, - ev.message, ev.correlation_id + SELECT ev.ts, ev.type, ev.severity, ev.source, e.slug AS entity_slug, + ev.data::text AS message, ev.correlation_id FROM events ev LEFT JOIN entities e ON e.id = ev.entity_id WHERE ($1::text IS NULL OR ev.severity = $1) @@ -676,13 +676,13 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { slug, _ := args["entity_slug"].(string) limit := int(getFloat(args, "limit", 20)) return queryRows(ctx, pool, ` - SELECT al.timestamp, al.actor_type, al.actor_label, + SELECT al.ts AS timestamp, al.actor_type, al.actor_id::text AS actor_label, al.action, al.method, al.path, - al.details::text AS details + al.detail::text AS details FROM audit_log al JOIN entities e ON e.id = al.entity_id WHERE e.slug = $1 - ORDER BY al.timestamp DESC + ORDER BY al.ts DESC LIMIT $2`, slug, limit), nil }) @@ -692,9 +692,7 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { return queryRows(ctx, pool, ` SELECT e.slug, e.type, e.state, COALESCE(st.health, 'unknown') AS health, - COALESCE(st.last_check_at::text, '') AS last_check, - COALESCE(st.disk_usage_pct, 0) AS disk_pct, - COALESCE(st.drift_count, 0) AS drift_count + COALESCE(st.last_check_at::text, '') AS last_check FROM entities e LEFT JOIN entity_status st ON st.entity_id = e.id ORDER BY st.health, e.slug diff --git a/plans/done/2026-07-08-fix-mcp-analysis-tools.md b/plans/done/2026-07-08-fix-mcp-analysis-tools.md new file mode 100644 index 0000000..6f079a9 --- /dev/null +++ b/plans/done/2026-07-08-fix-mcp-analysis-tools.md @@ -0,0 +1,228 @@ +# 2026-07-08 — Fix MCP analysis tools (SQL errors + missing probe data) + +**Status:** Done + +## Goal + +Fix MCP tools that fail silently or crash with SQL errors, and close the +gap that leaves all health data NULL. Hermes testing against `host:strong` +revealed the static read tools work but analysis/health tools are dead. + +--- + +## Full MCP tool test results (2026-07-08) + +Tested by running each MCP tool's SQL directly against the running dev DB +(docker exec oikos-postgres-1 psql). DB state: 168 entities, 194 +relationships, 54 knowledge docs, 40 agent activity rows. entity_status +has 0 rows, check_defs has 0. + +### ✅ Working (SQL valid, data present or empty as expected) + +| Tool | Notes | +|---|---| +| get_entity | Full attributes returned | +| list_entities | Filters work | +| get_relations | 194 edges. strong → 7 LXC hosts edges confirmed | +| search_knowledge | PostgreSQL FTS with ts_rank, returns ranked docs | +| get_entity_knowledge | Complex UNION query works, returns docs + investigations | +| list_lxcs | Returns all 33 LXCs with PVE IDs and IPs | +| get_lxc_state | SSH to Proxmox, confirmed working (agent_activity has 40 logs) | +| get_service_status | SSH-based, works | +| tail_log | SSH-based, works | +| ping_service | Works (returns unknown health, as expected) | +| get_agent_activity | 40 rows logged | +| get_audit_trail | Correct SQL, 1 audit row | +| get_signal_history | Correct SQL, 0 rows (no signals yet) | +| get_patterns | Correct SQL, 0 rows (no patterns learned yet) | +| get_skills | Correct SQL, 0 rows (no skills seeded) | +| query_metrics | Correct SQL (TimescaleDB functions work), 0 rows | +| get_trend | Correct SQL, 0 rows | +| whoami | Works (ws:mac-mini returns unknown health) | +| explain | Works for host:strong | +| preflight | Works, returns risk_class + approval | +| request_execution | SSH-based execution works | +| get_execution_status | Not tested (~likely works) | +| list_my_secrets | Not tested (~simple entities JOIN) | + +### ❌ Broken — SQL errors on column name mismatch + +| Tool | File:Line | Error | +|---|---|---| +| get_event_timeline | `server.go:433` | `ev.event_type` → column is `type` | +| | `server.go:433` | `ev.actor` → column is `source` | +| | `server.go:434` | `ev.message` → column is `data` (JSONB) | +| get_blast_radius | `server.go:116` | `slug` not in `blast_radius()` return | +| get_state_snapshot | `server.go:696` | `st.disk_usage_pct` doesn't exist | +| | `server.go:697` | `st.drift_count` doesn't exist | +| get_change_history | `server.go:679` | `al.timestamp` → column is `ts` | +| | `server.go:679` | `al.actor_label` doesn't exist | +| | `server.go:681` | `al.details` → column is `detail` | + +### ⚠️ Returns empty (no data, SQL is correct) + +| Tool | Root cause | +|---|---| +| get_health_summary | `INNER JOIN entity_status` — table has 0 rows | +| All health/status fields | No entity_status rows seeded, no check_defs → scheduler idle | + +--- + +## Fix 1 — `get_event_timeline`: wrong column names → SQL error + +**File:** `internal/mcp/server.go:432-440` + +| MCP query column | Actual events column | Migration ref | +|---|---|---| +| `ev.event_type` | `type` | `006_observability.up.sql:83` | +| `ev.actor` | `source` | `006_observability.up.sql:86` | +| `ev.message` | `data` (JSONB) | `006_observability.up.sql:87` | + +**Fix:** Change `ev.event_type` → `ev.type`, `ev.actor` → `ev.source`, +`ev.message` → `ev.data::text AS message`. + +**Risk class:** `reversible_low` — one-line string change. + +--- + +## Fix 2 — `get_blast_radius`: `slug` missing from function result + +**File:** `internal/mcp/server.go:115-117` + +```sql +SELECT slug, CAST(depth AS int) +FROM blast_radius((SELECT id FROM entities WHERE slug = $1), $2) +``` + +`blast_radius()` returns `TABLE(entity_id UUID, depth INT)` per +`migrations/002_entities.up.sql:37`. The `slug` column doesn't exist in +the function return set. + +**Fix:** Join `entities`: + +```sql +SELECT e.slug, CAST(b.depth AS int) +FROM blast_radius((SELECT id FROM entities WHERE slug = $1), $2) b +JOIN entities e ON e.id = b.entity_id +``` + +The `httpapi/impl.go:285` and `integration_test.go:277` already use this +pattern — copy it. + +**Risk class:** `reversible_low` — JOIN added to existing query. + +--- + +## Fix 3 — `get_state_snapshot`: nonexistent columns + +**File:** `internal/mcp/server.go:692-703` + +The `entity_status` table schema (migration `003_operations.up.sql:55`): + +| Column | Type | +|---|---| +| `entity_id` | UUID PK | +| `health` | TEXT | +| `last_check_at` | TIMESTAMPTZ | +| `details` | JSONB | +| `updated_at` | TIMESTAMPTZ | + +No `disk_usage_pct`, no `drift_count`. + +**Fix:** Remove the nonexistent columns: + +```sql +SELECT e.slug, e.type, e.state, + COALESCE(st.health, 'unknown') AS health, + COALESCE(st.last_check_at::text, '') AS last_check +FROM entities e +LEFT JOIN entity_status st ON st.entity_id = e.id +ORDER BY st.health, e.slug +LIMIT 200 +``` + +**Risk class:** `reversible_low` — remove 2 columns from SELECT. + +--- + +## Fix 4 — `get_change_history`: wrong column names + +**File:** `internal/mcp/server.go:678-686` + +The `audit_log` table schema (migration `006_observability.up.sql:53`): + +| MCP query column | Actual column | +|---|---| +| `al.timestamp` | `ts` | +| `al.actor_label` | does not exist (use `al.actor_id::text`) | +| `al.details` | `detail` | + +**Fix:** +```sql +SELECT al.ts AS timestamp, al.actor_type, al.actor_id::text AS actor_label, + al.action, al.method, al.path, + al.detail::text AS details +FROM audit_log al +JOIN entities e ON e.id = al.entity_id +WHERE e.slug = $1 +ORDER BY al.ts DESC +LIMIT $2 +``` + +**Risk class:** `reversible_low` — column name corrections. + +--- + +## Fix 5 — `get_health_summary` returns no rows: seed `entity_status` + +**File:** `internal/db/seed.go` + +Problem: +- `IngestInventorySeed` creates entities but never creates `entity_status` rows. +- `get_health_summary` uses `INNER JOIN entity_status`, so it returns empty. +- The scheduler only populates `entity_status` when `check_defs` exist and + produce probe results. No check_defs → no status rows → empty health. + +**Fix:** After entity creation in `IngestInventorySeed`, upsert a +`entity_status` row with `health='unknown'` for each entity. This gives +the fleet a visible baseline (all unknown) instead of empty results. + +```sql +INSERT INTO entity_status (entity_id, health, updated_at) +VALUES ($1, 'unknown', now()) +ON CONFLICT (entity_id) DO NOTHING +``` + +**Risk class:** `config_mutation` — changes seed behavior; only runs on `oikos seed`. + +--- + +## Fix 6 — Gap: no `check_defs` seeded → control loop idle + +**Not in this plan.** The scheduler at `internal/scheduler/scheduler.go:58` +logs "no enabled check_defs" and exits every cycle. Without check_defs +there are no probe results, no signal lifecycle, and health stays +permanently unknown. + +This requires: +- Define probe targets for each service in `seeds/inventory.yaml` + (HTTP URLs, TCP ports, disk thresholds, cert expiry) +- Add `check_defs` ingestion to `internal/db/seed.go` + +Will be a **separate plan** — it's feature work, not a bug fix. See +`seeds/inventory.yaml` for the existing service URL attributes that can +be translated to check configs. + +--- + +## Verification + +1. Build and deploy to dev. +2. Run `oikos seed` (re-ingest). +3. Call each previously-broken tool via Hermes or direct SQL: + - `get_health_summary` → returns rows (all `unknown` initially). + - `get_event_timeline` → no SQL error (may return empty, which is fine). + - `get_blast_radius` for `host:strong` → returns its 7 LXCs with depth. + - `get_state_snapshot` → returns all 168 entities (all unknown health). + - `get_change_history` → no SQL error (may return empty). +4. `make test` passes. diff --git a/plans/index.md b/plans/index.md index dafeb18..67d514a 100644 --- a/plans/index.md +++ b/plans/index.md @@ -10,6 +10,7 @@ went sideways, open an investigation. | ---- | ----- | ------ | | 2026-07-05 | [Oikos Prometheus LXC](2026-07-05-oikos-prometheus-lxc.md) | Planned | | 2026-07-08 | [Plan vs implementation cross-reference](2026-07-08-plan-implementation-audit.md) | Planned | +| 2026-07-08 | [Fix MCP analysis tools](2026-07-08-fix-mcp-analysis-tools.md) | Done | ## Done