## 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>
92 lines
5.1 KiB
Go
92 lines
5.1 KiB
Go
package database
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"time"
|
|
|
|
"github.com/jackc/pgx/v5"
|
|
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
|
"github.com/modelcontextprotocol/registry/pkg/model"
|
|
)
|
|
|
|
// Common database errors
|
|
var (
|
|
ErrNotFound = errors.New("record not found")
|
|
ErrAlreadyExists = errors.New("record already exists")
|
|
ErrInvalidInput = errors.New("invalid input")
|
|
ErrDatabase = errors.New("database error")
|
|
ErrInvalidVersion = errors.New("invalid version: cannot publish duplicate version")
|
|
ErrMaxServersReached = errors.New("maximum number of versions for this server reached (10000): please reach out at https://github.com/modelcontextprotocol/registry to explain your use case")
|
|
)
|
|
|
|
// ServerFilter defines filtering options for server queries
|
|
type ServerFilter struct {
|
|
Name *string // for finding versions of same server
|
|
RemoteURL *string // for duplicate URL detection
|
|
UpdatedSince *time.Time // for incremental sync filtering
|
|
SubstringName *string // for substring search on name
|
|
Version *string // for exact version matching
|
|
IsLatest *bool // for filtering latest versions only
|
|
IncludeDeleted *bool // for including deleted packages in results (default: exclude)
|
|
}
|
|
|
|
// Database defines the interface for database operations
|
|
type Database interface {
|
|
// CreateServer inserts a new server version with official metadata
|
|
CreateServer(ctx context.Context, tx pgx.Tx, serverJSON *apiv0.ServerJSON, officialMeta *apiv0.RegistryExtensions) (*apiv0.ServerResponse, error)
|
|
// UpdateServer updates an existing server record
|
|
UpdateServer(ctx context.Context, tx pgx.Tx, serverName, version string, serverJSON *apiv0.ServerJSON) (*apiv0.ServerResponse, error)
|
|
// SetServerStatus updates the status of a specific server version
|
|
SetServerStatus(ctx context.Context, tx pgx.Tx, serverName, version string, status model.Status, statusMessage *string) (*apiv0.ServerResponse, error)
|
|
// SetAllVersionsStatus updates the status of all versions of a server in a single query
|
|
SetAllVersionsStatus(ctx context.Context, tx pgx.Tx, serverName string, status model.Status, statusMessage *string) ([]*apiv0.ServerResponse, error)
|
|
// ListServers retrieve server entries with optional filtering
|
|
ListServers(ctx context.Context, tx pgx.Tx, filter *ServerFilter, cursor string, limit int) ([]*apiv0.ServerResponse, string, error)
|
|
// GetServerByName retrieve a single server by its name
|
|
GetServerByName(ctx context.Context, tx pgx.Tx, serverName string, includeDeleted bool) (*apiv0.ServerResponse, error)
|
|
// GetServerByNameAndVersion retrieve specific version of a server by server name and version
|
|
GetServerByNameAndVersion(ctx context.Context, tx pgx.Tx, serverName string, version string, includeDeleted bool) (*apiv0.ServerResponse, error)
|
|
// GetAllVersionsByServerName retrieve all versions of a server by server name
|
|
GetAllVersionsByServerName(ctx context.Context, tx pgx.Tx, serverName string, includeDeleted bool) ([]*apiv0.ServerResponse, error)
|
|
// GetCurrentLatestVersion retrieve the current latest version of a server by server name
|
|
GetCurrentLatestVersion(ctx context.Context, tx pgx.Tx, serverName string) (*apiv0.ServerResponse, error)
|
|
// CountServerVersions count the number of versions for a server
|
|
CountServerVersions(ctx context.Context, tx pgx.Tx, serverName string) (int, error)
|
|
// CheckVersionExists check if a specific version exists for a server
|
|
CheckVersionExists(ctx context.Context, tx pgx.Tx, serverName, version string) (bool, error)
|
|
// UnmarkAsLatest marks the current latest version of a server as no longer latest
|
|
UnmarkAsLatest(ctx context.Context, tx pgx.Tx, serverName string) error
|
|
// SetLatestVersion sets is_latest=true on the given version and false on all other
|
|
// versions of the same server. Passing an empty version clears is_latest for all rows.
|
|
// Callers must hold the per-server publish lock to avoid races.
|
|
SetLatestVersion(ctx context.Context, tx pgx.Tx, serverName, version string) error
|
|
// AcquirePublishLock acquires an exclusive advisory lock for publishing a server
|
|
// This prevents race conditions when multiple versions are published concurrently
|
|
AcquirePublishLock(ctx context.Context, tx pgx.Tx, serverName string) error
|
|
// InTransaction executes a function within a database transaction
|
|
InTransaction(ctx context.Context, fn func(ctx context.Context, tx pgx.Tx) error) error
|
|
// Close closes the database connection
|
|
Close() error
|
|
}
|
|
|
|
// InTransactionT is a generic helper that wraps InTransaction for functions returning a value
|
|
// This exists because Go does not support generic methods on interfaces - only the Database interface
|
|
// method InTransaction (without generics) can exist, so we provide this generic wrapper function.
|
|
// This is a common pattern in Go for working around this language limitation.
|
|
func InTransactionT[T any](ctx context.Context, db Database, fn func(ctx context.Context, tx pgx.Tx) (T, error)) (T, error) {
|
|
var result T
|
|
var fnErr error
|
|
|
|
err := db.InTransaction(ctx, func(txCtx context.Context, tx pgx.Tx) error {
|
|
result, fnErr = fn(txCtx, tx)
|
|
return fnErr
|
|
})
|
|
|
|
if err != nil {
|
|
var zero T
|
|
return zero, err
|
|
}
|
|
|
|
return result, nil
|
|
}
|