Skip to content

chore(observability): audit-trail coverage for notifications, gate-verdict publish, and dead exports #2908

Description

@JSONbored

Summary

A cluster of related observability/audit-trail gaps found during a 2026-07-04 audit: notification send/suppress/fail events have no durable audit trail, two unrelated systems both use the word "audit" which invites confusion, the successful publish of a gate verdict isn't itself audited, and a handful of observability exports have zero production callers.

Part of #1667.

Context

Notification audit-trail gapnotifyActionToDiscord/notifyActionToSlack (src/services/notify-discord.ts:51-119) have no audit trail at all. On suppression (no webhook configured, or the URL fails validation) both silently return. On delivery failure, each only does console.warn(...) to stdout — never persisted anywhere durable. The only call site (src/services/agent-action-executor.ts:337-338) wraps both in .catch(() => undefined), so even a thrown error is fully swallowed. Compare to src/notifications/service.ts's "badge" channel, which already writes a durable notification_deliveries row with status pending/suppressed/delivered — a working pattern to follow. (Note: src/notifications/service.ts:118-121's recipient-opted-out suppression path is a separate, likely-intentional case — do NOT add a durable record there without confirming that a privacy-motivated opt-out shouldn't itself be silently unrecorded; that's a judgment call, not obviously a bug.)

Two unrelated "audit" systemssrc/selfhost/audit.ts (logAudit/AuditEventType) is a stdout-only JSON logger for exactly 4 queue-lifecycle events (job_complete/job_dead/job_error/job_rate_limited), called only from sqlite-queue.ts/pg-queue.ts. The actual durable audit_events DB table (src/db/schema.ts:1114-1132) is written exclusively via recordAuditEvent (src/db/repositories.ts:2335), called from many services. These are two entirely separate systems that happen to share a name — worth a doc-comment cross-reference in selfhost/audit.ts so nobody (including a future audit) conflates them again. Separately, there's a real internal inconsistency within selfhost/audit.ts's own scope: the pre-admission GitHub rate-limit deferral path (sqlite-queue.ts:710-747, pg-queue.ts ~979-1020) does NOT call logAudit, while the sibling post-attempt rate-limited path (sqlite-queue.ts:868, pg-queue.ts ~1184) does — both are "rate-limit deferral" in spirit but only one is captured in that log stream.

Gate-verdict publish isn't audited — the moment a gate check-run's verdict actually gets published to GitHub (src/queue/processors.ts:7671-7726, createOrUpdateGateCheckRun/recordPublishedGateCheckSummary) does not call recordAuditEvent on success; it only upserts check_summaries (no actor/outcome/detail vocabulary). Only the failure/degraded sub-paths of the same function are audited today (auditGateCheckPermissionMissing, auditPrVisibilitySkip). This means audit_events cannot directly answer "when was PR X's gate verdict published and what did it say" — an operator has to correlate check_summaries manually.

Dead observability exports — zero production call sites found for: flushOpenTelemetry (src/selfhost/otel.ts:386-390; shutdownOpenTelemetry is a different function that IS wired into src/server.ts's shutdown handlers and does not call this one), and registerMetricMeta/DEFAULT_BUCKETS/resetMetrics (src/selfhost/metrics.ts:168-171,138,255-257). registerMetricMeta being unused is worth checking directly: it means every production metric name may be going out via renderMetrics (wired live at src/server.ts:721) with no registered HELP/TYPE line, which would be a Prometheus-scraping quality issue distinct from plain dead-code cleanup.

Requirements

  • Add audit coverage for Discord/Slack notification send/suppress/fail, following the notification_deliveries pattern already used for the badge channel. Keep it best-effort (an audit-write failure must never break the notification attempt) and do not let a webhook URL, token, or secret leak into audit metadata.
  • Add a doc-comment cross-reference between src/selfhost/audit.ts and src/db/repositories.ts's recordAuditEvent so the naming collision is self-explanatory to future readers. Decide whether to also add a logAudit call for the pre-admission rate-limit-deferral path for consistency with its sibling.
  • Decide whether gate-verdict publish deserves its own audit_events row (a high-frequency event — weigh the volume/value tradeoff explicitly in the PR description) or whether check_summaries should be documented as the canonical record for that specific event type instead. Either is an acceptable outcome as long as it's a deliberate, documented decision rather than an oversight.
  • For each dead observability export: either wire it up (e.g. have shutdownOpenTelemetry call flushOpenTelemetry if that turns out to be a real bug, or have metric registration actually call registerMetricMeta) or remove it. Check /metrics output directly (a real self-hosted instance or a local test) to confirm whether HELP/TYPE lines are actually missing before deciding.

Deliverables

  • One or more focused PRs (notification-audit-coverage and gate-verdict-audit-decision can likely be one PR; the dead-export cleanup can be a separate smaller PR) with regression tests for each new/changed audit path.
  • A clear written rationale in each PR description for any "document as intentional, don't change" decision (e.g. the opted-out-recipient suppression case).

Expected outcome

An operator can query audit_events (or an explicitly-documented alternate table) to answer "was this contributor-affecting notification/decision made, and what happened" for every one of: review published, PR merged/closed, manual-review hold, notification sent/denied/failed, config-driven suppression, rate-limit deferral, guardrail hold — matching the coverage bar the original audit was asked to check. No dead observability export is left silently unreachable without a documented reason.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions