From 453d46563fe03dfa2b0a215fd9106c90728b9eda Mon Sep 17 00:00:00 2001
From: Chuck <chuck@example.com>
Date: Mon, 29 Jun 2026 12:05:02 -0400
Subject: [PATCH] fix(tools-tab): resolve second-pass review findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Wrap per-plugin subprocess.run in try/except TimeoutExpired/OSError so
  one plugin's failure appends a result entry and continues the loop
  rather than collapsing the whole batch into a 500
- Validate double_sided_copies divisibility against chain_length
  (horizontal axis) or parallel (vertical axis) after the range check;
  reads effective axis from the current request or stored config
- Exclude double_sided_fields from the generic key-merge loop so
  double_sided_enabled/copies/axis are never written as root-level keys
- Fix tools.html copy: "then restores the stash" removed — git_pull
  stashes changes but never pops them
- Check r.ok and d.status in loadGitInfo before building the panel;
  backend error messages now surface instead of silently showing a
  false-clean state

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 web_interface/blueprints/api_v3.py            | 38 ++++++++++++++-----
 .../templates/v3/partials/tools.html          | 16 ++++++--
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py
index 8b634f81..2f59d3b8 100644
--- a/web_interface/blueprints/api_v3.py
+++ b/web_interface/blueprints/api_v3.py
@@ -829,6 +829,19 @@ def save_main_config():
                     return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400
                 if not (2 <= copies <= 8):
                     return jsonify({'status': 'error', 'message': "Double-sided copies must be between 2 and 8"}), 400
+                # Validate divisibility against the relevant hardware dimension.
+                # Use axis from this request if provided, else from stored config.
+                hw = current_config.get('display', {}).get('hardware', {})
+                effective_axis = (data.get('double_sided_axis')
+                                  or current_config.get('display', {}).get('double_sided', {}).get('axis', 'horizontal'))
+                if effective_axis == 'horizontal':
+                    chain_length = int(hw.get('chain_length', 2) or 2)
+                    if chain_length % copies != 0:
+                        return jsonify({'status': 'error', 'message': f"Double-sided copies ({copies}) must divide chain length ({chain_length}) evenly"}), 400
+                elif effective_axis == 'vertical':
+                    parallel = int(hw.get('parallel', 1) or 1)
+                    if parallel % copies != 0:
+                        return jsonify({'status': 'error', 'message': f"Double-sided copies ({copies}) must divide parallel ({parallel}) evenly"}), 400
                 ds_config['copies'] = copies
 
             if 'double_sided_axis' in data:
@@ -1082,6 +1095,8 @@ def save_main_config():
                 continue
             if key in vegas_fields:
                 continue
+            if key in double_sided_fields:
+                continue
             # For any remaining keys (including plugin keys), use deep merge to preserve existing settings
             if key in current_config and isinstance(current_config[key], dict) and isinstance(data[key], dict):
                 # Deep merge to preserve existing settings
@@ -1673,15 +1688,20 @@ def execute_system_action():
                 for p in sorted(plugins_dir.iterdir()):
                     req = p / 'requirements.txt'
                     if p.is_dir() and req.exists():
-                        r = subprocess.run(
-                            [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)],
-                            capture_output=True, text=True, timeout=60
-                        )
-                        results.append({
-                            'plugin': p.name,
-                            'ok': r.returncode == 0,
-                            'output': _truncate_output(r.stdout, r.stderr)
-                        })
+                        try:
+                            r = subprocess.run(
+                                [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)],
+                                capture_output=True, text=True, timeout=60
+                            )
+                            results.append({
+                                'plugin': p.name,
+                                'ok': r.returncode == 0,
+                                'output': _truncate_output(r.stdout, r.stderr)
+                            })
+                        except subprocess.TimeoutExpired:
+                            results.append({'plugin': p.name, 'ok': False, 'output': 'pip install timed out'})
+                        except OSError as exc:
+                            results.append({'plugin': p.name, 'ok': False, 'output': str(exc)})
             ok_count = sum(1 for r in results if r['ok'])
             all_ok = all(r['ok'] for r in results) if results else True
             return jsonify({
diff --git a/web_interface/templates/v3/partials/tools.html b/web_interface/templates/v3/partials/tools.html
index 8f2b7c4e..bf4cf0a5 100644
--- a/web_interface/templates/v3/partials/tools.html
+++ b/web_interface/templates/v3/partials/tools.html
@@ -17,7 +17,7 @@
             <div class="flex items-start justify-between gap-4">
                 <div>
                     <p class="text-sm font-medium text-gray-900">Pull latest (rebase)</p>
-                    <p class="text-xs text-gray-500 mt-0.5">Stashes local changes, runs <code class="bg-gray-100 px-1 rounded">git pull --rebase</code>, then restores the stash.</p>
+                    <p class="text-xs text-gray-500 mt-0.5">Stashes any local changes, then runs <code class="bg-gray-100 px-1 rounded">git pull --rebase</code>. The stash is preserved but not re-applied.</p>
                 </div>
                 <button id="btn-git-pull" onclick="toolsAction('git_pull', 'btn-git-pull', 'result-git-pull')"
                         class="shrink-0 inline-flex items-center px-3 py-2 border border-gray-300 text-sm font-medium rounded-md text-gray-700 bg-white hover:bg-gray-50">
@@ -264,8 +264,16 @@
         if (!panel) return;
 
         fetch('/api/v3/system/git-info')
-            .then(r => r.json())
+            .then(r => {
+                if (!r.ok) return r.json().then(d => Promise.reject(d.message || `HTTP ${r.status}`));
+                return r.json();
+            })
             .then(d => {
+                if (d.status === 'error') {
+                    panel.innerHTML = `<span class="text-sm text-red-600">${escHtml(d.message || 'Git info unavailable.')}</span>`;
+                    return;
+                }
+
                 const dirtyBadge = d.dirty
                     ? '<span class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium bg-yellow-100 text-yellow-800">dirty</span>'
                     : '<span class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium bg-green-100 text-green-800">clean</span>';
@@ -295,8 +303,8 @@
                 html += `</div>`;
                 panel.innerHTML = html;
             })
-            .catch(() => {
-                panel.innerHTML = '<span class="text-sm text-red-600">Could not load git info.</span>';
+            .catch(err => {
+                panel.innerHTML = `<span class="text-sm text-red-600">Could not load git info: ${escHtml(String(err))}</span>`;
             });
     }