Skip to content

fix(observability): stop index_advisor poisoning PostgREST prepared statements; unrank two telemetry bugs - #1932

Merged
njrini99-code merged 6 commits into
mainfrom
agent/incident-triage-2026-09-09
Sep 10, 2026
Merged

fix(observability): stop index_advisor poisoning PostgREST prepared statements; unrank two telemetry bugs#1932
njrini99-code merged 6 commits into
mainfrom
agent/incident-triage-2026-09-09

Conversation

@njrini99-code

@njrini99-code njrini99-code commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Production incident triage — 2026-09-09 export (238 incidents, 72h)

Root cause of the outage (fixed live in prod at 22:23:21Z)

extensions.index_advisor() runs DEALLOCATE ALL. helm_debug_db_analysis_snapshot() called it, /api/cron/db-table-health invokes that over PostgREST every hour at :07, and PostgREST reuses integer-named prepared statements across its pool — so every request landing on the poisoned backend failed with SQLSTATE 26000 until the connection recycled. Migration 20260909230000 removes the call. Verified: the 23:07 tick ran clean; 0 new 26000 events since.

200 of the 220 groups on the board were this one cause — 133 proven by message text, 67 by timing (every occurrence inside a 26000 burst window; burst minutes are 1.83% of the 72h window, so 67/67 with zero exceptions is not chance).

Board state (production admin_events, contract STEP 6)

before after
unanalysed fingerprints 191 0
unresolved fingerprints 232 14 (all categorised, all deliberately held — see below)
rca_analysis rows written 310, 0 malformed
admin_error_resolutions ledger rows 297 (every prior ledger row in this repo had fixed_in_sha = NULL; these carry the SHA)

New defects found and fixed here

  1. use-message-reactions.ts queried without a live session (8011d1b9, 42501 on golf_message_reactions). On iOS WKWebView a backgrounded tab fires before its token is attached; PostgREST runs it as anon; that table deliberately grants anon nothing (its siblings only fail silently because they still carry legacy blanket grants). Gate on getSession() and reload on SIGNED_IN/TOKEN_REFRESHED, as use-presence.ts already does. No grant, no RLS change — the database was right. Tests added.
  2. Player badge poll hammered a dead session (f402b5ab). getPlayerNotificationCounts now returns authExpired like the coach action, and the context trips stopPolling().
  3. Local dev servers were writing into production telemetry (e9f122e7 ×194 and 11 siblings — 100% /Users/… stack paths, 0 /var/task). vercel env pull had written VERCEL_ENV="production" into .env.local; telemetry-gate.ts now requires NODE_ENV=production, and the forged lines are deleted locally. A running dev server keeps its old env until restarted.
  4. resolveTriageMember never passed p_fixed_in_sha; npm run triage could not run at all (-r dotenv/config loads .env, which has no Supabase keys); the NODE_ENV guard had broken 5 tests I had not run. All fixed.
  5. scripts/triage-write-analyses.ts — the operator write path the triage contract describes but nothing implemented.

Held open on purpose

  • 12 × ALREADY FIXED — the local-dev leak; resolve after this PR is live and they stay quiet.
  • 8011d1b9 FIX HERE — fixed in this PR; resolve once live.
  • 38fd54c6 FIX HERE — Inngest signing key fails validation in prod. Owner action: rotate INNGEST_SIGNING_KEY/INNGEST_EVENT_KEY in Vercel Production and redeploy.
  • 5725d96a NEEDS MORE EVIDENCE/api/health readiness probe exceeded its 2.5s budget 41× in 72h on real Vercel (returns 503 to uptime monitors). Needs per-request durations from runtime logs scoped to one deploymentId.

Owner actions I could not take

  • REVOKE EXECUTE ON FUNCTION extensions.index_advisor(text) FROM PUBLIC is the structural hardening; it is a no-op as postgres (owner is supabase_admin). Nothing calls it today, but the hazard is intact.
  • scripts/__tests__/check-migration-headers.test.mjs fails on main already: six migrations from 09-06/09-08 lack -- ROLLBACK:/-- VERIFY: headers.
  • Supabase advisors: auth_rls_initplan (7 WARN, incl. two golf_rounds policies) and multiple_permissive_policies (140 WARN) are real perf debt, not incidents.
  • Stale supabase/migrations/HELD.md rows; ledger drift for 20260909130000_helm_jobs_depth_qualify_queue (in prod, no file on disk).

Why the self-heal loop never closed (fixed here, 880023b)

