From cc5a5716cf2768a4c06eed7513d1afbc22897f33 Mon Sep 17 00:00:00 2001
From: catlog22 <catlog22@github.com>
Date: Wed, 28 Jan 2026 08:25:59 +0800
Subject: [PATCH] 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
---
 ccw/src/core/routes/skills-routes.ts          | 27 ++++++++----
 .../dashboard-js/views/skills-manager.js      | 43 ++++++++++++++++---
 2 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/ccw/src/core/routes/skills-routes.ts b/ccw/src/core/routes/skills-routes.ts
index 45055ad7..d2756730 100644
--- a/ccw/src/core/routes/skills-routes.ts
+++ b/ccw/src/core/routes/skills-routes.ts
@@ -64,19 +64,28 @@ function getDisabledSkillsConfigPath(location: SkillLocation, projectPath: strin
 
 /**
  * Load disabled skills configuration
+ * Throws on JSON parse errors to surface config corruption
  */
 function loadDisabledSkillsConfig(location: SkillLocation, projectPath: string): DisabledSkillsConfig {
   const configPath = getDisabledSkillsConfigPath(location, projectPath);
-  try {
-    if (existsSync(configPath)) {
-      const content = readFileSync(configPath, 'utf8');
-      const config = JSON.parse(content);
-      return { skills: config.skills || {} };
-    }
-  } catch (error) {
-    console.error(`[Skills] Failed to load disabled skills config: ${error}`);
+  
+  if (!existsSync(configPath)) {
+    return { skills: {} };
+  }
+  
+  try {
+    const content = readFileSync(configPath, 'utf8');
+    const config = JSON.parse(content);
+    return { skills: config.skills || {} };
+  } catch (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: {} };
 }
 
 /**
diff --git a/ccw/src/templates/dashboard-js/views/skills-manager.js b/ccw/src/templates/dashboard-js/views/skills-manager.js
index 65a05eec..c13dd2c7 100644
--- a/ccw/src/templates/dashboard-js/views/skills-manager.js
+++ b/ccw/src/templates/dashboard-js/views/skills-manager.js
@@ -219,6 +219,7 @@ function renderSkillCard(skill, location, isDisabled = false) {
             ${location}
           </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'}"
+                  data-skill-toggle="${escapeHtml(folderName)}"
                   onclick="event.stopPropagation(); toggleSkillEnabled('${escapeHtml(folderName)}', '${location}', ${!isDisabled})"
                   title="${isDisabled ? t('skills.enable') : t('skills.disable')}">
             <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 ==========
 
+// Track loading state for skill toggle operations
+var toggleLoadingSkills = {};
+
 async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
-  const action = currentlyEnabled ? 'disable' : 'enable';
-  const confirmMessage = currentlyEnabled 
+  // Prevent double-click
+  var loadingKey = skillName + '-' + location;
+  if (toggleLoadingSkills[loadingKey]) return;
+
+  var action = currentlyEnabled ? 'disable' : 'enable';
+  var confirmMessage = currentlyEnabled 
     ? t('skills.disableConfirm', { name: skillName })
     : t('skills.enableConfirm', { name: skillName });
   
   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 {
-    const response = await fetch('/api/skills/' + encodeURIComponent(skillName) + '/' + action, {
+    var response = await fetch('/api/skills/' + encodeURIComponent(skillName) + '/' + action, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ location, projectPath })
+      body: JSON.stringify({ location: location, projectPath: projectPath })
     });
 
     if (!response.ok) {
-      const error = await response.json();
-      throw new Error(error.message || 'Operation failed');
+      // Robust JSON parsing with fallback
+      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
@@ -460,7 +484,9 @@ async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
     renderSkillsView();
 
     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');
     }
   } catch (err) {
@@ -468,6 +494,9 @@ async function toggleSkillEnabled(skillName, location, currentlyEnabled) {
     if (window.showToast) {
       showToast(err.message || t('skills.toggleError'), 'error');
     }
+  } finally {
+    // Clear loading state
+    delete toggleLoadingSkills[loadingKey];
   }
 }