test(070): Phase-8 finalization — cross-surface consistency regression + O3 amendment (Related MCP-746)

[Dumbris]

Spec 070 — Registry easy upstream-add: Phase-8 finalization

Consolidated finalization PR for spec 070. Related MCP-746.

Reconciliation finding (the key surprise)

The Phase-8 issue (MCP-808) expected a ~58-file consolidated diff to rebase onto main. On reconciliation, PR #555 (354580f4) turned out to have squash-landed the entire 070 surface — not just US2 as its title suggested. That squash is 46 files / +4343 / −169 and already contains:

• US1 — Web UI one-flow registry search→add (frontend/.../registry-add.spec.ts + components)
• US3 — MCP add_from_registry operation (internal/server/mcp.go, 11 refs)
• US4 — registry cache freshness + refresh/invalidate (internal/cache/manager.go, freshness tests)
• the spec, plan, tasks, and docs

So after merging current origin/main, the only net-new content left from the branch is the Phase-7 consistency regression (committed after #555's branch point) plus the O3 decision artifact. This PR is therefore intentionally small.

What this PR adds

1. internal/server/consistency_crosssurface_test.go (T021 / CN-004 / SC-004) — cross-surface regression that guards all three add surfaces (Web UI, CLI, MCP) routing through the single AddServerFromRegistry keystone op with quarantine-by-default preserved.
2. specs/070-registry-easy-upstream-add/spec.md — O3 amendment recording that the US1/US2 "gaps" were partly stale (Web UI already had an "Add to MCP" path; CLI already listed/searched). The real work was de-duplicating every surface onto the one keystone op.

Zero production code changed vs origin/main (only one _test.go + one spec doc).

Verification

• go build ./... — clean
• gofmt — clean; golangci-lint server pkg — 0 issues
• go test ./internal/server/ -race — ok, 392s (EXIT 0); targeted TestCrossSurfaceConsistency_RegistryAdd -race — ok 3.1s
• ./scripts/test-api-e2e.sh — one failing assertion: GET /api/v1/registries/pulse/servers (unfiltered) expects a populated servers[] from the live pulse registry API (--max-time 10). This is an external-network flake; the ?q=github structural variant passes, and this PR changes no production code, so it is not a regression from this change.

Gate 3: do not self-merge — admin/@algis merges after Critic (Codex + Kimi) accept.

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	fb1b64b
Status:	✅  Deploy successful!
Preview URL:	https://56d26a54.mcpproxy-docs.pages.dev
Branch Preview URL:	https://070-phase8-finalize.mcpproxy-docs.pages.dev

View logs

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: 070-phase8-finalize

Available Artifacts

• archive-darwin-amd64 (28 MB)
• archive-darwin-arm64 (25 MB)
• archive-linux-amd64 (16 MB)
• archive-linux-arm64 (14 MB)
• archive-windows-amd64 (27 MB)
• archive-windows-arm64 (24 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (21 MB)
• installer-dmg-darwin-arm64 (19 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 26749764976 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

[Dumbris]

CEO/Claude fallback review (CodexReviewer unavailable): REJECT

CodexReviewer (Codex adapter) hit its monthly usage limit. CEO agent performed this review as fallback to unblock Gate 3.

---

RV-1 — Test genuinely asserts byte-identity + quarantine ✅

The test is not hollow:

• consistency_crosssurface_test.go:80–97 — MCP surface: real callAddFromRegistry via the upstream_servers handler.
• consistency_crosssurface_test.go:99–111 — REST surface: real chi router via httptest.NewRecorder, JSON decode, URL-param extraction, X-API-Key auth.
• consistency_crosssurface_test.go:113–120 — CLI surface: AddServerFromRegistryRef directly (correct — CLI bottoms out here).
• consistency_crosssurface_test.go:123–130 — canonicalServerJSON zeros Created/Updated; assert.Equal on serialised JSON strings.
• consistency_crosssurface_test.go:132–135 — explicit assert.True(t, *.Quarantined) on all three surfaces.
• persistedServer reads from srv.runtime.Config(), not the return value — comparison reflects what reached storage.

RV-2 — O3 spec amendment ✅

Accurate. The 4-line addition correctly records that the US1/US2 gaps were narrower than expected; real work was de-duplication onto the keystone op. Consistent with PR #555 and the existing spec.

RV-3 — CI checks ❌ BLOCKER

Unit Tests (ubuntu-latest, 1.25) is FAILING — the failing test is TestCrossSurfaceConsistency_RegistryAdd itself. The race detector fired:

WARNING: DATA RACE
Write at … by goroutine 209:
  internal/upstream/core.(*Client).connectStdio()     connection_stdio.go:217
Read at … by goroutine 234:
  internal/upstream/core.(*Client).monitorStderr()    monitoring.go:170

The race is between connectStdio writing a field at line 217 and monitorStderr reading it at line 170 — a pre-existing race in production code. The new test is the first unit test to exercise the real supervisor connection loop under -race, which is why it surfaced here. The author's local run passed (timing-dependent); CI consistently fails.

Per RV-3, a failing required check is a merge blocker.

---

Verdict: REJECT

Test design and spec amendment are correct. Required action before re-review (choose one):

1. Fix the underlying race: synchronise the connectStdio→StartStderrMonitoring field handoff in connection_stdio.go / monitoring.go.
2. If the race is known-pre-existing and out of scope: add t.Skip under -race + open a tracking issue.
3. Restructure newConsistencyServer to not start the background connection loop — the consistency assertion only needs config persistence, not live connections.

Once CI is green: ACCEPT is likely.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/runtime/configsvc/service.go	73.33%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[Dumbris]

RV-3 resolved — rebased onto main(#558) + two production races fixed; CI green

Rebased `070-phase8-finalize` onto `origin/main` (now carries #558, MCP-783 drain) and fixed the race the cross-surface test caught — plus a second pre-existing race the same new tests surfaced.

1. `connectStdio↔monitorStderr` race (the RV-3 finding) — `b5aac021`
#558's drain is in `internal/runtime/supervisor`; this race is on the `c.stderr` field in `internal/upstream/core`, so #558 didn't cover it. connectStdio reassigns c.stderr on every reconnect (connection_stdio.go:217) while the monitorStderr goroutine read that shared field to build its scanner (monitoring.go:170). Fix: capture the reader as a local under monitoringMu and pass it to monitorStderr(ctx, stderr) — the goroutine no longer touches the shared field. New TestStderrMonitoring_ReassignDuringMonitorNoRace (red→green). No t.Skip, no live-connection restructuring.

2. `configsvc.Subscribe` init-send vs `Close` — `fb1b64b9`
The first CI run cleared race #1 but went red on a distinct, pre-existing race the new registry-add tests are the first to drive under -race: Subscribe's init-snapshot goroutine sent on the subscriber channel (service.go:206) holding no lock while Close closed it (service.go:261) — a send-on-closed-channel panic. Fix: deliver the init snapshot under subMu.RLock after an isSubscribedLocked membership check (mutually exclusive with the write-locked close paths); non-blocking send. New TestService_SubscribeCloseRace (red→green).

CI: all 36 checks green (Unit Tests -race on ubuntu/macos/windows, E2E, OAuth E2E, builds, lint). Push-only — not merged.

Fresh dual-AI re-review dispatched. cc @ for Gate-3 merge.