fix(skills): improve robustness of enable/disable operations

- Add rollback in moveDirectory when rmSync fails after cpSync
- Add transaction rollback in disable/enableSkill when config save fails
- Surface config corruption by throwing on JSON parse errors
- Add robust JSON error parsing with fallback in frontend
- Add loading state and double-click prevention for toggle button
This commit is contained in:
catlog22
2026-01-28 08:25:59 +08:00
parent af05874510
commit cc5a5716cf
2 changed files with 54 additions and 16 deletions

View File

@@ -64,19 +64,28 @@ function getDisabledSkillsConfigPath(location: SkillLocation, projectPath: strin
/** /**
* Load disabled skills configuration * Load disabled skills configuration
* Throws on JSON parse errors to surface config corruption
*/ */
function loadDisabledSkillsConfig(location: SkillLocation, projectPath: string): DisabledSkillsConfig { function loadDisabledSkillsConfig(location: SkillLocation, projectPath: string): DisabledSkillsConfig {
const configPath = getDisabledSkillsConfigPath(location, projectPath); const configPath = getDisabledSkillsConfigPath(location, projectPath);
if (!existsSync(configPath)) {
return { skills: {} };
}
try { try {
if (existsSync(configPath)) {
const content = readFileSync(configPath, 'utf8'); const content = readFileSync(configPath, 'utf8');
const config = JSON.parse(content); const config = JSON.parse(content);
return { skills: config.skills || {} }; return { skills: config.skills || {} };
}
} catch (error) { } catch (error) {
console.error(`[Skills] Failed to load disabled skills config: ${error}`); // Throw on JSON parse errors to surface config corruption
if (error instanceof SyntaxError) {
throw new Error(`Config file corrupted: ${configPath}`);
} }
// Log and return empty for other errors (permission, etc.)
console.error(`[Skills] Failed to load disabled skills config: ${error}`);
return { skills: {} }; return { skills: {} };
}
} }
/** /**

View File

@@ -219,6 +219,7 @@ function renderSkillCard(skill, location, isDisabled = false) {
${location} ${location}
</span> </span>
<button class="p-1.5 rounded-lg transition-colors ${isDisabled ? 'text-green-600 hover:bg-green-100' : 'text-amber-600 hover:bg-amber-100'}" <button class="p-1.5 rounded-lg transition-colors ${isDisabled ? 'text-green-600 hover:bg-green-100' : 'text-amber-600 hover:bg-amber-100'}"
data-skill-toggle="${escapeHtml(folderName)}"
onclick="event.stopPropagation(); toggleSkillEnabled('${escapeHtml(folderName)}', '${location}', ${!isDisabled})" onclick="event.stopPropagation(); toggleSkillEnabled('${escapeHtml(folderName)}', '${location}', ${!isDisabled})"
title="${isDisabled ? t('skills.enable') : t('skills.disable')}"> title="${isDisabled ? t('skills.enable') : t('skills.disable')}">
<i data-lucide="${isDisabled ? 'toggle-left' : 'toggle-right'}" class="w-4 h-4"></i> <i data-lucide="${isDisabled ? 'toggle-left' : 'toggle-right'}" class="w-4 h-4"></i>
@@ -432,24 +433,47 @@ function editSkill(skillName, location) {
// ========== Enable/Disable Skills Functions ========== // ========== Enable/Disable Skills Functions ==========
// Track loading state for skill toggle operations
var toggleLoadingSkills = {};
async function toggleSkillEnabled(skillName, location, currentlyEnabled) { async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
const action = currentlyEnabled ? 'disable' : 'enable'; // Prevent double-click
const confirmMessage = currentlyEnabled var loadingKey = skillName + '-' + location;
if (toggleLoadingSkills[loadingKey]) return;
var action = currentlyEnabled ? 'disable' : 'enable';
var confirmMessage = currentlyEnabled
? t('skills.disableConfirm', { name: skillName }) ? t('skills.disableConfirm', { name: skillName })
: t('skills.enableConfirm', { name: skillName }); : t('skills.enableConfirm', { name: skillName });
if (!confirm(confirmMessage)) return; if (!confirm(confirmMessage)) return;
// Set loading state
toggleLoadingSkills[loadingKey] = true;
var toggleBtn = document.querySelector('[data-skill-toggle="' + skillName + '"]');
if (toggleBtn) {
toggleBtn.disabled = true;
toggleBtn.innerHTML = '<i data-lucide="loader-2" class="w-4 h-4 animate-spin"></i>';
if (window.lucide) lucide.createIcons();
}
try { try {
const response = await fetch('/api/skills/' + encodeURIComponent(skillName) + '/' + action, { var response = await fetch('/api/skills/' + encodeURIComponent(skillName) + '/' + action, {
method: 'POST', method: 'POST',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ location, projectPath }) body: JSON.stringify({ location: location, projectPath: projectPath })
}); });
if (!response.ok) { if (!response.ok) {
const error = await response.json(); // Robust JSON parsing with fallback
throw new Error(error.message || 'Operation failed'); var errorMessage = 'Operation failed';
try {
var error = await response.json();
errorMessage = error.message || errorMessage;
} catch (jsonErr) {
errorMessage = response.statusText || errorMessage;
}
throw new Error(errorMessage);
} }
// Close detail panel if open // Close detail panel if open
@@ -460,7 +484,9 @@ async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
renderSkillsView(); renderSkillsView();
if (window.showToast) { if (window.showToast) {
const message = currentlyEnabled ? t('skills.disabled') : t('skills.enabled'); var message = currentlyEnabled
? t('skills.disableSuccess', { name: skillName })
: t('skills.enableSuccess', { name: skillName });
showToast(message, 'success'); showToast(message, 'success');
} }
} catch (err) { } catch (err) {
@@ -468,6 +494,9 @@ async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
if (window.showToast) { if (window.showToast) {
showToast(err.message || t('skills.toggleError'), 'error'); showToast(err.message || t('skills.toggleError'), 'error');
} }
} finally {
// Clear loading state
delete toggleLoadingSkills[loadingKey];
} }
} }