fix(review): honest scan banner and approved-state review screen (Spec fix-review-screen)

.github/workflows/pr-build.yml

@@ -199,6 +199,19 @@ jobs:
         shell: bash
         run: npm ci --prefix bench/tscg
 
+      # tiktoken-go downloads the BPE into a shared temp dir and ends with an
+      # atomic rename. `go test ./...` runs packages in parallel, so on Windows
+      # the losing package hits "Access is denied" on the rename (seen as
+      # TestBaseline_CountToolWithSchemaParity). Populating the cache first
+      # makes every later tokenizer construction a cache hit. Same fix as
+      # unit-tests.yml.
+      - name: Warm the tiktoken vocabulary cache
+        if: '!(matrix.goos == ''windows'' && matrix.goarch == ''arm64'')'
+        shell: bash
+        run: go run ./bench/cmd/warmtiktoken
+        env:
+          TIKTOKEN_CACHE_DIR: ${{ runner.temp }}/tiktoken
+
       - name: Run tests (skip binary E2E tests - not compatible with cross-compilation)
         # Skip tests for Windows ARM64 (cross-compilation - can't run ARM64 binaries on AMD64 runner)
         if: '!(matrix.goos == ''windows'' && matrix.goarch == ''arm64'')'
@@ -210,6 +223,8 @@ jobs:
           else
             go test -tags nogui -v -skip "E2E|Binary|MCPProtocol" ./...
           fi
+        env:
+          TIKTOKEN_CACHE_DIR: ${{ runner.temp }}/tiktoken
 
       - name: Build binary and create archives
         shell: bash

ROADMAP.md

