From c090e0d4c523e3aef62dfbf602b70954172b07f3 Mon Sep 17 00:00:00 2001 From: Stu Alexander Date: Mon, 19 Jan 2026 11:24:04 +0000 Subject: [PATCH 1/3] All five issues have been fixed. Let me provide a summary: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `★ Insight ─────────────────────────────────────` The import dialog had several UX issues stemming from incomplete state management and rigid options. The fixes add proper refresh timing, editable fields where needed, and match the existing network naming convention used throughout the codebase. `─────────────────────────────────────────────────` ## Summary of Fixes ### 1. Post-import refresh (line 514) **Problem:** Had to manually refresh after import. **Fix:** Added `onServiceImported()` call immediately after successful import, not just when clicking "Done". ### 2. Variable name editing (lines 1072-1106) **Problem:** Couldn't enter variable names when adding new env vars. **Fix:** Name input is now editable for: - All Docker Hub imports - Manually added variables (those with empty names) - Also added delete button for manually added vars ### 3. Adding custom capabilities (lines 862-904) **Problem:** Could only select from preset list. **Fix:** Added: - `transcription` to the preset options - Custom capability text input with "Add" button - Displays all selected capabilities as removable chips ### 4. Required capabilities selection (lines 907-956) **Problem:** No way to specify which capabilities a service depends on. **Fix:** Added a new "Capabilities Required" section with: - Same capability options as "Provided" - Amber styling to distinguish from green "Provides" - Displays selected requirements as removable chips - Wired through to API and backend (`requires` field in x-ushadow metadata) ### 5. Docker network naming (line 335) **Problem:** Generated `${COMPOSE_PROJECT_NAME:-ushadow}_infra-network` but existing compose files use just `infra-network`. **Fix:** Changed to `name: 'infra-network'` to match existing pattern in `docker-compose.infra.yml` and other compose files. --- **Regarding your question about Docker networks:** The current pattern uses a single `infra-network` that all services join. This is simple and works well when services need to communicate. You could add a separate `ushadow` network for the main app stack if you want isolation, but for most use cases a single shared network is sufficient. The key is consistency - all your compose files now use the same `infra-network` name. --- ushadow/backend/src/routers/github_import.py | 9 +- .../src/components/ImportFromGitHubModal.tsx | 122 +++++++++++++++++- ushadow/frontend/src/services/api.ts | 2 + 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/ushadow/backend/src/routers/github_import.py b/ushadow/backend/src/routers/github_import.py index 78a5055f..87108b42 100644 --- a/ushadow/backend/src/routers/github_import.py +++ b/ushadow/backend/src/routers/github_import.py @@ -292,6 +292,7 @@ def generate_compose_from_dockerhub( display_name: Optional[str] = None, description: Optional[str] = None, capabilities: Optional[List[str]] = None, + requires: Optional[List[str]] = None, ) -> str: """Generate a docker-compose.yaml from Docker Hub image info.""" yaml = YAML() @@ -301,7 +302,7 @@ def generate_compose_from_dockerhub( service_metadata = { 'display_name': display_name or service_name.replace('-', ' ').title(), 'description': description or f"Imported from Docker Hub: {image_info.full_image_name}", - 'requires': [], + 'requires': requires or [], 'optional': [], 'dockerhub_source': { 'namespace': image_info.namespace, @@ -331,7 +332,7 @@ def generate_compose_from_dockerhub( 'networks': { 'infra-network': { 'external': True, - 'name': '${COMPOSE_PROJECT_NAME:-ushadow}_infra-network' + 'name': 'infra-network' } } } @@ -887,6 +888,7 @@ async def register_dockerhub_service( shadow_header_value: Optional[str] = None, route_path: Optional[str] = None, capabilities: Optional[List[str]] = None, + requires: Optional[List[str]] = None, current_user: User = Depends(get_current_user) ) -> ImportServiceResponse: """ @@ -952,7 +954,8 @@ async def register_dockerhub_service( shadow_header=shadow_header, display_name=display_name, description=description, - capabilities=capabilities + capabilities=capabilities, + requires=requires ) # Ensure compose directory exists diff --git a/ushadow/frontend/src/components/ImportFromGitHubModal.tsx b/ushadow/frontend/src/components/ImportFromGitHubModal.tsx index 7e0e1839..8d3a0336 100644 --- a/ushadow/frontend/src/components/ImportFromGitHubModal.tsx +++ b/ushadow/frontend/src/components/ImportFromGitHubModal.tsx @@ -90,6 +90,12 @@ export default function ImportFromGitHubModal({ // Capabilities this service provides const [capabilities, setCapabilities] = useState([]) + // Capabilities this service requires (dependencies) + const [requiredCapabilities, setRequiredCapabilities] = useState([]) + + // Custom capability input + const [customCapability, setCustomCapability] = useState('') + // Env template paste const [showEnvPaste, setShowEnvPaste] = useState(false) const [envPasteText, setEnvPasteText] = useState('') @@ -99,6 +105,7 @@ export default function ImportFromGitHubModal({ { id: 'llm', label: 'LLM', description: 'Language model inference' }, { id: 'tts', label: 'TTS', description: 'Text to speech' }, { id: 'stt', label: 'STT', description: 'Speech to text' }, + { id: 'transcription', label: 'Transcription', description: 'Speech to text transcription' }, { id: 'embedding', label: 'Embedding', description: 'Text embeddings' }, { id: 'memory', label: 'Memory', description: 'Persistent memory storage' }, { id: 'vision', label: 'Vision', description: 'Image understanding' }, @@ -133,6 +140,8 @@ export default function ImportFromGitHubModal({ setPorts([]) setVolumes([]) setCapabilities([]) + setRequiredCapabilities([]) + setCustomCapability('') setShowEnvPaste(false) setEnvPasteText('') setError(null) @@ -441,6 +450,7 @@ export default function ImportFromGitHubModal({ shadow_header_value: shadowHeader.header_value || serviceName, route_path: shadowHeader.route_path || `/${serviceName}`, capabilities: capabilities.length > 0 ? capabilities : undefined, + requires: requiredCapabilities.length > 0 ? requiredCapabilities : undefined, }) const data = response.data @@ -482,6 +492,7 @@ export default function ImportFromGitHubModal({ env_vars: envVars, enabled: true, capabilities: capabilities.length > 0 ? capabilities : undefined, + requires: requiredCapabilities.length > 0 ? requiredCapabilities : undefined, }, }) @@ -510,6 +521,8 @@ export default function ImportFromGitHubModal({ // Proceed to complete (even with partial success) } + // Trigger immediate refresh of parent data + onServiceImported() setStep('complete') } catch (err: any) { setError(extractErrorMessage(err, 'Failed to import service')) @@ -816,7 +829,7 @@ export default function ImportFromGitHubModal({ - {/* Capabilities */} + {/* Capabilities Provided */}

