From b1494b281a3660963d508143e1aa2ca87ce9f861 Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 05:50:50 +0000 Subject: [PATCH 1/8] docs(observability): standalone portable observability design (#1237) --- README.md | 4 + docs/observability.md | 519 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 523 insertions(+) create mode 100644 docs/observability.md diff --git a/README.md b/README.md index 5ac14990..4919e749 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,10 @@ Dispatch supports three authentication models: ### Operational Notes +#### Observability + +For the standalone, backend-neutral signal/state catalogue, dashboard view specification, alert catalogue, and integration approaches (independent of any worker client or monitoring stack), see [Standalone Portable Observability](docs/observability.md). + #### Basic-auth brute-force protection In `DISPATCH_AUTH_MODE=basic`, `src/middleware.ts` rate-limits failed Basic-auth attempts to **5 attempts per minute per source IP**; a successful password authentication resets the counter. A request that exceeds the limit receives `429 Too Many Requests` with `Retry-After` and security headers until the window resets. diff --git a/docs/observability.md b/docs/observability.md new file mode 100644 index 00000000..ab00fb7e --- /dev/null +++ b/docs/observability.md @@ -0,0 +1,519 @@ +# Standalone Portable Observability for Dispatch + +Design only. This document defines Dispatch observability independently of +Courier and of any operator's monitoring environment. It specifies the signals +and states Dispatch already owns, a backend-neutral dashboard and alert +specification, and integration approaches — without choosing an exporter, +dashboard product, or rule engine. No implementation, requeues, claim release, +worker credential changes, dashboard/rule assets, deployments, or home-ops +configuration are part of this design. + +- Tracking issue: misospace/dispatch#1237 +- Informational counterpart (not a prerequisite): misospace/courier#289 + +--- + +## 1. Purpose and scope + +Dispatch owns **work-supply and task-lifecycle truth**: which issues are +eligible, who owns them, whether an execution is live, how work is handed out, +acknowledged, settled, reclaimed, and withheld. Observability must answer, from +Dispatch's own state alone: + +1. Is there eligible work, and is any of it failing to reach a worker? +2. Is claimed/assigned work actually being executed, or is ownership stale? +3. How did work end — settled, blocked on a human, parked from exhaustion, or + stale because upstream is gone? +4. Is the supply pipeline (sync, groomer, admission, scheduler) healthy, or is + the queue empty because the pipeline stopped? + +An observer must be able to consume this **without Courier**: any worker client, +or none, is a valid deployment. Optional correlation with a connector/operator +is composition (join on an exported identifier at the consumer), never a +required shared identifier, dashboard, phase, or release cadence. + +### In scope + +- A **signal/state catalogue** with units, timestamp/reset semantics, bounded + dimensions, privacy constraints, and missing/unknown states. +- A **dashboard view specification** as backend-neutral state predicates. +- An **alert catalogue** covering stale ownership, failure to hand out eligible + work, unavailable upstream evidence, and normal inactivity. +- An **integration matrix** for optional Prometheus/VictoriaMetrics and + OTel-based approaches. +- Healthy/idle/degraded examples for a non-Courier consumer. +- The current instrumentation and the unknowns. + +### Non-goals + +No implementation, requeues, claim release, worker credential changes, +dashboard/rule assets, deployments, or home-ops configuration. Exporter and +dependency choices require a later decision (see [§10](#10-open-questions-and-deferred-decisions)). + +--- + +## 2. Boundaries and design principles + +- **Dispatch is standalone.** Every signal in this document is derived from + Dispatch's own database or its own HTTP surface. Nothing depends on Courier + telemetry, phases, dashboards, release cadence, or identifiers. +- **Backend-neutral.** Signals are specified as state predicates and + dimensions. The document does not assume Prometheus, VictoriaMetrics, OTel, + Grafana, Alertmanager, or any vendor. Each integration is a mapping, not a + requirement. +- **Bounded, low-cardinality dimensions.** Allowed label values are enumerable + (status, lane, reason code, run type). **Do not** use arbitrary repository + names, issue/PR numbers, task IDs, credentials, commit SHAs, or raw error + strings as metric labels — those belong in logs/events and drill-down views. +- **Explicit unknown.** "No data" and "zero" are different. Freshness and + upstream reads have a first-class `unknown` state and must be surfaced as + such, never silently coerced to healthy. +- **GitHub is the source of truth; Postgres is a cache.** Label-driven state + (issue status, `agent/*`, dependencies) is authoritative on GitHub and can + lag in Dispatch between syncs. Signals describe the cache and must not be + presented as the authoritative forge state. +- **No secrets, no PII.** Tokens, credentials, and personal data never appear + in a label, metric name, or exported event. + +--- + +## 3. Current instrumentation inventory + +Dispatch today has **no metrics endpoint, no `/metrics`, no OpenTelemetry, and +no structured counters/timers**. Observability is limited to a health endpoint, +in-process scheduler liveness, and append-only database tables. (Confirmed by +searching the repository: no Prometheus/OTel/metrics integration exists.) + +| Source | Location | What it exposes | Limitation | +|---|---|---|---| +| Health endpoint | `src/app/api/health/route.ts` | `{ok, database, version, authMode, scheduler, schedulerOverdue}`; HTTP 503 when the DB is unreachable | Liveness only; no queue/lease/queue-depth detail | +| Scheduler liveness | `src/lib/scheduler.ts:203-219` (`schedulerState()`) | Per job: `lastRunAt` (ISO or null), `intervalMs`, `overdue` (no run in > 2× interval) | In-process, `globalThis`; lost on restart; only reports armed jobs | +| Scheduler jobs | `src/lib/scheduler.ts:88-140` | `sync` 15m, `groomer` 10m, `pr-followup` 15m, `ci-failures` 30m, `prune-closed` 24h, `reconcile` 30m, `stale-work` 5m, `pr-fix-sweep` disabled | Default-disabled unless `DISPATCH_SCHEDULER_ENABLED=true`; jobs are loopback HTTP POSTs | +| Audit log | `prisma/schema.prisma:324-342`; `GET /api/audit` | actor, action, repo, issue number, success, errorMessage, before/after labels, notes, createdAt | Free-string `action`; read-only listing; no aggregation | +| Agent runs | `prisma/schema.prisma:254-287` | agentName, runType, status, startedAt/finishedAt, summary, errorMessage, touchedIssueUrls, escalated outcome | `status` and `runType` are free strings | +| Issue sync runs | `prisma/schema.prisma:699-712` | status, reposFetched, syncedCount, syncType (scheduled/manual/automation), startedAt/completedAt | No failure-reason taxonomy | +| Automation sync runs | `prisma/schema.prisma:530-545` | status, reposFetched, workflowsFetched, runsFetched, startedAt/completedAt | Automation view only | +| Sync lock | `prisma/schema.prisma:719-725` | Single row serializing scheduled sync | Not a signal by itself | +| In-memory rate limiting | `src/lib/rate-limit.ts` | Per-instance counters | Not exported; per-replica | + +**Gap:** there is no way today to see queue depth/age, active leases, stale +ownership, PR-fix hand-out/reclaim state, admission withholding, grooming +freshness, or settlement outcomes as signals. This document specifies them. + +--- + +## 4. Signal and state catalogue + +### 4.1 Naming, kinds, and semantics + +- **Naming:** `dispatch___` (e.g. + `dispatch_pr_fix_oldest_queued_seconds`). +- **Kinds:** `gauge` (current value), `counter` (monotonic total), `histogram` + (bounded buckets). Because Dispatch has no in-process event counters, most + "counters" are derived from append-only tables and are **not process- + monotonic**. Model them either as gauges or as windowed deltas computed by the + exporter; document which. Counters derived from the DB do **not** reset on + process restart, unlike a conventional process counter. +- **Timestamp semantics:** gauges are "as of scrape". Event-derived totals use + the row's own timestamp (`createdAt`, `at`, `startedAt`/`completedAt`) and are + windowed by the exporter (e.g. `[5m]`, `[1h]`). +- **Reset semantics:** DB-derived totals never reset; the window is the reset. + If an in-process exporter is later added, its counters reset on restart and + must be paired with a start-time gauge. +- **Unknown:** any dimension value not in the allowlist maps to `unknown`; an + absent baseline (e.g. never groomed) is `unknown`, distinct from `fresh`. + +### 4.2 Work supply + +| Signal | Kind | Unit | Source | Dimensions (allowlisted) | Semantics | +|---|---|---|---|---|---| +| `dispatch_issues` | gauge | count | `Issue.state`, `Issue.labels`, `Issue.currentLane` | `status` (`backlog`,`ready`,`in-progress`,`in-review`,`blocked`,`done`,`unknown`), `lane`, `state` (`open`,`closed`) | Current cached issue count by status/lane | +| `dispatch_issues_open_claimable` | gauge | count | derived (open, not `done`/`blocked`, claimable status) | `lane` | Eligible-to-work supply | +| `dispatch_issue_age_seconds` | histogram | seconds | `Issue.createdAt` | `status`, `lane` | Waiting time since creation, for the tail that is starving | +| `dispatch_issue_stale_sync_seconds` | gauge | seconds | `now - Issue.lastSyncedAt` | — | Cache staleness | +| `dispatch_dependency_blocked` | gauge | count | open issues with open `blocked_by` / body deps | `lane` | Eligible-but-withheld by dependencies | + +Status values are the six canonical labels (`src/types/index.ts:138-153`). +`claimable` is not the same as `ready`: `status/in-progress` and +`status/in-review` are active but not new supply (`src/lib/agent-queue.ts:206-213`). + +### 4.3 Assignment vs active execution + +Assignment is label-only (`agent/*`, `owner/*`) and has no lease. Execution is a +`Lease` and/or `AgentWork` row. These are distinct and must not be conflated. + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_issue_assigned` | gauge | count | `Issue.labels` `agent/*` | `lane` | Label-assigned (may be idle) | +| `dispatch_leases_active` | gauge | count | `Lease.expiredAt > now` | — | Live ownership | +| `dispatch_lease_age_seconds` | histogram | seconds | `now - Lease.renewedAt` / `createdAt` | — | How long ownership has been held | +| `dispatch_leases_expired` | gauge | count | `Lease.expiredAt <= now` | — | Ownership past TTL (recoverable) | +| `dispatch_assigned_without_execution` | gauge | count | assigned (`agent/*`) and no active lease/`AgentWork` | `lane` | Assignment that never became execution | + +`DEFAULT_LEASE_TTL_MS = 30 min` (`src/lib/lease.ts:18`). Lease active means +`expiredAt > now`; there is no lease status column — the issue's status label is +the state (`src/lib/lease.ts:99-112`). Orphan leases are released and audited: +a corrupted checkpoint at `src/lib/lease.ts:168-180`, and a lease whose +referenced issue is missing at `src/lib/lease.ts:191-204`. + +### 4.4 AgentWork lifecycle + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_agent_work` | gauge | count | `AgentWork.state` | `state` (`CLAIMED`,`IN_PROGRESS`,`BLOCKED`,`DONE`,`RELEASED`,`STALE`,`unknown`) | Active + terminal work rows | +| `dispatch_agent_work_checkpoint` | gauge | count | `AgentWork.checkpoint` | `checkpoint` (8-value enum) | Progress distribution | +| `dispatch_agent_work_heartbeat_age_seconds` | histogram | seconds | `now - AgentWork.lastHeartbeatAt` | `state` | Liveness of active work | +| `dispatch_agent_work_stale_total` | gauge | count | `AgentWork.state = STALE` | — | Recovered/stale work | +| `dispatch_agent_work_release_pending` | gauge | count | `staleClaimReleasePending = true` | — | Stale recovery in flight (retryable) | + +Enums are fixed: states `CLAIMED, IN_PROGRESS, BLOCKED, DONE, RELEASED, STALE`; +checkpoints `CLAIMED, REPO_PREPARED, BRANCH_CREATED, CHANGES_MADE, +TESTS_RUNNING, PR_OPENED, DONE, BLOCKED` (`prisma/schema.prisma:731-749`). +Active states are `CLAIMED/IN_PROGRESS/BLOCKED`; internal AgentWork lease is +5 min (`src/lib/agent-work.ts:168,209`). The stale sweep marks STALE with a +`staleClaimReleasePending` retry marker, releases the GitHub claim, then +finalizes (`src/lib/stale-work.ts`). Sweep defaults: max age 5 min, batch 50. + +### 4.5 PR-fix hand-out, acknowledgement, reclaim + +The PR-fix queue is a second supply path with its own lifecycle. **An empty +PR-fix listing does not prove there is no assigned issue work** — the two paths +are independent. + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_pr_fix_items` | gauge | count | `PrFixQueueItem.status` | `status` (7-value enum), `lane` (3-value enum) | Queue composition | +| `dispatch_pr_fix_oldest_queued_seconds` | gauge | seconds | `now - queuedAt` for `QUEUED` | `lane` | Head-of-line wait | +| `dispatch_pr_fix_fix_attempts` | histogram | count | `PrFixQueueItem.fixAttempts` | `lane` | Attempts per item | +| `dispatch_pr_fix_handout_age_seconds` | histogram | seconds | `now - dispatchedAt` for stamped `QUEUED` | `lane` | Unacknowledged hand-out age | +| `dispatch_pr_fix_unacked_handouts` | gauge | count | `QUEUED`, `dispatchedGeneration = generation`, no matching ack | `lane` | Hand-outs that may be stranded | +| `dispatch_pr_fix_reclaims_total` | gauge | count | `PrFixHistory.action = reclaim` | — | Reopens of stamped-unacked hand-outs | +| `dispatch_pr_fix_handout_reclaims` | histogram | count | `PrFixQueueItem.handoutReclaims` | — | Per-item reclaim count vs cap | + +Statuses: `QUEUED, FIXED, BLOCKED, PARKED, STALE, IGNORED, WAITING_EXTERNAL`; +lanes `NORMAL, ESCALATED, NEEDS_HUMAN` (`prisma/schema.prisma:124-138`). +`generation` is the work identity (bumped on a fresh attempt); `fixAttempts` is +bounded by `PR_FIX_MAX_ATTEMPTS` default 5 (`src/lib/pr-fix-queue.ts:549-552`). +Hand-out timeout `PR_FIX_HANDOUT_TIMEOUT_MS` default 30 min; reclaim cap +`PR_FIX_MAX_RECLAIMS` default 3 (`pr-fix-queue.ts:579-597`). Past the cap the +item is **PARKED + `no_progress`** — bounded exhaustion, never a human decision +(`pr-fix-queue.ts:937-1010`). The `pr-fix-sweep` scheduler job is +default-disabled, so reclaim may be absent in a default deployment. + +### 4.6 PR-fix settlement + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_pr_fix_settled_total` | gauge (windowed) | count | `PrFixHistory.action = mark` with a terminal `status` (`FIXED`,`BLOCKED`,`PARKED`,`STALE`,`IGNORED`) | `status`, `outcome`, `type` | How work ended. Exclude `mark` transitions to `QUEUED` (external-wait retry, re-enqueue) so this counts settlements, not every mark | +| `dispatch_pr_fix_human_decisions` | gauge | count | `status = BLOCKED` (outcome `human_decision`) | `lane` | **Genuine** human decisions awaiting a call | +| `dispatch_pr_fix_parked` | gauge | count | `status = PARKED` | `outcome` (`no_progress`,`infra_failure`,`security_failure`) | Operational exhaustion, **not** a human decision | +| `dispatch_pr_fix_stale` | gauge | count | `status = STALE` | — | Upstream PR merged/closed | +| `dispatch_pr_fix_waiting_external` | gauge | count | `status = WAITING_EXTERNAL`, `retryAt` | — | External-dependency wait, with cooldown | +| `dispatch_pr_fix_oldest_human_decision_seconds` | gauge | seconds | `now - updatedAt` for `BLOCKED` | — | Age of the oldest unresolved human decision | + +Outcomes: `human_decision, no_progress, infra_failure, security_failure` +(`src/types/index.ts:266`). Types: `MERGE_CONFLICT, CI_FAILURE, +REVIEW_FEEDBACK, OTHER`. The distinction is load-bearing: **BLOCKED** is a +genuine human decision (carries `needs-human`); **PARKED** is bounded exhaustion +or an operational failure and must not be presented as a decision +(`src/lib/pr-fix-surfacing.ts:175-230`). `WAITING_EXTERNAL` cooldown defaults to +6h (`PR_FIX_EXTERNAL_WAIT_COOLDOWN_MS`, bounded 60s–7d). + +### 4.7 Admission and dependency withholding + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_queue_withheld` | gauge | count | admission decision (`enforce`) | `reason` (12 reason codes) | Eligible `status/ready` issues withheld | +| `dispatch_queue_admission_mode` | gauge | 0/1 | `DISPATCH_QUEUE_ADMISSION_MODE` | `mode` (`off`,`audit`,`enforce`) | Which mode is active | +| `dispatch_queue_withheld_oldest_seconds` | gauge | seconds | oldest withheld issue age | `reason` | How long work has been withheld | + +Reason codes (`src/lib/queue-admission.ts:50-63`): `dependency_blocked, +grooming_unknown, grooming_stale, grooming_issue_changed, grooming_unverified, +grooming_run_missing, grooming_not_applied, grooming_partial, +grooming_readiness_missing, grooming_not_ready, grooming_evidence_mismatch, +escalation_lane_mismatch`. In `audit` nothing is filtered; in `enforce` withheld +items are removed from worker pickup (still listable with `?includeWithheld=true` +as `claimable:false`). PR-fix items are never gated. + +### 4.8 Grooming freshness + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_grooming_runs_total` | gauge (windowed) | count | `GroomingRun.status`, `stage` | `status`, `stage`, `dry_run` | Grooming activity | +| `dispatch_grooming_freshness` | gauge | count | `Issue.groomedIssueFingerprint` / `groomingStaleAt` | `freshness` (`fresh`,`stale`,`unknown`) | Freshness of applied decisions | +| `dispatch_grooming_backoff` | gauge | count | `Issue.groomingRetryAfter > now` | — | Issues backing off after an unreadable read | +| `dispatch_grooming_stale_reasons` | gauge | count | `Issue.groomingStaleReasons` | `reason` (allowlisted) | Why decisions went stale | + +`GroomingFreshnessStatus = unknown | fresh | stale` +(`src/lib/groomer/freshness.ts`). `groomedIssueFingerprint NULL` means freshness +is **unknown** — never groomed since freshness tracking, or the baseline could +not be recorded — and is distinct from `fresh`. `status`, `stage`, and +`applyOutcome` are free strings today (observed `status`: `running, completed, +partial, failed, dry_run_completed, unverifiable, stale`; `stage`: `selected, +context_built, explored, planned, validated, skipped, applied`), so label sets +must be allowlisted with an `unknown` bucket. + +### 4.9 Sync and scheduler freshness + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_scheduler_up` | gauge | 0/1 | scheduler enabled + token present | — | Scheduler armed at all | +| `dispatch_scheduler_job_last_success_timestamp_seconds` | gauge | unix seconds | `schedulerState().jobs[].lastRunAt` | `job` (8 names) | Last completed tick | +| `dispatch_scheduler_job_overdue` | gauge | 0/1 | `schedulerState().jobs[].overdue` | `job` | No run in > 2× interval | +| `dispatch_sync_last_success_timestamp_seconds` | gauge | unix seconds | newest `IssueSyncRun.completedAt` with status success | `type` | Supply-pipeline freshness | +| `dispatch_sync_failures_total` | gauge (windowed) | count | `IssueSyncRun.status != success` | `type` | Failed syncs | +| `dispatch_repo_stale_sync_seconds` | gauge | seconds | `now - AutomationRepo.lastSyncedAt` | — | Per-repo cache staleness (aggregate max) | +| `dispatch_health_up` | gauge | 0/1 | `GET /api/health` ok | — | Process + DB liveness | +| `dispatch_scheduler_healthy` | gauge | 0/1 | derived: `dispatch_scheduler_up == 1` **and** no `dispatch_scheduler_job_overdue == 1` | — | Scheduler armed and every job ticking on time | + +Job names (`src/lib/scheduler.ts:88-140`): `sync, groomer, pr-followup, +ci-failures, prune-closed, reconcile, stale-work, pr-fix-sweep`. `overdue` +means no run in more than two intervals; `lastRunAt` is in-process and resets on +restart, so a freshly started process reports `null` until the first tick. + +### 4.10 Failure and outcome signals + +| Signal | Kind | Unit | Source | Dimensions | Semantics | +|---|---|---|---|---|---| +| `dispatch_audit_failures_total` | gauge (windowed) | count | `AuditLog.success = false` | `action` (allowlisted) | Failed mutations | +| `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Run throughput/outcomes | +| `dispatch_agent_run_failures_total` | gauge (windowed) | count | `AgentRun.status = failed` | `run_type` | Hard run failures | +| `dispatch_scheduler_job_errors_total` | gauge (windowed) | count | job HTTP non-2xx/409 (logs today) | `job` | Requires a future counter; **unknown** today | + +`AuditLog.action`, `AgentRun.status`, and `AgentRun.runType` are free strings. +Allowlist the observed values and bucket the rest as `unknown`; never label by +`errorMessage`. + +--- + +## 5. Dashboard view specification + +Backend-neutral. Each view is described by the state predicate it evaluates; the +mapping to panels, queries, or widgets is left to the consumer. Recommended +layout is rows of related signals. + +| Row | Views (predicate) | Question answered | +|---|---|---| +| **1. Work supply** | `dispatch_issues` by `status`/`lane`; `dispatch_issues_open_claimable`; `dispatch_issue_age_seconds` p50/p95/max | Is there eligible work, and how old is it? | +| **2. Execution** | `dispatch_issue_assigned`; `dispatch_leases_active`; `dispatch_agent_work` by `state`; `dispatch_agent_work_heartbeat_age_seconds` | Is assigned work actually executing? | +| **3. Ownership health** | `dispatch_leases_expired`; `dispatch_agent_work` `STALE`; `dispatch_assigned_without_execution` | Is any ownership stale or orphaned? | +| **4. PR-fix supply** | `dispatch_pr_fix_items` by `status`/`lane`; `dispatch_pr_fix_oldest_queued_seconds` | What PR-fix work is queued, and for how long? | +| **5. PR-fix hand-out health** | `dispatch_pr_fix_unacked_handouts`; `dispatch_pr_fix_handout_age_seconds`; `dispatch_pr_fix_reclaims_total` | Are hand-outs reaching and being acknowledged by workers? | +| **6. Settlement** | `dispatch_pr_fix_settled_total` by `status`/`outcome`/`type` | How is work ending? | +| **7. Human decisions** | `dispatch_pr_fix_human_decisions`; `dispatch_pr_fix_oldest_human_decision_seconds`; `dispatch_queue_withheld` by `reason` | What genuinely needs a human, and how long has it waited? | +| **8. Pipeline freshness** | `dispatch_scheduler_job_overdue`; `dispatch_scheduler_job_last_success_timestamp_seconds`; `dispatch_sync_last_success_timestamp_seconds`; `dispatch_repo_stale_sync_seconds`; `dispatch_health_up` | Did the supply pipeline stop? | +| **9. Grooming** | `dispatch_grooming_freshness`; `dispatch_grooming_stale_reasons`; `dispatch_grooming_backoff` | Is the admission gate starved by stale/unknown grooming? | +| **10. Failures** | `dispatch_audit_failures_total`; `dispatch_agent_run_failures_total` | Are mutations/runs failing? | + +**Separation requirement.** The dashboard must keep these distinct, because +collapsing them produces wrong operator conclusions: + +- **Settled/closed** (`FIXED`, `STALE`, `IGNORED`; issues `status/done`) — + terminal, not work. +- **Awaiting review** (issues `status/in-review`; `WAITING_EXTERNAL`) — active, + not stuck. +- **Stale claims** (`dispatch_leases_expired`, `AgentWork STALE`) — recoverable + ownership loss. +- **Blocked dependencies** (`dispatch_dependency_blocked`, + `withheld: dependency_blocked`) — correct withholding, not a bug. +- **Genuine human decisions** (`BLOCKED` / `human_decision`) — the only class + that is a request for a person. `PARKED` must not appear here. +- **Operational exhaustion** (`PARKED`) — bounded failure; requeueable by an + operator, not a design question. + +An empty PR-fix listing is a valid state and must not be read as "no work": the +issue queue and the PR-fix queue are independent supply paths. Show both. + +--- + +## 6. Alert catalogue + +Conditions are backend-neutral state predicates. `for` is the minimum duration +the predicate must hold before firing (avoid flapping). Severity is a +recommendation; the consumer maps it. + +| # | Alert | Condition (predicate) | Severity | `for` | Why actionable | Recovery / operator action | +|---|---|---|---|---|---|---| +| A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | +| A2 | No successful sync | `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale; issues will not enter the queue | Check `IssueSyncRun` errors and GitHub reachability | +| A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `increase(dispatch_agent_runs_total{run_type="implement"}[30m]) == 0` **and** `dispatch_scheduler_healthy == 1` | warning | 30m | Work exists and the pipeline is healthy, but no implement runs start — supply is not reaching workers. Suppressed while A1/A2 fire, so a stopped pipeline is not misreported as a worker problem | Check worker availability/credentials; confirm `next-task` returns `implement` | +| A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | +| A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | +| A7 | Unacknowledged hand-outs | `dispatch_pr_fix_unacked_handouts > 0` for > hand-out timeout | warning | 35m | A worker may have died before materializing its run; item is stranded | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | +| A8 | PR-fix head-of-line | `dispatch_pr_fix_oldest_queued_seconds > threshold` | warning | 30m | Work is queued but not draining | Check worker concurrency, lane routing, and `fixAttempts` saturation | +| A9 | Human decisions piling up | `dispatch_pr_fix_human_decisions > 0` **and** oldest age > threshold | info/warning | 1h | A person must decide; PARKED is excluded so this is real | Review the BLOCKED item's decision context and act | +| A10 | Admission withholding | `dispatch_queue_withheld{reason="grooming_unknown"}` or `{reason="grooming_stale"}` sustained | warning | 1h | The admission gate is starving the queue on missing/stale grooming | Fix grooming pipeline; or operator admission override for a specific issue | +| A11 | Grooming backoff | `dispatch_grooming_backoff > 0` sustained | warning | 30m | Upstream GitHub reads are failing; issues cannot be groomed | Check GitHub reachability/rate limits | +| A12 | Audit failures | `increase(dispatch_audit_failures_total[15m]) > 0` | warning | 5m | Mutations are failing (labels/claims) | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | +| A13 | Upstream evidence unavailable | `dispatch_repo_stale_sync_seconds > 2× sync interval` **or** `dispatch_grooming_backoff > 0` | warning | 15m | Admission/settlement decisions lack current evidence; the forge could not be read | Check GitHub reachability/rate limits; treat `unknown` as unknown, never healthy | + +### Normal inactivity must NOT alert + +- Empty issue queue with `dispatch_issues_open_claimable == 0`: **no alert**. + An idle fleet is healthy. +- Empty PR-fix listing: **no alert**. It is independent of the issue queue. +- `dispatch_agent_runs_total == 0` when there is no eligible work: **no alert**. +- No `BLOCKED` items: **no alert**. Absence of a human decision is good. +- No `status/ready` issue with `unknown` freshness (no issue withheld as + `grooming_unknown`): **no alert**. Never-groomed backlog is not a fault. +- `lastRunAt == null` immediately after process start: **no alert** until at + least one interval has elapsed. + +Alerts must be gated on **eligible work existing** (A4, A6) or on a **pipeline +that should have run** (A1–A3, A13). "Nothing happened" is not a failure. + +--- + +## 7. Integration matrix + +Optional approaches. **None is required, and none is chosen here.** Neither +Grafana nor Alertmanager is assumed; the dashboard and evaluator are explicit +choices at composition time. + +| Approach | Transport | State source | Dashboard choice | Evaluator choice | Cardinality control | Notes / gaps | +|---|---|---|---|---|---|---| +| **A. Pull exporter (Prometheus / VictoriaMetrics)** | HTTP `/metrics` scrape | Read-only DB queries (state gauges) + windowed totals from append-only tables | Any (Grafana, VM UI, custom) | Prometheus rules / vmalert / custom | Enforce label allowlists at the exporter | Needs a new read-only exporter; no in-process counters exist, so totals are windowed DB deltas | +| **B. OTel push metrics** | OTLP push | Same as A, emitted as OTel instruments | Any OTel-compatible backend | OTel-native or downstream | Use OTel attribute allowlists; avoid high-cardinality attributes | Adds an OTel SDK/dependency; exporter/collector choice deferred | +| **C. OTel logs/events** | OTLP logs | `AuditLog`, `PrFixHistory`, `AgentRun`, `IssueSyncRun` as events | Any log backend | Backend rules | Event attributes may carry IDs; **keep them out of metric labels** | Good for drill-down; not a substitute for gauges | +| **D. Snapshot API** | HTTP JSON | A read-only `GET` that returns the catalogue as JSON | Any | Any | Consumer controls | Optional convenience; would be a small, separately-scoped implementation | +| **E. Direct SQL / warehouse** | Batch | Postgres read replica | BI tool | BI/scheduled queries | Query-side | Couples to schema; acceptable for operators who already run a warehouse | + +**Recommended composition:** A or B for state gauges and windowed totals; C for +drill-down. Exporter, collector, dashboard, and evaluator choices are +**deferred** ([§10](#10-open-questions-and-deferred-decisions)). Correlation with +an external connector is done at the consumer by joining on an exported +identifier (e.g. repository + issue number), never by requiring a shared ID in +Dispatch. + +--- + +## 8. Worked examples for a non-Courier consumer + +These illustrate interpretation for a consumer that knows nothing about Courier. +Values are illustrative. + +### 8.1 Healthy + +``` +dispatch_health_up 1 +dispatch_scheduler_up 1 +dispatch_scheduler_job_overdue{job="sync"} 0 +dispatch_sync_last_success_timestamp_seconds (2m ago) +dispatch_issues{status="ready",state="open"} 12 +dispatch_issues_open_claimable 9 +dispatch_leases_active 4 +dispatch_agent_work{state="IN_PROGRESS"} 4 +dispatch_agent_work{state="STALE"} 0 +dispatch_pr_fix_items{status="QUEUED"} 3 +dispatch_pr_fix_unacked_handouts 0 +dispatch_pr_fix_human_decisions 1 +``` +**Read:** supply is flowing, execution is live, ownership is fresh, hand-outs +are acknowledged. A1–A8 and A10–A13 silent; A9 fires only if the one human +decision has aged past the threshold. This is the expected steady state. + +### 8.2 Idle + +``` +dispatch_health_up 1 +dispatch_scheduler_up 1 +dispatch_scheduler_job_overdue{job="sync"} 0 +dispatch_issues_open_claimable 0 +dispatch_agent_work{state="IN_PROGRESS"} 0 +dispatch_pr_fix_items{status="QUEUED"} 0 +dispatch_agent_runs_total{run_type="implement"} 0 +``` +**Read:** nothing to do. No alert should fire. In particular, A4 must be gated +on `open_claimable > 0`, and empty PR-fix must not alert. This is the example +that most often produces false alarms if inactivity is treated as failure. + +### 8.3 Degraded — scheduler stopped, work piling up + +``` +dispatch_health_up 1 +dispatch_scheduler_up 1 +dispatch_scheduler_job_overdue{job="sync"} 1 # last tick 45m ago, interval 15m +dispatch_sync_last_success_timestamp_seconds (45m ago) +dispatch_issues_open_claimable 20 +dispatch_agent_runs_total{run_type="implement"} 0 # nothing starting +dispatch_leases_expired 2 +dispatch_agent_work{state="STALE"} 2 +dispatch_queue_withheld{reason="grooming_unknown"} 6 +``` +**Read:** A1 (scheduler overdue) and A2 (no sync) fire first. A4 does **not** +fire here: its `dispatch_scheduler_healthy == 1` guard is false while A1 is +active, so a stopped pipeline is not misreported as a worker problem. A5 fires +on the expired leases/stale work. A10 fires on sustained `grooming_unknown` +withholding. The operator checks the scheduler logs, then the grooming pipeline. +Recovery is `sync` re-arming; the stale sweep recovers ownership. Once the +scheduler is healthy again, A4 becomes the relevant signal if runs still do not +start. + +### 8.4 Degraded — worker hand-outs stranded (PR-fix) + +``` +dispatch_pr_fix_items{status="QUEUED"} 8 +dispatch_pr_fix_unacked_handouts 5 +dispatch_pr_fix_handout_age_seconds{quantile="0.5"} 2400 # 40m > 30m timeout +dispatch_pr_fix_reclaims_total 2 +dispatch_agent_runs_total{run_type="followup-pr"} 1 +``` +**Read:** A7 fires — hand-outs older than the 30m timeout without an ack. Either +workers are not acknowledging (they should after durably materializing a run), +or the reclaim sweep is disabled (default). Recovery: enable the reclaim sweep +or have an operator requeue. This is distinct from A8 (head-of-line age) and +from A9 (a human decision). + +--- + +## 9. Privacy, cardinality, and security + +- **Label allowlists only.** Metric labels are drawn from fixed enums: status, + lane, reason code, job name, run type, state/checkpoint. Anything else is + `unknown`. +- **Never label by** repository name (unless the tracked-repo set is small and + operator-approved), issue/PR number, task ID, commit SHA, credential, agent + name (potentially sensitive), or error string. These belong in logs/events and + drill-down views. +- **No secrets or PII** in metric names, labels, or exported events. Token tiers + and identities are never exported. +- **Auth:** the health endpoint is public; everything else is authenticated + (bearer maintainer/worker, Basic, OIDC, or signed webhook). Any future + exporter must be read-only and scoped accordingly; it must not accept + mutations. +- **Source-of-truth caveat:** signals reflect Dispatch's cache. Label-driven + state is authoritative on GitHub and can lag; do not present cache state as + forge state in an alert that requires a human to act on the forge. + +--- + +## 10. Open questions and deferred decisions + +Exporter and dependency choices are **explicitly deferred**; this document is a +design, not a decision. + +- **Exporter shape:** pull exporter (A) vs OTel push (B) vs snapshot API (D). + Trade-offs: pull is simple and scrape-native; push fits an existing collector; + a snapshot API is vendor-neutral but adds an endpoint. +- **Counter modeling:** since Dispatch has no in-process counters, decide + whether DB-derived totals are gauges or windowed deltas, and how the consumer + computes rates. +- **Repo dimension:** whether `repo` is a bounded, acceptable label (small, + configured tracked set) or must be aggregated. +- **Scheduler state is in-process** (`globalThis`) and resets on restart; decide + whether to persist last-run timestamps for cross-restart freshness. +- **GitHub-native state** (native `blocked_by` links, PR review/merge state) is + read through the forge; decide how to represent `unknown` when a read fails + without writing back to Postgres. +- **Free-string dimensions** (`AgentRun.status`, `GroomingRun.status/stage/ + applyOutcome`, `PrFixHistory.action`, `AuditLog.action`) need a published + allowlist with an `unknown` bucket, or conversion to enums in a later change. +- **Reclaim observability:** the `pr-fix-sweep` job is default-disabled, so + reclaim signals may be absent; decide how to surface that state explicitly. +- **Acknowledgement gap:** issue claims have no ack (only PR-fix hand-outs do); + decide whether stale-claim detection is sufficient without one. +- **Retention and windowing:** how long event tables feed windowed totals, and + whether the exporter queries the primary or a replica. +- **OTel semantic conventions:** whether to map to standard metric/log + conventions or keep Dispatch-prefixed names. From 2684f7252d4b4d64c46f003f299a786721ee9a8b Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 15:49:06 +0000 Subject: [PATCH 2/8] docs(observability): address review feedback on A4/A7 alert semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A4: replace increase()/windowed-gauge counter-style rate function with a new as-of-scrape gauge dispatch_agent_runs_started_in_window{run_type,window}, and add an explicit fresh-sync guard so A2-style pipeline failures no longer satisfy A4 (scheduler ticking while sync fails is routed to A2/A13). - A7: introduce dispatch_pr_fix_overdue_unacked (age-tied: now - dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS) and use it for the alert, leaving the aggregate unacked count for the dashboard only. Steady churn no longer fires A7. - §4.1: spell out that increase()/rate() are counter-only and DB-derived totals must use delta()-style window deltas or _in_window as-of-scrape gauges. - A12: switch to delta() to match the new convention. - §8 examples: align 8.3 with the new A4 guards; add 8.5 showing PR-fix steady churn keeps A7 silent and explains why the earlier counter-style predicate was hazardous. --- docs/observability.md | 69 +++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 16 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index ab00fb7e..12f96bbc 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -114,6 +114,14 @@ freshness, or settlement outcomes as signals. This document specifies them. monotonic**. Model them either as gauges or as windowed deltas computed by the exporter; document which. Counters derived from the DB do **not** reset on process restart, unlike a conventional process counter. +- **`increase()` and `rate()` are counter-only.** They assume a monotonically + increasing series with well-defined resets. DB-derived totals in this document + are **windowed deltas**, not counters, so evaluators must query them via + `delta()`-style window deltas or, where the question is "did anything happen + in the last N", via an as-of-scrape gauge such as + `dispatch_<...>_in_window{..., window=""}` (see §4.10). Using + `increase()` on a gauge-derived series produces false negatives and positives + as the window rolls. - **Timestamp semantics:** gauges are "as of scrape". Event-derived totals use the row's own timestamp (`createdAt`, `at`, `startedAt`/`completedAt`) and are windowed by the exporter (e.g. `[5m]`, `[1h]`). @@ -186,7 +194,8 @@ are independent. | `dispatch_pr_fix_oldest_queued_seconds` | gauge | seconds | `now - queuedAt` for `QUEUED` | `lane` | Head-of-line wait | | `dispatch_pr_fix_fix_attempts` | histogram | count | `PrFixQueueItem.fixAttempts` | `lane` | Attempts per item | | `dispatch_pr_fix_handout_age_seconds` | histogram | seconds | `now - dispatchedAt` for stamped `QUEUED` | `lane` | Unacknowledged hand-out age | -| `dispatch_pr_fix_unacked_handouts` | gauge | count | `QUEUED`, `dispatchedGeneration = generation`, no matching ack | `lane` | Hand-outs that may be stranded | +| `dispatch_pr_fix_unacked_handouts` | gauge | count | `QUEUED`, `dispatchedGeneration = generation`, no matching ack | `lane` | Hand-outs that may be stranded (aggregate, includes both fresh and overdue) | +| `dispatch_pr_fix_overdue_unacked` | gauge | count | subset of unacked where `now - dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS` | `lane` | **Age-tied** overdue hand-outs: items past the configured timeout still waiting on an ack. Use this for A7 ([§6](#6-alert-catalogue)) — the aggregate `dispatch_pr_fix_unacked_handouts` is intended for the dashboard view, not for firing alerts, because steady churn keeps it above zero indefinitely even when every hand-out is acknowledged quickly | | `dispatch_pr_fix_reclaims_total` | gauge | count | `PrFixHistory.action = reclaim` | — | Reopens of stamped-unacked hand-outs | | `dispatch_pr_fix_handout_reclaims` | histogram | count | `PrFixQueueItem.handoutReclaims` | — | Per-item reclaim count vs cap | @@ -198,7 +207,8 @@ Hand-out timeout `PR_FIX_HANDOUT_TIMEOUT_MS` default 30 min; reclaim cap `PR_FIX_MAX_RECLAIMS` default 3 (`pr-fix-queue.ts:579-597`). Past the cap the item is **PARKED + `no_progress`** — bounded exhaustion, never a human decision (`pr-fix-queue.ts:937-1010`). The `pr-fix-sweep` scheduler job is -default-disabled, so reclaim may be absent in a default deployment. +default-disabled, so reclaim may be absent in a default deployment, and the +overdue signal can rise without an automated sweeper to drain it — see A7. ### 4.6 PR-fix settlement @@ -276,13 +286,16 @@ restart, so a freshly started process reports `null` until the first tick. | Signal | Kind | Unit | Source | Dimensions | Semantics | |---|---|---|---|---|---| | `dispatch_audit_failures_total` | gauge (windowed) | count | `AuditLog.success = false` | `action` (allowlisted) | Failed mutations | -| `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Run throughput/outcomes | +| `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed delta only**: the rows are append-only and the window is the reset, so the exporter computes rates over the chosen window (e.g. `[5m]`, `[1h]`). It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions | +| `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs that **started in the previous N seconds**, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Distinguishes "no starts" from "no windowed total", and survives process restarts because it is derived from the row timestamp | | `dispatch_agent_run_failures_total` | gauge (windowed) | count | `AgentRun.status = failed` | `run_type` | Hard run failures | | `dispatch_scheduler_job_errors_total` | gauge (windowed) | count | job HTTP non-2xx/409 (logs today) | `job` | Requires a future counter; **unknown** today | `AuditLog.action`, `AgentRun.status`, and `AgentRun.runType` are free strings. Allowlist the observed values and bucket the rest as `unknown`; never label by -`errorMessage`. +`errorMessage`. The default `window` allowlist for `dispatch_agent_runs_started_in_window` +covers the alerting needs in [§6](#6-alert-catalogue); consumers may add other +windows but must keep cardinality bounded. --- @@ -337,15 +350,15 @@ recommendation; the consumer maps it. | A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | | A2 | No successful sync | `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale; issues will not enter the queue | Check `IssueSyncRun` errors and GitHub reachability | | A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | -| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `increase(dispatch_agent_runs_total{run_type="implement"}[30m]) == 0` **and** `dispatch_scheduler_healthy == 1` | warning | 30m | Work exists and the pipeline is healthy, but no implement runs start — supply is not reaching workers. Suppressed while A1/A2 fire, so a stopped pipeline is not misreported as a worker problem | Check worker availability/credentials; confirm `next-task` returns `implement` | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent, yet no implement run has started in the last 30 minutes — supply is not reaching workers. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement` | | A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | | A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | -| A7 | Unacknowledged hand-outs | `dispatch_pr_fix_unacked_handouts > 0` for > hand-out timeout | warning | 35m | A worker may have died before materializing its run; item is stranded | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | +| A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | | A8 | PR-fix head-of-line | `dispatch_pr_fix_oldest_queued_seconds > threshold` | warning | 30m | Work is queued but not draining | Check worker concurrency, lane routing, and `fixAttempts` saturation | | A9 | Human decisions piling up | `dispatch_pr_fix_human_decisions > 0` **and** oldest age > threshold | info/warning | 1h | A person must decide; PARKED is excluded so this is real | Review the BLOCKED item's decision context and act | | A10 | Admission withholding | `dispatch_queue_withheld{reason="grooming_unknown"}` or `{reason="grooming_stale"}` sustained | warning | 1h | The admission gate is starving the queue on missing/stale grooming | Fix grooming pipeline; or operator admission override for a specific issue | | A11 | Grooming backoff | `dispatch_grooming_backoff > 0` sustained | warning | 30m | Upstream GitHub reads are failing; issues cannot be groomed | Check GitHub reachability/rate limits | -| A12 | Audit failures | `increase(dispatch_audit_failures_total[15m]) > 0` | warning | 5m | Mutations are failing (labels/claims) | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | +| A12 | Audit failures | `delta(dispatch_audit_failures_total[15m]) > 0` | warning | 5m | Mutations are failing (labels/claims). The predicate uses a window-delta evaluator (per §4.1) because `dispatch_audit_failures_total` is a gauge-derived total, not a monotonic counter | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | | A13 | Upstream evidence unavailable | `dispatch_repo_stale_sync_seconds > 2× sync interval` **or** `dispatch_grooming_backoff > 0` | warning | 15m | Admission/settlement decisions lack current evidence; the forge could not be read | Check GitHub reachability/rate limits; treat `unknown` as unknown, never healthy | ### Normal inactivity must NOT alert @@ -354,6 +367,10 @@ recommendation; the consumer maps it. An idle fleet is healthy. - Empty PR-fix listing: **no alert**. It is independent of the issue queue. - `dispatch_agent_runs_total == 0` when there is no eligible work: **no alert**. +- PR-fix steady-churn where every hand-out is acknowledged within the timeout: + **no alert** for A7 even though `dispatch_pr_fix_unacked_handouts > 0` — the + age-tied `dispatch_pr_fix_overdue_unacked` is zero. Aggregate unacked is + surfaced on the dashboard only. - No `BLOCKED` items: **no alert**. Absence of a human decision is good. - No `status/ready` issue with `unknown` freshness (no issue withheld as `grooming_unknown`): **no alert**. Never-groomed backlog is not a fault. @@ -361,7 +378,9 @@ recommendation; the consumer maps it. least one interval has elapsed. Alerts must be gated on **eligible work existing** (A4, A6) or on a **pipeline -that should have run** (A1–A3, A13). "Nothing happened" is not a failure. +that should have run** (A1–A3, A13) or on an **overdue age** (A7, A8, A9, A11). +"Nothing happened" is not a failure, and "something exists right now" is not +a failure when nothing is past its timeout. --- @@ -373,7 +392,7 @@ choices at composition time. | Approach | Transport | State source | Dashboard choice | Evaluator choice | Cardinality control | Notes / gaps | |---|---|---|---|---|---|---| -| **A. Pull exporter (Prometheus / VictoriaMetrics)** | HTTP `/metrics` scrape | Read-only DB queries (state gauges) + windowed totals from append-only tables | Any (Grafana, VM UI, custom) | Prometheus rules / vmalert / custom | Enforce label allowlists at the exporter | Needs a new read-only exporter; no in-process counters exist, so totals are windowed DB deltas | +| **A. Pull exporter (Prometheus / VictoriaMetrics)** | HTTP `/metrics` scrape | Read-only DB queries (state gauges, as-of-scrape windowed gauges, windowed deltas over append-only tables) | Any (Grafana, VM UI, custom) | Prometheus rules / vmalert / custom | Enforce label allowlists at the exporter | Needs a new read-only exporter; no in-process counters exist, so totals are windowed DB deltas. Use `delta()` (or direct comparison of `_in_window` as-of-scrape gauges) for "did anything happen in the last N" predicates — never `increase()` per §4.1 | | **B. OTel push metrics** | OTLP push | Same as A, emitted as OTel instruments | Any OTel-compatible backend | OTel-native or downstream | Use OTel attribute allowlists; avoid high-cardinality attributes | Adds an OTel SDK/dependency; exporter/collector choice deferred | | **C. OTel logs/events** | OTLP logs | `AuditLog`, `PrFixHistory`, `AgentRun`, `IssueSyncRun` as events | Any log backend | Backend rules | Event attributes may carry IDs; **keep them out of metric labels** | Good for drill-down; not a substitute for gauges | | **D. Snapshot API** | HTTP JSON | A read-only `GET` that returns the catalogue as JSON | Any | Any | Consumer controls | Optional convenience; would be a small, separately-scoped implementation | @@ -436,35 +455,53 @@ dispatch_scheduler_up 1 dispatch_scheduler_job_overdue{job="sync"} 1 # last tick 45m ago, interval 15m dispatch_sync_last_success_timestamp_seconds (45m ago) dispatch_issues_open_claimable 20 -dispatch_agent_runs_total{run_type="implement"} 0 # nothing starting +dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} 0 # nothing starting dispatch_leases_expired 2 dispatch_agent_work{state="STALE"} 2 dispatch_queue_withheld{reason="grooming_unknown"} 6 ``` **Read:** A1 (scheduler overdue) and A2 (no sync) fire first. A4 does **not** fire here: its `dispatch_scheduler_healthy == 1` guard is false while A1 is -active, so a stopped pipeline is not misreported as a worker problem. A5 fires -on the expired leases/stale work. A10 fires on sustained `grooming_unknown` -withholding. The operator checks the scheduler logs, then the grooming pipeline. +active, **and** the fresh-sync guard `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` fails because the last sync is 45m ago against a 15m interval. A stopped pipeline is therefore routed to A2 (sync freshness) and never to A4, so it is not misreported as a worker problem. A5 fires on the expired leases/stale work. A10 fires on sustained `grooming_unknown` withholding. The operator checks the scheduler logs, then the grooming pipeline. Recovery is `sync` re-arming; the stale sweep recovers ownership. Once the -scheduler is healthy again, A4 becomes the relevant signal if runs still do not -start. +scheduler is healthy again AND the sync is fresh, A4 becomes the relevant +signal if runs still do not start. ### 8.4 Degraded — worker hand-outs stranded (PR-fix) ``` dispatch_pr_fix_items{status="QUEUED"} 8 dispatch_pr_fix_unacked_handouts 5 +dispatch_pr_fix_overdue_unacked 5 dispatch_pr_fix_handout_age_seconds{quantile="0.5"} 2400 # 40m > 30m timeout dispatch_pr_fix_reclaims_total 2 dispatch_agent_runs_total{run_type="followup-pr"} 1 ``` -**Read:** A7 fires — hand-outs older than the 30m timeout without an ack. Either +**Read:** A7 fires — the **age-tied** `dispatch_pr_fix_overdue_unacked` is 5 +(the same items the histogram counts above the timeout). Either workers are not acknowledging (they should after durably materializing a run), or the reclaim sweep is disabled (default). Recovery: enable the reclaim sweep or have an operator requeue. This is distinct from A8 (head-of-line age) and from A9 (a human decision). +### 8.5 Healthy — PR-fix steady churn (A7 must NOT fire) + +``` +dispatch_pr_fix_items{status="QUEUED"} 3 +dispatch_pr_fix_unacked_handouts 2 # recent hand-outs exist +dispatch_pr_fix_overdue_unacked 0 # none past timeout +dispatch_pr_fix_handout_age_seconds{quantile="0.5"} 30 # < 60s old +dispatch_pr_fix_reclaims_total 0 +dispatch_agent_runs_total{run_type="followup-pr"} 47 +``` +**Read:** workers are acknowledging quickly and there is no stranded work. A7 +must **not** fire even though `dispatch_pr_fix_unacked_handouts > 0`. The +earlier counter-style predicate was hazardous for exactly this steady-churn +state — the aggregate held above zero whenever any item was recently handed +out, even with every individual hand-out acknowledged within seconds. The +age-tied `dispatch_pr_fix_overdue_unacked` is zero, so A7 stays silent and the +unacked aggregate is correctly surfaced on the dashboard only. + --- ## 9. Privacy, cardinality, and security From 89cee2a1945b4cdbef45899b6d71adbe1efb5107 Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 16:55:41 +0000 Subject: [PATCH 3/8] docs(observability): correct sync-status values and make A4 pickup-time based MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #1239: - IssueSyncRun persists running/completed/failed, never 'success'; sync freshness and failure signals now match 'completed'/'failed', with the observed enum documented and cited. A2 gains an explicit absent-series case so a never-successful first sync fires instead of going silent. - AgentRun rows are inserted at tasks/report finish with a retrospective optional startedAt, so dispatch_agent_runs_started_in_window measures reported starts, not pickups; its blind spot is documented. - New pickup-time signal dispatch_work_pickups_in_window (AgentWork.createdAt at agent-work/start, new Lease.createdAt at claim) and derived dispatch_execution_active guard replace it in A4, so a long-running worker in flight no longer false-fires A4. - New §8.6 worked example (in-flight worker, no report yet, A4 silent); normal-inactivity list and §5 execution row updated accordingly. --- docs/observability.md | 62 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index 12f96bbc..92bbf53d 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -157,6 +157,7 @@ Assignment is label-only (`agent/*`, `owner/*`) and has no lease. Execution is a | `dispatch_lease_age_seconds` | histogram | seconds | `now - Lease.renewedAt` / `createdAt` | — | How long ownership has been held | | `dispatch_leases_expired` | gauge | count | `Lease.expiredAt <= now` | — | Ownership past TTL (recoverable) | | `dispatch_assigned_without_execution` | gauge | count | assigned (`agent/*`) and no active lease/`AgentWork` | `lane` | Assignment that never became execution | +| `dispatch_execution_active` | gauge | 0/1 | derived: `dispatch_leases_active >= 1` **or** any `AgentWork` in `CLAIMED`/`IN_PROGRESS` | — | Something is executing right now. Used as the A4 in-flight guard: a task picked up before the pickup window still suppresses A4 for its whole duration | `DEFAULT_LEASE_TTL_MS = 30 min` (`src/lib/lease.ts:18`). Lease active means `expiredAt > now`; there is no lease status column — the issue's status label is @@ -270,8 +271,8 @@ must be allowlisted with an `unknown` bucket. | `dispatch_scheduler_up` | gauge | 0/1 | scheduler enabled + token present | — | Scheduler armed at all | | `dispatch_scheduler_job_last_success_timestamp_seconds` | gauge | unix seconds | `schedulerState().jobs[].lastRunAt` | `job` (8 names) | Last completed tick | | `dispatch_scheduler_job_overdue` | gauge | 0/1 | `schedulerState().jobs[].overdue` | `job` | No run in > 2× interval | -| `dispatch_sync_last_success_timestamp_seconds` | gauge | unix seconds | newest `IssueSyncRun.completedAt` with status success | `type` | Supply-pipeline freshness | -| `dispatch_sync_failures_total` | gauge (windowed) | count | `IssueSyncRun.status != success` | `type` | Failed syncs | +| `dispatch_sync_last_success_timestamp_seconds` | gauge | unix seconds | newest `IssueSyncRun.completedAt` with `status = "completed"` | `type` | Supply-pipeline freshness. Emitted **only when a completed run exists** — on a fresh deployment with no sync yet there is no series, and absence must be evaluated as stale, not silent (see A2) | +| `dispatch_sync_failures_total` | gauge (windowed) | count | `IssueSyncRun.status = "failed"` | `type` | Failed syncs | | `dispatch_repo_stale_sync_seconds` | gauge | seconds | `now - AutomationRepo.lastSyncedAt` | — | Per-repo cache staleness (aggregate max) | | `dispatch_health_up` | gauge | 0/1 | `GET /api/health` ok | — | Process + DB liveness | | `dispatch_scheduler_healthy` | gauge | 0/1 | derived: `dispatch_scheduler_up == 1` **and** no `dispatch_scheduler_job_overdue == 1` | — | Scheduler armed and every job ticking on time | @@ -281,21 +282,31 @@ ci-failures, prune-closed, reconcile, stale-work, pr-fix-sweep`. `overdue` means no run in more than two intervals; `lastRunAt` is in-process and resets on restart, so a freshly started process reports `null` until the first tick. +`IssueSyncRun.status` is a free string whose only persisted values are +`running` (created at lock acquisition, `src/lib/sync-lock.ts:160`), +`completed` (manual and scheduled success paths, +`src/app/api/sync/route.ts:67`, `src/app/api/sync/scheduled/route.ts:151`), and +`failed` (error path, `src/app/api/sync/scheduled/route.ts:227`). There is **no** +`success` value: predicates must match `"completed"`/`"failed"`, allowlist these, +and bucket anything else as `unknown`. + ### 4.10 Failure and outcome signals | Signal | Kind | Unit | Source | Dimensions | Semantics | |---|---|---|---|---|---| | `dispatch_audit_failures_total` | gauge (windowed) | count | `AuditLog.success = false` | `action` (allowlisted) | Failed mutations | | `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed delta only**: the rows are append-only and the window is the reset, so the exporter computes rates over the chosen window (e.g. `[5m]`, `[1h]`). It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions | -| `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs that **started in the previous N seconds**, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Distinguishes "no starts" from "no windowed total", and survives process restarts because it is derived from the row timestamp | +| `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs whose **reported start time** falls in the previous N seconds, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Survives process restarts because it is derived from the row timestamp. **Blind spot:** `AgentRun` rows are inserted by `tasks/report` when work *finishes* (`src/app/api/agents/[agentName]/tasks/report/route.ts:571`), and `startedAt` is a retrospective optional field defaulting to report time. A worker still mid-task (running longer than the window) is invisible to this gauge until it reports. It measures *reported starts*, not real-time pickups — do not use it alone to detect "nothing being handed out"; use `dispatch_work_pickups_in_window` for that | +| `dispatch_work_pickups_in_window` | gauge (as-of-scrape) | count | new `AgentWork` rows (`AgentWork.createdAt >= now() - window`) **or** newly created `Lease` rows (`Lease.createdAt >= now() - window`) | `window` (allowlisted: `1800s`, `3600s`) | Work **picked up** in the previous N seconds, recorded at pickup time: `AgentWork` rows are created by `POST /api/agent-work/start` (`src/lib/agent-work.ts:186`) and `Lease` rows by claim via `upsertLease` (`src/lib/lease.ts:69`, `createdAt = DB now()`). Lease *renewals* do not count (they reuse the original `createdAt`). Because the timestamp is written when the worker takes the work — not when it reports — a just-started long task counts immediately even if its report lands an hour later (or never) | | `dispatch_agent_run_failures_total` | gauge (windowed) | count | `AgentRun.status = failed` | `run_type` | Hard run failures | | `dispatch_scheduler_job_errors_total` | gauge (windowed) | count | job HTTP non-2xx/409 (logs today) | `job` | Requires a future counter; **unknown** today | `AuditLog.action`, `AgentRun.status`, and `AgentRun.runType` are free strings. Allowlist the observed values and bucket the rest as `unknown`; never label by `errorMessage`. The default `window` allowlist for `dispatch_agent_runs_started_in_window` -covers the alerting needs in [§6](#6-alert-catalogue); consumers may add other -windows but must keep cardinality bounded. +and `dispatch_work_pickups_in_window` covers the alerting needs in +[§6](#6-alert-catalogue); consumers may add other windows but must keep +cardinality bounded. --- @@ -308,7 +319,7 @@ layout is rows of related signals. | Row | Views (predicate) | Question answered | |---|---|---| | **1. Work supply** | `dispatch_issues` by `status`/`lane`; `dispatch_issues_open_claimable`; `dispatch_issue_age_seconds` p50/p95/max | Is there eligible work, and how old is it? | -| **2. Execution** | `dispatch_issue_assigned`; `dispatch_leases_active`; `dispatch_agent_work` by `state`; `dispatch_agent_work_heartbeat_age_seconds` | Is assigned work actually executing? | +| **2. Execution** | `dispatch_issue_assigned`; `dispatch_leases_active`; `dispatch_execution_active`; `dispatch_work_pickups_in_window`; `dispatch_agent_work` by `state`; `dispatch_agent_work_heartbeat_age_seconds` | Is assigned work actually executing, and is new work being picked up? | | **3. Ownership health** | `dispatch_leases_expired`; `dispatch_agent_work` `STALE`; `dispatch_assigned_without_execution` | Is any ownership stale or orphaned? | | **4. PR-fix supply** | `dispatch_pr_fix_items` by `status`/`lane`; `dispatch_pr_fix_oldest_queued_seconds` | What PR-fix work is queued, and for how long? | | **5. PR-fix hand-out health** | `dispatch_pr_fix_unacked_handouts`; `dispatch_pr_fix_handout_age_seconds`; `dispatch_pr_fix_reclaims_total` | Are hand-outs reaching and being acknowledged by workers? | @@ -348,9 +359,9 @@ recommendation; the consumer maps it. | # | Alert | Condition (predicate) | Severity | `for` | Why actionable | Recovery / operator action | |---|---|---|---|---|---|---| | A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | -| A2 | No successful sync | `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale; issues will not enter the queue | Check `IssueSyncRun` errors and GitHub reachability | +| A2 | No successful sync | `dispatch_sync_last_success_timestamp_seconds` series **absent** (no `IssueSyncRun` has ever completed — the never-successful first sync must not be silent) **or** `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale, or has never flowed; issues will not enter the queue. Consumers must evaluate the absent-series case as firing (e.g. `absent(...) == 1` in Prometheus), because `time() - > x` yields no data, not true | Check `IssueSyncRun` errors and GitHub reachability | | A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | -| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent, yet no implement run has started in the last 30 minutes — supply is not reaching workers. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement` | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. The pickup signal is written when a worker claims/starts work (`AgentWork.createdAt` / `Lease.createdAt`), not when it reports, so a task in flight keeps `dispatch_execution_active` at 1 and suppresses A4 for its full duration — a long-running worker (or a slow one that never reports) is never misread as "nothing handed out" (see §8.6); a *hung* task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement` | | A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | | A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | | A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | @@ -376,6 +387,10 @@ recommendation; the consumer maps it. `grooming_unknown`): **no alert**. Never-groomed backlog is not a fault. - `lastRunAt == null` immediately after process start: **no alert** until at least one interval has elapsed. +- One or more workers mid-task: **no alert** for A4. In-flight work keeps + `dispatch_execution_active == 1` (live lease or active `AgentWork` row) for + the whole task, independent of when — or whether — the worker reports; see + §8.6. A hung task is caught by A5 and the heartbeat-age signal, not A4. Alerts must be gated on **eligible work existing** (A4, A6) or on a **pipeline that should have run** (A1–A3, A13) or on an **overdue age** (A7, A8, A9, A11). @@ -455,7 +470,8 @@ dispatch_scheduler_up 1 dispatch_scheduler_job_overdue{job="sync"} 1 # last tick 45m ago, interval 15m dispatch_sync_last_success_timestamp_seconds (45m ago) dispatch_issues_open_claimable 20 -dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} 0 # nothing starting +dispatch_execution_active 0 # nothing executing +dispatch_work_pickups_in_window{window="1800s"} 0 # nothing picked up dispatch_leases_expired 2 dispatch_agent_work{state="STALE"} 2 dispatch_queue_withheld{reason="grooming_unknown"} 6 @@ -465,7 +481,7 @@ fire here: its `dispatch_scheduler_healthy == 1` guard is false while A1 is active, **and** the fresh-sync guard `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` fails because the last sync is 45m ago against a 15m interval. A stopped pipeline is therefore routed to A2 (sync freshness) and never to A4, so it is not misreported as a worker problem. A5 fires on the expired leases/stale work. A10 fires on sustained `grooming_unknown` withholding. The operator checks the scheduler logs, then the grooming pipeline. Recovery is `sync` re-arming; the stale sweep recovers ownership. Once the scheduler is healthy again AND the sync is fresh, A4 becomes the relevant -signal if runs still do not start. +signal if work still is not picked up. ### 8.4 Degraded — worker hand-outs stranded (PR-fix) @@ -502,6 +518,32 @@ out, even with every individual hand-out acknowledged within seconds. The age-tied `dispatch_pr_fix_overdue_unacked` is zero, so A7 stays silent and the unacked aggregate is correctly surfaced on the dashboard only. +### 8.6 Healthy — long-running worker, no report yet (A4 must NOT fire) + +``` +dispatch_health_up 1 +dispatch_scheduler_up 1 +dispatch_scheduler_job_overdue{job="sync"} 0 +dispatch_sync_last_success_timestamp_seconds (6m ago) +dispatch_issues_open_claimable 7 # backlog exists +dispatch_execution_active 1 # lease live, AgentWork IN_PROGRESS +dispatch_leases_active 1 +dispatch_agent_work{state="IN_PROGRESS"} 1 +dispatch_agent_work_heartbeat_age_seconds 40 # seconds — worker alive +dispatch_work_pickups_in_window{window="1800s"} 0 # the pickup was 45m ago +dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} 0 # nothing reported yet +``` +**Read:** one worker picked up an implement task 45 minutes ago and is still +executing; the backlog is real but the fleet is not idle. A4 must **not** fire: +`dispatch_execution_active == 1` fails its in-flight guard even though both +30-minute window gauges are zero. This is the state a *reported-starts*-only +predicate misfires on, because `AgentRun` rows — and their optional +retrospective `startedAt` — only exist once `tasks/report` lands. If this +worker instead died mid-task without reporting, its lease expires and A5 fires +(expired lease / stale work), and a live-but-silent worker is caught by the +`dispatch_agent_work_heartbeat_age_seconds` trend — not A4. Only when nothing +is executing **and** nothing new is picked up does A4 become the relevant alert. + --- ## 9. Privacy, cardinality, and security From 11a2eb54ef5e53dce72ad3ae616bf1fc44732763 Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 17:11:42 +0000 Subject: [PATCH 4/8] docs(observability): close pickup-signal holes from independent review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of 89cee2a found two design holes; both verified against source and fixed: - System-emitter contamination: the hosted groomer writes its own Lease rows (groomer/run.ts:250) and runType="groom" AgentRun rows (groomer/run.ts:1196), which would permanently fake pickups/busy state. Pickup/execution/reported signals now exclude system identities via an exporter-side filter (agent name stays out of labels per the privacy section). - Canonical-loop coverage: next-task -> execute -> report writes no claim/AgentWork row, so pickups+execution alone starve A4's evidence for report-only fleets. Added dispatch_agent_runs_reported_in_window (AgentRun.createdAt at tasks/report insert) as a third A4 suppressor and documented the residual blind spot (mid-task, unmaterialized worker) as an explicit 'Dispatch cannot observe this fleet' warning tied to the acknowledgement-gap open question. - execution_active excludes BLOCKED rows (active but not flowing); noted against the §4.4 active-state list. - dispatch_sync_failures_total undercounts manual sync errors (manual catch returns 500 without persisting 'failed', sync/route.ts:80-82); A2 freshness is the backstop — documented. - §8.6 example made internally consistent: lease liveness attributed to the re-claim renewal path and AgentWork checkpoint heartbeat. --- docs/observability.md | 64 +++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index 92bbf53d..6ce46a90 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -157,7 +157,7 @@ Assignment is label-only (`agent/*`, `owner/*`) and has no lease. Execution is a | `dispatch_lease_age_seconds` | histogram | seconds | `now - Lease.renewedAt` / `createdAt` | — | How long ownership has been held | | `dispatch_leases_expired` | gauge | count | `Lease.expiredAt <= now` | — | Ownership past TTL (recoverable) | | `dispatch_assigned_without_execution` | gauge | count | assigned (`agent/*`) and no active lease/`AgentWork` | `lane` | Assignment that never became execution | -| `dispatch_execution_active` | gauge | 0/1 | derived: `dispatch_leases_active >= 1` **or** any `AgentWork` in `CLAIMED`/`IN_PROGRESS` | — | Something is executing right now. Used as the A4 in-flight guard: a task picked up before the pickup window still suppresses A4 for its whole duration | +| `dispatch_execution_active` | gauge | 0/1 | derived: an unexpired `Lease` **or** an `AgentWork` in `CLAIMED`/`IN_PROGRESS` exists, **counted for worker identities only** (see the system-emitter exclusion below) | — | Something is executing right now. Used as the A4 in-flight guard: a task picked up before the pickup window still suppresses A4 for its whole duration. `BLOCKED` AgentWork rows are deliberately excluded — blocked work is active (§4.4) but not flowing, and must not mask an otherwise idle fleet | `DEFAULT_LEASE_TTL_MS = 30 min` (`src/lib/lease.ts:18`). Lease active means `expiredAt > now`; there is no lease status column — the issue's status label is @@ -165,6 +165,16 @@ the state (`src/lib/lease.ts:99-112`). Orphan leases are released and audited: a corrupted checkpoint at `src/lib/lease.ts:168-180`, and a lease whose referenced issue is missing at `src/lib/lease.ts:191-204`. +**System-emitter exclusion.** The hosted groomer takes its own lease without any +worker involvement (`upsertLease({ agentName: "hosted-groomer", … })`, +`src/lib/groomer/run.ts:250-254`) on its normal schedule, and it writes +`AgentRun` rows with `runType: "groom"` as internal bookkeeping +(`src/lib/groomer/run.ts:1196-1200`). Groomer leases and `groom` runs must be +excluded from `dispatch_execution_active`, `dispatch_work_pickups_in_window`, and +`dispatch_agent_runs_reported_in_window` — otherwise a running groomer makes the +fleet look permanently busy. The exclusion is an exporter-side filter on +`agentName`/`runType`, never a metric label (§9). + ### 4.4 AgentWork lifecycle | Signal | Kind | Unit | Source | Dimensions | Semantics | @@ -288,7 +298,11 @@ restart, so a freshly started process reports `null` until the first tick. `src/app/api/sync/route.ts:67`, `src/app/api/sync/scheduled/route.ts:151`), and `failed` (error path, `src/app/api/sync/scheduled/route.ts:227`). There is **no** `success` value: predicates must match `"completed"`/`"failed"`, allowlist these, -and bucket anything else as `unknown`. +and bucket anything else as `unknown`. Note the coverage gap: the manual-sync +error path returns a 500 **without** persisting `failed` +(`src/app/api/sync/route.ts:80-82`), leaving the row `running` forever — so +`dispatch_sync_failures_total` undercounts manual failures and A2 freshness is +the backstop for them. ### 4.10 Failure and outcome signals @@ -296,15 +310,17 @@ and bucket anything else as `unknown`. |---|---|---|---|---|---| | `dispatch_audit_failures_total` | gauge (windowed) | count | `AuditLog.success = false` | `action` (allowlisted) | Failed mutations | | `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed delta only**: the rows are append-only and the window is the reset, so the exporter computes rates over the chosen window (e.g. `[5m]`, `[1h]`). It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions | -| `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs whose **reported start time** falls in the previous N seconds, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Survives process restarts because it is derived from the row timestamp. **Blind spot:** `AgentRun` rows are inserted by `tasks/report` when work *finishes* (`src/app/api/agents/[agentName]/tasks/report/route.ts:571`), and `startedAt` is a retrospective optional field defaulting to report time. A worker still mid-task (running longer than the window) is invisible to this gauge until it reports. It measures *reported starts*, not real-time pickups — do not use it alone to detect "nothing being handed out"; use `dispatch_work_pickups_in_window` for that | -| `dispatch_work_pickups_in_window` | gauge (as-of-scrape) | count | new `AgentWork` rows (`AgentWork.createdAt >= now() - window`) **or** newly created `Lease` rows (`Lease.createdAt >= now() - window`) | `window` (allowlisted: `1800s`, `3600s`) | Work **picked up** in the previous N seconds, recorded at pickup time: `AgentWork` rows are created by `POST /api/agent-work/start` (`src/lib/agent-work.ts:186`) and `Lease` rows by claim via `upsertLease` (`src/lib/lease.ts:69`, `createdAt = DB now()`). Lease *renewals* do not count (they reuse the original `createdAt`). Because the timestamp is written when the worker takes the work — not when it reports — a just-started long task counts immediately even if its report lands an hour later (or never) | +| `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs whose **reported start time** falls in the previous N seconds, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Survives process restarts because it is derived from the row timestamp. **Blind spot:** `AgentRun` rows are inserted by `tasks/report` when work *finishes* (`src/app/api/agents/[agentName]/tasks/report/route.ts:571`), and `startedAt` is a retrospective optional field defaulting to report time. A worker still mid-task (running longer than the window) is invisible to this gauge until it reports. It measures *reported starts*, not real-time pickups — do not use it alone to detect "nothing being handed out"; use `dispatch_work_pickups_in_window` / `dispatch_agent_runs_reported_in_window` for that | +| `dispatch_work_pickups_in_window` | gauge (as-of-scrape) | count | new `AgentWork` rows (`AgentWork.createdAt >= now() - window`) **or** newly created `Lease` rows (`Lease.createdAt >= now() - window`), worker identities only (system-emitter exclusion, §4.3) | `window` (allowlisted: `1800s`, `3600s`) | Work **picked up** in the previous N seconds, recorded at pickup time: `AgentWork` rows are created by `POST /api/agent-work/start` (`src/lib/agent-work.ts:186`) and `Lease` rows by claim via `upsertLease` (`src/lib/lease.ts:69`, `createdAt = DB now()`). Lease *renewals* do not count (they reuse the original `createdAt`) | +| `dispatch_agent_runs_reported_in_window` | gauge (as-of-scrape) | count | `AgentRun.createdAt >= now() - window` (row insertion by `tasks/report`), worker run types only — `groom` runs from the hosted groomer are excluded (§4.3) | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Completed tasks **reported** in the previous N seconds. Throughput evidence for the canonical loop (next-task → execute → report), which — unlike claim/agent-work — writes no pickup row, so pickups alone under-count a fleet that delivers without materializing a lease. A row lands only at finish, so this gauge cannot see a single task still in flight; that is what `dispatch_execution_active` covers | | `dispatch_agent_run_failures_total` | gauge (windowed) | count | `AgentRun.status = failed` | `run_type` | Hard run failures | | `dispatch_scheduler_job_errors_total` | gauge (windowed) | count | job HTTP non-2xx/409 (logs today) | `job` | Requires a future counter; **unknown** today | `AuditLog.action`, `AgentRun.status`, and `AgentRun.runType` are free strings. Allowlist the observed values and bucket the rest as `unknown`; never label by -`errorMessage`. The default `window` allowlist for `dispatch_agent_runs_started_in_window` -and `dispatch_work_pickups_in_window` covers the alerting needs in +`errorMessage`. The default `window` allowlist shared by +`dispatch_agent_runs_started_in_window`, `dispatch_work_pickups_in_window`, and +`dispatch_agent_runs_reported_in_window` covers the alerting needs in [§6](#6-alert-catalogue); consumers may add other windows but must keep cardinality bounded. @@ -319,7 +335,7 @@ layout is rows of related signals. | Row | Views (predicate) | Question answered | |---|---|---| | **1. Work supply** | `dispatch_issues` by `status`/`lane`; `dispatch_issues_open_claimable`; `dispatch_issue_age_seconds` p50/p95/max | Is there eligible work, and how old is it? | -| **2. Execution** | `dispatch_issue_assigned`; `dispatch_leases_active`; `dispatch_execution_active`; `dispatch_work_pickups_in_window`; `dispatch_agent_work` by `state`; `dispatch_agent_work_heartbeat_age_seconds` | Is assigned work actually executing, and is new work being picked up? | +| **2. Execution** | `dispatch_issue_assigned`; `dispatch_leases_active`; `dispatch_execution_active`; `dispatch_work_pickups_in_window`; `dispatch_agent_runs_reported_in_window`; `dispatch_agent_work` by `state`; `dispatch_agent_work_heartbeat_age_seconds` | Is assigned work actually executing, and is new work being picked up and reported? | | **3. Ownership health** | `dispatch_leases_expired`; `dispatch_agent_work` `STALE`; `dispatch_assigned_without_execution` | Is any ownership stale or orphaned? | | **4. PR-fix supply** | `dispatch_pr_fix_items` by `status`/`lane`; `dispatch_pr_fix_oldest_queued_seconds` | What PR-fix work is queued, and for how long? | | **5. PR-fix hand-out health** | `dispatch_pr_fix_unacked_handouts`; `dispatch_pr_fix_handout_age_seconds`; `dispatch_pr_fix_reclaims_total` | Are hand-outs reaching and being acknowledged by workers? | @@ -361,7 +377,7 @@ recommendation; the consumer maps it. | A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | | A2 | No successful sync | `dispatch_sync_last_success_timestamp_seconds` series **absent** (no `IssueSyncRun` has ever completed — the never-successful first sync must not be silent) **or** `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale, or has never flowed; issues will not enter the queue. Consumers must evaluate the absent-series case as firing (e.g. `absent(...) == 1` in Prometheus), because `time() - > x` yields no data, not true | Check `IssueSyncRun` errors and GitHub reachability | | A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | -| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. The pickup signal is written when a worker claims/starts work (`AgentWork.createdAt` / `Lease.createdAt`), not when it reports, so a task in flight keeps `dispatch_execution_active` at 1 and suppresses A4 for its full duration — a long-running worker (or a slow one that never reports) is never misread as "nothing handed out" (see §8.6); a *hung* task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement` | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the hosted groomer's system rows (§4.3), so a busy groomer cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | | A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | | A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | | A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | @@ -526,23 +542,28 @@ dispatch_scheduler_up 1 dispatch_scheduler_job_overdue{job="sync"} 0 dispatch_sync_last_success_timestamp_seconds (6m ago) dispatch_issues_open_claimable 7 # backlog exists -dispatch_execution_active 1 # lease live, AgentWork IN_PROGRESS -dispatch_leases_active 1 +dispatch_execution_active 1 # AgentWork IN_PROGRESS (worker identity) +dispatch_leases_active 1 # kept live by the worker's re-claim renewal (src/lib/lease.ts:55-57) dispatch_agent_work{state="IN_PROGRESS"} 1 -dispatch_agent_work_heartbeat_age_seconds 40 # seconds — worker alive +dispatch_agent_work_heartbeat_age_seconds 40 # seconds — checkpoint heartbeat extends lastHeartbeatAt (src/lib/agent-work.ts:220-227) dispatch_work_pickups_in_window{window="1800s"} 0 # the pickup was 45m ago -dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} 0 # nothing reported yet +dispatch_agent_runs_reported_in_window{window="1800s"} 0 # nothing finished/reported yet +dispatch_agent_runs_started_in_window{run_type="implement", window="1800s"} 0 # no report row exists at all ``` **Read:** one worker picked up an implement task 45 minutes ago and is still executing; the backlog is real but the fleet is not idle. A4 must **not** fire: -`dispatch_execution_active == 1` fails its in-flight guard even though both -30-minute window gauges are zero. This is the state a *reported-starts*-only -predicate misfires on, because `AgentRun` rows — and their optional -retrospective `startedAt` — only exist once `tasks/report` lands. If this -worker instead died mid-task without reporting, its lease expires and A5 fires -(expired lease / stale work), and a live-but-silent worker is caught by the +`dispatch_execution_active == 1` fails its in-flight guard even though all +three 30-minute window gauges are zero. This is the state a +*reported-starts*-only predicate misfires on, because `AgentRun` rows — and +their optional retrospective `startedAt` — only exist once `tasks/report` +lands. If this worker died mid-task, its checkpoint heartbeats stop, the +agent-work sweep marks the row `STALE` (§4.4) and the lease expires — A5 fires +on stale work/expired ownership; a live-but-silent worker is caught by the `dispatch_agent_work_heartbeat_age_seconds` trend — not A4. Only when nothing -is executing **and** nothing new is picked up does A4 become the relevant alert. +is executing, nothing new is picked up, **and** nothing is reported does A4 +become the relevant alert. (A fleet that delivers through the canonical loop +without ever materializing a claim or agent-work row is unobservable mid-task; +see the blind spot stated in A4.) --- @@ -591,7 +612,10 @@ design, not a decision. - **Reclaim observability:** the `pr-fix-sweep` job is default-disabled, so reclaim signals may be absent; decide how to surface that state explicitly. - **Acknowledgement gap:** issue claims have no ack (only PR-fix hand-outs do); - decide whether stale-claim detection is sufficient without one. + decide whether stale-claim detection is sufficient without one. Related: the + canonical loop (next-task → execute → report) writes no pickup row, so A4's + mid-task visibility depends on workers materializing a claim or agent-work + start ([§6 A4](#6-alert-catalogue) documents the residual blind spot). - **Retention and windowing:** how long event tables feed windowed totals, and whether the exporter queries the primary or a replica. - **OTel semantic conventions:** whether to map to standard metric/log From c087b0b75716bc6a0da899a7e6969361569a4f88 Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 17:21:26 +0000 Subject: [PATCH 5/8] docs(observability): allowlist task run types in the reported-in-window gauge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second independent review pass found the reported-in-window gauge filter ("worker run types, exclude groom") repeated the system-emitter leak via the heartbeat route's runType="heartbeat" AgentRun rows (heartbeat/route.ts:100-104) and the legacy agent-runs ingestion route (agent-runs/route.ts:78): a polling-only fleet would keep the gauge above zero and permanently disarm A4. The filter is now an allowlist of the task run types (implement/followup-pr), and the §4.3 exclusion paragraph names all three non-task emitters. Also scoped the §6 mid-task bullet to materialized in-flight work so it cannot be read against A4's documented blind spot. --- docs/observability.md | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index 6ce46a90..8ad46305 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -169,10 +169,18 @@ referenced issue is missing at `src/lib/lease.ts:191-204`. worker involvement (`upsertLease({ agentName: "hosted-groomer", … })`, `src/lib/groomer/run.ts:250-254`) on its normal schedule, and it writes `AgentRun` rows with `runType: "groom"` as internal bookkeeping -(`src/lib/groomer/run.ts:1196-1200`). Groomer leases and `groom` runs must be -excluded from `dispatch_execution_active`, `dispatch_work_pickups_in_window`, and -`dispatch_agent_runs_reported_in_window` — otherwise a running groomer makes the -fleet look permanently busy. The exclusion is an exporter-side filter on +(`src/lib/groomer/run.ts:1196-1200`). The heartbeat endpoint likewise writes an +`AgentRun` row per poll with `runType: "heartbeat"` +(`src/app/api/agents/[agentName]/heartbeat/route.ts:100-104`) for the worker's +own identity, and the legacy ingestion route accepts client-supplied run types +(`src/app/api/agent-runs/route.ts:78`). These are not task flow: groomer leases +and every non-task `AgentRun` row must be excluded from +`dispatch_execution_active`, `dispatch_work_pickups_in_window`, and +`dispatch_agent_runs_reported_in_window` — the last by **allowlisting task run +types** (`implement`, `followup-pr`) rather than subtracting `groom`, so a +heartbeat-only, zero-delivery fleet cannot keep the reported gauge above zero. +Otherwise a running groomer or a merely polling fleet makes the fleet look +permanently busy. The exclusion is an exporter-side filter on `agentName`/`runType`, never a metric label (§9). ### 4.4 AgentWork lifecycle @@ -312,7 +320,7 @@ the backstop for them. | `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed delta only**: the rows are append-only and the window is the reset, so the exporter computes rates over the chosen window (e.g. `[5m]`, `[1h]`). It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions | | `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs whose **reported start time** falls in the previous N seconds, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Survives process restarts because it is derived from the row timestamp. **Blind spot:** `AgentRun` rows are inserted by `tasks/report` when work *finishes* (`src/app/api/agents/[agentName]/tasks/report/route.ts:571`), and `startedAt` is a retrospective optional field defaulting to report time. A worker still mid-task (running longer than the window) is invisible to this gauge until it reports. It measures *reported starts*, not real-time pickups — do not use it alone to detect "nothing being handed out"; use `dispatch_work_pickups_in_window` / `dispatch_agent_runs_reported_in_window` for that | | `dispatch_work_pickups_in_window` | gauge (as-of-scrape) | count | new `AgentWork` rows (`AgentWork.createdAt >= now() - window`) **or** newly created `Lease` rows (`Lease.createdAt >= now() - window`), worker identities only (system-emitter exclusion, §4.3) | `window` (allowlisted: `1800s`, `3600s`) | Work **picked up** in the previous N seconds, recorded at pickup time: `AgentWork` rows are created by `POST /api/agent-work/start` (`src/lib/agent-work.ts:186`) and `Lease` rows by claim via `upsertLease` (`src/lib/lease.ts:69`, `createdAt = DB now()`). Lease *renewals* do not count (they reuse the original `createdAt`) | -| `dispatch_agent_runs_reported_in_window` | gauge (as-of-scrape) | count | `AgentRun.createdAt >= now() - window` (row insertion by `tasks/report`), worker run types only — `groom` runs from the hosted groomer are excluded (§4.3) | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Completed tasks **reported** in the previous N seconds. Throughput evidence for the canonical loop (next-task → execute → report), which — unlike claim/agent-work — writes no pickup row, so pickups alone under-count a fleet that delivers without materializing a lease. A row lands only at finish, so this gauge cannot see a single task still in flight; that is what `dispatch_execution_active` covers | +| `dispatch_agent_runs_reported_in_window` | gauge (as-of-scrape) | count | task `AgentRun` rows (`AgentRun.createdAt >= now() - window`, `runType` allowlisted to the task types `implement`/`followup-pr` — heartbeat/groom/legacy rows excluded, §4.3) | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Completed tasks **reported** in the previous N seconds. Throughput evidence for the canonical loop (next-task → execute → report), which — unlike claim/agent-work — writes no pickup row, so pickups alone under-count a fleet that delivers without materializing a lease. A row lands only at finish, so this gauge cannot see a single task still in flight; that is what `dispatch_execution_active` covers | | `dispatch_agent_run_failures_total` | gauge (windowed) | count | `AgentRun.status = failed` | `run_type` | Hard run failures | | `dispatch_scheduler_job_errors_total` | gauge (windowed) | count | job HTTP non-2xx/409 (logs today) | `job` | Requires a future counter; **unknown** today | @@ -403,10 +411,12 @@ recommendation; the consumer maps it. `grooming_unknown`): **no alert**. Never-groomed backlog is not a fault. - `lastRunAt == null` immediately after process start: **no alert** until at least one interval has elapsed. -- One or more workers mid-task: **no alert** for A4. In-flight work keeps - `dispatch_execution_active == 1` (live lease or active `AgentWork` row) for - the whole task, independent of when — or whether — the worker reports; see - §8.6. A hung task is caught by A5 and the heartbeat-age signal, not A4. +- Workers mid-task **with the work materialized** (live lease or active + `AgentWork` row): **no alert** for A4 — in-flight work keeps + `dispatch_execution_active == 1` for the whole task, independent of when — or + whether — the worker reports; see §8.6. A claim-less canonical-loop worker + mid-single-task is the documented blind spot in A4, not this case. A hung + materialized task is caught by A5 and the heartbeat-age signal, not A4. Alerts must be gated on **eligible work existing** (A4, A6) or on a **pipeline that should have run** (A1–A3, A13) or on an **overdue age** (A7, A8, A9, A11). From 734ce300822bde40db803fec11ec21ea593811ae Mon Sep 17 00:00:00 2001 From: Courier Date: Tue, 13 Oct 2026 17:22:06 +0000 Subject: [PATCH 6/8] docs(observability): align A4 rationale wording with the full emitter filter --- docs/observability.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/observability.md b/docs/observability.md index 8ad46305..0be8d8a5 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -385,7 +385,7 @@ recommendation; the consumer maps it. | A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | | A2 | No successful sync | `dispatch_sync_last_success_timestamp_seconds` series **absent** (no `IssueSyncRun` has ever completed — the never-successful first sync must not be silent) **or** `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale, or has never flowed; issues will not enter the queue. Consumers must evaluate the absent-series case as firing (e.g. `absent(...) == 1` in Prometheus), because `time() - > x` yields no data, not true | Check `IssueSyncRun` errors and GitHub reachability | | A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | -| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the hosted groomer's system rows (§4.3), so a busy groomer cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the non-task system rows (groomer, heartbeat, legacy ingestion) per the §4.3 filter, so a busy groomer or a merely polling fleet cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | | A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | | A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | | A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | From 61179853ad983830a67dce46afe906cf38ed28b0 Mon Sep 17 00:00:00 2001 From: Courier Date: Wed, 14 Oct 2026 06:30:14 +0000 Subject: [PATCH 7/8] docs(observability): pin A6 as age future-gap; evaluate A12 via as-of-scrape in-window gauge A6: dispatch_assigned_without_execution is an aggregate that stays nonzero under healthy assignment churn, and per-item waiting age is not derivable from persisted columns (no assignedAt; updatedAt/lastSyncedAt refresh on unrelated changes; only actions-route assignments audit as assign_agent). Pin the alert as a future gap, mark the gauge dashboard-only, add the steady-churn no-alert example (8.7) and the age-source open question. A12: delta() over an already-windowed rolling count can cancel a fresh event to zero on window roll. Replace dispatch_audit_failures_total with dispatch_audit_failures_in_window (as-of-scrape, window allowlist) and evaluate it directly; align 4.1/4.10/5/7 with the rule that delta() is sound only on cumulative totals. --- docs/observability.md | 102 +++++++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 25 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index 0be8d8a5..195be0e3 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -111,17 +111,26 @@ freshness, or settlement outcomes as signals. This document specifies them. - **Kinds:** `gauge` (current value), `counter` (monotonic total), `histogram` (bounded buckets). Because Dispatch has no in-process event counters, most "counters" are derived from append-only tables and are **not process- - monotonic**. Model them either as gauges or as windowed deltas computed by the - exporter; document which. Counters derived from the DB do **not** reset on - process restart, unlike a conventional process counter. -- **`increase()` and `rate()` are counter-only.** They assume a monotonically - increasing series with well-defined resets. DB-derived totals in this document - are **windowed deltas**, not counters, so evaluators must query them via - `delta()`-style window deltas or, where the question is "did anything happen - in the last N", via an as-of-scrape gauge such as - `dispatch_<...>_in_window{..., window=""}` (see §4.10). Using - `increase()` on a gauge-derived series produces false negatives and positives - as the window rolls. + monotonic**. Model them either as gauges or as windowed counts recomputed by + the exporter at each scrape; document which. Counters derived from the DB do + **not** reset on process restart, unlike a conventional process counter. +- **`increase()`, `rate()`, and `delta()` are counter/cumulative-only — and the + distinction is load-bearing here.** They all assume a monotonically increasing + series. DB counts in this document are exported as **as-of-scrape windowed + gauges** — `dispatch_<...>_in_window{..., window=""}` (see §4.10) — + where each scrape recomputes the count over its window. Such a series is not + monotonic, so **no** rate-style function applies to it. In particular, + `delta()` is sound only over a *cumulative* (non-decreasing) exported total; + applied to an already-windowed count it silently loses events: a rolling 15m + failure count can read `1` at both ends of the evaluation window while an + older failure expired and a new one arrived, giving `delta == 0` and missing + the failure entirely. Therefore: "did anything happen in the last N" evaluates + the as-of-scrape gauge **directly** (e.g. + `dispatch_<...>_in_window{window="900s"} > 0`), never via `increase()`, + `rate()`, or `delta()`. The `_total` suffix that remains on some rows of this + catalogue is historical; those rows are windowed counts under this same rule, + not monotonic counters, and must not fire alerts through rate-style + evaluators. - **Timestamp semantics:** gauges are "as of scrape". Event-derived totals use the row's own timestamp (`createdAt`, `at`, `startedAt`/`completedAt`) and are windowed by the exporter (e.g. `[5m]`, `[1h]`). @@ -156,7 +165,7 @@ Assignment is label-only (`agent/*`, `owner/*`) and has no lease. Execution is a | `dispatch_leases_active` | gauge | count | `Lease.expiredAt > now` | — | Live ownership | | `dispatch_lease_age_seconds` | histogram | seconds | `now - Lease.renewedAt` / `createdAt` | — | How long ownership has been held | | `dispatch_leases_expired` | gauge | count | `Lease.expiredAt <= now` | — | Ownership past TTL (recoverable) | -| `dispatch_assigned_without_execution` | gauge | count | assigned (`agent/*`) and no active lease/`AgentWork` | `lane` | Assignment that never became execution | +| `dispatch_assigned_without_execution` | gauge | count | assigned (`agent/*`) and no active lease/`AgentWork` | `lane` | Assignment that never became execution. **Dashboard aggregate — must not fire alerts.** Steady assignment churn keeps it above zero continuously even when every assignment is picked up within seconds, and per-item waiting age is not derivable from persisted columns (`Issue` has no `assignedAt`; `updatedAt` is refreshed by any GitHub change at `src/lib/issue-sync.ts:115`, `lastSyncedAt` by claims/webhooks). See A6's future-gap pinning and §8.7 | | `dispatch_execution_active` | gauge | 0/1 | derived: an unexpired `Lease` **or** an `AgentWork` in `CLAIMED`/`IN_PROGRESS` exists, **counted for worker identities only** (see the system-emitter exclusion below) | — | Something is executing right now. Used as the A4 in-flight guard: a task picked up before the pickup window still suppresses A4 for its whole duration. `BLOCKED` AgentWork rows are deliberately excluded — blocked work is active (§4.4) but not flowing, and must not mask an otherwise idle fleet | `DEFAULT_LEASE_TTL_MS = 30 min` (`src/lib/lease.ts:18`). Lease active means @@ -316,8 +325,8 @@ the backstop for them. | Signal | Kind | Unit | Source | Dimensions | Semantics | |---|---|---|---|---|---| -| `dispatch_audit_failures_total` | gauge (windowed) | count | `AuditLog.success = false` | `action` (allowlisted) | Failed mutations | -| `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed delta only**: the rows are append-only and the window is the reset, so the exporter computes rates over the chosen window (e.g. `[5m]`, `[1h]`). It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions | +| `dispatch_audit_failures_in_window` | gauge (as-of-scrape) | count | `AuditLog.success = false` **and** `createdAt >= now() - window` | `action` (allowlisted), `window` (allowlisted: `900s`, `3600s`) | Failed mutations in the previous N seconds, recomputed at scrape time. Evaluated **directly** (e.g. `{window="900s"} > 0` for A12) — never via `increase()`, `rate()`, or `delta()` (§4.1). Replaces the earlier `dispatch_audit_failures_total` windowed gauge, which A12 misused through `delta()` | +| `dispatch_agent_runs_total` | gauge (windowed) | count | `AgentRun.runType`, `status` | `run_type`, `status` | Outcome distribution across `AgentRun` rows. **Windowed count only**: the rows are append-only and the window is the reset, so the exporter recomputes the count over the chosen window at each scrape. It is **not** process-monotonic and **must not** be evaluated with `increase()`-style rate functions (§4.1) | | `dispatch_agent_runs_started_in_window` | gauge (as-of-scrape) | count | `AgentRun.startedAt >= now() - window` | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Number of runs whose **reported start time** falls in the previous N seconds, computed at scrape time. Used directly (e.g. `== 0`) — never via `increase()`. Survives process restarts because it is derived from the row timestamp. **Blind spot:** `AgentRun` rows are inserted by `tasks/report` when work *finishes* (`src/app/api/agents/[agentName]/tasks/report/route.ts:571`), and `startedAt` is a retrospective optional field defaulting to report time. A worker still mid-task (running longer than the window) is invisible to this gauge until it reports. It measures *reported starts*, not real-time pickups — do not use it alone to detect "nothing being handed out"; use `dispatch_work_pickups_in_window` / `dispatch_agent_runs_reported_in_window` for that | | `dispatch_work_pickups_in_window` | gauge (as-of-scrape) | count | new `AgentWork` rows (`AgentWork.createdAt >= now() - window`) **or** newly created `Lease` rows (`Lease.createdAt >= now() - window`), worker identities only (system-emitter exclusion, §4.3) | `window` (allowlisted: `1800s`, `3600s`) | Work **picked up** in the previous N seconds, recorded at pickup time: `AgentWork` rows are created by `POST /api/agent-work/start` (`src/lib/agent-work.ts:186`) and `Lease` rows by claim via `upsertLease` (`src/lib/lease.ts:69`, `createdAt = DB now()`). Lease *renewals* do not count (they reuse the original `createdAt`) | | `dispatch_agent_runs_reported_in_window` | gauge (as-of-scrape) | count | task `AgentRun` rows (`AgentRun.createdAt >= now() - window`, `runType` allowlisted to the task types `implement`/`followup-pr` — heartbeat/groom/legacy rows excluded, §4.3) | `run_type` (allowlisted), `window` (allowlisted: `1800s`, `3600s`) | Completed tasks **reported** in the previous N seconds. Throughput evidence for the canonical loop (next-task → execute → report), which — unlike claim/agent-work — writes no pickup row, so pickups alone under-count a fleet that delivers without materializing a lease. A row lands only at finish, so this gauge cannot see a single task still in flight; that is what `dispatch_execution_active` covers | @@ -326,10 +335,11 @@ the backstop for them. `AuditLog.action`, `AgentRun.status`, and `AgentRun.runType` are free strings. Allowlist the observed values and bucket the rest as `unknown`; never label by -`errorMessage`. The default `window` allowlist shared by -`dispatch_agent_runs_started_in_window`, `dispatch_work_pickups_in_window`, and -`dispatch_agent_runs_reported_in_window` covers the alerting needs in -[§6](#6-alert-catalogue); consumers may add other windows but must keep +`errorMessage`. The default `window` allowlists shared by +`dispatch_agent_runs_started_in_window`, `dispatch_work_pickups_in_window`, +`dispatch_agent_runs_reported_in_window` (`1800s`, `3600s`) and +`dispatch_audit_failures_in_window` (`900s`, `3600s`) cover the alerting needs +in [§6](#6-alert-catalogue); consumers may add other windows but must keep cardinality bounded. --- @@ -351,7 +361,7 @@ layout is rows of related signals. | **7. Human decisions** | `dispatch_pr_fix_human_decisions`; `dispatch_pr_fix_oldest_human_decision_seconds`; `dispatch_queue_withheld` by `reason` | What genuinely needs a human, and how long has it waited? | | **8. Pipeline freshness** | `dispatch_scheduler_job_overdue`; `dispatch_scheduler_job_last_success_timestamp_seconds`; `dispatch_sync_last_success_timestamp_seconds`; `dispatch_repo_stale_sync_seconds`; `dispatch_health_up` | Did the supply pipeline stop? | | **9. Grooming** | `dispatch_grooming_freshness`; `dispatch_grooming_stale_reasons`; `dispatch_grooming_backoff` | Is the admission gate starved by stale/unknown grooming? | -| **10. Failures** | `dispatch_audit_failures_total`; `dispatch_agent_run_failures_total` | Are mutations/runs failing? | +| **10. Failures** | `dispatch_audit_failures_in_window{window="900s"}`; `dispatch_agent_run_failures_total` | Are mutations/runs failing? | **Separation requirement.** The dashboard must keep these distinct, because collapsing them produces wrong operator conclusions: @@ -387,13 +397,13 @@ recommendation; the consumer maps it. | A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | | A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the non-task system rows (groomer, heartbeat, legacy ingestion) per the §4.3 filter, so a busy groomer or a merely polling fleet cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | | A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | -| A6 | Assigned without execution | `dispatch_assigned_without_execution > 0` | warning | 30m | Assigned but never picked up; invisible in a queue-depth-only view | Verify the worker polls its lane; check admission withholding | +| A6 | Assigned without execution | **Future gap — must NOT fire on the aggregate.** Intended predicate: an individual **open, eligible** issue (not `done`/`blocked`) has carried `agent/*` with no active lease/`AgentWork` for longer than a threshold T. Per-item waiting age is not derivable from persisted columns today — label assignment carries no timestamp (`Issue` has no `assignedAt`; `updatedAt`/`lastSyncedAt` refresh on unrelated changes), and only actions-route assignments produce `assign_agent` audit rows (`src/app/api/issues/actions/route.ts:14`; claims audit as `claim_issue` but materialize a lease, so they fall outside this predicate), so a GitHub-side assignment has no age source at all. Firing `dispatch_assigned_without_execution > 0` sustained would misreport healthy churn as failure (§8.7), so this row stays pinned — an exporter that ships an age source promotes it to a real alert (§10) | info (pinned; not firing) | 30m (T, once age exists) | Assigned but never picked up; invisible in a queue-depth-only view. Until the age gap closes, execution-loss detection is owned by A5 (expired leases / STALE work) and pickup starvation by A4/A10 | Verify the worker polls its lane; check admission withholding | | A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | | A8 | PR-fix head-of-line | `dispatch_pr_fix_oldest_queued_seconds > threshold` | warning | 30m | Work is queued but not draining | Check worker concurrency, lane routing, and `fixAttempts` saturation | | A9 | Human decisions piling up | `dispatch_pr_fix_human_decisions > 0` **and** oldest age > threshold | info/warning | 1h | A person must decide; PARKED is excluded so this is real | Review the BLOCKED item's decision context and act | | A10 | Admission withholding | `dispatch_queue_withheld{reason="grooming_unknown"}` or `{reason="grooming_stale"}` sustained | warning | 1h | The admission gate is starving the queue on missing/stale grooming | Fix grooming pipeline; or operator admission override for a specific issue | | A11 | Grooming backoff | `dispatch_grooming_backoff > 0` sustained | warning | 30m | Upstream GitHub reads are failing; issues cannot be groomed | Check GitHub reachability/rate limits | -| A12 | Audit failures | `delta(dispatch_audit_failures_total[15m]) > 0` | warning | 5m | Mutations are failing (labels/claims). The predicate uses a window-delta evaluator (per §4.1) because `dispatch_audit_failures_total` is a gauge-derived total, not a monotonic counter | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | +| A12 | Audit failures | `dispatch_audit_failures_in_window{window="900s"} > 0` | warning | 5m | Mutations are failing (labels/claims). The predicate evaluates the as-of-scrape 15-minute gauge **directly**, so a new failure fires even when an older one expires from the window at the same moment — a `delta()` over a rolling windowed count could read zero there (§4.1) | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | | A13 | Upstream evidence unavailable | `dispatch_repo_stale_sync_seconds > 2× sync interval` **or** `dispatch_grooming_backoff > 0` | warning | 15m | Admission/settlement decisions lack current evidence; the forge could not be read | Check GitHub reachability/rate limits; treat `unknown` as unknown, never healthy | ### Normal inactivity must NOT alert @@ -406,6 +416,10 @@ recommendation; the consumer maps it. **no alert** for A7 even though `dispatch_pr_fix_unacked_handouts > 0` — the age-tied `dispatch_pr_fix_overdue_unacked` is zero. Aggregate unacked is surfaced on the dashboard only. +- Assignment steady-churn where every `agent/*` assignment gains a + lease/`AgentWork` within seconds: **no alert** for A6 even though + `dispatch_assigned_without_execution > 0` — see §8.7. A6 is pinned as a + future gap and never fires on the aggregate. - No `BLOCKED` items: **no alert**. Absence of a human decision is good. - No `status/ready` issue with `unknown` freshness (no issue withheld as `grooming_unknown`): **no alert**. Never-groomed backlog is not a fault. @@ -418,7 +432,8 @@ recommendation; the consumer maps it. mid-single-task is the documented blind spot in A4, not this case. A hung materialized task is caught by A5 and the heartbeat-age signal, not A4. -Alerts must be gated on **eligible work existing** (A4, A6) or on a **pipeline +Alerts must be gated on **eligible work existing** (A4; A6 is pinned as a +future gap and fires nothing until it gains per-item age) or on a **pipeline that should have run** (A1–A3, A13) or on an **overdue age** (A7, A8, A9, A11). "Nothing happened" is not a failure, and "something exists right now" is not a failure when nothing is past its timeout. @@ -433,7 +448,7 @@ choices at composition time. | Approach | Transport | State source | Dashboard choice | Evaluator choice | Cardinality control | Notes / gaps | |---|---|---|---|---|---|---| -| **A. Pull exporter (Prometheus / VictoriaMetrics)** | HTTP `/metrics` scrape | Read-only DB queries (state gauges, as-of-scrape windowed gauges, windowed deltas over append-only tables) | Any (Grafana, VM UI, custom) | Prometheus rules / vmalert / custom | Enforce label allowlists at the exporter | Needs a new read-only exporter; no in-process counters exist, so totals are windowed DB deltas. Use `delta()` (or direct comparison of `_in_window` as-of-scrape gauges) for "did anything happen in the last N" predicates — never `increase()` per §4.1 | +| **A. Pull exporter (Prometheus / VictoriaMetrics)** | HTTP `/metrics` scrape | Read-only DB queries (state gauges, as-of-scrape windowed gauges, windowed counts over append-only tables) | Any (Grafana, VM UI, custom) | Prometheus rules / vmalert / custom | Enforce label allowlists at the exporter | Needs a new read-only exporter; no in-process counters exist. Evaluate "did anything happen in the last N" predicates **directly** against the as-of-scrape `_in_window` gauges — never `increase()` (any series here is non-monotonic) and never `delta()` over an already-windowed count (window roll can cancel a fresh event to zero); `delta()` is sound only if the exporter instead emits cumulative totals (§4.1) | | **B. OTel push metrics** | OTLP push | Same as A, emitted as OTel instruments | Any OTel-compatible backend | OTel-native or downstream | Use OTel attribute allowlists; avoid high-cardinality attributes | Adds an OTel SDK/dependency; exporter/collector choice deferred | | **C. OTel logs/events** | OTLP logs | `AuditLog`, `PrFixHistory`, `AgentRun`, `IssueSyncRun` as events | Any log backend | Backend rules | Event attributes may carry IDs; **keep them out of metric labels** | Good for drill-down; not a substitute for gauges | | **D. Snapshot API** | HTTP JSON | A read-only `GET` that returns the catalogue as JSON | Any | Any | Consumer controls | Optional convenience; would be a small, separately-scoped implementation | @@ -575,6 +590,32 @@ become the relevant alert. (A fleet that delivers through the canonical loop without ever materializing a claim or agent-work row is unobservable mid-task; see the blind spot stated in A4.) +### 8.7 Healthy — assignment steady churn (A6 must NOT fire) + +``` +dispatch_health_up 1 +dispatch_scheduler_up 1 +dispatch_sync_last_success_timestamp_seconds (3m ago) +dispatch_issues{status="ready",state="open"} 5 +dispatch_issue_assigned 3 # label-assigned supply +dispatch_assigned_without_execution 1 # just assigned; gains a lease <60s later +dispatch_leases_active 2 +dispatch_execution_active 1 +dispatch_work_pickups_in_window{window="1800s"} 4 +dispatch_agent_runs_reported_in_window{window="1800s"} 3 +``` +**Read:** workers label-assign, then pick up within seconds, so the +`dispatch_assigned_without_execution` aggregate is nonzero at most scrapes — +each individual assignment waits under a minute, but the waits overlap. A +`for: 30m` timer over the aggregate does not help: overlapping short waits keep +it *continuously* nonzero and an aggregate-fired A6 would alert on a perfectly +healthy fleet. This is the same false-positive class the A7 review fix removed by +making the predicate age-tied, but A6 has no age-tied equivalent today: label +assignment carries no persisted timestamp, so per-item waiting age cannot be +computed (see A6's future-gap pinning and §10). This state must **not** alert; +real ownership loss here is caught by A5 (expired lease / `STALE` work) and +pickup starvation by A4/A10. + --- ## 9. Privacy, cardinality, and security @@ -607,8 +648,9 @@ design, not a decision. Trade-offs: pull is simple and scrape-native; push fits an existing collector; a snapshot API is vendor-neutral but adds an endpoint. - **Counter modeling:** since Dispatch has no in-process counters, decide - whether DB-derived totals are gauges or windowed deltas, and how the consumer - computes rates. + whether DB-derived totals are as-of-scrape windowed counts or exported + cumulative totals (the only shape `delta()` is sound on — §4.1), and how the + consumer evaluates them. - **Repo dimension:** whether `repo` is a bounded, acceptable label (small, configured tracked set) or must be aggregated. - **Scheduler state is in-process** (`globalThis`) and resets on restart; decide @@ -626,6 +668,16 @@ design, not a decision. canonical loop (next-task → execute → report) writes no pickup row, so A4's mid-task visibility depends on workers materializing a claim or agent-work start ([§6 A4](#6-alert-catalogue) documents the residual blind spot). +- **Assignment age source (A6 graduation):** A6 is pinned as a future gap + because "assigned without execution" has no per-item age. Candidate sources: + persist an `assignedAt`-style timestamp when an `agent/*` label first appears + in the sync diff (covers GitHub-side and Dispatch-side assignments alike), + or derive age from `assign_agent` AuditLog rows (covers only actions-route + assignments — GitHub-side label adds never hit the audit path, and claims + audit as `claim_issue` but materialize a lease, so they are outside A6's + predicate). No AuditLog + pruning exists today, but retention must be settled before a join-based age + is relied on. Decide before unpinning A6 ([§6 A6](#6-alert-catalogue), §8.7). - **Retention and windowing:** how long event tables feed windowed totals, and whether the exporter queries the primary or a replica. - **OTel semantic conventions:** whether to map to standard metric/log From f4b2c2e45ef0cb94aa1c3d91ae1acdebc8b04d03 Mon Sep 17 00:00:00 2001 From: Courier Date: Wed, 14 Oct 2026 07:50:16 +0000 Subject: [PATCH 8/8] docs(observability): make the alert catalogue and its invariants checkable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review finding that the document's stated invariants had no enforcement location: the catalogue, the IssueSyncRun status enum, and the "nothing happened must not alert" separation were asserted only in prose. - §6 gains an `Enabled` column that is the wiring contract: `yes` means a consumer may evaluate the predicate, `no` means the row documents a known gap and must not be wired into an evaluator. A6 is the only disabled row, so its `for` cell no longer carries a live `30m` duration that would make it look like a rule (it is now `— (T applies only when promoted)`). - New docs/observability.test.ts enforces, in npm run test: A1-A13 present once with required columns; disabled rows carry no duration and severity info; no alert condition uses increase()/rate()/delta() (§4.1); every signal named in an alert condition is defined in §4; every file:line citation resolves to an existing file and line; and the documented IssueSyncRun.status values equal those the three writer files actually write. Each check has a cardinality guard so it cannot pass vacuously. - Two bare `pr-fix-queue.ts:NNN` citations normalized to repo-relative paths. - New §11 states what the check covers (structure and source drift) and what it does not (alert runtime behavior — no exporter exists yet). Verified: npm run lint (0 errors, 1 pre-existing warning in src/app/login/page.tsx:39), npm run typecheck (clean), npm run test (4311 passed, 16 skipped, 1 todo). Mutation-checked: restoring A6's duration, an undefined signal name, and an out-of-range citation each fail the new file. Co-Authored-By: opencode --- docs/observability.md | 67 +++++++++--- docs/observability.test.ts | 208 +++++++++++++++++++++++++++++++++++++ 2 files changed, 258 insertions(+), 17 deletions(-) create mode 100644 docs/observability.test.ts diff --git a/docs/observability.md b/docs/observability.md index 195be0e3..9c337093 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -232,9 +232,9 @@ lanes `NORMAL, ESCALATED, NEEDS_HUMAN` (`prisma/schema.prisma:124-138`). `generation` is the work identity (bumped on a fresh attempt); `fixAttempts` is bounded by `PR_FIX_MAX_ATTEMPTS` default 5 (`src/lib/pr-fix-queue.ts:549-552`). Hand-out timeout `PR_FIX_HANDOUT_TIMEOUT_MS` default 30 min; reclaim cap -`PR_FIX_MAX_RECLAIMS` default 3 (`pr-fix-queue.ts:579-597`). Past the cap the +`PR_FIX_MAX_RECLAIMS` default 3 (`src/lib/pr-fix-queue.ts:579-597`). Past the cap the item is **PARKED + `no_progress`** — bounded exhaustion, never a human decision -(`pr-fix-queue.ts:937-1010`). The `pr-fix-sweep` scheduler job is +(`src/lib/pr-fix-queue.ts:937-1010`). The `pr-fix-sweep` scheduler job is default-disabled, so reclaim may be absent in a default deployment, and the overdue signal can rise without an automated sweeper to drain it — see A7. @@ -390,21 +390,29 @@ Conditions are backend-neutral state predicates. `for` is the minimum duration the predicate must hold before firing (avoid flapping). Severity is a recommendation; the consumer maps it. -| # | Alert | Condition (predicate) | Severity | `for` | Why actionable | Recovery / operator action | -|---|---|---|---|---|---|---| -| A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | -| A2 | No successful sync | `dispatch_sync_last_success_timestamp_seconds` series **absent** (no `IssueSyncRun` has ever completed — the never-successful first sync must not be silent) **or** `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | critical | 5m | Supply is stale, or has never flowed; issues will not enter the queue. Consumers must evaluate the absent-series case as firing (e.g. `absent(...) == 1` in Prometheus), because `time() - > x` yields no data, not true | Check `IssueSyncRun` errors and GitHub reachability | -| A3 | Health/DB down | `dispatch_health_up == 0` | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | -| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the non-task system rows (groomer, heartbeat, legacy ingestion) per the §4.3 filter, so a busy groomer or a merely polling fleet cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | -| A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | -| A6 | Assigned without execution | **Future gap — must NOT fire on the aggregate.** Intended predicate: an individual **open, eligible** issue (not `done`/`blocked`) has carried `agent/*` with no active lease/`AgentWork` for longer than a threshold T. Per-item waiting age is not derivable from persisted columns today — label assignment carries no timestamp (`Issue` has no `assignedAt`; `updatedAt`/`lastSyncedAt` refresh on unrelated changes), and only actions-route assignments produce `assign_agent` audit rows (`src/app/api/issues/actions/route.ts:14`; claims audit as `claim_issue` but materialize a lease, so they fall outside this predicate), so a GitHub-side assignment has no age source at all. Firing `dispatch_assigned_without_execution > 0` sustained would misreport healthy churn as failure (§8.7), so this row stays pinned — an exporter that ships an age source promotes it to a real alert (§10) | info (pinned; not firing) | 30m (T, once age exists) | Assigned but never picked up; invisible in a queue-depth-only view. Until the age gap closes, execution-loss detection is owned by A5 (expired leases / STALE work) and pickup starvation by A4/A10 | Verify the worker polls its lane; check admission withholding | -| A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | -| A8 | PR-fix head-of-line | `dispatch_pr_fix_oldest_queued_seconds > threshold` | warning | 30m | Work is queued but not draining | Check worker concurrency, lane routing, and `fixAttempts` saturation | -| A9 | Human decisions piling up | `dispatch_pr_fix_human_decisions > 0` **and** oldest age > threshold | info/warning | 1h | A person must decide; PARKED is excluded so this is real | Review the BLOCKED item's decision context and act | -| A10 | Admission withholding | `dispatch_queue_withheld{reason="grooming_unknown"}` or `{reason="grooming_stale"}` sustained | warning | 1h | The admission gate is starving the queue on missing/stale grooming | Fix grooming pipeline; or operator admission override for a specific issue | -| A11 | Grooming backoff | `dispatch_grooming_backoff > 0` sustained | warning | 30m | Upstream GitHub reads are failing; issues cannot be groomed | Check GitHub reachability/rate limits | -| A12 | Audit failures | `dispatch_audit_failures_in_window{window="900s"} > 0` | warning | 5m | Mutations are failing (labels/claims). The predicate evaluates the as-of-scrape 15-minute gauge **directly**, so a new failure fires even when an older one expires from the window at the same moment — a `delta()` over a rolling windowed count could read zero there (§4.1) | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | -| A13 | Upstream evidence unavailable | `dispatch_repo_stale_sync_seconds > 2× sync interval` **or** `dispatch_grooming_backoff > 0` | warning | 15m | Admission/settlement decisions lack current evidence; the forge could not be read | Check GitHub reachability/rate limits; treat `unknown` as unknown, never healthy | +| # | Alert | Condition (predicate) | Enabled | Severity | `for` | Why actionable | Recovery / operator action | +| --- | --- | --- | --- | --- | --- | --- | --- | +| A1 | Scheduler job overdue | `dispatch_scheduler_job_overdue{job=...} == 1` | yes | critical | 10m | Every queue is fed by these jobs; a stopped timer starves the loop silently | Inspect logs; restart/verify `DISPATCH_SCHEDULER_ENABLED`; supervisor re-arms, but a persistent stop is a real fault | +| A2 | No successful sync | `dispatch_sync_last_success_timestamp_seconds` series **absent** (no `IssueSyncRun` has ever completed — the never-successful first sync must not be silent) **or** `time() - dispatch_sync_last_success_timestamp_seconds > 2× sync interval` | yes | critical | 5m | Supply is stale, or has never flowed; issues will not enter the queue. Consumers must evaluate the absent-series case as firing (e.g. `absent(...) == 1` in Prometheus), because `time() - > x` yields no data, not true | Check `IssueSyncRun` errors and GitHub reachability | +| A3 | Health/DB down | `dispatch_health_up == 0` | yes | critical | 2m | The service cannot serve work | Check DB connectivity; `/api/health` returns 503 | +| A4 | Eligible work not handed out | `dispatch_issues_open_claimable > 0` **and** `dispatch_execution_active == 0` **and** `dispatch_work_pickups_in_window{window="1800s"} == 0` **and** `dispatch_agent_runs_reported_in_window{window="1800s"} == 0` **and** `dispatch_scheduler_healthy == 1` **and** `(time() - dispatch_sync_last_success_timestamp_seconds) <= 2 × sync interval` | yes | warning | 30m | Work exists **and** nothing is executing **and** nothing has been picked up or reported in 30 minutes **and** every scheduled job is ticking **and** the last sync is fresh enough that A2 would be silent — an idle fleet while work waits, i.e. supply is not reaching workers. Three suppressors cover both delivery modes: a task in flight keeps `dispatch_execution_active` at 1 for its full duration (claim/agent-work workers — see §8.6); a fresh claim/start raises the pickup gauge; and any finished task's report lands in the reported gauge even for the canonical loop (next-task → execute → report), which writes no pickup row. Both gauges exclude the non-task system rows (groomer, heartbeat, legacy ingestion) per the §4.3 filter, so a busy groomer or a merely polling fleet cannot mask an idle fleet — nor disarm A4 the other way. Residual blind spot (documented, not solvable from the DB): a worker that materializes no claim/agent-work and runs a *single* task longer than the window without reporting is invisible while mid-task, and A4 fires; that firing is a truthful "Dispatch cannot observe this fleet" warning, and materializing a claim or agent-work start removes the ambiguity. A *hung* claimed task is caught by A5 and the AgentWork heartbeat-age signal instead. The fresh-sync guard is the explicit spelling of A2-suppresses-A4: a scheduler that ticks on time while sync is failing (or has never completed, so the sync series is absent and the guard cannot pass) no longer satisfies A4, and is routed to A2/A13 instead | Check worker availability/credentials; confirm `next-task` returns `implement`; confirm workers materialize a claim or agent-work start | +| A5 | Stale ownership | `dispatch_leases_expired > 0` or `dispatch_agent_work{state="STALE"} > 0` | yes | warning | 15m | Ownership is lost; the issue may be stuck in-progress | Stale sweep releases it; if it persists, inspect the sweep job (A1) | +| A6 | Assigned without execution | **Future gap — must NOT fire on the aggregate.** Intended predicate: an individual **open, eligible** issue (not `done`/`blocked`) has carried `agent/*` with no active lease/`AgentWork` for longer than a threshold T. Per-item waiting age is not derivable from persisted columns today — label assignment carries no timestamp (`Issue` has no `assignedAt`; `updatedAt`/`lastSyncedAt` refresh on unrelated changes), and only actions-route assignments produce `assign_agent` audit rows (`src/app/api/issues/actions/route.ts:14`; claims audit as `claim_issue` but materialize a lease, so they fall outside this predicate), so a GitHub-side assignment has no age source at all. Firing `dispatch_assigned_without_execution > 0` sustained would misreport healthy churn as failure (§8.7), so this row stays pinned — an exporter that ships an age source promotes it to a real alert (§10) | no — pinned future gap | info (pinned; not firing) | — (T applies only when promoted) | Assigned but never picked up; invisible in a queue-depth-only view. Until the age gap closes, execution-loss detection is owned by A5 (expired leases / STALE work) and pickup starvation by A4/A10 | Verify the worker polls its lane; check admission withholding | +| A7 | Unacknowledged hand-outs (overdue) | `dispatch_pr_fix_overdue_unacked > 0` sustained | yes | warning | 35m | A worker may have died before materializing its run; item is stranded past the configured hand-out timeout. The predicate is age-tied to `dispatchedAt` (per-row `now − dispatchedAt > PR_FIX_HANDOUT_TIMEOUT_MS`), not the aggregate unacked count, so steady churn does not fire | Reclaim sweep (`pr-fix-sweep`, default-disabled) or operator requeue | +| A8 | PR-fix head-of-line | `dispatch_pr_fix_oldest_queued_seconds > threshold` | yes | warning | 30m | Work is queued but not draining | Check worker concurrency, lane routing, and `fixAttempts` saturation | +| A9 | Human decisions piling up | `dispatch_pr_fix_human_decisions > 0` **and** oldest age > threshold | yes | info/warning | 1h | A person must decide; PARKED is excluded so this is real | Review the BLOCKED item's decision context and act | +| A10 | Admission withholding | `dispatch_queue_withheld{reason="grooming_unknown"}` or `{reason="grooming_stale"}` sustained | yes | warning | 1h | The admission gate is starving the queue on missing/stale grooming | Fix grooming pipeline; or operator admission override for a specific issue | +| A11 | Grooming backoff | `dispatch_grooming_backoff > 0` sustained | yes | warning | 30m | Upstream GitHub reads are failing; issues cannot be groomed | Check GitHub reachability/rate limits | +| A12 | Audit failures | `dispatch_audit_failures_in_window{window="900s"} > 0` | yes | warning | 5m | Mutations are failing (labels/claims). The predicate evaluates the as-of-scrape 15-minute gauge **directly**, so a new failure fires even when an older one expires from the window at the same moment — a `delta()` over a rolling windowed count could read zero there (§4.1) | Inspect `AuditLog.errorMessage` for the action; **do not** label by error string | +| A13 | Upstream evidence unavailable | `dispatch_repo_stale_sync_seconds > 2× sync interval` **or** `dispatch_grooming_backoff > 0` | yes | warning | 15m | Admission/settlement decisions lack current evidence; the forge could not be read | Check GitHub reachability/rate limits; treat `unknown` as unknown, never healthy | + +The `Enabled` column is the wiring contract: `yes` means a consumer **may** +evaluate the predicate and map it to a rule; `no` means the row documents a known +gap and **must not** be wired into an evaluator. A6 is the only disabled row — +its intended predicate (per-issue assignment age T) and the missing age source +are spelled out in the row itself, and it graduates to `yes` via +[§10](#10-open-questions-and-deferred-decisions) once a per-item assignment age +exists. ### Normal inactivity must NOT alert @@ -682,3 +690,28 @@ design, not a decision. whether the exporter queries the primary or a replica. - **OTel semantic conventions:** whether to map to standard metric/log conventions or keep Dispatch-prefixed names. + +--- + +## 11. Design self-verification + +Because this document is design-only, its invariants are enforced by an +executable check at `docs/observability.test.ts`, run as part of `npm run test` +— not by production code, which has no exporter yet. + +- **Alert structure:** alert IDs `A1`–`A13` appear exactly once, every row has + the required columns, and the required cells are non-empty. +- **Enablement contract:** disabled rows carry no `for` duration and a severity + of `info`, so a `no` row can never look like a live rule. +- **Rate functions:** no alert condition evaluates a signal with `increase()`, + `rate()`, or `delta()` (§4.1). +- **Signal closure:** every `dispatch_*` signal named in an alert condition is + defined in the [§4](#4-signal-and-state-catalogue) catalogue. +- **Citation drift:** every `file:line` citation in this document resolves to an + existing file and an existing line, so a refactor that moves code fails here. +- **Source drift:** the `IssueSyncRun.status` values the predicates match are + exactly the values the source writes. + +This verifies **structure and source drift only**. It does not exercise alert +runtime behavior — no exporter exists yet, so there is nothing to evaluate +against ([§10](#10-open-questions-and-deferred-decisions)). diff --git a/docs/observability.test.ts b/docs/observability.test.ts new file mode 100644 index 00000000..8bc754a7 --- /dev/null +++ b/docs/observability.test.ts @@ -0,0 +1,208 @@ +import { describe, it, expect } from "vitest"; +import { existsSync, readFileSync } from "node:fs"; +import path from "node:path"; + +const DOC_REL = "docs/observability.md"; + +// `fileURLToPath(new URL(..., import.meta.url))` is unusable here: the jsdom +// environment's `URL` yields an object node:url rejects. `import.meta.dirname` +// is a plain string in the vitest transform, and vitest always runs from the +// repo root, so walk up as a safety net rather than trusting either blindly. +function repoRoot(): string { + let dir = path.resolve(import.meta.dirname, ".."); + while (!existsSync(path.join(dir, DOC_REL)) && dir !== path.dirname(dir)) { + dir = path.dirname(dir); + } + return dir; +} + +const REPO_ROOT = repoRoot(); +const doc = readFileSync(path.join(REPO_ROOT, DOC_REL), "utf8"); +const docLines = doc.split("\n"); + +const readRepo = (relative: string): string => readFileSync(path.join(REPO_ROOT, relative), "utf8"); + +/** Cells of every `|`-row between `heading` and the next heading. */ +function rowsAfterHeading(heading: string): string[][] { + const start = docLines.findIndex((l) => l.startsWith(heading)); + expect(start, `${DOC_REL} is missing the "${heading}" heading`).toBeGreaterThan(-1); + const rows: string[][] = []; + for (let i = start + 1; i < docLines.length; i += 1) { + const line = docLines[i]; + if (line.startsWith("#")) break; + if (!line.startsWith("|")) continue; + rows.push( + line + .split("|") + .slice(1, -1) + .map((c) => c.trim()), + ); + } + return rows; +} + +function alertTable(): string[][] { + return rowsAfterHeading("## 6. Alert catalogue"); +} + +const CATALOGUE = alertTable(); +const HEADER = CATALOGUE[0]; +const ALERTS = CATALOGUE.filter((cells) => /^A\d+$/.test(cells[0])); +const col = (name: string): number => HEADER.indexOf(name); + +const CONDITION = col("Condition (predicate)"); +const ENABLED = col("Enabled"); +const SEVERITY = col("Severity"); +const FOR = col("`for`"); +const WHY = col("Why actionable"); +const RECOVERY = col("Recovery / operator action"); + +/** Required, non-empty cells paired with a readable label for failure output. */ +const REQUIRED_CELLS: [number, string][] = [ + [col("Alert"), "Alert"], + [CONDITION, "Condition"], + [ENABLED, "Enabled"], + [SEVERITY, "Severity"], + [FOR, "for"], + [WHY, "Why actionable"], + [RECOVERY, "Recovery"], +]; + +/** Signal names defined by the §4 catalogue: first cell is `` `dispatch_x` ``. */ +const DEFINED_SIGNALS = new Set( + docLines + .filter((l) => l.startsWith("|")) + .map((l) => { + const first = l.split("|")[1]?.trim() ?? ""; + return /^`(dispatch_[a-z0-9_]+)`$/.exec(first)?.[1]; + }) + .filter((n): n is string => Boolean(n)), +); + +describe("observability design doc: §6 alert catalogue structure", () => { + it("defines A1–A13 exactly once, each row complete", () => { + const ids = ALERTS.map((cells) => cells[0]); + const expected = Array.from({ length: 13 }, (_, i) => `A${i + 1}`); + expect(ids, `§6 alert IDs drifted; expected exactly ${expected.join(", ")}`).toEqual(expected); + const missing = REQUIRED_CELLS.filter(([idx]) => idx === -1).map(([, label]) => label); + expect(missing, `§6 table is missing the column(s): ${missing.join(", ")}`).toEqual([]); + + const problems: string[] = []; + for (const cells of ALERTS) { + const id = cells[0]; + if (cells.length !== HEADER.length) { + problems.push(`${id}: ${cells.length} cells, header has ${HEADER.length}`); + } + for (const [idx, label] of REQUIRED_CELLS) { + if (!cells[idx]) problems.push(`${id}: empty "${label}" cell`); + } + } + expect(problems, `Fix the malformed rows in §6 of ${DOC_REL}:\n ${problems.join("\n ")}`).toEqual([]); + }); + + it("marks exactly A6 disabled, and disabled rows carry no duration and severity info", () => { + const problems: string[] = []; + const disabled: string[] = []; + for (const cells of ALERTS) { + const id = cells[0]; + const enabled = cells[ENABLED]; + if (/^no\b/.test(enabled)) disabled.push(id); + else if (enabled !== "yes") problems.push(`${id}: Enabled cell is "${enabled}", expected "yes" or "no …"`); + + if (!/^no\b/.test(enabled)) continue; + if (/\d+\s*(m|s|h|min)/.test(cells[FOR])) { + problems.push(`${id}: disabled but its \`for\` cell is a duration ("${cells[FOR]}")`); + } + if (!cells[SEVERITY].includes("info")) { + problems.push(`${id}: disabled but severity is "${cells[SEVERITY]}", expected info`); + } + if (!cells[CONDITION].includes("must NOT fire")) { + problems.push(`${id}: disabled but its condition does not say "must NOT fire"`); + } + } + expect(disabled, `§6 must pin exactly A6 as disabled; got: ${disabled.join(", ") || "none"}`).toEqual([ + "A6", + ]); + expect(problems, `Fix §6 in ${DOC_REL}:\n ${problems.join("\n ")}`).toEqual([]); + }); +}); + +describe("observability design doc: §4.1/§4 signal invariants", () => { + it("uses no rate-style function in any alert condition (§4.1)", () => { + const offenders = ALERTS.filter((cells) => /\b(increase|rate|delta)\s*\(/.test(cells[CONDITION])).map( + (cells) => cells[0], + ); + expect( + offenders, + `${DOC_REL} §4.1 forbids increase()/rate()/delta(); offending alert conditions: ${offenders.join(", ")}`, + ).toEqual([]); + }); + + it("only names signals defined in the §4 catalogue", () => { + expect( + DEFINED_SIGNALS.size, + `${DOC_REL} §4 no longer defines any \`dispatch_*\` signals — this check would pass vacuously`, + ).toBeGreaterThan(0); + const unknown = new Map(); + for (const cells of ALERTS) { + for (const signal of cells[CONDITION].match(/dispatch_[a-z0-9_]+/g) ?? []) { + if (DEFINED_SIGNALS.has(signal)) continue; + unknown.set(signal, [...(unknown.get(signal) ?? []), cells[0]]); + } + } + const detail = [...unknown].map(([s, ids]) => `${s} (used by ${ids.join(", ")})`).join("; "); + expect(unknown.size, `${DOC_REL}: alert conditions reference undefined signals: ${detail}`).toBe(0); + }); +}); + +describe("observability design doc: source drift", () => { + it("resolves every `file:line` citation", () => { + // Paths with a `/` only: the bare-filename form is a drift bug of its own. + const CITATION = /`([A-Za-z0-9_./[\]-]+\.(?:ts|tsx|prisma|yaml|yml|json|md)):(\d+)(?:-(\d+))?`/g; + const matches = [...doc.matchAll(CITATION)]; + expect(matches.length, `${DOC_REL} has no backticked file:line citations to check`).toBeGreaterThan(0); + const problems: string[] = []; + for (const match of matches) { + const [citation, file, start, end] = match; + const abs = path.join(REPO_ROOT, file); + const lastLine = Number(end ?? start); + if (!file.includes("/")) { + problems.push(`${citation} is a bare filename — cite the full repo-relative path`); + continue; + } + if (!existsSync(abs)) { + problems.push(`${citation} — file does not exist`); + continue; + } + const lineCount = readFileSync(abs, "utf8").split("\n").length; + if (lineCount < lastLine) problems.push(`${citation} — file has only ${lineCount} lines`); + } + expect(problems, `Fix these citations in ${DOC_REL}:\n ${problems.join("\n ")}`).toEqual([]); + }); + + it("keeps the §4.9 `IssueSyncRun.status` values equal to the values source writes", () => { + const OPEN = "whose only persisted values are"; + const CLOSE = "There is **no**"; + const from = doc.indexOf(OPEN); + const to = doc.indexOf(CLOSE, from); + expect(from, `${DOC_REL} §4.9 no longer has the IssueSyncRun.status sentence`).toBeGreaterThan(-1); + expect(to).toBeGreaterThan(from); + const documented = new Set( + [...doc.slice(from + OPEN.length, to).matchAll(/`([a-z_]+)`/g)].map((m) => m[1]), + ); + expect( + documented.size, + `${DOC_REL} §4.9 names no \`IssueSyncRun.status\` values — this check would pass vacuously`, + ).toBeGreaterThan(0); + + // These three are the only IssueSyncRun status writers (grep-verified); + // src/app/api/automation/sync/route.ts delegates to the shared lock path. + const writers = ["src/lib/sync-lock.ts", "src/app/api/sync/route.ts", "src/app/api/sync/scheduled/route.ts"]; + const written = new Set( + writers.flatMap((file) => [...readRepo(file).matchAll(/status:\s*["']([a-z]+)["']/g)].map((m) => m[1])), + ); + + const detail = `documented: ${[...documented].sort().join(", ") || "none"}; written: ${[...written].sort().join(", ") || "none"}`; + expect([...documented].sort(), `${DOC_REL} §4.9 vs source — ${detail}`).toEqual([...written].sort()); + }); +}); \ No newline at end of file