fix(orb): stop four silent alarm gaps in relay-drain, crash handling, DLQ, and reversal signals - #9179
Merged
JSONbored merged 1 commit intoJul 27, 2026
Conversation
… DLQ, and reversal signals Four related observability blind spots where the alarm was off exactly when it mattered: - Relay-drain staleness (#9128): a drain that only ever throws stamped no failure signal at all, and "never drained" read a flat -1 that could never cross the firing threshold. Counts drain failures (loopover_orb_relay_drain_consecutive_failures), ages "never drained" from process boot instead of -1 in both the seconds-since-last gauge and isOrbRelayRegistrationAlerting, and applies the same boot-aging fix to the clock-skew sibling gauge. Adds LoopoverOrbRelayDrainFailing to catch a flapping drain the existing 30m no-progress alert can't see. - PostHog vs. unhandled rejections (#9133): enableExceptionAutocapture captured a rejection but never rethrew or exited, silently downgrading a fatal crash-and-restart into a dead worker loop inside a still-"healthy" process. installSelfHostCrashHandlers (new) is now the sole, unconditional source of truth for uncaughtException/unhandledRejection, installed first thing in main(); enableExceptionAutocapture is now off so posthog-node never competes with it. - Queue/metrics blind spots during a DB incident (#9139): a throwing gauge sampler vanished entirely (no series, no counter); every queue-backlog alert silently went inactive during exactly the DB outage they exist to catch. Counts sampler failures (loopover_metrics_sampler_errors_total) and emits a -1 sentinel so absence is visible. Adds a generic up==0 rule for the always-co-started observability-bundle exporters, and an absent() rule for a dead backup exporter. Backs loopover_dlq_dead_lettered_recent with the self-host queue's own recentDeadCount (the cloud-worker audit_events source is unreachable on self-host). Writes a review_audit dead_lettered row from the self-host dead-letter path. Fixes alertmanager.yml's LoopOverTargetDown/LoopoverTargetDown capitalization mismatch and extends the metric-name-reference drift test to cover alertmanager.yml's alertname references too. - Orphaned review_targets (#9136): repoints the anomaly-alerter's reversal and DLQ signals off the dead review_targets table onto the live review_audit ledger directly (also fixing a target_id namespace mismatch that meant the old join could never match even while review_targets was live). Adds checkReviewSourceFreshness, a generalizable staleness check + alert so the next table a downstream module silently stops writing is loud, not silent. byStatus/manualRate/ stuckRetryable/failed, computeCalibration, submitter-reputation.ts, and ams-miner-cohort.ts remain review_targets-sourced -- deferred, see the PR description for the full scope. Closes #9128 Closes #9133 Closes #9139 Advances #9136
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 243d43a | Commit Preview URL Branch Preview URL |
Jul 27 2026, 05:54 AM |
Bundle ReportChanges will increase total bundle size by 1 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9179 +/- ##
==========================================
- Coverage 93.90% 92.68% -1.22%
==========================================
Files 817 818 +1
Lines 81039 81099 +60
Branches 24611 24625 +14
==========================================
- Hits 76096 75167 -929
- Misses 3568 4848 +1280
+ Partials 1375 1084 -291
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This was referenced Jul 27, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 27, 2026
…w_targets (#9136) getSubmitterReputation, getSubmitterReputationAcrossInstall, and listSubmitterCohortRows read `review_targets` over a 90-day recency window -- but review_targets has had no live writer since the 2026-06-22 convergence cutover, so that window reads a shrinking set that goes permanently empty around 2026-09-20 (following #9015's partial fix and #9179's repoint of the anomaly-alerter's reversal/DLQ signals). Repointed onto the same live ledgers #9179 and getSubmitterCadence (#9015) already read: review_audit's pr_outcome rows (the realized outcome, not just the bot's own prediction) joined to pull_requests for the submitter's login, restricted to each target's LATEST pr_outcome row so a redelivered webhook can't double-count. ams-miner-cohort.ts required no changes -- it consumes listSubmitterCohortRows directly and picks the repoint up for free; its stale doc comments are updated to match. Before restoring the signal, give evaluateVisualVisionGate's and evaluateScreenshotTableVisionGate's `low_reputation` skip a compensating advisory finding: silently skipping the one extra AI check a low-reputation submitter's confirmed visual change would otherwise get reproduces #9015's "suspicion buys less scrutiny" shape. Remaining scope, tracked under #9136: ops.ts's computeAgentHealth (byStatus/byVerdict/failedRows/manualRate/stuckRetryable/failed) and computeCalibration still read review_targets directly. Deferred because review_targets' non-terminal states (queued/reviewing/error/error_retryable) and the attempt-exhausted 'failed' bucket have no live per-target equivalent post-cutover -- gate_decision's own `decision` column only ever records 'merge' | 'close' | 'hold', not the full status enum review_targets tracked. See the PR description for the full reader inventory.
JSONbored
added a commit
that referenced
this pull request
Jul 27, 2026
…w_targets (#9136) getSubmitterReputation, getSubmitterReputationAcrossInstall, and listSubmitterCohortRows read `review_targets` over a 90-day recency window -- but review_targets has had no live writer since the 2026-06-22 convergence cutover, so that window reads a shrinking set that goes permanently empty around 2026-09-20 (following #9015's partial fix and #9179's repoint of the anomaly-alerter's reversal/DLQ signals). Repointed onto the same live ledgers #9179 and getSubmitterCadence (#9015) already read: review_audit's pr_outcome rows (the realized outcome, not just the bot's own prediction) joined to pull_requests for the submitter's login, restricted to each target's LATEST pr_outcome row so a redelivered webhook can't double-count. ams-miner-cohort.ts required no changes -- it consumes listSubmitterCohortRows directly and picks the repoint up for free; its stale doc comments are updated to match. Before restoring the signal, give evaluateVisualVisionGate's and evaluateScreenshotTableVisionGate's `low_reputation` skip a compensating advisory finding: silently skipping the one extra AI check a low-reputation submitter's confirmed visual change would otherwise get reproduces #9015's "suspicion buys less scrutiny" shape. Remaining scope, tracked under #9136: ops.ts's computeAgentHealth (byStatus/byVerdict/failedRows/manualRate/stuckRetryable/failed) and computeCalibration still read review_targets directly. Deferred because review_targets' non-terminal states (queued/reviewing/error/error_retryable) and the attempt-exhausted 'failed' bucket have no live per-target equivalent post-cutover -- gate_decision's own `decision` column only ever records 'merge' | 'close' | 'hold', not the full status enum review_targets tracked. See the PR description for the full reader inventory.
JSONbored
added a commit
that referenced
this pull request
Jul 27, 2026
…le clock skew in broker mode, and bound the upstream-drift fallback fetch (#9230) * fix(upstream): bound the raw-github fallback fetch with a timeout (#9165) fetchTrackedSource's raw-GitHub fallback was the only fetch in ruleset.ts without a timeout, so a stalled raw.githubusercontent.com connection hung the whole scheduled refreshUpstreamDrift job. Route it through the same timeoutFetch every other call site in the file already uses, and confirm the Promise.all fan-out degrades per-source rather than all-or-nothing. Sweep the rest of src/ for the same bare-fetch shape: three GitHub calls in github-oauth.ts (device-flow start/poll, web-OAuth code exchange) and one call each in linear-adapter.ts and registry/sync.ts had no timeout and are now bounded the same way. * fix(selfhost): sample clock skew in broker mode and alert on staleness (#9156) recordClockSkewFromResponse's only call site is inside the local GitHub App JWT mint path, which a brokered self-host never reaches (mintInstallationToken returns from the broker branch first) -- so a brokered deployment's clock skew gauge read a hard 0, indistinguishable from "clock is fine". Sample it from fetchBrokeredInstallationToken's own response instead, on both success and failure. 0 is a real, reachable skew value (a perfectly synced clock), so it can't double as "never sampled" -- clockSkewSecondsSample() now returns NaN until the first real sample lands. The companion loopover_clock_skew_sample_age_seconds gauge (added in #7000 precisely so a stale reading is distinguishable from a fresh one) was wired to zero alert rules and zero dashboard panels. Add LoopoverClockSkewSampleStale (fires past 2h, tolerating normal ~hourly mint-timing jitter) and the matching Grafana stat panel. * fix(review): repoint the reputation quality signal off orphaned review_targets (#9136) getSubmitterReputation, getSubmitterReputationAcrossInstall, and listSubmitterCohortRows read `review_targets` over a 90-day recency window -- but review_targets has had no live writer since the 2026-06-22 convergence cutover, so that window reads a shrinking set that goes permanently empty around 2026-09-20 (following #9015's partial fix and #9179's repoint of the anomaly-alerter's reversal/DLQ signals). Repointed onto the same live ledgers #9179 and getSubmitterCadence (#9015) already read: review_audit's pr_outcome rows (the realized outcome, not just the bot's own prediction) joined to pull_requests for the submitter's login, restricted to each target's LATEST pr_outcome row so a redelivered webhook can't double-count. ams-miner-cohort.ts required no changes -- it consumes listSubmitterCohortRows directly and picks the repoint up for free; its stale doc comments are updated to match. Before restoring the signal, give evaluateVisualVisionGate's and evaluateScreenshotTableVisionGate's `low_reputation` skip a compensating advisory finding: silently skipping the one extra AI check a low-reputation submitter's confirmed visual change would otherwise get reproduces #9015's "suspicion buys less scrutiny" shape. Remaining scope, tracked under #9136: ops.ts's computeAgentHealth (byStatus/byVerdict/failedRows/manualRate/stuckRetryable/failed) and computeCalibration still read review_targets directly. Deferred because review_targets' non-terminal states (queued/reviewing/error/error_retryable) and the attempt-exhausted 'failed' bucket have no live per-target equivalent post-cutover -- gate_decision's own `decision` column only ever records 'merge' | 'close' | 'hold', not the full status enum review_targets tracked. See the PR description for the full reader inventory. * chore(cf): regenerate worker-configuration.d.ts after rebasing onto main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related ORB observability bugs where the alarm is off exactly when it matters. All four share the same shape: a sentinel/silent-failure/orphaned-data pattern that makes a real incident look like "no data" instead of "firing."
orb(observability): the relay-drain staleness alarm is suppressed by its own worst failure — 'never drained' reads -1, and a deploy silences a firing alert #9128 — relay-drain staleness suppressed by its own worst failure. A drain that only ever throws stamped no failure signal anywhere, and "never drained" read a flat
-1that could never cross the Prometheus firing threshold. Fixed: drain failures are now counted (loopover_orb_relay_drain_consecutive_failures,loopover_orb_relay_drains_total{result="failed"}); "never drained" now ages from process boot instead of a flat-1, in both theloopover_orb_relay_drain_seconds_since_lastgauge andisOrbRelayRegistrationAlerting; the siblingloopover_clock_skew_sample_age_secondsgauge got the same boot-aging fix (no alert reads it today, but the same hole would silently defeat a future one). AddedLoopoverOrbRelayDrainFailingto catch a flapping drain the existing 30m no-progress alert can't see.orb(selfhost): enabling PostHog silently disables crash-on-unhandled-rejection — a dead worker loop now survives inside a 'healthy' process #9133 — enabling PostHog silently disabled crash-on-unhandled-rejection.
enableExceptionAutocapturecaptured a rejection but never rethrew or exited, silently downgrading a fatal crash-and-restart into a dead worker loop inside a still-"200 OK" process. Fixed:installSelfHostCrashHandlers(new,src/selfhost/process-lifecycle.ts) is now the sole, unconditional source of truth for bothuncaughtExceptionandunhandledRejection, installed first thing inmain(), independent of whether telemetry is configured.enableExceptionAutocaptureis nowfalseso posthog-node never installs a competing listener (avoiding both a double-captured exception and its foreign-listener-count heuristic silently changing its own exit behavior). The stale comment asserting posthog-node's autocapture already covered this is corrected. Deferred (noted, not implemented): asserting worker-loop liveness in/health— flagged as a real followup, out of scope for this batch.orb(observability): the queue-backlog alerts deactivate during the DB incident they exist to catch, and both DLQ-rate surfaces are structurally always zero #9139 — queue-backlog alerts deactivate during the exact DB incident they exist to catch; DLQ-rate always zero. A throwing gauge sampler in
renderMetricsvanished entirely (no series, no counter), so every queue-backlog alert silently went INACTIVE during a Postgres outage — precisely the moment they exist to fire. Fixed: sampler failures are now counted (loopover_metrics_sampler_errors_total{metric}) and a-1sentinel is emitted so the gauge's absence is itself visible; newLoopoverMetricsSamplerFailingalert. AddedLoopoverObservabilityExporterDown, a genericup==0rule scoped to the exporters that are always co-started with Prometheus/Alertmanager themselves (node-exporter/cadvisor/redis/observability-stack) — deliberately NOT a blanket rule over every job, since postgres/qdrant/gpu/rees/browserless/backup are each separately profile-gated and would misfire permanently on an install that never enabled that add-on. AddedLoopoverBackupExporterMissing(absent(loopover_backup_files)) alongside the existingLoopoverBackupMissing, so a dead backup exporter (vs. "backup profile never enabled") is distinguishable — this one is a deliberate always-nudge, documented in the rule's own comment, mirroring the existingsqliteBackupAdvisoryboot warning. Fixedalertmanager.yml'sLoopOverTargetDown/LoopoverTargetDowncapitalization mismatch (the commented inhibit-rule example silently never matched the real rule) and extendedtest/unit/alerts-metric-name-references.test.tsto assert everyalertname="…"reference inalertmanager.yml, including commented examples, resolves to a real rule.loopover_dlq_dead_lettered_recentis now backed by the self-host queue's ownrecentDeadCount(newDurableQueuemethod, implemented in bothpg-queue.tsandsqlite-queue.ts) — the cloud-workeraudit_eventssource it read before is structurally unreachable on self-host (server.tsnever callsworker.queue). The self-host dead-letter path now writes areview_auditrow withevent_type='dead_lettered'(scoped to jobs with an identifiable repo+PR) — this also directly feeds orb(observability): the orphaned review_targets table silently disables the anomaly alerter, the reputation quality signal, and the AMS bridge — and the reputation half expires to empty around 2026-09-20 #9136's DLQ signal below. Decided-and-documented (not implemented): thedlq.tswebhook self-heal re-drive is Cloudflare-MessageBatch-specific and cannot run on self-host as-is; self-host's ownreviveDeadLetterJobs()sweep already provides a comparable (if not identical) generic revival safety net for any dead-lettered job type, including webhooks, so this is not a bare gap — porting the exact rate-limit-aware single-redrive behavior is flagged as a real, separately-scoped followup.orb(observability): the orphaned review_targets table silently disables the anomaly alerter, the reputation quality signal, and the AMS bridge — and the reputation half expires to empty around 2026-09-20 #9136 — the orphaned
review_targetstable silently disabled the anomaly alerter (partial fix; two sub-parts deferred, documented below).review_targetshas had no live writer anywhere since the 2026-06-22 convergence cutover. Fixed:computeAgentHealth's reversal and DLQ signals are repointed offreview_targetsdirectly onto the livereview_auditledger — parsingrepo/numberout ofreview_audit's owntarget_idinstead of joiningreview_targetsfor them, which also fixes a separate bug the join had:review_audit.target_idisowner/repo#123whilereview_targets.idisproject:kind:owner/repo#123— a different namespace the join could never actually match, even whilereview_targetswas still live. TherecentAutoActionsdenominator is repointed ontoreview_audit's owngate_decisionrows. AddedcheckReviewSourceFreshness+loopover_review_source_freshgauge +LoopoverReviewSourceStalealert — the generalizable fix: any table an ops/reputation module treats as a live, windowed source now gets checked for a row inside its own consumer's window, so the next orphaning is loud instead of silent for months.byStatus/manualRate/stuckRetryable/failed/configIssues/holdOnlyincomputeAgentHealth, andcomputeCalibrationentirely (recommendedFloor,disputedCloseCount, calibration bins) remainreview_targets-sourced and therefore still silently zero — repointing these needs reconstructing a full review-state-machine equivalent frompull_requests+review_audit, which is a substantially larger, separately-scoped rewrite.submitter-reputation.ts'sgetSubmitterReputation/getSubmitterReputationAcrossInstall/listSubmitterCohortRows(the reputation quality signal, which will expire to permanently empty around 2026-09-20) andams-miner-cohort.ts(feat(ui): maintainer-facing AMS miner contributor-mix dashboard panel #6488, degrades topresent: false) are untouched. Both are correctly flagged by the newcheckReviewSourceFreshness'sreview_targetsentry once it goes stale (it already reads stale today, sincereview_targetshas no live writer at all).Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not touchsite/,CNAME, or**/lovable/**.Closes #9128
Closes #9133
Closes #9139
Addresses #9136 (partial — see Summary above for exactly what's fixed vs. deferred; not closing since the reputation-signal and AMS-cohort sub-parts remain unaddressed)
Validation
git diff --checknpm run typechecknpm run selfhost:validate-observabilitynpm run selfhost:env-reference:check(regeneratedapps/loopover-ui/src/lib/selfhost-env-reference.tsafter the newGITHUB_APP_SLUGreads inpg-queue.ts/sqlite-queue.ts)npm run db:schema-drift:checknpm run db:migrations:checknpm run docs:drift-checknpm run manifest:drift-checknpx vitest run+--coverageon every touchedsrc/**file individually: 100% statements/branches/lines on every new/changed line (the only uncovered lines reported anywhere are pre-existing, unrelated to this diff — verified by line number against the diff).npm run test:changed(full affected-test sweep): 455 files, 11165 tests, all passing, 0 failures.npm run test:coverage,npm run test:ci,npm audit --audit-level=moderate,ui:build/ui:test/ui:lint/ui:typecheck,test:workers,build:mcp/test:mcp-pack— intentionally left to CI per explicit instruction for this batch; every individually-affected test file was run locally with coverage instead (see above).If any required check was skipped, explain why:
npm run test:ci/npm run test:coverage/npm auditgate was intentionally not run locally for this PR — CI runs it, and targeted per-file coverage + a fulltest:changedsweep (11165 tests) were run locally instead to keep iteration fast across a four-issue batch.src/server.tsisignored incodecov.yml(module-scopeserve()side effect, same aspackages/discovery-index/src/server.ts), so the wiring changes there (gauge registrations, theinstallSelfHostCrashHandlerscall site,bootAtMsthreading) are exempt from patch coverage by design; the underlying logic each wiring line calls is separately, fully unit-tested.Safety
Notes
prometheus/rules/alerts.ymlandalertmanager/alertmanager.ymlare touched as a direct, necessary part of fixing orb(observability): the relay-drain staleness alarm is suppressed by its own worst failure — 'never drained' reads -1, and a deploy silences a firing alert #9128/orb(observability): the queue-backlog alerts deactivate during the DB incident they exist to catch, and both DLQ-rate surfaces are structurally always zero #9139 (new alert rules, a real capitalization bug fix) — outside the public.loopover.yml's examplewantedPathslist, but this is a maintainer PR where that contributor-gate restriction doesn't apply.apps/loopover-ui/src/lib/selfhost-env-reference.tsis a regenerated artifact (npm run selfhost:env-reference), committed per house convention — the only diff isGITHUB_APP_SLUG's recorded first-reference file, since the two new self-host queue reads of that var now appear earlier in the scanned file list.