diff --git a/packages/onboarding/src/complete-credential.ts b/packages/onboarding/src/complete-credential.ts index c292bc897f..5c562d78f2 100644 --- a/packages/onboarding/src/complete-credential.ts +++ b/packages/onboarding/src/complete-credential.ts @@ -198,10 +198,29 @@ export type CompleteCredentialArgs = CommonArgs & seedTenantFn?: (args: SeedTenantArgs) => ReturnType; }; +/** + * Resolves the tenant a caller's bench work lands on. An exact slug match + * against `expectedSlug` (the computed personal-bench slug) always wins. + * + * By default the match is strict: a mismatch resolves to `undefined`. + * The boot seeder (`apps/hub/src/system-seed.ts`) depends on that — it + * must keep throwing until the root bench is actually visible to the + * admin, never seed onto the first principal that happens to be visible. + * + * The connect-flow callers (`testAndPersistCredential`, the OAuth + * duplicate-callback recovery `recentlyConnectedCredential`, and the + * `/complete-setup` route) pass `fallbackToFirstPrincipal` so a caller + * that already has a bench but under a different slug — a seeded admin + * whose only membership is the root bench (CL-7506) — resolves to the + * first principal in page order instead of hard-failing + * `no-personal-bench`. Only a response with zero principals yields + * `undefined`, with or without the flag. + */ export async function findPersonalTenant( api: ApiCall, cookies: string[], expectedSlug: string, + opts: { fallbackToFirstPrincipal?: boolean } = {}, ): Promise { const response = await api("GET", "/api/me/principals", undefined, cookies); const summary = parseAs( @@ -209,7 +228,9 @@ export async function findPersonalTenant( response.data, "principals response", ); - const own = summary.data.find((p) => p.tenantSlug === expectedSlug); + const own = + summary.data.find((p) => p.tenantSlug === expectedSlug) ?? + (opts.fallbackToFirstPrincipal ? summary.data[0] : undefined); if (!own) return undefined; const tenantResponse = await api( @@ -408,7 +429,14 @@ export async function testAndPersistCredential( args: TestAndPersistCredentialArgs, ): Promise { const expectedSlug = personalTenantSlug(args.userEmail, args.userId); - const tenant = await findPersonalTenant(args.api, args.cookies, expectedSlug); + const tenant = await findPersonalTenant( + args.api, + args.cookies, + expectedSlug, + { + fallbackToFirstPrincipal: true, + }, + ); if (!tenant) return { kind: "no-personal-bench" }; const descriptor = CONNECTOR_REGISTRY[args.provider]; diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index 377beb050f..eec070c722 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -269,7 +269,14 @@ async function recentlyConnectedCredential( const now = args.now ?? Date.now; try { const expectedSlug = personalTenantSlug(args.userEmail, args.userId); - const tenant = await findPersonalTenant(api, cookies, expectedSlug); + // The recovery check serves the same connect flow as + // `testAndPersistCredential`, so it resolves through the same + // fallback — a seeded admin (root bench only) whose browser + // double-fires the callback must recover as connected, not see a + // strict miss turn into `state_expired` (CL-7506). + const tenant = await findPersonalTenant(api, cookies, expectedSlug, { + fallbackToFirstPrincipal: true, + }); if (!tenant) return undefined; const listed = await api( @@ -892,7 +899,9 @@ export function createOnboardingRoutes( let tenantId: string | undefined; try { const expectedSlug = personalTenantSlug(user.email, user.id); - const tenant = await findPersonalTenant(api, cookies, expectedSlug); + const tenant = await findPersonalTenant(api, cookies, expectedSlug, { + fallbackToFirstPrincipal: true, + }); if (!tenant) { return c.json( makeErrorEnvelope({ diff --git a/packages/onboarding/test/complete-credential.test.ts b/packages/onboarding/test/complete-credential.test.ts index ea7fbfc69e..02b04bee77 100644 --- a/packages/onboarding/test/complete-credential.test.ts +++ b/packages/onboarding/test/complete-credential.test.ts @@ -6,6 +6,7 @@ import { pristineScheduledDefinitionHandshake } from "../../seeding/test/helpers import { completeCredentialSetup, ensureSeeded, + findPersonalTenant, modelSourceFor, testAndPersistCredential, } from "../src/complete-credential"; @@ -2086,3 +2087,175 @@ describe("ensureSeeded (the slow half)", () => { ).rejects.toThrow("the hub rejected the deployment with status 500"); }); }); + +// CL-7506: a seeded admin's only membership is the root bench (slug = the +// org's own slug), which never equals personalTenantSlug(userEmail, +// userId) — the connect flow must fall back to that existing principal +// rather than 409 with no_personal_bench. The fallback is opt-in: the +// boot seeder (apps/hub/src/system-seed.ts) calls with no options and +// depends on the strict match refusing to run until the root bench is +// actually visible to the admin. +describe("findPersonalTenant", () => { + function principalsPage( + principals: { + principalId: string; + tenantId: string; + tenantSlug: string; + }[], + ) { + return { + status: 200, + data: { + data: principals.map((p) => ({ + kind: "user", + status: "active", + roles: [], + tenantName: p.tenantSlug, + ...p, + })), + nextCursor: null, + }, + cookies: [], + }; + } + + function tenantFor(slug: string, domain: string) { + return { + status: 200, + data: { + id: slug, + name: slug, + slug, + domain, + parentId: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + cookies: [], + }; + } + + function seedAdminHub() { + return async (method: string, path: string) => { + if (method === "GET" && path === "/api/me/principals") { + return principalsPage([ + { + principalId: "prn_root", + tenantId: "ten_root", + tenantSlug: "acme", + }, + ]); + } + if (method === "GET" && path === "/api/tenants/ten_root") { + return tenantFor("acme", "acme.bench.local"); + } + throw new Error(`unexpected call: ${method} ${path}`); + }; + } + + test("strict by default: a slug mismatch resolves to nothing, so the boot seeder keeps throwing until the root bench is visible", async () => { + // personalTenantSlug("alice@example.com", "user_1") is "alice-user1"; + // the only principal is the root bench "acme". Without the fallback + // flag this must NOT silently resolve — system-seed pins on it. + expect( + await findPersonalTenant(seedAdminHub(), ["session=abc"], "alice-user1"), + ).toBeUndefined(); + }); + + test("with the fallback flag, the seeded-admin shape resolves to the root bench instead of no-personal-bench", async () => { + expect( + await findPersonalTenant(seedAdminHub(), ["session=abc"], "alice-user1", { + fallbackToFirstPrincipal: true, + }), + ).toEqual({ + tenantId: "ten_root", + tenantSlug: "acme", + principalId: "prn_root", + tenantDomain: "acme.bench.local", + }); + }); + + test("a caller with zero principals still gets no-personal-bench, fallback or not", async () => { + const api: ApiCall = async (method, path) => { + if (method === "GET" && path === "/api/me/principals") { + return principalsPage([]); + } + throw new Error(`unexpected call: ${method} ${path}`); + }; + + expect( + await findPersonalTenant(api, ["session=abc"], "alice-user1"), + ).toBeUndefined(); + expect( + await findPersonalTenant(api, ["session=abc"], "alice-user1", { + fallbackToFirstPrincipal: true, + }), + ).toBeUndefined(); + }); + + test("an exact slug match wins even when a mismatched principal comes first", async () => { + const api: ApiCall = async (method, path) => { + if (method === "GET" && path === "/api/me/principals") { + return principalsPage([ + { + principalId: "prn_root", + tenantId: "ten_root", + tenantSlug: "acme", + }, + { + principalId: PRINCIPAL_ID, + tenantId: TENANT_ID, + tenantSlug: TENANT_SLUG, + }, + ]); + } + if (method === "GET" && path === `/api/tenants/${TENANT_ID}`) { + return tenantFor(TENANT_SLUG, "alice-user1.bench.local"); + } + throw new Error(`unexpected call: ${method} ${path}`); + }; + + expect(await findPersonalTenant(api, ["session=abc"], TENANT_SLUG)).toEqual( + { + tenantId: TENANT_ID, + tenantSlug: TENANT_SLUG, + principalId: PRINCIPAL_ID, + tenantDomain: "alice-user1.bench.local", + }, + ); + }); + + test("the fallback pick is the first principal in page order", async () => { + const api: ApiCall = async (method, path) => { + if (method === "GET" && path === "/api/me/principals") { + return principalsPage([ + { + principalId: "prn_first", + tenantId: "ten_first", + tenantSlug: "first-bench", + }, + { + principalId: "prn_second", + tenantId: "ten_second", + tenantSlug: "second-bench", + }, + ]); + } + if (method === "GET" && path === "/api/tenants/ten_first") { + return tenantFor("first-bench", "first.bench.local"); + } + throw new Error(`unexpected call: ${method} ${path}`); + }; + + expect( + await findPersonalTenant(api, ["session=abc"], "alice-user1", { + fallbackToFirstPrincipal: true, + }), + ).toEqual({ + tenantId: "ten_first", + tenantSlug: "first-bench", + principalId: "prn_first", + tenantDomain: "first.bench.local", + }); + }); +}); diff --git a/packages/onboarding/test/complete-setup-routes.test.ts b/packages/onboarding/test/complete-setup-routes.test.ts index 4306ade0be..8d6f9c5a08 100644 --- a/packages/onboarding/test/complete-setup-routes.test.ts +++ b/packages/onboarding/test/complete-setup-routes.test.ts @@ -161,6 +161,67 @@ describe("POST /complete-setup", () => { } }); + // CL-7506: a seeded admin's only membership is the root bench, whose + // slug is the org's own — never the computed personal-bench slug. The + // fallback must resolve that principal here too, answering from the + // read instead of 409ing. + test("a root-bench-only admin (slug mismatch) resolves through the fallback and reports unseeded, not 409", async () => { + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ + data: [ + { + principalId: "prn_root", + tenantId: "ten_root", + tenantName: "acme", + tenantSlug: "acme", + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }), + ); + hub.get("/api/tenants/ten_root", (c) => + c.json({ + id: "ten_root", + name: "acme", + slug: "acme", + domain: "acme.bench.local", + parentId: null, + createdAt: TIMESTAMP, + updatedAt: TIMESTAMP, + }), + ); + hub.get("/api/tenants/ten_root/assets", (c) => c.json([])); + hub.get("/api/tenants/ten_root/workflows/deployments", (c) => c.json([])); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const app = mountAuthenticated( + createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => ({ + outcome: "pushed" as const, + commitSha: "a".repeat(40), + }), + log: () => undefined, + pendingSeedStore: createInMemoryPendingSeedStore(testCipher()), + }), + ); + + const response = await app.request("/api/onboarding/complete-setup", { + method: "POST", + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { kind: string }; + expect(body.kind).toBe("unseeded"); + } finally { + server.stop(true); + } + }); + test("an already fully seeded bench reports ready without needing a pending row", async () => { const hub = new Hono(); principalsRoute(hub); diff --git a/packages/onboarding/test/openrouter-connect-routes.test.ts b/packages/onboarding/test/openrouter-connect-routes.test.ts index 27454ab4c7..c3653814b1 100644 --- a/packages/onboarding/test/openrouter-connect-routes.test.ts +++ b/packages/onboarding/test/openrouter-connect-routes.test.ts @@ -36,7 +36,12 @@ const MOCK_TIMESTAMP = "2026-01-01T00:00:00.000Z"; * test that reaches one fails loudly with a 404, which is exactly the * proof this suite wants that the fast half never asks for a deploy. */ -function mockHub() { +function mockHub(args: { tenantSlug?: string } = {}) { + // `tenantSlug` doubles as the seeded-admin shape: set it to "acme" and + // the only principal is the root bench, whose slug never equals the + // signed-in user's computed personal-bench slug (CL-7506). + const tenantSlug = args.tenantSlug ?? "user-1-user1"; + const tenantDomain = `${tenantSlug}.bench.local`; const credentials: { id: string; tenantId: string; @@ -56,7 +61,7 @@ function mockHub() { principalId: "prn_1", tenantId: "ten_1", tenantName: "Alice's workbench", - tenantSlug: "user-1-user1", + tenantSlug, kind: "user", status: "active", roles: [], @@ -69,8 +74,8 @@ function mockHub() { c.json({ id: "ten_1", name: "Alice's workbench", - slug: "user-1-user1", - domain: "user-1-user1.bench.local", + slug: tenantSlug, + domain: tenantDomain, parentId: null, createdAt: MOCK_TIMESTAMP, updatedAt: MOCK_TIMESTAMP, @@ -508,6 +513,42 @@ describe("GET /oauth/openrouter/callback", () => { } }); + // CL-7506: the recovery lookup serves the same connect flow, so it + // must resolve through the fallback too — a seeded admin (root bench + // "acme" only, slug never equals the computed "user-1-user1") whose + // browser double-fires the callback gets `connected`, not + // `state_expired`, on the second arrival. + test("a seeded admin's duplicate callback recovers as connected even though only the root bench matches", async () => { + const server = Bun.serve({ + port: 0, + fetch: mockHub({ tenantSlug: "acme" }).fetch, + }); + try { + const app = connectRoutes({ + hubUrl: `http://localhost:${server.port}`, + openrouterConnect: { + exchange: async () => ({ ok: true, key: "sk-or-v1-minted" }), + connectCredential: connectCredentialAgainstMockHub, + }, + }); + const { response: started } = await startConnect(app); + const cookie = stateCookie(started); + const path = "/api/onboarding/oauth/openrouter/callback?code=auth_code_1"; + + await app.request(path, { headers: { cookie } }); + const second = await app.request(path, { headers: { cookie } }); + + const secondRedirect = new URL( + second.headers.get("location") ?? "", + "https://x", + ); + expect(secondRedirect.searchParams.get("outcome")).toBe("connected"); + expect(secondRedirect.searchParams.get("tenantSlug")).toBe("acme"); + } finally { + server.stop(true); + } + }); + test("a genuinely stale state (no matching recent credential) still errors honestly", async () => { const app = connectRoutes({ openrouterConnect: { diff --git a/packages/onboarding/test/routes.test.ts b/packages/onboarding/test/routes.test.ts index fe6732dc9b..7f3ff32248 100644 --- a/packages/onboarding/test/routes.test.ts +++ b/packages/onboarding/test/routes.test.ts @@ -10,6 +10,7 @@ import type { MiddlewareHandler } from "hono"; import { Hono } from "hono"; import { createNoopCredentialCipher } from "@intx/crypto"; import { createOnboardingRoutes } from "../src/routes"; +import { testAndPersistCredential } from "../src/complete-credential"; import { createInMemoryPendingSeedStore } from "../src/pending-seed"; import { createProviderHealthStore } from "@corbits/connections/provider-health"; import { HubApiError } from "@corbits/hub-api-client"; @@ -537,3 +538,82 @@ describe("POST /complete", () => { ).toBe(true); }); }); + +// CL-7506: the connect route runs the real fast half — which resolves the +// tenant through findPersonalTenant with the fallback flag — so a seeded +// admin whose only membership is the root bench (slug "acme", never the +// computed "alice-user1") must connect, not 409 no_personal_bench. The +// persistence half is stubbed at its own seams; the hub serves the reads +// the fast half itself performs. +describe("POST /complete — seeded-admin fallback", () => { + test("connects onto the root bench when no principal matches the personal slug", async () => { + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ + data: [ + { + principalId: "prn_root", + tenantId: "ten_root", + tenantName: "acme", + tenantSlug: "acme", + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }), + ); + hub.get("/api/tenants/ten_root", (c) => + c.json({ + id: "ten_root", + name: "acme", + slug: "acme", + domain: "acme.bench.local", + parentId: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }), + ); + hub.get("/api/tenants/ten_root/assets", (c) => c.json([])); + hub.get("/api/tenants/ten_root/workflows/deployments", (c) => c.json([])); + 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, + testAndPersistCredentialFn: (args) => + testAndPersistCredential({ + ...args, + ensureProviderFn: async (_api, _cookies, seedArgs) => + `prv_${seedArgs.name}`, + ensureCredentialFn: async (_api, _cookies, persistArgs) => + `cred_${persistArgs.providerId}`, + seedCatalogFn: async () => ({ hasCompletionCapableModel: true }), + }), + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/complete", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ provider: "anthropic", apiKey: "sk-ant-x" }), + }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { + kind: string; + tenantSlug: string; + }; + expect(body.kind).toBe("provisioning"); + expect(body.tenantSlug).toBe("acme"); + } finally { + server.stop(true); + } + }); +});