fix(review): honest scan banner and approved-state review screen (Spec fix-review-screen) - #1470
Merged
Merged
Conversation
…ition capture Stamp tool definition changes in storage, record exported tool names on the scan context, compose scan coverage (current, stale, not_captured, tools_not_scanned, scanning, none) with honest per-tool verdicts, capture quarantined definitions after a settled scan, and print the coverage in review show.
… test mutating it
Deploying mcpproxy-docs with
|
| Latest commit: |
29db668
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d2dd342a.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-review-screen-scan-and-a.mcpproxy-docs.pages.dev |
F1 (fixed): scan coverage compares a definition change against the time the tool definitions were exported (ScanContext.ToolsExportedAt) instead of the engine's StartedAt, so a change landing between export and job start is not reported as covered. Legacy jobs fall back to StartedAt. F2 (fixed): ReviewScreen resets the scanning/rescanning/error state when the server name changes, so a rescan on one server no longer leaks into the next. F3 (fixed): a trusted server with no captured tools reads as approved and offers quarantine on Web and macOS. F4 (fixed): APIClient.quarantineServer escapes the server name in the path.
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37046263210 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
11 tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes for users
Catalog search for
githubnow puts GitHub's own server first on every surface. Before, the official registry's alphabeticalio.github.*page decided what was fetched, so the real server never appeared.limit. Results are ordered by how well the name matches (the publisher equals the query, an exact name, a name prefix, a name word, a substring or description, and last a match through the namespace alone), then official source, verified publisher, popularity, title. A namespace-only match such asio.github.06ketan/slideshotranks last..github/) and name-prefix (/github) queries, one page each, concurrently. The registry only matchesserver.name, so a multi-word query also searches its hyphenated phrase. If the main query times out, the expansion hits are still shown.verifiedis narrower: it is true for a built-in official-protocol entry only when the publisher's namespace owns the source repository. A re-publisher or an entry that names someone else's repo is not verified.officialkeeps meaning "from a built-in source".limitrepos, best-ranked first, so one search cannot spend the hourly budget.fetchand Dockermcp/fetchare one entry).catalog searchhelp text describes the new order. No flag or column changed. REST and MCP wire shapes are unchanged (no field added or removed); only the values ofverified,titleanddescriptionand the result order change.Audit findings fixed
C1 (catalog ranking), found not fixed in the final done-check. Also closes the SC-008 gap: the old hand-written fixture hid the live bug, so the fixture is now recorded from the live registry.
Spec
specs/109-ux-navigation-consistency(spec US5, FR-060, FR-061, SC-008, edge cases; data-model §9; contracts; research D36; plan; tasks Phase 16 T166-T175; quickstart recipe; parity matrix row 14; acceptance index SC-008). Spec 110 gets an amendment line. Docs:docs/api/rest-api.md,docs/cli/catalog-commands.md,docs/features/catalog-popularity.md.Tests added
internal/registries:recorded_registry_test.go(fake registry semantics, and a test that proves the live bug on the recorded corpus),official_catalog_test.go,search_catalog_test.go,catalog_hit_signals_test.go,catalog_rank_tier_test.go,catalog_typed_test.go,catalog_warm_behind_test.go(run under-race -count=20andGOMAXPROCS=1).catalog_github_order.jsonis the union of three real registry responses recorded 2026-10-02 (RECORD_LIVE_REGISTRY=1re-records it). The REST, MCP, CLI, vitest and XCTest legs replay it.catalog-card-signals.spec.ts, extendedadd-server-catalog.spec.tsandcatalog-order-parity.spec.ts. macOS:testCardShowsVerifiedNotOfficialBadgeand extended parity assertions.Deviations from the plan (recorded in research D36)
io.github.<user>, otherwise the second DNS label, not the last label. The last-label rule madecom.quranmajeed.time/prayer-timesan owner match fortimeand ranked it above the reference and Dockertime; this also fixes the publisher shown on the card.starsBorrowed(the negation of "eligible"), so hand-built hits stay eligible and existing tests keep their meaning.catalog-order-parity.spec.tscounted everycatalog-result-*node as a card; its filter now also excludes the new publisher and popularity ids.TestSearchAll_PerformanceBudgetcloses its hung server beforeClose, because timed-out fetches now keep their connections until released.Follow-ups (low)
search_serverstool description still says "official-first"; it is frozen by three schema goldens, so changing it means regenerating them.