@@ -1036,6 +1036,6 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—`
 | [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) |
 | [107-server-edition-sso-hardening](./specs/107-server-edition-sso-hardening/) | `shipped` | 126/126 (100%) |
 | [108-profiles-v3](./specs/108-profiles-v3/) | `shipped` | 185/186 (99%) |
-| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `shipped` | 216/217 (100%) |
+| [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `shipped` | 233/234 (100%) |
 | [110-catalog-popularity](./specs/110-catalog-popularity/) | `in-flight` | 19/23 (83%) |
 | [112-client-header-forwarding](./specs/112-client-header-forwarding/) | `shipped` | 38/40 (95%) |

cmd/mcpproxy/review_cmd.go

@@ -224,6 +224,9 @@ func formatReviewResponse(format string, raw []byte, full bool) error {
 	}
 	server, _ := value["server"].(map[string]interface{})
 	fmt.Printf("Server: %v\n", server["name"])
+	if line := reviewScanLine(server); line != "" {
+		fmt.Println(line)
+	}
 	rows := make([][]string, 0)
 	if tools, ok := value["tools"].([]interface{}); ok {
 		for _, item := range tools {
@@ -250,6 +253,44 @@ func formatReviewResponse(format string, raw []byte, full bool) error {
 	return nil
 }
 
+// reviewScanLine renders the scan coverage of `review show` in the same words
+// as the Web and macOS review screens. It returns "" when the payload carries
+// no scan or predates coverage.
+func reviewScanLine(server map[string]interface{}) string {
+	scan, _ := server["scan"].(map[string]interface{})
+	coverage, _ := scan["coverage"].(string)
+	if coverage == "" {
+		return ""
+	}
+	rescan := "run: mcpproxy security rescan " + fmt.Sprint(server["name"])
+	switch coverage {
+	case "current":
+		risk, _ := scan["risk_score"].(float64)
+		scanned, _ := scan["tools_scanned"].(float64)
+		return fmt.Sprintf("Scan: %v · risk %d/100 · covers all %d tools", scan["verdict"], int(risk), int(scanned))
+	case "stale":
+		var tools []string
+		if list, ok := scan["unscanned_tools"].([]interface{}); ok {
+			for _, item := range list {
+				tools = append(tools, fmt.Sprint(item))
+			}
+		}
+		noun := "tools"
+		if len(tools) == 1 {
+			noun = "tool"
+		}
+		return fmt.Sprintf("Scan: out of date (%d %s changed or added after the last scan: %s); %s", len(tools), noun, strings.Join(tools, ", "), rescan)
+	case "not_captured":
+		return "Scan: not checked against tool definitions: they have not been captured yet; fetch them with Fetch tool definitions on the Web or macOS review screen"
+	case "tools_not_scanned":
+		return "Scan: the last scan did not analyse tool definitions (0 exported); " + rescan
+	case "scanning":
+		return "Scan: in progress"
+	default:
+		return "Scan: not scanned yet; " + rescan
+	}
+}
+
 func reviewSchemaText(label string, schema interface{}) string {
 	if schema == nil {
 		return ""

cmd/mcpproxy/review_cmd_test.go

@@ -229,6 +229,39 @@ func TestReviewShowJSONNeverRevealsComposerRedaction(t *testing.T) {
 	require.Contains(t, output, "••••23")
 }
 
+func TestFormatReviewShowPrintsScanCoverage(t *testing.T) {
+	show := func(scan string) string {
+		payload := `{"data":{"server":{"name":"notes"` + scan + `},"tools":[]}}`
+		return captureReviewOutput(t, func() error { return formatReviewResponse("table", []byte(payload), false) })
+	}
+
+	stale := show(`,"scan":{"verdict":"clean","risk_score":0,"coverage":"stale","tools_scanned":5,"unscanned_tools":["notes"]}`)
+	require.Contains(t, stale, "Scan: out of date (1 tool changed or added after the last scan: notes); run: mcpproxy security rescan notes")
+	require.NotContains(t, stale, "risk 0/100")
+
+	staleMany := show(`,"scan":{"verdict":"warnings","coverage":"stale","tools_scanned":5,"unscanned_tools":["a","b"]}`)
+	require.Contains(t, staleMany, "Scan: out of date (2 tools changed or added after the last scan: a, b); run: mcpproxy security rescan notes")
+
+	current := show(`,"scan":{"verdict":"clean","risk_score":0,"coverage":"current","tools_scanned":5}`)
+	require.Contains(t, current, "Scan: clean · risk 0/100 · covers all 5 tools")
+	require.Less(t, strings.Index(current, "Server: notes"), strings.Index(current, "Scan: clean"))
+
+	notCaptured := show(`,"scan":{"verdict":"not_scanned","coverage":"not_captured"}`)
+	require.Contains(t, notCaptured, "Scan: not checked against tool definitions: they have not been captured yet")
+	require.Contains(t, notCaptured, "Fetch tool definitions")
+	require.NotContains(t, notCaptured, "clean")
+
+	require.Contains(t, show(`,"scan":{"verdict":"clean","coverage":"tools_not_scanned"}`),
+		"Scan: the last scan did not analyse tool definitions (0 exported); run: mcpproxy security rescan notes")
+	require.Contains(t, show(`,"scan":{"verdict":"not_scanned","coverage":"none"}`),
+		"Scan: not scanned yet; run: mcpproxy security rescan notes")
+	require.Contains(t, show(`,"scan":{"verdict":"not_scanned","coverage":"scanning"}`), "Scan: in progress")
+
+	// No scan key (or a daemon that predates coverage): no Scan line at all.
+	require.NotContains(t, show(``), "Scan:")
+	require.NotContains(t, show(`,"scan":{"verdict":"clean"}`), "Scan:")
+}
+
 func captureReviewOutput(t *testing.T, fn func() error) string {
 	t.Helper()
 	previous := os.Stdout

docs/api/rest-api.md

@@ -939,7 +939,8 @@ The review payload of one server: a summary of the server (secrets in the URL, h
   "success": true,
   "data": {
     "server": {"name": "filesystem", "transport": "stdio", "quarantined": true, "trust_mode": "manual",
-               "scan": {"verdict": "clean", "risk_score": 0}, "definitions_captured": true},
+               "scan": {"verdict": "clean", "risk_score": 0, "coverage": "current", "tools_scanned": 2},
+               "definitions_captured": true},
     "tools": [
       {"name": "edit_file", "description": "Make line-based edits to a text file", "input_schema": {"type": "object"},
        "annotations": {"destructiveHint": true}, "tier": "destructive", "approval_status": "pending",
@@ -955,8 +956,9 @@ The review payload of one server: a summary of the server (secrets in the URL, h
 
 - `tier` is `read`, `write`, `destructive`, `unannotated` (annotations captured, no hints) or `unknown` (nothing captured, a record from before the review screen). It comes from one function, so the Web UI, macOS, `mcpproxy tools list --tier` and the MCP `quarantine_security` inspect operations show the same value.
 - `approval_status` is `approved`, `pending` (shown as "New, needs review") or `changed` ("Changed, needs review").
-- `scan_verdict` is `dangerous`, `warnings`, `clean` or `not_scanned`.
-- `definitions_captured: false` returns `tools: []`; `POST /api/v1/servers/{id}/discover-tools` captures the definitions without indexing them.
+- `scan.coverage` says whether the scan verdict describes the definitions in the payload: `current` (the latest completed scan analysed every captured definition as it is now), `stale` (some definitions were added or changed after that scan; `scan.unscanned_tools` lists them), `not_captured` (no definitions captured), `tools_not_scanned` (the scan completed but exported no tool definitions), `scanning` (a scan is running) or `none` (no completed scan). Show `risk_score` only for `current`. `scan.tools_scanned` is the number of definitions that scan exported.
+- `scan_verdict` is `dangerous`, `warnings`, `clean` or `not_scanned`. `clean` means the latest scan covered this tool's current definition and found nothing; a tool whose definition changed after the scan is `not_scanned` (or carries its held verdict).
+- `definitions_captured: false` returns `tools: []`; `POST /api/v1/servers/{id}/discover-tools` captures the definitions without indexing them. After a baseline scan has listed a still-quarantined server's tools, MCPProxy runs the same capture itself.
 - Descriptions are returned verbatim and must be rendered as inert text.
 
 The review decisions use these routes (all existing):

docs/cli/review-commands.md

@@ -52,6 +52,15 @@ Input schema:
 {"type":"object"}
 ```
 
