diff --git a/packages/junior/src/app.ts b/packages/junior/src/app.ts index 10c6546c88..f25771a50b 100644 --- a/packages/junior/src/app.ts +++ b/packages/junior/src/app.ts @@ -693,7 +693,7 @@ export async function createApp(options?: JuniorAppOptions): Promise { validateBuildIncludesPluginPackages(pluginConfig, virtualConfig); } validateBuildIncludesPluginRuntimeRegistrations(plugins, virtualConfig); - validatePlugins(plugins); + validatePlugins(configuredPlugins?.registrations ?? []); getDb(); const shouldValidatePluginCatalog = hasConfiguredPluginCatalog(pluginConfig) || diff --git a/packages/junior/src/chat/briefs/registration.ts b/packages/junior/src/chat/briefs/registration.ts index c1320dd3dd..e771766574 100644 --- a/packages/junior/src/chat/briefs/registration.ts +++ b/packages/junior/src/chat/briefs/registration.ts @@ -1,6 +1,3 @@ -import type { PluginRegistration } from "@sentry/junior-plugin-api"; -import { briefsTaskRegistration } from "./task"; - type BriefsConfig = Readonly<{ enabled?: boolean }>; let configuredBriefs: BriefsConfig = {}; @@ -16,8 +13,3 @@ export function setBriefsConfig(config?: { enabled?: boolean }): BriefsConfig { export function isBriefsEnabled(): boolean { return configuredBriefs.enabled === true; } - -/** Return enabled core task registrations. */ -export function coreTaskRegistrations(): PluginRegistration[] { - return isBriefsEnabled() ? [briefsTaskRegistration] : []; -} diff --git a/packages/junior/src/chat/briefs/task.ts b/packages/junior/src/chat/briefs/task.ts index 6e4f30128b..0b84263a18 100644 --- a/packages/junior/src/chat/briefs/task.ts +++ b/packages/junior/src/chat/briefs/task.ts @@ -189,8 +189,8 @@ export async function updateConversationBrief( ); } -/** Core task registration kept outside the installed plugin catalog. */ -export const briefsTaskRegistration: PluginRegistration = { +/** Register Briefs outside the installed plugin catalog. */ +export const briefsFeatureRegistration: PluginRegistration = { manifest: { name: "briefs", displayName: "Briefs", diff --git a/packages/junior/src/chat/core-features/README.md b/packages/junior/src/chat/core-features/README.md new file mode 100644 index 0000000000..788a4828cf --- /dev/null +++ b/packages/junior/src/chat/core-features/README.md @@ -0,0 +1,5 @@ +# Core features + +Core features use the plugin runtime contract. They do not use plugin discovery or appear in the installed plugin catalog. + +`registration.ts` owns core feature registration and reserved names. diff --git a/packages/junior/src/chat/core-features/registration.ts b/packages/junior/src/chat/core-features/registration.ts new file mode 100644 index 0000000000..6e8ea900b1 --- /dev/null +++ b/packages/junior/src/chat/core-features/registration.ts @@ -0,0 +1,40 @@ +import type { PluginRegistration } from "@sentry/junior-plugin-api"; +import { isBriefsEnabled } from "@/chat/briefs/registration"; +import { briefsFeatureRegistration } from "@/chat/briefs/task"; + +const coreFeatures = [ + { isEnabled: isBriefsEnabled, registration: briefsFeatureRegistration }, +]; +/** Return all core registrations, including disabled features. */ +export function allCoreFeatureRegistrations(): PluginRegistration[] { + return coreFeatures.map((feature) => feature.registration); +} + +/** Return enabled core feature registrations. */ +export function coreFeatureRegistrations(): PluginRegistration[] { + return coreFeatures + .filter((feature) => feature.isEnabled()) + .map((feature) => feature.registration); +} + +/** Return enabled core features followed by installed plugins. */ +export function runtimeFeatureRegistrations( + plugins: PluginRegistration[], +): PluginRegistration[] { + return [...coreFeatureRegistrations(), ...plugins]; +} + +/** Reject a plugin that claims a core feature name. */ +export function assertNoCoreFeatureNameCollisions( + plugins: PluginRegistration[], +): void { + const coreNames = new Set( + allCoreFeatureRegistrations().map((feature) => feature.manifest.name), + ); + for (const plugin of plugins) { + const name = plugin.manifest.name; + if (coreNames.has(name)) { + throw new Error(`Plugin registration name "${name}" is reserved by core`); + } + } +} diff --git a/packages/junior/src/chat/plugins/agent-hooks.ts b/packages/junior/src/chat/plugins/agent-hooks.ts index 9069fb01d3..36198790f4 100644 --- a/packages/junior/src/chat/plugins/agent-hooks.ts +++ b/packages/junior/src/chat/plugins/agent-hooks.ts @@ -57,7 +57,10 @@ import { z } from "zod"; import { workspaceRepoCheckoutPath } from "@/chat/workspaces/checkout-path"; import { listWorkspaceNamesByRepository } from "@/chat/workspaces/store"; import { createCodeChangePublisher } from "@/chat/code/publisher"; -import { coreTaskRegistrations } from "@/chat/briefs/registration"; +import { + assertNoCoreFeatureNameCollisions, + runtimeFeatureRegistrations, +} from "@/chat/core-features/registration"; /** Signal that a plugin intentionally denied a tool execution. */ export class PluginHookDeniedError extends Error { @@ -382,6 +385,7 @@ function logInvalidPromptContributions(args: { /** Validate plugin identity before it can affect process-wide hooks. */ export function validatePlugins(plugins: PluginRegistration[]): void { + assertNoCoreFeatureNameCollisions(plugins); const seen = new Set(); for (const plugin of plugins) { const name = plugin.manifest.name; @@ -462,7 +466,7 @@ export async function getPluginSystemPromptContributions( ): Promise { const contributions: PluginPromptContributionContext[] = []; let totalChars = 0; - for (const plugin of getPlugins()) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.systemPrompt; if (!hook) { @@ -532,7 +536,7 @@ export async function getPluginUserPromptContributions(args: { const contributions: PluginPromptContributionContext[] = []; let totalChars = 0; let totalContextBytes = 0; - for (const plugin of getPlugins()) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.userPrompt; if (!hook) { @@ -612,7 +616,7 @@ export function getPluginTools( sandbox: PluginSandbox = createSandboxCapability(context.workspace), ): Record { const tools: Record = {}; - for (const plugin of getPlugins()) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.tools; if (!hook) { @@ -872,7 +876,7 @@ export function getPluginRoutes(options: { const seen = new Set(); const methodsByPath = new Map>(); - for (const plugin of getPlugins()) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.routes; if (!hook) { @@ -991,7 +995,7 @@ export function getPluginRoutes(options: { export function getPluginApiRoutes(): PluginApiRouteRegistration[] { const routes: PluginApiRouteRegistration[] = []; - for (const plugin of getPlugins()) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.apiRoutes; if (!hook) { @@ -1356,7 +1360,7 @@ export async function getPluginOperationalReports( nowMs: number, ): Promise { const reports: PluginOperationalReport[] = []; - for (const plugin of [...coreTaskRegistrations(), ...getPlugins()]) { + for (const plugin of runtimeFeatureRegistrations(getPlugins())) { const pluginName = plugin.manifest.name; const hook = plugin.hooks?.operationalReport; if (!hook) { diff --git a/packages/junior/src/chat/plugins/conversation-events.ts b/packages/junior/src/chat/plugins/conversation-events.ts index e5fa2388d7..585e88a89e 100644 --- a/packages/junior/src/chat/plugins/conversation-events.ts +++ b/packages/junior/src/chat/plugins/conversation-events.ts @@ -5,7 +5,7 @@ import type { PluginConversationEventValue, PluginRegistration, } from "@sentry/junior-plugin-api"; -import { briefsTaskRegistration } from "@/chat/briefs/task"; +import { allCoreFeatureRegistrations } from "@/chat/core-features/registration"; import { getConversationEventStore } from "@/chat/db"; import { getPlugins } from "./agent-hooks"; @@ -80,7 +80,7 @@ export function renderPluginConversationEvent(args: { namespace: string; version: number; }): ConversationEventPresentation | undefined { - const plugin = [briefsTaskRegistration, ...getPlugins()].find( + const plugin = [...allCoreFeatureRegistrations(), ...getPlugins()].find( (candidate) => candidate.manifest.name === args.namespace, ); const definition = plugin?.conversationEvents?.find( diff --git a/packages/junior/src/chat/plugins/task-runner.ts b/packages/junior/src/chat/plugins/task-runner.ts index a4f7262590..5ae13eaea4 100644 --- a/packages/junior/src/chat/plugins/task-runner.ts +++ b/packages/junior/src/chat/plugins/task-runner.ts @@ -47,7 +47,7 @@ import { getTurnRecord, type TurnRecord, } from "@/chat/task-execution/checkpoint"; -import { coreTaskRegistrations } from "@/chat/briefs/registration"; +import { runtimeFeatureRegistrations } from "@/chat/core-features/registration"; import { getPlugins } from "./agent-hooks"; import { pluginTaskId, @@ -442,7 +442,7 @@ function taskPluginContext( } function findPluginTask(message: PluginTaskQueueMessage) { - const plugin = [...coreTaskRegistrations(), ...getPlugins()].find( + const plugin = runtimeFeatureRegistrations(getPlugins()).find( (candidate) => candidate.manifest.name === message.plugin, ); if (!plugin?.tasks || !Object.hasOwn(plugin.tasks, message.name)) { @@ -458,11 +458,9 @@ export async function scheduleSessionCompletedPluginTasks( options: ScheduleSessionCompletedPluginTasksOptions = {}, ): Promise { const coreParams = pluginTaskParamsSchema.parse(params); - const taskRegistrations = [ - ...getPlugins(), - ...coreTaskRegistrations(), - ].flatMap((plugin) => - Object.keys(plugin.tasks ?? {}).map((name) => ({ name, plugin })), + const taskRegistrations = runtimeFeatureRegistrations(getPlugins()).flatMap( + (plugin) => + Object.keys(plugin.tasks ?? {}).map((name) => ({ name, plugin })), ); if (taskRegistrations.length === 0) { return; diff --git a/packages/junior/src/chat/plugins/user-pages.ts b/packages/junior/src/chat/plugins/user-pages.ts index 2573fc4324..eb32ba988e 100644 --- a/packages/junior/src/chat/plugins/user-pages.ts +++ b/packages/junior/src/chat/plugins/user-pages.ts @@ -15,11 +15,12 @@ import { getDb } from "@/chat/db"; import { createPluginLogger } from "@/chat/plugins/logging"; import { resolveViewerUser } from "@/chat/plugins/viewer"; import { getPlugins } from "@/chat/plugins/agent-hooks"; +import { runtimeFeatureRegistrations } from "@/chat/core-features/registration"; /** List safe navigation metadata for registered plugin user pages. */ export function readPluginUserPageLinks(): PluginUserPageLink[] { return pluginUserPageLinksSchema.parse( - getPlugins().flatMap((plugin) => + runtimeFeatureRegistrations(getPlugins()).flatMap((plugin) => (plugin.userPages ?? []).map((page) => ({ description: page.description, id: page.id, @@ -50,7 +51,7 @@ export async function readPluginUserPage(input: { pluginName: string; query: PluginUserPageInput; }): Promise { - const plugin = getPlugins().find( + const plugin = runtimeFeatureRegistrations(getPlugins()).find( (candidate) => candidate.manifest.name === input.pluginName, ); const page = plugin?.userPages?.find( diff --git a/packages/junior/src/cli/plugins.ts b/packages/junior/src/cli/plugins.ts index 206dbd6360..39726e5bb4 100644 --- a/packages/junior/src/cli/plugins.ts +++ b/packages/junior/src/cli/plugins.ts @@ -21,6 +21,7 @@ import { validatePluginRegistrations, } from "@/chat/plugins/validation"; import { loadAppPluginSet } from "@/plugin-module"; +import { runtimeFeatureRegistrations } from "@/chat/core-features/registration"; import { pluginCliRegistrationsFromPluginSet, pluginCatalogConfigFromPluginSet, @@ -226,14 +227,17 @@ async function loadPluginRegistrations(args: { runtimePlugins: PluginRegistration[]; }> { const pluginSet = args.pluginSet; + const cliPlugins = runtimeFeatureRegistrations( + pluginCliRegistrationsFromPluginSet(pluginSet), + ).filter((registration) => registration.cli); if (!pluginSet) { - return { cliPlugins: [], runtimePlugins: [] }; + args.validateConfiguredCommands?.(cliPlugins); + return { cliPlugins, runtimePlugins: [] }; } - const cliPlugins = pluginCliRegistrationsFromPluginSet(pluginSet); const runtimePlugins = pluginRuntimeRegistrationsFromPluginSet(pluginSet); const pluginConfig = pluginCatalogConfigFromPluginSet(pluginSet); - validatePlugins(runtimePlugins); + validatePlugins(pluginSet.registrations); const previousPluginCatalogConfig = pluginCatalogRuntime.setConfig(pluginConfig); try { diff --git a/packages/junior/tests/unit/core-feature-registration.test.ts b/packages/junior/tests/unit/core-feature-registration.test.ts new file mode 100644 index 0000000000..de1181974c --- /dev/null +++ b/packages/junior/tests/unit/core-feature-registration.test.ts @@ -0,0 +1,41 @@ +import { defineJuniorPlugin } from "@sentry/junior-plugin-api"; +import { afterEach, describe, expect, it } from "vitest"; +import { setBriefsConfig } from "@/chat/briefs/registration"; +import { + allCoreFeatureRegistrations, + coreFeatureRegistrations, +} from "@/chat/core-features/registration"; +import { validatePlugins } from "@/chat/plugins/agent-hooks"; + +const briefsPlugin = defineJuniorPlugin({ + manifest: { + name: "briefs", + displayName: "Briefs", + description: "Conflicting Briefs plugin", + }, +}); + +afterEach(() => { + setBriefsConfig(undefined); +}); + +describe("core feature registration", () => { + it("enables Briefs from app config but keeps its registration available", () => { + expect(coreFeatureRegistrations()).toEqual([]); + expect( + allCoreFeatureRegistrations().map((feature) => feature.manifest.name), + ).toEqual(["briefs"]); + + setBriefsConfig({ enabled: true }); + + expect( + coreFeatureRegistrations().map((feature) => feature.manifest.name), + ).toEqual(["briefs"]); + }); + + it("reserves core feature names", () => { + expect(() => validatePlugins([briefsPlugin])).toThrow( + 'Plugin registration name "briefs" is reserved by core', + ); + }); +});