fix(admin): harden Sentry/Vercel observability and error-identity attribution - #777
Merged
njrini99-code merged 1 commit intoJul 3, 2026
Merged
Conversation
…ribution
Bucket D/E of the HelmV3 stabilization brief (admin-rollup root-cause
gating was already fixed separately in the Supabase drift PR).
- TriageQueue: an `app`-origin incident with events but zero known
user_id/user_email now shows "unknown user" instead of "0 users" —
the raw count only means "no event carried an identity" (anonymous/
system failure, or identity wasn't wired into the observed-action
call), not that nobody was affected. Sentry-origin items keep the
literal count since Sentry's userCount is a real zero-means-zero
metric.
- observed-action.ts: withAdminObserved now accepts an optional
contextFrom(args) callback so a caller that already knows its
subject (e.g. generateRoundRecap always knows roundId, even though
the wrapper's own resolveObservedUser() only ever sees the
*authenticated* user) can attribute admin_events rows to the right
round/player/team instead of relying on auth identity alone. Wired
into generateRoundRecap as the concrete example from the brief.
Enrichment failures are swallowed — they can never mask the real
error or block logging.
- vercel-api.ts: fetchVercelWebInsights previously mapped an HTTP
401/403 from the web-insights endpoint to `{ visitors: 0 }` wrapped
in status:'ok' — indistinguishable from genuinely quiet traffic.
Now surfaces status:'error' like fetchVercelDeployments already
does; /admin/deploys already treats any non-'ok' status as
PanelNoData, so no page change was needed.
- New docs: SENTRY_ADMIN_READ_API.md (required SENTRY_READ_TOKEN
scopes, why not to reuse the CI sourcemap token, fail-soft state
table) and VERCEL_ADMIN_DEPLOYS_RUNBOOK.md (required env vars,
failure-mode table, links to the CI preview-pending runbook).
Sentry read API (sentry-api.ts) and the Vercel deploys page were
already fail-soft (unconfigured/error envelopes, never throw) —
verified, not changed.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughAdds two operations docs (Sentry admin read API, Vercel deploys runbook), extends ChangesAdmin observability, error-handling, and operations docs
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoHarden admin observability: correct Sentry/Vercel fail-soft + richer error attribution
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
97 rules 1. src/app/admin not in registry
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+22
to
+37
| /** | ||
| * "0 users" reads as "this affected nobody," which is misleading for `app` | ||
| * incidents: affectedUsers there is a count of DISTINCT KNOWN identities | ||
| * (user_id/user_email), so 0 usually means the failure happened before/ | ||
| * outside auth (anonymous, system/cron, or identity wasn't wired into the | ||
| * observed-action call) — not that zero people were impacted. Sentry-origin | ||
| * items use Sentry's own userCount, which IS a real zero-means-zero metric, | ||
| * so only `app` incidents get the "unknown" wording. | ||
| */ | ||
| export function affectedUsersLabel(item: Pick<TriageItem, 'origin' | 'affectedUsers' | 'occurrences'>): string { | ||
| if (item.origin === 'app' && item.affectedUsers === 0 && item.occurrences > 0) { | ||
| return 'unknown user'; | ||
| } | ||
| const n = item.affectedUsers; | ||
| return `${n} user${n === 1 ? '' : 's'}`; | ||
| } |
There was a problem hiding this comment.
2. src/app/admin not in registry 📘 Rule violation ⚙ Maintainability
This PR changes Helm Bridge admin functionality under src/app/admin/**, but memory/registry.yml has no mapping covering src/app/admin/**. This violates the requirement to update the feature registry when touching unmapped feature areas.
Agent Prompt
## Issue description
Modified code under `src/app/admin/**` is not represented in `memory/registry.yml` feature mappings.
## Issue Context
This PR changes `src/app/admin/_components/TriageQueue.tsx` (Helm Bridge admin), but `memory/registry.yml` does not include any `code.routes/components/...` glob that matches `src/app/admin/**`.
## Fix Focus Areas
- memory/registry.yml[955-1010]
- src/app/admin/_components/TriageQueue.tsx[1-14]
- src/app/admin/_components/TriageQueue.tsx[22-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+22
to
+37
| /** | ||
| * "0 users" reads as "this affected nobody," which is misleading for `app` | ||
| * incidents: affectedUsers there is a count of DISTINCT KNOWN identities | ||
| * (user_id/user_email), so 0 usually means the failure happened before/ | ||
| * outside auth (anonymous, system/cron, or identity wasn't wired into the | ||
| * observed-action call) — not that zero people were impacted. Sentry-origin | ||
| * items use Sentry's own userCount, which IS a real zero-means-zero metric, | ||
| * so only `app` incidents get the "unknown" wording. | ||
| */ | ||
| export function affectedUsersLabel(item: Pick<TriageItem, 'origin' | 'affectedUsers' | 'occurrences'>): string { | ||
| if (item.origin === 'app' && item.affectedUsers === 0 && item.occurrences > 0) { | ||
| return 'unknown user'; | ||
| } | ||
| const n = item.affectedUsers; | ||
| return `${n} user${n === 1 ? '' : 's'}`; | ||
| } |
There was a problem hiding this comment.
3. admin-platform doc not updated 📘 Rule violation ⚙ Maintainability
This PR changes admin triage/user-impact labeling and admin observability failure behavior, but no corresponding memory/features/* documentation update was made. This can cause the feature’s current-state documentation to drift from actual admin behavior.
Agent Prompt
## Issue description
Business/UX behavior changed in admin observability/triage, but `memory/features/*` docs were not updated to reflect the new behavior.
## Issue Context
The triage queue copy now renders `unknown user` for certain `app`-origin incidents instead of `0 users`, and Vercel web-insights now surfaces auth failures as `status: 'error'` instead of returning fake zeros.
## Fix Focus Areas
- memory/features/admin-platform.md[1-78]
- src/app/admin/_components/TriageQueue.tsx[22-37]
- src/lib/admin/vercel-api.ts[109-149]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+150
to
+156
| // The round detail page already knows the round it's rendering — wire | ||
| // that into admin_events on failure instead of relying solely on the | ||
| // authenticated user id (which is null for any unauthenticated/expired | ||
| // session edge case, and never carries which round/player was involved | ||
| // either way). | ||
| contextFrom: ([roundId]) => ({ roundId }), | ||
| }, |
There was a problem hiding this comment.
4. Coverage scanner brittle 🐞 Bug ☼ Reliability
The static action coverage scanner’s WRAP_RE assumes the withAdminObserved opts object contains no
nested braces, but the new contextFrom usage introduces nested {} via `([roundId]) => ({ roundId
})`. This can truncate the captured optsSrc and make wrapper coverage/feature extraction tests
fragile (and potentially miss regressions if opts ordering changes).
Agent Prompt
## Issue description
`scanActionFile()` in `src/lib/admin/__tests__/coverage-scanner.ts` uses a regex capture `({[^}]*})` for the withAdminObserved opts object and explicitly relies on “non-nested object literal” formatting. The PR introduces a nested brace sequence inside opts (`contextFrom: ([roundId]) => ({ roundId })`), which violates that assumption and can lead to truncated option capture and brittle coverage checks.
## Issue Context
This is test/tooling code, but it enforces the wrapper-coverage contract across many server action files; fragility here can reduce confidence in the observability coverage guarantees.
## Fix Focus Areas
- src/lib/admin/__tests__/coverage-scanner.ts[33-55]
- src/app/golf/actions/round-recap.ts[145-158]
## What to change
- Replace `WRAP_RE`’s `\{[^}]*\}` capture with a small balanced-brace extractor:
- Regex-match up to the start of the opts object (`withAdminObserved(..., {`), then scan forward counting `{`/`}` (skipping over strings/comments as needed) to find the matching closing brace.
- Alternatively, switch to a lightweight TS/JS parser (e.g., TypeScript compiler API) since this is test-only.
- Add/adjust a unit test fixture demonstrating `contextFrom: (...) => ({ ... })` so the scanner behavior is locked in.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This was referenced Jul 16, 2026
This was referenced Jul 23, 2026
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
1 participant
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
Bucket D/E of the HelmV3 stabilization brief (the admin-rollup 42501 root-cause gating fix from Bucket C was already shipped separately in #775, discovered while building the Supabase drift guard).
app-origin incident with events but zero knownuser_id/user_emailnow shows "unknown user" instead of "0 users." The raw count only means "no event carried an identity" (anonymous/system failure, or identity wasn't wired into the observed-action call) — not that nobody was affected. Sentry-origin items keep the literal count since Sentry'suserCountis a real zero-means-zero metric.withAdminObservednow accepts an optionalcontextFrom(args)callback so a caller that already knows its subject (e.g.generateRoundRecapalways knowsroundId, even though the wrapper's ownresolveObservedUser()only ever sees the authenticated user) can attributeadmin_eventsrows to the right round/player/team instead of relying on auth identity alone. Wired intogenerateRoundRecapas the concrete example named in the brief. Enrichment failures are swallowed — they can never mask the real error or block logging.fetchVercelWebInsightspreviously mapped an HTTP 401/403 from the web-insights endpoint to{ visitors: 0 }wrapped instatus: 'ok'— indistinguishable from genuinely quiet traffic. Now surfacesstatus: 'error'likefetchVercelDeploymentsalready does./admin/deploysalready treats any non-'ok'status asPanelNoData, so no page change was needed — verified by reading the consumer, not assumed.docs/operations/SENTRY_ADMIN_READ_API.md(requiredSENTRY_READ_TOKENscopes, why not to reuse the CI sourcemap token, fail-soft state table) anddocs/operations/VERCEL_ADMIN_DEPLOYS_RUNBOOK.md(required env vars, failure-mode table, links to the CI preview-pending runbook).Verified, not changed
Sentry's read API (
sentry-api.ts) and the Vercel deploys page were already fail-soft (unconfigured/error envelopes, never throw) before this PR — confirmed by reading the code, documented in the new runbooks rather than re-implemented.Test plan
npm run typecheck— clean.npm run lint— clean (--max-warnings 0).npm run test:run— 429 files / 4405 passed, 39 skipped (was 428/4398; +1 file/+7 tests, matching what's added/modified).observed-action.test.ts(contextFrom derivation, omission, and failure-safety),vercel-api.test.ts(403 now surfacesstatus: 'error'instead of fake zeros — this test previously asserted the bug being fixed),affected-users-label.test.ts(new — wording logic in isolation).Related
Made with Cursor