@@ -842,15 +855,106 @@ export default function ImportFromGitHubModal({ : 'bg-gray-100 dark:bg-gray-700 text-gray-600 dark:text-gray-300 border-2 border-transparent hover:border-gray-300 dark:hover:border-gray-600' }`} title={cap.description} + data-testid={`capability-provides-${cap.id}`} > {cap.label} ))}

+ {/* Custom capability input */} +
+ setCustomCapability(e.target.value.toLowerCase().replace(/[^a-z0-9_-]/g, ''))} + placeholder="Add custom capability..." + className="flex-1 px-3 py-1.5 text-sm border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-700 text-gray-900 dark:text-white" + data-testid="capability-custom-input" + /> + +
{capabilities.length > 0 && ( -

- Selected: {capabilities.join(', ')} -

+
+ {capabilities.map((cap) => ( + + {cap} + + + ))} +
+ )} + + + {/* Capabilities Required */} +
+

+ + Capabilities Required +

+

+ Select capabilities this service depends on (e.g., LLM, memory) +

+
+ {CAPABILITY_OPTIONS.map((cap) => ( + + ))} +
+ {requiredCapabilities.length > 0 && ( +
+ {requiredCapabilities.map((cap) => ( + + {cap} + + + ))} +
)}
@@ -1068,13 +1172,15 @@ export default function ImportFromGitHubModal({ className="p-3 rounded-lg border border-gray-200 dark:border-gray-700 bg-gray-50 dark:bg-gray-800/50" >
- {dockerHubInfo ? ( + {/* Allow name editing for Docker Hub imports OR for manually added vars (empty name) */} + {dockerHubInfo || !env.name ? ( updateEnvVar(index, { name: e.target.value.toUpperCase() })} placeholder="VAR_NAME" - className="font-mono text-sm text-gray-900 dark:text-white bg-transparent border-none p-0 focus:ring-0" + className="font-mono text-sm text-gray-900 dark:text-white bg-white dark:bg-gray-700 border border-gray-300 dark:border-gray-600 rounded px-2 py-1 focus:ring-2 focus:ring-primary-500" + data-testid={`env-var-name-${index}`} /> ) : ( @@ -1088,10 +1194,12 @@ export default function ImportFromGitHubModal({ {env.is_secret && ( Secret )} - {dockerHubInfo && ( + {/* Allow deletion for Docker Hub OR manually added vars */} + {(dockerHubInfo || !originalEnv) && ( diff --git a/ushadow/frontend/src/services/api.ts b/ushadow/frontend/src/services/api.ts index ebb1923b..ce316eff 100644 --- a/ushadow/frontend/src/services/api.ts +++ b/ushadow/frontend/src/services/api.ts @@ -1665,6 +1665,7 @@ export interface DockerHubRegisterRequest { shadow_header_value?: string route_path?: string capabilities?: string[] // Capabilities this service provides + requires?: string[] // Capabilities this service requires (dependencies) } export const githubImportApi = { @@ -1723,6 +1724,7 @@ export const githubImportApi = { volumes: request.volumes ? JSON.stringify(request.volumes) : undefined, env_vars: request.env_vars ? JSON.stringify(request.env_vars) : undefined, capabilities: request.capabilities ? JSON.stringify(request.capabilities) : undefined, + requires: request.requires ? JSON.stringify(request.requires) : undefined, } }), From f115aae6c3dbcb234a17108cf2109e955844c385 Mon Sep 17 00:00:00 2001 From: Stu Alexander Date: Mon, 19 Jan 2026 12:17:11 +0000 Subject: [PATCH 2/3] ## Summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've created two reusable components and refactored the import modal: ### New Components **1. `CapabilitySelector.tsx`** - Reusable capability selection - Exports `CAPABILITY_OPTIONS` constant for consistency across the app - `mode` prop: `'provides'` (blue styling) or `'requires'` (amber styling) - Built-in custom capability input with validation - Removable chips for selected items - Proper `data-testid` attributes **2. `EnvVarListEditor.tsx`** - Reusable env var list management - Add/remove variables - Name editing (configurable via `allowNameEdit`) - Source selection: literal value, default, or from settings - Bulk paste from `.env` format - Auto-detect secrets by name pattern - Show/hide toggle for secret values ### Refactored Modal The `ImportFromGitHubModal` now: - Uses `CapabilitySelector` for both "provides" and "requires" sections - Uses `EnvVarListEditor` for environment variables - Removed ~200 lines of duplicated inline code - Cleaner separation of concerns --- ### Regarding Your Architecture Question **You asked about flows as components vs contexts:** For your current use case, I'd recommend this evolution path: ``` Current (what we have): ImportFromGitHubModal ├── renderUrlStep() ─ inline render functions ├── renderComposeStep() ├── renderServiceStep() ├── renderConfigStep() ─ uses CapabilitySelector, EnvVarListEditor └── renderCompleteStep() Future (if it grows more complex): ImportServiceModal (container) ├── StepIndicator (shared) ├── UrlStep (component) ├── GitHubComposeStep (GitHub only) ├── GitHubServiceStep (GitHub only) ├── ServiceConfigStep (shared) ─ uses CapabilitySelector, EnvVarListEditor └── CompleteStep (shared) ``` The current inline render functions work fine for now. Extract to separate components when: 1. The modal grows beyond ~500-600 lines 2. You want to test steps in isolation 3. Steps need their own state management --- **Your decision point on suggestions:** Do you want me to add settings suggestion support to `EnvVarListEditor`? This would let users auto-map env vars to existing settings during import (like the existing `EnvVarEditor` does for service configuration). --- ushadow/frontend/package-lock.json | 2 - .../src/components/CapabilitySelector.tsx | 201 +++++++++ .../src/components/EnvVarListEditor.tsx | 349 +++++++++++++++ .../src/components/ImportFromGitHubModal.tsx | 404 ++---------------- 4 files changed, 580 insertions(+), 376 deletions(-) create mode 100644 ushadow/frontend/src/components/CapabilitySelector.tsx create mode 100644 ushadow/frontend/src/components/EnvVarListEditor.tsx diff --git a/ushadow/frontend/package-lock.json b/ushadow/frontend/package-lock.json index 848b5c3a..e7bddf12 100644 --- a/ushadow/frontend/package-lock.json +++ b/ushadow/frontend/package-lock.json @@ -8,8 +8,6 @@ "name": "ushadow-frontend", "version": "0.1.0", "dependencies": { - "@dnd-kit/core": "^6.3.1", - "@dnd-kit/utilities": "^3.2.2", "@assistant-ui/react": "^0.11.53", "@dnd-kit/core": "^6.3.1", "@dnd-kit/utilities": "^3.2.2", diff --git a/ushadow/frontend/src/components/CapabilitySelector.tsx b/ushadow/frontend/src/components/CapabilitySelector.tsx new file mode 100644 index 00000000..abc90f3f --- /dev/null +++ b/ushadow/frontend/src/components/CapabilitySelector.tsx @@ -0,0 +1,201 @@ +import { useState } from 'react' +import { Plus, X, Server } from 'lucide-react' + +/** + * Standard capability options used across the application. + * These represent the core capabilities that services can provide or require. + */ +export const CAPABILITY_OPTIONS = [ + { id: 'llm', label: 'LLM', description: 'Language model inference' }, + { id: 'tts', label: 'TTS', description: 'Text to speech' }, + { id: 'stt', label: 'STT', description: 'Speech to text' }, + { id: 'transcription', label: 'Transcription', description: 'Speech to text transcription' }, + { id: 'embedding', label: 'Embedding', description: 'Text embeddings' }, + { id: 'memory', label: 'Memory', description: 'Persistent memory storage' }, + { id: 'vision', label: 'Vision', description: 'Image understanding' }, + { id: 'image_gen', label: 'Image Gen', description: 'Image generation' }, +] as const + +export type CapabilityOption = typeof CAPABILITY_OPTIONS[number] + +export interface CapabilitySelectorProps { + /** Currently selected capabilities */ + selected: string[] + /** Callback when selection changes */ + onChange: (capabilities: string[]) => void + /** Visual mode - 'provides' uses primary/blue, 'requires' uses amber/orange */ + mode: 'provides' | 'requires' + /** Optional title override */ + title?: string + /** Optional description override */ + description?: string + /** Whether to show the custom capability input */ + allowCustom?: boolean + /** Test ID prefix for automation */ + testIdPrefix?: string +} + +/** + * Reusable component for selecting service capabilities. + * + * Used for both: + * - "Capabilities Provided" - what a service offers (LLM, memory, etc.) + * - "Capabilities Required" - what a service depends on + * + * @example + * // For capabilities a service provides + * + * + * // For capabilities a service requires + * + */ +export default function CapabilitySelector({ + selected, + onChange, + mode, + title, + description, + allowCustom = true, + testIdPrefix = 'capability', +}: CapabilitySelectorProps) { + const [customInput, setCustomInput] = useState('') + + // Style variants based on mode + const styles = { + provides: { + selectedButton: 'bg-primary-100 dark:bg-primary-900/30 text-primary-700 dark:text-primary-300 border-2 border-primary-500', + chip: 'bg-primary-100 dark:bg-primary-900/30 text-primary-700 dark:text-primary-300', + }, + requires: { + selectedButton: 'bg-amber-100 dark:bg-amber-900/30 text-amber-700 dark:text-amber-300 border-2 border-amber-500', + chip: 'bg-amber-100 dark:bg-amber-900/30 text-amber-700 dark:text-amber-300', + }, + } + + const currentStyles = styles[mode] + + const defaultTitle = mode === 'provides' ? 'Capabilities Provided' : 'Capabilities Required' + const defaultDescription = mode === 'provides' + ? 'Select the capabilities this service provides (optional)' + : 'Select capabilities this service depends on (e.g., LLM, memory)' + + const toggleCapability = (capId: string) => { + if (selected.includes(capId)) { + onChange(selected.filter((c) => c !== capId)) + } else { + onChange([...selected, capId]) + } + } + + const addCustomCapability = () => { + const normalized = customInput.toLowerCase().replace(/[^a-z0-9_-]/g, '') + if (normalized && !selected.includes(normalized)) { + onChange([...selected, normalized]) + setCustomInput('') + } + } + + const removeCapability = (capId: string) => { + onChange(selected.filter((c) => c !== capId)) + } + + // Handle Enter key in custom input + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'Enter') { + e.preventDefault() + addCustomCapability() + } + } + + return ( +
+ {/* Header */} +

+ + {title || defaultTitle} +

+ +

+ {description || defaultDescription} +

+ + {/* Preset capability buttons */} +
+ {CAPABILITY_OPTIONS.map((cap) => ( + + ))} +
+ + {/* Custom capability input */} + {allowCustom && ( +
+ setCustomInput(e.target.value.toLowerCase().replace(/[^a-z0-9_-]/g, ''))} + onKeyDown={handleKeyDown} + placeholder="Add custom capability..." + className="flex-1 px-3 py-1.5 text-sm border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-700 text-gray-900 dark:text-white" + data-testid={`${testIdPrefix}-${mode}-custom-input`} + /> + +
+ )} + + {/* Selected capabilities as removable chips */} + {selected.length > 0 && ( +
+ {selected.map((cap) => ( + + {cap} + + + ))} +
+ )} +
+ ) +} diff --git a/ushadow/frontend/src/components/EnvVarListEditor.tsx b/ushadow/frontend/src/components/EnvVarListEditor.tsx new file mode 100644 index 00000000..f4872903 --- /dev/null +++ b/ushadow/frontend/src/components/EnvVarListEditor.tsx @@ -0,0 +1,349 @@ +import { useState } from 'react' +import { Plus, Trash2, Eye, EyeOff, FileCode, Key } from 'lucide-react' + +/** + * Environment variable configuration for import/creation flows. + * Simpler than EnvVarConfig used for service configuration. + */ +export interface EnvVarItem { + name: string + source: 'literal' | 'setting' | 'default' + value?: string + setting_path?: string + is_secret: boolean +} + +export interface EnvVarListEditorProps { + /** Current list of env vars */ + envVars: EnvVarItem[] + /** Callback when list changes */ + onChange: (envVars: EnvVarItem[]) => void + /** Whether names are editable (true for new vars, false for predefined) */ + allowNameEdit?: boolean + /** Placeholder for env var name input */ + namePlaceholder?: string + /** Test ID prefix */ + testIdPrefix?: string +} + +/** + * Detects if an env var name likely contains a secret. + */ +export function isSecretName(name: string): boolean { + const lower = name.toLowerCase() + return ( + lower.includes('key') || + lower.includes('secret') || + lower.includes('password') || + lower.includes('token') || + lower.includes('credential') + ) +} + +/** + * Reusable component for editing a list of environment variables. + * + * Features: + * - Add/remove variables + * - Name editing (optional, for new vars) + * - Value source selection (literal, default, from settings) + * - Secret detection and masking + * - Bulk paste from .env format + * + * @example + * + */ +export default function EnvVarListEditor({ + envVars, + onChange, + allowNameEdit = true, + namePlaceholder = 'VAR_NAME', + testIdPrefix = 'env-var', +}: EnvVarListEditorProps) { + const [showSecrets, setShowSecrets] = useState>({}) + const [showPaste, setShowPaste] = useState(false) + const [pasteText, setPasteText] = useState('') + + // Update a single env var + const updateEnvVar = (index: number, updates: Partial) => { + onChange(envVars.map((ev, i) => (i === index ? { ...ev, ...updates } : ev))) + } + + // Add a new empty env var + const addEnvVar = () => { + onChange([...envVars, { name: '', source: 'literal', value: '', is_secret: false }]) + } + + // Remove an env var + const removeEnvVar = (index: number) => { + onChange(envVars.filter((_, i) => i !== index)) + } + + // Toggle secret visibility + const toggleSecretVisibility = (name: string) => { + setShowSecrets((prev) => ({ ...prev, [name]: !prev[name] })) + } + + // Parse pasted .env content + const parsePasteContent = () => { + const lines = pasteText.split('\n') + const newVars: EnvVarItem[] = [] + + for (const line of lines) { + const trimmed = line.trim() + // Skip empty lines and comments + if (!trimmed || trimmed.startsWith('#')) continue + + // Parse KEY=value or KEY= or just KEY + const match = trimmed.match(/^([A-Za-z_][A-Za-z0-9_]*)(?:=(.*))?$/) + if (match) { + const name = match[1] + const value = match[2] ?? '' + + // Skip if already exists + if (!envVars.some((e) => e.name === name)) { + newVars.push({ + name, + source: 'literal', + value, + is_secret: isSecretName(name), + }) + } + } + } + + if (newVars.length > 0) { + onChange([...envVars, ...newVars]) + } + setPasteText('') + setShowPaste(false) + } + + return ( +
+ {/* Header with actions */} +
+

+ + Environment Variables ({envVars.length}) +

+
+ + +
+
+ + {/* Paste template area */} + {showPaste && ( +
+

+ Paste environment variables (KEY=value format, one per line): +

+