Open
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive email infrastructure system, including a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant ServerAction as Server Action
participant Database
participant EmailService as Email Service
participant Provider as SMTP/Ethereal
Client->>ServerAction: Trigger email (register/RSVP/admin)
ServerAction->>Database: Query user email(s)
Database-->>ServerAction: Return email address(es)
ServerAction->>EmailService: sendEmail(email, template)
EmailService->>Provider: mailer.send(payload)
Provider-->>EmailService: Result
EmailService-->>ServerAction: Success/Error
ServerAction-->>Client: Response status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Deploying hackkit with
|
| Latest commit: |
2845846
|
| Status: | 🚫 Build failed. |
Co-authored-by: Matthew Anderson <mjanderson1227@gmail.com>"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
packages/email/tsconfig.json (1)
1-14: Consider extending the shared base configuration.The repository has a shared TypeScript base configuration at
packages/tsconfig/base.jsonthat defines common compiler options likedeclaration,declarationMap, andcomposite. Extending it would reduce duplication and ensure consistent settings across packages.That said,
packages/config/tsconfig.jsonfollows the same standalone pattern, so this may be intentional for certain internal packages.♻️ Proposed refactor to extend base config
{ + "extends": "../tsconfig/base.json", "compilerOptions": { "jsx": "react-jsx", - "esModuleInterop": true, - "forceConsistentCasingInFileNames": true, - "isolatedModules": true, - "moduleResolution": "node", - "preserveWatchOutput": true, - "skipLibCheck": true, - "noEmit": true, - "strict": true + "noEmit": true }, "exclude": ["node_modules"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/tsconfig.json` around lines 1 - 14, This tsconfig.json duplicates shared compiler options; change it to extend the repository's shared TypeScript base config and keep only package-specific overrides: add an "extends" entry referencing the shared base, remove duplicated keys from "compilerOptions" (like declaration, declarationMap, composite, etc.) so only package-specific settings such as "jsx": "react-jsx", "noEmit": true, and any necessary overrides remain, and preserve the "exclude" array; update the file named tsconfig.json in this package and ensure compilerOptions now merge with the shared base rather than redefining common options.packages/email/templates/test.tsx (1)
2-2: Remove unused import.
ComponentPropsis imported but never used in this file.🧹 Proposed fix
import { Button, Html, Head, Body } from "@react-email/components"; -import { type ComponentProps } from "react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/templates/test.tsx` at line 2, Remove the unused import ComponentProps from the top of the file by deleting the import specifier or the entire import statement (the "ComponentProps" token in the import declaration) in packages/email/templates/test.tsx so the file no longer imports an unused symbol.packages/email/templates/test2.tsx (1)
1-22: This template appears to be unused and duplicatestest.tsx.Based on
apps/web/src/lib/utils/server/email.ts:46-56, thesendExampleEmailfunction sends plain text content rather than rendering eithertest.tsxortest2.tsxtemplates. Consider:
- Removing this duplicate template if it serves no purpose.
- If both templates are intended for local development/preview (via
email dev), consolidate or differentiate them meaningfully.- The
ComponentPropsimport on line 2 is unused and should be removed.🧹 If keeping the file, remove unused import
import { Button, Html, Head, Body } from "@react-email/components"; -import { type ComponentProps } from "react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/templates/test2.tsx` around lines 1 - 22, The file packages/email/templates/test2.tsx appears to be an unused duplicate of test.tsx and also contains an unused import ComponentProps; either remove this duplicate template or consolidate/differentiate it for dev preview, and if you keep it remove the unused import (ComponentProps) from the top and ensure any intended usage is wired from sendExampleEmail (in apps/web/src/lib/utils/server/email.ts — currently sending plain text) so that the correct template (test.tsx or test2.tsx) is rendered when emails are previewed or sent.packages/email/package.json (1)
12-13: Consider using compiled output paths formainandtypes.Pointing
mainandtypesto.tsfiles works in a monorepo with proper TypeScript project references, but it requires consuming packages to compile the TypeScript. If this package might be published or consumed outside the workspace, consider using a build step and pointing to compiled outputs.For a private monorepo package, this is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/package.json` around lines 12 - 13, The package.json currently points "main" and "types" at TypeScript source files ("main": "./index.ts", "types": "./index.ts"); change these to the compiled outputs (e.g., "main": "./dist/index.js", "types": "./dist/index.d.ts"), add a build script (e.g., "build": "tsc -p tsconfig.build.json") and ensure the package publishes the dist folder (add "files": ["dist"] or a prepare script to run the build) so consumers get compiled JS and declaration files instead of needing to compile the .ts sources.packages/email/senders/ethereal.ts (1)
9-19: Avoid creating a new Ethereal account for every email.
createTestAccount()insidesend()makes every delivery do an extra network round-trip before the actual send. Since this sender is selected as a service implementation, that will make local/test flows noticeably slower and more failure-prone than necessary. Create the test account/transporter once inetherealSender()and reuse it across sends.♻️ Suggested shape
export const etherealSender = (): HKEmailer<SMTPTransport.SentMessageInfo> => { + const transporterPromise = createTestAccount().then((testAccount) => + createTransport({ + host: testAccount.smtp.host, + port: testAccount.smtp.port, + secure: testAccount.smtp.secure, + auth: { + user: testAccount.user, + pass: testAccount.pass, + }, + }), + ); return { async send({ text, from, to, subject, body }: SendEmailParams) { - const testAccount = await createTestAccount(); - const transporter = createTransport({ - host: testAccount.smtp.host, - port: testAccount.smtp.port, - secure: testAccount.smtp.secure, - auth: { - user: testAccount.user, - pass: testAccount.pass, - }, - }); + const transporter = await transporterPromise; const info = await transporter.sendMail({ from, to, subject, text, html: await pretty(await render(body)), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/senders/ethereal.ts` around lines 9 - 19, The send() method currently calls createTestAccount() and builds a transporter per call, causing extra network round-trips; instead, move the createTestAccount() and createTransport() logic out of send() into the etherealSender() factory so the test account and transporter are created once and reused. Initialize testAccount and transporter when etherealSender() runs (or lazily on first call) and have send() use the prebuilt transporter to deliver messages; update references to createTestAccount, createTransport, transporter, and send accordingly to ensure no per-send network setup occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 27: The file ends without a trailing newline after the
SMTP_PASS="password" entry; open the .env.example and add a single POSIX newline
character at EOF so the last line (SMTP_PASS) is terminated properly, then save
and commit the change.
In `@apps/web/src/actions/email.ts`:
- Around line 35-38: The current code in apps/web/src/actions/email.ts fans out
sends with Promise.all over emails.map(({ email }) => sendExampleEmail(email)),
which performs all sends inline on the request path; change this to avoid
concurrent outbound sends by either enqueuing each recipient to your background
job queue (e.g., call enqueueSendEmail or createSendEmailJob for each email) or
send in controlled batches/limited concurrency (use a p-limit style limiter or
chunk the emails and call sendExampleEmail sequentially per chunk) so the HTTP
request returns quickly and you avoid provider throttling and duplicate sends on
retry.
- Around line 12-33: The audience enum and filtering logic are inconsistent with
the UI and treat "all" like "rsvpdOnly"; update the enum and filter to use a
single shared literal set (pick and standardize on one spelling used by the UI,
e.g. "rsvpedOnly" or change the UI to "rsvpdOnly"), then change the action
handling of parsedInput.sendTo so that: if sendTo === "all" do not apply any
userCommonData.isRSVPed WHERE clause, if sendTo === "<rsvp-only-literal>" filter
sql`${userCommonData.isRSVPed} = 1`, and if sendTo === "notRsvpdOnly" filter
sql`${userCommonData.isRSVPed} = 0`; ensure the z.object enum, the
SendExampleEmailForm emission, and the db.select .where logic refer to the exact
same literal names (and remove the existing fallback that always applies
isRSVPed = 1).
- Around line 39-41: The catch block currently returns the raw exception typed
as a string ("e as string"); convert the runtime error to a real string before
returning to satisfy the z.string().optional() schema. Replace the typed cast
with logic that derives a string, e.g. compute an errorMessage using e
instanceof Error ? e.message : String(e) (or include stack with e.stack if
desired), keep console.error(e) for logging, and return { success: false, error:
errorMessage } instead of using "e as string". This change should be applied in
the catch block that returns the object with keys success and error.
In `@apps/web/src/actions/registration.ts`:
- Around line 114-120: Wrap the call to sendRegistrationSuccessEmail in a
best-effort try/catch so email failures don’t make the already-committed
registration reject: around the sendRegistrationSuccessEmail(email, {...}) call
catch any error, log it (include context like email and userCommonData.hackerTag
or user id) and do not rethrow; alternatively, if you have a background queue
API (e.g., enqueueEmailJob or queueService), push the email task to that queue
instead of sending inline.
- Around line 115-119: The sendRegistrationSuccessEmail call is passing the
Drizzle column object userCommonData.hackerTag instead of the actual registered
hacker tag string; update the payload to pass the real value (e.g.,
userData.hackerTag or the createdUser.hackerTag returned after insert) to the
hackerTag field in the sendRegistrationSuccessEmail(...) call so the email
template receives a string rather than a schema column object.
In `@apps/web/src/actions/rsvp.ts`:
- Around line 18-24: The DB update using
db.update(...).set(...).where(eq(userCommonData.clerkID, userId)).returning(...)
already commits isRSVPed=true before email is sent, so
wrap/sendRSVPConfirmationEmail(email) must not cause the whole action to fail:
call sendRSVPConfirmationEmail(email) inside a try/catch and on error log the
failure (including email and userId) and do not rethrow; alternatively push the
email payload to an outbox/background job queue (e.g., an enqueue function)
instead of sending synchronously from this action so db.update and returning({
email: ... }) remains the success path.
In `@apps/web/src/app/admin/emails/page.tsx`:
- Around line 11-21: The page is incorrectly gated by a prior check for
PermissionType.VIEW_EVENTS, causing users with SEND_EMAILS but not VIEW_EVENTS
to be blocked; remove the preliminary userHasPermission(user,
PermissionType.VIEW_EVENTS) check (and its notFound() return) so the only
authorization is the isUserAuthorized check using userHasPermission(user,
PermissionType.SEND_EMAILS) (or replace with an email-specific read permission
if desired) and ensure only notFound() is returned when the SEND_EMAILS check
fails.
In `@apps/web/src/app/admin/emails/SendExampleEmailForm.tsx`:
- Around line 20-57: The Select's chosen audience is never stored or sent
because handleSend always calls executeAsync({ sendTo: "all" }); fix by adding
local state (e.g., const [audience, setAudience] = useState("all")) and wiring
the Select to update it (use the Select's onValueChange/onChange to call
setAudience with the selected SelectItem value); then change handleSend to call
executeAsync({ sendTo: audience }) so the values from SelectItem ("all",
"rsvpedOnly", "notRsvpedOnly") are actually used. Ensure the Select's
defaultValue is initialized from the same state variable.
In `@apps/web/src/env.ts`:
- Around line 22-29: The env schema currently defines SMTP_PORT and SMTP_SECURE
as z.string(), but SMTPTransport.Options expects a number and boolean; update
the schema in apps/web/src/env.ts to coerce/parse these values to the correct
types (e.g. use Zod's z.coerce.number() for SMTP_PORT and z.coerce.boolean() or
an equivalent preprocess/transform for SMTP_SECURE) so that the parsed env
object yields a numeric port and boolean secure value and downstream code using
SMTPTransport.Options can rely on correct types without manual conversion.
In `@apps/web/src/lib/utils/server/email.ts`:
- Around line 21-23: The email helpers currently use the literal placeholder
"<[EMAIL_ADDRESS]>" as the "from" header in mailer.send calls (seen in the
mailer.send blocks), which is invalid; replace this by reading a real sender
address from configuration/environment (e.g., process.env.SMTP_FROM or a
config.get value) and reuse that single value across all helpers instead of
hardcoding the placeholder; update the mailer.send invocations in the functions
that construct messages so they set from to the configured sender and ensure the
config lookup is done once (module-level constant) and referenced by each
helper.
- Around line 6-17: The mailer is hardcoded to etherealSender() so the
emailService flag is ignored; update the mailer initialization in
apps/web/src/lib/utils/server/email.ts to choose the transport based on the
configured emailService (e.g., process.env.EMAIL_SERVICE or the module's config)
— if emailService === "smtp" instantiate smtpSender(...) with the existing SMTP
options (host, port, secure, auth user/pass), otherwise fall back to
etherealSender(); ensure the created variable mailer (and any exported send
functions) use this conditional instance so SMTP is actually used when
configured.
In `@packages/email/senders/ethereal.ts`:
- Around line 27-31: The Ethereal preview URL (getTestMessageUrl(info)) is being
logged unconditionally in the shared sender and can leak email contents; update
the sender in packages/email/senders/ethereal.ts (the code around the send
function that uses info and getTestMessageUrl) to stop logging the preview URL
by default and only emit it when an explicit local debug flag or env var is set
(e.g., ETHEREAL_PREVIEW=true or a DEBUG_LOCAL boolean), i.e., wrap the
console.log("Preview URL: %s", previewUrl) behind that flag check and avoid
exporting or returning the preview URL from the shared library API so only local
test scripts can opt in.
In `@packages/email/templates/rsvp-confirmation.tsx`:
- Around line 4-10: The RSVPConfirmationEmail component currently destructures a
required prop (ConfirmationEmailProps.name) which crashes when called with
undefined; update RSVPConfirmationEmail to accept optional props by changing
ConfirmationEmailProps.name to optional (name?: string) or remove the props
parameter entirely and adjust the function signature accordingly so
RSVPConfirmationEmail can be invoked without arguments (also update any related
types/usages such as sendRSVPConfirmationEmail and the call sites that pass or
omit props).
In `@packages/email/tsconfig.json`:
- Line 3: The "jsx":"react-jsx" line uses spaces for indentation while the rest
of the tsconfig.json uses tabs; update that line to use a tab character for
indentation so it matches the file's existing tab-based formatting (locate the
"jsx":"react-jsx" entry and replace its leading spaces with a tab).
---
Nitpick comments:
In `@packages/email/package.json`:
- Around line 12-13: The package.json currently points "main" and "types" at
TypeScript source files ("main": "./index.ts", "types": "./index.ts"); change
these to the compiled outputs (e.g., "main": "./dist/index.js", "types":
"./dist/index.d.ts"), add a build script (e.g., "build": "tsc -p
tsconfig.build.json") and ensure the package publishes the dist folder (add
"files": ["dist"] or a prepare script to run the build) so consumers get
compiled JS and declaration files instead of needing to compile the .ts sources.
In `@packages/email/senders/ethereal.ts`:
- Around line 9-19: The send() method currently calls createTestAccount() and
builds a transporter per call, causing extra network round-trips; instead, move
the createTestAccount() and createTransport() logic out of send() into the
etherealSender() factory so the test account and transporter are created once
and reused. Initialize testAccount and transporter when etherealSender() runs
(or lazily on first call) and have send() use the prebuilt transporter to
deliver messages; update references to createTestAccount, createTransport,
transporter, and send accordingly to ensure no per-send network setup occurs.
In `@packages/email/templates/test.tsx`:
- Line 2: Remove the unused import ComponentProps from the top of the file by
deleting the import specifier or the entire import statement (the
"ComponentProps" token in the import declaration) in
packages/email/templates/test.tsx so the file no longer imports an unused
symbol.
In `@packages/email/templates/test2.tsx`:
- Around line 1-22: The file packages/email/templates/test2.tsx appears to be an
unused duplicate of test.tsx and also contains an unused import ComponentProps;
either remove this duplicate template or consolidate/differentiate it for dev
preview, and if you keep it remove the unused import (ComponentProps) from the
top and ensure any intended usage is wired from sendExampleEmail (in
apps/web/src/lib/utils/server/email.ts — currently sending plain text) so that
the correct template (test.tsx or test2.tsx) is rendered when emails are
previewed or sent.
In `@packages/email/tsconfig.json`:
- Around line 1-14: This tsconfig.json duplicates shared compiler options;
change it to extend the repository's shared TypeScript base config and keep only
package-specific overrides: add an "extends" entry referencing the shared base,
remove duplicated keys from "compilerOptions" (like declaration, declarationMap,
composite, etc.) so only package-specific settings such as "jsx": "react-jsx",
"noEmit": true, and any necessary overrides remain, and preserve the "exclude"
array; update the file named tsconfig.json in this package and ensure
compilerOptions now merge with the shared base rather than redefining common
options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2eeee4b-fa83-4608-bcdb-e1c380009317
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.env.exampleapps/web/package.jsonapps/web/src/actions/email.tsapps/web/src/actions/registration.tsapps/web/src/actions/rsvp.tsapps/web/src/app/admin/emails/SendExampleEmailForm.tsxapps/web/src/app/admin/emails/page.tsxapps/web/src/app/admin/layout.tsxapps/web/src/env.tsapps/web/src/lib/constants/permission.tsapps/web/src/lib/utils/server/email.tspackages/config/hackkit.config.tspackages/email/index.tspackages/email/package.jsonpackages/email/send_test.tspackages/email/senders/ethereal.tspackages/email/senders/smtp.tspackages/email/templates/registration-confirmation.tsxpackages/email/templates/rsvp-confirmation.tsxpackages/email/templates/test.tsxpackages/email/templates/test2.tsxpackages/email/tsconfig.jsonpackages/email/types.ts
| SMTP_PORT="25" | ||
| SMTP_SECURE="true" | ||
| SMTP_USER="user" | ||
| SMTP_PASS="password" No newline at end of file |
There was a problem hiding this comment.
**Add a trailing newline at the end of the file.**POSIX defines a line as a sequence of non-newline characters terminated by a newline, and a text file as consisting of such lines. When using version control systems like Git, having a newline at the end of files can prevent unnecessary diffs.
🧹 Proposed fix
SMTP_HOST="mailserver.hackkit.org"
SMTP_PORT="25"
SMTP_SECURE="true"
SMTP_USER="user"
-SMTP_PASS="password"
+SMTP_PASS="password"
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SMTP_PASS="password" | |
| SMTP_PASS="password" | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 27-27: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 27-27: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 27-27: [UnorderedKey] The SMTP_PASS key should go before the SMTP_PORT key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example at line 27, The file ends without a trailing newline after the
SMTP_PASS="password" entry; open the .env.example and add a single POSIX newline
character at EOF so the last line (SMTP_PASS) is terminated properly, then save
and commit the change.
Comment on lines
+12
to
+33
| .schema(z.object({ sendTo: z.enum(["all", "rsvpdOnly", "notRsvpdOnly"]) })) | ||
| .outputSchema( | ||
| z.object({ success: z.boolean(), error: z.string().optional() }), | ||
| ) | ||
| .action(async ({ parsedInput: { sendTo }, ctx: { user } }) => { | ||
| if (!userHasPermission(user, PermissionType.SEND_EMAILS)) { | ||
| return { | ||
| success: false, | ||
| error: "You do not have permission to send emails.", | ||
| }; | ||
| } | ||
|
|
||
| let emails = await db | ||
| .select({ email: userCommonData.email }) | ||
| .from(userCommonData) | ||
| .where( | ||
| sendTo == "rsvpdOnly" | ||
| ? sql`${userCommonData.isRSVPed} = 1` | ||
| : sendTo == "notRsvpdOnly" | ||
| ? sql`${userCommonData.isRSVPed} = 0` | ||
| : sql`${userCommonData.isRSVPed} = 1`, | ||
| ); |
There was a problem hiding this comment.
Fix the audience literals and the "all" branch together.
Line 12 accepts "rsvpdOnly", but apps/web/src/app/admin/emails/SendExampleEmailForm.tsx Line 47 emits "rsvpedOnly". Also, the fallback on Lines 27-33 still filters isRSVPed = 1, so "all" and "RSVP'd only" currently target the same users. Use one shared set of literals and skip the RSVP filter entirely for "all".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/actions/email.ts` around lines 12 - 33, The audience enum and
filtering logic are inconsistent with the UI and treat "all" like "rsvpdOnly";
update the enum and filter to use a single shared literal set (pick and
standardize on one spelling used by the UI, e.g. "rsvpedOnly" or change the UI
to "rsvpdOnly"), then change the action handling of parsedInput.sendTo so that:
if sendTo === "all" do not apply any userCommonData.isRSVPed WHERE clause, if
sendTo === "<rsvp-only-literal>" filter sql`${userCommonData.isRSVPed} = 1`, and
if sendTo === "notRsvpdOnly" filter sql`${userCommonData.isRSVPed} = 0`; ensure
the z.object enum, the SendExampleEmailForm emission, and the db.select .where
logic refer to the exact same literal names (and remove the existing fallback
that always applies isRSVPed = 1).
Comment on lines
+35
to
+38
| try { | ||
| await Promise.all( | ||
| emails.map(({ email }) => sendExampleEmail(email)), | ||
| ); |
There was a problem hiding this comment.
Don't fan out every send concurrently on the request path.
Promise.all() here opens one outbound send per recipient. For any non-trivial audience, that risks provider throttling, request timeouts, and duplicate sends on retry. Queue or batch this work instead of doing it inline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/actions/email.ts` around lines 35 - 38, The current code in
apps/web/src/actions/email.ts fans out sends with Promise.all over emails.map(({
email }) => sendExampleEmail(email)), which performs all sends inline on the
request path; change this to avoid concurrent outbound sends by either enqueuing
each recipient to your background job queue (e.g., call enqueueSendEmail or
createSendEmailJob for each email) or send in controlled batches/limited
concurrency (use a p-limit style limiter or chunk the emails and call
sendExampleEmail sequentially per chunk) so the HTTP request returns quickly and
you avoid provider throttling and duplicate sends on retry.
Comment on lines
+39
to
+41
| } catch (e) { | ||
| console.error(e); | ||
| return { success: false, error: e as string }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/actions/email.ts | head -60Repository: acmutsa/HackKit
Length of output: 1672
🏁 Script executed:
rg -A 5 -B 5 "outputSchema" apps/web/src/lib/safe-action.tsRepository: acmutsa/HackKit
Length of output: 41
🏁 Script executed:
fd "safe-action" --type fRepository: acmutsa/HackKit
Length of output: 90
🏁 Script executed:
cat -n apps/web/src/lib/safe-action.tsRepository: acmutsa/HackKit
Length of output: 1832
🏁 Script executed:
rg "outputSchema" apps/web/src/actions/ -A 2Repository: acmutsa/HackKit
Length of output: 235
🏁 Script executed:
rg "next-safe-action" apps/web/package.jsonRepository: acmutsa/HackKit
Length of output: 92
Ensure error is converted to string at runtime.
Line 41 uses e as string, which only changes the TypeScript type without converting the Error object at runtime. This violates the z.string().optional() output schema contract, causing validation failures or invalid error data on the client.
Suggested fix
} catch (e) {
console.error(e);
- return { success: false, error: e as string };
+ return {
+ success: false,
+ error: e instanceof Error ? e.message : "Failed to send emails",
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (e) { | |
| console.error(e); | |
| return { success: false, error: e as string }; | |
| } catch (e) { | |
| console.error(e); | |
| return { | |
| success: false, | |
| error: e instanceof Error ? e.message : "Failed to send emails", | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/actions/email.ts` around lines 39 - 41, The catch block
currently returns the raw exception typed as a string ("e as string"); convert
the runtime error to a real string before returning to satisfy the
z.string().optional() schema. Replace the typed cast with logic that derives a
string, e.g. compute an errorMessage using e instanceof Error ? e.message :
String(e) (or include stack with e.stack if desired), keep console.error(e) for
logging, and return { success: false, error: errorMessage } instead of using "e
as string". This change should be applied in the catch block that returns the
object with keys success and error.
Comment on lines
+114
to
+120
| if (c.featureFlags.extra.emailService) { | ||
| await sendRegistrationSuccessEmail(email, { | ||
| firstName: userData.firstName, | ||
| lastName: userData.lastName, | ||
| hackerTag: userCommonData.hackerTag, | ||
| email, | ||
| }); |
There was a problem hiding this comment.
Don't let email delivery make a committed registration look failed.
This runs after the transaction succeeds. If sendRegistrationSuccessEmail() throws here, the user is already persisted but the action still rejects, which can prompt retries and immediately hit the duplicate-registration path. Make this best-effort or move it to a background job/queue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/actions/registration.ts` around lines 114 - 120, Wrap the call
to sendRegistrationSuccessEmail in a best-effort try/catch so email failures
don’t make the already-committed registration reject: around the
sendRegistrationSuccessEmail(email, {...}) call catch any error, log it (include
context like email and userCommonData.hackerTag or user id) and do not rethrow;
alternatively, if you have a background queue API (e.g., enqueueEmailJob or
queueService), push the email task to that queue instead of sending inline.
Comment on lines
+6
to
+17
| // const mailer = smtpSender({ | ||
| // host: process.env.SMTP_HOST, | ||
| // port: Number(process.env.SMTP_PORT), | ||
| // secure: process.env.SMTP_SECURE === "true", | ||
| // auth: { | ||
| // user: process.env.SMTP_USER, | ||
| // pass: process.env.SMTP_PASS, | ||
| // }, | ||
| // }); | ||
|
|
||
| // Testing mailer | ||
| const mailer = etherealSender(); |
There was a problem hiding this comment.
Honor emailService when building the mailer.
This always instantiates etherealSender() and leaves the SMTP branch commented out. When config is set to "smtp", mail still goes through Ethereal, so the new feature flag never actually switches transports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/utils/server/email.ts` around lines 6 - 17, The mailer is
hardcoded to etherealSender() so the emailService flag is ignored; update the
mailer initialization in apps/web/src/lib/utils/server/email.ts to choose the
transport based on the configured emailService (e.g., process.env.EMAIL_SERVICE
or the module's config) — if emailService === "smtp" instantiate smtpSender(...)
with the existing SMTP options (host, port, secure, auth user/pass), otherwise
fall back to etherealSender(); ensure the created variable mailer (and any
exported send functions) use this conditional instance so SMTP is actually used
when configured.
Comment on lines
+21
to
+23
| await mailer.send({ | ||
| from: "<[EMAIL_ADDRESS]>", | ||
| to: email, |
There was a problem hiding this comment.
Replace the placeholder from header with a real sender address.
Line 22, Line 37, and Line 49 use "<[EMAIL_ADDRESS]>", which is placeholder text rather than a mailbox. SMTP providers commonly reject or rewrite messages like this. Pull the sender from config/env once and reuse it across all helpers.
Also applies to: 36-38, 48-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/utils/server/email.ts` around lines 21 - 23, The email
helpers currently use the literal placeholder "<[EMAIL_ADDRESS]>" as the "from"
header in mailer.send calls (seen in the mailer.send blocks), which is invalid;
replace this by reading a real sender address from configuration/environment
(e.g., process.env.SMTP_FROM or a config.get value) and reuse that single value
across all helpers instead of hardcoding the placeholder; update the mailer.send
invocations in the functions that construct messages so they set from to the
configured sender and ensure the config lookup is done once (module-level
constant) and referenced by each helper.
Comment on lines
+27
to
+31
| console.log("Message sent: %s", info.messageId); | ||
|
|
||
| // Get the Ethereal URL to preview this email | ||
| const previewUrl = getTestMessageUrl(info); | ||
| console.log("Preview URL: %s", previewUrl); |
There was a problem hiding this comment.
Don't log Ethereal preview URLs in library code.
That URL is effectively a bearer link to the email contents. Logging it from the shared sender leaks message data to anyone with log access, which is risky once real user emails start flowing through staging or other shared environments. Gate it behind an explicit local-debug flag, or surface it only from the dedicated test script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/email/senders/ethereal.ts` around lines 27 - 31, The Ethereal
preview URL (getTestMessageUrl(info)) is being logged unconditionally in the
shared sender and can leak email contents; update the sender in
packages/email/senders/ethereal.ts (the code around the send function that uses
info and getTestMessageUrl) to stop logging the preview URL by default and only
emit it when an explicit local debug flag or env var is set (e.g.,
ETHEREAL_PREVIEW=true or a DEBUG_LOCAL boolean), i.e., wrap the
console.log("Preview URL: %s", previewUrl) behind that flag check and avoid
exporting or returning the preview URL from the shared library API so only local
test scripts can opt in.
Comment on lines
+4
to
+10
| interface ConfirmationEmailProps { | ||
| name: string; | ||
| } | ||
|
|
||
| export default function RSVPConfirmationEmail({ | ||
| name, | ||
| }: ConfirmationEmailProps) { |
There was a problem hiding this comment.
This required prop will crash the RSVP email path.
sendRSVPConfirmationEmail currently accepts props?, and apps/web/src/actions/rsvp.ts, Line 24 calls it without props. Because apps/web/src/lib/utils/server/email.ts invokes this as RSVPConfirmationEmail(props), destructuring name from undefined throws before the email is sent. Since name isn't used here, make the props optional or remove them entirely.
🐛 Minimal fix
-interface ConfirmationEmailProps {
- name: string;
-}
-
-export default function RSVPConfirmationEmail({
- name,
-}: ConfirmationEmailProps) {
+interface ConfirmationEmailProps {
+ name?: string;
+}
+
+export default function RSVPConfirmationEmail(
+ _props: ConfirmationEmailProps = {},
+) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface ConfirmationEmailProps { | |
| name: string; | |
| } | |
| export default function RSVPConfirmationEmail({ | |
| name, | |
| }: ConfirmationEmailProps) { | |
| interface ConfirmationEmailProps { | |
| name?: string; | |
| } | |
| export default function RSVPConfirmationEmail( | |
| _props: ConfirmationEmailProps = {}, | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/email/templates/rsvp-confirmation.tsx` around lines 4 - 10, The
RSVPConfirmationEmail component currently destructures a required prop
(ConfirmationEmailProps.name) which crashes when called with undefined; update
RSVPConfirmationEmail to accept optional props by changing
ConfirmationEmailProps.name to optional (name?: string) or remove the props
parameter entirely and adjust the function signature accordingly so
RSVPConfirmationEmail can be invoked without arguments (also update any related
types/usages such as sendRSVPConfirmationEmail and the call sites that pass or
omit props).
| @@ -0,0 +1,14 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "jsx":"react-jsx", | |||
There was a problem hiding this comment.
Inconsistent indentation: spaces instead of tabs.
Line 3 uses spaces for indentation while the rest of the file uses tabs.
🔧 Proposed fix
{
"compilerOptions": {
- "jsx":"react-jsx",
+ "jsx": "react-jsx",
"esModuleInterop": true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/email/tsconfig.json` at line 3, The "jsx":"react-jsx" line uses
spaces for indentation while the rest of the tsconfig.json uses tabs; update
that line to use a tab character for indentation so it matches the file's
existing tab-based formatting (locate the "jsx":"react-jsx" entry and replace
its leading spaces with a tab).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Emailing is an important way of communicating with hackathon participants leading up to and following the event. We want to try to make hackathon emails easier by building email management functionality
What
packages/emailtemplatesfolder to store email designs (built withreact-email)config/config.hackkit.tsapps/webSEND_EMAILSenv.tsnow requires environment variables for SMTP if emailService is set to "smtp"Satisfies
satisfies #235
resolves #101
resolves #193
Summary by CodeRabbit
New Features
Configuration