diff --git a/packages/junior-evals/evals/behavior-harness.ts b/packages/junior-evals/evals/behavior-harness.ts index 0e8e2478..95433ec3 100644 --- a/packages/junior-evals/evals/behavior-harness.ts +++ b/packages/junior-evals/evals/behavior-harness.ts @@ -3,6 +3,10 @@ import { fileURLToPath } from "node:url"; import type { Message } from "chat"; import { executeWithReplay } from "vitest-evals/replay"; import type { JsonValue } from "vitest-evals/harness"; +import { + createPluginAppFixture, + type PluginAppFixture, +} from "@junior-tests/fixtures/plugin-app"; import { createSlackRuntime } from "@/chat/app/factory"; import type { AssistantLifecycleEvent } from "@/chat/runtime/slack-runtime"; import type { JuniorRuntimeServiceOverrides } from "@/chat/app/services"; @@ -19,8 +23,7 @@ import { deleteMcpStoredOAuthCredentials, getLatestMcpAuthSessionForUserProvider, } from "@/chat/mcp/auth-store"; -import { setPluginPackages } from "@/chat/plugins/package-discovery"; -import { getPluginOAuthConfig } from "@/chat/plugins/registry"; +import { getPluginOAuthConfig, setPluginConfig } from "@/chat/plugins/registry"; import { generateAssistantReply } from "@/chat/respond"; import { getStateAdapter } from "@/chat/state/adapter"; import { resetSkillDiscoveryCache } from "@/chat/skills"; @@ -399,7 +402,6 @@ const HARNESS_ENV_KEYS = [ "EVAL_ENABLE_TEST_CREDENTIALS", "EVAL_TEST_CREDENTIAL_TOKEN", "JUNIOR_BASE_URL", - "JUNIOR_EXTRA_PLUGIN_ROOTS", "JUNIOR_EVAL_ENABLE_FAULTS", "JUNIOR_EVAL_FAULT_SANDBOX_BASH_STREAM_INTERRUPTS", "JUNIOR_STATE_ADAPTER", @@ -909,6 +911,7 @@ interface HarnessEnvironment { configuredPluginDirs: string[]; configuredSkillDirs: string[]; envSnapshot: EnvSnapshot; + pluginApp?: PluginAppFixture; stateAdapter: HarnessStateAdapter; } @@ -916,70 +919,90 @@ async function setupHarnessEnvironment( scenario: EvalScenario, ): Promise { const envSnapshot = snapshotEnv(HARNESS_ENV_KEYS); + let pluginApp: PluginAppFixture | undefined; - const configuredSkillDirs = - scenario.overrides?.skill_dirs?.map(resolveEvalRelativePath) ?? []; - const configuredPluginDirs = - scenario.overrides?.plugin_dirs?.map(resolveEvalRelativePath) ?? []; - const autoCompleteMcpOauthProviders = new Set( - scenario.overrides?.auto_complete_mcp_oauth?.map((p) => p.trim()) ?? [], - ); - const autoCompleteOauthProviders = new Set( - scenario.overrides?.auto_complete_oauth?.map((p) => p.trim()) ?? [], - ); - const authRequesterUsers = new Set( - scenario.events.flatMap((event) => - "message" in event - ? [event.message.author?.user_id?.trim() || "U-test"] - : event.user_id - ? [event.user_id] - : [], - ), - ); - if (authRequesterUsers.size === 0) { - authRequesterUsers.add("U-test"); - } + try { + const configuredSkillDirs = + scenario.overrides?.skill_dirs?.map(resolveEvalRelativePath) ?? []; + const configuredPluginDirs = + scenario.overrides?.plugin_dirs?.map(resolveEvalRelativePath) ?? []; + const autoCompleteMcpOauthProviders = new Set( + scenario.overrides?.auto_complete_mcp_oauth?.map((p) => p.trim()) ?? [], + ); + const autoCompleteOauthProviders = new Set( + scenario.overrides?.auto_complete_oauth?.map((p) => p.trim()) ?? [], + ); + const authRequesterUsers = new Set( + scenario.events.flatMap((event) => + "message" in event + ? [event.message.author?.user_id?.trim() || "U-test"] + : event.user_id + ? [event.user_id] + : [], + ), + ); + if (authRequesterUsers.size === 0) { + authRequesterUsers.add("U-test"); + } - if (scenario.overrides?.enable_test_credentials) { - process.env.EVAL_ENABLE_TEST_CREDENTIALS = "1"; - if (scenario.overrides.test_credential_token) { - process.env.EVAL_TEST_CREDENTIAL_TOKEN = - scenario.overrides.test_credential_token; + if (scenario.overrides?.enable_test_credentials) { + process.env.EVAL_ENABLE_TEST_CREDENTIALS = "1"; + if (scenario.overrides.test_credential_token) { + process.env.EVAL_TEST_CREDENTIAL_TOKEN = + scenario.overrides.test_credential_token; + } } - } - const sandboxBashStreamInterrupts = - scenario.overrides?.faults?.sandbox_bash_stream_interrupts; - if ( - typeof sandboxBashStreamInterrupts === "number" && - Number.isFinite(sandboxBashStreamInterrupts) && - sandboxBashStreamInterrupts > 0 - ) { - process.env.JUNIOR_EVAL_ENABLE_FAULTS = "1"; - process.env.JUNIOR_EVAL_FAULT_SANDBOX_BASH_STREAM_INTERRUPTS = String( - Math.floor(sandboxBashStreamInterrupts), + const sandboxBashStreamInterrupts = + scenario.overrides?.faults?.sandbox_bash_stream_interrupts; + if ( + typeof sandboxBashStreamInterrupts === "number" && + Number.isFinite(sandboxBashStreamInterrupts) && + sandboxBashStreamInterrupts > 0 + ) { + process.env.JUNIOR_EVAL_ENABLE_FAULTS = "1"; + process.env.JUNIOR_EVAL_FAULT_SANDBOX_BASH_STREAM_INTERRUPTS = String( + Math.floor(sandboxBashStreamInterrupts), + ); + } + process.env.JUNIOR_BASE_URL = "https://junior.example.com"; + process.env.JUNIOR_STATE_ADAPTER = "memory"; + pluginApp = + configuredPluginDirs.length > 0 + ? await createPluginAppFixture(configuredPluginDirs, { + linkNodeModules: Boolean( + scenario.overrides?.plugin_packages?.length, + ), + }) + : undefined; + setPluginConfig({ packages: scenario.overrides?.plugin_packages ?? [] }); + + const stateAdapter = getStateAdapter(); + await stateAdapter.connect(); + resetSkillDiscoveryCache(); + await cleanupHarnessThreadState(stateAdapter, scenario.events); + await cleanupMcpAuthState( + authRequesterUsers, + autoCompleteMcpOauthProviders, ); - } - process.env.JUNIOR_BASE_URL = "https://junior.example.com"; - process.env.JUNIOR_STATE_ADAPTER = "memory"; - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify(configuredPluginDirs); - setPluginPackages(scenario.overrides?.plugin_packages ?? []); + await cleanupOAuthTokens(authRequesterUsers, autoCompleteOauthProviders); - const stateAdapter = getStateAdapter(); - await stateAdapter.connect(); - resetSkillDiscoveryCache(); - await cleanupHarnessThreadState(stateAdapter, scenario.events); - await cleanupMcpAuthState(authRequesterUsers, autoCompleteMcpOauthProviders); - await cleanupOAuthTokens(authRequesterUsers, autoCompleteOauthProviders); - - return { - authRequesterUsers, - autoCompleteMcpOauthProviders, - autoCompleteOauthProviders, - configuredPluginDirs, - configuredSkillDirs, - envSnapshot, - stateAdapter, - }; + return { + authRequesterUsers, + autoCompleteMcpOauthProviders, + autoCompleteOauthProviders, + configuredPluginDirs, + configuredSkillDirs, + envSnapshot, + ...(pluginApp ? { pluginApp } : {}), + stateAdapter, + }; + } catch (error) { + resetSkillDiscoveryCache(); + setPluginConfig(undefined); + envSnapshot.restore(); + await pluginApp?.cleanup(); + throw error; + } } async function teardownHarnessEnvironment( @@ -987,7 +1010,7 @@ async function teardownHarnessEnvironment( env: HarnessEnvironment, ): Promise { resetSkillDiscoveryCache(); - setPluginPackages(undefined); + setPluginConfig(undefined); await cleanupHarnessThreadState(env.stateAdapter, scenario.events); await cleanupMcpAuthState( env.authRequesterUsers, @@ -998,6 +1021,7 @@ async function teardownHarnessEnvironment( env.autoCompleteOauthProviders, ); env.envSnapshot.restore(); + await env.pluginApp?.cleanup(); } // --------------------------------------------------------------------------- diff --git a/packages/junior-evals/tests/unit/harness/behavior-harness.test.ts b/packages/junior-evals/tests/unit/harness/behavior-harness.test.ts index c28df54d..29597516 100644 --- a/packages/junior-evals/tests/unit/harness/behavior-harness.test.ts +++ b/packages/junior-evals/tests/unit/harness/behavior-harness.test.ts @@ -217,6 +217,22 @@ describe("behavior harness", () => { ]); }); + it("restores cwd when setup fails after creating a plugin fixture", async () => { + const cwd = process.cwd(); + + await expect( + runEvalScenario({ + events: [], + overrides: { + plugin_dirs: ["evals/fixtures/plugins"], + plugin_packages: ["../bad-package"], + }, + }), + ).rejects.toThrow("Plugin package names must be valid npm package names"); + + expect(process.cwd()).toBe(cwd); + }); + it("collects created canvas metadata from captured Slack API calls", () => { const artifacts = collectSlackArtifactsFromCapturedCalls([ { diff --git a/packages/junior/src/app.ts b/packages/junior/src/app.ts index 4150e384..100f8d5a 100644 --- a/packages/junior/src/app.ts +++ b/packages/junior/src/app.ts @@ -1,8 +1,13 @@ import { Hono } from "hono"; -import { setConfigDefaults } from "@/chat/configuration/defaults"; +import { + getConfigDefaults, + setConfigDefaults, +} from "@/chat/configuration/defaults"; import { logException } from "@/chat/logging"; -import { setPluginPackages } from "@/chat/plugins/package-discovery"; -import { setPluginConfig } from "@/chat/plugins/registry"; +import { + getPluginCatalogSignature, + setPluginConfig, +} from "@/chat/plugins/registry"; import type { PluginConfig } from "@/chat/plugins/types"; import { GET as diagnosticsGET } from "@/handlers/diagnostics"; import { GET as dashboardGET } from "@/handlers/diagnostics-dashboard"; @@ -47,25 +52,86 @@ async function resolveBuildPluginConfig(): Promise { try { const mod: { plugins?: PluginConfig } = await import("#junior/config"); return mod.plugins; - } catch { - // Virtual module unavailable (not running in Nitro context). - // Fall back to env var for dev mode and tests. - const env = process.env.JUNIOR_PLUGIN_PACKAGES; - if (env) { - try { - return { packages: JSON.parse(env) }; - } catch {} + } catch (error) { + if (!isMissingVirtualConfig(error)) { + throw error; } + const packages = readEnvPluginPackages(); + if (packages) { + return { packages }; + } + return undefined; + } +} + +function isMissingVirtualConfig(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + const code = (error as { code?: string }).code; + return ( + (code === "ERR_PACKAGE_IMPORT_NOT_DEFINED" || + code === "ERR_MODULE_NOT_FOUND" || + code === "MODULE_NOT_FOUND") && + error.message.includes("#junior/config") + ); +} + +function readEnvPluginPackages(): string[] | undefined { + const env = process.env.JUNIOR_PLUGIN_PACKAGES; + if (!env) { return undefined; } + + let parsed: unknown; + try { + parsed = JSON.parse(env); + } catch (error) { + throw new Error("JUNIOR_PLUGIN_PACKAGES must be valid JSON", { + cause: error, + }); + } + + if ( + !Array.isArray(parsed) || + parsed.some((value) => typeof value !== "string" || !value.trim()) + ) { + throw new Error( + "JUNIOR_PLUGIN_PACKAGES must be a JSON array of package names", + ); + } + + return parsed; +} + +function hasConfiguredPluginCatalog(config: PluginConfig | undefined): boolean { + if (!config) { + return false; + } + + return Boolean( + config.packages?.length || Object.keys(config.manifests ?? {}).length, + ); } /** Create a Hono app with all Junior routes. */ export async function createApp(options?: JuniorAppOptions): Promise { const pluginConfig = options?.plugins ?? (await resolveBuildPluginConfig()); - setPluginPackages(pluginConfig?.packages); - setPluginConfig(pluginConfig); - setConfigDefaults(options?.configDefaults); + const shouldValidatePluginCatalog = + hasConfiguredPluginCatalog(pluginConfig) || + Boolean(Object.keys(options?.configDefaults ?? {}).length); + const previousPluginConfig = setPluginConfig(pluginConfig); + const previousConfigDefaults = getConfigDefaults(); + try { + setConfigDefaults(options?.configDefaults); + if (shouldValidatePluginCatalog) { + getPluginCatalogSignature(); + } + } catch (error) { + setPluginConfig(previousPluginConfig); + setConfigDefaults(previousConfigDefaults); + throw error; + } const waitUntil = options?.waitUntil ?? (await defaultWaitUntil()); diff --git a/packages/junior/src/build/copy-build-content.ts b/packages/junior/src/build/copy-build-content.ts index 6fb00537..27708d60 100644 --- a/packages/junior/src/build/copy-build-content.ts +++ b/packages/junior/src/build/copy-build-content.ts @@ -1,14 +1,14 @@ -import { cpSync, existsSync, mkdirSync, readdirSync } from "node:fs"; +import { cpSync, existsSync, mkdirSync, readdirSync, statSync } from "node:fs"; import path from "node:path"; import { discoverInstalledPluginPackageContent } from "@/chat/plugins/package-discovery"; import { globToRegex } from "@/build/glob-to-regex"; -import { resolvePackageDir } from "@/build/resolve-package"; +import { isValidPackageName, resolvePackageDir } from "@/package-resolution"; /** Copy app directory and plugin manifests into the server output. */ export function copyAppAndPluginContent( cwd: string, serverRoot: string, - packageNames?: string[], + packageNames?: unknown, ): void { copyIfExists(path.join(cwd, "app"), path.join(serverRoot, "app")); @@ -17,13 +17,10 @@ export function copyAppAndPluginContent( }); for (const root of packagedContent.manifestRoots) { if (existsSync(path.join(root, "plugin.yaml"))) { - const relative = path.relative(cwd, root); - if (!relative || path.isAbsolute(relative) || relative.startsWith("..")) { - continue; - } + const manifestPath = path.join(root, "plugin.yaml"); copyIfExists( - path.join(root, "plugin.yaml"), - path.join(serverRoot, relative, "plugin.yaml"), + manifestPath, + resolveServerOutputPath(cwd, serverRoot, manifestPath), ); continue; } @@ -38,46 +35,116 @@ export function copyAppAndPluginContent( /** Copy extra file patterns into server output for files the bundler cannot trace. */ export function copyIncludedFiles( + cwd: string, serverRoot: string, - patterns?: string[], + patterns?: unknown, ): void { - if (!patterns?.length) return; + if (patterns === undefined) return; + if (!Array.isArray(patterns)) { + throw new Error( + "includeFiles must be an array of package subpath patterns", + ); + } + if (patterns.length === 0) return; + for (const pattern of patterns) { - const normalized = pattern.replace(/^node_modules\//, ""); - const parts = normalized.split("/"); - const pkgName = parts[0].startsWith("@") - ? `${parts[0]}/${parts[1]}` - : parts[0]; - const subpath = parts.slice(pkgName.includes("/") ? 2 : 1).join("/"); - const fileGlob = path.basename(subpath); - const subDir = path.dirname(subpath); - - const pkgDir = resolvePackageDir(pkgName); - if (!pkgDir) continue; + if (typeof pattern !== "string" || !pattern.trim()) { + throw new Error("includeFiles entries must be package subpath patterns"); + } + const { pkgName, subDir, fileGlob } = parseIncludePattern(pattern); + + const pkgDir = resolvePackageDir(cwd, pkgName); + if (!pkgDir) { + throw new Error( + `includeFiles entry "${pattern}" references package "${pkgName}", but it could not be resolved`, + ); + } const sourceDir = path.join(pkgDir, subDir); - if (!existsSync(sourceDir)) continue; + if (!isDirectory(sourceDir)) { + throw new Error( + `includeFiles entry "${pattern}" references missing directory ${sourceDir}`, + ); + } const entries = readdirSync(sourceDir); const re = fileGlob.includes("*") ? globToRegex(fileGlob) : null; + let matched = false; + let copied = false; for (const entry of entries) { if (re ? !re.test(entry) : entry !== fileGlob) continue; - copyIfExists( - path.join(sourceDir, entry), - path.join(serverRoot, "node_modules", pkgName, subDir, entry), + matched = true; + copied = + copyIfExists( + path.join(sourceDir, entry), + path.join(serverRoot, "node_modules", pkgName, subDir, entry), + ) || copied; + } + + if (!matched) { + throw new Error( + `includeFiles entry "${pattern}" did not match any files in ${sourceDir}`, + ); + } + if (!copied) { + throw new Error( + `includeFiles entry "${pattern}" matched files in ${sourceDir} but did not copy any existing files`, ); } } } -function copyIfExists(source: string, target: string): void { +function parseIncludePattern(pattern: string): { + fileGlob: string; + pkgName: string; + subDir: string; +} { + const normalized = pattern.trim().replace(/^node_modules\//, ""); + const parts = normalized.split("/"); + if ( + !normalized || + path.isAbsolute(normalized) || + parts.some((part) => !part || part === "." || part === "..") + ) { + throw new Error( + `includeFiles entry "${pattern}" must be a package subpath pattern`, + ); + } + + const isScopedPackage = parts[0].startsWith("@"); + const packagePartCount = isScopedPackage ? 2 : 1; + const pkgName = parts.slice(0, packagePartCount).join("/"); + const subpath = parts.slice(packagePartCount).join("/"); + if (!pkgName || !isValidPackageName(pkgName) || !subpath) { + throw new Error( + `includeFiles entry "${pattern}" must include a package subpath`, + ); + } + + return { + pkgName, + subDir: path.dirname(subpath), + fileGlob: path.basename(subpath), + }; +} + +function isDirectory(targetPath: string): boolean { + try { + return statSync(targetPath).isDirectory(); + } catch { + return false; + } +} + +function copyIfExists(source: string, target: string): boolean { if (!existsSync(source)) { - return; + return false; } mkdirSync(path.dirname(target), { recursive: true }); cpSync(source, target, { recursive: true }); + return true; } function copyRootIntoServerOutput( @@ -85,10 +152,44 @@ function copyRootIntoServerOutput( serverRoot: string, root: string, ): void { - const relative = path.relative(cwd, root); - if (!relative || path.isAbsolute(relative) || relative.startsWith("..")) { - return; + copyIfExists(root, resolveServerOutputPath(cwd, serverRoot, root)); +} + +function resolveServerOutputPath( + cwd: string, + serverRoot: string, + sourcePath: string, +): string { + const relative = path.relative(cwd, sourcePath); + if (isLocalRelativePath(relative)) { + return path.join(serverRoot, relative); + } + + const nodeModulesRelative = nodeModulesRelativePath(sourcePath); + if (nodeModulesRelative) { + return path.join(serverRoot, nodeModulesRelative); + } + + throw new Error( + `Cannot copy configured plugin content outside the app root or node_modules: ${sourcePath}`, + ); +} + +function isLocalRelativePath(relativePath: string): boolean { + return ( + Boolean(relativePath) && + !path.isAbsolute(relativePath) && + relativePath !== ".." && + !relativePath.startsWith(`..${path.sep}`) + ); +} + +function nodeModulesRelativePath(sourcePath: string): string | null { + const parts = path.resolve(sourcePath).split(path.sep); + const nodeModulesIndex = parts.lastIndexOf("node_modules"); + if (nodeModulesIndex === -1 || nodeModulesIndex === parts.length - 1) { + return null; } - copyIfExists(root, path.join(serverRoot, relative)); + return path.join("node_modules", ...parts.slice(nodeModulesIndex + 1)); } diff --git a/packages/junior/src/build/resolve-package.ts b/packages/junior/src/build/resolve-package.ts deleted file mode 100644 index 04aab24c..00000000 --- a/packages/junior/src/build/resolve-package.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { existsSync } from "node:fs"; -import path from "node:path"; -import { fileURLToPath } from "node:url"; - -/** Resolve a package to its root directory using import.meta.resolve. */ -export function resolvePackageDir(pkgName: string): string | undefined { - try { - const resolved = import.meta.resolve(pkgName); - const entry = resolved.startsWith("file://") - ? fileURLToPath(resolved) - : resolved; - // Walk up from the resolved entry to find the package root (contains package.json). - let dir = path.dirname(entry); - while (dir !== path.dirname(dir)) { - if (existsSync(path.join(dir, "package.json"))) return dir; - dir = path.dirname(dir); - } - } catch { - // Package not resolvable from this module - } - return undefined; -} diff --git a/packages/junior/src/chat/configuration/defaults.ts b/packages/junior/src/chat/configuration/defaults.ts index 648f9af9..51da909c 100644 --- a/packages/junior/src/chat/configuration/defaults.ts +++ b/packages/junior/src/chat/configuration/defaults.ts @@ -2,15 +2,37 @@ import { isPluginConfigKey } from "@/chat/plugins/registry"; let installDefaults: Record = {}; +function cloneDefaults( + defaults: Record, +): Record { + return structuredClone(defaults) as Record; +} + +function isConfigDefaultsRecord( + defaults: unknown, +): defaults is Record { + return ( + typeof defaults === "object" && + defaults !== null && + !Array.isArray(defaults) + ); +} + /** Store install-wide config defaults; keys must be registered plugin config keys. */ export function setConfigDefaults( defaults: Record | undefined, ): void { - if (!defaults) { + if (defaults === undefined) { installDefaults = {}; return; } + if (!isConfigDefaultsRecord(defaults)) { + throw new Error( + "configDefaults must be an object keyed by plugin config key", + ); + } + for (const key of Object.keys(defaults)) { if (!isPluginConfigKey(key)) { throw new Error( @@ -19,10 +41,10 @@ export function setConfigDefaults( } } - installDefaults = { ...defaults }; + installDefaults = cloneDefaults(defaults); } /** Return the install-wide configuration defaults (empty object when none set). */ export function getConfigDefaults(): Record { - return installDefaults; + return cloneDefaults(installDefaults); } diff --git a/packages/junior/src/chat/plugins/package-discovery.ts b/packages/junior/src/chat/plugins/package-discovery.ts index dcc26a63..ddd93430 100644 --- a/packages/junior/src/chat/plugins/package-discovery.ts +++ b/packages/junior/src/chat/plugins/package-discovery.ts @@ -1,10 +1,14 @@ import path from "node:path"; import { discoverNodeModulesDirs, isDirectory, isFile } from "@/chat/discovery"; +import { + isValidPackageName, + resolvePackageLocation, +} from "@/package-resolution"; interface InstalledJuniorContentPackage { name: string; dir: string; - nodeModulesDir: string | null; + nodeModulesDir?: string; hasRootPluginManifest: boolean; hasPluginsDir: boolean; hasSkillsDir: boolean; @@ -36,7 +40,12 @@ function uniqueStringsInOrder(values: string[]): string[] { function pathForTracingInclude(cwd: string, targetPath: string): string | null { const relative = path.relative(cwd, targetPath); - if (!relative || path.isAbsolute(relative)) { + if ( + !relative || + path.isAbsolute(relative) || + relative === ".." || + relative.startsWith(`..${path.sep}`) + ) { return null; } @@ -44,28 +53,49 @@ function pathForTracingInclude(cwd: string, targetPath: string): string | null { return normalized.startsWith(".") ? normalized : `./${normalized}`; } -let configuredPluginPackages: string[] | undefined; +/** Normalize and validate configured plugin package names. */ +export function normalizePluginPackageNames(packageNames: unknown): string[] { + if (packageNames === undefined) { + return []; + } + + if (!Array.isArray(packageNames)) { + throw new Error("plugins.packages must be an array of package names"); + } -/** Set the runtime plugin package allowlist. Called by `createApp()`. */ -export function setPluginPackages(packages: string[] | undefined): void { - configuredPluginPackages = packages; + const normalized: string[] = []; + const seen = new Set(); + for (const packageName of packageNames) { + const normalizedPackageName = + typeof packageName === "string" ? packageName.trim() : ""; + if (!normalizedPackageName || !isValidPackageName(normalizedPackageName)) { + throw new Error("Plugin package names must be valid npm package names"); + } + if (seen.has(normalizedPackageName)) { + continue; + } + seen.add(normalizedPackageName); + normalized.push(normalizedPackageName); + } + return normalized; +} + +function formatNodeModulesDirs(candidateNodeModulesDirs: string[]): string { + return candidateNodeModulesDirs.length > 0 + ? candidateNodeModulesDirs.join(", ") + : "none found"; } function resolvePackageDirFromName( + cwd: string, packageName: string, candidateNodeModulesDirs: string[], -): { dir: string; nodeModulesDir: string } | null { - for (const nodeModulesDir of candidateNodeModulesDirs) { - const packageDir = path.join(nodeModulesDir, ...packageName.split("/")); - if (isDirectory(packageDir)) { - return { - dir: path.resolve(packageDir), - nodeModulesDir: path.resolve(nodeModulesDir), - }; - } - } - - return null; +): { dir: string; nodeModulesDir?: string } | null { + return ( + resolvePackageLocation(cwd, packageName, { + nodeModulesDirs: candidateNodeModulesDirs, + }) ?? null + ); } function readPluginPackageFlags(dir: string): { @@ -90,33 +120,34 @@ function readPluginPackageFlags(dir: string): { function discoverDeclaredPackages( packageNames: string[], candidateNodeModulesDirs: string[], + cwd: string, ): InstalledJuniorContentPackage[] { const discovered: InstalledJuniorContentPackage[] = []; - const seenPackageNames = new Set(); const seenPackageDirs = new Set(); for (const packageName of packageNames) { const resolved = resolvePackageDirFromName( + cwd, packageName, candidateNodeModulesDirs, ); if (!resolved) { - continue; + throw new Error( + `Plugin package "${packageName}" was configured but could not be resolved from node_modules or package resolution (${formatNodeModulesDirs(candidateNodeModulesDirs)})`, + ); } - if ( - seenPackageNames.has(packageName) || - seenPackageDirs.has(resolved.dir) - ) { + if (seenPackageDirs.has(resolved.dir)) { continue; } const pluginFlags = readPluginPackageFlags(resolved.dir); if (!pluginFlags) { - continue; + throw new Error( + `Plugin package "${packageName}" was configured but does not contain plugin content; expected plugin.yaml, plugins/, or skills/ in ${resolved.dir}`, + ); } - seenPackageNames.add(packageName); seenPackageDirs.add(resolved.dir); discovered.push({ name: packageName, @@ -131,7 +162,7 @@ function discoverDeclaredPackages( export interface DiscoverInstalledPluginPackageContentOptions { nodeModulesDirs?: string[]; - packageNames?: string[]; + packageNames?: unknown; } /** Discover plugin package content from explicitly declared package names. */ @@ -140,13 +171,14 @@ export function discoverInstalledPluginPackageContent( options?: DiscoverInstalledPluginPackageContentOptions, ): InstalledPluginPackageContent { const resolvedCwd = path.resolve(cwd); - const packageNames = options?.packageNames ?? configuredPluginPackages ?? []; + const packageNames = normalizePluginPackageNames(options?.packageNames); const nodeModulesDirs = options?.nodeModulesDirs ?? discoverNodeModulesDirs(resolvedCwd); const discoveredPackages = discoverDeclaredPackages( packageNames, nodeModulesDirs, + resolvedCwd, ); const manifestRoots: string[] = []; diff --git a/packages/junior/src/chat/plugins/registry.ts b/packages/junior/src/chat/plugins/registry.ts index 2ba29567..e1d10ba8 100644 --- a/packages/junior/src/chat/plugins/registry.ts +++ b/packages/junior/src/chat/plugins/registry.ts @@ -8,7 +8,11 @@ import { createGitHubAppBroker } from "./auth/github-app-broker"; import { parsePluginManifest } from "./manifest"; import { createOAuthBearerBroker } from "./auth/oauth-bearer-broker"; import { createApiHeadersBroker } from "./auth/api-headers-broker"; -import { discoverInstalledPluginPackageContent } from "./package-discovery"; +import { + discoverInstalledPluginPackageContent, + type InstalledPluginPackageContent, + normalizePluginPackageNames, +} from "./package-discovery"; import type { PluginBrokerDeps, PluginConfig, @@ -130,39 +134,9 @@ function normalizePluginRoots(roots: string[]): string[] { return resolved; } -function getExtraPluginRoots(): string[] { - const raw = process.env.JUNIOR_EXTRA_PLUGIN_ROOTS?.trim(); - if (!raw) { - return []; - } - - if (raw.startsWith("[")) { - try { - const parsed = JSON.parse(raw); - if (Array.isArray(parsed)) { - return normalizePluginRoots( - parsed.filter((value): value is string => typeof value === "string"), - ); - } - } catch { - return []; - } - } - - return normalizePluginRoots( - raw - .split(path.delimiter) - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0), - ); -} - function getPluginCatalogSource(): PluginCatalogSource { - const packagedContent = discoverInstalledPluginPackageContent(); - const localRoots = normalizePluginRoots([ - ...pluginRoots(), - ...getExtraPluginRoots(), - ]); + const packagedContent = discoverConfiguredPluginPackageContent(); + const localRoots = normalizePluginRoots(pluginRoots()); const manifestRoots = normalizePluginRoots([ ...localRoots, ...packagedContent.manifestRoots, @@ -181,6 +155,42 @@ function getPluginCatalogSource(): PluginCatalogSource { }; } +function normalizePluginConfig( + config: PluginConfig | undefined, +): PluginConfig | undefined { + if (!config) { + return undefined; + } + + return { + packages: normalizePluginPackageNames(config.packages), + ...(config.manifests + ? { manifests: structuredClone(config.manifests) } + : {}), + }; +} + +function clonePluginConfig( + config: PluginConfig | undefined, +): PluginConfig | undefined { + if (!config) { + return undefined; + } + + return { + packages: [...(config.packages ?? [])], + ...(config.manifests + ? { manifests: structuredClone(config.manifests) } + : {}), + }; +} + +function discoverConfiguredPluginPackageContent(): InstalledPluginPackageContent { + return discoverInstalledPluginPackageContent(process.cwd(), { + packageNames: pluginConfig?.packages, + }); +} + function buildLoadedPluginState( source: PluginCatalogSource, ): LoadedPluginState { @@ -310,9 +320,18 @@ function ensurePluginsLoaded(): LoadedPluginState { // --- Sync exports --- -/** Set install-wide plugin configuration before plugin discovery. */ -export function setPluginConfig(config: PluginConfig | undefined): void { - pluginConfig = config; +/** Set install-wide plugin configuration and return the previous value for rollback. */ +export function setPluginConfig( + config: PluginConfig | undefined, +): PluginConfig | undefined { + const previousConfig = clonePluginConfig(pluginConfig); + pluginConfig = normalizePluginConfig(config); + return previousConfig; +} + +/** Return installed plugin package content from the active plugin configuration. */ +export function getPluginPackageContent(): InstalledPluginPackageContent { + return discoverConfiguredPluginPackageContent(); } /** Return the current plugin catalog signature used for cache invalidation. */ diff --git a/packages/junior/src/handlers/diagnostics.ts b/packages/junior/src/handlers/diagnostics.ts index de944371..e45946c6 100644 --- a/packages/junior/src/handlers/diagnostics.ts +++ b/packages/junior/src/handlers/diagnostics.ts @@ -1,8 +1,10 @@ import { readFileSync } from "node:fs"; import path from "node:path"; import { homeDir } from "@/chat/discovery"; -import { discoverInstalledPluginPackageContent } from "@/chat/plugins/package-discovery"; -import { getPluginProviders } from "@/chat/plugins/registry"; +import { + getPluginPackageContent, + getPluginProviders, +} from "@/chat/plugins/registry"; import { discoverSkills } from "@/chat/skills"; function readDescriptionText(): string | undefined { @@ -19,7 +21,7 @@ function readDescriptionText(): string | undefined { /** Return a runtime discovery snapshot for built-app diagnostics. */ export async function GET(): Promise { - const packagedContent = discoverInstalledPluginPackageContent(); + const packagedContent = getPluginPackageContent(); const skills = await discoverSkills(); return Response.json({ diff --git a/packages/junior/src/nitro.ts b/packages/junior/src/nitro.ts index 1fb3fdfa..4989981d 100644 --- a/packages/junior/src/nitro.ts +++ b/packages/junior/src/nitro.ts @@ -48,6 +48,7 @@ export function juniorNitro(options: JuniorNitroOptions = {}): { options.plugins?.packages, ); copyIncludedFiles( + cwd, nitro.options.output.serverDir, options.includeFiles, ); diff --git a/packages/junior/src/package-resolution.ts b/packages/junior/src/package-resolution.ts new file mode 100644 index 00000000..5d858746 --- /dev/null +++ b/packages/junior/src/package-resolution.ts @@ -0,0 +1,204 @@ +import { statSync } from "node:fs"; +import { createRequire } from "node:module"; +import path from "node:path"; + +export interface ResolvedPackageLocation { + dir: string; + nodeModulesDir?: string; +} + +export interface ResolvePackageLocationOptions { + nodeModulesDirs?: string[]; +} + +function isDirectory(targetPath: string): boolean { + try { + return statSync(targetPath).isDirectory(); + } catch { + return false; + } +} + +function isFile(targetPath: string): boolean { + try { + return statSync(targetPath).isFile(); + } catch { + return false; + } +} + +function uniqueResolvedPathsInOrder(values: string[]): string[] { + const seen = new Set(); + const resolved: string[] = []; + for (const value of values) { + const normalized = path.resolve(value); + if (seen.has(normalized)) { + continue; + } + seen.add(normalized); + resolved.push(normalized); + } + return resolved; +} + +function ancestorNodeModulesDirs(cwd: string): string[] { + const dirs: string[] = []; + let current = path.resolve(cwd); + + while (true) { + const nodeModulesDir = path.join(current, "node_modules"); + if (isDirectory(nodeModulesDir)) { + dirs.push(nodeModulesDir); + } + + const parent = path.dirname(current); + if (parent === current) { + break; + } + current = parent; + } + + return dirs; +} + +function packageDirInNodeModules( + nodeModulesDir: string, + packageName: string, +): string | undefined { + const packageDir = path.join(nodeModulesDir, ...packageName.split("/")); + return isDirectory(packageDir) ? path.resolve(packageDir) : undefined; +} + +function isValidPackageSegment(segment: string): boolean { + return ( + Boolean(segment) && + segment !== "." && + segment !== ".." && + !segment.startsWith(".") && + /^[A-Za-z0-9._~-]+$/.test(segment) + ); +} + +/** Return whether a string is shaped like an npm package name. */ +export function isValidPackageName(packageName: string): boolean { + if ( + !packageName || + packageName.includes("\\") || + path.isAbsolute(packageName) + ) { + return false; + } + + const parts = packageName.split("/"); + if (parts[0].startsWith("@")) { + return ( + parts.length === 2 && + parts[0].length > 1 && + isValidPackageSegment(parts[0].slice(1)) && + isValidPackageSegment(parts[1]) + ); + } + + return parts.length === 1 && isValidPackageSegment(parts[0]); +} + +function findPackageRoot(entryPath: string): string | undefined { + let dir = path.dirname(entryPath); + while (dir !== path.dirname(dir)) { + if (isFile(path.join(dir, "package.json"))) { + return path.resolve(dir); + } + dir = path.dirname(dir); + } + return undefined; +} + +function findPackageNodeModulesDir( + packageDir: string, + packageName: string, +): string | undefined { + const parts = path.resolve(packageDir).split(path.sep); + const packageParts = packageName.split("/"); + + for (let index = parts.length - 1; index >= 0; index -= 1) { + if (parts[index] !== "node_modules") { + continue; + } + const candidatePackageParts = parts.slice( + index + 1, + index + 1 + packageParts.length, + ); + if (candidatePackageParts.join("/") !== packageParts.join("/")) { + continue; + } + return path.resolve(parts.slice(0, index + 1).join(path.sep) || path.sep); + } + + return undefined; +} + +function resolvePackageWithNode( + cwd: string, + packageName: string, +): ResolvedPackageLocation | undefined { + let requireFromCwd: NodeJS.Require; + try { + requireFromCwd = createRequire(path.join(cwd, "package.json")); + } catch { + return undefined; + } + + for (const specifier of [`${packageName}/package.json`, packageName]) { + try { + const resolved = requireFromCwd.resolve(specifier); + const dir = specifier.endsWith("/package.json") + ? path.dirname(resolved) + : findPackageRoot(resolved); + if (!dir) { + continue; + } + const nodeModulesDir = findPackageNodeModulesDir(dir, packageName); + return { + dir, + ...(nodeModulesDir ? { nodeModulesDir } : {}), + }; + } catch { + // Try the next Node resolution form. + } + } + + return undefined; +} + +/** Resolve a package root from an app cwd without requiring the package to expose a JS entry point. */ +export function resolvePackageLocation( + cwd: string, + packageName: string, + options?: ResolvePackageLocationOptions, +): ResolvedPackageLocation | undefined { + if (!isValidPackageName(packageName)) { + return undefined; + } + + const nodeModulesDirs = uniqueResolvedPathsInOrder([ + ...(options?.nodeModulesDirs ?? []), + ...ancestorNodeModulesDirs(cwd), + ]); + + for (const nodeModulesDir of nodeModulesDirs) { + const dir = packageDirInNodeModules(nodeModulesDir, packageName); + if (dir) { + return { dir, nodeModulesDir }; + } + } + + return resolvePackageWithNode(cwd, packageName); +} + +/** Resolve a package root directory from an app cwd. */ +export function resolvePackageDir( + cwd: string, + packageName: string, +): string | undefined { + return resolvePackageLocation(cwd, packageName)?.dir; +} diff --git a/packages/junior/tests/fixtures/plugin-app.ts b/packages/junior/tests/fixtures/plugin-app.ts new file mode 100644 index 00000000..ba71b517 --- /dev/null +++ b/packages/junior/tests/fixtures/plugin-app.ts @@ -0,0 +1,103 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +export interface PluginAppFixture { + cleanup(): Promise; + root: string; +} + +export interface PluginAppFixtureOptions { + linkNodeModules?: boolean; +} + +async function exists(targetPath: string): Promise { + try { + await fs.stat(targetPath); + return true; + } catch { + return false; + } +} + +async function isDirectory(targetPath: string): Promise { + try { + return (await fs.stat(targetPath)).isDirectory(); + } catch { + return false; + } +} + +async function listPluginDirs(root: string): Promise { + if (await exists(path.join(root, "plugin.yaml"))) { + return [root]; + } + + const entries = await fs.readdir(root, { withFileTypes: true }); + const pluginDirs: string[] = []; + for (const entry of entries) { + if (!entry.isDirectory()) { + continue; + } + const candidate = path.join(root, entry.name); + if (await exists(path.join(candidate, "plugin.yaml"))) { + pluginDirs.push(candidate); + } + } + return pluginDirs; +} + +async function linkNodeModules(root: string, fromCwd: string): Promise { + const source = path.join(fromCwd, "node_modules"); + if (!(await isDirectory(source))) { + return; + } + await fs.symlink(source, path.join(root, "node_modules"), "dir"); +} + +async function nextLinkPath( + pluginsRoot: string, + source: string, +): Promise { + const base = path.basename(source); + let candidate = path.join(pluginsRoot, base); + let suffix = 2; + while (await exists(candidate)) { + candidate = path.join(pluginsRoot, `${base}-${suffix}`); + suffix += 1; + } + return candidate; +} + +/** Create a temporary app/plugins root so tests exercise normal plugin discovery. */ +export async function createPluginAppFixture( + pluginRoots: string[], + options: PluginAppFixtureOptions = {}, +): Promise { + const previousCwd = process.cwd(); + const root = await fs.mkdtemp(path.join(os.tmpdir(), "junior-plugin-app-")); + const pluginsRoot = path.join(root, "app", "plugins"); + await fs.mkdir(pluginsRoot, { recursive: true }); + if (options.linkNodeModules) { + await linkNodeModules(root, previousCwd); + } + + for (const pluginRoot of pluginRoots) { + for (const pluginDir of await listPluginDirs(pluginRoot)) { + await fs.symlink( + pluginDir, + await nextLinkPath(pluginsRoot, pluginDir), + "dir", + ); + } + } + + process.chdir(root); + return { + root, + async cleanup() { + process.chdir(previousCwd); + await fs.rm(root, { recursive: true, force: true }); + }, + }; +} diff --git a/packages/junior/tests/integration/example-build-discovery.test.ts b/packages/junior/tests/integration/example-build-discovery.test.ts index ab2a4521..c9901286 100644 --- a/packages/junior/tests/integration/example-build-discovery.test.ts +++ b/packages/junior/tests/integration/example-build-discovery.test.ts @@ -38,7 +38,6 @@ function buildJuniorPackage(): void { CI: "true", JUNIOR_SKIP_SNAPSHOT: "1", }; - delete env.JUNIOR_EXTRA_PLUGIN_ROOTS; delete env.SKILL_DIRS; execFileSync("pnpm", ["--filter", "@sentry/junior", "build"], { diff --git a/packages/junior/tests/integration/mcp-auth-runtime-slack.test.ts b/packages/junior/tests/integration/mcp-auth-runtime-slack.test.ts index a39f11b1..f96c1fd3 100644 --- a/packages/junior/tests/integration/mcp-auth-runtime-slack.test.ts +++ b/packages/junior/tests/integration/mcp-auth-runtime-slack.test.ts @@ -13,6 +13,10 @@ import { createTestThread, type TestThread, } from "../fixtures/slack-harness"; +import { + createPluginAppFixture, + type PluginAppFixture, +} from "../fixtures/plugin-app"; const { agentProbe, @@ -264,16 +268,18 @@ function expectProcessingReactionLifecycles(args: { } describe("mcp auth runtime slack integration", () => { + let pluginApp: PluginAppFixture | undefined; + beforeEach(async () => { resetAgentProbe(); resetSlackApiMockState(); process.env = { ...ORIGINAL_ENV, JUNIOR_BASE_URL: "https://junior.example.com", - JUNIOR_EXTRA_PLUGIN_ROOTS: JSON.stringify([EVAL_MCP_PLUGIN_ROOT]), JUNIOR_STATE_ADAPTER: "memory", SLACK_BOT_TOKEN: "xoxb-test-token", }; + pluginApp = await createPluginAppFixture([EVAL_MCP_PLUGIN_ROOT]); vi.resetModules(); chatRuntimeModule = await import("../fixtures/chat-runtime"); @@ -289,7 +295,9 @@ describe("mcp auth runtime slack integration", () => { }); afterEach(async () => { - await stateAdapterModule.disconnectStateAdapter(); + await stateAdapterModule?.disconnectStateAdapter(); + await pluginApp?.cleanup(); + pluginApp = undefined; process.env = { ...ORIGINAL_ENV }; }); diff --git a/packages/junior/tests/integration/mcp-oauth-callback-slack.test.ts b/packages/junior/tests/integration/mcp-oauth-callback-slack.test.ts index 5bb94c83..5adaea10 100644 --- a/packages/junior/tests/integration/mcp-oauth-callback-slack.test.ts +++ b/packages/junior/tests/integration/mcp-oauth-callback-slack.test.ts @@ -9,6 +9,10 @@ import { getCapturedSlackFileUploadCalls, resetSlackApiMockState, } from "../msw/handlers/slack-api"; +import { + createPluginAppFixture, + type PluginAppFixture, +} from "../fixtures/plugin-app"; const { generateAssistantReplyMock } = vi.hoisted(() => ({ generateAssistantReplyMock: vi.fn(), @@ -44,6 +48,7 @@ let mcpOauthCallbackHarnessModule: McpOauthCallbackHarnessModule; let pluginRegistryModule: PluginRegistryModule; let stateAdapterModule: StateAdapterModule; let turnSessionStoreModule: TurnSessionStoreModule; +let pluginApp: PluginAppFixture | undefined; async function createPendingAuthSession(args: { conversationId: string; @@ -98,8 +103,8 @@ describe("mcp oauth callback slack integration", () => { ...ORIGINAL_ENV, JUNIOR_STATE_ADAPTER: "memory", JUNIOR_BASE_URL: "https://junior.example.com", - JUNIOR_EXTRA_PLUGIN_ROOTS: JSON.stringify([EVAL_MCP_PLUGIN_ROOT]), }; + pluginApp = await createPluginAppFixture([EVAL_MCP_PLUGIN_ROOT]); vi.resetModules(); artifactStateModule = await import("@/chat/state/artifacts"); @@ -118,7 +123,9 @@ describe("mcp oauth callback slack integration", () => { }); afterEach(async () => { - await stateAdapterModule.disconnectStateAdapter(); + await stateAdapterModule?.disconnectStateAdapter(); + await pluginApp?.cleanup(); + pluginApp = undefined; process.env = { ...ORIGINAL_ENV }; }); diff --git a/packages/junior/tests/integration/oauth-callback-slack.test.ts b/packages/junior/tests/integration/oauth-callback-slack.test.ts index f962ebce..210e9f9e 100644 --- a/packages/junior/tests/integration/oauth-callback-slack.test.ts +++ b/packages/junior/tests/integration/oauth-callback-slack.test.ts @@ -4,6 +4,10 @@ import { getCapturedSlackApiCalls, resetSlackApiMockState, } from "../msw/handlers/slack-api"; +import { + createPluginAppFixture, + type PluginAppFixture, +} from "../fixtures/plugin-app"; const { generateAssistantReplyMock } = vi.hoisted(() => ({ generateAssistantReplyMock: vi.fn(), @@ -27,6 +31,7 @@ type TurnSessionStoreModule = typeof import("@/chat/state/turn-session-store"); let stateAdapterModule: StateAdapterModule; let oauthCallbackHarnessModule: OAuthCallbackHarnessModule; let turnSessionStoreModule: TurnSessionStoreModule; +let pluginApp: PluginAppFixture | undefined; describe("oauth callback slack integration", () => { beforeEach(async () => { @@ -43,8 +48,8 @@ describe("oauth callback slack integration", () => { ...ORIGINAL_ENV, JUNIOR_STATE_ADAPTER: "memory", JUNIOR_BASE_URL: "https://junior.example.com", - JUNIOR_EXTRA_PLUGIN_ROOTS: JSON.stringify([EVAL_OAUTH_PLUGIN_ROOT]), }; + pluginApp = await createPluginAppFixture([EVAL_OAUTH_PLUGIN_ROOT]); vi.resetModules(); stateAdapterModule = await import("@/chat/state/adapter"); oauthCallbackHarnessModule = @@ -55,7 +60,9 @@ describe("oauth callback slack integration", () => { }); afterEach(async () => { - await stateAdapterModule.disconnectStateAdapter(); + await stateAdapterModule?.disconnectStateAdapter(); + await pluginApp?.cleanup(); + pluginApp = undefined; process.env = { ...ORIGINAL_ENV }; }); diff --git a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts index e2f63a52..c69313df 100644 --- a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts @@ -1,5 +1,9 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + createPluginAppFixture, + type PluginAppFixture, +} from "../fixtures/plugin-app"; const ORIGINAL_ENV = { ...process.env }; const FIXTURE_PLUGIN_ROOT = path.resolve( @@ -72,6 +76,7 @@ function proxiedRequest(input: { describe("sandbox egress proxy integration", () => { let modules: LoadedModules; + let pluginApp: PluginAppFixture | undefined; beforeEach(async () => { process.env = { @@ -79,15 +84,17 @@ describe("sandbox egress proxy integration", () => { EVAL_ENABLE_TEST_CREDENTIALS: "1", EVAL_TEST_CREDENTIAL_TOKEN: "integration-egress-token", JUNIOR_BASE_URL: BASE_URL, - JUNIOR_EXTRA_PLUGIN_ROOTS: JSON.stringify([FIXTURE_PLUGIN_ROOT]), JUNIOR_SECRET: "integration-secret", JUNIOR_STATE_ADAPTER: "memory", }; + pluginApp = await createPluginAppFixture([FIXTURE_PLUGIN_ROOT]); modules = await loadModules(); }); afterEach(async () => { - await modules.state.disconnectStateAdapter(); + await modules?.state.disconnectStateAdapter(); + await pluginApp?.cleanup(); + pluginApp = undefined; process.env = { ...ORIGINAL_ENV }; }); diff --git a/packages/junior/tests/unit/app-config.test.ts b/packages/junior/tests/unit/app-config.test.ts new file mode 100644 index 00000000..681af6ee --- /dev/null +++ b/packages/junior/tests/unit/app-config.test.ts @@ -0,0 +1,172 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { createApp } from "@/app"; +import { + getConfigDefaults, + setConfigDefaults, +} from "@/chat/configuration/defaults"; +import { getPluginProviders, setPluginConfig } from "@/chat/plugins/registry"; + +const originalCwd = process.cwd(); +const originalPluginPackages = process.env.JUNIOR_PLUGIN_PACKAGES; +const tempDirs: string[] = []; + +async function makeTempDir(): Promise { + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-app-config-"), + ); + tempDirs.push(tempDir); + return tempDir; +} + +async function writePluginPackage( + root: string, + packageName: string, + pluginName: string, +): Promise { + const packageRoot = path.join( + root, + "node_modules", + ...packageName.split("/"), + ); + await fs.mkdir(packageRoot, { recursive: true }); + await fs.writeFile( + path.join(packageRoot, "plugin.yaml"), + [ + `name: ${pluginName}`, + `description: ${pluginName} plugin`, + "config-keys:", + " - org", + ].join("\n"), + "utf8", + ); +} + +afterEach(async () => { + process.chdir(originalCwd); + setPluginConfig(undefined); + setConfigDefaults(undefined); + if (originalPluginPackages === undefined) { + delete process.env.JUNIOR_PLUGIN_PACKAGES; + } else { + process.env.JUNIOR_PLUGIN_PACKAGES = originalPluginPackages; + } + for (const tempDir of tempDirs.splice(0)) { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); + +describe("createApp plugin config", () => { + it("fails loudly when the env plugin package fallback is malformed", async () => { + process.env.JUNIOR_PLUGIN_PACKAGES = "not-json"; + + await expect(createApp()).rejects.toThrow( + "JUNIOR_PLUGIN_PACKAGES must be valid JSON", + ); + }); + + it("fails loudly when the env plugin package fallback is not a package list", async () => { + process.env.JUNIOR_PLUGIN_PACKAGES = JSON.stringify({ + packages: ["@acme/junior-plugin"], + }); + + await expect(createApp()).rejects.toThrow( + "JUNIOR_PLUGIN_PACKAGES must be a JSON array of package names", + ); + }); + + it("fails loudly when configured plugin package names are invalid", async () => { + await expect( + createApp({ + plugins: { + packages: ["../plugins"], + }, + }), + ).rejects.toThrow("Plugin package names must be valid npm package names"); + }); + + it("fails loudly when configured plugin packages are not an array", async () => { + await expect( + createApp({ + plugins: { + packages: "@acme/junior-plugin" as unknown as string[], + }, + }), + ).rejects.toThrow("plugins.packages must be an array of package names"); + }); + + it("rolls back plugin config when config default validation fails", async () => { + const tempRoot = await makeTempDir(); + await writePluginPackage(tempRoot, "@acme/base-plugin", "base"); + await writePluginPackage(tempRoot, "@acme/next-plugin", "next"); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ + name: "temp-junior-app", + private: true, + dependencies: { + "@acme/base-plugin": "1.0.0", + "@acme/next-plugin": "1.0.0", + }, + }), + "utf8", + ); + process.chdir(tempRoot); + + await createApp({ + plugins: { packages: ["@acme/base-plugin"] }, + configDefaults: { "base.org": "sentry" }, + }); + + await expect( + createApp({ + plugins: { packages: ["@acme/next-plugin"] }, + configDefaults: { "missing.org": "sentry" }, + }), + ).rejects.toThrow( + 'configDefaults: "missing.org" is not a registered plugin config key', + ); + + expect(getPluginProviders().map((plugin) => plugin.manifest.name)).toEqual([ + "base", + ]); + expect(getConfigDefaults()).toEqual({ "base.org": "sentry" }); + }); + + it("fails startup and rolls back config when a configured plugin package is missing", async () => { + const tempRoot = await makeTempDir(); + await writePluginPackage(tempRoot, "@acme/base-plugin", "base"); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ + name: "temp-junior-app", + private: true, + dependencies: { + "@acme/base-plugin": "1.0.0", + }, + }), + "utf8", + ); + process.chdir(tempRoot); + + await createApp({ + plugins: { packages: ["@acme/base-plugin"] }, + configDefaults: { "base.org": "sentry" }, + }); + + await expect( + createApp({ + plugins: { packages: ["@acme/missing-plugin"] }, + }), + ).rejects.toThrow( + 'Plugin package "@acme/missing-plugin" was configured but could not be resolved', + ); + + expect(getPluginProviders().map((plugin) => plugin.manifest.name)).toEqual([ + "base", + ]); + expect(getConfigDefaults()).toEqual({ "base.org": "sentry" }); + }); +}); diff --git a/packages/junior/tests/unit/build/copy-build-content.test.ts b/packages/junior/tests/unit/build/copy-build-content.test.ts new file mode 100644 index 00000000..21121311 --- /dev/null +++ b/packages/junior/tests/unit/build/copy-build-content.test.ts @@ -0,0 +1,283 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + copyAppAndPluginContent, + copyIncludedFiles, +} from "@/build/copy-build-content"; + +const tempDirs: string[] = []; + +function makeTempDir(): string { + const tempDir = fs.mkdtempSync( + path.join(os.tmpdir(), "junior-copy-build-content-"), + ); + tempDirs.push(tempDir); + return tempDir; +} + +function writePackage( + root: string, + packageName: string, + options: { entryPoint?: boolean } = {}, +): string { + const packageDir = path.join(root, "node_modules", ...packageName.split("/")); + fs.mkdirSync(packageDir, { recursive: true }); + const entryPoint = options.entryPoint ?? true; + fs.writeFileSync( + path.join(packageDir, "package.json"), + JSON.stringify({ + name: packageName, + ...(entryPoint ? { main: "index.js" } : {}), + }), + "utf8", + ); + if (entryPoint) { + fs.writeFileSync(path.join(packageDir, "index.js"), "export {};\n", "utf8"); + } + return packageDir; +} + +afterEach(() => { + for (const tempDir of tempDirs.splice(0)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +describe("copyIncludedFiles", () => { + it("copies configured package files into node_modules output", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + copyIncludedFiles(cwd, serverRoot, ["yaml/package.json"]); + + expect( + fs.existsSync( + path.join(serverRoot, "node_modules", "yaml", "package.json"), + ), + ).toBe(true); + }); + + it("fails when a configured package cannot be resolved", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => + copyIncludedFiles(cwd, serverRoot, ["@acme/missing-plugin/dist/*.js"]), + ).toThrow( + 'includeFiles entry "@acme/missing-plugin/dist/*.js" references package "@acme/missing-plugin", but it could not be resolved', + ); + }); + + it("fails when includeFiles is not an array", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => + copyIncludedFiles(cwd, serverRoot, "yaml/package.json"), + ).toThrow("includeFiles must be an array of package subpath patterns"); + }); + + it("fails when an includeFiles entry is not a string pattern", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => copyIncludedFiles(cwd, serverRoot, [42])).toThrow( + "includeFiles entries must be package subpath patterns", + ); + }); + + it("fails when a configured include pattern matches no files", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => + copyIncludedFiles(cwd, serverRoot, ["yaml/dist/no-such-file.js"]), + ).toThrow( + 'includeFiles entry "yaml/dist/no-such-file.js" did not match any files', + ); + }); + + it("fails when a configured include pattern has no package subpath", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => copyIncludedFiles(cwd, serverRoot, ["yaml"])).toThrow( + 'includeFiles entry "yaml" must include a package subpath', + ); + }); + + it("fails when a configured include pattern has a malformed package name", () => { + const cwd = process.cwd(); + const serverRoot = makeTempDir(); + + expect(() => copyIncludedFiles(cwd, serverRoot, ["@/dist/*.js"])).toThrow( + 'includeFiles entry "@/dist/*.js" must include a package subpath', + ); + }); + + it("resolves configured packages from the app root", () => { + const cwd = makeTempDir(); + const serverRoot = makeTempDir(); + const packageDir = writePackage(cwd, "@acme/local-provider"); + fs.mkdirSync(path.join(packageDir, "dist"), { recursive: true }); + fs.writeFileSync( + path.join(packageDir, "dist", "provider.js"), + "export {};\n", + "utf8", + ); + fs.writeFileSync( + path.join(cwd, "package.json"), + JSON.stringify({ name: "app" }), + "utf8", + ); + + copyIncludedFiles(cwd, serverRoot, ["@acme/local-provider/dist/*.js"]); + + expect( + fs.readFileSync( + path.join( + serverRoot, + "node_modules", + "@acme", + "local-provider", + "dist", + "provider.js", + ), + "utf8", + ), + ).toBe("export {};\n"); + }); + + it("fails when a matched include pattern copies no existing files", () => { + const cwd = makeTempDir(); + const serverRoot = makeTempDir(); + const packageDir = writePackage(cwd, "@acme/broken-provider"); + const distDir = path.join(packageDir, "dist"); + fs.mkdirSync(distDir, { recursive: true }); + fs.symlinkSync( + path.join(distDir, "missing.js"), + path.join(distDir, "broken.js"), + ); + + expect(() => + copyIncludedFiles(cwd, serverRoot, ["@acme/broken-provider/dist/*.js"]), + ).toThrow( + 'includeFiles entry "@acme/broken-provider/dist/*.js" matched files', + ); + }); + + it("resolves configured packages from ancestor node_modules without a package entry point", () => { + const workspaceRoot = makeTempDir(); + const cwd = path.join(workspaceRoot, "apps", "example"); + const serverRoot = makeTempDir(); + const packageDir = writePackage(workspaceRoot, "@acme/content-provider", { + entryPoint: false, + }); + fs.mkdirSync(path.join(packageDir, "dist"), { recursive: true }); + fs.writeFileSync( + path.join(packageDir, "dist", "provider.js"), + "export {};\n", + "utf8", + ); + fs.mkdirSync(cwd, { recursive: true }); + fs.writeFileSync( + path.join(cwd, "package.json"), + JSON.stringify({ + name: "example", + dependencies: { + "@acme/content-provider": "1.0.0", + }, + }), + "utf8", + ); + fs.writeFileSync( + path.join(workspaceRoot, "package.json"), + JSON.stringify({ name: "workspace", private: true }), + "utf8", + ); + + copyIncludedFiles(cwd, serverRoot, ["@acme/content-provider/dist/*.js"]); + + expect( + fs.readFileSync( + path.join( + serverRoot, + "node_modules", + "@acme", + "content-provider", + "dist", + "provider.js", + ), + "utf8", + ), + ).toBe("export {};\n"); + }); +}); + +describe("copyAppAndPluginContent", () => { + it("copies configured plugin packages resolved from ancestor node_modules", () => { + const workspaceRoot = makeTempDir(); + const cwd = path.join(workspaceRoot, "apps", "example"); + const serverRoot = makeTempDir(); + const packageDir = writePackage(workspaceRoot, "@acme/ancestor-plugin", { + entryPoint: false, + }); + + fs.mkdirSync(path.join(packageDir, "skills", "demo"), { recursive: true }); + fs.writeFileSync( + path.join(packageDir, "plugin.yaml"), + "name: ancestor\ndescription: Ancestor plugin\n", + "utf8", + ); + fs.writeFileSync( + path.join(packageDir, "skills", "demo", "SKILL.md"), + "---\nname: demo\ndescription: Demo\n---\n", + "utf8", + ); + fs.mkdirSync(cwd, { recursive: true }); + fs.writeFileSync( + path.join(cwd, "package.json"), + JSON.stringify({ + name: "example", + dependencies: { + "@acme/ancestor-plugin": "1.0.0", + }, + }), + "utf8", + ); + fs.writeFileSync( + path.join(workspaceRoot, "package.json"), + JSON.stringify({ name: "workspace", private: true }), + "utf8", + ); + + copyAppAndPluginContent(cwd, serverRoot, ["@acme/ancestor-plugin"]); + + expect( + fs.existsSync( + path.join( + serverRoot, + "node_modules", + "@acme", + "ancestor-plugin", + "plugin.yaml", + ), + ), + ).toBe(true); + expect( + fs.existsSync( + path.join( + serverRoot, + "node_modules", + "@acme", + "ancestor-plugin", + "skills", + "demo", + "SKILL.md", + ), + ), + ).toBe(true); + }); +}); diff --git a/packages/junior/tests/unit/config/config-defaults.test.ts b/packages/junior/tests/unit/config/config-defaults.test.ts index f2c941e7..c4af6432 100644 --- a/packages/junior/tests/unit/config/config-defaults.test.ts +++ b/packages/junior/tests/unit/config/config-defaults.test.ts @@ -39,10 +39,43 @@ describe("install config defaults", () => { ); }); + it("rejects null defaults", () => { + expect(() => + setConfigDefaults(null as unknown as Record), + ).toThrow("configDefaults must be an object keyed by plugin config key"); + }); + + it("rejects array defaults", () => { + expect(() => + setConfigDefaults([] as unknown as Record), + ).toThrow("configDefaults must be an object keyed by plugin config key"); + }); + it("does not mutate the input object", () => { const input = { "sentry.org": "sentry" }; setConfigDefaults(input); input["sentry.org"] = "changed"; expect(getConfigDefaults()["sentry.org"]).toBe("sentry"); }); + + it("does not share nested input values", () => { + const input = { + "sentry.org": { slug: "sentry" }, + }; + setConfigDefaults(input); + input["sentry.org"].slug = "changed"; + expect(getConfigDefaults()["sentry.org"]).toEqual({ slug: "sentry" }); + }); + + it("does not expose mutable defaults", () => { + setConfigDefaults({ "sentry.org": "sentry" }); + getConfigDefaults()["sentry.org"] = "changed"; + expect(getConfigDefaults()["sentry.org"]).toBe("sentry"); + }); + + it("does not expose nested mutable defaults", () => { + setConfigDefaults({ "sentry.org": { slug: "sentry" } }); + (getConfigDefaults()["sentry.org"] as { slug: string }).slug = "changed"; + expect(getConfigDefaults()["sentry.org"]).toEqual({ slug: "sentry" }); + }); }); diff --git a/packages/junior/tests/unit/config/package-discovery.test.ts b/packages/junior/tests/unit/config/package-discovery.test.ts index be56afd7..d5e79a56 100644 --- a/packages/junior/tests/unit/config/package-discovery.test.ts +++ b/packages/junior/tests/unit/config/package-discovery.test.ts @@ -7,9 +7,26 @@ import { discoverInstalledPluginPackageContent } from "@/chat/plugins/package-di async function writePluginPackage( nodeModulesRoot: string, packageName: string, + options: { entryPoint?: boolean } = {}, ): Promise { const packageRoot = path.join(nodeModulesRoot, ...packageName.split("/")); await fs.mkdir(path.join(packageRoot, "skills", "demo"), { recursive: true }); + const entryPoint = options.entryPoint ?? true; + await fs.writeFile( + path.join(packageRoot, "package.json"), + JSON.stringify({ + name: packageName, + ...(entryPoint ? { main: "index.js" } : {}), + }), + "utf8", + ); + if (entryPoint) { + await fs.writeFile( + path.join(packageRoot, "index.js"), + "export {};\n", + "utf8", + ); + } await fs.writeFile( path.join(packageRoot, "plugin.yaml"), "name: demo\ndescription: demo\n", @@ -66,6 +83,100 @@ describe("plugin package discovery", () => { ); }); + it("fails when an explicit plugin package is not installed", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + await fs.mkdir(path.join(tempRoot, "node_modules"), { recursive: true }); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ name: "temp", private: true }), + "utf8", + ); + + expect(() => + discoverInstalledPluginPackageContent(tempRoot, { + packageNames: ["@acme/missing-plugin"], + }), + ).toThrow( + 'Plugin package "@acme/missing-plugin" was configured but could not be resolved from node_modules', + ); + }); + + it("reports configured package resolution errors when cwd has no package manifest", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + + expect(() => + discoverInstalledPluginPackageContent(tempRoot, { + packageNames: ["@acme/missing-plugin"], + }), + ).toThrow( + 'Plugin package "@acme/missing-plugin" was configured but could not be resolved', + ); + }); + + it("fails when an explicit plugin package is not a package name", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ name: "temp", private: true }), + "utf8", + ); + + expect(() => + discoverInstalledPluginPackageContent(tempRoot, { + packageNames: ["../plugins"], + }), + ).toThrow("Plugin package names must be valid npm package names"); + }); + + it("fails when an explicit scoped plugin package is malformed", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ name: "temp", private: true }), + "utf8", + ); + + expect(() => + discoverInstalledPluginPackageContent(tempRoot, { + packageNames: ["@acme"], + }), + ).toThrow("Plugin package names must be valid npm package names"); + }); + + it("fails when an explicit plugin package has no plugin content", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + const packageRoot = path.join( + tempRoot, + "node_modules", + "@acme", + "not-a-plugin", + ); + await fs.mkdir(packageRoot, { recursive: true }); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ name: "temp", private: true }), + "utf8", + ); + + expect(() => + discoverInstalledPluginPackageContent(tempRoot, { + packageNames: ["@acme/not-a-plugin"], + }), + ).toThrow( + 'Plugin package "@acme/not-a-plugin" was configured but does not contain plugin content', + ); + }); + it("keeps nearest node_modules package when duplicate package names exist", async () => { const tempRoot = await fs.mkdtemp( path.join(os.tmpdir(), "junior-package-discovery-"), @@ -147,6 +258,44 @@ describe("plugin package discovery", () => { ); }); + it("resolves explicit packageNames through ancestor node_modules package resolution", async () => { + const tempRoot = await fs.mkdtemp( + path.join(os.tmpdir(), "junior-package-discovery-"), + ); + const appRoot = path.join(tempRoot, "apps", "example"); + const packageRoot = await writePluginPackage( + path.join(tempRoot, "node_modules"), + "@acme/junior-plugin-ancestor", + { entryPoint: false }, + ); + + await fs.mkdir(appRoot, { recursive: true }); + await fs.writeFile( + path.join(appRoot, "package.json"), + JSON.stringify({ + name: "example", + dependencies: { + "@acme/junior-plugin-ancestor": "1.0.0", + }, + }), + "utf8", + ); + await fs.writeFile( + path.join(tempRoot, "package.json"), + JSON.stringify({ name: "workspace", private: true }), + "utf8", + ); + + const discovered = discoverInstalledPluginPackageContent(appRoot, { + packageNames: ["@acme/junior-plugin-ancestor"], + }); + + expect(discovered.packageNames).toContain("@acme/junior-plugin-ancestor"); + expect(discovered.manifestRoots).toContain(packageRoot); + expect(discovered.skillRoots).toContain(path.join(packageRoot, "skills")); + expect(discovered.tracingIncludes).toEqual([]); + }); + it("does not fallback scan when explicit packageNames is empty", async () => { const tempRoot = await fs.mkdtemp( path.join(os.tmpdir(), "junior-package-discovery-"), diff --git a/packages/junior/tests/unit/plugins/plugin-registry-packages.test.ts b/packages/junior/tests/unit/plugins/plugin-registry-packages.test.ts index 6fa49592..f247acbd 100644 --- a/packages/junior/tests/unit/plugins/plugin-registry-packages.test.ts +++ b/packages/junior/tests/unit/plugins/plugin-registry-packages.test.ts @@ -5,16 +5,19 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { PluginConfig } from "@/chat/plugins/types"; const originalCwd = process.cwd(); +let configuredPackageNames: string[] = []; async function setPackages(packageNames: string[]): Promise { - const { setPluginPackages } = - await import("@/chat/plugins/package-discovery"); - setPluginPackages(packageNames); + configuredPackageNames = packageNames; + await setConfig({ packages: packageNames }); } async function setConfig(config: PluginConfig): Promise { const { setPluginConfig } = await import("@/chat/plugins/registry"); - setPluginConfig(config); + setPluginConfig({ + ...config, + packages: config.packages ?? configuredPackageNames, + }); } async function expectRegistryLoadFailure( @@ -530,6 +533,7 @@ async function writeBundlingOnlyPlugin(tempRoot: string): Promise { } afterEach(() => { + configuredPackageNames = []; process.chdir(originalCwd); vi.resetModules(); vi.doUnmock("@/chat/discovery"); diff --git a/packages/junior/tests/unit/skills/skills.test.ts b/packages/junior/tests/unit/skills/skills.test.ts index a1c73c64..415ec4cd 100644 --- a/packages/junior/tests/unit/skills/skills.test.ts +++ b/packages/junior/tests/unit/skills/skills.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { createPluginAppFixture } from "../../fixtures/plugin-app"; import { getCapabilityProvider } from "@/chat/capabilities/catalog"; import { discoverSkills, @@ -31,16 +32,10 @@ const stubSkills: SkillMetadata[] = [ disableModelInvocation: true, }, ]; -const ORIGINAL_EXTRA_PLUGIN_ROOTS = process.env.JUNIOR_EXTRA_PLUGIN_ROOTS; describe("skills", () => { afterEach(() => { resetSkillDiscoveryCache(); - if (ORIGINAL_EXTRA_PLUGIN_ROOTS === undefined) { - delete process.env.JUNIOR_EXTRA_PLUGIN_ROOTS; - } else { - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = ORIGINAL_EXTRA_PLUGIN_ROOTS; - } }); it("discovers valid skills from configured skill directories", async () => { @@ -225,20 +220,24 @@ describe("skills", () => { "utf8", ); - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify([pluginRoot]); + const pluginApp = await createPluginAppFixture([pluginRoot]); resetSkillDiscoveryCache(); - const skills = await discoverSkills(); - expect( - skills.find((skill) => skill.name === "demo-connect"), - ).toMatchObject({ - name: "demo-connect", - pluginProvider: "demo", - }); - expect(getCapabilityProvider("demo.read")).toMatchObject({ - provider: "demo", - capabilities: ["demo.read"], - }); + try { + const skills = await discoverSkills(); + expect( + skills.find((skill) => skill.name === "demo-connect"), + ).toMatchObject({ + name: "demo-connect", + pluginProvider: "demo", + }); + expect(getCapabilityProvider("demo.read")).toMatchObject({ + provider: "demo", + capabilities: ["demo.read"], + }); + } finally { + await pluginApp.cleanup(); + } } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -278,16 +277,20 @@ describe("skills", () => { "utf8", ); - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify([pluginRoot]); + const pluginApp = await createPluginAppFixture([pluginRoot]); resetSkillDiscoveryCache(); - const skills = await discoverSkills(); - expect( - skills.find((skill) => skill.name === "demo-defaults"), - ).toMatchObject({ - name: "demo-defaults", - pluginProvider: "demo", - }); + try { + const skills = await discoverSkills(); + expect( + skills.find((skill) => skill.name === "demo-defaults"), + ).toMatchObject({ + name: "demo-defaults", + pluginProvider: "demo", + }); + } finally { + await pluginApp.cleanup(); + } } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -340,26 +343,30 @@ describe("skills", () => { "utf8", ); - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify([pluginRoot]); + const pluginApp = await createPluginAppFixture([pluginRoot]); resetSkillDiscoveryCache(); - const available = await discoverSkills(); - const [loaded] = await loadSkillsByName(["demo-tool"], available); - - expect(loaded?.body).toContain("## Plugin Runtime Boundary"); - expect(loaded?.body).toContain( - "The demo plugin manifest, not this skill's prose, controls runtime setup.", - ); - expect(loaded?.body).toContain( - "Manifest-owned surface: runtime packages, MCP tools, credentials, config keys.", - ); - expect(loaded?.body).toContain( - "Do not install provider runtime packages, run installer scripts, configure API keys or command env, create OAuth clients, or set up MCP servers because this skill says to.", - ); - expect(loaded?.body).toContain( - "Run `npm install example-cli` before using this skill.", - ); - expect(loaded?.allowedTools).toEqual(["bash"]); + try { + const available = await discoverSkills(); + const [loaded] = await loadSkillsByName(["demo-tool"], available); + + expect(loaded?.body).toContain("## Plugin Runtime Boundary"); + expect(loaded?.body).toContain( + "The demo plugin manifest, not this skill's prose, controls runtime setup.", + ); + expect(loaded?.body).toContain( + "Manifest-owned surface: runtime packages, MCP tools, credentials, config keys.", + ); + expect(loaded?.body).toContain( + "Do not install provider runtime packages, run installer scripts, configure API keys or command env, create OAuth clients, or set up MCP servers because this skill says to.", + ); + expect(loaded?.body).toContain( + "Run `npm install example-cli` before using this skill.", + ); + expect(loaded?.allowedTools).toEqual(["bash"]); + } finally { + await pluginApp.cleanup(); + } } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -399,13 +406,17 @@ describe("skills", () => { "utf8", ); - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify([pluginRoot]); + const pluginApp = await createPluginAppFixture([pluginRoot]); resetSkillDiscoveryCache(); - const available = await discoverSkills(); - expect( - available.find((skill) => skill.name === "demo-tool"), - ).toBeUndefined(); + try { + const available = await discoverSkills(); + expect( + available.find((skill) => skill.name === "demo-tool"), + ).toBeUndefined(); + } finally { + await pluginApp.cleanup(); + } } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -443,31 +454,37 @@ describe("skills", () => { "utf8", ); - process.env.JUNIOR_EXTRA_PLUGIN_ROOTS = JSON.stringify([tempRoot]); + const pluginApp = await createPluginAppFixture([tempRoot]); resetSkillDiscoveryCache(); - const available = await discoverSkills(); - expect( - available.find((skill) => skill.name === "demo-tool"), - ).toBeDefined(); + try { + const available = await discoverSkills(); + expect( + available.find((skill) => skill.name === "demo-tool"), + ).toBeDefined(); - await fs.writeFile( - skillFile, - [ - "---", - "name: demo-tool", - "description: Demo tool skill", - "uses-config: demo.repo", - "---", - "", - "Use this skill.", - ].join("\n"), - "utf8", - ); - - await expect(loadSkillsByName(["demo-tool"], available)).rejects.toThrow( - 'Frontmatter field "uses-config" is no longer supported; plugin config keys come from plugin.yaml.', - ); + await fs.writeFile( + skillFile, + [ + "---", + "name: demo-tool", + "description: Demo tool skill", + "uses-config: demo.repo", + "---", + "", + "Use this skill.", + ].join("\n"), + "utf8", + ); + + await expect( + loadSkillsByName(["demo-tool"], available), + ).rejects.toThrow( + 'Frontmatter field "uses-config" is no longer supported; plugin config keys come from plugin.yaml.', + ); + } finally { + await pluginApp.cleanup(); + } } finally { await fs.rm(tempRoot, { recursive: true, force: true }); }