From 76636600b0decb5577bcfff5d436eee5830b2ad4 Mon Sep 17 00:00:00 2001 From: Eric Hansen Date: Tue, 5 May 2026 20:38:29 -0500 Subject: [PATCH 1/4] fix(plugins): show dev-branch state instead of fake upgrade for local installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a plugin is installed from a local path that is a git checkout on a branch (active dev work), the TUI's upgrade detector previously fell back to ``git describe --tags --abbrev=0`` to find the nearest ancestor tag and compared that to the highest tag on origin. This produced a misleading ``↑ vX.Y.Z`` arrow even though pressing ``[u]`` could not actually upgrade the plugin -- ``copilot plugin update`` only does ``git pull`` on the current branch, so the user's dev branch never advances to the release tag. Now we distinguish three install states: 1. Tag-pinned (detached HEAD on an exact tag): unchanged. Show ``↑ vX.Y.Z`` if origin has a higher tag; ``[u]`` works via the CLI. 2. Dev checkout (HEAD on a branch -- including a branch whose tip happens to coincide with a tag): NEW. Show ``dev: `` in the Upgrade column instead of an arrow. The detail pane explains the local install state, surfaces the latest origin tag for context, and shows the suggested git command to pin to a release. Pressing ``[u]`` short- circuits with a helpful message pointing at the source repo -- it does not call the CLI. 3. Other (no tags, missing path, not git): unchanged. Branch state is authoritative. We deliberately ignore exact-tag detection when HEAD is on a branch, because ``copilot plugin update`` will ``git pull`` the branch rather than ``git checkout`` the tag, so we cannot promise tag-based upgrades will land. Also clears stale ``upgrade_available``/``upgrade_version`` in the no- upgrade fallback of ``_apply_upgrades`` so a row that flips from provisional-upgradable to fresh-no-upgrade does not still trigger the CLI when ``[u]`` is pressed. This is purely informational. We do NOT re-introduce any git-checkout logic in the upgrade handler -- that was the path PR #31 took and PR #32 deliberately reverted, after upstream confirmed ``copilot plugin update`` handles fresh-tag local installs correctly. The bug here is in our display heuristics, not in the upgrade action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilotsetup/data/plugins.py | 4 + src/copilotsetup/plugin_upgrades.py | 116 +++++++++++--- src/copilotsetup/tabs/plugins.py | 59 ++++++- src/copilotsetup/upgrade_cache.py | 7 +- tests/test_plugin_upgrades.py | 174 ++++++++++++++++++++- tests/test_plugins_tab.py | 228 ++++++++++++++++++++++++++++ 6 files changed, 557 insertions(+), 31 deletions(-) diff --git a/src/copilotsetup/data/plugins.py b/src/copilotsetup/data/plugins.py index 29dac8d..0099d07 100644 --- a/src/copilotsetup/data/plugins.py +++ b/src/copilotsetup/data/plugins.py @@ -43,6 +43,10 @@ class PluginInfo: upgrade_summary: str = "" upgrade_version: str = "" upgrade_provisional: bool = False + dev_summary: str = "" + dev_branch: str = "" + dev_commits_ahead: int = 0 + latest_release: str = "" @property def status(self) -> str: diff --git a/src/copilotsetup/plugin_upgrades.py b/src/copilotsetup/plugin_upgrades.py index c0d037f..f455c4e 100644 --- a/src/copilotsetup/plugin_upgrades.py +++ b/src/copilotsetup/plugin_upgrades.py @@ -4,9 +4,15 @@ (e.g. ``v0.11.2``). Detection compares the current tag against the highest semver tag on ``origin``. -Local plugins living on a regular branch are also supported: the nearest -ancestor tag (via ``git describe --tags --abbrev=0``) is used as the current -version. A ``config_version`` fallback covers repos with no local tags. +Plugins installed from a local source path that is a git checkout on a +*branch* (active dev work) are reported as ``STATUS_LOCAL_DEV`` instead of +falsely claiming they can be upgraded. ``copilot plugin update`` only does +``git pull`` on the current branch, so an arrow that implies "click to +upgrade to vX.Y.Z" would be misleading — the user manages their own checkout. + +A ``config_version`` fallback (used when no git tag describes HEAD and we +have no branch info) covers older installs whose package.json version is +the only signal. """ from __future__ import annotations @@ -29,6 +35,10 @@ STATUS_NO_UPSTREAM = "no-upstream" STATUS_NO_PATH = "no-path" STATUS_ERROR = "error" +# HEAD is on a branch (not detached on a tag); user is doing local dev work. +# We do not present this as upgradable because ``copilot plugin update`` would +# only ``git pull`` the branch — it cannot meaningfully "upgrade to vX.Y.Z". +STATUS_LOCAL_DEV = "local-dev" @dataclass @@ -42,6 +52,8 @@ class PluginUpgradeInfo: current_version: str = "" latest_version: str = "" network_verified: bool = False + dev_branch: str = "" + dev_commits_ahead: int = 0 @property def upgrade_available(self) -> bool: @@ -51,6 +63,13 @@ def upgrade_available(self) -> bool: def summary(self) -> str: return f"↑ {self.latest_version}" if self.upgrade_available else "" + @property + def dev_summary(self) -> str: + """Human-readable dev state, e.g. ``dev: feat/x`` or empty when not on a branch.""" + if self.status != STATUS_LOCAL_DEV or not self.dev_branch: + return "" + return f"dev: {self.dev_branch}" + def _git_env(*, gh_token_timeout: float = 5.0) -> dict[str, str]: """Build a non-interactive, auth-aware git environment.""" @@ -120,22 +139,41 @@ def _parse_semver(tag: str) -> tuple[int, int, int] | None: return int(match.group(1)), int(match.group(2)), int(match.group(3)) -def _get_current_tag(path: Path) -> str | None: - """Return the version tag for HEAD. - - Tries ``--exact-match`` first (detached-HEAD installs), then falls back to - ``--abbrev=0`` which finds the nearest ancestor tag (branch-based repos). - """ +def _get_exact_tag(path: Path) -> str | None: + """Return the tag at HEAD, or None if HEAD is not on an exact tag.""" result = _run_git(["describe", "--tags", "--exact-match", "HEAD"], path, timeout=5.0) if result.returncode == 0 and result.stdout.strip(): return result.stdout.strip() - # Fallback: nearest ancestor tag (works when HEAD is ahead of a tag) + return None + + +def _get_ancestor_tag(path: Path) -> str | None: + """Return the nearest ancestor tag for HEAD (no exact-match required).""" result = _run_git(["describe", "--tags", "--abbrev=0", "HEAD"], path, timeout=5.0) if result.returncode == 0 and result.stdout.strip(): return result.stdout.strip() return None +def _get_head_branch(path: Path) -> str | None: + """Return the branch HEAD is on, or None if HEAD is detached.""" + result = _run_git(["symbolic-ref", "--quiet", "--short", "HEAD"], path, timeout=5.0) + if result.returncode == 0 and result.stdout.strip(): + return result.stdout.strip() + return None + + +def _get_commits_ahead(path: Path, base_ref: str) -> int: + """Return the number of commits HEAD is ahead of ``base_ref``, or 0 on error.""" + result = _run_git(["rev-list", "--count", f"{base_ref}..HEAD"], path, timeout=5.0) + if result.returncode != 0: + return 0 + try: + return int(result.stdout.strip()) + except ValueError: + return 0 + + def _list_remote_tags(path: Path) -> list[str]: result = _run_git(["ls-remote", "--tags", "origin"], path, timeout=10.0) if result.returncode != 0: @@ -197,22 +235,32 @@ def check_plugin( info.detail = "not a git checkout" return info - # Detect current version: exact tag → nearest ancestor tag → config.json - current = _get_current_tag(path) - if current is None and config_version: - # Synthesize a tag-like string so semver comparison works + # Determine HEAD state. Branch is authoritative: if HEAD is on a branch, + # we treat it as a dev install regardless of whether HEAD happens to also + # be at an exact tag — because ``copilot plugin update`` will ``git pull`` + # the branch, not check out the tag, so we cannot promise tag-based + # upgrades land. Only a detached HEAD on an exact tag uses the upgrade + # flow. + branch = _get_head_branch(path) + exact_tag = _get_exact_tag(path) if branch is None else None + + if exact_tag is not None: + current: str | None = exact_tag + elif branch is None and config_version: + # Detached HEAD with no exact tag — use config_version as a best-effort + # current version so we can still compare against origin tags. v = config_version.strip() if _parse_semver(v) is not None: current = v elif _parse_semver(f"v{v}") is not None: current = f"v{v}" - if current is None: - info.status = STATUS_NO_UPSTREAM - info.detail = "HEAD is not on a version tag" - return info - - info.current_version = current + else: + current = None + else: + current = None + # Always probe remote — even in dev mode, latest_version provides useful + # context ("origin has v2.0.2") for the detail pane. fetch_failed = False if _cached_latest is not None: remote_tags = [_cached_latest] @@ -231,6 +279,34 @@ def check_plugin( info.network_verified = True latest = _highest_semver_tag(remote_tags) if remote_tags else None + + # Branch mode → STATUS_LOCAL_DEV. We surface latest_version (when known) so + # the detail pane can show "origin has vX.Y.Z" but the table never claims + # this is a one-click upgrade. + if branch is not None: + info.status = STATUS_LOCAL_DEV + info.dev_branch = branch + ancestor = _get_ancestor_tag(path) + if ancestor: + info.current_version = ancestor + info.dev_commits_ahead = _get_commits_ahead(path, ancestor) + if latest is not None: + info.latest_version = latest + if ancestor and info.dev_commits_ahead: + info.detail = f"branch {branch} ({info.dev_commits_ahead} past {ancestor})" + elif ancestor: + info.detail = f"branch {branch} (at {ancestor})" + else: + info.detail = f"branch {branch} (no ancestor tag)" + return info + + if current is None: + info.status = STATUS_NO_UPSTREAM + info.detail = "HEAD is not on a version tag" + return info + + info.current_version = current + if latest is None: info.status = STATUS_NO_UPSTREAM info.detail = "no semver tags on origin" diff --git a/src/copilotsetup/tabs/plugins.py b/src/copilotsetup/tabs/plugins.py index b987f4c..a79ea66 100644 --- a/src/copilotsetup/tabs/plugins.py +++ b/src/copilotsetup/tabs/plugins.py @@ -104,9 +104,35 @@ def _apply_upgrades(self, result_map: dict, gen: int, provisional: bool = False) upgrade_summary=info.summary, upgrade_version=info.latest_version, upgrade_provisional=provisional, + dev_summary="", + dev_branch="", + dev_commits_ahead=0, + latest_release=info.latest_version, + ) + elif info and info.dev_summary: + item = replace( + item, + upgrade_available=False, + upgrade_summary="", + upgrade_version="", + upgrade_provisional=provisional, + dev_summary=info.dev_summary, + dev_branch=info.dev_branch, + dev_commits_ahead=info.dev_commits_ahead, + latest_release=info.latest_version, ) else: - item = replace(item, upgrade_summary="—", upgrade_provisional=provisional) + item = replace( + item, + upgrade_available=False, + upgrade_summary="—", + upgrade_version="", + upgrade_provisional=provisional, + dev_summary="", + dev_branch="", + dev_commits_ahead=0, + latest_release=(info.latest_version if info else ""), + ) new_items.append(item) self._items = new_items self._apply_filter() @@ -116,9 +142,14 @@ def key_for(self, item: PluginInfo) -> str: def row_for(self, item: PluginInfo) -> tuple: status: Status = item.status # type: ignore[assignment] - upgrade = item.upgrade_summary if item.upgrade_summary else "…" - if item.upgrade_provisional and item.upgrade_summary: - upgrade = f"{item.upgrade_summary} ⏳" + if item.upgrade_summary: + upgrade = item.upgrade_summary + elif item.dev_summary: + upgrade = item.dev_summary + else: + upgrade = "…" + if item.upgrade_provisional and (item.upgrade_summary or item.dev_summary): + upgrade = f"{upgrade} ⏳" return ( item.name, item.source, @@ -138,6 +169,14 @@ def detail_for(self, item: PluginInfo) -> str: if item.upgrade_summary: suffix = " [dim](cached)[/]" if item.upgrade_provisional else "" parts.append(f"[bold]Upgrade:[/] [green]{item.upgrade_summary}[/green]{suffix}") + elif item.dev_summary: + suffix = " [dim](cached)[/]" if item.upgrade_provisional else "" + parts.append(f"[bold]Local install:[/] [yellow]{item.dev_summary}[/yellow]{suffix}") + if item.dev_commits_ahead: + parts.append(f" [dim]{item.dev_commits_ahead} commit(s) past last ancestor tag[/dim]") + if item.latest_release: + parts.append(f" [dim]Latest release on origin:[/dim] {item.latest_release}") + parts.append(" [dim]To pin to a release: cd ; git checkout ; copilot plugin install .[/dim]") if item.reason: parts.append(f"[bold]Reason:[/] {item.reason}") if item.install_path: @@ -220,6 +259,18 @@ def handle_upgrade(self) -> None: if item is None: self.notify("No plugin selected", severity="warning", title="Upgrade") return + if item.dev_summary: + branch = item.dev_branch or "current branch" + target = item.latest_release or "" + self.notify( + f"{item.name}: local dev install on branch [bold]{branch}[/]. " + f"To pin to a release, run in the source repo: " + f"[bold]git checkout {target}; copilot plugin install .[/]", + severity="warning", + title="Upgrade", + timeout=12, + ) + return if not item.upgrade_available: self.notify( f"{item.name}: no upgrade available", diff --git a/src/copilotsetup/upgrade_cache.py b/src/copilotsetup/upgrade_cache.py index 6e2c955..7b4deb9 100644 --- a/src/copilotsetup/upgrade_cache.py +++ b/src/copilotsetup/upgrade_cache.py @@ -13,6 +13,7 @@ from copilotsetup.config import upgrade_cache_json from copilotsetup.plugin_upgrades import ( + STATUS_LOCAL_DEV, STATUS_UP_TO_DATE, STATUS_UPGRADABLE, PluginUpgradeInfo, @@ -99,7 +100,11 @@ def get_or_check( info = check_plugin(install_path, name, config_version, _cached_latest=cached_latest) if cached_latest is None: # Only cache when network was actually consulted. - cacheable = info.status == STATUS_UPGRADABLE or (info.status == STATUS_UP_TO_DATE and info.network_verified) + cacheable = ( + info.status == STATUS_UPGRADABLE + or (info.status == STATUS_UP_TO_DATE and info.network_verified) + or (info.status == STATUS_LOCAL_DEV and info.network_verified and bool(info.latest_version)) + ) if cacheable: version_to_cache = info.latest_version or info.current_version if version_to_cache: diff --git a/tests/test_plugin_upgrades.py b/tests/test_plugin_upgrades.py index 89a88b3..f4f87ec 100644 --- a/tests/test_plugin_upgrades.py +++ b/tests/test_plugin_upgrades.py @@ -187,16 +187,22 @@ def mock_run_git(args, cwd, *, timeout=30.0): assert "not on a version tag" in info.detail -# --- check_plugin: ancestor tag fallback --- +# --- check_plugin: dev-checkout (branch HEAD) --- -def test_check_plugin_ancestor_tag(tmp_path): - """When HEAD is ahead of a tag, --abbrev=0 should find the ancestor tag.""" +def test_check_plugin_dev_checkout_on_branch(tmp_path): + """When HEAD is on a branch (not a tag), status is STATUS_LOCAL_DEV — never STATUS_UPGRADABLE. + + This is the core bug fix: the previous behavior used `git describe --abbrev=0` + to fall back to an ancestor tag and falsely report the dev branch as upgradable. + """ + from copilotsetup.plugin_upgrades import STATUS_LOCAL_DEV def mock_run_git(args, cwd, *, timeout=30.0): from unittest.mock import MagicMock result = MagicMock() + result.stderr = "" if args[0] == "rev-parse": result.returncode = 0 result.stdout = "true" @@ -204,26 +210,176 @@ def mock_run_git(args, cwd, *, timeout=30.0): result.returncode = 0 result.stdout = "" elif args[0] == "describe" and "--exact-match" in args: + # HEAD is not on an exact tag (we're on a dev branch). result.returncode = 1 result.stdout = "" elif args[0] == "describe" and "--abbrev=0" in args: + # Nearest ancestor tag is v1.1.0. result.returncode = 0 result.stdout = "v1.1.0\n" + elif args[0] == "symbolic-ref": + # On branch feat/gh-writer. + result.returncode = 0 + result.stdout = "feat/gh-writer\n" + elif args[0] == "rev-list": + # 4 commits past v1.1.0. + result.returncode = 0 + result.stdout = "4\n" elif args[0] == "ls-remote": result.returncode = 0 - result.stdout = "abc123\trefs/tags/v1.1.0\ndef456\trefs/tags/v1.2.0\n" + result.stdout = "abc\trefs/tags/v1.1.0\ndef\trefs/tags/v2.0.2\n" else: result.returncode = 1 result.stdout = "" + return result + + with patch("copilotsetup.plugin_upgrades._run_git", side_effect=mock_run_git): + info = check_plugin(str(tmp_path), "test-plugin") + + assert info.status == STATUS_LOCAL_DEV + assert info.upgrade_available is False # critical: no fake upgrade arrow + assert info.dev_branch == "feat/gh-writer" + assert info.dev_commits_ahead == 4 + assert info.current_version == "v1.1.0" # for context only + assert info.latest_version == "v2.0.2" # surfaced for detail pane + assert info.summary == "" # no row-level "↑ vX.Y.Z" + assert info.dev_summary == "dev: feat/gh-writer" + + +def test_check_plugin_dev_checkout_no_ancestor_tag(tmp_path): + """Dev checkout with no ancestor tag still reports STATUS_LOCAL_DEV.""" + from copilotsetup.plugin_upgrades import STATUS_LOCAL_DEV + + def mock_run_git(args, cwd, *, timeout=30.0): + from unittest.mock import MagicMock + + result = MagicMock() result.stderr = "" + if args[0] == "rev-parse": + result.returncode = 0 + result.stdout = "true" + elif args[0] == "fetch": + result.returncode = 0 + result.stdout = "" + elif args[0] == "describe": + result.returncode = 1 + result.stdout = "" + elif args[0] == "symbolic-ref": + result.returncode = 0 + result.stdout = "main\n" + elif args[0] == "rev-list": + result.returncode = 1 + result.stdout = "" + elif args[0] == "ls-remote": + result.returncode = 0 + result.stdout = "" + else: + result.returncode = 1 + result.stdout = "" + return result + + with patch("copilotsetup.plugin_upgrades._run_git", side_effect=mock_run_git): + info = check_plugin(str(tmp_path), "test-plugin") + + assert info.status == STATUS_LOCAL_DEV + assert info.dev_branch == "main" + assert info.current_version == "" # no ancestor tag known + assert info.latest_version == "" + assert info.upgrade_available is False + + +def test_check_plugin_detached_on_exact_tag_still_works(tmp_path): + """Marketplace install: detached HEAD on an exact tag → normal upgrade flow.""" + + def mock_run_git(args, cwd, *, timeout=30.0): + from unittest.mock import MagicMock + + result = MagicMock() + result.stderr = "" + if args[0] == "rev-parse": + result.returncode = 0 + result.stdout = "true" + elif args[0] == "fetch": + result.returncode = 0 + result.stdout = "" + elif args[0] == "describe" and "--exact-match" in args: + result.returncode = 0 + result.stdout = "v1.0.0\n" + elif args[0] == "symbolic-ref": + # Detached HEAD — would never be called in this path, but be defensive. + result.returncode = 1 + result.stdout = "" + elif args[0] == "ls-remote": + result.returncode = 0 + result.stdout = "abc\trefs/tags/v1.0.0\ndef\trefs/tags/v2.0.0\n" + else: + result.returncode = 1 + result.stdout = "" return result with patch("copilotsetup.plugin_upgrades._run_git", side_effect=mock_run_git): info = check_plugin(str(tmp_path), "test-plugin") assert info.status == STATUS_UPGRADABLE - assert info.current_version == "v1.1.0" - assert info.latest_version == "v1.2.0" + assert info.current_version == "v1.0.0" + assert info.latest_version == "v2.0.0" + assert info.upgrade_available is True + assert info.dev_branch == "" + assert info.dev_summary == "" + + +def test_check_plugin_branch_at_exact_tag_still_dev(tmp_path): + """Regression: HEAD on a branch whose tip is also at an exact tag → STATUS_LOCAL_DEV. + + Even though ``git describe --exact-match`` succeeds (branch HEAD coincides + with a tag), ``copilot plugin update`` would do ``git pull `` rather + than ``git checkout ``, so we must NOT show a one-click upgrade + arrow. Branch state is authoritative. + """ + from copilotsetup.plugin_upgrades import STATUS_LOCAL_DEV + + def mock_run_git(args, cwd, *, timeout=30.0): + from unittest.mock import MagicMock + + result = MagicMock() + result.stderr = "" + if args[0] == "rev-parse": + result.returncode = 0 + result.stdout = "true" + elif args[0] == "fetch": + result.returncode = 0 + result.stdout = "" + elif args[0] == "symbolic-ref": + # On branch ``main`` whose HEAD coincides with v1.0.0. + result.returncode = 0 + result.stdout = "main\n" + elif args[0] == "describe" and "--exact-match" in args: + # If anything still asks for exact-match, HEAD is at v1.0.0. + result.returncode = 0 + result.stdout = "v1.0.0\n" + elif args[0] == "describe" and "--abbrev=0" in args: + result.returncode = 0 + result.stdout = "v1.0.0\n" + elif args[0] == "rev-list" and "--count" in args: + result.returncode = 0 + result.stdout = "0\n" + elif args[0] == "ls-remote": + result.returncode = 0 + result.stdout = "abc\trefs/tags/v1.0.0\ndef\trefs/tags/v2.0.0\n" + else: + result.returncode = 1 + result.stdout = "" + return result + + with patch("copilotsetup.plugin_upgrades._run_git", side_effect=mock_run_git): + info = check_plugin(str(tmp_path), "test-plugin") + + assert info.status == STATUS_LOCAL_DEV, ( + f"branch HEAD that happens to coincide with a tag must still be dev, got {info.status}" + ) + assert info.dev_branch == "main" + assert info.upgrade_available is False + assert info.latest_version == "v2.0.0" # --- check_all --- @@ -329,6 +485,9 @@ def test_check_plugin_cached_latest_skips_fetch(tmp_path): subprocess.run(["git", "add", "."], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "commit", "-m", "init"], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "tag", "v1.0.0"], cwd=str(tmp_path), capture_output=True) + # Detach HEAD onto the tag — this matches what ``copilot plugin install `` + # does in production. A repo left on a branch is now treated as STATUS_LOCAL_DEV. + subprocess.run(["git", "checkout", "v1.0.0"], cwd=str(tmp_path), capture_output=True) from copilotsetup.plugin_upgrades import check_plugin @@ -350,6 +509,7 @@ def test_check_plugin_cached_latest_up_to_date(tmp_path): subprocess.run(["git", "add", "."], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "commit", "-m", "init"], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "tag", "v1.0.0"], cwd=str(tmp_path), capture_output=True) + subprocess.run(["git", "checkout", "v1.0.0"], cwd=str(tmp_path), capture_output=True) from copilotsetup.plugin_upgrades import check_plugin @@ -372,6 +532,8 @@ def test_check_plugin_fetch_fails_uses_local_tags(tmp_path): subprocess.run(["git", "add", "."], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "commit", "-m", "init"], cwd=str(tmp_path), capture_output=True) subprocess.run(["git", "tag", "v1.0.0"], cwd=str(tmp_path), capture_output=True) + # Detach onto v1.0.0 so HEAD is at an exact tag (marketplace-style install). + subprocess.run(["git", "checkout", "v1.0.0"], cwd=str(tmp_path), capture_output=True) # Add a higher local tag (simulates a previous successful fetch) subprocess.run(["git", "tag", "v2.0.0"], cwd=str(tmp_path), capture_output=True) diff --git a/tests/test_plugins_tab.py b/tests/test_plugins_tab.py index b651b48..77e1d7f 100644 --- a/tests/test_plugins_tab.py +++ b/tests/test_plugins_tab.py @@ -204,3 +204,231 @@ def test_replace_preserves_provisional(self): item = _make_plugin(upgrade_provisional=True) updated = replace(item, upgrade_summary="↑ v2.0.0") assert updated.upgrade_provisional is True + + +class TestDevState: + """Tests for local-dev install rendering and behavior.""" + + def _make_tab(self, items: list[PluginInfo]) -> MagicMock: + from copilotsetup.tabs.plugins import PluginsTab + + tab = MagicMock(spec=PluginsTab) + tab._items = list(items) + tab._active_gen = 1 + tab._apply_filter = MagicMock() + tab._apply_upgrades = PluginsTab._apply_upgrades.__get__(tab, PluginsTab) + return tab + + def test_apply_upgrades_propagates_dev_fields(self): + """Dev-state results populate dev_summary, dev_branch, dev_commits_ahead, latest_release.""" + from copilotsetup.plugin_upgrades import STATUS_LOCAL_DEV + + item = _make_plugin() + tab = self._make_tab([item]) + result_map = { + "test-plugin": PluginUpgradeInfo( + name="test-plugin", + path=None, + status=STATUS_LOCAL_DEV, + dev_branch="feat/gh-writer", + dev_commits_ahead=4, + current_version="v1.1.0", + latest_version="v2.0.2", + ), + } + + tab._apply_upgrades(result_map, gen=1, provisional=False) + + updated = tab._items[0] + assert updated.upgrade_available is False + assert updated.upgrade_summary == "" + assert updated.dev_summary == "dev: feat/gh-writer" + assert updated.dev_branch == "feat/gh-writer" + assert updated.dev_commits_ahead == 4 + assert updated.latest_release == "v2.0.2" + + def test_apply_upgrades_clears_dev_when_pinning_to_tag(self): + """If a previously-dev item now reports STATUS_UPGRADABLE, clear dev fields.""" + item = _make_plugin( + dev_summary="dev: feat/gh-writer", + dev_branch="feat/gh-writer", + dev_commits_ahead=4, + ) + tab = self._make_tab([item]) + result_map = { + "test-plugin": PluginUpgradeInfo( + name="test-plugin", + path=None, + status="upgradable", + latest_version="v2.0.2", + ), + } + + tab._apply_upgrades(result_map, gen=1, provisional=False) + + updated = tab._items[0] + assert updated.upgrade_available is True + assert updated.dev_summary == "" + assert updated.dev_branch == "" + assert updated.dev_commits_ahead == 0 + + def test_row_shows_dev_summary_in_upgrade_column(self): + """Dev installs show 'dev: ' in the Upgrade column instead of an arrow.""" + from copilotsetup.tabs.plugins import PluginsTab + + item = _make_plugin( + upgrade_summary="", + dev_summary="dev: feat/gh-writer", + dev_branch="feat/gh-writer", + ) + + tab = MagicMock(spec=PluginsTab) + tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) + row = tab.row_for(item) + + assert "dev: feat/gh-writer" in str(row[4]) + assert "↑" not in str(row[4]) + + def test_row_shows_hourglass_for_provisional_dev(self): + """Provisional dev results also get the ⏳ marker.""" + from copilotsetup.tabs.plugins import PluginsTab + + item = _make_plugin( + upgrade_summary="", + dev_summary="dev: main", + dev_branch="main", + upgrade_provisional=True, + ) + + tab = MagicMock(spec=PluginsTab) + tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) + row = tab.row_for(item) + + assert "dev: main" in str(row[4]) + assert "⏳" in str(row[4]) + + def test_handle_upgrade_short_circuits_for_dev_install(self): + """Pressing 'u' on a dev install must NOT call copilot CLI.""" + from copilotsetup.tabs.plugins import PluginsTab + + item = _make_plugin( + dev_summary="dev: feat/gh-writer", + dev_branch="feat/gh-writer", + latest_release="v2.0.2", + ) + + tab = MagicMock(spec=PluginsTab) + tab.get_selected_item = MagicMock(return_value=item) + tab.notify = MagicMock() + tab.refresh_data = MagicMock() + tab.handle_upgrade = PluginsTab.handle_upgrade.__get__(tab, PluginsTab) + + from unittest.mock import patch as _patch + + with _patch("copilotsetup.tabs.plugins.run_copilot") as mock_run: + tab.handle_upgrade() + + mock_run.assert_not_called() + tab.refresh_data.assert_not_called() + # Notification mentions branch and the suggested git command. + assert tab.notify.called + call_args = tab.notify.call_args + msg = call_args[0][0] + assert "feat/gh-writer" in msg + assert "git checkout v2.0.2" in msg + assert "copilot plugin install" in msg + + def test_handle_upgrade_normal_path_unchanged(self): + """Non-dev plugins with upgrade_available still call the CLI.""" + from copilotsetup.tabs.plugins import PluginsTab + + item = _make_plugin( + upgrade_available=True, + upgrade_summary="↑ v2.0.0", + upgrade_version="v2.0.0", + ) + + tab = MagicMock(spec=PluginsTab) + tab.get_selected_item = MagicMock(return_value=item) + tab.notify = MagicMock() + tab.refresh_data = MagicMock() + tab.handle_upgrade = PluginsTab.handle_upgrade.__get__(tab, PluginsTab) + + from unittest.mock import patch as _patch + + mock_result = MagicMock() + mock_result.returncode = 0 + mock_result.stdout = "ok" + mock_result.stderr = "" + + with ( + _patch("copilotsetup.tabs.plugins.run_copilot", return_value=mock_result) as mock_run, + _patch("copilotsetup.upgrade_cache.UpgradeCache.get_instance"), + ): + tab.handle_upgrade() + + mock_run.assert_called_once() + assert mock_run.call_args[0][:3] == ("plugin", "update", "test-plugin") + + def test_apply_upgrades_clears_stale_upgrade_state_on_no_upgrade(self): + """Regression: a fresh ``no-upgrade`` result must clear stale upgrade_available. + + Previously, the fallback branch in ``_apply_upgrades`` only updated + ``upgrade_summary='—'`` but left ``upgrade_available=True`` and + ``upgrade_version='v2.0.0'`` from a stale provisional pass. That made + ``[u]`` still trigger ``run_copilot`` even though the row showed no + upgrade available. + """ + from copilotsetup.plugin_upgrades import STATUS_UP_TO_DATE + + # Stale provisional state: upgrade was previously available. + item = _make_plugin( + upgrade_available=True, + upgrade_summary="↑ v2.0.0", + upgrade_version="v2.0.0", + upgrade_provisional=True, + ) + tab = self._make_tab([item]) + # Fresh result says we're up to date — no upgrade. + result_map = { + "test-plugin": PluginUpgradeInfo( + name="test-plugin", + path=None, + status=STATUS_UP_TO_DATE, + latest_version="v1.0.0", + ), + } + + tab._apply_upgrades(result_map, gen=1, provisional=False) + + updated = tab._items[0] + assert updated.upgrade_summary == "—" + assert updated.upgrade_available is False, "stale upgrade_available must be cleared so [u] does not trigger CLI" + assert updated.upgrade_version == "", "stale upgrade_version must be cleared" + assert updated.upgrade_provisional is False + + def test_detail_shows_commits_past_tag_for_dev_install(self): + """Detail pane renders the 'N commit(s) past last ancestor tag' line for dev installs. + + Regression: the gate previously required ``upgrade_version`` to be + truthy, but dev installs intentionally clear that field, so the line + was never rendered in practice. + """ + from copilotsetup.tabs.plugins import PluginsTab + + item = _make_plugin( + upgrade_summary="", + upgrade_version="", # cleared by dev path + dev_summary="dev: feat/gh-writer", + dev_branch="feat/gh-writer", + dev_commits_ahead=4, + latest_release="v2.0.2", + ) + + tab = MagicMock(spec=PluginsTab) + tab.detail_for = PluginsTab.detail_for.__get__(tab, PluginsTab) + detail = tab.detail_for(item) + + assert "4 commit(s) past last ancestor tag" in detail + assert "Latest release on origin" in detail + assert "v2.0.2" in detail From ba93e32e38957e279fdffa25d1f747c5fd4d76fe Mon Sep 17 00:00:00 2001 From: Eric Hansen Date: Tue, 5 May 2026 22:24:53 -0500 Subject: [PATCH 2/4] feat: add Marketplace/Source/Repo columns and raw JSON detail toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the single 'Source' column in the Plugins tab with three config-faithful columns (Marketplace, Source, Repo) that mirror the actual config.json fields, making install method immediately visible. Add [j] key binding across all tabs to toggle the detail pane between the human-readable Rich markup view and a raw JSON dump of the underlying data — helpful for diagnosing config issues. Changes: - data/plugins.py: Add source_type, source_repo, raw_data fields - tabs/plugins.py: Update columns, row_for, detail_for - tabs/base.py: Add _detail_raw_mode toggle and toggle_json_view() - app.py: Add j binding -> action_toggle_json - All data/*.py: Add raw_data field to every frozen dataclass - Populate raw_data from JSON source where available - tests: Fix column index references for new layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilotsetup/app.py | 1 + src/copilotsetup/data/extensions.py | 6 ++- src/copilotsetup/data/hooks.py | 5 +- src/copilotsetup/data/lsp_servers.py | 4 +- src/copilotsetup/data/plugins.py | 16 +++++- src/copilotsetup/data/settings.py | 6 ++- src/copilotsetup/tabs/base.py | 37 +++++++++++++- src/copilotsetup/tabs/extensions.py | 2 +- src/copilotsetup/tabs/hooks.py | 2 +- src/copilotsetup/tabs/lsp_servers.py | 2 +- src/copilotsetup/tabs/mcp_servers.py | 2 +- src/copilotsetup/tabs/plugins.py | 18 +++++-- src/copilotsetup/tabs/settings.py | 2 +- src/copilotsetup/widgets/footer_bar.py | 1 + tests/test_data_plugins.py | 71 ++++++++++++++++++++++++++ tests/test_plugins_tab.py | 20 ++++---- 16 files changed, 167 insertions(+), 28 deletions(-) diff --git a/src/copilotsetup/app.py b/src/copilotsetup/app.py index 53a9f85..e2f9bb0 100644 --- a/src/copilotsetup/app.py +++ b/src/copilotsetup/app.py @@ -77,6 +77,7 @@ class CopilotSetupApp(App[None]): Binding("u", "tab_action('u')", "Upgrade", show=False), Binding("m", "tab_action('m')", "Marketplace", show=False), Binding("h", "tab_action('h')", "Health", show=False), + Binding("j", "tab_action('j')", "JSON", show=False), ] def compose(self) -> ComposeResult: diff --git a/src/copilotsetup/data/extensions.py b/src/copilotsetup/data/extensions.py index d1d545c..d3661f1 100644 --- a/src/copilotsetup/data/extensions.py +++ b/src/copilotsetup/data/extensions.py @@ -3,7 +3,7 @@ from __future__ import annotations import json -from dataclasses import dataclass +from dataclasses import dataclass, field from copilotsetup.config import extensions_dir @@ -15,6 +15,7 @@ class ExtensionInfo: name: str path: str = "" version: str = "" + raw_data: dict = field(default_factory=dict, hash=False, compare=False) class ExtensionProvider: @@ -29,6 +30,7 @@ def load(self) -> list[ExtensionInfo]: if not entry.is_dir(): continue version = "" + data: dict = {} pkg_json = entry / "package.json" if pkg_json.is_file(): try: @@ -36,5 +38,5 @@ def load(self) -> list[ExtensionInfo]: version = str(data.get("version", "") or "") except (json.JSONDecodeError, OSError): pass - result.append(ExtensionInfo(name=entry.name, path=str(entry), version=version)) + result.append(ExtensionInfo(name=entry.name, path=str(entry), version=version, raw_data=data)) return result diff --git a/src/copilotsetup/data/hooks.py b/src/copilotsetup/data/hooks.py index 5e2bc70..2a22904 100644 --- a/src/copilotsetup/data/hooks.py +++ b/src/copilotsetup/data/hooks.py @@ -3,7 +3,7 @@ from __future__ import annotations import logging -from dataclasses import dataclass +from dataclasses import dataclass, field from copilotsetup.config import config_json from copilotsetup.utils.file_io import read_json @@ -18,6 +18,7 @@ class HookInfo: event: str command: str hook_type: str = "command" + raw_data: dict = field(default_factory=dict, hash=False, compare=False) class HookProvider: @@ -40,5 +41,5 @@ def load(self) -> list[HookInfo]: command = entry.get("command", "") if not command: continue - result.append(HookInfo(event=str(event_name), command=str(command))) + result.append(HookInfo(event=str(event_name), command=str(command), raw_data=dict(entry))) return result diff --git a/src/copilotsetup/data/lsp_servers.py b/src/copilotsetup/data/lsp_servers.py index 5a7c044..2865afe 100644 --- a/src/copilotsetup/data/lsp_servers.py +++ b/src/copilotsetup/data/lsp_servers.py @@ -2,7 +2,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from copilotsetup.config import lsp_config_json from copilotsetup.platform_ops import validate_lsp_binary @@ -17,6 +17,7 @@ class LspInfo: command: str args: tuple[str, ...] = () binary_ok: bool = False + raw_data: dict = field(default_factory=dict, hash=False, compare=False) @property def status(self) -> str: @@ -52,6 +53,7 @@ def load(self) -> list[LspInfo]: command=command, args=tuple(str(a) for a in args), binary_ok=binary_ok, + raw_data=dict(entry), ) ) return result diff --git a/src/copilotsetup/data/plugins.py b/src/copilotsetup/data/plugins.py index 0099d07..4c52632 100644 --- a/src/copilotsetup/data/plugins.py +++ b/src/copilotsetup/data/plugins.py @@ -3,7 +3,7 @@ from __future__ import annotations import json -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from copilotsetup.config import config_json, installed_plugins_dir @@ -31,6 +31,8 @@ class PluginInfo: name: str source: str = "" + source_type: str = "" + source_repo: str = "" version: str = "" installed: bool = False disabled: bool = False @@ -47,6 +49,7 @@ class PluginInfo: dev_branch: str = "" dev_commits_ahead: int = 0 latest_release: str = "" + raw_data: dict = field(default_factory=dict, hash=False, compare=False) @property def status(self) -> str: @@ -128,10 +131,20 @@ def load(self) -> list[PluginInfo]: if f.is_file() and f.name.endswith(".agent.md") ] + # Extract source info from config.json source object + source_obj = entry.get("source") + source_type = "" + source_repo = "" + if isinstance(source_obj, dict): + source_type = str(source_obj.get("source", "")) + source_repo = str(source_obj.get("repo", "") or source_obj.get("path", "")) + result.append( PluginInfo( name=str(name), source=marketplace or "local", + source_type=source_type, + source_repo=source_repo, version=version, installed=True, disabled=not enabled, @@ -140,6 +153,7 @@ def load(self) -> list[PluginInfo]: bundled_skills=tuple(bundled_skills), bundled_servers=tuple(bundled_servers), bundled_agents=tuple(bundled_agents), + raw_data=dict(entry), ) ) return result diff --git a/src/copilotsetup/data/settings.py b/src/copilotsetup/data/settings.py index 20f6cba..1c5560c 100644 --- a/src/copilotsetup/data/settings.py +++ b/src/copilotsetup/data/settings.py @@ -4,7 +4,7 @@ import json import logging -from dataclasses import dataclass +from dataclasses import dataclass, field from copilotsetup.config import config_json from copilotsetup.utils.file_io import read_json @@ -37,6 +37,7 @@ class SettingInfo: display_name: str value: str value_type: str = "string" + raw_data: dict = field(default_factory=dict, hash=False, compare=False) @property def status(self) -> str: @@ -96,6 +97,7 @@ def load(self) -> list[SettingInfo]: display_name=flat_key, value=_format_value(sub_val), value_type=_value_type_for(sub_val), + raw_data={flat_key: sub_val}, ) ) elif isinstance(val, list): @@ -105,6 +107,7 @@ def load(self) -> list[SettingInfo]: display_name=key, value=_format_value(val), value_type="list", + raw_data={key: val}, ) ) elif isinstance(val, (str, bool, int, float)): @@ -114,6 +117,7 @@ def load(self) -> list[SettingInfo]: display_name=key, value=str(val), value_type=_value_type_for(val), + raw_data={key: val}, ) ) return result diff --git a/src/copilotsetup/tabs/base.py b/src/copilotsetup/tabs/base.py index 5ff7ce6..4ec3635 100644 --- a/src/copilotsetup/tabs/base.py +++ b/src/copilotsetup/tabs/base.py @@ -81,6 +81,7 @@ def __init__(self, tab_label: str = "", **kwargs: Any) -> None: self._current_filter: str = "" self._load_gen = itertools.count() self._active_gen: int = 0 + self._detail_raw_mode: bool = False # --- compose -------------------------------------------------------------- @@ -240,7 +241,10 @@ def _on_row_highlighted(self, event: DataTable.RowHighlighted) -> None: detail.show_empty() return item = self._filtered_items[event.cursor_row] - detail.set_content(self.detail_for(item)) + if self._detail_raw_mode: + detail.set_content(self._raw_detail_for(item)) + else: + detail.set_content(self.detail_for(item)) def get_selected_item(self) -> Any | None: """Return the domain object for the currently highlighted row.""" @@ -250,6 +254,36 @@ def get_selected_item(self) -> Any | None: return self._filtered_items[idx] return None + def _raw_detail_for(self, item: Any) -> str: + """Return pretty-printed JSON of the item's raw_data or raw_entry.""" + import json as _json + + raw = getattr(item, "raw_data", None) or getattr(item, "raw_entry", None) + if not raw: + return "[dim]No raw data available[/]" + try: + text = _json.dumps(raw, indent=2, default=str, ensure_ascii=False) + except (TypeError, ValueError): + text = repr(raw) + # Escape Rich markup characters + text = text.replace("[", "\\[") + return f"[bold reverse] RAW JSON [/bold reverse]\n\n{text}" + + def toggle_json_view(self) -> None: + """Toggle between human-readable and raw JSON detail views.""" + item = self.get_selected_item() + if item is None: + return + self._detail_raw_mode = not self._detail_raw_mode + try: + detail = self.query_one("#detail-pane", DetailPane) + except Exception: + return + if self._detail_raw_mode: + detail.set_content(self._raw_detail_for(item)) + else: + detail.set_content(self.detail_for(item)) + # --- action dispatch ------------------------------------------------------ _ACTION_MAP: ClassVar[dict[str, str]] = { @@ -260,6 +294,7 @@ def get_selected_item(self) -> Any | None: "u": "handle_upgrade", "m": "handle_marketplace", "h": "handle_health", + "j": "toggle_json_view", } def dispatch_action(self, key: str) -> None: diff --git a/src/copilotsetup/tabs/extensions.py b/src/copilotsetup/tabs/extensions.py index 642768d..ae15fb4 100644 --- a/src/copilotsetup/tabs/extensions.py +++ b/src/copilotsetup/tabs/extensions.py @@ -11,7 +11,7 @@ class ExtensionsTab(BaseTab): tab_name = "Extensions" columns: ClassVar[list[tuple[str, int]]] = [("Name", 30), ("Version", 15), ("Path", 50)] - available_actions: ClassVar[list[str]] = [] + available_actions: ClassVar[list[str]] = ["j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) diff --git a/src/copilotsetup/tabs/hooks.py b/src/copilotsetup/tabs/hooks.py index 94006c2..8174fb9 100644 --- a/src/copilotsetup/tabs/hooks.py +++ b/src/copilotsetup/tabs/hooks.py @@ -15,7 +15,7 @@ class HooksTab(BaseTab): ("Command", 50), ("Type", 10), ] - available_actions: ClassVar[list[str]] = [] + available_actions: ClassVar[list[str]] = ["j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) diff --git a/src/copilotsetup/tabs/lsp_servers.py b/src/copilotsetup/tabs/lsp_servers.py index 607827a..58bfcd8 100644 --- a/src/copilotsetup/tabs/lsp_servers.py +++ b/src/copilotsetup/tabs/lsp_servers.py @@ -17,7 +17,7 @@ class LspServersTab(BaseTab): ("Status", 10), ("Reason", 25), ] - available_actions: ClassVar[list[str]] = [] + available_actions: ClassVar[list[str]] = ["j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) diff --git a/src/copilotsetup/tabs/mcp_servers.py b/src/copilotsetup/tabs/mcp_servers.py index 38f2c1e..3752be1 100644 --- a/src/copilotsetup/tabs/mcp_servers.py +++ b/src/copilotsetup/tabs/mcp_servers.py @@ -22,7 +22,7 @@ class McpServersTab(BaseTab): ("Health", 10), ("Reason", 25), ] - available_actions: ClassVar[list[str]] = ["a", "x", "h"] + available_actions: ClassVar[list[str]] = ["a", "x", "h", "j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) diff --git a/src/copilotsetup/tabs/plugins.py b/src/copilotsetup/tabs/plugins.py index a79ea66..1fcb564 100644 --- a/src/copilotsetup/tabs/plugins.py +++ b/src/copilotsetup/tabs/plugins.py @@ -17,13 +17,15 @@ class PluginsTab(BaseTab): tab_name = "Plugins" columns: ClassVar[list[tuple[str, int]]] = [ ("Name", 20), - ("Source", 12), + ("Marketplace", 16), + ("Source", 8), + ("Repo", 28), ("Version", 10), ("Status", 10), ("Upgrade", 12), ("Reason", 20), ] - available_actions: ClassVar[list[str]] = ["a", "x", "t", "u"] + available_actions: ClassVar[list[str]] = ["a", "x", "t", "u", "j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) @@ -152,7 +154,9 @@ def row_for(self, item: PluginInfo) -> tuple: upgrade = f"{upgrade} ⏳" return ( item.name, - item.source, + item.marketplace or "—", + item.source_type or "—", + item.source_repo or "—", item.version, status_cell(status), upgrade, @@ -162,7 +166,9 @@ def row_for(self, item: PluginInfo) -> tuple: def detail_for(self, item: PluginInfo) -> str: parts = [ f"[bold]Name:[/] {item.name}", - f"[bold]Source:[/] {item.source}", + f"[bold]Marketplace:[/] {item.marketplace or '—'}", + f"[bold]Source:[/] {item.source_type or '—'}", + f"[bold]Repo:[/] {item.source_repo or '—'}", f"[bold]Version:[/] {item.version or '(unknown)'}", f"[bold]Status:[/] {item.status}", ] @@ -176,7 +182,9 @@ def detail_for(self, item: PluginInfo) -> str: parts.append(f" [dim]{item.dev_commits_ahead} commit(s) past last ancestor tag[/dim]") if item.latest_release: parts.append(f" [dim]Latest release on origin:[/dim] {item.latest_release}") - parts.append(" [dim]To pin to a release: cd ; git checkout ; copilot plugin install .[/dim]") + path = item.install_path or "" + tag = item.latest_release or "" + parts.append(f" [dim]To pin to a release: cd {path}; git checkout {tag}; copilot plugin install .[/dim]") if item.reason: parts.append(f"[bold]Reason:[/] {item.reason}") if item.install_path: diff --git a/src/copilotsetup/tabs/settings.py b/src/copilotsetup/tabs/settings.py index b8da587..5c6d447 100644 --- a/src/copilotsetup/tabs/settings.py +++ b/src/copilotsetup/tabs/settings.py @@ -45,7 +45,7 @@ class SettingsTab(BaseTab): ("Value", 25), ("Type", 10), ] - available_actions: ClassVar[list[str]] = ["e"] + available_actions: ClassVar[list[str]] = ["e", "j"] def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) diff --git a/src/copilotsetup/widgets/footer_bar.py b/src/copilotsetup/widgets/footer_bar.py index 0707a40..ed8553c 100644 --- a/src/copilotsetup/widgets/footer_bar.py +++ b/src/copilotsetup/widgets/footer_bar.py @@ -19,6 +19,7 @@ "u": "Upgrade", "m": "Marketplace", "h": "Health", + "j": "JSON", } diff --git a/tests/test_data_plugins.py b/tests/test_data_plugins.py index 1805806..179aad1 100644 --- a/tests/test_data_plugins.py +++ b/tests/test_data_plugins.py @@ -252,3 +252,74 @@ def test_set_plugin_enabled_with_jsonc_config(tmp_path, monkeypatch): assert set_plugin_enabled("p", False) is True reloaded = json.loads((tmp_path / "config.json").read_text(encoding="utf-8")) assert reloaded["installedPlugins"][0]["enabled"] is False + + +# ── source_type, source_repo, raw_data tests ───────────────────────── + + +def test_marketplace_plugin_source_fields(tmp_path, monkeypatch): + """Marketplace installs have empty source_type/source_repo.""" + monkeypatch.setenv("COPILOT_HOME", str(tmp_path)) + cfg = {"installedPlugins": [{"name": "m", "marketplace": "copilot-config", "version": "1.0.0"}]} + (tmp_path / "config.json").write_text(json.dumps(cfg)) + items = PluginProvider().load() + assert items[0].source_type == "" + assert items[0].source_repo == "" + + +def test_github_direct_source_fields(tmp_path, monkeypatch): + """GitHub-direct installs populate source_type='github' and source_repo.""" + monkeypatch.setenv("COPILOT_HOME", str(tmp_path)) + cfg = { + "installedPlugins": [ + { + "name": "g", + "marketplace": "", + "source": {"source": "github", "repo": "owner/repo"}, + } + ] + } + (tmp_path / "config.json").write_text(json.dumps(cfg)) + items = PluginProvider().load() + assert items[0].source_type == "github" + assert items[0].source_repo == "owner/repo" + + +def test_local_path_source_fields(tmp_path, monkeypatch): + """Local-path installs populate source_type='local' and source_repo=path.""" + monkeypatch.setenv("COPILOT_HOME", str(tmp_path)) + cfg = { + "installedPlugins": [ + { + "name": "lp", + "marketplace": "", + "source": {"source": "local", "path": "/abs/path"}, + } + ] + } + (tmp_path / "config.json").write_text(json.dumps(cfg)) + items = PluginProvider().load() + assert items[0].source_type == "local" + assert items[0].source_repo == "/abs/path" + + +def test_bare_string_source_stays_empty(tmp_path, monkeypatch): + """A bare string source value (not a dict) leaves fields empty.""" + monkeypatch.setenv("COPILOT_HOME", str(tmp_path)) + cfg = {"installedPlugins": [{"name": "bs", "marketplace": "", "source": "just-a-string"}]} + (tmp_path / "config.json").write_text(json.dumps(cfg)) + items = PluginProvider().load() + assert items[0].source_type == "" + assert items[0].source_repo == "" + + +def test_raw_data_populated(tmp_path, monkeypatch): + """raw_data should contain the original config dict.""" + monkeypatch.setenv("COPILOT_HOME", str(tmp_path)) + entry = {"name": "rd", "marketplace": "copilot", "version": "2.0.0"} + cfg = {"installedPlugins": [entry]} + (tmp_path / "config.json").write_text(json.dumps(cfg)) + items = PluginProvider().load() + assert items[0].raw_data["name"] == "rd" + assert items[0].raw_data["marketplace"] == "copilot" + assert items[0].raw_data["version"] == "2.0.0" diff --git a/tests/test_plugins_tab.py b/tests/test_plugins_tab.py index 77e1d7f..dd8d34c 100644 --- a/tests/test_plugins_tab.py +++ b/tests/test_plugins_tab.py @@ -152,8 +152,8 @@ def test_row_shows_hourglass_for_provisional(self): row = tab.row_for(item) # Upgrade column is index 4 - assert "⏳" in str(row[4]) - assert "↑ v2.0.0" in str(row[4]) + assert "⏳" in str(row[6]) + assert "↑ v2.0.0" in str(row[6]) def test_row_no_hourglass_for_fresh(self): """Fresh (non-provisional) results should not show ⏳.""" @@ -169,8 +169,8 @@ def test_row_no_hourglass_for_fresh(self): tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) row = tab.row_for(item) - assert "⏳" not in str(row[4]) - assert "↑ v2.0.0" in str(row[4]) + assert "⏳" not in str(row[6]) + assert "↑ v2.0.0" in str(row[6]) def test_row_shows_hourglass_for_provisional_no_upgrade(self): """Provisional 'no upgrade' should also show ⏳ suffix.""" @@ -185,8 +185,8 @@ def test_row_shows_hourglass_for_provisional_no_upgrade(self): tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) row = tab.row_for(item) - assert "⏳" in str(row[4]) - assert "—" in str(row[4]) + assert "⏳" in str(row[6]) + assert "—" in str(row[6]) class TestPluginInfoProvisionalField: @@ -286,8 +286,8 @@ def test_row_shows_dev_summary_in_upgrade_column(self): tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) row = tab.row_for(item) - assert "dev: feat/gh-writer" in str(row[4]) - assert "↑" not in str(row[4]) + assert "dev: feat/gh-writer" in str(row[6]) + assert "↑" not in str(row[6]) def test_row_shows_hourglass_for_provisional_dev(self): """Provisional dev results also get the ⏳ marker.""" @@ -304,8 +304,8 @@ def test_row_shows_hourglass_for_provisional_dev(self): tab.row_for = PluginsTab.row_for.__get__(tab, PluginsTab) row = tab.row_for(item) - assert "dev: main" in str(row[4]) - assert "⏳" in str(row[4]) + assert "dev: main" in str(row[6]) + assert "⏳" in str(row[6]) def test_handle_upgrade_short_circuits_for_dev_install(self): """Pressing 'u' on a dev install must NOT call copilot CLI.""" From 1838fbb7062c7d015df228af473b346920d1aea3 Mon Sep 17 00:00:00 2001 From: Eric Hansen Date: Tue, 5 May 2026 23:15:50 -0500 Subject: [PATCH 3/4] fix(mcp): update Health column and detail pane after probe completes Previously the health probe only showed a toast notification but never wrote the result back to the table. Now _update_health replaces the frozen item, updates the cell via update_cell, and refreshes the detail pane if the probed row is selected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilotsetup/tabs/mcp_servers.py | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/copilotsetup/tabs/mcp_servers.py b/src/copilotsetup/tabs/mcp_servers.py index 3752be1..4e8581e 100644 --- a/src/copilotsetup/tabs/mcp_servers.py +++ b/src/copilotsetup/tabs/mcp_servers.py @@ -4,6 +4,8 @@ import os import shlex +from contextlib import suppress +from dataclasses import replace from typing import Any, ClassVar from copilotsetup.data.mcp_servers import McpServerInfo, McpServerProvider @@ -172,6 +174,7 @@ def _probe() -> None: result = probe_server_entry(item.name, dict(item.raw_entry)) latency = f" ({result.latency_ms}ms)" if result.latency_ms else "" detail = f": {result.detail}" if result.detail else "" + latency_str = f"{result.latency_ms}ms" if result.latency_ms else "" if result.health == "ok": msg = f"[bold]{item.name}[/] — ✓ ok{latency}{detail}" self.app.call_from_thread(self.notify, msg, title="Health") @@ -183,6 +186,7 @@ def _probe() -> None: severity="error", title="Health", ) + self.app.call_from_thread(self._update_health, item.name, result.health, latency_str) except Exception as exc: self.app.call_from_thread( self.notify, @@ -192,3 +196,31 @@ def _probe() -> None: ) threading.Thread(target=_probe, daemon=True).start() + + def _update_health(self, name: str, health: str, latency: str) -> None: + """Write probe result back into the data list and table cell.""" + from textual.widgets import DataTable + + for lst in (self._items, self._filtered_items): + for i, it in enumerate(lst): + if it.name == name: + lst[i] = replace(it, health=health, health_latency=latency) + break + + table = self.query_one("#tab-table", DataTable) + with suppress(Exception): + table.update_cell(name, "Health", health or "—") + + # Refresh detail pane if this row is currently selected + selected = self.get_selected_item() + if selected and selected.name == name: + from copilotsetup.widgets.detail_pane import DetailPane + + try: + detail = self.query_one("#detail-pane", DetailPane) + if self._detail_raw_mode: + detail.set_content(self._raw_detail_for(selected)) + else: + detail.set_content(self.detail_for(selected)) + except Exception: + pass From 558badfcc6230d61f364c59cf64b8e62cdee94ce Mon Sep 17 00:00:00 2001 From: Eric Hansen Date: Tue, 5 May 2026 23:20:20 -0500 Subject: [PATCH 4/4] fix: pass explicit column keys so update_cell works add_column(key=header) gives each column a stable key matching its header string. Without this, Textual auto-generates anonymous keys and update_cell('Health', ...) silently fails via suppress(Exception). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilotsetup/tabs/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copilotsetup/tabs/base.py b/src/copilotsetup/tabs/base.py index 4ec3635..3174781 100644 --- a/src/copilotsetup/tabs/base.py +++ b/src/copilotsetup/tabs/base.py @@ -99,7 +99,7 @@ def compose(self) -> ComposeResult: def on_mount(self) -> None: table = self.query_one("#tab-table", DataTable) for header, width in self.columns: - table.add_column(header, width=width) + table.add_column(header, width=width, key=header) table.loading = True self.query_one("#empty-state", Label).display = False self._start_load()