feat: add automated welcome campaign system#275
Merged
Conversation
Add a parallel automated campaigns layer alongside the existing manual campaigns. The welcome campaign fires bi-monthly (1st and 15th) to new members since the last run, with a one-time bootstrap for early adopters (signupAfter: 2025-03-19). - Schema: add `type` column to campaigns table, new `automated_campaign_configs` table - Service: computeNextRunAt, ensureWelcomeConfig, bootstrapWelcomeCampaign, runWelcomeCampaign, processAutomatedCampaigns - Scheduler: "0 0 1,15 * *" cron for automated campaign processing - API: GET/PATCH/POST endpoints under /api/admin/automated-campaigns - Frontend: Manual/Automated tabs, Auto badge on campaign list, WelcomeCampaignCard with enable toggle, template editor, trigger button - Tests: 18 new tests covering computeNextRunAt boundaries, bootstrap idempotency, and processAutomatedCampaigns logic https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
…ssues
- Remove {{unsubscribe_url}} footer from HTML template (sendSingleCampaignEmail
already appends its own unsubscribe footer)
- Add onConflictDoNothing to ensureWelcomeConfig for concurrent deploy safety
- Set lastRunAt BEFORE running campaign to prevent duplicate bootstrap on retry
- Add atomic claim (UPDATE...WHERE nextRunAt=old) in processAutomatedCampaigns
to prevent duplicate sends on horizontal scaling
- Route bootstrap errors to ErrorLogger (not just console.error)
- Add .min(1) validation on subject/htmlBody in PATCH endpoint
- Remove unused imports and (c as any) type assertion
- Update tests for new onConflictDoNothing flow
https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds automated campaigns: DB table and campaign type, server service for scheduling/execution, admin routes (list/patch/trigger), cron at 00:00 UTC on 1st/15th, client hooks/UI for management, and comprehensive tests covering scheduling and orchestration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Client UI
participant API as Server API
participant Service as Automated<br/>Campaign Service
participant DB as Database
User->>Client: Click "Trigger Now"
Client->>Client: Show confirmation dialog
User->>Client: Confirm
Client->>API: POST /api/admin/automated-campaigns/:key/trigger (optional signupAfter)
API->>Service: runWelcomeCampaign(signupAfter, signupBefore, configId)
Service->>DB: Load config by id
DB-->>Service: AutomatedCampaignConfig
Service->>DB: Query users by signup window
DB-->>Service: Recipients[]
alt Recipients found
Service->>DB: Insert campaigns record (type: 'automated')
DB-->>Service: campaign.id
Service->>Service: triggerCampaignSend(campaign.id)
Service-->>API: {campaignId, totalRecipients}
else No recipients
Service-->>API: {skipped: true}
end
API->>DB: Update config lastRunAt, nextRunAt
API-->>Client: Success response
Client->>Client: Show success toast
Client->>Client: Invalidate queries
sequenceDiagram
autonumber
participant Scheduler as Cron Scheduler
participant Service as Automated<br/>Campaign Service
participant DB as Database
participant Logger as ErrorLogger
Scheduler->>Scheduler: Trigger at 00:00 UTC (1st & 15th)
Scheduler->>Service: processAutomatedCampaigns()
Service->>DB: SELECT enabled configs WHERE nextRunAt <= now
DB-->>Service: [AutomatedCampaignConfig...]
loop For each config
Service->>DB: UPDATE (atomic claim) WHERE id = ? AND nextRunAt = ?
alt Claim successful
Service->>Service: runWelcomeCampaign()
Service->>DB: Insert campaign + trigger sends
Service->>DB: Update nextRunAt
else Claim failed
Service->>Service: Skip (another instance claimed)
end
end
Service-->>Scheduler: void
Scheduler->>Logger: Log failures (if any)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
- Replace error.message leak with static error string in trigger endpoint - Add rate limiter (2 req/15min) to manual trigger endpoint to prevent accidental duplicate campaign sends https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
Dead code flagged by architecture review — defined but never imported, and incorrectly uses createInsertSchema for a "select" schema. https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
This was referenced Mar 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/hooks/use-campaigns.ts`:
- Around line 238-286: The client hardcodes AUTO_CAMPAIGNS_KEY and derived URLs
used by useAutomatedCampaigns, useUpdateAutomatedCampaign, and
useTriggerAutomatedCampaign; move this contract into the shared api object in
shared/routes.ts by adding route entries for list, patch (/:key) and trigger
(/:key/trigger) with method, path, responses and optional input, then replace
AUTO_CAMPAIGNS_KEY and string interpolation in those three hooks to import and
reference the shared api route constants instead of hardcoded strings so client
and server share the canonical route definitions.
In `@server/routes.ts`:
- Around line 2487-2515: The manual trigger is not idempotent: two requests can
read the same automatedConfigsTable.lastRunAt and both call
automatedCampaignService.runWelcomeCampaign, causing duplicate emails; fix by
performing an atomic claim-and-advance in storage/service before running the
campaign. Modify the flow so the route calls a single service method (e.g.,
automatedCampaignService.claimAndRunWelcomeCampaign or add a DB-backed claim
function) that 1) reads automatedConfigsTable.lastRunAt and nextRunAt, 2)
atomically updates lastRunAt/nextRunAt (compare-and-swap using where id and
expected lastRunAt) to reserve the window, and 3) only if the CAS succeeds runs
runWelcomeCampaign and returns the result; if CAS fails, return a skipped/409
response. Ensure computeNextRunAt is used when computing the new nextRunAt
inside that atomic operation and remove the separate db.update after
runWelcomeCampaign in the route so the update occurs only within the claimed
transaction.
- Around line 2405-2465: The route handlers "/api/admin/automated-campaigns" and
"/api/admin/automated-campaigns/:key" are doing DB access via
automatedConfigsTable and inline Zod validation in the route; move DB logic into
the storage abstraction and reuse shared Zod schemas: add methods on IStorage
(e.g., getAutomatedConfigs() and updateAutomatedConfig(key, updates))
implemented in server/storage.ts that encapsulate the Drizzle queries touching
automatedConfigsTable, and create a shared Zod schema (e.g.,
AutomatedCampaignUpdateSchema) in shared/routes.ts to validate req.body; then
update the route handlers to call authStorage.getAutomatedConfigs() /
authStorage.updateAutomatedConfig(key, parsed.data) and validate with the shared
schema instead of inline queries/definitions.
In `@server/services/automatedCampaigns.ts`:
- Around line 327-353: The code currently sets lastRunAt/nextRunAt on
automatedCampaignConfigs before calling runWelcomeCampaign, which loses users if
runWelcomeCampaign fails; change the flow so claiming and marking as processed
are separated: either add/use a dedicated lease/claim column (e.g.,
claimToken/claimExpiry) on automatedCampaignConfigs and atomically set that to
claim the job before calling runWelcomeCampaign, leaving lastRunAt/nextRunAt
unchanged until after runWelcomeCampaign returns success, or keep the existing
claim but do not update lastRunAt (or nextRunAt) until after runWelcomeCampaign
completes successfully; ensure you update lastRunAt/nextRunAt in a final
db.update() only when runWelcomeCampaign returns successfully (use config.id to
locate the record) and release/expire the claim on failure so another instance
can retry.
- Around line 213-231: The current bootstrap uses a read-time guard
(config.lastRunAt) which is racy under concurrency; change the claim to be
atomic by performing the update with a conditional WHERE
(automatedCampaignConfigs.id = config.id AND automatedCampaignConfigs.lastRunAt
IS NULL) and use the DB's RETURNING to determine whether this process claimed
the bootstrap; if the update returns no rows, bail and do not send the cohort.
Modify the block that calls
db.update(...).set({...}).where(eq(automatedCampaignConfigs.id, config.id)) to
include the additional lastRunAt IS NULL condition, capture the returned row(s)
from db.update and only proceed to send the welcome campaign when a row was
returned, leaving computeNextRunAt, now, and updatedAt logic intact.
In `@server/services/scheduler.ts`:
- Around line 413-423: The cron task scheduling for automated campaigns uses
cron.schedule without a timezone, so it will run in the host's local timezone
despite the comment; update the cron.schedule call that pushes into cronTasks
(the one invoking processAutomatedCampaigns) to pass the timezone option {
timezone: "UTC" } as the second argument so the job runs at 00:00 UTC on the 1st
and 15th, leaving the try/catch and ErrorLogger.error handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62fdd601-636e-4bcd-982b-cdc759e444fe
📒 Files selected for processing (8)
client/src/hooks/use-campaigns.tsclient/src/pages/AdminCampaigns.tsxserver/routes.tsserver/services/automatedCampaigns.test.tsserver/services/automatedCampaigns.tsserver/services/scheduler.test.tsserver/services/scheduler.tsshared/schema.ts
Comment on lines
+238
to
+286
| const AUTO_CAMPAIGNS_KEY = "/api/admin/automated-campaigns"; | ||
|
|
||
| // GET /api/admin/automated-campaigns | ||
| export function useAutomatedCampaigns() { | ||
| return useQuery<AutomatedCampaignConfig[]>({ | ||
| queryKey: [AUTO_CAMPAIGNS_KEY], | ||
| queryFn: () => fetchJson(AUTO_CAMPAIGNS_KEY), | ||
| }); | ||
| } | ||
|
|
||
| // PATCH /api/admin/automated-campaigns/:key | ||
| export function useUpdateAutomatedCampaign() { | ||
| const queryClient = useQueryClient(); | ||
| const { toast } = useToast(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: ({ key, ...updates }: { key: string } & Partial<{ | ||
| subject: string; | ||
| htmlBody: string; | ||
| textBody: string; | ||
| enabled: boolean; | ||
| }>) => | ||
| fetchJson(`${AUTO_CAMPAIGNS_KEY}/${key}`, { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(updates), | ||
| }), | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: [AUTO_CAMPAIGNS_KEY] }); | ||
| toast({ title: "Config updated", description: "Automated campaign config saved." }); | ||
| }, | ||
| onError: (err: Error) => { | ||
| toast({ title: "Error", description: err.message, variant: "destructive" }); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // POST /api/admin/automated-campaigns/:key/trigger | ||
| export function useTriggerAutomatedCampaign() { | ||
| const queryClient = useQueryClient(); | ||
| const { toast } = useToast(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: ({ key, signupAfter }: { key: string; signupAfter?: string }) => | ||
| fetchJson(`${AUTO_CAMPAIGNS_KEY}/${key}/trigger`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(signupAfter ? { signupAfter } : {}), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Promote this endpoint contract into shared/routes.ts.
AUTO_CAMPAIGNS_KEY and the derived URLs introduce a new client/server contract outside the shared api object. That makes path, input, and response drift likely as the server evolves.
As per coding guidelines "Define route constants in the api object in shared/routes.ts with method, path, responses, and optional input. Never hardcode route path strings like '/api/monitors' in server or client code. Reference these constants throughout the codebase."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/use-campaigns.ts` around lines 238 - 286, The client
hardcodes AUTO_CAMPAIGNS_KEY and derived URLs used by useAutomatedCampaigns,
useUpdateAutomatedCampaign, and useTriggerAutomatedCampaign; move this contract
into the shared api object in shared/routes.ts by adding route entries for list,
patch (/:key) and trigger (/:key/trigger) with method, path, responses and
optional input, then replace AUTO_CAMPAIGNS_KEY and string interpolation in
those three hooks to import and reference the shared api route constants instead
of hardcoded strings so client and server share the canonical route definitions.
Comment on lines
+2405
to
+2465
| // GET /api/admin/automated-campaigns — list all configs | ||
| app.get("/api/admin/automated-campaigns", isAuthenticated, async (req: any, res) => { | ||
| try { | ||
| const userId = req.user?.claims?.sub; | ||
| if (!userId) return res.status(401).json({ message: "Unauthorized" }); | ||
| const user = await authStorage.getUser(userId); | ||
| if (!user || user.tier !== "power") return res.status(403).json({ message: "Admin access required" }); | ||
| if (userId !== APP_OWNER_ID) return res.status(403).json({ message: "Owner access required" }); | ||
|
|
||
| const configs = await db | ||
| .select() | ||
| .from(automatedConfigsTable) | ||
| .orderBy(automatedConfigsTable.key); | ||
|
|
||
| res.json(configs); | ||
| } catch (error: any) { | ||
| console.error("Error fetching automated campaign configs:", error); | ||
| res.status(500).json({ message: "Failed to fetch automated campaign configs" }); | ||
| } | ||
| }); | ||
|
|
||
| // PATCH /api/admin/automated-campaigns/:key — update config | ||
| app.patch("/api/admin/automated-campaigns/:key", isAuthenticated, async (req: any, res) => { | ||
| try { | ||
| const userId = req.user?.claims?.sub; | ||
| if (!userId) return res.status(401).json({ message: "Unauthorized" }); | ||
| const user = await authStorage.getUser(userId); | ||
| if (!user || user.tier !== "power") return res.status(403).json({ message: "Admin access required" }); | ||
| if (userId !== APP_OWNER_ID) return res.status(403).json({ message: "Owner access required" }); | ||
|
|
||
| const { key } = req.params; | ||
| const updateSchema = z.object({ | ||
| subject: z.string().min(1).optional(), | ||
| htmlBody: z.string().min(1).optional(), | ||
| textBody: z.string().optional(), | ||
| enabled: z.boolean().optional(), | ||
| }).strict(); | ||
|
|
||
| const parsed = updateSchema.safeParse(req.body); | ||
| if (!parsed.success) { | ||
| return res.status(400).json({ message: "Invalid request body", errors: parsed.error.flatten() }); | ||
| } | ||
|
|
||
| const updates = parsed.data; | ||
| if (Object.keys(updates).length === 0) { | ||
| return res.status(400).json({ message: "No fields to update" }); | ||
| } | ||
|
|
||
| const [updated] = await db | ||
| .update(automatedConfigsTable) | ||
| .set({ ...updates, updatedAt: new Date() }) | ||
| .where(eq(automatedConfigsTable.key, key)) | ||
| .returning(); | ||
|
|
||
| if (!updated) return res.status(404).json({ message: "Config not found" }); | ||
|
|
||
| res.json(updated); | ||
| } catch (error: any) { | ||
| console.error("Error updating automated campaign config:", error); | ||
| res.status(500).json({ message: "Failed to update automated campaign config" }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep the route layer thin.
These new handlers read/write db directly and define request schemas inline. That sidesteps the shared validation contract and storage abstraction the repo requires for new API surfaces.
As per coding guidelines "Never put database queries or Drizzle ORM calls directly in route handlers — all database access must go through methods on the IStorage interface implemented in server/storage.ts." and "Validate all incoming request bodies, query parameters, and path parameters using Zod schemas defined in shared/routes.ts."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes.ts` around lines 2405 - 2465, The route handlers
"/api/admin/automated-campaigns" and "/api/admin/automated-campaigns/:key" are
doing DB access via automatedConfigsTable and inline Zod validation in the
route; move DB logic into the storage abstraction and reuse shared Zod schemas:
add methods on IStorage (e.g., getAutomatedConfigs() and
updateAutomatedConfig(key, updates)) implemented in server/storage.ts that
encapsulate the Drizzle queries touching automatedConfigsTable, and create a
shared Zod schema (e.g., AutomatedCampaignUpdateSchema) in shared/routes.ts to
validate req.body; then update the route handlers to call
authStorage.getAutomatedConfigs() / authStorage.updateAutomatedConfig(key,
parsed.data) and validate with the shared schema instead of inline
queries/definitions.
Comment on lines
+2487
to
+2515
| const [config] = await db | ||
| .select() | ||
| .from(automatedConfigsTable) | ||
| .where(eq(automatedConfigsTable.key, key)) | ||
| .limit(1); | ||
|
|
||
| if (!config) return res.status(404).json({ message: "Config not found" }); | ||
|
|
||
| const signupAfter = parsed.data.signupAfter | ||
| ? new Date(parsed.data.signupAfter) | ||
| : config.lastRunAt || new Date("2025-03-19T00:00:00Z"); | ||
| const signupBefore = new Date(); | ||
|
|
||
| const result = await automatedCampaignService.runWelcomeCampaign({ | ||
| signupAfter, | ||
| signupBefore, | ||
| configId: config.id, | ||
| }); | ||
|
|
||
| if ("skipped" in result) { | ||
| res.json({ skipped: true, reason: "No new recipients" }); | ||
| } else { | ||
| // Update lastRunAt and nextRunAt | ||
| const now = new Date(); | ||
| const nextRunAt = automatedCampaignService.computeNextRunAt(now); | ||
| await db | ||
| .update(automatedConfigsTable) | ||
| .set({ lastRunAt: now, nextRunAt, updatedAt: now }) | ||
| .where(eq(automatedConfigsTable.id, config.id)); |
There was a problem hiding this comment.
Make the manual trigger idempotent before it writes emails.
Two concurrent requests can both read the same config.lastRunAt on Lines 2487-2498, both call runWelcomeCampaign() on Lines 2500-2504, and both email the same cohort before either update on Lines 2512-2515 lands. Reuse the scheduler’s compare-and-swap claim logic here, or move this flow behind one service/storage method that claims and advances the window atomically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes.ts` around lines 2487 - 2515, The manual trigger is not
idempotent: two requests can read the same automatedConfigsTable.lastRunAt and
both call automatedCampaignService.runWelcomeCampaign, causing duplicate emails;
fix by performing an atomic claim-and-advance in storage/service before running
the campaign. Modify the flow so the route calls a single service method (e.g.,
automatedCampaignService.claimAndRunWelcomeCampaign or add a DB-backed claim
function) that 1) reads automatedConfigsTable.lastRunAt and nextRunAt, 2)
atomically updates lastRunAt/nextRunAt (compare-and-swap using where id and
expected lastRunAt) to reserve the window, and 3) only if the CAS succeeds runs
runWelcomeCampaign and returns the result; if CAS fails, return a skipped/409
response. Ensure computeNextRunAt is used when computing the new nextRunAt
inside that atomic operation and remove the separate db.update after
runWelcomeCampaign in the route so the update occurs only within the claimed
transaction.
Comment on lines
+327
to
+353
| // Atomically claim this run by updating nextRunAt + lastRunAt. | ||
| // If another instance already claimed it, the WHERE won't match and we skip. | ||
| const [claimed] = await db | ||
| .update(automatedCampaignConfigs) | ||
| .set({ | ||
| lastRunAt: now, | ||
| nextRunAt, | ||
| updatedAt: now, | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(automatedCampaignConfigs.id, config.id), | ||
| eq(automatedCampaignConfigs.nextRunAt, config.nextRunAt), | ||
| ) | ||
| ) | ||
| .returning(); | ||
|
|
||
| if (!claimed) { | ||
| console.log(`[AutoCampaign] Config '${config.key}' already claimed by another instance, skipping.`); | ||
| continue; | ||
| } | ||
|
|
||
| const result = await runWelcomeCampaign({ | ||
| signupAfter, | ||
| signupBefore, | ||
| configId: config.id, | ||
| }); |
There was a problem hiding this comment.
Don't treat a claimed run as a completed run.
Lines 329-342 move lastRunAt before runWelcomeCampaign() is attempted on Lines 349-353. That timestamp becomes the next signupAfter, so any failure after the claim marks the window as processed and those users are skipped forever. Keep a separate claim/lease marker, or only advance lastRunAt after the campaign has been created and handed off successfully.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/automatedCampaigns.ts` around lines 327 - 353, The code
currently sets lastRunAt/nextRunAt on automatedCampaignConfigs before calling
runWelcomeCampaign, which loses users if runWelcomeCampaign fails; change the
flow so claiming and marking as processed are separated: either add/use a
dedicated lease/claim column (e.g., claimToken/claimExpiry) on
automatedCampaignConfigs and atomically set that to claim the job before calling
runWelcomeCampaign, leaving lastRunAt/nextRunAt unchanged until after
runWelcomeCampaign returns success, or keep the existing claim but do not update
lastRunAt (or nextRunAt) until after runWelcomeCampaign completes successfully;
ensure you update lastRunAt/nextRunAt in a final db.update() only when
runWelcomeCampaign returns successfully (use config.id to locate the record) and
release/expire the claim on failure so another instance can retry.
- Make bootstrap claim atomic with WHERE lastRunAt IS NULL + RETURNING
to prevent concurrent deploys from sending duplicate welcome emails
- Add { timezone: "UTC" } to automated campaign cron.schedule to ensure
it fires at 00:00 UTC regardless of host timezone
https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
server/routes.ts (1)
2493-2521:⚠️ Potential issue | 🔴 CriticalClaim the manual run before sending any emails.
Lines 2493-2521 still split read → send → update. Two concurrent triggers can send the same cohort twice, a crash after
runWelcomeCampaign()can replay the same window, and the laternowon Line 2516 will skip signups created after Line 2504 but before the update. Reuse the scheduler’s compare-and-swap claim with one capturedrunAttimestamp and only send when the claim succeeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes.ts` around lines 2493 - 2521, The handler currently reads config, calls automatedCampaignService.runWelcomeCampaign, then updates automatedConfigsTable, which allows double-sends or replay on crash; instead perform a compare-and-swap claim first: capture the current lastRunAt (config.lastRunAt) and compute a single runAt timestamp (e.g., now), then attempt an atomic update on automatedConfigsTable that sets lastRunAt=runAt, nextRunAt=automatedCampaignService.computeNextRunAt(runAt), updatedAt=runAt with a WHERE clause that matches the config id AND the original lastRunAt (or other claim marker); only if that update affects 1 row proceed to call automatedCampaignService.runWelcomeCampaign using the captured runAt and signup window derived from runAt, otherwise return skipped; this ensures the claim is owned before sending and avoids races/crashes replaying the same window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes.ts`:
- Around line 2405-2423: Replace the hardcoded "/api/admin/automated-campaigns"
handler with a route constant defined in the shared routes registry (add an
entry like api.admin.automatedCampaigns with method, path, responses and
optional input in shared/routes.ts) and reference that constant instead of the
literal path in server/routes.ts; update all related handlers mentioned (lines
2427-2465, 2468-2528) the same way. Also change every response payload to
conform to the standard error/response contract ({ message: string, code: string
}) and use the declared route.responses shapes from shared/routes.ts for success
and error responses so client/server stay consistent. Ensure you use the unique
identifiers present in the file (the Express handler for automated campaign
endpoints in server/routes.ts and the api object in shared/routes.ts) when
making these edits.
In `@shared/schema.ts`:
- Around line 144-145: The campaigns.type column is currently free-form text so
invalid values can be persisted; change its schema definition from
text("type")... to a constrained enum or a check constraint (e.g., create a DB
enum "campaign_type" with values "manual" and "automated" or add a CHECK("type"
IN ('manual','automated'))), keep the default as "manual" and notNull, and
update any related validation (insertCampaignSchema) to use the same enum
type/values; also add a migration to alter the existing column to the new
enum/check and convert or reject invalid rows.
---
Duplicate comments:
In `@server/routes.ts`:
- Around line 2493-2521: The handler currently reads config, calls
automatedCampaignService.runWelcomeCampaign, then updates automatedConfigsTable,
which allows double-sends or replay on crash; instead perform a compare-and-swap
claim first: capture the current lastRunAt (config.lastRunAt) and compute a
single runAt timestamp (e.g., now), then attempt an atomic update on
automatedConfigsTable that sets lastRunAt=runAt,
nextRunAt=automatedCampaignService.computeNextRunAt(runAt), updatedAt=runAt with
a WHERE clause that matches the config id AND the original lastRunAt (or other
claim marker); only if that update affects 1 row proceed to call
automatedCampaignService.runWelcomeCampaign using the captured runAt and signup
window derived from runAt, otherwise return skipped; this ensures the claim is
owned before sending and avoids races/crashes replaying the same window.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 45aa8433-a9ec-4ca1-9824-a633d1e989b5
📒 Files selected for processing (2)
server/routes.tsshared/schema.ts
Comment on lines
+2405
to
+2423
| // GET /api/admin/automated-campaigns — list all configs | ||
| app.get("/api/admin/automated-campaigns", isAuthenticated, async (req: any, res) => { | ||
| try { | ||
| const userId = req.user?.claims?.sub; | ||
| if (!userId) return res.status(401).json({ message: "Unauthorized" }); | ||
| const user = await authStorage.getUser(userId); | ||
| if (!user || user.tier !== "power") return res.status(403).json({ message: "Admin access required" }); | ||
| if (userId !== APP_OWNER_ID) return res.status(403).json({ message: "Owner access required" }); | ||
|
|
||
| const configs = await db | ||
| .select() | ||
| .from(automatedConfigsTable) | ||
| .orderBy(automatedConfigsTable.key); | ||
|
|
||
| res.json(configs); | ||
| } catch (error: any) { | ||
| console.error("Error fetching automated campaign configs:", error); | ||
| res.status(500).json({ message: "Failed to fetch automated campaign configs" }); | ||
| } |
There was a problem hiding this comment.
Define the automated-campaign endpoints in @shared/routes.
These handlers introduce a new API surface with hardcoded path strings and non-standard error bodies ({ message }, { message, errors }, and the rate-limit { message } payload). That bypasses the shared route/error contract and makes client/server handling drift-prone immediately. As per coding guidelines "Define route constants in the api object in shared/routes.ts with method, path, responses, and optional input. Never hardcode route path strings like '/api/monitors' in server or client code. Reference these constants throughout the codebase." and "All API error responses must follow the { message: string, code: string } JSON shape."
Also applies to: 2427-2465, 2468-2528
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes.ts` around lines 2405 - 2423, Replace the hardcoded
"/api/admin/automated-campaigns" handler with a route constant defined in the
shared routes registry (add an entry like api.admin.automatedCampaigns with
method, path, responses and optional input in shared/routes.ts) and reference
that constant instead of the literal path in server/routes.ts; update all
related handlers mentioned (lines 2427-2465, 2468-2528) the same way. Also
change every response payload to conform to the standard error/response contract
({ message: string, code: string }) and use the declared route.responses shapes
from shared/routes.ts for success and error responses so client/server stay
consistent. Ensure you use the unique identifiers present in the file (the
Express handler for automated campaign endpoints in server/routes.ts and the api
object in shared/routes.ts) when making these edits.
Comment on lines
+144
to
145
| type: text("type").default("manual").notNull(), // 'manual' | 'automated' | ||
| filters: jsonb("filters"), // { tier?: string[], signupBefore?, signupAfter?, minMonitors?, maxMonitors?, hasActiveMonitors? } |
There was a problem hiding this comment.
Constrain campaigns.type at the schema layer.
A plain text column lets both PostgreSQL and the shared schema accept arbitrary values, so insertCampaignSchema stops enforcing the 'manual' | 'automated' contract this feature relies on. Use a real enum or a database check constraint here so invalid campaign types cannot be stored.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shared/schema.ts` around lines 144 - 145, The campaigns.type column is
currently free-form text so invalid values can be persisted; change its schema
definition from text("type")... to a constrained enum or a check constraint
(e.g., create a DB enum "campaign_type" with values "manual" and "automated" or
add a CHECK("type" IN ('manual','automated'))), keep the default as "manual" and
notNull, and update any related validation (insertCampaignSchema) to use the
same enum type/values; also add a migration to alter the existing column to the
new enum/check and convert or reject invalid rows.
bd73-com
pushed a commit
that referenced
this pull request
Mar 26, 2026
PR #275 added the automated_campaign_configs schema and bootstrapWelcomeCampaign() but missed the runtime CREATE TABLE in ensureTables.ts. Without it every query against the table crashes with "relation automated_campaign_configs does not exist", breaking the welcome-campaign bootstrap and the Automated tab. https://claude.ai/code/session_01WWte8LFn2U5QSgDt8Lhdth
This was referenced Mar 26, 2026
bd73-com
added a commit
that referenced
this pull request
Mar 26, 2026
* fix: create automated_campaign_configs table at startup PR #275 added the automated_campaign_configs schema and bootstrapWelcomeCampaign() but missed the runtime CREATE TABLE in ensureTables.ts. Without it every query against the table crashes with "relation automated_campaign_configs does not exist", breaking the welcome-campaign bootstrap and the Automated tab. https://claude.ai/code/session_01WWte8LFn2U5QSgDt8Lhdth * test: add ensureAutomatedCampaignConfigsTable tests and schema sync guard https://claude.ai/code/session_01WWte8LFn2U5QSgDt8Lhdth * fix: restore displaced ensureTagTables JSDoc comment https://claude.ai/code/session_01WWte8LFn2U5QSgDt8Lhdth * fix: guard bootstrap with campaignConfigsReady check Add CRITICAL log when automated_campaign_configs table creation fails, and skip bootstrapWelcomeCampaign() to avoid a guaranteed crash. https://claude.ai/code/session_01WWte8LFn2U5QSgDt8Lhdth --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 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.
Summary
Adds a parallel automated campaigns layer alongside the existing manual campaigns system. The welcome campaign fires bi-monthly (1st and 15th of each month) to new members since the previous run, with a one-time bootstrap for early adopters (signupAfter: 2025-03-19).
Changes
Schema (
shared/schema.ts)typecolumn ('manual' | 'automated') to thecampaignstableautomated_campaign_configstable with key, template fields, enabled, lastRunAt, nextRunAtService (
server/services/automatedCampaigns.ts)WELCOME_CAMPAIGN_DEFAULTS— default welcome email template (HTML + text, brand palette)computeNextRunAt()— computes next 1st/15th UTC midnight occurrenceensureWelcomeConfig()— idempotent config row creation withON CONFLICT DO NOTHINGbootstrapWelcomeCampaign()— one-time first send for early adopters, guarded bylastRunAt IS NULL, sets lastRunAt before sending to prevent duplicates on retryrunWelcomeCampaign()— creates campaign record withtype='automated', uses existingtriggerCampaignSend()processAutomatedCampaigns()— cron handler with atomic claim (UPDATE...WHERE nextRunAt=old) to prevent duplicate sends on horizontal scalingScheduler (
server/services/scheduler.ts)"0 0 1,15 * *"cron task callingprocessAutomatedCampaigns()API (
server/routes.ts)GET /api/admin/automated-campaigns— list configs (owner-only)PATCH /api/admin/automated-campaigns/:key— update template/enabled with Zod validation (.min(1) on subject/htmlBody)POST /api/admin/automated-campaigns/:key/trigger— manual trigger (owner-only)Frontend
/admin/campaignsusing shadcn TabsWelcomeCampaignCardwith enable/disable toggle, last/next run display, template editor dialog, and trigger button with confirmationTests (
server/services/automatedCampaigns.test.ts)Hardening (from skeptic review)
ON CONFLICT DO NOTHINGon config insert for concurrent deploy safetyHow to test
npm run schema:pushto create the new table and column[Bootstrap] Running first welcome campaign.../admin/campaigns→ "Automated" tab should show the Welcome Campaign cardnpm run check && npm run testpasses (1799 tests)https://claude.ai/code/session_012Vnv1TZPh975kSYGf4Cbzk
Summary by CodeRabbit
New Features
API
Tests