Conversation
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
|
@aggmoulik is attempting to deploy a commit to the OpenStatus Team on Vercel. A member of the Team first needs to authorize it. |
![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 17 files
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="packages/importers/src/providers/betterstack/provider.ts">
<violation number="1" location="packages/importers/src/providers/betterstack/provider.ts:113">
P2: The monitor-groups fetch + mapping block is copy-pasted identically in both branches of the `if (statusPages.length > 0)` / `else`. Move it after the conditional to eliminate the duplication — `pageId` is already in scope.</violation>
</file>
<file name="packages/api/src/service/import.ts">
<violation number="1" location="packages/api/src/service/import.ts:41">
P2: Unknown provider names silently fall through to `createStatuspageProvider()`. Separate the `default` case to throw an error for unrecognized providers, so misconfigurations fail fast instead of producing confusing downstream errors.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
Signed-off-by: Moulik Aggarwal <qwertymoulik@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds a new Better Stack Uptime importer to @openstatus/importers and wires it through the API import pipeline and Dashboard UI so users can preview/run imports (including monitors) from Better Stack into OpenStatus.
Changes:
- Introduces a new Better Stack provider (Zod API types, client w/ pagination, mappers, orchestration, fixtures, and tests).
- Extends the API import pipeline to support a
betterstackprovider, monitor imports, and monitor plan-limit warnings. - Updates Dashboard import form and icons to support Better Stack-specific inputs and toggles.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/importers/src/providers/betterstack/api-types.ts | Zod schemas + TS types for Better Stack API resources. |
| packages/importers/src/providers/betterstack/client.ts | Better Stack HTTP client (Bearer auth, JSON:API-style pagination). |
| packages/importers/src/providers/betterstack/client.test.ts | Tests for Better Stack client (auth, pagination, errors, parsing). |
| packages/importers/src/providers/betterstack/fixtures.ts | Shared mock payloads for Better Stack provider tests. |
| packages/importers/src/providers/betterstack/index.ts | Barrel exports for the Better Stack provider package entry. |
| packages/importers/src/providers/betterstack/mapper.ts | Mapping functions from Better Stack shapes to OpenStatus insert shapes. |
| packages/importers/src/providers/betterstack/mapper.test.ts | Unit tests for all Better Stack mappers. |
| packages/importers/src/providers/betterstack/provider.ts | Provider orchestration (validate + phase generation). |
| packages/importers/src/providers/betterstack/provider.test.ts | Provider integration tests (phase structure, filtering, resources). |
| packages/importers/src/index.ts | Registers betterstack in IMPORT_PROVIDERS and re-exports. |
| packages/importers/package.json | Adds ./betterstack and ./betterstack/fixtures export paths. |
| packages/importers/README.md | New package-level docs incl. provider architecture and phase mapping. |
| packages/icons/src/betterstack.tsx | Adds Better Stack icon component. |
| packages/icons/src/index.tsx | Exports the new Better Stack icon. |
| packages/api/src/service/import.ts | Adds provider routing, monitor limit warnings, and monitor write phase. |
| packages/api/src/router/import.ts | Accepts betterstack provider + Better Stack-specific inputs and options. |
| apps/dashboard/src/components/forms/components/form-import.tsx | UI support for Better Stack selection, token help text, status page id, and monitor toggle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 3. Monitor count limit | ||
| const monitorsPhase = summary.phases.find((p) => p.phase === "monitors"); | ||
| if (monitorsPhase && monitorsPhase.resources.length > 0) { | ||
| const maxMonitors = config.limits.monitors; | ||
| const existingMonitors = await db | ||
| .select() | ||
| .from(monitor) | ||
| .where(eq(monitor.workspaceId, config.workspaceId)) | ||
| .all(); | ||
| const remaining = maxMonitors - existingMonitors.length; | ||
| if (remaining <= 0) { | ||
| summary.errors.push( | ||
| `Monitor limit reached (${maxMonitors}). Upgrade your plan to import monitors.`, | ||
| ); | ||
| } else if (monitorsPhase.resources.length > remaining) { | ||
| summary.errors.push( | ||
| `Only ${remaining} of ${monitorsPhase.resources.length} monitors can be imported due to plan limit (${maxMonitors}).`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new monitor limit warning logic in addLimitWarnings() isn't covered by the existing packages/api/src/service/import.test.ts suite (which already tests components/custom-domain/subscribers). Adding tests for the monitors phase warnings (under/over limit, existing monitors) would help prevent regressions.
| @@ -182,6 +237,18 @@ export async function runImport(config: { | |||
| phase.status = "skipped"; | |||
| } | |||
| break; | |||
There was a problem hiding this comment.
When targetPageId is falsy (no page created and no pageId provided), the monitorGroups/componentGroups case currently does nothing unless includeComponents === false. That leaves the phase in its original "completed" state even though nothing was written. Explicitly mark the phase as skipped (and ideally update resource statuses) when targetPageId is missing.
| } else if (config.options?.includeComponents === false) { | ||
| phase.status = "skipped"; |
There was a problem hiding this comment.
Similar to the monitorGroups case: if targetPageId is not set, the sections phase will silently do nothing (unless includeComponents === false), leaving the phase reported as completed. Set phase.status = "skipped" (and update resource statuses) when there is no page to attach groups/sections to.
| } else if (config.options?.includeComponents === false) { | |
| phase.status = "skipped"; | |
| } else if ( | |
| config.options?.includeComponents === false || | |
| !targetPageId | |
| ) { | |
| phase.status = "skipped"; | |
| for (const resource of phase.resources) { | |
| resource.status = "skipped"; | |
| } |
| // Monitor Groups → Component Groups (always fetched regardless of status pages) | ||
| const monitorGroups = await client.getMonitorGroups(); | ||
| const groupResources: ResourceResult[] = monitorGroups.map((g) => ({ | ||
| sourceId: g.id, | ||
| name: g.attributes.name, | ||
| status: "created" as const, | ||
| data: mapMonitorGroup(g, config.workspaceId, pageId), | ||
| })); | ||
| phases.push({ | ||
| phase: "monitorGroups", | ||
| status: "completed", | ||
| resources: groupResources, | ||
| }); | ||
|
|
||
| // Phase 5: Incidents → Status Reports | ||
| const incidents = await client.getIncidents(); | ||
| const incidentResources: ResourceResult[] = incidents.map((inc) => ({ | ||
| sourceId: inc.id, | ||
| name: inc.attributes.name ?? `Incident ${inc.id}`, | ||
| status: "created" as const, | ||
| data: mapIncidentToStatusReport(inc, config.workspaceId, pageId), | ||
| })); | ||
| phases.push({ | ||
| phase: "incidents", | ||
| status: "completed", | ||
| resources: incidentResources, | ||
| }); |
There was a problem hiding this comment.
run() always emits an incidents phase even when no page context exists (config.pageId is undefined and betterstackStatusPageId filtering yields no status page). In that situation the service layer won't be able to write incidents, but the phase will still be reported as completed. Consider only emitting page-scoped phases (sections/monitorGroups/incidents) when you have a page to attach to, or mark them as skipped with a clear reason in the summary.
| const maxMonitors = limits.monitors; | ||
| const remaining = maxMonitors - existingMonitors.length; | ||
|
|
||
| if (remaining <= 0) { |
There was a problem hiding this comment.
If remaining <= 0, writeMonitorsPhase sets phase.status = "failed" and returns, but it does not update individual resource.status values. Since importer providers initially mark resources as created, this can leave the summary claiming monitors were created when none were written. Mark all monitor resources as skipped/failed (with an explanatory resource.error) before returning so the summary accurately reflects what happened.
| if (remaining <= 0) { | |
| if (remaining <= 0) { | |
| for (const resource of phase.resources) { | |
| resource.status = "skipped"; | |
| resource.error = `Skipped: monitor limit reached (max ${maxMonitors})`; | |
| } |
| validate: async (config) => { | ||
| try { | ||
| const client = createBetterstackClient(config.apiKey); | ||
| await client.getMonitors(); | ||
| return { valid: true }; | ||
| } catch (err) { | ||
| return { | ||
| valid: false, | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }; |
There was a problem hiding this comment.
validate() currently calls client.getMonitors(), which fetches all monitor pages due to requestAllPages(). That can be very slow/expensive for accounts with many monitors and makes validation heavier than necessary. Consider validating with a single lightweight request (e.g., fetch the first page only / a dedicated "me" endpoint / limit=1 style request) so validation only checks auth/permissions without pulling full datasets.
| const monitorsPhase = summary.phases.find((p) => p.phase === "monitors"); | ||
| if (monitorsPhase && monitorsPhase.resources.length > 0) { | ||
| const maxMonitors = config.limits.monitors; | ||
| const existingMonitors = await db | ||
| .select() | ||
| .from(monitor) | ||
| .where(eq(monitor.workspaceId, config.workspaceId)) | ||
| .all(); | ||
| const remaining = maxMonitors - existingMonitors.length; |
There was a problem hiding this comment.
In addLimitWarnings(), the monitor limit check loads all existing monitors with .select().from(monitor)...all() just to compute a count. For larger workspaces this is unnecessarily heavy; prefer a COUNT(*) (or a minimal select({ id: monitor.id })) so the warning check stays cheap.
Summary
Adds a Better Stack Uptime import provider to the
@openstatus/importerspackage, enabling users to migrate their monitoring setup from Better Stack into OpenStatus.This is the first importer that can import actual monitors (HTTP checks with URLs, frequency, regions) — the existing Statuspage importer only imports status page data.
What gets imported
monitortablepagetablepageComponentGrouptablepageComponentGrouptablestatusReport+statusReportUpdatetablesField mapping highlights
30s,1m,5m...)us,eu,as,au) mapped to specific Fly regions (iad,fra,sin,syd)started_at,acknowledged_at,resolved_at) converted to synthetic status report updates (investigating→identified→resolved)pagination.nextlinksChanges
New files (10)
packages/importers/src/providers/betterstack/— Full provider implementation:api-types.ts— Zod schemas for Better Stack API responsesclient.ts— HTTP client with Bearer auth and JSON:API paginationmapper.ts— 9 transformation functions (monitor, frequency, regions, status page, incidents, etc.)provider.ts— 5-phase import orchestrationindex.ts— Barrel exportsfixtures.ts— Mock data (3 monitors, 1 group, 1 status page, 2 sections, 3 incidents)client.test.ts— 8 tests (endpoints, auth, pagination, errors)mapper.test.ts— 25 tests (all mapping functions with edge cases)provider.test.ts— 7 tests (validate, phases, filtering)packages/icons/src/betterstack.tsx— Better Stack icon componentpackages/importers/README.md— Package documentation with architecture, pipeline flow, and "adding a new provider" guideModified files (6)
packages/importers/src/index.ts— Registerbetterstackprovider inIMPORT_PROVIDERSpackages/importers/package.json— Add./betterstackand./betterstack/fixturesexport pathspackages/icons/src/index.tsx— Export Better Stack iconpackages/api/src/service/import.ts— AddwriteMonitorsPhase(),createProvider()routing, monitor limit warnings,monitors/monitorGroups/sectionsphase casespackages/api/src/router/import.ts— Accept"betterstack"provider,betterstackStatusPageId,includeMonitorsoptionapps/dashboard/src/components/forms/components/form-import.tsx— Better Stack radio button, provider-specific fields, monitors toggleTest plan
bun testinpackages/importers/— 75 tests pass (40 new + 35 existing)