registry/internal/database/database.go
Radoslav Dimitrov 0cb1f6bf52
fix: recalculate isLatest after status changes (#1081) (#1201)
## 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>
2026-04-25 11:59:28 -04:00

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
}