fix(catalog-console): run registry lookup automatically, not on click (#62)
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 22s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 22s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
Review feedback: making the registry lookup an on-demand 'Check registry' button was a deviation from the design intent, not a style choice. The registry lookup is the one check a human reviewer genuinely cannot do by eye -- it's what caught firecrawl-mcp's unrelated npm publisher in the seed data. Gating it behind a button makes it optional, and an optional check is the one a tired maintainer skips at 11pm -- exactly the failure mode this tool exists to defend against. The friction belongs on approval, never on information. EntryCard now kicks off its registry lookup automatically at construction time (i.e. as soon as _render_cards() builds the cards for a loaded review), one RegistryLookupWorker (QThread) per changed entry, all starting concurrently as the cards are built. Nothing about the lookup's pure logic changed -- catalog_review.lookup_registry_info was already fail-soft (RegistryInfo(available=False) on any fetch problem, never an exception) and can_sign() never depended on registry state, so a dead registry still cannot gate review or signing. Renamed the button 'Check registry' -> 'Re-check' and kept it wired to the same _run_registry_lookup(), for manually retrying a failed/unavailable lookup. Label copy now reads loading... while a lookup is in flight (was 'not checked yet.' / 'checking...'), matching the loading -> result | unavailable per-entry states. No change to acknowledge-gating or the TOCTOU blob-SHA pin in catalog_review.py. ruff check/format clean; full suite still 321 passed, 1 pre-existing unrelated skip -- catalog_review.py (the tested pure-logic module) is untouched, only catalog_console.py's GUI wiring moved from button-triggered to auto-triggered.
This commit is contained in:
+26
-9
@@ -15,10 +15,16 @@ Flow: Load -> Review -> Sign.
|
||||
blob SHA for the rest of this review pass.
|
||||
2. Review -- a semantic diff (catalog_review.diff_catalogs), one card per
|
||||
changed entry, with risk annotations
|
||||
(catalog_review.entry_risk_findings) and a live registry
|
||||
lookup. Every changed entry must be individually
|
||||
acknowledged (its checkbox ticked) before Sign unlocks.
|
||||
There is no "acknowledge all" -- see catalog_review.py.
|
||||
(catalog_review.entry_risk_findings). A registry lookup for
|
||||
each entry's npm/PyPI package kicks off automatically, one
|
||||
worker thread per entry, the moment the cards are built --
|
||||
it is the one check a reviewer can't do by eye, so it must
|
||||
never depend on a click. It fails soft (a dead registry
|
||||
shows "unavailable", never blocks review or Sign) and a
|
||||
per-card "Re-check" button covers manual retries. Every
|
||||
changed entry must be individually acknowledged (its
|
||||
checkbox ticked) before Sign unlocks. There is no
|
||||
"acknowledge all" -- see catalog_review.py.
|
||||
3. Sign -- re-fetches the current blob SHA and refuses to sign unless
|
||||
it still matches the pinned SHA from step 1 (TOCTOU fix:
|
||||
catalog_review.can_sign). On success, writes
|
||||
@@ -497,11 +503,11 @@ class EntryCard(QWidget):
|
||||
flabel.setStyleSheet("color: #1565c0;")
|
||||
layout.addWidget(flabel)
|
||||
|
||||
self.registry_label = plain_label("Registry lookup: not checked yet.")
|
||||
self.registry_label = plain_label("Registry lookup: loading...")
|
||||
layout.addWidget(self.registry_label)
|
||||
check_btn = QPushButton("Check registry")
|
||||
check_btn.clicked.connect(self._run_registry_lookup)
|
||||
layout.addWidget(check_btn)
|
||||
recheck_btn = QPushButton("Re-check")
|
||||
recheck_btn.clicked.connect(self._run_registry_lookup)
|
||||
layout.addWidget(recheck_btn)
|
||||
|
||||
self.blocking = any(f.severity == "blocking" for f in findings)
|
||||
self.checkbox = QCheckBox(
|
||||
@@ -519,13 +525,24 @@ class EntryCard(QWidget):
|
||||
)
|
||||
layout.addWidget(self.checkbox)
|
||||
|
||||
# Registry lookup is the one check a reviewer can't do by eye -- it's
|
||||
# what catches a typosquatted/hijacked package (it already caught
|
||||
# firecrawl-mcp in the seed data). It must run automatically as soon
|
||||
# as the card exists, not wait on a click a tired maintainer might
|
||||
# skip at 11pm. Off the GUI thread (RegistryLookupWorker is a
|
||||
# QThread) and fails soft: a dead/slow registry can never gate
|
||||
# review or signing, it just leaves this entry's lookup showing
|
||||
# "unavailable". The "Re-check" button above stays for retrying a
|
||||
# failed/unavailable lookup by hand.
|
||||
self._run_registry_lookup()
|
||||
|
||||
def _run_registry_lookup(self):
|
||||
entry = self.change.new or {}
|
||||
refs = review.extract_package_refs(entry)
|
||||
if not refs:
|
||||
self.registry_label.setText("Registry lookup: no npm/PyPI package in this entry.")
|
||||
return
|
||||
self.registry_label.setText("Registry lookup: checking...")
|
||||
self.registry_label.setText("Registry lookup: loading...")
|
||||
self._worker = RegistryLookupWorker(refs, self._all_entry_ids)
|
||||
self._worker.done.connect(self._on_registry_result)
|
||||
self._worker.start()
|
||||
|
||||
Reference in New Issue
Block a user