diff --git a/PREVENT_CONFLICTS.md b/PREVENT_CONFLICTS.md index f836d95..e611744 100644 --- a/PREVENT_CONFLICTS.md +++ b/PREVENT_CONFLICTS.md @@ -40,7 +40,7 @@ This document outlines strategies to minimize merge conflicts and improve produc ## Discrete Action Items ### Immediate Actions -1. **Refactor Plugin Registration**: Modify `PluginManager` to accept a list of plugins without hardcoding defaults in the class itself. Move default plugin logic to a separate `DefaultPlugins` module. +1. [x] **Refactor Plugin Registration**: Modify `PluginManager` to accept a list of plugins without hardcoding defaults in the class itself. Move default plugin logic to a separate `DefaultPlugins` module. (Completed 2025-02-18) 2. **Harden Types**: systematically review `src/core/interfaces.ts` and replace `any` with generic types or `unknown` with validation. specifically target `RequestContext`, `DataSourceProvider`, and `DataActionProvider`. 3. **Decompose `dataprompt.ts`**: Extract `createDefaultGenkit` and `loadUserGenkitInstance` into a separate `GenkitFactory` module. diff --git a/src/core/config.manager.ts b/src/core/config.manager.ts index 066a918..0e8d066 100644 --- a/src/core/config.manager.ts +++ b/src/core/config.manager.ts @@ -3,9 +3,7 @@ import { z } from 'genkit'; import { findUp } from 'find-up'; import { DatapromptPlugin } from './interfaces.js'; import { DatapromptUserConfig, DatapromptConfig } from './config.js'; -import { firestorePlugin } from '../plugins/firebase/public.js'; -import { schedulerPlugin } from '../plugins/scheduler/index.js'; -import { fetchPlugin } from '../plugins/fetch/index.js'; +import { resolvePlugins } from './default-plugins.js'; import { pathToFileURL } from 'node:url'; const CoreSecretsSchema = z.object({ @@ -67,7 +65,7 @@ export class ConfigManager { mergedConfig.schemaFile = path.resolve(rootDir, mergedConfig.schemaFile); // Resolve plugins based on the potentially overridden plugins array. - mergedConfig.plugins = this.#resolvePlugins(userConfig?.plugins); + mergedConfig.plugins = resolvePlugins(userConfig?.plugins); // Validate the final, fully merged secrets object. this.#validateSecrets(mergedConfig); @@ -92,14 +90,6 @@ export class ConfigManager { return packageJsonPath ? path.dirname(packageJsonPath) : this.#projectRoot; } - #resolvePlugins = (userPlugins: DatapromptPlugin[] = []): DatapromptPlugin[] => { - const plugins = [...userPlugins]; - if (!plugins.some(p => p.name === 'firestore')) plugins.push(firestorePlugin()); - if (!plugins.some(p => p.name === 'fetch')) plugins.push(fetchPlugin()); - if (!plugins.some(p => p.name === 'schedule')) plugins.push(schedulerPlugin()); - return plugins; - } - // This method now only validates and throws on error. It does not return a value. #validateSecrets = (config: Partial): void => { let validationSchema = CoreSecretsSchema; diff --git a/src/core/default-plugins.ts b/src/core/default-plugins.ts new file mode 100644 index 0000000..c03f012 --- /dev/null +++ b/src/core/default-plugins.ts @@ -0,0 +1,20 @@ +import { firestorePlugin } from '../plugins/firebase/public.js'; +import { schedulerPlugin } from '../plugins/scheduler/index.js'; +import { fetchPlugin } from '../plugins/fetch/index.js'; +import { DatapromptPlugin } from './interfaces.js'; + +export function getDefaultPlugins(): DatapromptPlugin[] { + return [ + firestorePlugin(), + fetchPlugin(), + schedulerPlugin(), + ]; +} + +export function resolvePlugins(userPlugins: DatapromptPlugin[] = []): DatapromptPlugin[] { + const plugins = [...userPlugins]; + if (!plugins.some(p => p.name === 'firestore')) plugins.push(firestorePlugin()); + if (!plugins.some(p => p.name === 'fetch')) plugins.push(fetchPlugin()); + if (!plugins.some(p => p.name === 'schedule')) plugins.push(schedulerPlugin()); + return plugins; +} diff --git a/src/core/plugin.manager.ts b/src/core/plugin.manager.ts index 2828672..a95c35e 100644 --- a/src/core/plugin.manager.ts +++ b/src/core/plugin.manager.ts @@ -1,8 +1,5 @@ import { DatapromptConfig } from './config.js'; import { DatapromptPlugin, DataActionProvider, DataSourceProvider, TriggerProvider } from './interfaces.js'; -import { firestorePlugin } from '../plugins/firebase/public.js'; -import { schedulerPlugin } from '../plugins/scheduler/index.js'; -import { fetchPlugin } from '../plugins/fetch/index.js'; export class PluginManager { #dataSources = new Map(); @@ -10,7 +7,7 @@ export class PluginManager { #triggers = new Map(); // Added map for triggers constructor(config: DatapromptConfig) { - const allPlugins = this.#resolvePlugins(config.plugins); + const allPlugins = config.plugins; for (const plugin of allPlugins) { this.#registerPlugin(plugin); @@ -33,14 +30,6 @@ export class PluginManager { } } - #resolvePlugins = (userPlugins: DatapromptPlugin[] = []): DatapromptPlugin[] => { - const plugins = [...userPlugins]; - if (!plugins.some(p => p.name === 'firestore')) plugins.push(firestorePlugin()); - if (!plugins.some(p => p.name === 'fetch')) plugins.push(fetchPlugin()); - if (!plugins.some(p => p.name === 'schedule')) plugins.push(schedulerPlugin()); - return plugins; - } - public getDataSource(name: string): DataSourceProvider { const provider = this.#dataSources.get(name); if (!provider) { diff --git a/tests/config.manager.test.ts b/tests/config.manager.test.ts index e415f59..cad9bdf 100644 --- a/tests/config.manager.test.ts +++ b/tests/config.manager.test.ts @@ -46,6 +46,11 @@ describe('ConfigManager with a config file', () => { expect(config.promptsDir).toBe(path.resolve(tempDir, 'custom_prompts')); expect(config.secrets.MY_CUSTOM_SECRET).toBe('secret_value'); expect(config.secrets.GOOGLEAI_API_KEY).toBe('test-key-from-env'); // from default + + // Assert default plugins are present + expect(config.plugins.some(p => p.name === 'fetch')).toBe(true); + expect(config.plugins.some(p => p.name === 'firestore')).toBe(true); + expect(config.plugins.some(p => p.name === 'schedule')).toBe(true); // CLEANUP delete process.env.GOOGLEAI_API_KEY; @@ -81,6 +86,11 @@ describe('ConfigManager without a config file', () => { expect(config.secrets.GOOGLE_APPLICATION_CREDENTIALS).toBe('./fake-creds.json'); expect(config.promptsDir).toBe(path.resolve(tempDir, 'prompts')); + // Assert default plugins are present + expect(config.plugins.some(p => p.name === 'fetch')).toBe(true); + expect(config.plugins.some(p => p.name === 'firestore')).toBe(true); + expect(config.plugins.some(p => p.name === 'schedule')).toBe(true); + // CLEANUP delete process.env.GOOGLEAI_API_KEY; delete process.env.GOOGLE_APPLICATION_CREDENTIALS; diff --git a/tests/integration/prompt.test.ts b/tests/integration/prompt.test.ts index 3a7bf99..42888b8 100644 --- a/tests/integration/prompt.test.ts +++ b/tests/integration/prompt.test.ts @@ -12,6 +12,7 @@ import { DatapromptConfig } from '../../src/core/config.js'; import { DatapromptFile, RequestContext } from '../../src/core/interfaces.js'; import { createRoute } from '../../src/routing/route-builder.js'; import { SchemaMap, registerUserSchemas } from '../../src/utils/schema-loader.js'; +import { getDefaultPlugins } from '../../src/core/default-plugins.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -106,7 +107,7 @@ describe('Prompt Class Integration Tests', () => { GOOGLEAI_API_KEY: process.env.GOOGLEAI_API_KEY || 'test-key', GOOGLE_APPLICATION_CREDENTIALS: './fake-creds.json' }, - plugins: [] + plugins: getDefaultPlugins() }; ai = new Genkit({ plugins: [googleAI({ apiKey: config.secrets.GOOGLEAI_API_KEY })] }); diff --git a/tests/unit/plugin.manager.test.ts b/tests/unit/plugin.manager.test.ts index 8ab4f3a..af2821e 100644 --- a/tests/unit/plugin.manager.test.ts +++ b/tests/unit/plugin.manager.test.ts @@ -3,15 +3,16 @@ import { describe, it, expect } from 'vitest'; import { PluginManager } from '../../src/core/plugin.manager.js'; import { DatapromptConfig } from '../../src/core/config.js'; import { DatapromptPlugin } from '../../src/core/interfaces.js'; +import { getDefaultPlugins } from '../../src/core/default-plugins.js'; describe('PluginManager', () => { - it('should register default plugins if none are provided', () => { + it('should register provided plugins', () => { const mockConfig: DatapromptConfig = { rootDir: '/mock', promptsDir: '/mock/prompts', schemaFile: '/mock/schema.ts', secrets: {}, - plugins: [] // Start with an empty plugins array + plugins: getDefaultPlugins() }; const manager = new PluginManager(mockConfig); @@ -22,7 +23,7 @@ describe('PluginManager', () => { expect(actions.map(a => a.name)).toEqual(expect.arrayContaining(['firestore'])); }); - it('should register user-provided plugins alongside defaults', () => { + it('should register user-provided plugins alongside others', () => { const userPlugin: DatapromptPlugin = { name: 'custom-plugin', createDataSource: () => ({ name: 'custom-source', fetchData: async () => ({}) }), @@ -34,7 +35,7 @@ describe('PluginManager', () => { promptsDir: '/mock/prompts', schemaFile: '/mock/schema.ts', secrets: {}, - plugins: [userPlugin] + plugins: [userPlugin, ...getDefaultPlugins()] }; const manager = new PluginManager(mockConfig);