refactor(web): let hosts handle request logging

AGENTS.md

@@ -194,7 +194,7 @@ Reviews send changes back for these more often than for anything else. Get them
 
 - **Build only for consumers in scope.** Do not add a package, interface, adapter, option, or second supported path for a consumer the change does not ship, such as a second host, a framework that "can't" import CSS, or a caller nobody has asked for. Extract shared code when the second real consumer arrives. When two ways of doing something would both be supported, pick one and enforce it.
 - **Every file, dependency, and call has a job you can name.** Skip work the behavior does not need: an upfront validation pass when each item is validated before use, an RPC whose result does not reach the output, a config file that restates defaults. If nothing breaks when you delete it, delete it.
-- **Use ecosystem types directly.** Library code takes `*zap.SugaredLogger` and tally in Go, and a pino `Logger` and OpenTelemetry meter and tracer in web code, with a silent or no-op default. Do not define a homegrown `Logger` interface. Wrap a dependency only where the repository actually swaps implementations, which is what an extension contract is for.
+- **Use ecosystem types directly.** Library code takes `*zap.SugaredLogger` and tally in Go, and OpenTelemetry meter and tracer in web code, with no-op defaults. Web libraries emit logs through a synchronous, request-scoped callback; the host chooses the logger and its request context. Pino belongs in the reference hosts. Do not define a homegrown `Logger` interface. Wrap a dependency only where the repository actually swaps implementations, which is what an extension contract is for.
 - **The library owns behavior; the host owns integration.** `platform/` and domain library code are framework-neutral and ship the complete behavior. In web code that includes layout, styling, pages, and API calls. A host supplies the server, auth, logging, metrics, transport, and configuration, and stays thin. Framework adapters (Next.js and the like) live in the host that uses them. Library code depends on the structural shape of a client, not on generated bindings or a transport. Only the host chooses implementations.
 - **Enforce boundaries; do not just document them.** Use Bazel `visibility` and tests that fail on forbidden imports. Generated bindings shared by in-repo consumers (such as a host and its e2e test) stay with the contract and are private to those consumers. Consumers outside the repository generate their own from the `.proto` sources.
 - **Mirror the repository layout in every language.** Go code uses the root tree. A workspace for another language repeats that layout inside itself: `web/platform`, `web/service`, `web/test/e2e`, and `web/tool`, not the repo-level `test/` or `tool/`. Do not substitute a framework's default layout (`web/e2e`), and do not call one host's wiring `platform`.

doc/rfc/web-ui.md

@@ -72,7 +72,7 @@ Summary is the default request view, and `?view=history` makes History shareable
 
 ## Observability and identity
 
-Logging, metrics, and tracing are injected the way the Go services take zap and tally. The module accepts a pino logger, an OpenTelemetry meter, and an OpenTelemetry tracer, and default to a silent logger and the OpenTelemetry API's no-op providers. Every gateway read is timed and traced, and a failure is logged once under pino's `err` key with the request ID. The module times page loads too. The reference host logs with pino and exposes Prometheus metrics, from startup, only when a port is configured. It registers no tracer provider, so its spans are no-ops; a host that registers OpenTelemetry tracing can also add a gateway interceptor that propagates trace context. A host with another metrics client adapts it to the meter interface; if such adapters prove large, a smaller tally-shaped interface may replace it.
+Metrics and tracing use injected OpenTelemetry meters and tracers, defaulting to the API's no-op providers. Logging uses a synchronous `onLog` callback passed with each page request: severity, message, and structured metadata, including the request ID and the original error under `error`. The host binds the callback to its request-scoped logger; the libraries have no logging-library dependency. Omitting the callback is silent, and callback exceptions cannot change page outcomes. Every gateway read and page load is timed and traced. The reference hosts write Pino JSON logs, serializing errors under `err`, and SubmitQueue exposes Prometheus metrics only when a port is configured. The reference host registers no tracer provider, so its spans are no-ops; a host that registers OpenTelemetry tracing can also add a gateway interceptor that propagates trace context.
 
 Identity has no shared library to depend on, so the host authenticates and passes an opaque principal. The module never sees credentials.
 

