feat(services): migrate monitor domain tRPC (PR 4/N)

[mxkaske]

Summary

Stacked on #2104. Migrates the tRPC monitor router — all 14 procedures — onto @openstatus/services/monitor. The Connect RPC monitor handler and the v1 REST monitors/* endpoints stay untouched for now; they're separate external-API surfaces with their own test suites and warrant their own follow-up PRs.

What lands

Services (packages/services/src/monitor/)

CRUD + relation management + list / get:

• createMonitor, cloneMonitor, deleteMonitor, deleteMonitors (bulk soft-delete), getMonitor, listMonitors.
• updateMonitorGeneral — keeps the existing tRPC jobType-agnostic update (HTTP ↔ TCP ↔ DNS switch allowed, called out as a code smell in the explore but kept for dashboard-compatibility).
• Field-specific setters: updateMonitorRetry, updateMonitorFollowRedirects, updateMonitorOtel, updateMonitorPublic, updateMonitorResponseTime.
• Relation setters (transactional + validated): updateMonitorSchedulingRegions (plan-limit checks + private-location validation), updateMonitorTags, updateMonitorNotifiers.
• bulkUpdateMonitors — batched toggle of public / active.
• All mutations inside withTransaction, emit emitAudit. Soft delete cascades to monitor_tag_to_monitor, notifications_to_monitors, and page_component rows.

Shared internals

• validateTagIds / validateNotificationIds / validatePrivateLocationIds — workspace-scoped, dedupe input.
• pickDefaultRegions(workspace) — ports the old "random 4 free / 6 paid, excluding deprecated" logic.
• serialiseAssertions / headersToDbJson — moved out of the router.
• countMonitorsInWorkspace for quota checks in create / clone.

list.ts — batched enrichment

listMonitors runs two IN queries (tags + incidents) regardless of row count. getMonitor reuses the same batch path with an additional pair of IN queries for notifications / privateLocations. Pairs well with the 10_000 sentinel tRPC passes.

Surface — external contract preserved

packages/api/src/router/monitor.ts — every procedure is a thin wrapper:

• delete catches NotFoundError for idempotency (same pattern as status-report / maintenance / incident).
• new and updateGeneral keep the testHttp / testTcp / testDns pre-save endpoint check at the tRPC layer. Services unconditionally save; pre-validation is caller-specific UX.

Deliberately out of scope

• Connect RPC monitor handler (apps/server/src/routes/rpc/services/monitor/) — still uses its own validators.ts / limits.ts / converters. Migrating it to services is a self-contained follow-up (4b), doesn't block this PR.
• v1 REST (apps/server/src/routes/v1/monitors/*) — four separate endpoints that insert monitors directly. Another external surface; dedicated PR.
• packages/api/src/service/import.ts monitor writes — covered by the plan's dedicated PR 9.

Enforcement

• Biome noRestrictedImports scope adds packages/api/src/router/monitor.ts. The Connect monitor handler stays out of scope until 4b.
• Subpath export @openstatus/services/monitor.

Tests

• __tests__/monitor.test.ts — create (http / tcp / dns), delete cascade, bulk delete, clone, tags / notifiers validation (forbidden cross-workspace, dedupe), list / get workspace isolation + soft-delete hiding, updateMonitorGeneral round-trip.

Test plan

• Service integration tests
• Dashboard /monitors list + get + create flow
• Dashboard edit forms — tags / notifiers / regions / retry / public / otel / response-time
• Dashboard clone + delete (single + bulk)
• Connect RPC monitor suite unaffected (unchanged)

Stack

• feat(services): scaffold domain service layer (PR 0/N) #2100 scaffold → feat(services): migrate status-report domain (PR 1/N) #2101 status-report → feat(services): migrate maintenance domain (PR 2/N) #2103 maintenance → feat(services): migrate incident domain (PR 3/N) #2104 incident → this PR monitor (tRPC)
• Follow-ups: Connect monitor handler migration (4b), then PR 5 notification → PR 6 page-component → …

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
openstatus-dashboard	Ready	Preview, Comment	Apr 24, 2026 9:34am
openstatus-status-page	Ready	Preview, Comment	Apr 24, 2026 9:34am
openstatus-web	Ready	Preview, Comment	Apr 24, 2026 9:34am

[cubic-dev-ai]

5 issues found across 14 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/services/src/monitor/schemas.ts">

<violation number="1" location="packages/services/src/monitor/schemas.ts:90">
P1: `timeout`/`degradedAfter` should enforce the same numeric bounds as the monitor validation schema to prevent invalid persisted response-time settings.</violation>
</file>

<file name="packages/services/src/monitor/internal.ts">

<violation number="1" location="packages/services/src/monitor/internal.ts:163">
P2: `jsonBody` assertions are silently discarded during monitor serialization, so valid assertions from input are not persisted.</violation>
</file>

<file name="packages/services/src/monitor/clone.ts">

<violation number="1" location="packages/services/src/monitor/clone.ts:37">
P2: Cloning copies the source monitor `status`, so new clones can start with stale `error/degraded` health instead of an initial active state.</violation>
</file>

<file name="packages/services/src/monitor/delete.ts">

<violation number="1" location="packages/services/src/monitor/delete.ts:33">
P2: Single delete does not filter out already soft-deleted monitors, so repeated deletes re-run side effects and emit duplicate delete audits instead of returning NotFound.</violation>
</file>

<file name="packages/services/src/monitor/list.ts">

<violation number="1" location="packages/services/src/monitor/list.ts:79">
P2: Scope incident enrichment by workspace to prevent cross-workspace data leakage from inconsistent relation data.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[mxkaske]

Review response (Cubic findings)

All five valid — fixed in 1d7414b1.

✅ 1. timeout / degradedAfter bounds (P1)

My UpdateMonitorResponseTimeInput had z.number().int() with no bounds, while the canonical insertMonitorSchema caps both fields at 0..60_000 ms. Aligned to match — invalid values are rejected at the service boundary instead of being persisted.

✅ 2. jsonBody assertions silently dropped (P2)

JsonBodyAssertion exists in @openstatus/assertions (line 348 of v1.ts); I just didn't map it in serialiseAssertions. The old tRPC router had the same gap, so this is a strict improvement — adding the missing branch means jsonBody assertions now round-trip correctly to the stored JSON.

✅ 3. Clone inherits stale status (P2)

cloneMonitor was spreading { ...source } into the insert, carrying the source's current status (which could be error / degraded). Destructured status out and forced "active" — freshly cloned monitors settle on their own first check. The old tRPC had the same bug; this tightens.

✅ 4. Single delete re-runs side effects on soft-deleted rows (P2)

deleteMonitor's pre-check was missing isNull(monitor.deletedAt), so a repeated delete would re-run the tag / notifier / page-component cascades and emit a duplicate audit. Added the filter; the second delete now returns NotFoundError, which the tRPC wrapper swallows → external behaviour is still "idempotent" from the caller's perspective, but without the duplicate side effects internally.

✅ 5. List/get enrichment missing workspace scope (P2)

Threaded workspaceId through enrichMonitorsBatch and added eq(…workspaceId, workspaceId) to all four relation queries — tags, incidents, notifications, private-locations. Defence-in-depth: none of those tables enforce workspace ownership at the FK level, so inconsistent data wouldn't leak. Same pattern as the monitor-enrichment fix on #2104.

---

Local typecheck + biome clean on all five changes.

[mxkaske]

@claude review

[claude]

Claude finished @mxkaske's task in 2m 30s —— View job

---

Code Review

• Gather context and understand the PR
• Read service layer files (create, clone, delete, list, update, relations, internal, schemas, index)
• Read tRPC router wrapper
• Read test file
• Post review findings

---

Overall this is a well-structured migration. The 5 Cubic issues are all correctly addressed in 1d7414b, and the patterns are consistent with the earlier PRs in the stack. A few things worth calling out below.

---

Issues

Medium — bulkUpdateMonitors emits audit events for unmatched IDs (update.ts:239)

The for...await loop emits a monitor.bulk_update audit event for every ID in input.ids, including IDs that the WHERE clause silently skipped (wrong workspace, already soft-deleted). The actual DB rows aren't touched for those IDs, but the audit log records that they were. The PR description notes this matches pre-migration behaviour, but the old code had no audit trail at all — so this is new pollution, not preservation.

Suggest constraining the audit loop to IDs that were actually matched:

// Instead of: for (const id of input.ids)
const updatedRows = await tx
  .select({ id: monitor.id })
  .from(monitor)
  .where(and(inArray(monitor.id, input.ids), eq(monitor.workspaceId, ctx.workspace.id), isNull(monitor.deletedAt)))
  .all();
for (const { id } of updatedRows) { ... }

Or, fetch the matching IDs first and use those for both the update and the audit loop. Fix this →

---

Low — updateMonitorOtel serializes otelHeaders inline instead of using headersToDbJson (update.ts:139)

// Current
otelHeaders: input.otelHeaders ? JSON.stringify(input.otelHeaders) : undefined,

// Consistent with the rest of the module
otelHeaders: headersToDbJson(input.otelHeaders),

headersToDbJson has the same behaviour (undefined → undefined, array → JSON.stringify(array)), so this is a drop-in substitution. Minor, but it's the only place that bypasses the shared helper. Fix this →

---

Low — deleteMonitors always blames ids[0] in the NotFoundError (delete.ts:90)

if (existing.length !== ids.length) {
  throw new NotFoundError("monitor", ids[0] ?? -1);  // always blames first id
}

When a mix of valid and invalid IDs is passed, the error always reports ids[0] even if that one exists. The actual missing ID can be computed cheaply:

const foundIds = new Set(existing.map(r => r.id));
const missingId = ids.find(id => !foundIds.has(id)) ?? -1;
throw new NotFoundError("monitor", missingId);

---

Low — Missing test for the double-delete fix

The Cubic P2 fix adds isNull(monitor.deletedAt) to deleteMonitor to prevent re-running side-effects on a second call. There's no test asserting the new behaviour:

test("second delete returns NotFoundError (idempotency guard)", async () => {
  // ... create, delete once, then:
  await expect(
    deleteMonitor({ ctx: teamCtx, input: { id: row.id } })
  ).rejects.toBeInstanceOf(NotFoundError);
});

---

Low — UpdateMonitorRetryInput.retry has no bounds (schemas.ts:62)

The Cubic P1 finding added gte(0).lte(60_000) bounds to timeout/degradedAfter. The retry field in the same file is z.number().int() with no min/max. The DB column has default(3) but no CHECK constraint, and insertMonitorSchema also has no bounds on retry — so this is consistent with the rest of the codebase. Just worth noting if tighter enforcement is a goal.

---

Nit — listMonitors ordering is implicit

list.ts:203-208 sorts by (active DESC, createdAt DESC) when order: "desc", which puts active monitors above inactive ones. This is good dashboard UX, but the ListMonitorsInput.order parameter is named "asc"/"desc" without documenting what it sorts by. A future caller might reasonably assume it controls createdAt only and be surprised by the active primary key. A short schema-level comment (or renaming to something like activeFirst) would make the intent clear.

---

Positives

• All 5 Cubic issues correctly resolved — jsonBody assertion mapping, status exclusion from clone, double-delete guard, timeout/degradedAfter bounds, and workspace scoping on all 4 enrichment queries.
• getMonitorInWorkspace used consistently as the load-verify-mutate pattern before every write — no bare updates.
• Batched enrichment in enrichMonitorsBatch avoids the N+1 pattern for list/get.
• updateMonitorSchedulingRegions correctly validates plan limits before entering the transaction (fail-fast).
• Service/tRPC boundary is clean: testHttp/testTcp/testDns pre-save checks stay in the tRPC layer, service unconditionally persists.
• Soft-delete cascade to monitorTagsToMonitors, notificationsToMonitors, and pageComponent is complete.
• pickDefaultRegions correctly ports the "random 4 free / 6 paid, excluding deprecated" logic.

[mxkaske]

@claude review

[claude]

Claude finished @mxkaske's task in 2m 34s —— View job

---

Code Review

• Gather context and understand the PR
• Read current state of service files (update.ts, delete.ts, list.ts, internal.ts, schemas.ts, relations.ts, clone.ts)
• Read tRPC router wrapper
• Read test file
• Post review findings

---

All four findings from the previous review are correctly addressed in 7e5822f9. The implementation is clean. A few low-signal observations below.

---

Verified fixes

Medium — bulkUpdateMonitors audit pollution ✅

update.ts:233–255 — .returning({ id: monitor.id }) now bounds the audit loop to rows the WHERE clause actually matched. The workspace + soft-delete filters in the WHERE are also correct.

Low — updateMonitorOtel inline JSON.stringify ✅

update.ts:139 — Now uses headersToDbJson(input.otelHeaders), consistent with every other place in the module.

Low — deleteMonitors always blamed ids[0] ✅

delete.ts:94–96 — Correctly computes the first actually-missing ID by diffing existing against the deduplicated input set.

Test — double-delete regression ✅

monitor.test.ts:248–270 — Test creates, deletes, then asserts the second delete throws NotFoundError. Matches the isNull(deletedAt) guard added in the Cubic P2 fix.

---

New observations

Low — cloneMonitor test doesn't exercise the status reset

monitor.test.ts:317–343 — The Cubic P2 fix (forced status: "active" in clone) has no assertion backing it. The test only checks id, name, and url. A source in error/degraded state would be the meaningful case, but the test creates a fresh monitor which already has status: "active". The fix is structurally correct (clone.ts:33 destructures status out before spread), just unwitnessed by the test suite.

Low — deleteMonitors bulk mutations drop workspace constraint after pre-check

delete.ts:99–109 — After the pre-check verifies all IDs belong to the workspace, the subsequent update + three deletes use inArray(monitor.id, ids) alone, without eq(monitor.workspaceId, ...). It's safe because:

1. The pre-check is inside the same transaction (serialized)
2. ids is deduplicated before the check

...but it's a layered-defense opportunity if ever copy-pasted to a context without a prior pre-check. Worth noting, not worth blocking on.

Nit — tRPC list discards totalSize but still pays for a count query

list.ts:193–212 — listMonitors runs a count(*) and the full row query in parallel, then returns { items, totalSize }. The tRPC list wrapper at monitor.ts:135 destructures only items and discards totalSize. Since both queries run in parallel the overhead is one extra round-trip, and the design correctly serves future callers that need totalSize. Not a bug, just worth knowing if list ever lands under tight latency budgets.

---

Positives

• All previous findings resolved correctly, with clean test coverage for the double-delete case.
• bulkUpdateMonitors fix is idiomatic: one .returning({ id }) call replaces the prior "iterate over input ids" pattern cleanly.
• headersToDbJson is now used consistently across the entire module.
• internal.ts:28–47 — getMonitorInWorkspace correctly filters isNull(monitor.deletedAt), making the double-delete guard automatic for every caller that goes through it.
• enrichMonitorsBatch workspace-scoping is complete across all four relation queries (tags, incidents, notifications, private-locations).

---