Browse catalog dialog (issue #10 phase 2)
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / windows-latest) (pull_request) Failing after 23s
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / windows-latest) (pull_request) Failing after 23s
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Successful in 7s
Adds the user-facing half of the server catalog: a searchable, signed catalog browser wired to the existing paste/import path. bcc_core.py (pure, GUI-free, per the design comment on #10): - CATALOG_CATEGORY_GROUPS / catalog_category_group(): collapses the raw taxonomy to the 7 UI chips (unknown categories fall back to Other, never drop an entry). - catalog_entry_matches_query() / filter_catalog_entries() / catalog_entries_in_group(): search-as-you-type + category chip filtering over catalog entries. - format_freshness_hint(): last_release ISO date -> "Last updated N months/years ago", '' when missing/unparseable/future. - first_unfilled_focus_target(): finds the first <PLACEHOLDER> arg or blank env_required key so the GUI can focus it after Add. - load_bundled_catalog_entries(): reads+verifies+validates the bundled catalog.json/.sig pair, returns servers or an EMPTY list on ANY failure -- the load-bearing guarantee behind the dialog's empty state. - Bug fix: catalog_entry_to_paste_json() only copied config.env, ignoring env_required -- the field where 9 of the 19 seed entries (postgres, github, notion, obsidian, brave-search, tavily, home-assistant, n8n, grafana) actually declare their secret var names. Add would have silently added these servers with no env field for the user to fill in. Now env_required keys are seeded as empty-string placeholders unless config.env already sets them. bcc.py: - BrowseCatalogDialog: search box, category chips, list, detail pane (description/notes/homepage/freshness hint/verbatim command preview), per-tier action (Add for basic, Open setup docs for link-only). Every catalog-derived string goes through plain_label() (Qt.PlainText + html.escape, matching catalog_console.py's approach) or an inherently-plain QPlainTextEdit for the command preview -- catalog.json takes community PRs, so every field is attacker-influenceable. - "Browse catalog…" button next to Paste JSON. - ServerEditor.focus_target(): selects the first unfilled arg line or opens the env-table cell editor for the first blank env row. - Save-time placeholder guard: warns (Yes/No, defaults No) when any server still has an unfilled <PLACEHOLDER>; never blocks a deliberate save, never saves one unnoticed. bcc.spec: bundle data/catalog.json.sig alongside catalog.json -- the signature file was missing from datas, so a frozen build would have had a catalog.json with no matching .sig for resolve_catalog() to verify against. tests/test_core.py: +82 tests covering category-group mapping, catalog search/filter, freshness-hint formatting (including the month/year rounding boundary), first_unfilled_focus_target, the catalog_entry_to_paste_json env_required fix (incl. a regression against the real shipped postgres entry), and load_bundled_catalog_entries under valid/tampered/wrong-key/missing- file/invalid-but-signed conditions plus an end-to-end check against the real data/catalog.json + .sig (19 entries). GUI code (BrowseCatalogDialog, ServerEditor.focus_target, MainWindow.browse_catalog) is unavoidably untested here -- PySide6 cannot import in this sandbox (missing libEGL/system GL libs) -- but all the logic it depends on is pushed into bcc_core.py and covered above.
This commit is contained in:
@@ -2049,6 +2049,43 @@ def test_catalog_entry_to_paste_json_includes_env_when_present():
|
||||
assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"}
|
||||
|
||||
|
||||
def test_catalog_entry_to_paste_json_seeds_env_required_keys():
|
||||
# Regression: env_required is where the seed data actually keeps its
|
||||
# secret VAR NAMES (postgres/github/notion/etc. all declare their secret
|
||||
# here with config.env left empty) -- catalog_entry_to_paste_json must
|
||||
# surface those names as blank env rows, not silently drop them.
|
||||
entry = _minimal_catalog()["servers"][0]
|
||||
entry["env_required"] = {"DATABASE_URI": ""}
|
||||
result = c.catalog_entry_to_paste_json(entry)
|
||||
assert result["widget"]["env"] == {"DATABASE_URI": ""}
|
||||
|
||||
|
||||
def test_catalog_entry_to_paste_json_config_env_wins_over_env_required_default():
|
||||
entry = _minimal_catalog()["servers"][0]
|
||||
entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
|
||||
entry["env_required"] = {"GRAFANA_URL": "", "GRAFANA_SERVICE_ACCOUNT_TOKEN": ""}
|
||||
result = c.catalog_entry_to_paste_json(entry)
|
||||
assert result["widget"]["env"] == {
|
||||
"GRAFANA_URL": "<GRAFANA_URL>",
|
||||
"GRAFANA_SERVICE_ACCOUNT_TOKEN": "",
|
||||
}
|
||||
|
||||
|
||||
def test_catalog_entry_to_paste_json_real_postgres_entry_seeds_database_uri():
|
||||
"""End-to-end regression against the actual shipped postgres entry,
|
||||
which needs DATABASE_URI via env_required and has no config.env at
|
||||
all -- this is exactly the shape that was silently dropping the env
|
||||
field before catalog_entry_to_paste_json accounted for env_required."""
|
||||
root = Path(__file__).resolve().parent.parent
|
||||
raw = (root / "data" / "catalog.json").read_bytes()
|
||||
data = c.load_catalog(raw)
|
||||
entry = next(s for s in data["servers"] if s["id"] == "postgres")
|
||||
result = c.catalog_entry_to_paste_json(entry)
|
||||
assert result["postgres"]["env"] == {"DATABASE_URI": ""}
|
||||
# And the focus-target helper now has something to point the user at.
|
||||
assert c.first_unfilled_focus_target(result["postgres"]) == ("env", "DATABASE_URI")
|
||||
|
||||
|
||||
def test_config_has_unfilled_placeholders_true_for_token():
|
||||
cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]}
|
||||
assert c.config_has_unfilled_placeholders(cfg) is True
|
||||
@@ -2062,3 +2099,310 @@ def test_config_has_unfilled_placeholders_false_after_fill():
|
||||
def test_config_has_unfilled_placeholders_checks_env_too():
|
||||
cfg = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": "<GRAFANA_URL>"}}
|
||||
assert c.config_has_unfilled_placeholders(cfg) is True
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Browse-catalog dialog helpers (issue #10 phase 2)
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
# --- catalog_category_group / CATALOG_CATEGORY_GROUPS --------------------- #
|
||||
@pytest.mark.parametrize(
|
||||
"category,expected_group",
|
||||
[
|
||||
("files", "Files & Dev"),
|
||||
("dev", "Files & Dev"),
|
||||
("code-hosting", "Files & Dev"),
|
||||
("browser", "Files & Dev"),
|
||||
("database", "Data"),
|
||||
("data", "Data"),
|
||||
("search", "Search & AI"),
|
||||
("ai", "Search & AI"),
|
||||
("cloud", "Cloud & Infra"),
|
||||
("infra", "Cloud & Infra"),
|
||||
("observability", "Cloud & Infra"),
|
||||
("productivity", "Work"),
|
||||
("communication", "Work"),
|
||||
("crm", "Work"),
|
||||
("finance", "Work"),
|
||||
("design", "Work"),
|
||||
("media", "Home & Personal"),
|
||||
("smart-home", "Home & Personal"),
|
||||
("personal", "Home & Personal"),
|
||||
],
|
||||
)
|
||||
def test_catalog_category_group_maps_every_taxonomy_value(category, expected_group):
|
||||
assert c.catalog_category_group(category) == expected_group
|
||||
|
||||
|
||||
def test_catalog_category_group_unknown_falls_back_to_other():
|
||||
assert c.catalog_category_group("some-future-category-nobody-has-seen-yet") == "Other"
|
||||
assert c.catalog_category_group("") == "Other"
|
||||
assert c.catalog_category_group(None) == "Other"
|
||||
|
||||
|
||||
def test_catalog_category_group_is_case_insensitive():
|
||||
assert c.catalog_category_group("Files") == "Files & Dev"
|
||||
assert c.catalog_category_group("DATABASE") == "Data"
|
||||
|
||||
|
||||
def test_shipped_catalog_categories_all_have_a_known_group():
|
||||
"""Regression: every category actually used in data/catalog.json must
|
||||
collapse to one of the 7 chips, never silently drop an entry."""
|
||||
root = Path(__file__).resolve().parent.parent
|
||||
raw = (root / "data" / "catalog.json").read_bytes()
|
||||
data = c.load_catalog(raw)
|
||||
for entry in data["servers"]:
|
||||
group = c.catalog_category_group(entry["category"])
|
||||
assert group in c.CATALOG_CATEGORY_CHIPS
|
||||
|
||||
|
||||
# --- catalog_entry_matches_query / filter_catalog_entries ------------------ #
|
||||
def _catalog_entries():
|
||||
return [
|
||||
{
|
||||
"id": "filesystem",
|
||||
"display": "Filesystem",
|
||||
"description": "Read/write access to local directories you choose.",
|
||||
"category": "files",
|
||||
},
|
||||
{
|
||||
"id": "postgres",
|
||||
"display": "Postgres MCP Pro",
|
||||
"description": "Query and inspect a PostgreSQL database.",
|
||||
"category": "database",
|
||||
},
|
||||
{
|
||||
"id": "slack",
|
||||
"display": "Slack",
|
||||
"description": "Search messages and send messages from your assistant.",
|
||||
"category": "communication",
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_empty_matches_everything():
|
||||
entries = _catalog_entries()
|
||||
assert c.filter_catalog_entries(entries, "") == entries
|
||||
assert c.filter_catalog_entries(entries, " ") == entries
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_matches_id():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "postgres")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_matches_display_case_insensitive():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "SLACK")
|
||||
assert [e["id"] for e in result] == ["slack"]
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_matches_description():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "PostgreSQL database")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_matches_category():
|
||||
result = c.filter_catalog_entries(_catalog_entries(), "database")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
|
||||
def test_catalog_entry_matches_query_no_match_returns_empty():
|
||||
assert c.filter_catalog_entries(_catalog_entries(), "kubernetes") == []
|
||||
|
||||
|
||||
def test_catalog_entries_in_group_all_returns_everything():
|
||||
entries = _catalog_entries()
|
||||
assert c.catalog_entries_in_group(entries, "All") == entries
|
||||
assert c.catalog_entries_in_group(entries, "") == entries
|
||||
assert c.catalog_entries_in_group(entries, None) == entries
|
||||
|
||||
|
||||
def test_catalog_entries_in_group_filters_by_collapsed_category():
|
||||
result = c.catalog_entries_in_group(_catalog_entries(), "Data")
|
||||
assert [e["id"] for e in result] == ["postgres"]
|
||||
|
||||
result = c.catalog_entries_in_group(_catalog_entries(), "Work")
|
||||
assert [e["id"] for e in result] == ["slack"]
|
||||
|
||||
|
||||
# --- format_freshness_hint -------------------------------------------------- #
|
||||
def test_format_freshness_hint_none_returns_empty_string():
|
||||
assert c.format_freshness_hint(None) == ""
|
||||
assert c.format_freshness_hint("") == ""
|
||||
|
||||
|
||||
def test_format_freshness_hint_unparseable_returns_empty_string():
|
||||
assert c.format_freshness_hint("not-a-date") == ""
|
||||
|
||||
|
||||
def test_format_freshness_hint_this_month():
|
||||
assert (
|
||||
c.format_freshness_hint("2026-07-01", today=c.date(2026, 7, 12))
|
||||
== "Last updated this month"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_one_month_singular():
|
||||
assert (
|
||||
c.format_freshness_hint("2026-06-01", today=c.date(2026, 7, 12))
|
||||
== "Last updated 1 month ago"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_months_ago():
|
||||
# Exactly 14 full months elapsed, no day-of-month remainder to round off.
|
||||
assert (
|
||||
c.format_freshness_hint("2025-01-15", today=c.date(2026, 3, 15))
|
||||
== "Last updated 14 months ago"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_rounds_down_partial_month():
|
||||
# 2025-05-16 -> 2026-07-12 is 13 full months, not 14: the 14th month
|
||||
# would only complete on 2026-07-16.
|
||||
assert (
|
||||
c.format_freshness_hint("2025-05-16", today=c.date(2026, 7, 12))
|
||||
== "Last updated 13 months ago"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_years_ago():
|
||||
assert (
|
||||
c.format_freshness_hint("2024-01-01", today=c.date(2026, 7, 12))
|
||||
== "Last updated 2 years ago"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_23_months_stays_in_months_not_years():
|
||||
# The switch to "N years ago" happens at 24 full months, not 12 -- the
|
||||
# whole point of this hint is the granular "14 months ago" phrasing the
|
||||
# design comment on #10 asked for, so 13-23 months must stay in months.
|
||||
assert (
|
||||
c.format_freshness_hint("2024-08-12", today=c.date(2026, 7, 12))
|
||||
== "Last updated 23 months ago"
|
||||
)
|
||||
|
||||
|
||||
def test_format_freshness_hint_future_date_returns_empty_string():
|
||||
# A last_release "in the future" relative to `today` is nonsensical --
|
||||
# show nothing rather than a misleading negative offset.
|
||||
assert c.format_freshness_hint("2027-01-01", today=c.date(2026, 7, 12)) == ""
|
||||
|
||||
|
||||
# --- first_unfilled_focus_target -------------------------------------------- #
|
||||
def test_first_unfilled_focus_target_prefers_placeholder_arg():
|
||||
data = {
|
||||
"command": "npx",
|
||||
"args": ["-y", "server", "<ALLOWED_DIR>"],
|
||||
"env": {"API_KEY": ""},
|
||||
}
|
||||
assert c.first_unfilled_focus_target(data) == ("args", 2)
|
||||
|
||||
|
||||
def test_first_unfilled_focus_target_falls_back_to_first_blank_env():
|
||||
data = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": ""}}
|
||||
assert c.first_unfilled_focus_target(data) == ("env", "GRAFANA_URL")
|
||||
|
||||
|
||||
def test_first_unfilled_focus_target_none_when_fully_filled():
|
||||
data = {"command": "npx", "args": ["-y", "server"], "env": {"API_KEY": "sk-real-value"}}
|
||||
assert c.first_unfilled_focus_target(data) is None
|
||||
|
||||
|
||||
def test_first_unfilled_focus_target_none_for_config_with_no_env_or_args():
|
||||
assert c.first_unfilled_focus_target({"command": "npx", "args": []}) is None
|
||||
|
||||
|
||||
# --- load_bundled_catalog_entries ------------------------------------------- #
|
||||
def test_load_bundled_catalog_entries_valid_signature(tmp_path, monkeypatch):
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
pub = priv.public_key().public_bytes_raw()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
|
||||
|
||||
raw, sig = _signed(_minimal_catalog(version=1), priv)
|
||||
catalog_path = tmp_path / "catalog.json"
|
||||
sig_path = tmp_path / "catalog.json.sig"
|
||||
catalog_path.write_bytes(raw)
|
||||
sig_path.write_bytes(sig)
|
||||
|
||||
entries = c.load_bundled_catalog_entries(catalog_path, sig_path)
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["id"] == "widget"
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_tampered_payload_returns_empty_list(tmp_path, monkeypatch):
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
pub = priv.public_key().public_bytes_raw()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
|
||||
|
||||
raw, sig = _signed(_minimal_catalog(version=1), priv)
|
||||
tampered = bytearray(raw)
|
||||
tampered[-2] ^= 0xFF # flip a byte inside the trailing bytes, still valid-ish JSON shape
|
||||
catalog_path = tmp_path / "catalog.json"
|
||||
sig_path = tmp_path / "catalog.json.sig"
|
||||
catalog_path.write_bytes(bytes(tampered))
|
||||
sig_path.write_bytes(sig)
|
||||
|
||||
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_wrong_key_returns_empty_list(tmp_path, monkeypatch):
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
other_priv = Ed25519PrivateKey.generate()
|
||||
other_pub = other_priv.public_key().public_bytes_raw()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [other_pub])
|
||||
|
||||
raw, sig = _signed(_minimal_catalog(version=1), priv) # signed by the WRONG key
|
||||
catalog_path = tmp_path / "catalog.json"
|
||||
sig_path = tmp_path / "catalog.json.sig"
|
||||
catalog_path.write_bytes(raw)
|
||||
sig_path.write_bytes(sig)
|
||||
|
||||
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_missing_files_returns_empty_list(tmp_path):
|
||||
assert c.load_bundled_catalog_entries(tmp_path / "nope.json", tmp_path / "nope.json.sig") == []
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_missing_sig_returns_empty_list(tmp_path, monkeypatch):
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
pub = priv.public_key().public_bytes_raw()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
|
||||
|
||||
raw, _sig = _signed(_minimal_catalog(version=1), priv)
|
||||
catalog_path = tmp_path / "catalog.json"
|
||||
catalog_path.write_bytes(raw)
|
||||
missing_sig_path = tmp_path / "catalog.json.sig" # never written
|
||||
|
||||
assert c.load_bundled_catalog_entries(catalog_path, missing_sig_path) == []
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_invalid_but_signed_returns_empty_list(tmp_path, monkeypatch):
|
||||
"""A payload that verifies but fails validate_catalog() (disallowed
|
||||
command) must still come back empty -- signing is necessary, not
|
||||
sufficient."""
|
||||
priv = Ed25519PrivateKey.generate()
|
||||
pub = priv.public_key().public_bytes_raw()
|
||||
monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
|
||||
|
||||
raw, sig = _signed(_catalog_with({"config": {"command": "bash", "args": []}}), priv)
|
||||
catalog_path = tmp_path / "catalog.json"
|
||||
sig_path = tmp_path / "catalog.json.sig"
|
||||
catalog_path.write_bytes(raw)
|
||||
sig_path.write_bytes(sig)
|
||||
|
||||
assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
|
||||
|
||||
|
||||
def test_load_bundled_catalog_entries_real_shipped_catalog():
|
||||
"""End-to-end regression against the actual bundled data/catalog.json +
|
||||
.sig, using the real CATALOG_PUBKEYS (no monkeypatch) -- this is what
|
||||
the Browse dialog actually calls on startup."""
|
||||
root = Path(__file__).resolve().parent.parent
|
||||
entries = c.load_bundled_catalog_entries(
|
||||
root / "data" / "catalog.json", root / "data" / "catalog.json.sig"
|
||||
)
|
||||
assert len(entries) == 19
|
||||
assert {e["id"] for e in entries} >= {"filesystem", "github", "slack", "postgres"}
|
||||
|
||||
Reference in New Issue
Block a user