From 989b9ea2e59c03c63b0776795818da602e82db83 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Tue, 9 Jun 2026 23:14:36 -0700 Subject: [PATCH 01/21] fix(deep): aggregate Consul health metric in the evidence gather MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final link in the chain: the orchestrator now forms the right Consul hypothesis (#256) with a verifiable prediction (#257), but the evidence gather queried the BARE consul_health_service_status metric — which returns one series per (node x status), mixed 0/1 — so the extractor produced zero usable observations and the keystone returned 'absent' ('couldn't verify'). Trace: impala runs kept proposing 'impala-statestore unhealthy' then gathering +0 observations. Fix: planPredictionQuery now appends a Consul-aware hint to the metric-threshold gather prompt — query it AGGREGATED for the service in the hypothesis: max by (service_name) (consul_health_service_status{service_name="X",status="passing"}) → one clean value (1 passing / 0 failing) the keystone can verify → confirm. Only triggers when the metric is consul_health_service_status (no false positives for ordinary metrics). tsc clean; 2 new tests + full suite green. Closes the form→predict→gather→verify chain for bare-metal Consul incidents. --- src/workflows/steps/hypothesis-requery.test.ts | 15 +++++++++++++++ src/workflows/steps/hypothesis-requery.ts | 14 ++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/workflows/steps/hypothesis-requery.test.ts b/src/workflows/steps/hypothesis-requery.test.ts index ac106055..311b3208 100644 --- a/src/workflows/steps/hypothesis-requery.test.ts +++ b/src/workflows/steps/hypothesis-requery.test.ts @@ -97,6 +97,21 @@ describe("planPredictionQuery", () => { expect(plan!.prompt).toContain("T2"); }); + it("adds the Consul aggregation hint for consul_health_service_status so the gather gets a usable value", () => { + const plan = planPredictionQuery( + hyp({ kind: "metric-threshold", metric: "consul_health_service_status", op: "<", value: 1 }), + ); + expect(plan!.role).toBe("metrics"); + expect(plan!.prompt).toContain("max by (service_name)"); + expect(plan!.prompt).toContain("status=\"passing\""); + expect(plan!.prompt).toMatch(/Consul bare-metal health metric/i); + }); + + it("does NOT add the Consul hint for ordinary metrics (no false positives)", () => { + const plan = planPredictionQuery(hyp({ kind: "metric-threshold", metric: "http_p99", op: ">", value: 5 })); + expect(plan!.prompt).not.toContain("max by (service_name)"); + }); + it("maps log-pattern to the logs role and reflects present/absent", () => { const present = planPredictionQuery(hyp({ kind: "log-pattern", pattern: "OOMKilled" })); expect(present!.role).toBe("logs"); diff --git a/src/workflows/steps/hypothesis-requery.ts b/src/workflows/steps/hypothesis-requery.ts index cfe1eff6..00fd2d57 100644 --- a/src/workflows/steps/hypothesis-requery.ts +++ b/src/workflows/steps/hypothesis-requery.ts @@ -92,12 +92,22 @@ export function planPredictionQuery( const ret = (phase: Phase) => `Return ONLY JSON: ${PHASE_META[phase].extractorSchema}`; switch (p.kind) { - case "metric-threshold": + case "metric-threshold": { + // Consul health is multi-row by nature: `consul_health_service_status` + // returns one series per (node × status), so querying the BARE metric + // yields mixed 0/1 rows the extractor can't reduce to a value → the gather + // comes back with zero usable observations and the keystone can never + // verify the hypothesis. Tell the gather to aggregate it for the affected + // service (the service named in the hypothesis) so it gets one clean value. + const consulHint = /consul_health_service_status/i.test(p.metric) + ? `\nNOTE: this is a Consul bare-metal health metric — querying it bare returns many rows (per node × status) with no usable value. Query it AGGREGATED for the service named in the hypothesis: max by (service_name) (consul_health_service_status{service_name="",status="passing"}). Report that single value (1 = passing, 0 = failing).` + : ""; return { role: "metrics", phase: "metrics", - prompt: `${preamble}\nPrediction to test: metric "${p.metric}" is ${p.op} ${p.value} during the incident.${window}\nRun ONE or TWO targeted queries for that exact metric over the window and report its actual value(s). Do not query unrelated metrics.\n${ret("metrics")}`, + prompt: `${preamble}\nPrediction to test: metric "${p.metric}" is ${p.op} ${p.value} during the incident.${window}\nRun ONE or TWO targeted queries for that exact metric over the window and report its actual value(s). Do not query unrelated metrics.${consulHint}\n${ret("metrics")}`, }; + } case "log-pattern": { const expectPresent = p.present !== false; return { From a77bfa4e25fd1c77b4abfa06549560158ee6efe8 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Tue, 9 Jun 2026 23:43:42 -0700 Subject: [PATCH 02/21] fix(deep): guard against the Consul 'not deployed in k8s' category error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trace of a bd-management run showed the keystone CONFIRMING a true-but-wrong cause: 'bd-management is not deployed in the cluster (no k8s pod exists)'. It's true (a Consul service has no k8s pod) but it is NOT the root cause — and the run had already gathered consul_health evidence proving the service is bare-metal Consul. The skill tells the model not to do this, but it does ~half the time; a prompt can't guarantee it. Deterministic guard in the conclude path: if the run gathered any consul_health_service_status evidence AND the confirmed cause asserts a missing k8s deployment ('not deployed', 'no k8s pod', 'deployment missing/absent'), reject it and make the agent confirm via the Consul health signal instead. Genuine k8s incidents (e.g. agw-admin-ui's deleted namespace) gather no consul_health evidence, so the guard never fires for them — proven by a control test. tsc clean; 2 new tests (guard fires for Consul / allows genuine k8s); suite green. --- src/agents/orchestrator.test.ts | 43 +++++++++++++++++++++++++++++++++ src/agents/orchestrator.ts | 18 ++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/agents/orchestrator.test.ts b/src/agents/orchestrator.test.ts index 6f935e72..873c1e75 100644 --- a/src/agents/orchestrator.test.ts +++ b/src/agents/orchestrator.test.ts @@ -774,3 +774,46 @@ describe("runOrchestrator — onMoveBoundary park hook (PR-2c)", () => { expect(result.outcome).toBe("aborted"); }); }); + +describe("runOrchestrator — Consul category-error guard", () => { + it("rejects a 'not deployed in the cluster' confirm when the run gathered consul_health evidence", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "bd-management", + gatherEvidence: async () => [{ phase: "metrics", subject: "consul_health_service_status{service_name=\"bd-management\"}", value: 0 }], + evaluate: () => "satisfied", + decideMove: scripted([ + { type: "hypothesize", hypothesis: h("bd-management is not deployed in the cluster (no k8s pod exists)") }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "no pod" }, + null, + ]), + }), + ); + // True-but-wrong: the service IS Consul (we saw consul_health), so "no k8s pod" + // is not the root cause — the confirm is blocked and the run exhausts. + expect(result.outcome).toBe("exhausted"); + expect(result.confirmed).toBeUndefined(); + expect(result.trace.some((t) => t.move === "conclude" && /bare-metal Consul service/.test(t.detail))).toBe(true); + }); + + it("ALLOWS a 'deployment not present' confirm for a genuine k8s service (no consul evidence — agw-admin-ui)", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "agw-admin-ui", + gatherEvidence: async () => [{ phase: "infra", subject: "agw-admin-ui namespace", text: "namespace not found" }], + evaluate: () => "satisfied", + decideMove: scripted([ + { type: "hypothesize", hypothesis: h("agw-admin-ui namespace deleted, deployment not present in the cluster") }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "namespace gone" }, + ]), + }), + ); + // Genuine k8s absence — no consul_health evidence → guard does NOT fire. + expect(result.outcome).toBe("confirmed"); + expect(result.confirmed?.hypothesis).toContain("namespace deleted"); + }); +}); diff --git a/src/agents/orchestrator.ts b/src/agents/orchestrator.ts index 1203c80f..fb51fc00 100644 --- a/src/agents/orchestrator.ts +++ b/src/agents/orchestrator.ts @@ -487,6 +487,24 @@ export async function runOrchestrator(deps: OrchestratorDeps): Promise /consul_health_service_status/i.test(o.subject)); + const claimsK8sAbsence = /\b(not deployed|no k8s pod|no pod exists|deployment (is )?missing|deployment does not exist|not present in (the )?cluster)\b/i.test(lead.hypothesis.hypothesis); + if (sawConsulEvidence && claimsK8sAbsence) { + record({ + move: "conclude", + detail: `not confirmed — "${lead.hypothesis.hypothesis}" claims a missing k8s deployment, but consul_health evidence shows this is a bare-metal Consul service (no k8s object by design). Confirm via its consul_health_service_status signal instead.`, + }); + stall++; + break; + } record({ move: "conclude", detail: `confirmed: ${lead.hypothesis.hypothesis}` }); return finish("confirmed", lead.hypothesis); } From e940cd973cd5cc0c3391c05851a98dee7629149f Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Wed, 10 Jun 2026 09:03:21 -0700 Subject: [PATCH 03/21] fix(deep): guard against observability-tooling false-confirms (minimax) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation surfaced a second wrong-layer failure: investigating the idle minimax-m25-metrics-proxy, the orchestrator ruled out every real hypothesis, then chased its OWN query errors — 'datasource with UID loki not found' (MI350 has no Loki datasource configured) appearing as 404s in logs — and confirmed 'Grafana datasource misconfigured' as the root cause. That's a query-layer artifact, not why the service is unhealthy. Guard (same pattern as the Consul category-error guard, same conclude path): reject a confirmed cause that blames the observability tooling (mentions 'datasource' or 'grafana') UNLESS the incident service is itself an observability component (grafana/loki/prometheus/...). Forces the agent back to the service's own signals; an idle service with no real cause then exhausts/pauses honestly instead of inventing a tooling cause. tsc clean; 2 new tests (guard fires for a service / allows a grafana incident); suite green. --- src/agents/orchestrator.test.ts | 38 +++++++++++++++++++++++++++++++++ src/agents/orchestrator.ts | 17 +++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/agents/orchestrator.test.ts b/src/agents/orchestrator.test.ts index 873c1e75..71e70eb1 100644 --- a/src/agents/orchestrator.test.ts +++ b/src/agents/orchestrator.test.ts @@ -817,3 +817,41 @@ describe("runOrchestrator — Consul category-error guard", () => { expect(result.confirmed?.hypothesis).toContain("namespace deleted"); }); }); + +describe("runOrchestrator — observability-artifact guard", () => { + it("rejects a confirm that blames the observability tooling (Grafana/datasource) for a service incident", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "minimax-m25-metrics-proxy", + evaluate: () => "satisfied", + decideMove: scripted([ + { type: "hypothesize", hypothesis: h("Grafana datasource for Loki is misconfigured — datasource not found") }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "datasource not found in logs" }, + null, + ]), + }), + ); + // Query-layer artifact, not a service root cause → blocked → exhausts. + expect(result.outcome).toBe("exhausted"); + expect(result.confirmed).toBeUndefined(); + expect(result.trace.some((t) => t.move === "conclude" && /observability tooling/.test(t.detail))).toBe(true); + }); + + it("ALLOWS a datasource/Grafana cause when the incident service IS an observability component", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "grafana", + evaluate: () => "satisfied", + decideMove: scripted([ + { type: "hypothesize", hypothesis: h("grafana datasource provisioning failed on restart") }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "provisioning error" }, + ]), + }), + ); + expect(result.outcome).toBe("confirmed"); // grafana itself — legitimately about the observability stack + }); +}); diff --git a/src/agents/orchestrator.ts b/src/agents/orchestrator.ts index fb51fc00..498d94b4 100644 --- a/src/agents/orchestrator.ts +++ b/src/agents/orchestrator.ts @@ -505,6 +505,23 @@ export async function runOrchestrator(deps: OrchestratorDeps): Promise Date: Wed, 10 Jun 2026 13:14:42 -0700 Subject: [PATCH 04/21] =?UTF-8?q?fix(deep):=20repair=20invalid=20OR-chain?= =?UTF-8?q?=20LogQL=20=E2=86=92=20fixes=20'no=20evidence=20gathered'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating minimax-m25-vllm-bench-4gpu (inv_01KTS9EK21G042S6YAS5MSCTE7), every log query came back empty and the orchestrator kept reporting 'couldn't verify (no evidence gathered)'. Root cause: gpt-oss generates an OR-chain of repeated line filters to search multiple terms — {container_name="svc"} |= "replicas" or {container_name="svc"} |= "scale" or {container_name="svc"} |= "deployment" which is INVALID LogQL (`or` joins label-filter expressions, not `{sel} |=` line-filter pipelines). Loki rejects it with HTTP 400 ('parse error ... unexpected {'), so the gather returns ZERO log observations → no log evidence for any log-based hypothesis (and contributes to MI350 investigations going inconclusive). Fix: coerceLokiArgs (already on the Loki tool exec path, tool-utils.ts:345) now detects the repeated-selector OR-chain and collapses it into the correct single regex line filter — {sel} |~ "replicas|scale|deployment". Single filters and already-regex queries pass through untouched; terms are deduped + regex-escaped. tsc clean; 3 new tests; full suite green. Symptom: log queries 400 → 'no evidence gathered'. Root cause: invalid OR-chain LogQL. Fix: tool-utils.ts coerceLokiArgs. Regression: tool-utils.test.ts:84. --- src/workflows/tool-utils.test.ts | 20 ++++++++++++++++++++ src/workflows/tool-utils.ts | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/workflows/tool-utils.test.ts b/src/workflows/tool-utils.test.ts index 45afd3e9..7acc68d2 100644 --- a/src/workflows/tool-utils.test.ts +++ b/src/workflows/tool-utils.test.ts @@ -80,3 +80,23 @@ describe("coerceLokiArgs — direction/limit", () => { expect(out.limit).toBe(100); }); }); + +describe("coerceLokiArgs — invalid OR-chain LogQL repair", () => { + // gpt-oss emits `{sel} |= "a" or {sel} |= "b" or {sel} |= "c"` to search multiple + // terms, which is invalid LogQL → Loki HTTP 400 → zero log evidence gathered. + // Collapse it into a single regex line filter. + it("collapses a repeated-selector OR-chain into one regex line filter", () => { + const out = coerceLokiArgs({ + direction: "backward", + limit: 50, + logql: '{container_name="svc"} |= "replicas" or {container_name="svc"} |= "scale" or {container_name="svc"} |= "deployment"', + }); + expect(out.logql).toBe('{container_name="svc"} |~ "replicas|scale|deployment"'); + }); + + it("dedupes repeated terms and preserves a single valid line filter untouched", () => { + expect(coerceLokiArgs({ logql: '{app="x"} |= "err" or {app="x"} |= "err"' }).logql).toBe('{app="x"} |~ "err"'); + expect(coerceLokiArgs({ logql: '{app="x"} |= "error"' }).logql).toBe('{app="x"} |= "error"'); // single filter: no rewrite + expect(coerceLokiArgs({ logql: '{app="x"} |~ "a|b"' }).logql).toBe('{app="x"} |~ "a|b"'); // already regex: untouched + }); +}); diff --git a/src/workflows/tool-utils.ts b/src/workflows/tool-utils.ts index dcef1c89..a191714c 100644 --- a/src/workflows/tool-utils.ts +++ b/src/workflows/tool-utils.ts @@ -188,6 +188,27 @@ export function coerceLokiArgs(args: Record): Record x[1])]; + const pattern = [...new Set(terms.filter(Boolean))] + .map((t) => t.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")) + .join("|"); + if (pattern) { + coerced.logql = `${selector} |~ "${pattern}"`; + quirkHit("loki-coerce:or-chain-to-regex", { terms: terms.length }); + } + } return coerced; } From e50d691d6a24f2add9ae1d639f7c904ea6d20d19 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 00:13:26 -0700 Subject: [PATCH 05/21] fix(deep): stop Consul skill over-steering k8s services into operator-pause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating minimax-m25-vllm-bench-4gpu (a k8s Deployment scaled to 0 replicas, kube_deployment_spec_replicas=0), the orchestrator spent all ~30 moves chasing Consul hypotheses ('is it a bare-metal Consul service / not registered in Consul / health check unknown...'), never proposed the obvious 'deployment scaled to 0', and operator-paused. Root cause: #256 injects the Consul runbook into EVERY orchestrator run (always-on), which over-steers k8s services into Consul tunnel-vision — a regression for the k8s case introduced by the Consul fixes. Two parts: - Skill: lead the 'When investigating' section with DETERMINE SERVICE TYPE FIRST — query kube_deployment_status_replicas; if it returns data the service IS a k8s Deployment (0 = scaled to 0 = a valid root cause, conclude it), and ONLY treat as Consul when there's no kube_deployment metric. Stops the Consul tunnel-vision. - Guard: the Consul category-error guard no longer fires when the run also saw kube_deployment/kube_pod evidence — a real k8s Deployment's k8s-absence cause is legitimate (only consul_health-AND-no-kube services are the bare-metal ones). tsc clean; 1 new test (k8s service not blocked); suite green. --- skills/consul-bare-metal-discovery.md | 19 +++++++++++++++++-- src/agents/orchestrator.test.ts | 26 ++++++++++++++++++++++++++ src/agents/orchestrator.ts | 7 ++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/skills/consul-bare-metal-discovery.md b/skills/consul-bare-metal-discovery.md index d59ba0d7..b9fb72d1 100644 --- a/skills/consul-bare-metal-discovery.md +++ b/skills/consul-bare-metal-discovery.md @@ -16,11 +16,26 @@ scope: This stack runs services on bare-metal hosts (not K8s). They are registered in Consul and export health status via the `consul_health_service_status` metric. ## When investigating a root cause (read this first) -If a service appears down but has **no Kubernetes Deployment / Pod**, that is EXPECTED here — it is a bare-metal Consul service, not a k8s workload. Do NOT report "deployment missing" or "not deployed in the cluster" as the root cause. Its health signal is the Consul metric: +**FIRST determine the service type — do NOT assume Consul.** This stack runs BOTH +k8s Deployments and bare-metal Consul services, so check which one this is before +forming Consul hypotheses. Query the k8s deployment metric for the service: +``` +kube_deployment_status_replicas{deployment=""} (or kube_deployment_spec_replicas) +``` +- **If that metric RETURNS DATA, the service IS a Kubernetes Deployment** → investigate + k8s causes, NOT Consul. A spec/available value of **0 means the deployment is scaled + to 0 replicas**, which is itself a complete, valid root cause for unavailability — + conclude that and stop. Do not pivot to Consul hypotheses for a k8s service. +- **ONLY if there is NO `kube_deployment_*` metric for the service** is it a bare-metal + Consul service. Then do NOT report "deployment missing / not deployed in the cluster" + (it has no k8s objects by design) — its health signal is the Consul metric: ``` max by (service_name) (consul_health_service_status{service_name="",status="passing"}) ``` -A value of `0` (or no row) means the bare-metal service is failing its Consul health check — that is the real signal to investigate (check the host process, its logs via the bare-metal logLabels, and any upstream it depends on). Only conclude a k8s cause for services that actually have k8s objects. +A value of `0` (or no row) means the bare-metal Consul service is failing its health +check — investigate the host process, its logs via the bare-metal logLabels, and any +upstream it depends on. Don't keep proposing Consul hypotheses for a service that +returned no `consul_health_service_status` data — it isn't a Consul service. ### To CONFIRM it (so the test actually verifies, not "absent") When you hypothesize that a bare-metal service is unhealthy, attach this EXACT checkable prediction — the keystone matches the metric name literally, so use it verbatim: diff --git a/src/agents/orchestrator.test.ts b/src/agents/orchestrator.test.ts index 71e70eb1..9360225e 100644 --- a/src/agents/orchestrator.test.ts +++ b/src/agents/orchestrator.test.ts @@ -855,3 +855,29 @@ describe("runOrchestrator — observability-artifact guard", () => { expect(result.outcome).toBe("confirmed"); // grafana itself — legitimately about the observability stack }); }); + +describe("runOrchestrator — Consul guard does not block genuine k8s services", () => { + it("ALLOWS a 'not deployed' confirm when the run ALSO saw kube_deployment evidence (real k8s scaled to 0)", async () => { + let i = 0; + const result = await runOrchestrator( + makeDeps({ + incidentService: "minimax-m25-vllm-bench-4gpu", + // The run gathered BOTH a (no-data) consul probe AND real kube_deployment + // evidence → it IS a k8s Deployment, so a k8s-absence cause is legitimate. + gatherEvidence: async () => (i++ === 0 + ? [{ phase: "metrics", subject: "consul_health_service_status{service_name=\"x\"}", value: 0 }] + : [{ phase: "metrics", subject: "kube_deployment_spec_replicas{deployment=\"minimax-m25-vllm-bench-4gpu\"}", value: 0 }]), + evaluate: () => "satisfied", + decideMove: scripted([ + { type: "hypothesize", hypothesis: h("minimax-m25-vllm-bench-4gpu deployment is not deployed (scaled to 0, no pod exists)") }, + { type: "query", target: 0 }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "spec replicas 0" }, + ]), + }), + ); + // kube_deployment evidence present → guard does NOT fire → confirmed. + expect(result.outcome).toBe("confirmed"); + }); +}); diff --git a/src/agents/orchestrator.ts b/src/agents/orchestrator.ts index 498d94b4..d8a860f3 100644 --- a/src/agents/orchestrator.ts +++ b/src/agents/orchestrator.ts @@ -496,8 +496,13 @@ export async function runOrchestrator(deps: OrchestratorDeps): Promise /consul_health_service_status/i.test(o.subject)); + // ...but NOT if the run also saw real k8s deployment evidence — then the + // service IS a k8s Deployment (e.g. scaled to 0 replicas), so a k8s-absence + // cause is legitimate. Only services with consul_health AND no kube_deployment + // evidence are the bare-metal Consul ones the guard targets. + const sawK8sDeploymentEvidence = evidence.some((o) => /kube_deployment|kube_pod|kube_replicaset|kube_statefulset/i.test(o.subject)); const claimsK8sAbsence = /\b(not deployed|no k8s pod|no pod exists|deployment (is )?missing|deployment does not exist|not present in (the )?cluster)\b/i.test(lead.hypothesis.hypothesis); - if (sawConsulEvidence && claimsK8sAbsence) { + if (sawConsulEvidence && !sawK8sDeploymentEvidence && claimsK8sAbsence) { record({ move: "conclude", detail: `not confirmed — "${lead.hypothesis.hypothesis}" claims a missing k8s deployment, but consul_health evidence shows this is a bare-metal Consul service (no k8s object by design). Confirm via its consul_health_service_status signal instead.`, From 314a9d09ce0c98352eaca388a33e89483c266a00 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 00:38:07 -0700 Subject: [PATCH 06/21] fix(deep): only inject the Consul runbook for actual Consul services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real flaw behind the k8s over-steer: #256 injected the bare-metal/Consul runbook into EVERY orchestrator run (getAllForScope always-on). So a Kubernetes service (e.g. minimax-m25-vllm-bench-4gpu, scaled to 0 replicas) got Consul context it should never see and the agent burned its whole budget on Consul hypotheses, never diagnosing the obvious k8s cause — exactly what the operator flagged. Fix: in handleOrchestratorInvestigate, drop infrastructure-type-specific runbooks (tagged consul/bare-metal) UNLESS the incident service is actually Consul-tracked — i.e. discovery recorded a consul_health_service_status metric for it. Signal is already in the registry: impala's metric is consul_health_service_status (Consul), minimax's is kube_deployment_*/vllm (k8s). No hardcoded service names (avoids the fazbd* product names in the live Consul list), auto-correct as discovery updates. A k8s investigation now has ZERO Consul in its context. Consul services still get the runbook. tsc clean; 1 updated + 1 new ws-handler test; suite green. --- src/server/ws-handler.test.ts | 33 +++++++++++++++++++++++++++++++++ src/server/ws-handler.ts | 20 ++++++++++++++------ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/server/ws-handler.test.ts b/src/server/ws-handler.test.ts index 29adb16b..b7b0ee5c 100644 --- a/src/server/ws-handler.test.ts +++ b/src/server/ws-handler.test.ts @@ -718,6 +718,9 @@ describe("handleClientMessage — orchestrator_investigate", () => { clearStackCaches(S); const deps = mockDeps(); (deps.config as any).agent.autonomousInvestigationEnabled = true; + // Consul-tracked service (discovery recorded a consul_health metric) → the + // bare-metal/Consul runbook IS relevant and gets injected. + (deps.config as any).services = [{ name: "payments-api", metrics: [{ query: 'consul_health_service_status{service_name="payments-api"}', description: "consul health" }], logLabels: {} }]; const ctx = mockCtx(); const skill = { id: "consul", @@ -772,6 +775,36 @@ describe("handleClientMessage — orchestrator_investigate", () => { }), ); }); + + it("does NOT inject the Consul runbook for a k8s service (no consul_health metric)", async () => { + clearStackCaches(S); + const deps = mockDeps(); + (deps.config as any).agent.autonomousInvestigationEnabled = true; + // k8s service — its discovered metric is kube_deployment, not consul_health. + (deps.config as any).services = [{ name: "web-frontend", metrics: [{ query: 'kube_deployment_status_replicas{deployment="web-frontend"}', description: "replicas" }], logLabels: {} }]; + const ctx = mockCtx(); + const consulSkill = { id: "consul", title: "Consul Bare Metal", services: [], alerts: [], tags: ["consul", "bare-metal"], scope: ["investigation"], filePath: "skills/consul.md", body: "Bare-metal Consul services..." }; + deps.skillStore = { + getAllForScopeEnabled: vi.fn(() => [consulSkill]), + formatForPrompt: vi.fn(() => "## Team Knowledge (Skills)\n..."), + } as any; + (deps.db.getInvestigation as ReturnType).mockReturnValue({ + id: "inv_k8s", service: "web-frontend", query: "web-frontend down", status: "complete", + report: JSON.stringify({ summary: "web-frontend down", timeRange: { from: "2026-06-09T00:00:00Z", to: "2026-06-09T01:00:00Z" } }), + }); + const orchestrate = vi.fn().mockResolvedValue({ outcome: "exhausted", hypotheses: [], evidence: [], trace: [], stats: { moves: 0, toolCalls: 0, tokensSpent: 0, strikes: 0, depth: 0, subagents: 0, elapsedMs: 0 } }); + (createMastraAdapters as ReturnType).mockResolvedValueOnce({ + chatAgent: { chat: vi.fn() }, investigationAgent: { investigate: vi.fn() }, discoverAgent: undefined, orchestrate, refineReport: vi.fn().mockResolvedValue(null), + }); + + await callHandler({ type: "orchestrator_investigate", investigationId: "inv_k8s" }, vi.fn(), deps, ctx); + + // The only skill (Consul) is filtered out for a k8s service → no skillContext. + expect(orchestrate).toHaveBeenCalledWith( + "web-frontend down", + expect.objectContaining({ skillContext: undefined, skills: undefined }), + ); + }); }); describe("handleClientMessage — orchestrator_accept (PR-6b)", () => { diff --git a/src/server/ws-handler.ts b/src/server/ws-handler.ts index 2e285f46..2add0482 100644 --- a/src/server/ws-handler.ts +++ b/src/server/ws-handler.ts @@ -610,15 +610,23 @@ async function handleOrchestratorInvestigate( neighbors.delete(investigation.service); const dependencies = [...neighbors]; - // Stack-level team knowledge for the decide-move brain. Unlike a per-query - // investigation, the orchestrator explores freely, so inject ALL enabled - // investigation-scoped skills (the discovery-style getAllForScope) rather than - // token-matching — e.g. the bare-metal/Consul runbook, so the agent doesn't - // mistake a Consul service's missing k8s Deployment for the root cause. + // Stack-level team knowledge for the decide-move brain. The orchestrator + // explores freely, so inject the enabled investigation-scoped skills as + // always-on context — EXCEPT infrastructure-type-specific runbooks that don't + // apply to this service. The bare-metal/Consul runbook in particular must NOT + // reach a Kubernetes service: it steers the agent into Consul hypotheses and a + // plain k8s incident (e.g. a Deployment scaled to 0) never gets diagnosed. + // A service is Consul-tracked iff discovery recorded a consul_health_service_status + // metric for it; otherwise it's k8s and the Consul runbook is dropped. + const incidentEntry = allServices.find((s) => s.name === investigation.service); + const incidentIsConsul = (incidentEntry?.metrics ?? []).some((mtr) => /consul_health_service_status/i.test(mtr.query ?? "")); let skillContext: string | undefined; let investigationSkills: Skill[] | undefined; if (deps.skillStore) { - const skills = deps.skillStore.getAllForScopeEnabled("investigation", deps.db.getDisabledSkills(stackId)); + let skills = deps.skillStore.getAllForScopeEnabled("investigation", deps.db.getDisabledSkills(stackId)); + if (!incidentIsConsul) { + skills = skills.filter((skill) => !(skill.tags ?? []).some((tag) => /^(consul|bare-metal)$/i.test(tag))); + } if (skills.length > 0) { skillContext = deps.skillStore.formatForPrompt(skills); investigationSkills = skills; From c6f27d772c52a0a5817b58e3d64b300d59b0fcd1 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 01:12:45 -0700 Subject: [PATCH 07/21] refactor(deep): move infra-type knowledge out of the engine into the skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orchestrator and ws-handler had hardcoded Consul-specific logic (a conclude-time category-error guard keyed on consul_health_service_status, and a skill-injection scope that special-cased the Consul runbook). That coupled the engine to one infrastructure type. Replace it with a generic mechanism: a skill declares an optional `appliesToServiceMetric` substring in its frontmatter, and the engine injects that skill only when the incident service's discovered metric queries contain it. The Consul runbook now declares `appliesToServiceMetric: consul_health_service_status` — all infra-type knowledge lives in the skill, none in the engine. - orchestrator.ts: drop the Consul category-error guard (keep the generic observability-artifact guard) - ws-handler.ts: generic appliesToServiceMetric filter, no consul_health regex - hypothesis-requery.ts: generalize the Consul aggregation hint to any bare (un-aggregated) metric selector - store.ts: parse + round-trip the appliesToServiceMetric field A k8s service (e.g. a Deployment scaled to 0) now receives zero Consul context and diagnoses cleanly; Consul services still get the runbook via the declared metric match. --- skills/consul-bare-metal-discovery.md | 1 + src/agents/orchestrator.test.ts | 69 ------------------- src/agents/orchestrator.ts | 23 ------- src/server/ws-handler.test.ts | 10 +-- src/server/ws-handler.ts | 25 ++++--- src/skills/store.test.ts | 27 ++++++++ src/skills/store.ts | 12 +++- .../steps/hypothesis-requery.test.ts | 15 ++-- src/workflows/steps/hypothesis-requery.ts | 19 ++--- 9 files changed, 77 insertions(+), 124 deletions(-) diff --git a/skills/consul-bare-metal-discovery.md b/skills/consul-bare-metal-discovery.md index b9fb72d1..abb668fa 100644 --- a/skills/consul-bare-metal-discovery.md +++ b/skills/consul-bare-metal-discovery.md @@ -2,6 +2,7 @@ title: Consul Bare-Metal Service Discovery services: [] alerts: [] +appliesToServiceMetric: consul_health_service_status tags: - discovery - consul diff --git a/src/agents/orchestrator.test.ts b/src/agents/orchestrator.test.ts index 9360225e..ea8acff9 100644 --- a/src/agents/orchestrator.test.ts +++ b/src/agents/orchestrator.test.ts @@ -775,49 +775,6 @@ describe("runOrchestrator — onMoveBoundary park hook (PR-2c)", () => { }); }); -describe("runOrchestrator — Consul category-error guard", () => { - it("rejects a 'not deployed in the cluster' confirm when the run gathered consul_health evidence", async () => { - const result = await runOrchestrator( - makeDeps({ - incidentService: "bd-management", - gatherEvidence: async () => [{ phase: "metrics", subject: "consul_health_service_status{service_name=\"bd-management\"}", value: 0 }], - evaluate: () => "satisfied", - decideMove: scripted([ - { type: "hypothesize", hypothesis: h("bd-management is not deployed in the cluster (no k8s pod exists)") }, - { type: "query", target: 0 }, - { type: "test", target: 0 }, - { type: "conclude", leading: 0, confidence: 0.9, rationale: "no pod" }, - null, - ]), - }), - ); - // True-but-wrong: the service IS Consul (we saw consul_health), so "no k8s pod" - // is not the root cause — the confirm is blocked and the run exhausts. - expect(result.outcome).toBe("exhausted"); - expect(result.confirmed).toBeUndefined(); - expect(result.trace.some((t) => t.move === "conclude" && /bare-metal Consul service/.test(t.detail))).toBe(true); - }); - - it("ALLOWS a 'deployment not present' confirm for a genuine k8s service (no consul evidence — agw-admin-ui)", async () => { - const result = await runOrchestrator( - makeDeps({ - incidentService: "agw-admin-ui", - gatherEvidence: async () => [{ phase: "infra", subject: "agw-admin-ui namespace", text: "namespace not found" }], - evaluate: () => "satisfied", - decideMove: scripted([ - { type: "hypothesize", hypothesis: h("agw-admin-ui namespace deleted, deployment not present in the cluster") }, - { type: "query", target: 0 }, - { type: "test", target: 0 }, - { type: "conclude", leading: 0, confidence: 0.9, rationale: "namespace gone" }, - ]), - }), - ); - // Genuine k8s absence — no consul_health evidence → guard does NOT fire. - expect(result.outcome).toBe("confirmed"); - expect(result.confirmed?.hypothesis).toContain("namespace deleted"); - }); -}); - describe("runOrchestrator — observability-artifact guard", () => { it("rejects a confirm that blames the observability tooling (Grafana/datasource) for a service incident", async () => { const result = await runOrchestrator( @@ -855,29 +812,3 @@ describe("runOrchestrator — observability-artifact guard", () => { expect(result.outcome).toBe("confirmed"); // grafana itself — legitimately about the observability stack }); }); - -describe("runOrchestrator — Consul guard does not block genuine k8s services", () => { - it("ALLOWS a 'not deployed' confirm when the run ALSO saw kube_deployment evidence (real k8s scaled to 0)", async () => { - let i = 0; - const result = await runOrchestrator( - makeDeps({ - incidentService: "minimax-m25-vllm-bench-4gpu", - // The run gathered BOTH a (no-data) consul probe AND real kube_deployment - // evidence → it IS a k8s Deployment, so a k8s-absence cause is legitimate. - gatherEvidence: async () => (i++ === 0 - ? [{ phase: "metrics", subject: "consul_health_service_status{service_name=\"x\"}", value: 0 }] - : [{ phase: "metrics", subject: "kube_deployment_spec_replicas{deployment=\"minimax-m25-vllm-bench-4gpu\"}", value: 0 }]), - evaluate: () => "satisfied", - decideMove: scripted([ - { type: "hypothesize", hypothesis: h("minimax-m25-vllm-bench-4gpu deployment is not deployed (scaled to 0, no pod exists)") }, - { type: "query", target: 0 }, - { type: "query", target: 0 }, - { type: "test", target: 0 }, - { type: "conclude", leading: 0, confidence: 0.9, rationale: "spec replicas 0" }, - ]), - }), - ); - // kube_deployment evidence present → guard does NOT fire → confirmed. - expect(result.outcome).toBe("confirmed"); - }); -}); diff --git a/src/agents/orchestrator.ts b/src/agents/orchestrator.ts index d8a860f3..0638fe15 100644 --- a/src/agents/orchestrator.ts +++ b/src/agents/orchestrator.ts @@ -487,29 +487,6 @@ export async function runOrchestrator(deps: OrchestratorDeps): Promise /consul_health_service_status/i.test(o.subject)); - // ...but NOT if the run also saw real k8s deployment evidence — then the - // service IS a k8s Deployment (e.g. scaled to 0 replicas), so a k8s-absence - // cause is legitimate. Only services with consul_health AND no kube_deployment - // evidence are the bare-metal Consul ones the guard targets. - const sawK8sDeploymentEvidence = evidence.some((o) => /kube_deployment|kube_pod|kube_replicaset|kube_statefulset/i.test(o.subject)); - const claimsK8sAbsence = /\b(not deployed|no k8s pod|no pod exists|deployment (is )?missing|deployment does not exist|not present in (the )?cluster)\b/i.test(lead.hypothesis.hypothesis); - if (sawConsulEvidence && !sawK8sDeploymentEvidence && claimsK8sAbsence) { - record({ - move: "conclude", - detail: `not confirmed — "${lead.hypothesis.hypothesis}" claims a missing k8s deployment, but consul_health evidence shows this is a bare-metal Consul service (no k8s object by design). Confirm via its consul_health_service_status signal instead.`, - }); - stall++; - break; - } // OBSERVABILITY-ARTIFACT GUARD: a cause that blames the monitoring/query // tooling itself — a Grafana datasource missing/misconfigured, a // "datasource not found" error — is almost always a query-layer artifact diff --git a/src/server/ws-handler.test.ts b/src/server/ws-handler.test.ts index b7b0ee5c..1e0c02ff 100644 --- a/src/server/ws-handler.test.ts +++ b/src/server/ws-handler.test.ts @@ -718,8 +718,8 @@ describe("handleClientMessage — orchestrator_investigate", () => { clearStackCaches(S); const deps = mockDeps(); (deps.config as any).agent.autonomousInvestigationEnabled = true; - // Consul-tracked service (discovery recorded a consul_health metric) → the - // bare-metal/Consul runbook IS relevant and gets injected. + // The Consul runbook declares `appliesToServiceMetric: consul_health_service_status`. + // This service's discovered metric matches → the runbook IS relevant and gets injected. (deps.config as any).services = [{ name: "payments-api", metrics: [{ query: 'consul_health_service_status{service_name="payments-api"}', description: "consul health" }], logLabels: {} }]; const ctx = mockCtx(); const skill = { @@ -729,6 +729,7 @@ describe("handleClientMessage — orchestrator_investigate", () => { alerts: [], tags: ["consul"], scope: ["investigation"], + appliesToServiceMetric: "consul_health_service_status", filePath: "skills/consul.md", body: "Bare-metal Consul services have no k8s Deployment.", }; @@ -780,10 +781,11 @@ describe("handleClientMessage — orchestrator_investigate", () => { clearStackCaches(S); const deps = mockDeps(); (deps.config as any).agent.autonomousInvestigationEnabled = true; - // k8s service — its discovered metric is kube_deployment, not consul_health. + // k8s service — its discovered metric is kube_deployment, not consul_health, + // so the runbook's `appliesToServiceMetric` does NOT match → it is filtered out. (deps.config as any).services = [{ name: "web-frontend", metrics: [{ query: 'kube_deployment_status_replicas{deployment="web-frontend"}', description: "replicas" }], logLabels: {} }]; const ctx = mockCtx(); - const consulSkill = { id: "consul", title: "Consul Bare Metal", services: [], alerts: [], tags: ["consul", "bare-metal"], scope: ["investigation"], filePath: "skills/consul.md", body: "Bare-metal Consul services..." }; + const consulSkill = { id: "consul", title: "Consul Bare Metal", services: [], alerts: [], tags: ["consul", "bare-metal"], scope: ["investigation"], appliesToServiceMetric: "consul_health_service_status", filePath: "skills/consul.md", body: "Bare-metal Consul services..." }; deps.skillStore = { getAllForScopeEnabled: vi.fn(() => [consulSkill]), formatForPrompt: vi.fn(() => "## Team Knowledge (Skills)\n..."), diff --git a/src/server/ws-handler.ts b/src/server/ws-handler.ts index 2add0482..6ea9f41d 100644 --- a/src/server/ws-handler.ts +++ b/src/server/ws-handler.ts @@ -612,21 +612,24 @@ async function handleOrchestratorInvestigate( // Stack-level team knowledge for the decide-move brain. The orchestrator // explores freely, so inject the enabled investigation-scoped skills as - // always-on context — EXCEPT infrastructure-type-specific runbooks that don't - // apply to this service. The bare-metal/Consul runbook in particular must NOT - // reach a Kubernetes service: it steers the agent into Consul hypotheses and a - // plain k8s incident (e.g. a Deployment scaled to 0) never gets diagnosed. - // A service is Consul-tracked iff discovery recorded a consul_health_service_status - // metric for it; otherwise it's k8s and the Consul runbook is dropped. + // always-on context. + // Generic skill targeting: a skill that declares `appliesToServiceMetric` is + // only eligible when the incident service's discovered metric queries contain + // that substring. This keeps infra-type knowledge (e.g. the Consul health + // metric) in the skill's frontmatter, not hardcoded in the engine. Untargeted + // skills are always eligible. const incidentEntry = allServices.find((s) => s.name === investigation.service); - const incidentIsConsul = (incidentEntry?.metrics ?? []).some((mtr) => /consul_health_service_status/i.test(mtr.query ?? "")); + const incidentMetricQueries = (incidentEntry?.metrics ?? []).map((mtr) => (mtr.query ?? "").toLowerCase()); let skillContext: string | undefined; let investigationSkills: Skill[] | undefined; if (deps.skillStore) { - let skills = deps.skillStore.getAllForScopeEnabled("investigation", deps.db.getDisabledSkills(stackId)); - if (!incidentIsConsul) { - skills = skills.filter((skill) => !(skill.tags ?? []).some((tag) => /^(consul|bare-metal)$/i.test(tag))); - } + const skills = deps.skillStore + .getAllForScopeEnabled("investigation", deps.db.getDisabledSkills(stackId)) + .filter((skill) => { + if (!skill.appliesToServiceMetric) return true; + const needle = skill.appliesToServiceMetric.toLowerCase(); + return incidentMetricQueries.some((q) => q.includes(needle)); + }); if (skills.length > 0) { skillContext = deps.skillStore.formatForPrompt(skills); investigationSkills = skills; diff --git a/src/skills/store.test.ts b/src/skills/store.test.ts index 1c0726c9..8f096883 100644 --- a/src/skills/store.test.ts +++ b/src/skills/store.test.ts @@ -192,6 +192,33 @@ Steps here`, expect(store.getAll()).toHaveLength(0); }); + + it("round-trips appliesToServiceMetric through save", async () => { + await store.save("targeted", { + title: "Targeted Skill", + services: [], + alerts: [], + tags: [], + appliesToServiceMetric: "consul_health_service_status", + }, "body"); + expect(store.getById("targeted")!.appliesToServiceMetric).toBe("consul_health_service_status"); + }); + }); + + describe("appliesToServiceMetric", () => { + it("parses the field from frontmatter, defaulting to undefined when absent", async () => { + await writeFile( + join(dir, "targeted.md"), + `---\ntitle: Targeted\nservices: []\nalerts: []\ntags: []\nappliesToServiceMetric: consul_health_service_status\n---\nBody`, + ); + await writeFile( + join(dir, "untargeted.md"), + `---\ntitle: Untargeted\nservices: []\nalerts: []\ntags: []\n---\nBody`, + ); + await store.loadAll(); + expect(store.getById("targeted")!.appliesToServiceMetric).toBe("consul_health_service_status"); + expect(store.getById("untargeted")!.appliesToServiceMetric).toBeUndefined(); + }); }); describe("delete", () => { diff --git a/src/skills/store.ts b/src/skills/store.ts index a9cc8d23..8e81bf6d 100644 --- a/src/skills/store.ts +++ b/src/skills/store.ts @@ -20,6 +20,11 @@ export interface SkillMetadata { alerts: string[]; tags: string[]; scope: SkillScope[]; + /** Optional generic targeting: this skill is only eligible for a service + * whose discovered metric queries contain this substring (case-insensitive). + * Keeps infra-type knowledge (e.g. a Consul health metric) in the skill, + * not hardcoded in the engine. Untargeted skills (undefined) are always eligible. */ + appliesToServiceMetric?: string; filePath: string; } @@ -129,6 +134,9 @@ export class SkillStore { alerts: Array.isArray(data.alerts) ? data.alerts.map(String) : [], tags: Array.isArray(data.tags) ? data.tags.map(String) : [], scope, + appliesToServiceMetric: typeof data.appliesToServiceMetric === "string" && data.appliesToServiceMetric.trim() + ? data.appliesToServiceMetric.trim() + : undefined, filePath, body: content.trim(), }); @@ -226,7 +234,7 @@ export class SkillStore { /** Save a skill (create or update). Returns the saved skill. */ async save( id: string | undefined, - frontmatter: { title: string; services: string[]; alerts: string[]; tags: string[]; scope?: SkillScope[] }, + frontmatter: { title: string; services: string[]; alerts: string[]; tags: string[]; scope?: SkillScope[]; appliesToServiceMetric?: string }, body: string, ): Promise { const skillId = id @@ -245,6 +253,7 @@ export class SkillStore { alerts: frontmatter.alerts, tags: frontmatter.tags, scope, + ...(frontmatter.appliesToServiceMetric ? { appliesToServiceMetric: frontmatter.appliesToServiceMetric } : {}), }); await writeFile(filePath, content, "utf-8"); @@ -256,6 +265,7 @@ export class SkillStore { alerts: frontmatter.alerts, tags: frontmatter.tags, scope, + appliesToServiceMetric: frontmatter.appliesToServiceMetric?.trim() || undefined, filePath, body, }; diff --git a/src/workflows/steps/hypothesis-requery.test.ts b/src/workflows/steps/hypothesis-requery.test.ts index 311b3208..d6d42f32 100644 --- a/src/workflows/steps/hypothesis-requery.test.ts +++ b/src/workflows/steps/hypothesis-requery.test.ts @@ -97,19 +97,20 @@ describe("planPredictionQuery", () => { expect(plan!.prompt).toContain("T2"); }); - it("adds the Consul aggregation hint for consul_health_service_status so the gather gets a usable value", () => { + it("adds a generic aggregation hint for a bare metric selector so the gather gets a usable value", () => { const plan = planPredictionQuery( hyp({ kind: "metric-threshold", metric: "consul_health_service_status", op: "<", value: 1 }), ); expect(plan!.role).toBe("metrics"); - expect(plan!.prompt).toContain("max by (service_name)"); - expect(plan!.prompt).toContain("status=\"passing\""); - expect(plan!.prompt).toMatch(/Consul bare-metal health metric/i); + expect(plan!.prompt).toMatch(/bare metric selector/i); + expect(plan!.prompt).toMatch(/aggregate it to the one series/i); }); - it("does NOT add the Consul hint for ordinary metrics (no false positives)", () => { - const plan = planPredictionQuery(hyp({ kind: "metric-threshold", metric: "http_p99", op: ">", value: 5 })); - expect(plan!.prompt).not.toContain("max by (service_name)"); + it("does NOT add the aggregation hint when the metric is already aggregated", () => { + const plan = planPredictionQuery( + hyp({ kind: "metric-threshold", metric: 'max by (service_name) (consul_health_service_status{status="passing"})', op: "<", value: 1 }), + ); + expect(plan!.prompt).not.toMatch(/bare metric selector/i); }); it("maps log-pattern to the logs role and reflects present/absent", () => { diff --git a/src/workflows/steps/hypothesis-requery.ts b/src/workflows/steps/hypothesis-requery.ts index 00fd2d57..43462d45 100644 --- a/src/workflows/steps/hypothesis-requery.ts +++ b/src/workflows/steps/hypothesis-requery.ts @@ -93,19 +93,20 @@ export function planPredictionQuery( switch (p.kind) { case "metric-threshold": { - // Consul health is multi-row by nature: `consul_health_service_status` - // returns one series per (node × status), so querying the BARE metric - // yields mixed 0/1 rows the extractor can't reduce to a value → the gather - // comes back with zero usable observations and the keystone can never - // verify the hypothesis. Tell the gather to aggregate it for the affected - // service (the service named in the hypothesis) so it gets one clean value. - const consulHint = /consul_health_service_status/i.test(p.metric) - ? `\nNOTE: this is a Consul bare-metal health metric — querying it bare returns many rows (per node × status) with no usable value. Query it AGGREGATED for the service named in the hypothesis: max by (service_name) (consul_health_service_status{service_name="",status="passing"}). Report that single value (1 = passing, 0 = failing).` + // A bare metric selector with no aggregation function can return many + // series (one per label combination), which the extractor can't reduce to + // a single value → the gather comes back with no usable observation and the + // keystone can never verify the hypothesis. If the prediction's metric has + // no aggregation operator, tell the gather to aggregate it down to the one + // series for the affected service so it gets a clean value. + const hasAggregation = /\b(sum|max|min|avg|count|quantile|topk|bottomk|group|stddev|stdvar)\b/i.test(p.metric); + const aggregateHint = !hasAggregation + ? `\nNOTE: "${p.metric}" looks like a bare metric selector — querying it directly may return many series (one per label combination) with no single usable value. If so, aggregate it to the one series for the service named in the hypothesis (e.g. max by () ({})) and report that single value.` : ""; return { role: "metrics", phase: "metrics", - prompt: `${preamble}\nPrediction to test: metric "${p.metric}" is ${p.op} ${p.value} during the incident.${window}\nRun ONE or TWO targeted queries for that exact metric over the window and report its actual value(s). Do not query unrelated metrics.${consulHint}\n${ret("metrics")}`, + prompt: `${preamble}\nPrediction to test: metric "${p.metric}" is ${p.op} ${p.value} during the incident.${window}\nRun ONE or TWO targeted queries for that exact metric over the window and report its actual value(s). Do not query unrelated metrics.${aggregateHint}\n${ret("metrics")}`, }; } case "log-pattern": { From df872b3c514a12b0d5359bf4ade3903379eab4a8 Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 01:45:37 -0700 Subject: [PATCH 08/21] feat(deep): verify absence-based root causes (scaled-to-zero / deleted) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keystone could not confirm an absence. When the brain proposed the correct cause for a workload scaled to zero replicas, the prediction's gather returned no data (pod-runtime metrics vanish at zero replicas), so evaluatePrediction scored it 'absent' → 'couldn't verify', the correct cause was discarded, and the run drifted to a wrong cause that happened to gather evidence (GPU OOM, cross-service blame) or wall-clocked. Across repeated runs on the same scaled-to-zero incident, only the run that happened to phrase a luckily-verifiable prediction succeeded. Two generic, infra-agnostic verification paths for an absence: - (B) state metric that survives at zero: the decide-move prompt steers the brain to predict over a STATE metric (e.g. kube_deployment_status_replicas < 1) that still reports 0, instead of a runtime metric that disappears. - (A) explicit absence prediction: extend the existing log-pattern present:false mechanism to infra-status. evaluatePrediction confirms an absence only when infra evidence was actually gathered (mirrors the log-pattern haveLogs guard — never confirm on a query that may not have run). schemas.ts threads present through the decide-move parse; hypothesis-requery asks the gather to report the resource's actual state including its absence. --- src/agents/orchestrator-llm.ts | 3 ++- src/workflows/schemas.ts | 2 +- src/workflows/steps/corroboration.test.ts | 20 +++++++++++++++++++ src/workflows/steps/corroboration.ts | 17 +++++++++++++--- .../steps/hypothesis-requery.test.ts | 7 +++++++ src/workflows/steps/hypothesis-requery.ts | 10 ++++++++-- 6 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/agents/orchestrator-llm.ts b/src/agents/orchestrator-llm.ts index 565ad3aa..f3745080 100644 --- a/src/agents/orchestrator-llm.ts +++ b/src/agents/orchestrator-llm.ts @@ -148,7 +148,7 @@ Moves — emit EXACTLY ONE as a single JSON object (no prose, no code fence): add a candidate cause with a CHECKABLE prediction. PREDICTION is one of: {"kind":"metric-threshold","metric":"","op":">"|"<"|">="|"<=","value":} {"kind":"log-pattern","pattern":"","present":true|false} - {"kind":"infra-status","resource":"","status":""} + {"kind":"infra-status","resource":"","status":"","present":true|false} {"kind":"change-in-window","withinMinutesBefore":} - {"move":"query","target":} gather read-only evidence for that hypothesis's prediction. - {"move":"test","target":} score that hypothesis against gathered evidence. @@ -170,6 +170,7 @@ Rules: - CROSS-SERVICE CAUSES NEED A FOLLOW-CAUSE: observing that a dependency is unhealthy is only CORRELATIONAL. To conclude that a dependency caused this incident you MUST follow-cause into it first and establish the failure there — you cannot confirm "caused by " from the incident service's metrics alone. - GROUND EVERY CLAIM IN OBSERVED EVIDENCE: a hypothesis/rationale may only state metric values, replica counts, pod statuses, or service names you ACTUALLY saw in gathered evidence. Never invent specifics — do NOT write "1/2 replicas ready", "OOMKilled", or name a service you did not observe in the evidence. If you didn't query it, you can't claim it. A plausible-sounding story with numbers you didn't measure is a FALSE confirmation, not a root cause. - DON'T ASSUME KUBERNETES: a service may run outside k8s (Consul-registered, a VM, an external endpoint). The absence of a k8s Deployment/Pod for a service is NOT automatically the root cause — only conclude "not deployed / deployment missing" if the evidence shows the service IS a k8s workload whose Deployment genuinely vanished (e.g. kube_deployment_* metrics existed before and are now gone). If a service has no k8s objects but is monitored elsewhere (Consul/up{}/health checks), investigate THAT health signal instead of reporting "not deployed". +- VERIFYING AN ABSENCE (scaled to zero / no replicas / not running / deleted): the confirming signal here is the ABSENCE of something, which a normal threshold over a runtime metric CANNOT catch — that metric vanishes when the resource is at zero, so the gather returns no data and the cause can NEVER be confirmed. Predict it one of two verifiable ways instead: (a) over a STATE metric that still reports a value at zero — e.g. {"kind":"metric-threshold","metric":"kube_deployment_status_replicas","op":"<","value":1} reads 0 and confirms "scaled to zero"; or (b) assert the absence explicitly with present:false — e.g. {"kind":"infra-status","resource":"","status":"running","present":false}. NEVER predict "scaled to zero" over a pod-runtime metric (kube_pod_*, vllm:*, request rates) — those disappear at zero replicas and leave the hypothesis unverifiable. - Be decisive — your budget is limited. Prefer the most likely cause first. Output ONLY the JSON object for your chosen move.`; diff --git a/src/workflows/schemas.ts b/src/workflows/schemas.ts index bf6fb994..42e140cb 100644 --- a/src/workflows/schemas.ts +++ b/src/workflows/schemas.ts @@ -88,7 +88,7 @@ export const ParallelEvidenceSchema = z.object({ export const HypothesisPredictionSchema = z.discriminatedUnion("kind", [ z.object({ kind: z.literal("metric-threshold"), metric: z.string(), op: z.enum([">", "<", ">=", "<="]), value: z.number() }), z.object({ kind: z.literal("log-pattern"), pattern: z.string(), present: z.boolean().optional() }), - z.object({ kind: z.literal("infra-status"), resource: z.string().optional(), status: z.string() }), + z.object({ kind: z.literal("infra-status"), resource: z.string().optional(), status: z.string(), present: z.boolean().optional() }), z.object({ kind: z.literal("change-in-window"), withinMinutesBefore: z.number() }), ]); diff --git a/src/workflows/steps/corroboration.test.ts b/src/workflows/steps/corroboration.test.ts index fdbc380a..da42ffd4 100644 --- a/src/workflows/steps/corroboration.test.ts +++ b/src/workflows/steps/corroboration.test.ts @@ -84,6 +84,26 @@ describe("evaluatePrediction", () => { expect(evaluatePrediction({ kind: "infra-status", resource: "payments", status: "OOMKilled" }, obs)).toBe("absent"); }); + it("infra-status present:false: confirms an absence (scaled to zero) only when infra evidence exists", () => { + // We gathered infra evidence for the deployment and it is NOT "running" + // (0 replicas) → the predicted absence is confirmed. + const scaledToZero: NormalizedObservation[] = [ + { phase: "infra", subject: "vllm-bench deployment", text: "spec.replicas=0, available=0, no pods scheduled" }, + ]; + expect(evaluatePrediction({ kind: "infra-status", resource: "vllm-bench", status: "running", present: false }, scaledToZero)).toBe("satisfied"); + + // The resource IS running → the predicted absence is contradicted. + const running: NormalizedObservation[] = [ + { phase: "infra", subject: "vllm-bench deployment", text: "2/2 replicas running" }, + ]; + expect(evaluatePrediction({ kind: "infra-status", resource: "vllm-bench", status: "running", present: false }, running)).toBe("contradicted"); + + // No infra evidence gathered at all → unknown, not a confirmation (mirrors + // log-pattern present:false; never confirm an absence on a query that may + // simply not have run). + expect(evaluatePrediction({ kind: "infra-status", resource: "vllm-bench", status: "running", present: false }, [])).toBe("absent"); + }); + it("change-in-window: satisfied when a change lands within the window before the incident", () => { const obs: NormalizedObservation[] = [ { phase: "changes", subject: "MR #4412 shrink db pool", timestamp: "2026-04-02T13:55:00Z" }, diff --git a/src/workflows/steps/corroboration.ts b/src/workflows/steps/corroboration.ts index 8ebc060d..4b031f3f 100644 --- a/src/workflows/steps/corroboration.ts +++ b/src/workflows/steps/corroboration.ts @@ -49,8 +49,11 @@ export type HypothesisPrediction = | { kind: "metric-threshold"; metric: string; op: ">" | "<" | ">=" | "<="; value: number } /** A log pattern is present (or, with present:false, absent). */ | { kind: "log-pattern"; pattern: string; present?: boolean } - /** An infra resource is in a given status, e.g. checkout-api OOMKilled. */ - | { kind: "infra-status"; resource?: string; status: string } + /** An infra resource is in a given status, e.g. checkout-api OOMKilled. + * With present:false the prediction asserts the status/resource is ABSENT — + * e.g. "no running pod / scaled to zero / deleted" — which a normal threshold + * can't confirm (the runtime signal vanishes when the resource is gone). */ + | { kind: "infra-status"; resource?: string; status: string; present?: boolean } /** A change (deploy/MR) landed within N minutes before the incident. */ | { kind: "change-in-window"; withinMinutesBefore: number }; @@ -133,7 +136,15 @@ export function evaluatePrediction( if (wantResource && !normalize(o.subject).includes(wantResource)) return false; return normalize(o.text ?? "").includes(wantStatus) || normalize(o.subject).includes(wantStatus); }); - return matches.length > 0 ? "satisfied" : "absent"; + const expectPresent = prediction.present !== false; + if (expectPresent) return matches.length > 0 ? "satisfied" : "absent"; + // present:false → predicting the status/resource is ABSENT (scaled to zero, + // no running pod, deleted). Mirror log-pattern: an absence only CONFIRMS if + // we actually gathered infra evidence to judge against — with none, the + // query may simply not have run, so it's unknown, not confirmed. + const haveInfra = observations.some((o) => o.phase === "infra"); + if (!haveInfra) return "absent"; + return matches.length > 0 ? "contradicted" : "satisfied"; } case "change-in-window": { const changes = observations.filter((o) => o.phase === "changes" && o.timestamp); diff --git a/src/workflows/steps/hypothesis-requery.test.ts b/src/workflows/steps/hypothesis-requery.test.ts index d6d42f32..8d92620d 100644 --- a/src/workflows/steps/hypothesis-requery.test.ts +++ b/src/workflows/steps/hypothesis-requery.test.ts @@ -132,6 +132,13 @@ describe("planPredictionQuery", () => { expect(plan!.prompt).toContain("CrashLoopBackOff"); }); + it("infra-status present:false asks the gather to confirm an absence (scaled to zero / deleted)", () => { + const plan = planPredictionQuery(hyp({ kind: "infra-status", resource: "vllm-bench", status: "running", present: false })); + expect(plan!.role).toBe("infrastructure"); + expect(plan!.prompt).toMatch(/ABSENT/); + expect(plan!.prompt).toMatch(/zero replicas|no running pods|does not exist/i); + }); + it("maps change-in-window to the changes role and includes incident onset", () => { const plan = planPredictionQuery( hyp({ kind: "change-in-window", withinMinutesBefore: 30 }), diff --git a/src/workflows/steps/hypothesis-requery.ts b/src/workflows/steps/hypothesis-requery.ts index 43462d45..416efc43 100644 --- a/src/workflows/steps/hypothesis-requery.ts +++ b/src/workflows/steps/hypothesis-requery.ts @@ -117,12 +117,18 @@ export function planPredictionQuery( prompt: `${preamble}\nPrediction to test: log pattern "${p.pattern}" is ${expectPresent ? "PRESENT" : "ABSENT"} during the incident.${window}\nSearch the logs for that exact pattern and report matching lines (or confirm none found). One or two targeted queries only.\n${ret("logs")}`, }; } - case "infra-status": + case "infra-status": { + const expectPresent = p.present !== false; + const resource = p.resource ?? "(the affected resource)"; + const ask = expectPresent + ? `resource "${resource}" has status "${p.status}".${window}\nQuery infrastructure/Kubernetes for that resource's status and report it.` + : `resource "${resource}" is ABSENT — not "${p.status}" (e.g. scaled to zero, no ready pods, or deleted).${window}\nQuery infrastructure/Kubernetes for that resource and report its actual state — explicitly report if it has zero replicas, no running pods, or does not exist.`; return { role: "infrastructure", phase: "infra", - prompt: `${preamble}\nPrediction to test: resource "${p.resource ?? "(the affected resource)"}" has status "${p.status}".${window}\nQuery infrastructure/Kubernetes for that resource's status and report it. One or two targeted queries only.\n${ret("infra")}`, + prompt: `${preamble}\nPrediction to test: ${ask} One or two targeted queries only.\n${ret("infra")}`, }; + } case "change-in-window": return { role: "changes", From 00339dab1c957ca747f709b4c12bfe9235c82bbb Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 09:03:28 -0700 Subject: [PATCH 09/21] feat(deep): generic service-type consistency guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The absence-verification fix made 'scaled to zero' confirmable — which is correct for a genuine k8s workload but ALSO let a Consul service be falsely confirmed as 'a Kubernetes Deployment scaled to zero' (5x re-run: impala, a consul_health-tracked service, hit this 1/5). With the hardcoded Consul guard removed, nothing enforced that a conclusion's infra framing matches the service's actual type. Add a generic conclude-time guard: derive the service's infrastructure type from its discovered identity metric FAMILY (kube_* → k8s, consul_* → Consul) and reject a confirm whose framing contradicts it — a k8s/deployment/replica cause for a Consul service, or a Consul cause for a kube_deployment workload. Driven by the metric family, not a per-service or Consul-only list; silent when the identity is an unrecognised family (up{}, etc.). Threads the incident service's identity metrics (ws-handler → agents.orchestrate → runAutonomousOrchestrator → OrchestratorDeps). Also catches the reverse hallucination seen in the re-run (a k8s service confirmed with a 'consul registration' tail). --- src/agents/orchestrator-llm.ts | 4 +++ src/agents/orchestrator.test.ts | 63 +++++++++++++++++++++++++++++++++ src/agents/orchestrator.ts | 39 ++++++++++++++++++++ src/server/agents.ts | 5 +++ src/server/ws-handler.ts | 5 +-- 5 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/agents/orchestrator-llm.ts b/src/agents/orchestrator-llm.ts index f3745080..9c59eee8 100644 --- a/src/agents/orchestrator-llm.ts +++ b/src/agents/orchestrator-llm.ts @@ -385,6 +385,9 @@ export interface RunAutonomousOrchestratorOptions { /** All known service names — the cross-service guard checks these too, so a * false-confirm blaming another service is caught even with an empty dep graph. */ knownServices?: string[]; + /** The incident service's discovered identity metric queries — feeds the + * service-type consistency guard (metric family → infra type). */ + incidentServiceMetrics?: string[]; /** Interactive strike-limit hook (increment 5). Absent → the strike limit * stops directly. Wired by the orchestrate adapter to the WS pause card. */ onOperatorPause?: ( @@ -448,6 +451,7 @@ export async function runAutonomousOrchestrator( dependencies: opts.dependencies, incidentService: opts.incidentService, knownServices: opts.knownServices, + incidentServiceMetrics: opts.incidentServiceMetrics, onOperatorPause: opts.onOperatorPause, signal: opts.signal, onMoveBoundary: opts.onMoveBoundary, diff --git a/src/agents/orchestrator.test.ts b/src/agents/orchestrator.test.ts index ea8acff9..a0be1ed1 100644 --- a/src/agents/orchestrator.test.ts +++ b/src/agents/orchestrator.test.ts @@ -812,3 +812,66 @@ describe("runOrchestrator — observability-artifact guard", () => { expect(result.outcome).toBe("confirmed"); // grafana itself — legitimately about the observability stack }); }); + +describe("runOrchestrator — service-type consistency guard", () => { + const k8sConclude = (hyp_: string) => ({ + evaluate: () => "satisfied" as const, + decideMove: scripted([ + { type: "hypothesize", hypothesis: h(hyp_) }, + { type: "query", target: 0 }, + { type: "test", target: 0 }, + { type: "conclude", leading: 0, confidence: 0.9, rationale: "evidence backs it" }, + null, + ]), + }); + + it("rejects a k8s 'scaled to zero' confirm for a Consul-registered service (category error)", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "impala", + incidentServiceMetrics: ['consul_health_service_status{service_name="impala"}'], + ...k8sConclude("impala is a Kubernetes Deployment scaled to zero replicas"), + }), + ); + // impala's identity metric is consul_health → a k8s-deployment cause is a + // category error; the confirm is blocked and the run exhausts. + expect(result.outcome).toBe("exhausted"); + expect(result.confirmed).toBeUndefined(); + expect(result.trace.some((t) => t.move === "conclude" && /Consul-registered|consul_health/.test(t.detail))).toBe(true); + }); + + it("ALLOWS a k8s 'scaled to zero' confirm for a genuine k8s service (the minimax case)", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "minimax-m25-vllm-bench-4gpu", + incidentServiceMetrics: ['kube_deployment_status_replicas{deployment="minimax-m25-vllm-bench-4gpu"}'], + ...k8sConclude("deployment is scaled to zero replicas"), + }), + ); + expect(result.outcome).toBe("confirmed"); + }); + + it("rejects a Consul cause for a k8s workload (reverse category error / hallucinated registry)", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "minimax-m25-vllm-bench-4gpu", + incidentServiceMetrics: ['kube_deployment_status_replicas{deployment="minimax-m25-vllm-bench-4gpu"}'], + ...k8sConclude("service was removed from k8s but still has active consul registration"), + }), + ); + expect(result.outcome).toBe("exhausted"); + expect(result.trace.some((t) => t.move === "conclude" && /Consul cause|no Consul registry/.test(t.detail))).toBe(true); + }); + + it("stays silent when the service's identity metric family is unknown (no false rejection)", async () => { + const result = await runOrchestrator( + makeDeps({ + incidentService: "edge-proxy", + incidentServiceMetrics: ['up{job="edge-proxy"}'], + ...k8sConclude("edge-proxy deployment scaled to zero"), + }), + ); + // No recognised k8s/consul identity → the guard can't judge → it doesn't fire. + expect(result.outcome).toBe("confirmed"); + }); +}); diff --git a/src/agents/orchestrator.ts b/src/agents/orchestrator.ts index 0638fe15..d03c0f51 100644 --- a/src/agents/orchestrator.ts +++ b/src/agents/orchestrator.ts @@ -158,6 +158,14 @@ export interface OrchestratorDeps { * another service is caught even when the dependency graph is empty/missing * (inc-7 #3 — the keystone can be independently true but not causally linked). */ knownServices?: string[]; + /** The incident service's discovered identity metric queries (e.g. the + * registry's PromQL). Used by the service-type consistency guard to reject a + * conclusion whose infrastructure framing contradicts the service's actual + * type — e.g. naming a "k8s Deployment scaled to zero" cause for a service + * whose identity metric is consul_health (Consul-registered, not k8s), or a + * "Consul" cause for a kube_deployment-tracked workload. Generic: the type is + * read from the metric family, not from any hardcoded service list. */ + incidentServiceMetrics?: string[]; /** * Strikes-limit hook: instead of silently stopping at the strike limit, ask a * human. "continue" resets the strike counter and resumes the loop (the other @@ -504,6 +512,37 @@ export async function runOrchestrator(deps: OrchestratorDeps): Promise s.name), signal: abort.signal, lead, skillContext, skills: investigationSkills }, + { timeRange, ctx: { incidentTime: timeRange?.from }, dependencies, incidentService: investigation.service, knownServices: allServices.map((s) => s.name), incidentServiceMetrics: incidentMetricQueries, signal: abort.signal, lead, skillContext, skills: investigationSkills }, agents.orchestrate, persistingSend, registry, @@ -929,7 +929,7 @@ async function handleOrchestratorAccept( async function runOrchestratorStreamed( investigationId: string, focus: string, - opts: { timeRange?: { from: string; to: string }; ctx?: { incidentTime?: string }; dependencies?: string[]; incidentService?: string; knownServices?: string[]; signal?: AbortSignal; lead?: string; skillContext?: string; skills?: Skill[] }, + opts: { timeRange?: { from: string; to: string }; ctx?: { incidentTime?: string }; dependencies?: string[]; incidentService?: string; knownServices?: string[]; incidentServiceMetrics?: string[]; signal?: AbortSignal; lead?: string; skillContext?: string; skills?: Skill[] }, orchestrate: StackAgents["orchestrate"], send: (m: ServerMessage) => void, registry: OrchestratorRunRegistry, @@ -944,6 +944,7 @@ async function runOrchestratorStreamed( dependencies: opts.dependencies, incidentService: opts.incidentService, knownServices: opts.knownServices, + incidentServiceMetrics: opts.incidentServiceMetrics, signal: opts.signal, lead: opts.lead, skillContext: opts.skillContext, From 034d5a5d107e3e05e9a668775a4a5d11a70a1f5f Mon Sep 17 00:00:00 2001 From: Wilson Li Date: Thu, 11 Jun 2026 11:01:56 -0700 Subject: [PATCH 10/21] feat(eval): deep-investigation quality harness (measure, stop flying blind) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orchestrator is LLM-driven and non-deterministic — the same incident can confirm the right cause, wall-clock, or confirm a wrong one across runs. Until now 'quality is bad' was a vibe from slow manual batches; there was no way to tell whether a change helped. This harness turns a batch of orchestrator runs into objective rates: - src/eval/deep-eval.ts — pure scorer + CLI. Scores each run against a label: correct / confident-wrong / category-error / honest-inconclusive. Headline metrics: correctRate and confidentWrongRate (the 'do no harm' bar, target 0). --save / --compare / --max-confident-wrong / --max-category-error / --min-correct. - src/eval/deep-investigation-run.mjs — live batch runner (baseline → orchestrator, auto-continue at operator-pauses), writes a results JSON the scorer reads. Makes the previously-manual batch reproducible. - fixtures/deep-investigation-labels.json — labeled incidents (infra type, expected cause keywords, wrong-cause patterns, acceptable inconclusive outcomes). - baselines/deep-2026-06-11.json — first scorecard after Fix 1: 40% correct, 20% confident-wrong (the GPU fabrication), 0 category-error. 10 scorer unit tests. Establishes the measurement loop to gate the un-gate on 0% category-error / 0% fabrication / correct-on-clear-cases. --- CLAUDE.md | 5 + src/eval/baselines/deep-2026-06-11.json | 20 ++ src/eval/baselines/deep-latest.json | 20 ++ src/eval/deep-eval.test.ts | 96 ++++++ src/eval/deep-eval.ts | 273 ++++++++++++++++++ src/eval/deep-investigation-run.mjs | 118 ++++++++ .../fixtures/deep-investigation-labels.json | 29 ++ 7 files changed, 561 insertions(+) create mode 100644 src/eval/baselines/deep-2026-06-11.json create mode 100644 src/eval/baselines/deep-latest.json create mode 100644 src/eval/deep-eval.test.ts create mode 100644 src/eval/deep-eval.ts create mode 100644 src/eval/deep-investigation-run.mjs create mode 100644 src/eval/fixtures/deep-investigation-labels.json diff --git a/CLAUDE.md b/CLAUDE.md index 03dceeec..d45efb69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,10 @@ npm run test:discover-eval # Score discovery output quality (gates at npx tsx src/eval/rca-eval.ts # Score RCA report quality npx tsx src/eval/rca-eval.ts --save # Score + save baseline npx tsx src/eval/rca-eval.ts --compare src/eval/baselines/2026-03-22.json # Compare to baseline +# Deep-investigation (autonomous orchestrator) quality: run a batch live, then score it +node src/eval/deep-investigation-run.mjs /tmp/runs.json # live batch (server on :3000) +npx tsx src/eval/deep-eval.ts --results /tmp/runs.json # score: correct / confident-wrong / category-error rates +npx tsx src/eval/deep-eval.ts --results /tmp/runs.json --compare src/eval/baselines/deep-2026-06-11.json --max-confident-wrong 0 npx vitest run # Run all tests npx vitest run src/path # Run a single test file npx tsc --noEmit # Type check @@ -71,6 +75,7 @@ npx tsc --noEmit # Type check | Shared PromQL parser | `src/lib/prom-metric.ts` — `extractMetricExpression()`, imported by both server metric extraction and the web MetricsPanel empty-card titles | | Investigation export helpers | `src/web/lib/exportInvestigation.ts` — `downloadPng` (html-to-image + font preload), `downloadMarkdown`, `copyMarkdown` | | RCA eval harness | `src/eval/rca-eval.ts` — scores RCA reports on 5 quality dimensions, baselines in `src/eval/baselines/` | +| Deep-investigation eval | `src/eval/deep-eval.ts` (scorer: correct / confident-wrong / category-error rates) + `src/eval/deep-investigation-run.mjs` (live batch runner). Labels: `src/eval/fixtures/deep-investigation-labels.json`. The orchestrator is non-deterministic — this turns a manual batch into objective rates | | LLM quirk workarounds | `src/agents/shared/prepare-step.ts` (`prepareStep` hook) | | LLM retry & graceful failure | `src/agents/shared/llm-retry.ts` (`withLlmRetry`, `safeAgentRetryConfig`), `src/agents/shared/llm-errors.ts` (`LlmUnavailableError`, `isLlmUnavailable`). Tool-using agent paths only retry when `readOnlyTools: true` to avoid replaying write tool calls | | Shared types | `src/types/` — RCA report, agent interfaces, LLM types, WebSocket protocol | diff --git a/src/eval/baselines/deep-2026-06-11.json b/src/eval/baselines/deep-2026-06-11.json new file mode 100644 index 00000000..10927bd3 --- /dev/null +++ b/src/eval/baselines/deep-2026-06-11.json @@ -0,0 +1,20 @@ +{ + "correctRate": 40, + "confidentWrongRate": 20, + "correct": 2, + "confidentWrong": 1, + "categoryError": 0, + "labeled": 5, + "perService": { + "impala": { + "runs": 3, + "correct": 1, + "confidentWrong": 0 + }, + "minimax-m25-vllm-bench-4gpu": { + "runs": 2, + "correct": 1, + "confidentWrong": 1 + } + } +} \ No newline at end of file diff --git a/src/eval/baselines/deep-latest.json b/src/eval/baselines/deep-latest.json new file mode 100644 index 00000000..10927bd3 --- /dev/null +++ b/src/eval/baselines/deep-latest.json @@ -0,0 +1,20 @@ +{ + "correctRate": 40, + "confidentWrongRate": 20, + "correct": 2, + "confidentWrong": 1, + "categoryError": 0, + "labeled": 5, + "perService": { + "impala": { + "runs": 3, + "correct": 1, + "confidentWrong": 0 + }, + "minimax-m25-vllm-bench-4gpu": { + "runs": 2, + "correct": 1, + "confidentWrong": 1 + } + } +} \ No newline at end of file diff --git a/src/eval/deep-eval.test.ts b/src/eval/deep-eval.test.ts new file mode 100644 index 00000000..3a5d5b91 --- /dev/null +++ b/src/eval/deep-eval.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect } from "vitest"; +import { scoreRun, scoreRuns, type IncidentLabel } from "./deep-eval.js"; + +const consulLabel: IncidentLabel = { + service: "impala", + infraType: "consul", + expectedCauseKeywords: ["consul", "health check"], + wrongCausePatterns: ["kubernetes", "scaled to zero", "0 replicas"], + acceptableInconclusive: ["wall-clock", "operator-pause"], +}; + +const k8sLabel: IncidentLabel = { + service: "vllm-bench", + infraType: "k8s", + expectedCauseKeywords: ["scaled to zero", "0 replicas", "not running", "cleaned up"], + wrongCausePatterns: ["consul", "gpu", "readiness probe"], + acceptableInconclusive: ["wall-clock"], +}; + +describe("scoreRun", () => { + it("marks a confirmed correct Consul cause as correct", () => { + const s = scoreRun({ service: "impala", outcome: "confirmed", rootCause: "impala is a bare-metal Consul service with failing health checks" }, consulLabel); + expect(s.verdict).toBe("correct"); + expect(s.categoryError).toBe(false); + }); + + it("flags a k8s cause for a Consul service as confident-wrong + category-error", () => { + const s = scoreRun({ service: "impala", outcome: "confirmed", rootCause: "impala is a Kubernetes Deployment scaled to zero replicas" }, consulLabel); + expect(s.verdict).toBe("confident-wrong"); + expect(s.categoryError).toBe(true); + expect(s.matchedWrongPattern).toBeTruthy(); + }); + + it("flags a fabricated GPU cause for a scaled-to-zero k8s service as confident-wrong (not category-error)", () => { + const s = scoreRun({ service: "vllm-bench", outcome: "confirmed", rootCause: "GPU resource exhaustion or CUDA error causing degradation" }, k8sLabel); + expect(s.verdict).toBe("confident-wrong"); + expect(s.matchedWrongPattern).toBe("gpu"); + expect(s.categoryError).toBe(false); // gpu is a fabrication, not a cross-type term + }); + + it("accepts the 'not running / cleaned up' family as correct for a scaled-to-zero k8s service", () => { + const s = scoreRun({ service: "vllm-bench", outcome: "confirmed", rootCause: "Benchmark job was never created or was cleaned up after completion" }, k8sLabel); + expect(s.verdict).toBe("correct"); + }); + + it("treats a wall-clock as honest-inconclusive when listed acceptable", () => { + const s = scoreRun({ service: "impala", outcome: "wall-clock", rootCause: null }, consulLabel); + expect(s.verdict).toBe("honest-inconclusive"); + }); + + it("treats an unlisted non-confirmed outcome as unexpected-inconclusive", () => { + const s = scoreRun({ service: "impala", outcome: "error", rootCause: null }, consulLabel); + expect(s.verdict).toBe("unexpected-inconclusive"); + }); + + it("marks a run with no label as unlabeled", () => { + const s = scoreRun({ service: "unknown-svc", outcome: "confirmed", rootCause: "anything" }, undefined); + expect(s.verdict).toBe("unlabeled"); + }); + + it("a confirmed cause with no expected keyword is confident-wrong even without a wrong pattern", () => { + const s = scoreRun({ service: "vllm-bench", outcome: "confirmed", rootCause: "network partition between regions" }, k8sLabel); + expect(s.verdict).toBe("confident-wrong"); + expect(s.matchedWrongPattern).toBeNull(); + }); +}); + +describe("scoreRuns aggregate", () => { + it("computes rates and per-service breakdown over a mixed batch", () => { + const runs = [ + { service: "impala", outcome: "confirmed", rootCause: "Consul health check failing" }, // correct + { service: "impala", outcome: "wall-clock", rootCause: null }, // honest + { service: "impala", outcome: "confirmed", rootCause: "Kubernetes Deployment scaled to zero" }, // confident-wrong + cat-error + { service: "vllm-bench", outcome: "confirmed", rootCause: "deployment scaled to zero, 0 replicas" }, // correct + { service: "vllm-bench", outcome: "confirmed", rootCause: "GPU resource exhaustion" }, // confident-wrong + ]; + const card = scoreRuns(runs, [consulLabel, k8sLabel]); + expect(card.total).toBe(5); + expect(card.labeled).toBe(5); + expect(card.correct).toBe(2); + expect(card.confidentWrong).toBe(2); + expect(card.categoryError).toBe(1); + expect(card.honestInconclusive).toBe(1); + expect(card.correctRate).toBe(40); + expect(card.confidentWrongRate).toBe(40); + expect(card.perService["impala"]).toEqual({ runs: 3, correct: 1, confidentWrong: 1 }); + expect(card.perService["vllm-bench"]).toEqual({ runs: 2, correct: 1, confidentWrong: 1 }); + }); + + it("zero labeled runs → 0 rates, not NaN", () => { + const card = scoreRuns([{ service: "ghost", outcome: "confirmed", rootCause: "x" }], [consulLabel]); + expect(card.labeled).toBe(0); + expect(card.correctRate).toBe(0); + expect(card.confidentWrongRate).toBe(0); + }); +}); diff --git a/src/eval/deep-eval.ts b/src/eval/deep-eval.ts new file mode 100644 index 00000000..85cbb9ea --- /dev/null +++ b/src/eval/deep-eval.ts @@ -0,0 +1,273 @@ +/** + * deep-eval.ts — deep-investigation (autonomous orchestrator) quality scoring CLI + * + * The orchestrator is LLM-driven and non-deterministic: the same incident can + * confirm the right cause, wall-clock, or confirm a wrong one on different runs. + * Eyeballing a manual batch can't tell whether a change helped. This harness + * turns a batch of orchestrator runs into objective rates so accuracy regressions + * (category errors, fabrications, confident-wrong confirms) are measured, not guessed. + * + * It scores RUN RESULTS (not live execution) so it's fast and deterministic. Produce + * the results with the live runner, then score them here. + * + * Usage: + * npx tsx src/eval/deep-eval.ts --results /tmp/orch-batch-results.json + * npx tsx src/eval/deep-eval.ts --results runs.json --save + * npx tsx src/eval/deep-eval.ts --results runs.json --compare src/eval/baselines/deep-2026-06-11.json + * npx tsx src/eval/deep-eval.ts --results runs.json --max-confident-wrong 0 --min-correct 50 + * + * Results file: a JSON array of run objects. Only three fields are read: + * { "service": "", "outcome": "confirmed|wall-clock|...", "rootCause": "" } + * (the live runner's output is a superset of this — it is read directly). + * + * Gates (CI): + * --max-confident-wrong N : exit 1 if confident-wrong runs exceed N (default: off) + * --max-category-error N : exit 1 if category-error runs exceed N (default: off) + * --min-correct PCT : exit 1 if correct-rate (of labeled runs) is below PCT + */ + +import { readFileSync, writeFileSync, mkdirSync } from "fs"; +import { resolve, dirname } from "path"; +import { fileURLToPath } from "url"; + +// ── Types ─────────────────────────────────────────────────────────────────── + +export interface IncidentLabel { + service: string; + infraType: string; + summary?: string; + /** A confirmed cause is CORRECT if its text contains any of these (case-insensitive). */ + expectedCauseKeywords: string[]; + /** A confirmed cause is clearly WRONG if its text contains any of these — a + * cross-infra-type category error or a fabrication for this service. */ + wrongCausePatterns: string[]; + /** Non-confirmed outcomes that are HONEST (not a quality failure) for this incident. */ + acceptableInconclusive: string[]; +} + +export interface DeepRun { + service: string; + outcome?: string | null; + rootCause?: string | null; +} + +export type RunVerdict = + | "correct" // confirmed AND matches an expected cause AND no wrong pattern + | "confident-wrong" // confirmed but not correct (category error / fabrication / off-target) + | "honest-inconclusive" // not confirmed, and the outcome is an acceptable decline + | "unexpected-inconclusive" // not confirmed, but the outcome wasn't listed acceptable + | "unlabeled"; // no label for this service — can't score + +export interface RunScore { + service: string; + outcome: string; + rootCause: string | null; + verdict: RunVerdict; + /** confident-wrong specifically because the cause names the other infra type. */ + categoryError: boolean; + /** confident-wrong specifically because the cause matched a flagged wrong pattern. */ + matchedWrongPattern: string | null; +} + +export interface Scorecard { + total: number; + labeled: number; + correct: number; + confidentWrong: number; + categoryError: number; + honestInconclusive: number; + unexpectedInconclusive: number; + unlabeled: number; + /** correct / labeled, as a 0–100 integer (0 when no labeled runs). */ + correctRate: number; + /** confidentWrong / labeled, 0–100. The "do no harm" bar — target 0. */ + confidentWrongRate: number; + perService: Record; + runs: RunScore[]; +} + +// ── Scoring (pure, exported for tests) ─────────────────────────────────────── + +const norm = (s: string | null | undefined): string => (s ?? "").toLowerCase(); + +function firstMatch(haystack: string, needles: string[]): string | null { + for (const n of needles) { + if (n && haystack.includes(n.toLowerCase())) return n; + } + return null; +} + +/** Score one run against its label. */ +export function scoreRun(run: DeepRun, label: IncidentLabel | undefined): RunScore { + const outcome = (run.outcome ?? "").toLowerCase() || "unknown"; + const cause = run.rootCause ?? null; + const base = { service: run.service, outcome, rootCause: cause }; + + if (!label) { + return { ...base, verdict: "unlabeled", categoryError: false, matchedWrongPattern: null }; + } + + if (outcome !== "confirmed") { + const honest = label.acceptableInconclusive.map((s) => s.toLowerCase()).includes(outcome); + return { + ...base, + verdict: honest ? "honest-inconclusive" : "unexpected-inconclusive", + categoryError: false, + matchedWrongPattern: null, + }; + } + + // Confirmed: judge the cause text. + const text = norm(cause); + const wrong = firstMatch(text, label.wrongCausePatterns); + const hasExpected = firstMatch(text, label.expectedCauseKeywords) !== null; + const correct = hasExpected && !wrong; + // A category error is a wrong confirm that names the OTHER infra type. We treat + // any matched wrong pattern as the trigger; the distinction in reporting is + // whether the matched pattern is an infra-type term vs. a fabrication term, but + // for the headline rate both are "confident-wrong". + const INFRA_TYPE_TERMS = ["kubernetes", "k8s", "consul", "namespace", "deployment"]; + const categoryError = wrong !== null && INFRA_TYPE_TERMS.some((t) => wrong.toLowerCase().includes(t)); + + return { + ...base, + verdict: correct ? "correct" : "confident-wrong", + categoryError, + matchedWrongPattern: wrong, + }; +} + +/** Aggregate a batch of runs against the label set. */ +export function scoreRuns(runs: DeepRun[], labels: IncidentLabel[]): Scorecard { + const byService = new Map(labels.map((l) => [l.service, l])); + const scored = runs.map((r) => scoreRun(r, byService.get(r.service))); + + const card: Scorecard = { + total: scored.length, + labeled: scored.filter((s) => s.verdict !== "unlabeled").length, + correct: scored.filter((s) => s.verdict === "correct").length, + confidentWrong: scored.filter((s) => s.verdict === "confident-wrong").length, + categoryError: scored.filter((s) => s.categoryError).length, + honestInconclusive: scored.filter((s) => s.verdict === "honest-inconclusive").length, + unexpectedInconclusive: scored.filter((s) => s.verdict === "unexpected-inconclusive").length, + unlabeled: scored.filter((s) => s.verdict === "unlabeled").length, + correctRate: 0, + confidentWrongRate: 0, + perService: {}, + runs: scored, + }; + if (card.labeled > 0) { + card.correctRate = Math.round((card.correct / card.labeled) * 100); + card.confidentWrongRate = Math.round((card.confidentWrong / card.labeled) * 100); + } + for (const s of scored) { + if (s.verdict === "unlabeled") continue; + const ps = (card.perService[s.service] ??= { runs: 0, correct: 0, confidentWrong: 0 }); + ps.runs++; + if (s.verdict === "correct") ps.correct++; + if (s.verdict === "confident-wrong") ps.confidentWrong++; + } + return card; +} + +// ── CLI ────────────────────────────────────────────────────────────────────── + +function loadLabels(): IncidentLabel[] { + const here = dirname(fileURLToPath(import.meta.url)); + const raw = JSON.parse(readFileSync(resolve(here, "fixtures/deep-investigation-labels.json"), "utf-8")); + return raw.labels as IncidentLabel[]; +} + +function getFlag(name: string): string | undefined { + const i = process.argv.indexOf(name); + return i >= 0 ? process.argv[i + 1] : undefined; +} + +function printCard(card: Scorecard): void { + const pad = (n: number) => String(n).padStart(3); + console.log("\n=== Deep-investigation eval ==="); + console.log(`runs: ${card.total} (labeled ${card.labeled}, unlabeled ${card.unlabeled})`); + console.log(` ✓ correct ${pad(card.correct)} (${card.correctRate}% of labeled)`); + console.log(` ✗ confident-wrong ${pad(card.confidentWrong)} (${card.confidentWrongRate}% of labeled) ← target 0`); + console.log(` of which category-error ${pad(card.categoryError)}`); + console.log(` ◐ honest-inconclusive ${pad(card.honestInconclusive)}`); + console.log(` ? unexpected-inconclusive ${pad(card.unexpectedInconclusive)}`); + console.log("\nper service:"); + for (const [svc, ps] of Object.entries(card.perService)) { + console.log(` ${svc}: ${ps.correct}/${ps.runs} correct, ${ps.confidentWrong} confident-wrong`); + } + const wrong = card.runs.filter((r) => r.verdict === "confident-wrong"); + if (wrong.length) { + console.log("\nconfident-wrong confirms:"); + for (const r of wrong) { + console.log(` ✗ ${r.service}: "${r.rootCause}"${r.matchedWrongPattern ? ` [matched "${r.matchedWrongPattern}"]` : ""}`); + } + } +} + +function main(): void { + const resultsPath = getFlag("--results"); + if (!resultsPath) { + console.error("error: --results is required (a JSON array of run objects)"); + process.exit(2); + } + const runs = JSON.parse(readFileSync(resolve(resultsPath), "utf-8")) as DeepRun[]; + if (!Array.isArray(runs)) { + console.error("error: results file must be a JSON array"); + process.exit(2); + } + const labels = loadLabels(); + const card = scoreRuns(runs, labels); + printCard(card); + + if (process.argv.includes("--save")) { + const here = dirname(fileURLToPath(import.meta.url)); + const dir = resolve(here, "baselines"); + mkdirSync(dir, { recursive: true }); + const summary = { + correctRate: card.correctRate, + confidentWrongRate: card.confidentWrongRate, + correct: card.correct, + confidentWrong: card.confidentWrong, + categoryError: card.categoryError, + labeled: card.labeled, + perService: card.perService, + }; + const out = resolve(dir, "deep-latest.json"); + writeFileSync(out, JSON.stringify(summary, null, 2)); + console.log(`\nsaved baseline → ${out}`); + } + + const comparePath = getFlag("--compare"); + if (comparePath) { + const base = JSON.parse(readFileSync(resolve(comparePath), "utf-8")); + const d = (k: "correctRate" | "confidentWrongRate") => card[k] - (base[k] ?? 0); + console.log(`\nvs baseline ${comparePath}:`); + console.log(` correctRate: ${base.correctRate ?? "?"}% → ${card.correctRate}% (${d("correctRate") >= 0 ? "+" : ""}${d("correctRate")})`); + console.log(` confidentWrongRate ${base.confidentWrongRate ?? "?"}% → ${card.confidentWrongRate}% (${d("confidentWrongRate") >= 0 ? "+" : ""}${d("confidentWrongRate")})`); + } + + // Gates + let failed = false; + const maxCW = getFlag("--max-confident-wrong"); + if (maxCW !== undefined && card.confidentWrong > Number(maxCW)) { + console.error(`\nGATE FAIL: confident-wrong ${card.confidentWrong} > ${maxCW}`); + failed = true; + } + const maxCE = getFlag("--max-category-error"); + if (maxCE !== undefined && card.categoryError > Number(maxCE)) { + console.error(`GATE FAIL: category-error ${card.categoryError} > ${maxCE}`); + failed = true; + } + const minCorrect = getFlag("--min-correct"); + if (minCorrect !== undefined && card.correctRate < Number(minCorrect)) { + console.error(`GATE FAIL: correctRate ${card.correctRate}% < ${minCorrect}%`); + failed = true; + } + process.exit(failed ? 1 : 0); +} + +// Only run main() as a CLI, not when imported by tests. +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/src/eval/deep-investigation-run.mjs b/src/eval/deep-investigation-run.mjs new file mode 100644 index 00000000..a7dc5c4f --- /dev/null +++ b/src/eval/deep-investigation-run.mjs @@ -0,0 +1,118 @@ +// deep-investigation-run.mjs — live runner for the deep-investigation eval. +// +// Drives the orchestrator over a batch of incidents against a RUNNING server, +// records each outcome + confirmed cause, and writes a results JSON that +// deep-eval.ts scores. This makes the previously-manual batch reproducible. +// +// Per incident: (1) run a baseline investigation, (2) fire orchestrator_investigate +// on it, (3) auto-continue at operator-pauses (capped), (4) record the outcome. +// +// Usage (server must be running on :3000): +// node src/eval/deep-investigation-run.mjs [out.json] +// MAX_CONTINUES=3 node src/eval/deep-investigation-run.mjs incidents.json /tmp/runs.json +// npx tsx src/eval/deep-eval.ts --results /tmp/runs.json +// +// incidents.json: [{ "stack": "