CS-229 [Improvement] - Send notification when DNS record needs to be fixed on the Trust Portal settings#3247
CS-229 [Improvement] - Send notification when DNS record needs to be fixed on the Trust Portal settings#3247github-actions[bot] wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 2/5
- In
apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts, the health-check loop exits on the first domain API failure (break), so remaining verified domains are silently skipped and can stay stale/misconfigured without checks — change this to per-domain error handling (continue) and separate thenullcases fromisDomainMisconfigured()before merging. - In
apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts,maxDurationis set with a millisecond-style expression even though Trigger.dev expects seconds, which stretches the timeout from ~15 minutes to ~10.4 days and weakens runaway-job protection — set it to a true seconds value (for example15 * 60) before merging.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
… api failure instead of aborting in domain check schedule
|
@cubic-dev-ai please review it |
@chasprowebdev I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 2/5
- In
apps/api/src/trust-portal/email.service.ts,sendDomainMisconfiguredEmailcallstriggerEmailwithouttrustPortal: true, so these messages can be routed through thedefaultchannel instead of the Trust Portal path, creating a concrete delivery/templating regression for users—addtrustPortal: truein this method (and run a quick send-path test) before merging.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai please review it |
@chasprowebdev I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 3/5
- In
apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts,TRUST_PORTAL_PROJECT_IDis treated as required but not actually used in the Vercel call, so runs can be silently skipped for configuration reasons without improving correctness; this risks missed domain-health checks and delayed alerts — either wire the project ID into the API request or remove it as a hard requirement before merging. - In
apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts, independent Vercel checks and member email sends are awaited sequentially, which can throttle throughput as org/member counts grow; merging as-is may cause slower schedules and late notifications under load — parallelize independent I/O (with sensible concurrency limits) to de-risk scalability.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts">
<violation number="1" location="apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts:100">
P2: Use `Promise.allSettled` instead of `Promise.all` to isolate per-trust failures in the batch. A single unhandled exception from one trust (e.g., a DB update failure or unexpected network error) currently aborts the entire scheduled run and abandons all other in-flight trust tasks. The per-trust mapper should be wrapped so individual failures are logged without failing the whole task.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return { checked: 0, misconfigured: 0, notified: 0 }; | ||
| } | ||
|
|
||
| const results = await Promise.all( |
There was a problem hiding this comment.
P2: Use Promise.allSettled instead of Promise.all to isolate per-trust failures in the batch. A single unhandled exception from one trust (e.g., a DB update failure or unexpected network error) currently aborts the entire scheduled run and abandons all other in-flight trust tasks. The per-trust mapper should be wrapped so individual failures are logged without failing the whole task.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts, line 100:
<comment>Use `Promise.allSettled` instead of `Promise.all` to isolate per-trust failures in the batch. A single unhandled exception from one trust (e.g., a DB update failure or unexpected network error) currently aborts the entire scheduled run and abandons all other in-flight trust tasks. The per-trust mapper should be wrapped so individual failures are logged without failing the whole task.</comment>
<file context>
@@ -99,65 +97,77 @@ export const checkDomainHealthSchedule = schedules.task({
- let checked = 0;
- let misconfigured = 0;
- let notified = 0;
+ const results = await Promise.all(
+ trusts.map(async (trust) => {
+ const domain = trust.domain!;
</file context>
This is an automated pull request to merge chas/dns-record-notification into dev.
It was created by the [Auto Pull Request] action.
Summary by cubic
Adds a daily health check for Trust Portal custom domains. When Vercel reports bad DNS, we unverify the domain and email org admins with a fix link, sent via the Trust Portal channel (CS-229).
New Features
trust-portal-check-domain-healthruns daily at 06:00 UTC; checks Vercel domain config in parallel, logs results, continues on per-domain API failure, and uses a 15mmaxDuration.domainVerifiedtofalseand emails owners/admins with a “Review Domain Settings” link to${NEXT_PUBLIC_APP_URL}/{orgId}/trust/portal-settings.TrustDomainMisconfiguredEmail; notifications are sent through the Trust Portal channel.Migration
VERCEL_TEAM_IDandVERCEL_AUTH_TOKEN; checks are skipped if missing.TRUST_PORTAL_PROJECT_IDis no longer required.NEXT_PUBLIC_APP_URL(defaults to https://app.trycomp.ai) for the settings link.Written for commit 83a60fc. Summary will update on new commits.