Skip to content

feat(rewrite): add Phase 3 monitoring ingestion and catalogs - #398

Merged
rajohan merged 3 commits into
mira/greenfield-rewrite-phase-three-agentsfrom
mira/greenfield-rewrite-phase-three-monitoring
Aug 24, 2026
Merged

feat(rewrite): add Phase 3 monitoring ingestion and catalogs#398
rajohan merged 3 commits into
mira/greenfield-rewrite-phase-three-agentsfrom
mira/greenfield-rewrite-phase-three-monitoring

Conversation

@mira-2026

@mira-2026 mira-2026 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add complete-snapshot monitoring ingestion plus immutable report, incident, and Dashboard-notification catalogs
  • expose 13 validated tRPC procedures with capability and principal-kind boundaries, typed expected errors, production request-context wiring, and a real HTTP composition proof
  • add exact monitoring transport budgets, atomic realtime events, bounded catalog mutations, generated contract artifacts, parity inventory, and Phase 3 progress evidence
  • harden idempotent producer replay, clock-regression ordering, retention overflow, report-linked deletion, and database admission behavior
  • address review findings with default keyset indexes, composed body-profile limits, shared principal/filter policies, explicit wake-failure observability, incident-generation validation, and targeted cascade/nullability regressions

Behavior and regression coverage

  • automation principals can submit one normalized complete monitoring snapshot and produce reports, incident lifecycle state, notifications, and realtime events in one immediate transaction
  • authenticated callers can list/load incidents and reports; scoped sessions or automations may upsert reports, while notification producers and browser-session acknowledgement actions retain exact principal-kind boundaries
  • report and notification producer replays are exact and idempotent; divergent immutable IDs return CONFLICT without partial writes or duplicate events
  • notification producers must reference the current persisted incident generation; mismatches return NOT_FOUND without a notification row or realtime event
  • incident-generation notifications retain the authenticated forward link /incidents?incidentId=… with reportId: null, because a persistent generation can span multiple report observations; the hidden reader lands in the immediately stacked browser slice before cutover
  • monitoring snapshot ingestion and report upsert use an exact non-batchable 640 KiB transport profile while semantic aggregate limits remain 512 KiB; unknown or malformed procedure paths keep the 64 KiB default
  • mixed ordinary request-body profiles compose by their largest allowance, while authentication and WebAuthn retain their restrictive namespace ceilings
  • unfiltered and cursor-based report, notification, and incident lists use matching composite keyset indexes without temporary sorting
  • notification bulk work and report-linked cascades are bounded; linked rows use one compact snapshot-required invalidation, and oversized report deletion returns PRECONDITION_FAILED
  • mutation time remains monotonic across clock regression, and producer timestamps that cannot leave realtime-retention room return declared BAD_REQUEST failures without writes, events, or wakeups
  • post-commit realtime wake failures are logged safely while SQLite remains authoritative and adaptive polling recovers the missed wake
  • generated procedure/realtime schemas remain fail-closed for runtime-only Valibot refinements and document all five snapshot-capable realtime topics

Verification

  • Repository lint: cd greenfield && bun run lint
  • Repository formatting: cd greenfield && bun run format:check
  • Frontend build: bun run build:frontend — not applicable to this isolated greenfield slice; browser artifacts were built by cd greenfield && bun run build:release
  • Frontend tests/coverage: bun run test:frontend:coverage — legacy frontend is outside this slice; full greenfield browser coverage ran below
  • Backend build: bun run build:backend — not applicable to this isolated greenfield slice; process artifacts were built by cd greenfield && bun run build:release
  • Backend tests/coverage: bun run test:backend:coverage — legacy backend is outside this slice; full greenfield coverage ran below
  • Focused regression tests: latest catalog/service/procedure review suite (19/19), plus the complete greenfield suite below
  • Manual UI/API smoke check, if relevant — inactive pre-cutover slice with no new browser workflow; real HTTP composition and release-build tests cover the executable boundary

Additional greenfield gates:

  • bun run typecheck
  • bun run check:boundaries
  • bun run docs:check
  • bun run db:check
  • bun run test:coverage — 1,474 tests, 0 failures, 92.25% line coverage (37,004/40,112)
  • bun run build:release — clean-source release for 488425875a815b1806f77a0ba75342937c2d8e13

Risk checklist

  • No secrets, tokens, .env files, database dumps, or runtime state committed
  • Auth, Gateway, terminal, file, Docker, or settings changes were reviewed carefully
  • New/changed API routes enforce the expected authentication and validation
  • Migrations or data-shape changes include a rollout/rollback note, if relevant
  • Runtime/reconnect behavior preserves ordering, idempotency, and recovery
  • No visible UI change in this slice

The migration edits update the unpublished fresh-database greenfield baseline only. This slice remains inactive until the final supervised cutover; rollback before cutover is to revert the slice and regenerate the fresh baseline and release artifacts.

Deployment / operations

  • No deploy/restart needed
  • Deploy/restart needed after merge: none; this stack remains inactive until supervised cutover
  • Config/secrets changes needed: none
  • Rollback path verified: revert this PR before cutover and regenerate the unpublished fresh-database baseline/release artifacts

Notes for reviewers

  • focus on immediate-transaction atomicity, exact replay handling, bounded deletion, and monotonic event timing
  • review the qualified 640 KiB request profile together with the 512 KiB semantic budgets and fail-closed unknown-path behavior
  • producer timestamp overflow is a catalog-specific validation failure; incompatible clock/retention configuration fails fast before the service is exposed
  • stack base: feat(rewrite): add Phase 3 agent directory #397 at locked head 47fac9196081c19deb1c16034f09d63e92aa33f2

@mira-2026
mira-2026 requested a review from rajohan as a code owner August 7, 2026 18:06
@mira-2026 mira-2026 added status: needs-review Ready for human or agent review type: feature Adds user-visible functionality type: tests Adds or updates automated tests/coverage area: database Database dashboard, Postgres, PgBouncer, or query views area: openclaw OpenClaw gateway/API integration and runtime behavior area: ops Operational actions, deploys, services, backups, and health area: backend Backend API, server routes, services, and integrations labels Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added monitoring snapshot submission for authorized automation processes.
    • Added incident lookup, notification management, and report listing, viewing, creation, updating, and deletion.
    • Added realtime updates for monitoring events with seven-day retention.
    • Reports now include status and optional summaries; notifications can link to reports and include a source.
    • Added validation, pagination, size limits, capability controls, and clear error handling across monitoring workflows.
  • Documentation
    • Updated architecture and implementation progress documentation to reflect monitoring and catalog support.

