diff --git a/docs/seed-reconciliation.md b/docs/seed-reconciliation.md index beb7b6ae8..6307d7738 100644 --- a/docs/seed-reconciliation.md +++ b/docs/seed-reconciliation.md @@ -129,7 +129,10 @@ A genuine redeploy is the only honest repair. `apps/hub/src/env-credential-plant.ts` delegates to `plantEnvProviderCredentials` (`packages/onboarding`): keyed by the provider's stable credential name, a provider already carrying an -active credential is skipped outright — a rotated or hand-renamed key -is never touched. Removing an env var never deletes the planted -credential: credentials are operator data once planted, not seeds to -garbage-collect. +active credential is not probed and its key is not overwritten — a +rotated or hand-renamed key is never touched. `seedCatalog` still +runs against that existing credential (`existingCredentialId`, no +`apiKey`) so a hub restart backfills newly curated models additively: +missing rows are planted, existing ones 409-skip, nothing is deleted. +Removing an env var never deletes the planted credential: credentials +are operator data once planted, not seeds to garbage-collect. diff --git a/packages/hub-client/src/catalog-seed-data.ts b/packages/hub-client/src/catalog-seed-data.ts index 742bdfd3d..25d52038b 100644 --- a/packages/hub-client/src/catalog-seed-data.ts +++ b/packages/hub-client/src/catalog-seed-data.ts @@ -39,8 +39,9 @@ export type CatalogProviderSpec = { export type CatalogProviderSeed = { readonly provider: CatalogProviderSpec; - /** 2-4 sensible defaults: enough to make the model picker useful, - * never the provider's entire model list. */ + /** Curated defaults for the model picker — enough to be useful, never the + * provider's entire list. Anthropic ships six; other providers typically + * stay in a small 2–5 band. */ readonly models: readonly CatalogModelSpec[]; }; @@ -55,6 +56,14 @@ export const CATALOG_SEEDS: Readonly< }, models: [ { canonicalName: "claude-sonnet-5", displayName: "Claude Sonnet 5" }, + { canonicalName: "claude-opus-5", displayName: "Claude Opus 5" }, + { canonicalName: "claude-opus-4-8", displayName: "Claude Opus 4.8" }, + { + canonicalName: "claude-haiku-4-5-20251001", + displayName: "Claude Haiku 4.5", + }, + { canonicalName: "claude-fable-5", displayName: "Claude Fable 5" }, + { canonicalName: "claude-sonnet-4-6", displayName: "Claude Sonnet 4.6" }, ], }, openai: { diff --git a/packages/hub-client/test/catalog-seed-data.test.ts b/packages/hub-client/test/catalog-seed-data.test.ts index 79e098f73..18da04833 100644 --- a/packages/hub-client/test/catalog-seed-data.test.ts +++ b/packages/hub-client/test/catalog-seed-data.test.ts @@ -1,9 +1,10 @@ // CATALOG_SEEDS is pure data consumed by seedCatalog (seed.ts) to give // every supported credential provider a browsable, launchable model // catalog. These tests guard the shape every entry must hold — one seed -// per SupportedCredentialProvider, 2-4 curated models, and a provider -// spec whose adapter plugin matches credential-test.ts's own mapping — -// so a newly added provider (like xAI) can't silently drift out of sync. +// per SupportedCredentialProvider, a small curated model set (Anthropic +// has six; others typically 2–5), and a provider spec whose adapter +// plugin matches credential-test.ts's own mapping — so a newly added +// provider (like xAI) can't silently drift out of sync. import { describe, expect, test } from "bun:test"; import { @@ -24,11 +25,14 @@ describe("CATALOG_SEEDS", () => { expect(seededProviders).toEqual(supportedProviders); }); - test("every seed lists between 2 and 4 curated models", () => { + test("every non-exception seed stays inside the generic curated-size bound", () => { for (const [provider, seed] of Object.entries(CATALOG_SEEDS) as [ SupportedCredentialProvider, (typeof CATALOG_SEEDS)[SupportedCredentialProvider], ][]) { + // Anthropic is the curated six-model set (Sonnet 5 first); openai and + // google-genai keep their smaller curated lists. Everyone else stays + // inside the generic 2–5 bound. if (provider === "anthropic" || provider === "openai") continue; if (provider === "google-genai") continue; expect(seed.models.length).toBeGreaterThanOrEqual(2); @@ -43,6 +47,26 @@ describe("CATALOG_SEEDS", () => { } }); + test("anthropic seeds Claude models behind the anthropic adapter", () => { + const seed = CATALOG_SEEDS.anthropic; + expect(seed.provider).toEqual({ + name: "anthropic", + plugin: "anthropic", + baseURL: "https://api.anthropic.com", + }); + expect(seed.models.map((m) => m.canonicalName)).toEqual([ + "claude-sonnet-5", + "claude-opus-5", + "claude-opus-4-8", + "claude-haiku-4-5-20251001", + "claude-fable-5", + "claude-sonnet-4-6", + ]); + for (const model of seed.models) { + expect(model.displayName.length).toBeGreaterThan(0); + } + }); + test("xai seeds Grok models behind the openai-compatible adapter", () => { const seed = CATALOG_SEEDS.xai; expect(seed.provider).toEqual({ diff --git a/packages/hub-client/test/seed.test.ts b/packages/hub-client/test/seed.test.ts index 7d137a5c4..d4f672ad0 100644 --- a/packages/hub-client/test/seed.test.ts +++ b/packages/hub-client/test/seed.test.ts @@ -1461,6 +1461,8 @@ describe("seedCatalog", () => { test("fresh run creates the full provider-to-offering chain", async () => { const { lines, log } = collector(); + const modelPosts: string[] = []; + const offeringPosts: { modelId: string; providerId: string }[] = []; const handler: FakeHandler = (method, path, body) => { if (method === "POST" && path === `/api/tenants/${TENANT_ID}/providers`) return { status: 201, data: providerRow("prv_1", "anthropic") }; @@ -1472,11 +1474,14 @@ describe("seedCatalog", () => { if ( method === "POST" && path === `/api/tenants/${TENANT_ID}/catalog/models` - ) + ) { + const canonicalName = (body as { canonicalName: string }).canonicalName; + modelPosts.push(canonicalName); return { status: 201, - data: catalogModelRow("mdl_1", "claude-sonnet-5"), + data: catalogModelRow(`mdl_${modelPosts.length}`, canonicalName), }; + } if ( method === "POST" && path === `/api/tenants/${TENANT_ID}/catalog/providers` @@ -1489,25 +1494,30 @@ describe("seedCatalog", () => { method === "POST" && path === `/api/tenants/${TENANT_ID}/catalog/offerings` ) { - // Anthropic Direct's claude-sonnet-5 is a probed deployment in the - // pinned catalog, so the offering is created carrying what that - // probe observed rather than an empty capability list. + // Every Anthropic Direct model in the curated six is an + // exact-deployment probe in the pinned catalog, so each offering + // carries what that probe observed rather than an empty list. const offeringBody = body as { modelId: string; providerId: string; priority: number; capabilities: string[]; }; - expect(offeringBody.modelId).toBe("mdl_1"); expect(offeringBody.providerId).toBe("cpv_1"); expect(offeringBody.priority).toBe(0); + expect(offeringBody.capabilities.length).toBeGreaterThan(0); expect(offeringBody.capabilities).toContain("plain-text"); expect(offeringBody.capabilities).toContain( "function-calling-multi-turn", ); + offeringPosts.push(offeringBody); return { status: 201, - data: catalogOfferingRow("off_1", "mdl_1", "cpv_1"), + data: catalogOfferingRow( + `off_${offeringPosts.length}`, + offeringBody.modelId, + offeringBody.providerId, + ), }; } return undefined; @@ -1521,13 +1531,32 @@ describe("seedCatalog", () => { log, }); + expect(modelPosts).toEqual([ + "claude-sonnet-5", + "claude-opus-5", + "claude-opus-4-8", + "claude-haiku-4-5-20251001", + "claude-fable-5", + "claude-sonnet-4-6", + ]); + expect(offeringPosts.map((o) => o.modelId)).toEqual([ + "mdl_1", + "mdl_2", + "mdl_3", + "mdl_4", + "mdl_5", + "mdl_6", + ]); + const output = lines.join("\n"); expect(output).toContain("created provider anthropic"); expect(output).toContain("created credential anthropic-default"); expect(output).toContain("created catalog model claude-sonnet-5"); expect(output).toContain("created catalog provider anthropic"); expect(output).toContain("created catalog offering"); - expect(output).toContain("catalog ready: anthropic/claude-sonnet-5"); + expect(output).toContain( + "catalog ready: anthropic/claude-sonnet-5, claude-opus-5, claude-opus-4-8, claude-haiku-4-5-20251001, claude-fable-5, claude-sonnet-4-6", + ); }); test("an Ollama offering's quirks carry that model's real context-window ceiling, not the built-in 4096 default", async () => { @@ -1982,6 +2011,14 @@ describe("seedCatalog", () => { let modelPosts = 0; let catalogProviderPosts = 0; let offeringPosts = 0; + const anthropicModels = [ + "claude-sonnet-5", + "claude-opus-5", + "claude-opus-4-8", + "claude-haiku-4-5-20251001", + "claude-fable-5", + "claude-sonnet-4-6", + ]; const handler: FakeHandler = (method, path) => { if (method === "POST" && path === `/api/tenants/${TENANT_ID}/providers`) { providerPosts += 1; @@ -2024,7 +2061,9 @@ describe("seedCatalog", () => { return { status: 200, data: { - data: [catalogModelRow("mdl_1", "claude-sonnet-5")], + data: anthropicModels.map((name, index) => + catalogModelRow(`mdl_${index + 1}`, name), + ), nextCursor: null, }, }; @@ -2066,9 +2105,9 @@ describe("seedCatalog", () => { expect(providerPosts).toBe(1); expect(credentialPosts).toBe(1); - expect(modelPosts).toBe(1); + expect(modelPosts).toBe(6); expect(catalogProviderPosts).toBe(1); - expect(offeringPosts).toBe(1); + expect(offeringPosts).toBe(6); const output = lines.join("\n"); expect(output).toContain("provider anthropic already exists (skipped)"); diff --git a/packages/onboarding/src/plant-env-credentials.ts b/packages/onboarding/src/plant-env-credentials.ts index 93584cced..8f20b544b 100644 --- a/packages/onboarding/src/plant-env-credentials.ts +++ b/packages/onboarding/src/plant-env-credentials.ts @@ -14,8 +14,9 @@ // `@workbench/hub-client` functions `completeCredentialSetup` calls), // reused here exactly as onboarding's own guided step uses them. This // module's only job is the env-map-to-provider translation, the -// idempotency check that skips a provider already carrying a working -// credential (never overwriting a rotated or renamed key), and folding +// idempotency check that skips overwriting a provider already carrying +// a working credential (never rotating a renamed key), backfills that +// provider's curated catalog additively on every hub boot, and folding // a single provider's failure into a log line instead of an exception — // one bad or rate-limited key must never stop every other provider from // planting, and must never stop the hub itself from starting. @@ -159,7 +160,7 @@ async function findActiveCredential( cookies: string[], tenantId: string, provider: SupportedCredentialProvider, -): Promise { +): Promise<{ id: string } | undefined> { const name = inferenceCredentialName(provider); let cursor: string | undefined; do { @@ -173,11 +174,13 @@ async function findActiveCredential( listed.data, "credentials response", ); - if (page.data.some((c) => c.name === name && c.status === "active")) - return true; + const match = page.data.find( + (c) => c.name === name && c.status === "active", + ); + if (match !== undefined) return { id: match.id }; cursor = page.nextCursor ?? undefined; } while (cursor !== undefined); - return false; + return undefined; } /** @@ -185,9 +188,12 @@ async function findActiveCredential( * present in `envProviderKeys`, at the given tenant, idempotently: * * - A provider already carrying an active credential of that name is - * skipped outright — no live probe, no `seedCatalog` call — so an - * already-rotated or hand-renamed key is never touched, and a hub - * restart never re-probes a provider that already works. + * not probed and its key is not overwritten — an already-rotated or + * hand-renamed key stays put. `seedCatalog` still runs against that + * existing credential (`existingCredentialId`, no `apiKey`) so a + * hub restart backfills newly curated models additively. `seedCatalog` + * is ensure-then-create: missing rows are planted, existing ones + * 409-skip, nothing is deleted. * - A provider with no existing credential is proven with a real, * free call (`testProviderCredential`) before anything is persisted; * a failed probe is reported and skipped, never thrown, so one bad @@ -201,10 +207,9 @@ async function findActiveCredential( * proven key, and this is reported honestly as `"blocked"` — never * logged as planted. * - * Calling this twice with the same env plants nothing the second time: - * every provider it planted on the first call now has an active - * credential, so the second call's per-provider check short-circuits - * to "skipped" before any probe or plant runs. + * Calling this twice with the same env plants the credential once: the + * second call's per-provider check skips probe and key write, then + * backfills the curated catalog against the already-active row. */ export async function plantEnvProviderCredentials( args: PlantEnvProviderCredentialsArgs, @@ -212,6 +217,29 @@ export async function plantEnvProviderCredentials( const testCredential = args.testCredential ?? testProviderCredential; const runSeedCatalog = args.seedCatalogFn ?? seedCatalog; const outcomes: PlantEnvProviderCredentialsOutcome[] = []; + const suppressedLog = () => { + // seedCatalog's own step-by-step log is suppressed here: this + // module reports exactly one summary line per provider below, + // never the per-row created/skipped detail seedCatalog logs for + // its other callers (`workbench seed`, the guided step). + }; + + function catalogSeedArgs( + provider: SupportedCredentialProvider, + extra: + { readonly apiKey: string } | { readonly existingCredentialId: string }, + ): SeedCatalogArgs { + const baseURL = args.envProviderBaseUrls?.[provider]; + return { + api: args.api, + cookies: args.cookies, + tenantId: args.tenantId, + provider, + log: suppressedLog, + ...extra, + ...(baseURL !== undefined ? { baseURLOverride: baseURL } : {}), + }; + } for (const [provider, apiKey] of Object.entries(args.envProviderKeys) as [ SupportedCredentialProvider, @@ -225,8 +253,22 @@ export async function plantEnvProviderCredentials( ); if (alreadyActive) { const name = inferenceCredentialName(provider); + try { + await runSeedCatalog( + catalogSeedArgs(provider, { + existingCredentialId: alreadyActive.id, + }), + ); + } catch (cause) { + const message = cause instanceof Error ? cause.message : String(cause); + args.log( + `env credential plant: ${provider} failed to backfill catalog: ${message}`, + ); + outcomes.push({ provider, status: "failed", message }); + continue; + } args.log( - `env credential plant: ${provider} already has an active credential named ${name} (skipped) — the env key was not planted; rotate the existing ${name} credential in Plugins if you meant to replace it`, + `env credential plant: ${provider} already has an active credential named ${name} (skipped) — the env key was not planted; rotate the existing ${name} credential in Plugins if you meant to replace it. Curated catalog models were backfilled additively.`, ); outcomes.push({ provider, status: "skipped" }); continue; @@ -245,33 +287,8 @@ export async function plantEnvProviderCredentials( continue; } - const suppressedLog = () => { - // seedCatalog's own step-by-step log is suppressed here: this - // module reports exactly one summary line per provider below, - // never the per-row created/skipped detail seedCatalog logs for - // its other callers (`workbench seed`, the guided step). - }; - const seedCatalogArgs: SeedCatalogArgs = - baseURL !== undefined - ? { - api: args.api, - cookies: args.cookies, - tenantId: args.tenantId, - provider, - apiKey, - baseURLOverride: baseURL, - log: suppressedLog, - } - : { - api: args.api, - cookies: args.cookies, - tenantId: args.tenantId, - provider, - apiKey, - log: suppressedLog, - }; try { - await runSeedCatalog(seedCatalogArgs); + await runSeedCatalog(catalogSeedArgs(provider, { apiKey })); } catch (cause) { const message = cause instanceof Error ? cause.message : String(cause); args.log(`env credential plant: ${provider} failed to plant: ${message}`); diff --git a/packages/onboarding/test/complete-credential.test.ts b/packages/onboarding/test/complete-credential.test.ts index 81abe3de0..7b2ccf72e 100644 --- a/packages/onboarding/test/complete-credential.test.ts +++ b/packages/onboarding/test/complete-credential.test.ts @@ -5,6 +5,7 @@ import type { WorkflowPusher, } from "@workbench/hub-client"; import { + CATALOG_SEEDS, SETUP_AGENT_ASSET_NAME, SidecarUnavailableError, } from "@workbench/hub-client"; @@ -1293,12 +1294,15 @@ describe("completeCredentialSetup", () => { // Every ensure-then-create helper hit its 409 branch on the second // pass and listed the row it already created on the first — nothing - // was ever created twice. + // was ever created twice. Anthropic's curated seed is several models + // (one POST each for model and offering); the rest of the chain is + // still a single provider/credential row. + const anthropicCatalogSize = CATALOG_SEEDS.anthropic.models.length; expect(assetCreatePosts).toBe(4); expect(deploymentCreatePosts).toBe(4); - expect(catalogModelCreatePosts).toBe(1); + expect(catalogModelCreatePosts).toBe(anthropicCatalogSize); expect(catalogProviderCreatePosts).toBe(1); - expect(catalogOfferingCreatePosts).toBe(1); + expect(catalogOfferingCreatePosts).toBe(anthropicCatalogSize); expect(credentialCreatePosts).toBe(1); // The second pass is itself an explicit submission and rotates the // existing row rather than leaving it untouched (the CL-6103 fix, diff --git a/packages/onboarding/test/plant-env-credentials.test.ts b/packages/onboarding/test/plant-env-credentials.test.ts index 40133293f..99a4f41fc 100644 --- a/packages/onboarding/test/plant-env-credentials.test.ts +++ b/packages/onboarding/test/plant-env-credentials.test.ts @@ -207,10 +207,10 @@ describe("plantEnvProviderCredentials", () => { expect(lines[0]).not.toContain("sk-ant-real"); }); - test("skips a provider that already has an active credential, without probing or seeding", async () => { + test("an already-active credential skips probe and key rotation, but backfills the curated catalog", async () => { const { log, lines } = collector(); let probed = false; - let seeded = false; + const seedCatalogCalls: SeedCatalogArgs[] = []; const outcomes = await plantEnvProviderCredentials({ api: credentialsApi(new Set(["anthropic-default"])), cookies: [], @@ -221,25 +221,56 @@ describe("plantEnvProviderCredentials", () => { probed = true; return { ok: true }; }, - seedCatalogFn: async () => { - seeded = true; + seedCatalogFn: async (args) => { + seedCatalogCalls.push(args); return { hasCompletionCapableModel: true }; }, }); expect(outcomes).toEqual([{ provider: "anthropic", status: "skipped" }]); expect(probed).toBe(false); - expect(seeded).toBe(false); + expect(seedCatalogCalls).toHaveLength(1); + expect(seedCatalogCalls[0]?.existingCredentialId).toBe( + "cred_anthropic-default", + ); + expect(seedCatalogCalls[0]?.apiKey).toBeUndefined(); expect(lines).toHaveLength(1); expect(lines[0]).toContain("skipped"); - // The rotated env key was never planted, and the message says where - // to fix that. expect(lines[0]).toContain("was not planted"); + expect(lines[0]).toContain("backfilled"); expect(lines[0]).toContain("rotate"); expect(lines[0]).toContain("Plugins"); expect(lines[0]).not.toContain("sk-ant-rotated"); }); + test("a catalog backfill failure on an already-active credential is reported, never thrown", async () => { + const { log, lines } = collector(); + const outcomes = await plantEnvProviderCredentials({ + api: credentialsApi(new Set(["anthropic-default"])), + cookies: [], + tenantId: TENANT_ID, + envProviderKeys: { anthropic: "sk-ant-real" }, + log, + testCredential: async () => { + throw new Error("must not probe an already-active credential"); + }, + seedCatalogFn: async () => { + throw new Error("catalog POST failed"); + }, + }); + + expect(outcomes).toEqual([ + { + provider: "anthropic", + status: "failed", + message: "catalog POST failed", + }, + ]); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("failed to backfill catalog"); + expect(lines[0]).toContain("catalog POST failed"); + }); + test("a failed probe is reported, never persisted, and never thrown", async () => { const { log, lines } = collector(); let seeded = false; @@ -301,6 +332,35 @@ describe("plantEnvProviderCredentials", () => { ); }); + test("an already-active ollama credential still threads its base URL into the catalog backfill", async () => { + const { log } = collector(); + const seedCatalogCalls: SeedCatalogArgs[] = []; + const outcomes = await plantEnvProviderCredentials({ + api: credentialsApi(new Set(["ollama-default"])), + cookies: [], + tenantId: TENANT_ID, + envProviderKeys: { ollama: "ollama" }, + envProviderBaseUrls: { ollama: "https://home-mac.example.ts.net" }, + log, + testCredential: async () => { + throw new Error("must not probe an already-active credential"); + }, + seedCatalogFn: async (args) => { + seedCatalogCalls.push(args); + return { hasCompletionCapableModel: true }; + }, + }); + + expect(outcomes).toEqual([{ provider: "ollama", status: "skipped" }]); + expect(seedCatalogCalls[0]?.existingCredentialId).toBe( + "cred_ollama-default", + ); + expect(seedCatalogCalls[0]?.apiKey).toBeUndefined(); + expect(seedCatalogCalls[0]?.baseURLOverride).toBe( + "https://home-mac.example.ts.net", + ); + }); + test("one provider's failed probe never blocks another provider's plant", async () => { const { log } = collector(); const active = new Set(); @@ -329,12 +389,12 @@ describe("plantEnvProviderCredentials", () => { expect(planted).toEqual(["openai"]); }); - test("calling twice plants once — the second call finds the credential already active", async () => { + test("calling twice plants the credential once and backfills the catalog on every boot", async () => { const { log } = collector(); const active = new Set(); const api = credentialsApi(active); let probeCount = 0; - let seedCount = 0; + const seedCalls: SeedCatalogArgs[] = []; const args = { api, cookies: [], @@ -345,8 +405,8 @@ describe("plantEnvProviderCredentials", () => { probeCount += 1; return { ok: true as const }; }, - seedCatalogFn: async () => { - seedCount += 1; + seedCatalogFn: async (seedArgs: SeedCatalogArgs) => { + seedCalls.push(seedArgs); // A real `seedCatalog` call is what makes the credential active; // the fake mirrors that side effect so the second call's // pre-check sees it. @@ -361,7 +421,10 @@ describe("plantEnvProviderCredentials", () => { expect(first).toEqual([{ provider: "anthropic", status: "planted" }]); expect(second).toEqual([{ provider: "anthropic", status: "skipped" }]); expect(probeCount).toBe(1); - expect(seedCount).toBe(1); + expect(seedCalls).toHaveLength(2); + expect(seedCalls[0]?.apiKey).toBe("sk-ant-real"); + expect(seedCalls[1]?.existingCredentialId).toBe("cred_anthropic-default"); + expect(seedCalls[1]?.apiKey).toBeUndefined(); }); test("a revoked existing credential of the same name blocks the plant instead of being reported as planted", async () => { @@ -420,6 +483,7 @@ describe("plantEnvProviderCredentials", () => { test("findActiveCredential follows nextCursor instead of reading only page one", async () => { const { log } = collector(); let probed = false; + const seedCatalogCalls: SeedCatalogArgs[] = []; const outcomes = await plantEnvProviderCredentials({ // The active "anthropic-default" row lives on page two only. api: paginatedCredentialsApi([ @@ -434,9 +498,16 @@ describe("plantEnvProviderCredentials", () => { 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", + ); }); });