From 641cbfba3190742a5f53609fc54e5b04b0113071 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:49:38 -0400 Subject: [PATCH] fix(web): report a no-op plugin update as already up to date update_plugin() returns True both for a real update and for "nothing to do" (a ZIP-installed monorepo plugin already at the registry version, a bundled plugin). With no git commit to compare, POST /plugins/update called every such success "updated successfully", so Check & Update All counted most official plugins as updated on every run. The route now reads what changed off the plugin itself (commit, else manifest version, else last_updated) and returns data.update_status (updated / up_to_date / local_only). The update-all toast is summarised by PluginInstallManager.summarizeUpdateResults from that status, falling back to the message for older servers. Co-Authored-By: Claude Opus 5 --- test/js/unit/test_update_all.js | 27 ++++++++ test/web_interface/test_update_all_plugins.py | 61 +++++++++++++++++++ web_interface/blueprints/api_v3/plugins.py | 27 +++++++- .../static/v3/js/plugins/install_manager.js | 52 ++++++++++++++++ web_interface/static/v3/plugins_manager.js | 31 +++++----- 5 files changed, 179 insertions(+), 19 deletions(-) diff --git a/test/js/unit/test_update_all.js b/test/js/unit/test_update_all.js index acf29099..539fefc3 100644 --- a/test/js/unit/test_update_all.js +++ b/test/js/unit/test_update_all.js @@ -146,6 +146,33 @@ const noSleep = { sleep: async () => {} }; ok('...and is reported as a failure', results.find(x => x.pluginId === 'static-image').success === false); } + console.log('\nsummary: a no-op update is not counted as updated'); + { + const answer = (update_status, message) => ({ success: true, result: { status: 'success', message, data: { update_status } } }); + const results = [ + answer('updated', 'Plugin a updated to version 2.8.0'), + // ZIP-installed monorepo plugin already at the registry version: the + // route used to call this "updated successfully". + answer('up_to_date', 'Plugin stock-news already up to date (version 2.8.0)'), + answer('up_to_date', 'Plugin clock already up to date (commit abcdef1)'), + answer('local_only', 'Plugin mine is managed locally and does not receive registry updates'), + { success: false, error: { error_code: 'PLUGIN_UPDATE_FAILED' } }, + ]; + const s = Manager.summarizeUpdateResults(results); + ok('counts come from update_status', + s.updated === 1 && s.upToDate === 2 && s.localOnly === 1 && s.failed === 1, s); + ok('toast text names each outcome', + s.text === '1 updated, 2 already up to date, 1 managed locally, 1 failed', s.text); + ok('a failure alongside an update is a warning', s.type === 'warning', s.type); + ok('an older server that only says so in the message is still up to date', + Manager.updateOutcome({ success: true, result: { message: 'Plugin x already up to date (commit 1234567)' } }) === 'up_to_date'); + ok('a success without a status or telltale message counts as updated', + Manager.updateOutcome({ success: true, result: { message: 'Plugin x updated successfully' } }) === 'updated'); + const allNoop = Manager.summarizeUpdateResults([answer('up_to_date', ''), answer('up_to_date', '')]); + ok('nothing to do is a success toast with no "updated"', + allNoop.type === 'success' && allNoop.text === '2 already up to date', allNoop); + } + console.log(`\n${pass} passed, ${fail} failed`); process.exit(fail ? 1 : 0); })().catch(e => { console.error(e); process.exit(1); }); diff --git a/test/web_interface/test_update_all_plugins.py b/test/web_interface/test_update_all_plugins.py index c9ad9b39..899092ad 100644 --- a/test/web_interface/test_update_all_plugins.py +++ b/test/web_interface/test_update_all_plugins.py @@ -80,6 +80,67 @@ class TestUpdateRouteRejectsStarlarkIds: assert [c.args[0] for c in store.update_plugin.call_args_list] == ['stock-news', 'starlark-apps'] +class TestUpdateRouteReportsNoOps: + """update_plugin() answers True when there was nothing to do. + + A ZIP-installed monorepo plugin (no .git) already at the registry version + is the common case for official plugins. The route used to call that + "updated successfully", so Check & Update All counted it as updated. + """ + + def _install(self, tmp_path, pid, version, last_updated='2026-09-01'): + (tmp_path / pid).mkdir() + (tmp_path / pid / 'manifest.json').write_text( + '{"id": "%s", "version": "%s", "last_updated": "%s"}' % (pid, version, last_updated), + encoding='utf-8') + + def test_a_zip_plugin_already_at_the_registry_version_is_up_to_date(self, client, store, tmp_path): + self._install(tmp_path, 'stock-news', '2.8.0') + body = client.post('/api/v3/plugins/update', json={'plugin_id': 'stock-news'}).get_json() + assert body['status'] == 'success' + assert body['data']['update_status'] == 'up_to_date' + assert 'already up to date' in body['message'], body + assert 'updated successfully' not in body['message'] + + def test_a_zip_plugin_reinstalled_at_a_new_version_is_updated(self, client, store, tmp_path): + self._install(tmp_path, 'stock-news', '2.6.2') + + def reinstall(pid): + (tmp_path / pid / 'manifest.json').write_text( + '{"id": "%s", "version": "2.8.0", "last_updated": "2026-09-01"}' % pid, + encoding='utf-8') + return True + store.update_plugin.side_effect = reinstall + + body = client.post('/api/v3/plugins/update', json={'plugin_id': 'stock-news'}).get_json() + assert body['data']['update_status'] == 'updated' + assert '2.8.0' in body['message'], body + + def test_a_git_plugin_whose_commit_is_unchanged_is_up_to_date(self, client, store, tmp_path): + self._install(tmp_path, 'clock', '1.0.0') + store._get_local_git_info.return_value = {'sha': 'abcdef1234567', 'branch': 'main'} + body = client.post('/api/v3/plugins/update', json={'plugin_id': 'clock'}).get_json() + assert body['data']['update_status'] == 'up_to_date' + + def test_a_git_plugin_that_moved_is_updated(self, client, store, tmp_path): + self._install(tmp_path, 'clock', '1.0.0') + store._get_local_git_info.side_effect = [ + {'sha': 'aaaaaaa000', 'branch': 'main'}, # before + {'sha': 'aaaaaaa000', 'branch': 'main'}, # is-git check + {'sha': 'bbbbbbb111', 'branch': 'main'}, # after + ] + body = client.post('/api/v3/plugins/update', json={'plugin_id': 'clock'}).get_json() + assert body['data']['update_status'] == 'updated', body + + def test_a_local_only_plugin_says_so(self, client, store, tmp_path): + (tmp_path / 'mine').mkdir() + (tmp_path / 'mine' / 'manifest.json').write_text( + '{"id": "mine", "version": "0.1.0", "local_only": true}', encoding='utf-8') + body = client.post('/api/v3/plugins/update', json={'plugin_id': 'mine'}).get_json() + assert body['data']['update_status'] == 'local_only' + store.update_plugin.assert_not_called() + + class TestInstalledListContract: """What update-all filters on is what /plugins/installed publishes.""" diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index c9643e54..8d410c36 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -988,6 +988,7 @@ def update_plugin(): ) current_last_updated = None + current_version = None current_commit = None current_branch = None @@ -997,6 +998,7 @@ def update_plugin(): with open(manifest_path, 'r', encoding='utf-8') as f: manifest = json.load(f) current_last_updated = manifest.get('last_updated') + current_version = manifest.get('version') if manifest.get('local_only'): logger.debug("Skipping update for local-only plugin: %s", plugin_id) if api_v3.operation_history: @@ -1006,7 +1008,9 @@ def update_plugin(): status="skipped", details={"reason": "local_only"} ) - return success_response(message=f'Plugin {plugin_id} is managed locally and does not receive registry updates') + return success_response( + data={'update_status': 'local_only'}, + message=f'Plugin {plugin_id} is managed locally and does not receive registry updates') except Exception as e: logger.debug("Could not read local manifest for plugin: %s", e) @@ -1036,12 +1040,14 @@ def update_plugin(): if success: updated_last_updated = current_last_updated + updated_version = current_version try: if manifest_path.exists(): import json with open(manifest_path, 'r', encoding='utf-8') as f: manifest = json.load(f) updated_last_updated = manifest.get('last_updated', current_last_updated) + updated_version = manifest.get('version', current_version) except Exception as e: logger.debug("Could not read updated manifest after update: %s", e) @@ -1053,15 +1059,28 @@ def update_plugin(): updated_commit = git_info_after.get('sha') updated_branch = git_info_after.get('branch') or updated_branch + # update_plugin() answers True for "nothing to do" as well as for + # a real update (a ZIP-installed monorepo plugin already at the + # registry version, a bundled plugin), so what changed is read off + # the plugin itself: its git commit, else its manifest. + update_status = 'updated' message = f'Plugin {plugin_id} updated successfully' if current_commit and updated_commit and current_commit == updated_commit: + update_status = 'up_to_date' message = f'Plugin {plugin_id} already up to date (commit {updated_commit[:7]})' elif updated_commit: message = f'Plugin {plugin_id} updated to commit {updated_commit[:7]}' if updated_branch: message += f' on branch {updated_branch}' + elif updated_version and updated_version != current_version: + message = f'Plugin {plugin_id} updated to version {updated_version}' elif updated_last_updated and updated_last_updated != current_last_updated: message = f'Plugin {plugin_id} refreshed (Last Updated {updated_last_updated})' + elif not current_commit: + update_status = 'up_to_date' + message = f'Plugin {plugin_id} already up to date' + if updated_version: + message += f' (version {updated_version})' remote_commit_short = remote_commit[:7] if remote_commit else None if remote_commit_short and updated_commit and remote_commit_short != updated_commit[:7]: @@ -1093,14 +1112,16 @@ def update_plugin(): "version": version, "previous_commit": current_commit[:7] if current_commit else None, "commit": updated_commit[:7] if updated_commit else None, - "branch": updated_branch + "branch": updated_branch, + "update_status": update_status } ) return success_response( data={ 'last_updated': updated_last_updated, - 'commit': updated_commit + 'commit': updated_commit, + 'update_status': update_status }, message=message ) diff --git a/web_interface/static/v3/js/plugins/install_manager.js b/web_interface/static/v3/js/plugins/install_manager.js index 49702332..f7d284f9 100644 --- a/web_interface/static/v3/js/plugins/install_manager.js +++ b/web_interface/static/v3/js/plugins/install_manager.js @@ -168,6 +168,58 @@ const PluginInstallManager = { } return results; + }, + + /** + * Classify one POST /plugins/update answer. + * + * The route reports what actually happened in `data.update_status` + * (`updated`, `up_to_date`, `local_only`). A plugin the updater had + * nothing to do for -- e.g. a ZIP-installed monorepo plugin already at the + * registry version -- is still a success response, so it must not be + * counted as updated. Older servers only say so in the message. + * + * @param {Object} entry - One element of updateAll()'s results + * @returns {string} 'failed' | 'updated' | 'up_to_date' | 'local_only' + */ + updateOutcome(entry) { + if (!entry || !entry.success) return 'failed'; + const result = entry.result || {}; + const status = result.data && result.data.update_status; + if (status === 'up_to_date' || status === 'local_only' || status === 'updated') { + return status; + } + const message = typeof result.message === 'string' ? result.message : ''; + if (message.includes('already up to date')) return 'up_to_date'; + if (message.includes('managed locally')) return 'local_only'; + return 'updated'; + }, + + /** + * Summarise updateAll()'s results for the Check & Update All toast. + * + * @param {Array} results - updateAll()'s results + * @returns {{updated: number, upToDate: number, localOnly: number, failed: number, text: string, type: string}} + */ + summarizeUpdateResults(results) { + const counts = { updated: 0, up_to_date: 0, local_only: 0, failed: 0 }; + for (const entry of (Array.isArray(results) ? results : [])) { + counts[this.updateOutcome(entry)]++; + } + const parts = []; + if (counts.updated > 0) parts.push(`${counts.updated} updated`); + if (counts.up_to_date > 0) parts.push(`${counts.up_to_date} already up to date`); + if (counts.local_only > 0) parts.push(`${counts.local_only} managed locally`); + if (counts.failed > 0) parts.push(`${counts.failed} failed`); + const type = counts.failed > 0 ? (counts.updated > 0 ? 'warning' : 'error') : 'success'; + return { + updated: counts.updated, + upToDate: counts.up_to_date, + localOnly: counts.local_only, + failed: counts.failed, + text: parts.join(', '), + type + }; } }; diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index 39483a8a..5d277a7f 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -1999,22 +1999,21 @@ function runUpdateAllPlugins() { showNotification('No plugins to update.', 'info'); return; } - let updated = 0, upToDate = 0, failed = 0; - for (const r of results) { - if (!r.success) { - failed++; - } else if (r.result && r.result.message && r.result.message.includes('already up to date')) { - upToDate++; - } else { - updated++; - } - } - const parts = []; - if (updated > 0) parts.push(`${updated} updated`); - if (upToDate > 0) parts.push(`${upToDate} already up to date`); - if (failed > 0) parts.push(`${failed} failed`); - const type = failed > 0 ? (updated > 0 ? 'warning' : 'error') : 'success'; - showNotification(parts.join(', '), type); + // Counted by install_manager.js from each answer's update_status: + // a no-op update is "already up to date", not "updated". A cached + // install_manager.js from before that helper gets a plain count. + const manager = window.PluginInstallManager; + const summary = (manager && typeof manager.summarizeUpdateResults === 'function') + ? manager.summarizeUpdateResults(results) + : (() => { + const failed = results.filter(r => !r.success).length; + const checked = results.length - failed; + return { + text: `${checked} checked` + (failed ? `, ${failed} failed` : ''), + type: failed ? (checked ? 'warning' : 'error') : 'success' + }; + })(); + showNotification(summary.text, summary.type); }) .catch(error => { console.error('Error updating all plugins:', error);