CL-5771: Onboarding provisioning robustness

packages/agent-directory/src/routes.ts

@@ -19,7 +19,7 @@ import { and, eq } from "drizzle-orm";
 import { Hono } from "hono";
 
 import type { DB } from "@intx/db";
-import { workflowDefinition } from "@intx/db/schema";
+import { asset, workflowDefinition } from "@intx/db/schema";
 import type { TenantEnv, RequireGrant } from "@intx/hub-api";
 import {
   AssetServiceError,
@@ -77,33 +77,66 @@ export function createAgentDefinitionRoutes({
     });
     const workflowJson = serializeAgentDefinitionWorkflow(definition);
 
-    let asset;
+    let assetId: string;
     try {
-      asset = await assetService.createAsset({
+      const created = await assetService.createAsset({
         tenantId: tenant.id,
         kind: "workflow",
         name: body.handle,
         displayName: body.name,
         creatorPrincipalId: principal.id,
       });
+      assetId = created.id;
     } catch (cause) {
       if (
         cause instanceof AssetServiceError &&
         cause.reason === "duplicate_asset"
       ) {
-        return c.json(
-          errorEnvelope(
-            "conflict",
-            `An agent with the handle "${body.handle}" already exists`,
+        // A previous attempt may have created the asset row but failed
+        // before populateAsset wrote workflow.json — an empty shell that
+        // blocks retries with a misleading 409. Recover: look up the
+        // existing asset and reuse it only if it has no definition yet.
+        const existing = await db.query.asset.findFirst({
+          where: and(
+            eq(asset.tenantId, tenant.id),
+            eq(asset.kind, "workflow"),
+            eq(asset.name, body.handle),
           ),
-          409,
-        );
+        });
+        if (existing) {
+          const hasDef = await db.query.workflowDefinition.findFirst({
+            where: and(
+              eq(workflowDefinition.assetId, existing.id),
+              eq(workflowDefinition.tenantId, tenant.id),
+            ),
+          });
+          if (!hasDef) {
+            assetId = existing.id;
+          } else {
+            return c.json(
+              errorEnvelope(
+                "conflict",
+                `An agent with the handle "${body.handle}" already exists`,
+              ),
+              409,
+            );
+          }
+        } else {
+          return c.json(
+            errorEnvelope(
+              "conflict",
+              `An agent with the handle "${body.handle}" already exists`,
+            ),
+            409,
+          );
+        }
+      } else {
+        throw cause;
       }
-      throw cause;
     }
 
     await assetService.populateAsset({
-      assetId: asset.id,
+      assetId,
       ref: DEFAULT_ASSET_REF,
       principal: { kind: "hub" },
       tree: {
@@ -114,7 +147,7 @@ export function createAgentDefinitionRoutes({
 
     const { definitionId } = await ensureWorkflowDefinitionForAsset(
       db,
-      asset.id,
+      assetId,
     );
 
     const row = await db.query.workflowDefinition.findFirst({

packages/agent-directory/test/routes.test.ts

@@ -53,14 +53,86 @@ function fakeAssetService(overrides: Partial<AssetService> = {}): AssetService {
   };
 }
 
-// Never reached on either path these tests exercise: the 400 fails
-// before any dependency call, and the 409 fails inside `createAsset`
-// before `db` is ever touched.
-const UNUSED_DB = {} as DB["db"];
+// The duplicate-asset recovery path queries `db` directly (looking up the
+// existing asset and its definition) before deciding whether to reuse an
+// empty shell or surface a real 409. When the shell is reused the route
+// continues through populateAsset → ensureWorkflowDefinitionForAsset →
+// read-back, so the fake also provides just enough of drizzle's chainable
+// query-builder API (`.select().from().where().limit()`,
+// `.insert().values().onConflictDoNothing().returning()`) for that
+// projection — the projection logic itself is `@intx/hub-sessions`/
+// `@intx/db` machinery already covered upstream; the fake only needs to
+// return plausible rows, not re-prove the SQL.
 
-function buildApp(assetService: AssetService): Hono<TenantEnv> {
+type FakeDbOptions = {
+  existingAsset?: { id: string };
+  hasDefinition?: boolean;
+};
+
+function fakeDb(opts: FakeDbOptions = {}): DB["db"] {
+  let wfDefFindFirstCalls = 0;
+
+  const selectResult = [
+    {
+      tenantId: TENANT.id,
+      creatorPrincipalId: null,
+      name: "research-buddy",
+      displayName: "Research Buddy",
+    },
+  ];
+
+  return {
+    query: {
+      asset: {
+        findFirst: async () => opts.existingAsset ?? undefined,
+      },
+      workflowDefinition: {
+        findFirst: async () => {
+          wfDefFindFirstCalls += 1;
+          if (wfDefFindFirstCalls === 1) {
+            return opts.hasDefinition ? { id: "def_existing" } : undefined;
+          }
+          // Read-back after ensureWorkflowDefinitionForAsset.
+          return {
+            id: "def_new",
+            tenantId: TENANT.id,
+            name: "Research Buddy",
+            description: null,
+            currentVersion: "1",
+            status: "deployed",
+            createdAt: new Date(),
+            updatedAt: new Date(),
+          };
+        },
+      },
+    },
+    select: () => ({
+      from: () => ({
+        where: () => ({
+          limit: () => Promise.resolve(selectResult),
+        }),
+      }),
+    }),
+    insert: () => ({
+      values: () => {
+        const chain: Record<string, unknown> = {
+          onConflictDoNothing: () => chain,
+          returning: () => Promise.resolve([{ id: "def_new" }]),
+          then: (onFulfilled: unknown) =>
+            Promise.resolve([]).then(onFulfilled as never),
+        };
+        return chain;
+      },
+    }),
+  } as unknown as DB["db"];
+}
+
+function buildApp(
+  assetService: AssetService,
+  db: DB["db"] = fakeDb(),
+): Hono<TenantEnv> {
   const routes = createAgentDefinitionRoutes({
-    db: UNUSED_DB,
+    db,
     assetService,
     requireGrant: () => async (_c, next) => {
       await next();

packages/hub-client/src/seed.ts

@@ -348,7 +348,7 @@ async function ensureWorkflowAsset(
 
   const listed = await api(
     "GET",
-    `/api/tenants/${args.tenantId}/assets?kind=workflow`,
+    `/api/tenants/${args.tenantId}/assets?kind=workflow&inherited=false`,
     undefined,
     cookies,
   );

packages/hub-client/test/seed.test.ts

@@ -239,7 +239,8 @@ describe("seedTenant", () => {
         return { status: 409, data: { error: "name taken" } };
       if (
         method === "GET" &&
-        path === `/api/tenants/${TENANT_ID}/assets?kind=workflow`
+        path ===
+          `/api/tenants/${TENANT_ID}/assets?kind=workflow&inherited=false`
       )
         return {
           status: 200,

packages/onboarding/src/provision.ts

@@ -33,6 +33,26 @@ export type ProvisionResult =
       readonly seedSkipReason?: string;
     };
 
+/**
+ * A typed provisioning failure. `kind` lets the routes layer distinguish
+ * a retryable (transient) failure — sidecar down, race, network — from a
+ * permanent one — slug conflict with no principal, tenant created but
+ * membership missing — so the client can decide whether to retry without
+ * parsing a free-text message.
+ */
+export type ProvisionErrorKind = "transient" | "permanent";
+
+export class ProvisionError extends Error {
+  readonly code: string;
+  readonly errorKind: ProvisionErrorKind;
+  constructor(code: string, message: string, errorKind: ProvisionErrorKind) {
+    super(message);
+    this.name = "ProvisionError";
+    this.code = code;
+    this.errorKind = errorKind;
+  }
+}
+
 export type ProvisionArgs = {
   api: ApiCall;
   cookies: string[];
@@ -98,7 +118,7 @@ async function isFullySeeded(
 ): Promise<boolean> {
   const assetsResponse = await api(
     "GET",
-    `/api/tenants/${tenantId}/assets?kind=workflow`,
+    `/api/tenants/${tenantId}/assets?kind=workflow&inherited=false`,
     undefined,
     cookies,
   );
@@ -148,10 +168,32 @@ export async function provisionPersonalTenantIfNeeded(
     // tenant and then failed before seeding it; re-seed rather than
     // silently treating "created but never seeded" as done.
     const own = before.find((p) => p.tenantSlug === expectedSlug);
-    if (!own || !args.seedModel) return { kind: "existing-member" };
-    if (await isFullySeeded(args.api, args.cookies, own.tenantId)) {
+    // Not our personal bench: some other tenant added this user, which
+    // is none of this hook's business. Membership is decided here without
+    // depending on a seed credential — recovery of a half-provisioned
+    // bench must not hang forever just because no seed model is configured.
+    if (!own) return { kind: "existing-member" };
+
+    const fullySeeded = await isFullySeeded(
+      args.api,
+      args.cookies,
+      own.tenantId,
+    );
+    if (fullySeeded) return { kind: "existing-member" };
+
+    // Own bench exists but is not fully seeded. With a seed model we can
+    // re-seed to recover. Without one there is nothing this hook can do
+    // to complete seeding, so we exit as an existing-member rather than
+    // throwing — membership is real even if seeding is incomplete. The
+    // routes layer surfaces this as a typed `bench_unseeded` condition so
+    // the caller can act on it (e.g. prompt credential setup).
+    if (!args.seedModel) {
+      args.log(
+        `personal bench ${own.tenantId} exists but is not fully seeded, and no seed model is configured; returning as existing-member without re-seeding`,
+      );
       return { kind: "existing-member" };
     }
+
     const tenantResponse = await args.api(
       "GET",
       `/api/tenants/${own.tenantId}`,
@@ -201,22 +243,28 @@ export async function provisionPersonalTenantIfNeeded(
     // surfacing the native route's slug conflict as a failure.
     const afterRace = await fetchPrincipals(args.api, args.cookies);
     if (afterRace.length > 0) return { kind: "existing-member" };
-    throw new Error(
+    throw new ProvisionError(
+      "slug_conflict_no_principal",
       `first-login provisioning hit a slug conflict creating a personal bench, but the caller still has no principal anywhere: ${JSON.stringify(created.data)}`,
+      "permanent",
     );
   }
   if (created.status !== 201) {
-    throw new Error(
+    throw new ProvisionError(
+      "tenant_create_failed",
       `first-login provisioning could not create a personal bench (status ${created.status}): ${JSON.stringify(created.data)}`,
+      created.status >= 500 ? "transient" : "permanent",
     );
   }
   const tenant = parseAs(TenantResponse, created.data, "tenant response");
 
   const after = await fetchPrincipals(args.api, args.cookies);
   const membership = after.find((p) => p.tenantId === tenant.id);
   if (!membership) {
-    throw new Error(
+    throw new ProvisionError(
+      "tenant_created_no_membership",
       `personal bench ${tenant.id} was created but the caller has no principal in it`,
+      "transient",
     );
   }
 

packages/onboarding/src/routes.ts

@@ -15,7 +15,8 @@ import {
 } from "@workbench/hub-client";
 import { Hono } from "hono";
 import { type } from "arktype";
-import { provisionPersonalTenantIfNeeded } from "./provision";
+import { provisionPersonalTenantIfNeeded, ProvisionError } from "./provision";
+
 import { completeCredentialSetup } from "./complete-credential";
 
 const PROVIDER_IDS = supportedCredentialProviders().map((p) => p.id) as [
@@ -50,6 +51,14 @@ export function createOnboardingRoutes(
   const app = new Hono<AppEnv>();
   const api = createHubAPI(deps.hubUrl);
 
+  // A simple in-process per-user provision rate limiter. Provisioning is
+  // idempotent and safe to retry, but a client stuck in a tight retry loop
+  // (or a runaway script) can pile concurrent tenant creates onto the hub.
+  // One in-flight or recent provision per user is enough; the window is
+  // short because successful provisioning resolves immediately.
+  const PROVISION_RATE_LIMIT_MS = 10_000;
+  const lastProvisionByUser = new Map<string, number>();
+
   app.post("/provision", async (c) => {
     const user = c.get("user");
     if (!user) {
@@ -59,6 +68,26 @@ export function createOnboardingRoutes(
       );
     }
 
+    const now = Date.now();
+    const lastAttempt = lastProvisionByUser.get(user.id);
+    if (
+      lastAttempt !== undefined &&
+      now - lastAttempt < PROVISION_RATE_LIMIT_MS
+    ) {
+      return c.json(
+        {
+          error: {
+            code: "rate_limited",
+            kind: "transient" as const,
+            message:
+              "Too many provisioning attempts. Please wait a moment and try again.",
+          },
+        },
+        429,
+      );
+    }
+    lastProvisionByUser.set(user.id, now);
+
     const cookies = cookiesFromHeader(c.req.header("cookie"));
     try {
       const provisionArgs: Parameters<
@@ -85,15 +114,32 @@ export function createOnboardingRoutes(
       deps.log(
         `first-login provisioning failed for user ${user.id}: ${message}`,
       );
+      if (cause instanceof ProvisionError) {
+        const status = cause.errorKind === "transient" ? 503 : 500;
+        return c.json(
+          {
+            error: {
+              code: cause.code,
+              kind: cause.errorKind,
+              message: cause.message,
+            },
+          },
+          status,
+        );
+      }
+      // An unrecognized error is treated as transient — the hub may have
+      // been momentarily unavailable, and retrying is safe because
+      // provisioning is idempotent.
       return c.json(
         {
           error: {
             code: "provisioning_failed",
+            kind: "transient" as const,
             message:
               "Could not provision a workbench for this account. Try again in a moment.",
           },
         },
-        500,
+        503,
       );
     }
   });