Files
oikos/plans/done/2026-07-08-fix-mcp-analysis-tools.md
dtoro 3ea43adcfd
Some checks failed
ci / build-test (push) Has been cancelled
ci / docker-build (push) Has been cancelled
fix: repair 5 MCP analysis tools with SQL errors + seed entity_status rows
- 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)
2026-07-08 12:27:25 +02:00

229 lines
7.5 KiB
Markdown

# 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.