Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Repo Gate Evidence | |
The supplied PR description reports validation outcomes but does not list executable commands. It mentions a full preflight, focused tests, typechecks, and a frozen install without command lines. The … | Add a Validation section to the complete PR description. List each command and its result: a focused Vitest command for the changed test files, affected workspace typechecks, bun run build:admin, bun run build:donor, `bun run build:miss… |
| Generated Mirrors |  |
The authoritative PR diff changes no generated skill or agent mirror, canonical skill source, or skill sync tooling. The visible PR description does not show bun run skills:sync or `bun run skills:v… |
Provide the complete PR description, or confirm that it contains both exact commands: bun run skills:sync and bun run skills:verify. |
-
Check name Status Explanation Title check The title "Modernize Sentry, content, email and browser payment SDKs" names the main change: upgrading the Sentry, content-rendering, Resend and browser Stripe SDKs. It does not use the preferred conv… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Tenant Safety PASS. The PR does not change auth, Supabase behavior, database queries, migrations, API routes, server actions, or dashboard data loading. The changed source adds Resend email-ID validation and update… Description check The description covers the SDK version changes, the Sentry policy and release resolution changes, the inbound email ID validation, and the qualification evidence, with links to the records. It also in… Full details: Title check
-
Explanation
The title "Modernize Sentry, content, email and browser payment SDKs" names the main change: upgrading the Sentry, content-rendering, Resend and browser Stripe SDKs. It does not use the preferred conventional-commit prefix, but the requirements only prefer that style and do not mandate it.
Full details: Tenant Safety
-
Explanation
PASS. The PR does not change auth, Supabase behavior, database queries, migrations, API routes, server actions, or dashboard data loading. The changed source adds Resend email-ID validation and updates Sentry, Stripe, parsing, and sanitization behavior. Supabase and workspace auth/database dependency entries remain unchanged, and no changed diff lines alter tenant or session scoping or RLS assumptions.
Full details: Description check
-
Explanation
The description covers the SDK version changes, the Sentry policy and release resolution changes, the inbound email ID validation, and the qualification evidence, with links to the records. It also includes the repository's deploy checklist, with items marked as done. The checklist items are checked, but a few claims cannot be verified from the diff alone (for example, CI results and Preview readiness). The description is largely complete, so the check passes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Repo Gate Evidence
-
Explanation
The supplied PR description reports validation outcomes but does not list executable commands. It mentions a full preflight, focused tests, typechecks, and a frozen install without command lines. The PR changes three apps, two packages, shared scripts, and eight unit-test files. The full description is truncated, and no complete description artifact is available to confirm a later Validation section. The standing failure therefore remains unresolved.
Resolution
Add a Validation section to the complete PR description. List each command and its result: a focused Vitest command for the changed test files, affected workspace typechecks,
bun run build:admin,bun run build:donor,bun run build:missionary,bun install --frozen-lockfile, andbun run ci:preflight -- --full.
Full details: Generated Mirrors
-
Explanation
The authoritative PR diff changes no generated skill or agent mirror, canonical skill source, or skill sync tooling. The visible PR description does not show
bun run skills:syncorbun run skills:verify, but the description is explicitly truncated. The required command-mention check cannot be verified from the available metadata.Resolution
Provide the complete PR description, or confirm that it contains both exact commands:
bun run skills:syncandbun run skills:verify.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Shadscan workspace auditRaw pooled score: 48/100. Engine 0.17.0, ruleset 2026.08.46.
6 library projects are reported separately and excluded from the pooled application score. No library score floor or adjusted score is used. Gate: passed. All raw findings remain in the attached JSON report, including proven scanner limitations and accepted product choices. Raw failures and reviewed classifications
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Shadcn/UI Review
Reviewed from the perspective of shadcn/ui correctness, Base UI composition, and Maia visual language. I left no inline comments because this PR does not change product UI, shared primitives, theme tokens, or components.json.
1. FINAL VERDICT
SAFE TO MERGE
This is an SDK qualification PR. It does not introduce shadcn API misuse, Maia drift, or custom product markup that should have used shared components.
2. EXECUTIVE SUMMARY
- What the PR is doing. It qualifies current Sentry, content sanitization, Resend, and browser Stripe SDKs (Sentry 11.9.0, html-react-parser 6.1.9, isomorphic-dompurify 4.6.0 / DOMPurify 3.4.17, Resend 6.34.1, Stripe.js 10.1.0). It tightens Sentry collection policy, adds regression tests, and records qualification docs. React Stripe stays 7.0.0.
- What it gets right. Shared UI ownership is untouched.
packages/ui,components.json,styles/globals.css, and app TSX/CSS are not in the diff. Shadscan and React Doctor files only refresh hashes after lock/package changes. Sanitization tests exerciseSafeHtmlas a security boundary, not as a new design-system surface. - Biggest shadcn or Maia risks. None in this diff. Pre-existing Shadscan 48/100 floors on unchanged
packages/uiare not this PR. - What matters most. Keep this merge scoped to SDKs. Do not treat hash-only scanner updates as new UI work.
Plain language: The screens people click through did not change. Only libraries, privacy policy, tests, and docs moved. There is nothing here to restyle or recompose in Maia.
3. PROJECT CONTEXT SNAPSHOT
Recorded from cd packages/ui && bunx --bun shadcn@latest info --json plus packages/ui/components.json.
- packageManager: bun (
bunx --bun shadcn@latest) - framework: Next.js App Router apps; shadcn package reports
Manual - isRSC:
packages/uiconfigrsc: false(components may be client). Apps remain App Router / RSC. Unchanged by this PR. - aliases:
@/components,@/lib/utils,@/components/shadcn,@/lib,@/hooks. Product imports stay@asym/ui/components/shadcn/*. - style:
base-maia(presetbc5ed0K, Maia, zinc, Figtree, radius default) - base:
base(Base UIrender, not RadixasChild) - iconLibrary: lucide
- tailwindVersion: v4
- tailwindCssFile:
packages/ui/styles/globals.css - installed components relevant to this PR: none. Field, Alert, Empty, Button, Dialog, Sonner, and the rest remain installed in
packages/ui/components/shadcnand were not edited.
The repo already uses Maia. This PR does not move toward or away from that target.
4. PR IMPACT MAP
- What changed. 28 files vs
develop(0026bcbe...116b97cf): app/package manifests,bun.lock, Sentry policy/scripts, email Resend pin, SafeHtml/Sentry/Stripe/Resend tests, modernization docs, React Doctor exception hashes, Shadscan finding hashes. - Which shadcn components were touched. None.
- Which components should have been used. None. There is no new product form, overlay, empty state, badge, skeleton, or alert surface.
- Shared primitives. No.
packages/uiis unchanged.SafeHtmlinpackages/libis also unchanged; only tests were added. - Theme tokens / styling system. No.
components.jsonandglobals.cssare unchanged. Lockfile UI packages (@base-ui/react, lucide, Tailwind, Maia fonts) did not bump. - Maia direction. Neutral. Visual language is not in scope.
UI surfaces: none.
Forms / overlays / icons / registry / alias rewrites: none.
5. HARD BLOCKERS
None.
Checked against fail conditions: no wrong Base vs Radix API, no overlay titles, no Field/InputGroup work, no product custom markup replacing shadcn, no raw Tailwind colors in product UI, no wrong aliases or icon library, no fake Button isLoading/isPending, no broken Tabs/Card/Avatar/menu composition, no theme edits outside globals.css, no shared-UI Maia drift.
6. HIGH RISK ISSUES
None.
tooling/shadscan/findings.json only updates sha256 for apps/admin/package.json and packages/lib/package.json. Finding rules, classifications, and floors are the same. That is lock-evidence refresh, not a new UI primitive.
7. MEDIUM RISK ISSUES
None.
8. LOW RISK ISSUES AND SUGGESTIONS
None required for merge.
Suggestion only: keep using the existing Donor/Admin/Missionary Preview smoke checklist already on this PR to confirm Stripe Elements and CMS HTML still render after the parser/DOMPurify/Stripe.js pins. That is runtime confidence, not a shadcn composition defect.
9. MAIA FIT ASSESSMENT
- Does the changed UI feel like Maia? There is no changed product UI.
- Where it aligns. Shared
base-maiaownership is preserved by omission. - Where it drifts. It does not.
- Acceptable? Yes. Neutral is the correct outcome for an SDK PR.
Sanitizer fixtures that keep color: red inside HTML strings are XSS/style-preservation tests, not product tokens. Do not rewrite those fixtures to text-destructive.
10. WHAT THE PR GETS RIGHT
- Component choice. No invented app-local shadcn fork.
- Composition. No Radix
asChildand no Baserendermisuse, because no overlay/form code landed. - Tokens. No raw Tailwind colors or
dark:overrides in product UI. - Maia. Exact
base-maia/ zinc / Base UI contract is left alone. - Icons. lucide is unchanged; no Button icon work.
- Forms. No new fields, so FieldGroup/Field rules are not in play.
11. ORDERED FIX PLAN FROM FIRST TO LAST
No shadcn fixes are required before merge.
- Merge the SDK qualification as scoped.
- Leave pre-existing Shadscan floors on
packages/uifor a UI-owned PR. - Optional later: visual Preview smoke of Stripe Elements and sanitized CMS HTML after the version pins.
This order matters because inventing UI refactors here would mix design-system work into an SDK cohort and hide real Maia issues in the wrong diff.
12. VALIDATION PLAN BEFORE MERGE
- Inspected
bunx --bun shadcn@latest info --jsonfrompackages/ui:base-maia,base, lucide, Tailwind v4,styles/globals.css. - Component docs lookup: not applicable; no shadcn component APIs changed or added.
- Installed components: still present under
packages/ui/components/shadcn; none imported by this diff. - Imports/aliases: no new UI imports.
- Base vs Radix: no trigger/Select/ToggleGroup/Slider/Accordion usage.
- Forms: no FieldGroup/Field surface.
- Overlays: no Dialog/Sheet/Drawer.
- Buttons: no fake loading props.
- Icons: no
data-iconwork and no library swap. - Theme: no edits to
packages/ui/styles/globals.css. - Maia visual check: not applicable to this diff; Preview smoke remains a release gate, not a composition blocker.
- Raw colors /
dark:overrides: none in product UI. Test HTML strings are sanitizer fixtures.
13. WHAT TO WATCH IN RE REVIEW
- Closest second look: only if a later commit adds app TSX,
packages/ui,components.json, orglobals.css. - Human visual check: Stripe Elements and sanitized rich text on Preview, if those SDK pins are exercised in hosted environments. Not a Maia markup review of this diff.
- Structural check: confirm
packages/uiand app UI files stay out of the merge-base tree.
14. FOLLOW UP IDEAS
- Continue lowering the existing Shadscan 48/100 floors on unchanged
packages/uiin a dedicated UI PR. - Do not use this PR to restyle
SafeHtmlinto a shadcn Card/Alert. It is a sanitization host for trusted-after-sanitize HTML.
15. OPEN QUESTIONS
None for shadcn/Maia correctness. Hosted CI and exact-commit Preview smoke are called out on the PR itself and sit outside this design-system review.
Sent by Cursor Automation: Shadcn UI Review
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Critical Bug Check
Reviewed from the perspective of high-severity correctness: data loss, crashes, security holes, and significant user-facing breakage. No confirmed issues met that bar, so there are no inline comments on this review.
Verdict
No critical bugs found. This is a dependency modernization plus matching policy/config tests. I did not open a follow-up fix PR.
Plain language: The PR updates error-tracking, email, HTML sanitizing, and the browser Stripe loader. I walked the payment, email, and error-reporting paths looking for a real way a donor, missionary, or operator could lose money, leak private data, or hit a crash. I could not construct a plausible trigger, so nothing here should block merge on correctness grounds from this check.
What this PR changes
Technical: Range 0026bcbe8...116b97cf2 (chore/AL-1965-observability-content-sdks → develop). Unique production commit 41f75602f plus follow-up policy/docs commits. Pins: @sentry/nextjs 11.5.0 → 11.9.0, @stripe/stripe-js 10.0.0 → 10.1.0, resend 6.32.1 → 6.34.1, html-react-parser 6.1.8 → 6.1.9, isomorphic-dompurify 4.5.0 → 4.6.0 (DOMPurify 3.4.16 → 3.4.17). react-stripe-js stays 7.0.0. Core still uses Next.js App Router Cache Components and Instant Navigation. Product apps import withSentryConfig from @sentry/nextjs/config and do not set tunnelRoute.
Plain language: This is a planned SDK bump, not a rewrite of checkout, webhooks, or permissions. The interesting bits are how Sentry is configured, how Stripe.js is loaded in the browser, how Resend verifies inbound mail, and how donated/mission HTML is sanitized before render.
Paths traced
Sentry collection policy and Next config
Technical: getSentryCompatibilityOptions still disables PII (sendDefaultPii: false, no user/cookies/bodies, no genAI/MCP/queues/GraphQL/DB query capture) and now also denies referer/referrer. buildSentryNextConfigOptions dropped unsupported release.inject and webpack.treeshake.removeDebugLogging in favor of SDK release.name injection, telemetry: false, and bundleSizeOptimizations.excludeDebugStatements. Sourcemap upload remains token-gated (sourcemaps.disable when no auth token). onRequestError = Sentry.captureRequestError is unchanged in admin, donor, and missionary apps. Sentry 11.9 Cache Components behavior: prerendered shells do not emit sentry-trace; the client pageload span waits for a resume tag until DOMContentLoaded or a 10s timeout. That wait is tracing-only; it does not block UI, auth, or payments.
Plain language: Error reports still must not include names, cookies, form bodies, or page referrers. Release tagging moved to the supported API, so source maps should still line up. The new “wait for resume” behavior only delays how Sentry starts a page-load trace; screens still render and checkout still runs.
Browser Stripe loader
Technical: @stripe/stripe-js 10.1 shares one in-flight script load across concurrent loadStripe calls (the 10.0 hang fix) while still constructing a Stripe instance per publishable key. Donor checkout continues to call loadStripe with the campaign publishable key rather than the server getStripe() module singleton. A same-key concurrent load no longer serializes behind a stuck first promise; two different keys still produce two Stripe(key) constructions.
Plain language: Opening two checkout flows at once should no longer freeze the Stripe.js script. A donor paying one campaign still gets that campaign’s Stripe account, not another campaign’s.
Resend verify and send
Technical: Inbound webhook verification remains synchronous Svix (resend.webhooks.verify is still not async in 6.34; Core tests assert object equality without await). Send still uses emails.send, not batch. 6.34 rejects dot-segment path IDs (. / ..) as path-traversal protection; production IDs are UUIDs / email_* and are unaffected.
Plain language: Incoming email webhooks are still checked the same way before Core trusts them. Normal message IDs keep working. Only obviously unsafe path IDs like . or .. are rejected, which is the intended hardening.
HTML sanitizer
Technical: isomorphic-dompurify 4.6 / DOMPurify 3.4.17 plus html-react-parser 6.1.9. Core still sanitizes through the HTML profile in packages/lib/html/sanitize.ts before SafeHtml parse. Existing tests still require scripts, javascript: URLs, and event handlers to be stripped while inline styles remain allowed under that profile.
Plain language: Untrusted HTML is still cleaned before it is shown. Script tags and click-to-run JavaScript links should still be removed. This bump is hardening and dependency maintenance, not a new allow-list.
Candidates that did not meet the bar
These were traced and rejected because they lack a concrete user-facing trigger in this repo, or they are observability/config risk rather than data loss, auth bypass, or a crash:
- Removing
release.inject— current Sentry docs no longer list it; the SDK injects fromrelease.name; tests assert the old property is absent. Worst case is release grouping in Sentry, not a product outage. - Switching webpack treeshake to
bundleSizeOptimizations— this is the 11.7+ supported option for webpack and Turbopack; debug statements are still excluded. - Resend verify becoming async — changelog and Core tests still show a sync return; no missing-
awaithole on the webhook path. - Sentry 11.9 prerender-trace / resume wait — privacy/tracing fix with a bounded client timeout; does not gate checkout or mutations.
- MCP/queues collection defaults — this PR explicitly turns them off; that is a leak reduction, not a leftover capture path.
- Neon integration default — Core does not use
@neondatabase. getStripe()process singleton vs key rotation — pre-existing server helper; browser checkout does not go through it.release.createwithout a token — pre-existing; sourcemaps stay disabled without credentials.
No auth, payment, webhook-signature, tenant, or mutation contract in this diff introduces a silent write loss or permission bypass.
Validation performed
Read the PR range, unique commits, lockfile pins, Sentry policy/config, donor loadStripe usage, Resend send/verify/inbound modules, and sanitizer/SafeHtml tests. Cross-checked Sentry 11.7–11.9, Stripe.js 10.1, Resend 6.34, and DOMPurify 3.4.17 release notes against Core call sites. Did not claim CI that was not run in this automation pass.
Sent by Cursor Automation: Critical Bug Finding
QA Smoke Preview DeploymentsLabel: qa:smoke Preview URLs: |
Headless PR Preview Smoke QAResultPASS Triggerqa:smoke CommitPreview URLs
Projects
EvidenceSelected Playwright preview smoke projects passed. NotesNo secrets were printed. No credentials were printed. No production deploys were run. |
There was a problem hiding this comment.
Thermo-Nuclear Code Quality Review
Reviewed from the perspective of thermo-nuclear code quality and clean-code maintainability. No inline comments: there are no confirmed issues to attach to a diff hunk.
Verdict
No high-confidence issues. This PR does not introduce a structural regression, spaghetti growth, 1k-line file, unnecessary wrapper, or architecture-boundary leak. The production shape is the existing one: one Sentry policy object, one Next build-options builder, and pin-only upgrades for parser, sanitizer, Resend, and browser Stripe.js.
Findings
No high-confidence findings.
Suspected issues were checked against installed SDK types, Sentry 11.9.0 internals, and the surrounding Core contracts, then dropped:
release.injectremoval is not a missed release injection.SentryBuildOptionsomitsinject.@sentry/nextjs11.9.0'sgetBuildPluginOptionsalways sets bundlerinject: falseand injectsrelease.nameitself. Re-adding Core's flag would be a no-op or a type error, not a clearer design.bundleSizeOptimizations.excludeDebugStatementsis the supported 11.9 API, not a rename that drops tree-shaking. Webpack still falls backexcludeDebugStatements ?? removeDebugLogging. The same object is spread into the build plugin, so Turbopack gets the debug-statement strip the old webpack-onlytreeshakeoption missed. Keeping both keys would preserve incidental complexity.- Denying
referer/referreris the correct shared privacy fix. NodeRequestData.filterKeyValueDatamatches header keys with a case-insensitive substring include; it never rewrites query values insideReferer. Splitting browser (keep filtered referrer) vs Node (deny) would add a runtime branch for no privacy gain. Ten initializers already spreadgetSentryCompatibilityOptions(); extending that object is the code-judo move, not scattering deny lists into app files. dataCollection.mcp/queues: falseis not dead config. 11.9 adds MCP collection and documents an MCP→genAI fallback. Setting both false next to the existinggenAI: falsekeeps the v10 privacy posture. Tests readgetClient()?.getDataCollectionOptions()on a real SDK client.- Historical
*11.8.0*qualification JSON cannot select the install.package.json,bun.lock, anddocs/maintenance/modernization/observability-content-sdks-current-qualification.jsonall pin11.9.0. Campaign docs point at the*-current-*records. No installer readsinitialTarget.
Plain language: the upgrades stay in the files that already owned these SDKs. Nothing new was bolted onto unrelated flows, and the privacy/build knobs match what Sentry 11.9 actually implements.
Validation
bunx vitest runonsentry-policy,sentry-request-privacy,sentry-browser-privacy,sentry-capture,stripe-sdk-loader,resend-sdk-security,safe-html,safe-html-client,sentry-release-sourcemaps— 42 tests, 9 files, pass.node scripts/verify/sentry-release-sourcemaps.mjs— PASS (shared next-config usage, no-token sourcemap disable,telemetry: false,bundleSizeOptimizations.excludeDebugStatements, token-present upload, Turbo hash inputs).bun run typecheck --filter=@asym/lib— pass (getSentryCompatibilityOptions()stillsatisfiesParameters<typeof init>[0]on@sentry/nextjs11.9.0).- Installed pins inspected in
node_modules:@sentry/nextjs@11.9.0,@stripe/stripe-js@10.1.0(RELEASE_TRAIN = endive),resend@6.34.1,isomorphic-dompurify@4.6.0/dompurify@3.4.17,html-react-parser@6.1.9.
Full typecheck:admin / typecheck:donor / typecheck:missionary were not run; the Sentry options contract lives in @asym/lib and typechecked there.
What I checked
- Diff
0026bcbe8b601586c29bba422675ce5889edd7b1...116b97cf27327865cc0c78696b66335e517e7c7fand surrounding callers. packages/lib/monitoring/sentry-policy.ts(35 lines) and all 10 Sentry initializers that spread it (admin/donor/missionary client, server, edge, plus sharedinitSentry).scripts/sentry/next-config.mjsagainst@sentry/nextjs11.9SentryBuildOptions,getBuildPluginOptions.js, and webpackexcludeDebugStatementsfallback.scripts/verify/sentry-release-sourcemaps.mjsand its unit tests.- SafeHtml server + client paths with DOMPurify 3.4.17 /
html-react-parser6.1.9. - Stripe.js 10.1 concurrent
loadStripeon both default andpureentrypoints (script-tag intercept, not a livejs.stripe.comfetch). - Resend 6.34.1 webhook signature verification and ID validation without network calls.
- File sizes: production files stay well under 1k lines (
sentry-policy35,next-config75, verifier 179). - Campaign qualification hashes for the 19 runtime inputs vs current file bytes.
Notes
- Browser Stripe.js was proven as a loader contract (one script tag, shared promise), not as a live Elements session.
@stripe/react-stripe-jsremains 7.0.0 by design. - Denying the Referer header removes a debugging breadcrumb in exchange for not shipping query secrets from Node request URLs. That is an explicit privacy tradeoff, not a regression against the v10 collection posture.
- Dated
*11.8.0*qualification files remain in-tree as campaign evidence. They are not the active pin.
Sent by Cursor Automation: Thermonuclear Cursor Code Review
There was a problem hiding this comment.
Improve Codebase Architecture Review
Reviewed from the perspective of module depth, locality, leverage, and inventory-seam ownership. I left one resolvable inline comment for the confirmed inventory issue.
Verdict
This PR improves product-module architecture. Sentry collection policy, the Resend adapter, SafeHtml sanitization, and the Sentry Next config builder hide real complexity behind small existing interfaces. One non-blocking Medium finding remains: the modernization inventory verification list treats the current 11.9.0 freeze and three superseded 11.8.0 records as the same evidence.
Architectural Findings
Finding 1: Inventory verification mixes current freeze with superseded records
Severity: Medium
Location: docs/maintenance/modernization/dependencies.json verification arrays at 1612-1618 for Sentry, copied onto react-stripe-js 1756-1762, stripe-js 1806-1812, html-react-parser 5012-5018, isomorphic-dompurify 5138-5144, and resend 6665-6671
Architectural concern: The inventory verification interface lists current freeze files and superseded 11.8.0 files as equal proof. The owning current-qualification module already records those three 11.8.0 files as historicalRecords. The checkpoint README already states they no longer own active version or integration guidance. Callers of the inventory must hold two contradictory version truths.
Required change: For these six identities, set verification to only observability-content-sdks.md, observability-content-sdks-current-qualification.json, and observability-content-sdks-current-releases.json. Do not list the three original 11.8.0 files in verification. Keep them only through historicalRecords on the current qualification. Optional: add a qualification field pointing at the current qualification JSON, matching dnd-kit and payload identities. Do not move or rewrite the historical JSON files unless historicalRecords paths update in the same change.
Technical explanation:
Verification is the inventory module interface for "what currently proves this pin". Implementation detail (dated RED or GREEN evidence) already has a seam: historicalRecords on observability-content-sdks-current-qualification.json. Flattening both into one array makes the interface as wide as the file set and leaks which record is live. currentUses already shows 11.9.0, 6.1.9, 4.6.0, 6.34.1, and 10.1.0. The extra three paths reintroduce 11.8.0 as if it still owned the freeze. That is a shallow pass-through list: deleting those three entries concentrates ownership in the current qualification module instead of scattering version truth across inventory plus three live JSON files plus historicalRecords.
Plain-language explanation:
The pin list currently says "look at these six files to know what we shipped." Three of those files still describe the previous 11.8.0 freeze. The new qualification file already labels them as history. Anyone (human or AI) reading the inventory can pick the wrong file and think 11.8.0 is still the active pin. Drop the three old files from the inventory list. Leave them on disk as history, pointed to from historicalRecords.
Architectural impact:
Locality of the active freeze is split. Cognitive load rises because the same cohort owns two version numbers in one interface. AI navigability suffers: a later agent can cite observability-content-sdks-releases.json as current and reopen a finished 11.8.0 to 11.9.0 upgrade. Maintainers then re-qualify the wrong tarball.
Suggested deepening opportunity:
Make verification a deep, small interface: current guide plus current qualification plus current releases. Let historicalRecords remain the only adapter for superseded bytes. Optionally add the existing qualification field used by dnd-kit so inventory callers do not have to guess which JSON is live.
Deletion test observations
- Extra verification entries for the three 11.8.0 files: fail. Deleting them concentrates current-pin complexity into the current qualification and current releases modules. They add interface surface without hiding behavior.
packages/lib/monitoring/sentry-policy.ts: pass. Deleting it would copy Referer deny, MCP off, and queues off into ten initializers. Callers today spread one object.packages/email/resend/sdk.tsas the sole Resend construction adapter: pass. Deleting it would pushnew Resendinto inbound, webhook, and future callers.- SafeHtml plus
packages/lib/html/sanitize.ts: pass. Deleting the sanitize module would leak DOMPurify profile details into every renderer. scripts/sentry/next-config.mjsbuilder: pass. Deleting it would re-spread unsupportedrelease.injectand debug-statement flags across app Next configs.packages/lib/stripe.tsgetStripe(): pre-existing unused helper. Deleting it concentrates nothing in this PR. Checkout already callsloadStripewith a normalized runtime key. Do not route checkout through this helper.
Validation
- First vitest run on stale VM packages failed 10 of 41 because node_modules still had Sentry 11.5.0 and Resend 6.32.1. Not an architecture defect.
bun install --frozen-lockfilethen focused vitest: 8 files, 41 passed on Sentry 11.9.0, Resend 6.34.1, Stripe.js 10.1.0, and isomorphic-dompurify 4.6.0.- Files: sentry-policy, sentry-browser-privacy, sentry-request-privacy, safe-html, safe-html-client, stripe-sdk-loader, resend-sdk-security, sentry-release-sourcemaps.
What I checked
- Full three-dot diff
0026bcbe8...116b97cf2(28 files). - CONTEXT.md domain language; ADR-0001 Twenty retirement; ADR-0029 Resend identity; ADR-0011 Stripe phone lane. This PR does not reopen those ADRs.
- Sentry policy seam versus app-local sample rates; nine live inits plus unused
initSentry; next-config builder; envelope tests on the real 11.9.0 SDK. @asym/emailpublic interface versus Payload email-resend (separate owner).- SafeHtml versus Payload Lexical public renderer (separate seam).
- Donor checkout
loadStripe(normalizedPublishableKey)versus unusedgetStripe()env-key helper. - Inventory currentUses versus verification versus historicalRecords versus README ownership of current-releases.
- TDD overlay: behavior tests sit on existing public seams. Qualification JSON and README are provenance; no fake RED test is required.
Disproved and not raised: collapsing sample rates into sentry-policy; routing checkout through getStripe(); renaming getSentryCompatibilityOptions; treating unused initSentry as this PR; counting the JSON file set as fragmentation; demanding sendDefaultPii or tunnel; conflating Payload email with @asym/email; conflating Lexical with SafeHtml; REQUEST_CHANGES for OpenSpec task 7.2 still open for CI and previews.
Notes
Non-blocking. Product modules are deeper after this PR. Fix the six copied verification bags so the inventory interface names only the live freeze. Historical 11.8.0 bytes can stay on disk through historicalRecords.
Sent by Cursor Automation: Improve Codebase Architecture PR Review
There was a problem hiding this comment.
Pre-Mortem Bug Finder
Reviewed from the perspective of failure-mode, invariant, input-partition, and mutation analysis against develop (0026bcbe8) → 116b97cf2.
Verdict: SAFE TO MERGE WITH FIXES.
This PR pins Sentry 11.9.0, Resend 6.34.1, Stripe.js 10.1.0, html-react-parser 6.1.9, and isomorphic-dompurify 4.6.0, then tightens Sentry collection policy and the release/sourcemap builder. Happy-path qualification is real. The remaining risk is not a crash on the default path.
Hidden failures that matter after merge:
- Silent observability collapse if commit env vars are the self-named INC-2566 placeholders Sentry 11.9 now ignores, because Core still copies them into explicit
release.name. - Security-boundary tests that do not cover the ID encodings this Resend bump claims to close.
- Privacy envelopes proven only on a stripped-down RequestData fixture, not on production default integrations.
I left separate inline comments for each issue. No auth, billing, or tenant-boundary break was found in the changed paths.
Sent by Cursor Automation: Pre-Mortem Bug Finder
There was a problem hiding this comment.
Bug Finder v2
Verdict: SAFE TO MERGE
No confirmed bugs. No high-confidence likely bugs. No merge blockers in the changed or affected runtime paths.
What this PR changes
Technical: Pins @sentry/nextjs 11.5.0→11.9.0, isomorphic-dompurify 4.5.0→4.6.0, html-react-parser 6.1.8→6.1.9, resend 6.32.1→6.34.1, and @stripe/stripe-js 10.0.0→10.1.0. Shared Sentry policy now denies Referer/Referrer and opts out of dataCollection.mcp / queues. withSentryConfig drops unsupported release.inject and webpack-only webpack.treeshake.removeDebugLogging, and uses telemetry: false plus bundleSizeOptimizations.excludeDebugStatements. New tests lock those contracts. Apps keep calling getSentryCompatibilityOptions(); no auth, payment, webhook, or migration product logic is rewritten.
Plain language: This is a qualified SDK refresh plus a tighter Sentry privacy policy. Donor checkout, email send/webhooks, and rich-text rendering keep the same Core wrappers; the libraries underneath get current patches.
Root-cause traces that did not become bugs
release.injectremoved (scripts/sentry/next-config.mjs:55). 11.9 types omitinject(SentryBuildOptions.releaseisOmit<..., 'inject'>). The Next.js SDK always injectsrelease.namebecause the bundler plugin's inject breaks the App Router. Removing the old flag is cleanup, not a source-map regression.webpack.treeshake→bundleSizeOptimizations(scripts/sentry/next-config.mjs:71-73). 11.7 made the top-level option work on webpack and Turbopack; Core apps use Turbopack. Keepingwebpack.treeshakewould miss debug-statement stripping on the live bundler.dataCollection.mcp/queues(packages/lib/monitoring/sentry-policy.ts:22-24). 11.9resolveDataCollectionOptionsdefaults both to collect payloads. Omitting them would start sending MCP/queue bodies. The PR opts out at the shared policy that every app init spreads.- Referer deny (
packages/lib/monitoring/sentry-policy.ts:16). Node RequestData does not filter query values inside Referer; denying the whole header is the source-level fix. Envelope tests coverreferer,Referer, andreferrer. - Stripe 10.1 concurrent
loadStripe. 10.1 shares one in-flight script (#992). Donor checkout callsloadStripeincreateReadyRuntimeConfig; concurrent callers no longer race duplicate scripts. - Resend 6.34.0 path hardening. SDK rejects dot-segments before fetch. Core inbound wrappers do not use
resend.inboxes; support-hub “inboxes” are Postgres. Inbox field renames in 6.33/6.34 do not hit this repo. - 11.9 Cache Components tracing. All three apps set
cacheComponents: true. 11.9 stops prerender/ISR pages from carrying another visitor's trace. That is a privacy/correctness fix, not a hang in Core's wrappers. tunnelRoutecredentials: 'omit'. Core never setstunnelRoute. The 11.9 cookie-omit change does not apply.
Evidence
- Installed the lockfile graph here: Sentry 11.9.0, Resend 6.34.1, isomorphic-dompurify 4.6.0, html-react-parser 6.1.9,
@stripe/stripe-js10.1.0, jsdom 30.1.3. - Local Vitest: 8 files / 41 tests passed (
sentry-policy,sentry-request-privacy,sentry-browser-privacy,safe-html,safe-html-client,stripe-sdk-loader,resend-sdk-security,sentry-release-sourcemaps). node scripts/verify/sentry-release-sourcemaps.mjsPASS.- CI on this head: lint, typecheck, test-unit, test-e2e, test-e2e-smoke, build, format, integrity, migrate, smoke, ci-gate, integration-gate SUCCESS. Vercel admin/donor/missionary previews completed. GitHub CodeRabbit: 0 actionable comments. Codex: no findings. Local CodeRabbit CLI 0.9.1 is
not_authenticatedin this environment; GitHub CodeRabbit is the completed pass.
Possible follow-ups (not merge blockers)
stackFrameVariablesstill inherits the SDK defaulttruebecause Core never set it. That default existed before this bump whendataCollectionis provided. Opting out would be extra privacy hardening, not a regression fix.- Sentry 11.7 stopped sending a Node process session once request sessions exist. Crash-free / release-health numbers can shift. Watch dashboards after deploy; do not treat that as a product defect.
No inline comments: nothing in the diff has a traced, PR-introduced failure that a resolvable thread should block on.
Sent by Cursor Automation: Bug Finder 2.0
There was a problem hiding this comment.
Critical Bug Check
Reviewed commit: 02e9ad93660159c03009516741bc2206fcc15250.
Review completed against develop (4e31ff33fc63063dafd8678298e74ce0c2eee894) ... 02e9ad93660159c03009516741bc2206fcc15250.
No critical bugs found in the reviewed changes.
Outstanding P0/P1 findings: 0. No optional P2 findings.
This re-review covered the SDK pin bump and the later release-name and inbound-ID corrections. Sentry 11.9.0 still injects release.name when release.create is not false; bundleSizeOptimizations.excludeDebugStatements, telemetry, dataCollection.mcp, and dataCollection.queues match the published option types. Resend 6.34.1 still verifies webhooks synchronously and still sends Idempotency-Key. The new receiving-ID check accepts only a nonempty ASCII letter, digit, underscore, or hyphen segment before the SDK builds /emails/receiving/${id}. Release resolution skips a trimmed value equal to its own environment key and continues, which matches Sentry's placeholder rule for the keys this builder sets explicitly. Stripe.js 10.1 shares one script load and still constructs a separate instance per publishable key.
The five existing review threads on this PR are resolved. This run adds no new inline threads.
Validation used the PR diff, inbound and checkout call paths, and the published 11.9.0, 6.34.1, and 10.1.0 package sources. The workspace does not have those SDK packages installed, so the focused unit tests were not re-executed here.
Sent by Cursor Automation: Critical Bug Finding
Updates Sentry to 11.9.0, html-react-parser to 6.1.9, isomorphic-dompurify to 4.6.0 (DOMPurify 3.4.17), Resend to 6.34.1 and Stripe.js to 10.1.0. React Stripe stays at 7.0.0. The branch integrates develop 4e31ff3 and freezes tested versions and tarball integrity.
The shared Sentry policy keeps sensitive collection disabled. Real Node/browser SDK envelopes retain default integrations and redact runtime request/referrer sentinels. Release resolution skips self-named environment placeholders while retaining valid-name precedence. MCP/queue checks prove resolved disabled settings; adding a matching collector path requires emitted-payload qualification. Receiving and attachment helpers reject malformed path-segment IDs before SDK construction and preserve valid UUID URLs and response mapping. Sanitizer, webhook signature, retry/idempotency and concurrent Stripe.js contracts are covered. Backend payment serialization remains separate.
Final local preflight and current-head CI pass all three production builds and 9,104 unit tests across 968 files; 53 existing tests in six files remain skipped. The 274 focused tests, 15 uncached workspace typechecks and six SDK fixture typechecks pass. The frozen installation inputs remain unchanged. Doctor retains 293 exact exceptions with zero actionable findings; Shadscan remains 48/100 with unchanged app floors. All 29 changed files are reviewed and review conversations are reconciled, including independent runtime and documentation re-review.
An existing elapsed-time readiness assertion intermittently exceeded its five-second limit locally (latest: 5,035 ms). The unchanged 25-test file and normal full gate passed on retry; test assertions, scope and quality floors are unchanged.
The latest GitHub Codex rerun could not complete because the account review quota was reached. Its earlier finding is fixed and all native review threads are resolved; the exact runtime/document corrections have independent source review. The managed failed-review summary is retained as an external limitation.
All three Preview deployments are READY on 02e9ad9 and their browser smoke projects pass. The isolated Admin Preview fixture expires at 2026-10-24T04:20:15Z; requalify that preview after expiry. No production execution, schema transition or payment cutover is part of this PR. Other owning-family security findings remain open; this cohort introduces no advisory pair and removes three, with no new exception or override.
Validation: CI, integration, Preview smoke, current qualification, release freeze, behavior and evidence limits.
Previews: Admin, Donor, Missionary.
Deploy checklist
No new material issues were found; the documented CI and Preview gates still apply before merge.Summary
This PR updates Sentry, HTML rendering, Resend and browser Stripe SDKs, adds focused regression tests, and records qualification evidence.
Reviews (5) · Last reviewed commit: "docs: distinguish dated SDK evidence fro..." · Reviewed by Greptile