Skip to content

feat(catalog): real popularity signal — GitHub stars + Docker pulls (Spec 110) - #1384

Closed
Dumbris wants to merge 27 commits into
mainfrom
110-catalog-popularity
Closed

Dumbris wants to merge 27 commits into
mainfrom
110-catalog-popularity

Conversation

@Dumbris

@Dumbris Dumbris commented Sep 26, 2026

Copy link
Copy Markdown
Member

Summary

Spec 110 (PR A, backend): a real popularity signal for the catalog. Today CatalogHit.Popularity is never populated, so the FR-060 popularity tiebreak always compares 0 vs 0, and the empty-query Popular section is identical to Official. This PR resolves the finding deferred twice in #1383's review rounds.

Stacked on #1383 (109-j-catalog-add-server). Spec, plan and tasks are in specs/110-catalog-popularity/.

Why data alone was not enough

  • SearchAll built the sections from the list already truncated to limit, so Popular was only a reshuffle of the top official hits.
  • All three default sources have official provenance, and Rank uses popularity as its third key. Official, "in rank order", was therefore already popularity order, so it would have matched Popular even with real stars. This amends Spec 109 FR-060: Official now uses source order.

What changes

  • Docker Hub pull_count → installs, taken from the payload the parser already fetches. Docker star_count is ignored because its scale is not comparable to GitHub stars.
  • GitHub stars come from SourceCodeURL (GET api.github.com/repos/{o}/{r}) through the SSRF-hardened registry client:
    • 4 concurrent requests, a deduplicated queue capped at 256, and a rolling budget of 50/h (4,000/h with MCPPROXY_GITHUB_TOKEN).
    • A pause driven by GitHub's rate-limit headers, plus ETag revalidation.
    • A bbolt bucket catalog_popularity: 24h TTL, stale values still served, a failed refresh keeps the last-known stars, capped at 5,000 keys.
  • The search is never blocked on GitHub. After the per-source fan-out, the search waits at most 800 ms for missing stars, and the fetch finishes in the background for the next search. Popularity problems never show up in unavailable[].
  • Ranking compares stars first, then installs (never summed).
  • Sections are built from the full pool before truncation:
    • An empty query fetches up to 50 per source.
    • Official is in source order.
    • Popular includes only hits with a real signal, at most one per GitHub repo, and is empty rather than padded.
  • MCPPROXY_CATALOG_POPULARITY=false turns off all fetches. The E2E script sets it so tests never call api.github.com.

Behavior changes to note

  • TestCatalogSearch_EmptyQueryReturnsEmptyResults now expects sections.popular to be empty for a fixture with no signal. The old assertion only passed because of this bug.
  • Known limitation: the Docker source fetches a single page (10 images by default), so Popular ranks only within what each source returns.

Not in this PR

PR B (FR-012) adds the ★/⤓ badges in the Web UI, macOS and the CLI table.

Testing

  • go test -race ./internal/registries/... ./cmd/mcpproxy/..., plus internal/httpapi and internal/runtime with -tags server and the CI skip regex: pass. -count=3 on registries: pass.
  • golangci-lint v2 with .github/.golangci.yml, run both bare and with --build-tags server: 0 issues on the touched packages.
  • ./scripts/test-api-e2e.sh: 70 passed, 0 failed.
  • The SC-001 to SC-004 fixture tests cover:
    • Popular ≠ Official, with Popular led by the most-starred hit.
    • No signal → Popular is empty.
    • A slow GitHub → the search stays bounded.
    • A 403 with exhausted rate limit → no requests until reset, stale values still served.

Review

  • Spec: zcode round 1 raised 9 findings, all folded into the spec.
  • Code: zcode, opencode and codex were all out of quota, so round 2 was an independent read-only Opus reviewer. It raised 4 findings, all fixed, each with a regression test shown to fail without the fix. A cross-model code review is still owed once a quota resets.

🤖 Generated with Claude Code

Dumbris and others added 27 commits September 25, 2026 23:59
Every <dialog class="modal"> now opens via showModal()/close() through a
shared useDialogOpen composable instead of the `open` attribute or a
`modal-open` class toggle, so a modal can never be painted over by the
sidebar or header regardless of z-index (the Add Server modal bug, H4).
Adds frontend/src/assets/z-index.css as the single source for the
sidebar < header < dropdown < modal < toast scale and retires the
ad-hoc z-40 on the sidebar's drawer-side.

Also folds in two small header changes that touch the same files:
Add-to-MCPProxy button state and header layout groundwork.
Cross-surface UX fixes with no dependency on later PRs in the spec:

- `/` lands on the Overview panel (interim step before Home in a later
  PR); `/usage` and `/overview` stay deep-linkable (FR-051).
- Server detail and Settings tabs read `?tab=` on mount and write it
  back with router.replace on change, keeping other query params
  (FR-016).
- Settings is named "Settings" everywhere (sidebar, route title, H1,
  document title), its tabs use line icons instead of emoji, and the
  "Server Edition" tab is gated on the runtime edition
  (systemStore.status.edition, now carried on every SSE status frame)
  rather than on whether the config happens to have a server_edition
  key (FR-056).
- Tool review states use one vocabulary — Approved / New, needs review
  / Changed, needs review — with the `awaiting` filter value removed;
  the Tools page's "Needs review" stat is a link to /review, backed by
  interim redirects to /servers (?status=needs_review) and
  /servers/:name (?tab=tools) until a later PR ships the real review
  views (FR-027, T026a).
- One pure function, contracts.AnnotationTier, computes a tool's tier
  (read/write/destructive/unannotated) from its annotations; GET /tools
  and GET /servers/{id}/tools carry it, and the Web Tools page and CLI
  `tools list --tier` (`--risk` kept as an alias) use it instead of each
  deriving their own — the CLI's old `--risk` read a field
  (annotations.operation_type) that doesn't exist on a real payload, so
  it matched nothing (FR-028, X11).
- Count charts (calls per tool, activity over time) use integer ticks,
  and the excluded "never completed a call" group is titled "Calls to
  unknown tools" (FR-074).
- "Add to MCPProxy" (Repositories + CLI `registry add`), flipping to
  "Added ✓ · Open" after a successful add (FR-063).
- The header ProfileSwitcher is hidden until at least one profile
  exists (FR-057).
…PR a)

- ToolLabels.swift: one shared approval-state and tier label table
  (Approved / New, needs review / Changed, needs review; Read / Write /
  Destructive / Unannotated / Unknown), matching the Web/CLI vocabulary.
  ServerDetailView's approval badge and ToolsView's new tier badge both
  go through it.
- ServerTool/SearchTool decode the backend-computed `tier` field
  (contracts.AnnotationTier) rather than deriving one from raw
  annotations.
- ServerBrowseView: "Add to MCPProxy", flipping to "Added ✓ · Open"
  (opens the server) after a successful add.
