Fix agent settings regression (vibe-kanban) (#766)

* Revert "Perfect! I've successfully implemented the fix for the agent settings flickering issue. Here's a summary of what was accomplished: (#696)"

This reverts commit 1fd5a9501b.

* ##  **Regression Fix Complete!**

**Summary of changes made:**

### 🎯 **Performance Issues Fixed:**
1. **Eliminated infinite useEffect loop** - Removed `serverParsedProfiles` from dependencies and moved JSON parsing inside the effect
2. **Removed problematic cleanup effect** - Deleted the `useEffect(() => { return () => { reloadSystem(); }; }, []);` that was causing 201+ requests per second
3. **Stabilized React dependencies** - Only primitive values (`serverProfilesContent`, `isDirty`) in dependency arrays

### 🔧 **Functionality Preserved:**
1. **Individual save buttons work** - `handleExecutorConfigSave` function retained with `onSave` prop
2. **Proper system reloading** - Added `reloadSystem()` calls after successful saves (where they belong)
3. **All existing features intact** - Form editor, JSON editor, configuration management all work as before

### 📊 **Expected Impact:**
- **Before**: 201+ requests/second, flickering UI, poor performance, broken individual saves
- **After**: 1 clean request, smooth loading, working individual saves, proper functionality

###  **Quality Checks:**
- **Rust linting**:  Passed (cargo clippy)
- **Code formatting**:  Applied (prettier)
- **Frontend linting**:  0 errors, 99 warnings (acceptable)

The agent settings page should now perform optimally with no flickering or infinite request loops, while preserving all the button functionality that was broken in the original PR #696.

* ##  **Duplicate Save Buttons Fix Complete!**

**Summary of what was accomplished:**

### 🔧 **Fix Applied:**
- **Conditionally hide global save button** in form editor mode
- Added `{!useFormEditor &&` wrapper around sticky save bar
- Updated comment to clarify "Save button for JSON editor mode only"

### 📊 **Expected Behavior:**
- **Form Editor Mode**: Only individual "Save Configuration" buttons (one per config form)
- **JSON Editor Mode**: Only global "Save Agent Configurations" button
- **Clear UX**: No more confusion about which save button to use

###  **Quality Checks Passed:**
- **Rust linting**:  Passed (cargo clippy)
- **Code formatting**:  Applied (prettier - all files unchanged, properly formatted)
- **Frontend linting**:  0 errors, 99 warnings (acceptable per your requirements)

### 🎯 **User Experience Improvement:**
- **Eliminated confusion** - users now see only one save method per editor mode
- **Intuitive workflow** - form editing uses granular saves, JSON editing uses bulk saves
- **Preserved functionality** - all save capabilities retained, just organized better
- **Matches original intent** - comment already indicated this should be JSON editor only

The agent settings page now has clean, contextual save functionality based on which editor mode you're using!

* ##  **Critical User Feedback Issues Fixed!**

**Summary of accomplishments:**

### 🔧 **Issue 1: Missing reloadSystem() Regression (FIXED)**
- ** Added missing `reloadSystem()` call** in `handleDeleteConfiguration()` function (line 225)
- ** Restored global system consistency** - deleted configurations now immediately disappear from all screens
- ** Matches other save functions** - consistent behavior across all save operations

### 🔧 **Issue 2: Missing Error Messages (FIXED)**
- ** Added error state management**: `const [profilesError, setProfilesError] = useState<string | null>(null);`
- ** Added red error alert UI** component (lines 375-379) that matches the green success alert style
- ** Updated all save functions** to show user-friendly error messages:
  - `handleSaveProfiles()`: "Failed to save agent configurations. Please try again."
  - `handleExecutorConfigSave()`: "Failed to save configuration. Please try again."
  - `handleDeleteConfiguration()`: "Failed to delete configuration. Please try again."
- ** Clear errors before new attempts** - `setProfilesError(null)` in all save functions

### 📊 **User Experience Improvements**

**Before:**
-  Delete configs still showed in other screens until random reload
-  Save failures showed no user feedback (only console.error)
-  Users confused when saves failed silently

**After:**
-  **Immediate global consistency** - changes appear everywhere instantly
-  **Clear error feedback** - red alerts show exactly what went wrong
-  **Professional UX** - users always know the status of their actions

###  **Quality Checks Passed**
- **Rust linting**:  Passed (cargo clippy)
- **Code formatting**:  Applied (prettier formatted AgentSettings.tsx)
- **Frontend linting**:  0 errors, 99 warnings (acceptable)

The agent settings now provide complete, professional user feedback for all operations - users will always know whether their saves/deletes succeeded or failed, and all changes are immediately reflected across the entire application!

* Perfect! Everything is working correctly now:

##  **TypeScript Errors Fixed Successfully!**

### 🔧 **Issues Resolved:**

1. ** Variable redeclaration error**:
   - **Problem**: `profilesError` was declared twice (line 35 from hook, line 44 as state)
   - **Fix**: Renamed the state variable to `saveError` to avoid conflict

2. ** TypeScript type errors**:
   - All type compatibility issues resolved by fixing the variable naming conflict

### 📊 **Final Status Check:**

- ** TypeScript compilation**: 0 errors (passed `npx tsc --noEmit`)
- ** Rust linting**: 0 warnings, 0 errors (cargo clippy passed)
- ** Code formatting**: Applied successfully (prettier formatted AgentSettings.tsx)
- ** Frontend linting**: 0 errors, 99 warnings (acceptable per requirements)

### 🎯 **What the Error State Does Now:**

- **`profilesError`**: From `useProfiles()` hook - handles server/network errors
- **`saveError`**: Our new state - handles save operation errors with user-friendly messages

Both error states are displayed properly:
- Server errors appear in the first error alert
- Save operation errors appear in the second error alert (red destructive alert)

The agent settings page now has complete error handling with proper TypeScript types and no compilation errors! 🎉
This commit is contained in:
Alex Netsch
2025-09-17 19:18:38 +02:00
committed by GitHub
parent cfc8684e4b
commit 766985541f
2 changed files with 86 additions and 36 deletions

View File

@@ -28,7 +28,6 @@ interface ExecutorConfigFormProps {
disabled?: boolean; disabled?: boolean;
isSaving?: boolean; isSaving?: boolean;
isDirty?: boolean; isDirty?: boolean;
hideSaveButton?: boolean;
} }
import schemas from 'virtual:executor-schemas'; import schemas from 'virtual:executor-schemas';
@@ -42,7 +41,6 @@ export function ExecutorConfigForm({
disabled = false, disabled = false,
isSaving = false, isSaving = false,
isDirty = false, isDirty = false,
hideSaveButton = false,
}: ExecutorConfigFormProps) { }: ExecutorConfigFormProps) {
const [formData, setFormData] = useState(value || {}); const [formData, setFormData] = useState(value || {});
const [validationErrors, setValidationErrors] = useState< const [validationErrors, setValidationErrors] = useState<
@@ -105,7 +103,7 @@ export function ExecutorConfigForm({
widgets={shadcnTheme.widgets} widgets={shadcnTheme.widgets}
templates={shadcnTheme.templates} templates={shadcnTheme.templates}
> >
{onSave && !hideSaveButton && ( {onSave && (
<div className="flex justify-end pt-4"> <div className="flex justify-end pt-4">
<Button <Button
type="submit" type="submit"

View File

@@ -41,6 +41,7 @@ export function AgentSettings() {
// Local editor state (draft that may differ from server) // Local editor state (draft that may differ from server)
const [localProfilesContent, setLocalProfilesContent] = useState(''); const [localProfilesContent, setLocalProfilesContent] = useState('');
const [profilesSuccess, setProfilesSuccess] = useState(false); const [profilesSuccess, setProfilesSuccess] = useState(false);
const [saveError, setSaveError] = useState<string | null>(null);
// Form-based editor state // Form-based editor state
const [useFormEditor, setUseFormEditor] = useState(true); const [useFormEditor, setUseFormEditor] = useState(true);
@@ -51,9 +52,6 @@ export function AgentSettings() {
const [localParsedProfiles, setLocalParsedProfiles] = useState<any>(null); const [localParsedProfiles, setLocalParsedProfiles] = useState<any>(null);
const [isDirty, setIsDirty] = useState(false); const [isDirty, setIsDirty] = useState(false);
// After successful saves, we refresh the global system state
// so other parts of the app see new/updated profiles immediately.
// Sync server state to local state when not dirty // Sync server state to local state when not dirty
useEffect(() => { useEffect(() => {
if (!isDirty && serverProfilesContent) { if (!isDirty && serverProfilesContent) {
@@ -163,6 +161,9 @@ export function AgentSettings() {
return; return;
} }
// Clear any previous errors
setSaveError(null);
try { try {
// Validate that the configuration exists // Validate that the configuration exists
if ( if (
@@ -220,10 +221,11 @@ export function AgentSettings() {
setProfilesSuccess(true); setProfilesSuccess(true);
setTimeout(() => setProfilesSuccess(false), 3000); setTimeout(() => setProfilesSuccess(false), 3000);
// Refresh global system so new profiles are available elsewhere // Refresh global system so deleted configs are removed elsewhere
reloadSystem(); reloadSystem();
} catch (saveError: unknown) { } catch (saveError: unknown) {
console.error('Failed to save deletion to backend:', saveError); console.error('Failed to save deletion to backend:', saveError);
setSaveError('Failed to delete configuration. Please try again.');
} }
} catch (error) { } catch (error) {
console.error('Error deleting configuration:', error); console.error('Error deleting configuration:', error);
@@ -247,6 +249,9 @@ export function AgentSettings() {
}; };
const handleSaveProfiles = async () => { const handleSaveProfiles = async () => {
// Clear any previous errors
setSaveError(null);
try { try {
const contentToSave = const contentToSave =
useFormEditor && localParsedProfiles useFormEditor && localParsedProfiles
@@ -263,10 +268,11 @@ export function AgentSettings() {
setLocalProfilesContent(contentToSave); setLocalProfilesContent(contentToSave);
} }
// Ensure other parts of the app get the new profiles // Refresh global system so new profiles are available elsewhere
reloadSystem(); reloadSystem();
} catch (err: unknown) { } catch (err: unknown) {
console.error('Failed to save profiles:', err); console.error('Failed to save profiles:', err);
setSaveError('Failed to save agent configurations. Please try again.');
} }
}; };
@@ -294,6 +300,49 @@ export function AgentSettings() {
markDirty(updatedProfiles); markDirty(updatedProfiles);
}; };
const handleExecutorConfigSave = async (formData: unknown) => {
if (!localParsedProfiles || !localParsedProfiles.executors) return;
// Clear any previous errors
setSaveError(null);
// Update the parsed profiles with the saved config
const updatedProfiles = {
...localParsedProfiles,
executors: {
...localParsedProfiles.executors,
[selectedExecutorType]: {
...localParsedProfiles.executors[selectedExecutorType],
[selectedConfiguration]: {
[selectedExecutorType]: formData,
},
},
},
};
// Update state
setLocalParsedProfiles(updatedProfiles);
// Save the updated profiles directly
try {
const contentToSave = JSON.stringify(updatedProfiles, null, 2);
await saveProfiles(contentToSave);
setProfilesSuccess(true);
setIsDirty(false);
setTimeout(() => setProfilesSuccess(false), 3000);
// Update the local content as well
setLocalProfilesContent(contentToSave);
// Refresh global system so new profiles are available elsewhere
reloadSystem();
} catch (err: unknown) {
console.error('Failed to save profiles:', err);
setSaveError('Failed to save configuration. Please try again.');
}
};
if (profilesLoading) { if (profilesLoading) {
return ( return (
<div className="flex items-center justify-center py-8"> <div className="flex items-center justify-center py-8">
@@ -323,6 +372,12 @@ export function AgentSettings() {
</Alert> </Alert>
)} )}
{saveError && (
<Alert variant="destructive">
<AlertDescription>{saveError}</AlertDescription>
</Alert>
)}
<Card> <Card>
<CardHeader> <CardHeader>
<CardTitle>Coding Agent Configurations</CardTitle> <CardTitle>Coding Agent Configurations</CardTitle>
@@ -363,13 +418,13 @@ export function AgentSettings() {
<SelectValue placeholder="Select executor type" /> <SelectValue placeholder="Select executor type" />
</SelectTrigger> </SelectTrigger>
<SelectContent> <SelectContent>
{Object.keys(localParsedProfiles.executors) {Object.keys(localParsedProfiles.executors).map(
.sort((a, b) => a.localeCompare(b)) (type) => (
.map((type) => (
<SelectItem key={type} value={type}> <SelectItem key={type} value={type}>
{type} {type}
</SelectItem> </SelectItem>
))} )
)}
</SelectContent> </SelectContent>
</Select> </Select>
</div> </div>
@@ -397,16 +452,11 @@ export function AgentSettings() {
{Object.keys( {Object.keys(
localParsedProfiles.executors[selectedExecutorType] || localParsedProfiles.executors[selectedExecutorType] ||
{} {}
) ).map((configuration) => (
.sort((a, b) => a.localeCompare(b)) <SelectItem key={configuration} value={configuration}>
.map((configuration) => ( {configuration}
<SelectItem </SelectItem>
key={configuration} ))}
value={configuration}
>
{configuration}
</SelectItem>
))}
<SelectItem value="__create__"> <SelectItem value="__create__">
Create new... Create new...
</SelectItem> </SelectItem>
@@ -457,10 +507,10 @@ export function AgentSettings() {
formData formData
) )
} }
onSave={handleExecutorConfigSave}
disabled={profilesSaving} disabled={profilesSaving}
isSaving={profilesSaving} isSaving={profilesSaving}
isDirty={isDirty} isDirty={isDirty}
hideSaveButton
/> />
)} )}
</div> </div>
@@ -496,20 +546,22 @@ export function AgentSettings() {
</CardContent> </CardContent>
</Card> </Card>
{/* Sticky Save bar (used for both editors) */} {/* Save button for JSON editor mode only */}
<div className="sticky bottom-0 z-10 bg-background/80 backdrop-blur-sm border-t py-4"> {!useFormEditor && (
<div className="flex justify-end"> <div className="sticky bottom-0 z-10 bg-background/80 backdrop-blur-sm border-t py-4">
<Button <div className="flex justify-end">
onClick={handleSaveProfiles} <Button
disabled={!isDirty || profilesSaving || !!profilesError} onClick={handleSaveProfiles}
> disabled={!isDirty || profilesSaving || !!profilesError}
{profilesSaving && ( >
<Loader2 className="mr-2 h-4 w-4 animate-spin" /> {profilesSaving && (
)} <Loader2 className="mr-2 h-4 w-4 animate-spin" />
Save Agent Configurations )}
</Button> Save Agent Configurations
</Button>
</div>
</div> </div>
</div> )}
</div> </div>
); );
} }