+When the review payload carries scan coverage, a `Scan:` line follows `Server:`. It says whether the baseline scan describes the definitions shown, in the same words as the Web UI and the macOS app:
+
+```
+Server: notes
+Scan: out of date (1 tool changed or added after the last scan: notes); run: mcpproxy security rescan notes
+```
+
+A scan that covers every captured tool reads `Scan: clean · risk 0/100 · covers all 5 tools`. When definitions have not been captured, the line says so and points to **Fetch tool definitions** on the Web or macOS review screen; there is no CLI command for that capture.
+
 Descriptions and schemas come from the upstream server and are shown as plain text; they are not verified. Without `--full` only the first line of each description is shown and the schemas are left out.
 
 ## review approve

docs/features/security-quarantine.md

@@ -203,6 +203,23 @@ Every surface offers the same four decisions. Only the scan-gated approval can r
 | Tray on Windows and Linux (Go tray) | A server in the "Security Quarantine" submenu opens the Web UI at `/review/<name>` |
 | MCP | `quarantine_security` with `list_quarantined`, `inspect_quarantined`, `inspect_tools`, `approve_tool`, `approve_all_tools`, `block_tool`, `block_all_tools` (admin only). There is no server-level approve over MCP by design: an agent cannot release a quarantined server |
 
+### Scan coverage on the review screen
+
+The scan line of the review screen says whether the baseline scan describes the definitions you are looking at:
+
+| Coverage | What the screen shows |
+|----------|-----------------------|
+| current | `Baseline scan: clean · risk 0/100 · covers all 5 tools`, in the colour of the verdict. The risk score appears only here |
+| stale | A warning: a tool definition changed or was added after the last scan. It names the tools and the last result, and offers **Rescan**. A rug pull after the scan therefore never reads as clean |
+| not captured | A warning that the scan was not checked against tool definitions, with **Fetch tool definitions** |
+| no tools scanned | A warning that the last scan did not analyse tool definitions, with **Rescan** |
+| scanning | `Scan in progress…` |
+| none | `Not scanned yet.` with **Scan now** |
+
+Each tool's scan verdict follows the same rule: `clean` only when the scan covered that tool's current definition. After a baseline scan has listed a quarantined server's tools, MCPProxy captures the definitions itself, so the review list is not empty until someone clicks **Fetch tool definitions**. With `security.auto_baseline_scan: false` and no manual scan nothing is started automatically.
+
+On a server that is not quarantined, the review tab shows approved state: approved tools read **Approved** or **Blocked** (no Approve or Reject), the heading says the server is approved, and **Manage tools** and **Quarantine to review again…** are offered. The second is the existing quarantine action behind a confirmation. Only a new or changed tool shows Approve and Reject.
+
 ### Scan a Server for TPAs (MCP)
 
 The `quarantine_security` tool can also run and read the TPA scan, so an agent