make swagger, after adding contracts.Tier / Tool.Tier.
Verified findings against 109-a-quick-wins (PR-a of Spec 109).

FIXED:

- z-index: --z-header/--z-sidebar shipped inverted, so the sticky
  header painted over the open mobile drawer sidebar instead of the
  other way around (the same class of stacking bug this scale exists
  to prevent, on the header/sidebar pair instead of sidebar/modal).
  Swapped the token values, updated the doc comment and
  z-index-scale.spec.ts's ordering assertion.

- useDialogOpen: never listened for the native `close` event a
  showModal()-opened <dialog> fires on Escape, so the driving Vue
  state desynced from the DOM the first time a user hit Escape on
  Repositories.vue's three dialogs, UserTokens/UserServers/
  UserActivity's dialogs, OnboardingWizard or ConnectModal — the
  dialog stayed closed forever after that (no reopen). Added an
  `onClose` hook plus a `close` listener guarded against looping back
  on our own close() calls, wired every affected consumer's own
  close/reset function through it. AddServerModal/AddSecretModal
  (already safe via useModalA11y's capture-phase Escape handler) get
  the same wiring for defense in depth. New test:
  use-dialog-open-native-close.spec.ts.

- Search tier: GET /index/search always reported tier=unannotated
  regardless of a tool's real annotations, disagreeing with the same
  tool's tier on GET /servers/{id}/tools. The reported cause (a missing
  "annotations" key in server.searchResultsToMaps's tool map) was one
  level above the real one: the Bleve ToolDocument never stored
  annotations at all, so a search hit could not carry them no matter
  what the map-shaping code did. Added a stored (not indexed)
  annotations_json field to the Bleve schema, round-tripped through
  toolDocument/readToolMetadata, requested it in the shared search
  Fields list, and now surface it in searchResultsToMaps. New tests:
  bleve_annotations_test.go, search_tools_tier_test.go.

- Tools.vue: selectStatCard's toggle-to-total branch dropped the
  filterApproval reset that existed before this PR removed the old
  "pending" stat card. Total is the row's other "reset" gesture
  (clearFilters resets both filters too); restored it. New test:
  tools-total-card-resets-approval.spec.ts.

- macOS ServerBrowseView: the "Added checkmark Open" flow keyed
  addedServers on the registry catalog's display name, but the backend
  can assign a different name (empty name falls back to the entry id,
  a conflict gets de-duplicated) and echoes it in the response body,
  which AddServerResult discarded entirely. Now decoded and used,
  matching the Web UI's `result.server?.name || server.name`. New
  test: APIClientAddServerFromRegistryTests.swift.

- macOS ToolsView: the tier badge rendered nothing at all for a nil
  tier (an older core not yet sending the field), while the Web UI
  always renders one, defaulting to "Unannotated". Added
  ToolRow.displayTier and used it unconditionally. New tests in
  ToolLabelsTests.swift.

- cmd/mcpproxy/registry_cmd.go: registryAddErrorOutput's doc comment
  was glued to registryAddMessage by a missing blank line, hiding it
  from godoc. Reordered; cosmetic only.

REJECTED (false positive for this PR):

- scripts/test-api-e2e.sh's blanket `pkill -f "mcpproxy.*serve"` /
  `pkill -f "launcher-server.*--port 39933"` in its cleanup trap is a
  real, pre-existing operational risk (can kill another worktree's or
  the user's own running instance), but git blame shows these lines
  predate every commit in this PR and none of this PR's three commits
  touch the file. No tasks.md for Spec 108/109 exists in this repo to
  substantiate the finding's claim that removing them was a task this
  PR was itself scoped to do. Left alone here rather than fixed blind
  without the actual spec to guide the exact requirement (own-PID
  tracking vs. a decoy-process proof) on a script every worktree's E2E
  run depends on; flagged instead as a follow-up task.

DEFERRED (genuine, not implemented — infra, not a rejection):

- e2e/web-ui-sweep/visual-a11y-sweep.spec.ts has no elementFromPoint
  assertion proving the z-index fix above holds in a real browser.
  Adding one needs the real launcher (scripts/run-web-smoke.sh) and a
  live Playwright run to confirm it actually catches the regression
  rather than being vacuously true — this fix-and-verify pass could
  not do that locally, so an unverified test was not committed;
  flagged as a follow-up task instead.

Verification: go build ./... and -tags server both clean; go vet and
gofmt clean on touched files; go test -race on internal/index,
internal/server (CI skip regex, both bare and -tags server variants),
internal/httpapi, internal/contracts, cmd/mcpproxy, internal/config,
internal/oauth, internal/storage, internal/serveredition/* all green;
golangci-lint v2 (bare and --build-tags server) on touched packages
shows only 6 pre-existing staticcheck issues in test files this PR
never touched; frontend npm ci + npm run build (vue-tsc + vite) +
npx vitest run all green (147 files / 1393 tests, package-lock.json
unchanged); swift test full suite green except one known
environment-caused failure (AppLifecycleTests, caused by a real
~/.mcpproxy/tray-lifecycle.jsonl already present on this shared
machine from outside this session — unrelated to any file this PR
touches, reproduces identically before any of these changes).

Not pushed; branch left detached per instructions.
Verified each round-2 finding against current source before touching
anything; fixed the genuine ones test-first, deferred one with reasons
below. No false positives to reject this round.

FIXED:

1. (high) Repositories.vue: closeAddRegistry()/closeDeleteRegistry() are
   wired as useDialogOpen's native-close handler (Escape/backdrop), but
   their own addingRegistry/deletingRegistry guard silently skipped the
   showAddRegistry/showDeleteRegistry flip. A native close already happened
   in the DOM by the time that fires and cannot be undone, so the guard just
   desynced Vue state from an already-closed dialog — reopening became a
   no-op forever (round 1's H4 bug, reintroduced by round 1's own fix). Split
   the native-close handler from the Cancel-button handler; the Cancel/
   Delete-cancel buttons keep the guard (now via :disabled) since an
   explicit click CAN be blocked mid-submit.

2. (medium) OnboardingWizard.vue: dismiss() (also wired as useDialogOpen's
   onClose) awaited up to three sequential engagement-bookkeeping calls
   before emit('close'). props.show stayed true for that whole window, so
   reopening the wizard from the sidebar Setup entry was a no-op. emit
   synchronously first; the bookkeeping now runs decoupled in the
   background.

3. (low) ConnectModal.vue: close() (same onClose role) reset several result/
   preview fields but not disconnectTarget, so Escape during the "Disconnect
   X?" confirm sub-panel left it stale for the next open.

4. (medium-high) internal/runtime/lifecycle.go: annotations_json is only
   written at (re)index time, but the reindex trigger is a Hash comparison
   that deliberately EXCLUDES annotations (see calculateToolApprovalHash's
   own comment in tool_quarantine.go: annotations are unstable across
   reconnections, and hashing them in before caused false
   "tool_description_changed" spam on every reconnect). So an
   already-indexed, otherwise-unchanged tool could never pick up newly
   observed annotations short of a manual reindex. Fixed WITHOUT touching
   Hash/change-detection (which would reintroduce that exact regression):
   applyDifferentialToolUpdate now tracks annotation-only diffs separately
   and silently refreshes just those documents (no approval/quarantine
   state, no "Tool schema changed" log, no hash rewrite). Covered by
   TestApplyDifferentialToolUpdate_BackfillsAnnotationsForUnchangedTool and
   ...IsIdempotent.

6. (low-medium) RegistryModels.swift: RegistryAddServerErrorBody decoded a
   `message` key, but writeRegistryAddError (internal/httpapi/server.go)
   serializes the field as `error` — every other error envelope in this API
   uses the same shape. err?.message was therefore always nil against the
   real backend, so a failed Add-from-Registry always showed the generic
   "HTTP 400: ..." text instead of the actual reason. Fixed the CodingKeys
   mapping and added a test that asserts on the decoded message text (the
   round-1 test used a stub keyed `message`, which "passed" without ever
   checking the value — a green test over a broken decode).

7. (low) Same files: round 1's own comment/test claimed the backend
   "de-duplicates on conflict" for a name collision. It doesn't —
   AddServer (internal/server/server.go) hard-fails with duplicate_name; no
   rename/suffix logic exists anywhere in that path. Corrected the
   misleading comments in RegistryModels.swift and ServerBrowseView.swift,
   and reworked the test's success-case stub to model the one case that IS
   real (empty entry name falls back to the entry id) instead of an
   invented de-dup suffix.

8. (medium) Settings.vue: hasServerEdition depends on systemStore.status,
   populated asynchronously by App.vue's SSE connection in its own
   onMounted. On a reload of /settings?tab=teams, Settings.vue's onMounted
   could run its `?tab=` membership check before that status frame lands,
   silently dropping `teams` with no retry once it arrives. Added a
   one-shot retry gated on hasServerEdition, guarded so it never overrides
   a tab the user picks in the meantime and is a no-op when the edition
   really is personal.

DEFERRED (real, not a false positive, but out of scope for a quick-wins
fix round):

5. (low) internal/index/bleve.go: a pre-105/pre-this-PR on-disk index opens
   with its OWN persisted mapping, which never declared annotations_json —
   so that field falls to Bleve's dynamic-mapping default (Index=true,
   contributes to _all) on a legacy index, while a freshly created index
   gets the explicit Index=false mapping added in round 1. Confirmed real:
   this is genuine BM25 divergence between legacy and fresh indexes.
   NOT fixed here: the obvious-looking fix (disable dynamic mapping) is
   actively wrong — output_schema_json has NO explicit field mapping at all
   and already relies on the same dynamic-mapping fallback in every index,
   old and new alike; disabling it would silently stop storing that field
   on every newly created index. A correct fix needs an actual Bleve
   mapping-version/migration mechanism (there is none today — the
   OutputSchemaHashSchemaVersion precedent lives entirely in the BBolt
   approval-record store, not the search index), which is a properly scoped
   follow-up, not a round-2 quick fix.

Verification: go build ./... and -tags server; go test -race
./internal/runtime/... (192s, clean); golangci-lint v2 bare and
--build-tags server (only pre-existing, unrelated issues, e.g.
event_bus.go:887 reflect.Ptr); frontend npm run build + npx vitest run
(151 files, 1401 tests, all green; package-lock.json unmodified); swift
test (1179 tests, 1 failure — AppLifecycleTests.
testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests, a
documented pre-existing environmental flake tied to a real
~/.mcpproxy/tray-lifecycle.jsonl on this machine, unrelated to this diff
and already failing on main).
Verified each round-3 finding against current source before touching
anything; fixed the genuine one test-first, confirmed one is already
covered by existing tests, and reaffirmed one deferral. No false
positives to reject this round — all three findings described the
code accurately.

FIXED:

1. (medium, T011a) scripts/test-api-e2e.sh's cleanup trap used a
   blanket `pkill -f "mcpproxy.*serve"` / `pkill -f
   "launcher-server.*--port 39933"`. Round 1 rejected this same finding
   on the premise that no tasks.md substantiates it being an in-scope
   task for this PR; that premise was false — specs/109-ux-navigation-
   consistency/tasks.md (present on the sibling 109-ux-navigation-
   consistency branch, both branched from the same commit) lists T011a
   explicitly under "Phase 2: PR 109-a — quick-wins". Live-verified the
   actual risk before fixing: this machine had a real tray-managed
   `mcpproxy serve` (PID 15758) and a second, unrelated worktree's test
   instance running at the same time — the unfixed script's blanket
   pkill would have killed both. Replaced both blanket kills with
   `descendant_pids()`, which snapshots the actual OS-process
   descendants of $MCPPROXY_PID (recursively, via `pgrep -P`) BEFORE
   touching it, then reaps only those specific PIDs (and only if still
   alive) as a fallback for the launcher-test fixture getting orphaned
   before mcpproxy's own graceful-shutdown reap path runs. Added
   scripts/test-api-e2e-cleanup-check.sh (the check T011a specifies):
   starts a decoy `mcpproxy serve` on another port with a scratch data
   dir, runs the full E2E script, and asserts the decoy is still alive
   afterward. Ran it for real: 65/66 sub-tests passed (the one failure
   is the pre-existing, unrelated "server-edition binary present"
   check, which needs a separately built ./mcpproxy-server and fails
   identically on main), and the decoy — plus this machine's real tray
   core — survived the run.

2. (low-medium, T007 Playwright half) e2e/web-ui-sweep/
   visual-a11y-sweep.spec.ts had no browser-level proof that the H4
   z-index fix (frontend/src/assets/z-index.css) holds under real
   layout and paint, only jsdom token-ordering coverage. Added "mobile
   drawer sidebar paints above the sticky header, not under it (H4)":
   opens the mobile drawer against a real launched instance and asserts
   via `elementFromPoint` that the drawer, not the header, is the
   topmost element at a point they both occupy. Verified it actually
   catches the regression it targets, not just a code read: temporarily
   re-inverted --z-header/--z-sidebar back to round-1's shipped bug,
   rebuilt, and reran — the new test timed out failing exactly as
   expected; restored the tokens and reran the full 25-test suite
   clean. One real finding surfaced while building this: daisyUI's
   drawer opens via a CSS `visibility`/`opacity` transition
   (`allow-discrete`, ~0.2-0.3s), so `elementFromPoint` checked on the
   very next frame after the checkbox flips can still legitimately see
   the header for a couple of frames — normal opening animation, not
   the H4 regression (a permanently wrong settled z-index). The
   assertion polls for the settled state instead of asserting
   immediately, so it does not flake on that transition.

ALREADY COVERED (not a gap, no change needed):

- The finding's other T007 half — "a vitest asserting none of
  Repositories.vue's three dialogs still uses the `:open` binding" —
  already exists: frontend/tests/unit/z-index-scale.spec.ts (added in
  the original PR commit 8bc8d23de, kept through round 1) has both
  "Repositories.vue dialogs use showModal()/close(), not :open
  (FR-055)" (lines 71-88, checking exactly the `:open` binding and all
  three data-test ids) and a broader "every <dialog class=modal> drives
  its open state imperatively" sweep across all 8 affected components,
  Repositories.vue included. Wrote a duplicate test file first, then
  found this and deleted it rather than ship two guards for the same
  invariant.

DEFERRED (confirmed real, reaffirming round 2's deferral — not fixed
here):

3. internal/index/bleve.go: re-verified the pre-105/pre-this-PR
   legacy-index dynamic-mapping divergence against current source.
   annotations_json still has no on-disk mapping-version guard,
   RebuildIndex() is still a documented no-op, and output_schema_json
   still has no explicit field mapping of its own — so it's still true
   that disabling dynamic mapping to close this gap would silently stop
   indexing output_schema_json on every freshly created index, on top
   of every index that already exists today. Round 2's reasoning holds
   exactly as written: a correct fix needs an actual Bleve
   mapping-version/migration mechanism, which is properly scoped design
   work, not a quick-wins round change. Flagging a follow-up task for
   that design work separately rather than leaving it only in this
   commit message.

Verification: go build ./... and -tags server (-o /dev/null, per the
server-tags-build-overwrite gotcha) both clean. No Go, frontend/src, or
native code touched this round, so go test -race / vitest / swift test
are unaffected by this diff; ran golangci-lint v2 anyway (bare and
--build-tags server) — 16 and 19 pre-existing issues respectively, none
in any file this round touched (reflect.Ptr inlining, deprecated
TopK/Features test usage, deprecated ecdsa.PublicKey.X/Y, ParseDir
deprecation — all pre-existing on files untouched by any of the three
rounds). scripts/test-api-e2e-cleanup-check.sh and the new Playwright
test were both run for real against a locally built instance, not just
read.

Not pushed; branch left detached per instructions.
Round-4 zcode findings on the E2E cleanup trap (T011a) and a stale
Tools.vue stat-card bug. Confirmed genuine, fixed test-first:

- F3 (medium): descendant_pids had no test of its own. Extracted it to
  scripts/descendant-pids.sh (sourced by test-api-e2e.sh) and added
  scripts/descendant-pids.test.sh, a hermetic unit test that builds a
  real 3-level process tree and proves the recursive walk reaches a
  grandchild and great-grandchild, not just direct children. Also
  extended test-api-e2e-cleanup-check.sh to assert a REAL orphan of the
  run (the launcher-test fixture, left running by test_launcher_lifecycle
  Step 5) is actually gone after cleanup, not just that the decoy
  survives. Wired both scripts into the Makefile
  (test-descendant-pids, test-e2e-cleanup-check).
- F4 (medium): the audit_log sub-test's AUDIT_PID/AUDIT_DATA_DIR/
  AUDIT_JSONL_DIR/AUDIT_SERVER_LOG were only reaped on its own
  fall-through path; an early exit while that sub-test is running leaked
  them. cleanup() now reaps them too, idempotently.
- F1 (medium, partial fix): the descendant snapshot was taken once,
  before sending the kill signal, missing a child spawned during
  mcpproxy's own graceful-shutdown window. Now re-snapshotted on every
  poll of the wait loop while mcpproxy is confirmed still alive. The
  case where mcpproxy has already exited (crash/OOM) before cleanup()
  even runs is not fixable without reintroducing the system-wide pattern
  match T011a deliberately removed (kills unrelated mcpproxy instances)
  — documented as an accepted, narrow residual gap.
- F2 (low): the reap loop now records each descendant's command name at
  snapshot time and re-checks it before SIGKILL, so a PID reused by an
  unrelated process in the snapshot-to-reap window is skipped instead of
  killed.
- F6 (low): test-api-e2e-cleanup-check.sh's decoy port was a fixed
  default (only DECOY_DIR was made unique), colliding between concurrent
  runs. Now picks an OS-assigned free port.
- F7 (low): added --max-time to the two curl calls in
  test-api-e2e-cleanup-check.sh that lacked it, matching every curl call
  in test-api-e2e.sh.
- Tools.vue activeStatCard (low, flagged rounds 1 and 2, never actually
  fixed): it only inspected filterStatus, so an approval-only filter
  (filterStatus empty) still rang the Total stat card as active even
  though the table was filtered. Total now reads as active only when no
  filter narrows the table. New regression test:
  tools-active-stat-card-approval-only.spec.ts (reproduced the bug
  first, then fixed).

F5 (EXIT trap not firing on SIGINT) does not hold up under a faithful
reproduction: sending SIGINT to the actual process group (what a
terminal's Ctrl-C really delivers) already ran cleanup() correctly on
the pre-round-4 code, because the foreground child dies from the same
signal, bash's `wait` unblocks, and an untrapped terminating signal
still fires the EXIT trap. Verified this directly (group-wide kill -INT
and kill -TERM both fired the trap on the unmodified script; only a
single-PID kill -INT, which is not what a terminal sends, appeared to
hang, and that appearance was itself an artifact of test harness
backgrounding setting SIGINT to ignore). Kept explicit `trap ... INT
TERM` handlers anyway as harmless, idiomatic defense-in-depth for a
supervisor that signals only this process's PID, with an idempotent
cleanup() (CLEANUP_DONE guard) so it cannot double-run.

Verified locally: go build ./... clean (no Go files touched by this
commit); golangci-lint v2 (bare and --build-tags server) shows only
pre-existing unrelated findings; frontend build + full vitest run
(152 files / 1402 tests) green; scripts/descendant-pids.test.sh green;
a full scripts/test-api-e2e-cleanup-check.sh run against a real built
mcpproxy/mcpproxy-server passed all 70 E2E tests, confirmed the decoy
instance survived on a freshly-picked port, and confirmed the real
launcher-server orphan was reaped.
Verify each round-6 finding against the code before fixing; fix genuine
ones test-first, reject/defer two that are out of scope here.

1. internal/index/bleve.go (finding 1, high): an index created before the
   annotations_json field mapping existed keeps its old mapping forever
   after bleve.Open (no SetMapping in vendored bleve v2.6.1), so the
   differential-update backfill's annotations JSON gets indexed as free
   text via bleve's dynamic-field defaults, leaking into the field-less
   `_all` composite field and skewing BM25 ranking on any upgraded (not
   freshly installed) deployment. A full fix needs index-mapping-version
   tracking plus rebuild-from-storage, which doesn't exist yet
   (RebuildIndex is a stub) -- flagged as a follow-up task instead of
   building that here. Applied a bounded, tested mitigation:
   warnIfMappingPredatesAnnotations logs an actionable warning on open;
   toolMapping.Dynamic = false closes the whole bug class for indexes
   created from now on; and an explicit output_schema_json field mapping
   (previously undeclared, relying on the same dynamic fallback) closes a
   second, pre-existing instance of the same leak that surfaced only once
   Dynamic was turned off. New tests: bleve_mapping_migration_test.go.

2. cmd/mcpproxy/tools_cmd.go (finding 4, medium): --tier/--risk/--status/
   --approval were only ever applied on the global (no --server) tools-list
   path; `--server=x --tier destructive` silently printed every tool
   unfiltered. Client-mode (daemon-backed) now reuses
   applyGlobalToolFilters, since GET /api/v1/servers/{id}/tools shares
   enrichServerTools with the global endpoint and carries the same fields.
   Standalone (no-daemon) mode gets a new filterToolMetadataByTier for
   --tier (computed locally via contracts.AnnotationTier) and a fast,
   explicit error for --status/--approval, which need daemon-persisted
   state this path has no access to. A zcode re-review of this fix caught
   a residual message bug it introduced: emptying a non-empty standalone
   result via --tier fell into the "server doesn't support tools"
   diagnostic meant for a server with genuinely zero tools;
   standaloneNoToolsMessage now tells the two cases apart.

3. native/macos ServerBrowseView.swift / RegistryModels.swift (finding 2,
   medium): "Add to MCPProxy" -> "Added (checkmark) Open" state was tracked
   by bare server.id, while the same file's own cross-registry search
   dedupe already keys by "registry::id" (catalog ids collide across
   registries, MCP-866) -- two colliding cards from different registries
   could flip together. Added RepositoryServer.addedKey and switched both
   addedServers call sites to it. A zcode re-review found the same
   collision class still live in ForEach(results)'s SwiftUI identity and in
   addingID; fixed both, plus a stale doc comment that still described the
   bug this commit fixes.

4. frontend/src/components/AuthErrorModal.vue (finding 3, medium): the
   last dialog left on the old v-if/div modal pattern while its four
   siblings were migrated to the browser top layer (FR-055); a 401 raised
   while one of those is open painted underneath it. Migrated to a native
   <dialog> via the existing useDialogOpen/useModalA11y composables,
   deliberately without the click-catcher backdrop form the other dialogs
   use (clicking outside must not dismiss this one -- a pre-existing,
   audited rule). Replaced the onMounted form-reset with a
   watch(props.show), since the dialog no longer unmounts/remounts on
   every open/close the way the v-if version did.

5. scripts/test-api-e2e-cleanup-check.sh (finding 5, medium): the T011a
   proof's launcher-server pgrep check had no pre-run baseline, so a stray
   left by an earlier crashed run or a parallel worktree's own E2E run
   produced a false FAIL. Snapshots PIDs before running and diffs with
   comm -13 so only a genuinely new PID counts as this run's own leak.

Rejected (not fixed here):
- Finding 6 (specs/109-ux-navigation-consistency/tasks.md T026 vs macOS
  DashboardView.swift): tasks.md lives on branch docs/specs-108-109, not
  this branch, so it can't be edited from here. The same tasks.md's own
  "Follow-ups (not in this spec)" section already lists "a native macOS
  Usage view" as deferred future work, directly contradicting T026's
  in-scope macOS clause -- a spec/tasks inconsistency for whoever owns
  that branch to reconcile, not a reason to invent a new macOS chart
  feature inside a quick-wins PR. Flagged as a follow-up task.

Every fix is test-first: internal/index, cmd/mcpproxy and native/macos
each got new regression tests confirmed to fail before the fix (or, for
the macOS swift package which has no such build gate here, verified by
temporarily reverting and re-running) and pass after.

Reviewed with zcode (GLM), the mandated external reviewer for this
branch. It returned a clean pass on internal/index/bleve.go +
cmd/mcpproxy/tools_cmd.go (one residual message bug found and fixed, see
above), a clean pass with three genuine follow-ups on the macOS
ServerBrowseView/RegistryModels change (all three folded into this
commit), and a clean pass with no defects on
scripts/test-api-e2e-cleanup-check.sh. The AuthErrorModal.vue chunk could
not get a completed zcode pass despite five retries over ~20 minutes --
the shared zcode/GLM account was rate-limited (HTTP 429, "Rate limit
reached for requests") by concurrent sibling review sessions running the
same review round in parallel worktrees, not by anything in this diff.
Verified by hand instead, against the identical, already-shipped and
already-tested pattern in AddSecretModal.vue / AddServerModal.vue /
ConnectModal.vue / OnboardingWizard.vue, plus the full frontend build
(vue-tsc + vite) and vitest suite (152 files / 1402 tests, including
auth-single-surface.spec.ts) passing.

Local verification: go build ./...; go test -race on internal/index,
cmd/mcpproxy, internal/runtime (+ subpackages), and an internal/httpapi
tool/tier/search subset; golangci-lint v2 (bare and --build-tags server)
clean on every touched Go package, no new issues anywhere else; npm ci &&
npm run build && npx vitest run (152/152 files, 1402/1402 tests), then
package-lock.json left unchanged (no dependency changes); swift build +
swift test (one pre-existing, unrelated failure confirmed present with
and without this diff: AppLifecycleTests.
testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests).
Round 7 findings, verified against the code (zcode unavailable this
round: 5-hour usage limit, resets 10:35 — reviewed manually instead,
same as the finding's own note):

- frontend/src/composables/useDialogOpen.ts (high, confirmed): a
  showModal() dialog's own Escape handling fires a native `cancel`
  event and, unless that event is canceled, closes the dialog —
  independently of useModalA11y's document-level keydown listener,
  whose preventDefault() only affects the keydown event and does not
  stop the platform's cancel/close algorithm. AuthErrorModal relies on
  its close() being a no-op while canClose is false, but the native
  path ignored that gate entirely: Escape closed the <dialog> element
  regardless, desyncing it from isOpen() (same bricking shape as the
  existing H4 fix) and defeating the "must not be dismissable" intent
  the whole modal exists for. Fixed by always preventing the native
  `cancel` event in useDialogOpen and routing every Escape-driven close
  through the one close() callback that already owns the permission
  check, the same way useModalA11y's keydown handler does. Covered by
  frontend/tests/unit/use-dialog-open-cancel-prevented.spec.ts,
  confirmed red before the fix (dispatching a synthetic `cancel` event
  closed the dialog) and green after.

- internal/index/bleve.go (medium, confirmed, carried from round 6):
  round 6 only warned when an on-disk Bleve index predated the
  annotations_json field mapping, leaving an in-place upgrade with a
  silently stale mapping (and the resulting free-text/`_all` leakage)
  until an operator noticed the log line and deleted the directory by
  hand. Manager.RebuildIndex is a pre-existing unwired no-op stub, not
  a path to fix this. Replaced the warn-only function with one that
  closes the stale index, removes its directory, and creates a fresh
  one with the current mapping automatically — safe because
  index.bleve is a derived search cache, not a source of truth: an
  empty index makes every server's tools look newly discovered, and
  the normal discovery path (applyDifferentialToolUpdate) already
  reindexes everything as each server reconnects at startup, the same
  way it backfills a brand-new install. Extended
  bleve_mapping_migration_test.go with a regression test asserting the
  rebuilt index carries the current mapping and rejects free-text
  search on annotations_json, with no operator step.

- native/macos/MCPProxy/Views/DashboardView.swift (medium, carried
  from rounds 5-6): re-scoped rather than implemented. Spec 109 T026's
  "macOS DashboardView.swift equivalent label" describes the same
  integer-tick + "Calls to unknown tools" fix web's CallHistogram.vue
  got, but macOS has no per-tool call-histogram anywhere to attach it
  to — the file's only chart-like element (Token Distribution) is a
  per-server token-SIZE bar list with no ticked axis and no
  "unresolved tool name" concept, a different metric keyed by server
  name rather than call outcome. Spec 109's own tasks.md lists "a
  native macOS Usage view" as a Follow-up explicitly "not in this
  spec" — building one now would be new scope, not a quick win. Added
  an explanatory comment recording that scope decision next to
  tokenDistributionSection so this stops being silently re-flagged
  every round; tracked for the eventual macOS Usage view instead.

Verification: go build ./...; go test -race ./internal/index/...;
golangci-lint v2 (bare and --build-tags server) clean of new issues;
frontend npm ci + vitest run (153 files / 1405 tests green) + vue-tsc
build; swift build + swift test (1182 tests, 1 pre-existing failure —
AppLifecycleTests.testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests
fails on this machine because ~/.mcpproxy/tray-lifecycle.jsonl already
exists from real daily use, unrelated to and unaffected by this
comment-only Swift change; reproduces identically without it).
…R-060/061/065)

- internal/registries/catalog.go: SearchAll fans out to every enabled
  registry in parallel with a per-source timeout, merges, de-dups by
  (source, id), ranks with the pure Rank function, and returns
  unavailable[] for failed/timed-out sources. Empty query returns
  official/popular sections.
- internal/secretlike: shared D13 secret-like-name heuristic.
- internal/shellwords: small quote-aware command-line tokenizer used to
  split an install command into CatalogInstall{command, args}.
- internal/secret/refname.go: RefName computes the FR-065 keyring ref
  name per field kind (env/header), with collision suffixing, pinned by
  the shared internal/secret/testdata/ref_names.json fixture.
- internal/secret: keyring provider now reports an availability reason
  (IsAvailableWithReason / Resolver.KeyringAvailability), surfaced by
  GET /secrets/config as keyring_available/keyring_reason.
- internal/httpapi/catalog.go: GET /catalog/search REST endpoint (no
  admin gate), with 'added' computed only over the caller's visible
  servers (FR-007).
…chment (FR-064)

- configimport: DetectFormat falls back to the new 'url' and 'command'
  formats for a single-line paste that is neither JSON nor TOML;
  URLParser/CommandParser map them to a remote or local/stdio server.
  SuggestNameFromURL/SuggestNameFromCommand derive a sensible default
  name (preferring the package name for npx/uvx/pipx runners).
- httpapi: POST /servers/import/json?preview=true accepts url/command
  content and its response gains summary, tags and per-env/header
  secret_like/empty_or_placeholder previews, built from the
  already-redacted URL/Command/Args so no raw secret reaches the new
  fields either.
- cliclient.CatalogSearch calls GET /catalog/search.
- cmd/mcpproxy/catalog_cmd.go: catalog search/show/add, daemon-first
  with an in-process SearchAll fallback for search; 'registry
  search'/'registry add' now print a deprecation notice pointing at
  their 'catalog' equivalent (both remain functional aliases).
- registries.BuildCatalogHit/ToCatalogResult exported so a single-entry
  lookup (catalog show) builds the same shape SearchAll uses.

Fixes a bug caught while wiring this up: the 'added' join treated every
configured server as matching by install-target alone, so a
registry-sourced server (source_registry_id set) could make a
DIFFERENT source's identical-looking entry falsely read added=true.
Only a manual add (no source_registry_id) matches by target alone; a
registry-sourced one also needs its own source_registry_id to match.
Fixed in both internal/httpapi/catalog.go and cmd/mcpproxy/catalog_cmd.go,
with a regression test in internal/httpapi/catalog_test.go.
Each value is written to the OS keyring under its per-kind ref name
(internal/secret.RefName: <server>-env-<name> / <server>-header-<name>)
and the config gets ${keyring:<ref>} instead of the raw value. An env
var and a header of the same name write two distinct entries; a
pre-existing entry is left unchanged and the new ref gets a numeric
suffix (D28). Refuses outright, before writing anything, when the
keyring is unavailable.
- views/AddServer.vue: Catalog (default) · Paste · Import · Manual tabs,
  synced to ?tab= via router.replace; ?source= narrows the Catalog tab
  only and never selects a tab.
- components/CatalogSearch.vue: searches every enabled catalog source
  (GET /catalog/search), renders official/popular sections for an empty
  query, 'Add to MCPProxy' -> 'Added ✓ · Open', and a secrets
  prompt for entries with required_inputs. Catalog-source text (title,
  description, publisher) is rendered as plain text only (D19).
- components/PasteServer.vue: detects a pasted URL/command/JSON/TOML via
  the extended import-preview endpoint and shows its summary/tags before
  anything is added.
- components/ManualServerForm.vue / ImportServersPanel.vue: the Manual
  and Import tabs.
- components/SecretToggle.vue + composables/useSecretFields.ts: the
  shared Value/Secret toggle (FR-065) used by all three tabs that take
  env/header input — writes to the OS keyring via the shared refName
  algorithm, rolls back only the secrets a failed add itself wrote.
- utils/secretRef.ts / secretLike.ts: TS ports of the Go refName/
  secret-like-name helpers, pinned against the shared
  internal/secret/testdata/ref_names.json fixture (copied into
  tests/unit/fixtures/). ServerDetail.vue's old suggestSecretName (which
  dropped the field kind) now delegates to the shared helper.
- router: /repositories redirects to /add-server?tab=catalog; TopHeader's
  "+ Add Server" now opens the new page.
- components/CatalogSourcesSettings.vue: catalog-SOURCE management
  (add/edit/delete a registry) moved out of the retired Repositories.vue
  into Settings -> Catalog sources; server BROWSING moved to
  CatalogSearch.vue. The 5 Repositories-specific tests were retired or
  retargeted (settings-catalog-sources.spec.ts, z-index-scale.spec.ts);
  add-server-catalog.spec.ts now covers the FR-063 label behavior.

Known gap (documented, not fixed in this PR): AddServerModal.vue and its
other callers (Servers.vue, Dashboard.vue, OnboardingWizard.vue) are left
as-is — retiring them fully cascades into ~9 existing test files
(protocol detection, duplicate-endpoint, trust-mode, a11y) that are out
of this PR's scope; TopHeader's primary entry point was the one rewired.
Omitted, it searches every enabled catalog source via registries.SearchAll
(FR-060) and returns the merged, ranked ServerEntry list in the same JSON
shape a single-registry search already returns — no 'added' field (that's
REST-only, contracts/rest-api.md#catalog). list_registries' wording moves to
'catalog source' terminology; both tools' descriptions point at the new
all-sources default.

Updates the frozen toolslist goldens deliberately (search_servers/
list_registries added to both the pre-099 and pre-105 enumerated-delta
gates, plus TestMenuSurface_ExactDeltaFromPreFeature's per-field
assertSearchServersDelta/assertListRegistriesDelta) since this changes a
built-in MCP tool's schema (registry: required -> optional).
… 109 FR-060/065)

- API/CatalogModels.swift: Codable mirrors of GET /api/v1/catalog/search's
  DTOs (CatalogResult, CatalogInstall, CatalogInput, CatalogSections),
  distinct from RegistryModels' RepositoryServer (that JSON stays
  unchanged).
- API/APIClient.swift: searchCatalog(query:source:tag:limit:) calling the
  new endpoint.
- Models/SecretRefName.swift: Swift port of internal/secret/refname.go's
  RefName (FR-065), pinned against the shared ref_names.json fixture in
  MCPProxyTests/CatalogTests.swift. ServerDetailView's old
  suggestedSecretName (which dropped the field kind) now delegates to it.
- Models/SecretLikeName.swift: Swift port of the D13 secret-like-name
  heuristic.

Known gap (documented, not fixed in this PR): the Add Server sheet's
Catalog/Paste tabs and the Registries-sidebar-item removal are NOT
implemented — this PR lands the shared, contract-critical data
layer (verified via swift test) that a follow-up UI PR builds on, mirroring
the same scoped-down decision made on the Web side for AddServerModal's
other callers.
…context scoping (T099)

- registries: a registry's explicit isSecret:false (or omitted) on a
  secret-shaped name still serves secret_like:true; an explicit true
  passes through unchanged.
- httpapi: FR-007 scoping is not agent-token-specific — a non-admin
  AuthTypeUser session (server edition's OAuth user identity) scoped to
  one server sees the same added:true/false split an agent token does.
…FR-060/065/067)

High
- secret: IsAvailableWithReason (GET /secrets/config keyring_available,
  and the CLI's applySecretFlags pre-check) now consults the macOS write
  gate (writesEnabled()) before the read-only probe, not after. Before
  this, a headless `mcpproxy serve` or the CLI's own in-process resolver
  reported keyring_available:true and then had every Store() call
  immediately fail with ErrKeyringUnavailable, because only Store() (not
  the probe) checked the gate.
- web(CatalogSearch): confirmAdd() closed the secrets dialog and cleared
  addError unconditionally after addResult(), even on failure — addResult
  never throws (api.ts always resolves {success:false}), so a failed
  add-after-secret-write looked identical to success. addResult now
  returns a success bool; confirmAdd only closes the dialog on success and
  rolls back the just-written secret otherwise.

Medium
- web: rollbackSecrets was exported but never called by any of the three
  add surfaces (CatalogSearch, PasteServer, ManualServerForm) when the
  add-server call failed after a successful secret write, orphaning a
  keyring entry and forcing a retry onto a -2-suffixed name. All three now
  track resolveSecretFields' writtenRefs and roll them back on a later
  failure.
- cli(upstream add): applySecretFlags (writes to the OS keyring) ran
  before validateTrustModeFlag, so a typo'd --trust-mode orphaned an
  already-written secret. Moved trust-mode validation first. Also:
  applySecretFlags now rolls back its own partial writes when the second
  of two --secret-env/--secret-header flags fails; runUpstreamAddDaemonMode
  and runUpstreamAddConfigMode now return (added bool, err error) so a
  --if-not-exists skip (nil error, added=false) is distinguishable from a
  genuine add, and runUpstreamAdd rolls back any written secrets via defer
  unless added ends up true (covers the daemon/config-load/save failure
  paths too, not just the skip).
- mcp(search_servers): 'registry' omitted returned bare registries.ServerEntry
  items with none of the catalog fields (title/publisher/verified/official/
  popularity/source) contracts/mcp-tools.md requires — MCP callers got none
  of the FR-060 ranking evidence REST/CLI callers already had. Added
  mcpCatalogServerEntry (embeds ServerEntry, encoding/json promotes its
  fields, plus the catalog fields alongside).
- httpapi(catalog): GET /catalog/search set results to the full ranked list
  even when sections was populated (empty q), contradicting "Empty q →
  results: []". Now emptied whenever sections is set.
- registries/httpapi/cli(catalog): source= was filtered AFTER SearchAll had
  already truncated to limit, so a narrower source's real matches ranked
  below an official/verified source could vanish entirely. SearchOptions
  gained a Source field; SearchAll now filters before ranking/truncation.
  Both httpapi.handleCatalogSearch and the CLI's catalogSearchInProcess
  switched to it, dropping their duplicated post-hoc filter helpers.

Rejected
- registries(catalog): Popularity is never populated in production
  (registries.ServerEntry carries no stars/installs field, and no
  registry-protocol parser in this codebase fills one — confirmed only
  tests ever construct a Popularity value), so the FR-060 popularity
  tiebreak always compares 0-vs-0 and the "Popular" section degrades to
  the same ranked list. Not fixed here: sourcing a real popularity signal
  (GitHub stars API, npm download counts, or a registry-reported count)
  is a new external-data integration — new calls, caching, rate-limit
  handling — sized for its own spec/plan, not a bounded review-round fix.

Tests: internal/secret (macOS write-gate cases), internal/server
(search_servers catalog fields), internal/httpapi (empty-q results,
source-filter-before-truncation), cmd/mcpproxy (CLI source-filter,
applySecretFlags self-rollback, --if-not-exists added=false), frontend
vitest (CatalogSearch/PasteServer/ManualServerForm rollback-on-failure).

Verification: go build ./...; go test -race on internal/secret,
internal/registries, internal/httpapi, cmd/mcpproxy; go test (and
go test -race, -skip per CLAUDE.md) on internal/server; golangci-lint v2
bare and --build-tags server on all touched packages (0 new issues); cd
frontend && npm ci && npm run build && npx vitest run (155 files / 1428
tests green); package-lock.json unchanged.
…T109/T103)

High
- macOS(AddServerView): the tray shipped the catalog data layer (CatalogModels,
  SecretLikeName, SecretRefName) but never wired it into any UI — AddServerView
  only had Import/Manual tabs, so a macOS user had no way to reach the new
  catalog search/add-server flow the Web UI, CLI (`catalog search|show|add`)
  and MCP (`search_servers`) all gained in this PR. Added CatalogView.swift
  (aggregated GET /api/v1/catalog/search across every enabled source, Official/
  Popular sections, "Add to MCPProxy" -> "Added checkmark Open") and
  PasteServerView.swift (paste a URL/command/config, preview-detect via
  POST /api/v1/servers/import/json?preview=true, fill in detected fields) as
  new Catalog/Paste tabs on AddServerView, ordered Catalog - Paste - Import -
  Manual to match the Web UI's views/AddServer.vue (T103/T109). The generic
  "Add Server" entry points (toolbar +, prominent button, empty-state button,
  dashboard shortcut) now open on Catalog by default instead of Import/Manual.
- macOS(MainWindow): removed the Registries sidebar item per T109 — it's fully
  superseded by the new Catalog tab for discovery. Registry SOURCE management
  (add/edit/remove a registry) moved to a new Settings -> Catalog Sources tab
  (CatalogSourcesTab, replacing the old RegistriesView's "Manage registries"
  half); the old per-registry ServerBrowseView "Discover servers" half is
  removed outright, superseded by the aggregated catalog search. Updated
  docs/registries.md's Web UI/macOS surface bullets to match (they described
  the pre-109-j Repositories page and Registries sidebar tab).

Both required a small backend-facing addition: a Swift port of the Web UI's
resolveSecretFields (SecretFieldResolver.swift, kind-aware so an env var and a
header of the same name never collide on one keyring ref, FR-065) plus the
APIClient calls it needs (getSecretRefs, deleteSecret, previewImportContent)
and the decode models (SecretRefEntry, ImportPreviewServer/Field). Pure
assembly/field-building logic (buildValues, buildFields, makeServerConfig) is
unit-tested without a network call, mirroring ManualServerForm's existing
pure-seam pattern; addServerFromRegistry/searchCatalog were already present
from the T104/T108 rounds and needed no changes.

Not reviewed
- zcode (the mandated reviewer) was unavailable for this entire round —
  account-wide 5-hour usage-quota exhaustion (ProviderBusinessError 1308),
  same block reported in round 2, not expected to clear inside this task's
  window. Per this task's standing instructions codex is off-limits for 2
  days and only zcode was mandated, so no reviewer ran; this fix is unreviewed
  by an external model. Flagging rather than silently proceeding as if
  reviewed.

Verification: swift build (clean) + swift test (1203/1203 passing, excluding
one pre-existing environment-dependent failure — AppLifecycleTests'
real-instance-root check fails identically on the unmodified base commit, a
leftover ~/.mcpproxy/tray-lifecycle.jsonl on this machine, unrelated to this
diff); go build ./... (clean, no Go files touched); golangci-lint v2 (pre-
existing unrelated staticcheck/govet issues only, same as base). No frontend
files touched.
…-T013)

Implements the FR-001..FR-009 core of the catalog popularity signal:
GitHubRepoKey normalizer, the PopularityProvider seam (Lookup/Resolve,
Fresh/Stale/Negative/Absent states), a cached rate-limited
githubStarsProvider (bbolt-backed store, 4-worker pool, rolling budget,
breaker), parseDocker's pull_count -> Installs, and Rank/buildSections/
SearchAll wiring so Official stays in source order while Popular is
built from the full pre-truncation, pre-Rank-sort pool.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…014-T019)

Wires the Spec 110 popularity signal into production paths:
- internal/runtime installs a bbolt-backed githubStarsProvider at startup
  and closes it on shutdown.
- cmd/mcpproxy installs a memory-only provider for the CLI's in-process
  'catalog search' fallback and 'catalog show'.
- scripts/test-api-e2e.sh exports MCPPROXY_CATALOG_POPULARITY=false so
  E2E never depends on api.github.com (SC-005).
- docs/registries.md documents the two popularity sources, the
  MCPPROXY_GITHUB_TOKEN budget bump, and the kill switch.
- T014: updates a Spec 109 catalog test that asserted the old
  Popular-equals-ranked-pool behavior to the new SC-002 empty-Popular
  semantics now that Popular only ever shows a real signal.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Load the catalog_popularity bucket eagerly at provider construction and
trim it to the FR-008 cap. With lazy loading the in-memory key count
restarted at zero after each restart, so the bbolt bucket could grow
past the cap without bound.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
- A 200 that is oversized, not JSON, or lacks stargazers_count is a failed
  fetch: keep the last-known stars/ETag instead of recording 0 for 24h.
- After Close, workers drop buffered keys and a shutdown-cancelled request
  is not persisted as a transport error.
- applyCachedStars falls back to source-native stars when a refresh during
  Resolve's wait turns a hit Negative (404/451).
- Resolve on a closed provider returns at once; Close wakes pending waiters.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Deploying mcpproxy-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9386f9c
Status: ✅  Deploy successful!
Preview URL: https://c7723609.mcpproxy-docs.pages.dev
Branch Preview URL: https://110-catalog-popularity.mcpproxy-docs.pages.dev

View logs

@github-actions

Copy link
Copy Markdown
Contributor

📦 Build Artifacts

Workflow Run: View Run
Branch: 110-catalog-popularity

Available Artifacts

  • archive-darwin-amd64 (30 MB)
  • archive-darwin-arm64 (27 MB)
  • archive-linux-amd64 (18 MB)
  • archive-linux-arm64 (16 MB)
  • archive-windows-amd64 (30 MB)
  • archive-windows-arm64 (26 MB)
  • frontend-dist-pr (0 MB)
  • installer-dmg-darwin-amd64 (24 MB)
  • installer-dmg-darwin-arm64 (22 MB)
  • smart-mcp-proxymcpproxy-goPLPSKW.dockerbuild (0 MB)

How to Download

Option 1: GitHub Web UI (easiest)

  1. Go to the workflow run page linked above
  2. Scroll to the bottom "Artifacts" section
  3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 36214737261 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 83.33333% with 96 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/registries/popularity_github.go 82.14% 38 Missing and 22 partials ⚠️
internal/registries/popularity_store.go 66.66% 9 Missing and 9 partials ⚠️
internal/registries/catalog.go 90.47% 7 Missing and 3 partials ⚠️
cmd/mcpproxy/catalog_cmd.go 0.00% 6 Missing ⚠️
internal/registries/popularity_key.go 92.59% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Dumbris
Dumbris changed the base branch from 109-j-catalog-add-server to main September 28, 2026 11:20
@Dumbris

Dumbris commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Superseded by the merged and verified PR #1408. Closing this conflicting duplicate catalog-popularity branch.

@Dumbris Dumbris closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants