Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions docs/RUNBOOK.md
Original file line number Diff line number Diff line change
Expand Up @@ -670,3 +670,52 @@ the two ways a reporting repo could go silent, so this should now only happen
when a repo leaves the estate entirely - deleted, or its installation removed.
That case is inherent and bounded: it needs a repo to disappear while red, and
it clears itself within 24h.

## LLM backend unusable

The `[grug] LLM backend unusable` monitor fires when a review backend answers
401, 402, 403 or 404. That is not overload - it is broken.

| Status | Meaning | Fix |
|---|---|---|
| 401 / 403 | API key wrong or revoked | Rotate the key in SSM, redeploy |
| 402 | Unpaid / out of credits | Top up the account |
| 404 | Endpoint or model name gone | Check the vendor changed neither |

### Why it matters even though reviews still work

The Cave (self-hosted Spark models) is Elder's PRIMARY path. OpenRouter and
Poolside are a bounded, single-shot **overload valve** behind it - the
deliberate 2026-07-14 call to stop a Cave outage leaving a review
`all_failed`.

So a dead SaaS backend does not fail reviews on its own. It removes the
safety net, silently. The next Cave blip then goes straight to "Grug could
not review this", and the only person told is the PR author, who cannot pay
an invoice or rotate a key.

That is exactly what happened between 2026-07-27 and 08-03: OpenRouter
returned `http_402` four times and Poolside `http_404` three times. Both
halves of the valve were down at once and nothing said so, because the only
log token was `llm_backend_http_failed`, which also fires for ordinary 429s
and is therefore not alertable.

### Triage

```bash
pup logs search --query "llm_backend_unusable" \
--from "$(date -u -v-24H +%Y-%m-%dT%H:%M:%SZ)" --to "$(date -u +%Y-%m-%dT%H:%M:%SZ)"
```

The log carries `backend`, `status` and `failure_class:config_or_billing`.

Note the two backends are **not** interchangeable: they run different models
(OpenRouter serves Claude, Poolside serves Laguna), so losing one halves the
fallback rather than degrading it evenly.

### Check the other personas too

`judge_findings`, `summarize_pr` and `answer_pr_question` use Poolside and
OpenRouter as their **primary** backend via `select_backend`'s per-install
round robin - not as a fallback. A dead SaaS backend degrades those directly,
not just Elder's safety net.
50 changes: 50 additions & 0 deletions infra/pulumi/components/dd_monitors.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class _MonitorBundle:
persona_dispatch_unhandled: datadog.Monitor
elder_llm_degraded: datadog.Monitor
enforcement_gap: datadog.Monitor
backend_unusable: datadog.Monitor
cf_secret_mismatch: datadog.Monitor
uptime: datadog.SyntheticsTest
credential_acquisition_fail: datadog.Monitor
Expand Down Expand Up @@ -156,6 +157,29 @@ def poller_cronjob_unhealthy_query() -> str:
)


def backend_unusable_query(env: str) -> str:
"""An LLM backend that is not busy but BROKEN - unpaid, revoked, gone.

Keys on `llm_backend_unusable` (401/402/403/404), never on
`llm_backend_http_failed`, which also carries ordinary 429/5xx overload -
the exact condition the SaaS fallback exists to absorb. Alerting on that
would page for the system working as designed, so it would be muted, so
the real signal would be lost with it.

Why this monitor exists (#818): between 2026-07-27 and 08-03 OpenRouter
answered `http_402` four times (out of credits) and Poolside `http_404`
three times (dead endpoint). BOTH halves of Elder's overload valve were
down simultaneously. The valve's whole job is to stop a Cave outage
failing a review, so with it dead a Cave blip went straight to "Grug
could not review this" - and the only person told was the PR author, who
can do nothing about an unpaid invoice.
"""
return (
f'logs("service:grug-* env:{env} llm_backend_unusable")'
'.index("*").rollup("count").last("15m") > 0'
)