Walkthrough

The PR adds monitoring snapshot ingestion, incident and notification catalog operations, report CRUD operations, realtime event contracts, database fields, capability enforcement, request-size policies, server wiring, documentation generation, and parity updates.

Changes

Monitoring foundation

Layer / File(s) Summary
Contracts and persistence schema
greenfield/src/contracts/*, greenfield/src/server/database/*, greenfield/migrations/*
Adds monitoring, incident, notification, report, realtime, capability, pagination, timestamp, size, and status contracts. Adds report references, report status and summary fields, indexes, and migration metadata.
Catalog repository and service
greenfield/src/server/domains/monitoring/repository.ts, greenfield/src/server/domains/monitoring/catalogService.ts, greenfield/src/server/domains/monitoring/catalogRecords.ts, greenfield/src/server/domains/monitoring/serialization.ts
Adds validated catalog reads and mutations, cursor pagination, idempotent upserts, notification operations, report deletion safeguards, record conversion, canonical JSON handling, realtime events, and post-commit wakeups.
Routes and server integration
greenfield/src/server/domains/monitoring/*Routes.ts, greenfield/src/server/domains/monitoring/procedures.ts, greenfield/src/server/trpc/*, greenfield/src/app/*
Adds automation-only snapshot ingestion, capability-scoped catalog routes, typed tRPC error mapping, monitoring service context dependencies, root router registration, shared principal-kind checks, and the 640 KiB monitoring request limit.
Validation and published parity
greenfield/scripts/documentation/*, greenfield/src/*test*, greenfield/docs/*, greenfield/src/test/parity/*
Adds contract, service, route, request-policy, documentation, end-to-end, migration, query-plan, and parity coverage for monitoring functionality.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AutomationPrincipal
  participant MonitoringRoute
  participant MonitoringService
  participant MonitoringRepository
  participant ReportCatalog
  AutomationPrincipal->>MonitoringRoute: submitCompleteSnapshot
  MonitoringRoute->>MonitoringService: validate and ingest snapshot
  MonitoringService->>MonitoringRepository: commit snapshot and report
  MonitoringService->>ReportCatalog: expose persisted report
  ReportCatalog-->>AutomationPrincipal: return report result
Loading

Possibly related PRs

  • rajohan/Mira-Dashboard#201: Covers related monitoring and report persistence, status, summary, notification links, and report CRUD behavior.
  • rajohan/Mira-Dashboard#378: Provides the greenfield monitoring, contract, tRPC, documentation, and realtime foundation extended by this PR.
  • rajohan/Mira-Dashboard#382: Introduces monitoring lifecycle code that this PR extends with catalog, ingestion, reporting, and expanded contracts.

Suggested labels: area: notifications

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: Phase 3 monitoring ingestion and catalog support.
Description check ✅ Passed The description covers the required summary, behavior, verification, risk, deployment, rollback, and reviewer-notes sections with concrete details.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
greenfield/src/app/trpcRequestPolicy.ts (1)

147-162: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

A mixed batch can apply a smaller body limit than a monitoring procedure needs.

The chain assigns requestBodyMaximumBytes by last write. Monitoring is written first, so any later flag overwrites it. The test at greenfield/src/app/trpcRequestPolicy.test.ts line 113 shows rejectsBatch: false for monitoring.submitCompleteSnapshot, so a batch may contain a monitoring procedure together with a task-content procedure. In that batch the task-content limit wins. If that limit is below 640 KiB, the request is rejected with 413 even though the monitoring procedure is within its own budget.

Select the maximum of the applicable limits instead of the last one.

🐛 Proposed fix
     let requestBodyMaximumBytes = trpcRequestBodyMaximumBytes;
-    if (input.containsMonitoringProcedure) {
-        requestBodyMaximumBytes = monitoringRequestBodyMaximumBytes;
-    }
-    if (input.containsTaskContentProcedure) {
-        requestBodyMaximumBytes = taskContentRequestBodyMaximumBytes;
-    }
-    if (input.containsTaskProgressProcedure) {
-        requestBodyMaximumBytes = taskProgressRequestBodyMaximumBytes;
-    }
-    if (input.containsAuthenticationProcedure) {
-        requestBodyMaximumBytes = authenticationRequestBodyMaximumBytes;
-    }
-    if (input.containsWebAuthnProcedure) {
-        requestBodyMaximumBytes = webAuthnRequestBodyMaximumBytes;
-    }
+    if (input.containsMonitoringProcedure) {
+        requestBodyMaximumBytes = Math.max(
+            requestBodyMaximumBytes,
+            monitoringRequestBodyMaximumBytes
+        );
+    }
+    if (input.containsTaskContentProcedure) {
+        requestBodyMaximumBytes = Math.max(
+            requestBodyMaximumBytes,
+            taskContentRequestBodyMaximumBytes
+        );
+    }
+    if (input.containsTaskProgressProcedure) {
+        requestBodyMaximumBytes = Math.max(
+            requestBodyMaximumBytes,
+            taskProgressRequestBodyMaximumBytes
+        );
+    }
+    if (input.containsAuthenticationProcedure) {
+        requestBodyMaximumBytes = Math.max(
+            requestBodyMaximumBytes,
+            authenticationRequestBodyMaximumBytes
+        );
+    }
+    if (input.containsWebAuthnProcedure) {
+        requestBodyMaximumBytes = Math.max(
+            requestBodyMaximumBytes,
+            webAuthnRequestBodyMaximumBytes
+        );
+    }

Note: if authentication and WebAuthn limits are intentionally restrictive ceilings rather than allowances, keep their assignment as overrides and apply the maximum only to the monitoring, task-content, and task-progress limits.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/app/trpcRequestPolicy.ts` around lines 147 - 162, Update the
request-body limit selection in the policy function around
trpcRequestBodyMaximumBytes so monitoring, task-content, and task-progress
procedure limits are combined by selecting the maximum applicable value rather
than by last assignment. Preserve authentication and WebAuthn as intentional
restrictive overrides if that is the existing contract, and keep the resulting
limit sufficient for mixed batches containing monitoring procedures.
🧹 Nitpick comments (9)
greenfield/src/server/domains/monitoring/testSupport/services.ts (1)

6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Include the method name in the unexpected-call message.

Every member shares one message. If a test triggers an unexpected call, the defect does not identify which method ran. Naming the method shortens diagnosis.

♻️ Proposed refactor
-function unexpectedMonitoringServiceCall(): Effect.Effect<never> {
-    return Effect.die(new Error("Test monitoring service received an unexpected call"));
-}
+function unexpectedMonitoringServiceCall(method: string): () => Effect.Effect<never> {
+    return () =>
+        Effect.die(
+            new Error(`Test monitoring service received an unexpected call: ${method}`)
+        );
+}

Then call it per member, for example deleteReport: unexpectedMonitoringServiceCall("deleteReport").

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/server/domains/monitoring/testSupport/services.ts` around
lines 6 - 8, Update unexpectedMonitoringServiceCall to accept the invoked
monitoring method name and include it in the defect message, then pass each
member’s method name at its call site (such as deleteReport). Ensure
unexpected-call failures identify the specific member that was triggered.
greenfield/src/server/domains/monitoring/service.ts (1)

165-172: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Record suppressed wake failures.

The catch block discards the error with no log and no metric. A persistent wakeEventPump failure then degrades realtime latency silently, and the only external symptom is that subscribers fall back to polling. Recovery is correct, but the condition is invisible to operators.

Consider accepting an optional logger or failure callback in MonitoringServiceDependencies and emitting one warning per suppressed failure. catalogService.wakeAfterChange suppresses failures the same way, so a shared helper would cover both monitoring services.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/server/domains/monitoring/service.ts` around lines 165 - 172,
Record suppressed wakeEventPump failures instead of silently ignoring them:
extend MonitoringServiceDependencies with an optional logger or failure
callback, invoke it once from wakeAfterCommit’s catch block, and preserve SQLite
authority plus adaptive-polling recovery. Apply the same reporting mechanism to
catalogService.wakeAfterChange, preferably through a shared helper.
greenfield/src/app/dashboardServer.ts (1)

266-281: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the repeated wake closure.

() => options.applicationRuntime.services.realtimeEvents.wake() now appears four times in this function, for the agent, task, monitoring, and monitoring catalog services. Bind it once above the service construction and reuse the reference.

♻️ Proposed refactor
+        const wakeEventPump = () =>
+            options.applicationRuntime.services.realtimeEvents.wake();
         const monitoringRepository = createMonitoringRepository(
             database,
             databaseRuntime
         );
         const monitoringService = createMonitoringService({
             ...(domainNow === undefined ? {} : { nowMs: () => domainNow().getTime() }),
             repository: monitoringRepository,
-            wakeEventPump: () =>
-                options.applicationRuntime.services.realtimeEvents.wake(),
+            wakeEventPump,
         });
         const monitoringCatalogService = createMonitoringCatalogService({
             ...(domainNow === undefined ? {} : { nowMs: () => domainNow().getTime() }),
             repository: monitoringRepository,
-            wakeEventPump: () =>
-                options.applicationRuntime.services.realtimeEvents.wake(),
+            wakeEventPump,
         });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/app/dashboardServer.ts` around lines 266 - 281, Extract the
repeated realtime event wake callback in the surrounding dashboard server
function into a single local reference before constructing services, then pass
that reference to the agent, task, monitoring, and monitoring catalog service
factories instead of recreating the closure.
greenfield/src/server/domains/monitoring/repository.ts (1)

331-385: 🚀 Performance & Scalability | 🔵 Trivial

Consider indexes that match the default keyset order.

The three listing methods sort by (occurredAt|lastSeenAt DESC, id DESC) with no required filter. The current schema provides only partial or prefix indexes for these tables: incidents_active_monitor_seen_idx is partial on state = 'active', notifications_unread_occurred_idx is partial on read_at IS NULL, and reports_kind_occurred_id_idx leads with kind. An unfiltered page therefore needs a scan and a sort. Page size stays bounded, so latency grows with total table size rather than page size. If these tables are expected to grow, add composite indexes on (occurred_at, id) for reports and notifications, and (last_seen_at, id) for incidents.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/server/domains/monitoring/repository.ts` around lines 331 -
385, Add composite indexes matching the default keyset ordering for the three
listing methods: reports and notifications on occurred_at plus id, and incidents
on last_seen_at plus id. Define them in the schema/index configuration near the
existing table indexes, without partial predicates or leading filter columns, so
unfiltered listReports, listNotifications, and listIncidents queries can use the
ordering index.
greenfield/src/server/domains/monitoring/catalogRoutes.ts (1)

37-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the principal-kind guard into one helper.

The same guard block appears three times: here twice, and once in greenfield/src/server/domains/monitoring/ingestionRoutes.ts at lines 10-20. A shared helper removes the duplication and keeps the FORBIDDEN message policy in one place.

♻️ Proposed shared guard
// greenfield/src/server/trpc/trpc.ts
export function principalKindProcedure(
    capability: ApplicationCapability,
    kind: PrincipalKind,
    message: string
) {
    return capabilityProcedure(capability).use(({ ctx, next }) => {
        if (ctx.principal.kind !== kind) {
            throw new TRPCError({ code: "FORBIDDEN", message });
        }
        return next({ ctx });
    });
}
-const notificationProducerProcedure = capabilityProcedure("notifications:write").use(
-    ({ ctx, next }) => {
-        if (ctx.principal.kind !== "automation") {
-            throw new TRPCError({
-                code: "FORBIDDEN",
-                message: "An automation principal is required",
-            });
-        }
-        return next({ ctx });
-    }
-);
+const notificationProducerProcedure = principalKindProcedure(
+    "notifications:write",
+    "automation",
+    "An automation principal is required"
+);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/server/domains/monitoring/catalogRoutes.ts` around lines 37 -
58, Extract the repeated principal-kind checks from
notificationProducerProcedure, notificationSessionWriteProcedure, and the
corresponding ingestion route into a shared principalKindProcedure helper in
trpc.ts. Have the helper accept the capability, expected PrincipalKind, and
forbidden message, while preserving each procedure’s existing capability and
message.
greenfield/src/app/trpcRequestPolicy.test.ts (1)

124-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assert the Math.max invariant instead of equality.

serverRequestBodyMaximumBytes is the maximum of the task-content limit and the monitoring limit. The equality holds only while the monitoring limit is the larger value. If the task-content limit later exceeds 640 KiB, this test fails although the policy remains correct.

♻️ Proposed assertion
-        expect(monitoringRequestBodyMaximumBytes).toBe(serverRequestBodyMaximumBytes);
+        expect(serverRequestBodyMaximumBytes).toBeGreaterThanOrEqual(
+            monitoringRequestBodyMaximumBytes
+        );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/app/trpcRequestPolicy.test.ts` at line 124, Update the
assertion in the trpc request policy test to verify that
serverRequestBodyMaximumBytes equals the maximum of the task-content and
monitoring limits, rather than asserting equality with
monitoringRequestBodyMaximumBytes. Preserve the existing policy values and
reference the relevant limit symbols already used by the test.
greenfield/src/contracts/monitoring.ts (2)

20-35: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Reduce the repeated encoding passes on the ingestion path.

encodedJsonBytes runs JSON.stringify plus a character-by-character utf8ByteLength loop for every monitoring JSON object. One snapshot can contain up to 100 problem details objects plus report metadata, each up to 64 KiB. monitoringMutationInputFitsBudget then serializes and scans the whole payload again, up to 512 KiB. This runs on the request thread for each submission.

Use Buffer.byteLength(json, "utf8") for the byte count. It is a native single pass and removes the per-character loop.

⚡ Proposed change
 function encodedJsonBytes(value: JsonObject): number {
-    return utf8ByteLength(JSON.stringify(value));
+    return Buffer.byteLength(JSON.stringify(value), "utf8");
 }
 export function monitoringMutationInputFitsBudget(value: unknown): boolean {
-    return utf8ByteLength(JSON.stringify(value)) <= monitoringMutationInputMaximumBytes;
+    return (
+        Buffer.byteLength(JSON.stringify(value), "utf8") <=
+        monitoringMutationInputMaximumBytes
+    );
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/contracts/monitoring.ts` around lines 20 - 35, Replace the
character-scanning byte calculation used by encodedJsonBytes and
monitoringMutationInputFitsBudget with Buffer.byteLength(JSON.stringify(value),
"utf8"). Preserve both existing budget comparisons and JSON serialization
behavior while removing reliance on utf8ByteLength.

259-318: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Collapse the four duplicated incident timestamp predicates.

activeIncidentTimesAreConsistent and activeIncidentSummaryTimesAreConsistent have identical bodies. resolvedIncidentTimesAreConsistent and resolvedIncidentSummaryTimesAreConsistent are also identical. The summary entries are a strict subset of the record entries, so two structural predicates cover all four call sites.

Also add JSDoc to the two summary predicates. The other exported predicates in this file document their return value.

♻️ Proposed shape
-export function activeIncidentTimesAreConsistent(incident: ActiveIncident): boolean {
-    return incident.lastSeenAtMs >= incident.firstSeenAtMs;
-}
+/** `@returns` Whether an incident's observation window is monotonic. */
+export function incidentSeenTimesAreConsistent(incident: {
+    firstSeenAtMs: number;
+    lastSeenAtMs: number;
+}): boolean {
+    return incident.lastSeenAtMs >= incident.firstSeenAtMs;
+}
+
+/** `@returns` Whether a resolved incident's lifecycle timestamps are monotonic. */
+export function resolvedIncidentLifecycleIsConsistent(incident: {
+    firstSeenAtMs: number;
+    lastSeenAtMs: number;
+    resolvedAtMs: number;
+}): boolean {
+    return (
+        incidentSeenTimesAreConsistent(incident) &&
+        incident.resolvedAtMs >= incident.lastSeenAtMs
+    );
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/contracts/monitoring.ts` around lines 259 - 318, Collapse the
duplicate predicates in incidentRecordSchema and incidentSummarySchema by
reusing the existing activeIncidentTimesAreConsistent and
resolvedIncidentTimesAreConsistent functions for both full records and
summaries, removing the two summary-specific predicate functions. Preserve the
existing validation checks and add return-value JSDoc to the two retained
exported predicates.
greenfield/src/contracts/incidents.ts (1)

33-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated filter-schema helpers in two new contract files. uniqueFilterSchema and enumFilterSchema are defined twice with identical bodies. Only the maximum constant differs, and both files set it to 16. Extract one pair of helpers into a shared contract module that takes the maximum as a parameter.

  • greenfield/src/contracts/incidents.ts#L33-L58: remove the local helpers and import the shared versions, passing incidentFilterMaximum.
  • greenfield/src/contracts/notifications.ts#L36-L61: remove the local helpers and import the shared versions, passing notificationFilterMaximum.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@greenfield/src/contracts/incidents.ts` around lines 33 - 58, Extract the
duplicated uniqueFilterSchema and enumFilterSchema implementations into a shared
contract module with the maximum-length value supplied as a parameter. In
greenfield/src/contracts/incidents.ts lines 33-58, remove the local helpers,
import the shared versions, and pass incidentFilterMaximum; apply the same
removal and import in greenfield/src/contracts/notifications.ts lines 36-61,
passing notificationFilterMaximum.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@greenfield/src/contracts/reports.ts`:
- Around line 176-180: Create a separate producer access contract near
reportWriteAccess that includes principalKinds: ["automation"], then update
reports.upsert to use this contract instead of reportWriteAccess. Keep the
existing reportWriteAccess contract for deletion or other session-principal
operations so report production is limited to automation principals.

In `@greenfield/src/server/database/schema/notifications.ts`:
- Around line 29-31: Update the deleteReport flow to capture linked
notifications, call unit.deleteNotifications with their IDs before
unit.deleteReport(input.id), and emit a notification deleted event for each
removed notification so per-notification deletion events are recorded instead of
relying on the foreign-key cascade.

In `@greenfield/src/server/database/validation/notifications.ts`:
- Around line 48-49: Update the notification validation schemas used by
insertIncidentNotification so nullable reportId and source fields retain their
generated nullable wrappers instead of using raw uuidV7TextSchema or
monitoringReportSourceSchema values. Extend notifications.test.ts with valid
insert fixtures covering reportId: null and source: null, while preserving
existing non-null validation behavior.

In `@greenfield/src/server/domains/monitoring/catalogErrors.ts`:
- Line 3: Update the tagged-error schema definitions using Schema.TaggedError to
replace the unsupported Schema.Literals calls with Schema.Literal("incident",
"report", ...), while preserving the existing Schema.Literal("report") value and
the declared Effect 4 API usage.

In `@greenfield/src/server/domains/monitoring/repository.ts`:
- Around line 620-644: Update the repository’s listReports composition to wrap
both report summary and detail reads in a single
dependencies.repository.withReadTransaction callback. Within that callback, use
the provided MonitoringReader to call reader.listReports and reader.findReport,
preserving the existing response behavior while ensuring both reads share one
SQLite snapshot.

In `@greenfield/src/server/domains/monitoring/routeEffects.ts`:
- Around line 21-63: Update runMonitoringEffect to execute the effect with
Effect.runPromiseExit and inspect the returned failure cause using
Exit.isFailure and Cause.squash before applying the existing domain-error
instanceof mappings to TRPCError. Preserve the current success return path and
rethrow unmapped errors.

---

Outside diff comments:
In `@greenfield/src/app/trpcRequestPolicy.ts`:
- Around line 147-162: Update the request-body limit selection in the policy
function around trpcRequestBodyMaximumBytes so monitoring, task-content, and
task-progress procedure limits are combined by selecting the maximum applicable
value rather than by last assignment. Preserve authentication and WebAuthn as
intentional restrictive overrides if that is the existing contract, and keep the
resulting limit sufficient for mixed batches containing monitoring procedures.

---

Nitpick comments:
In `@greenfield/src/app/dashboardServer.ts`:
- Around line 266-281: Extract the repeated realtime event wake callback in the
surrounding dashboard server function into a single local reference before
constructing services, then pass that reference to the agent, task, monitoring,
and monitoring catalog service factories instead of recreating the closure.

In `@greenfield/src/app/trpcRequestPolicy.test.ts`:
- Line 124: Update the assertion in the trpc request policy test to verify that
serverRequestBodyMaximumBytes equals the maximum of the task-content and
monitoring limits, rather than asserting equality with
monitoringRequestBodyMaximumBytes. Preserve the existing policy values and
reference the relevant limit symbols already used by the test.

In `@greenfield/src/contracts/incidents.ts`:
- Around line 33-58: Extract the duplicated uniqueFilterSchema and
enumFilterSchema implementations into a shared contract module with the
maximum-length value supplied as a parameter. In
greenfield/src/contracts/incidents.ts lines 33-58, remove the local helpers,
import the shared versions, and pass incidentFilterMaximum; apply the same
removal and import in greenfield/src/contracts/notifications.ts lines 36-61,
passing notificationFilterMaximum.

In `@greenfield/src/contracts/monitoring.ts`:
- Around line 20-35: Replace the character-scanning byte calculation used by
encodedJsonBytes and monitoringMutationInputFitsBudget with
Buffer.byteLength(JSON.stringify(value), "utf8"). Preserve both existing budget
comparisons and JSON serialization behavior while removing reliance on
utf8ByteLength.
- Around line 259-318: Collapse the duplicate predicates in incidentRecordSchema
and incidentSummarySchema by reusing the existing
activeIncidentTimesAreConsistent and resolvedIncidentTimesAreConsistent
functions for both full records and summaries, removing the two summary-specific
predicate functions. Preserve the existing validation checks and add
return-value JSDoc to the two retained exported predicates.

In `@greenfield/src/server/domains/monitoring/catalogRoutes.ts`:
- Around line 37-58: Extract the repeated principal-kind checks from
notificationProducerProcedure, notificationSessionWriteProcedure, and the
corresponding ingestion route into a shared principalKindProcedure helper in
trpc.ts. Have the helper accept the capability, expected PrincipalKind, and
forbidden message, while preserving each procedure’s existing capability and
message.

In `@greenfield/src/server/domains/monitoring/repository.ts`:
- Around line 331-385: Add composite indexes matching the default keyset
ordering for the three listing methods: reports and notifications on occurred_at
plus id, and incidents on last_seen_at plus id. Define them in the schema/index
configuration near the existing table indexes, without partial predicates or
leading filter columns, so unfiltered listReports, listNotifications, and
listIncidents queries can use the ordering index.

In `@greenfield/src/server/domains/monitoring/service.ts`:
- Around line 165-172: Record suppressed wakeEventPump failures instead of
silently ignoring them: extend MonitoringServiceDependencies with an optional
logger or failure callback, invoke it once from wakeAfterCommit’s catch block,
and preserve SQLite authority plus adaptive-polling recovery. Apply the same
reporting mechanism to catalogService.wakeAfterChange, preferably through a
shared helper.

In `@greenfield/src/server/domains/monitoring/testSupport/services.ts`:
- Around line 6-8: Update unexpectedMonitoringServiceCall to accept the invoked
monitoring method name and include it in the defect message, then pass each
member’s method name at its call site (such as deleteReport). Ensure
unexpected-call failures identify the specific member that was triggered.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ab2c994d-a745-492f-9bb1-fd0af6d36ae4

📥 Commits

Reviewing files that changed from the base of the PR and between 47fac91 and 2e4576d.

⛔ Files ignored due to path filters (41)
  • greenfield/docs/generated/procedures.md is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/realtime-events.md is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/agents.status.realtime.payload.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.createPrincipal.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.createPrincipal.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.disablePrincipal.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.listPrincipals.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.replaceCapabilities.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/automationSecurity.replaceCapabilities.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/events.stream.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/incidents.get.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/incidents.get.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/incidents.list.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/incidents.list.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/monitoring.incidents.realtime.payload.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/monitoring.notifications.realtime.payload.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/monitoring.reports.realtime.payload.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/monitoring.submitCompleteSnapshot.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/monitoring.submitCompleteSnapshot.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.clearRead.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.clearRead.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.delete.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.delete.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.list.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.list.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.markAllRead.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.markAllRead.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.markRead.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.markRead.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.upsert.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/notifications.upsert.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.delete.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.delete.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.get.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.get.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.list.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.list.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.upsert.input.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/reports.upsert.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/securityAudit.listEvents.output.schema.json is excluded by !**/generated/** and included by **/*
  • greenfield/docs/generated/schemas/tasks.records.realtime.payload.schema.json is excluded by !**/generated/** and included by **/*
📒 Files selected for processing (62)
  • greenfield/docs/architecture/greenfield-rewrite/application-architecture.md
  • greenfield/docs/architecture/greenfield-rewrite/progress.md
  • greenfield/migrations/20260804022252_dashboard-foundation/migration.sql
  • greenfield/migrations/20260804022252_dashboard-foundation/snapshot.json
  • greenfield/scripts/documentation/artifacts.test.ts
  • greenfield/scripts/documentation/jsonSchema.test.ts
  • greenfield/scripts/documentation/jsonSchema.ts
  • greenfield/src/app/dashboardServer.test.ts
  • greenfield/src/app/dashboardServer.ts
  • greenfield/src/app/server.ts
  • greenfield/src/app/trpcHttpHandler.ts
  • greenfield/src/app/trpcRequestPolicy.test.ts
  • greenfield/src/app/trpcRequestPolicy.ts
  • greenfield/src/contracts/agentRealtime.ts
  • greenfield/src/contracts/contractRegistry.ts
  • greenfield/src/contracts/incidents.ts
  • greenfield/src/contracts/monitoring.test.ts
  • greenfield/src/contracts/monitoring.ts
  • greenfield/src/contracts/monitoringIngestion.ts
  • greenfield/src/contracts/monitoringRealtime.ts
  • greenfield/src/contracts/notifications.ts
  • greenfield/src/contracts/realtime.ts
  • greenfield/src/contracts/registry.ts
  • greenfield/src/contracts/reports.test.ts
  • greenfield/src/contracts/reports.ts
  • greenfield/src/contracts/security.ts
  • greenfield/src/contracts/taskRealtime.ts
  • greenfield/src/server/database/schema/automationPrincipalCapabilities.ts
  • greenfield/src/server/database/schema/notifications.ts
  • greenfield/src/server/database/schema/reports.ts
  • greenfield/src/server/database/validation/notifications.test.ts
  • greenfield/src/server/database/validation/notifications.ts
  • greenfield/src/server/database/validation/reports.test.ts
  • greenfield/src/server/database/validation/reports.ts
  • greenfield/src/server/database/validation/testSupport/rows.ts
  • greenfield/src/server/domains/monitoring/catalogErrors.ts
  • greenfield/src/server/domains/monitoring/catalogRecords.ts
  • greenfield/src/server/domains/monitoring/catalogRoutes.ts
  • greenfield/src/server/domains/monitoring/catalogService.test.ts
  • greenfield/src/server/domains/monitoring/catalogService.ts
  • greenfield/src/server/domains/monitoring/ingestionRoutes.ts
  • greenfield/src/server/domains/monitoring/normalization.ts
  • greenfield/src/server/domains/monitoring/procedures.test.ts
  • greenfield/src/server/domains/monitoring/procedures.ts
  • greenfield/src/server/domains/monitoring/realtimeEvents.ts
  • greenfield/src/server/domains/monitoring/repository.ts
  • greenfield/src/server/domains/monitoring/routeEffects.ts
  • greenfield/src/server/domains/monitoring/serialization.ts
  • greenfield/src/server/domains/monitoring/service.ts
  • greenfield/src/server/domains/monitoring/serviceBoundary.test.ts
  • greenfield/src/server/domains/monitoring/snapshotLifecycle.ts
  • greenfield/src/server/domains/monitoring/testSupport/monitoringService.ts
  • greenfield/src/server/domains/monitoring/testSupport/services.ts
  • greenfield/src/server/domains/realtime/retention.ts
  • greenfield/src/server/test/support/requestContext.ts
  • greenfield/src/server/trpc/appRouter.ts
  • greenfield/src/server/trpc/context.test.ts
  • greenfield/src/server/trpc/context.ts
  • greenfield/src/server/trpc/procedureErrorPolicy.ts
  • greenfield/src/shared/databaseMigrationManifest.ts
  • greenfield/src/test/parity/fixtures/greenfield-contracts.json
  • greenfield/src/test/parity/fixtures/legacy-endpoints.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: dashboard-checks
  • GitHub Check: Analyze JavaScript and TypeScript
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-08-07T17:05:36.638Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 397
File: greenfield/src/server/domains/agents/service.test.ts:225-239
Timestamp: 2026-08-07T17:05:36.638Z
Learning: In Bun test files, write rejection assertions as `expect(promise).rejects...` without `await`. The repository's installed matcher types return `void`, and ESLint's `typescript(await-thenable)` rule rejects awaiting these matcher assertions.

Applied to files:

  • greenfield/scripts/documentation/artifacts.test.ts
  • greenfield/src/contracts/monitoring.test.ts
  • greenfield/src/contracts/reports.test.ts
  • greenfield/src/server/database/validation/notifications.test.ts
  • greenfield/src/app/trpcRequestPolicy.test.ts
  • greenfield/src/server/database/validation/reports.test.ts
  • greenfield/src/app/dashboardServer.test.ts
  • greenfield/src/server/domains/monitoring/serviceBoundary.test.ts
  • greenfield/src/server/trpc/context.test.ts
  • greenfield/src/server/domains/monitoring/catalogService.test.ts
  • greenfield/scripts/documentation/jsonSchema.test.ts
  • greenfield/src/server/domains/monitoring/procedures.test.ts
🔇 Additional comments (75)
greenfield/docs/architecture/greenfield-rewrite/application-architecture.md (1)

373-379: LGTM!

Also applies to: 392-392

greenfield/docs/architecture/greenfield-rewrite/progress.md (1)

15-15: LGTM!

Also applies to: 845-866

greenfield/scripts/documentation/artifacts.test.ts (1)

98-152: LGTM!

greenfield/scripts/documentation/jsonSchema.test.ts (1)

25-31: LGTM!

Also applies to: 47-47, 116-125, 269-326, 455-459

greenfield/src/test/parity/fixtures/greenfield-contracts.json (1)

177-228: LGTM!

greenfield/src/test/parity/fixtures/legacy-endpoints.json (1)

81-81: LGTM!

Also applies to: 94-94, 777-777, 868-868, 881-881, 1637-1676, 1832-1832

greenfield/scripts/documentation/jsonSchema.ts (1)

40-65: LGTM!

Also applies to: 97-97, 152-219, 391-399

greenfield/src/contracts/monitoring.test.ts (1)

11-60: LGTM!

greenfield/src/app/dashboardServer.test.ts (1)

9-14: LGTM!

Also applies to: 119-128, 166-238

greenfield/src/server/domains/monitoring/catalogService.test.ts (1)

27-57: LGTM!

Also applies to: 59-145, 147-282, 284-363, 365-421, 423-492, 494-548

greenfield/src/server/domains/monitoring/procedures.test.ts (2)

30-64: LGTM!

Also applies to: 66-119, 121-180, 182-207, 215-243, 245-317


208-214: 📐 Maintainability & Code Quality

No change needed for this literal.

completedAtMs: 310_001 tests maximumSnapshotFutureSkewMilliseconds (5m = 300_000ms) with nowMs: 10_000; oneDayMs is used for realtime retention, not this boundary.

			> Likely an incorrect or invalid review comment.
greenfield/src/server/domains/monitoring/serviceBoundary.test.ts (2)

9-14: LGTM!

Also applies to: 66-66, 86-86, 132-132, 150-150, 179-179


111-111: 🩺 Stability & Availability

No change needed. The monitoring service awaits wakeEventPump() inside a try/catch, so the asynchronous rejection is consumed and does not become unhandled.

greenfield/src/server/trpc/context.test.ts (1)

4-7: LGTM!

Also applies to: 36-37, 68-69, 95-96, 130-131, 171-172

greenfield/src/shared/databaseMigrationManifest.ts (1)

16-18: 🗄️ Data Integrity & Integration

Record digest values match the current migration assets.

greenfield/src/server/domains/monitoring/catalogRecords.ts (1)

28-127: LGTM!

greenfield/src/server/domains/monitoring/catalogService.ts (4)

515-538: LGTM!


695-729: LGTM!


649-669: 🗄️ Data Integrity & Integration

No change needed. The notification contract requires incidentId and incidentGeneration to be present together, and the notification persistence schema enforces the same invariant with both input-side validation and database checks.

			> Likely an incorrect or invalid review comment.

462-468: 🗄️ Data Integrity & Integration

Confirm snapshot-required invalidation semantics before using a report id here.

monitoring.notifications accepts snapshot-required for notification entities, but the visible consumer handling does not show whether clients ignore entityId for this operation or look up entityId as a notification id. If clients resolve entityId, replace this with a valid notification id or omit per-event invalidation.

greenfield/src/server/domains/monitoring/serialization.ts (1)

8-77: LGTM!

greenfield/src/server/domains/realtime/retention.ts (1)

2-2: LGTM!

greenfield/src/server/trpc/context.ts (1)

3-4: LGTM!

Also applies to: 38-39, 58-59, 89-90

greenfield/src/server/test/support/requestContext.ts (1)

10-15: LGTM!

Also applies to: 397-398, 426-428, 514-515, 541-543

greenfield/src/server/domains/monitoring/repository.ts (5)

1-26: LGTM!

Also applies to: 71-133


143-256: LGTM!


534-548: LGTM!

Also applies to: 571-582


658-659: LGTM!


419-426: 🗄️ Data Integrity & Integration

No change needed. catalogService.deleteReport emits a snapshot-required realtime event for linked notifications before the report deletes, so subscribers do not keep stale notification entries.

greenfield/src/server/domains/monitoring/normalization.ts (3)

9-9: LGTM!

Also applies to: 70-70


189-191: LGTM!


114-114: 🗄️ Data Integrity & Integration

No change needed.

serializeCanonicalMonitoringJson is already the shared implementation used across monitoring serialization and produces the same ordering and key-sorting behavior as the former local canonicalJson.

greenfield/src/server/domains/monitoring/service.ts (3)

24-24: LGTM!

Also applies to: 68-77


136-147: LGTM!


228-229: LGTM!

Also applies to: 309-309

greenfield/src/server/domains/monitoring/realtimeEvents.ts (2)

63-82: LGTM!


93-93: LGTM!

Also applies to: 105-107

greenfield/src/server/domains/monitoring/snapshotLifecycle.ts (1)

124-124: LGTM!

greenfield/src/app/server.ts (2)

6-7: LGTM!

Also applies to: 143-144


186-187: LGTM!

greenfield/src/app/trpcHttpHandler.ts (2)

4-5: LGTM!

Also applies to: 42-43


220-221: LGTM!

greenfield/src/app/dashboardServer.ts (2)

8-10: LGTM!

Also applies to: 89-90


295-296: LGTM!

greenfield/src/server/domains/monitoring/ingestionRoutes.ts (1)

10-30: LGTM!

greenfield/src/server/domains/monitoring/procedures.ts (1)

5-17: LGTM!

greenfield/src/server/trpc/appRouter.ts (1)

2-11: LGTM!

Also applies to: 44-47, 60-63

greenfield/src/server/trpc/procedureErrorPolicy.ts (1)

209-256: LGTM!

greenfield/src/app/trpcRequestPolicy.ts (1)

40-52: LGTM!

Also applies to: 196-242

greenfield/src/server/domains/monitoring/testSupport/monitoringService.ts (1)

92-92: 🩺 Stability & Availability

Production pump call sites already handle async pumps.

greenfield/src/server/domains/monitoring/catalogRoutes.ts (1)

33-35: 🔒 Security & Privacy

No change needed for incident access. Incident lifecycle read contracts and realtime routing also use reports:read, and the capability catalog does not define a separate incident capability.

greenfield/migrations/20260804022252_dashboard-foundation/migration.sql (1)

142-142: LGTM!

Also applies to: 228-233, 263-267, 598-598

greenfield/migrations/20260804022252_dashboard-foundation/snapshot.json (1)

4-7: LGTM!

The snapshot entries match migration.sql for the new columns, the cascading foreign key, the index, and both check constraints.

Also applies to: 1343-1352, 1363-1372, 1533-1552, 2563-2577, 3529-3542, 4284-4284, 4475-4480

greenfield/src/contracts/incidents.ts (1)

60-128: LGTM!

The newest-first ordering predicate matches the repository ordering desc(lastSeenAt), desc(id), and the cursor consistency check binds nextCursor to the final row.

Also applies to: 130-178

greenfield/src/contracts/monitoring.ts (2)

38-72: LGTM!

monitoringLinkPathSchema requires an absolute path, rejects protocol-relative // prefixes, backslashes, and whitespace. The notification and incident consistency checks match the notifications_incident_pair_check, notifications_read_order_check, and incident time constraints in the migration.

Also applies to: 94-115, 126-126, 135-173, 180-258, 320-371, 386-397


373-384: 🎯 Functional Correctness

No action needed.

There is no configured lint rule requiring sorted object keys, so this is not an actionable lint failure.

			> Likely an incorrect or invalid review comment.
greenfield/src/contracts/monitoringRealtime.ts (1)

6-7: LGTM!

The operation vocabularies stay inside the realtime_events_operation_check constraint, and each snapshotProcedure matches a declared query contract.

Also applies to: 20-26, 39-39, 75-102

greenfield/src/contracts/notifications.ts (1)

63-155: LGTM!

The newest-first predicate and cursor consistency check match the repository ordering desc(occurredAt), desc(id). The producer and session write access split keeps automation out of the read-acknowledgement procedures.

Also applies to: 157-253, 255-354

greenfield/src/server/database/schema/notifications.ts (1)

12-12: LGTM!

The source column and the notifications_report_id_idx index match migration.sql and snapshot.json.

Also applies to: 32-35, 62-62

greenfield/src/server/database/schema/reports.ts (1)

15-26: LGTM!

The column enum, the default, and the reports_status_check constraint match migration.sql and snapshot.json.

greenfield/src/server/database/validation/notifications.test.ts (1)

63-64: LGTM!

The 201-character source case sits exactly one character above the 200-character bound in monitoringReportSourceSchema.

greenfield/src/server/database/validation/reports.test.ts (1)

17-18: LGTM!

The fixture covers summary: null, and the 2001-character case sits one character above the 2000-character bound.

Also applies to: 34-35

greenfield/src/server/database/validation/reports.ts (1)

9-10: LGTM!

The status refinement matches the reports_status_check vocabulary, and reports.test.ts proves the nullable summary refinement still accepts null.

Also applies to: 28-29

greenfield/src/contracts/agentRealtime.ts (1)

5-6: LGTM!

Also applies to: 33-42

greenfield/src/contracts/monitoringIngestion.ts (1)

1-39: LGTM!

greenfield/src/contracts/realtime.ts (1)

12-15: LGTM!

greenfield/src/contracts/registry.ts (1)

55-55: LGTM!

greenfield/src/contracts/reports.test.ts (1)

1-54: LGTM!

greenfield/src/contracts/reports.ts (1)

1-169: LGTM!

Also applies to: 181-269

greenfield/src/contracts/security.ts (1)

97-101: LGTM!

greenfield/src/contracts/taskRealtime.ts (1)

4-5: LGTM!

Also applies to: 33-42

greenfield/src/server/database/schema/automationPrincipalCapabilities.ts (1)

21-21: LGTM!

greenfield/src/server/database/validation/testSupport/rows.ts (1)

52-54: LGTM!

greenfield/src/contracts/contractRegistry.ts (1)

2-20: LGTM!

Also applies to: 30-33, 75-79

Comment thread greenfield/src/contracts/reports.ts
Comment thread greenfield/src/server/database/schema/notifications.ts
Comment thread greenfield/src/server/database/validation/notifications.ts
Comment thread greenfield/src/server/domains/monitoring/catalogErrors.ts
Comment thread greenfield/src/server/domains/monitoring/repository.ts
Comment thread greenfield/src/server/domains/monitoring/routeEffects.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2363318c9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread greenfield/src/server/domains/monitoring/realtimeEvents.ts
Comment thread greenfield/src/server/domains/monitoring/catalogService.ts Outdated
@rajohan
rajohan merged commit cf34550 into main Aug 24, 2026
9 checks passed
@rajohan
rajohan deleted the mira/greenfield-rewrite-phase-three-monitoring branch August 24, 2026 03:59
rajohan added a commit that referenced this pull request Aug 24, 2026
## Summary

- add an authenticated `/reports` reader with bounded summary pages,
status/free-form kind/source filters, exact large-document loading, safe
Markdown rendering, and confirmed deletion
- add the net-new authenticated `/incidents` reader as a hidden
deep-link target with lifecycle/severity filters, a selectable
virtualized TanStack Table, and exact detail loading
- wire lazy report/incident browser contracts, precise Query cache
roots, durable realtime invalidation/resync fallback, reviewed frontend
parity, and Phase 3 progress evidence

## Behavior and regression coverage

- report and incident lists use server-owned keyset pagination with
cancellation signals and identity deduplication across overlapping pages
- validated UUIDv7 search state loads exact detail independently of
first-page or list availability; malformed deep links issue no detail
request
- report bodies are fetched only after explicit selection or a valid
deep link, so list calls never carry the potentially large Markdown
document
- the existing shared Markdown renderer keeps raw HTML inert
- successful report deletion removes the record from every cached
filtered page before refresh; fixed `NOT_FOUND` and bounded
`PRECONDITION_FAILED` outcomes do not expose server text, and a failed
post-delete refresh cannot resurrect the row
- `/incidents` is intentionally registered and titled inside the
authenticated shell while absent from main navigation; Reports links to
it, and monitoring notification deep links can target the persistent
incident generation rather than an arbitrary report observation
- `monitoring.reports` and `monitoring.incidents` use coalesced
invalidation, terminal-resync recovery, and a 30-second fallback refresh
- the incident table uses the shared virtualizer at 50 rows and gives
recurring generations distinct accessible button names
- cached list/detail data remains usable during transient refetch
failures

## Verification

- [x] Repository lint: `cd greenfield && bun run lint`
- [x] Repository formatting: `cd greenfield && bun run format:check`
- [ ] Frontend build: `bun run build:frontend` — legacy frontend is
outside this isolated greenfield slice; `cd greenfield && bun run
build:browser` and the release build are green
- [ ] Frontend tests/coverage: `bun run test:frontend:coverage` — legacy
frontend is outside this slice; the complete greenfield browser and
coverage suites ran below
- [ ] Backend build: `bun run build:backend` — no backend behavior
changes; the greenfield process artifacts were built by the release gate
- [ ] Backend tests/coverage: `bun run test:backend:coverage` — legacy
backend is outside this slice; the complete greenfield Bun suite ran
below
- [x] Focused regression tests: monitoring
route/query/realtime/virtualization suite (17/17)
- [ ] Manual UI/API smoke check, if relevant — inactive pre-cutover
slice; deterministic authenticated route tests exercise the visible
workflows

Additional greenfield gates:

- `bun run typecheck`
- `bun run check:boundaries`
- `bun run docs:check`
- `bun run db:check`
- `bun scripts/runTestSuite.ts src/test/parity/parityInventory.test.ts`
— 4/4
- `bun run test:browser` — 84/84, 344 assertions
- `bun run test:coverage` — 1,492 tests, 0 failures, 92.31% line
coverage (37,971/41,134)
- `bun run build:release` — clean-source release for
`a8b4b414cc561cb67f0febb6120e0381468e829e`

## Risk checklist

- [x] No secrets, tokens, `.env` files, database dumps, or runtime state
committed
- [x] Auth, Gateway, terminal, file, Docker, or settings changes were
reviewed carefully
- [x] New/changed API routes enforce the expected authentication and
validation
- [x] Migrations or data-shape changes include a rollout/rollback note,
if relevant — none in this slice
- [x] Runtime/reconnect behavior preserves ordering, idempotency, and
recovery
- [x] UI changes include screenshots or a short description of visible
changes — `/reports` gains a filterable list/detail workspace;
`/incidents` gains a hidden filterable table/detail workspace

## Deployment / operations

- [x] No deploy/restart needed
- [ ] Deploy/restart needed after merge: none; the greenfield stack
remains inactive until supervised cutover
- [ ] Config/secrets changes needed: none
- [x] Rollback path verified: revert the single browser commit before
cutover and rebuild the unpublished greenfield release

## Notes for reviewers

- focus on large-detail isolation, deep-link/list independence,
post-delete cache behavior, accessible virtualization, and terminal
realtime recovery
- `/incidents` is intentionally hidden from main navigation but
reachable from Reports and durable monitoring notification links
- stack base: #398 at locked head
`488425875a815b1806f77a0ba75342937c2d8e13`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Backend API, server routes, services, and integrations area: database Database dashboard, Postgres, PgBouncer, or query views area: openclaw OpenClaw gateway/API integration and runtime behavior area: ops Operational actions, deploys, services, backups, and health status: needs-review Ready for human or agent review type: feature Adds user-visible functionality type: tests Adds or updates automated tests/coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants