From 43305ad18f277b729549fd4bde5f167d99538de7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 5 Jun 2026 04:25:21 +0000 Subject: [PATCH] PAP-10287 close teams catalog review gaps Co-Authored-By: Paperclip --- cli/src/__tests__/teams.test.ts | 10 +++- cli/src/commands/client/teams.ts | 55 ++++++++++++++++--- .../shared/src/validators/teams-catalog.ts | 2 +- .../__tests__/teams-catalog-service.test.ts | 14 +++++ server/src/services/teams-catalog.ts | 32 ++++++++--- 5 files changed, 94 insertions(+), 19 deletions(-) diff --git a/cli/src/__tests__/teams.test.ts b/cli/src/__tests__/teams.test.ts index d4213e50384..defbeb48425 100644 --- a/cli/src/__tests__/teams.test.ts +++ b/cli/src/__tests__/teams.test.ts @@ -330,8 +330,12 @@ describe("teams CLI commands", () => { "agents/cto/AGENTS.md", "--collision-strategy", "skip", + "--secret-value", + "agent:cto:OPENAI_API_KEY=sk-test", + "--adapter-override", + "cto=opencode_local", "--company-id", - "company-1", + "company/1", "--api-base", "http://paperclip.test", "--api-key", @@ -340,13 +344,15 @@ describe("teams CLI commands", () => { ]); expect(fetchMock).toHaveBeenCalledWith( - "http://paperclip.test/api/companies/company-1/teams/catalog/ref/install?ref=product-engineering", + "http://paperclip.test/api/companies/company%2F1/teams/catalog/ref/install?ref=product-engineering", expect.objectContaining({ method: "POST", body: JSON.stringify({ agents: ["cto"], collisionStrategy: "skip", selectedFiles: ["agents/cto/AGENTS.md"], + adapterOverrides: { cto: { adapterType: "opencode_local" } }, + secretValues: { "agent:cto:OPENAI_API_KEY": "sk-test" }, }), }), ); diff --git a/cli/src/commands/client/teams.ts b/cli/src/commands/client/teams.ts index 127a6063a6b..e5acdde6ed4 100644 --- a/cli/src/commands/client/teams.ts +++ b/cli/src/commands/client/teams.ts @@ -45,6 +45,8 @@ interface TeamPreviewOptions extends BaseClientOptions { interface TeamInstallOptions extends TeamPreviewOptions { requestApprovalOnForbidden?: boolean; approvalIssueId?: string; + secretValue?: string[]; + adapterOverride?: string[]; } interface TeamInstallApprovalFallbackResult { @@ -206,6 +208,8 @@ export function registerTeamCommands(program: Command): void { .option("--collision-strategy ", "Import collision strategy (rename, skip, replace)") .option("--name-override ", "Override an imported entity name; may be repeated", collectOptionValue, [] as string[]) .option("--selected-file ", "Restrict install to selected portable file; may be repeated", collectOptionValue, [] as string[]) + .option("--secret-value ", "Secret env input value for install; may be repeated", collectOptionValue, [] as string[]) + .option("--adapter-override ", "Adapter type override for an imported agent slug; may be repeated", collectOptionValue, [] as string[]) .option("--allow-external-sources", "Allow GitHub, URL, or skills.sh skill sources declared by the catalog team", false) .option("--allow-unpinned-optional-sources", "Allow optional-team external skill sources that are not pinned to a commit", false) .option("--allow-local-path-sources", "Development only: allow local-path skill sources declared by the catalog team", false) @@ -342,7 +346,7 @@ async function getCatalogTeamFile( function catalogTeamCompanyPath(companyId: string | undefined, catalogRef: string, action: "preview" | "install") { if (!companyId) throw new Error("Company ID is required."); const params = new URLSearchParams({ ref: catalogRef.trim() }); - return `/api/companies/${companyId}/teams/catalog/ref/${action}?${params.toString()}`; + return `/api/companies/${encodeURIComponent(companyId)}/teams/catalog/ref/${action}?${params.toString()}`; } function buildTeamOptions(opts: TeamPreviewOptions): CatalogTeamImportOptions { @@ -358,7 +362,11 @@ function buildTeamOptions(opts: TeamPreviewOptions): CatalogTeamImportOptions { } function buildTeamInstallOptions(opts: TeamInstallOptions): CatalogTeamInstallOptions { - return buildTeamOptions(opts); + return removeUndefined({ + ...buildTeamOptions(opts), + adapterOverrides: parseAdapterOverrides(opts.adapterOverride), + secretValues: parseSecretValues(opts.secretValue), + }); } const INSTALL_APPROVAL_FALLBACK_MESSAGES = [ @@ -448,12 +456,7 @@ function parseNameOverrides(values: string[] | undefined): Record = {}; for (const raw of values) { - const separator = raw.indexOf("="); - if (separator <= 0) { - throw new Error(`Invalid --name-override "${raw}". Use slug=name.`); - } - const slug = raw.slice(0, separator).trim(); - const name = raw.slice(separator + 1).trim(); + const [slug, name] = parseKeyValueOption(raw, "--name-override", "slug=name"); if (!slug || !name) { throw new Error(`Invalid --name-override "${raw}". Use slug=name.`); } @@ -462,6 +465,42 @@ function parseNameOverrides(values: string[] | undefined): Record | undefined { + if (!values || values.length === 0) return undefined; + const result: Record = {}; + for (const raw of values) { + const [key, value] = parseKeyValueOption(raw, "--secret-value", "key=value"); + if (!key) { + throw new Error(`Invalid --secret-value "${raw}". Use key=value.`); + } + result[key] = value; + } + return result; +} + +function parseAdapterOverrides( + values: string[] | undefined, +): CatalogTeamInstallOptions["adapterOverrides"] | undefined { + if (!values || values.length === 0) return undefined; + const result: NonNullable = {}; + for (const raw of values) { + const [slug, adapterType] = parseKeyValueOption(raw, "--adapter-override", "slug=type"); + if (!slug || !adapterType) { + throw new Error(`Invalid --adapter-override "${raw}". Use slug=type.`); + } + result[slug] = { adapterType }; + } + return result; +} + +function parseKeyValueOption(raw: string, flag: string, format: string): [string, string] { + const separator = raw.indexOf("="); + if (separator <= 0) { + throw new Error(`Invalid ${flag} "${raw}". Use ${format}.`); + } + return [raw.slice(0, separator).trim(), raw.slice(separator + 1).trim()]; +} + function removeUndefined>(input: T): T { return Object.fromEntries(Object.entries(input).filter(([, value]) => value !== undefined)) as T; } diff --git a/packages/shared/src/validators/teams-catalog.ts b/packages/shared/src/validators/teams-catalog.ts index a61945cfea6..00d3abf923e 100644 --- a/packages/shared/src/validators/teams-catalog.ts +++ b/packages/shared/src/validators/teams-catalog.ts @@ -147,7 +147,7 @@ export const catalogTeamPreviewSchema = z.object({ export const catalogTeamInstallSchema = catalogTeamPreviewSchema.extend({ adapterOverrides: z.record(z.string().min(1), portabilityAdapterOverrideSchema).optional(), secretValues: z.record(z.string().min(1), z.string()).optional(), -}); +}).strict(); export const catalogTeamSkillPreparationSchema = z.object({ type: catalogTeamSkillRequirementTypeSchema, diff --git a/server/src/__tests__/teams-catalog-service.test.ts b/server/src/__tests__/teams-catalog-service.test.ts index 0aeb786787c..11b10fb38e5 100644 --- a/server/src/__tests__/teams-catalog-service.test.ts +++ b/server/src/__tests__/teams-catalog-service.test.ts @@ -236,6 +236,20 @@ describe("teamsCatalogService", () => { expect(mockCompanySkillService.importFromSource).not.toHaveBeenCalled(); }); + it("surfaces post-import catalog skill install failures as warnings", async () => { + mockCompanySkillService.installFromCatalog.mockRejectedValueOnce(new Error("catalog unavailable")); + const svc = teamsCatalogService({} as any); + + const result = await svc.installCatalogTeam("company-1", "core-exec-team"); + + expect(mockCompanyPortabilityService.importBundle).toHaveBeenCalled(); + expect(result.warnings).toEqual( + expect.arrayContaining([ + expect.stringContaining("catalog unavailable"), + ]), + ); + }); + it("injects safe claude_local adapter defaults for every bundled agent when no overrides are supplied", async () => { const svc = teamsCatalogService({} as any); diff --git a/server/src/services/teams-catalog.ts b/server/src/services/teams-catalog.ts index 20f105280f1..93db66319c5 100644 --- a/server/src/services/teams-catalog.ts +++ b/server/src/services/teams-catalog.ts @@ -258,6 +258,10 @@ export function readCatalogTeamProvenance( }; } +function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + export async function resolveCatalogTeamReference(reference: string): Promise<{ team: CatalogTeam | null; ambiguous: boolean }> { const trimmed = reference.trim(); if (!trimmed) return { team: null, ambiguous: false }; @@ -864,20 +868,32 @@ export function teamsCatalogService(db: Db) { const warnings: string[] = []; for (const skill of prepared.skillPreparations) { if (skill.action === "blocked") { - throw unprocessable(skill.reason ?? `Catalog team skill source ${skill.ref} is blocked.`); + warnings.push(skill.reason ?? `Catalog team skill source ${skill.ref} is blocked.`); + continue; } if (skill.action === "catalog_install_required") { - if (!skill.catalogSkillId) throw unprocessable(`Catalog skill requirement ${skill.ref} is missing catalogSkillId.`); - const result = await companySkills.installFromCatalog(companyId, { - catalogSkillId: skill.catalogSkillId, - }); - warnings.push(...result.warnings); + if (!skill.catalogSkillId) { + warnings.push(`Catalog skill requirement ${skill.ref} is missing catalogSkillId.`); + continue; + } + try { + const result = await companySkills.installFromCatalog(companyId, { + catalogSkillId: skill.catalogSkillId, + }); + warnings.push(...result.warnings); + } catch (error) { + warnings.push(`Catalog skill requirement ${skill.ref} could not be installed after team import: ${formatError(error)}`); + } continue; } if (skill.action === "external_import_required") { const source = skill.sourceLocator ?? skill.ref; - const result = await companySkills.importFromSource(companyId, source); - warnings.push(...result.warnings); + try { + const result = await companySkills.importFromSource(companyId, source); + warnings.push(...result.warnings); + } catch (error) { + warnings.push(`External skill source ${source} could not be imported after team import: ${formatError(error)}`); + } } } return warnings;