From 4cdef05fe24f99c5ffd5f1c53d9a7019cd0890bb Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 5 Jun 2026 03:49:03 +0000 Subject: [PATCH] Address team catalog review feedback --- cli/src/commands/client/teams.ts | 58 ++++++++- .../shared/src/validators/teams-catalog.ts | 2 +- server/src/services/teams-catalog.ts | 121 +++++++++++++----- 3 files changed, 138 insertions(+), 43 deletions(-) diff --git a/cli/src/commands/client/teams.ts b/cli/src/commands/client/teams.ts index 730073948e2..f35fcfcac19 100644 --- a/cli/src/commands/client/teams.ts +++ b/cli/src/commands/client/teams.ts @@ -39,7 +39,10 @@ interface TeamPreviewOptions extends BaseClientOptions { allowLocalPathSources?: boolean; } -interface TeamInstallOptions extends TeamPreviewOptions {} +interface TeamInstallOptions extends TeamPreviewOptions { + secretValue?: string[]; + adapterOverride?: string[]; +} export function registerTeamCommands(program: Command): void { const teams = program.command("teams").description("App-shipped team catalog operations"); @@ -192,6 +195,8 @@ export function registerTeamCommands(program: Command): void { .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) + .option("--secret-value ", "Secret/plain env input value for required team install inputs; may be repeated", collectOptionValue, [] as string[]) + .option("--adapter-override ", "Adapter override JSON for an imported agent slug; may be repeated", collectOptionValue, [] as string[]) .action(async (catalogRef: string, opts: TeamInstallOptions) => { try { const ctx = resolveCommandContext(opts, { requireCompany: true }); @@ -320,7 +325,11 @@ function buildTeamOptions(opts: TeamPreviewOptions): CatalogTeamImportOptions { } function buildTeamInstallOptions(opts: TeamInstallOptions): CatalogTeamInstallOptions { - return buildTeamOptions(opts); + return removeUndefined({ + ...buildTeamOptions(opts), + secretValues: parseKeyValueOptions(opts.secretValue, "--secret-value"), + adapterOverrides: parseAdapterOverrides(opts.adapterOverride), + }); } function buildSourcePolicy(opts: TeamPreviewOptions): CatalogTeamSourcePolicy | undefined { @@ -333,19 +342,54 @@ function buildSourcePolicy(opts: TeamPreviewOptions): CatalogTeamSourcePolicy | } function parseNameOverrides(values: string[] | undefined): Record | undefined { + return parseKeyValueOptions(values, "--name-override"); +} + +function parseKeyValueOptions(values: string[] | undefined, flagName: string): Record | undefined { if (!values || values.length === 0) return undefined; const result: Record = {}; for (const raw of values) { const separator = raw.indexOf("="); if (separator <= 0) { - throw new Error(`Invalid --name-override "${raw}". Use slug=name.`); + throw new Error(`Invalid ${flagName} "${raw}". Use key=value.`); + } + const key = raw.slice(0, separator).trim(); + const value = raw.slice(separator + 1).trim(); + if (!key || !value) { + throw new Error(`Invalid ${flagName} "${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 separator = raw.indexOf("="); + if (separator <= 0) { + throw new Error(`Invalid --adapter-override "${raw}". Use slug='{"adapterType":"...","adapterConfig":{}}'.`); } const slug = raw.slice(0, separator).trim(); - const name = raw.slice(separator + 1).trim(); - if (!slug || !name) { - throw new Error(`Invalid --name-override "${raw}". Use slug=name.`); + const json = raw.slice(separator + 1).trim(); + if (!slug || !json) { + throw new Error(`Invalid --adapter-override "${raw}". Use slug='{"adapterType":"...","adapterConfig":{}}'.`); } - result[slug] = name; + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch (error) { + throw new Error(`Invalid --adapter-override JSON for "${slug}": ${error instanceof Error ? error.message : String(error)}`); + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error(`Invalid --adapter-override for "${slug}". Expected an object.`); + } + const override = parsed as NonNullable[string]; + if (typeof override.adapterType !== "string" || override.adapterType.trim().length === 0) { + throw new Error(`Invalid --adapter-override for "${slug}". Missing adapterType.`); + } + result[slug] = override; } return result; } 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/services/teams-catalog.ts b/server/src/services/teams-catalog.ts index f04a96c2030..e532a828667 100644 --- a/server/src/services/teams-catalog.ts +++ b/server/src/services/teams-catalog.ts @@ -143,11 +143,6 @@ let cachedCatalogManifest: { } | null = null; function loadCatalogManifest(): CatalogManifestFile { - if (!existsSync(catalogManifestPath)) { - throw new Error( - `Teams catalog manifest not found at ${catalogManifestPath}. Run pnpm --filter @paperclipai/teams-catalog build:manifest.`, - ); - } return JSON.parse(readFileSync(catalogManifestPath, "utf8")) as CatalogManifestFile; } @@ -336,12 +331,35 @@ function yamlScalar(value: string | number | boolean | null) { return JSON.stringify(value); } -function renderStringArrayYaml(key: string, values: string[]) { - if (values.length === 0) return []; - return [ - `${key}:`, - ...values.map((value) => ` - ${yamlScalar(value)}`), - ]; +function renderYamlValue(key: string, value: unknown, indent = 0): string[] { + const prefix = " ".repeat(indent); + if (Array.isArray(value)) { + if (value.length === 0) return []; + const lines = [`${prefix}${key}:`]; + for (const entry of value) { + if (typeof entry === "string" || typeof entry === "number" || typeof entry === "boolean" || entry === null) { + lines.push(`${prefix} - ${yamlScalar(entry)}`); + continue; + } + if (isPlainRecord(entry)) { + lines.push(`${prefix} -`); + for (const [nestedKey, nestedValue] of Object.entries(entry)) { + lines.push(...renderYamlValue(nestedKey, nestedValue, indent + 4)); + } + } + } + return lines; + } + if (isPlainRecord(value)) { + const nestedLines = Object.entries(value).flatMap(([nestedKey, nestedValue]) => + renderYamlValue(nestedKey, nestedValue, indent + 2), + ); + return nestedLines.length > 0 ? [`${prefix}${key}:`, ...nestedLines] : []; + } + if (typeof value === "string" || typeof value === "number" || typeof value === "boolean" || value === null) { + return [`${prefix}${key}: ${yamlScalar(value)}`]; + } + return []; } function renderSyntheticCompanyMarkdown(team: CatalogTeam) { @@ -457,30 +475,69 @@ function parseYamlFrontmatter(raw: string): Record { content: line.trim(), })) .filter((line) => line.content.length > 0 && !line.content.startsWith("#")); + if (lines.length === 0) return {}; + const parsed = parseYamlBlock(lines, 0, lines[0]!.indent); + return isPlainRecord(parsed.value) ? parsed.value : {}; +} + +function parseYamlBlock( + lines: Array<{ indent: number; content: string }>, + startIndex: number, + indentLevel: number, +): { value: unknown; nextIndex: number } { + let index = startIndex; + if (index >= lines.length || lines[index]!.indent < indentLevel) { + return { value: {}, nextIndex: index }; + } + + const isArray = lines[index]!.indent === indentLevel && lines[index]!.content.startsWith("-"); + if (isArray) { + const values: unknown[] = []; + while (index < lines.length) { + const line = lines[index]!; + if (line.indent < indentLevel) break; + if (line.indent !== indentLevel || !line.content.startsWith("-")) break; + + const remainder = line.content.slice(1).trim(); + index += 1; + if (!remainder) { + const nested = parseYamlBlock(lines, index, indentLevel + 2); + values.push(nested.value); + index = nested.nextIndex; + continue; + } + values.push(parseYamlScalar(remainder)); + } + return { value: values, nextIndex: index }; + } + const record: Record = {}; - for (let index = 0; index < lines.length; index += 1) { + while (index < lines.length) { const line = lines[index]!; - if (line.indent !== 0) continue; - const separatorIndex = line.content.indexOf(":"); - if (separatorIndex <= 0) continue; - const key = line.content.slice(0, separatorIndex).trim(); - const remainder = line.content.slice(separatorIndex + 1).trim(); - if (remainder) { - record[key] = parseYamlScalar(remainder); + if (line.indent < indentLevel) break; + if (line.indent !== indentLevel) { + index += 1; continue; } - const values: string[] = []; - while (index + 1 < lines.length && lines[index + 1]!.indent > line.indent) { - const next = lines[index + 1]!; - if (next.indent === line.indent + 2 && next.content.startsWith("-")) { - const value = next.content.slice(1).trim(); - if (value) values.push(String(parseYamlScalar(value))); - } + + const separatorIndex = line.content.indexOf(":"); + if (separatorIndex <= 0) { index += 1; + continue; } - record[key] = values; + + const key = line.content.slice(0, separatorIndex).trim(); + const remainder = line.content.slice(separatorIndex + 1).trim(); + index += 1; + if (!remainder) { + const nested = parseYamlBlock(lines, index, indentLevel + 2); + record[key] = nested.value; + index = nested.nextIndex; + continue; + } + record[key] = parseYamlScalar(remainder); } - return record; + return { value: record, nextIndex: index }; } function parseFrontmatterMarkdown(raw: string): MarkdownDoc { @@ -502,13 +559,7 @@ function renderSimpleMarkdown(frontmatter: Record, body: string const lines = ["---"]; for (const [key, value] of Object.entries(frontmatter)) { if (value === undefined) continue; - if (Array.isArray(value)) { - lines.push(...renderStringArrayYaml(key, value.filter((entry): entry is string => typeof entry === "string"))); - continue; - } - if (typeof value === "string" || typeof value === "number" || typeof value === "boolean" || value === null) { - lines.push(`${key}: ${yamlScalar(value)}`); - } + lines.push(...renderYamlValue(key, value)); } lines.push("---", ""); const cleanBody = body.trim();