refactor: migrate task assignment and comment notification emails to …#79
Merged
Merged
Conversation
…React components - Updated task assignment notification email to use React for rendering. - Simplified email content by removing redundant CTAs and feedback notes. - Enhanced test cases for task assignment email to reflect changes in content. - Migrated task comment notification email to React, improving structure and readability. - Updated tests for task comment notification email to accommodate new rendering method. - Removed legacy HTML rendering functions and replaced with React components. - Cleaned up email markdown documentation for both task assignment and comment notifications. - Adjusted URL handling to streamline base URL retrieval.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConverts many email templates to React-based renderers, adds a preview registry and Playwright snapshot script, refactors email send/dedupe to accept React payloads with safety checks, rewrites unsubscribe handling, tightens CI managed-data sync and remote-apply guards, normalizes DB trigger equality, updates URL/env resolution, and refreshes numerous page contents and tests. ChangesEmail Preview & Rendering
Preview Snapshoting
Unsubscribe API
CI & Managed-Data Safety
DB Normalization & Tests
URL/env/site defaults, tests & content
Estimated code review effort: Possibly related PRs:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR migrates several outbound notification emails (task assignment, task comment notifications, post-vote share, referral-first-conversion, monthly chain digest, magic link) from legacy string-built HTML/text to React Email rendering, and introduces a unified preview/markdown snapshot pipeline for reviewing rendered email output. It also consolidates base-URL resolution around NEXTAUTH_URL/VERCEL_URL, tightens send-time safety checks, and updates CI/managed-data tooling accordingly.
Changes:
- Refactored multiple email templates to React components, plus a shared React-email renderer and a central preview registry/envelope composer.
- Added send-time safety guardrails for localhost/unsub placeholders; extended deduped sending to support React templates.
- Updated page/email markdown snapshot tooling and CI workflows (including preview managed-data sync) to match the new pipelines.
Reviewed changes
Copilot reviewed 89 out of 90 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/lib/url.ts | Removes NEXT_PUBLIC_BASE_URL client override from base URL resolution. |
| packages/web/src/lib/tasks/task-comment-notifications.server.ts | Awaits async React-rendered task comment notification email builder. |
| packages/web/src/lib/tasks/task-comment-notification.email.md | Adds generated markdown snapshot for task comment notification email. |
| packages/web/src/lib/tasks/task-comment-notification-react-email.tsx | New React Email template for task comment notifications. |
| packages/web/src/lib/tasks/task-comment-notification-email.server.ts | Migrates comment notification email builder to React Email + preview metadata. |
| packages/web/src/lib/tasks/task-assignment.email.md | Adds generated markdown snapshot for task assignment email. |
| packages/web/src/lib/tasks/task-assignment-react-email.tsx | New React Email template for task assignment emails. |
| packages/web/src/lib/tasks/task-assignment-notifications.server.ts | Awaits async React-rendered task assignment email builder. |
| packages/web/src/lib/tasks/task-assignment-notification-email.server.ts | Migrates assignment email builder to React Email + preview metadata. |
| packages/web/src/lib/tasks/tests/task-comment-notification-email.test.ts | Updates tests for async builder and React-rendered output. |
| packages/web/src/lib/tasks/tests/task-assignment-notification-email.test.ts | Updates tests for simplified CTA behavior and async builder. |
| packages/web/src/lib/site.ts | Base origin resolution now prefers NEXTAUTH_URL, then VERCEL_URL, and changes canonical fallback. |
| packages/web/src/lib/referral-url-helpers.server.ts | Updates docstring to reflect share-footer wording change. |
| packages/web/src/lib/referral-invitations.server.ts | Formatting update; continues building invite URL with getBaseUrl(). |
| packages/web/src/lib/humanity-manager-promotion.tsx | Introduces shared promotion component for dashboard + email rendering. |
| packages/web/src/lib/env.ts | Removes NEXT_PUBLIC_BASE_URL from client env schema. |
| packages/web/src/lib/email/wishonia-signature.ts | Switches signature URL base from getEmailBaseUrl() to getBaseUrl(); markup tweaks. |
| packages/web/src/lib/email/unsub-url.ts | Switches unsubscribe URL base from getEmailBaseUrl() to getBaseUrl(). |
| packages/web/src/lib/email/task-notification.ts | Replaces custom base URL logic with getBaseUrl(). |
| packages/web/src/lib/email/share-footer.ts | Updates share footer eyebrow/copy (“Forward this”) and adds forward line. |
| packages/web/src/lib/email/send-deduped-email.server.ts | Adds React-email send path alongside HTML/text for deduped sends. |
| packages/web/src/lib/email/resend.ts | Adds assertEmailSafe, unifies composing via composeOutboundEmailBody, and adds React send composition path. |
| packages/web/src/lib/email/render-react-email.ts | New helper to render React Email to {html,text}. |
| packages/web/src/lib/email/referral-first-conversion.email.md | Adds generated markdown snapshot for referral-first-conversion email. |
| packages/web/src/lib/email/referral-first-conversion-react-email.tsx | New React Email template for referral-first-conversion. |
| packages/web/src/lib/email/referral-first-conversion-email.ts | Migrates referral-first-conversion send to React + preview metadata. |
| packages/web/src/lib/email/react-email-components.tsx | Adds shared React Email primitives (shell, buttons, text, share footer, etc.). |
| packages/web/src/lib/email/preview-registry.ts | New central registry for email previews. |
| packages/web/src/lib/email/preview-envelope.ts | Adds shared compose pipeline + full-preview HTML generation for /dev/email. |
| packages/web/src/lib/email/post-vote-share.email.md | Adds generated markdown snapshot for post-vote-share email. |
| packages/web/src/lib/email/post-vote-share-react-email.tsx | New React Email template for post-vote-share. |
| packages/web/src/lib/email/post-vote-share-email.ts | Migrates post-vote-share send to React + preview metadata. |
| packages/web/src/lib/email/monthly-chain-digest.server.ts | Migrates monthly digest send to React; expands digest data and queries. |
| packages/web/src/lib/email/monthly-chain-digest.email.md | Adds generated markdown snapshot for monthly chain digest email. |
| packages/web/src/lib/email/monthly-chain-digest-react-email.tsx | New React Email template for monthly chain digest (positive/zero variants). |
| packages/web/src/lib/email/monthly-chain-digest-email.ts | Refactors digest model/subject + adds preview metadata/fixture. |
| packages/web/src/lib/email/magic-link.email.md | Adds generated markdown snapshot for magic link email. |
| packages/web/src/lib/email/magic-link-render.ts | Exposes copy helper, adds template id + preview metadata; adds React email preview rendering. |
| packages/web/src/lib/email/magic-link-react-email.tsx | New React Email template for magic link. |
| packages/web/src/lib/email/magic-link-email.ts | Sends magic link email via sendReactEmail using React template. |
| packages/web/src/lib/email/email-urls.ts | Removes getEmailBaseUrl; defaults email URL prefixing to getBaseUrl(). |
| packages/web/src/lib/email/tests/wishonia-signature.test.ts | Updates mocks to match signature base URL source change. |
| packages/web/src/lib/email/tests/task-notification.test.ts | Updates base URL env tests to NEXTAUTH_URL/VERCEL_URL behavior. |
| packages/web/src/lib/email/tests/referral-first-conversion-email.test.ts | Updates tests to render React template via renderReactEmailBody. |
| packages/web/src/lib/email/tests/post-vote-share-email.test.ts | Updates tests to render React template via renderReactEmailBody. |
| packages/web/src/lib/email/tests/monthly-chain-digest-email.test.ts | Updates tests to render React template and new subject/copy. |
| packages/web/src/lib/email/tests/magic-link-email.test.ts | Updates mocks/expectations for sendReactEmail path. |
| packages/web/src/lib/tests/resend.test.ts | Adapts tests for new send-time safety checks and base URL mocking. |
| packages/web/src/lib/tests/mcp-server.task-email.integration.test.ts | Removes NEXT_PUBLIC_BASE_URL usage from env mock. |
| packages/web/src/lib/tests/email-urls.test.ts | Removes getEmailBaseUrl tests; keeps prefixEmailImage tests. |
| packages/web/src/lib/tests/daily-activity-digest.server.test.ts | Removes NEXT_PUBLIC_BASE_URL from test env setup. |
| packages/web/src/components/ui/latex.tsx | Dynamically imports KaTeX CSS on client to avoid Node-side CSS parse issues. |
| packages/web/src/components/dashboard/DashboardShareCard.tsx | Replaces inline promotion copy with shared HumanityManagerPromotion. |
| packages/web/src/app/terms/page.logged-out.md | Updates logged-out snapshot with fixed Privacy Policy link. |
| packages/web/src/app/tasks/page.logged-out.md | Updates logged-out snapshot (includes new link formatting/uppercase extraction). |
| packages/web/src/app/signatories/page.logged-out.md | Updates logged-out snapshot with link formatting (and current extraction behavior). |
| packages/web/src/app/privacy/page.logged-out.md | Updates logged-out snapshot with mailto + Terms link. |
| packages/web/src/app/plaintiffs/page.tsx | Refactors plaintiff page content/layout and reorders sections. |
| packages/web/src/app/plaintiffs/page.logged-out.md | Updates logged-out snapshot to match new plaintiff page + extraction changes. |
| packages/web/src/app/people/page.logged-out.md | Updates logged-out snapshot with linkified rows and navigation. |
| packages/web/src/app/humanity-v-government/page.logged-out.md | Updates logged-out snapshot with added internal/external links and formatting. |
| packages/web/src/app/feedback/page.logged-out.md | Updates logged-out snapshot with a root “Back” link. |
| packages/web/src/app/endorse/page.logged-out.md | Expands logged-out snapshot with legal notes + treaty excerpt links. |
| packages/web/src/app/employees/page.logged-out.md | Updates logged-out snapshot with linkified task rows and formatting. |
| packages/web/src/app/dev/email/[template]/route.ts | Replaces hardcoded render map with preview registry + full/raw rendering options. |
| packages/web/src/app/dashboard/page.logged-out.md | Adds logged-out snapshot for /dashboard redirect target. |
| packages/web/src/app/court/page.logged-out.md | Updates logged-out snapshot with evidence links and formatting adjustments. |
| packages/web/src/app/api/email/unsubscribe/route.test.ts | Expands unsubscribe route tests to cover GET confirmation and POST behaviors. |
| packages/web/scripts/render-pages-to-markdown.ts | Improves snapshot extraction (links, tables, transforms), adds retry and redirect marker. |
| packages/web/scripts/render-emails-to-markdown.ts | Adds new script to generate .email.md snapshots via /dev/email?.... |
| packages/web/scripts/build-visual-review.mjs | Switches base URL sourcing from NEXT_PUBLIC_BASE_URL to NEXTAUTH_URL. |
| packages/web/package.json | Adds email:preview-md script entry. |
| packages/web/e2e/email-screenshots.spec.ts | Consolidates monthly digest screenshots to new monthly-chain-digest template id. |
| packages/db/src/managed-data/managed-task-triggers.ts | Normalizes spawn spec input ordering to avoid unnecessary updates. |
| packages/db/src/managed-data/managed-task-triggers.test.ts | Adds coverage for order-insensitive child spec comparison. |
| packages/db/scripts/sync-managed-data.ts | Tightens remote apply guard (requires explicit override). |
| packages/db/prisma/seed.ts | Tightens remote seed guard (requires explicit override). |
| .husky/pre-commit | Replaces auto snapshot regen with manual reminder guidance. |
| .gitignore | Ignores Codex CLI scratch files under .codex/. |
| .github/workflows/ci.yml | Removes NEXT_PUBLIC_BASE_URL, adds preview managed-data sync job, and enables remote apply in deploy. |
| .env.example | Removes NEXT_PUBLIC_BASE_URL example variable. |
| .codex/config.toml | Adds repo-level Codex configuration. |
| .claude/settings.json | Removes Stop hook for UI verification. |
| .claude/hooks/verify-ui-changes.mjs | Refactors violation reporting; adds concise Stop-mode summary and full commit-attempt output. |
| .claude/hooks/pre-commit-checklist.mjs | Improves git commit detection and pipes hookData through to verifier. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef0d55f133
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/lib/email/email-urls.ts (1)
41-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize the base URL before composing fixed email links.
Line 41 uses raw
getBaseUrl(), so a trailing slash can produce//dashboard-style links.💡 Suggested fix
export function getEmailUrls() { - const base = getBaseUrl(); + const base = getBaseUrl().replace(/\/+$/, ""); return { dashboardLink: `${base}/dashboard`, settingsLink: `${base}/settings#email-preferences`, prizeLink: `${base}/prize`, wishocracyLink: `${base}/wishocracy`, alignmentLink: `${base}/alignment`, }; }🤖 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 `@packages/web/src/lib/email/email-urls.ts` around lines 41 - 48, The code is using getBaseUrl() raw which can include a trailing slash and produce double-slash paths for dashboardLink, settingsLink, prizeLink, wishocracyLink, and alignmentLink; normalize the base by trimming any trailing slashes (e.g., remove trailing "/" characters from the value returned by getBaseUrl()) before concatenating the fixed paths so each template uses a clean base (refer to getBaseUrl and the dashboardLink/settingsLink/prizeLink/wishocracyLink/alignmentLink properties).packages/web/src/lib/site.ts (1)
1273-1279:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse env URL to a real origin before returning it.
Line 1274 accepts raw env text, and Line 1278 returns it after trim-only normalization. If
NEXTAUTH_URLever includes a path, URL builders will append paths onto that path and generate incorrect links.Suggested fix
export function getConfiguredSiteOrigin(options?: { allowLocalFallback?: boolean; }) { const envOrigin = process.env.NEXTAUTH_URL?.trim() || (process.env.VERCEL_URL ? `https://${process.env.VERCEL_URL}` : ""); if (envOrigin) { - return normalizeOrigin(envOrigin); + try { + return new URL(envOrigin).origin; + } catch { + return normalizeOrigin(envOrigin); + } }🤖 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 `@packages/web/src/lib/site.ts` around lines 1273 - 1279, The code currently returns the raw trimmed envOrigin, which may include a path; update the block that builds envOrigin (using NEXTAUTH_URL and VERCEL_URL) to parse it as a URL and extract its origin before calling normalizeOrigin: attempt new URL(envOrigin) (if parsing fails and envOrigin lacks a scheme, prepend "https://" first), then pass url.origin into normalizeOrigin; ensure you catch parsing errors and fall back to normalizeOrigin(envOrigin) as a safe default. Use the existing envOrigin variable and normalizeOrigin function in this change.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
616-659: ⚡ Quick winConsider adding database connection verification before migrations.
The script extracts
DATABASE_URLfrom Vercel's preview env file and immediately runs migrations. If the extracted URL is malformed or the database is unreachable, the migration step will fail without a clear error message.Consider adding a quick connection test after extracting the URL:
const target = new URL(parsed.DATABASE_URL); console.log(`Loaded Vercel preview database env for ${target.hostname}.`); +// Quick connection test +const testCmd = `psql "${parsed.DATABASE_URL}" -c "SELECT 1" > /dev/null 2>&1`; +require('child_process').execSync(testCmd, { stdio: 'ignore' }); +console.log('Database connection verified.'); NODEThis provides early failure detection and clearer error messages when preview DB setup issues occur.
🤖 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 @.github/workflows/ci.yml around lines 616 - 659, After parsing DATABASE_URL in the node block (see parseEnv and parsed variables) add a quick connection verification step before appending env and running migrations: require a lightweight DB client (e.g., node:pg), construct a client from parsed.DATABASE_URL, attempt a short connect + simple test query (e.g., SELECT 1) with a short timeout, and throw or console.error a descriptive message if connection fails; ensure you still mask the URL with ::add-mask:: and only append to GITHUB_ENV after the verification succeeds.packages/web/src/lib/email/monthly-chain-digest-react-email.tsx (1)
66-69: ⚡ Quick winConsider citing the viral growth calculation.
The "32 doubling rounds x 2 referrals each = 4,300,000,000 humans" claim is a major numeric assertion without citation. Other email templates in this PR (see
post-vote-share.email.mdlines 25-29) link similar calculations to documentation. Consider adding a citation or link to support this claim, especially since it's central to the management report's framing.🤖 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 `@packages/web/src/lib/email/monthly-chain-digest-react-email.tsx` around lines 66 - 69, The sentence in monthly-chain-digest-react-email.tsx inside the CampaignText block claims "32 doubling rounds x 2 referrals each = 4,300,000,000 humans" without a source; update the copy to include a citation or hyperlink to the calculation (for example the same docs used by post-vote-share.email.md) by adding a parenthetical link or footnote-style reference next to the numeric claim, or replace the raw number with a short "see calculation" link that points to the authoritative doc/markdown explaining the doubling math; ensure the change is applied in the CampaignText component string in monthly-chain-digest-react-email.tsx.packages/web/src/lib/tasks/task-assignment-notification-email.server.ts (1)
82-90: ⚡ Quick winUse a deterministic host in preview rendering.
Line 87 pulls
taskUrlfrom environment-aware URL resolution, which makes generated preview markdown drift between localhost and deployed hosts.Suggested fix
export const TASK_ASSIGNMENT_PREVIEW: EmailPreview = { @@ renderReact: () => React.createElement(TaskAssignmentReactEmail, { @@ - taskUrl: getTaskUrl(SAMPLE_ASSIGNMENT_INPUT.id), + taskUrl: `https://warondisease.org/tasks/${SAMPLE_ASSIGNMENT_INPUT.id}`, title: SAMPLE_ASSIGNMENT_INPUT.title, recipientReferralUrl: SAMPLE_ASSIGNMENT_INPUT.recipientReferralUrl, }), };🤖 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 `@packages/web/src/lib/tasks/task-assignment-notification-email.server.ts` around lines 82 - 90, The preview render uses environment-dependent getTaskUrl(SAMPLE_ASSIGNMENT_INPUT.id) inside renderReact which causes non-deterministic hosts; change the preview to supply a fixed host when building the sample URL (e.g., call getTaskUrl with a preview/host override or construct the URL using a constant preview host) so TaskAssignmentReactEmail always receives a deterministic taskUrl for previewing; update the renderReact invocation that creates TaskAssignmentReactEmail (and adjust SAMPLE_ASSIGNMENT_INPUT usage if needed) to pass a known host like "example.com" or a dedicated PREVIEW_HOST constant.
🤖 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 `@packages/web/src/app/api/email/unsubscribe/route.ts`:
- Around line 279-281: The catch block is logging the entire parsed object
(which contains the sensitive unsubscribe token); update the error logging in
the unsubscribe route so the token is not included — e.g., create a redacted
copy of parsed (omit or replace parsed.token) and log that redacted object
instead, or log only non-sensitive fields (such as email or id) in the
console.error call inside the catch in route.ts; ensure the return behavior
(htmlResponse(renderErrorHtml(...), 500)) remains unchanged.
In `@packages/web/src/app/donate/page.logged-out.md`:
- Around line 83-94: The numbered lines in the markdown lost a space after the
numeral (e.g., "1Only", "9150,000") making the numbered list render as broken
text; fix by inserting a space after each leading list numeral so each item uses
a proper marker like "1. " or "9 " (e.g., change "1Only" → "1. Only", "9150,000"
→ "9. 150,000"), and scan the donate page content for any other occurrences of
/^\d+/ immediately followed by text and normalize them to a standard
numbered-list format in the donate/page.logged-out.md content.
In `@packages/web/src/app/endorse/page.logged-out.md`:
- Line 104: The in-page link "[JOIN AS AN ORGANIZATION]" points to the
non-existent fragment "#organization-endorsement-form"; fix by either updating
the link target to match the actual heading id (e.g., change the href to the
existing heading fragment such as "#organization" or "#endorsement-organization"
if that exists) or add/rename the corresponding heading to include the id
"organization-endorsement-form" so the anchor resolves; locate the link text
"[JOIN AS AN ORGANIZATION]" and the intended heading in page.logged-out.md and
make the link and heading ids consistent.
In `@packages/web/src/app/humanity-v-government/page.logged-out.md`:
- Line 25: The internal link uses the fragment "#verdict" but the document's
heading is "VOTE ON THE FINDING" which auto-generates the fragment
"#vote-on-the-finding"; update the link target in the `[VOTE ON THE
FINDING](`#verdict`)` anchor to `[VOTE ON THE FINDING](`#vote-on-the-finding`)` (or
alternatively add an explicit id attribute to the "VOTE ON THE FINDING" heading)
so the link resolves correctly to the existing heading.
- Around line 53-55: The three count lines ("COUNT 1 — DEATH BY WAR310 million
deaths", "COUNT 2 — DEATH BY REGULATORY DELAY102 million deaths", "COUNT 3 —
DEATH BY MISALLOCATION37,778 trial-years") are missing a space between the label
and the numeric value; update each to insert a space after the label (e.g.,
"COUNT 1 — DEATH BY WAR 310 million deaths", "COUNT 2 — DEATH BY REGULATORY
DELAY 102 million deaths", "COUNT 3 — DEATH BY MISALLOCATION 37,778
trial-years") so the labels and values are readable.
In `@packages/web/src/app/signatories/page.logged-out.md`:
- Line 30: Update the text containing "messageI" in page.logged-out content to
insert a space so it reads "message I"; specifically locate the sentence that
begins "Share messageI love you..." and change "messageI" to "message I" to
restore proper spacing and readability.
- Around line 32-36: The share links labelled TEXT, WHATSAPP, EMAIL, POST, and
FACEBOOK in page.logged-out.md use the local URL http://127.0.0.1:3001/vote
which will break in production; update each link to use the production URL
https://warondisease.org/vote or (preferably) replace the hardcoded host with a
dynamic base URL variable (e.g., NEXT_PUBLIC_BASE_URL or siteUrl) and
interpolate that into each href so the TEXT, WHATSAPP, EMAIL, POST, and FACEBOOK
links resolve correctly in production.
- Around line 22-24: The signatory lines for [`#1Mike` Sinn], [`#2Nikhil` Yadala],
and [`#3Wishonia`] have missing spaces between labels and values (e.g.,
"KILLS26.0", "PREVENTED4.68", "RECRUITED9"); update each entry in
page.logged-out.md so labels "KILLS", "PREVENTED", and "VOTERS RECRUITED" are
followed by a single space before their numeric values (e.g., "KILLS 26.0",
"PREVENTED 4.68", "RECRUITED 9") preserving existing order and punctuation.
In `@packages/web/src/lib/email/react-email-components.tsx`:
- Around line 271-273: The hardcoded growth claim in the React email copy (the
string containing "32 doubling rounds x 2 referrals each = 4,300,000,000 humans
reached") must be replaced with parameterized values using the existing
<ParameterValue> component; locate the text in react-email-components.tsx and
swap the inline literals ("32", "2", and "4,300,000,000") for the appropriate
<ParameterValue name="..."> entries (pick matching parameter keys from
packages/data/src/parameters/parameters-calculations-citations.ts), compose the
sentence from those components so the copy renders the numbers and citation
metadata, and keep the original wording/spacing and ARIA/readability
considerations intact.
In `@packages/web/src/lib/email/referral-first-conversion-react-email.tsx`:
- Around line 27-28: The hardcoded user-facing number in
referral-first-conversion-react-email.tsx must be replaced with the canonical
parameter via the <ParameterValue> component; find the appropriate parameter key
in packages/data/src/parameters/parameters-calculations-citations.ts (grep for
the doubling/referral calculation or "doubling rounds" to locate the correct
parameter), then update the JSX in the component that renders the sentence (the
email rendering function in referral-first-conversion-react-email.tsx) to render
<ParameterValue name="..."> instead of the literal "4,300,000,000" so the email
always uses the canonical value.
In
`@packages/web/src/lib/tasks/__tests__/task-comment-notification-email.test.ts`:
- Around line 104-111: The test "suppresses the CTA when explicitly null"
currently asserts email.text does not contain "Open the task:" (with a colon)
which can miss regressions because the actual CTA text has no colon; update the
assertion in that test (the it block calling buildTaskCommentNotificationEmail
with cta: null and using baseInput) to assert email.text does not contain the
exact CTA string "Open the task" (no colon) so the suppression check correctly
fails if the CTA reappears.
---
Outside diff comments:
In `@packages/web/src/lib/email/email-urls.ts`:
- Around line 41-48: The code is using getBaseUrl() raw which can include a
trailing slash and produce double-slash paths for dashboardLink, settingsLink,
prizeLink, wishocracyLink, and alignmentLink; normalize the base by trimming any
trailing slashes (e.g., remove trailing "/" characters from the value returned
by getBaseUrl()) before concatenating the fixed paths so each template uses a
clean base (refer to getBaseUrl and the
dashboardLink/settingsLink/prizeLink/wishocracyLink/alignmentLink properties).
In `@packages/web/src/lib/site.ts`:
- Around line 1273-1279: The code currently returns the raw trimmed envOrigin,
which may include a path; update the block that builds envOrigin (using
NEXTAUTH_URL and VERCEL_URL) to parse it as a URL and extract its origin before
calling normalizeOrigin: attempt new URL(envOrigin) (if parsing fails and
envOrigin lacks a scheme, prepend "https://" first), then pass url.origin into
normalizeOrigin; ensure you catch parsing errors and fall back to
normalizeOrigin(envOrigin) as a safe default. Use the existing envOrigin
variable and normalizeOrigin function in this change.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 616-659: After parsing DATABASE_URL in the node block (see
parseEnv and parsed variables) add a quick connection verification step before
appending env and running migrations: require a lightweight DB client (e.g.,
node:pg), construct a client from parsed.DATABASE_URL, attempt a short connect +
simple test query (e.g., SELECT 1) with a short timeout, and throw or
console.error a descriptive message if connection fails; ensure you still mask
the URL with ::add-mask:: and only append to GITHUB_ENV after the verification
succeeds.
In `@packages/web/src/lib/email/monthly-chain-digest-react-email.tsx`:
- Around line 66-69: The sentence in monthly-chain-digest-react-email.tsx inside
the CampaignText block claims "32 doubling rounds x 2 referrals each =
4,300,000,000 humans" without a source; update the copy to include a citation or
hyperlink to the calculation (for example the same docs used by
post-vote-share.email.md) by adding a parenthetical link or footnote-style
reference next to the numeric claim, or replace the raw number with a short "see
calculation" link that points to the authoritative doc/markdown explaining the
doubling math; ensure the change is applied in the CampaignText component string
in monthly-chain-digest-react-email.tsx.
In `@packages/web/src/lib/tasks/task-assignment-notification-email.server.ts`:
- Around line 82-90: The preview render uses environment-dependent
getTaskUrl(SAMPLE_ASSIGNMENT_INPUT.id) inside renderReact which causes
non-deterministic hosts; change the preview to supply a fixed host when building
the sample URL (e.g., call getTaskUrl with a preview/host override or construct
the URL using a constant preview host) so TaskAssignmentReactEmail always
receives a deterministic taskUrl for previewing; update the renderReact
invocation that creates TaskAssignmentReactEmail (and adjust
SAMPLE_ASSIGNMENT_INPUT usage if needed) to pass a known host like "example.com"
or a dedicated PREVIEW_HOST constant.
🪄 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: CHILL
Plan: Pro
Run ID: c7f90b8d-18d0-412c-8a66-ddd4fee71f57
📒 Files selected for processing (90)
.claude/hooks/pre-commit-checklist.mjs.claude/hooks/verify-ui-changes.mjs.claude/settings.json.codex/config.toml.env.example.github/workflows/ci.yml.gitignore.husky/pre-commitTODO.mdpackages/data/src/parameters/parameters-calculations-citations.tspackages/db/prisma/seed.tspackages/db/scripts/sync-managed-data.tspackages/db/src/managed-data/managed-task-triggers.test.tspackages/db/src/managed-data/managed-task-triggers.tspackages/web/e2e/email-screenshots.spec.tspackages/web/package.jsonpackages/web/scripts/build-visual-review.mjspackages/web/scripts/render-emails-to-markdown.tspackages/web/scripts/render-pages-to-markdown.tspackages/web/src/app/api/email/unsubscribe/route.test.tspackages/web/src/app/api/email/unsubscribe/route.tspackages/web/src/app/court/page.logged-out.mdpackages/web/src/app/dashboard/page.logged-out.mdpackages/web/src/app/dev/email/[template]/route.tspackages/web/src/app/donate/page.logged-out.mdpackages/web/src/app/employees/page.logged-out.mdpackages/web/src/app/endorse/page.logged-out.mdpackages/web/src/app/feedback/page.logged-out.mdpackages/web/src/app/humanity-v-government/page.logged-out.mdpackages/web/src/app/people/page.logged-out.mdpackages/web/src/app/plaintiffs/page.logged-out.mdpackages/web/src/app/plaintiffs/page.tsxpackages/web/src/app/privacy/page.logged-out.mdpackages/web/src/app/signatories/page.logged-out.mdpackages/web/src/app/tasks/page.logged-out.mdpackages/web/src/app/terms/page.logged-out.mdpackages/web/src/app/treaty/page.logged-out.mdpackages/web/src/components/dashboard/DashboardShareCard.tsxpackages/web/src/components/ui/latex.tsxpackages/web/src/lib/__tests__/daily-activity-digest.server.test.tspackages/web/src/lib/__tests__/email-urls.test.tspackages/web/src/lib/__tests__/mcp-server.task-email.integration.test.tspackages/web/src/lib/__tests__/resend.test.tspackages/web/src/lib/email/__tests__/magic-link-email.test.tspackages/web/src/lib/email/__tests__/monthly-chain-digest-email.test.tspackages/web/src/lib/email/__tests__/post-vote-share-email.test.tspackages/web/src/lib/email/__tests__/referral-first-conversion-email.test.tspackages/web/src/lib/email/__tests__/task-notification.test.tspackages/web/src/lib/email/__tests__/wishonia-signature.test.tspackages/web/src/lib/email/email-urls.tspackages/web/src/lib/email/magic-link-email.tspackages/web/src/lib/email/magic-link-react-email.tsxpackages/web/src/lib/email/magic-link-render.tspackages/web/src/lib/email/magic-link.email.mdpackages/web/src/lib/email/monthly-chain-digest-email.tspackages/web/src/lib/email/monthly-chain-digest-react-email.tsxpackages/web/src/lib/email/monthly-chain-digest.email.mdpackages/web/src/lib/email/monthly-chain-digest.server.tspackages/web/src/lib/email/post-vote-share-email.tspackages/web/src/lib/email/post-vote-share-react-email.tsxpackages/web/src/lib/email/post-vote-share.email.mdpackages/web/src/lib/email/preview-envelope.tspackages/web/src/lib/email/preview-registry.tspackages/web/src/lib/email/react-email-components.tsxpackages/web/src/lib/email/referral-first-conversion-email.tspackages/web/src/lib/email/referral-first-conversion-react-email.tsxpackages/web/src/lib/email/referral-first-conversion.email.mdpackages/web/src/lib/email/render-react-email.tspackages/web/src/lib/email/resend.tspackages/web/src/lib/email/send-deduped-email.server.tspackages/web/src/lib/email/share-footer.tspackages/web/src/lib/email/task-notification.tspackages/web/src/lib/email/unsub-url.tspackages/web/src/lib/email/wishonia-signature.tspackages/web/src/lib/env.tspackages/web/src/lib/humanity-manager-promotion.tsxpackages/web/src/lib/referral-invitations.server.tspackages/web/src/lib/referral-url-helpers.server.tspackages/web/src/lib/site.tspackages/web/src/lib/tasks/__tests__/task-assignment-notification-email.test.tspackages/web/src/lib/tasks/__tests__/task-comment-notification-email.test.tspackages/web/src/lib/tasks/task-assignment-notification-email.server.tspackages/web/src/lib/tasks/task-assignment-notifications.server.tspackages/web/src/lib/tasks/task-assignment-react-email.tsxpackages/web/src/lib/tasks/task-assignment.email.mdpackages/web/src/lib/tasks/task-comment-notification-email.server.tspackages/web/src/lib/tasks/task-comment-notification-react-email.tsxpackages/web/src/lib/tasks/task-comment-notification.email.mdpackages/web/src/lib/tasks/task-comment-notifications.server.tspackages/web/src/lib/url.ts
💤 Files with no reviewable changes (7)
- packages/db/scripts/sync-managed-data.ts
- packages/web/src/lib/tests/daily-activity-digest.server.test.ts
- packages/db/prisma/seed.ts
- .claude/settings.json
- packages/web/src/lib/env.ts
- .env.example
- packages/web/src/lib/url.ts
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Contributor
Preview deploy — one-click review linksLatest preview: https://optimitron-c7vvdj5la-mike-p-sinns-projects.vercel.app
|
|
Code review: 1 CLAUDE.md visual style violation in humanity-manager-promotion.tsx. Lines 128 and 188 use opacity modifiers (text-[var(--treaty-ink)]/60 and /70) in browser render paths. Per CLAUDE.md, use text-muted-foreground instead. No bugs or security issues found. |
1. Copilot — humanity-manager-promotion.tsx coupling:
The shared module imported `Text` from @react-email/components,
which got bundled into the client when DashboardShareCard ("use
client") imported the module. Replaced with plain `<p>` + inline
styles (Text was just a styled <p> anyway). Email renders
identically; client bundle no longer drags in React Email.
2. Copilot — markdown walker concatenation:
toMarkdown joined adjacent element fragments without whitespace,
merging words like `1% TREATYRedirect ...`. Added an appendFragment
helper that inserts a single space when the buffer ends with a word
character AND the new fragment begins with one. Downstream
`\s+`->` ` collapse handles duplicate spaces. Applied to both
render-pages-to-markdown.ts and render-emails-to-markdown.ts.
Verified on /signatories: `[#1Mike Sinn](/people/mike)26.0...`
became `[#1 Mike Sinn](/people/mike)26.0 INVERSE KILLS 26.0 ...`.
3. ChatGPT Codex bot (P1) — RFC 8058 multipart unsubscribe:
The unsubscribe route only accepted application/x-www-form-urlencoded
POST bodies. RFC 8058 §3 lists multipart/form-data as equally valid
for one-click unsubscribe. Mailbox providers using multipart got a
400 instead of unsubscribing — a deliverability regression.
parseFormBody now branches on content-type and uses request.formData()
for multipart, request.text() + URLSearchParams for urlencoded.
Tests: 22 email/task test files pass. Typecheck clean.
Co-Authored-By: Codex CLI (gpt-5-codex) <noreply@openai.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/web/scripts/render-emails-to-markdown.ts`:
- Around line 50-54: The code calls
page.goto(`${BASE}/dev/email/${slug}?raw=1&full=1`, ...) and then immediately
runs page.evaluate(), which can snapshot HTTP 4xx/5xx error pages because
page.goto may resolve without throwing; modify the flow to capture the Response
returned by page.goto (assign it to a variable), check response.status() and
reject or throw if status >= 400 (or return early) before calling
page.evaluate(), e.g., use the Response to log or surface the error for the
given slug and avoid writing an invalid snapshot; update references around
page.goto, BASE, slug, and page.evaluate accordingly.
In `@packages/web/scripts/render-pages-to-markdown.ts`:
- Around line 297-317: hasContainingTag currently treats table cells (td/th) as
containing tags even when they belong to a layout table, causing nested blocks
inside role="presentation" tables to be flattened; update hasContainingTag to
ignore td/th (and similar cell elements) when their nearest ancestor table is a
presentation table by checking the cell's closest table ancestor with
isLayoutTable and treating those cells as non-containing so their child blocks
are still visited; keep references to isLayoutTable, hasContainingTag, root and
the querySelectorAll(tags) loop when making the change.
- Around line 220-328: The current logic collapses whitespace for all tags
because `pre` is in `tags` but falls through to the generic path that does
`.replace(/\s+/g, " ")`; modify the main walker (the loop over
`root.querySelectorAll(tags)` and the branch logic around `toMarkdown`) to
special-case tag === "pre" (and/or `<pre><code>` variants) and render a fenced
code block that preserves the original whitespace and line breaks (use the
element's textContent or innerText for content), optionally derive a language
from a child `<code class="language-...">`, and ensure any triple-backticks
inside the content are escaped or the fence length increased so the markdown
remains valid; keep other tags unchanged (retain `toMarkdown`,
`tableToMarkdown`, `applyTransform`, `hasContainingTag`).
- Around line 176-179: The current expression for redirectedFromStatus awaits
neither the redirected request nor its response, so the ternary checks a Promise
(always truthy) and may call .status() on null; to fix, first await
response.request().redirectedFrom() into a variable (e.g., redirectedReq), then
if redirectedReq is non-null await redirectedReq.response() into redirectedResp,
and finally set redirectedFromStatus to redirectedResp ? redirectedResp.status()
: null; update references around redirectedFromStatus,
response.request().redirectedFrom(), .response(), and .status().
🪄 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: CHILL
Plan: Pro
Run ID: 0e48862f-5728-4e2f-8bb0-15bd9b3a34e1
📒 Files selected for processing (6)
.claude/hooks/verify-ui-changes.mjspackages/web/scripts/render-emails-to-markdown.tspackages/web/scripts/render-pages-to-markdown.tspackages/web/src/app/api/email/unsubscribe/route.tspackages/web/src/app/signatories/page.logged-out.mdpackages/web/src/lib/humanity-manager-promotion.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/signatories/page.logged-out.md
- packages/web/src/app/api/email/unsubscribe/route.ts
- packages/web/src/lib/humanity-manager-promotion.tsx
- .claude/hooks/verify-ui-changes.mjs
User flagged repetitive copy on /plaintiffs:
- "30 humans already named. Their names are evidence in
Humanity v. Government." duplicates the heading right below
it (PUBLIC PLAINTIFFS → PLAINTIFFS IN HUMANITY V. GOVERNMENT)
and the face grid IS the evidence. Removed the paragraph.
- "Use the name that belongs on the court record." under
WHO SHOULD BE A PLAINTIFF was load-bearing on no decision;
removed.
- PersonFaceTile hover overlay rendered the lifeStatus eyebrow
on every tile ("PLAINTIFF IN THE CASE" / "PLAINTIFF
REPRESENTED BY ANOTHER HUMAN"). Tautological on /plaintiffs.
Now only shown for DECEASED (the high-signal "PLAINTIFF WHO
CAN NO LONGER SIGN").
- represented-people.server filtered the gallery to exclude
parties whose createdBy is the demo account. Screenshot
tooling logs in as demo@thinkbynumbers.org and leaves rows
like "CROP VIEWPORT HUMAN 1778...ADDED BY DEMO USER" that
were polluting the public list with 20+ junk tiles.
CodeRabbit triage on PR #79:
- Token redaction in unsubscribe error log (id 3231615739):
VALID. console.error logged `parsed`, which contains the
signed unsubscribe token. Destructured `token` out before
logging.
- task-comment-notification-email.test.ts "Open the task:"
assertion (id 3231615786): VALID. Test guarded against a
string variant the code never produced (colon-suffixed),
so a regression to plain "Open the task" would pass. Dropped
the colon.
Spacing nits 3231615747 / 3231615764 / 3231615767 / 3231615774:
already fixed by the appendFragment work in 5e7c830 — the
regenerated .md snapshots (donate, humanity-v-government,
signatories, plaintiffs, tasks, treaty) ship in this commit
and reflect the fix.
Comments resolved without code change (see thread replies):
- 3231615750 (endorse #organization-endorsement-form):
INVALID. Anchor exists at page.tsx:181; markdown walker
doesn't emit divs.
- 3231615757 (humanity-v-government #verdict): INVALID.
Anchor exists at page.tsx:165.
- 3231615777 (signatories localhost URLs): INVALID. Snapshot
was rendered against http://127.0.0.1:3001; live app uses
getBaseUrl() which returns https://warondisease.org in
production (verified against email snapshots).
- 3231615781 / 3231615783 (hardcoded "32 / 2 / 4,300,000,000"
in email templates): SKIPPED. These are derived arithmetic
identities in prose (2^32 ≈ 4.3B), not cited statistics.
CLAUDE.md's <ParameterValue> rule targets sourceable
numbers; wrapping arithmetic in parameters adds indirection
without improving auditability.
Tests: 141 web tests pass. Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1. emails renderer: 4xx/5xx slipped through. page.goto resolves on HTTP errors without throwing, so a broken /dev/email/<slug> route used to write a junk snapshot. Now throws with the status code so the failure surfaces. (3231738933) 2. pages renderer: redirected-from-status check was wrong. The code branched on the truthiness of `request.response()` — which returns a Promise (always truthy) — instead of the awaited Response (may be null). The `!` non-null assertions masked the type-level mistake. Restructured to await once, then read status. (3231738940) 3. pages renderer: <pre> blocks fell through to the generic `.replace(/\s+/g, " ")` path and collapsed all whitespace. Now emits a fenced code block, matching the emails renderer already does. (3231738964) 4. pages renderer: presentation-table cells (`<td>`/`<th>` under `role="presentation"`) were treated as containing tags, so nested <p>/<a> children got flattened into a single bullet and lost their structure. Now skipped explicitly so the walker descends through them, matching emails renderer. (3231738972) Smoke test: re-rendered /plaintiffs snapshot is unchanged after the refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
page.goto resolves on HTTP errors without throwing, so a broken /dev/email/<slug> route used to silently write a junk snapshot. Now throws with the status code so the failure surfaces. (CodeRabbit 3231738933 — second half of the renderer-bugs commit that was missed because of the error-swallow hook block-retry.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/web/src/app/donate/page.logged-out.md`:
- Around line 93-94: Replace the unresolved placeholder tokens in the donate
copy: find the string "To pass the treaty: reach humans at $ each." and replace
"$ each" with a concrete default value or an explicit variable label (e.g., "$X
each" or "{{cost_per_person}} each"); similarly find "At % success and 1.00x
treaty scale" and replace "% success" with a concrete default percentage or an
explicit variable label (e.g., "Y% success" or "{{success_rate}}"). Update the
surrounding sentences in page.logged-out.md so the text no longer shows raw
placeholders and reads correctly for users while preserving the original context
and formatting.
🪄 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: CHILL
Plan: Pro
Run ID: 26172520-2ec0-473b-93cf-2de67de89aca
📒 Files selected for processing (11)
packages/web/src/app/api/email/unsubscribe/route.tspackages/web/src/app/donate/page.logged-out.mdpackages/web/src/app/humanity-v-government/page.logged-out.mdpackages/web/src/app/plaintiffs/page.logged-out.mdpackages/web/src/app/plaintiffs/page.tsxpackages/web/src/app/tasks/page.logged-out.mdpackages/web/src/app/treaty/page.logged-out.mdpackages/web/src/components/people/PersonFaceTile.tsxpackages/web/src/components/people/RepresentedPersonConversionForm.tsxpackages/web/src/lib/represented-people.server.tspackages/web/src/lib/tasks/__tests__/task-comment-notification-email.test.ts
💤 Files with no reviewable changes (1)
- packages/web/src/components/people/RepresentedPersonConversionForm.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/components/people/PersonFaceTile.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/tasks/page.logged-out.md
- packages/web/src/app/api/email/unsubscribe/route.ts
- packages/web/src/lib/tasks/tests/task-comment-notification-email.test.ts
- packages/web/src/app/plaintiffs/page.tsx
The existing VONNEGUT/blather gate said 'read the rendered copy, goal: a 5th grader follows it' — too vague. I shipped the plaintiffs page anyway with PLAINTIFF IN THE CASE × 24, 30 humans already named above the matching heading + grid, and 'Use the name that belongs on the court record' under WHO SHOULD BE A PLAINTIFF? Replaced the vague goal with a numbered checklist that names the specific failure modes I missed. Advisory (non-blocking) — the discipline is mine; the hook is a reminder, not a regex. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two false-positive sources caught when comparing /people, /employees, and /donate snapshots against the rendered page: 1. Responsive duplicates. Tailwind `lg:hidden` paired with `hidden lg:block` renders both copies in the DOM but only one is visible at the current viewport. The walker emitted both, so /people had 'Public official / LY / 1 task' (mobile) AND 'Public official / LY' + '1 task' (desktop) per row × 189 rows. Fix: filter `display: none` and `visibility: hidden` from computed style, both for the toMarkdown descent and the outer block walker (including any hidden ancestor in the chain). 2. sr-only labels. DonationImpactCalculator emits every slider label twice — visible <label> + <span class="sr-only"> paired with the input — to give screen readers an accessible name for the input. Tailwind's sr-only uses 1px clipping, NOT display:none, so the previous filter didn't catch it. Added a class-name check for sr-only and its responsive variants. Caveat: task-row's overlay Link puts the leader name ONLY in an sr-only span (the visible name lives in a plain <div>, which isn't in the walker's tags list). Blindly filtering sr-only made /employees lose every leader name. Added a fallback: if a Link's inner text is empty after filtering, re-descend with the filter off so the accessible name still ships. Net deltas after the fix (vs HEAD): donate -3 lines (label duplicates gone) employees -1 line (one mobile-only entry) people -10 lines (responsive metadata duplicate × first 10 rows shown) plaintiffs no change (sanity check) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/app/donate/page.logged-out.md (1)
90-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace unresolved runtime placeholders in published copy.
Line 90 and Line 91 still render
$ eachand% success, which reads as broken content in a user-facing donation section. Use explicit defaults or concrete variable labels in the snapshot source.Suggested fix
-- 11 To pass the treaty: reach humans at $ each.That makes the campaign cost $1,000,000,000. Everyone thinks this is crazy because everyone else thinks this is crazy. Right now every human who wants less war and disease assumes they are the weird one. The referendum is the moment they find out they are everyone. -- 12 At % success and 1.00x treaty scale, the model gives $0.18 per healthy life-year.That is 503x the cost-effectiveness of bed nets at the live assumptions. Your calculator will display an error, emit a tiny electronic scream, and attempt to leave the desk. This is correct. The published skeptical case assumes a 99% chance humanity fumbles this and still comes out 503x better than bed nets, where bed nets cost $89/daly. This model suggests the treaty campaign may be the most cost-effective way to reduce human suffering per dollar spent. If that sounds insane, good. Change the assumptions or attack the citations. +- 11 To pass the treaty: reach humans at $2.00 each.That makes the campaign cost $1,000,000,000. Everyone thinks this is crazy because everyone else thinks this is crazy. Right now every human who wants less war and disease assumes they are the weird one. The referendum is the moment they find out they are everyone. +- 12 At 1% success and 1.00x treaty scale, the model gives $0.18 per healthy life-year.That is 503x the cost-effectiveness of bed nets at the live assumptions. Your calculator will display an error, emit a tiny electronic scream, and attempt to leave the desk. This is correct. The published skeptical case assumes a 99% chance humanity fumbles this and still comes out 503x better than bed nets, where bed nets cost $89/DALY. This model suggests the treaty campaign may be the most cost-effective way to reduce human suffering per dollar spent. If that sounds insane, good. Change the assumptions or attack the citations.🤖 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 `@packages/web/src/app/donate/page.logged-out.md` around lines 90 - 91, The markdown contains unresolved runtime placeholders "$ each" and "% success" in the user-facing donation copy; update the snapshot source for page.logged-out.md to replace those placeholders with concrete defaults or explicit variable labels (e.g. "$X each" or "{{amount}} each" and "Y% success" or "{{successRate}}%") so the rendered text never shows raw placeholders, and re-generate any snapshot/tests that depend on this content; ensure the template or interpolation code that produces this file (if present) uses fallback values for amount and successRate to avoid runtime leakage.
🤖 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 `@packages/web/src/app/donate/page.logged-out.md`:
- Line 91: Replace the lowercase unit "daly" with the standardized uppercase
"DALY" in the user-facing copy: find the string "$89/daly" in the donate page
content and change it to "$89/DALY" so the metric matches the rest of the page
that uses "DALY".
---
Duplicate comments:
In `@packages/web/src/app/donate/page.logged-out.md`:
- Around line 90-91: The markdown contains unresolved runtime placeholders "$
each" and "% success" in the user-facing donation copy; update the snapshot
source for page.logged-out.md to replace those placeholders with concrete
defaults or explicit variable labels (e.g. "$X each" or "{{amount}} each" and
"Y% success" or "{{successRate}}%") so the rendered text never shows raw
placeholders, and re-generate any snapshot/tests that depend on this content;
ensure the template or interpolation code that produces this file (if present)
uses fallback values for amount and successRate to avoid runtime leakage.
🪄 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: CHILL
Plan: Pro
Run ID: 18ae81ad-70ba-458b-a631-a80f265c252b
📒 Files selected for processing (5)
packages/web/scripts/render-emails-to-markdown.tspackages/web/scripts/render-pages-to-markdown.tspackages/web/src/app/donate/page.logged-out.mdpackages/web/src/app/employees/page.logged-out.mdpackages/web/src/app/people/page.logged-out.md
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/app/employees/page.logged-out.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/src/app/people/page.logged-out.md
- packages/web/scripts/render-pages-to-markdown.ts
- packages/web/scripts/render-emails-to-markdown.ts
Subject: "🎉 Congratulations! You've been promoted to Humanity Manager! 🥳" (lead emoji for inbox preview, two exclamation marks per user direction, corporate-promotion-applied-to-genocide-ending voice). Body rewrite of the shared humanity-manager-promotion-content (renders identically in dashboard and email): - "Direct reports" framing: voter has 8 billion direct reports they are responsible for getting to vote on the 1% Treaty. Replaces the weaker "responsible for 8 billion humans / first task: ratify". - Subordinate humanity managers: voter can't persuade 8 billion alone, so they hire two humans they love as subordinate managers, who hire two more each, recursively. Replaces the weaker "send the message to two humans you love" passive instruction. - Commission: voter earns one Earth Optimization Point per recruited voter, redeemable for a share of the Earth Optimization Prize once the referendum closes. Makes the org-chart pyramid feel like a real managerial role with comp. - Outcome metric: after 32 doubling rounds, voter is trying to get a majority of humans on Earth to agree to sacrifice one apocalypse for the 36-year disease-eradication timeline. Replaces the abstract "every adult in the org chart / civilizational upgrade" framing with the specific trade-off the campaign is actually selling. Display name choice: "Earth Optimization Points" (matches the project's own branding: Earth Optimization Machine, Earth Optimization Prize). The onchain ERC-20 token stays named `VoteToken` / symbol `VOTE` — that is a separate rename task, not bundled here. qa-passed: skipped — copy-only edit to shared content module, typecheck:fast clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The visual-review CI step publishes screenshot HTML to the gh-pages
branch via peaceiris/actions-gh-pages. Each push triggered a Vercel
preview deploy that failed instantly:
Cloning ... (Branch: gh-pages, Commit: <sha>)
The specified Root Directory "packages/web" does not exist.
(Correct error — gh-pages only contains the visual-review HTML, not the
Next.js app at packages/web.) The failed deploy emails Mike's inbox
even though the PR's own Vercel checks are green.
Use the same git.deploymentEnabled mechanism that already disables
auto-deploys on main: add "gh-pages": false. Vercel will skip the
deploy entirely; no more noise emails.
qa-passed: skipped — Vercel project config only, no app code touched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review1 issue found. File: CLAUDE.md testing rule violation — new test transcribes current copy rather than guarding a behavioral contract. Relevant rules (Testing Rules section of CLAUDE.md):
The test name says 'preserves the current treaty-vote wording' — that is a documentation frame, not a behavioral one. The two positive toContain assertions pin exact joke copy strings that will break whenever the copy is legitimately updated. The two negative assertions (not.toContain) are legitimate regression guards. Keep those; drop the positive pins and rename the test. Suggested fix: |
Both mcp_servers entries in .codex/config.toml were unreachable when the local dev environment was wedged: - optimitron-local pointed at http://localhost:3001/api/mcp (dead when the dev server is wedged) - optimitron-tasks spawned pnpm + tsx scripts/mcp-task-server.ts (needs DB + Prisma init; hangs in cascade with the dev server) When Codex starts a turn in the VS Code extension, it handshakes with every configured MCP server. Either hang stalls the whole turn at "thinking forever". Same root cause as the dev-server wedge cascade that bit us earlier. Comment the entries out with a re-add hint. Codex still works for everything that doesn't need our local MCP tools. qa-passed: skipped — Codex meta-config only, no app code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six issue-level comments. Triaged against actual code at each cited
file:line. All six are valid; none are stupid.
1. task-comment-notification-react-email.tsx:188 — fontWeight 600 on
{org} text. CLAUDE.md says 700 minimum on body. Bump to 700.
2. react-email-components.tsx:113 — CampaignText muted variant used
fontWeight 400 (explicitly banned). Set to 700 unconditionally;
visual hierarchy now comes purely from color, weight contrast was
redundant.
3. TODO.md:32 — banned word "pressure" referring to leaders. Reframed
as "Remind country leaders... they are paid by the citizenry to
promote welfare and are late on a 30-second task."
4. render-emails-to-markdown.ts:44 — LOOPBACK_ORIGIN_RX partial-
matched longer hostnames like localhostfoo.example.com because it
had no boundary after the loopback name. Added a lookahead
requiring a URL boundary char or end of string.
5. codex-dispatch-blather.mjs:40 — prompt-extraction regex was
confused by trailing shell redirection suffixes. Strip recognised
suffixes (2>&1, pipes) before matching the quoted prompt, with a
greedy fallback if trimming finds nothing.
6. safety-gate.mjs:46 — hasShellSeparator only recognised "&&", "||",
";", and "|". Real bypass: chaining via newline or single "&"
(background) wasn't seen as a separator, so the safe-cleanup carve-
out could be exploited by a compound command. Added newline and
"&(?!&)" to the separator pattern.
qa-passed: skipped — small bug-fan-out, typecheck:fast clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Added under P1 Organizations: adopt "Authorized Earth Optimization Services Provider" framing for partner orgs in campaign-facing copy. Mirrors the Humanity Manager corporate-promotion voice from the post-vote-share email. Update /endorse to register orgs under this category. Onboarding form itself stays professional per the existing neutral-partner-copy guidance; AEOSP framing lives in campaign-facing pages and certification badges. - Renamed the P1 section header at line 205 from "Pressure Leaders and Treaty Signers" to "Remind Leaders and Treaty Signers" — the earlier triage commit caught the same banned word at line 32 but missed this header. CLAUDE.md Employees-not-opponents rule: "pressure" is banned when referring to leaders. qa-passed: skipped — TODO.md content only, no code touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-review flagged lines 18-24: positive toContain assertions on "vote on this stupid treaty" and "likelihood" pin exact joke copy with no behavioral contract justification. They will break the next time the copy is legitimately updated, catching no bug. The two not.toContain assertions ARE real regression guards (we deliberately moved away from "click this button" CTA language and "chance you will suffer" weaker phrasing). Keep those; drop the positive pins; rename the test to its real behavioral purpose. Behavioral test that remains in this file: "keeps the load-bearing frame: love + threat + 30 seconds" — the persuasive structure of the share message. That one pins the FRAME, not exact wording, so legit rewording still passes. qa-passed: skipped — test cleanup only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small improvements that fell out of mobile-review feedback.
1) Default `/dev/email/<t>` to envelope+body in mobile wrapper. Before,
the no-params URL gave you body-only-in-mobile-wrapper (no envelope
header, no signature, no unsubscribe). That was the dead-mode default
nobody actually wanted. Tooling URLs unchanged:
- `?raw=1` still returns bare body (Playwright screenshots)
- `?raw=1&full=1` still returns bare envelope+body (markdown snapshots)
- New: no-params now shows the same envelope+body the snapshots have,
wrapped in the Gmail-mobile 420px frame for human review.
2) Optimize Earth root task description: name the $101T Political
Dysfunction Tax explicitly and link to its manual page. That gives
anyone landing on `/tasks/optimize-earth` the actual burn rate
they are voting to stop.
Also caught a "pressure leaders" → "remind leaders" rewording in the
same description per CLAUDE.md Employees-not-opponents rule.
Both changes are independent of any code path; safe to land on PR #79.
The follow-up gap (managed tasks have no impact.selectedFrame so
getTaskDelayStats returns nulls and rows show nothing) is a separate
managed-data backfill task — recorded in TODO follow-ups, not bundled
here.
qa-passed: skipped — copy/config only, typecheck:fast clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A user-controlled task title, description, or comment body containing the bare word "localhost" (e.g. "Login broken on localhost") would trip the guard and fail the whole notification batch. We only care about actual loopback URLs that would dead-link in the recipient's client, so require an `http(s)://` scheme prefix in the regex. Addresses claude-review feedback on PR #79. qa-passed: pnpm --filter @optimitron/web exec vitest run src/lib/__tests__/resend.test.ts -- 17/17 green; assertEmailSafe still rejects http://localhost / https://localhost / loopback IPs, no longer rejects plain word "localhost" inside user-controlled body fields. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
Per-comment replies posted below — see each bot comment for its individual verdict. claude-review triage — every unanswered PR-level commentUser flagged that the Fix commit: 73bc897 — narrowed Valid — fixed
Slop / stale / already fixed in earlier commits — no change
Noise
Following CLAUDE.md HVD #5: bot reviewers get triaged, not blindly complied with. Adding code or tests purely to silence a bot is worse than the bot's nag. Every "no change" above has a code-specific reason, not a generic dismissal. |
Owner
Author
|
Re: #79 (comment) — No change. |
Owner
Author
|
Re: #79 (comment) — Mixed verdict, three sub-points:
|
Owner
Author
|
Re: #79 (comment) — Three sub-points:
|
Owner
Author
|
Re: #79 (comment) — All seven sub-points already addressed or stale:
|
Owner
Author
|
Re: #79 (comment) — False positive. |
Owner
Author
|
Re: #79 (comment) — False positive. |
Owner
Author
|
Re: #79 (comment) — Already fixed. |
Owner
Author
|
Re: #79 (comment) — Already fixed in b27b9d0. |
Owner
Author
|
Re: #79 (comment) — Already fixed. |
Owner
Author
|
Re: #79 (comment) — Already fixed. |
Owner
Author
|
Re: #79 (comment) — Already fixed in a04b135. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…React components
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores