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 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
chasprowebdev
changed the title
… 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).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai please review this |
@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, recipient targeting relies on role-name substring checks (includes('owner')/includes('admin')), which can notify the wrong users when role naming varies and lead to missed or noisy incident communication — switch to RBAC permission checks or exact, canonical role matching before merging. - In
apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts, unboundedPromise.allSettledfan-out for Vercel checks and email sends can hit provider rate limits and leave domain-health runs partially processed at higher tenant counts — add bounded concurrency (or batching/backoff) and verify behavior under load before merging.
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: Unbounded `Promise.allSettled` parallelism for Vercel domain checks and email triggers can cause rate-limit failures and incomplete runs at scale.</violation>
<violation number="2" location="apps/api/src/trigger/trust-portal/check-domain-health-schedule.ts:124">
P2: Recipient selection uses brittle substring matching on role names (`includes('owner')` and `includes('admin')`) instead of RBAC permission checks or exact role matching. This can mis-target notifications if role naming conventions change or custom roles contain these substrings.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| data: { domainVerified: false }, | ||
| }); | ||
|
|
||
| const adminOrOwnerMembers = trust.organization.members.filter( |
There was a problem hiding this comment.
P2: Recipient selection uses brittle substring matching on role names (includes('owner') and includes('admin')) instead of RBAC permission checks or exact role matching. This can mis-target notifications if role naming conventions change or custom roles contain these substrings.
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 124:
<comment>Recipient selection uses brittle substring matching on role names (`includes('owner')` and `includes('admin')`) instead of RBAC permission checks or exact role matching. This can mis-target notifications if role naming conventions change or custom roles contain these substrings.</comment>
<file context>
@@ -0,0 +1,194 @@
+ data: { domainVerified: false },
+ });
+
+ const adminOrOwnerMembers = trust.organization.members.filter(
+ (m) =>
+ m.role &&
</file context>
| return { checked: 0, misconfigured: 0, notified: 0 }; | ||
| } | ||
|
|
||
| const settled = await Promise.allSettled( |
There was a problem hiding this comment.
P2: Unbounded Promise.allSettled parallelism for Vercel domain checks and email triggers can cause rate-limit failures and incomplete runs at scale.
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>Unbounded `Promise.allSettled` parallelism for Vercel domain checks and email triggers can cause rate-limit failures and incomplete runs at scale.</comment>
<file context>
@@ -0,0 +1,194 @@
+ return { checked: 0, misconfigured: 0, notified: 0 };
+ }
+
+ const settled = await Promise.allSettled(
+ trusts.map(async (trust) => {
+ const domain = trust.domain!;
</file context>
1 participant
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, isolates per-trust failures withPromise.allSettled, logs metrics, continues on API errors, and uses a 15mmaxDuration.domainVerifiedtofalseand emails owners/admins with a “Review Domain Settings” link to${NEXT_PUBLIC_APP_URL}/{orgId}/trust/portal-settings; email sends run in parallel and log any failures without blocking others.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 a3a6d2a. Summary will update on new commits.