From 9d45f8820df7ecd518ef80e98d4a2a66c01598dc Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 9 May 2026 21:53:57 -0500 Subject: [PATCH] Address LLM Wiki host review feedback Co-Authored-By: Paperclip --- packages/adapter-utils/src/server-utils.ts | 112 +++------- packages/db/src/runtime-config.test.ts | 2 + packages/plugins/sdk/src/testing.ts | 224 +++++++++++++++----- server/src/services/plugin-host-services.ts | 6 - 4 files changed, 199 insertions(+), 145 deletions(-) diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index ee172e3122f..cbaf9002354 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -84,46 +84,6 @@ const DEFAULT_PAPERCLIP_SPACE_ID = "default"; const PAPERCLIP_CONFIG_BASENAME = "config.json"; const PATH_SEGMENT_RE = /^[a-zA-Z0-9_-]+$/; -function expandHomePrefix(value: string): string { - if (value === "~") return os.homedir(); - if (value.startsWith("~/")) return path.resolve(os.homedir(), value.slice(2)); - return value; -} - -function resolvePaperclipHomeDirForAdapter(homeOverride?: string, env: NodeJS.ProcessEnv = process.env): string { - const raw = homeOverride?.trim() || env.PAPERCLIP_HOME?.trim(); - if (raw) return path.resolve(expandHomePrefix(raw)); - return path.resolve(os.homedir(), ".paperclip"); -} - -function resolvePaperclipInstanceIdForAdapter(instanceIdOverride?: string, env: NodeJS.ProcessEnv = process.env): string { - const raw = instanceIdOverride?.trim() || env.PAPERCLIP_INSTANCE_ID?.trim() || DEFAULT_PAPERCLIP_INSTANCE_ID; - if (!PATH_SEGMENT_RE.test(raw)) { - throw new Error(`Invalid PAPERCLIP_INSTANCE_ID '${raw}'.`); - } - return raw; -} - -function resolvePaperclipInstanceRootForAdapter(input: { - homeDir?: string; - instanceId?: string; - env?: NodeJS.ProcessEnv; -} = {}): string { - return path.resolve( - resolvePaperclipHomeDirForAdapter(input.homeDir, input.env), - "instances", - resolvePaperclipInstanceIdForAdapter(input.instanceId, input.env), - ); -} - -function resolvePaperclipInstanceConfigPathForAdapter(input: { - homeDir?: string; - instanceId?: string; - env?: NodeJS.ProcessEnv; -} = {}): string { - return path.resolve(resolvePaperclipInstanceRootForAdapter(input), PAPERCLIP_CONFIG_BASENAME); -} - function readJsonIfPresent(filePath: string): unknown | null { if (!existsSync(filePath)) return null; try { @@ -141,58 +101,40 @@ function isPaperclipRuntimeConfig(value: unknown): boolean { return isObject(value) && isObject(value.database) && isObject(value.server); } -function readActiveSpaceIdFromRegistry(instanceConfigPath: string): string | null { - const parsed = readJsonIfPresent(instanceConfigPath); - if (!isObject(parsed) || isPaperclipRuntimeConfig(parsed)) return null; - const activeSpaceId = parsed.activeSpaceId; - return typeof activeSpaceId === "string" && PATH_SEGMENT_RE.test(activeSpaceId) ? activeSpaceId : null; -} - -function hasLegacyDefaultSpaceInstall(input: { - homeDir?: string; - instanceId?: string; - env?: NodeJS.ProcessEnv; -} = {}): boolean { - return isPaperclipRuntimeConfig(readJsonIfPresent(resolvePaperclipInstanceConfigPathForAdapter(input))); -} - -function resolvePaperclipSpaceIdForAdapter(input: { - homeDir?: string; - instanceId?: string; - spaceId?: string; - env?: NodeJS.ProcessEnv; -} = {}): string { - const raw = - input.spaceId?.trim() || - input.env?.PAPERCLIP_SPACE_ID?.trim() || - process.env.PAPERCLIP_SPACE_ID?.trim() || - readActiveSpaceIdFromRegistry(resolvePaperclipInstanceConfigPathForAdapter(input)) || - DEFAULT_PAPERCLIP_SPACE_ID; - if (!PATH_SEGMENT_RE.test(raw)) { - throw new Error(`Invalid PAPERCLIP_SPACE_ID '${raw}'.`); - } - return raw; -} - -function resolvePaperclipSpacesRootForAdapter(input: { - homeDir?: string; - instanceId?: string; - env?: NodeJS.ProcessEnv; -} = {}): string { - return path.resolve(resolvePaperclipInstanceRootForAdapter(input), "spaces"); -} - export function resolvePaperclipSpaceRootForAdapter(input: { homeDir?: string; instanceId?: string; spaceId?: string; env?: NodeJS.ProcessEnv; } = {}): string { - const spaceId = resolvePaperclipSpaceIdForAdapter(input); - if (spaceId === DEFAULT_PAPERCLIP_SPACE_ID && hasLegacyDefaultSpaceInstall(input)) { - return resolvePaperclipInstanceRootForAdapter(input); + const env = input.env ?? process.env; + const homeRaw = input.homeDir?.trim() || env.PAPERCLIP_HOME?.trim(); + const expandedHome = + homeRaw === "~" + ? os.homedir() + : homeRaw?.startsWith("~/") + ? path.resolve(os.homedir(), homeRaw.slice(2)) + : homeRaw; + const homeDir = path.resolve(expandedHome || path.resolve(os.homedir(), ".paperclip")); + const instanceId = input.instanceId?.trim() || env.PAPERCLIP_INSTANCE_ID?.trim() || DEFAULT_PAPERCLIP_INSTANCE_ID; + if (!PATH_SEGMENT_RE.test(instanceId)) throw new Error(`Invalid PAPERCLIP_INSTANCE_ID '${instanceId}'.`); + + const instanceRoot = path.resolve(homeDir, "instances", instanceId); + const instanceConfigPath = path.resolve(instanceRoot, PAPERCLIP_CONFIG_BASENAME); + const instanceConfig = readJsonIfPresent(instanceConfigPath); + const registrySpaceId = isObject(instanceConfig) && !isPaperclipRuntimeConfig(instanceConfig) + ? instanceConfig.activeSpaceId + : null; + const spaceId = input.spaceId?.trim() || + env.PAPERCLIP_SPACE_ID?.trim() || + (typeof registrySpaceId === "string" && PATH_SEGMENT_RE.test(registrySpaceId) ? registrySpaceId : null) || + DEFAULT_PAPERCLIP_SPACE_ID; + if (!PATH_SEGMENT_RE.test(spaceId)) throw new Error(`Invalid PAPERCLIP_SPACE_ID '${spaceId}'.`); + + if (spaceId === DEFAULT_PAPERCLIP_SPACE_ID && isPaperclipRuntimeConfig(instanceConfig)) { + return instanceRoot; } - return path.resolve(resolvePaperclipSpacesRootForAdapter(input), spaceId); + return path.resolve(instanceRoot, "spaces", spaceId); } const SENSITIVE_ENV_KEY = /(key|token|secret|password|passwd|authorization|cookie)/i; const REDACTED_LOG_VALUE = "***REDACTED***"; diff --git a/packages/db/src/runtime-config.test.ts b/packages/db/src/runtime-config.test.ts index 8e853d17709..a29248c7b08 100644 --- a/packages/db/src/runtime-config.test.ts +++ b/packages/db/src/runtime-config.test.ts @@ -113,6 +113,8 @@ describe("resolveDatabaseTarget", () => { process.env.PAPERCLIP_HOME = home; delete process.env.PAPERCLIP_CONFIG; delete process.env.DATABASE_URL; + delete process.env.PAPERCLIP_INSTANCE_ID; + delete process.env.PAPERCLIP_SPACE_ID; const target = resolveDatabaseTarget(); diff --git a/packages/plugins/sdk/src/testing.ts b/packages/plugins/sdk/src/testing.ts index 5bee4e3305b..0deb99eaab0 100644 --- a/packages/plugins/sdk/src/testing.ts +++ b/packages/plugins/sdk/src/testing.ts @@ -35,6 +35,8 @@ import type { PluginWorkspace, AgentSession, AgentSessionEvent, + PluginLocalFolderEntry, + PluginLocalFolderStatus, } from "./types.js"; import type { PluginEnvironmentValidateConfigParams, @@ -436,6 +438,8 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { const agents = new Map(); const goals = new Map(); const projectWorkspaces = new Map(); + const localFolderStatuses = new Map(); + const localFolderFiles = new Map(); const sessions = new Map(); const sessionEventCallbacks = new Map void>(); @@ -447,6 +451,43 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { const actionHandlers = new Map) => Promise>(); const toolHandlers = new Map Promise>(); + function localFolderKey(companyId: string, folderKey: string): string { + return `${companyId}:${folderKey}`; + } + + function localFolderFileKey(companyId: string, folderKey: string, relativePath: string): string { + return `${localFolderKey(companyId, folderKey)}:${relativePath}`; + } + + function normalizeLocalFolderRelativePath(relativePath: string): string { + const parts: string[] = []; + for (const segment of relativePath.split(/[\\/]+/)) { + if (!segment || segment === ".") continue; + if (segment === "..") throw new Error("Local folder path traversal is not allowed"); + parts.push(segment); + } + return parts.join("/"); + } + + function notConfiguredLocalFolderStatus(folderKey: string): PluginLocalFolderStatus { + return { + folderKey, + configured: false, + path: null, + realPath: null, + access: "readWrite", + readable: false, + writable: false, + requiredDirectories: [], + requiredFiles: [], + missingDirectories: [], + missingFiles: [], + healthy: false, + problems: [{ code: "not_configured", message: "No local folder path is configured." }], + checkedAt: new Date().toISOString(), + }; + } + function issueRelationSummary(issueId: string) { const issue = issues.get(issueId); if (!issue) throw new Error(`Issue not found: ${issueId}`); @@ -543,7 +584,7 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { }, async configure(input) { requireCapability(manifest, capabilitySet, "local.folders"); - return { + const status = { folderKey: input.folderKey, configured: true, path: input.path, @@ -558,77 +599,98 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { healthy: true, problems: [], checkedAt: new Date().toISOString(), - }; + } satisfies PluginLocalFolderStatus; + localFolderStatuses.set(localFolderKey(input.companyId, input.folderKey), status); + return status; }, - async status(_companyId, folderKey) { + async status(companyId, folderKey) { requireCapability(manifest, capabilitySet, "local.folders"); - return { - folderKey, - configured: false, - path: null, - realPath: null, - access: "readWrite", - readable: false, - writable: false, - requiredDirectories: [], - requiredFiles: [], - missingDirectories: [], - missingFiles: [], - healthy: false, - problems: [{ code: "not_configured", message: "No local folder path is configured." }], - checkedAt: new Date().toISOString(), - }; + return localFolderStatuses.get(localFolderKey(companyId, folderKey)) ?? notConfiguredLocalFolderStatus(folderKey); }, - async list(_companyId, folderKey, options) { + async list(companyId, folderKey, options) { requireCapability(manifest, capabilitySet, "local.folders"); + const status = localFolderStatuses.get(localFolderKey(companyId, folderKey)); + if (!status?.configured) throw new Error("Local folder is not configured"); + const prefix = normalizeLocalFolderRelativePath(options?.relativePath ?? ""); + const prefixWithSlash = prefix ? `${prefix}/` : ""; + const entries = new Map(); + for (const [key, contents] of localFolderFiles) { + const filePrefix = `${localFolderKey(companyId, folderKey)}:`; + if (!key.startsWith(filePrefix)) continue; + const filePath = key.slice(filePrefix.length); + if (prefix && filePath !== prefix && !filePath.startsWith(prefixWithSlash)) continue; + const remainder = prefix ? filePath.slice(prefixWithSlash.length) : filePath; + const [name] = remainder.split("/"); + if (!name) continue; + const entryPath = prefix ? `${prefix}/${name}` : name; + const isNested = remainder.includes("/"); + if (!options?.recursive && isNested) { + entries.set(entryPath, { + path: entryPath, + name, + kind: "directory", + size: null, + modifiedAt: null, + }); + continue; + } + entries.set(filePath, { + path: filePath, + name: filePath.split("/").pop() ?? filePath, + kind: "file", + size: Buffer.byteLength(contents, "utf8"), + modifiedAt: null, + }); + } + const maxEntries = options?.maxEntries && options.maxEntries > 0 ? options.maxEntries : entries.size; + const allEntries = [...entries.values()].sort((a, b) => a.path.localeCompare(b.path)); return { folderKey, relativePath: options?.relativePath ?? null, - entries: [], - truncated: false, + entries: allEntries.slice(0, maxEntries), + truncated: allEntries.length > maxEntries, }; }, - async readText() { + async readText(companyId, folderKey, relativePath) { requireCapability(manifest, capabilitySet, "local.folders"); - throw new Error("Test harness local folder readText is not implemented"); + const normalizedPath = normalizeLocalFolderRelativePath(relativePath); + const contents = localFolderFiles.get(localFolderFileKey(companyId, folderKey, normalizedPath)); + if (contents === undefined) throw new Error(`Local folder file not found: ${relativePath}`); + return contents; }, - async writeTextAtomic(_companyId, folderKey) { + async writeTextAtomic(companyId, folderKey, relativePath, contents) { requireCapability(manifest, capabilitySet, "local.folders"); - return { + const status = localFolderStatuses.get(localFolderKey(companyId, folderKey)) ?? { folderKey, - configured: false, - path: null, - realPath: null, + configured: true, + path: `memory://${manifest.id}/${companyId}/${folderKey}`, + realPath: `memory://${manifest.id}/${companyId}/${folderKey}`, access: "readWrite", - readable: false, - writable: false, + readable: true, + writable: true, requiredDirectories: [], requiredFiles: [], missingDirectories: [], missingFiles: [], - healthy: false, - problems: [{ code: "not_configured", message: "No local folder path is configured." }], + healthy: true, + problems: [], checkedAt: new Date().toISOString(), - }; + } satisfies PluginLocalFolderStatus; + if (status.access !== "readWrite" || !status.writable) { + throw new Error("Local folder is not configured for writes"); + } + localFolderStatuses.set(localFolderKey(companyId, folderKey), status); + localFolderFiles.set(localFolderFileKey(companyId, folderKey, normalizeLocalFolderRelativePath(relativePath)), contents); + return status; }, - async deleteFile(_companyId, folderKey) { + async deleteFile(companyId, folderKey, relativePath) { requireCapability(manifest, capabilitySet, "local.folders"); - return { - folderKey, - configured: false, - path: null, - realPath: null, - access: "readWrite", - readable: false, - writable: false, - requiredDirectories: [], - requiredFiles: [], - missingDirectories: [], - missingFiles: [], - healthy: false, - problems: [{ code: "not_configured", message: "No local folder path is configured." }], - checkedAt: new Date().toISOString(), - }; + const status = localFolderStatuses.get(localFolderKey(companyId, folderKey)) ?? notConfiguredLocalFolderStatus(folderKey); + if (status.configured && (status.access !== "readWrite" || !status.writable)) { + throw new Error("Local folder is not configured for writes"); + } + localFolderFiles.delete(localFolderFileKey(companyId, folderKey, normalizeLocalFolderRelativePath(relativePath))); + return status; }, }, events: { @@ -1213,10 +1275,64 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { } satisfies PluginManagedSkillResolution; }, async reset(skillKey, companyId) { - const resolved = await this.reconcile(skillKey, companyId); + const existing = await this.get(skillKey, companyId); + const declaration = manifest.skills?.find((skill) => skill.skillKey === skillKey); + if (!declaration) return existing; + const now = new Date(); + const skill = { + id: existing.skill?.id ?? randomUUID(), + companyId, + key: `plugin/${manifest.id.replace(/[^a-z0-9]+/g, "-").replace(/^-|-$/g, "")}/${skillKey}`, + slug: declaration.slug ?? skillKey, + name: declaration.displayName, + description: declaration.description ?? null, + markdown: declaration.markdown ?? `# ${declaration.displayName}\n`, + sourceType: "catalog", + sourceLocator: null, + sourceRef: null, + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "SKILL.md", kind: "skill" }], + metadata: { + sourceKind: "catalog", + pluginManagedResource: { + pluginKey: manifest.id, + resourceKind: "skill", + resourceKey: skillKey, + }, + }, + createdAt: existing.skill?.createdAt ?? now, + updatedAt: now, + } satisfies CompanySkill; + const nowIso = now.toISOString(); + const existingEntity = [...entities.values()].find((entity) => + entity.entityType === "managed_resource" && + entity.scopeKind === "company" && + entity.scopeId === companyId && + entity.externalId === `${manifest.id}:skill:${skillKey}`, + ); + const record: PluginEntityRecord = { + id: existingEntity?.id ?? randomUUID(), + entityType: "managed_resource", + scopeKind: "company", + scopeId: companyId, + externalId: `${manifest.id}:skill:${skillKey}`, + title: declaration.displayName, + status: null, + data: { resourceKind: "skill", resourceKey: skillKey, skillId: skill.id, skill }, + createdAt: existingEntity?.createdAt ?? nowIso, + updatedAt: nowIso, + }; + entities.set(record.id, record); return { - ...resolved, - status: resolved.skill ? "reset" : resolved.status, + pluginKey: manifest.id, + resourceKind: "skill", + resourceKey: skillKey, + companyId, + skillId: skill.id, + skill, + status: "reset", + defaultDrift: null, } satisfies PluginManagedSkillResolution; }, }, diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index 3465a804b52..ba4b7c4c147 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -889,9 +889,6 @@ export function buildHostServices( }); const status = await inspectStoredLocalFolder(companyId, params.folderKey); assertWritableConfiguredLocalFolder(status); - if (status.access !== "readWrite" || !status.writable) { - throw new Error("Local folder is not configured for writes"); - } await writePluginLocalFolderTextAtomic(status.realPath!, params.relativePath, params.contents); return inspectStoredLocalFolder(companyId, params.folderKey); }, @@ -900,9 +897,6 @@ export function buildHostServices( const companyId = ensureCompanyId(params.companyId); const status = await inspectStoredLocalFolder(companyId, params.folderKey); assertWritableConfiguredLocalFolder(status); - if (status.access !== "readWrite" || !status.writable) { - throw new Error("Local folder is not configured for writes"); - } await deletePluginLocalFolderFile(status.realPath!, params.relativePath); return inspectStoredLocalFolder(companyId, params.folderKey); },