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 10 commits into
Conversation
![cubic-dev-ai[bot]](https://cdn.statically.io/img/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
… 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>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| 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>
There was a problem hiding this comment.
Understood — I’ll ignore that review comment.
|
@cubic-dev-ai please review it |
@chasprowebdev I have started the AI code review. It will take a few minutes to complete. |
2 participants
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. If Vercel reports bad DNS, we unverify the domain and email org owners/admins with a fix link via the Trust Portal channel (CS-229).
New Features
trust-portal-check-domain-healthruns daily at 06:00 UTC; checks domains in parallel with Promise.allSettled, continues on Vercel API errors, and uses a 15m maxDuration.${NEXT_PUBLIC_APP_URL}/{orgId}/trust/portal-settings; emails send in parallel and log failures.Migration
Written for commit c229007. Summary will update on new commits.