web/platform/logging.test.ts

@@ -0,0 +1,27 @@
+import { describe, expect, it, vi } from "vitest";
+import { createLogCallback } from "./logging.js";
+
+describe("log delivery", () => {
+  it("preserves errors and request context without mutating caller metadata", () => {
+    const error = new Error("upstream detail");
+    const metadata = Object.freeze({ error, queue: "repo/main", requestId: "other" });
+    const onLog = vi.fn();
+    const log = createLogCallback(onLog, { requestId: "request-1" });
+
+    log("warn", "Backend call failed", metadata);
+
+    expect(onLog).toHaveBeenCalledWith("warn", "Backend call failed", {
+      error,
+      queue: "repo/main",
+      requestId: "request-1",
+    });
+    expect(metadata.requestId).toBe("other");
+  });
+
+  it("isolates a failing logging sink", () => {
+    const log = createLogCallback(() => {
+      throw new Error("logging unavailable");
+    });
+    expect(() => log("error", "Backend call failed", {})).not.toThrow();
+  });
+});

web/platform/logging.ts

@@ -0,0 +1,23 @@
+export type LogLevel = "info" | "warn" | "error";
+export type LogMetadata = Readonly<Record<string, unknown>>;
+
+/** Synchronous host callback; metadata may contain an unmodified Error under `error`. */
+export type LogCallback = (
+  level: LogLevel,
+  message: string,
+  metadata: LogMetadata,
+) => void;
+
+/** Absent callbacks are silent; callback failures cannot change application outcomes. */
+export function createLogCallback(
+  onLog?: LogCallback,
+  context: LogMetadata = {},
+): LogCallback {
+  return (level, message, metadata) => {
+    try {
+      onLog?.(level, message, { ...metadata, ...context });
+    } catch {
+      // Log delivery must not turn an upstream failure into a page failure.
+    }
+  };
+}

web/platform/package.json

@@ -19,7 +19,11 @@
       "types": "./dist/path-segment.d.ts",
       "import": "./dist/path-segment.js"
     },
