The same pattern for reading an error response body appeared in three
places across the auth handlers:
- `internal/api/handlers/v0/auth/github_at.go` (twice)
- `internal/api/handlers/v0/auth/github_oidc.go` (once)
A shared `readBody` helper function added to `common.go` now handles
this in one place.
The `io` import is removed from both files since it is no longer used
directly there.
---------
Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
## What this does
Fixes an open redirect plus a handful of small, mostly-invisible
hardening changes around it.
## What's in it
- **The fix**: trailing-slash redirects no longer let `//evil.com/`
redirect off-host.
- **SSRF block** on the DNS/HTTP namespace verification — refuses to
dial loopback, RFC1918, link-local, multicast, CGNAT, or
IP-literal/single-label \"domains\".
- **Slow-client guard**: `ReadTimeout` and `IdleTimeout` on the HTTP
server (no `WriteTimeout` — would risk cutting off legitimate
multi-package publishes).
- **Defence-in-depth headers** on the UI (`/`) and 404 responses.
- **5xx responses** stop leaking raw DB error text to clients (logged
server-side instead).
- **Search filter** stops treating `%` and `_` as wildcards.
- **Validators**: PyPI/NuGet identifiers URL-escaped before fetching;
MCPB no longer follows redirects.
- **Misc**: `Version` field bounded to 255 chars; logs quote user input;
`BlockedNamespaces` denylist now catches subdomain claimants.
Each change is its own commit so they can be reviewed (or reverted)
independently.
## Test plan
- [ ] CI green
- [ ] Synthetic publish in staging for each package type (npm, pypi,
nuget, oci, mcpb) — confirms validator changes don't regress
- [ ] Quick browser check of `/` after deploy to confirm UI still loads
with the new CSP
- [ ] `curl https://<host>//evil.com/` — confirm the open redirect is
closed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
Follow-up to #1215 and #1220. Both of those addressed *individual* slow
queries; this PR addresses *concurrency* — under today's scraper load
the cursor query is fast on average (mean 42ms) but the connection pool
saturates and queue depth blows up at the Go HTTP layer.
## Diagnosis
`pg_stat_statements` (added in #1215) made this possible to see:
```
max_ms mean_ms calls template
10,755 41.8 94,009 plain cursor pagination, no filter
7,277 214.7 14,770 ILIKE substring filter
7,638 330.8 7,656 ILIKE substring + is_latest filter
```
Mean times are healthy. But `max_exec_time` of 7–10s on the cursor
query, combined with sustained ~15 req/s from scrapers (ServiceNow's
148.139.x.x range, anonymous `node` user-agent, etc) saturates
`MaxConns=30 × 2 pods = 60`. New requests queue at the Go HTTP layer;
nginx-side p99 hits 35s; eventually scrapers time out at 60s, retry, and
amplify the queue.
Today's two ongoing alerts are both this pattern:
- 17:35–17:40 UTC `Availability dropped below 95%` — 4,526 GET requests
in 5 min, hundreds of 504s
- 18:17 UTC `Publish Endpoint Latency` re-fire — same scraper
concurrency dragging the publish path
Critically, the symptoms today were also visible during yesterday's
incident, but yesterday's broken cursor (#1215) was the dominant cause.
After #1215 the cursor is fast individually; concurrency now becomes the
next bottleneck.
## Changes
### pgxpool (`internal/database/postgres.go`)
| Setting | Before | After |
|---------|-------:|------:|
| `MaxConns` | 30 | **60** per pod |
| `MinConns` | 5 | **10** per pod |
| `MaxConnIdleTime` | 30 min | (unchanged) |
| `MaxConnLifetime` | 2 h | (unchanged) |
2 pods × 60 = 120 total app connections. Cuts queue depth roughly in
half at current scraper load.
### PG cluster (`deploy/pkg/k8s/postgres.go`)
- `max_connections: 100 → 200` — required to support the larger pool.
120 (app) + ~10 (PG internals: autovacuum, replication, admin) + 70
headroom.
- Added explicit `resources:` block — previously the pod had no resource
limits, making node-level OOM behaviour unpredictable.
| | Request | Limit |
|---|--------:|------:|
| memory | 512Mi | **4Gi** |
| cpu | 200m | 1500m |
## Resource budget
| Node | Now | After |
|------|----:|------:|
| dy89 (PG node) | 39% mem (~2.4 GiB / 6 GiB) | ~65% mem worst-case
(~3.9 GiB) |
| 2yxm | 36% mem | unchanged |
Both nodes well within capacity. CPU usage <50% on both, plenty of
headroom.
PG worst-case memory math:
- 200 conns × ~15 MB per backend = 3 GiB
- + `shared_buffers` 128 MiB
- + `maintenance_work_mem`, OS overhead, etc.
- ≈ 3–4 GiB total
## Deployment caveat
`max_connections` is a postmaster-level setting → CNPG triggers a PG
restart on the change. Same in-pod restart shape as the
pg_stat_statements deploy yesterday — registry pods see ~30s of DB
unavailability, covered by v1.7.1's retry-with-backoff (8 attempts, 1→8s
capped). One registry pod may bounce once before recovering, like
yesterday.
**Time the merge for a low-traffic UTC window.**
Order of operations matters within the deploy itself:
1. Pulumi applies the new CNPG spec → PG restarts with
`max_connections=200`
2. Rolling deploy of registry pods picks up `MaxConns=60` config
3. New conns are accepted up to 200 limit
Pulumi's standard ordering does step 1 before step 2 in this scenario
(Pulumi resource graph: CNPG cluster precedes Deployment). If for any
reason it doesn't, the worst case is `too many connections` errors
during a small window — Self-correcting once the rollout completes.
## Test plan
- [x] `go build ./...` clean for app + deploy
- [x] `make lint` clean
- [x] `go test -race ./internal/database/...` green
- [ ] On merge: staging deploy applies the spec change cleanly; PG
restarts; pool reaches 60 max
- [ ] On prod deploy: same; verify no `too many connections` errors
during the rollout window
- [ ] After deploy: query `SHOW max_connections` returns 200; query
pg_stat_statements after a scraper burst, confirm `max_exec_time` for
cursor query no longer hits 10s
## Out of scope
- **ILIKE substring search** (`server_name ILIKE '%foo%'`) is
unindexable. Three pg_stat_statements variants run with means 141–333ms
and max 4–7s. Worth replacing with a `pg_trgm` GIN index or full-text
search in a separate PR.
- **Per-IP nginx rate limiting** — defends against scraper retry storms
regardless of pool size.
- **`Cache-Control` on `/v0/servers`** — let nginx absorb
scraper-repeated cursors.
- **Pre-existing `superfluous WriteHeader` warnings** — separate Huma
framework issue.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
Same shape of bug as #1215's cursor fix, different filter. The RemoteURL
filter built its WHERE predicate using `jsonb_array_elements` + `->>`
extraction, which the planner can't translate into a GIN search.
Replaced with a JSONB containment predicate `@>` that uses
`idx_servers_json_remotes`.
## Diagnosis (from the per-phase publish slog #1215 added)
Today's first publish-latency alert at 17:08 UTC pointed straight at
this:
```
publish complete server_name=dev.storage/mcp version=1.10.1 total_ms=14822
validate_ms=0 lock_ms=238 remotes_ms=10980 version_checks_ms=535 unmark_ms=3014 create_ms=53
```
`remotes_ms=10,980` on a server with **one** remote URL pointed at
`validateNoDuplicateRemoteURLs`, which runs the RemoteURL filter against
the database. EXPLAIN ANALYZE on prod confirmed:
```
Filter: (status != 'deleted') AND EXISTS(SubPlan 1)
Rows Removed by Filter: 21,092 ← scans the whole table
Buffers: shared hit=20,028
Execution Time: 10,005 ms
```
## Fix
`internal/database/postgres.go:99` rewritten:
```sql
-- before (planner can't use GIN index for this)
EXISTS (SELECT 1 FROM jsonb_array_elements(value->'remotes') AS remote
WHERE remote->>'url' = $1)
-- after (GIN-indexable)
value -> 'remotes' @> jsonb_build_array(jsonb_build_object('url', $1::text))
```
Semantically equivalent for our schema (`url` is always a string, NOT
NULL).
## Local benchmark — 20K rows, prod-shaped JSONB (~1KB/row)
| Form | Buffer hits | Index used | Time |
|------|------------:|------------|-----:|
| OLD | 40,398 | none — full scan | 17.0 ms |
| NEW | **294** | **`idx_servers_json_remotes` GIN bitmap** | **1.6 ms**
|
Maps to prod's 10,005 ms cold-cache observation — same shape of speedup
we got from the cursor fix.
## Scope honesty
This PR addresses the **publish slowness path**
(`validateNoDuplicateRemoteURLs` in the publish transaction). It
directly resolves today's 17:08 UTC `Publish Endpoint Latency` alert.
It does **not** address:
- Today's 17:35–17:40 UTC `Availability dropped below 95%` alert.
Per-investigation: only 4 publishes happened in the entire 17:00–17:50
window and zero during the burst minutes — so my initial "concurrent
slow publishes starved the pool" hypothesis was wrong. The actual cause
was scraper-driven concurrency (4,500+ requests in 5 min) on plain
cursor pagination + ILIKE substring search, where individual queries are
fast (mean 42 ms) but tail latency under concurrency reaches 10 s
server-side and 20–35 s nginx-level due to pool queueing.
- The 18:17 UTC `Publish Endpoint Latency` re-fire (same scraper-driven
concurrency).
A follow-up PR will raise pool size + PG `max_connections` + add
explicit PG resource limits to absorb concurrency without queue blowup.
That fix has a different shape (config) and a different risk profile (PG
restart for `max_connections`); kept separate so this surgical SQL
change can ship cleanly.
## Test plan
- [x] `go build ./...` clean
- [x] `make lint` clean
- [x] `go test -race ./internal/database/... ./internal/service/...`
green — existing `TestPostgreSQL_ListServers/filter by remote URL`
covers semantic correctness
- [x] EXPLAIN ANALYZE comparison locally on 20K rows + prod-shaped JSONB
- [ ] After merge + deploy: verify on prod via `EXPLAIN ANALYZE`;
confirm `remotes_ms` in `publish complete` slog drops to single-digit ms
## Out of scope (separate follow-ups)
- pgxpool `MaxConns` / `MinConns` bump + PG `max_connections` raise + PG
`resources` limits — addresses the cursor-tail and ILIKE-search
concurrency that this PR doesn't touch
- Per-IP rate limiting at nginx — defends against scraper retry storms
regardless of query speed
- `Cache-Control` headers on `/v0/servers` — let nginx absorb scraper
repeats
- Pre-existing `superfluous WriteHeader` warnings — separate Huma
framework issue
- ILIKE substring search on `server_name` is unindexable — could be
replaced with `pg_trgm` GIN index or full-text search
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
Three changes from the 2026-04-27 incident:
1. **Cursor SQL fix** — `addCursorCondition` rewritten to use a
row-constructor comparison `(server_name, version) > ($1, $2)` instead
of the OR-decomposed form. Postgres can index-seek on the row
constructor; it can't on the OR form, which scanned from the start of
the index and filtered rows before the cursor. Cost grew linearly with
cursor depth.
2. **Per-phase publish timings restored** — `createServerInTransaction`
was simplified during #1211 review to log only `validate_ms`. The
incident showed publishes spending 50+ s in `acquire_lock` /
`version_checks` / `db_create` while `validate_ms` reported a few
hundred ms — the diagnostic signal was hidden. Restored timings for
every phase, refactored into a small `runPhase` helper.
3. **`pg_stat_statements` enabled** in the CNPG cluster spec. We had no
aggregate query visibility during the incident — could only EXPLAIN
queries we happened to suspect.
## Cursor fix evidence
Local benchmark, 100K rows, cold cache:
| Form | Rows filtered | Buffer reads | Time |
|------|--------------:|-------------:|-----:|
| OLD | 80,001 | 7,679 | 31.6 ms |
| NEW | 2 | 1 | **0.05 ms** |
Maps to prod's measured 8,911 buffer hits → 760 ms. End-to-end API:
`/v0/servers?limit=100&cursor=…` returns in 4–7 ms regardless of depth.
Risk check: confirmed `(server_name, version)` index exists in that
column order (both `servers_pkey` and `idx_servers_name_version`), and
both columns are `NOT NULL` so row-constructor comparison can't silently
drop rows.
## Tests
- New
`TestPostgreSQL_PerformanceScenarios/compound_cursor_across_versions_of_same_server`
pins multi-version pagination semantics. The existing cursor tests only
exercised the fallback (single-component cursor) and degenerate (one
version per server) cases.
- `make lint` clean, `go test -race ./internal/... ./cmd/...` green.
## Deployment
The cursor + slog changes are zero-downtime. The CNPG spec change
triggers a brief PG restart (single-instance cluster). After the
restart, run once on prod:
```bash
kubectl exec -i registry-pg-1 -c postgres \
--context gke_mcp-registry-prod_us-central1-b_mcp-registry-prod \
-- psql -U postgres -c "CREATE EXTENSION IF NOT EXISTS pg_stat_statements"
```
Time the merge for a low-traffic UTC window. v1.7.1's DB
retry-with-backoff covers the brief PG restart.
## Out of scope
`MaxConns` bump, per-IP rate limiting at nginx, response caching, the
pre-existing `superfluous WriteHeader` warnings — separate follow-ups.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
Fixes#1081. When the version flagged `is_latest` was soft-deleted, no
other version was promoted, and because `GetCurrentLatestVersion` didn't
filter deleted rows, subsequent publishes with a lower semver never
became latest — leaving `/v0/servers/{name}/versions/latest` permanently
404.
This PR has two parts:
### 1. Code fix — recalculate `is_latest` on every status change
- Recalculate `is_latest` after every status mutation
(`UpdateServerStatus`, `UpdateAllVersionsStatus`, and the inline status
change in `UpdateServer`), in the same transaction that already holds
the per-server advisory lock.
- Winner selection reuses the existing `CompareVersions` logic: highest
non-deleted version gets the flag; if every version is deleted, the
highest deleted version keeps it so admin lookups via
`includeDeleted=true` still resolve the server (important for restore
flows).
- New `SetLatestVersion` DB primitive issues two statements (clear old,
set new) — `idx_unique_latest_per_server` is a non-deferrable unique
partial index, so a single `UPDATE` that flips two rows trips the
constraint.
### 2. One-shot migration — heal servers already affected (no manual SQL
needed)
`014_heal_is_latest.sql` promotes the highest non-deleted version on
every server whose `is_latest` flag is on a deleted row (or absent
entirely). Picker parses `major.minor.patch` numerically and falls back
to `published_at` for non-semver or as a tiebreaker. Heals
`io.github.containers/kubernetes-mcp-server` to `0.0.61` automatically
and catches any other servers silently affected by the original bug,
without hardcoded version numbers.
Picker behavior vs. `CompareVersions`:
- Standard `M.m.p` semver: matches exactly, including the
backport-after-delete case (e.g. published 1.0.1 then 1.0.0 hotfix after
deleting 2.0.0 → picks 1.0.1, not 1.0.0).
- Non-semver: falls through to `published_at DESC`. Reasonable.
- Prereleases (`1.0.0-alpha` vs `1.0.0`): both parse to `(1,0,0)`,
tiebreak by `published_at`. Diverges from `CompareVersions` (which
always says `1.0.0 > 1.0.0-alpha`). Edge case requires (a) server hit by
this bug AND (b) prereleases at the top of the version list. Acceptable;
the publisher can flip it with one `mcp-publisher status` call (which
now triggers proper recalc thanks to part 1).
## Verification
End-to-end curl repro of the exact scenario from the issue, run against
both `main` and this branch:
**Before (main):**
```
publish 1.0.0 → 200
publish 0.0.59 → 200
PATCH .../1.0.0/status {deleted} → 200
GET /versions/latest → 404 ← bug
publish 0.0.60 → 200
GET /versions/latest → 404 ← bug persists, 0.0.60 has isLatest=False
```
**After (this branch):**
```
publish 1.0.0 → 200
publish 0.0.59 → 200
PATCH .../1.0.0/status {deleted} → 200
GET /versions/latest → 200 (0.0.59, isLatest=true)
publish 0.0.60 → 200
GET /versions/latest → 200 (0.0.60, isLatest=true)
```
## Test plan
- [x] Service tests covering: soft-delete latest → next highest active
promoted; the exact #1081 repro end-to-end through the service;
all-versions-deleted still keeps highest addressable for admin;
restore-to-higher-version reclaims latest.
- [x] Migration tests covering: kubernetes-mcp-server scenario (heals to
highest semver); backport scenario (semver beats most-recent-published);
no-is_latest-row defensive case; all-deleted server left untouched;
healthy server left untouched; non-semver versions fall back to
`published_at`.
- [x] Full unit test suite (`./internal/... ./cmd/...`) passes against a
real Postgres.
- [x] `make lint` (0 issues) and `make validate` pass.
- [x] Manual curl repro against a running registry confirms the fix
resolves the failure mode and `main` still reproduces it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
We keep bumping `go 1.26.x` across the repo every time a new Go patch
drops
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
- **`go.mod` / `deploy/go.mod`**: `go 1.25.8` → `go 1.26` (minor only —
"this module needs Go 1.26")
- **All CI workflows**: `go-version-file: 'go.mod'` → `go-version:
'stable'` — CI always uses the latest stable Go, no more
bump PRs
- **govulncheck**: removed hardcoded `go-version-input: '1.26.1'` →
`go-version-input: 'stable'`
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
(Updated auto-generated PR description from @tadasant / Claude on Feb
22, 2026)
## Motivation and Context
Fixes#637
Server lifecycle management (deprecation and yanking) is a standard
requirement for package registries. Use cases like rebranding, security
vulnerabilities, and end-of-life servers demand the ability to mark
servers as deprecated or deleted, with optional messaging to guide
consumers toward alternatives.
## Changes
### New API endpoints
- **`PATCH /v0/servers/{serverName}/versions/{version}/status`** —
Update the lifecycle status of a single server version
- **`PATCH /v0/servers/{serverName}/status`** — Update the lifecycle
status of all versions of a server in a single transaction
Both endpoints accept:
```json
{
"status": "active|deprecated|deleted",
"statusMessage": "Optional message (max 500 chars) explaining the change"
}
```
### New CLI command
- **`mcp-publisher status <serverName> [version]`** — Update server
lifecycle status from the command line
- `--status` (required): `active`, `deprecated`, or `deleted`
- `--message`: Optional status message
- `--all-versions`: Apply to all versions of the server (replaces the
`<version>` argument)
### API response changes
`RegistryExtensions` now includes two new fields:
- `statusChangedAt` — Timestamp of the last status change
- `statusMessage` — Optional free-form message (e.g., deprecation
reason, migration guidance)
### List endpoint changes
- New `include_deleted` query parameter on `GET /v0/servers` — defaults
to `false`, hiding deleted servers from listings
- When `updated_since` is provided, `include_deleted` is automatically
set to `true` for incremental sync
### Database
- Migration `013_add_status_fields.sql`: Adds `status_changed_at` (NOT
NULL, initialized from `published_at`) and `status_message` columns to
the `servers` table
### Design decisions (from review discussion)
- **Dedicated status endpoints** rather than overloading the existing
edit endpoint
- **Bidirectional status transitions** — all transitions between
`active`, `deprecated`, and `deleted` are allowed
- **`statusMessage` as free-form text** — no structured `nextName` or
`alternativeUrl` fields (per maintainer feedback, to avoid complexity
around chaining, trust transfer, etc.)
- **Publish permission** grants status change access (not limited to
admin/edit)
- **Status message auto-cleared** when transitioning back to `active`
Good, now I have the full picture. Here's the addendum:
### Consumer-facing API changes
The new status fields live inside
`_meta["io.modelcontextprotocol.registry/official"]` on every
`ServerResponse`. This is the existing `RegistryExtensions` object — two
new fields are added alongside the existing ones:
**Before:**
```json
{
"server": { "name": "com.example/my-server", "...": "..." },
"_meta": {
"io.modelcontextprotocol.registry/official": {
"status": "active",
"publishedAt": "2025-06-01T00:00:00Z",
"updatedAt": "2025-06-15T00:00:00Z",
"isLatest": true
}
}
}
```
**After:**
```json
{
"server": { "name": "com.example/my-server", "...": "..." },
"_meta": {
"io.modelcontextprotocol.registry/official": {
"status": "deprecated",
"statusChangedAt": "2026-02-01T12:00:00Z",
"statusMessage": "Deprecated in favor of com.example/my-server-v2",
"publishedAt": "2025-06-01T00:00:00Z",
"updatedAt": "2026-02-01T12:00:00Z",
"isLatest": true
}
}
}
```
| New field | Type | Present when |
|-----------|------|-------------|
| `statusChangedAt` | `string` (RFC3339) | Always — initialized to
`publishedAt` for existing records |
| `statusMessage` | `string` (max 500 chars) | Only when set —
`null`/omitted for active servers (auto-cleared on transition to active)
|
These fields appear on **every endpoint that returns a
`ServerResponse`**: `GET /v0/servers`, `GET /v0/servers/{name}`, `GET
/v0/servers/{name}/versions/{version}`, and the new PATCH status
endpoints.
### List endpoint filtering
`GET /v0/servers` gains an `include_deleted` query parameter:
| Scenario | Behavior |
|----------|----------|
| No `include_deleted` param | Deleted servers hidden (default `false`)
|
| `include_deleted=true` | Deleted servers included in results |
| `updated_since` provided | `include_deleted` forced to `true` for
incremental sync |
## How Has This Been Tested?
- Unit tests for status handler (`status_test.go`) covering status
transitions, permission checks, validation errors, and the all-versions
endpoint
- Unit tests for the CLI command (`status_test.go`)
- Integration with existing server/edit handler tests
- Database layer tests for the new status update and query methods
- Tested locally with `make dev-compose`
## Types of changes
- [x] New feature (non-breaking change which adds functionality)
## Checklist
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [x] I have added or updated documentation as needed
## Open items from review
- [ ] `include_deleted` silent override: When `updated_since` is
provided and `include_deleted=false` is explicitly passed, should return
a 400 error rather than silently overriding
([thread](https://github.com/modelcontextprotocol/registry/pull/893#discussion_r2100879360))
— agreed upon but may not yet be implemented
## Summary
Fixes a race condition where a server could have no version marked as
`isLatest` if the database operations failed independently.
## Changes
- **CreateServer signature**: Now accepts `oldLatestVersionID` parameter
to atomically unmark previous latest version
- **PostgreSQL implementation**: Uses a transaction to ensure UPDATE
(unmark old latest) and INSERT (create new version) happen atomically
- **Service layer**: Maintains `WithPublishLock` for concurrent
operation serialization
- **Two-level protection**: Advisory lock prevents races, transaction
ensures atomicity
## Test plan
- [x] Existing race condition tests pass
- [x] Integration tests pass
- [x] Unit tests pass
🤖 Generated with [Claude Code](https://claude.ai/code)
---------
Co-authored-by: Claude <noreply@anthropic.com>
## Summary
Fixes race conditions in concurrent server publishing that could result
in data integrity violations:
- Multiple versions marked as `isLatest=true` for the same server
- Duplicate (serverId, version) combinations
- Same server name assigned different serverIDs
## Solution
Uses two complementary approaches for correctness:
### 1. PostgreSQL Advisory Locks
- Serializes concurrent publishes per server name using
`pg_advisory_xact_lock`
- Uses FNV-1a hash to convert server name to lock ID
- Transaction-scoped locks auto-release on commit/rollback
- **Required after #465**: The serverId introduction means concurrent
publishes must atomically coordinate whether to reuse an existing
serverId (for additional versions) or generate a new one (for first
version). Without locks, race conditions would assign different
serverIDs to versions of the same server.
### 2. Unique Constraints (Migration 007)
Two new indexes provide data integrity guarantees:
- **idx_unique_server_version**: Ensures (serverId, version) is unique
across all versions
- **idx_unique_latest_version**: Partial index ensuring only one version
per serverId has `isLatest=true`
These constraints serve as defense-in-depth safety nets. Even if lock
logic had a bug, the database would reject integrity violations.
Also drops redundant `idx_servers_version_id` since `version_id` is
already the primary key.
## Why WithPublishLock validation is needed after #465
The `ctx.Err()` check at the start of `WithPublishLock` is critical
because:
1. After #465 introduced serverId, the publish flow must atomically
check existing versions and assign/reuse serverId
2. If the context is cancelled during lock acquisition, we could
deadlock
3. Checking ctx validity before acquiring locks prevents these deadlocks
## Testing
Added `TestPublishConcurrentVersionsNoRace` with 100 concurrent
goroutines publishing different versions of the same server. All succeed
with:
- Exactly one version marked as latest
- All versions sharing the same serverId
- No duplicate (serverId, version) combinations
NB: this only covers the in memory db, not the postgres db. It'd be nice
to write tests for this one as well, but our current setup is a bit
annoying for this. Separately looking into ways to easily run all our
tests with real postgres and ideally scrap memory.go completely.
Wielded Claude Code to do this; across many iterations and have reviewed
all the code + largely rewritten the PR commentary here.
This PR implements the server ID consistency changes from
[#396](https://github.com/modelcontextprotocol/registry/issues/396). Our
goal is to:
- Rename `id` to `versionId` to make it clear that it is the ID for a
specific version, not a server
- Introduce `serverId` as a net new concept. This will be 1:1 with
`name`. While not strictly necessary (we could just use name as a PK),
the convention of having a UUID as an ID on our objects is just a
helpful subjective architecture decision. Plus also leaves open the door
to reconsidering the "should we allow renames" question.
## Key Changes
### 🗄️ Database Schema Changes
- **Renamed**: `id` column → `version_id` (unique identifier for each
server version)
### 📋 Metadata Structure Updates
- **JSON Schema**: Updated
`_meta.io.modelcontextprotocol.registry/official` to include:
- `serverId`: Consistent ID across all versions of a server
- `versionId`: Unique ID for this specific version (renamed from `id`)
### 🔧 API Endpoint Updates
- **New endpoint structure**: Changed from version-ID-based to
server-ID-based URLs:
- `GET /v0/servers/{server_id}` - Get latest version of server
- `GET /v0/servers/{server_id}?version=X.X.X` - Get specific version
- `GET /v0/servers/{server_id}/versions` - List all versions of a server
- `PUT /v0/servers/{server_id}?version=X.X.X` - Edit specific version
- **Consistent URL patterns**: All server endpoints now use `server_id`
as the primary identifier
### Database Migration Strategy
The migration (`005_add_server_id_rename_version_id.sql`):
1. Adds new `version_id` column
2. Generates deterministic UUIDs for servers with the same name
3. Updates JSON metadata to add `serverId` and rename `id` to
`versionId`
4. Copies existing `id` values to `version_id` column
5. Drops old `id` column and makes `version_id` the new primary key
## Testing
Besides of course automated tests passing, I:
1. Set up my local db with the pre-005-migration state, imported all the
production data
2. Ran the migration
3. Spot checked some diffs of the data that resulted --
```json
// before
{
"name": "io.github.ruvnet/ruv-swarm",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"id": "0a9cda7f-2b73-4839-844b-6582f335de42",
"is_latest": false,
"updated_at": "2025-09-10T17:21:34.92137012Z",
"published_at": "2025-09-10T17:15:32.69690433Z"
}
},
"version": "1.0.18",
"repository": {
"url": "",
"source": ""
},
"description": "Neural network swarm orchestration with WebAssembly acceleration and MCP integration"
}
// after
{
"name": "io.github.ruvnet/ruv-swarm",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"serverId": "e8538952-96af-4a33-9e31-7fbed012521e",
"is_latest": false,
"versionId": "0a9cda7f-2b73-4839-844b-6582f335de42",
"updated_at": "2025-09-10T17:21:34.92137012Z",
"published_at": "2025-09-10T17:15:32.69690433Z"
}
},
"version": "1.0.18",
"repository": {
"url": "",
"source": ""
},
"description": "Neural network swarm orchestration with WebAssembly acceleration and MCP integration"
}
```
Servers that have multiple versions stayed consistent in their
`serverId`:
```json
// First version
{
"name": "app.getdialer/dialer",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"serverId": "80f08bdb-f3e4-4f19-b072-1676ae49b663",
"is_latest": false,
"versionId": "04f25b17-331d-48d7-b08b-37baece208a7",
"updated_at": "2025-09-09T00:16:49.162620072Z",
"published_at": "2025-09-08T23:47:09.452137971Z"
}
},
"status": "active",
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
"remotes": [
{
"url": "https://getdialer.app/mcp",
"type": "streamable-http"
},
{
"url": "https://getdialer.app/sse",
"type": "sse"
}
],
"version": "1.0.0",
"repository": {
"url": "",
"source": ""
},
"description": "An MCP server that provides your you make outbound phone calls using your own phone number"
}
// Second version
{
"name": "app.getdialer/dialer",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"serverId": "80f08bdb-f3e4-4f19-b072-1676ae49b663",
"is_latest": true,
"versionId": "faa26cb9-3aad-415e-aece-f715fff5fe5e",
"updated_at": "2025-09-09T00:16:49.107842808Z",
"published_at": "2025-09-09T00:16:49.107842808Z"
}
},
"status": "active",
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
"remotes": [
{
"url": "https://getdialer.app/mcp",
"type": "streamable-http"
},
{
"url": "https://getdialer.app/sse",
"type": "sse"
}
],
"version": "1.0.1",
"repository": {
"url": "",
"source": ""
},
"description": "An MCP server that provides lets you make outbound phone calls using your own phone number"
}
```
And I also tried publishing a test server anew; looked good:
```bash
⏺ Bash(curl -s "http://localhost:8080/v0/servers/102dc4e4-6ca8-40e6-aa11-cd45763d3352" | jq '.')
⎿ {
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
"name": "io.modelcontextprotocol.anonymous/test-claude-migration-server",
"description": "A test server to verify the new migration and API endpoints work correctly",
"status": "active",
"repository": {
"url": "https://github.com/example/test-claude-migration-server",
"source": "github",
"id": "example/test-claude-migration-server"
},
"version": "1.0.0",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"serverId": "102dc4e4-6ca8-40e6-aa11-cd45763d3352",
"versionId": "48c9cd55-7bc4-4ac2-896e-59e81eb653e4",
"published_at": "2025-09-12T18:33:23.264203-07:00",
"updated_at": "2025-09-12T18:33:23.264203-07:00",
"is_latest": true
}
}
}
⏺ Bash(curl -s "http://localhost:8080/v0/servers/102dc4e4-6ca8-40e6-aa11-cd45763d3352/versions" | jq '.')
⎿ {
"servers": [
{
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json",
"name": "io.modelcontextprotocol.anonymous/test-claude-migration-server",
"description": "A test server to verify the new migration and API endpoints work correctly",
"status": "active",
"repository": {
"url": "https://github.com/example/test-claude-migration-server",
"source": "github",
"id": "example/test-claude-migration-server"
},
"version": "1.0.0",
"_meta": {
"io.modelcontextprotocol.registry/official": {
"serverId": "102dc4e4-6ca8-40e6-aa11-cd45763d3352",
"versionId": "48c9cd55-7bc4-4ac2-896e-59e81eb653e4",
"published_at": "2025-09-12T18:33:23.264203-07:00",
"updated_at": "2025-09-12T18:33:23.264203-07:00",
"is_latest": true
}
}
}
],
"metadata": {
"count": 1
}
}
```
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Preeti Dewani <preetidewani1990@gmail.com>
Co-authored-by: adam jones <adamj+git@anthropic.com>
Co-authored-by: Adam Jones <adamj@anthropic.com>
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR should fix the recent "conn busy" issue caused by the
PostgreSQL implementation using a single database connection (*pgx.Conn)
instead of a connection pool (*pgxpool.Pool). When multiple concurrent
requests try to access the endpoint, they would conflict trying to use
the same connection.
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Builds locally
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
No
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Related to #400, #405 and #407
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
summary
- use the indexed primary key (`id`) for `getbyid`, `updateserver`, and
list ordering/cursor, instead of extracting `_meta...id` from jsonb
- in `updateserver`, error if the body tries to change the id; set
`meta.official.id` when missing
changes
- database: `getbyid` → `where id = $1`
- database: `updateserver` → `where id = $2`; fail on id mismatch; set
id if missing
- database: `list` orders by `id` and paginates via `id > $cursor`
- no schema or api changes
<details>
<summary>performance</summary>
- env: repo docker compose postgres, 5k–10k rows, warm cache
- getbyid: `id` ≈ 0.02 ms vs jsonb path ≈ 0.80 ms
- update: `id` ≈ 0.15 ms vs jsonb path ≈ 0.78 ms
- list (limit 100): order by `id` ≈ 0.13 ms vs jsonb path id ≈ 2.24 ms
(~17x slower)
</details>
<details>
<summary>explain analyze snippets</summary>
getbyid by `id`
```
Index Scan using idx_benchmark_servers_id on benchmark_servers (actual time≈0.01–0.02 ms)
Buffers: shared hit≈3
```
getbyid by jsonb path
```
Seq Scan on benchmark_servers (actual time≈0.43–0.83 ms)
Filter: ((value->'_meta'->'io.modelcontextprotocol.registry/official'->>'id') = $1)
Rows Removed by Filter: ≈4999
Buffers: shared hit≈455
```
update by `id`
```
Update on benchmark_servers …
-> Index Scan using idx_benchmark_servers_id … (actual time≈0.007 ms)
Execution Time: ≈0.15 ms
```
update by jsonb path
```
Update on benchmark_servers …
-> Seq Scan on benchmark_servers … Rows Removed by Filter: ≈4999
Execution Time: ≈0.78 ms
```
</details>
<details>
<summary>reproduce quickly</summary>
- start postgres: `docker compose up -d postgres`
- create a simple table (id varchar pk, value jsonb) with several
thousand rows
- compare plans: `explain (analyze, buffers) select value from … where
id = $1` vs the jsonb path predicate
</details>
---------
Co-authored-by: Adam Jones <adamj@anthropic.com>
## Summary
- Adds `updated_since`, `search`, and `version` query parameters to `GET
/v0/servers`
- Enables incremental synchronization via RFC3339 timestamp filtering
- Provides simple substring search on server names
- Supports latest version filtering with `version=latest`
This implementation provides the core filtering capabilities needed for
efficient registry synchronization and improved server discovery.
Fixes#291Fixes#135
## Summary
Updates the `_meta` field structure to be compliant with the MCP
specification by using proper `prefix/name` format for vendor
extensions.
### Changes
- **Registry metadata**: `io.modelcontextprotocol.registry` →
`io.modelcontextprotocol.registry/official`
- **Publisher metadata**: `publisher` →
`io.modelcontextprotocol.registry/publisher-provided`
- **Database migration**: Updates indexes to use new field paths
- **Updated all code**: Service layer, validators, tests, and seed data
### Format Before
```json
{
"_meta": {
"publisher": { "license": "MIT" },
"io.modelcontextprotocol.registry": { "id": "...", "is_latest": true }
}
}
```
### Format After
```json
{
"_meta": {
"io.modelcontextprotocol.registry/publisher-provided": { "license": "MIT" },
"io.modelcontextprotocol.registry/official": { "id": "...", "is_latest": true }
}
}
```
This resolves the naming collision with the planned top-level
`publisher` field and ensures compliance with MCP specification
requirements.
Fixes#356
## Motivation and Context
The database logic was getting a bit unwieldy and fragile - especially
because business logic that should really have lived at the registry
service level was being pushed into the persistence level.
This PR tightens the interface, simplifies the code there, as well as
rips out a bunch of mocks etc. we don't need.
## How Has This Been Tested?
Locally everything works
## Breaking Changes
The list endpoint was surprisingly always filtering for isLatest only,
which seems like a bug? I have removed this filter.
<!-- Provide a brief summary of your changes -->
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR eliminates duplicate struct definitions by migrating
the publisher tool to use canonical types from pkg/model, creating a
single source of truth across the codebase. The publisher tool
previously had its own copies of structs that were duplicated, ie.
ServerJSON, Package, Repository, VersionDetail, EnvironmentVariable,
RuntimeArgument and a few others which was not optimal
**Changes in the PR:**
- Refactored the structs to single canonical types located at pkg/model
- Refactor the implementation to use/build canonical structures directly
- Remove all duplicate definitions (5 structs eliminated)
- Publisher now uses model.ServerJSON, model.Package, model.Repository,
etc.
- Renamed some of the structs to better reflect their use case, i.e.
ServerJSON instead of ServerDetail
- Fixes an issue where the Remote property was missing from the previous
publisher copy of the Server
**Benefits:**
- Single source of truth - Schema changes in one place
- Type safety - Compiler catches mismatches
- Enhanced validation - Field constraints and bson tags
- Zero breaking changes - Same CLI interface and JSON output
- Future-proof - New canonical fields automatically available
Supersedes: #217 and #240
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Locally
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
No
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
---------
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
## Summary
Implements `PUT /v0/servers/{id}` admin edit endpoint to address part of
issue #182, providing an API-based solution for server moderation
without requiring direct database access.
- **New endpoint**: `PUT /v0/servers/{id}` for editing existing servers
- **Security**: Uses existing JWT auth with `PermissionActionEdit`
permissions
- **Anti-undelete protection**: Added `ServerStatusDeleted` status that
cannot be reverted
- **Full implementation**: Database, service, API, and comprehensive
test coverage
## Key Use Cases
- Mark malicious servers as `deleted` for takedown
- Update server metadata for moderation actions
Related to: https://github.com/modelcontextprotocol/registry/issues/182
(but that is not _done_ yet because we need admins to be able to login
(probably with #283), and need to add docs on how to do the takedown,
and probably guidelines on what we will/won't takedown)
## Summary
- Add reverse-DNS namespace format validation for server names
(namespace/name)
- Validate remote URLs match the DNS namespace derived from hostname
- Prevent duplicate remote URLs across different servers
- Support localhost/development URLs without namespace restrictions
- Add comprehensive test coverage for validation logic
- Update database layer to support remote_url filtering
Fixes#180 by ensuring proper namespace governance and preventing URL
conflicts.
---------
Co-authored-by: Claude <noreply@anthropic.com>
This implements the versioning approach agreed upon in issue #158:
## Key Changes
### Documentation
- Added comprehensive versioning guide (docs/versioning.md)
- Updated FAQ with versioning guidance and best practices
- Enhanced schema description with SHOULD recommendations
### Schema Updates
- Added 255-character limit for version strings in schema.json
- Enhanced version field description with semantic versioning guidance
### Implementation
- New isSemanticVersion() function for proper semver detection
- Enhanced compareSemanticVersions() with prerelease support
- Implemented compareVersions() strategy:
* Both semver: use semantic comparison
* Neither semver: use timestamp comparison
* Mixed: semver versions always take precedence
- Updated publish logic to determine "latest" using new strategy
- Added version length validation (255 char max)
### Versioning Strategy
1. Version MUST be string up to 255 characters
2. SHOULD use semantic versioning for predictable ordering
3. SHOULD align with package versions to reduce confusion
4. MAY use prerelease labels for registry-specific versions
5. Registry attempts semver parsing, falls back to timestamp ordering
6. Clients SHOULD follow same comparison logic
Resolves#158
---------
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: adam jones <domdomegg@users.noreply.github.com>
<!-- Provide a brief summary of your changes -->
This PR serves as a placeholder and brings together all of the changes
related to supporting vendor extensions passed through the API. The idea
is to have a single place to track and review the work as it comes
together (even if it’s still a bit of a work in progress).
**The changes in this PR cover:**
* Updates on the data model
* Updates on both databases (in-memory and postgresql), added a new
table to PSQL for tracking the extensions leaving the server table only
for the clean server.json properties
* Updates the seed.json file (thus the big line change)
* Adds a seed-migrate tool to help migrating an existing seed file to
the new format
* Updates the seed import mechanism to support this
* Updates the existing tests - unit, integration and validation tests
* Adds the publish handler to the OpenAPI spec (it was missing)
* Fixes linting errors
* Updates on the API side on (publish, list, get) to support the new
format of `ServerResponse` -
```
{
"server": {},
"x-io.modelcontextprotocol.registry": {}, /*not available for publish since this gets populated by the registry*/
"x-publisher": {}
}
```
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
Motivated by the discussions in:
https://github.com/modelcontextprotocol/registry/discussions/284
## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Locally and by running all of the tests
## Breaking Changes
<!-- Will users need to update their code or configurations? -->
Yes, it changes both the API and the format of the seed file
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
---
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: adam jones <domdomegg+git@gmail.com>
Fixes#228
## Motivation and Context
See #19
## How Has This Been Tested?
- Run locally
- Run in docker compose
- Run infra in minikube
- Unit and integration tests all passing :)
## Breaking Changes
Yes: users will not be able to access any data they used to have in
custom mongodb. We are accepting this breaking change, given the
registry is in experimental development status.
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [x] I have added or updated documentation as needed
## Additional context
I have not implemented connection pooling support for simplicity, and
because cloudnative-pg already provides built-in connection pooling with
pgBouncer so if we want it later we can switch to this endpoint easily.
Mesut Oezdil
Radoslav Dimitrov
Preeti Dewani
adam jones
Tadas Antanavicius
nate nowack