diff --git a/cli/src/__tests__/teams.test.ts b/cli/src/__tests__/teams.test.ts index 41113566a71..d4213e50384 100644 --- a/cli/src/__tests__/teams.test.ts +++ b/cli/src/__tests__/teams.test.ts @@ -502,6 +502,36 @@ describe("teams CLI commands", () => { expect(errorSpy).not.toHaveBeenCalled(); }); + it("does not request board approval for unrelated forbidden install errors", async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ error: "Agent key cannot access another company" }, 403)); + vi.spyOn(process, "exit").mockImplementation(((code?: string | number | null) => { + throw new Error(`exit:${code ?? 0}`); + }) as typeof process.exit); + + await expect(runCommand([ + "teams", + "install", + "product-engineering", + "--company-id", + "company-1", + "--request-approval-on-forbidden", + "--api-base", + "http://paperclip.test", + "--api-key", + "token", + ])).rejects.toThrow("exit:1"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith( + "http://paperclip.test/api/companies/company-1/teams/catalog/ref/install?ref=product-engineering", + expect.objectContaining({ + method: "POST", + body: JSON.stringify({}), + }), + ); + expect(String(errorSpy.mock.calls[0]?.[0])).toContain("Agent key cannot access another company"); + }); + it("surfaces server blocks for unsafe local-path catalog team sources", async () => { fetchMock.mockResolvedValueOnce(jsonResponse({ error: 'Local path skill source "../unsafe" is development-only and is not allowed for catalog team install.', diff --git a/cli/src/commands/client/company.ts b/cli/src/commands/client/company.ts index bafdbd00d54..682b82ac608 100644 --- a/cli/src/commands/client/company.ts +++ b/cli/src/commands/client/company.ts @@ -1750,11 +1750,11 @@ async function resolveCurrentCompanyId(ctx: { companyId?: string; api: { get( } function isBoardAccessRequiredError(error: unknown): error is ApiRequestError { - return error instanceof ApiRequestError && error.status === 403 && error.message.includes("Board access required"); + return error instanceof ApiRequestError && error.status === 403 && error.message.toLowerCase().includes("board access required"); } function isInstanceAdminRequiredError(error: unknown): error is ApiRequestError { - return error instanceof ApiRequestError && error.status === 403 && error.message.includes("Instance admin"); + return error instanceof ApiRequestError && error.status === 403 && error.message.toLowerCase().includes("instance admin"); } function addCompanyJsonPost(parent: Command, name: string, description: string, pathSuffix: string): void { diff --git a/cli/src/commands/client/teams.ts b/cli/src/commands/client/teams.ts index 2b4b9a1a5d2..127a6063a6b 100644 --- a/cli/src/commands/client/teams.ts +++ b/cli/src/commands/client/teams.ts @@ -361,12 +361,16 @@ function buildTeamInstallOptions(opts: TeamInstallOptions): CatalogTeamInstallOp return buildTeamOptions(opts); } +const INSTALL_APPROVAL_FALLBACK_MESSAGES = [ + "missing permission: agents:create", + "missing permission: can create agents", +]; + function shouldRequestInstallApproval(error: unknown, opts: TeamInstallOptions): error is ApiRequestError { - return Boolean( - (opts.requestApprovalOnForbidden || isPaperclipTaskRun()) && - error instanceof ApiRequestError && - error.status === 403, - ); + if (!(opts.requestApprovalOnForbidden || isPaperclipTaskRun())) return false; + if (!(error instanceof ApiRequestError) || error.status !== 403) return false; + const message = error.message.toLowerCase(); + return INSTALL_APPROVAL_FALLBACK_MESSAGES.some((expected) => message.includes(expected)); } function isPaperclipTaskRun(): boolean { diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index ef05717309e..19990d3e528 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -64,6 +64,7 @@ const assetSvc = { const secretSvc = { create: vi.fn(async () => ({ id: "secret-created" })), + remove: vi.fn(async () => true), normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record) => config), normalizeEnvBindingsForPersistence: vi.fn(async (_companyId: string, env: Record) => env), syncEnvBindingsForTarget: vi.fn(async () => []), @@ -133,6 +134,7 @@ describe("company portability", () => { beforeEach(() => { vi.clearAllMocks(); secretSvc.create.mockResolvedValue({ id: "secret-created" }); + secretSvc.remove.mockResolvedValue(true); secretSvc.normalizeAdapterConfigForPersistence.mockImplementation(async (_companyId, config) => config); secretSvc.normalizeEnvBindingsForPersistence.mockImplementation(async (_companyId, env) => env); secretSvc.syncEnvBindingsForTarget.mockResolvedValue([]); @@ -1501,6 +1503,69 @@ describe("company portability", () => { ); }); + it("removes import secrets created before a later import failure", async () => { + const portability = companyPortabilityService({} as any); + agentSvc.list.mockResolvedValue([]); + secretSvc.create.mockResolvedValueOnce({ id: "secret-created-for-failed-import" }); + agentSvc.create.mockRejectedValueOnce(new Error("agent create failed")); + + await expect(portability.importBundle({ + source: { + type: "inline", + files: { + "COMPANY.md": [ + "---", + "name: Import", + "includes:", + " - agents/coder/AGENTS.md", + "---", + "", + ].join("\n"), + "agents/coder/AGENTS.md": [ + "---", + "name: Coder", + "slug: coder", + "kind: agent", + "---", + "", + "# Coder", + "", + ].join("\n"), + ".paperclip.yaml": [ + "schema: paperclip/v1", + "agents:", + " coder:", + " adapter:", + " type: codex_local", + " config: {}", + " inputs:", + " env:", + " OPENAI_API_KEY:", + " kind: secret", + " requirement: required", + "", + ].join("\n"), + }, + }, + include: { + company: false, + agents: true, + projects: false, + issues: false, + }, + target: { + mode: "existing_company", + companyId: "company-1", + }, + collisionStrategy: "rename", + secretValues: { + "agent:coder:OPENAI_API_KEY": "sk-imported", + }, + }, "user-1")).rejects.toThrow("agent create failed"); + + expect(secretSvc.remove).toHaveBeenCalledWith("secret-created-for-failed-import"); + }); + it("reparents imported roots to pre-existing target managers before resolving imported hierarchy", async () => { const portability = companyPortabilityService({} as any); agentSvc.list.mockResolvedValue([ diff --git a/server/src/__tests__/teams-catalog-service.test.ts b/server/src/__tests__/teams-catalog-service.test.ts index 389d36089f3..5e8af4089db 100644 --- a/server/src/__tests__/teams-catalog-service.test.ts +++ b/server/src/__tests__/teams-catalog-service.test.ts @@ -186,6 +186,46 @@ describe("teamsCatalogService", () => { ); }); + it("forces catalog previews to exclude company metadata even when requested", async () => { + const svc = teamsCatalogService({} as any); + + await svc.previewCatalogTeamImport("company-1", "content-machine", { + include: { company: true, agents: false }, + }); + + expect(mockCompanyPortabilityService.previewImport).toHaveBeenCalledWith( + expect.objectContaining({ + include: expect.objectContaining({ + company: false, + agents: false, + }), + }), + { mode: "agent_safe", sourceCompanyId: "company-1" }, + ); + }); + + it("preflights imports before installing catalog skills", async () => { + mockCompanyPortabilityService.previewImport.mockResolvedValueOnce({ + include: { company: false, agents: true, projects: true, issues: true, skills: true }, + targetCompanyId: "company-1", + targetCompanyName: "Paperclip", + collisionStrategy: "rename", + selectedAgentSlugs: ["ceo"], + plan: { companyAction: "none", agentPlans: [], projectPlans: [], issuePlans: [] }, + manifest: { agents: [], skills: [], projects: [], issues: [], envInputs: [], includes: { company: false, agents: true, projects: true, issues: true, skills: true }, company: null, schemaVersion: 1, generatedAt: new Date().toISOString(), source: null, sidebar: null }, + files: {}, + envInputs: [], + warnings: [], + errors: ["Safe import does not allow process adapter type."], + }); + const svc = teamsCatalogService({} as any); + + await expect(svc.installCatalogTeam("company-1", "core-exec-team")).rejects.toMatchObject({ status: 422 }); + + expect(mockCompanySkillService.installFromCatalog).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.importBundle).not.toHaveBeenCalled(); + }); + 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/routes/teams-catalog.ts b/server/src/routes/teams-catalog.ts index a5eeba3a0a8..bacbc45f0c0 100644 --- a/server/src/routes/teams-catalog.ts +++ b/server/src/routes/teams-catalog.ts @@ -69,7 +69,7 @@ export function teamsCatalogRoutes(db: Db) { category: firstQueryString(req.query.category), q: firstQueryString(req.query.q), }); - res.json(listCatalogTeams(query)); + res.json(await listCatalogTeams(query)); }); router.get("/teams/catalog/:catalogId/files", async (req, res) => { @@ -82,7 +82,7 @@ export function teamsCatalogRoutes(db: Db) { router.get("/teams/catalog/:catalogId", async (req, res) => { assertAuthenticated(req); const catalogRef = firstQueryString(req.query.ref) ?? (req.params.catalogId as string); - res.json(getCatalogTeamOrThrow(catalogRef)); + res.json(await getCatalogTeamOrThrow(catalogRef)); }); router.get("/companies/:companyId/teams/catalog/installed", async (req, res) => { diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index 131500c3d3a..71d046366fa 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -3052,6 +3052,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { envInputs: CompanyPortabilityEnvInput[], secretValues: Record | null | undefined, actorUserId: string | null | undefined, + createdSecretIds: string[] = [], ) { if (envInputs.length === 0) return; const missingRequired = envInputs.filter((input) => { @@ -3088,6 +3089,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { }, { userId: actorUserId ?? null, agentId: null }, ); + createdSecretIds.push(secret.id); writeManifestEnvBinding(manifest, input, { type: "secret_ref", secretId: secret.id, @@ -4385,13 +4387,16 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } return true; }); - await materializeImportEnvInputValues( - targetCompany.id, - sourceManifest, - importEnvInputs, - input.secretValues, - actorUserId, - ); + const createdImportSecretIds: string[] = []; + try { + await materializeImportEnvInputValues( + targetCompany.id, + sourceManifest, + importEnvInputs, + input.secretValues, + actorUserId, + createdImportSecretIds, + ); if (include.company) { const logoPath = sourceManifest.company?.logoPath ?? null; @@ -4950,17 +4955,23 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } } - return { - company: { - id: targetCompany.id, - name: targetCompany.name, - action: companyAction, - }, - agents: resultAgents, - projects: resultProjects, - envInputs: sourceManifest.envInputs ?? [], - warnings, - }; + return { + company: { + id: targetCompany.id, + name: targetCompany.name, + action: companyAction, + }, + agents: resultAgents, + projects: resultProjects, + envInputs: sourceManifest.envInputs ?? [], + warnings, + }; + } catch (error) { + for (const secretId of createdImportSecretIds) { + await secrets.remove(secretId).catch(() => undefined); + } + throw error; + } } return { diff --git a/server/src/services/teams-catalog.ts b/server/src/services/teams-catalog.ts index 3d3042dd0b5..cc0c1bd9ad0 100644 --- a/server/src/services/teams-catalog.ts +++ b/server/src/services/teams-catalog.ts @@ -1,4 +1,3 @@ -import { existsSync, readFileSync, statSync } from "node:fs"; import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -131,50 +130,66 @@ export interface CatalogTeamFileDetail { } const serviceDir = path.dirname(fileURLToPath(import.meta.url)); -const repoRoot = path.resolve(serviceDir, "../../.."); -const catalogPackageRoot = path.join(repoRoot, "packages/teams-catalog"); -const catalogManifestPath = path.join(catalogPackageRoot, "generated/catalog.json"); +const catalogPackageRootCandidates = buildCatalogPackageRootCandidates(); +let catalogPackageRoot = catalogPackageRootCandidates[0]!; +let catalogManifestPath = path.join(catalogPackageRoot, "generated/catalog.json"); let cachedCatalogManifest: { manifest: CatalogManifestFile; + manifestPath: string; mtimeMs: number; size: number; } | 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; +function buildCatalogPackageRootCandidates() { + const configuredRoot = process.env.PAPERCLIP_TEAMS_CATALOG_DIR?.trim(); + const candidates = [ + ...(configuredRoot ? [path.resolve(configuredRoot)] : []), + path.resolve(process.cwd(), "packages/teams-catalog"), + path.resolve(serviceDir, "../../..", "packages/teams-catalog"), + ]; + return Array.from(new Set(candidates)); } -function getCatalogManifest() { - if (!existsSync(catalogManifestPath)) { - throw new Error( - `Teams catalog manifest not found at ${catalogManifestPath}. Run pnpm --filter @paperclipai/teams-catalog build:manifest.`, - ); +async function statCatalogManifest() { + for (const candidateRoot of catalogPackageRootCandidates) { + const candidateManifestPath = path.join(candidateRoot, "generated/catalog.json"); + try { + const stats = await fs.stat(candidateManifestPath); + catalogPackageRoot = candidateRoot; + catalogManifestPath = candidateManifestPath; + return { stats, manifestPath: candidateManifestPath }; + } catch { + // Try the next known runtime layout before reporting all checked paths. + } } - const stats = statSync(catalogManifestPath); + throw new Error( + `Teams catalog manifest not found. Checked: ${catalogPackageRootCandidates.map((root) => path.join(root, "generated/catalog.json")).join(", ")}. Run pnpm --filter @paperclipai/teams-catalog build:manifest.`, + ); +} + +async function getCatalogManifest() { + const { stats, manifestPath } = await statCatalogManifest(); if ( cachedCatalogManifest + && cachedCatalogManifest.manifestPath === manifestPath && cachedCatalogManifest.mtimeMs === stats.mtimeMs && cachedCatalogManifest.size === stats.size ) { return cachedCatalogManifest.manifest; } - const manifest = loadCatalogManifest(); + const manifest = JSON.parse(await fs.readFile(manifestPath, "utf8")) as CatalogManifestFile; cachedCatalogManifest = { manifest, + manifestPath, mtimeMs: stats.mtimeMs, size: stats.size, }; return manifest; } -function getCatalogTeams() { - const manifest = getCatalogManifest(); +async function getCatalogTeams() { + const manifest = await getCatalogManifest(); return manifest.teams.map((team) => ({ ...team, packageName: manifest.packageName, @@ -196,9 +211,9 @@ function searchText(team: CatalogTeam) { ].join("\n").toLowerCase(); } -export function listCatalogTeams(query: CatalogTeamListQuery = {}): CatalogTeam[] { +export async function listCatalogTeams(query: CatalogTeamListQuery = {}): Promise { const normalizedQuery = query.q?.trim().toLowerCase() ?? ""; - return getCatalogTeams() + return (await getCatalogTeams()) .filter((team) => !query.kind || team.kind === query.kind) .filter((team) => !query.category || team.category === query.category) .filter((team) => !normalizedQuery || searchText(team).includes(normalizedQuery)) @@ -242,10 +257,10 @@ export function readCatalogTeamProvenance( }; } -export function resolveCatalogTeamReference(reference: string): { team: CatalogTeam | null; ambiguous: boolean } { +export async function resolveCatalogTeamReference(reference: string): Promise<{ team: CatalogTeam | null; ambiguous: boolean }> { const trimmed = reference.trim(); if (!trimmed) return { team: null, ambiguous: false }; - const teams = getCatalogTeams(); + const teams = await getCatalogTeams(); const exact = teams.find((team) => team.id === trimmed || team.key === trimmed); if (exact) return { team: exact, ambiguous: false }; @@ -256,8 +271,8 @@ export function resolveCatalogTeamReference(reference: string): { team: CatalogT return { team: null, ambiguous: false }; } -export function getCatalogTeamOrThrow(reference: string): CatalogTeam { - const result = resolveCatalogTeamReference(reference); +export async function getCatalogTeamOrThrow(reference: string): Promise { + const result = await resolveCatalogTeamReference(reference); if (result.ambiguous) { throw conflict(`Catalog team slug "${reference}" is ambiguous. Use an id or key.`); } @@ -310,7 +325,7 @@ export async function readCatalogTeamFile( reference: string, relativePath = "TEAM.md", ): Promise { - const team = getCatalogTeamOrThrow(reference); + const team = await getCatalogTeamOrThrow(reference); const resolved = resolveCatalogTeamFile(team, relativePath); if (resolved.fileEntry.kind === "asset") { @@ -414,8 +429,8 @@ function renderSyntheticCompanyMarkdown(team: CatalogTeam) { return lines.join("\n"); } -function catalogProvenance(team: CatalogTeam) { - const manifest = getCatalogManifest(); +async function catalogProvenance(team: CatalogTeam) { + const manifest = await getCatalogManifest(); return { catalogId: team.id, catalogKey: team.key, @@ -428,8 +443,8 @@ function catalogProvenance(team: CatalogTeam) { }; } -function renderCatalogProvenanceYaml(team: CatalogTeam, targetManager: CatalogTargetManagerReference | null) { - const provenance = catalogProvenance(team); +async function renderCatalogProvenanceYaml(team: CatalogTeam, targetManager: CatalogTargetManagerReference | null) { + const provenance = await catalogProvenance(team); const agentSlugs = Array.from(new Set(team.agentSlugs)).sort(); const projectSlugs = Array.from(new Set(team.projectSlugs)).sort(); const taskSlugs = team.files @@ -798,15 +813,16 @@ function buildPortabilityInput( source: CompanyPortabilitySource, options: CatalogTeamImportOptions, ): CompanyPortabilityPreview { + const requestedInclude = options.include ?? {}; return { source, include: { - company: false, agents: true, projects: true, issues: true, skills: true, - ...(options.include ?? {}), + ...requestedInclude, + company: false, }, target: { mode: "existing_company", @@ -853,7 +869,7 @@ export function teamsCatalogService(db: Db) { catalogRef: string, options: CatalogTeamImportOptions = {}, ): Promise { - const team = getCatalogTeamOrThrow(catalogRef); + const team = await getCatalogTeamOrThrow(catalogRef); const warnings: string[] = []; const errors: string[] = []; @@ -877,7 +893,7 @@ export function teamsCatalogService(db: Db) { typeof files[".paperclip.yaml"] === "string" ? parseYamlFrontmatter(files[".paperclip.yaml"]) : {}; - const generatedExtension = parseYamlFrontmatter(renderCatalogProvenanceYaml(team, targetManager)); + const generatedExtension = parseYamlFrontmatter(await renderCatalogProvenanceYaml(team, targetManager)); files[".paperclip.yaml"] = renderYamlFile(mergePlainRecords(existingExtension, generatedExtension)); rewriteAgentCatalogSkillRefs(team, files); @@ -979,10 +995,6 @@ export function teamsCatalogService(db: Db) { throw unprocessable(`Catalog team source preparation failed: ${prepared.errors.join("; ")}`); } - const warnings = [ - ...prepared.warnings, - ...await prepareSkillInstalls(companyId, prepared), - ]; const importInput: CompanyPortabilityImport = { ...buildPortabilityInput(companyId, prepared.source, options), adapterOverrides: withSafeCatalogAdapterDefaults( @@ -991,6 +1003,19 @@ export function teamsCatalogService(db: Db) { ), secretValues: options.secretValues, }; + const importPreview = await portability.previewImport(importInput, { + mode: "agent_safe", + sourceCompanyId: companyId, + }); + if (importPreview.errors.length > 0) { + throw unprocessable(`Catalog team import preview has errors: ${importPreview.errors.join("; ")}`); + } + const warnings = [ + ...prepared.warnings, + ...importPreview.warnings, + `Catalog agents without explicit overrides default to ${DEFAULT_SAFE_CATALOG_ADAPTER_TYPE}. Pass adapterOverrides to use a different supported adapter.`, + ...await prepareSkillInstalls(companyId, prepared), + ]; const result = await portability.importBundle( importInput, options.actor?.userId ?? (options.actor?.actorType === "user" ? options.actor.actorId : null), @@ -1022,7 +1047,7 @@ export function teamsCatalogService(db: Db) { */ async function listInstalledCatalogTeams(companyId: string): Promise { const companyAgents = await agents.list(companyId); - const currentTeams = getCatalogTeams(); + const currentTeams = await getCatalogTeams(); const currentById = new Map(currentTeams.map((team) => [team.id, team])); type Aggregate = {