diff --git a/packages/onboarding/src/bench-provisioning.ts b/packages/onboarding/src/bench-provisioning.ts index 626231692..5867da2cf 100644 --- a/packages/onboarding/src/bench-provisioning.ts +++ b/packages/onboarding/src/bench-provisioning.ts @@ -31,6 +31,7 @@ // composition root decides how a session is minted for a user, and this // module stays out of the auth mechanism entirely. +import { reportError } from "@corbits/error-sink"; import { type ApiCall, type WorkflowPusher } from "@workbench/hub-client"; import { ensureSeeded } from "./complete-credential"; import { isFullySeeded } from "./provision"; @@ -297,6 +298,11 @@ export function createBenchProvisioner( void drainOnce().catch((cause: unknown) => { const message = cause instanceof Error ? cause.message : String(cause); logError(`bench provisioning drain failed: ${message}`); + // Not scoped to any one bench — a whole-tick failure (listDue + // itself throwing, say) rather than one seed's own provisioning + // failure, which already carries tenant context via holdOff + // (CL-7234). + reportError(cause, { operation: "bench_provisioning_drain" }); }); } diff --git a/packages/onboarding/src/plant-env-credentials.ts b/packages/onboarding/src/plant-env-credentials.ts index e63111097..1657f34e5 100644 --- a/packages/onboarding/src/plant-env-credentials.ts +++ b/packages/onboarding/src/plant-env-credentials.ts @@ -26,6 +26,7 @@ import { paginatedSchema, ProviderResponse, } from "@intx/types"; +import { reportError } from "@corbits/error-sink"; import { inferenceCredentialName, OLLAMA_PLACEHOLDER_SECRET, @@ -327,7 +328,17 @@ export async function plantEnvProviderCredentials( }), ); } catch (cause) { - const message = cause instanceof Error ? cause.message : String(cause); + reportError(cause, { + operation: "env_credential_plant_backfill_catalog", + tenantId: args.tenantId, + extra: { provider }, + }); + // Never the raw cause detail (this module handles a live API + // key, CL-7234) — sanitizeProviderMessage is the same redaction + // the probe-failure path below already applies. + const message = sanitizeProviderMessage( + cause instanceof Error ? cause.message : String(cause), + ); args.log( `env credential plant: ${provider} failed to backfill catalog: ${message}`, ); @@ -357,7 +368,16 @@ export async function plantEnvProviderCredentials( try { await runSeedCatalog(catalogSeedArgs(provider, { apiKey })); } catch (cause) { - const message = cause instanceof Error ? cause.message : String(cause); + reportError(cause, { + operation: "env_credential_plant", + tenantId: args.tenantId, + extra: { provider }, + }); + // Never the raw cause detail — this catch runs right after a live + // API key was used to seed the catalog (CL-7234). + const message = sanitizeProviderMessage( + cause instanceof Error ? cause.message : String(cause), + ); args.log(`env credential plant: ${provider} failed to plant: ${message}`); outcomes.push({ provider, status: "failed", message }); continue; diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index af5fa371f..508dc3217 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -25,6 +25,7 @@ import { Hono } from "hono"; import { type } from "arktype"; import type { AccessPolicyStore } from "@workbench/access-policy"; import { generateRefId, makeErrorEnvelope } from "@workbench/hub-client"; +import { reportError } from "@corbits/error-sink"; import { personalTenantSlug, @@ -72,16 +73,30 @@ function assertNonEmpty(arr: T[]): asserts arr is [T, ...T[]] { function reportOnboardingError( logError: (line: string) => void, args: { + /** The @corbits/error-sink operation name for this call site — one + * per route action, snake_case, never the failure code itself + * (CL-7234): a `ProvisionError`'s own `code` is an unbounded + * per-failure taxonomy, and using it as `operation` would fragment + * "provisioning is broken" across N sink operations instead of one. */ + operation: string; userAction: string; code: string; userMessage: string; cause: unknown; + tenantId?: string; + extra?: Record; }, ): ReturnType { const refId = generateRefId(); const detail = args.cause instanceof Error ? args.cause.message : String(args.cause); logError(`[${refId}] ${args.userAction} failed (${args.code}): ${detail}`); + reportError(args.cause, { + operation: args.operation, + refId, + ...(args.tenantId !== undefined ? { tenantId: args.tenantId } : {}), + ...(args.extra !== undefined ? { extra: args.extra } : {}), + }); return makeErrorEnvelope({ code: args.code, userMessage: args.userMessage, @@ -433,10 +448,12 @@ export function createOnboardingRoutes( ? "Sign-ups aren't open for this account yet. Contact your workspace admin for access." : "Setting up your workbench hit a snag — we're on it. Try again in a moment."; const envelope = reportOnboardingError(deps.logError ?? deps.log, { + operation: "onboarding_provision", userAction: `first-login provisioning for user ${user.id}`, code: cause.code, userMessage, cause, + extra: { userId: user.id, code: cause.code }, }); return c.json( { @@ -449,11 +466,13 @@ export function createOnboardingRoutes( // been momentarily unavailable, and retrying is safe because // provisioning is idempotent. const envelope = reportOnboardingError(deps.logError ?? deps.log, { + operation: "onboarding_provision", userAction: `first-login provisioning for user ${user.id}`, code: "provisioning_failed", userMessage: "Setting up your workbench hit a snag — we're on it. Try again in a moment.", cause, + extra: { userId: user.id }, }); return c.json( { error: { ...envelope.error, kind: "transient" as const } }, @@ -736,6 +755,9 @@ export function createOnboardingRoutes( pushWorkflow: deps.pushWorkflow, log: deps.log, }; + // Known once `runTestAndPersistCredential` resolves; a failure + // before that point (the credential itself, say) has no tenant yet. + let tenantId: string | undefined; try { // The fast half, and only the fast half (CL-6457): persist the // credential, seed its catalog, answer. Deploying this bench's @@ -776,6 +798,7 @@ export function createOnboardingRoutes( // follow. The credential is proven-durable here whether or not the // agents have finished deploying. deps.providerHealth?.clear(result.tenantId, parsed.provider); + tenantId = result.tenantId; const status = await provisioningStatus(cookies, result); if (status.kind === "ready") { @@ -809,11 +832,14 @@ export function createOnboardingRoutes( // disk (CL-6360). The raw detail is logged behind a refId; the // client only ever sees a fixed consumer sentence plus that refId. const envelope = reportOnboardingError(deps.logError ?? deps.log, { + operation: "onboarding_complete", userAction: `credential setup for user ${user.id}`, code: "credential_setup_failed", userMessage: "Your key was added, but finishing your workbench setup hit a snag — we're on it. Try again in a moment.", cause, + ...(tenantId !== undefined ? { tenantId } : {}), + extra: { userId: user.id }, }); return c.json({ error: envelope.error }, 500); } @@ -843,6 +869,9 @@ export function createOnboardingRoutes( } const cookies = cookiesFromHeader(c.req.header("cookie")); + // Known once the tenant lookup below resolves; a lookup failure + // itself has no tenant yet. + let tenantId: string | undefined; try { const expectedSlug = personalTenantSlug(user.email, user.id); const tenant = await findPersonalTenant(api, cookies, expectedSlug); @@ -856,6 +885,7 @@ export function createOnboardingRoutes( 409, ); } + tenantId = tenant.tenantId; const status = await provisioningStatus(cookies, tenant); if (status.kind === "ready") { @@ -879,11 +909,14 @@ export function createOnboardingRoutes( return c.json(status, 200); } catch (cause) { const envelope = reportOnboardingError(deps.logError ?? deps.log, { + operation: "onboarding_complete_setup", userAction: `complete-setup for user ${user.id}`, code: "complete_setup_failed", userMessage: "Finishing your workbench setup hit a snag — we're on it. Try again in a moment.", cause, + ...(tenantId !== undefined ? { tenantId } : {}), + extra: { userId: user.id }, }); return c.json({ error: envelope.error }, 500); } @@ -906,6 +939,9 @@ export function createOnboardingRoutes( } const cookies = cookiesFromHeader(c.req.header("cookie")); + // Known once the tenant lookup below resolves; a lookup failure + // itself has no tenant yet. + let tenantId: string | undefined; try { const expectedSlug = personalTenantSlug(user.email, user.id); const tenant = await findPersonalTenant(api, cookies, expectedSlug); @@ -919,15 +955,19 @@ export function createOnboardingRoutes( 409, ); } + tenantId = tenant.tenantId; return c.json(await provisioningStatus(cookies, tenant), 200); } catch (cause) { const envelope = reportOnboardingError(deps.logError ?? deps.log, { + operation: "onboarding_provisioning_status", userAction: `provisioning status for user ${user.id}`, code: "provisioning_status_failed", userMessage: "Checking on your agents hit a snag — we're on it. Try again in a moment.", cause, + ...(tenantId !== undefined ? { tenantId } : {}), + extra: { userId: user.id }, }); return c.json({ error: envelope.error }, 500); } diff --git a/packages/onboarding/test/report-error-routing.test.ts b/packages/onboarding/test/report-error-routing.test.ts new file mode 100644 index 000000000..01f6e5e53 --- /dev/null +++ b/packages/onboarding/test/report-error-routing.test.ts @@ -0,0 +1,233 @@ +// CL-7234: every caught failure in this package's routes, background +// drain, and env-credential-plant paths must reach @corbits/error-sink's +// reportError with the operation/tenant context it expects — the same +// precedent ../src/provision.ts already sets. This mocks +// @corbits/error-sink and dynamically imports each module under test +// afterward (the same recipe packages/workflow-deploy-source and +// packages/webhook-triggers already use for this exact kind of +// assertion), since a static top-level import would bind the real +// module before any mock could apply. +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import type { AppEnv } from "@intx/hub-api"; +import type { MiddlewareHandler } from "hono"; +import { Hono } from "hono"; +import { createNoopCredentialCipher } from "@intx/crypto"; + +let reportErrorCalls: [unknown, Record][] = []; +beforeEach(async () => { + reportErrorCalls = []; + await mock.module("@corbits/error-sink", () => ({ + reportError: (error: unknown, context: Record) => { + reportErrorCalls.push([error, context]); + return "ref_test"; + }, + })); +}); +afterEach(() => { + mock.restore(); +}); + +const { createOnboardingRoutes } = await import("../src/routes"); +const { createInMemoryPendingSeedStore } = await import("../src/pending-seed"); +const { createBenchProvisioner } = await import("../src/bench-provisioning"); +const { plantEnvProviderCredentials } = + await import("../src/plant-env-credentials"); + +const pendingSeedStore = createInMemoryPendingSeedStore( + createNoopCredentialCipher(), +); + +const asUser: MiddlewareHandler = async (c, next) => { + c.set("user", { id: "user_1", email: "user_1@example.com" } as never); + await next(); +}; + +function mountAuthenticated(routes: Hono): Hono { + const app = new Hono(); + app.use("*", asUser); + app.route("/", routes); + return app; +} + +describe("routes.ts routes caught errors through reportError", () => { + test("a failure with no tenant known yet reports operation + userId, no tenantId", async () => { + const routes = createOnboardingRoutes({ + hubUrl: "http://127.0.0.1:0", + pushWorkflow: async () => ({ + outcome: "pushed" as const, + commitSha: "a".repeat(40), + }), + log: () => undefined, + pendingSeedStore, + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/provision", { method: "POST" }); + + expect(response.status).toBe(503); + expect(reportErrorCalls).toHaveLength(1); + const [, context] = reportErrorCalls[0] as [ + unknown, + Record, + ]; + expect(context.operation).toBe("onboarding_provision"); + expect(context.tenantId).toBeUndefined(); + expect((context.extra as { userId: string }).userId).toBe("user_1"); + }); + + test("a provisioning-status failure after the tenant is found reports its tenantId", async () => { + // Matches complete-setup-routes.test.ts's fixture shape exactly: + // personalTenantSlug("user_1@example.com", "user_1") === "user-1-user1". + const TENANT_ID = "ten_1"; + const TENANT_SLUG = "user-1-user1"; + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ + data: [ + { + principalId: "prn_1", + tenantId: TENANT_ID, + tenantName: "user_1's workbench", + tenantSlug: TENANT_SLUG, + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }), + ); + hub.get(`/api/tenants/${TENANT_ID}`, (c) => + c.json({ + id: TENANT_ID, + name: "user_1's workbench", + slug: TENANT_SLUG, + domain: "user-1-user1.bench.local", + parentId: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }), + ); + // Malformed on purpose: seededWorkflowNames' own parseAs rejects this, + // throwing well after tenantId is already known. + hub.get(`/api/tenants/${TENANT_ID}/assets`, (c) => + c.json({ notAnArray: true }), + ); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const routes = createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => ({ + outcome: "pushed" as const, + commitSha: "a".repeat(40), + }), + log: () => undefined, + pendingSeedStore, + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/provisioning-status"); + + expect(response.status).toBe(500); + expect(reportErrorCalls).toHaveLength(1); + const [, context] = reportErrorCalls[0] as [ + unknown, + Record, + ]; + expect(context.operation).toBe("onboarding_provisioning_status"); + expect(context.tenantId).toBe(TENANT_ID); + expect((context.extra as { userId: string }).userId).toBe("user_1"); + } finally { + server.stop(true); + } + }); +}); + +describe("bench-provisioning.ts's whole-drain failure reports through reportError", () => { + test("a listDue failure (not scoped to any one bench) reports operation only", async () => { + const provisioner = createBenchProvisioner({ + api: (async () => { + throw new Error("unused"); + }) as unknown as Parameters[0]["api"], + hubUrl: "https://bench.example.com", + store: { + listDue: async () => { + throw new Error("db unreachable"); + }, + read: async () => undefined, + put: async () => undefined, + clear: async () => undefined, + }, + pushWorkflow: async () => ({ + outcome: "pushed" as const, + commitSha: "a".repeat(40), + }), + sessionFor: async () => ["better-auth.session_token=minted"], + log: () => undefined, + }); + + provisioner.wake(); + // wake() is fire-and-forget; give its internal drainOnce().catch a + // turn to run before asserting. + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(reportErrorCalls).toHaveLength(1); + const [, context] = reportErrorCalls[0] as [ + unknown, + Record, + ]; + expect(context.operation).toBe("bench_provisioning_drain"); + expect(context.tenantId).toBeUndefined(); + }); +}); + +describe("plant-env-credentials.ts catches report through reportError and never leak the raw cause", () => { + // No existing provider row for any credential this test plants — the + // real network calls `findProviderId`/`findActiveCredential` make + // aren't overridable test seams, so the fake `api` has to answer them + // honestly (empty pages) to reach the seedCatalogFn override at all. + const noExistingProvidersApi: Parameters< + typeof plantEnvProviderCredentials + >[0]["api"] = async (_method, path) => { + if (path.includes("/providers")) { + return { status: 200, data: { data: [], nextCursor: null }, cookies: [] }; + } + throw new Error(`unexpected call in test fake: ${path}`); + }; + + test("a catalog-plant failure reports tenantId + provider, and sanitizes a secret-shaped cause message", async () => { + const outcomes = await plantEnvProviderCredentials({ + api: noExistingProvidersApi, + cookies: [], + tenantId: "tnt_env", + envProviderKeys: { anthropic: "sk-ant-real-secret-value-123456" }, + log: () => undefined, + testCredential: async () => ({ ok: true }), + seedCatalogFn: async () => { + throw new Error("seed failed for key sk-ant-real-secret-value-123456"); + }, + }); + + expect(outcomes).toEqual([ + { + provider: "anthropic", + status: "failed", + message: expect.stringContaining("[redacted]") as unknown as string, + }, + ]); + expect(outcomes[0]?.message).not.toContain("sk-ant-real-secret-value"); + + expect(reportErrorCalls).toHaveLength(1); + const [cause, context] = reportErrorCalls[0] as [ + unknown, + Record, + ]; + expect(context.operation).toBe("env_credential_plant"); + expect(context.tenantId).toBe("tnt_env"); + expect(context.extra).toEqual({ provider: "anthropic" }); + // reportError itself is the one place the unredacted cause is + // still allowed to travel — error-sink does its own redaction pass + // before anything reaches a log sink. + expect(cause).toBeInstanceOf(Error); + }); +});