feat(bridge): max out error tracking into Helm Bridge for golf + baseball - #894
Conversation
…ball Close every P0/P1 gap found by a 10-surface audit of the error-capture pipeline so all errors — client, server, edge, cron, background — land in error_logs + admin_events (the Bridge), not just Sentry. Platform: - onRequestError now writes unhandled server/RSC/route errors to the Bridge (nodejs direct; edge via new INTERNAL_LOG_KEY-guarded relay route), with control-flow + already-logged dedup (bridge-logged-marker) - process-level unhandledRejection/uncaughtException capture (throttled) - server beforeSend derives per-app sport tag; client tags /admin, /lifting, marketing distinctly; golf gets Sentry.setUser parity with baseball Client transport: - /api/log-error fetch: keepalive, response.ok check, one retry, sendBeacon fallback; chunk-load/hydration errorKind tagging - root error.tsx now reports via RouteErrorBoundary; new baseball root error.tsx; duplicate admin-logger-client global listeners removed - ~24 toast-only/silent failure sites wired to logError across golf, baseball, lifting (messaging, roster, calendar, settings, lift logging, watchlist, postgame, uploads); zero-feedback sites also gained toasts Server/API: - new withRouteHandler wrapper; 5 silent routes retrofitted (baseball staff context, both iCal feeds, putt-tendencies incl. error-echo fix, gcal sync) - recordJobRun marks >=400 responses failed; genome-nightly, weekly-coach- email, ingest-sync, backfills, process-sequences, admin-digest all log and return honest statuses; Inngest functions get Bridge logging - baseball auth parity: logLogin/logSignup/logSecurityEvent; middleware route-gate DB errors + idle signOut failures now visible - CRM zone (owner-excluded from withAdminObserved): logging-only instrumentation, log + rethrow, exclusion tests untouched - pipeline self-reliability: Bridge insert failures alert Sentry (throttled), RLS denial storms escalate past skipSentry, opt-in ADMIN_EVENTS_CAPTURE_PREVIEW flag for preview persistence Gates: typecheck, lint, unit (5350 tests), coverage contracts, prod build all pass. Adversarially reviewed (3 lenses, 10 findings resolved). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
Too many files changed for review. ( Bypass the limit by tagging |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…oist auth above try The Review Gate's helmv3-server-action-missing-auth-check rule only recognized inline auth.getUser() and requireAdmin(). All 7 golf CRM action files gate auth through a locally-defined getAuthedClient() (createClient + getUser + Unauthorized throw — verified in every file), so they were latent rule violations on main that never surfaced: CI semgrep scans only changed files, and this PR touches the CRM zone for the first time since the rule shipped (64 blocking findings). Add getAuthedClient to the rule's house-helper allowance, mirroring the existing requireAdmin pattern. Also hoist the getAuthedClient() call above the new try/catch wrappers so auth failures throw uncaught exactly as before this branch, rather than being Bridge-logged as errors. Verified: semgrep 0 findings across all 8 custom rules on the 7 files; typecheck, eslint, and CRM coverage-contract tests (40/40) pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
@coderabbitai full review |
Action performedFull review triggered. |
|
Deployment failed with the following error: |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
🤖 Mission Control — PR summary What it changes: Maxes out error-tracking instrumentation into Helm Bridge across GolfHelm + BaseballHelm — 79 files, +3,872 / −1,512. Broad sweep routing server actions, API routes ( Risk / areas reviewers should watch:
CI status: Automated by the Mission Control sweep — this is not a merge approval. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (79)
Summary by CodeRabbit
WalkthroughThis PR expands centralized observability across API routes, cron jobs, server actions, client components, hooks, Sentry instrumentation, and Helm Bridge ingestion. It also adds preview telemetry gating, error classification, route error boundaries, and structured authentication logging. ChangesObservability infrastructure
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 9 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/golf/(dashboard)/dashboard/calendar/page.tsx (1)
73-78: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAwait server telemetry before continuing or throwing.
logServerExceptionis asynchronous; discarding its promise can lose Bridge writes when the request completes. The calendar path also throws a new unmarked Error, allowing duplicate request-level capture.
src/app/golf/(dashboard)/dashboard/calendar/page.tsx#L73-L78: normalize once,await logServerException(reportedError, ...), then throw that same marked Error.src/app/golf/(dashboard)/dashboard/rounds/page.tsx#L50-L51: await the warning log before continuing withteamId = null.src/app/golf/(dashboard)/dashboard/rounds/page.tsx#L100-L101: await the warning log before continuing with an empty member result.Calendar-path fix
} catch (error) { - void logServerException(error, context, 'warning'); - throw new Error('Failed to load your team for the calendar. Please try again.'); + const reportedError = + error instanceof Error ? error : new Error(String(error)); + await logServerException(reportedError, context, 'warning'); + throw reportedError; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/`(dashboard)/dashboard/calendar/page.tsx around lines 73 - 78, Await logServerException in the calendar page catch block after normalizing the caught value into one marked reportedError, then throw that same marked Error instead of creating an unmarked replacement. In src/app/golf/(dashboard)/dashboard/calendar/page.tsx lines 73-78, apply both changes; in src/app/golf/(dashboard)/dashboard/rounds/page.tsx lines 50-51 and 100-101, await the warning telemetry before continuing with teamId = null or the empty member result.src/components/golf/calendar/EventDocumentsSection.tsx (1)
89-124: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch rejected document operations and clear state in
finally—src/components/golf/calendar/EventDocumentsSection.tsx:89-141,279-291.These paths only handle resolved
{ success: false }responses. A rejected server action bypasseslogError; loads remain active and attach/detach controls remain permanently pending. Wrap each operation withtry/catch/finally, guard state updates withcancelledin effects, and clearloading/pendingActioninfinally.Also applies to: 127-141, 279-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/golf/calendar/EventDocumentsSection.tsx` around lines 89 - 124, Update the document-loading effect and the attach/detach handlers around getEventDocuments, attachDocumentToEvent, and detachDocumentFromEvent to catch rejected operations, log errors consistently, and preserve user-facing failure handling. Guard effect state updates with cancelled, and move setLoading(false) and setPendingAction(null) into finally blocks so loading and pending controls always clear, including when server actions reject.src/lib/server-error-logger.ts (1)
417-431: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMark the exception before awaiting telemetry (
src/lib/server-error-logger.ts, Lines 422–430).Callers such as
crm-dedup.tsfire-and-forgetlogServerException()and immediately rethrow. The rethrow can reachonRequestErrorwhilecaptureServerTrace()is still awaiting database writes, causing duplicate Bridge and Sentry events.Proposed fix
export async function logServerException( error: Error | unknown, context: RoundErrorContext, severity: Exclude<ServerTraceSeverity, 'info'> = 'error' ): Promise<void> { const normalizedError = error instanceof Error ? error : new Error(String(error)); + if (error instanceof Error) { + markBridgeLogged(error); + } // Caller explicitly handed us an Error — preserve the exception path so // the stack trace is captured even at warning severity. await captureServerTrace(normalizedError.message, context, severity, normalizedError, true); - if (error instanceof Error) { - markBridgeLogged(error); - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/server-error-logger.ts` around lines 417 - 431, Update logServerException to call markBridgeLogged on the original Error before awaiting captureServerTrace, ensuring fire-and-forget callers are marked immediately; retain the existing guard so non-Error inputs are not marked and preserve the telemetry call behavior.src/app/golf/actions/crm-assignee.ts (1)
17-29: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftValidate server-action inputs at runtime before querying Supabase.
These exported actions trust TypeScript-only input shapes across a runtime boundary. Add zod schemas for IDs, enums, strings, arrays, and patch payloads before constructing Supabase queries.
src/app/golf/actions/crm-assignee.ts#L17-L29: validatecoach_idandassignee.src/app/golf/actions/crm-automations.ts#L144-L254: validate create/update/delete payloads and IDs.src/app/golf/actions/crm-dedup.ts#L174-L209: validate both coach IDs and reject malformed identifiers.src/app/golf/actions/crm-foundations.ts#L85-L587: validate suppression, note, task, segment, ID, and patch inputs.As per path instructions, “Validate every input with zod before passing to Supabase.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-assignee.ts` around lines 17 - 29, Validate every server-action input with zod before constructing Supabase queries. In src/app/golf/actions/crm-assignee.ts lines 17-29, update setCoachAssignee to validate coach_id and assignee. In src/app/golf/actions/crm-automations.ts lines 144-254, validate all create, update, delete payloads and IDs; in src/app/golf/actions/crm-dedup.ts lines 174-209, validate both coach IDs and reject malformed identifiers; and in src/app/golf/actions/crm-foundations.ts lines 85-587, validate suppression, note, task, segment, ID, and patch inputs.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.coderabbit/semgrep/helmv3.yml:
- Around line 108-119: The getAuthedClient() exemption in the Semgrep rule is
too broad because it allows Supabase access before authentication within the
same exported function. Constrain the exception so getAuthedClient() must occur
before any Supabase .from() or .rpc() operation, and add an ordering fixture
proving pre-auth reads/writes remain flagged while post-auth operations are
exempt.
In `@src/app/api/calendar/coach/`[token]/route.ts:
- Around line 175-185: Stop logging credential-bearing request URLs in the
calendar token routes: in src/app/api/calendar/coach/[token]/route.ts lines
175-185, update the logServerException call to use the static route template or
pathname without the token; in src/app/api/calendar/feeds/[token]/route.ts lines
293-302 and 326-336, remove the URL from both query-failure and exception
logging.
In `@src/app/api/cron/admin-digest/route.ts`:
- Around line 88-97: Update the admin-digest route handler around the result
failure check so that when !result.sent && !result.skipped, it returns a non-2xx
response or throws after logging the failure. Preserve the existing successful
response for sent or intentionally skipped results, and ensure scheduler/job-run
handling observes the failure status instead of ok: true.
In `@src/app/api/cron/v3/genome-backfill/route.ts`:
- Around line 88-97: Update the total-failure condition in the genome backfill
route to track player-level failures separately from orchestrator errors and
computed results. In the player-processing loop, count each player that fails,
then have the response return 500 only when that failed-player count equals
playerIds.length; preserve the existing 200 behavior for successful and
null_only results and keep errors available for the summary body.
In `@src/app/api/cron/v3/ingest-sync/route.ts`:
- Around line 41-46: Update POST and the recordJobRun callback in the
ingest-sync route to return an HTTP 500 response when the sync completes with no
successful connections and summary.errors is nonzero; preserve the existing
successful response when at least one connection succeeds, so recordJobRun
captures the actual outcome.
In `@src/app/api/cron/v3/standing-backfill/route.ts`:
- Around line 118-133: Update both fatal chunk-error response paths in the
standing backfill route to always return the existing partial BackfillSummary
with HTTP status 500, regardless of chunksProcessed. Preserve the summary fields
and error message, but remove the conditional status that returns 200 after
earlier chunks succeed.
In `@src/app/api/internal/log-server-error/route.ts`:
- Around line 58-61: Update the authentication guard in the log-server-error
route to return an empty 204 response when INTERNAL_LOG_KEY is unset, preserving
the 401 response for configured keys that do not match the x-internal-log-key
header.
In `@src/app/baseball/actions/auth.ts`:
- Around line 126-131: Update the failed-login security event calls near the
shown locations to pass baseball as the top-level sport argument expected by
logSecurityEvent, matching the existing logLogin and logSignup pattern. Apply
the same change to both occurrences and remove sport from the metadata object if
the helper now receives it separately.
In `@src/app/golf/actions/admin-data.ts`:
- Around line 1781-1785: Update the RPC handling in getAdminDashboardData so it
checks the returned response error (res.error) before interpreting empty data as
a valid null result. Route that error through the existing logServerError path
with the same admin-data context, while preserving the current catch handling
for thrown exceptions.
In `@src/app/golf/actions/crm-assignee.ts`:
- Around line 31-45: Update the error branch in setCoachAssignee so it throws
the server action’s established typed error after logServerError completes,
instead of returning { ok: false }. Preserve the existing error details and
logging metadata, and use the project’s standard typed-error mechanism so
callers can distinguish persistence failures from authorization and validation
errors.
In `@src/app/golf/actions/crm-dedup.ts`:
- Around line 118-145: Update the school/name grouping in the bySchoolName
construction and consumption to use an unambiguous structured key, preserving
the original normalized school and name values separately for output. Replace
the key.split(' ') decoding in the grouping loop with the structured
representation, ensuring schools or names containing spaces remain distinct and
produce the correct matchKey.
In `@src/app/golf/actions/crm-gmail-send.ts`:
- Around line 363-379: Sanitize CRM telemetry to avoid persisting recipient or
message content: in src/app/golf/actions/crm-gmail-send.ts:363-379, replace
failed-send samples with aggregate counts and sanitized reason categories; in
src/app/golf/actions/crm-gmail-send.ts:237-243 and
src/app/golf/actions/crm-manual-send.ts:97-106, remove or redact coachId and
subject while preserving the existing send flows.
In `@src/app/golf/actions/crm-manual-send.ts`:
- Around line 97-106: Update the logManualGmailTouch error-reporting call to
remove coachId and subject from the telemetry metadata. Replace them with
non-personal operation identifiers and sanitized failure-category information
while preserving the existing action, source, sport, and featureArea fields.
In `@src/app/golf/actions/crm-replies.ts`:
- Around line 90-124: All listed action catches start logServerException without
awaiting it before rethrowing. In src/app/golf/actions/crm-replies.ts ranges
90-124 and 133-157, src/app/golf/actions/crm-sequences.ts ranges 108-171,
189-214, 223-311, 327-391, 424-622, 640-740, and 807-997, and
src/app/golf/actions/crm-templates.ts ranges 116-150, 169-228, 251-328, 333-354,
364-419, 434-481, and 501-610, update each catch to await
logServerException(error, context) while swallowing logger failures, then
rethrow the original error unchanged.
In `@src/app/golf/actions/crm-sequences.ts`:
- Around line 570-582: Replace the nested logged-action calls with private
implementation functions to ensure each failure is logged only once: in
src/app/golf/actions/crm-sequences.ts:570-582, have the wrapper call a private
enrollment implementation instead of enrollCoachesInSequence; in
src/app/golf/actions/crm-templates.ts:204-227 and :278-327, have both paths
reuse the same private default-promotion implementation. Preserve the existing
wrapper logging and behavior.
In `@src/app/golf/actions/crm-templates.ts`:
- Around line 563-574: Update the email-sending flow around the fetch in the
relevant server action to avoid serializing or forwarding the complete cookie
header. Replace the self-fetch with the shared mail service or canonical
authenticated helper, and remove the direct cookies() usage from this action
while preserving the existing send behavior and authorization requirements.
In `@src/app/golf/actions/resend-activity.ts`:
- Around line 267-280: Replace the inline error stringification in the
emailRes.error, eventsRes.error, and catch block within the email-detail flow
with the existing describeError helper. Preserve the current log messages and
action metadata while passing each failure through describeError so Supabase
error fields remain searchable.
In `@src/app/golf/admin/crm/components/QuickActionsPanel.tsx`:
- Line 170: Update the toast.error calls in QuickActionsPanel and the CRM page
error handlers to pass the fallback or caught error message inside a
second-argument options object using the description property. Apply this to
src/app/golf/admin/crm/components/QuickActionsPanel.tsx lines 170-170, and
src/app/golf/admin/crm/page.tsx lines 457-457, 764-764, 802-802, and 1014-1014,
preserving each existing message and fallback.
In `@src/components/auth/baseball-sign-in-form.tsx`:
- Around line 92-97: Update the catch blocks in BaseballSignInForm at
src/components/auth/baseball-sign-in-form.tsx:92-97 and GolfSignInForm at
src/components/auth/golf-sign-in-form.tsx:139-144 to attribute errors as action:
'sign-in-flow' instead of 'loginAction', covering the full post-authentication
flow.
In `@src/components/golf/calendar/EventDetailModal.tsx`:
- Around line 562-571: Update the conflict-check handling around the visible
success branch and catch block in EventDetailModal so every unsuccessful check
clears the existing conflicts state, including results with success false and
thrown errors. Preserve setting result.data for successful checks, and retain
the existing logError call while ensuring the catch path also resets conflicts.
In `@src/instrumentation.ts`:
- Around line 279-295: Update the edge-runtime logging branch in the
instrumentation error handler to await the fetch POST, or schedule it through
the framework’s after hook, instead of swallowing it as fire-and-forget.
Preserve the existing payload, headers, and error handling while ensuring the
write is given time to complete before the request or runtime exits.
- Around line 330-345: Remove the explicit Sentry.captureException calls from
both handlers in registerProcessErrorHandlers to avoid duplicate reporting by
Sentry’s default process integrations. Preserve the existing error normalization
and logProcessErrorToBridge forwarding for unhandledRejection and
uncaughtException, along with the current fatality behavior.
In `@src/lib/admin/job-log.ts`:
- Around line 31-39: Update the failure telemetry handling around logServerEvent
to await its completion before returning, rather than discarding the promise
with void. Keep the existing try/catch so telemetry errors are swallowed and the
original job failure remains unaffected.
In `@src/lib/baseball/coachhelm/outcome-sweep.ts`:
- Around line 185-189: Add failed-update counts to the OutcomeSweepStats type
and populate that field from the existing failed counter in
recordActionOutcomes. Update the result handling around the failure roll-up
logging so callers can distinguish successful, partial, and fully failed sweeps,
including the all-updates-failed case, while preserving the existing
failureSamples reporting.
In `@src/lib/baseball/daily-contract/missed-sweep.ts`:
- Around line 247-268: Handle non-throwing writer failures in both aggregation
paths: in src/lib/baseball/daily-contract/missed-sweep.ts lines 247-268, update
the timeline write handling to increment timelineFailed and add a failure sample
when res.ok is false; in src/lib/baseball/tasks/reminder-sweep.ts lines 251-301,
apply the equivalent logic using deliveryFailed and its failure samples.
Preserve the existing exception handling and sample limit.
In `@src/lib/error-logging.ts`:
- Around line 305-315: Update sendToMonitoringService to guard JSON.stringify
failures caused by circular or otherwise unserializable context, preserving its
never-throw contract. Catch serialization errors and fall back to a minimal
monitoring payload containing only safely serializable core error information.
In `@src/lib/supabase/middleware.ts`:
- Around line 310-325: Thread NextFetchEvent through updateSession and both
middleware call sites, including middleware.ts and src/proxy.ts. In the
auth-failure telemetry block at src/lib/supabase/middleware.ts lines 310-325 and
the idle-timeout sign-out logging block at lines 553-567, wrap each fetch with
event.waitUntil(...) so the requests remain attached to the middleware lifetime.
In `@src/lib/telemetry-gate.ts`:
- Around line 22-30: Update the telemetry gate ordering so the CI/GITHUB_ACTIONS
and NEXT_PHASE guards execute first and unconditionally, before evaluating
ADMIN_EVENTS_FORCE_CAPTURE or ADMIN_EVENTS_CAPTURE_PREVIEW. Preserve the
existing opt-in behavior only after both hard guards reject test and build
environments.
In `@src/stores/auth-store.ts`:
- Around line 49-79: Update setUser, setCoach, setPlayer, and clear in the auth
store to remove stale Sentry context whenever values are absent or
authentication is cleared. Explicitly unset sport, user_role, org_id, and
org_name with undefined when appropriate, including setCoach(null),
setPlayer(null), and coaches without an organization ID, while preserving
current tags for valid values.
---
Outside diff comments:
In `@src/app/golf/`(dashboard)/dashboard/calendar/page.tsx:
- Around line 73-78: Await logServerException in the calendar page catch block
after normalizing the caught value into one marked reportedError, then throw
that same marked Error instead of creating an unmarked replacement. In
src/app/golf/(dashboard)/dashboard/calendar/page.tsx lines 73-78, apply both
changes; in src/app/golf/(dashboard)/dashboard/rounds/page.tsx lines 50-51 and
100-101, await the warning telemetry before continuing with teamId = null or the
empty member result.
In `@src/app/golf/actions/crm-assignee.ts`:
- Around line 17-29: Validate every server-action input with zod before
constructing Supabase queries. In src/app/golf/actions/crm-assignee.ts lines
17-29, update setCoachAssignee to validate coach_id and assignee. In
src/app/golf/actions/crm-automations.ts lines 144-254, validate all create,
update, delete payloads and IDs; in src/app/golf/actions/crm-dedup.ts lines
174-209, validate both coach IDs and reject malformed identifiers; and in
src/app/golf/actions/crm-foundations.ts lines 85-587, validate suppression,
note, task, segment, ID, and patch inputs.
In `@src/components/golf/calendar/EventDocumentsSection.tsx`:
- Around line 89-124: Update the document-loading effect and the attach/detach
handlers around getEventDocuments, attachDocumentToEvent, and
detachDocumentFromEvent to catch rejected operations, log errors consistently,
and preserve user-facing failure handling. Guard effect state updates with
cancelled, and move setLoading(false) and setPendingAction(null) into finally
blocks so loading and pending controls always clear, including when server
actions reject.
In `@src/lib/server-error-logger.ts`:
- Around line 417-431: Update logServerException to call markBridgeLogged on the
original Error before awaiting captureServerTrace, ensuring fire-and-forget
callers are marked immediately; retain the existing guard so non-Error inputs
are not marked and preserve the telemetry call behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5f929193-b527-461a-ab44-8cabc3b08ed4
📒 Files selected for processing (79)
.coderabbit/semgrep/helmv3.yml.env.examplesrc/app/admin/errors/page.tsxsrc/app/api/baseball/staff/context/route.tssrc/app/api/calendar/coach/[token]/route.tssrc/app/api/calendar/feeds/[token]/route.tssrc/app/api/crm/google-calendar/sync/route.tssrc/app/api/cron/admin-digest/route.tssrc/app/api/cron/process-sequences/route.tssrc/app/api/cron/v3/genome-backfill/route.tssrc/app/api/cron/v3/genome-nightly/route.tssrc/app/api/cron/v3/ingest-sync/route.tssrc/app/api/cron/v3/standing-backfill/route.tssrc/app/api/cron/v3/weekly-coach-email/route.tssrc/app/api/golf/players/[playerId]/putt-tendencies/route.tssrc/app/api/internal/log-server-error/route.tssrc/app/api/log-error/route.tssrc/app/baseball/(dashboard)/dashboard/roster/RosterClient.tsxsrc/app/baseball/actions/auth.tssrc/app/baseball/actions/coachhelm-actions.tssrc/app/baseball/actions/postgame.tssrc/app/baseball/error.tsxsrc/app/error.tsxsrc/app/golf/(dashboard)/dashboard/calendar/page.tsxsrc/app/golf/(dashboard)/dashboard/rounds/page.tsxsrc/app/golf/actions/__tests__/coverage-contract.observability.test.tssrc/app/golf/actions/admin-data.tssrc/app/golf/actions/auth.tssrc/app/golf/actions/crm-assignee.tssrc/app/golf/actions/crm-automations.tssrc/app/golf/actions/crm-dedup.tssrc/app/golf/actions/crm-foundations.tssrc/app/golf/actions/crm-gmail-send.tssrc/app/golf/actions/crm-manual-send.tssrc/app/golf/actions/crm-replies.tssrc/app/golf/actions/crm-sequences.tssrc/app/golf/actions/crm-templates.tssrc/app/golf/actions/resend-activity.tssrc/app/golf/admin/crm/components/QuickActionsPanel.tsxsrc/app/golf/admin/crm/components/ScheduleEventModal.tsxsrc/app/golf/admin/crm/page.tsxsrc/components/auth/baseball-sign-in-form.tsxsrc/components/auth/golf-sign-in-form.tsxsrc/components/baseball/performance/PlayerLiftToday.tsxsrc/components/baseball/postgame/PostgameReviewClient.tsxsrc/components/baseball/staff-decision-room/StaffDecisionRoomClient.tsxsrc/components/baseball/staff-decision-room/StaffDecisionRoomFairway.tsxsrc/components/coach/discover/DiscoverView.tsxsrc/components/fairway/pages/messages/FairwayMessages.tsxsrc/components/fairway/pages/settings/FairwaySettingsGeneral.tsxsrc/components/features/video-upload.tsxsrc/components/golf/calendar/AttendancePanel.tsxsrc/components/golf/calendar/EventDetailModal.tsxsrc/components/golf/calendar/EventDocumentsSection.tsxsrc/components/golf/calendar/PremiumCalendarClient.tsxsrc/components/lifting/groups/StrengthGroupsClient.tsxsrc/components/lifting/nutrition/NutritionPlanUploader.tsxsrc/components/lifting/sessions/LiveWeightRoomClient.tsxsrc/components/ui/avatar-upload.tsxsrc/hooks/golf/use-golf-messages.tssrc/hooks/golf/use-message-attachments.tssrc/hooks/use-messages.tssrc/hooks/use-watchlist.tssrc/instrumentation-client.tssrc/instrumentation.tssrc/lib/admin-logger-client.tssrc/lib/admin/job-log.tssrc/lib/admin/rls-denial.tssrc/lib/api/with-route-handler.tssrc/lib/baseball/coachhelm/outcome-sweep.tssrc/lib/baseball/daily-contract/missed-sweep.tssrc/lib/baseball/tasks/reminder-sweep.tssrc/lib/bridge-logged-marker.tssrc/lib/error-logging.tssrc/lib/inngest/functions.tssrc/lib/server-error-logger.tssrc/lib/supabase/middleware.tssrc/lib/telemetry-gate.tssrc/stores/auth-store.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (29)
.coderabbit/semgrep/helmv3.yml (1)
108-119: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -eu rule_file="$(fd -a '^helmv3\.yml$' .coderabbit | head -n1)" printf '%s\n' 'Reviewing rule:' sed -n '60,125p' "$rule_file" printf '%s\n' 'Locating existing Semgrep fixtures/tests:' fd -i 'semgrep|helmv3' . | sort rg -n -C3 'getAuthedClient|helmv3-server-action-missing-auth-check' .Repository: njrini99-code/helmv3
Length of output: 41338
🏁 Script executed:
#!/bin/bash set -eu rule_file=".coderabbit/semgrep/helmv3.yml" printf '%s\n' 'Rule excerpt:' sed -n '100,125p' "$rule_file" printf '\n%s\n' 'Search for semgrep fixtures or rule tests tied to helmv3-server-action-missing-auth-check:' rg -n -C 2 'helmv3-server-action-missing-auth-check|getAuthedClient|pattern-not-inside' .coderabbit docs src --glob '!**/node_modules/**' || true printf '\n%s\n' 'Show all current nosemgrep suppressions for this rule in golf actions:' rg -n 'nosemgrep: helmv3-server-action-missing-auth-check' src/app/golf/actions src/app/baseball/actions || trueRepository: njrini99-code/helmv3
Length of output: 34661
🏁 Script executed:
#!/bin/bash set -eu cat <<'PY' >/tmp/semgrep_semantics_check.py # Read-only reasoning aid: demonstrates the exact structural concern. # This script doesn't run semgrep; it just prints the logical shape that the rule matches. unsafe = """export async function unsafe() { await supabase.from('golf_players').select(); await getAuthedClient(); }""" safe = """export async function safe() { const { supabase } = await getAuthedClient(); await supabase.from('golf_players').select(); }""" print("unsafe shape contains getAuthedClient anywhere before/after DB call: yes") print("safe shape contains getAuthedClient anywhere before/after DB call: yes") print("Therefore a blanket pattern-not-inside keyed only on getAuthedClient would exclude both.") PY python3 /tmp/semgrep_semantics_check.pyRepository: njrini99-code/helmv3
Length of output: 391
🏁 Script executed:
#!/bin/bash set -eu python3 - <<'PY' # Quick structural check using a tiny text simulation of the described rule shape. # If a function body contains the token `getAuthedClient`, a negative "inside" # constraint on that token suppresses the whole function regardless of position. cases = { "db_before_auth": "await supabase.from('x'); await getAuthedClient();", "auth_before_db": "await getAuthedClient(); await supabase.from('x');", } for name, body in cases.items(): suppressed = "getAuthedClient" in body print(f"{name}: suppressed={suppressed}") PYRepository: njrini99-code/helmv3
Length of output: 222
Constrain the
getAuthedClient()exemption.coderabbit/semgrep/helmv3.yml:114-119—pattern-not-insidesuppresses the rule whenevergetAuthedClient(...)appears anywhere in an exported action, so a.from()/.rpc()call before auth can slip through. Add an ordering fixture and requiregetAuthedClient()to precede any Supabase write/read in the exception, or split it into a narrower helper-specific rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit/semgrep/helmv3.yml around lines 108 - 119, The getAuthedClient() exemption in the Semgrep rule is too broad because it allows Supabase access before authentication within the same exported function. Constrain the exception so getAuthedClient() must occur before any Supabase .from() or .rpc() operation, and add an ordering fixture proving pre-auth reads/writes remain flagged while post-auth operations are exempt.Sources: Coding guidelines, Path instructions
src/app/api/calendar/coach/[token]/route.ts (1)
175-185: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not log calendar feed credentials.
These routes authenticate with a token in the path, so
request.urlsends the live bearer credential to Bridge/Sentry. Log only the pathname or the route template.
src/app/api/calendar/coach/[token]/route.ts#L175-L185: replacerequest.urlwithnew URL(request.url).pathname—preferably the static route template to avoid retaining the token.src/app/api/calendar/feeds/[token]/route.ts#L293-L302: remove the credential-bearing URL from the query-failure log.src/app/api/calendar/feeds/[token]/route.ts#L326-L336: remove it from exception logging as well.- url: request.url, + url: '/api/calendar/coach/[token]',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.} catch (error) { await logServerException(error, { action: 'calendarCoachFeedApi.get', route: '/api/calendar/coach/[token]', url: '/api/calendar/coach/[token]', source: 'route_handler', sport: 'golf', featureArea: 'calendar', handled: false, statusCode: 500, });📍 Affects 2 files
src/app/api/calendar/coach/[token]/route.ts#L175-L185(this comment)src/app/api/calendar/feeds/[token]/route.ts#L293-L302src/app/api/calendar/feeds/[token]/route.ts#L326-L336🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/calendar/coach/`[token]/route.ts around lines 175 - 185, Stop logging credential-bearing request URLs in the calendar token routes: in src/app/api/calendar/coach/[token]/route.ts lines 175-185, update the logServerException call to use the static route template or pathname without the token; in src/app/api/calendar/feeds/[token]/route.ts lines 293-302 and 326-336, remove the URL from both query-failure and exception logging.src/app/api/cron/admin-digest/route.ts (1)
88-97: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return a failure status when the digest was not sent.
src/app/api/cron/admin-digest/route.ts:88-97logs the failure but still resolves withok: true, sorecordJobRunand the scheduler treat a failed delivery as successful. Throw or return a non-2xx response for!result.sent && !result.skipped.Proposed fix
if (!result.sent && !result.skipped) { await logServerError( `admin-digest send failed: ${result.reason ?? 'unknown'}`, { action: 'cron.admin-digest', source: 'cron' }, 'error', ); + return NextResponse.json( + { ok: false, ...result, reds: reds.length }, + { status: 500 }, + ); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!result.sent && !result.skipped) { // A real send failure (not "ops transport unconfigured" — that's // skipped=true and expected in dev/preview). await logServerError( `admin-digest send failed: ${result.reason ?? 'unknown'}`, { action: 'cron.admin-digest', source: 'cron' }, 'error', ); return NextResponse.json( { ok: false, ...result, reds: reds.length }, { status: 500 }, ); } return NextResponse.json({ ok: true, ...result, reds: reds.length });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/cron/admin-digest/route.ts` around lines 88 - 97, Update the admin-digest route handler around the result failure check so that when !result.sent && !result.skipped, it returns a non-2xx response or throws after logging the failure. Preserve the existing successful response for sent or intentionally skipped results, and ensure scheduler/job-run handling observes the failure status instead of ok: true.src/app/api/cron/v3/genome-backfill/route.ts (1)
88-97: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
src/app/api/cron/v3/genome-backfill/route.ts:91does not prove every player failed.
errorscounts orchestrator errors, whilecomputedexcludes successfulnull_onlyplayers. One errored player plus several valid null-only results therefore returns 500 and triggers failed-run telemetry. Track failed players separately and require that count to equalplayerIds.length.Proposed fix
let computed = 0; let nullOnly = 0; let errors = 0; +let playersWithErrors = 0; for (const pid of playerIds) { try { const r = await computeGenomeForPlayer(pid); - if (r.errors > 0) errors += r.errors; + if (r.errors > 0) { + errors += r.errors; + playersWithErrors += 1; + } if (r.dimensions_computed > 0) computed += 1; else nullOnly += 1; } catch (err) { errors += 1; + playersWithErrors += 1; // ... } } -if (playerIds.length > 0 && errors > 0 && computed === 0) { +if (playerIds.length > 0 && playersWithErrors === playerIds.length && computed === 0) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let computed = 0; let nullOnly = 0; let errors = 0; let playersWithErrors = 0; for (const pid of playerIds) { try { const r = await computeGenomeForPlayer(pid); if (r.errors > 0) { errors += r.errors; playersWithErrors += 1; } if (r.dimensions_computed > 0) computed += 1; else nullOnly += 1; } catch (err) { errors += 1; playersWithErrors += 1; // ... } } // Total failure: every player errored and nothing computed. A partial // failure (some computed, some errored) still returns 200 — the summary // body carries the error count for the caller to inspect. if (playerIds.length > 0 && playersWithErrors === playerIds.length && computed === 0) { // Not logged here: this route is wrapped in recordJobRun (job-log.ts), // which already writes a "Cron failed" Bridge event for any >=400 // response — logging again here would double-write error_logs/ // admin_events/Sentry for the same failure. Per-player exceptions are // still logged individually above, in the loop. return NextResponse.json(body, { status: 500 });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/cron/v3/genome-backfill/route.ts` around lines 88 - 97, Update the total-failure condition in the genome backfill route to track player-level failures separately from orchestrator errors and computed results. In the player-processing loop, count each player that fails, then have the response return 500 only when that failed-player count equals playerIds.length; preserve the existing 200 behavior for successful and null_only results and keep errors available for the summary body.src/app/api/cron/v3/ingest-sync/route.ts (1)
41-46: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
src/app/api/cron/v3/ingest-sync/route.ts:41records total sync failure as completed.
recordJobRunsees the final HTTP 200 even when every connection throws andsummary.errorsis nonzero. Return 500 when no connection succeeds so job telemetry and retries reflect the actual outcome.Proposed fix
summary.duration_ms = Date.now() - startedAt; -return NextResponse.json(summary); +const status = + rows.length > 0 && summary.synced === 0 && summary.errors > 0 + ? 500 + : 200; +return NextResponse.json(summary, { status });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/cron/v3/ingest-sync/route.ts` around lines 41 - 46, Update POST and the recordJobRun callback in the ingest-sync route to return an HTTP 500 response when the sync completes with no successful connections and summary.errors is nonzero; preserve the existing successful response when at least one connection succeeds, so recordJobRun captures the actual outcome.src/app/api/cron/v3/standing-backfill/route.ts (1)
118-133: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep the partial summary, but return a failure status.
src/app/api/cron/v3/standing-backfill/route.ts:118-133and174-188return HTTP 200 whenever an earlier chunk succeeded. The cron invocation still terminated prematurely, so monitoring will mark an incomplete backfill successful and may not retry it. Return the summary with status 500 for every fatal chunk error.- chunksProcessed === 0 ? { status: 500 } : undefined, + { status: 500 },Also applies to: 174-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/cron/v3/standing-backfill/route.ts` around lines 118 - 133, Update both fatal chunk-error response paths in the standing backfill route to always return the existing partial BackfillSummary with HTTP status 500, regardless of chunksProcessed. Preserve the summary fields and error message, but remove the conditional status that returns 200 after earlier chunks succeed.src/app/api/internal/log-server-error/route.ts (1)
58-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return 204 when
INTERNAL_LOG_KEYis unconfigured —src/app/api/internal/log-server-error/route.ts:58-61.The current condition returns 401 when the key is unset, contradicting the documented silent no-op contract.
Proposed fix
const expected = process.env.INTERNAL_LOG_KEY; -if (!expected || request.headers.get('x-internal-log-key') !== expected) { +if (!expected) { + return new NextResponse(null, { status: 204 }); +} +if (request.headers.get('x-internal-log-key') !== expected) { return NextResponse.json({ ok: false }, { status: 401 }); }As per path instructions, “when unset, the related routes are documented to silently no-op rather than returning errors.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const expected = process.env.INTERNAL_LOG_KEY; if (!expected) { return new NextResponse(null, { status: 204 }); } if (request.headers.get('x-internal-log-key') !== expected) { return NextResponse.json({ ok: false }, { status: 401 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/internal/log-server-error/route.ts` around lines 58 - 61, Update the authentication guard in the log-server-error route to return an empty 204 response when INTERNAL_LOG_KEY is unset, preserving the 401 response for configured keys that do not match the x-internal-log-key header.Source: Path instructions
src/app/baseball/actions/auth.ts (1)
126-131: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate
sportto the security event’s top-level field.
src/app/baseball/actions/auth.ts— Lines 126 and 465 passsportonly in metadata, butlogSecurityEventdoes not forward it tologAdminEvent. These events therefore cannot be reliably filtered as Baseball events. Extract and passsport, aslogLoginandlogSignupalready do.Proposed helper fix
export async function logSecurityEvent( title: string, severity: AdminEventSeverity, metadata?: Record<string, unknown>, userId?: string, ): Promise<string | null> { + const sport = metadata?.sport as 'golf' | 'baseball' | 'shared' | undefined; return logAdminEvent({ eventType: 'security', title, severity, metadata, userId, source: 'auth', + sport, }); }Also applies to: 465-467
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/auth.ts` around lines 126 - 131, Update the failed-login security event calls near the shown locations to pass baseball as the top-level sport argument expected by logSecurityEvent, matching the existing logLogin and logSignup pattern. Apply the same change to both occurrences and remove sport from the metadata object if the helper now receives it separately.src/app/golf/actions/admin-data.ts (1)
1781-1785: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle the RPC’s returned error, not only thrown exceptions.
src/app/golf/actions/admin-data.ts— Line 1781 never catches a normal Supabase RPC failure because the call resolves withres.error. Check it before treating empty data as a legitimatenullresult.Proposed fix
const res = await rpc('get_platform_health_stats'); + if (res.error) { + void logServerError( + `[admin-data] get_platform_health_stats errored: ${describeError(res.error)}`, + { action: 'admin_data.getAdminDashboardData', featureArea: 'admin' }, + ); + return null; + } const rows = res.data ?? [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const res = await rpc('get_platform_health_stats'); if (res.error) { void logServerError( `[admin-data] get_platform_health_stats errored: ${describeError(res.error)}`, { action: 'admin_data.getAdminDashboardData', featureArea: 'admin' }, ); return null; } const rows = res.data ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/admin-data.ts` around lines 1781 - 1785, Update the RPC handling in getAdminDashboardData so it checks the returned response error (res.error) before interpreting empty data as a valid null result. Route that error through the existing logServerError path with the same admin-data context, while preserving the current catch handling for thrown exceptions.src/app/golf/actions/crm-assignee.ts (1)
31-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not collapse the database failure into
{ ok: false }.
src/app/golf/actions/crm-assignee.ts:31-45prevents callers from distinguishing authorization, validation, and persistence failures. Throw a typed error after logging, consistent with the server-action contract.As per path instructions, “Throw on error, never silently return
{ success: false }without a typed reason.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-assignee.ts` around lines 31 - 45, Update the error branch in setCoachAssignee so it throws the server action’s established typed error after logServerError completes, instead of returning { ok: false }. Preserve the existing error details and logging metadata, and use the project’s standard typed-error mechanism so callers can distinguish persistence failures from authorization and validation errors.Source: Path instructions
src/app/golf/actions/crm-dedup.ts (1)
118-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore an unambiguous school/name grouping key.
src/app/golf/actions/crm-dedup.ts:121-145cannot safely decode a space-delimited key."North Carolina Jane Doe"becomesschool="north"andname="carolina", and distinct school/name pairs can collide into one duplicate group. Store the values separately or use a structured key.Proposed fix
- const key = `${school} ${name}`; + const key = JSON.stringify([school, name]); ... - const [school, name] = key.split(' '); + const [school, name] = JSON.parse(key) as [string, string];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const school = norm(row.school); const name = norm(row.name); if (school && name) { const key = JSON.stringify([school, name]); const list = bySchoolName.get(key) ?? []; list.push(row); bySchoolName.set(key, list); } } const groups: DuplicateGroup[] = []; // Track which coach ids already landed in an email group so we don't emit a // redundant school+name group that's a strict subset of an email match. const emittedIds = new Set<string>(); for (const [email, coaches] of byEmail) { if (coaches.length < 2) continue; coaches.forEach((c) => emittedIds.add(c.id)); groups.push({ matchKey: email, matchType: 'email', coaches }); } for (const [key, coaches] of bySchoolName) { if (coaches.length < 2) continue; // Skip if every member already appears in an email group above. if (coaches.every((c) => emittedIds.has(c.id))) continue; const [school, name] = JSON.parse(key) as [string, string]; groups.push({ matchKey: `${name} @ ${school}`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-dedup.ts` around lines 118 - 145, Update the school/name grouping in the bySchoolName construction and consumption to use an unambiguous structured key, preserving the original normalized school and name values separately for output. Replace the key.split(' ') decoding in the grouping loop with the structured representation, ensuring schools or names containing spaces remain distinct and produce the correct matchKey.src/app/golf/actions/crm-gmail-send.ts (1)
363-379: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
CRM telemetry now persists recipient-identifying and message content. Replace these values with aggregate counts and sanitized reason categories.
src/app/golf/actions/crm-gmail-send.ts#L363-L379: remove names, schools, and raw provider reasons fromsamples.src/app/golf/actions/crm-gmail-send.ts#L237-L243: remove or redactcoachIdandsubject.src/app/golf/actions/crm-manual-send.ts#L97-L106: remove or redactcoachIdandsubject.📍 Affects 2 files
src/app/golf/actions/crm-gmail-send.ts#L363-L379(this comment)src/app/golf/actions/crm-gmail-send.ts#L237-L243src/app/golf/actions/crm-manual-send.ts#L97-L106🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-gmail-send.ts` around lines 363 - 379, Sanitize CRM telemetry to avoid persisting recipient or message content: in src/app/golf/actions/crm-gmail-send.ts:363-379, replace failed-send samples with aggregate counts and sanitized reason categories; in src/app/golf/actions/crm-gmail-send.ts:237-243 and src/app/golf/actions/crm-manual-send.ts:97-106, remove or redact coachId and subject while preserving the existing send flows.src/app/golf/actions/crm-manual-send.ts (1)
97-106: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
src/app/golf/actions/crm-manual-send.ts:104persists CRM identifiers and subject content.Do not include
coachIdorsubjectin centralized telemetry. Use non-personal operation identifiers and sanitized failure categories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-manual-send.ts` around lines 97 - 106, Update the logManualGmailTouch error-reporting call to remove coachId and subject from the telemetry metadata. Replace them with non-personal operation identifiers and sanitized failure-category information while preserving the existing action, source, sport, and featureArea fields.src/app/golf/actions/crm-replies.ts (1)
90-124: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Await exception logging before rethrowing the original error.
These catches start
logServerExceptionand immediately throw. The helper marks the original error only after capture completes, so the framework can observe it first and create a duplicate event; pending Bridge writes may also be dropped.
src/app/golf/actions/crm-replies.ts#L90-L124: await logging with a swallowed logger failure before rethrowing.src/app/golf/actions/crm-replies.ts#L133-L157: await logging with a swallowed logger failure before rethrowing.src/app/golf/actions/crm-sequences.ts#L108-L171: update both catches.src/app/golf/actions/crm-sequences.ts#L189-L214: update the catch.src/app/golf/actions/crm-sequences.ts#L223-L311: update all mutation catches.src/app/golf/actions/crm-sequences.ts#L327-L391: update both step catches.src/app/golf/actions/crm-sequences.ts#L424-L622: update all enrollment catches.src/app/golf/actions/crm-sequences.ts#L640-L740: update all enrollment-state catches.src/app/golf/actions/crm-sequences.ts#L807-L997: update the performance catch.src/app/golf/actions/crm-templates.ts#L116-L150: update the catch.src/app/golf/actions/crm-templates.ts#L169-L228: update the catch.src/app/golf/actions/crm-templates.ts#L251-L328: update the catch.src/app/golf/actions/crm-templates.ts#L333-L354: update the catch.src/app/golf/actions/crm-templates.ts#L364-L419: update the catch.src/app/golf/actions/crm-templates.ts#L434-L481: update the catch.src/app/golf/actions/crm-templates.ts#L501-L610: update the catch.Use:
await logServerException(error, context).catch(() => {}); throw error;📍 Affects 3 files
src/app/golf/actions/crm-replies.ts#L90-L124(this comment)src/app/golf/actions/crm-replies.ts#L133-L157src/app/golf/actions/crm-sequences.ts#L108-L171src/app/golf/actions/crm-sequences.ts#L189-L214src/app/golf/actions/crm-sequences.ts#L223-L311src/app/golf/actions/crm-sequences.ts#L327-L391src/app/golf/actions/crm-sequences.ts#L424-L622src/app/golf/actions/crm-sequences.ts#L640-L740src/app/golf/actions/crm-sequences.ts#L807-L997src/app/golf/actions/crm-templates.ts#L116-L150src/app/golf/actions/crm-templates.ts#L169-L228src/app/golf/actions/crm-templates.ts#L251-L328src/app/golf/actions/crm-templates.ts#L333-L354src/app/golf/actions/crm-templates.ts#L364-L419src/app/golf/actions/crm-templates.ts#L434-L481src/app/golf/actions/crm-templates.ts#L501-L610🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-replies.ts` around lines 90 - 124, All listed action catches start logServerException without awaiting it before rethrowing. In src/app/golf/actions/crm-replies.ts ranges 90-124 and 133-157, src/app/golf/actions/crm-sequences.ts ranges 108-171, 189-214, 223-311, 327-391, 424-622, 640-740, and 807-997, and src/app/golf/actions/crm-templates.ts ranges 116-150, 169-228, 251-328, 333-354, 364-419, 434-481, and 501-610, update each catch to await logServerException(error, context) while swallowing logger failures, then rethrow the original error unchanged.src/app/golf/actions/crm-sequences.ts (1)
570-582: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid calling one logged server action from another logged wrapper.
Inner failures are logged and rethrown, then logged again by the outer catch.
src/app/golf/actions/crm-sequences.ts#L570-L582: call a private enrollment implementation instead ofenrollCoachesInSequence.src/app/golf/actions/crm-templates.ts#L204-L227: call a private default-promotion implementation.src/app/golf/actions/crm-templates.ts#L278-L327: use the same private default-promotion implementation.📍 Affects 2 files
src/app/golf/actions/crm-sequences.ts#L570-L582(this comment)src/app/golf/actions/crm-templates.ts#L204-L227src/app/golf/actions/crm-templates.ts#L278-L327🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-sequences.ts` around lines 570 - 582, Replace the nested logged-action calls with private implementation functions to ensure each failure is logged only once: in src/app/golf/actions/crm-sequences.ts:570-582, have the wrapper call a private enrollment implementation instead of enrollCoachesInSequence; in src/app/golf/actions/crm-templates.ts:204-227 and :278-327, have both paths reuse the same private default-promotion implementation. Preserve the existing wrapper logging and behavior.src/app/golf/actions/crm-templates.ts (1)
563-574: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not forward the complete session cookie header through a self-fetch.
src/app/golf/actions/crm-templates.ts— Lines 567-574 serialize every caller cookie and send it to a URL derived from configuration. Invoke the shared mail service directly or use the canonical authenticated helper instead of relaying browser credentials.As per path instructions, “Flag any direct use of
cookies()/headers()outside the canonical helpers.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/crm-templates.ts` around lines 563 - 574, Update the email-sending flow around the fetch in the relevant server action to avoid serializing or forwarding the complete cookie header. Replace the self-fetch with the shared mail service or canonical authenticated helper, and remove the direct cookies() usage from this action while preserving the existing send behavior and authorization requirements.Source: Path instructions
src/app/golf/actions/resend-activity.ts (1)
267-280: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use
describeErrorso Supabase failures remain searchable.
src/app/golf/actions/resend-activity.ts— Lines 268, 271, and 279 stringify non-Errorobjects as[object Object]. Use the existingdescribeErrorhelper, which preserves Supabasecode,message,details, andhint.Proposed fix
+import { describeError } from '`@/lib/utils/describe-error`'; -`${emailRes.error instanceof Error ? emailRes.error.message : String(emailRes.error)}` +`${describeError(emailRes.error)}` -`${eventsRes.error instanceof Error ? eventsRes.error.message : String(eventsRes.error)}` +`${describeError(eventsRes.error)}` -`${err instanceof Error ? err.message : String(err)}` +`${describeError(err)}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { describeError } from '`@/lib/utils/describe-error`'; if (emailRes.error) { await logServerError(`[resend-activity] email detail query failed: ${describeError(emailRes.error)}`, { action: 'resend_activity.getEmailDetail' }); } if (eventsRes.error) { await logServerError(`[resend-activity] email detail events query failed: ${describeError(eventsRes.error)}`, { action: 'resend_activity.getEmailDetail' }); } return { email: (emailRes.data as EmailRow) ?? null, events: ((eventsRes.data ?? []) as EmailEventRow[]), }; } catch (err) { await logServerError(`[resend-activity] email detail threw: ${describeError(err)}`, { action: 'resend_activity.getEmailDetail' }); throw err;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/actions/resend-activity.ts` around lines 267 - 280, Replace the inline error stringification in the emailRes.error, eventsRes.error, and catch block within the email-detail flow with the existing describeError helper. Preserve the current log messages and action metadata while passing each failure through describeError so Supabase error fields remain searchable.src/app/golf/admin/crm/components/QuickActionsPanel.tsx (1)
170-170: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix
toast.errorsignature.Sonner's
toast.errorexpects an options object as its second argument, not a string. Passing a string directly will cause it to be ignored (or fail type checking) instead of displaying as the description. Wrap the second argument in{ description: ... }.
src/app/golf/admin/crm/components/QuickActionsPanel.tsx#L170-L170:toast.error('Failed to log contact', { description: err instanceof Error ? err.message : 'Please try again.' });src/app/golf/admin/crm/page.tsx#L457-L457:toast.error('Failed to load coaches', { description: err instanceof Error ? err.message : 'Please refresh and try again.' });src/app/golf/admin/crm/page.tsx#L764-L764:toast.error('Failed to update coach', { description: err instanceof Error ? err.message : 'Please try again.' });src/app/golf/admin/crm/page.tsx#L802-L802:toast.error('Failed to update coaches', { description: err instanceof Error ? err.message : 'Please try again.' });src/app/golf/admin/crm/page.tsx#L1014-L1014:toast.error('Bulk action failed', { description: err instanceof Error ? err.message : 'Please try again.' });📍 Affects 2 files
src/app/golf/admin/crm/components/QuickActionsPanel.tsx#L170-L170(this comment)src/app/golf/admin/crm/page.tsx#L457-L457src/app/golf/admin/crm/page.tsx#L764-L764src/app/golf/admin/crm/page.tsx#L802-L802src/app/golf/admin/crm/page.tsx#L1014-L1014🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/golf/admin/crm/components/QuickActionsPanel.tsx` at line 170, Update the toast.error calls in QuickActionsPanel and the CRM page error handlers to pass the fallback or caught error message inside a second-argument options object using the description property. Apply this to src/app/golf/admin/crm/components/QuickActionsPanel.tsx lines 170-170, and src/app/golf/admin/crm/page.tsx lines 457-457, 764-764, 802-802, and 1014-1014, preserving each existing message and fallback.src/components/auth/baseball-sign-in-form.tsx (1)
92-97: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use accurate sign-in-flow attribution instead of
loginAction.Both catch blocks cover post-authentication storage, URL processing, and navigation as well as the action call. Labeling every exception
loginActioncreates false authentication incidents.
src/components/auth/baseball-sign-in-form.tsx#L92-L97: useaction: 'sign-in-flow', or narrow this catch to theloginActioncall.src/components/auth/golf-sign-in-form.tsx#L139-L144: useaction: 'sign-in-flow', or split login and post-login navigation into separately attributed catches.📍 Affects 2 files
src/components/auth/baseball-sign-in-form.tsx#L92-L97(this comment)src/components/auth/golf-sign-in-form.tsx#L139-L144🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/auth/baseball-sign-in-form.tsx` around lines 92 - 97, Update the catch blocks in BaseballSignInForm at src/components/auth/baseball-sign-in-form.tsx:92-97 and GolfSignInForm at src/components/auth/golf-sign-in-form.tsx:139-144 to attribute errors as action: 'sign-in-flow' instead of 'loginAction', covering the full post-authentication flow.src/components/golf/calendar/EventDetailModal.tsx (1)
562-571: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear stale conflicts and log returned failures —
src/components/golf/calendar/EventDetailModal.tsx:562-571.A
{ success: false }result is ignored, and the catch path leaves the previousconflictsvalue intact. Users can therefore see obsolete scheduling guidance after the latest check failed.Proposed fix
if (result.success && result.data) { setConflicts(result.data as ConflictData); + } else { + setConflicts(null); + logError( + new Error(result.error || 'Failed to check schedule conflicts'), + { component: 'EventDetailModal', action: 'check-schedule-conflicts', sport: 'golf' }, + 'medium' + ); } } catch (err) { + setConflicts(null); logError(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (result.success && result.data) { setConflicts(result.data as ConflictData); } else { setConflicts(null); logError( new Error(result.error || 'Failed to check schedule conflicts'), { component: 'EventDetailModal', action: 'check-schedule-conflicts', sport: 'golf' }, 'medium' ); } } catch (err) { setConflicts(null); // Conflict check failed - continue without warning logError( err instanceof Error ? err : new Error(String(err)), { component: 'EventDetailModal', action: 'check-schedule-conflicts', sport: 'golf' }, 'medium' );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/golf/calendar/EventDetailModal.tsx` around lines 562 - 571, Update the conflict-check handling around the visible success branch and catch block in EventDetailModal so every unsuccessful check clears the existing conflicts state, including results with success false and thrown errors. Preserve setting result.data for successful checks, and retain the existing logError call while ensuring the catch path also resets conflicts.src/instrumentation.ts (2)
279-295: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '220,340p' src/instrumentation.tsRepository: njrini99-code/helmv3
Length of output: 4929
🌐 Web query:
Next.js onRequestError edge runtime async hook fetch await waitUntil instrumentation💡 Result:
In Next.js, the
onRequestErrorhook ininstrumentation.tsis the standard way to capture and report server-side errors [1][2][3]. Handling asynchronous tasks (likefetchcalls to an observability provider) withinonRequestErrorrequires careful attention to the runtime and the server's lifecycle: 1. Awaiting Tasks: It is highly recommended toawaitany asynchronous tasks directly withinonRequestError[1][2]. Next.js triggers this hook when the server captures an error; ensuring tasks are awaited allows the error reporting to complete before the request handling finishes [1]. 2. Edge Runtime Considerations: When using the Edge Runtime, environments are highly constrained and often terminate execution immediately after the response is sent. To perform asynchronous work that should outlive the main request (such as background logging), you should use theafter()API [4][5]. 3. Theafter()API: As of Next.js 15.1+,after()is the preferred, platform-agnostic way to register background tasks that should run after the response has been sent [4][5]. Internally,after()uses the platform'swaitUntil()mechanism to keep the serverless function alive until the provided promise settles [4][5]. 4. Integration withonRequestError: Recent improvements (e.g., PR#91523) ensure that Next.js now properly registers promises returned byonRequestErrorwithafter()in specific contexts (like Server Actions), preventing async error reporting from being discarded [6]. If you are operating in an environment whereafter()orwaitUntil()is not natively available, you must rely on standardawaitpatterns within theonRequestErrorfunction, though this will block the request response until the reporting task completes [1][2]. For custom observability, you can detect the runtime usingprocess.env.NEXT_RUNTIME === 'edge'to conditionally load different error-handling logic if necessary [1][2].Citations:
- 1: https://nextjs.org/docs/app/api-reference/file-conventions/instrumentation
- 2: https://nextjs.org/docs/15/app/api-reference/file-conventions/instrumentation
- 3: https://nextjs.org/blog/next-15
- 4: https://nextjs.org/docs/app/api-reference/functions/after
- 5: https://vercel.com/docs/functions/functions-api-reference/vercel-functions-package
- 6: vercel/next.js#91523
Await the edge bridge write.
src/instrumentation.ts:283-295
fetch(...)is fire-and-forget here, so the edge runtime can exit before the POST reaches/api/internal/log-server-error. Useawait fetch(...)inside thetry, or wrap it inafter(() => fetch(...)), so error telemetry is not dropped on edge failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/instrumentation.ts` around lines 279 - 295, Update the edge-runtime logging branch in the instrumentation error handler to await the fetch POST, or schedule it through the framework’s after hook, instead of swallowing it as fire-and-forget. Preserve the existing payload, headers, and error handling while ensuring the write is given time to complete before the request or runtime exits.
330-345: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -eu rg -n -C4 \ "defaultIntegrations|onUncaughtExceptionIntegration|onUnhandledRejectionIntegration|captureException|uncaughtException|unhandledRejection" \ src package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: njrini99-code/helmv3
Length of output: 11251
🏁 Script executed:
#!/bin/bash set -eu echo "== instrumentation.ts outline ==" ast-grep outline src/instrumentation.ts --view expanded | sed -n '1,220p' echo echo "== instrumentation.ts around Sentry init and process handlers ==" sed -n '1,220p' src/instrumentation.ts | cat -n echo sed -n '220,380p' src/instrumentation.ts | cat -nRepository: njrini99-code/helmv3
Length of output: 17920
🌐 Web query:
Sentry Node.js SDK unhandledRejection uncaughtException default integrations process.on captureException duplicate events💡 Result:
The Sentry Node.js SDK includes built-in integrations that automatically capture uncaught exceptions and unhandled promise rejections [1][2]. These are managed by the
OnUncaughtExceptionandOnUnhandledRejectionintegrations, respectively, both of which are enabled by default [1][3][4]. Regarding duplicate events and manual capturing: 1. Automatic vs. Manual Capture: Because Sentry automatically registers global handlers for these events, you do not need to manually callSentry.captureExceptionfor top-level uncaught exceptions or unhandled rejections; doing so manually may result in duplicate reports if the SDK also catches the same event [5][6]. If you choose to manage these events manually, you must disable the default integrations [1][5]. 2. Disabling Default Integrations: To stop the SDK from automatically capturing these, you can setdefaultIntegrations: falsein yourSentry.init()call or filter out specific integrations by modifying theintegrationsarray [1]. For example, to remove only these specific handlers: Sentry.init({ integrations: (integrations) => integrations.filter(i =>!['OnUncaughtException', 'OnUnhandledRejection'].includes(i.name)) }); 3. Duplicate Prevention (Dedupe): The SDK also includes aDedupeintegration (enabled by default) that helps prevent duplicate error events from being sent to Sentry [1][7]. It compares consecutive errors based on their message, type, value, and stack trace [7]. If you are seeing duplicates, ensure that you are not manually capturing exceptions that the SDK is already handling automatically, as theDedupeintegration only suppresses events that are highly similar and captured in immediate succession [5][7]. If you find that an error is being captured twice, verify if it is being caught in atry/catchblock whereSentry.captureException(e)is called, while simultaneously being bubbled up to the global scope where the default Sentry integrations automatically catch it [5][6]. Removing the manualcaptureExceptioncall is the recommended fix in such cases [5].Citations:
- 1: https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/
- 2: https://docs.sentry.dev/platforms/javascript/guides/node/
- 3: https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/unhandledrejection/
- 4: https://docs.sentry.io/platforms/javascript/guides/express/configuration/integrations/onuncaughtexception/
- 5: https://docs.sentry.io/platforms/javascript/guides/node/troubleshooting/
- 6: https://docs.sentry.io/platforms/javascript/guides/node/usage/
- 7: https://getsentry-sentry-javascript.mintlify.app/integrations/dedupe
Remove the manual Sentry capture from these process handlers
src/instrumentation.ts:334-344.Sentry.init()leaves the default NodeunhandledRejection/uncaughtExceptionintegrations enabled, so thesecaptureException(...)calls will report the same failure twice. Keep the Bridge forwarding only, or disable/filter those integrations if this code needs to own capture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/instrumentation.ts` around lines 330 - 345, Remove the explicit Sentry.captureException calls from both handlers in registerProcessErrorHandlers to avoid duplicate reporting by Sentry’s default process integrations. Preserve the existing error normalization and logProcessErrorToBridge forwarding for unhandledRejection and uncaughtException, along with the current fatality behavior.src/lib/admin/job-log.ts (1)
31-39: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Await failure telemetry before returning (
src/lib/admin/job-log.ts, Lines 31–39).The route returns while
logServerEvent()is still pending, so serverless shutdown can discard the failed-run event. Await it inside the existingtry/catch; this still preserves the original response if logging fails.Proposed fix
try { - void logServerEvent( + await logServerEvent( `Cron failed: ${jobType}`, { action: `cron.${jobType}`, source: 'cron', errorDetails: message }, 'error', - ).catch(() => {}); + ); } catch { /* never mask the real failure */ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { await logServerEvent( `Cron failed: ${jobType}`, { action: `cron.${jobType}`, source: 'cron', errorDetails: message }, 'error', ); } catch { /* never mask the real failure */ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/admin/job-log.ts` around lines 31 - 39, Update the failure telemetry handling around logServerEvent to await its completion before returning, rather than discarding the promise with void. Keep the existing try/catch so telemetry errors are swallowed and the original job failure remains unaffected.src/lib/baseball/coachhelm/outcome-sweep.ts (1)
185-189: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Expose partial write failures in the sweep result.
src/lib/baseball/coachhelm/outcome-sweep.ts— Lines 259-283 log failed updates but still return a success-shaped result. If every update fails,recordActionOutcomesreports success and the UI says no data was available. AddfailedtoOutcomeSweepStatsand let callers distinguish full, partial, and failed runs.Also applies to: 259-283
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/baseball/coachhelm/outcome-sweep.ts` around lines 185 - 189, Add failed-update counts to the OutcomeSweepStats type and populate that field from the existing failed counter in recordActionOutcomes. Update the result handling around the failure roll-up logging so callers can distinguish successful, partial, and fully failed sweeps, including the all-updates-failed case, while preserving the existing failureSamples reporting.src/lib/baseball/daily-contract/missed-sweep.ts (1)
247-268: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Count resolved
{ ok: false }results as delivery failures.Both writer contracts can report failure without throwing, but the new aggregation only handles exceptions.
src/lib/baseball/daily-contract/missed-sweep.ts#L247-L268: incrementtimelineFailedand record a sample whenres.okis false.src/lib/baseball/tasks/reminder-sweep.ts#L251-L301: incrementdeliveryFailedand record a sample whenres.okis false.📍 Affects 2 files
src/lib/baseball/daily-contract/missed-sweep.ts#L247-L268(this comment)src/lib/baseball/tasks/reminder-sweep.ts#L251-L301🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/baseball/daily-contract/missed-sweep.ts` around lines 247 - 268, Handle non-throwing writer failures in both aggregation paths: in src/lib/baseball/daily-contract/missed-sweep.ts lines 247-268, update the timeline write handling to increment timelineFailed and add a failure sample when res.ok is false; in src/lib/baseball/tasks/reminder-sweep.ts lines 251-301, apply the equivalent logic using deliveryFailed and its failure samples. Preserve the existing exception handling and sample limit.src/lib/error-logging.ts (1)
305-315: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard payload serialization (
src/lib/error-logging.ts, Lines 305–315).
JSON.stringify()can throw for circular context—particularly the arbitraryreasonattached by the global rejection handler. That escapessendToMonitoringService()and violates its never-throw contract. Catch serialization failures and send a minimal payload.Proposed fix
function sendToMonitoringService(logEntry: ErrorLogEntry): void { if (typeof window === 'undefined') return; - const body = JSON.stringify({ - message: logEntry.error.message, - stack: logEntry.error.stack, - context: logEntry.context, - severity: logEntry.severity, - timestamp: logEntry.timestamp, - url: window.location.href, - }); + let body: string; + try { + body = JSON.stringify({ + message: logEntry.error.message, + stack: logEntry.error.stack, + context: logEntry.context, + severity: logEntry.severity, + timestamp: logEntry.timestamp, + url: window.location.href, + }); + } catch { + body = JSON.stringify({ + message: logEntry.error.message, + severity: logEntry.severity, + timestamp: logEntry.timestamp, + url: window.location.href, + }); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function sendToMonitoringService(logEntry: ErrorLogEntry): void { if (typeof window === 'undefined') return; let body: string; try { body = JSON.stringify({ message: logEntry.error.message, stack: logEntry.error.stack, context: logEntry.context, severity: logEntry.severity, timestamp: logEntry.timestamp, url: window.location.href, }); } catch { body = JSON.stringify({ message: logEntry.error.message, severity: logEntry.severity, timestamp: logEntry.timestamp, url: window.location.href, }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/error-logging.ts` around lines 305 - 315, Update sendToMonitoringService to guard JSON.stringify failures caused by circular or otherwise unserializable context, preserving its never-throw contract. Catch serialization errors and fall back to a minimal monitoring payload containing only safely serializable core error information.src/lib/supabase/middleware.ts (1)
310-325: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n -C4 --type=ts '\bupdateSession\s*\(' . rg -n -C4 --type=ts '\bwaitUntil\s*\(' .Repository: njrini99-code/helmv3
Length of output: 21823
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,120p' src/lib/supabase/middleware.ts printf '\n---\n' sed -n '300,340p' src/lib/supabase/middleware.ts printf '\n---\n' sed -n '540,580p' src/lib/supabase/middleware.ts printf '\n---\n' sed -n '1,120p' src/proxy.ts printf '\n---\n' sed -n '1,80p' middleware.tsRepository: njrini99-code/helmv3
Length of output: 15556
🌐 Web query:
Next.js middleware NextFetchEvent.waitUntil background fetch request lifetime docs💡 Result:
In Next.js, the
event.waitUntil()method available on theNextFetchEventobject is used to extend the lifetime of a request (such as in Middleware or API routes) to allow background tasks to complete after the response has been sent [1][2][3]. Key points regarding the lifetime and behavior ofwaitUntil(): 1. Lifetime Limitation: The background work initiated bywaitUntil()is not guaranteed to run indefinitely. It is constrained by the execution timeout of the underlying serverless environment (e.g., Vercel Functions or Edge Runtime) [4][5]. If the tasks do not settle before the function's maximum execution duration is reached, they will be terminated [4][5]. 2. Reliability:waitUntil()is designed for best-effort background tasks, such as logging, analytics, or simple data updates [1][3]. Because it lacks built-in mechanisms for retries, error handling, or persistent queuing, it is not recommended for critical background jobs that must succeed [4]. 3. Execution Context:waitUntil()extends the lifecycle of the request handler until all passed promises have settled [6][5]. This prevents the runtime from shutting down the environment immediately after the primary response is returned to the client [5]. 4. Platform Dependency: WhileNextFetchEvent.waitUntilis a core part of the Next.js middleware specification, the ability to rely onwaitUntilfor background work is highly dependent on the deployment platform's support for keeping serverless invocations "warm" or alive after the response [5]. When hosting on platforms like Vercel, this behavior is explicitly supported to enable post-response processing [6][5]. For critical background tasks that require durability, retries, or long-running execution, using a dedicated background job or message queue service is recommended overwaitUntil()[4].Citations:
- 1: https://nextjs.org/docs/14/app/building-your-application/routing/middleware
- 2: https://nextjs.org/docs/app/api-reference/file-conventions/proxy
- 3: https://github.com/vercel/next.js/blob/v15.5.6/docs/01-app/03-api-reference/03-file-conventions/middleware.mdx
- 4: https://www.inngest.com/blog/vercel-cloudflare-wait-until
- 5: https://dev.to/mattlewandowski93/vercels-new-waituntil-30g9
- 6: https://nextjs.org/docs/app/api-reference/functions/after
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "NextFetchEvent|waitUntil\(|middleware\(request|proxy\(request|event:" src middleware.ts .next 2>/dev/null || true printf '\n---\n' sed -n '1,80p' middleware.ts printf '\n---\n' sed -n '1,120p' src/proxy.ts printf '\n---\n' sed -n '410,470p' src/lib/supabase/middleware.tsRepository: njrini99-code/helmv3
Length of output: 33619
Attach these telemetry fetches to the middleware lifetime
src/lib/supabase/middleware.ts#L310-L325: wrap the auth-failurefetch(...)inevent.waitUntil(...); otherwise it can be dropped when middleware returns.src/lib/supabase/middleware.ts#L553-L567: do the same for the idle-timeout sign-out log.middleware.ts#L18-L21/src/proxy.ts#L46-L67: threadNextFetchEventintoupdateSession(...)so both call sites can pass the event through.📍 Affects 1 file
src/lib/supabase/middleware.ts#L310-L325(this comment)src/lib/supabase/middleware.ts#L553-L567🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/supabase/middleware.ts` around lines 310 - 325, Thread NextFetchEvent through updateSession and both middleware call sites, including middleware.ts and src/proxy.ts. In the auth-failure telemetry block at src/lib/supabase/middleware.ts lines 310-325 and the idle-timeout sign-out logging block at lines 553-567, wrap each fetch with event.waitUntil(...) so the requests remain attached to the middleware lifetime.src/lib/telemetry-gate.ts (1)
22-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make CI and build guards unconditional (
src/lib/telemetry-gate.ts, Lines 38–46).
ADMIN_EVENTS_FORCE_CAPTUREcurrently returns before the CI andNEXT_PHASEchecks, contradicting Lines 26–28 and allowing test/build telemetry into production tables. Move both hard guards ahead of every opt-in.Proposed fix
export function shouldPersistAdminTables(): boolean { - if (process.env.ADMIN_EVENTS_FORCE_CAPTURE === '1') return true; if (process.env.NEXT_PHASE === 'phase-production-build') return false; if (process.env.CI === 'true' || process.env.GITHUB_ACTIONS === 'true') return false; + if (process.env.ADMIN_EVENTS_FORCE_CAPTURE === '1') return true;Also applies to: 38-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/telemetry-gate.ts` around lines 22 - 30, Update the telemetry gate ordering so the CI/GITHUB_ACTIONS and NEXT_PHASE guards execute first and unconditionally, before evaluating ADMIN_EVENTS_FORCE_CAPTURE or ADMIN_EVENTS_CAPTURE_PREVIEW. Preserve the existing opt-in behavior only after both hard guards reject test and build environments.src/stores/auth-store.ts (1)
49-79: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C2 'removeTag\(|setTag\(' src \ --type=ts --type=tsx rg -n '"`@sentry/nextjs`"' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: njrini99-code/helmv3
Length of output: 186
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- src/stores/auth-store.ts ---' nl -ba src/stores/auth-store.ts | sed -n '1,220p' echo echo '--- Sentry dependency refs ---' rg -n '"`@sentry/nextjs`"|`@sentry/nextjs`' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true echo echo '--- Sentry tag API search in repo ---' rg -n 'removeTag\(|setTag\(|setUser\(' src package.json 2>/dev/null || trueRepository: njrini99-code/helmv3
Length of output: 231
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- auth-store usages ---' rg -n '\bsetCoach\(|\bsetPlayer\(|\bclear\(' src --glob '!src/stores/auth-store.ts' || true echo echo '--- package version ---' python3 - <<'PY' import json, os for path in ['package.json', 'package-lock.json', 'pnpm-lock.yaml', 'yarn.lock']: if os.path.exists(path): print(f'FILE {path}') if path.endswith('package.json'): data=json.load(open(path)) deps={**data.get('dependencies',{}), **data.get('devDependencies',{}), **data.get('peerDependencies',{})} print(deps.get('`@sentry/nextjs`')) PYRepository: njrini99-code/helmv3
Length of output: 4869
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' src/stores/auth-store.ts | cat -nRepository: njrini99-code/helmv3
Length of output: 3682
🌐 Web query:
Sentry JavaScript SDK 10.62.0 remove tag API setTag clear tag scope💡 Result:
In the Sentry JavaScript SDK (including version 10.62.0), there is no dedicated API method named removeTag [1]. To effectively remove or unset a tag from a scope, you can pass undefined as the value to the setTag method [2][1]: Sentry.setTag('your-tag-key', undefined); When this is executed, the Sentry SDK will treat the tag as unset, and it will no longer be included in subsequent events captured within that scope [2]. Regarding clearing tags from a scope: 1. Individual Tags: Use the method mentioned above (setTag with undefined) to remove specific tags [2]. 2. Entire Scope: To clear all data from a scope, you can use the scope.clear method [3][4]. This resets the scope to its default state, removing all tags, user context, breadcrumbs, and other context information [3]. Note that top-level Sentry methods like Sentry.setTag typically write to the current isolation scope [3][4]. If you need to manage tags in a specific, non-global scope, you should use the scope object directly (e.g., within a withScope callback) [4].
Citations:
- 1: getsentry/sentry-javascript#2218
- 2: https://getsentry-sentry-javascript.mintlify.app/core/scopes
- 3: https://develop.sentry.dev/sdk/foundations/state-management/scopes/
- 4: https://docs.sentry.io/platforms/javascript/enriching-events/scopes/
Clear stale Sentry tags on auth transitions (
src/stores/auth-store.ts:49-79).
clear()only callsSentry.setUser(null), souser_role,org_id,org_name, andsportremain on the active scope (src/stores/auth-store.ts:49-55,76-79).setCoach(null)/setPlayer(null)never clear prior role/org tags, and a coach withoutorganization.idleaves stale org data behind (src/stores/auth-store.ts:58-70).Unset them on every transition with
Sentry.setTag(key, undefined)when the value is absent, or clear the scope before setting the next user.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/auth-store.ts` around lines 49 - 79, Update setUser, setCoach, setPlayer, and clear in the auth store to remove stale Sentry context whenever values are absent or authentication is cleared. Explicitly unset sport, user_role, org_id, and org_name with undefined when appropriate, including setCoach(null), setPlayer(null), and coaches without an organization ID, while preserving current tags for valid values.
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
1 participant
What
Closes every P0/P1 gap from an exhaustive 10-surface audit of the error-capture pipeline, so all errors in GolfHelm and BaseballHelm land in the Helm Bridge (
error_logs+admin_events) as well as Sentry. 78 files, logging-additive by design — no behavior changes except explicitly sanctioned ones (toasts on previously zero-feedback failures, honest non-200s on cron total-failures, putt-tendencies no longer echoes raw error text).Headline fixes
onRequestErrornow writes to the Bridge (nodejs direct; edge via newINTERNAL_LOG_KEYrelay/api/internal/log-server-error), with control-flow + already-logged dedupunhandledRejection/uncaughtException→ Sentry + Bridge, throttled 20/minkeepalive,response.okcheck, one retry,sendBeaconfallback/baseball/error.tsxroot boundary;logLogin/logSignup/logSecurityEventauth parity; middleware route-gate DB errors visible; outcome-sweep/postgame/sweep cores instrumented; 9 client surfaces wired tologError(was zero)withAdminObserved/registry exclusions and their tests are untouched — see DecisionslogError(messaging, calendar, roster, settings, Lift Lab set logging, uploads, CRM panels)recordJobRunmarks ≥400 responses failed; genome-nightly/weekly-coach-email/ingest-sync/backfills/process-sequences/admin-digest all log + return honest statuses; Inngest gets Bridge logging conventionskipSentryafter 5/10min per table+verbADMIN_EVENTS_CAPTURE_PREVIEW=1(CI/build guards still absolute)Sentry.setUserparity; per-app sport tag server-side;/admin,/lifting, marketing tagged distinctly client-side; duplicate global window listeners unified;infoseverity chip in /admin/errorsDecisions needing owner sign-off
liftingas a first-class Bridge sport bucket (needs enum migration); baseball checks in the integrity-check SQL function; verifyingINTERNAL_LOG_KEYis set in Vercel prod (team-scoped vars hide from CLI — please check dashboard).Verification
npm run typecheck/npm run lint— passnpm test— 552 files, 5,350 passed / 14 skippednpm run build(webpack prod) — passonRequestError, Sentry double-capture, alert-storm throttles)🤖 Generated with Claude Code
https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg