From d52575b5251cb1ac1ce52e0eb4417dd72f6fc4b0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:55:40 -0700 Subject: [PATCH] docs(observability): disambiguate audit.ts from recordAuditEvent, document gate-check-publish audit decision Two doc-only clarifications from a self-host observability audit: - src/selfhost/audit.ts's logAudit is a stdout-only logger for 4 queue- lifecycle events, not the durable audit_events DB writer -- a naming trap a future reader (including this repo's own prior audit) can easily fall into. Cross-references recordAuditEvent in db/repositories.ts. - The successful gate-check-run publish path writes to check_summaries but not audit_events. Documents this as an intentional design decision (check_summaries is the purpose-built canonical record for this high-frequency event; downstream merge/close/hold actions are already audited separately) rather than a gap needing a fix. No behavior change. The two other items originally flagged in this area are already resolved elsewhere: notification send/suppress/fail audit coverage was added by #2881 (notify-discord.ts's auditExternalNotification), and the observability exports flagged as possibly-dead (registerMetricMeta, DEFAULT_BUCKETS, resetMetrics, flushOpenTelemetry) turned out to all be legitimately in use (extensively by tests, or as internal defaults) or a standalone utility with a distinct purpose from shutdown -- none warrant removal. --- src/queue/processors.ts | 8 ++++++++ src/selfhost/audit.ts | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index cb9a429992..a7c38c28ba 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -5238,6 +5238,14 @@ type PublicSurfaceOutputFailure = { error: string; }; +// Intentionally writes to check_summaries only, not audit_events (#2908): this fires on every successful gate- +// check publish, which is a very high-frequency event (every review pass, potentially several times per PR as +// it iterates) -- check_summaries is the purpose-built, already-queryable canonical record for "when was this +// check published and what did it conclude" (repo/PR/headSha/checkRunId/conclusion/detailsUrl), so a parallel +// audit_events row would roughly double that table's volume for no new queryable information. The DOWNSTREAM +// actions this verdict triggers (merge/close/hold) are already fully audited via recordNativeGateDecision and +// agent-action-executor's audit() closure. Only the FAILURE/degraded sub-paths of the caller below are audited +// (auditGateCheckPermissionMissing, auditPrVisibilitySkip) -- that asymmetry is deliberate, not a gap. async function recordPublishedGateCheckSummary( env: Env, args: { diff --git a/src/selfhost/audit.ts b/src/selfhost/audit.ts index f2bb1b2833..a369292ccf 100644 --- a/src/selfhost/audit.ts +++ b/src/selfhost/audit.ts @@ -2,6 +2,11 @@ // operators can grep / pipe to their log aggregator (Loki, CloudWatch, Datadog, etc.) without any extra // setup. Written to process.stdout so it is captured by Docker's default json-file log driver and is // accessible via `docker compose logs gittensory`. +// +// NOT the durable audit_events DB table (#2908): this module is a stdout-only logger for exactly the 4 queue- +// lifecycle events below, called only from sqlite-queue.ts/pg-queue.ts. For the actual queryable audit trail of +// contributor-affecting decisions (review published, PR merged/closed, notification sent/failed, guardrail +// hold, ...), see recordAuditEvent in ../db/repositories.ts. import { otelTraceLogFields } from "./otel";