feat(services): migrate maintenance domain (PR 2/N)

[mxkaske]

Summary

Stacked on #2101. Second domain onto the service layer. Every maintenance write and read goes through @openstatus/services/maintenance; tRPC / Connect / Slack are thin adapters. Same shape as the status-report PR.

What lands

Services (packages/services/src/maintenance/)

• createMaintenance, updateMaintenance, deleteMaintenance, listMaintenances, getMaintenance, notifyMaintenance.
• Date range invariant (from < to) enforced via Zod refine on CreateMaintenanceInput and again inside updateMaintenance when the partial update could cross the invariant.
• All mutations wrapped in withTransaction, emit emitAudit before returning.
• Internal helpers (validatePageComponentIds, updatePageComponentAssociations, getMaintenanceInWorkspace, getPageComponentIdsForMaintenance, getPageComponentsForMaintenance) duplicated from status-report for this PR. Noted as the extraction trigger once a third consumer arrives.

Surfaces — external contracts preserved

• tRPC (packages/api/src/router/maintenance.ts): delete / list / new / update call services. new still returns { ...maintenance, notifySubscribers } for dashboard consumers. list passes limit: 10_000 (same sentinel strategy as the status-report list after the PR 1 Cubic fix).
• Connect RPC (apps/server/src/routes/rpc/services/maintenance/): proto → parse → service → convert; external Connect API unchanged.
• Slack (interactions.ts): createMaintenance branch goes through services. Also extracts getPageUrl / getReportUrl into apps/server/src/routes/slack/page-urls.ts so interactions.ts becomes db-free and can be added to the Biome scope.
• emailRouter (packages/api/src/router/email/index.ts): sendMaintenance becomes a thin wrapper over notifyMaintenance.

Enforcement

