PAP-10287 close teams catalog review gaps

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-06-05 04:25:21 +00:00
1 parent 4234c9bbea
commit 43305ad18f
5 files changed
+94 -19

No files matched your search

+8 -2
View File
@@ -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" },
}),
}),
);
+47 -8
View File
@@ -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 <strategy>", "Import collision strategy (rename, skip, replace)")
.option("--name-override <slug=name>", "Override an imported entity name; may be repeated", collectOptionValue, [] as string[])
.option("--selected-file <path>", "Restrict install to selected portable file; may be repeated", collectOptionValue, [] as string[])
.option("--secret-value <key=value>", "Secret env input value for install; may be repeated", collectOptionValue, [] as string[])
.option("--adapter-override <slug=type>", "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<string, string
if (!values || values.length === 0) return undefined;
const result: Record<string, string> = {};
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<string, string
return result;
}
function parseSecretValues(values: string[] | undefined): Record<string, string> | undefined {
if (!values || values.length === 0) return undefined;
const result: Record<string, string> = {};
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<CatalogTeamInstallOptions["adapterOverrides"]> = {};
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<T extends Record<string, unknown>>(input: T): T {
return Object.fromEntries(Object.entries(input).filter(([, value]) => value !== undefined)) as T;
}
@@ -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,
@@ -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);
+24 -8
View File
@@ -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;