Merged
Conversation
used `pnpm turbo gen`
📝 WalkthroughWalkthroughThis pull request consolidates utility functions scattered across multiple apps and packages into a centralized Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: Large scope spanning 50+ files across multiple packages with heterogeneous changes. While many changes follow identical patterns (import replacement), the diff includes substantial structural removals (611-line utils.ts consolidation), complex router refactors with multiple API namespace migrations (discord, permissions, forms, logger), environment schema adjustments, and ESLint rule changes. Review requires verification of import correctness across diverse file types, validation of API namespace usage consistency, and confirmation that no utility logic was lost during consolidation. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
TODO: look into moving this into some other package. IMO: `scripts`
Added jsdocs from chat too. not sure if they're accurate
…tHacks/forge into utils/create-utils-package
Contributor
Author
|
@DGoel1602 can you do |
Made console an eslint error not warn. This forces us to actually choose when to use console, when to use logger, and when to do nothing. We still need to look into actually handling errors on the frontend.
DVidal1205
added
Major
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx (1)
380-386:⚠️ Potential issue | 🟠 MajorReset
loadingwhen the pre-submit steps fail.If
fileToBase64()oruploadResume.mutateAsync()throws,createHacker.onSettled()never runs, so the form stays stuck in the loading state after this toast. Clear the flag here, or move cleanup into afinally.Suggested fix
} catch (error) { + setLoading(false); // TODO: look into not logging into the console // eslint-disable-next-line no-console console.error("Error uploading resume or creating hacker:", error); toast.error( "Something went wrong while processing your application.", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx` around lines 380 - 386, The catch block in hacker-application-form.tsx currently logs the error and shows a toast but never resets the form loading state when fileToBase64() or uploadResume.mutateAsync() throws; update the catch to clear the loading flag (call the component's loading setter, e.g. setLoading(false) or setIsSubmitting(false) used by this form) or move the cleanup into a finally so that createHacker.onSettled() is not the sole place that clears loading; ensure you reference the same loading state variable used elsewhere in this component so the form is not left stuck.packages/db/scripts/bootstrap-superadmin.ts (1)
76-82:⚠️ Potential issue | 🟠 MajorBreaking change: Overwrites existing role without validation.
If this Discord role is already linked and assigned to other users, updating it to Superadmin grants them full permissions without confirmation. This could unintentionally elevate privileges for multiple users.
Why this matters: The script assumes the role is either unused or should become Superadmin, but doesn't verify this assumption.
Consider one of these approaches:
- Safest: Require the role to be unassigned before updating
- Alternative: Prompt for confirmation showing how many users will be affected
- Or: Create a new Superadmin role instead of reusing existing ones
As per coding guidelines, database changes should have backwards compatibility or documented breaking changes.
🛡️ Proposed validation check
if (existingRole) { console.log( `Discord role ${discordRoleId} is already linked to role: ${existingRole.name}`, ); + + // Check if other users have this role + const usersWithRole = await db.query.Permissions.findMany({ + where: (t, { eq }) => eq(t.roleId, existingRole.id), + }); + + if (usersWithRole.length > 0) { + console.warn( + `⚠️ WARNING: ${usersWithRole.length} user(s) currently have this role.` + ); + console.warn( + ` Updating to Superadmin will grant them full permissions.` + ); + console.warn(` Consider creating a new role instead.\n`); + process.exit(1); + } + console.log(` Updating permissions to superadmin level...\n`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/scripts/bootstrap-superadmin.ts` around lines 76 - 82, The update to Roles (db.update(Roles).set({...}).where(eq(Roles.discordRoleId, discordRoleId))) unconditionally rewrites an existing Discord role to Superadmin; change it to first check whether any users are assigned to that role and abort (or require confirmation) if assignments exist: perform a count query against the user-role assignment table used by the app (the table that links users to Roles) for the given Roles.discordRoleId, and if count > 0 return/log an error and do not run the db.update; only run db.update(Roles)... when the assignment count is zero (or implement an alternate path to create a new Superadmin role instead of reusing the existing Roles record).packages/db/scripts/get_prod_db.ts (2)
69-72:⚠️ Potential issue | 🟠 MajorAvoid raw
process.envaccess in this script.This file already uses the validated
envmodule, butenvNreintroduces directprocess.envaccess. Please move the child-process env construction behindpackages/db/src/env.ts(or another config helper) and import that here instead. As per coding guidelines,!(**/{env,*.config}.{js,ts,tsx}): Validated env access: Flag any direct usage of process.env outside of env.ts config files and .config. files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/scripts/get_prod_db.ts` around lines 69 - 72, The script currently constructs child-process environment using raw process.env (see envN) which violates the validated-env rule; instead, move the logic that builds the child process environment (merging PGPASSWORD into process env) into the shared env helper (e.g., packages/db/src/env.ts) and export a function or object (e.g., getChildProcessEnv or CHILD_PROCESS_ENV) that this script imports and uses; update get_prod_db.ts to call parsePg() for credentials and then import the prepared env from env.ts rather than referencing process.env directly (replace envN usage with the imported helper).
78-101:⚠️ Potential issue | 🟠 MajorReplace shell interpolation with
spawn()and explicit argv for psql calls.The code interpolates
port,user, andobjectNameintoexec()commands, which runs through a shell. While these values come from parsed environment and hardcoded sources here, this pattern violates best practice: special characters in DATABASE_URL credentials could break the command, and the approach doesn't scale if inputs become dynamic.Use
spawn()with explicit argv and pass SQL via stdin:Example fix
import { spawn } from "child_process"; import { pipeline } from "stream/promises"; async function runPsql(args: string[], sqlInput?: string, env: NodeJS.ProcessEnv) { return new Promise<void>((resolve, reject) => { const child = spawn("psql", args, { env, stdio: ["pipe", "inherit", "inherit"] }); child.on("error", reject); child.on("close", (code) => { code === 0 ? resolve() : reject(new Error(`psql exited with code ${code}`)); }); if (sqlInput) { child.stdin.write(sqlInput); } child.stdin.end(); }); } // For truncate: pass SQL via stdin instead of heredoc if (args.includes("--truncate")) { console.log("Truncating all tables in DB"); await runPsql( ["-h", "localhost", "-p", port, "-U", user, "-d", "local"], `SET session_replication_role = replica; DO $$ DECLARE r RECORD; BEGIN FOR r IN (SELECT tablename FROM pg_tables WHERE schemaname = 'public') LOOP EXECUTE 'TRUNCATE TABLE ' || quote_ident(r.tablename) || ' CASCADE'; END LOOP; END $$; SET session_replication_role = DEFAULT;`, envN ); } // For restore: stream input file console.log("Inserting prod rows into local DB"); try { await new Promise<void>((resolve, reject) => { const child = spawn("psql", ["-h", "localhost", "-p", port, "-U", user, "local"], { env: envN, stdio: ["pipe", "inherit", "inherit"] }); child.on("error", reject); child.on("close", (code) => { code === 0 ? resolve() : reject(new Error(`psql exited with code ${code}`)); }); pipeline(fs.createReadStream(objectName), child.stdin).catch(reject); }); } finally { await unlink(objectName); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/scripts/get_prod_db.ts` around lines 78 - 101, The current use of execAsync with shell-interpolated commands (the calls around execAsync that use port, user, envN and objectName) should be replaced with child_process.spawn-based calls that pass psql and its arguments as an argv array (e.g., ["-h","localhost","-p",port,"-U",user,"-d","local"]) to avoid shell interpolation; for the truncate block send the multi-statement SQL via stdin to the spawned psql process and for the restore block stream the file into psql.stdin (use stream pipeline or fs.createReadStream(objectName) -> child.stdin), propagate errors on 'error' and non-zero exit codes from 'close', and keep using envN for the environment and unlink(objectName) in the finally block.apps/blade/src/app/api/trpc/[trpc]/route.ts (1)
10-15:⚠️ Potential issue | 🟠 MajorRestrict CORS to specific origins in production.
The wildcard
Access-Control-Allow-Origin: *allows any website to call this API endpoint. Since the handler uses authenticated sessions (line 52), this configuration could expose user data to malicious origins. The comment on line 8 acknowledges this needs to be extended.🔒 Proposed fix: Configure allowed origins
+const allowedOrigins = [ + process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000", + // Add other allowed origins +]; + const setCorsHeaders = (res: Response) => { - res.headers.set("Access-Control-Allow-Origin", "*"); + const origin = req.headers.get("origin"); + if (origin && allowedOrigins.includes(origin)) { + res.headers.set("Access-Control-Allow-Origin", origin); + res.headers.set("Access-Control-Allow-Credentials", "true"); + } res.headers.set("Access-Control-Request-Method", "*"); res.headers.set("Access-Control-Allow-Methods", "OPTIONS, GET, POST"); - res.headers.set("Access-Control-Allow-Headers", "*"); + res.headers.set("Access-Control-Allow-Headers", "content-type"); };Note: You'll need to pass
reqtosetCorsHeadersor refactor to access the origin header.As per coding guidelines: "apps/blade/** ... No secrets or API keys in client-side code" — while this is server-side, overly permissive CORS can leak authenticated data to any origin.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/api/trpc/`[trpc]/route.ts around lines 10 - 15, The current setCorsHeaders function sets Access-Control-Allow-Origin to "*" which is too permissive; update setCorsHeaders (and its callers) to accept the incoming Request or origin string, read the request's Origin header, and compare it against a configured allowlist (e.g., from process.env or a constants list) — if the origin is allowed set Access-Control-Allow-Origin to that origin, otherwise omit or set a safe default; only apply the allowlist in production mode and keep the existing wildcard (or development allowlist) for non-production; ensure you update any code that calls setCorsHeaders to pass the Request/Origin so the function can perform the check.apps/tk/src/commands/cat.ts (1)
46-46:⚠️ Potential issue | 🟡 MinorDiscarding the reply promise may hide errors.
Using
voidsuppresses the promise result. Ifinteraction.replyfails, you won't know. Consider awaiting or adding.catch()for observability.🔧 Suggested fix
- void interaction.reply({ embeds: [embed] }); + await interaction.reply({ embeds: [embed] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tk/src/commands/cat.ts` at line 46, The reply promise is being discarded by using `void interaction.reply(...)`; update the `cat` command's reply handling to await the promise and surface errors—either make the enclosing function async and `await interaction.reply({...})` inside a try/catch that logs the error, or append `.catch(...)` to `interaction.reply` to log failures; target the `interaction.reply` call in the `cat` command and ensure errors are logged for observability.packages/api/package.json (1)
29-35:⚠️ Potential issue | 🟡 MinorRemove the stale
./utilsexport from packages/api/package.json.The
./utilsexport points to./src/utils.ts, which no longer exists. With the migration to@forge/utils, this export declaration should be removed to avoid breaking any code that might import from it.{ "exports": { ".": { "types": "./dist/index.d.ts", "default": "./src/index.ts" }, "./env": { "types": "./dist/env.d.ts", "default": "./src/env.ts" } // Remove "./utils" export—utils are now in `@forge/utils` } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/package.json` around lines 29 - 35, Remove the stale "./utils" export from the package exports in packages/api/package.json: locate the "exports" object that currently includes a "./utils" entry pointing to "./src/utils.ts" and delete that "./utils" key (and its types/default entries) so imports no longer point to a non-existent file now that utilities live in the `@forge/utils` package; ensure the remaining exports (e.g., ".", "./env") remain unchanged and package still builds.apps/blade/src/app/_components/dashboard/hackathon-dashboard/point-leaderboard.tsx (1)
145-159:⚠️ Potential issue | 🟡 MinorAdd missing
keyprop to mapped elements.React requires a unique
keyprop when rendering lists to optimize reconciliation. The dummy loader items and activeTop items are missing keys.🔧 Proposed fix
{!activeTop ? ( - dummy.map((v, i) => { + dummy.map((v, i) => ( const t = hackathons.getClassTeam(v); return ( <div + key={v} className={`flex flex-row justify-between border p-1.5 px-2 ${i == 0 ? "rounded-t-lg font-semibold" : i == dummy.length - 1 ? "rounded-b-lg" : ""}`}And for the activeTop mapping at line 162:
{activeTop.map((v, i) => { const t = hackathons.getClassTeam(v.class || "Alchemist"); return ( <div + key={v.id} className={`flex flex-row justify-between border p-1 px-2 ...`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/dashboard/hackathon-dashboard/point-leaderboard.tsx` around lines 145 - 159, The mapped list items rendered in dummy.map (inside the point-leaderboard component) are missing a React key; add a unique key prop to the root element returned by dummy.map (e.g., key={v} or key={`loader-${i}`} or another stable id) where dummy, hackathons.getClassTeam, and Loader are used, and likewise add a unique key prop to the root element of the activeTop mapping (use a stable id like item.id or the index as a fallback) so React can reconcile the list correctly.apps/blade/src/app/_components/dashboard/hackathon-dashboard/upcoming-events.tsx (1)
21-27:⚠️ Potential issue | 🟠 MajorThe upcoming-events window is shifted by one day.
startaddsoneDayOffsetbefore the comparison, so this filter is not selecting the next five hours. It effectively targets events from roughly 24–19 hours ago.Suggested fix
const upcomingEvents = events .filter((event) => { - const oneDayOffset = 24 * 60 * 60 * 1000; - const start = new Date(event.start_datetime).getTime() + oneDayOffset; + const start = new Date(event.start_datetime).getTime(); return ( event.hackathonId != null && start >= now && start <= fiveHoursLater ); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/dashboard/hackathon-dashboard/upcoming-events.tsx` around lines 21 - 27, The filter in the upcomingEvents computation incorrectly adds oneDayOffset to event.start_datetime (see the oneDayOffset constant and the start variable inside events.filter), shifting the window by 24 hours; remove the addition of oneDayOffset (use the event start time directly, e.g., compute start = new Date(event.start_datetime).getTime()) so the comparisons against now and fiveHoursLater correctly select events in the next five hours.apps/blade/src/app/_components/forms/form-responder-client.tsx (1)
86-89:⚠️ Potential issue | 🟠 MajorDo not fail open when the dues check errors.
If
validatePaidDuesfails, this code setshasPaidDuestotrue, so dues-only forms still render when membership status was never verified. That weakens the gate and produces a partial-success UI.Suggested fix
- const duesCheckFailed = !!duesQuery.error; - const hasPaidDues = duesCheckFailed - ? true - : (duesQuery.data?.duesPaid ?? false); + if (duesQuery.error) { + return <div>Error loading dues status</div>; + } + + const hasPaidDues = duesQuery.data?.duesPaid ?? false;Based on learnings, "gating rendering should occur only when all required data fetches succeed. Do not render partial success when some queries fail; instead, implement a unified loading/state or error handling that surfaces a single, coherent state once all data is ready or failed."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/forms/form-responder-client.tsx` around lines 86 - 89, The code currently sets hasPaidDues to true when duesQuery.error (duesCheckFailed), which "fails open"; instead, stop rendering gated forms when the validatePaidDues/duesQuery fetch fails by treating errors as a non-success state and surfacing an error/loading UI until the query succeeds. Concretely, change the logic around duesQuery/duesCheckFailed/hasPaidDues so hasPaidDues is derived only from successful data (e.g., hasPaidDues = duesQuery.data?.duesPaid ?? false) and add a guard in the FormResponderClient render path that if duesQuery.isLoading or duesQuery.isError (duesCheckFailed) you show a unified loading/error state rather than rendering the dues-only form; ensure any references to validatePaidDues result/errors are propagated to that same unified state.packages/api/src/routers/dues-payment.ts (1)
77-88:⚠️ Potential issue | 🔴 CriticalAuthorize the checkout session before returning or logging it.
Line 81 retrieves any Stripe Checkout Session by ID, but this handler never verifies that
session.metadata.member_idbelongs toctx.session.user.id. A leaked or replayedsession_idwould expose another member’s payment status/email, and the new Discord log would attribute that payment to the caller.As per coding guidelines, `packages/api/**`: "Authorization checks in every procedure".🔒 Proposed fix
orderSuccess: protectedProcedure .input(z.string()) .query(async ({ input, ctx }) => { const stripe = new Stripe(env.STRIPE_SECRET_KEY, { typescript: true }); const session = await stripe.checkout.sessions.retrieve(input); + const member = await db + .select({ id: Member.id }) + .from(Member) + .where(eq(Member.userId, ctx.session.user.id)) + .limit(1); + + if (member.length === 0 || session.metadata?.member_id !== member[0]?.id) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Checkout session not found.", + }); + } await discord.log({ message: `A member has successfully paid their dues. ${session.amount_total}`, title: "Dues Paid", color: "success_green",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/dues-payment.ts` around lines 77 - 88, The orderSuccess procedure retrieves a Stripe Checkout Session but does not authorize it; before calling discord.log or returning the session, check that the retrieved session (from stripe.checkout.sessions.retrieve(input)) has session.metadata.member_id and that it equals ctx.session.user.id (or throw an authentication/authorization error); update the orderSuccess handler to perform this ownership check and short-circuit with an appropriate error response if the IDs do not match, then proceed to discord.log only after successful verification.apps/blade/src/app/_components/dashboard/member-dashboard/event/event-showcase.tsx (1)
157-173:⚠️ Potential issue | 🟠 MajorRender each past event’s own timestamps in the loop.
Lines 163 and 172 still format
mostRecent.*, so every card in this list shows the first event’s start/end time instead of the currentevent.🐛 Proposed fix
- {time.formatDateTime(mostRecent.start_datetime)} + {time.formatDateTime(event.start_datetime)} @@ - {time.formatDateTime(mostRecent.end_datetime)} + {time.formatDateTime(event.end_datetime)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/dashboard/member-dashboard/event/event-showcase.tsx` around lines 157 - 173, The loop rendering past events is using mostRecent.start_datetime/mostRecent.end_datetime so every card shows the first event's timestamps; update the JSX inside the loop to call time.formatDateTime(event.start_datetime) and time.formatDateTime(event.end_datetime) (replacing references to mostRecent) so each rendered card uses that iteration's event timestamps.packages/auth/src/config.ts (1)
66-79:⚠️ Potential issue | 🟠 MajorKeep Discord auto-join off the session-creation critical path.
Line 75 now awaits the Discord callback during session creation. Because failures are still swallowed, the behavioral change here is mainly that sign-in latency and timeouts now depend on Discord/network performance.
As per coding guidelines, `packages/auth/**`: "Session management and token handling".🛠️ Safer pattern
- await discord.handleDiscordOAuthCallback(discordUserId); + void discord.handleDiscordOAuthCallback(discordUserId).catch( + (error) => { + console.error("Error in Discord auto join hook:", error); + }, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/src/config.ts` around lines 66 - 79, The after hook currently awaits discord.handleDiscordOAuthCallback which blocks session creation; change it to fire-and-forget so Discord joining runs off the critical path: keep the user lookup (db.query.User.findFirst with eq(User.id, session.userId)) but do not await discord.handleDiscordOAuthCallback(discordUserId); instead invoke it asynchronously (e.g., wrap in Promise.resolve().then(...) or setImmediate/void) and catch/log errors inside that detached promise so failures are recorded but do not affect the session flow.packages/api/src/routers/hackers/queries.ts (1)
36-42:⚠️ Potential issue | 🟠 MajorFix inconsistent future hackathon lookup logic between functions.
getHacker(line 38) usesgt(t.endDate, now)whilegetAllHackers(line 180) usesgt(t.startDate, now). During an active hackathon, these return different hackathons:
getHackerreturns the current hackathon (hasn't ended)getAllHackersreturns the next hackathon (hasn't started yet)The comment in both functions says "grab a FUTURE hackathon with a start date CLOSEST to now," which describes
startDatelogic. AligngetHackerto usestartDatefor consistency.Change in getHacker (lines 36–42):
const now = new Date(); const futureHackathons = await db.query.Hackathon.findMany({ - where: (t, { gt }) => gt(t.endDate, now), + where: (t, { gt }) => gt(t.startDate, now), orderBy: (t, { asc }) => [asc(t.startDate)], limit: 1, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/hackers/queries.ts` around lines 36 - 42, In getHacker, the future hackathon lookup uses gt(t.endDate, now) which returns current hackathons; change the predicate to gt(t.startDate, now) so it matches getAllHackers and the comment ("grab a FUTURE hackathon with a start date CLOSEST to now"); update the call to db.query.Hackathon.findMany inside getHacker (the same place that sets hackathon = futureHackathons[0]) to use startDate in the where clause while keeping the orderBy asc(t.startDate) and limit: 1.packages/api/src/routers/event.ts (1)
54-57:⚠️ Potential issue | 🟡 MinorVariable shadowing:
formsshadows the imported namespace.The query result variable
formson line 54 shadows theformsnamespace imported from@forge/utilson line 32. This could cause confusion and potential bugs if someone tries to useforms.createForm()or similar methods within this scope.🔧 Suggested fix: rename the local variable
- const forms = await db + const formSchemas = await db .select() .from(FormsSchemas) .where(inArray(FormsSchemas.slugName, formSlugs)); - if (forms.length === 0) { + if (formSchemas.length === 0) { return events.map((event) => ({Apply similar renames throughout the
getEventsandgetFeedbackprocedures whereformsis used as a variable name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 54 - 57, The local variable named `forms` returned from the DB query in the `getEvents` and `getFeedback` procedures shadows the imported `forms` namespace; rename the query result variable (e.g., to `formRecords` or `fetchedForms`) wherever you see the snippet using `const forms = await db.select().from(FormsSchemas)...` so references to the imported `forms` namespace (from `@forge/utils`) remain unambiguous and update any subsequent uses in those procedures to the new variable name.
🟡 Minor comments (6)
apps/blade/src/app/api/trpc/[trpc]/route.ts-62-66 (1)
62-66:⚠️ Potential issue | 🟡 MinorReplace
console.errorwithlogger.errorto align with the codebase pattern.The TODO on line 63 correctly flags that
console.errorisn't ideal. Theeslint-disablebypasses the linting rule designed to prevent console usage. Throughout the codebase, errors are logged using thelogger.error()utility frompackages/utils—use that here instead.Suggested fix
onError({ error, path }) { - // TODO: look into not logging into the console - // eslint-disable-next-line no-console - console.error(`>>> tRPC Error on '${path}'`, error.message); + logger.error(`tRPC Error on '${path}'`, { + message: error.message, + code: error.code, + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/api/trpc/`[trpc]/route.ts around lines 62 - 66, In the onError({ error, path }) handler replace the console.error call with the shared logger.error utility: import logger from the packages/utils logger and call logger.error with the same message and error (e.g., logger.error(`>>> tRPC Error on '${path}'`, error)); remove the eslint-disable-next-line no-console since console will no longer be used and ensure the import for logger is added at the top of the file so the onError handler uses logger.error consistently with the codebase.apps/blade/src/app/_components/admin/hackathon/judge-assignment/judges-client.tsx-82-87 (1)
82-87:⚠️ Potential issue | 🟡 MinorDon’t echo raw server error text in this client alert.
Because this is a client component,
err.messagecan leak backend details into the browser. Keep the detailed error in your reporting/logging path, but show a generic failure message here.Suggested fix
} catch (err: unknown) { - const message = err instanceof Error ? err.message : String(err); - // TODO: look into not logging into the console + // TODO: route `err` through the shared logger/reporting utility. // eslint-disable-next-line no-console console.error("Failed to generate room:", err); - alert(`Failed to generate room: ${message}`); + alert("Failed to generate room. Please try again."); } finally {As per coding guidelines:
apps/blade/**: No secrets or API keys in client-side code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/admin/hackathon/judge-assignment/judges-client.tsx` around lines 82 - 87, In the catch block inside judges-client.tsx (the error handler for the room generation code), stop showing raw server error text in the UI: replace the alert that uses err.message with a generic user-facing message (e.g., "Failed to generate room. Please try again or contact support.") and keep the detailed error only in logging/reporting (console.error or your telemetry call) so backend details are not exposed to the browser; ensure the error handling around the same catch (the code referencing err, message, console.error) is updated accordingly.packages/utils/src/google.ts-21-21 (1)
21-21:⚠️ Potential issue | 🟡 MinorRemove the unnecessary
as stringtype assertion.
EVENTS.GOOGLE_PERSONIFY_EMAILis already a string literal constant, so the type assertion is redundant. Per the TypeScript coding guidelines, avoid type assertions likeasin favor of proper typing—TypeScript already knows the correct type here.Fix
const auth = new google.auth.JWT( env.GOOGLE_CLIENT_EMAIL, undefined, GOOGLE_PRIVATE_KEY, [gapiCalendar, gapiGmailSend, gapiGmailSettingsSharing], - EVENTS.GOOGLE_PERSONIFY_EMAIL as string, + EVENTS.GOOGLE_PERSONIFY_EMAIL, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/google.ts` at line 21, The array entry uses an unnecessary type assertion on EVENTS.GOOGLE_PERSONIFY_EMAIL; remove the `as string` so the entry becomes just EVENTS.GOOGLE_PERSONIFY_EMAIL, and if the containing array or variable needs an explicit type, declare it (e.g., string[] or a narrower union) instead of asserting here; update any related declarations that relied on the assertion to use proper typing rather than `as`.packages/email/src/env.ts-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorAdd email format validation to
LISTMONK_FROM_EMAIL.The field currently accepts any string, which means invalid email addresses can pass startup validation and only fail when Listmonk rejects them at runtime. Use
z.email()to catch configuration errors early.Suggested fix
- LISTMONK_FROM_EMAIL: z.string(), + LISTMONK_FROM_EMAIL: z.email(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/email/src/env.ts` at line 9, LISTMONK_FROM_EMAIL currently allows any string; change its schema to validate email syntax by replacing LISTMONK_FROM_EMAIL: z.string() with an email validator (e.g., LISTMONK_FROM_EMAIL: z.string().email()) in the env schema so invalid addresses are rejected at startup; update the export in packages/email/src/env.ts accordingly and run tests/validation to ensure the new schema passes.packages/db/scripts/seed_devdb.ts-443-445 (1)
443-445:⚠️ Potential issue | 🟡 MinorTypo in error message.
Line 444 has "sav" instead of "save".
✏️ Fix
- message: `Failed to sav limited prod db to minio. Error: ${stringify(error)}`, + message: `Failed to save limited prod db to minio. Error: ${stringify(error)}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/scripts/seed_devdb.ts` around lines 443 - 445, Fix the typo in the error message string for the Minio save failure: update the message property (associated with the same object that has title "Failed to save limited prod db to minio") to read "Failed to save limited prod db to minio. Error: ${stringify(error)}" instead of "sav".packages/utils/src/time.ts-78-91 (1)
78-91:⚠️ Potential issue | 🟡 MinorMisleading comment: says "5 hours" but adds 1 day.
Line 79 states "Create a new Date object 5 hours behind the original" but line 81 actually adds 1 day (
setDate(... + 1)). This appears to compensate for the-1 dayadjustment when storing events in the database, but the comment is confusing.📝 Suggested fix: correct the comment
export const formatDateTime = (date: Date) => { - // Create a new Date object 5 hours behind the original + // Adjust date forward by 1 day to compensate for database storage offset const adjustedDate = new Date(date.getTime()); adjustedDate.setDate(adjustedDate.getDate() + 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/time.ts` around lines 78 - 91, The comment above formatDateTime is inaccurate: it says "Create a new Date object 5 hours behind the original" while the code actually adds one day via adjustedDate.setDate(adjustedDate.getDate() + 1). Update the comment to accurately describe the operation (e.g., "Create a new Date object and shift it forward by one day to compensate for stored -1 day adjustment") and ensure it references adjustedDate and the setDate call so future readers understand why the date is incremented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a36e0d1d-cd2a-4495-bcb8-53ec5b9e7110
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (110)
.nvmrcMIGRATION_STATUS.mdapps/blade/package.jsonapps/blade/src/app/_components/admin/club/events/event-details.tsxapps/blade/src/app/_components/admin/club/events/events-table.tsxapps/blade/src/app/_components/admin/club/events/view-attendance-button.tsxapps/blade/src/app/_components/admin/club/members/member-profile.tsxapps/blade/src/app/_components/admin/club/members/scanner.tsxapps/blade/src/app/_components/admin/forms/editor/client.tsxapps/blade/src/app/_components/admin/forms/editor/linker.tsxapps/blade/src/app/_components/admin/hackathon/events/event-details.tsxapps/blade/src/app/_components/admin/hackathon/events/events-table.tsxapps/blade/src/app/_components/admin/hackathon/events/view-attendance-button.tsxapps/blade/src/app/_components/admin/hackathon/judge-assignment/judges-client.tsxapps/blade/src/app/_components/admin/roles/roleedit.tsxapps/blade/src/app/_components/admin/roles/roletable.tsxapps/blade/src/app/_components/dashboard/hackathon-dashboard/point-leaderboard.tsxapps/blade/src/app/_components/dashboard/hackathon-dashboard/team-points.tsxapps/blade/src/app/_components/dashboard/hackathon-dashboard/upcoming-events.tsxapps/blade/src/app/_components/dashboard/hacker-dashboard/past-hackathons.tsxapps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsxapps/blade/src/app/_components/dashboard/member-dashboard/download-qr-pass.tsxapps/blade/src/app/_components/dashboard/member-dashboard/event/event-showcase.tsxapps/blade/src/app/_components/forms/connection-handler.tsapps/blade/src/app/_components/forms/form-responder-client.tsxapps/blade/src/app/_components/forms/form-runner.tsxapps/blade/src/app/_components/forms/form-view-edit-client.tsxapps/blade/src/app/_components/forms/utils.tsapps/blade/src/app/_components/navigation/session-navbar.tsxapps/blade/src/app/_components/shared/scanner.tsxapps/blade/src/app/admin/forms/[slug]/page.tsxapps/blade/src/app/api/membership/route.tsapps/blade/src/app/api/trpc/[trpc]/route.tsapps/blade/src/app/judge/session/route.tsapps/blade/src/lib/utils.tsapps/club/package.jsonapps/club/src/app/_components/contact/contact-form.tsxapps/club/src/app/_components/landing/calendar.tsxapps/club/src/lib/utils.tsapps/cron/eslint.config.jsapps/cron/package.jsonapps/cron/src/crons/_example.tsapps/cron/src/crons/alumni-assign.tsapps/cron/src/crons/animals.tsapps/cron/src/crons/backup-filtered-db.tsapps/cron/src/crons/leetcode.tsapps/cron/src/crons/reminder.tsapps/cron/src/crons/role-sync.tsapps/cron/src/structs/CronBuilder.tsapps/gemiknights/src/app/_components/ui/background-gradient-animation.tsxapps/gemiknights/src/lib/utils.tsapps/tk/eslint.config.jsapps/tk/package.jsonapps/tk/src/commands/capybara.tsapps/tk/src/commands/cat.tsapps/tk/src/commands/dog.tsapps/tk/src/commands/duck.tsapps/tk/src/commands/fact.tsapps/tk/src/commands/fox.tsapps/tk/src/commands/goat.tsapps/tk/src/commands/joke.tsapps/tk/src/commands/weather.tsapps/tk/src/deploy-commands.tsapps/tk/src/index.tspackages/api/eslint.config.jspackages/api/package.jsonpackages/api/src/env.tspackages/api/src/routers/auth.tspackages/api/src/routers/csv-importer.tspackages/api/src/routers/dues-payment.tspackages/api/src/routers/email.tspackages/api/src/routers/event-feedback.tspackages/api/src/routers/event.tspackages/api/src/routers/forms.tspackages/api/src/routers/guild.tspackages/api/src/routers/hackers/mutations.tspackages/api/src/routers/hackers/pagination.tspackages/api/src/routers/hackers/queries.tspackages/api/src/routers/judge.tspackages/api/src/routers/member.tspackages/api/src/routers/misc.tspackages/api/src/routers/passkit.tspackages/api/src/routers/resume.tspackages/api/src/routers/roles.tspackages/api/src/routers/user.tspackages/api/src/trpc.tspackages/api/src/utils.tspackages/auth/src/config.tspackages/db/scripts/bootstrap-superadmin.tspackages/db/scripts/get_prod_db.tspackages/db/scripts/seed_devdb.tspackages/email/package.jsonpackages/email/src/env.tspackages/email/src/index.tspackages/utils/eslint.config.jspackages/utils/package.jsonpackages/utils/src/discord.tspackages/utils/src/env.tspackages/utils/src/events.tspackages/utils/src/forms.tspackages/utils/src/google.tspackages/utils/src/hackathons.tspackages/utils/src/index.tspackages/utils/src/logger.tspackages/utils/src/permissions.tspackages/utils/src/stripe.tspackages/utils/src/time.tspackages/utils/src/trpc.tspackages/utils/tsconfig.jsontooling/eslint/base.js
💤 Files with no reviewable changes (9)
- packages/api/src/env.ts
- apps/blade/src/app/_components/forms/utils.ts
- apps/gemiknights/src/lib/utils.ts
- apps/club/src/lib/utils.ts
- packages/api/eslint.config.js
- apps/cron/eslint.config.js
- apps/blade/src/lib/utils.ts
- packages/api/src/utils.ts
- apps/tk/eslint.config.js
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
apps/blade/src/app/_components/forms/form-runner.tsx (2)
188-191:⚠️ Potential issue | 🟡 MinorAdd accessibility attributes to the error display.
Screen readers won't announce this error when it appears dynamically. Adding
role="alert"ensures assistive technology users are informed of submission failures.♿ Proposed accessibility fix
{submitError && ( - <div className="rounded-md border border-destructive bg-destructive/10 p-4 text-destructive"> + <div + role="alert" + className="rounded-md border border-destructive bg-destructive/10 p-4 text-destructive" + > {submitError} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/forms/form-runner.tsx` around lines 188 - 191, The error message wrapper that renders when submitError is truthy should include accessibility attributes so screen readers announce it; update the JSX in the FormRunner component where submitError is rendered (the conditional that returns the <div className="rounded-md border border-destructive bg-destructive/10 p-4 text-destructive">) to add role="alert" (and optionally aria-atomic="true") to that div so assistive technologies are notified when the error appears.
128-128:⚠️ Potential issue | 🟡 MinorEmpty banner div appears incomplete.
When
form.banneris truthy, an empty<div>is rendered. This looks like a placeholder that was never finished. Either render the banner content or remove this block.🔧 Possible fix if banner should display an image
- {form.banner && <div className="overflow-hidden rounded-lg"></div>} + {form.banner && ( + <div className="overflow-hidden rounded-lg"> + <img src={form.banner} alt="Form banner" className="w-full" /> + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/forms/form-runner.tsx` at line 128, The empty banner div in the FormRunner JSX renders when form.banner is truthy but contains no content; update the conditional block in apps/blade/src/app/_components/forms/form-runner.tsx to either remove the whole `{form.banner && <div className="overflow-hidden rounded-lg"></div>}` check or replace it with actual banner rendering (e.g., render an <img> or background image using the `form.banner` value, include an accessible alt or aria-label, and preserve the wrapper styles such as "overflow-hidden rounded-lg"); ensure you update the same JSX conditional that references `form.banner` so the banner displays correctly or the dead code is removed.packages/api/src/routers/roles.ts (2)
478-483:⚠️ Potential issue | 🟠 MajorPreload existing permissions for the batch matrix.
findFirstruns once per user/role pair inside the nested loops. Large batches will turn into N×M extra queries before any writes, which is avoidable in this shared API path. Fetch the existing relations once and index them in memory.Suggested direction
+ const existingPerms = await db + .select({ + id: Permissions.id, + userId: Permissions.userId, + roleId: Permissions.roleId, + }) + .from(Permissions) + .where( + and( + inArray(Permissions.userId, input.userIds), + inArray(Permissions.roleId, input.roleIds), + ), + ); + + const permByKey = new Map( + existingPerms.map((perm) => [`${perm.userId}:${perm.roleId}`, perm]), + ); + for (const [roleId, roleData] of Object.entries(cachedRoles)) { for (const [userId, userData] of Object.entries(cachedUsers)) { - const perm = await db.query.Permissions.findFirst({ - where: (t, { eq, and }) => - and(eq(t.userId, userId), eq(t.roleId, roleId)), - }); + const perm = permByKey.get(`${userId}:${roleId}`);As per coding guidelines,
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: No N+1 query patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/roles.ts` around lines 478 - 483, The nested loops currently call db.query.Permissions.findFirst for each userId/roleId pair (using cachedRoles, cachedUsers), causing an N×M query pattern; instead, perform a single preload query (e.g., db.query.Permissions.findMany) that filters for userId IN all cachedUsers keys and roleId IN all cachedRoles keys, build an in-memory index keyed by userId and roleId (e.g., `${userId}:${roleId}`), and then replace the per-pair findFirst call with a lookup from that map inside the loops so all existing permissions are resolved from memory without extra DB queries.
337-358:⚠️ Potential issue | 🟠 MajorPersist Blade state before syncing Discord.
These paths mutate Discord first and only then insert/delete the
Permissionsrow. If the DB write fails after a successful Discord call, Blade and Discord drift apart in the opposite direction from the one the comments explicitly allow. Make the DB write the source-of-truth step, then sync Discord as best effort and record any sync failure in the audit message.Suggested direction
- try { - await discord.addRoleToMember(user.discordUserId, role.discordRoleId); - logger.log( - `Successfully added Discord role ${role.discordRoleId} to user ${user.discordUserId}`, - ); - } catch (error) { - logger.error( - `Failed to add Discord role ${role.discordRoleId} to user ${user.discordUserId}:`, - error, - ); - } - await db.insert(Permissions).values({ roleId: input.roleId, userId: input.userId, }); + + let discordSynced = true; + try { + await discord.addRoleToMember(user.discordUserId, role.discordRoleId); + logger.log( + `Successfully added Discord role ${role.discordRoleId} to user ${user.discordUserId}`, + ); + } catch (error) { + discordSynced = false; + logger.error( + `Failed to add Discord role ${role.discordRoleId} to user ${user.discordUserId}:`, + error, + ); + }Apply the same ordering to revoke/batch, or move Discord sync to an async job/outbox if you want stronger recovery guarantees.
Also applies to: 400-421, 491-522
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/roles.ts` around lines 337 - 358, The current flow calls discord.addRoleToMember before persisting the Permissions row (db.insert(Permissions).values(...)), which can leave Blade and Discord out of sync if the DB write fails; change the order so you first await db.insert(Permissions).values({ roleId: input.roleId, userId: input.userId }) and only then attempt discord.addRoleToMember inside a try/catch, logging any discord error to the audit but treating the DB as the source of truth; apply the same reordering to the revoke and batch paths that currently mutate Discord first (the sections using discord.removeRoleFromMember / batch role ops) so DB writes happen before Discord sync.packages/db/scripts/seed_devdb.ts (1)
433-448:⚠️ Potential issue | 🟠 MajorMake Discord notification best-effort to prevent exit-code flip and missed cleanup.
The
discord.log()function has no internal error handling, so failures propagate to callers. This creates two bugs:
- Line 433 (success path): If logging fails, the exception is caught by the outer handler, causing
process.exit(1)instead ofexit(0)—a successful backup is reported as failed.- Line 443 (error path): If logging fails,
cleanUp()never runs because it comes after the log call.Move
cleanUp()before the Discord log in the catch block, and wrap logging in a helper that swallows errors:♻️ Suggested fix
+async function logToDiscordBestEffort( + payload: Parameters<typeof discord.log>[0], +) { + try { + await discord.log(payload); + } catch (logError) { + console.error("Discord log failed:", logError); + } +} + async function main() { try { // ... - await discord.log({ + await logToDiscordBestEffort({ title: `Successfully saved limited prod db to minio`, message: `Successfully saved limited prod db to minio. Run the get_prod_db.ts script to get it into your local dev db.`, color: "success_green", userId: "Host", }); process.exit(0); } catch (error) { console.error("Error during database seeding:", error); + await cleanUp(); - await discord.log({ + await logToDiscordBestEffort({ title: `Failed to save limited prod db to minio`, message: `Failed to sav limited prod db to minio. Error: ${stringify(error)}`, color: "uhoh_red", userId: "Host", }); - await cleanUp(); process.exit(1); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/scripts/seed_devdb.ts` around lines 433 - 448, The discord.log calls can throw and must not flip the process exit or block cleanup; create a safe wrapper (e.g., safeDiscordLog or try/catch around discord.log) that catches and swallows/logs errors without rethrowing, then: 1) on the success path (after the successful save) call safeDiscordLog so a logging failure won’t cause process.exit(1) and still call process.exit(0); 2) in the catch handler move the cleanUp() invocation before any discord.log call and call safeDiscordLog afterwards so cleanup runs even if logging fails; reference discord.log, cleanUp(), stringify(), and process.exit in the changes.packages/api/src/routers/dues-payment.ts (1)
85-90:⚠️ Potential issue | 🟠 MajorGate the “dues paid” audit on Stripe’s actual payment status.
checkout.sessions.retrieve()also returns open/expired/unpaid sessions. Logging success unconditionally here can create false payment records if the success page is revisited or a non-paid session ID is queried.Suggested fix
- await discord.log({ - message: `A member has successfully paid their dues. ${session.amount_total}`, - title: "Dues Paid", - color: "success_green", - userId: ctx.session.user.discordUserId, - }); + if (session.payment_status === "paid") { + await discord.log({ + message: `A member has successfully paid their dues. ${session.amount_total}`, + title: "Dues Paid", + color: "success_green", + userId: ctx.session.user.discordUserId, + }); + }As per coding guidelines,
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: "Integration points (Stripe, Discord, MinIO, PassKit)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/dues-payment.ts` around lines 85 - 90, The current discord.log call is unconditional and may record payments for open/expired/unpaid Stripe sessions; update the code that handles the result of checkout.sessions.retrieve() (the session variable) to only call discord.log when the Stripe session indicates a successful payment — e.g., check session.payment_status === "paid" (and optionally session.status === "complete") before logging; ensure you reference the retrieved session object and gate the discord.log invocation (the existing discord.log call and its payload) behind that conditional so only truly paid sessions produce the "Dues Paid" audit.packages/api/src/routers/forms.ts (1)
447-452:⚠️ Potential issue | 🟠 MajorDon’t let Discord logging fail a successful write.
This runs after the form response has already been inserted. If
discord.log(...)throws or times out, the mutation returns an error even though the submission is saved, which is a bad retry/duplication trap. Make the audit log best-effort here and in the other new post-write log calls below.Suggested fix
- await discord.log({ - title: `Form submitted to blade forms`, - message: `**Form submitted:** ${form.name}\n**User:** ${ctx.session.user.name}`, - color: "success_green", - userId: ctx.session.user.discordUserId, - }); + void discord + .log({ + title: `Form submitted to blade forms`, + message: `**Form submitted:** ${form.name}\n**User:** ${ctx.session.user.name}`, + color: "success_green", + userId: ctx.session.user.discordUserId, + }) + .catch((error) => { + logger.error("createResponse discord log failed:", error); + });As per coding guidelines,
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: "Error handling with user-facing messages".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/forms.ts` around lines 447 - 452, The discord.log call after saving a form must be made best-effort so it cannot turn a successful write into a reported failure; wrap the call to discord.log(...) in a try/catch (and optionally enforce a short timeout) inside the handler in forms.ts so any thrown errors or timeouts are caught and not re-thrown, log the failure to the server logger (e.g., ctx.logger.error or console.error) for diagnostics, and apply the same pattern to the other post-write log calls in this file to ensure audit logging never affects the mutation result.packages/api/src/routers/event.ts (1)
295-307:⚠️ Potential issue | 🔴 CriticalRemove the -1 day shift before database insert—dates stored in the database are inconsistent with Discord and Google Calendar.
The code shifts event dates back by 1 day when storing them in the database, but
startLocalIsoandendLocalIso(sent to Discord and Google Calendar) retain the correct dates. This creates a critical mismatch: users see wrong event dates in the app while external calendars show the correct ones.Since
getEventsreturns database values without compensation, all downstream consumers receive incorrect dates. Remove lines 296, 298 (and the corresponding shifts at lines 533, 535 in the update handler):const dayBeforeStart = new Date(startLocalDate); - dayBeforeStart.setDate(dayBeforeStart.getDate() - 1); const dayBeforeEnd = new Date(endLocalDate); - dayBeforeEnd.setDate(dayBeforeEnd.getDate() - 1); await db.insert(Event).values({ ...input, start_datetime: startLocalDate, end_datetime: endLocalDate,If the -1 shift was intentional for timezone handling, add a comment explaining why—but then you'll need to apply +1 compensation in all Event read queries to keep data consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 295 - 307, The database insert is subtracting one day from start/end before saving (via dayBeforeStart.setDate(dayBeforeStart.getDate() - 1) and dayBeforeEnd.setDate(... - 1)) while startLocalIso/endLocalIso (sent to Discord/Google) remain correct, causing a mismatch; remove the day-before adjustments so db.insert(Event).values uses startLocalDate/endLocalDate directly, and also remove the identical -1 shifts in the update handler (the update code that mirrors dayBeforeStart/dayBeforeEnd). If the shift was intentional for timezone handling, instead add a clear comment explaining why and implement a +1 compensation on all Event read paths (e.g., getEvents) so stored values remain consistent with external calendars.
🧹 Nitpick comments (9)
apps/blade/src/app/_components/forms/form-runner.tsx (1)
8-14: Optional: Simplify by removing intermediate type aliases.The type aliases on lines 13-14 add indirection without benefit. You can use the namespaced types directly throughout the file (e.g.,
forms.FormResponsePayload), which is already done for method calls.♻️ Proposed simplification
import * as forms from "@forge/utils/forms.client"; import { InstructionResponseCard } from "~/app/_components/forms/instruction-response-card"; import { QuestionResponseCard } from "~/app/_components/forms/question-response-card"; - -type FormResponsePayload = forms.FormResponsePayload; -type FormResponseUI = forms.FormResponseUI;Then update usages:
- initialResponses?: FormResponseUI; + initialResponses?: forms.FormResponseUI; ... - onSubmit: (payload: FormResponsePayload) => void; + onSubmit: (payload: forms.FormResponsePayload) => void; ... - const [responses, setResponses] = useState<FormResponseUI>( + const [responses, setResponses] = useState<forms.FormResponseUI>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/forms/form-runner.tsx` around lines 8 - 14, Remove the unnecessary intermediate type aliases FormResponsePayload and FormResponseUI: delete the two type alias lines and update any references to use the namespaced types forms.FormResponsePayload and forms.FormResponseUI directly (the import already present as import * as forms). Ensure all occurrences that referenced the aliases in this file (e.g., any prop types, function signatures or local variables) are switched to the forms.* variants.packages/utils/src/forms.ts (3)
169-172: Use the centralized logger instead of console.error.The
eslint-disablecomments indicate console usage is restricted. Since this package already imports alogger(per the discord.ts pattern), consider usinglogger.errorfor consistency with the rest of the utils package.♻️ Suggested fix
} catch (e) { - // eslint-disable-next-line no-console - console.error("Failed to regenerate image URL:", e); + logger.error("Failed to regenerate image URL:", e); }You'll need to import the logger:
import { logger } from "./logger";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/forms.ts` around lines 169 - 172, Replace the console.error usage in the catch block that logs "Failed to regenerate image URL:" with the centralized logger by importing logger (import { logger } from "./logger") and calling logger.error with the same message and the caught error (i.e., update the catch in the function where console.error is used to use logger.error instead); ensure the logger import is added to the top of packages/utils/src/forms.ts and remove the eslint-disable comment.
212-212: Slug generation may produce invalid or duplicate slugs.The current slug logic doesn't handle:
- Multiple consecutive spaces →
"my event"becomes"my--event"- Special characters →
"Event#1!"becomes"event-#1!"- Leading/trailing spaces
Consider a more robust slugification.
♻️ Suggested fix
- const slug_name = input.formData.name.toLowerCase().replaceAll(" ", "-"); + const slug_name = input.formData.name + .toLowerCase() + .trim() + .replace(/[^a-z0-9\s-]/g, "") + .replace(/\s+/g, "-") + .replace(/-+/g, "-");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/forms.ts` at line 212, Replace the fragile slug generation for slug_name with a robust slugify routine: trim input.formData.name, convert to lowercase, normalize whitespace to single spaces, replace spaces with hyphens, remove or replace non-alphanumeric characters (allow only a-z, 0-9 and hyphens), collapse multiple hyphens into one, and strip leading/trailing hyphens; implement this as a helper (e.g., slugify) and use it where slug_name is assigned, and if your system must avoid duplicates add a uniqueness check (e.g., append "-1", "-2" loop) using your existing slug uniqueness function or store lookup.
183-186: Same issue: prefer logger over console.error.For consistency, use
logger.errorhere as well.♻️ Suggested fix
} catch (e) { - // eslint-disable-next-line no-console - console.error("Failed to regenerate video URL:", e); + logger.error("Failed to regenerate video URL:", e); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/forms.ts` around lines 183 - 186, Replace the console.error call in the catch block that logs "Failed to regenerate video URL:" with logger.error; update the catch to call logger.error("Failed to regenerate video URL:", e) (or format the message consistently with other logs) and ensure the surrounding function (where the catch lives) has access to a logger instance—import or accept the same logger used elsewhere in forms.ts if necessary and remove the eslint-disable comment.packages/api/src/routers/event.ts (1)
531-545: Same -1 day shift in updateEvent.The same date adjustment pattern appears in
updateEvent. If this is intentional (e.g., timezone compensation), consider extracting it into a helper function with a descriptive name to clarify the intent.♻️ Extract date adjustment logic
// At the top of the file or in a utils module: function adjustEventDateForDatabase(date: Date): Date { const adjusted = new Date(date); adjusted.setDate(adjusted.getDate() - 1); return adjusted; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 531 - 545, The date adjustment (subtracting one day) used when updating events is duplicated and unclear; extract this into a helper like adjustEventDateForDatabase(date: Date): Date and replace the inline logic that creates dayBeforeStart/dayBeforeEnd in the updateEvent flow with calls to that helper, then use the returned Dates in the db.update(...).set({ start_datetime: ..., end_datetime: ..., ... }) for Event to make the intent explicit and reduce duplication.packages/api/src/routers/auth.ts (1)
4-6: Unused import:permissions.The
permissionsimport on line 4 doesn't appear to be used in this file. OnlydiscordandpermissionsServerare referenced.♻️ Remove unused import
-import { permissions } from "@forge/utils"; import * as discord from "@forge/utils/discord"; import * as permissionsServer from "@forge/utils/permissions.server";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/auth.ts` around lines 4 - 6, Remove the unused import "permissions" from the top of the file where imports are declared (currently: import { permissions } from "@forge/utils";) since only the modules exported as discord and permissionsServer are used; update the import list in packages/api/src/routers/auth.ts to only import the used symbols (discord and permissionsServer) and run the linter or TypeScript compile to ensure no other references to the removed symbol remain.packages/auth/src/config.ts (1)
75-80: Good: callback is now awaited. Consider using logger for error handling.The
awaitonhandleDiscordOAuthCallbackfixes the previous fire-and-forget issue. However, the catch block still usesconsole.errorwith an eslint-disable. For consistency with the utils package, consider importing and using the centralized logger.♻️ Optional: use centralized logger
+import { logger } from "@forge/utils"; + // In the catch block: } catch (error) { - // TODO: remove this eslint-disable - // eslint-disable-next-line no-console - console.error("Error in Discord auto join hook:", error); + logger.error("Error in Discord auto join hook:", error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/src/config.ts` around lines 75 - 80, The catch block after awaiting discord.handleDiscordOAuthCallback(discordUserId) still uses console.error and an eslint-disable; import and use the centralized logger from the utils package (e.g., logger) instead of console, remove the eslint-disable, and replace console.error("Error in Discord auto join hook:", error) with a logger.error call that includes a clear message and the caught error (use the same logger naming pattern used elsewhere in the repo to maintain consistency).packages/utils/src/discord.ts (1)
162-164: Embed footer timestamp is server-locale dependent.
new Date().toLocaleString()produces different formats depending on the server's locale settings. Consider using a fixed format or ISO string for consistency.♻️ Use explicit locale
footer: { - text: new Date().toLocaleString(), + text: new Date().toLocaleString("en-US", { + dateStyle: "medium", + timeStyle: "short", + }), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/discord.ts` around lines 162 - 164, The embed footer currently uses new Date().toLocaleString() which yields server-locale dependent output; change the footer timestamp to a consistent, locale-independent format (e.g., use new Date().toISOString() or format with Intl.DateTimeFormat using a fixed locale/UTC) in the footer object used when building embeds in packages/utils/src/discord.ts so all embeds show a stable, predictable timestamp.packages/api/src/routers/misc.ts (1)
183-186: Redundant timestamp: both footer.text and timestamp field.The embed includes
timestamp: new Date().toISOString()on line 186, which Discord renders automatically. Thefooter.textat line 184 also contains a timestamp. This creates duplicate time information.♻️ Remove redundant footer timestamp
footer: { - text: `Submitted at: ${new Date().toLocaleString()}`, + text: "Knight Hacks Recruiting", }, timestamp: new Date().toISOString(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/misc.ts` around lines 183 - 186, The embed currently sets both footer.text and timestamp (footer.text contains a human-readable time while timestamp: new Date().toISOString() produces Discord's automatic timestamp), causing duplicate times; remove the timestamp property (timestamp: new Date().toISOString()) from the embed object and keep footer.text as the single displayed timestamp so only one time is shown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routers/forms.ts`:
- Around line 31-36: The createForm mutation currently calls
forms.createForm(input) and ignores its return value, changing the tRPC response
to void; update the createForm resolver (the permProcedure .mutation block) to
return the created form by returning the result of forms.createForm(input)
(i.e., replace the discarded await with a returned value) while preserving the
existing permission check via permissions.controlPerms.or(["EDIT_FORMS"], ctx).
In `@packages/api/src/routers/roles.ts`:
- Around line 239-244: getDiscordRoleCounts is declared to return
Promise<Record<string, number> | null> but never returns null; wrap the
discord.api.get call in a try/catch (or otherwise mirror the error-handling
pattern used by the other Discord getters) and return null on failure so the
implementation matches its signature. Locate the getDiscordRoleCounts procedure
and the call to
discord.api.get(`/guilds/${DISCORD.KNIGHTHACKS_GUILD}/roles/member-counts`) and
add error handling that catches exceptions and returns null while keeping the
existing return type, or alternatively remove the | null from the declared
return type if you prefer callers to handle thrown errors.
In `@packages/auth/src/config.ts`:
- Line 10: Replace the relative import "import * as discord from
'../../utils/src/discord';" with the utils package's public import (use the
package export that exposes the discord utilities instead of the src path), e.g.
import from the utils package entrypoint or the package's exported discord path
(ensure the package index exports the discord symbols) so
packages/auth/src/config.ts uses the package API rather than a relative src
import.
In `@packages/utils/package.json`:
- Around line 24-46: The package.json currently lists runtime imports as
dev-only or undeclared; move "@forge/consts", "@forge/db", and "zod" from
devDependencies into dependencies so modules exported by ./forms, ./google, and
./permissions.server can resolve at runtime, and add "next" to peerDependencies
(matching your app's Next.js version) so imports like next/headers are declared
as a peer requirement; update the "dependencies" and "peerDependencies" entries
accordingly and ensure `@trpc/server` remains in peerDependencies as intended.
In `@packages/utils/src/discord.ts`:
- Around line 100-104: The current return expression accesses
members[0]?.user.id which can throw if a partial APIGuildMember has no user;
change the access to safely chain through the whole path (e.g., use
members[0]?.user?.id ?? null) and ensure the callers of this helper handle a
null result; update the function that performs the API call (the code using
members, APIGuildMember and Routes.guildMembersSearch) to use this optional
chaining instead of accessing .id directly.
In `@packages/utils/src/forms.client.ts`:
- Line 1: The file currently imports Zod incorrectly using a default import of
z; replace that with the named export so the symbol z is imported as Zod's named
export (use the named import form for z from zod) and ensure all uses of z in
this module remain unchanged so runtime errors like "z is undefined" are avoided
and the import style matches other modules (validators, env.ts).
In `@packages/utils/src/forms.ts`:
- Around line 231-240: Replace the unsafe spread of ...input when inserting into
FormsSchemas: explicitly map only the allowed columns (e.g., name:
input.formData.name, slugName: slug_name, formValidatorJson: jsonSchema.schema,
plus any permitted metadata fields from input) and do not pass input.section
directly; instead derive sectionId as you do and if sectionId is null set
section to a safe default like "General" or null (or omit the section column) to
avoid orphaned references; update the insert call that uses FormsSchemas,
slug_name, jsonSchema.schema, sectionId and formData to use this explicit field
mapping.
In `@packages/utils/src/permissions.server.ts`:
- Around line 43-60: getJudgeSessionFromCookie currently lets DB errors bubble
up and cause a 500; make it consistent with isJudgeAdmin by treating transient
DB failures as "not authenticated." Wrap the DB query in a try/catch inside
getJudgeSessionFromCookie, catch any error from the
db.select(...).from(JudgeSession)... call, optionally log the error, and return
null on failure so the function always returns a JudgeSession-like object or
null rather than throwing.
---
Outside diff comments:
In `@apps/blade/src/app/_components/forms/form-runner.tsx`:
- Around line 188-191: The error message wrapper that renders when submitError
is truthy should include accessibility attributes so screen readers announce it;
update the JSX in the FormRunner component where submitError is rendered (the
conditional that returns the <div className="rounded-md border
border-destructive bg-destructive/10 p-4 text-destructive">) to add role="alert"
(and optionally aria-atomic="true") to that div so assistive technologies are
notified when the error appears.
- Line 128: The empty banner div in the FormRunner JSX renders when form.banner
is truthy but contains no content; update the conditional block in
apps/blade/src/app/_components/forms/form-runner.tsx to either remove the whole
`{form.banner && <div className="overflow-hidden rounded-lg"></div>}` check or
replace it with actual banner rendering (e.g., render an <img> or background
image using the `form.banner` value, include an accessible alt or aria-label,
and preserve the wrapper styles such as "overflow-hidden rounded-lg"); ensure
you update the same JSX conditional that references `form.banner` so the banner
displays correctly or the dead code is removed.
In `@packages/api/src/routers/dues-payment.ts`:
- Around line 85-90: The current discord.log call is unconditional and may
record payments for open/expired/unpaid Stripe sessions; update the code that
handles the result of checkout.sessions.retrieve() (the session variable) to
only call discord.log when the Stripe session indicates a successful payment —
e.g., check session.payment_status === "paid" (and optionally session.status ===
"complete") before logging; ensure you reference the retrieved session object
and gate the discord.log invocation (the existing discord.log call and its
payload) behind that conditional so only truly paid sessions produce the "Dues
Paid" audit.
In `@packages/api/src/routers/event.ts`:
- Around line 295-307: The database insert is subtracting one day from start/end
before saving (via dayBeforeStart.setDate(dayBeforeStart.getDate() - 1) and
dayBeforeEnd.setDate(... - 1)) while startLocalIso/endLocalIso (sent to
Discord/Google) remain correct, causing a mismatch; remove the day-before
adjustments so db.insert(Event).values uses startLocalDate/endLocalDate
directly, and also remove the identical -1 shifts in the update handler (the
update code that mirrors dayBeforeStart/dayBeforeEnd). If the shift was
intentional for timezone handling, instead add a clear comment explaining why
and implement a +1 compensation on all Event read paths (e.g., getEvents) so
stored values remain consistent with external calendars.
In `@packages/api/src/routers/forms.ts`:
- Around line 447-452: The discord.log call after saving a form must be made
best-effort so it cannot turn a successful write into a reported failure; wrap
the call to discord.log(...) in a try/catch (and optionally enforce a short
timeout) inside the handler in forms.ts so any thrown errors or timeouts are
caught and not re-thrown, log the failure to the server logger (e.g.,
ctx.logger.error or console.error) for diagnostics, and apply the same pattern
to the other post-write log calls in this file to ensure audit logging never
affects the mutation result.
In `@packages/api/src/routers/roles.ts`:
- Around line 478-483: The nested loops currently call
db.query.Permissions.findFirst for each userId/roleId pair (using cachedRoles,
cachedUsers), causing an N×M query pattern; instead, perform a single preload
query (e.g., db.query.Permissions.findMany) that filters for userId IN all
cachedUsers keys and roleId IN all cachedRoles keys, build an in-memory index
keyed by userId and roleId (e.g., `${userId}:${roleId}`), and then replace the
per-pair findFirst call with a lookup from that map inside the loops so all
existing permissions are resolved from memory without extra DB queries.
- Around line 337-358: The current flow calls discord.addRoleToMember before
persisting the Permissions row (db.insert(Permissions).values(...)), which can
leave Blade and Discord out of sync if the DB write fails; change the order so
you first await db.insert(Permissions).values({ roleId: input.roleId, userId:
input.userId }) and only then attempt discord.addRoleToMember inside a
try/catch, logging any discord error to the audit but treating the DB as the
source of truth; apply the same reordering to the revoke and batch paths that
currently mutate Discord first (the sections using discord.removeRoleFromMember
/ batch role ops) so DB writes happen before Discord sync.
In `@packages/db/scripts/seed_devdb.ts`:
- Around line 433-448: The discord.log calls can throw and must not flip the
process exit or block cleanup; create a safe wrapper (e.g., safeDiscordLog or
try/catch around discord.log) that catches and swallows/logs errors without
rethrowing, then: 1) on the success path (after the successful save) call
safeDiscordLog so a logging failure won’t cause process.exit(1) and still call
process.exit(0); 2) in the catch handler move the cleanUp() invocation before
any discord.log call and call safeDiscordLog afterwards so cleanup runs even if
logging fails; reference discord.log, cleanUp(), stringify(), and process.exit
in the changes.
---
Nitpick comments:
In `@apps/blade/src/app/_components/forms/form-runner.tsx`:
- Around line 8-14: Remove the unnecessary intermediate type aliases
FormResponsePayload and FormResponseUI: delete the two type alias lines and
update any references to use the namespaced types forms.FormResponsePayload and
forms.FormResponseUI directly (the import already present as import * as forms).
Ensure all occurrences that referenced the aliases in this file (e.g., any prop
types, function signatures or local variables) are switched to the forms.*
variants.
In `@packages/api/src/routers/auth.ts`:
- Around line 4-6: Remove the unused import "permissions" from the top of the
file where imports are declared (currently: import { permissions } from
"@forge/utils";) since only the modules exported as discord and
permissionsServer are used; update the import list in
packages/api/src/routers/auth.ts to only import the used symbols (discord and
permissionsServer) and run the linter or TypeScript compile to ensure no other
references to the removed symbol remain.
In `@packages/api/src/routers/event.ts`:
- Around line 531-545: The date adjustment (subtracting one day) used when
updating events is duplicated and unclear; extract this into a helper like
adjustEventDateForDatabase(date: Date): Date and replace the inline logic that
creates dayBeforeStart/dayBeforeEnd in the updateEvent flow with calls to that
helper, then use the returned Dates in the db.update(...).set({ start_datetime:
..., end_datetime: ..., ... }) for Event to make the intent explicit and reduce
duplication.
In `@packages/api/src/routers/misc.ts`:
- Around line 183-186: The embed currently sets both footer.text and timestamp
(footer.text contains a human-readable time while timestamp: new
Date().toISOString() produces Discord's automatic timestamp), causing duplicate
times; remove the timestamp property (timestamp: new Date().toISOString()) from
the embed object and keep footer.text as the single displayed timestamp so only
one time is shown.
In `@packages/auth/src/config.ts`:
- Around line 75-80: The catch block after awaiting
discord.handleDiscordOAuthCallback(discordUserId) still uses console.error and
an eslint-disable; import and use the centralized logger from the utils package
(e.g., logger) instead of console, remove the eslint-disable, and replace
console.error("Error in Discord auto join hook:", error) with a logger.error
call that includes a clear message and the caught error (use the same logger
naming pattern used elsewhere in the repo to maintain consistency).
In `@packages/utils/src/discord.ts`:
- Around line 162-164: The embed footer currently uses new
Date().toLocaleString() which yields server-locale dependent output; change the
footer timestamp to a consistent, locale-independent format (e.g., use new
Date().toISOString() or format with Intl.DateTimeFormat using a fixed
locale/UTC) in the footer object used when building embeds in
packages/utils/src/discord.ts so all embeds show a stable, predictable
timestamp.
In `@packages/utils/src/forms.ts`:
- Around line 169-172: Replace the console.error usage in the catch block that
logs "Failed to regenerate image URL:" with the centralized logger by importing
logger (import { logger } from "./logger") and calling logger.error with the
same message and the caught error (i.e., update the catch in the function where
console.error is used to use logger.error instead); ensure the logger import is
added to the top of packages/utils/src/forms.ts and remove the eslint-disable
comment.
- Line 212: Replace the fragile slug generation for slug_name with a robust
slugify routine: trim input.formData.name, convert to lowercase, normalize
whitespace to single spaces, replace spaces with hyphens, remove or replace
non-alphanumeric characters (allow only a-z, 0-9 and hyphens), collapse multiple
hyphens into one, and strip leading/trailing hyphens; implement this as a helper
(e.g., slugify) and use it where slug_name is assigned, and if your system must
avoid duplicates add a uniqueness check (e.g., append "-1", "-2" loop) using
your existing slug uniqueness function or store lookup.
- Around line 183-186: Replace the console.error call in the catch block that
logs "Failed to regenerate video URL:" with logger.error; update the catch to
call logger.error("Failed to regenerate video URL:", e) (or format the message
consistently with other logs) and ensure the surrounding function (where the
catch lives) has access to a logger instance—import or accept the same logger
used elsewhere in forms.ts if necessary and remove the eslint-disable comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 74c8ae75-7481-4af7-a2e5-51370233b056
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (27)
apps/blade/src/app/_components/forms/connection-handler.tsapps/blade/src/app/_components/forms/form-responder-client.tsxapps/blade/src/app/_components/forms/form-runner.tsxapps/blade/src/app/_components/forms/form-view-edit-client.tsxapps/blade/src/app/judge/session/route.tspackages/api/package.jsonpackages/api/src/routers/auth.tspackages/api/src/routers/dues-payment.tspackages/api/src/routers/event-feedback.tspackages/api/src/routers/event.tspackages/api/src/routers/forms.tspackages/api/src/routers/hackers/mutations.tspackages/api/src/routers/member.tspackages/api/src/routers/misc.tspackages/api/src/routers/roles.tspackages/api/src/trpc.tspackages/auth/src/config.tspackages/db/scripts/seed_devdb.tspackages/utils/package.jsonpackages/utils/src/discord.tspackages/utils/src/forms.client.tspackages/utils/src/forms.tspackages/utils/src/google.tspackages/utils/src/index.tspackages/utils/src/permissions.server.tspackages/utils/src/permissions.tspackages/utils/src/stripe.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/utils/src/stripe.ts
- apps/blade/src/app/_components/forms/form-view-edit-client.tsx
- apps/blade/src/app/_components/forms/form-responder-client.tsx
- packages/api/src/routers/event-feedback.ts
- packages/api/src/trpc.ts
- packages/api/src/routers/hackers/mutations.ts
- packages/utils/src/permissions.ts
- apps/blade/src/app/_components/forms/connection-handler.ts
must108
approved these changes
Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
As discussed in #340, we have a bunch of utils that need to be consolidated into one package.
What
Resolves: #340
We are moving things to this package. Namely:
@forge/api/src/utils.ts)Test Plan
TBD
Checklist
TBD
db:pushbefore mergingSummary by CodeRabbit