diff --git a/e2e/ui/deliveries.spec.ts b/e2e/ui/deliveries.spec.ts index 282060ce..59c356fc 100644 --- a/e2e/ui/deliveries.spec.ts +++ b/e2e/ui/deliveries.spec.ts @@ -31,7 +31,10 @@ test("deliveries explain outcomes without exposing unauthorized content or raw m ).toHaveAttribute("href", "/admin/articles/9007199254740993"); await expect( news.getByRole("link", { name: "Service diagnostics", exact: true }), - ).toHaveAttribute("href", "/admin/devops/installation"); + ).toHaveAttribute( + "href", + "/admin/devops/cms-errors?q=4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2", + ); const failed = cards.nth(1); await expect( failed.getByRole("heading", { name: "9 catalog offers", exact: true }), diff --git a/e2e/ui/fixtures/deliveries-harness.tsx b/e2e/ui/fixtures/deliveries-harness.tsx index 75ef7e05..231b4ad9 100644 --- a/e2e/ui/fixtures/deliveries-harness.tsx +++ b/e2e/ui/fixtures/deliveries-harness.tsx @@ -19,6 +19,7 @@ const records: DeliveryCardItem[] = [ { ...shared, id: "delivery-authorized-news-1234567890", + errorId: "4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2", operationId: "operation-authorized-news-1234567890", context: deliveryContext( "news.update", diff --git a/src/app/admin/devops/cms-errors/page.test.ts b/src/app/admin/devops/cms-errors/page.test.ts index a10f3091..b01ea7b1 100644 --- a/src/app/admin/devops/cms-errors/page.test.ts +++ b/src/app/admin/devops/cms-errors/page.test.ts @@ -1,3 +1,4 @@ +import { renderToStaticMarkup } from "react-dom/server"; import { expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ read: vi.fn(), access: vi.fn() })); @@ -21,6 +22,14 @@ vi.mock("@/lib/error-monitor", () => ({ })); vi.mock("@/components/link", () => ({ default: () => null })); +vi.mock("next-intl/server", () => ({ + getTranslations: async () => (key: string) => key, +})); +vi.mock("./actions", () => ({ + assignCmsError: vi.fn(), + resolveCmsError: vi.fn(), +})); + import Page from "./page"; it("rejects access before reading diagnostic data", async () => { @@ -30,3 +39,39 @@ it("rejects access before reading diagnostic data", async () => { ); expect(mocks.read).not.toHaveBeenCalled(); }); + +it("shows the referenced occurrence while retaining the current group resolution", async () => { + mocks.access.mockImplementation( + (_permissions, permission) => permission === "devops.view", + ); + const old = { + id: "old-reference", + fingerprint: "same-group", + at: "2026-09-10T10:00:00Z", + release: "old-release", + source: "server", + event: "Delivery failed", + message: "Failure", + stack: "OLDER_ERROR_STACK", + context: { deliveryId: "older-delivery" }, + resolved: false, + }; + const newer = { + ...old, + id: "new-reference", + at: "2026-09-11T10:00:00Z", + release: "new-release", + stack: "NEWER_ERROR_STACK", + context: { deliveryId: "newer-delivery" }, + resolved: true, + }; + mocks.read.mockResolvedValue({ records: [old, newer], truncated: false }); + const html = renderToStaticMarkup( + await Page({ searchParams: Promise.resolve({ q: "old-reference" }) }), + ); + expect(html).toContain("OLDER_ERROR_STACK"); + expect(html).toContain("older-delivery"); + expect(html).not.toContain("NEWER_ERROR_STACK"); + expect(html).not.toContain("newer-delivery"); + expect(html).toContain("resolved"); +}); diff --git a/src/app/admin/devops/cms-errors/page.tsx b/src/app/admin/devops/cms-errors/page.tsx index 57091226..491d55ca 100644 --- a/src/app/admin/devops/cms-errors/page.tsx +++ b/src/app/admin/devops/cms-errors/page.tsx @@ -159,9 +159,14 @@ export default async function CmsErrorsPage({ .map(([fingerprint, events]) => { const metrics = summarizeErrorGroup(events); - const r = latestByGroup.get(fingerprint); - - if (!r) return null; + const groupLatest = latestByGroup.get(fingerprint); + if (!groupLatest || !metrics.latest) return null; + // Details follow the selected occurrence; triage belongs to the whole group. + const r = { + ...metrics.latest, + resolved: groupLatest.resolved, + assignment: groupLatest.assignment, + }; return (
diff --git a/src/app/admin/devops/deliveries/page.tsx b/src/app/admin/devops/deliveries/page.tsx index 97addde7..16843acb 100644 --- a/src/app/admin/devops/deliveries/page.tsx +++ b/src/app/admin/devops/deliveries/page.tsx @@ -3,10 +3,18 @@ import { getTranslations } from "next-intl/server"; import Link from "@/components/link"; import { DeliveryCard } from "@/features/operations/delivery-card"; import { deliveryContext } from "@/features/operations/delivery-context"; +import { + deliveryDiagnosticId, + isDeliveryReference, +} from "@/features/operations/delivery-diagnostics"; import { DeliveryRetry } from "@/features/operations/retry-button"; import { listEffects } from "@/features/operations/server"; import { canAccess, getAdminContext, PERMS } from "@/lib/permissions"; -export default async function DeliveryPage() { +export default async function DeliveryPage({ + searchParams, +}: { + searchParams: Promise>; +}) { const { session, permissions } = await getAdminContext(); if (!canAccess(permissions, PERMS.DEVOPS_VIEW, session.user.rank)) redirect("/admin"); @@ -16,9 +24,17 @@ export default async function DeliveryPage() { catalog: canAccess(permissions, PERMS.CATALOG_VIEW, session.user.rank), }; const t = await getTranslations("pages.admin.deliveries"); + let selectedId: string | undefined; let items: Awaited> | null = null; try { - items = await listEffects(); + const params = await searchParams; + selectedId = + typeof params.id === "string" + ? params.id + : params.id === undefined + ? undefined + : "invalid"; + items = await listEffects(selectedId); } catch { /* Preserve the distinction between unavailable and empty history. */ } @@ -29,7 +45,14 @@ export default async function DeliveryPage() {

{t("title")}

{t("scope")}

- + {t("refresh")} {canAccess(permissions, PERMS.ASSETS_IMPORT, session.user.rank) && ( @@ -59,6 +82,7 @@ export default async function DeliveryPage() { createdAt: timestamp(item.createdAt), availableAt: timestamp(item.availableAt), hasError: !!item.lastError, + errorId: deliveryDiagnosticId(item.lastError), context: deliveryContext(item.kind, item.resultJson, access), }} retry={ diff --git a/src/features/operations/README.md b/src/features/operations/README.md index 8e1c53f4..d28d7871 100644 --- a/src/features/operations/README.md +++ b/src/features/operations/README.md @@ -14,3 +14,16 @@ The delivery screen separates saved content from its subsequent background updat Content titles and links require the corresponding NEWS_VIEW or CATALOG_VIEW grant in addition to DEVOPS_VIEW. Raw result JSON and internal error text are never forwarded to the card. Each record shows its recorded time, staff/system actor, attempts, and scheduled retry time when applicable. Failures point to service diagnostics; exhausted attempts expose the existing permission-protected retry without repeating the content mutation. Delivery success does not imply Git publication or client refresh acknowledgement. Browser fixtures verify the real cards on desktop/mobile, including long titles, denied metadata, exact links, unknown states and collapsed technical references. The fixture retry slot does not execute a backend action; backend authorization and state changes have separate tests. + + +Delivery failures retain the original exception in the redacted CMS diagnostic store, +alongside a delivery UUID and the persisted operation UUID. Request correlation is +kept separately. Only the diagnostic UUID is stored in the outbox error field. +The delivery card opens the matching error record; that record links back to the +exact delivery, including records older than the latest 100 entries. Refresh keeps +this selection. A historical diagnostic lookup shows that occurrence's context and +stack while retaining the current group's assignment and resolution status. + +These links require existing DevOps access. Expired diagnostic records may no longer +be available after the error store's retention window; the delivery state remains +in SQL. Retry continues to require edit permission and an exhausted delivery. diff --git a/src/features/operations/delivery-card.tsx b/src/features/operations/delivery-card.tsx index ae8d1b22..0f0a9571 100644 --- a/src/features/operations/delivery-card.tsx +++ b/src/features/operations/delivery-card.tsx @@ -2,6 +2,7 @@ import { useFormatter, useTranslations } from "next-intl"; import type { ReactNode } from "react"; import Link from "@/components/link"; import { deliveryTopic } from "./delivery-context"; +import { deliveryDiagnosticLink } from "./delivery-diagnostics"; export interface DeliveryCardItem { id: string; @@ -13,6 +14,7 @@ export interface DeliveryCardItem { createdAt: string; availableAt: string; hasError: boolean; + errorId?: string | null; context: { title: string | null; href: string | null; @@ -87,7 +89,10 @@ export function DeliveryCard({ )} {item.hasError && state !== "done" && ( - + {t("diagnostics")} )} diff --git a/src/features/operations/delivery-diagnostics.test.ts b/src/features/operations/delivery-diagnostics.test.ts new file mode 100644 index 00000000..0f260e20 --- /dev/null +++ b/src/features/operations/delivery-diagnostics.test.ts @@ -0,0 +1,22 @@ +import { expect, it } from "vitest"; +import { + deliveryDiagnosticId, + deliveryDiagnosticLink, +} from "./delivery-diagnostics"; + +it("uses only an exact server-issued reference for a diagnostic link", () => { + const id = "4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2"; + expect(deliveryDiagnosticId(`Diagnostic reference: ${id}`)).toBe(id); + expect(deliveryDiagnosticLink(id)).toBe(`/admin/devops/cms-errors?q=${id}`); + for (const input of [ + null, + "", + "private SQL", + "Diagnostic reference: ../settings", + `Diagnostic reference: ${id}?secret=value`, + `Diagnostic reference: ${id}\n`, + ]) { + expect(deliveryDiagnosticId(input)).toBeNull(); + expect(deliveryDiagnosticLink(input)).toBe("/admin/devops/installation"); + } +}); diff --git a/src/features/operations/delivery-diagnostics.ts b/src/features/operations/delivery-diagnostics.ts new file mode 100644 index 00000000..ddf0f933 --- /dev/null +++ b/src/features/operations/delivery-diagnostics.ts @@ -0,0 +1,19 @@ +/** Only server-issued UUID references can become diagnostic/navigation targets. */ +export function isDeliveryReference(value: unknown): value is string { + return ( + typeof value === "string" && + /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i.test( + value, + ) + ); +} +export function deliveryDiagnosticId(value: string | null): string | null { + const prefix = "Diagnostic reference: "; + const id = value?.startsWith(prefix) ? value.slice(prefix.length) : null; + return isDeliveryReference(id) ? id : null; +} +export function deliveryDiagnosticLink(errorId: unknown): string { + return isDeliveryReference(errorId) + ? `/admin/devops/cms-errors?q=${encodeURIComponent(errorId)}` + : "/admin/devops/installation"; +} diff --git a/src/features/operations/delivery-errors.test.ts b/src/features/operations/delivery-errors.test.ts new file mode 100644 index 00000000..ed77225c --- /dev/null +++ b/src/features/operations/delivery-errors.test.ts @@ -0,0 +1,53 @@ +import { MySqlDialect } from "drizzle-orm/mysql-core"; +import { beforeEach, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ execute: vi.fn(), error: vi.fn() })); +vi.mock("@/lib/db", () => ({ db: { execute: mocks.execute } })); +vi.mock("@/lib/logger", () => ({ logger: { error: mocks.error } })); + +import { effectRepository, listEffects } from "./server"; + +const id = "840f4023-04cd-4d7b-92c5-047aa620029c"; +const operationId = "1dd40f61-b710-48c8-8534-c3a3f726780a"; +const errorId = "4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2"; +beforeEach(() => { + vi.resetAllMocks(); + mocks.error.mockReturnValue(errorId); + mocks.execute.mockResolvedValue([[]]); +}); +it("records the original failure with durable identity and stores only a safe diagnostic reference", async () => { + const error = new Error("private transport details"); + await effectRepository.fail( + { + id, + operationId, + token: "private-lease", + topic: "news.refresh", + attempts: 2, + }, + error, + ); + expect(mocks.error).toHaveBeenCalledWith( + "Operation delivery failed", + expect.objectContaining({ + module: "operations", + deliveryId: id, + durableOperationId: operationId, + error, + }), + ); + const query = new MySqlDialect().sqlToQuery(mocks.execute.mock.calls[0][0]); + expect(query.params).toContain(`Diagnostic reference: ${errorId}`); + expect(query.params).not.toContain(error.message); + expect(query.params).toContain("private-lease"); +}); +it("looks up an older delivery by exact ID independently of the latest-page limit", async () => { + await listEffects(id); + const query = new MySqlDialect().sqlToQuery(mocks.execute.mock.calls[0][0]); + expect(query.sql).toContain("WHERE e.id=?"); + expect(query.params).toContain(id); +}); +it("does not query the database for a malformed delivery reference", async () => { + await expect(listEffects("../settings")).resolves.toEqual([]); + expect(mocks.execute).not.toHaveBeenCalled(); +}); diff --git a/src/features/operations/dispatcher.test.ts b/src/features/operations/dispatcher.test.ts index 7e02737b..aeffb1d6 100644 --- a/src/features/operations/dispatcher.test.ts +++ b/src/features/operations/dispatcher.test.ts @@ -31,8 +31,9 @@ it("persists failures and continues to other pending work", async () => { complete: vi.fn(), fail: vi.fn(), }; - await dispatchEffects(repo, vi.fn().mockRejectedValue(Error("private"))); - expect(repo.fail).toHaveBeenCalledWith(effect); + const failure = Error("private"); + await dispatchEffects(repo, vi.fn().mockRejectedValue(failure)); + expect(repo.fail).toHaveBeenCalledWith(effect, failure); expect(repo.complete).not.toHaveBeenCalled(); }); it("bounds work per tick instead of draining indefinitely", async () => { diff --git a/src/features/operations/dispatcher.ts b/src/features/operations/dispatcher.ts index a6239c04..8174c26d 100644 --- a/src/features/operations/dispatcher.ts +++ b/src/features/operations/dispatcher.ts @@ -2,7 +2,7 @@ import type { EffectClaim } from "./model"; export interface EffectRepository { claim(): Promise; complete(claim: EffectClaim): Promise; - fail(claim: EffectClaim): Promise; + fail(claim: EffectClaim, error: unknown): Promise; } /** At-least-once delivery: handlers must tolerate replay after an expired claim. */ export async function dispatchEffects( @@ -14,8 +14,8 @@ export async function dispatchEffects( if (!claim) return; try { await deliver(claim); - } catch { - await repo.fail(claim); + } catch (error) { + await repo.fail(claim, error); continue; } await repo.complete(claim); diff --git a/src/features/operations/model.ts b/src/features/operations/model.ts index 21700773..7524cc70 100644 --- a/src/features/operations/model.ts +++ b/src/features/operations/model.ts @@ -59,6 +59,7 @@ export type EffectTopic = | "news.refresh"; export interface EffectClaim { id: string; + operationId?: string; token: string; topic: EffectTopic; attempts: number; diff --git a/src/features/operations/server.ts b/src/features/operations/server.ts index f39b9d17..5d2ae0fb 100644 --- a/src/features/operations/server.ts +++ b/src/features/operations/server.ts @@ -2,6 +2,8 @@ import "server-only"; import { randomUUID } from "node:crypto"; import { sql } from "drizzle-orm"; import { db } from "@/lib/db"; +import { logger } from "@/lib/logger"; +import { isDeliveryReference } from "./delivery-diagnostics"; import { type EffectClaim, type EffectTopic, @@ -60,11 +62,12 @@ export const effectRepository = { async claim(): Promise { return db.transaction(async (tx) => { const [rows] = await tx.execute( - sql`SELECT id,topic,attempts FROM cms_outbox WHERE (status='pending' AND available_at<=UTC_TIMESTAMP(3)) OR (status='running' AND lease_until @@ -82,15 +85,28 @@ export const effectRepository = { sql`UPDATE cms_outbox SET status='done',lease_token=NULL,lease_until=NULL,last_error=NULL WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`, ); }, - async fail(claim: EffectClaim) { + async fail(claim: EffectClaim, error: unknown) { + const errorId = logger.error("Operation delivery failed", { + module: "operations", + deliveryId: claim.id, + durableOperationId: claim.operationId, + topic: claim.topic, + attempt: claim.attempts, + error, + }); + const reference = isDeliveryReference(errorId) + ? `Diagnostic reference: ${errorId}` + : "Delivery failed; service diagnostics unavailable."; await db.execute( - sql`UPDATE cms_outbox SET status=${claim.attempts >= 8 ? "failed" : "pending"},available_at=DATE_ADD(UTC_TIMESTAMP(3),INTERVAL ${retryDelay(claim.attempts)} SECOND),lease_token=NULL,lease_until=NULL,last_error='Delivery failed; inspect the linked operation and service diagnostics.' WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`, + sql`UPDATE cms_outbox SET status=${claim.attempts >= 8 ? "failed" : "pending"},available_at=DATE_ADD(UTC_TIMESTAMP(3),INTERVAL ${retryDelay(claim.attempts)} SECOND),lease_token=NULL,lease_until=NULL,last_error=${reference} WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`, ); }, }; -export async function listEffects() { +export async function listEffects(id?: string) { + if (id !== undefined && !isDeliveryReference(id)) return []; + const filter = id === undefined ? sql`` : sql`WHERE e.id=${id}`; const [rows] = await db.execute( - sql`SELECT e.id,e.operation_id AS operationId,e.topic,e.status,e.attempts,e.last_error AS lastError,e.created_at AS createdAt,e.available_at AS availableAt,o.actor_id AS actorId,o.kind,o.result_json AS resultJson FROM cms_outbox e JOIN cms_operations o ON o.id=e.operation_id ORDER BY e.created_at DESC,e.id DESC LIMIT 100`, + sql`SELECT e.id,e.operation_id AS operationId,e.topic,e.status,e.attempts,e.last_error AS lastError,e.created_at AS createdAt,e.available_at AS availableAt,o.actor_id AS actorId,o.kind,o.result_json AS resultJson FROM cms_outbox e JOIN cms_operations o ON o.id=e.operation_id ${filter} ORDER BY e.created_at DESC,e.id DESC LIMIT 100`, ); return rows as unknown as Array<{ id: string; diff --git a/src/features/operations/worker.test.ts b/src/features/operations/worker.test.ts index dad459bb..59be7236 100644 --- a/src/features/operations/worker.test.ts +++ b/src/features/operations/worker.test.ts @@ -49,8 +49,9 @@ it("invalidates the shared news cache before acknowledging delivery", async () = expect(mocks.fail).not.toHaveBeenCalled(); }); it("keeps a failed news invalidation retryable", async () => { - mocks.news.mockRejectedValue(new Error("Redis unavailable")); + const error = new Error("Redis unavailable"); + mocks.news.mockRejectedValue(error); await drainOperationEffects(); - expect(mocks.fail).toHaveBeenCalledWith(effect); + expect(mocks.fail).toHaveBeenCalledWith(effect, error); expect(mocks.complete).not.toHaveBeenCalled(); }); diff --git a/src/lib/error-groups.test.ts b/src/lib/error-groups.test.ts index e78c53e6..0ee898a1 100644 --- a/src/lib/error-groups.test.ts +++ b/src/lib/error-groups.test.ts @@ -75,3 +75,21 @@ it("links server operation IDs to the audit search without trusting browser IDs" ]).auditLink, ).toBeNull(); }); + +it("links a server delivery error to its exact persisted delivery only", () => { + const id = "840f4023-04cd-4d7b-92c5-047aa620029c"; + const record = createErrorRecord( + "server", + "Delivery failed", + "private failure", + { module: "operations", deliveryId: id }, + ); + expect(errorOperationLink(record)).toBe(`/admin/devops/deliveries?id=${id}`); + expect(errorOperationLink({ ...record, source: "browser" })).toBeNull(); + expect( + errorOperationLink({ + ...record, + context: { module: "operations", deliveryId: "../settings" }, + }), + ).toBeNull(); +}); diff --git a/src/lib/error-groups.ts b/src/lib/error-groups.ts index e41f8f8a..6818cca5 100644 --- a/src/lib/error-groups.ts +++ b/src/lib/error-groups.ts @@ -1,8 +1,14 @@ +import { isDeliveryReference } from "@/features/operations/delivery-diagnostics"; import type { CmsErrorRecord } from "./error-monitor"; export function errorOperationLink(record: CmsErrorRecord): string | null { const context = record.context; if (record.source === "server") { + if ( + context.module === "operations" && + isDeliveryReference(context.deliveryId) + ) + return `/admin/devops/deliveries?id=${context.deliveryId}`; if (context.module === "news") { const id = String(context.articleId ?? ""); return /^[1-9]\d{0,18}$/.test(id)