def enforcement_gap_query(env: str) -> str:
"""Any repo grug cannot prove is gated by the DoR check.

Expand Down Expand Up @@ -759,6 +783,31 @@ def create_all(
# the DoR check, for >1h. DogStatsD metric emitted by enforcement.py.
# Thresholds on the value, never on the enforcement_type tag: see
# enforcement_gap_query for why the tag filter latched this (#716).
# An LLM backend that is BROKEN rather than busy. Digest tier on purpose:
# it is not a user-facing outage - Elder still reviews from the Cave - but
# it silently removes the fallback, so the next Cave blip becomes a failed
# review with no safety net. Something to see, not something to wake for.
backend_unusable = datadog.Monitor(
"grug-backend-unusable",
type="log alert",
name="[grug] LLM backend unusable (unpaid / revoked / gone)",
message=(
f"{_DIGEST}\n"
"A review backend returned 401/402/403/404 - it is not overloaded, "
"it is unusable. 402 = unpaid, 401/403 = key wrong or revoked, "
"404 = endpoint or model name gone.\n"
"This does NOT fail reviews on its own: the Cave is the primary "
"path. It removes the FALLBACK, so the next Cave outage has "
"nothing behind it and authors get told Grug could not review.\n"
"Runbook: docs/RUNBOOK.md#llm-backend-unusable"
),
query=backend_unusable_query(env),
tags=_common_tags(env, "grug-consumer") + ["llm:backend"],
notify_no_data=False,
priority=3,
opts=opts,
)

enforcement_gap = datadog.Monitor(
"grug-enforcement-gap",
type="metric alert",
Expand Down Expand Up @@ -861,6 +910,7 @@ def create_all(
persona_dispatch_unhandled=persona_dispatch_unhandled,
elder_llm_degraded=elder_llm_degraded,
enforcement_gap=enforcement_gap,
backend_unusable=backend_unusable,
cf_secret_mismatch=cf_secret_mismatch,
uptime=uptime,
credential_acquisition_fail=credential_acquisition_fail,
Expand Down
26 changes: 26 additions & 0 deletions infra/pulumi/tests/test_dd_monitors.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,3 +506,29 @@ def test_enforcement_gap_query_is_env_scoped() -> None:
"""Each stack alerts only on its own repos."""
assert "env:dev" in enforcement_gap_query("dev")
assert "env:prod" not in enforcement_gap_query("dev")


# --- #818: a dead LLM backend must reach an operator ------------------------


def test_backend_unusable_query_targets_the_terminal_token_only() -> None:
"""A 402 removed Elder's whole fallback and nobody was told.

Measured 2026-07-27..08-03: OpenRouter `http_402` x4 (unpaid) and
Poolside `http_404` x3 (dead endpoint) - BOTH halves of the SaaS
overload valve down at once, so a Cave blip went straight to "Grug
could not review this" and the only person informed was the PR author,
who cannot pay a bill.

Must key on `llm_backend_unusable`, never `llm_backend_http_failed`:
the latter also fires for ordinary 429/5xx overload, which is the
system working as designed.
"""
from components.dd_monitors import backend_unusable_query

q = backend_unusable_query("prod")
assert "llm_backend_unusable" in q
assert "llm_backend_http_failed" not in q, (
"alerting on the overload token would be pure noise"
)
assert "env:prod" in q
61 changes: 55 additions & 6 deletions services/_shared/llm_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,57 @@ def _saas_overload_fallback_config(backend: Backend) -> BackendConfig:
)


# A backend that answers with one of these is not busy - it is UNUSABLE, and
# no amount of retrying or falling back changes that. 401/403 = the key is
# wrong or revoked. 402 = the bill is unpaid. 404 = the endpoint or model name
# is gone. All four are operator problems with an operator fix.
_TERMINAL_BACKEND_STATUSES = frozenset({401, 402, 403, 404})


def is_terminal_backend_failure(status: int) -> bool:
"""Is this a config/billing failure rather than overload?

The distinction is load-bearing for #818. Measured 2026-07-27..08-03:
OpenRouter returned `http_402` (out of credits) four times and Poolside
`http_404` three times - BOTH halves of the SaaS overload valve were dead
at once. The valve exists precisely so a Cave outage does not fail a
review, so with both dead a Cave blip went straight to "Grug could not
review this", and the only person told was the PR author, who can do
nothing about an unpaid bill.

429 and 5xx are deliberately NOT terminal: those are the overload this
fallback was built for, and paging on them would be paging on the system
working as designed.
"""
return status in _TERMINAL_BACKEND_STATUSES


def _log_backend_failure(*, backend_value: str, status: int, error: str) -> None:
"""Log a backend HTTP failure under a token that matches its KIND.

Terminal failures get their own token so a monitor can alert on them.
Alerting on `llm_backend_http_failed` is not an option - it fires for
ordinary overload too, so it would be pure noise, which is why a dead
OpenRouter key sat unnoticed while it silently removed Elder's fallback.
"""
if is_terminal_backend_failure(status):
log.error(
"llm_backend_unusable",
extra={
"backend": backend_value,
"status": status,
"error": error,
# Named so the runbook and the monitor agree on vocabulary.
"failure_class": "config_or_billing",
},
)
return
log.warning(
"llm_backend_http_failed",
extra={"backend": backend_value, "status": status, "error": error},
)


def select_backend(installation_id: int) -> Backend:
"""Stable per-install backend pick via `installation_id % 2`.

Expand Down Expand Up @@ -2716,9 +2767,8 @@ def _run_review_arm(
backend=backend, kind="parse_failed", model=model,
error_text=f"{backend.value}: parse_failed: {err}", parse_err=err,
)
log.warning(
"llm_backend_http_failed",
extra={"backend": backend.value, "status": resp.status_code, "error": err},
_log_backend_failure(
backend_value=backend.value, status=resp.status_code, error=err,
)
return _ArmOutcome(
backend=backend, kind="http_failed",
Expand Down Expand Up @@ -3172,9 +3222,8 @@ def _review_diff_dispatch(
first_parse_fail = (backend, model, err)
last_error = f"{backend.value}: parse_failed: {err}"
continue
log.warning(
"llm_backend_http_failed",
extra={"backend": backend.value, "status": resp.status_code, "error": err},
_log_backend_failure(
backend_value=backend.value, status=resp.status_code, error=err,
)
last_error = f"{backend.value}: {err}"

Expand Down
65 changes: 65 additions & 0 deletions services/webhook/tests/test_llm_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2835,3 +2835,68 @@ def test_render_learnings_block_redacts_secret_before_truncation() -> None:
rows = [{"text": f"allow key {fake} in fixtures", "scope_path": ""}]
block = lc._render_learnings_block(rows)
assert fake not in block # redacted before it reached the block


# --- #818: a dead backend must page an operator, not degrade a review -------


def test_billing_and_auth_failures_are_classified_terminal():
"""A 402 is not overload - it is an unpaid bill, and no retry fixes it.

Measured 2026-07-27..2026-08-03: OpenRouter returned `http_402` four
times and Poolside `http_404` three times. Both halves of the SaaS
overload valve were dead simultaneously, so a Cave failure had NO
fallback - and the only visible consequence was an author being told
Grug could not review their PR.
"""
from llm_client import is_terminal_backend_failure

for status in (401, 402, 403, 404):
assert is_terminal_backend_failure(status) is True, status


def test_overload_and_transport_failures_are_not_terminal():
"""429/5xx are exactly what the fallback exists for. Treating them as
config failures would page an operator for normal load."""
from llm_client import is_terminal_backend_failure

for status in (408, 429, 500, 502, 503, 504):
assert is_terminal_backend_failure(status) is False, status


def test_terminal_backend_failure_emits_its_own_log_token():
"""It needs a DISTINCT token so a monitor can alert on it.

`llm_backend_http_failed` fires for ordinary overload too, so alerting
on it would be pure noise. A dead key or a dead endpoint is an operator
problem and must be separable.
"""
import logging

import llm_client

records = []

class _Cap(logging.Handler):
def emit(self, record):
records.append(record.getMessage())

lg = logging.getLogger("grug.llm_client")
h = _Cap()
lg.addHandler(h)
try:
llm_client._log_backend_failure(
backend_value="openrouter", status=402, error="insufficient credits",
)
llm_client._log_backend_failure(
backend_value="openrouter", status=503, error="overloaded",
)
finally:
lg.removeHandler(h)

assert "llm_backend_unusable" in records, (
"a billing/auth/config failure needs its own alertable token"
)
assert "llm_backend_http_failed" in records, (
"ordinary overload keeps the existing token"
)
Loading