Skip to content

Commit 8a353d5

Browse files
mxkaskeclaude
andauthored
feat(services): migrate status-report domain (PR 1/N) (#2101)
* feat(services): migrate status-report domain onto service layer First domain migration on top of the services scaffold (PR 0). Freezes the `ServiceContext` shape and the single-object `{ ctx, input }` argument convention across all three surfaces — tRPC / Connect / Slack. ## Services - `packages/services/src/status-report/` — create, add-update, resolve (convenience wrapper over add-update), update (metadata+associations), update-update (edit a single update row), delete, delete-update, list, get, notify. - Shared internals: `validatePageComponentIds`, `updatePageComponentAssociations`, `getReportInWorkspace`, `getReportUpdateInWorkspace`, `getUpdatesForReport`, `getPageComponentIdsForReport`. - All mutations run inside `withTransaction`, emit an `emitAudit` row before returning, and throw the typed `ServiceError` hierarchy. - Zod schemas are the contract; `.parse()` runs at every service entry as defence-in-depth. - `notifyStatusReport` is the separate-call notification dispatch that the dashboard invokes as its second mutation — preserves today's two-call pattern required by the Edge runtime. ## Surfaces - **tRPC** (`packages/api/src/router/statusReport.ts`): every procedure now calls a service. External contract is preserved bit-for-bit — `create` still returns `{ ...initialUpdate, notifySubscribers }`, `list` still returns the enriched `{updates,pageComponents,page}` shape the dashboard consumes, etc. `emailRouter.sendStatusReport` becomes a thin wrapper over `notifyStatusReport`. - **Connect RPC** (`apps/server/src/routes/rpc/services/status-report/`): handler extracts proto → parse → service call → convert back. Domain helpers previously exported from this folder (and consumed by Slack) are gone; callers go through services directly. - **Slack** (`apps/server/src/routes/slack/interactions.ts`): the four status-report branches (create / add-update / update / resolve) call services. The maintenance branch still writes to db directly — that migrates in PR 2. ## Adapters - `packages/api/src/service-adapter.ts` — `toServiceCtx(ctx)` + `toTRPCError(err)` (ServiceError → TRPCError). - `apps/server/src/routes/rpc/adapter.ts` — `toServiceCtx(rpcCtx)` + `toConnectError(err)`. Synthesises `actor.keyId` as `ws:<workspaceId>` until the auth interceptor captures the real key id (called out in the plan's open questions). - `apps/server/src/routes/slack/service-adapter.ts` — refetches the full workspace for the `PendingAction`'s workspaceId and builds a Slack actor context; `toSlackMessage(err)` renders human-readable failures. ## Enforcement - Biome `noRestrictedImports` (nursery rule) scoped to `packages/api/src/router/statusReport.ts` + `apps/server/src/routes/rpc/services/status-report/**`. Tests are ignored since setup/cleanup legitimately manipulates db state. - Subpath export `@openstatus/services/status-report` so surfaces import the domain barrel directly. - `packages/api` / `apps/server` now declare `@openstatus/services` as a dependency. ## Tests - Integration tests in `packages/services/src/status-report/__tests__/` cover happy paths, workspace isolation, cascading deletes, the `ForbiddenError` / `NotFoundError` paths, and the slack-actor audit branch. `expectAuditRow` asserts on the in-memory audit buffer. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(services): address Cubic review + unblock dashboard build - **Dashboard build**: `data-table-row-actions.tsx:35` accesses `row.original.page.customDomain`, which the old tRPC zod schema made non-nullable. My migrated service returned `page: Page | null`, breaking the type. Narrow `page` to non-null in the tRPC `get` / `list` wrappers via a tiny `narrowPage` helper that throws on the shouldn't- happen case (status report without its page) — matches the old zod parse behaviour. - **`listStatusReports` silent truncation (Cubic finding)**: the tRPC wrapper hard-coded `limit: 100`, silently dropping reports beyond that ceiling for workspaces with >100 reports. Service now allows an unbounded `limit` (min 1, no max); tRPC passes a `10_000` sentinel and Connect keeps its external `max=100` cap in the handler. - **`LIMIT_EXCEEDED` Connect mapping (Cubic finding)**: map to `Code.ResourceExhausted` rather than `Code.PermissionDenied` — quota/rate-limit is the documented meaning of ResourceExhausted in Connect/gRPC, while PermissionDenied means "authorised but not entitled". Correct semantics on the wire. Biome `include` key (Cubic's third finding) is verified correct for Biome 1.8.3 — OverridePattern schema documents `include` as the glob selector, and our override fires as expected (it flags the Connect test file in the scoped dir). No change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(services/status-report): address Cubic findings on the maintenance PR Applies to status-report since the same patterns were copy-pasted across both domains: - **Dedupe pageComponentIds** in `validatePageComponentIds` — duplicate ids in the input would hit the composite-PK constraint on `status_report_to_page_component` during insert. - **Batch list enrichment** — `listStatusReports` previously fanned out 3 queries per row (updates / component associations / page + page components). Dashboard calls it with `limit: 10_000`, so the N+1 was real. Rewritten as three batched `IN` queries regardless of row count; pages are deduplicated so reports sharing a page share the Page object. `getStatusReport` reuses the batch path with a singleton. - **Idempotent `delete`** — the old tRPC `statusReport.delete` silently succeeded when the row was missing; my service throws `NotFoundError`. Catch and swallow in the tRPC wrapper only. Connect still returns 404 on missing — external API semantics preserved. - **Connect numeric-id error** — `parsePageComponentIds` was throwing `invalidDateFormatError` for non-numeric input. Replaced with an inline `ConnectError(Code.InvalidArgument)` with the correct message. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(server): bind services + importers packages in dofigen (#2113) Adds `packages/services/package.json` and `packages/importers/package.json` to the `install` stage bind list so `bun install --frozen-lockfile` can resolve the dep graph inside the Docker build. Without this, the moment main receives: - `@openstatus/services` as a dep of `apps/server` (landing via the status-report PR that this branch targets), bun install fails with a missing workspace package. - `@openstatus/importers` becomes a transitive dep of `@openstatus/services` later in the stack (via the import-domain migration), so we bind it proactively in the same commit rather than landing a second dofigen touch-up. `packages/importers` already exists in the repo and isn't used by apps/server directly at this point, so binding it early is a no-op for bun until it actually becomes a dep. Alternative was two sequential fix PRs; one covers the whole stack cleaner. `Dockerfile` + `dofigen.lock` are regenerated from `dofigen.yml` via `dofigen update` — both are generated outputs that must stay in sync with the yml source. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * fix(services): stub RESEND_API_KEY for tests `packages/services/src/status-report/notify.ts` imports from `@openstatus/subscriptions`, which transitively pulls in `@openstatus/emails`. The emails package validates `RESEND_API_KEY` via `@t3-oss/env-core` at module-load time and throws on missing — which killed `@openstatus/services#test` in CI (neither the test.yml workflow nor local `pnpm test` sets the var). Adds a bun test preload that stubs the env var with a dummy value before any test file is imported. Tests never exercise the real Resend dispatch path, so this is scoped to unblocking the import graph rather than mocking the dispatcher itself. Added: - packages/services/bunfig.toml — points bun:test at the preload. - packages/services/test/preload.ts — sets the dummy env var. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(status-report): align with new service-layer error semantics Two CI failures surfaced after the services migration changed error shapes for status-report writes: - **tRPC `statusReport.create` / `.updateStatus` cross-workspace tests** expected `FORBIDDEN` when a caller passed pageComponentIds belonging to another workspace. The service's `validatePageComponentIds` now throws `NotFoundError` for the same case — intentionally, so cross- workspace lookups don't leak resource existence. Updated both tests to assert `NOT_FOUND` and left a comment pointing at the migration rationale. - **Connect `CreateStatusReport` page-mismatch test** asserted the error message contained "does not match the page ID". The service throws `ConflictError("pageId X does not match the page (Y) of the selected components.")` — no "ID" in the wording. Loosened the assertion to "does not match the page" so it matches the actual service string. Both are pure test updates — the new service behavior is the intended contract; the tests had drifted from the pre-migration wording. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(services): address post-migration review on #2101 Five findings from the PR review, fixed in one commit: **P1 — tRPC `LIMIT_EXCEEDED` → `TOO_MANY_REQUESTS`** `packages/api/src/service-adapter.ts` mapped `LIMIT_EXCEEDED` to `FORBIDDEN`. `FORBIDDEN` means "authenticated but not allowed to do this on this resource" — plan quota exhaustion is semantically different. Mirrors the Cubic fix on the Connect side (which moved from `PermissionDenied` to `ResourceExhausted`). **P2 — fragile Drizzle join result key in `enrichReportsBatch`** `packages/services/src/status-report/list.ts` read `row.status_report_to_page_component.statusReportId` — Drizzle auto-derives that key from the schema variable name at import time, so a rename would silently produce `undefined` at runtime. Replaced with explicit-column select (`reportId: statusReportsToPageComponents.statusReportId`) which is typed and rename-safe. **P2 — Slack `NOT_FOUND` messages leak internal row IDs** `NotFoundError` formats as `"<entity> <id> not found"` (e.g. `"page_component 17 not found"`). For API consumers that's fine — they're developers. For Slack workspace users those IDs are meaningless noise and surface internals. `toSlackMessage` now returns a fixed `"Couldn't find what you were looking for."` for `NOT_FOUND`; other `ServiceError` codes keep their hand-written messages (they're sentence-style to start with). **P3 — test cleanup without try/finally** `packages/services/src/status-report/__tests__/status-report.test.ts` had 13 tests that did inline `db.delete(statusReport)` cleanup at the end of their body — skipped on assertion failure. Replaced with a shared `createdReportIds` array + `afterEach` drain so cleanup runs regardless of test outcome. **P3 — hoisted `toServiceCtx` out of Slack `executeAction` switch** Every service-backed branch loaded the workspace row via `toServiceCtx`. Hoisted the call above the switch — halves db lookups per Slack action execution. The `createMaintenance` branch (still on db until PR 2) doesn't use the ctx but the extra lookup there is negligible. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(services/status-report): pageId follows components on update Two server-side tests were failing after the migration: - `updates pageId when changing components to different page` - `clears pageId when removing all components` Both expected `report.pageId` to track the association set: - Swap components to a different page → `pageId` moves. - Empty the association set → `pageId` becomes null. The service was enforcing a stricter invariant — it *threw* a `ConflictError` when the new components' page differed from the report's current page, and left `pageId` untouched when associations were cleared. That was an overcautious choice in the initial migration; no caller today wants a "report pinned to page X regardless of its components" guarantee. Simplified to: whenever `pageComponentIds` is supplied, `updateValues.pageId = validated.pageId`. The zero-component case sets `pageId = null`; the one-page case moves the report to that page. Mixed-page inputs are still rejected by `validatePageComponentIds`'s `componentPageIds.size > 1` check, so the only reachable states here are "one page" or "no components". Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 58ccc38 commit 8a353d5

32 files changed

Lines changed: 2357 additions & 1054 deletions

apps/server/Dockerfile

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ RUN \
4141
--mount=type=bind,target=packages/assertions/package.json,source=packages/assertions/package.json \
4242
--mount=type=bind,target=packages/theme-store/package.json,source=packages/theme-store/package.json \
4343
--mount=type=bind,target=packages/locales/package.json,source=packages/locales/package.json \
44+
--mount=type=bind,target=packages/services/package.json,source=packages/services/package.json \
45+
--mount=type=bind,target=packages/importers/package.json,source=packages/importers/package.json \
4446
--mount=type=cache,target=/root/.bun/install/cache,sharing=locked \
4547
bun install --production --frozen-lockfile --verbose
4648

@@ -62,11 +64,11 @@ COPY \
6264
RUN bun build --compile --sourcemap src/index.ts --outfile=app
6365

6466
# runtime
65-
FROM debian@sha256:4333240150a6924f878e05ec2c998aec95238010e0e4d2fec6161c90128c4652 AS runtime
67+
FROM debian@sha256:1a4701c321b1d28b1ff5f0230e766791e4b79b1d4c6c7a70064f4b297b1a330f AS runtime
6668
LABEL \
6769
io.dofigen.version="2.7.0" \
6870
org.opencontainers.image.authors="OpenStatus Team" \
69-
org.opencontainers.image.base.digest="sha256:4333240150a6924f878e05ec2c998aec95238010e0e4d2fec6161c90128c4652" \
71+
org.opencontainers.image.base.digest="sha256:1a4701c321b1d28b1ff5f0230e766791e4b79b1d4c6c7a70064f4b297b1a330f" \
7072
org.opencontainers.image.base.name="docker.io/debian:bullseye-slim" \
7173
org.opencontainers.image.description="REST API server with Hono framework for OpenStatus" \
7274
org.opencontainers.image.source="https://github.com/openstatusHQ/openstatus" \

apps/server/dofigen.lock

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ effective: |
1515
path: oven/bun
1616
digest: sha256:f20d9cf365ab35529384f1717687c739c92e6f39157a35a95ef06f4049a10e4a
1717
label:
18-
org.opencontainers.image.base.name: docker.io/oven/bun:1.3.6
1918
org.opencontainers.image.stage: install
19+
org.opencontainers.image.base.name: docker.io/oven/bun:1.3.6
2020
org.opencontainers.image.base.digest: sha256:f20d9cf365ab35529384f1717687c739c92e6f39157a35a95ef06f4049a10e4a
2121
workdir: /app/
2222
run:
@@ -86,14 +86,18 @@ effective: |
8686
source: packages/theme-store/package.json
8787
- target: packages/locales/package.json
8888
source: packages/locales/package.json
89+
- target: packages/services/package.json
90+
source: packages/services/package.json
91+
- target: packages/importers/package.json
92+
source: packages/importers/package.json
8993
build:
9094
fromImage:
9195
path: oven/bun
9296
digest: sha256:f20d9cf365ab35529384f1717687c739c92e6f39157a35a95ef06f4049a10e4a
9397
label:
94-
org.opencontainers.image.base.name: docker.io/oven/bun:1.3.6
9598
org.opencontainers.image.stage: build
9699
org.opencontainers.image.base.digest: sha256:f20d9cf365ab35529384f1717687c739c92e6f39157a35a95ef06f4049a10e4a
100+
org.opencontainers.image.base.name: docker.io/oven/bun:1.3.6
97101
workdir: /app/apps/server
98102
env:
99103
NODE_ENV: production
@@ -109,16 +113,16 @@ effective: |
109113
- bun build --compile --sourcemap src/index.ts --outfile=app
110114
fromImage:
111115
path: debian
112-
digest: sha256:4333240150a6924f878e05ec2c998aec95238010e0e4d2fec6161c90128c4652
116+
digest: sha256:1a4701c321b1d28b1ff5f0230e766791e4b79b1d4c6c7a70064f4b297b1a330f
113117
label:
114-
org.opencontainers.image.authors: OpenStatus Team
118+
io.dofigen.version: 2.7.0
115119
org.opencontainers.image.source: https://github.com/openstatusHQ/openstatus
120+
org.opencontainers.image.base.digest: sha256:1a4701c321b1d28b1ff5f0230e766791e4b79b1d4c6c7a70064f4b297b1a330f
116121
org.opencontainers.image.base.name: docker.io/debian:bullseye-slim
117-
io.dofigen.version: 2.7.0
122+
org.opencontainers.image.title: OpenStatus Server
118123
org.opencontainers.image.description: REST API server with Hono framework for OpenStatus
119-
org.opencontainers.image.base.digest: sha256:4333240150a6924f878e05ec2c998aec95238010e0e4d2fec6161c90128c4652
120124
org.opencontainers.image.vendor: OpenStatus
121-
org.opencontainers.image.title: OpenStatus Server
125+
org.opencontainers.image.authors: OpenStatus Team
122126
user:
123127
user: '1000'
124128
group: '1000'
@@ -145,17 +149,17 @@ effective: |
145149
retries: 3
146150
images:
147151
docker.io:
148-
library:
149-
debian:
150-
bullseye-slim:
151-
digest: sha256:4333240150a6924f878e05ec2c998aec95238010e0e4d2fec6161c90128c4652
152152
oven:
153153
bun:
154154
1.3.6:
155155
digest: sha256:f20d9cf365ab35529384f1717687c739c92e6f39157a35a95ef06f4049a10e4a
156+
library:
157+
debian:
158+
bullseye-slim:
159+
digest: sha256:1a4701c321b1d28b1ff5f0230e766791e4b79b1d4c6c7a70064f4b297b1a330f
156160
resources:
157161
dofigen.yml:
158-
hash: 733fba04030ad149a064b72cece51b1e515655e1b723263c61c881e301e388da
162+
hash: 0b4934ac4087020f7814554afb8423bb611e051830041107fdecd32afaa8dc28
159163
content: |
160164
# Files to exclude from Docker context
161165
ignore:
@@ -208,6 +212,8 @@ resources:
208212
- packages/assertions/package.json
209213
- packages/theme-store/package.json
210214
- packages/locales/package.json
215+
- packages/services/package.json
216+
- packages/importers/package.json
211217
run: bun install --production --frozen-lockfile --verbose
212218
cache:
213219
- /root/.bun/install/cache

apps/server/dofigen.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ builders:
4949
- packages/assertions/package.json
5050
- packages/theme-store/package.json
5151
- packages/locales/package.json
52+
- packages/services/package.json
53+
- packages/importers/package.json
5254
run: bun install --production --frozen-lockfile --verbose
5355
cache:
5456
- /root/.bun/install/cache

apps/server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
"@logtape/otel": "2.0.1",
2525
"@logtape/sentry": "2.0.1",
2626
"@openstatus/analytics": "workspace:*",
27+
"@openstatus/services": "workspace:*",
2728
"@openstatus/subscriptions": "workspace:*",
2829
"@openstatus/notification-discord": "workspace:*",
2930
"@openstatus/notification-google-chat": "workspace:*",
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { Code, ConnectError } from "@connectrpc/connect";
2+
import { type ServiceContext, ServiceError } from "@openstatus/services";
3+
import { ZodError } from "zod";
4+
5+
import type { RpcContext } from "./interceptors";
6+
7+
/**
8+
* Translate Connect RPC auth context into a `ServiceContext`.
9+
*
10+
* TODO: Once the auth interceptor captures the API key id, expose it here
11+
* as `actor.keyId`. Until then we synthesise `ws:<workspaceId>` so audit
12+
* records still have a non-empty actor identifier — see the plan's open
13+
* question "Does Connect's auth interceptor capture API key ID today?".
14+
*/
15+
export function toServiceCtx(rpcCtx: RpcContext): ServiceContext {
16+
return {
17+
workspace: rpcCtx.workspace,
18+
actor: { type: "apiKey", keyId: `ws:${rpcCtx.workspace.id}` },
19+
requestId: rpcCtx.requestId,
20+
};
21+
}
22+
23+
/**
24+
* Map any error thrown by a service call to a `ConnectError`. Preserves the
25+
* existing Connect error surface — granular reasons carried by the caller's
26+
* per-handler error helpers (in `errors.ts`) still bypass this mapper since
27+
* they throw `ConnectError` directly.
28+
*/
29+
export function toConnectError(err: unknown): never {
30+
if (err instanceof ConnectError) throw err;
31+
if (err instanceof ZodError) {
32+
throw new ConnectError(
33+
`Invalid request: ${err.message}`,
34+
Code.InvalidArgument,
35+
);
36+
}
37+
if (err instanceof ServiceError) {
38+
switch (err.code) {
39+
case "NOT_FOUND":
40+
throw new ConnectError(err.message, Code.NotFound);
41+
case "FORBIDDEN":
42+
throw new ConnectError(err.message, Code.PermissionDenied);
43+
case "UNAUTHORIZED":
44+
throw new ConnectError(err.message, Code.Unauthenticated);
45+
case "CONFLICT":
46+
throw new ConnectError(err.message, Code.InvalidArgument);
47+
case "VALIDATION":
48+
throw new ConnectError(err.message, Code.InvalidArgument);
49+
case "LIMIT_EXCEEDED":
50+
throw new ConnectError(err.message, Code.ResourceExhausted);
51+
case "INTERNAL":
52+
throw new ConnectError(err.message, Code.Internal);
53+
}
54+
}
55+
const message = err instanceof Error ? err.message : "Unknown error";
56+
throw new ConnectError(message, Code.Internal);
57+
}

apps/server/src/routes/rpc/services/status-report/__tests__/status-report.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,10 @@ describe("StatusReportService.CreateStatusReport", () => {
394394

395395
expect(res.status).toBe(400);
396396
const data = await res.json();
397-
expect(data.message).toContain("does not match the page ID");
397+
// Service throws `ConflictError("pageId X does not match the page
398+
// (Y) of the selected components.")` — assertion matches the new
399+
// service-layer wording.
400+
expect(data.message).toContain("does not match the page");
398401
});
399402

400403
test("creates status report when pageId matches component page", async () => {

0 commit comments

Comments
 (0)