-    "./styles.css": "./styles.css"
+    "./styles.css": "./styles.css",
+    "./logging": {
+      "types": "./dist/logging.d.ts",
+      "import": "./dist/logging.js"
+    }
   },
   "scripts": {
     "build": "node -e \"require('node:fs').rmSync('dist',{recursive:true,force:true})\" && tsc -p tsconfig.build.json",

web/service/shared/health.test.ts

@@ -1,18 +1,17 @@
 import { describe, expect, it, vi } from "vitest";
 
-import type { Logger } from "pino";
 import { createNextHealthRoute } from "./health";
 
-function recordingLogger() {
-  const error = vi.fn();
-  const logger = { child: () => logger, debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error } as unknown as Logger;
-  return { logger, error };
-}
-
 describe("createNextHealthRoute", () => {
+  it("still returns unavailable if log delivery throws", async () => {
+    const GET = createNextHealthRoute(
+      { gateway: () => { throw new Error("gateway unavailable"); } },
+      () => { throw new Error("logging unavailable"); },
+    );
+    expect((await GET()).status).toBe(503);
+  });
   it("is ready and uncached when every check passes", async () => {
-    const { logger } = recordingLogger();
-    const response = await createNextHealthRoute({ configuration: () => {}, gateway: async () => {} }, logger)();
+    const response = await createNextHealthRoute({ configuration: () => {}, gateway: async () => {} })();
     expect(response.status).toBe(200);
     expect(response.headers.get("cache-control")).toBe("no-store");
     await expect(response.json()).resolves.toEqual({ status: "ok" });
@@ -23,16 +22,16 @@ describe("createNextHealthRoute", () => {
     const gateway = vi.fn().mockRejectedValueOnce(cause).mockRejectedValueOnce(cause)
       .mockResolvedValueOnce(undefined).mockRejectedValueOnce(cause);
     const later = vi.fn();
-    const { logger, error } = recordingLogger();
-    const GET = createNextHealthRoute({ gateway, later }, logger);
+    const onLog = vi.fn();
+    const GET = createNextHealthRoute({ gateway, later }, onLog);
 
     expect((await GET()).status).toBe(503);
     expect((await GET()).status).toBe(503);
-    expect(error).toHaveBeenCalledOnce();
-    expect(error).toHaveBeenCalledWith({ check: "gateway", err: cause }, "Web readiness check failed");
+    expect(onLog).toHaveBeenCalledOnce();
+    expect(onLog).toHaveBeenCalledWith("error", "Web readiness check failed", { check: "gateway", error: cause });
     expect(later).not.toHaveBeenCalled();
     expect((await GET()).status).toBe(200);
     expect((await GET()).status).toBe(503);
-    expect(error).toHaveBeenCalledTimes(2);
+    expect(onLog).toHaveBeenCalledTimes(2);
   });
 });

web/service/shared/health.ts

@@ -1,4 +1,4 @@
-import type { Logger } from "pino";
+import { createLogCallback, type LogCallback } from "@submitqueue/web-platform/logging";
 
 export type HealthChecks = Readonly<Record<string, () => void | Promise<void>>>;
 
@@ -9,16 +9,17 @@ const NO_STORE = { "Cache-Control": "no-store" };
  * when all pass, otherwise 503. Each check's failure is logged once until it
  * recovers, so a polling orchestrator does not flood the logs.
  */
-export function createNextHealthRoute(checks: HealthChecks, logger: Logger) {
+export function createNextHealthRoute(checks: HealthChecks, onLog?: LogCallback) {
   const failing = new Set<string>();
+  const log = createLogCallback(onLog);
   return async function GET(): Promise<Response> {
     for (const [name, check] of Object.entries(checks)) {
       try {
         await check();
       } catch (cause) {
         if (!failing.has(name)) {
           failing.add(name);
-          logger.error({ check: name, err: cause }, "Web readiness check failed");
+          log("error", "Web readiness check failed", { check: name, error: cause });
         }
         return Response.json({ status: "unavailable" }, { status: 503, headers: NO_STORE });
       }

web/service/shared/logging.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from "vitest";
+import { pino } from "pino";
+import { createPinoLogCallback } from "./logging.js";
+
+describe("Pino log delivery", () => {
+  it("preserves severity, message, metadata and the serialized error stack", () => {
+    const lines: string[] = [];
+    const log = createPinoLogCallback(
+      pino({ level: "info" }, {
+        write: (line) => { lines.push(line); },
+      }),
+    );
+    const error = new Error("backend unavailable");
+    log("info", "Access denied", { requestId: "request-1" });
+    log("warn", "Backend call failed", { requestId: "request-2", error });
+    log("error", "Page load failed", { requestId: "request-3", error });
+
+    const entries = lines.map((line) => JSON.parse(line));
+    expect(entries).toMatchObject([
+      { level: 30, msg: "Access denied", requestId: "request-1" },
+      {
+        level: 40, msg: "Backend call failed", requestId: "request-2",
+        err: { message: error.message, stack: error.stack },
+      },
+      {
+        level: 50, msg: "Page load failed", requestId: "request-3",
+        err: { message: error.message, stack: error.stack },
+      },
+    ]);
+    expect(entries[1]).not.toHaveProperty("error");
+  });
+});

web/service/shared/logging.ts

@@ -0,0 +1,9 @@
+import type { LogCallback } from "@submitqueue/web-platform/logging";
+import type { Logger } from "pino";
+
+export function createPinoLogCallback(logger: Logger): LogCallback {
+  return (level, message, metadata) => {
+    const { error, ...fields } = metadata;
+    logger[level]("error" in metadata ? { ...fields, err: error } : fields, message);
+  };
+}

web/service/shared/package.json

@@ -46,6 +46,10 @@
       "types": "./dist/transport.d.ts",
       "import": "./dist/transport.js"
     },
+    "./logging": {
+      "types": "./dist/logging.d.ts",
+      "import": "./dist/logging.js"
+    },
     "./health": {
       "types": "./dist/health.d.ts",
       "import": "./dist/health.js"

web/service/stovepipe/src/app/healthz/route.ts

@@ -4,7 +4,7 @@ import { createNextHealthRoute } from "@submitqueue/web-service-shared/health";
 import { loadConfiguration } from "../../server/config";
 import { loadAuthConfiguration } from "../../server/auth";
 import { newConnectTransport } from "@submitqueue/web-service-shared/transport";
-import { logger } from "../../server/host";
+import { onLog } from "../../server/host";
 
 export const dynamic = "force-dynamic";
 
@@ -25,5 +25,5 @@ export const GET = createNextHealthRoute(
       await client.ping({ message: "stovepipe-web-readiness" });
     },
   },
-  logger,
+  onLog,
 );

web/service/stovepipe/src/next/page.tsx

@@ -1,6 +1,6 @@
 import { forbidden, notFound, redirect } from "next/navigation";
 import { requireAuthorization } from "./authorize";
-import { resolveHost } from "../server/host";
+import { onLog, resolveHost } from "../server/host";
 import { StovepipeApp } from "@submitqueue/web-stovepipe";
 import { NextNavigationProvider } from "@submitqueue/web-service-shared/navigation";
 export async function StovepipePage({
@@ -22,6 +22,7 @@ export async function StovepipePage({
   const result = await resolveHost().handle({
     path: `/${path.join("/")}`,
     search,
+    onLog,
   });
   if (result.kind === "not-found") notFound();
   if (result.kind === "forbidden") forbidden();

web/service/stovepipe/src/server/host.ts

@@ -3,12 +3,14 @@ import { createClient } from "@connectrpc/connect";
 import { Stovepipe } from "@submitqueue/api/stovepipe";
 import { createStovepipeWeb } from "@submitqueue/web-stovepipe/server";
 import { pino } from "pino";
+import { createPinoLogCallback } from "@submitqueue/web-service-shared/logging";
 import { loadConfiguration } from "./config";
 import { newConnectTransport } from "@submitqueue/web-service-shared/transport";
-export const logger = pino({
+const logger = pino({
   name: "stovepipe-web",
   level: process.env.LOG_LEVEL ?? "info",
 });
+export const onLog = createPinoLogCallback(logger);
 
 let host: ReturnType<typeof createStovepipeWeb> | undefined;
 export function resolveHost() {
@@ -21,7 +23,6 @@ export function resolveHost() {
     host = createStovepipeWeb({
       service,
       queues: configuration.queues,
-      logger,
     });
   }
   return host;

web/service/submitqueue/src/app/(protected)/[[...path]]/page.tsx

@@ -1,11 +1,16 @@
 import { createNextPage } from "../../../next/page";
 import { resolveReferenceWeb } from "../../../server/host";
 import { requireAuthorization } from "../../../next/authorize";
+import { resolveReferenceLogCallback } from "../../../server/observability";
 
 export const dynamic = "force-dynamic";
 export const revalidate = 0;
 
-const page = createNextPage({ web: resolveReferenceWeb, authenticate: requireAuthorization });
+const page = createNextPage({
+  web: resolveReferenceWeb,
+  authenticate: requireAuthorization,
+  onLog: resolveReferenceLogCallback(),
+});
 
 export const generateMetadata = page.generateMetadata;
 export default page.Page;

web/service/submitqueue/src/app/healthz/route.ts

@@ -2,7 +2,7 @@ import { createNextHealthRoute } from "@submitqueue/web-service-shared/health";
 import { loadAuthConfiguration } from "../../server/auth";
 import { loadCursorSecret, loadGatewayConfiguration } from "../../server/config";
 import { pingReferenceGateway } from "../../server/gateway";
-import { resolveReferenceLogger } from "../../server/observability";
+import { resolveReferenceLogCallback } from "../../server/observability";
 
 export const dynamic = "force-dynamic";
 
@@ -13,4 +13,4 @@ export const GET = createNextHealthRoute({
     loadGatewayConfiguration();
   },
   gateway: pingReferenceGateway,
-}, resolveReferenceLogger());
+}, resolveReferenceLogCallback());

web/service/submitqueue/src/next/page.tsx

@@ -3,7 +3,7 @@ import { forbidden, notFound, redirect } from "next/navigation";
 import { connection } from "next/server";
 import { cache } from "react";
 import { SubmitQueueApp } from "@submitqueue/web-submitqueue";
-import type { SubmitQueueWeb, WebSearchParams } from "@submitqueue/web-submitqueue/server";
+import type { SubmitQueueWeb, SubmitQueueWebRequest, WebSearchParams } from "@submitqueue/web-submitqueue/server";
 import { NextNavigationProvider } from "@submitqueue/web-service-shared/navigation";
 
 export interface NextPageProps {
@@ -19,6 +19,7 @@ export interface NextPageOptions {
    * It may call Next's `unauthorized()`; it runs before every load.
    */
   authenticate?: () => Promise<unknown>;
+  onLog?: SubmitQueueWebRequest["onLog"];
 }
 
 /**
@@ -36,6 +37,7 @@ export function createNextPage(options: NextPageOptions) {
     return web.handle({
       path, search: JSON.parse(encodedSearch) as WebSearchParams,
       ...(principal === undefined ? {} : { principal }),
+      ...(options.onLog ? { onLog: options.onLog } : {}),
     });
   });
 

web/service/submitqueue/src/server/host.ts

@@ -4,7 +4,7 @@ import { newHmacCursorCodec } from "@submitqueue/web-submitqueue/extension/curso
 import { createSubmitQueueWeb, type SubmitQueueWeb } from "@submitqueue/web-submitqueue/server";
 import { loadCursorSecret } from "./config";
 import { resolveReferenceGateway } from "./gateway";
-import { resolveReferenceLogger, resolveReferenceMeter } from "./observability";
+import { resolveReferenceMeter } from "./observability";
 
 let web: SubmitQueueWeb | undefined;
 
@@ -17,7 +17,6 @@ export function resolveReferenceWeb(): SubmitQueueWeb {
   web ??= createSubmitQueueWeb({
     gateway: resolveReferenceGateway,
     cursors: newHmacCursorCodec(loadCursorSecret()),
-    logger: resolveReferenceLogger(),
     meter: resolveReferenceMeter(),
   });
   return web;

web/service/submitqueue/src/server/observability.ts

@@ -4,6 +4,7 @@ import { metrics, type Meter } from "@opentelemetry/api";
 import { PrometheusExporter } from "@opentelemetry/exporter-prometheus";
 import { MeterProvider } from "@opentelemetry/sdk-metrics";
 import { pino, type Logger } from "pino";
+import { createPinoLogCallback } from "@submitqueue/web-service-shared/logging";
 import { loadMetricsPort } from "./config";
 
 let logger: Logger | undefined;
@@ -14,6 +15,10 @@ export function resolveReferenceLogger(): Logger {
   return logger;
 }
 
+export function resolveReferenceLogCallback() {
+  return createPinoLogCallback(resolveReferenceLogger());
+}
+
 let meter: Meter | undefined;
 
 /**

web/stovepipe/BUILD.bazel

@@ -28,7 +28,6 @@ web_library(
         ":node_modules/@submitqueue/web-platform",
         ":node_modules/@types/node",
         ":node_modules/@types/react",
-        ":node_modules/pino",
         ":node_modules/react",
     ],
 )

web/stovepipe/README.md

@@ -2,6 +2,8 @@
 
 A framework-neutral React UI with a queue request list and a request page containing the current lifecycle summary and retained history. The host supplies queue configuration, a structural `StovepipeService` client, authentication, transport, logging and OpenTelemetry providers.
 
+Logging uses a synchronous `onLog` callback passed on each `handle` request. Bind it to the host's request-scoped logger; it receives severity, message, and structured metadata containing the original `error`, correlation `requestId`, and page `path`. Omitting it is silent, and callback exceptions do not change page results. The library has no logging-library dependency.
+
 ## Integration
 
 Use the server entry's `createStovepipeWeb` to load a page and render its model with `StovepipeApp`. Mount beneath `WebNavigationProvider` when the host supplies client navigation and refresh. Without it, links and refresh use ordinary document navigation. Import `@submitqueue/web-stovepipe/styles.css`, which includes the common native styles from `@submitqueue/web-platform`.