• Biome noRestrictedImports scope now also covers:

  • packages/api/src/router/maintenance.ts
  • apps/server/src/routes/rpc/services/maintenance/**
  • apps/server/src/routes/slack/interactions.ts

  Carve-outs (outside scope): slack/page-urls.ts, slack/service-adapter.ts, slack/confirmation-store.ts, slack/workspace-resolver.ts — all legitimately need db for transport/URL/auth concerns.

• Subpath export @openstatus/services/maintenance.

Tests

• packages/services/src/maintenance/__tests__/maintenance.test.ts — happy paths, workspace isolation, cascade deletes, cross-workspace NotFoundError / ForbiddenError, Zod date-range rejection, slack-actor audit branch.

Behaviour-preserving notes

• Connect's deleteMaintenance still returns { success: true }; tRPC's delete returned .returning() rows which dashboard ignored. Returning [] as Array<never> preserves the observable shape (an empty array is indistinguishable from "no rows deleted" in the old code path when the mutation succeeds).
• updateMaintenance Connect semantics preserved: updatePageComponentIds: false leaves associations alone; true clears-and-replaces with the provided pageComponentIds.

Test plan

• Service integration tests (seeded turso)
• Existing Connect maintenance.test.ts HTTP suite still green (external contract unchanged)
• Dashboard /maintenances page — create / update / delete round-trips, notify flag still triggers the two-call pattern
• Slack createMaintenance action (approve / approve_notify / cancel) round-trips

Stack

• feat(services): scaffold domain service layer (PR 0/N) #2100 scaffold
• feat(services): migrate status-report domain (PR 1/N) #2101 status-report
• This PR maintenance
• Next: PR 3 incident → PR 4 monitor → …

🤖 Generated with Claude Code

[vercel]

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

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
openstatus-dashboard	Skipped		Apr 24, 2026 11:46am
openstatus-status-page	Skipped		Apr 24, 2026 11:46am
openstatus-web	Skipped		Apr 24, 2026 11:46am

[cubic-dev-ai]

5 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="apps/server/src/routes/rpc/services/maintenance/index.ts">

<violation number="1" location="apps/server/src/routes/rpc/services/maintenance/index.ts:32">
P2: Invalid page component IDs are mapped to `invalidDateFormatError`, producing the wrong error reason/message for this field.</violation>
</file>

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

<violation number="1" location="packages/services/src/maintenance/list.ts:98">
P1: `listMaintenances` introduces a per-row enrichment pattern that causes 2 additional queries per item (N+1 behavior). Batch-load component relations for all returned maintenance IDs instead of querying per record.</violation>
</file>

<file name="packages/api/src/router/maintenance.ts">

<violation number="1" location="packages/api/src/router/maintenance.ts:20">
P2: `delete` is no longer idempotent for missing records; it now throws `NOT_FOUND` instead of returning an empty array.</violation>

<violation number="2" location="packages/api/src/router/maintenance.ts:53">
P1: `list` now combines a `10_000` sentinel limit with an N+1 enrichment path, which can generate extremely high query volume.</violation>
</file>

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

<violation number="1" location="packages/services/src/maintenance/internal.ts:35">
P2: `pageComponentIds` are not deduplicated before association inserts, so duplicate IDs can cause composite primary-key violations during create/update.</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)

Fixes pushed across the stack — status-report in the parent PR (#2101), maintenance here after a rebase. Summary:

✅ 1. N+1 in listMaintenances / listStatusReports

Valid and real — with limit: 10_000 from tRPC this was pathological (≈30k queries for the status-report list, ≈20k for maintenance). Rewrote both enrichment paths to batch:

• listStatusReports: 3 batched IN queries regardless of row count (updates, component associations, pages+their components). Pages are deduplicated so reports sharing a page share the same Page object.
• listMaintenances: 1 batched IN query (component associations).
• getStatusReport / getMaintenance reuse the batch path with a singleton array — same code, same semantics.

✅ 2. 10_000 sentinel + N+1

Closed by #1 — the sentinel is fine now that the enrichment doesn't multiply with row count. Pagination-proper is still a follow-up if the dashboard ever grows paging UI.

✅ 3. invalidDateFormatError for non-numeric page component id

Copy-paste bug from me — same mistake in both Connect handlers. Replaced with an inline new ConnectError('Invalid page component id: "…"', Code.InvalidArgument). Correct reason + message on the wire.

✅ 4. delete no longer idempotent

Valid for both statusReport.delete and maintenance.delete — the old tRPC procedures silently succeeded when the row was already gone. Services now throw NotFoundError (right for Connect's external API); tRPC wrappers catch and swallow NotFoundError to preserve the old idempotent behaviour. deleteUpdate stays strict — its old code also threw (with FORBIDDEN), so 404 is actually more honest.

✅ 5. pageComponentIds not deduplicated

Valid — duplicate ids would violate the composite PK on both join tables. Deduped at the entry of validatePageComponentIds in both domains' internal helpers. One-liner.

---

Commits:

• feat/services-status-report → 3b7c6cf3: dedupe + batch list + idempotent delete + numeric-id error
• feat/services-maintenance (rebased on ^, force-pushed) → e70473b4: same shape for maintenance

Local typecheck and biome are clean on both branches.

[cubic-dev-ai]

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

[mxkaske]

@claude review

[claude]

Claude finished @mxkaske's task in 5m 23s —— View job

---

Review

Good news: all five issues from the Apr 23 review are resolved in the current state. A few new things worth a look, in descending priority.

---

Dead code in errors.ts — four helpers never called

apps/server/src/routes/rpc/handlers/maintenance/errors.ts

The handler only imports invalidDateFormatError and maintenanceIdRequiredError. The remaining four helpers — pageComponentNotFoundError, pageComponentsMixedPagesError, pageNotFoundError, pageIdComponentMismatchError — are never imported by index.ts. Service-layer errors from those paths flow through toConnectError in adapter.ts, which emits generic codes without the structured error-reason header these helpers would carry.

Options:

• Delete them (current callers get the same behavior via toConnectError) and wire them back in later if the richer structured errors are needed by clients.
• Or call them from within the catch block per-error type — but that conflicts with the existing one-liner toConnectError(err) pattern.

Either way, four exported functions that nothing calls are a readability trap. Fix this →

---

parsePageComponentIds silently coerces "" to 0

apps/server/src/routes/rpc/handlers/maintenance/index.ts:29-38

function parsePageComponentIds(ids: ReadonlyArray<string>): number[] {
  return ids.map((id) => {
    const n = Number(id);
    if (!Number.isFinite(n)) {
      throw new ConnectError(`Invalid page component id: "${id}"`, Code.InvalidArgument);
    }
    return n;
  });
}

Number("") === 0 and Number.isFinite(0) === true, so an empty string in the list passes the guard and becomes component ID 0. The service layer will ultimately throw NotFoundError("page_component", 0) which surfaces as a 404, but the client gets a confusing "not found" rather than "invalid argument". Number.parseInt(id, 10) returns NaN for "" and would correctly reject it. Low-risk in practice (proto3 clients shouldn't send "" in a repeated string for an ID field), but easy to tighten. Fix this →

---

Atomicity gap: notify=true in Connect createMaintenance

apps/server/src/routes/rpc/handlers/maintenance/index.ts:59-64

const record = await createMaintenance({...}); // transaction committed ✓

if (req.notify) {
  await notifyMaintenance({...}); // if this throws → 500 to client
}

The maintenance row is committed before notifyMaintenance runs. If notifyMaintenance throws (e.g. a transient error in dispatchMaintenanceUpdate), the Connect client sees a 500 and may retry createMaintenance — creating a duplicate. The tRPC path avoids this by splitting into two explicit calls (maintenance.new then email.sendMaintenance). Status-report has the same shape, so this is a consistent choice, not an error introduced here. Worth acknowledging explicitly in a follow-up or doc comment so the next person doesn't "fix" it by wrapping everything in one transaction (which would be worse — holding a write lock across an external dispatch).

---

What's resolved since the last review

• Double ctx construction in interactions.ts ✅ — outer ctx is now used throughout executeAction, including the createMaintenance case.
• getReportUrl null guard ✅ — returns null when page not found, symmetric with getPageUrl.
• Dead helpers getPageComponentIdsForMaintenance / getPageComponentsForMaintenance ✅ — removed; internal.ts is clean.
• createMaintenance Connect response uses request IDs ✅ — now re-fetches via getMaintenance so the response reflects deduplicated, validated stored IDs.
• Stale comment ✅ — removed.

What's solid

• enrichMaintenancesBatch — single IN join, correct grouping, getMaintenance reuses it with a singleton slice.
• Dedup via new Set at the entry of validatePageComponentIds — correct placement before any insert.
• Idempotent tRPC delete catches NotFoundError and returns []; Connect correctly surfaces 404.
• parsePageComponentIds now throws ConnectError(…, Code.InvalidArgument) for non-numeric strings.
• Date range invariant enforced at both Zod parse time (from < to refine on CreateMaintenanceInput) and at runtime in updateMaintenance (combines partial input with existing row — the partial-update edge case is handled correctly).
• notifyMaintenance correctly gates on ctx.workspace.limits["status-subscribers"].
• page-urls.ts extraction is clean — interactions.ts is now db-free and covered by the Biome scope.
• Test suite covers workspace isolation, cascade delete, cross-workspace guard, dedup, slack-actor audit branch, and Zod range rejection.

[mxkaske]

@cubic-dev-ai review

[cubic-dev-ai]

@cubic-dev-ai review

@mxkaske I have started the AI code review. It will take a few minutes to complete.