diff --git a/src/bundles/startup.ts b/src/bundles/startup.ts index 556e7725..87ce309f 100644 --- a/src/bundles/startup.ts +++ b/src/bundles/startup.ts @@ -50,8 +50,7 @@ import { validateBundleUrl } from "./url-validator.ts"; * * Absent for in-process platform sources (which don't go through * `startBundleSource` anyway) and for paths that don't yet plumb the - * deps (boot reload, configureBundle restart, connector eager-start — - * follow-up). + * deps (boot reload, connector eager-start — follow-up). */ export interface BundleMcpDeps { workspaceId: string; @@ -500,7 +499,7 @@ export async function startBundleSource( // pre-#195 installs whose ref doesn't carry the field. Mirrors the // URL-branch pattern below — without this the registered source // name would diverge from what install persisted, breaking - // `manage_app uninstall` for every catalog-installed mpak bundle + // uninstall for every catalog-installed mpak bundle // whose canonical id and package name produce different slugs // (e.g. `dev.mpak.nimblebraininc/echo` vs `@nimblebraininc/echo`). const serverName = ref.serverName ?? deriveServerName(ref.name); @@ -663,7 +662,7 @@ function buildLocalSource( /** * Slugified canonical reverse-DNS form persisted at install time. * When present, used as the source name so the registered key - * matches what `manage_app uninstall` looks up; falls back to + * matches what uninstall looks up; falls back to * `deriveServerName(manifest.name)` for legacy installs. */ serverName?: string; @@ -692,8 +691,7 @@ function buildLocalSource( // Mirror the named-bundle branch: honor a persisted ref.serverName // (slugified canonical id from install) before falling back to the // legacy short slug. Keeps registered source name in lockstep with - // what consumers (manage_app uninstall, lifecycle Map, web routes) - // look up by. + // what consumers (uninstall, lifecycle Map, web routes) look up by. const serverName = ref.serverName ?? deriveServerName(manifest.name); validateServerName(serverName); const mcpConfig = manifest.server.mcp_config; diff --git a/src/bundles/workspace-ops.ts b/src/bundles/workspace-ops.ts index bbe27a49..a2dc6e59 100644 --- a/src/bundles/workspace-ops.ts +++ b/src/bundles/workspace-ops.ts @@ -1,7 +1,10 @@ /** - * Workspace-scoped bundle install/uninstall operations. + * Workspace-scoped bundle install operation. * - * These are consumed by system tools for hot bundle management within workspaces. + * Consumed by connector install for hot bundle management within a + * workspace. (Uninstall is owned by `BundleLifecycleManager.uninstall`, + * which resolves the server name, clears credentials, and unregisters + * placements/config/automations in one place.) */ import type { EventSink } from "../engine/types.ts"; @@ -38,21 +41,20 @@ export async function installBundleInWorkspace( allowInsecureRemotes?: boolean; workDir?: string; /** - * Per-workspace host-resources deps. Caller (`system-tools` / - * `manage_app install`, `connector-tools`) pulls from Runtime. - * Passed through to `startBundleSource` so the spawned bundle's - * McpSource registers inbound `ai.nimblebrain/resources/*` handlers. + * Per-workspace host-resources deps. Caller (`connector-tools`, + * catalog/hot install) pulls from Runtime. Passed through to + * `startBundleSource` so the spawned bundle's McpSource registers + * inbound `ai.nimblebrain/resources/*` handlers. */ bundleMcp?: BundleMcpDeps; }, ): Promise { - // workDir default matches the sibling `uninstallBundleFromWorkspace` - // below — previously this function fell through to `""` and emitted - // relative paths from cwd (a latent bug). The new default routes - // through `~/.nimblebrain`, matching every other workspace-scoped - // entry point. A caller that explicitly passes `workDir: ""` now - // hits the `WorkspaceContext` constructor's empty-string rejection - // (deliberate — relative paths in this code path were never correct). + // Default workDir to `~/.nimblebrain` — previously this function fell + // through to `""` and emitted relative paths from cwd (a latent bug), + // out of step with every other workspace-scoped entry point. A caller + // that explicitly passes `workDir: ""` now hits the `WorkspaceContext` + // constructor's empty-string rejection (deliberate — relative paths in + // this code path were never correct). const workDir = opts?.workDir ?? defaultWorkDir(); const wsContext = new WorkspaceContext({ wsId, workDir }); const serverName = serverNameFromRef(bundleRef); @@ -83,44 +85,3 @@ export async function installBundleInWorkspace( meta: result.meta, }; } - -/** - * Uninstall a bundle from a specific workspace (hot — stops process and deregisters). - * - * Stops the MCP source, removes it from the registry, and clears the - * workspace-scoped credential file for the bundle (best-effort — - * failures are logged but do not fail the uninstall). Data directories - * are intentionally preserved. - * - * `serverName` is the resolved lifecycle key — caller is responsible - * for reading it from the persisted `BundleRef.serverName` (set at - * install time from `slugifyServerName(entry.id)`) with - * `deriveServerName(bundleName)` as a back-compat fallback for legacy - * refs. Passing the canonical name here would skip the slug and miss - * the registered source. - */ -export async function uninstallBundleFromWorkspace( - wsId: string, - bundleName: string, - serverName: string, - registry: ToolRegistry, - opts?: { workDir?: string }, -): Promise { - if (!registry.hasSource(serverName)) { - throw new Error(`No bundle "${serverName}" found in workspace "${wsId}"`); - } - - await registry.removeSource(serverName); - - // Best-effort credential cleanup — don't fail uninstall if it errors. - // Credentials are config, not data: they should not persist across uninstalls. - const workDir = opts?.workDir ?? defaultWorkDir(); - try { - await new WorkspaceContext({ wsId, workDir }).getCredentialStore().clearAll(bundleName); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - process.stderr.write( - `[workspace-ops] Failed to clear credentials for ${bundleName} in ${wsId}: ${msg}\n`, - ); - } -} diff --git a/src/cli/commands.ts b/src/cli/commands.ts index e0d772ff..2ec23edd 100644 --- a/src/cli/commands.ts +++ b/src/cli/commands.ts @@ -73,9 +73,8 @@ export function bundleList(configPath?: string, json?: boolean): void { /** nb bundle add — deprecated, bundles are now workspace-scoped */ export function bundleAdd(name: string, _configPath?: string): void { console.error( - `Instance-level bundles have been removed. Use workspace-scoped bundle management instead:\n` + - ` nb__manage_app install ${name}\n` + - `Or add the bundle to your workspace definition.`, + `Instance-level bundles have been removed. Install ${name} from the Apps catalog in settings, ` + + `or add the bundle to your workspace definition.`, ); process.exit(1); } @@ -97,9 +96,8 @@ export function bundleAddRemote( /** nb bundle remove — deprecated, bundles are now workspace-scoped */ export function bundleRemove(name: string, _configPath?: string): void { console.error( - `Instance-level bundles have been removed. Use workspace-scoped bundle management instead:\n` + - ` nb__manage_app uninstall ${name}\n` + - `Or remove the bundle from your workspace definition.`, + `Instance-level bundles have been removed. Uninstall ${name} from the Apps section in settings, ` + + `or remove the bundle from your workspace definition.`, ); process.exit(1); } diff --git a/src/config/features.ts b/src/config/features.ts index 791f01b6..398c14cc 100644 --- a/src/config/features.ts +++ b/src/config/features.ts @@ -5,6 +5,13 @@ * Opt-in flags (default false) are documented inline. */ export interface FeatureFlags { + /** + * Reserved. Gated conversational bundle install/uninstall/configure via + * `nb__manage_app`, which was removed (install/configure now live in the + * Apps catalog + CLI). Kept as a stable operator config knob for the + * bundle-management tool the delegation model contemplates reintroducing + * (single tool, explicit workspace param, per-call admin auth). + */ bundleManagement?: boolean; skillManagement?: boolean; delegation?: boolean; @@ -51,7 +58,6 @@ export function resolveFeatures(config?: FeatureFlags): ResolvedFeatures { */ export const FEATURE_TOOL_MAP: Record = { // Prefixed names (as seen by the LLM / MCP clients) - nb__manage_app: "bundleManagement", nb__delegate: "delegation", // Identity & workspace tools nb__manage_users: "userManagement", @@ -64,7 +70,6 @@ export const FEATURE_TOOL_MAP: Record = { skills__deactivate: "skillManagement", skills__move_scope: "skillManagement", // Unprefixed names (used during system tool registration) - manage_app: "bundleManagement", delegate: "delegation", manage_users: "userManagement", manage_workspaces: "workspaceManagement", diff --git a/src/config/privilege.ts b/src/config/privilege.ts index 0d703c93..fb6a2262 100644 --- a/src/config/privilege.ts +++ b/src/config/privilege.ts @@ -44,11 +44,6 @@ interface PrivilegeEntry { } const PRIVILEGE_CANDIDATES: PrivilegeEntry[] = [ - { - tool: "nb__manage_app", - feature: "bundleManagement", - describe: (input) => `${input.action} ${input.name}?`, - }, { // Creates land in the prompt as soon as they're written (always-load // skills) or on the next applicable turn (tool_affined). Gating @@ -102,14 +97,9 @@ export function createPrivilegeHook( const description = entry.describe(call.input); const approved = await gate.confirm(description, call.input); if (!approved) { - // Derive `action` from `input.action` when present (legacy shape used - // by `nb__manage_app`); otherwise fall back to the unprefixed tool - // name (`delete`, `move_scope` etc.) so audit consumers always see - // a non-empty action label. - const action = - typeof call.input.action === "string" - ? call.input.action - : (call.name.split("__").pop() ?? call.name); + // Audit label: the unprefixed tool name (`create`, `delete`, + // `move_scope`) so consumers always see a non-empty action. + const action = call.name.split("__").pop() ?? call.name; const target = call.input.name ?? call.input.id ?? null; eventSink.emit({ type: "audit.permission_denied", diff --git a/src/config/workspace-credentials.ts b/src/config/workspace-credentials.ts index 7d139d70..76eb1355 100644 --- a/src/config/workspace-credentials.ts +++ b/src/config/workspace-credentials.ts @@ -348,8 +348,9 @@ export interface ResolveUserConfigInput { gate?: ConfirmationGate; /** * If `true`, prompt for every field via the gate and persist responses to - * the workspace store. Used by the `nb__manage_app configure` TUI action. - * No effect when `gate?.supportsInteraction` is false. + * the workspace store — the interactive "re-enter all credentials" path, + * for updating existing values. No effect when `gate?.supportsInteraction` + * is false. */ forcePrompt?: boolean; } diff --git a/src/runtime/runtime.ts b/src/runtime/runtime.ts index 2f74ec18..1866d70d 100644 --- a/src/runtime/runtime.ts +++ b/src/runtime/runtime.ts @@ -251,8 +251,8 @@ export class Runtime { * Per-workspace host-resources deps factory. Set in `Runtime.start()` * after the resolver + rate-limit are constructed; consumed by every * install path that spawns a bundle (lifecycle.installNamed/Local/ - * Remote, system-tools manage_app install, connector-tools install, - * workspace-runtime boot reload). Returns `undefined` only when the + * Remote, connector-tools install, workspace-runtime boot reload). + * Returns `undefined` only when the * runtime is constructed without the host-resources subsystem wired * — never in production. */ @@ -281,7 +281,6 @@ export class Runtime { private readonly activeConversations = new Set(); private constructor( - _engine: AgentEngine, resolveModelFn: (modelString: string) => LanguageModelV3, store: ConversationStore, skillMatcher: SkillMatcher, @@ -486,9 +485,9 @@ export class Runtime { }, }; - // System tools (search, manage_app, bundle_status, delegate). Skill - // mutation lives in the dedicated `nb__skills` source — registered - // separately via `createPlatformSources`. + // System tools (search, status, delegate). Skill mutation lives in the + // dedicated `nb__skills` source — registered separately via + // `createPlatformSources`. // Use a late-bound holder so reloadSkills can reference `rt` after construction. const rtHolder: { rt?: Runtime } = {}; const boundReloadSkills = async () => { @@ -514,20 +513,6 @@ export class Runtime { const store = buildStore(config); const { contextSkills, skillMatcher } = buildSkills(config); - const defaultModelId = getDefaultModel(); - // Workspace-aware ToolRouter proxy: the engine calls availableTools()/execute() - // within runWithRequestContext(), so the proxy reads the current workspace's registry. - const workspaceToolRouter: ToolRouter = { - availableTools: () => { - if (!rtHolder.rt) throw new Error("Runtime not initialized"); - return rtHolder.rt.getRegistryForCurrentWorkspace().availableTools(); - }, - execute: (call) => { - if (!rtHolder.rt) throw new Error("Runtime not initialized"); - return rtHolder.rt.getRegistryForCurrentWorkspace().execute(call); - }, - }; - const engine = new AgentEngine(resolveModelFn(defaultModelId), workspaceToolRouter, events); // Request-scoped context — all identity/workspace reads go through AsyncLocalStorage. // Set via runWithRequestContext() in chat(), handleToolCall(), and MCP handler. @@ -541,24 +526,6 @@ export class Runtime { const manageUsersCtx = { getIdentity, userStore, provider: identityProvider }; const manageWorkspacesCtx = { getIdentity, workspaceStore }; const manageMembersCtx = { getIdentity, workspaceStore, userStore }; - const manageBundleCtx = { - getWorkspaceId, - workspaceStore, - workDir: resolveWorkDir(config), - configDir: config.configPath ? dirname(config.configPath) : undefined, - allowInsecureRemotes: config.allowInsecureRemotes, - // The runtime sink flows into every McpSource spawned by manage_app - // install/configure. Keeps chat-initiated bundle installs on the same - // live-update pipeline as boot-time bundle startup. - eventSink: events, - // Per-workspace host-resources deps factory. `installBundleInWorkspaceViaCtx` - // calls this for the target workspace and threads the result through - // `installBundleInWorkspace`'s opts so the spawned McpSource registers - // `ai.nimblebrain/resources/*` handlers. Without this, the agent's - // `manage_app install` path silently bypasses the host-resources - // capability — the production path that needs it most. - bundleMcpDepsFactory, - }; const noActiveToolPromotionRun = (toolName: string): ToolPromotionResult => ({ ok: false, toolName, @@ -620,7 +587,6 @@ export class Runtime { // Create Runtime with empty workspace registries first — needed by system tools const rt = new Runtime( - engine, resolveModelFn, store, skillMatcher, @@ -666,7 +632,7 @@ export class Runtime { manageUsersCtx, manageWorkspacesCtx, manageMembersCtx, - manageBundleCtx, + undefined, // reserved slot — was manageBundleCtx (nb__manage_app, removed) toolPromotionCtx, toolEligibilityCtx, ); @@ -940,7 +906,7 @@ export class Runtime { ...skill, body: skill.body + - `\n\n⚠️ Missing dependencies: ${missing.join(", ")}. Some capabilities may be unavailable. Install with nb__manage_app.`, + `\n\n⚠️ Missing dependencies: ${missing.join(", ")}. Some capabilities may be unavailable. Install the missing apps from the Apps catalog in settings.`, }; } } diff --git a/src/skills/core/bootstrap.md b/src/skills/core/bootstrap.md index 8c4e0079..4aa62520 100644 --- a/src/skills/core/bootstrap.md +++ b/src/skills/core/bootstrap.md @@ -31,10 +31,6 @@ metadata: - **nb__search** — Unified search tool. Use `scope: "tools"` to search installed tools by keyword (empty query lists everything). Use `scope: "registry"` to search the mpak registry for installable bundles. - **nb__manage_tools** — Patch your active tool list in one call. Input: `{ add?: ["source__tool", ...], remove?: ["source__tool", ...] }`. Use `add` to promote discovered tools so they become callable on the next turn. Use `remove` to release tools you no longer need (system tools `nb__*` cannot be released). Combine `add` and `remove` in one call when switching domains. - **nb__status** — Platform status. Default gives an overview (model, app count, skill count). Use `scope: "bundles"` for per-app health/version, `scope: "skills"` for loaded skills, `scope: "config"` for model and limit details. -- **nb__manage_app** — Install, uninstall, or configure apps: - - `install` — Download, prompt for credentials if needed, start. - - `uninstall` — Stop and remove. - - `configure` — Re-prompt for credentials on an existing app. ## Tool Discovery Workflow @@ -49,12 +45,11 @@ Never guess tool names. Never skip step 2 — a tool not in your active list is ## Credentials -All credentials are collected by the terminal — never in chat. +App credentials are never collected in chat. -- During install: if the bundle needs credentials, the terminal prompts automatically. -- After success: if the result says "Credentials were configured" — the bundle is ready. Do not suggest further setup. -- To update credentials: use `manage_app("configure", "bundle-name")`. -- If a bundle fails to start: offer to reconfigure. Nothing else. +- Installing apps and setting their credentials happens in the Apps section of settings (or the `nb` CLI), where each bundle's config fields are prompted securely. You cannot install or set credentials from chat. +- If a user asks to install an app or update its credentials, point them to the Apps section of settings. +- If a bundle fails to start for lack of credentials: tell the user and point them to settings. Nothing else. - **Never mention API keys, tokens, passwords, or connection strings in chat.** ## User Preferences diff --git a/src/skills/core/skill-authoring.md b/src/skills/core/skill-authoring.md index bea71515..e43bd307 100644 --- a/src/skills/core/skill-authoring.md +++ b/src/skills/core/skill-authoring.md @@ -78,7 +78,7 @@ If the skill needs specific tools: - Set allowed_tools to scope tool visibility: ["policy_search__*"] - Set requires_bundles to declare dependencies: ["@acme/policy-search"] - Before creating, use nb__search with scope "tools" to verify tools exist -- If tools are missing, tell the user and offer to install via nb__manage_app +- If tools are missing, tell the user and point them to the Apps section of settings to install the bundle that provides them ## Choosing the Right Scope diff --git a/src/tools/connector-tools.ts b/src/tools/connector-tools.ts index cde968a2..21645e6e 100644 --- a/src/tools/connector-tools.ts +++ b/src/tools/connector-tools.ts @@ -1282,9 +1282,8 @@ async function handleInstallRemoteOAuth( /** * Mpak (stdio) install. The bundle is fetched from whichever mpak * registry the SDK is pointed at, spawned as a subprocess, and - * registered in the workspace registry. Same mechanics as the chat - * agent's `bundleManagement.install` so both UI surfaces produce - * identical state. + * registered in the workspace registry via the shared + * `installBundleInWorkspace` primitive. * * Workspace-scope only — every stdio bundle is workspace-shared * today. A future per-user mpak install would need its own @@ -2073,9 +2072,8 @@ async function handleSetUserConfig( // Mode 1 (env_inject) bundles only read user_config at spawn — env // vars are baked in at fork time. Saving to the credential file is // necessary but not sufficient; without a respawn the running - // subprocess keeps using whatever it was launched with. Mirror the - // chat agent's `configureBundle` pattern so both the chat path and - // the UI path produce identical post-write state. + // subprocess keeps using whatever it was launched with. Respawn so the + // post-write state reflects the new credentials. const respawn = await respawnBundleAfterCredentialChange(ctx, wsId, bundleName, serverName); const populated = await probeUserConfigPopulated(ctx.runtime, wsId, bundleName, schema); @@ -2163,8 +2161,8 @@ async function respawnBundleAfterCredentialChange( } // Pass `name` (the scoped manifest name) so startBundleSource hits // the named-bundle path that resolves user_config from the - // workspace credential store. configDir is undefined — same as - // configureBundle's call site; named-bundle path doesn't need it. + // workspace credential store. configDir is undefined — the + // named-bundle path doesn't need it. await startBundleSource({ name: bundleName }, registry, ctx.runtime.getEventSink(), undefined, { wsId, workDir: ctx.runtime.getWorkDir(), diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts index 7f6c66b1..c9d64d7d 100644 --- a/src/tools/system-tools.ts +++ b/src/tools/system-tools.ts @@ -2,21 +2,13 @@ import { NoopEventSink } from "../adapters/noop-events.ts"; import type { BundleLifecycleManager } from "../bundles/lifecycle.ts"; import { getMpak } from "../bundles/mpak.ts"; import { deriveServerName } from "../bundles/paths.ts"; -import type { BundleMcpDeps } from "../bundles/startup.ts"; -import { startBundleSource } from "../bundles/startup.ts"; -import type { BundleManifest, BundleRef } from "../bundles/types.ts"; -import { - installBundleInWorkspace, - uninstallBundleFromWorkspace, -} from "../bundles/workspace-ops.ts"; +import type { BundleManifest } from "../bundles/types.ts"; import { isToolEnabled, type ResolvedFeatures } from "../config/features.ts"; import type { ConfirmationGate } from "../config/privilege.ts"; import { textContent } from "../engine/content-helpers.ts"; import type { EventSink, ToolPromotionControls, ToolResult, ToolSchema } from "../engine/types.ts"; import type { Runtime } from "../runtime/runtime.ts"; import type { Skill } from "../skills/types.ts"; -import { WorkspaceContext } from "../workspace/context.ts"; -import type { WorkspaceStore } from "../workspace/workspace-store.ts"; import { createManageConnectorsTool } from "./connector-tools.ts"; import { buildCoreResourceMap } from "./core-resources/index.ts"; import { createCoreToolDefs } from "./core-source.ts"; @@ -34,28 +26,6 @@ import { type ManageWorkspacesContext, } from "./workspace-mgmt-tools.ts"; -/** Context for workspace-aware bundle management. */ -export interface ManageBundleContext { - getWorkspaceId: () => string | null; - workspaceStore: WorkspaceStore; - workDir: string; - configDir: string | undefined; - allowInsecureRemotes?: boolean; - // Required — threaded into any McpSource spawned by this context so - // task-augmented tool progress reaches the SSE broadcast path. The - // manage_app install/configure flow spawns bundles the same way the - // platform does at boot; both paths need the live runtime sink. - eventSink: EventSink; - /** - * Per-workspace host-resources deps factory. Threaded into - * `installBundleInWorkspace` so the bundle's McpSource registers - * `ai.nimblebrain/resources/*` inbound handlers. Optional for test - * runtimes that don't wire the host-resources subsystem; production - * runtime always sets this. See `Runtime.start()`. - */ - bundleMcpDepsFactory?: (wsId: string) => BundleMcpDeps; -} - export type ToolPromotionContext = ToolPromotionControls; export interface ToolEligibilityContext { @@ -68,7 +38,7 @@ export type GetSkillsFn = () => { context: Skill[]; matchable: Skill[] }; /** * Factory that creates the `nb` system source as an in-process MCP server. * Merges core platform tools (list_apps, get_config, etc.) with system tools - * (search, manage_app, delegate, etc.) into a single "nb" source. + * (search, delegate, etc.) into a single "nb" source. * * Returns a started, ready-to-use source. Async because the underlying * `McpSource.start()` runs the SDK initialize handshake over the linked @@ -77,7 +47,10 @@ export type GetSkillsFn = () => { context: Skill[]; matchable: Skill[] }; export async function createSystemTools( getRegistry: () => ToolRegistry, _configPath?: string, - gate?: ConfirmationGate, + // Reserved slot — was the bundle-management ConfirmationGate consumed by + // `nb__manage_app`. The tool was removed; keep the positional slot stable + // (the file's reserved-slot convention) so every call site's arity holds. + _gate?: ConfirmationGate, lifecycle?: BundleLifecycleManager, delegateCtx?: DelegateContext, // skillDir + reloadSkills were here for the legacy `nb__manage_skill` @@ -95,7 +68,10 @@ export async function createSystemTools( manageUsersCtx?: ManageUsersContext, manageWorkspacesCtx?: ManageWorkspacesContext, manageMembersCtx?: ManageMembersContext, - manageBundleCtx?: ManageBundleContext, + // Reserved slot — was the workspace-scoped bundle-management context for + // `nb__manage_app` (removed). Kept (typed `unknown`) to hold the positional + // slot stable for every call site. Prune on the next signature shake-up. + _manageBundleCtx?: unknown, toolPromotionCtx?: ToolPromotionContext, toolEligibilityCtx?: ToolEligibilityContext, ): Promise { @@ -191,74 +167,6 @@ export async function createSystemTools( }; }, }, - { - name: "manage_app", - description: - "Install, uninstall, or configure an app. 'configure' prompts for API keys/credentials securely via the terminal. Requires user approval.", - inputSchema: { - type: "object", - properties: { - action: { - type: "string", - enum: ["install", "uninstall", "configure"], - description: "Action: install, uninstall, or configure (set credentials)", - }, - name: { - type: "string", - description: "Bundle name (e.g., @nimblebraininc/ipinfo)", - }, - }, - required: ["action", "name"], - }, - handler: async (input): Promise => { - const action = String(input.action); - const name = String(input.name); - if (!lifecycle || !manageBundleCtx) { - return { - content: textContent("Bundle management requires lifecycle context"), - isError: true, - }; - } - const wsId = manageBundleCtx.getWorkspaceId(); - if (!wsId) { - return { - content: textContent("Workspace context required for bundle management"), - isError: true, - }; - } - if (action === "install") { - return await installBundleInWorkspaceViaCtx( - name, - wsId, - lifecycle, - getRegistry(), - manageBundleCtx, - ); - } - if (action === "uninstall") { - return await uninstallBundleFromWorkspaceViaCtx( - name, - wsId, - lifecycle, - getRegistry(), - manageBundleCtx, - ); - } - if (action === "configure") { - return await configureBundle( - name, - getRegistry(), - manageBundleCtx.eventSink, - wsId, - manageBundleCtx.workDir, - gate, - mpakHome, - manageBundleCtx.bundleMcpDepsFactory?.(wsId), - ); - } - return { content: textContent(`Unknown action: ${action}`), isError: true }; - }, - }, createReadResourceTool(getRegistry), createStatusTool(getRegistry, getSkills, lifecycle, runtime), ]; @@ -293,8 +201,7 @@ export async function createSystemTools( runtime, getIdentity: manageWorkspacesCtx.getIdentity, // Workspace id is per-call — pull from the runtime's current - // workspace context (same source manage_app uses to know which - // workspace's bundles[] to mutate). + // workspace context to know which workspace's bundles[] to mutate. getWorkspaceId: () => runtime.getCurrentWorkspaceId(), }), ); @@ -736,252 +643,6 @@ function formatUptime(ms: number): string { return `${hours}h ${minutes % 60}m`; } -async function configureBundle( - name: string, - registry: ToolRegistry, - // Required — passed to the restarted McpSource so its task-augmented tool - // progress events reach SSE broadcasts (Synapse useDataSync). Without it, - // re-configuring a bundle's credentials silently breaks live updates for - // that bundle until the next full platform restart. - eventSink: EventSink, - // Workspace id + work directory — required because credentials are stored - // per-workspace (`{workDir}/workspaces/{wsId}/credentials/{bundle}.json`), - // not globally in `~/.mpak/config.json`. Threaded from the manage_app handler. - wsId: string, - workDir: string, - confirmGate?: ConfirmationGate, - mpakHome?: string, - bundleMcp?: BundleMcpDeps, -): Promise { - try { - const mpak = getMpak(mpakHome!); - const manifest = mpak.bundleCache.getBundleManifest(name) as BundleManifest | null; - const userConfig = manifest?.user_config; - - if (!confirmGate?.supportsInteraction) { - // Non-interactive (HTTP server mode): show exact config commands. - // Credentials are workspace-scoped, so include `-w ` in the hint. - if (!userConfig || Object.keys(userConfig).length === 0) { - return { content: textContent(`${name} has no configurable credentials.`), isError: false }; - } - const fields = Object.entries(userConfig) - .map( - ([key, cfg]) => - ` nb config set ${name} ${key}= -w ${wsId} # ${cfg.title ?? cfg.description ?? key}`, - ) - .join("\n"); - return { - content: textContent( - `Cannot configure interactively in server mode. Run in your terminal:\n\n${fields}\n\nThen restart the server.`, - ), - isError: true, - }; - } - - if (!userConfig || Object.keys(userConfig).length === 0) { - return { - content: textContent(`${name} has no configurable credentials.`), - isError: false, - }; - } - - // Resolve via the 3-tier workspace-scoped resolver. `forcePrompt: true` - // re-prompts for every field so users can update existing credentials. - // Prompted values are persisted to the workspace credential store at - // `{workDir}/workspaces/{wsId}/credentials/{bundle-slug}.json` — no - // round-trip through `~/.mpak/config.json`. Routed through - // `WorkspaceContext` so the store's wsId is bound and validated once. - await new WorkspaceContext({ wsId, workDir }).getCredentialStore().resolveUserConfig({ - bundleName: name, - userConfigSchema: userConfig, - gate: confirmGate, - forcePrompt: true, - }); - - // Restart the bundle via the shared primitive — same construction path - // as boot-time / agent install. `startBundleSource` reads the values we - // just persisted above from the workspace credential store. If this - // function diverges from that primitive the rest of the app silently - // breaks (sink plumbing, PYTHONPATH, data-dir layout, user_config - // resolution). Delegate instead: pass `wsId`+`workDir` and let - // `startBundleSource` derive the workspace-scoped data dir itself — - // never compute it here, or it drifts from the install-time layout - // and Upjack entity state disappears across restarts. - const serverName = deriveServerName(name); - if (registry.hasSource(serverName)) { - await registry.removeSource(serverName); - } - const result = await startBundleSource({ name }, registry, eventSink, undefined, { - wsId, - workDir, - bundleMcp, - }); - - const tools = await registry.availableTools(); - const count = tools.filter((t) => t.name.startsWith(`${result.sourceName}__`)).length; - return { - content: textContent(`Configured and restarted ${name}. ${count} tools available.`), - isError: false, - }; - } catch (err) { - return { - content: textContent( - `Failed to configure ${name}: ${err instanceof Error ? err.message : String(err)}`, - ), - isError: true, - }; - } -} - -/** - * Install a bundle in a workspace: spawn with plain server name, - * add to workspace.json bundles, seed lifecycle instance. - */ -async function installBundleInWorkspaceViaCtx( - name: string, - wsId: string, - lifecycle: BundleLifecycleManager, - registry: ToolRegistry, - ctx: ManageBundleContext, -): Promise { - try { - const bundleRef = { name } as BundleRef; - - // Spawn the bundle process with plain server name in workspace registry - const entry = await installBundleInWorkspace( - wsId, - bundleRef, - registry, - ctx.eventSink, - ctx.configDir, - { - allowInsecureRemotes: ctx.allowInsecureRemotes, - workDir: ctx.workDir, - bundleMcp: ctx.bundleMcpDepsFactory?.(wsId), - }, - ); - - // Seed lifecycle instance so it can be tracked/queried - lifecycle.seedInstance( - entry.serverName, - name, - bundleRef, - entry.meta ?? undefined, - wsId, - entry.dataDir, - ); - // Register placements + emit bundle.installed so the web shell's - // sidebar refreshes without a reboot when the chat agent installs - // a bundle on the user's behalf. - lifecycle.notifyInstalled(entry.serverName, wsId); - - // Add bundle to workspace.json - const ws = await ctx.workspaceStore.get(wsId); - if (ws) { - const already = ws.bundles.some((b) => "name" in b && b.name === name); - if (!already) { - await ctx.workspaceStore.update(wsId, { - bundles: [...ws.bundles, { name }], - }); - } - } - - const tools = await registry.availableTools(); - const count = tools.filter((t) => t.name.startsWith(`${entry.serverName}__`)).length; - return { - content: textContent( - `Installed ${name} in workspace ${wsId}. ${count} tools now available from ${entry.serverName}.`, - ), - isError: false, - }; - } catch (err) { - return { - content: textContent( - `Failed to install ${name} in workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`, - ), - isError: true, - }; - } -} - -/** - * Uninstall a bundle from a workspace: stop process, - * remove from workspace.json bundles, remove lifecycle instance. - */ -/** - * Resolve the lifecycle key for a bundle being uninstalled. - * - * Catalog install (Browse / `manage_app install`) writes the - * slugified canonical id (`slugifyServerName(entry.id)`) onto the - * BundleRef. Re-deriving from `bundleName` here would compute the - * OLD short slug and miss the registered source — the exact - * regression that broke `manage_app uninstall` for any bundle - * installed via the catalog after #195. - * - * Reads `ref.serverName` first, falling back to - * `deriveServerName(bundleName)` only for legacy installs that - * predate `serverName`-on-ref persistence. Exported so the regression - * is unit-testable independently of the full uninstall stack. - */ -export function resolveBundleServerName( - bundleName: string, - ws: { bundles: Array<{ name?: string; serverName?: string }> } | null, -): string { - const persisted = ws?.bundles.find((b) => b.name === bundleName); - return persisted?.serverName ?? deriveServerName(bundleName); -} - -async function uninstallBundleFromWorkspaceViaCtx( - name: string, - wsId: string, - lifecycle: BundleLifecycleManager, - registry: ToolRegistry, - ctx: ManageBundleContext, -): Promise { - try { - const ws = await ctx.workspaceStore.get(wsId); - const serverName = resolveBundleServerName(name, ws); - - // Protected check — pass wsId to look up the workspace-scoped instance - const instance = lifecycle.getInstance(serverName, wsId); - if (instance?.protected) { - throw new Error(`Cannot uninstall "${serverName}": bundle is protected`); - } - - // Stop process and deregister from tool registry. Thread workDir so - // the workspace credential file for this bundle is cleaned up as part - // of uninstall (best-effort inside uninstallBundleFromWorkspace). - await uninstallBundleFromWorkspace(wsId, name, serverName, registry, { - workDir: ctx.workDir, - }); - - // Remove lifecycle instance tracking - if (instance) { - lifecycle.transition(instance, "stopped"); - } - lifecycle.removeInstance(serverName, wsId); - - // Remove bundle from workspace.json - if (ws) { - await ctx.workspaceStore.update(wsId, { - bundles: ws.bundles.filter((b) => !("name" in b && b.name === name)), - }); - } - - return { - content: textContent(`Uninstalled ${serverName} from workspace ${wsId}.`), - isError: false, - }; - } catch (err) { - return { - content: textContent( - `Failed to uninstall ${name} from workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`, - ), - isError: true, - }; - } -} - function groupToolsBySource(all: Array<{ name: string; description: string }>): ToolResult { const groups = new Map(); for (const tool of all) { diff --git a/test/integration/bundles-workspace-bundle-mgmt.test.ts b/test/integration/bundles-workspace-bundle-mgmt.test.ts deleted file mode 100644 index 7728642f..00000000 --- a/test/integration/bundles-workspace-bundle-mgmt.test.ts +++ /dev/null @@ -1,352 +0,0 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { mkdtemp, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { afterAll, afterEach, beforeEach, describe, expect, it } from "bun:test"; -import type { EngineEvent, EventSink } from "../../src/engine/types.ts"; -import { BundleLifecycleManager } from "../../src/bundles/lifecycle.ts"; -import { ToolRegistry } from "../../src/tools/registry.ts"; -import type { ManageBundleContext } from "../../src/tools/system-tools.ts"; -import { WorkspaceStore } from "../../src/workspace/workspace-store.ts"; -import { extractText } from "../../src/engine/content-helpers.ts"; -import { - installBundleInWorkspace, -} from "../../src/bundles/workspace-ops.ts"; -import { deriveServerName } from "../../src/bundles/paths.ts"; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -function makeEventCollector(): EventSink & { events: EngineEvent[] } { - const events: EngineEvent[] = []; - return { - events, - emit(event: EngineEvent) { - events.push(event); - }, - }; -} - -/** Create a minimal echo MCP server bundle on disk with a valid MCPB manifest. */ -function createEchoBundleOnDisk(dir: string): string { - mkdirSync(dir, { recursive: true }); - - const nodeModulesPath = join(import.meta.dir, "../..", "node_modules"); - const serverCode = ` -const { Server } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/server/index.js"); -const { StdioServerTransport } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/server/stdio.js"); -const { ListToolsRequestSchema, CallToolRequestSchema } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/types.js"); - -async function main() { - const server = new Server( - { name: "echo-test", version: "0.1.0" }, - { capabilities: { tools: {} } }, - ); - server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: [{ name: "echo", description: "Echo input", inputSchema: { type: "object", properties: { message: { type: "string" } }, required: ["message"] } }], - })); - server.setRequestHandler(CallToolRequestSchema, async (req) => ({ - content: [{ type: "text", text: "Echo: " + req.params.arguments?.message }], - })); - const transport = new StdioServerTransport(); - await server.connect(transport); -} -main(); -`; - writeFileSync(join(dir, "server.cjs"), serverCode); - - const manifest = { - manifest_version: "0.4", - name: "@test/echo", - version: "1.0.0", - description: "Echo test bundle", - author: { name: "Test Author" }, - server: { - type: "node", - entry_point: "server.cjs", - mcp_config: { - command: "node", - args: ["${__dirname}/server.cjs"], - }, - }, - }; - writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest, null, 2)); - return dir; -} - -// --------------------------------------------------------------------------- -// Tests: manage_app tool with workspace context -// --------------------------------------------------------------------------- - -describe("manage_app — workspace-aware install/uninstall", () => { - let workDir: string; - let store: WorkspaceStore; - let currentWorkspaceId: string | null; - - beforeEach(async () => { - workDir = await mkdtemp(join(tmpdir(), "nb-ws-bundle-test-")); - store = new WorkspaceStore(workDir); - currentWorkspaceId = null; - }); - - afterEach(async () => { - await rm(workDir, { recursive: true, force: true }); - }); - - it("install in workspace uses plain server name (no compound key)", async () => { - // Create workspace - const ws = await store.create("Engineering", "engineering"); - currentWorkspaceId = ws.id; - - // Create bundle on disk - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-bundle")); - - const registry = new ToolRegistry(); - const sink = makeEventCollector(); - const lifecycle = new BundleLifecycleManager(registry, sink, undefined); - - const bundleRef = { path: bundleDir } as import("../../../src/bundles/types.ts").BundleRef; - const entry = await installBundleInWorkspace( - ws.id, - bundleRef, - registry, - sink, - undefined, - { workDir }, - ); - - // Entry has server name derived from manifest (not path) - // Manifest name is "@test/echo" → deriveServerName → "echo" - expect(entry.serverName).toBe("echo"); - - // Source should be registered with plain server name - expect(registry.hasSource(entry.serverName)).toBe(true); - - // Seed lifecycle and update workspace.json (what the tool handler does) - lifecycle.seedInstance(entry.serverName, bundleDir, bundleRef, entry.meta ?? undefined, ws.id); - await store.update(ws.id, { - bundles: [...ws.bundles, { path: bundleDir }], - }); - - // Verify workspace.json was updated - const updated = await store.get(ws.id); - expect(updated!.bundles).toHaveLength(1); - expect("path" in updated!.bundles[0] && updated!.bundles[0].path).toBe(bundleDir); - - // Verify lifecycle instance exists with wsId - const instance = lifecycle.getInstance(entry.serverName, ws.id); - expect(instance).toBeDefined(); - expect((instance as { wsId?: string }).wsId).toBe(ws.id); - - // Cleanup - await registry.removeSource(entry.serverName); - }, 15_000); - - it("uninstall from workspace updates workspace.json and removes server name", async () => { - // Create workspace - const ws = await store.create("Engineering", "engineering"); - currentWorkspaceId = ws.id; - - // Create and install bundle - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-bundle-uninstall")); - const bundleRef = { path: bundleDir } as import("../../../src/bundles/types.ts").BundleRef; - - const registry = new ToolRegistry(); - const sink = makeEventCollector(); - const lifecycle = new BundleLifecycleManager(registry, sink, undefined); - - const entry = await installBundleInWorkspace( - ws.id, - bundleRef, - registry, - sink, - undefined, - { workDir }, - ); - lifecycle.seedInstance(entry.serverName, bundleDir, bundleRef, entry.meta ?? undefined, ws.id); - await store.update(ws.id, { - bundles: [{ path: bundleDir }], - }); - - // Verify it's running - expect(registry.hasSource(entry.serverName)).toBe(true); - - // Uninstall — what the tool handler does - const serverName = deriveServerName(bundleDir); - const instance = lifecycle.getInstance(serverName, ws.id); - if (instance) lifecycle.transition(instance, "stopped"); - lifecycle.removeInstance(serverName, ws.id); - await registry.removeSource(serverName); - - // Remove from workspace.json - const wsBefore = await store.get(ws.id); - await store.update(ws.id, { - bundles: wsBefore!.bundles.filter( - (b) => !("path" in b && b.path === bundleDir), - ), - }); - - // Verify server name removed from registry - expect(registry.hasSource(serverName)).toBe(false); - - // Verify workspace.json updated - const wsAfter = await store.get(ws.id); - expect(wsAfter!.bundles).toHaveLength(0); - - // Verify lifecycle instance removed - expect(lifecycle.getInstance(serverName, ws.id)).toBeUndefined(); - }, 15_000); - - it("same bundle in two workspaces uses separate registries", async () => { - const ws1 = await store.create("Engineering", "engineering"); - const ws2 = await store.create("Marketing", "marketing"); - - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-multi-ws")); - const bundleRef = { path: bundleDir } as import("../../../src/bundles/types.ts").BundleRef; - - // Each workspace gets its own registry - const registry1 = new ToolRegistry(); - const registry2 = new ToolRegistry(); - const sink = makeEventCollector(); - - const entry1 = await installBundleInWorkspace( - ws1.id, bundleRef, registry1, sink, undefined, { workDir }, - ); - const entry2 = await installBundleInWorkspace( - ws2.id, bundleRef, registry2, sink, undefined, { workDir }, - ); - - // Same plain server name in both - expect(entry1.serverName).toBe(entry2.serverName); - - // Both registered in their respective registries - expect(registry1.hasSource(entry1.serverName)).toBe(true); - expect(registry2.hasSource(entry2.serverName)).toBe(true); - - // Different data dirs - expect(entry1.dataDir).not.toBe(entry2.dataDir); - expect(entry1.dataDir).toContain(ws1.id); - expect(entry2.dataDir).toContain(ws2.id); - - // Cleanup - await registry1.removeSource(entry1.serverName); - await registry2.removeSource(entry2.serverName); - }, 15_000); - - it("nimblebrain.json is NOT modified during workspace install/uninstall", async () => { - const ws = await store.create("Engineering", "engineering"); - const configPath = join(workDir, "nimblebrain.json"); - writeFileSync(configPath, JSON.stringify({ bundles: [] }, null, 2)); - - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-no-config")); - const bundleRef = { path: bundleDir } as import("../../../src/bundles/types.ts").BundleRef; - - const registry = new ToolRegistry(); - const sink = makeEventCollector(); - const lifecycle = new BundleLifecycleManager(registry, sink, configPath); - - // Install via workspace path - const entry = await installBundleInWorkspace( - ws.id, bundleRef, registry, sink, undefined, { workDir }, - ); - lifecycle.seedInstance(entry.serverName, bundleDir, bundleRef, entry.meta ?? undefined, ws.id); - await store.update(ws.id, { bundles: [{ path: bundleDir }] }); - - // nimblebrain.json should still have empty bundles - const config = JSON.parse(readFileSync(configPath, "utf-8")); - expect(config.bundles).toHaveLength(0); - - // Uninstall - await registry.removeSource(entry.serverName); - lifecycle.removeInstance(entry.serverName, ws.id); - await store.update(ws.id, { bundles: [] }); - - // nimblebrain.json still unchanged - const config2 = JSON.parse(readFileSync(configPath, "utf-8")); - expect(config2.bundles).toHaveLength(0); - }, 15_000); - - it("protected bundles cannot be uninstalled from workspace", async () => { - const ws = await store.create("Engineering", "engineering"); - - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-protected")); - const bundleRef = { path: bundleDir, protected: true } as import("../../../src/bundles/types.ts").BundleRef; - - const registry = new ToolRegistry(); - const sink = makeEventCollector(); - const lifecycle = new BundleLifecycleManager(registry, sink, undefined); - - const entry = await installBundleInWorkspace( - ws.id, bundleRef, registry, sink, undefined, { workDir }, - ); - lifecycle.seedInstance(entry.serverName, bundleDir, bundleRef, entry.meta ?? undefined, ws.id); - - // Try to uninstall — protected check should prevent it - const instance = lifecycle.getInstance(entry.serverName, ws.id); - expect(instance?.protected).toBe(true); - - // Simulate the check: - expect(() => { - if (instance?.protected) { - throw new Error(`Cannot uninstall "${entry.serverName}": bundle is protected`); - } - }).toThrow("Cannot uninstall"); - - // Cleanup - await registry.removeSource(entry.serverName); - }, 15_000); - - it("install→uninstall slug roundtrip: ref.serverName is honored end-to-end", async () => { - // Pins the contract that pre-fix was broken on the named-bundle - // (and path-bundle) branches of `startBundleSource`: the persisted - // `ref.serverName` (set by the catalog install path from - // `slugifyServerName(entry.id)`) MUST be the name registered in - // the ToolRegistry, AND the lifecycle Map key, AND what - // `manage_app uninstall` resolves to. Pre-fix, install persisted - // the canonical slug but startup re-derived `deriveServerName(name)` - // — registry was keyed on the short slug, uninstall looked up by - // the canonical slug, miss, throw. The would-have-caught-#1 test. - const ws = await store.create("Engineering", "engineering"); - const bundleDir = createEchoBundleOnDisk(join(workDir, "echo-roundtrip")); - - // Catalog install would persist a slug like "dev-mpak-nb-echo". - // Use a clearly-different value from `deriveServerName(manifest.name)` - // (which would compute "echo") so a regression to the old behavior - // produces "echo" not "dev-mpak-nb-echo" and the assertion fails. - const canonicalSlug = "dev-mpak-nb-echo"; - const bundleRef = { - path: bundleDir, - serverName: canonicalSlug, - } as import("../../src/bundles/types.ts").BundleRef; - - const registry = new ToolRegistry(); - const sink = makeEventCollector(); - const lifecycle = new BundleLifecycleManager(registry, sink, undefined); - - const entry = await installBundleInWorkspace( - ws.id, - bundleRef, - registry, - sink, - undefined, - { workDir }, - ); - - // Install path returns the persisted slug, NOT the manifest-derived short slug. - expect(entry.serverName).toBe(canonicalSlug); - expect(entry.serverName).not.toBe(deriveServerName("@test/echo")); - - // ToolRegistry source name agrees. - expect(registry.hasSource(canonicalSlug)).toBe(true); - expect(registry.hasSource("echo")).toBe(false); - - // Lifecycle Map key agrees too. - lifecycle.seedInstance(entry.serverName, bundleDir, bundleRef, entry.meta ?? undefined, ws.id); - expect(lifecycle.getInstance(canonicalSlug, ws.id)).toBeDefined(); - expect(lifecycle.getInstance("echo", ws.id)).toBeUndefined(); - - // Cleanup - await registry.removeSource(canonicalSlug); - }, 15_000); -}); diff --git a/test/integration/chat-endpoint-extensions.test.ts b/test/integration/chat-endpoint-extensions.test.ts index e6f9f804..1c525a53 100644 --- a/test/integration/chat-endpoint-extensions.test.ts +++ b/test/integration/chat-endpoint-extensions.test.ts @@ -22,7 +22,7 @@ function makeTool(name: string): ToolSchema { } function makeSystemTools(): ToolSchema[] { - return ["nb__search", "nb__manage_app", "nb__status", "nb__set_preferences"].map(makeTool); + return ["nb__search", "nb__delegate", "nb__status", "nb__set_preferences"].map(makeTool); } function makeSkill(opts: { allowedTools?: string[] } = {}): Skill { diff --git a/test/integration/configure-credentials.test.ts b/test/integration/configure-credentials.test.ts deleted file mode 100644 index e710a0fa..00000000 --- a/test/integration/configure-credentials.test.ts +++ /dev/null @@ -1,213 +0,0 @@ -/** - * Integration test for `nb__manage_app configure` — the interactive TUI path - * where a user is prompted for a bundle's user_config values, the resolver - * persists them to the workspace credential store, and the bundle is restarted. - * - * This test catches a specific regression class: if `configureBundle` ever - * stops threading `wsId` / `workDir` into `startBundleSource`, the restart - * throws after the bundle has already been removed from the registry — a - * permanent session-scoped break. We exercise the full flow end-to-end with a - * real subprocess MCP server so the restart path is actually executed. - * - * Companion to `test/integration/startup-credentials.test.ts` (which covers - * the same resolver wiring at boot time via `startBundleSource` directly). - */ - -import { afterAll, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { NoopEventSink } from "../../src/adapters/noop-events.ts"; -import { BundleLifecycleManager } from "../../src/bundles/lifecycle.ts"; -import { getWorkspaceCredentials } from "../../src/config/workspace-credentials.ts"; -import type { ConfirmationGate } from "../../src/config/privilege.ts"; -import { extractText } from "../../src/engine/content-helpers.ts"; -import { ToolRegistry } from "../../src/tools/registry.ts"; -import { createSystemTools } from "../../src/tools/system-tools.ts"; -import type { ManageBundleContext } from "../../src/tools/types.ts"; -import { WorkspaceStore } from "../../src/workspace/workspace-store.ts"; - -const BUNDLE_NAME = "@nbtest/configure-bundle"; -// Mpak cache directory convention: `@scope/name` → `scope-name`. -const BUNDLE_CACHE_SLUG = "nbtest-configure-bundle"; -// ToolRegistry source key: `deriveServerName` takes the post-slash segment. -const SERVER_NAME = "configure-bundle"; -const WS_ID = "ws_configure"; - -const rootDir = join(tmpdir(), `nb-configure-integ-${Date.now()}-${process.pid}`); - -afterAll(() => { - rmSync(rootDir, { recursive: true, force: true }); -}); - -interface Layout { - mpakHome: string; - workDir: string; -} - -/** - * Seed an mpak cache with a minimal but real MCP server that reads - * `NBTEST_API_KEY` from its process env. The manifest's `user_config` - * declares `api_key` as required and the `mcp_config.env` substitutes - * it in — identical shape to real registry bundles. - */ -async function seedFixture(root: string): Promise { - const mpakHome = join(root, "mpak-home"); - const workDir = join(root, "nb-home"); - const cacheDir = join(mpakHome, "cache", BUNDLE_CACHE_SLUG); - mkdirSync(cacheDir, { recursive: true }); - - const nodeModulesPath = join(import.meta.dir, "../..", "node_modules"); - const serverCode = ` -const { Server } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/server/index.js"); -const { StdioServerTransport } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/server/stdio.js"); -const { ListToolsRequestSchema, CallToolRequestSchema } = require("${nodeModulesPath}/@modelcontextprotocol/sdk/dist/cjs/types.js"); - -async function main() { - const server = new Server( - { name: "configure-bundle", version: "0.1.0" }, - { capabilities: { tools: {} } }, - ); - server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: [ - { name: "whoami", description: "echo the API key", inputSchema: { type: "object", properties: {} } }, - ], - })); - server.setRequestHandler(CallToolRequestSchema, async () => ({ - content: [{ type: "text", text: String(process.env.NBTEST_API_KEY ?? "") }], - })); - const transport = new StdioServerTransport(); - await server.connect(transport); -} -main(); -`; - writeFileSync(join(cacheDir, "server.cjs"), serverCode); - - const manifest = { - manifest_version: "0.3", - name: BUNDLE_NAME, - version: "0.1.0", - description: "Test bundle for the configure tool", - user_config: { - api_key: { type: "string", title: "API key", sensitive: true, required: true }, - }, - server: { - type: "node", - entry_point: "server.cjs", - mcp_config: { - command: "node", - args: ["${__dirname}/server.cjs"], - env: { NBTEST_API_KEY: "${user_config.api_key}" }, - }, - }, - }; - writeFileSync(join(cacheDir, "manifest.json"), JSON.stringify(manifest)); - writeFileSync( - join(cacheDir, ".mpak-meta.json"), - JSON.stringify({ - version: "0.1.0", - pulledAt: new Date().toISOString(), - platform: { os: process.platform, arch: process.arch }, - }), - ); - - // Create the workspace directory so WorkspaceStore.get resolves. - const store = new WorkspaceStore(workDir); - await store.create("configure", WS_ID.replace(/^ws_/, "")); - - return { mpakHome, workDir }; -} - -describe("manage_app configure — end-to-end", () => { - let layout: Layout; - let prevMpakHome: string | undefined; - - beforeEach(async () => { - const dir = join(rootDir, `case-${Date.now()}-${Math.random().toString(36).slice(2)}`); - layout = await seedFixture(dir); - prevMpakHome = process.env.MPAK_HOME; - process.env.MPAK_HOME = layout.mpakHome; - }); - - function restoreEnv(): void { - if (prevMpakHome === undefined) delete process.env.MPAK_HOME; - else process.env.MPAK_HOME = prevMpakHome; - } - - async function buildTools(gate: ConfirmationGate) { - const registry = new ToolRegistry(); - const sink = new NoopEventSink(); - const lifecycle = new BundleLifecycleManager(sink, undefined, false, layout.mpakHome); - const store = new WorkspaceStore(layout.workDir); - const ctx: ManageBundleContext = { - getWorkspaceId: () => WS_ID, - workspaceStore: store, - workDir: layout.workDir, - configDir: undefined, - eventSink: sink, - }; - const tools = await createSystemTools( - () => registry, - undefined, - gate, - lifecycle, - undefined, - undefined, - undefined, - undefined, - sink, - undefined, - undefined, - layout.mpakHome, - undefined, - undefined, - undefined, - ctx, - ); - return { tools, registry }; - } - - test( - "prompts, persists, restarts — bundle is registered after configure", - async () => { - try { - const gate: ConfirmationGate = { - supportsInteraction: true, - confirm: async () => true, - promptConfigValue: async () => "sk-configured-abc", - }; - const { tools, registry } = await buildTools(gate); - - const result = await tools.execute("manage_app", { - action: "configure", - name: BUNDLE_NAME, - }); - - // Restart succeeded — this is the regression we're guarding against. - // Previously `configureBundle` omitted wsId/workDir on the restart - // call and `startBundleSource` threw after the bundle was already - // deregistered, leaving a user-visible error and an empty registry. - expect(result.isError).toBe(false); - expect(extractText(result.content)).toMatch(/Configured and restarted/); - - // Bundle is back in the registry after restart. - expect(registry.hasSource(SERVER_NAME)).toBe(true); - - // Credential landed in the workspace credential store, not ~/.mpak/. - const creds = await getWorkspaceCredentials(WS_ID, BUNDLE_NAME, layout.workDir); - expect(creds).toEqual({ api_key: "sk-configured-abc" }); - } finally { - restoreEnv(); - } - }, - // 20s: spawns a real Node MCP subprocess and completes the full stdio - // handshake + tools/list. The platform's own MCP-stdio CONNECT_TIMEOUT - // is 30s (`src/tools/mcp-source.ts`), so the prior 10s outer guard - // tripped on slow CI runners while the platform was still well within - // its own connect budget. 20s matches the suite convention for - // subprocess-spawning tests (see remote-integration.test.ts) and - // stays under the platform ceiling so a genuine connect hang still - // surfaces as a failure rather than a test timeout. - 20_000, - ); -}); diff --git a/test/integration/dependency-checking.test.ts b/test/integration/dependency-checking.test.ts index e62ec72a..e4e34289 100644 --- a/test/integration/dependency-checking.test.ts +++ b/test/integration/dependency-checking.test.ts @@ -149,7 +149,7 @@ You are a data analyst. expect(getSystem()).toContain("You are a data analyst"); expect(getSystem()).toContain("Missing dependencies"); expect(getSystem()).toContain("@acme/data-tools"); - expect(getSystem()).toContain("nb__manage_app"); + expect(getSystem()).toContain("Apps catalog in settings"); await runtime.shutdown(); }); diff --git a/test/integration/runtime.test.ts b/test/integration/runtime.test.ts index 1da628d4..5cbe7ea2 100644 --- a/test/integration/runtime.test.ts +++ b/test/integration/runtime.test.ts @@ -319,7 +319,7 @@ Greet with enthusiasm! expect(names).toContain("nb__status"); expect(names).toContain("nb__delegate"); expect(names).toContain("nb__search"); - expect(names).toContain("nb__manage_app"); + expect(names).not.toContain("nb__manage_app"); // Verify core tools (including internal ones — availableTools returns all) expect(names).toContain("nb__set_model_config"); diff --git a/test/unit/audit-events.test.ts b/test/unit/audit-events.test.ts index 0fa89c35..dd020daa 100644 --- a/test/unit/audit-events.test.ts +++ b/test/unit/audit-events.test.ts @@ -165,13 +165,13 @@ describe("WorkspaceLogSink audit events", () => { const sink = new WorkspaceLogSink({ dir }); sink.emit({ type: "audit.permission_denied", - data: { tool: "nb__manage_app", action: "install", target: "@test/foo" }, + data: { tool: "skills__create", action: "create", target: "my-skill" }, }); const records = readLogRecords(join(dir, "workspace")); expect(records).toHaveLength(1); expect(records[0]!.event).toBe("audit.permission_denied"); - expect(records[0]!.tool).toBe("nb__manage_app"); + expect(records[0]!.tool).toBe("skills__create"); }); }); @@ -220,16 +220,16 @@ describe("createPrivilegeHook audit emission", () => { const result = await hook({ id: "call_1", - name: "nb__manage_app", - input: { action: "install", name: "@test/foo" }, + name: "skills__create", + input: { scope: "workspace", name: "my-skill" }, }); expect(result).toBeNull(); expect(events).toHaveLength(1); expect(events[0]!.type).toBe("audit.permission_denied"); - expect(events[0]!.data.tool).toBe("nb__manage_app"); - expect(events[0]!.data.action).toBe("install"); - expect(events[0]!.data.target).toBe("@test/foo"); + expect(events[0]!.data.tool).toBe("skills__create"); + expect(events[0]!.data.action).toBe("create"); + expect(events[0]!.data.target).toBe("my-skill"); }); it("does not emit audit event when gate approves", async () => { @@ -240,8 +240,8 @@ describe("createPrivilegeHook audit emission", () => { const result = await hook({ id: "call_1", - name: "nb__manage_app", - input: { action: "install", name: "@test/foo" }, + name: "skills__create", + input: { scope: "workspace", name: "my-skill" }, }); expect(result).not.toBeNull(); diff --git a/test/unit/features.test.ts b/test/unit/features.test.ts index 8533d70c..729b3d07 100644 --- a/test/unit/features.test.ts +++ b/test/unit/features.test.ts @@ -56,7 +56,7 @@ describe("isToolVisibleToRole", () => { it("shows non-admin tools to all roles", () => { expect(isToolVisibleToRole("nb__search", "member")).toBe(true); expect(isToolVisibleToRole("nb__set_preferences", "member")).toBe(true); - expect(isToolVisibleToRole("nb__manage_app", "member")).toBe(true); + expect(isToolVisibleToRole("nb__status", "member")).toBe(true); }); it("hides admin tools when role is null (unauthenticated)", () => { diff --git a/test/unit/skills.test.ts b/test/unit/skills.test.ts index b41de1e5..b099b675 100644 --- a/test/unit/skills.test.ts +++ b/test/unit/skills.test.ts @@ -556,7 +556,7 @@ describe("loadCoreSkills", () => { expect(bs.manifest.metadata!.keywords).toContain("mpak"); expect(bs.manifest.metadata!.triggers).toContain("what can you do"); expect(bs.body).toContain("nb__search"); - expect(bs.body).toContain("nb__manage_app"); + expect(bs.body).toContain("nb__manage_tools"); } finally { spy.mockRestore(); } diff --git a/test/unit/system-tools.test.ts b/test/unit/system-tools.test.ts index 62bcfbce..d2657653 100644 --- a/test/unit/system-tools.test.ts +++ b/test/unit/system-tools.test.ts @@ -1,14 +1,9 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { describe, expect, it } from "bun:test"; import { NoopEventSink } from "../../src/adapters/noop-events.ts"; import { extractText } from "../../src/engine/content-helpers.ts"; -import { mkdirSync, mkdtempSync, existsSync, rmSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; -import { BundleLifecycleManager } from "../../src/bundles/lifecycle.ts"; import { createSystemTools } from "../../src/tools/system-tools.ts"; import type { GetSkillsFn, - ManageBundleContext, ToolEligibilityContext, ToolPromotionContext, } from "../../src/tools/system-tools.ts"; @@ -22,10 +17,7 @@ import { } from "../../src/config/privilege.ts"; import type { ConfirmationGate } from "../../src/config/privilege.ts"; import { resolveFeatures } from "../../src/config/features.ts"; -import { getWorkspaceCredentials } from "../../src/config/workspace-credentials.ts"; import { isToolEligibleForPromotion } from "../../src/runtime/tool-eligibility.ts"; -import { readSkill } from "../../src/skills/writer.ts"; -import { WorkspaceStore } from "../../src/workspace/workspace-store.ts"; const noopSink = new NoopEventSink(); @@ -254,28 +246,6 @@ describe("System Tools", () => { // network calls"). Unit gate stays deterministic; smoke job continues to // validate the live wiring on main. - it("manage_app install requires lifecycle context", async () => { - const registry = await makeRegistry(); - const systemTools = await createSystemTools(() => registry); - const result = await systemTools.execute("manage_app", { - action: "install", - name: "@test/nonexistent-bundle-xyz", - }); - expect(result.isError).toBe(true); - expect(extractText(result.content)).toContain("lifecycle context"); - }, 15_000); - - it("manage_app uninstall requires lifecycle context", async () => { - const registry = await makeRegistry(); - const systemTools = await createSystemTools(() => registry); - const result = await systemTools.execute("manage_app", { - action: "uninstall", - name: "nonexistent", - }); - expect(result.isError).toBe(true); - expect(extractText(result.content)).toContain("lifecycle context"); - }); - it("tools() returns prefixed tool names", async () => { const registry = await makeRegistry(); const systemTools = await createSystemTools(() => registry); @@ -283,7 +253,7 @@ describe("System Tools", () => { const names = tools.map((t) => t.name); expect(names).toContain("nb__search"); expect(names).toContain("nb__manage_tools"); - expect(names).toContain("nb__manage_app"); + expect(names).not.toContain("nb__manage_app"); }); it("manage_tools requires at least one of add or remove to be non-empty", async () => { @@ -498,8 +468,8 @@ describe("ConfirmationGate", () => { const hook = createPrivilegeHook(gate, noopSink); const call = { id: "1", - name: "nb__manage_app", - input: { action: "install", name: "@test/bundle" }, + name: "skills__create", + input: { scope: "workspace", name: "test-skill" }, }; const result = await hook(call); expect(result).toBeNull(); @@ -513,8 +483,8 @@ describe("ConfirmationGate", () => { const hook = createPrivilegeHook(gate, noopSink); const call = { id: "1", - name: "nb__manage_app", - input: { action: "install", name: "@test/bundle" }, + name: "skills__create", + input: { scope: "workspace", name: "test-skill" }, }; const result = await hook(call); expect(result).toEqual(call); @@ -725,39 +695,6 @@ describe("status tool — scope: config", () => { // --------------------------------------------------------------------------- describe("System Tools — input validation", () => { - it("manage_app with unknown action returns error", async () => { - const registry = await makeRegistry(); - const systemTools = await createSystemTools(() => registry); - const result = await systemTools.execute("manage_app", { - action: "explode", - name: "test", - }); - expect(result.isError).toBe(true); - const text = extractText(result.content); - // Should indicate invalid action, not crash - expect(text.length).toBeGreaterThan(0); - }); - - it("manage_app install without name returns error", async () => { - const registry = await makeRegistry(); - const systemTools = await createSystemTools(() => registry); - const result = await systemTools.execute("manage_app", { - action: "install", - }); - expect(result.isError).toBe(true); - const text = extractText(result.content); - expect(text.length).toBeGreaterThan(0); - }); - - it("manage_app uninstall without name returns error", async () => { - const registry = await makeRegistry(); - const systemTools = await createSystemTools(() => registry); - const result = await systemTools.execute("manage_app", { - action: "uninstall", - }); - expect(result.isError).toBe(true); - }); - it("search with missing query defaults gracefully", async () => { const registry = await makeRegistry(); const systemTools = await createSystemTools(() => registry); @@ -769,232 +706,6 @@ describe("System Tools — input validation", () => { }); }); - -// --------------------------------------------------------------------------- -// manage_app configure — credential resolution via workspace store -// --------------------------------------------------------------------------- - -describe("manage_app configure — workspace-scoped credentials", () => { - let workDir: string; - let mpakHome: string; - // WORKSPACE_ID_RE requires ws_ prefix + [a-z0-9_] chars. Hyphens are invalid - // per the regex enforced by WorkspaceStore and now also by the credential - // store's input validation (PR #40 QA fixup). - const wsId = "ws_configure_test"; - const bundleName = "@testscope/configurable"; - - /** - * Write a fake manifest for `bundleName` into `{mpakHome}/cache/{slug}/` - * so `getMpak().bundleCache.getBundleManifest(name)` returns it without - * hitting the network. The cache dir layout is: - * `/-/manifest.json` - * matching `MpakBundleCache.getBundleCacheDirName()`. - */ - function writeFakeBundle(opts: { - userConfig?: Record; - }): void { - const safeName = bundleName.replace("@", "").replace("/", "-"); - const cacheDir = join(mpakHome, "cache", safeName); - mkdirSync(cacheDir, { recursive: true }); - - const manifest: Record = { - manifest_version: "0.4", - name: bundleName, - version: "1.0.0", - description: "Configurable test bundle", - author: { name: "Test" }, - server: { - type: "node", - entry_point: "server.js", - mcp_config: { - command: "node", - args: ["${__dirname}/server.js"], - }, - }, - }; - if (opts.userConfig) manifest.user_config = opts.userConfig; - writeFileSync(join(cacheDir, "manifest.json"), JSON.stringify(manifest, null, 2)); - } - - /** Create a bundle ctx + system tools wired for `manage_app configure`. */ - async function buildTools(gate: ConfirmationGate | undefined) { - const registry = new ToolRegistry(); - const sink = new NoopEventSink(); - const lifecycle = new BundleLifecycleManager(sink, undefined, false, mpakHome); - const store = new WorkspaceStore(workDir); - const ctx: ManageBundleContext = { - getWorkspaceId: () => wsId, - workspaceStore: store, - workDir, - configDir: undefined, - eventSink: sink, - }; - const tools = await createSystemTools( - () => registry, - undefined, - gate, - lifecycle, - undefined, - undefined, - undefined, - undefined, - sink, - undefined, - undefined, - mpakHome, - undefined, - undefined, - undefined, - ctx, - ); - return { tools, registry, sink }; - } - - beforeEach(() => { - workDir = mkdtempSync(join(tmpdir(), "nb-configure-test-")); - mpakHome = mkdtempSync(join(tmpdir(), "nb-configure-mpak-")); - }); - - afterEach(() => { - rmSync(workDir, { recursive: true, force: true }); - rmSync(mpakHome, { recursive: true, force: true }); - }); - - it("non-interactive gate with user_config returns error suggesting `nb config set ... -w `", async () => { - writeFakeBundle({ - userConfig: { - api_key: { type: "string", title: "API Key", required: true, sensitive: true }, - }, - }); - - const gate: ConfirmationGate = { - supportsInteraction: false, - confirm: async () => true, - promptConfigValue: async () => null, - }; - const { tools } = await buildTools(gate); - - const result = await tools.execute("manage_app", { - action: "configure", - name: bundleName, - }); - - expect(result.isError).toBe(true); - const text = extractText(result.content); - expect(text).toContain("Cannot configure interactively in server mode"); - expect(text).toContain(`nb config set ${bundleName} api_key= -w ${wsId}`); - }); - - it("bundle with no user_config reports nothing to configure (interactive)", async () => { - writeFakeBundle({}); - - const gate: ConfirmationGate = { - supportsInteraction: true, - confirm: async () => true, - promptConfigValue: async () => null, - }; - const { tools } = await buildTools(gate); - - const result = await tools.execute("manage_app", { - action: "configure", - name: bundleName, - }); - - expect(result.isError).toBe(false); - expect(extractText(result.content)).toContain("has no configurable credentials"); - }); - - it("bundle with no user_config reports nothing to configure (non-interactive)", async () => { - writeFakeBundle({}); - - const gate: ConfirmationGate = { - supportsInteraction: false, - confirm: async () => true, - promptConfigValue: async () => null, - }; - const { tools } = await buildTools(gate); - - const result = await tools.execute("manage_app", { - action: "configure", - name: bundleName, - }); - - expect(result.isError).toBe(false); - expect(extractText(result.content)).toContain("has no configurable credentials"); - }); - - it("interactive gate: prompted values are saved to the workspace credential store", async () => { - writeFakeBundle({ - userConfig: { - api_key: { type: "string", title: "API Key", required: true, sensitive: true }, - }, - }); - - const promptCalls: string[] = []; - const gate: ConfirmationGate = { - supportsInteraction: true, - confirm: async () => true, - promptConfigValue: async (field) => { - promptCalls.push(field.key); - return "sk-prompted-value"; - }, - }; - const { tools } = await buildTools(gate); - - // The restart via startBundleSource will fail (no real bundle zip), but - // credentials are persisted BEFORE restart — that's the behavior we're - // asserting. The tool result ends up isError=true for that reason, but - // the workspace credential store must contain the prompted value. - await tools.execute("manage_app", { - action: "configure", - name: bundleName, - }); - - expect(promptCalls).toEqual(["api_key"]); - - const creds = await getWorkspaceCredentials(wsId, bundleName, workDir); - expect(creds).not.toBeNull(); - expect(creds?.api_key).toBe("sk-prompted-value"); - }); - - it("forcePrompt: re-prompts even when a value already exists in the workspace store", async () => { - writeFakeBundle({ - userConfig: { - api_key: { type: "string", title: "API Key", required: true, sensitive: true }, - }, - }); - - // Seed an existing credential via the workspace store writer. - const { saveWorkspaceCredential } = await import( - "../../src/config/workspace-credentials.ts" - ); - await saveWorkspaceCredential(wsId, bundleName, "api_key", "sk-old-value", workDir); - - const promptCalls: string[] = []; - const gate: ConfirmationGate = { - supportsInteraction: true, - confirm: async () => true, - promptConfigValue: async (field) => { - promptCalls.push(field.key); - return "sk-new-value"; - }, - }; - const { tools } = await buildTools(gate); - - await tools.execute("manage_app", { - action: "configure", - name: bundleName, - }); - - // forcePrompt means the gate is called even though a stored value - // already exists — that's the whole point of `configure`. - expect(promptCalls).toEqual(["api_key"]); - - const creds = await getWorkspaceCredentials(wsId, bundleName, workDir); - expect(creds?.api_key).toBe("sk-new-value"); - }); -}); - // --------------------------------------------------------------------------- // nb__read_resource (#3) // --------------------------------------------------------------------------- diff --git a/test/unit/tools/surfacing.test.ts b/test/unit/tools/surfacing.test.ts index 9c0d53a2..6816f904 100644 --- a/test/unit/tools/surfacing.test.ts +++ b/test/unit/tools/surfacing.test.ts @@ -14,7 +14,7 @@ function makeTool(name: string): ToolSchema { } function makeSystemTools(count = 4): ToolSchema[] { - const names = ["nb__search", "nb__manage_app", "nb__status", "nb__set_preferences"]; + const names = ["nb__search", "nb__delegate", "nb__status", "nb__set_preferences"]; return names.slice(0, count).map(makeTool); } diff --git a/test/unit/workspace-ops-uninstall.test.ts b/test/unit/workspace-ops-uninstall.test.ts deleted file mode 100644 index 8fee23a2..00000000 --- a/test/unit/workspace-ops-uninstall.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { uninstallBundleFromWorkspace } from "../../src/bundles/workspace-ops.ts"; -import { ToolRegistry } from "../../src/tools/registry.ts"; -import { resolveBundleServerName } from "../../src/tools/system-tools.ts"; -import type { ToolSource } from "../../src/tools/types.ts"; - -/** - * Pins the contract `uninstallBundleFromWorkspace` was reshaped for in - * QA round 4: caller passes the *resolved* `serverName` (looked up from - * `workspace.json#bundles[].serverName` with `deriveServerName` as - * back-compat fallback), and the function targets that source — not a - * fresh `deriveServerName(bundleName)` derivation. - * - * Without this contract, a bundle installed via the catalog (which - * persists `slugifyServerName(entry.id)` on the BundleRef) couldn't be - * uninstalled by `manage_app uninstall @scope/name` — the agent path - * computed the OLD short slug and missed the registered source. - */ - -function makeFakeSource(name: string): ToolSource { - return { - name, - tools: async () => [], - execute: async () => ({ content: [], isError: false }), - stop: async () => {}, - }; -} - -describe("uninstallBundleFromWorkspace", () => { - test("removes the source whose name matches the resolved serverName, not deriveServerName(bundleName)", async () => { - const workDir = mkdtempSync(join(tmpdir(), "uninstall-test-")); - try { - const registry = new ToolRegistry(); - // Mirror what the catalog install path persists: a slugified - // canonical reverse-DNS form, distinct from what - // deriveServerName("@x/echo") would compute (which is "echo"). - const slugifiedServerName = "dev-mpak-x-echo"; - await registry.addSource(makeFakeSource(slugifiedServerName)); - expect(registry.hasSource(slugifiedServerName)).toBe(true); - - // Caller (system-tools.uninstallBundleFromWorkspaceViaCtx) is - // responsible for resolving the serverName from workspace.json - // before calling this helper. We pass it through here. - await uninstallBundleFromWorkspace( - "ws_acme", - "@x/echo", // bundleName from manage_app input - slugifiedServerName, // resolved from ref.serverName - registry, - { workDir }, - ); - - expect(registry.hasSource(slugifiedServerName)).toBe(false); - } finally { - rmSync(workDir, { recursive: true, force: true }); - } - }); - - test("throws when the resolved serverName isn't registered (defends against caller resolving wrong key)", async () => { - const workDir = mkdtempSync(join(tmpdir(), "uninstall-test-")); - try { - const registry = new ToolRegistry(); - await registry.addSource(makeFakeSource("dev-mpak-x-echo")); - - // Caller passes the OLD short-slug derivation by mistake — the - // helper rejects rather than silently no-oping. - await expect( - uninstallBundleFromWorkspace("ws_acme", "@x/echo", "echo", registry, { workDir }), - ).rejects.toThrow(/No bundle "echo" found/); - - // The real source is unaffected. - expect(registry.hasSource("dev-mpak-x-echo")).toBe(true); - } finally { - rmSync(workDir, { recursive: true, force: true }); - } - }); -}); - -/** - * The helper above pins the contract once the resolved serverName is - * already in hand. These tests pin the *resolver* itself — - * `manage_app uninstall`'s lookup against `workspace.json` — so a - * future refactor that drops the resolver doesn't silently re-introduce - * the catalog-installed-bundle uninstall regression. - */ -describe("resolveBundleServerName", () => { - test("returns the persisted slug for a catalog-installed bundle (post-#195)", () => { - const ws = { - bundles: [{ name: "@x/echo", serverName: "dev-mpak-x-echo" }], - }; - expect(resolveBundleServerName("@x/echo", ws)).toBe("dev-mpak-x-echo"); - }); - - test("falls back to deriveServerName for legacy refs missing serverName (pre-#195)", () => { - const ws = { - bundles: [{ name: "@x/echo" }], - }; - expect(resolveBundleServerName("@x/echo", ws)).toBe("echo"); - }); - - test("falls back to deriveServerName when workspace is null", () => { - expect(resolveBundleServerName("@x/echo", null)).toBe("echo"); - }); - - test("falls back to deriveServerName when bundle isn't in workspace.bundles", () => { - const ws = { - bundles: [{ name: "@y/other", serverName: "dev-mpak-y-other" }], - }; - expect(resolveBundleServerName("@x/echo", ws)).toBe("echo"); - }); -}); diff --git a/test/unit/workspace-uninstall-credentials.test.ts b/test/unit/workspace-uninstall-credentials.test.ts deleted file mode 100644 index 66478497..00000000 --- a/test/unit/workspace-uninstall-credentials.test.ts +++ /dev/null @@ -1,133 +0,0 @@ -/** - * Unit tests for credential cleanup on workspace bundle uninstall. - * - * Targets `src/bundles/workspace-ops.ts:uninstallBundleFromWorkspace`, - * the live path consumed by system-tools via ManageBundleContext. (A - * dead duplicate previously existed in `src/runtime/workspace-runtime.ts`; - * deleted in #195's slugify cleanup.) Must clean up the workspace-scoped - * credential file as part of uninstall (best-effort — failures log a - * warning but do not fail the uninstall). - */ - -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtemp, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { uninstallBundleFromWorkspace } from "../../src/bundles/workspace-ops.ts"; -import { - getWorkspaceCredentials, - saveWorkspaceCredential, -} from "../../src/config/workspace-credentials.ts"; -import { ToolRegistry } from "../../src/tools/registry.ts"; -import type { Tool, ToolSource } from "../../src/tools/types.ts"; - -const BUNDLE = "@nimblebraininc/newsapi"; -const OTHER_BUNDLE = "@acme/other"; -const WS_A = "ws_alpha"; -const WS_B = "ws_beta"; - -let workDir: string; - -beforeEach(async () => { - workDir = await mkdtemp(join(tmpdir(), "nb-uninstall-creds-")); -}); - -afterEach(async () => { - await rm(workDir, { recursive: true, force: true }); -}); - -/** Minimal in-memory ToolSource stub. Stop is a no-op. */ -function makeStubSource(name: string): ToolSource { - return { - name, - async start(): Promise {}, - async stop(): Promise {}, - async tools(): Promise { - return []; - }, - async execute(): Promise { - throw new Error("not implemented"); - }, - }; -} - -// `newsapi` is the legacy short-slug deriveServerName result for -// `@nimblebraininc/newsapi`. After #195 the catalog install path -// persists a slug like `dev-mpak-nimblebraininc-newsapi`; the test -// uses the legacy short slug to match what `system-tools` resolves -// from `deriveServerName(bundleName)` as the back-compat fallback -// when `ref.serverName` is absent on a pre-#195 install. -const SERVER_NAME = "newsapi"; - -describe("uninstallBundleFromWorkspace — credential cleanup", () => { - test("removes the credential file for the uninstalled bundle", async () => { - await saveWorkspaceCredential(WS_A, BUNDLE, "api_key", "sk-abc", workDir); - expect(await getWorkspaceCredentials(WS_A, BUNDLE, workDir)).toEqual({ - api_key: "sk-abc", - }); - - const registry = new ToolRegistry(); - registry.addSource(makeStubSource(SERVER_NAME)); - - await uninstallBundleFromWorkspace(WS_A, BUNDLE, SERVER_NAME, registry, { workDir }); - - expect(await getWorkspaceCredentials(WS_A, BUNDLE, workDir)).toBeNull(); - expect(registry.hasSource(SERVER_NAME)).toBe(false); - }); - - test("does not throw when no credential file exists", async () => { - const registry = new ToolRegistry(); - registry.addSource(makeStubSource(SERVER_NAME)); - - await expect( - uninstallBundleFromWorkspace(WS_A, BUNDLE, SERVER_NAME, registry, { workDir }), - ).resolves.toBeUndefined(); - expect(registry.hasSource(SERVER_NAME)).toBe(false); - }); - - test("uninstalling one bundle does not touch credentials of another bundle in the same workspace", async () => { - await saveWorkspaceCredential(WS_A, BUNDLE, "api_key", "sk-a", workDir); - await saveWorkspaceCredential(WS_A, OTHER_BUNDLE, "api_key", "sk-b", workDir); - - const registry = new ToolRegistry(); - registry.addSource(makeStubSource(SERVER_NAME)); - - await uninstallBundleFromWorkspace(WS_A, BUNDLE, SERVER_NAME, registry, { workDir }); - - expect(await getWorkspaceCredentials(WS_A, BUNDLE, workDir)).toBeNull(); - expect(await getWorkspaceCredentials(WS_A, OTHER_BUNDLE, workDir)).toEqual({ - api_key: "sk-b", - }); - }); - - test("uninstalling a bundle in one workspace does not touch the same bundle's credentials in another workspace", async () => { - await saveWorkspaceCredential(WS_A, BUNDLE, "api_key", "sk-alpha", workDir); - await saveWorkspaceCredential(WS_B, BUNDLE, "api_key", "sk-beta", workDir); - - const registry = new ToolRegistry(); - registry.addSource(makeStubSource(SERVER_NAME)); - - await uninstallBundleFromWorkspace(WS_A, BUNDLE, SERVER_NAME, registry, { workDir }); - - expect(await getWorkspaceCredentials(WS_A, BUNDLE, workDir)).toBeNull(); - expect(await getWorkspaceCredentials(WS_B, BUNDLE, workDir)).toEqual({ - api_key: "sk-beta", - }); - }); - - test("throws if the bundle is not registered (no source removal, no credential change)", async () => { - await saveWorkspaceCredential(WS_A, BUNDLE, "api_key", "sk-abc", workDir); - - const registry = new ToolRegistry(); - // No source registered. - - await expect( - uninstallBundleFromWorkspace(WS_A, BUNDLE, SERVER_NAME, registry, { workDir }), - ).rejects.toThrow(/No bundle "newsapi" found in workspace "ws_alpha"/); - - // Credentials preserved (uninstall threw before the cleanup step). - expect(await getWorkspaceCredentials(WS_A, BUNDLE, workDir)).toEqual({ - api_key: "sk-abc", - }); - }); -});