fix(tools-tab): resolve second-pass review findings

- 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>
This commit is contained in:
Chuck
2026-06-29 12:05:02 -04:00
co-authored by Claude Sonnet 4.6
parent 06ea4aa496
commit 453d46563f
2 changed files with 41 additions and 13 deletions
+29 -9
View File
@@ -829,6 +829,19 @@ def save_main_config():
return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400 return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400
if not (2 <= copies <= 8): if not (2 <= copies <= 8):
return jsonify({'status': 'error', 'message': "Double-sided copies must be between 2 and 8"}), 400 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 ds_config['copies'] = copies
if 'double_sided_axis' in data: if 'double_sided_axis' in data:
@@ -1082,6 +1095,8 @@ def save_main_config():
continue continue
if key in vegas_fields: if key in vegas_fields:
continue continue
if key in double_sided_fields:
continue
# For any remaining keys (including plugin keys), use deep merge to preserve existing settings # 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): if key in current_config and isinstance(current_config[key], dict) and isinstance(data[key], dict):
# Deep merge to preserve existing settings # Deep merge to preserve existing settings
@@ -1673,15 +1688,20 @@ def execute_system_action():
for p in sorted(plugins_dir.iterdir()): for p in sorted(plugins_dir.iterdir()):
req = p / 'requirements.txt' req = p / 'requirements.txt'
if p.is_dir() and req.exists(): if p.is_dir() and req.exists():
r = subprocess.run( try:
[sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)], r = subprocess.run(
capture_output=True, text=True, timeout=60 [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)],
) capture_output=True, text=True, timeout=60
results.append({ )
'plugin': p.name, results.append({
'ok': r.returncode == 0, 'plugin': p.name,
'output': _truncate_output(r.stdout, r.stderr) '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']) ok_count = sum(1 for r in results if r['ok'])
all_ok = all(r['ok'] for r in results) if results else True all_ok = all(r['ok'] for r in results) if results else True
return jsonify({ return jsonify({
+12 -4
View File
@@ -17,7 +17,7 @@
<div class="flex items-start justify-between gap-4"> <div class="flex items-start justify-between gap-4">
<div> <div>
<p class="text-sm font-medium text-gray-900">Pull latest (rebase)</p> <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> </div>
<button id="btn-git-pull" onclick="toolsAction('git_pull', 'btn-git-pull', 'result-git-pull')" <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"> 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; if (!panel) return;
fetch('/api/v3/system/git-info') 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 => { .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 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-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>'; : '<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>`; html += `</div>`;
panel.innerHTML = html; panel.innerHTML = html;
}) })
.catch(() => { .catch(err => {
panel.innerHTML = '<span class="text-sm text-red-600">Could not load git info.</span>'; panel.innerHTML = `<span class="text-sm text-red-600">Could not load git info: ${escHtml(String(err))}</span>`;
}); });
} }