From 8b221bdb4573964f700a158fe75db294d8ac24bb Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:22:42 -0700 Subject: [PATCH 1/3] Add tests for env-plant recognizing a Settings-connected credential A tenant that connects a provider in Settings names the credential after the connector's displayName ("Anthropic"), not this module's own "anthropic-default" convention, so the existing name-only match misses it. Covers the case: an active credential named "Anthropic" under the same provider must stop the env-plant from probing or creating a second row. --- .../test/plant-env-credentials.test.ts | 183 ++++++++++++++---- 1 file changed, 145 insertions(+), 38 deletions(-) diff --git a/packages/onboarding/test/plant-env-credentials.test.ts b/packages/onboarding/test/plant-env-credentials.test.ts index 99a4f41fc..20963f76f 100644 --- a/packages/onboarding/test/plant-env-credentials.test.ts +++ b/packages/onboarding/test/plant-env-credentials.test.ts @@ -13,43 +13,109 @@ function collector() { return { lines, log: (line: string) => lines.push(line) }; } -/** A minimal credentials-list ApiCall: answers - * `GET /api/tenants/:id/credentials` with whatever `active` set holds, - * and 404s everything else — `plantEnvProviderCredentials` never calls - * anything else through `api` itself (the plant/probe indirections are - * always passed as fakes in these tests). */ +/** The curated provider key a fake credential row's name belongs to — + * every fixture in this suite either uses the env-plant's own + * `-default` naming convention or (for the + * Settings-connected-credential tests) supplies its own explicit + * `providerKey`, so a bare fixture name is always `-default`. */ +function providerKeyOf(name: string): string { + return name.replace(/-default$/, ""); +} + +type CredentialFixture = { + readonly name: string; + readonly providerKey?: string; +}; + +function fixture(name: string): CredentialFixture { + return { name }; +} + +function providerRow(providerKey: string) { + return { + id: `prov_${providerKey}`, + tenantId: TENANT_ID, + name: providerKey, + plugin: "http", + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; +} + +function credentialRow( + fixture: CredentialFixture, + status: "active" | "revoked", + idPrefix: string, +) { + const providerKey = fixture.providerKey ?? providerKeyOf(fixture.name); + return { + id: `${idPrefix}${fixture.name}`, + tenantId: TENANT_ID, + providerId: `prov_${providerKey}`, + name: fixture.name, + type: "api_key", + status, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }; +} + +/** A minimal credentials-and-providers-list ApiCall: answers + * `GET /api/tenants/:id/credentials` with whatever `active`/`revoked` + * fixtures hold, `GET /api/tenants/:id/providers` with the provider row + * each fixture's `providerKey` belongs to, and 404s everything else — + * `plantEnvProviderCredentials` never calls anything else through `api` + * itself (the plant/probe indirections are always passed as fakes in + * these tests). */ function credentialsApi( - activeNames: Set, - revokedNames: Set = new Set(), + active: ReadonlySet | readonly CredentialFixture[], + revoked: ReadonlySet | readonly CredentialFixture[] = [], ): ApiCall { + // Read `active`/`revoked` fresh on every call rather than snapshotting + // once: several tests mutate the `Set` they passed in (mirroring a + // real `seedCatalog`'s side effect) after the fake is built, and rely + // on the next call seeing that mutation. + function currentFixtures( + entries: ReadonlySet | readonly CredentialFixture[], + ): CredentialFixture[] { + return [...entries].map((entry) => + typeof entry === "string" ? fixture(entry) : entry, + ); + } return async (method, path) => { if ( method === "GET" && path.startsWith(`/api/tenants/${TENANT_ID}/credentials`) ) { - const active = [...activeNames].map((name) => ({ - id: `cred_${name}`, - tenantId: TENANT_ID, - providerId: `prov_${name}`, - name, - type: "api_key", - status: "active", - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", - })); - const revoked = [...revokedNames].map((name) => ({ - id: `cred_revoked_${name}`, - tenantId: TENANT_ID, - providerId: `prov_${name}`, - name, - type: "api_key", - status: "revoked", - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", - })); + const rows = [ + ...currentFixtures(active).map((f) => + credentialRow(f, "active", "cred_"), + ), + ...currentFixtures(revoked).map((f) => + credentialRow(f, "revoked", "cred_revoked_"), + ), + ]; + return { + status: 200, + data: { data: rows, nextCursor: null }, + cookies: [], + }; + } + if ( + method === "GET" && + path.startsWith(`/api/tenants/${TENANT_ID}/providers`) + ) { + const providerKeys = new Set( + [...currentFixtures(active), ...currentFixtures(revoked)].map( + (f) => f.providerKey ?? providerKeyOf(f.name), + ), + ); return { status: 200, - data: { data: [...active, ...revoked], nextCursor: null }, + data: { + data: [...providerKeys].map((key) => providerRow(key)), + nextCursor: null, + }, cookies: [], }; } @@ -61,7 +127,21 @@ function credentialsApi( * the target row lives on page two, so a caller that only reads page * one would never see it. */ function paginatedCredentialsApi(pages: string[][]): ApiCall { + const providerKeys = new Set(pages.flat().map(providerKeyOf)); return async (method, path) => { + if ( + method === "GET" && + path.startsWith(`/api/tenants/${TENANT_ID}/providers`) + ) { + return { + status: 200, + data: { + data: [...providerKeys].map((key) => providerRow(key)), + nextCursor: null, + }, + cookies: [], + }; + } if (!( method === "GET" && path.startsWith(`/api/tenants/${TENANT_ID}/credentials`) @@ -77,16 +157,9 @@ function paginatedCredentialsApi(pages: string[][]): ApiCall { return { status: 200, data: { - data: names.map((name) => ({ - id: `cred_${name}`, - tenantId: TENANT_ID, - providerId: `prov_${name}`, - name, - type: "api_key", - status: "active", - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", - })), + data: names.map((name) => + credentialRow(fixture(name), "active", "cred_"), + ), nextCursor, }, cookies: [], @@ -243,6 +316,40 @@ describe("plantEnvProviderCredentials", () => { expect(lines[0]).not.toContain("sk-ant-rotated"); }); + test("a Settings-connected credential under the same provider is recognized, so booting with the env key skips the probe and never creates a second credential row", async () => { + const { log, lines } = collector(); + let probed = false; + const seedCatalogCalls: SeedCatalogArgs[] = []; + const outcomes = await plantEnvProviderCredentials({ + // Named "Anthropic" (the connector's displayName, the way + // `persistConnectorCredential` names a Settings-connected + // credential) rather than this module's own "anthropic-default" — + // both belong to the same `prov_anthropic` provider row. + api: credentialsApi([{ name: "Anthropic", providerKey: "anthropic" }]), + cookies: [], + tenantId: TENANT_ID, + envProviderKeys: { anthropic: "sk-ant-from-env" }, + log, + testCredential: async () => { + probed = true; + return { ok: true }; + }, + seedCatalogFn: async (args) => { + seedCatalogCalls.push(args); + return { hasCompletionCapableModel: true }; + }, + }); + + expect(outcomes).toEqual([{ provider: "anthropic", status: "skipped" }]); + expect(probed).toBe(false); + expect(seedCatalogCalls).toHaveLength(1); + expect(seedCatalogCalls[0]?.existingCredentialId).toBe("cred_Anthropic"); + expect(seedCatalogCalls[0]?.apiKey).toBeUndefined(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("Anthropic"); + expect(lines[0]).not.toContain("sk-ant-from-env"); + }); + test("a catalog backfill failure on an already-active credential is reported, never thrown", async () => { const { log, lines } = collector(); const outcomes = await plantEnvProviderCredentials({ From 01e8fd33066be90f7e0bb05af6974cc2886e01e0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:22:48 -0700 Subject: [PATCH 2/3] Env-plant: recognize any active credential on the same provider row plantEnvProviderCredentials matched an existing plant only by the literal name inferenceCredentialName(provider) ("anthropic-default"), but a Settings-connected credential is named after the connector's displayName ("Anthropic") instead. A tenant that connected Anthropic in Settings, on a hub booted with ANTHROPIC_API_KEY, never matched - a live probe call and a second credential row got planted on every boot. Match on the provider row (providerId) both paths key their credential to, resolved read-only so a provider nobody has connected yet isn't planted just to check. Fixes CL-7128. --- .../onboarding/src/plant-env-credentials.ts | 84 ++++++++++++++++--- 1 file changed, 74 insertions(+), 10 deletions(-) diff --git a/packages/onboarding/src/plant-env-credentials.ts b/packages/onboarding/src/plant-env-credentials.ts index 8f20b544b..27436cf43 100644 --- a/packages/onboarding/src/plant-env-credentials.ts +++ b/packages/onboarding/src/plant-env-credentials.ts @@ -21,7 +21,11 @@ // one bad or rate-limited key must never stop every other provider from // planting, and must never stop the hub itself from starting. -import { CredentialResponse, paginatedSchema } from "@intx/types"; +import { + CredentialResponse, + paginatedSchema, + ProviderResponse, +} from "@intx/types"; import { inferenceCredentialName, OLLAMA_PLACEHOLDER_SECRET, @@ -155,13 +159,53 @@ export type PlantEnvProviderCredentialsArgs = { seedCatalogFn?: (args: SeedCatalogArgs) => ReturnType; }; -async function findActiveCredential( +/** + * Looks up the provider row a curated provider's connections (env-plant + * and a Settings connect alike) both key their credential to — + * `persistConnectorCredential` and `seedCatalog`'s own `plantCredential` + * both `ensureProvider` this exact `{ name: provider }` pair, so a + * provider row's existence here means some path already connected this + * provider. Read-only: unlike `ensureProvider`, this never creates the + * row, so a provider nobody has connected yet correctly reads back as + * "no active credential" without planting a stub row ahead of a probe + * that might still fail. + */ +async function findProviderId( api: ApiCall, cookies: string[], tenantId: string, provider: SupportedCredentialProvider, -): Promise<{ id: string } | undefined> { - const name = inferenceCredentialName(provider); +): Promise { + const listed = await api( + "GET", + `/api/tenants/${tenantId}/providers?inherited=false`, + undefined, + cookies, + ); + const providers = parseAs( + paginatedSchema(ProviderResponse), + listed.data, + "providers response", + ).data; + return providers.find((p) => p.name === provider)?.id; +} + +/** + * An active credential is recognized by the provider row it belongs to + * (`providerId`), not by the credential's own name — a Settings-connected + * credential is named after the connector's `displayName` ("Anthropic"), + * while this module's own plant names its row + * `inferenceCredentialName(provider)` ("anthropic-default"). Both + * resolve to the same provider row (`findProviderId`), so matching on + * `providerId` recognizes either one instead of only the env-plant's own + * naming convention. + */ +async function findActiveCredential( + api: ApiCall, + cookies: string[], + tenantId: string, + providerId: string, +): Promise<{ id: string; name: string } | undefined> { let cursor: string | undefined; do { const path = @@ -175,9 +219,9 @@ async function findActiveCredential( "credentials response", ); const match = page.data.find( - (c) => c.name === name && c.status === "active", + (c) => c.providerId === providerId && c.status === "active", ); - if (match !== undefined) return { id: match.id }; + if (match !== undefined) return { id: match.id, name: match.name }; cursor = page.nextCursor ?? undefined; } while (cursor !== undefined); return undefined; @@ -245,14 +289,23 @@ export async function plantEnvProviderCredentials( SupportedCredentialProvider, string, ][]) { - const alreadyActive = await findActiveCredential( + const providerId = await findProviderId( args.api, args.cookies, args.tenantId, provider, ); + const alreadyActive = + providerId !== undefined + ? await findActiveCredential( + args.api, + args.cookies, + args.tenantId, + providerId, + ) + : undefined; if (alreadyActive) { - const name = inferenceCredentialName(provider); + const name = alreadyActive.name; try { await runSeedCatalog( catalogSeedArgs(provider, { @@ -302,13 +355,24 @@ export async function plantEnvProviderCredentials( // same name silently blocks the proven env key from ever being // stored. `findActiveCredential` already ruled out an *active* // credential before the probe; re-checking now is the only way to - // tell "planted" apart from "409-skipped against a dead row". - const nowActive = await findActiveCredential( + // tell "planted" apart from "409-skipped against a dead row". The + // provider row is guaranteed to exist by now — `seedCatalog` just + // `ensureProvider`d it while planting. + const nowProviderId = await findProviderId( args.api, args.cookies, args.tenantId, provider, ); + const nowActive = + nowProviderId !== undefined + ? await findActiveCredential( + args.api, + args.cookies, + args.tenantId, + nowProviderId, + ) + : undefined; if (!nowActive) { const name = inferenceCredentialName(provider); args.log( From 2573f161f4dfc5e84ea87cc5b400d88879d4520e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:47:49 -0700 Subject: [PATCH 3/3] Env-plant: paginate the provider lookup and match only inference credentials findProviderId read only page one of the providers list, unlike its sibling findActiveCredential, so a match on a later page was missed. The active-credential match also ignored credential type, so a non-inference row on the same provider could be mistaken for the plant. Paginate findProviderId the same way, and restrict the match to the types seedCatalog itself ever writes for an inference source (api_key, oauth_token). --- .../onboarding/src/plant-env-credentials.ts | 44 ++++-- .../test/plant-env-credentials.test.ts | 131 +++++++++++++++++- 2 files changed, 156 insertions(+), 19 deletions(-) diff --git a/packages/onboarding/src/plant-env-credentials.ts b/packages/onboarding/src/plant-env-credentials.ts index 27436cf43..e63111097 100644 --- a/packages/onboarding/src/plant-env-credentials.ts +++ b/packages/onboarding/src/plant-env-credentials.ts @@ -168,7 +168,9 @@ export type PlantEnvProviderCredentialsArgs = { * provider. Read-only: unlike `ensureProvider`, this never creates the * row, so a provider nobody has connected yet correctly reads back as * "no active credential" without planting a stub row ahead of a probe - * that might still fail. + * that might still fail. Paginated the same way `findActiveCredential` + * is — a tenant with enough providers to span a page must not lose a + * match that lands on page two. */ async function findProviderId( api: ApiCall, @@ -176,18 +178,23 @@ async function findProviderId( tenantId: string, provider: SupportedCredentialProvider, ): Promise { - const listed = await api( - "GET", - `/api/tenants/${tenantId}/providers?inherited=false`, - undefined, - cookies, - ); - const providers = parseAs( - paginatedSchema(ProviderResponse), - listed.data, - "providers response", - ).data; - return providers.find((p) => p.name === provider)?.id; + let cursor: string | undefined; + do { + const path = + cursor === undefined + ? `/api/tenants/${tenantId}/providers?inherited=false` + : `/api/tenants/${tenantId}/providers?inherited=false&cursor=${encodeURIComponent(cursor)}`; + const listed = await api("GET", path, undefined, cookies); + const page = parseAs( + paginatedSchema(ProviderResponse), + listed.data, + "providers response", + ); + const match = page.data.find((p) => p.name === provider); + if (match !== undefined) return match.id; + cursor = page.nextCursor ?? undefined; + } while (cursor !== undefined); + return undefined; } /** @@ -198,7 +205,11 @@ async function findProviderId( * `inferenceCredentialName(provider)` ("anthropic-default"). Both * resolve to the same provider row (`findProviderId`), so matching on * `providerId` recognizes either one instead of only the env-plant's own - * naming convention. + * naming convention. Restricted to the credential types `seedCatalog` + * itself ever writes for an inference source (`api_key`, `oauth_token`) + * so a non-inference row that happens to share the provider (never + * planted by either path today, but not a case this match should ever + * be fooled by) can't count as the plant. */ async function findActiveCredential( api: ApiCall, @@ -219,7 +230,10 @@ async function findActiveCredential( "credentials response", ); const match = page.data.find( - (c) => c.providerId === providerId && c.status === "active", + (c) => + c.providerId === providerId && + c.status === "active" && + (c.type === "api_key" || c.type === "oauth_token"), ); if (match !== undefined) return { id: match.id, name: match.name }; cursor = page.nextCursor ?? undefined; diff --git a/packages/onboarding/test/plant-env-credentials.test.ts b/packages/onboarding/test/plant-env-credentials.test.ts index 20963f76f..0240c86b4 100644 --- a/packages/onboarding/test/plant-env-credentials.test.ts +++ b/packages/onboarding/test/plant-env-credentials.test.ts @@ -25,6 +25,10 @@ function providerKeyOf(name: string): string { type CredentialFixture = { readonly name: string; readonly providerKey?: string; + /** Defaults to "api_key" — the type every fixture in this suite has + * used until the non-inference-type test, which sets this to a type + * `findActiveCredential` must not recognize as the plant. */ + readonly type?: "api_key" | "oauth_token" | "certificate" | "other"; }; function fixture(name: string): CredentialFixture { @@ -53,7 +57,7 @@ function credentialRow( tenantId: TENANT_ID, providerId: `prov_${providerKey}`, name: fixture.name, - type: "api_key", + type: fixture.type ?? "api_key", status, createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", @@ -68,15 +72,18 @@ function credentialRow( * itself (the plant/probe indirections are always passed as fakes in * these tests). */ function credentialsApi( - active: ReadonlySet | readonly CredentialFixture[], - revoked: ReadonlySet | readonly CredentialFixture[] = [], + active: + ReadonlySet | readonly CredentialFixture[], + revoked: + ReadonlySet | readonly CredentialFixture[] = [], ): ApiCall { // Read `active`/`revoked` fresh on every call rather than snapshotting // once: several tests mutate the `Set` they passed in (mirroring a // real `seedCatalog`'s side effect) after the fake is built, and rely // on the next call seeing that mutation. function currentFixtures( - entries: ReadonlySet | readonly CredentialFixture[], + entries: + ReadonlySet | readonly CredentialFixture[], ): CredentialFixture[] { return [...entries].map((entry) => typeof entry === "string" ? fixture(entry) : entry, @@ -167,6 +174,53 @@ function paginatedCredentialsApi(pages: string[][]): ApiCall { }; } +/** A paginated providers-list ApiCall for the findProviderId + * follow-nextCursor test: the target provider row lives on page two, + * paired with a single active credential already sitting on that + * provider so the test can assert the plant is recognized. */ +function paginatedProvidersApi( + pages: string[][], + activeCredentialName: string, +): ApiCall { + return async (method, path) => { + if ( + method === "GET" && + path.startsWith(`/api/tenants/${TENANT_ID}/providers`) + ) { + const url = new URL(path, "http://hub.test"); + const cursor = url.searchParams.get("cursor"); + const pageIndex = cursor === null ? 0 : Number(cursor); + const names = pages[pageIndex] ?? []; + const nextCursor = + pageIndex + 1 < pages.length ? String(pageIndex + 1) : null; + return { + status: 200, + data: { + data: names.map((key) => providerRow(key)), + nextCursor, + }, + cookies: [], + }; + } + if ( + method === "GET" && + path.startsWith(`/api/tenants/${TENANT_ID}/credentials`) + ) { + return { + status: 200, + data: { + data: [ + credentialRow(fixture(activeCredentialName), "active", "cred_"), + ], + nextCursor: null, + }, + cookies: [], + }; + } + throw new Error(`unexpected call: ${method} ${path}`); + }; +} + describe("envProviderKeysFrom", () => { test("reads every curated provider's conventional env var", () => { const keys = envProviderKeysFrom({ @@ -617,4 +671,73 @@ describe("plantEnvProviderCredentials", () => { "cred_anthropic-default", ); }); + + test("findProviderId follows nextCursor instead of reading only page one", async () => { + const { log } = collector(); + let probed = false; + const seedCatalogCalls: SeedCatalogArgs[] = []; + const outcomes = await plantEnvProviderCredentials({ + // The "anthropic" provider row lives on page two only. + api: paginatedProvidersApi( + [["other-provider"], ["anthropic"]], + "anthropic-default", + ), + cookies: [], + tenantId: TENANT_ID, + envProviderKeys: { anthropic: "sk-ant-real" }, + log, + testCredential: async () => { + probed = true; + return { ok: true as const }; + }, + seedCatalogFn: async (args) => { + seedCatalogCalls.push(args); + return { hasCompletionCapableModel: true }; + }, + }); + + expect(outcomes).toEqual([{ provider: "anthropic", status: "skipped" }]); + expect(probed).toBe(false); + expect(seedCatalogCalls[0]?.existingCredentialId).toBe( + "cred_anthropic-default", + ); + }); + + test("a non-inference credential type on the same provider is not recognized as the plant", async () => { + const { log } = collector(); + let probed = false; + const seedCatalogCalls: SeedCatalogArgs[] = []; + // An active "certificate"-typed row already sits on the anthropic + // provider — neither write path plants one of these today, but the + // match must not be fooled by it into skipping the probe. + const active = new Set([ + { + name: "anthropic-legacy-cert", + providerKey: "anthropic", + type: "certificate", + }, + ]); + const outcomes = await plantEnvProviderCredentials({ + api: credentialsApi(active), + cookies: [], + tenantId: TENANT_ID, + envProviderKeys: { anthropic: "sk-ant-real" }, + log, + testCredential: async () => { + probed = true; + return { ok: true as const }; + }, + seedCatalogFn: async (args) => { + seedCatalogCalls.push(args); + // Mirrors the real `seedCatalog`'s side effect: a fresh plant + // actually stores an active `api_key` credential under this name. + active.add({ name: "anthropic-default", providerKey: "anthropic" }); + return { hasCompletionCapableModel: true }; + }, + }); + + expect(probed).toBe(true); + expect(outcomes).toEqual([{ provider: "anthropic", status: "planted" }]); + expect(seedCatalogCalls[0]?.apiKey).toBe("sk-ant-real"); + }); });