Walked every Bridge page in prod (Overview, Incidents ×3 views, Health ×3, Jobs, Database ×3, Flight Recorder, Teams ×2, Users, Activity, Utilization, Golf ×3, Baseball ×2, Lift Lab ×2, Work log ×2, Deploys, Releases, Auth, Engineering OS, Ben + Leah) — none render blank. The loop was failing at all three stages:

Stage What was actually happening Fix
Diagnose 184/184 cron analyses (2026-09-03 → 09-09) opened with free prose → deriveRcaCategory = uncategorized → Close could auto-resolve nothing, Repair ranked nothing. The rca.ts prompt never asked for the canonical prefix. Cap of 8 groups/run vs a 183-group backlog → capped: true every run. Model now returns an explicit category enum; withCanonicalPrefix stamps FIX HERE / ALREADY FIXED / NOT A DEFECT / NEEDS MORE EVIDENCE before persist (stored shape unchanged, every existing row still parses). Cap 8 → 24; TIME_BUDGET_MS stays the real guard.
Repair Run 34347403858 did real work (failing test + fix for 530e91c6 in a worktree), then started npm test / preflight with run_in_background, said "I'll wait for the notification", ended its turn — the headless claude -p process exited, killed both tasks, lost the worktree, and failed for no REPAIR-SUMMARY. Workflow prompt + repair-contract.md STEP 4 forbid background execution in a one-shot session.
Bridge Loop tab labelled Repair "Local agent — on the owner's laptop" (retired 2026-09-05; it is a GHA workflow). New github-actions runner in selfheal-registry.ts with its own icon/reach copy.
Noise "The destination stream closed early." — Next's client-left-mid-RSC-stream message; 260 rows / 9 /golf/dashboard/* fingerprints / 0 users in 72h, and the one thing the dead Repair run spent itself on. Ignored at Sentry level and skipped for the Bridge write (instrumentation.ts, whole-message anchored; a stream error that merely mentions closing still writes).

CI fixes in 89f8fd8: registry mapping for the index_advisor migration, refreshed docs/generated/WORLD_MODEL.json, and on conflict (version) do nothing on the migration's ledger insert (the CLI records the version before the file body runs under supabase db reset). Snapshot Testing is Sentry's preprod size check — not investigated.

Tests: rca / rca-category / selfheal-registry / selfheal-capability / self-heal-circuit / instrumentation-duplicate-capture / selfheal-triage route / cron-registry — 168 passing; npm run typecheck clean.

Blank Bridge tabs — every destination checked (e93f400)

