chore: import upstream snapshot with attribution
CI / Shell Format Check (push) Has been cancelled
CI / Check Ruby (3.4) (push) Has been cancelled
CI / CI Config (push) Has been cancelled
CI / Test on Node ${{ matrix.node }} and ${{ matrix.os }}${{ matrix.shard && format(' (shard {0}/3)', matrix.shard) || '' }} (push) Has been cancelled
CI / Build on Node ${{ matrix.node }} (push) Has been cancelled
CI / Style Check (push) Has been cancelled
CI / Generate Assets (push) Has been cancelled
CI / Check Python (3.14) (push) Has been cancelled
CI / Check Python (3.9) (push) Has been cancelled
CI / Build Docs (push) Has been cancelled
CI / Code Scan Action (push) Has been cancelled
CI / Site tests (push) Has been cancelled
CI / webui tests (push) Has been cancelled
CI / Run Integration Tests (push) Has been cancelled
CI / Run Smoke Tests (push) Has been cancelled
CI / Go Tests (push) Has been cancelled
CI / Share Test (push) Has been cancelled
CI / Redteam (Production API) (push) Has been cancelled
CI / Redteam (Staging API) (push) Has been cancelled
CI / GitHub Actions Lint (push) Has been cancelled
CI / Check Ruby (3.0) (push) Has been cancelled
release-please / release-please (push) Has been cancelled
release-please / build (push) Has been cancelled
release-please / publish-npm (push) Has been cancelled
release-please / publish-npm-backfill (push) Has been cancelled
release-please / docker (push) Has been cancelled
release-please / publish-code-scan-action (push) Has been cancelled
release-please / attest-code-scan-action (push) Has been cancelled
Deploy local.promptfoo.app / Deploy to Cloudflare Pages (push) Has been cancelled
Test and Publish Multi-arch Docker Image / test (push) Has been cancelled
Test and Publish Multi-arch Docker Image / build-docker-and-push-digests (map[digest-suffix:linux-amd64 platform:linux/amd64 runner:ubuntu-latest]) (push) Has been cancelled
Test and Publish Multi-arch Docker Image / build-docker-and-push-digests (map[digest-suffix:linux-arm64 platform:linux/arm64 runner:ubuntu-24.04-arm]) (push) Has been cancelled
Test and Publish Multi-arch Docker Image / merge-docker-digests (push) Has been cancelled
Test and Publish Multi-arch Docker Image / Attest Multi-arch Image (push) Has been cancelled
Validate Renovate Config / Validate Renovate Configuration (push) Has been cancelled
CI / Shell Format Check (push) Has been cancelled
CI / Check Ruby (3.4) (push) Has been cancelled
CI / CI Config (push) Has been cancelled
CI / Test on Node ${{ matrix.node }} and ${{ matrix.os }}${{ matrix.shard && format(' (shard {0}/3)', matrix.shard) || '' }} (push) Has been cancelled
CI / Build on Node ${{ matrix.node }} (push) Has been cancelled
CI / Style Check (push) Has been cancelled
CI / Generate Assets (push) Has been cancelled
CI / Check Python (3.14) (push) Has been cancelled
CI / Check Python (3.9) (push) Has been cancelled
CI / Build Docs (push) Has been cancelled
CI / Code Scan Action (push) Has been cancelled
CI / Site tests (push) Has been cancelled
CI / webui tests (push) Has been cancelled
CI / Run Integration Tests (push) Has been cancelled
CI / Run Smoke Tests (push) Has been cancelled
CI / Go Tests (push) Has been cancelled
CI / Share Test (push) Has been cancelled
CI / Redteam (Production API) (push) Has been cancelled
CI / Redteam (Staging API) (push) Has been cancelled
CI / GitHub Actions Lint (push) Has been cancelled
CI / Check Ruby (3.0) (push) Has been cancelled
release-please / release-please (push) Has been cancelled
release-please / build (push) Has been cancelled
release-please / publish-npm (push) Has been cancelled
release-please / publish-npm-backfill (push) Has been cancelled
release-please / docker (push) Has been cancelled
release-please / publish-code-scan-action (push) Has been cancelled
release-please / attest-code-scan-action (push) Has been cancelled
Deploy local.promptfoo.app / Deploy to Cloudflare Pages (push) Has been cancelled
Test and Publish Multi-arch Docker Image / test (push) Has been cancelled
Test and Publish Multi-arch Docker Image / build-docker-and-push-digests (map[digest-suffix:linux-amd64 platform:linux/amd64 runner:ubuntu-latest]) (push) Has been cancelled
Test and Publish Multi-arch Docker Image / build-docker-and-push-digests (map[digest-suffix:linux-arm64 platform:linux/arm64 runner:ubuntu-24.04-arm]) (push) Has been cancelled
Test and Publish Multi-arch Docker Image / merge-docker-digests (push) Has been cancelled
Test and Publish Multi-arch Docker Image / Attest Multi-arch Image (push) Has been cancelled
Validate Renovate Config / Validate Renovate Configuration (push) Has been cancelled
This commit is contained in:
@@ -0,0 +1,544 @@
|
||||
# Plugins State Management Refactor Implementation Plan
|
||||
|
||||
## Overview
|
||||
|
||||
Refactor `Plugins.tsx` to remove interstitial state management between `PluginsTab.tsx` and the Zustand store. The current implementation maintains duplicate local state (`selectedPlugins`, `pluginConfig`, `hasUserInteracted`) with bi-directional sync effects. This refactor will derive state directly from the store and update the store directly on user interactions.
|
||||
|
||||
## Current State Analysis
|
||||
|
||||
### Three-Layer Architecture (to be simplified)
|
||||
|
||||
1. **Global State**: Zustand store (`useRedTeamConfig.ts`) - source of truth
|
||||
2. **Local State**: `Plugins.tsx` maintains `selectedPlugins`, `pluginConfig`, `hasUserInteracted`
|
||||
3. **Props**: `PluginsTab.tsx` receives state via props
|
||||
|
||||
### Key Files
|
||||
|
||||
- `src/app/src/pages/redteam/setup/components/Plugins.tsx` - Parent component with local state
|
||||
- `src/app/src/pages/redteam/setup/components/PluginsTab.tsx` - Child component receiving props
|
||||
- `src/app/src/pages/redteam/setup/hooks/useRedTeamConfig.ts` - Zustand store
|
||||
|
||||
### Current State Variables to Remove (Plugins.tsx)
|
||||
|
||||
- `const [selectedPlugins, setSelectedPlugins]` - lines 112-118
|
||||
- `const [hasUserInteracted, setHasUserInteracted]` - line 135
|
||||
- `const [pluginConfig, setPluginConfig]` - lines 136-144
|
||||
|
||||
### Current Sync Effects to Remove (Plugins.tsx)
|
||||
|
||||
- Effect 1 (lines 152-168): Syncs store → local state when `!hasUserInteracted`
|
||||
- Effect 2 (lines 171-199): Syncs local state → store when `hasUserInteracted`
|
||||
|
||||
### Store Protection Already Exists
|
||||
|
||||
The `updatePlugins` method (useRedTeamConfig.ts:272-311) already:
|
||||
|
||||
- Merges new plugin configs with existing configs
|
||||
- Compares output vs current state to prevent infinite loops
|
||||
- Only triggers updates when state actually changed
|
||||
|
||||
## Desired End State
|
||||
|
||||
After this refactor:
|
||||
|
||||
1. `Plugins.tsx` derives `selectedPlugins` and `pluginConfig` from `config.plugins` using `useMemo`
|
||||
2. All plugin mutations go directly to the Zustand store
|
||||
3. No local state synchronization effects
|
||||
4. `PluginsTab` has a new `setSelectedPlugins` prop for efficient bulk operations
|
||||
5. `onUserInteraction` prop is removed from `PluginsTab`
|
||||
|
||||
### How to Verify
|
||||
|
||||
1. All tests in `PluginsTab.test.tsx` pass
|
||||
2. Preset selection works (plugins are set correctly in store)
|
||||
3. Individual plugin toggle works (checkbox toggles update store)
|
||||
4. Select All/None buttons work correctly
|
||||
5. Clear All button works correctly
|
||||
6. Plugin configs (e.g., for `indirect-prompt-injection`) are preserved when toggling
|
||||
|
||||
## What We're NOT Doing
|
||||
|
||||
- Changing the Zustand store implementation
|
||||
- Modifying `CustomIntentsTab` or `CustomPoliciesTab`
|
||||
- Changing how policy/intent plugins are handled
|
||||
- Refactoring the `PluginConfigDialog` component
|
||||
- Changing test structure or test helpers
|
||||
|
||||
## Implementation Approach
|
||||
|
||||
The refactoring follows these principles:
|
||||
|
||||
1. **Remove duplicate state** - No local state that mirrors the store
|
||||
2. **Derive, don't store** - Use `useMemo` to compute `selectedPlugins` and `pluginConfig` from `config.plugins`
|
||||
3. **Direct store updates** - All mutations go straight to `updatePlugins`
|
||||
4. **Efficient bulk operations** - Add `setSelectedPlugins` for presets and bulk selection
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Refactor `Plugins.tsx`
|
||||
|
||||
### Overview
|
||||
|
||||
Remove local state and sync effects, replace with derived values and direct store updates.
|
||||
|
||||
### Changes Required:
|
||||
|
||||
#### 1. Remove Local State Variables
|
||||
|
||||
**File**: `src/app/src/pages/redteam/setup/components/Plugins.tsx`
|
||||
|
||||
**Remove lines 112-118** (selectedPlugins state):
|
||||
|
||||
```tsx
|
||||
// REMOVE THIS:
|
||||
const [selectedPlugins, setSelectedPlugins] = useState<Set<Plugin>>(() => {
|
||||
return new Set(
|
||||
config.plugins
|
||||
.map((plugin) => (typeof plugin === 'string' ? plugin : plugin.id))
|
||||
.filter((id) => id !== 'policy' && id !== 'intent') as Plugin[],
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
**Remove line 135** (hasUserInteracted state):
|
||||
|
||||
```tsx
|
||||
// REMOVE THIS:
|
||||
const [hasUserInteracted, setHasUserInteracted] = useState(false);
|
||||
```
|
||||
|
||||
**Remove lines 136-144** (pluginConfig state):
|
||||
|
||||
```tsx
|
||||
// REMOVE THIS:
|
||||
const [pluginConfig, setPluginConfig] = useState<LocalPluginConfig>(() => {
|
||||
const initialConfig: LocalPluginConfig = {};
|
||||
config.plugins.forEach((plugin) => {
|
||||
if (typeof plugin === 'object' && plugin.config) {
|
||||
initialConfig[plugin.id] = plugin.config;
|
||||
}
|
||||
});
|
||||
return initialConfig;
|
||||
});
|
||||
```
|
||||
|
||||
#### 2. Add Derived Values
|
||||
|
||||
**Add after the store hook calls (after line 108):**
|
||||
|
||||
```tsx
|
||||
// Derive selectedPlugins from config.plugins
|
||||
const selectedPlugins = useMemo(() => {
|
||||
return new Set(
|
||||
config.plugins
|
||||
.map((plugin) => (typeof plugin === 'string' ? plugin : plugin.id))
|
||||
.filter((id) => id !== 'policy' && id !== 'intent') as Plugin[],
|
||||
);
|
||||
}, [config.plugins]);
|
||||
|
||||
// Derive pluginConfig from config.plugins
|
||||
const pluginConfig = useMemo(() => {
|
||||
const configs: LocalPluginConfig = {};
|
||||
config.plugins.forEach((plugin) => {
|
||||
if (typeof plugin === 'object' && plugin.config) {
|
||||
configs[plugin.id] = plugin.config;
|
||||
}
|
||||
});
|
||||
return configs;
|
||||
}, [config.plugins]);
|
||||
```
|
||||
|
||||
#### 3. Remove Sync Effects
|
||||
|
||||
**Remove lines 152-168** (Effect 1 - config → local sync):
|
||||
|
||||
```tsx
|
||||
// REMOVE THIS ENTIRE EFFECT:
|
||||
useEffect(() => {
|
||||
if (!hasUserInteracted) {
|
||||
const configPlugins = new Set(
|
||||
config.plugins
|
||||
.map((plugin) => (typeof plugin === 'string' ? plugin : plugin.id))
|
||||
.filter((id) => id !== 'policy' && id !== 'intent') as Plugin[],
|
||||
);
|
||||
if (
|
||||
configPlugins.size !== selectedPlugins.size ||
|
||||
!Array.from(configPlugins).every((plugin) => selectedPlugins.has(plugin))
|
||||
) {
|
||||
setSelectedPlugins(configPlugins);
|
||||
}
|
||||
}
|
||||
}, [config.plugins, hasUserInteracted, selectedPlugins]);
|
||||
```
|
||||
|
||||
**Remove lines 171-199** (Effect 2 - local → config sync):
|
||||
|
||||
```tsx
|
||||
// REMOVE THIS ENTIRE EFFECT:
|
||||
useEffect(() => {
|
||||
if (hasUserInteracted) {
|
||||
const policyPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'policy');
|
||||
const intentPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'intent');
|
||||
const regularPlugins = Array.from(selectedPlugins).map((plugin) => {
|
||||
const existingConfig = pluginConfig[plugin];
|
||||
if (existingConfig && Object.keys(existingConfig).length > 0) {
|
||||
return {
|
||||
id: plugin,
|
||||
config: existingConfig,
|
||||
};
|
||||
}
|
||||
return plugin;
|
||||
});
|
||||
const allPlugins = [...regularPlugins, ...policyPlugins, ...intentPlugins];
|
||||
updatePlugins(allPlugins as Array<string | { id: string; config: any }>);
|
||||
}
|
||||
}, [selectedPlugins, pluginConfig, hasUserInteracted, config.plugins, updatePlugins]);
|
||||
```
|
||||
|
||||
#### 4. Refactor `handlePluginToggle`
|
||||
|
||||
**Replace the current implementation (lines 201-236) with:**
|
||||
|
||||
```tsx
|
||||
const handlePluginToggle = useCallback(
|
||||
(plugin: Plugin) => {
|
||||
// Preserve policy and intent plugins
|
||||
const policyPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'policy');
|
||||
const intentPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'intent');
|
||||
|
||||
// Get current regular plugins (excluding policy/intent)
|
||||
const currentRegularPlugins = config.plugins.filter((p) => {
|
||||
const id = typeof p === 'string' ? p : p.id;
|
||||
return id !== 'policy' && id !== 'intent';
|
||||
});
|
||||
|
||||
const isCurrentlySelected = selectedPlugins.has(plugin);
|
||||
|
||||
let newRegularPlugins: Config['plugins'];
|
||||
|
||||
if (isCurrentlySelected) {
|
||||
// Remove the plugin
|
||||
newRegularPlugins = currentRegularPlugins.filter((p) => {
|
||||
const id = typeof p === 'string' ? p : p.id;
|
||||
return id !== plugin;
|
||||
});
|
||||
} else {
|
||||
// Add the plugin
|
||||
addPlugin(plugin); // Add to recently used
|
||||
newRegularPlugins = [...currentRegularPlugins, plugin];
|
||||
}
|
||||
|
||||
// Combine all plugins and update store
|
||||
const allPlugins = [...newRegularPlugins, ...policyPlugins, ...intentPlugins];
|
||||
updatePlugins(allPlugins);
|
||||
},
|
||||
[config.plugins, selectedPlugins, updatePlugins, addPlugin],
|
||||
);
|
||||
```
|
||||
|
||||
#### 5. Add `setSelectedPlugins` Handler for Bulk Operations
|
||||
|
||||
**Add after `handlePluginToggle`:**
|
||||
|
||||
```tsx
|
||||
const setSelectedPlugins = useCallback(
|
||||
(newSelectedPlugins: Set<Plugin>) => {
|
||||
// Preserve policy and intent plugins
|
||||
const policyPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'policy');
|
||||
const intentPlugins = config.plugins.filter((p) => typeof p === 'object' && p.id === 'intent');
|
||||
|
||||
// Create new plugins array, preserving configs from existing plugins
|
||||
const newPluginsArray: Config['plugins'] = Array.from(newSelectedPlugins).map((plugin) => {
|
||||
const existing = config.plugins.find((p) => (typeof p === 'string' ? p : p.id) === plugin);
|
||||
if (existing && typeof existing === 'object' && existing.config) {
|
||||
return existing; // Preserve existing config
|
||||
}
|
||||
return plugin;
|
||||
});
|
||||
|
||||
// Combine all plugins and update store
|
||||
const allPlugins = [...newPluginsArray, ...policyPlugins, ...intentPlugins];
|
||||
updatePlugins(allPlugins);
|
||||
},
|
||||
[config.plugins, updatePlugins],
|
||||
);
|
||||
```
|
||||
|
||||
#### 6. Refactor `updatePluginConfig`
|
||||
|
||||
**Replace the current implementation (lines 238-257) with:**
|
||||
|
||||
```tsx
|
||||
const updatePluginConfig = useCallback(
|
||||
(plugin: string, newConfig: Partial<LocalPluginConfig[string]>) => {
|
||||
// Build new plugins array with updated config
|
||||
const newPlugins = config.plugins.map((p) => {
|
||||
const id = typeof p === 'string' ? p : p.id;
|
||||
if (id === plugin) {
|
||||
const existingConfig = typeof p === 'object' ? p.config || {} : {};
|
||||
return {
|
||||
id: plugin,
|
||||
config: { ...existingConfig, ...newConfig },
|
||||
};
|
||||
}
|
||||
return p;
|
||||
});
|
||||
|
||||
updatePlugins(newPlugins);
|
||||
},
|
||||
[config.plugins, updatePlugins],
|
||||
);
|
||||
```
|
||||
|
||||
#### 7. Update PluginsTab Props
|
||||
|
||||
**Update the PluginsTab component call (around line 431):**
|
||||
|
||||
```tsx
|
||||
<PluginsTab
|
||||
selectedPlugins={selectedPlugins}
|
||||
handlePluginToggle={handlePluginToggle}
|
||||
setSelectedPlugins={setSelectedPlugins} // NEW PROP
|
||||
pluginConfig={pluginConfig}
|
||||
updatePluginConfig={updatePluginConfig}
|
||||
recentlyUsedPlugins={recentlyUsedSnapshot}
|
||||
isRemoteGenerationDisabled={isRemoteGenerationDisabled}
|
||||
/>
|
||||
```
|
||||
|
||||
**Remove** the `onUserInteraction` prop.
|
||||
|
||||
### Success Criteria:
|
||||
|
||||
#### Automated Verification:
|
||||
|
||||
- [x] TypeScript compilation passes: `npm run tsc` from `src/app`
|
||||
- [x] Linting passes: `npm run lint`
|
||||
- [x] All PluginsTab tests pass: `npm run test:app -- src/pages/redteam/setup/components/PluginsTab.test.tsx`
|
||||
|
||||
**Implementation Note**: After completing this phase and all automated verification passes, proceed to Phase 2.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Update `PluginsTab.tsx`
|
||||
|
||||
### Overview
|
||||
|
||||
Update the PluginsTab component to use the new `setSelectedPlugins` prop and remove `onUserInteraction`.
|
||||
|
||||
### Changes Required:
|
||||
|
||||
#### 1. Update Props Interface
|
||||
|
||||
**File**: `src/app/src/pages/redteam/setup/components/PluginsTab.tsx`
|
||||
|
||||
**Replace lines 64-72:**
|
||||
|
||||
```tsx
|
||||
export interface PluginsTabProps {
|
||||
selectedPlugins: Set<Plugin>;
|
||||
handlePluginToggle: (plugin: Plugin) => void;
|
||||
setSelectedPlugins: (plugins: Set<Plugin>) => void; // NEW
|
||||
pluginConfig: LocalPluginConfig;
|
||||
updatePluginConfig: (plugin: string, newConfig: Partial<LocalPluginConfig[string]>) => void;
|
||||
recentlyUsedPlugins: Plugin[];
|
||||
isRemoteGenerationDisabled: boolean;
|
||||
// REMOVED: onUserInteraction
|
||||
}
|
||||
```
|
||||
|
||||
#### 2. Update Component Parameters
|
||||
|
||||
**Update lines 74-82:**
|
||||
|
||||
```tsx
|
||||
export default function PluginsTab({
|
||||
selectedPlugins,
|
||||
handlePluginToggle,
|
||||
setSelectedPlugins, // NEW
|
||||
pluginConfig,
|
||||
updatePluginConfig,
|
||||
recentlyUsedPlugins,
|
||||
isRemoteGenerationDisabled,
|
||||
}: PluginsTabProps): React.ReactElement {
|
||||
```
|
||||
|
||||
#### 3. Refactor `handlePresetSelect`
|
||||
|
||||
**Replace the current implementation (around lines 367-392):**
|
||||
|
||||
```tsx
|
||||
const handlePresetSelect = useCallback(
|
||||
(preset: { name: string; plugins: Set<Plugin> | ReadonlySet<Plugin> }) => {
|
||||
recordEvent('feature_used', {
|
||||
feature: 'redteam_config_plugins_preset_selected',
|
||||
preset: preset.name,
|
||||
});
|
||||
if (preset.name === 'Custom') {
|
||||
setIsCustomMode(true);
|
||||
} else {
|
||||
// Use setSelectedPlugins for efficient bulk update
|
||||
setSelectedPlugins(new Set(preset.plugins as Set<Plugin>));
|
||||
setIsCustomMode(false);
|
||||
}
|
||||
},
|
||||
[recordEvent, setSelectedPlugins],
|
||||
);
|
||||
```
|
||||
|
||||
#### 4. Refactor "Select All" Button
|
||||
|
||||
**Replace lines 485-494:**
|
||||
|
||||
```tsx
|
||||
onClick={() => {
|
||||
// Collect all filtered plugins and merge with existing selection
|
||||
const newSelected = new Set(selectedPlugins);
|
||||
filteredPlugins.forEach(({ plugin }) => {
|
||||
newSelected.add(plugin);
|
||||
});
|
||||
setSelectedPlugins(newSelected);
|
||||
}}
|
||||
```
|
||||
|
||||
#### 5. Refactor "Select None" Button
|
||||
|
||||
**Replace lines 499-508:**
|
||||
|
||||
```tsx
|
||||
onClick={() => {
|
||||
// Remove only the filtered plugins from selection
|
||||
const filteredPluginIds = new Set(filteredPlugins.map((p) => p.plugin));
|
||||
const newSelected = new Set(
|
||||
[...selectedPlugins].filter((p) => !filteredPluginIds.has(p)),
|
||||
);
|
||||
setSelectedPlugins(newSelected);
|
||||
}}
|
||||
```
|
||||
|
||||
#### 6. Refactor "Clear All" Button
|
||||
|
||||
**Replace lines 816-820:**
|
||||
|
||||
```tsx
|
||||
onClick={() => {
|
||||
setSelectedPlugins(new Set());
|
||||
}}
|
||||
```
|
||||
|
||||
### Success Criteria:
|
||||
|
||||
#### Automated Verification:
|
||||
|
||||
- [x] TypeScript compilation passes: `npm run tsc` from `src/app`
|
||||
- [x] Linting passes: `npm run lint`
|
||||
- [x] All PluginsTab tests pass: `npm run test:app -- src/pages/redteam/setup/components/PluginsTab.test.tsx`
|
||||
|
||||
**Implementation Note**: After completing this phase and all automated verification passes, proceed to Phase 3.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Verify and Fix Tests
|
||||
|
||||
### Overview
|
||||
|
||||
Run the full test suite and fix any test failures. Tests should mostly pass since they test end-to-end behavior (user interaction → store state), not implementation details.
|
||||
|
||||
### Changes Required:
|
||||
|
||||
#### 1. Run Full Test Suite
|
||||
|
||||
```bash
|
||||
cd src/app
|
||||
npm run test -- src/pages/redteam/setup/components/PluginsTab.test.tsx
|
||||
```
|
||||
|
||||
#### 2. Potential Test Adjustments
|
||||
|
||||
The tests should largely pass as-is because they:
|
||||
|
||||
- Verify store state after interactions (still works)
|
||||
- Use `userEvent` to simulate clicks (still works)
|
||||
- Don't mock the intermediate state management
|
||||
|
||||
However, if any tests reference `onUserInteraction` in expectations or setup, they will need updates.
|
||||
|
||||
**If needed, remove references to `onUserInteraction` in test mocks or assertions.**
|
||||
|
||||
### Success Criteria:
|
||||
|
||||
#### Automated Verification:
|
||||
|
||||
- [x] All 30+ tests in `PluginsTab.test.tsx` pass (44 tests passed)
|
||||
- [x] No TypeScript errors
|
||||
- [x] No linting errors
|
||||
|
||||
#### Manual Verification:
|
||||
|
||||
- [x] Start the dev server: `npm run dev:app`
|
||||
- [x] Navigate to Red Team Setup → Plugins page
|
||||
- [x] Select a preset (e.g., "Recommended") → verify plugins appear selected
|
||||
- [x] Toggle individual plugins → verify selection updates
|
||||
- [x] Use "Select all" → verify all visible plugins are selected
|
||||
- [x] Use "Select none" → verify filtered plugins are deselected
|
||||
- [x] Use "Clear All" in sidebar → verify all plugins are cleared
|
||||
- [x] Select `indirect-prompt-injection`, configure it, then toggle other plugins → verify config is preserved
|
||||
- [x] Refresh page → verify plugin selection persists (Zustand persistence)
|
||||
|
||||
**Implementation Note**: ✅ All automated and manual verification complete. Refactor is complete.
|
||||
|
||||
---
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Unit Tests (Existing)
|
||||
|
||||
The existing test suite in `PluginsTab.test.tsx` covers:
|
||||
|
||||
- Component rendering
|
||||
- Plugin search filtering
|
||||
- Category filtering
|
||||
- Selected plugins list display
|
||||
- Preset selection → store update
|
||||
- Plugin list item toggle → store update
|
||||
- Select All/None → store update
|
||||
- Clear All → store update
|
||||
|
||||
### Key Test Scenarios to Verify
|
||||
|
||||
1. **Preset Selection**: Clicking a preset card results in correct plugins in store
|
||||
2. **Single Plugin Toggle**: Clicking checkbox adds/removes plugin from store
|
||||
3. **Select All**: All filtered plugins added to store
|
||||
4. **Select None**: All filtered plugins removed from store (preserving others)
|
||||
5. **Clear All**: All plugins removed from store
|
||||
6. **Config Preservation**: Plugin configs survive toggle operations
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- Toggling a plugin that requires configuration
|
||||
- Preserving policy/intent plugins during regular plugin operations
|
||||
- Rapid toggling (React batching)
|
||||
|
||||
## Performance Considerations
|
||||
|
||||
### Improvements
|
||||
|
||||
- **Bulk operations** now update the store once instead of N times (N = number of plugins in operation)
|
||||
- **No duplicate renders** from local state → store → local state sync cycle
|
||||
- **Memoized derivation** prevents unnecessary recomputation
|
||||
|
||||
### Potential Concerns
|
||||
|
||||
- None expected; `useMemo` derivation is O(n) where n = number of plugins
|
||||
- Store's `updatePlugins` already has JSON comparison optimization
|
||||
|
||||
## Migration Notes
|
||||
|
||||
- No data migration needed; store format unchanged
|
||||
- Existing persisted configs will work without changes
|
||||
|
||||
## References
|
||||
|
||||
- Research document: `docs/research/2026-01-08-redteam-plugins-state-management.md`
|
||||
- Test file: `src/app/src/pages/redteam/setup/components/PluginsTab.test.tsx`
|
||||
- Zustand store: `src/app/src/pages/redteam/setup/hooks/useRedTeamConfig.ts`
|
||||
@@ -0,0 +1,754 @@
|
||||
# Proposal: A Layered Package System for Promptfoo
|
||||
|
||||
## Executive Summary
|
||||
|
||||
Promptfoo should move from "one published package that happens to contain many
|
||||
systems" to "one familiar full package backed by a small set of explicit package
|
||||
layers."
|
||||
|
||||
The recommendation is:
|
||||
|
||||
1. Keep `promptfoo` as the default full install and compatibility facade.
|
||||
2. Create private workspace packages first, then publish only the packages whose
|
||||
boundaries prove useful.
|
||||
3. Separate the low-dependency evaluation kernel from Node adapters, CLI,
|
||||
server/UI hosting, redteam, and provider families.
|
||||
4. Make dependency ownership visible and test the packed artifacts users install,
|
||||
not only the source tree.
|
||||
5. Preserve dual ESM/CommonJS support at public package boundaries during the
|
||||
transition.
|
||||
|
||||
This gives lightweight consumers a smaller dependency graph without making the
|
||||
normal `npm install promptfoo` experience worse. It also gives the team a safer
|
||||
path to provider packs and future products without forcing a flag day.
|
||||
|
||||
## Why Change
|
||||
|
||||
Today, the published root package owns several quite different responsibilities:
|
||||
|
||||
- Node library entrypoint
|
||||
- CLI
|
||||
- evaluation engine
|
||||
- database and migrations
|
||||
- sharing and persistence
|
||||
- view server
|
||||
- redteam workflows
|
||||
- many provider SDKs
|
||||
- the built web UI
|
||||
|
||||
That makes the root package convenient, but also broad:
|
||||
|
||||
- The root package currently has `83` direct runtime dependencies and `36`
|
||||
optional dependencies.
|
||||
- Before this prototype, the public library entrypoint in `src/index.ts`
|
||||
imported migrations, models, sharing, provider loading, and redteam APIs.
|
||||
The prototype starts separating that shape by moving the public Node API
|
||||
behind `src/node/evaluate.ts`, keeping the internal orchestration in the Node
|
||||
layer at `src/evaluate.ts`, and carving a first leaf-safe contract subset into
|
||||
`src/contracts/**` while `promptfoo` remains the facade.
|
||||
- `src/main.ts` and `src/commands/view.ts` are already outer-shell concerns,
|
||||
not core evaluation concerns.
|
||||
- Provider loading is centralized enough that optional/provider dependencies are
|
||||
still effectively part of the product shape.
|
||||
|
||||
The system has grown past the point where one package boundary expresses the
|
||||
architecture well.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Do not make existing users choose packages on day one.
|
||||
- Do not publish every internal boundary just because it exists.
|
||||
- Do not require a package-manager migration before the architecture improves.
|
||||
- Do not turn providers into runtime-installed plugins as a prerequisite for the
|
||||
split.
|
||||
- Do not combine the package split with an ESM-only migration.
|
||||
|
||||
## Design Principles
|
||||
|
||||
1. **One obvious default.**
|
||||
`promptfoo` remains the package most users install.
|
||||
|
||||
2. **Narrow leaves, convenient facade.**
|
||||
Lightweight consumers should not pay for servers, databases, CLIs, or provider
|
||||
SDKs they do not use.
|
||||
|
||||
3. **No runtime dependency magic.**
|
||||
Dependency installation happens at install/build/publish time, not when an eval
|
||||
starts or a server boots.
|
||||
|
||||
4. **Private boundaries before public promises.**
|
||||
We should first make internal ownership real inside the monorepo, then publish
|
||||
only the packages that survive real use.
|
||||
|
||||
5. **Dual-format correctness over format ideology.**
|
||||
Promptfoo already supports both ESM and CommonJS. Public packages should keep
|
||||
doing that until we intentionally decide otherwise.
|
||||
|
||||
6. **Package artifacts are the contract.**
|
||||
A source-tree green build is not enough; every published package must be packed,
|
||||
installed, imported, required, and exercised as a user would consume it.
|
||||
|
||||
## Recommended Package Topology
|
||||
|
||||
```mermaid
|
||||
flowchart TD
|
||||
schema["@promptfoo/schema"]
|
||||
core["@promptfoo/core"]
|
||||
node["@promptfoo/node"]
|
||||
redteam["@promptfoo/redteam"]
|
||||
providers["@promptfoo/provider-*"]
|
||||
view["@promptfoo/view-server"]
|
||||
cli["@promptfoo/cli"]
|
||||
facade["promptfoo"]
|
||||
|
||||
schema --> core
|
||||
core --> node
|
||||
core --> redteam
|
||||
core --> providers
|
||||
node --> view
|
||||
node --> cli
|
||||
redteam --> cli
|
||||
providers --> facade
|
||||
node --> facade
|
||||
redteam --> facade
|
||||
view --> facade
|
||||
cli --> facade
|
||||
```
|
||||
|
||||
### `@promptfoo/schema`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Config types and runtime schemas
|
||||
- JSON schema generation
|
||||
- Browser-safe shared contracts
|
||||
- Stable serialized result shapes where appropriate
|
||||
|
||||
**May depend on**
|
||||
|
||||
- schema libraries such as `zod`
|
||||
|
||||
**Must not depend on**
|
||||
|
||||
- `fs`, `@libsql/client`, Express, provider SDKs, CLI libraries, server code
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- It is the cleanest shared layer between library, CLI, server, UI, and docs.
|
||||
- It is also the safest first extraction.
|
||||
|
||||
### `@promptfoo/core`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Pure evaluation domain model
|
||||
- Test planning, prompt expansion, assertions, scoring, result aggregation
|
||||
- Provider/assertion interfaces
|
||||
- No direct filesystem, DB, HTTP-server, or provider-SDK assumptions
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/schema`
|
||||
- small domain utilities
|
||||
|
||||
**Must not depend on**
|
||||
|
||||
- persistence
|
||||
- web server
|
||||
- CLI
|
||||
- concrete provider SDK packages
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- This is the actual reusable engine people mean when they say "the Node package."
|
||||
- It should be possible to run it with fake providers and in-memory adapters.
|
||||
|
||||
### `@promptfoo/node`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Node adapters around core
|
||||
- config/file loading
|
||||
- local persistence
|
||||
- migrations
|
||||
- cache
|
||||
- share/report persistence helpers
|
||||
- current `evaluate()`-style Node API
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/core`
|
||||
- `@promptfoo/schema`
|
||||
- Node-only dependencies such as `@libsql/client`, `glob`, `chokidar`, `dotenv`
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- Most library users still want the batteries-included Node API.
|
||||
- This gives them that without forcing the same dependencies onto future
|
||||
browser-safe or embedded consumers.
|
||||
|
||||
### `@promptfoo/redteam`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- redteam generation, strategies, graders, plugins, reporting
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/core`
|
||||
- `@promptfoo/node` only where it truly needs Node adapters
|
||||
- redteam-specific dependencies
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- Redteam is now a substantial product surface with its own cadence,
|
||||
dependencies, docs, and CLI flows.
|
||||
|
||||
### `@promptfoo/view-server`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Express/socket server
|
||||
- API routes
|
||||
- static app serving
|
||||
- local UI hosting
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/node`
|
||||
- server-specific dependencies
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- The server is useful, but it is not intrinsic to every library consumer.
|
||||
|
||||
### `@promptfoo/cli`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- command registration
|
||||
- process lifecycle
|
||||
- update checks
|
||||
- terminal UX
|
||||
- orchestration across `node`, `redteam`, and `view-server`
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/node`
|
||||
- `@promptfoo/redteam`
|
||||
- `@promptfoo/view-server`
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- The CLI is a shell around the system, not the system itself.
|
||||
|
||||
### `@promptfoo/provider-*`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Provider-specific implementations and SDK dependencies
|
||||
- Examples:
|
||||
- `@promptfoo/provider-openai`
|
||||
- `@promptfoo/provider-anthropic`
|
||||
- `@promptfoo/provider-aws`
|
||||
- `@promptfoo/provider-google`
|
||||
- eventually a small `@promptfoo/providers-core` for zero-extra-dependency or
|
||||
very common providers
|
||||
|
||||
**May depend on**
|
||||
|
||||
- `@promptfoo/core`
|
||||
- provider SDKs owned by that package
|
||||
|
||||
**Why it exists**
|
||||
|
||||
- This is where the largest optional dependency savings eventually come from.
|
||||
- It also makes provider ownership and release notes much clearer.
|
||||
|
||||
**Important**
|
||||
|
||||
- Do not start by publishing dozens of provider packages.
|
||||
- First make provider registration explicit and let built-in providers move behind
|
||||
the same registry shape internally.
|
||||
|
||||
### `promptfoo`
|
||||
|
||||
**Purpose**
|
||||
|
||||
- Familiar full package
|
||||
- current CLI binaries
|
||||
- compatibility imports
|
||||
- "everything included" experience
|
||||
|
||||
**Behavior**
|
||||
|
||||
- Depends on the packages that define the full Promptfoo distribution.
|
||||
- Re-exports the stable Node API from `@promptfoo/node`.
|
||||
- Continues to ship the CLI users know.
|
||||
- Becomes the migration shield while the rest of the topology matures.
|
||||
|
||||
## Dependency Policy
|
||||
|
||||
### Ownership Rule
|
||||
|
||||
Every runtime dependency must have one declared owner:
|
||||
|
||||
- core runtime
|
||||
- node runtime
|
||||
- cli runtime
|
||||
- view-server runtime
|
||||
- redteam runtime
|
||||
- provider runtime
|
||||
- docs/app/dev-only
|
||||
|
||||
If a dependency is used by more than one package, we should prefer:
|
||||
|
||||
1. a smaller shared package only when the shared code is real, or
|
||||
2. duplicate declarations when the runtime ownership is legitimately separate.
|
||||
|
||||
Do not keep dependencies at the root merely because multiple packages happen to
|
||||
use them during the transition.
|
||||
|
||||
### Boundary Rule
|
||||
|
||||
- A package may import only from packages below it in the topology.
|
||||
- No package may deep-import another package's `src/**`.
|
||||
- Public imports go through explicit package exports.
|
||||
- UI code depends on schema/browser-safe contracts, not Node internals.
|
||||
|
||||
### Provider Rule
|
||||
|
||||
- Provider SDK dependencies belong to provider packages.
|
||||
- `core` knows only provider interfaces and registration metadata.
|
||||
- `node` may ship a default provider registry, but should not be the owner of every
|
||||
provider SDK forever.
|
||||
|
||||
### Optional Dependency Rule
|
||||
|
||||
- Use `optionalDependencies` only for genuinely optional platform/native
|
||||
capability, not as a substitute for package ownership.
|
||||
- Once a provider family has its own package, its SDK should leave the root
|
||||
optional-dependency bucket.
|
||||
|
||||
## Build and Repository Model
|
||||
|
||||
### Keep npm Workspaces for the First Stage
|
||||
|
||||
The repository already uses npm workspaces and has established CI around them.
|
||||
The split does not require an immediate package-manager migration.
|
||||
|
||||
Recommended first-stage workspace layout:
|
||||
|
||||
```text
|
||||
packages/
|
||||
schema/
|
||||
core/
|
||||
node/
|
||||
redteam/
|
||||
view-server/
|
||||
cli/
|
||||
provider-openai/
|
||||
provider-anthropic/
|
||||
src/app/
|
||||
site/
|
||||
```
|
||||
|
||||
`src/` can migrate inward over time. During the first phase, thin package entry
|
||||
files may point at existing source while we move code by ownership.
|
||||
|
||||
### Build Outputs
|
||||
|
||||
Each publishable package should produce:
|
||||
|
||||
```text
|
||||
dist/
|
||||
esm/
|
||||
cjs/
|
||||
types/
|
||||
```
|
||||
|
||||
and expose only supported entrypoints through `exports`.
|
||||
|
||||
Recommended package export shape:
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "module",
|
||||
"exports": {
|
||||
".": {
|
||||
"types": "./dist/types/index.d.ts",
|
||||
"import": "./dist/esm/index.js",
|
||||
"require": "./dist/cjs/index.cjs"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
For Node-targeted packages, TypeScript should be checked in `nodenext` mode so
|
||||
the compiler models Node's dual-format resolver. Browser/bundler packages can
|
||||
keep bundler-oriented settings where appropriate.
|
||||
|
||||
### Package Boundary Checks
|
||||
|
||||
Add CI checks for:
|
||||
|
||||
- illegal cross-package imports
|
||||
- public-export drift
|
||||
- missing dependency declarations
|
||||
- duplicate accidental dependencies
|
||||
- root dependency ownership
|
||||
- packed-tarball contents
|
||||
- ESM import smoke
|
||||
- CommonJS require smoke
|
||||
- type-resolution smoke
|
||||
|
||||
The strongest useful addition from the OpenClaw study is a generated dependency
|
||||
ownership report:
|
||||
|
||||
```text
|
||||
dependency owner direct users transitive size
|
||||
@libsql/client @promptfoo/node node ...
|
||||
express @promptfoo/view-server view-server ...
|
||||
@anthropic-ai/sdk provider-anthropic provider package ...
|
||||
```
|
||||
|
||||
That report should fail CI when:
|
||||
|
||||
- a dependency has no owner
|
||||
- a package imports a dependency it does not declare
|
||||
- a dependency owned by a leaf is still imported from a higher layer
|
||||
|
||||
### What We Should Borrow From OpenClaw
|
||||
|
||||
The useful lesson from OpenClaw is not "copy its exact repo layout." It is that
|
||||
dependency management gets simpler when ownership is explicit and package
|
||||
artifacts are tested like user-facing products.
|
||||
|
||||
Adopt:
|
||||
|
||||
- private workspaces as the first architectural boundary
|
||||
- leaf-package ownership of runtime dependencies
|
||||
- dependency ownership reports in CI
|
||||
- tarball checks and clean-install acceptance tests
|
||||
- install/update flows that are explicit, never hidden inside startup paths
|
||||
|
||||
Do not adopt blindly:
|
||||
|
||||
- an immediate package-manager migration
|
||||
- runtime-installable provider plugins before Promptfoo needs that product model
|
||||
- ESM-only publishing while Promptfoo still supports both `import` and `require`
|
||||
|
||||
## Publishing Model
|
||||
|
||||
### Recommendation: One Version Line at First
|
||||
|
||||
Use a fixed-version monorepo release for the first public split:
|
||||
|
||||
- all public `@promptfoo/*` packages share the same version
|
||||
- `promptfoo` depends on exact matching versions of internal public packages
|
||||
- release notes can still call out package-specific changes
|
||||
|
||||
This is easier for users, support, and rollback while boundaries are still
|
||||
forming. Independent versions become worth revisiting only after package
|
||||
consumption patterns are stable.
|
||||
|
||||
### Release Automation
|
||||
|
||||
Keep release automation centralized and extend the current flow rather than
|
||||
inventing a second release system immediately:
|
||||
|
||||
1. build all publishable packages
|
||||
2. run package-graph checks
|
||||
3. pack each package
|
||||
4. install packed artifacts into clean temp projects
|
||||
5. run ESM, CJS, CLI, server, and upgrade smokes
|
||||
6. publish with npm trusted publishing / provenance
|
||||
7. publish the `promptfoo` facade last
|
||||
|
||||
Promptfoo already publishes with provenance in the current release workflow, so
|
||||
this should be an extension of the existing release path rather than a parallel
|
||||
system.
|
||||
|
||||
### Artifact Acceptance
|
||||
|
||||
Before publish, every public package should prove:
|
||||
|
||||
- `npm pack` contents are complete
|
||||
- no source-only files are required at runtime
|
||||
- `import` works
|
||||
- `require` works
|
||||
- TypeScript resolves exported types
|
||||
- `promptfoo` facade still exposes current behavior
|
||||
- upgrade from the last published version works for the top-level package
|
||||
|
||||
The current smoke-test philosophy already says "test the built package, not
|
||||
source code." This proposal extends that idea from the root package to every
|
||||
public package.
|
||||
|
||||
## ESM and CommonJS Strategy
|
||||
|
||||
Promptfoo should remain dual-mode during the split.
|
||||
|
||||
### Rules
|
||||
|
||||
- Public packages ship both `import` and `require` conditions until we make a
|
||||
separate deprecation decision.
|
||||
- Source-of-truth implementation may remain ESM-first.
|
||||
- CJS builds are compatibility artifacts, not a second architecture.
|
||||
- No package may rely on unexported internal paths from another package.
|
||||
- Every public package gets both `import` and `require` smoke tests.
|
||||
|
||||
### Why
|
||||
|
||||
Modern Node can load more ESM from CommonJS than older Node versions could, but
|
||||
Promptfoo already promises a `require` entrypoint today. Keeping that promise
|
||||
while the package graph changes avoids combining two migrations into one.
|
||||
|
||||
## Developer Experience
|
||||
|
||||
The system should feel no worse locally than the repo does today.
|
||||
|
||||
### Required Properties
|
||||
|
||||
- one install at the repo root
|
||||
- one normal full build command
|
||||
- package-local test commands when working narrowly
|
||||
- no manual linking
|
||||
- no publishing knowledge required for ordinary feature work
|
||||
- docs/examples keep using `promptfoo` unless a smaller package is the point of
|
||||
the example
|
||||
|
||||
### Useful Commands
|
||||
|
||||
```bash
|
||||
npm run build
|
||||
npm test
|
||||
npm run test:package -- --package @promptfoo/node
|
||||
npm run deps:ownership
|
||||
npm run pack:check
|
||||
```
|
||||
|
||||
### Golden Path
|
||||
|
||||
Most contributors should continue to:
|
||||
|
||||
1. install once
|
||||
2. edit one package or normal source file
|
||||
3. run focused tests
|
||||
4. run the normal repo checks before merging
|
||||
|
||||
Package boundaries should make reasoning easier, not force every engineer to
|
||||
become a release engineer.
|
||||
|
||||
## Documentation System
|
||||
|
||||
### New Docs to Add
|
||||
|
||||
1. `docs/architecture/packages.md`
|
||||
- package graph
|
||||
- dependency rules
|
||||
- when to add a package
|
||||
|
||||
2. `docs/agents/package-development.md`
|
||||
- how to choose the owning package
|
||||
- package-boundary checks
|
||||
- ESM/CJS export rules
|
||||
|
||||
3. `site/docs/usage/packages.md`
|
||||
- "which package should I install?"
|
||||
- full package vs lightweight library vs provider packs
|
||||
|
||||
4. Per-package README template
|
||||
- purpose
|
||||
- install
|
||||
- supported imports
|
||||
- dependency notes
|
||||
- compatibility guarantees
|
||||
|
||||
### Public Message
|
||||
|
||||
For users, the story should be simple:
|
||||
|
||||
- Install `promptfoo` when you want the normal product.
|
||||
- Install `@promptfoo/node` when you want the Node library without the CLI/server.
|
||||
- Install provider packages only when you want explicit fine-grained control.
|
||||
|
||||
## Migration Plan
|
||||
|
||||
### Phase 0: Add Guardrails Without Moving Code
|
||||
|
||||
- add dependency ownership inventory
|
||||
- add package-artifact smoke harness
|
||||
- add package-boundary linting
|
||||
- add architecture docs
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- We can explain which dependency belongs to which future package.
|
||||
|
||||
### Phase 1: Extract the Safest Layers Privately
|
||||
|
||||
- create private workspaces for `schema`, `core`, and `node`
|
||||
- move exports and types behind those boundaries
|
||||
- keep `promptfoo` behavior unchanged
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- Existing CLI and Node consumers pass unchanged through the facade.
|
||||
|
||||
### Phase 2: Split Outer Shells Privately
|
||||
|
||||
- create private `cli` and `view-server`
|
||||
- move server dependencies out of node/core
|
||||
- move command orchestration out of the library layer
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- `@promptfoo/node` can build and test without CLI/server dependencies.
|
||||
|
||||
### Phase 3: Make Provider Registration Explicit
|
||||
|
||||
- define provider registration API
|
||||
- move a small pilot set behind provider packages internally
|
||||
- start with packages that have obvious heavy dependencies or ownership
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- Providers can be loaded through one registry path whether bundled or packaged.
|
||||
|
||||
### Phase 4: Publish the First Useful Subpackages
|
||||
|
||||
Recommended first public packages:
|
||||
|
||||
1. `@promptfoo/schema`
|
||||
2. `@promptfoo/node`
|
||||
3. `@promptfoo/view-server`
|
||||
|
||||
Keep `promptfoo` as the full facade.
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- Real users can consume the smaller packages without undocumented imports or
|
||||
hidden dependencies.
|
||||
|
||||
### Phase 5: Publish Provider Packs Selectively
|
||||
|
||||
Only publish provider packages where there is a clear benefit:
|
||||
|
||||
- large SDK footprint
|
||||
- unusual native/platform dependency
|
||||
- clear ownership
|
||||
- meaningful install-size reduction
|
||||
- independent release pressure
|
||||
|
||||
**Exit criterion**
|
||||
|
||||
- Provider packages reduce the root graph without making provider selection
|
||||
confusing.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
### A. Keep One Package Forever
|
||||
|
||||
**Pros**
|
||||
|
||||
- simplest publishing story
|
||||
- zero package churn
|
||||
|
||||
**Cons**
|
||||
|
||||
- dependency graph keeps growing
|
||||
- library consumers keep paying for unrelated surfaces
|
||||
- harder ownership and release reasoning
|
||||
|
||||
### B. Split Everything Immediately
|
||||
|
||||
**Pros**
|
||||
|
||||
- cleanest theoretical end state
|
||||
|
||||
**Cons**
|
||||
|
||||
- too many public promises at once
|
||||
- high migration risk
|
||||
- poor signal about which boundaries users actually need
|
||||
|
||||
### C. Recommended: Layered Split With a Facade
|
||||
|
||||
**Pros**
|
||||
|
||||
- gives smaller packages to users who need them
|
||||
- keeps today's default UX
|
||||
- makes ownership real before multiplying public APIs
|
||||
- lets us stop after any phase if the benefits flatten out
|
||||
|
||||
**Cons**
|
||||
|
||||
- a transitional period with some duplicated packaging work
|
||||
- requires disciplined export and dependency checks
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
| --------------------------------------------- | ------------------------------------------------------------------------------- |
|
||||
| Internal package churn leaks into public API | Publish only after private boundary use stabilizes |
|
||||
| Dual ESM/CJS builds become inconsistent | Artifact smokes for both import modes on every public package |
|
||||
| Developers slow down in a new monorepo layout | Keep root install/build/test commands as the golden path |
|
||||
| Provider package count becomes confusing | Publish provider packs selectively; keep `promptfoo` full install |
|
||||
| Version skew across packages | Start with fixed versions and exact internal deps |
|
||||
| Release workflow becomes fragile | Reuse current release pipeline, add package-graph and tarball acceptance checks |
|
||||
|
||||
## Decisions Requested
|
||||
|
||||
1. Approve the layered topology as the target direction.
|
||||
2. Approve a private-workspace-first migration rather than immediate public
|
||||
package proliferation.
|
||||
3. Approve `promptfoo` as the long-lived full facade.
|
||||
4. Approve fixed-version public packages for the first release cycle.
|
||||
5. Approve dual ESM/CommonJS support for all first-wave public packages.
|
||||
6. Approve investment in dependency ownership and package-acceptance CI before
|
||||
publishing subpackages.
|
||||
|
||||
## First Concrete Milestone
|
||||
|
||||
The first milestone should be deliberately modest:
|
||||
|
||||
1. Add dependency ownership reporting.
|
||||
2. Create private `packages/schema`, `packages/core`, and `packages/node`.
|
||||
3. Move the public Node API behind `@promptfoo/node` while keeping
|
||||
`promptfoo` exports unchanged.
|
||||
4. Prove that `@promptfoo/node` can build and test without CLI/server imports.
|
||||
5. Add packed-artifact smoke tests for the root package and the private node
|
||||
package.
|
||||
|
||||
If that milestone is not useful, we learn cheaply. If it is useful, the rest of
|
||||
the package system has a clear path.
|
||||
|
||||
## Appendix: Current-Code Signals
|
||||
|
||||
- Before this prototype, `src/index.ts` mixed the Node API with migrations,
|
||||
sharing, provider loading, and redteam exports. The prototype moves the Node
|
||||
orchestration into `src/node/evaluate.ts` as the first concrete seam.
|
||||
- `src/main.ts` is already a CLI shell around lower-level functionality.
|
||||
- `src/commands/view.ts` and `src/server/**` are natural `view-server` owners.
|
||||
- `src/providers/**` is already a natural future provider-package boundary.
|
||||
- `src/types/index.ts` is already being deconstructed, which points toward
|
||||
`schema` as a first extraction.
|
||||
|
||||
## Appendix: 2026 Packaging Notes
|
||||
|
||||
- Use explicit `exports` maps for public packages.
|
||||
- For Node-targeted packages, type-check in `nodenext` mode so TypeScript models
|
||||
the same `import`/`require` behavior Node uses.
|
||||
- Keep npm trusted publishing / provenance in the release path for every public
|
||||
package.
|
||||
- Treat `npm pack` plus clean-install smoke tests as the release contract, not an
|
||||
optional extra.
|
||||
|
||||
## References
|
||||
|
||||
- [Node.js package exports documentation](https://nodejs.org/api/packages.html)
|
||||
- [TypeScript module-system reference](https://www.typescriptlang.org/docs/handbook/modules/reference)
|
||||
- [npm trusted publishing documentation](https://docs.npmjs.com/trusted-publishers)
|
||||
- [npm provenance documentation](https://docs.npmjs.com/generating-provenance-statements)
|
||||
- [npm workspaces documentation](https://docs.npmjs.com/cli/v8/using-npm/workspaces/)
|
||||
@@ -0,0 +1,125 @@
|
||||
# Plan: `promptfoo eval -c <uuid>` Cloud Config Support
|
||||
|
||||
## Context
|
||||
|
||||
Currently, `promptfoo redteam run -c <uuid>` supports loading configs from Promptfoo Cloud by UUID, but `promptfoo eval -c` only accepts local file paths. This feature extends the same cloud config loading pattern to the eval command, allowing users to run `promptfoo eval -c <cloud-uuid>` to fetch and execute a config stored in Promptfoo Cloud.
|
||||
|
||||
The ticket (ENG-1770) has two parts:
|
||||
|
||||
1. **Open Source (this PR)**: Make the CLI accept a cloud UUID for `eval -c`
|
||||
2. **Cloud**: Show the `promptfoo eval -c <uuid>` command in the Cloud run modal (separate repo)
|
||||
|
||||
## Changes
|
||||
|
||||
### 1. Add `getEvalConfigFromCloud()` to `src/util/cloud.ts`
|
||||
|
||||
Create a new function modeled after the existing `getConfigFromCloud()` (line 88-114) but hitting a different endpoint for eval configs:
|
||||
|
||||
```typescript
|
||||
export async function getEvalConfigFromCloud(id: string): Promise<UnifiedConfig> {
|
||||
// Same pattern as getConfigFromCloud but using `configs/${id}` endpoint
|
||||
}
|
||||
```
|
||||
|
||||
- Endpoint: `GET /api/v1/configs/${id}` (eval configs, not redteam-specific)
|
||||
- Reuse existing `makeRequest()` helper (line 25) and `cloudConfig.isEnabled()` check pattern
|
||||
- Same error handling pattern as `getConfigFromCloud`
|
||||
|
||||
### 2. Add UUID detection to `src/commands/eval.ts`
|
||||
|
||||
In `doEval()` (line 107), add UUID detection **before** the existing config path processing at line 142. This mirrors the pattern in `src/redteam/commands/run.ts:62-79`.
|
||||
|
||||
```typescript
|
||||
// Before the existing config path processing (line 142)
|
||||
const UUID_REGEX = /^[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{12}$/;
|
||||
|
||||
if (cmdObj.config?.length === 1 && UUID_REGEX.test(cmdObj.config[0])) {
|
||||
const cloudConfigObj = await getEvalConfigFromCloud(cmdObj.config[0]);
|
||||
defaultConfig = cloudConfigObj;
|
||||
cmdObj.config = undefined;
|
||||
}
|
||||
```
|
||||
|
||||
Key behaviors:
|
||||
|
||||
- Only trigger when exactly one config path is provided and it's a UUID
|
||||
- Fetch the config from cloud and use it as `defaultConfig`
|
||||
- Clear `cmdObj.config` so `resolveConfigs` uses `defaultConfig` instead of trying to read a file
|
||||
- Import `getEvalConfigFromCloud` from `../../util/cloud`
|
||||
|
||||
### 3. Update eval command description
|
||||
|
||||
Update the `-c` option description at line 903-906 to mention cloud UUID support:
|
||||
|
||||
```typescript
|
||||
.option(
|
||||
'-c, --config <paths...>',
|
||||
'Path to configuration file or cloud config UUID. Automatically loads promptfooconfig.yaml',
|
||||
)
|
||||
```
|
||||
|
||||
### 4. Add tests
|
||||
|
||||
Create test cases in a new test file or add to existing eval command tests, following the pattern from `test/redteam/commands/run.test.ts`:
|
||||
|
||||
- Test UUID detection triggers cloud fetch
|
||||
- Test local file paths bypass UUID detection
|
||||
- Test error when cloud is not enabled
|
||||
- Test multiple config paths with UUID (should not trigger UUID detection)
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Change |
|
||||
| --------------------------------------------- | ----------------------------------------------------------------------- |
|
||||
| `src/util/cloud.ts` | Add `getEvalConfigFromCloud()` function |
|
||||
| `src/commands/eval.ts` | Add UUID detection + cloud fetch in `doEval()`, update `-c` description |
|
||||
| `test/commands/eval.test.ts` or new test file | Add tests for UUID cloud config |
|
||||
|
||||
## Existing Code to Reuse
|
||||
|
||||
- `makeRequest()` from `src/util/cloud.ts:25` - authenticated HTTP helper
|
||||
- `cloudConfig.isEnabled()` from `src/globalConfig/cloud.ts` - auth check
|
||||
- UUID regex pattern from `src/redteam/commands/run.ts:17`
|
||||
- `resolveConfigs()` from `src/util/config/load.ts:481` - existing config resolution (no changes needed)
|
||||
|
||||
## Verification
|
||||
|
||||
1. **Build**: `npm run build` should succeed
|
||||
2. **Lint**: `npm run l && npm run f`
|
||||
3. **Unit tests**: Run existing + new tests with `npx vitest src/commands/eval` and `npx vitest src/util/cloud`
|
||||
4. **Manual test** (if cloud access available):
|
||||
- `npm run local -- eval -c <valid-uuid> --env-file .env` should fetch config from cloud
|
||||
- `npm run local -- eval -c path/to/config.yaml` should still work as before
|
||||
- `npm run local -- eval -c <invalid-uuid-format>` should fall through to file resolution
|
||||
|
||||
## Decisions (Reconciled)
|
||||
|
||||
1. Use the OSS endpoint contract: `GET /api/v1/configs/:id` returning an envelope (`{ config: ... }`), not a raw payload.
|
||||
2. Support persisted config shape with `providers` and `tests`; loading must not require additional requests.
|
||||
3. Provider references in the config should use `promptfoo://provider/<uuid>`.
|
||||
4. Prompts should be emitted as plain strings.
|
||||
5. `tests` defaults to `[]` when missing.
|
||||
6. `description` falls back to `config.name` when missing.
|
||||
7. `--watch` is not supported when `-c <uuid>` is used. CLI should fail fast with a clear error.
|
||||
8. If `-c <value>` matches UUID format but cloud fetch fails (404/auth disabled), hard-fail.
|
||||
9. If multiple `-c` values are supplied and any value is a UUID, fail with an explicit error stating only one `-c` value is allowed for cloud UUID mode.
|
||||
10. Add tests in both places:
|
||||
- `test/commands/eval.test.ts` for UUID detection/CLI behavior
|
||||
- `test/util/cloud.test.ts` for `getEvalConfigFromCloud()` contract/error handling
|
||||
11. Scope is `eval` only (not `redteam eval`).
|
||||
12. No temporary UI note or minimum-version gating is required.
|
||||
13. In UUID mode, clear/ignore `defaultConfigPath` for the run to prevent accidental local reload/fallback behavior.
|
||||
14. Read-time schema normalization should normalize legacy fields (`providerIds`/`testCases`) into canonical fields (`providers`/`tests`).
|
||||
|
||||
### CLI Error Messages (exact draft text)
|
||||
|
||||
1. Multiple `-c` values with UUID mode:
|
||||
- `Cloud config UUID mode supports exactly one -c value. Use: promptfoo eval -c <cloud-config-uuid>`
|
||||
2. UUID mode with `--watch`:
|
||||
- `--watch is not supported when using a cloud config UUID with -c. Use a local config file path for watch mode.`
|
||||
3. UUID-shaped value with failed cloud fetch:
|
||||
- `Failed to load cloud eval config "<uuid>". <reason>. Cloud UUID inputs do not fall back to local file paths. Check authentication and that the UUID exists.`
|
||||
|
||||
## Remaining Follow-ups
|
||||
|
||||
None.
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user