Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PREVENT_CONFLICTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
14 changes: 2 additions & 12 deletions src/core/config.manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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);
Expand All @@ -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<DatapromptConfig>): void => {
let validationSchema = CoreSecretsSchema;
Expand Down
20 changes: 20 additions & 0 deletions src/core/default-plugins.ts
Original file line number Diff line number Diff line change
@@ -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;
}
13 changes: 1 addition & 12 deletions src/core/plugin.manager.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
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<string, DataSourceProvider>();
#actions = new Map<string, DataActionProvider>();
#triggers = new Map<string, TriggerProvider>(); // 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);
Expand All @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions tests/config.manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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 })] });
Expand Down
9 changes: 5 additions & 4 deletions tests/unit/plugin.manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 () => ({}) }),
Expand All @@ -34,7 +35,7 @@ describe('PluginManager', () => {
promptsDir: '/mock/prompts',
schemaFile: '/mock/schema.ts',
secrets: {},
plugins: [userPlugin]
plugins: [userPlugin, ...getDefaultPlugins()]
};

const manager = new PluginManager(mockConfig);
Expand Down