Visited every admin route in src/app/admin/** including the dynamic ones (errors/[fingerprint], users/[id] + Journey tab, users/[id]/view-as, teams/[id], thread/team|user, golf/tracer), every ?view= axis, /baseball/admin/demo-sessions, /golf/admin/demo-sessions, and /golf/admin/crm with all eight ?tab= values. Only /golf/admin/crm stayed blank, and only in a background tab.

Root cause, verified in the browser: React 19.2 batches streamed Suspense reveals and schedules the first batch with requestAnimationFrame. A hidden tab gets no frames, so the queued boundaries (B:0/S:0 golf loading, B:1/S:1 crm loading) sat in $RB with $RT unset until the tab was viewed. Calling $RV($RB) by hand, or a momentary tab activation, unblocked it every time. Upstream behaviour (React main still does this), not a Helm defect — but the CRM was the worst case because it did no data work until viewed and then gated its whole shell behind a second, client-side getUserResilient round trip.

Fix: sessionReady starts true (the admin layout already verified the request), the client re-check runs in the background, and only a definitive rejection redirects — a transient auth-server blip no longer bounces a signed-in admin to /golf/login. SIGNED_OUT still flips the gate. Not changed: the framework's reveal scheduling.

CI fixes (e93f400)

  • Supabase lint + RLS — migration 20260909230000 inserted its own schema_migrations row. scripts/db/apply.mjs appends that insert at apply time and the CLI writes it under db reset, so it collided (duplicate key … schema_migrations_pkey, statement 2). Removed; production already carries the version.
  • Static checksdocs/generated/HELM_FEATURE_MAP.md regenerated after the registry gained the migration glob.
  • Unit tests (3)CalendarSubscriptionsSheet.test.tsx › regenerate requires an inline confirm failed in CI, passes locally (9/9) and is untouched by this branch; treated as a flake unless it repeats.
  • Snapshot Testing (Sentry preprod) is not a required check.

Verification

npx tsc --noEmit clean · full vitest: 16,550 pass (the one failing file predates this branch) · npm run db:types no diff · gitleaks clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z

njrini99-code added a commit that referenced this pull request Sep 10, 2026
…make its ledger insert idempotent

Three CI failures on #1932: `knowledge_check` (the new migration mapped to no
feature), `world_model` (docs/generated/WORLD_MODEL.json stale), and the
local Supabase stack ("duplicate key value violates unique constraint
schema_migrations_pkey" — the CLI records the version before the file body's
own ledger insert runs).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
njrini99-code added a commit that referenced this pull request Sep 10, 2026
…row; refresh the feature map

/golf/admin/crm was the one Bridge destination that stayed on the
"Loading dashboard…" skeleton. Every other admin route, ?view=, dynamic
route and in-page tab renders. Root cause, verified in the browser:

- React 19.2 batches streamed Suspense reveals and schedules the first
  batch with requestAnimationFrame ($RC pushes to $RB, $RV runs on the
  next frame). A hidden tab gets no frames, so the queued boundaries
  (B:0/S:0 golf loading, B:1/S:1 crm loading) never reveal until the tab
  is viewed. Calling $RV($RB) by hand, or a momentary tab activation,
  unblocked it every time. Upstream behaviour (still rAF on React main),
  not a Helm defect — but it made the CRM the worst case, because the
  page did no data work at all until it was viewed AND then gated its
  entire shell behind a second, client-side getUserResilient round trip
  before the first crm_coaches request could start.

- sessionReady now starts true: /golf/admin/layout.tsx already verified
  the session and admin role for this request. The client re-check runs
  in the background and only redirects on a definitive rejection; a
  transient auth-server blip no longer bounces a signed-in admin to
  /golf/login. SIGNED_OUT still flips the gate and redirects.

- crm/loading.tsx keeps its cheap spinner (the server IS checking the
  admin session during that window) and its doc comment now describes
  the new first paint instead of the retired client gate.

CI fixes for PR #1932:

- 20260909230000_helm_debug_analysis_drop_index_advisor.sql no longer
  inserts its own schema_migrations row. scripts/db/apply.mjs appends
  that insert at apply time and the Supabase CLI writes it under
  `db reset`, so the in-file row collided with the CLI's own insert
  ("duplicate key value violates unique constraint
  schema_migrations_pkey", statement 2). Production already carries the
  version (verified in supabase_migrations.schema_migrations).

- docs/generated/HELM_FEATURE_MAP.md regenerated (gen-feature-map
  --check failed after the registry gained the migration glob).

Verified: tsc clean; vitest src/app/golf/admin/crm 225/225; npm run
knowledge:check green.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
njrini99-code and others added 6 commits September 9, 2026 22:45
…k two telemetry bugs

Triage of the 2026-09-09 72h incident export (238 actionable rows).

GROUP A — SQLSTATE 26000, ~150 of the 238 incidents, 9 affected users.
`extensions.index_advisor()` executes `DEALLOCATE ALL`. It was reached from
`public.helm_debug_db_analysis_snapshot()`, which `/api/cron/db-table-health`
invokes over PostgREST on the `7 * * * *` cron — so `DEALLOCATE ALL` landed on
a PostgREST backend connection and wiped every prepared statement PostgREST
held on it. PostgREST names its statements with an integer counter and keeps
reusing them, so every later request routed onto that connection failed with
`prepared statement "N" does not exist`, across auth, player hub, messaging,
stats, rounds, the CoachHelm v3 generator fleet, /api/jobs/consume and
/api/admin/log-event.

  Migration 20260906120100 (which introduced the call) reached prod at
  12:47:44Z on 2026-09-09; the first 26000 in the preceding 60h was at 13:07 —
  the first `7 * * * *` run after the function existed. Every burst thereafter
  starts at :07. Nothing else runs at :07.

20260909230000 removes the call; the `index_suggestion` category degrades to
the `note` row `flattenAnalysisSnapshot` already handles, so no TypeScript
change was needed. Applied to production 2026-09-09T22:23:21Z with owner
authorization; verified zero 26000 events since.

GROUP D1 — local dev was writing into the production Bridge.
`shouldPersistAdminTables()` ended at `VERCEL_ENV === 'production'`, and both
.env.local and .env.production.local on the owner's machine hardcode
VERCEL_ENV="production" (what `vercel env pull` writes). 287 of 1626 rows in
the 72h window carry local `/Users/.../app-page.runtime.dev.js` or
`webpack-internal:///(rsc)/` frames. Gate on NODE_ENV, which the Next CLI sets
before any .env* file loads and dotenv cannot overwrite. Same check added to
getRuntimeEnv() so the metadata tag stops lying under ADMIN_EVENTS_FORCE_CAPTURE.

GROUP D2 — client disconnects filed as unhandled server defects.
'The destination stream closed early.' (163 events) and bare 'aborted' (67)
were the export's two largest actionable incidents. Rule 3c only matched
`source === 'client'`, but Next's onRequestError tags these with the server
component/route/action that was streaming. New rule 3d matches regardless of
source; bare 'aborted' by exact equality so our own AbortSignal.timeout budget
('The operation was aborted due to timeout') stays actionable.

GROUP G — the observability pipeline dropping its own events.
`logEmailSuppressed()` forwarded a free-text call-site path into
`admin_events.source`, which is CHECK-constrained to 11 values, so every
suppressed-email event raised 23514 and was silently dropped. Pass 'system'
and move the path to metadata.callSite. Fixes all four gateCustomerEmail()
callers at once.

GROUP F (partial) — `logServerException` normalized non-Errors with
`String(error)`, turning a PostgrestError into '[object Object]' in
admin_events. Use describeError(), already imported. The broader
describe-error.ts change proposed during triage is NOT included: adversarial
review showed it would break a currently-passing regression test.

Verification: npx tsc --noEmit clean; 1247 tests pass across
src/lib/observability/supabase, src/lib/admin/database, src/lib/admin/incidents
and error-trace-classification. Local Supabase was unavailable (Docker not
running), so db:lint:functions could not run; the migration was instead
verified by structural diff against the live function body and by its own
VERIFY block against production.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
…npm run triage

The NODE_ENV guard added in 8d8435a broke three existing telemetry-gate
tests. They stub VERCEL_ENV and assert the production/preview paths, but
vitest runs under NODE_ENV=test, so the new dev-machine guard rejected every
deployment scenario. Every Vercel deployment — production AND preview — runs
the app under NODE_ENV=production because `next build`/`next start` set it,
so the fixture now models that and the tests that mean "a developer's laptop"
override it explicitly.

Adds the assertion that was missing entirely: the gate returns true on the
real production path, and false from a dev machine even with a hardcoded
VERCEL_ENV=production (and VERCEL=1 — `vercel env pull` writes both verbatim,
so neither is a trustworthy signal off Vercel).

Also: `npm run triage` could not run at all. `-r dotenv/config` loads `.env`,
which carries no Supabase keys, so the contract's STEP 1 collector died on
"NEXT_PUBLIC_SUPABASE_URL is missing or a placeholder" — which is why the
last triage pass hand-rolled its SQL instead. Uses the DOTENV_CONFIG_PATH
convention already established by `coachhelm:regen` and `trace:db`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
…ion, stop the dead-session badge poll, record fixed_in_sha

Board (72h, production): 191 unanalysed fingerprints -> 0; 232 unresolved -> 14,
all 14 categorised and deliberately held (12 ALREADY FIXED pending this PR's
merge, 2 FIX HERE, 1 NEEDS MORE EVIDENCE). 310 rca_analysis rows written, 297
ledger rows, 0 malformed, 0 new SQLSTATE 26000 since the 22:23Z hotfix.

Two new defects found while closing it:

- use-message-reactions.ts queried golf_message_reactions without checking for
  a live session. On iOS WKWebView a backgrounded tab can fire before its
  token is attached, PostgREST runs the request as `anon`, and that table
  deliberately grants anon nothing (20260908160000) — so it fails 42501 where
  its siblings fail silently. Gate refresh()/setReaction() on getSession() and
  re-run on SIGNED_IN/TOKEN_REFRESHED, exactly as use-presence.ts already does.
  No grant, no RLS change: the database was right. Tests added.

- getPlayerNotificationCounts returned {success:false,'Not authenticated'} for
  an expired session, which withAdminObserved persisted to the Bridge on every
  45s poll; the coach action already returned a silent authExpired. Mirror it,
  and trip the player branch's stopPolling() the way the coach branch does.

Also:
- resolveTriageMember never passed p_fixed_in_sha, so every ledger row this
  repo ever wrote had a NULL SHA and a recurrence could not be traced to the
  change that supposedly fixed it. Pass it through.
- scripts/triage-write-analyses.ts: the operator path triage-contract.md STEP 3
  describes but nothing implemented — validates the exact shapes that fail
  silently (suspectFiles as strings, non-canonical prefix) before writing.
- cron-monitors tests model NODE_ENV=production like every Vercel deployment.
- telemetry-gate.ts documents the `next start` variant the NODE_ENV guard
  cannot catch (96 rows measured); VERCEL/VERCEL_ENV removed from the local
  env files, which is the only defence for that case.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
…ound gates in Repair, drop client-abort noise

Diagnose: every one of the 184 analyses the Vercel cron wrote 2026-09-03 →
09-09 opened with free prose, derived to `uncategorized`, and so Close could
resolve nothing (`isAutoResolvable`) and Repair ranked nothing. The prompt
never asked for the canonical prefix. `runRcaAnalysis` now asks the model for
an explicit `category` enum and stamps the matching `FIX HERE` /
`ALREADY FIXED` / `NOT A DEFECT` / `NEEDS MORE EVIDENCE` opening on
`suggestedFix` before persisting (`withCanonicalPrefix`). Stored shape is
unchanged so every existing row still parses. Cap raised 8 → 24 groups per
run; with a 183-group backlog `capped: true` was every run while each run
finished in under a minute — `TIME_BUDGET_MS` remains the real guard.

Repair: run 34347403858 (2026-09-09) wrote a correct failing test and fix,
started `npm run preflight` and `npm test` with run_in_background, said
"I'll wait for the notification", ended its turn — and the headless
`claude -p` process exited, killed both, discarded the worktree and failed
for having no REPAIR-SUMMARY. The workflow prompt and repair-contract.md now
forbid background execution outright.

Bridge: Repair has run in GitHub Actions since 2026-09-05 but the Loop tab
still said "Local agent — on the owner's laptop", sending an operator to wake
a machine that is not the runner. New `github-actions` runner in the
registry, with its own icon and reach text.

Noise: "The destination stream closed early." is Next's own message when the
browser leaves while an RSC payload is streaming — 260 rows across nine
/golf/dashboard/* fingerprints in 72h, zero affected users, and the one thing
the failed Repair run spent itself on. Ignored at the Sentry level and skipped
for the Bridge write, anchored to the whole message.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
…make its ledger insert idempotent

Three CI failures on #1932: `knowledge_check` (the new migration mapped to no
feature), `world_model` (docs/generated/WORLD_MODEL.json stale), and the
local Supabase stack ("duplicate key value violates unique constraint
schema_migrations_pkey" — the CLI records the version before the file body's
own ledger insert runs).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
…row; refresh the feature map

/golf/admin/crm was the one Bridge destination that stayed on the
"Loading dashboard…" skeleton. Every other admin route, ?view=, dynamic
route and in-page tab renders. Root cause, verified in the browser:

- React 19.2 batches streamed Suspense reveals and schedules the first
  batch with requestAnimationFrame ($RC pushes to $RB, $RV runs on the
  next frame). A hidden tab gets no frames, so the queued boundaries
  (B:0/S:0 golf loading, B:1/S:1 crm loading) never reveal until the tab
  is viewed. Calling $RV($RB) by hand, or a momentary tab activation,
  unblocked it every time. Upstream behaviour (still rAF on React main),
  not a Helm defect — but it made the CRM the worst case, because the
  page did no data work at all until it was viewed AND then gated its
  entire shell behind a second, client-side getUserResilient round trip
  before the first crm_coaches request could start.

- sessionReady now starts true: /golf/admin/layout.tsx already verified
  the session and admin role for this request. The client re-check runs
  in the background and only redirects on a definitive rejection; a
  transient auth-server blip no longer bounces a signed-in admin to
  /golf/login. SIGNED_OUT still flips the gate and redirects.

- crm/loading.tsx keeps its cheap spinner (the server IS checking the
  admin session during that window) and its doc comment now describes
  the new first paint instead of the retired client gate.

CI fixes for PR #1932:

- 20260909230000_helm_debug_analysis_drop_index_advisor.sql no longer
  inserts its own schema_migrations row. scripts/db/apply.mjs appends
  that insert at apply time and the Supabase CLI writes it under
  `db reset`, so the in-file row collided with the CLI's own insert
  ("duplicate key value violates unique constraint
  schema_migrations_pkey", statement 2). Production already carries the
  version (verified in supabase_migrations.schema_migrations).

- docs/generated/HELM_FEATURE_MAP.md regenerated (gen-feature-map
  --check failed after the registry gained the migration glob).

Verified: tsc clean; vitest src/app/golf/admin/crm 225/225; npm run
knowledge:check green.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGiUYKH3fXJrVnxPxH2h8Z
@njrini99-code
njrini99-code force-pushed the agent/incident-triage-2026-09-09 branch from e93f400 to e07a2cc Compare September 10, 2026 02:46
@njrini99-code
njrini99-code merged commit 6ea9e2c into main Sep 10, 2026
28 checks passed
@njrini99-code
njrini99-code deleted the agent/incident-triage-2026-09-09 branch September 10, 2026 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant