diff --git a/cmd/pad/cmd_item.go b/cmd/pad/cmd_item.go index 529d15a6..877d6f66 100644 --- a/cmd/pad/cmd_item.go +++ b/cmd/pad/cmd_item.go @@ -199,7 +199,14 @@ Run with --help-collections to see available collections and their status values client, _ := getClient() ws := getWorkspace() - collSlug := normalizeCollectionSlug(args[0]) + // Send the RAW slug the user typed; the server's exact-match-first + // resolver (BUG-2578) handles shorthand without letting an alias + // shadow a real collection of the same singular name (BUG-2630). + // The alias fallback below preserves shorthand against servers that + // predate the resolver. The schema fetch and the create both go + // through the SAME fallback so typed --field values parse against — + // and the item lands in — one collection, never a mismatched pair. + rawSlug := args[0] title := args[1] // Build fields JSON from flags @@ -228,7 +235,14 @@ Run with --help-collections to see available collections and their status values // degrades gracefully: all values stay as strings, matching // pre-fix behavior. var collSchema models.CollectionSchema - if coll, err := client.GetCollection(ws, collSlug); err == nil { + // The schema lookup hits exact-match-only GetCollection, which does + // NOT resolve slugs server-side, so its not-found is never + // authoritative about an alias — always retry (nil gate) so a typed + // --field parses against the aliased collection's schema (Codex r2 + // P2). Best-effort: a miss degrades to string fields either way. + if coll, err := cli.WithCollectionAliasFallback(rawSlug, nil, func(slug string) (*models.Collection, error) { + return client.GetCollection(ws, slug) + }); err == nil { _ = json.Unmarshal([]byte(coll.Schema), &collSchema) } for _, kv := range fieldFlags { @@ -298,7 +312,9 @@ Run with --help-collections to see available collections and their status values input.AgentRoleID = &role.ID } - item, err := client.CreateItem(ws, collSlug, input) + item, err := cli.WithCollectionAliasFallback(rawSlug, client.CollectionNotFoundIsAuthoritative, func(slug string) (*models.Item, error) { + return client.CreateItem(ws, slug, input) + }) if err != nil { // TASK-788: emit structured marker so MCP stdio classifier // can surface ErrPlanLimitExceeded instead of ErrServerError. @@ -486,7 +502,12 @@ Examples: var err error if len(args) > 0 { - items, err = client.ListCollectionItems(ws, normalizeCollectionSlug(args[0]), params) + // Raw slug first so an exact collection name is never shadowed + // by its singular alias (BUG-2630); the fallback preserves + // shorthand against pre-resolver servers (BUG-2578). + items, err = cli.WithCollectionAliasFallback(args[0], client.CollectionNotFoundIsAuthoritative, func(slug string) ([]models.Item, error) { + return client.ListCollectionItems(ws, slug, params) + }) } else { items, err = client.ListItems(ws, params) } @@ -1557,9 +1578,8 @@ Examples: ws := getWorkspace() input := map[string]any{ - "target_collection": normalizeCollectionSlug(args[1]), - "actor": "user", - "source": "cli", + "actor": "user", + "source": "cli", } // Parse field overrides @@ -1575,7 +1595,15 @@ Examples: input["field_overrides"] = overrides } - moved, err := client.MoveItemWithForce(ws, args[0], input, force) + // Raw target slug first so an exact collection name is never + // shadowed by its alias (BUG-2630); the fallback preserves shorthand + // against pre-resolver servers (BUG-2578). Retrying on + // collection-not-found is safe: that error means the move never + // mutated anything, so no double-write can result. + moved, err := cli.WithCollectionAliasFallback(args[1], client.CollectionNotFoundIsAuthoritative, func(slug string) (*models.Item, error) { + input["target_collection"] = slug + return client.MoveItemWithForce(ws, args[0], input, force) + }) if err != nil { // IDEA-1494 R3 P1: render the open-children rejection // the same way the regular update path does, so diff --git a/internal/cli/capabilities_probe_test.go b/internal/cli/capabilities_probe_test.go new file mode 100644 index 00000000..d0324b1e --- /dev/null +++ b/internal/cli/capabilities_probe_test.go @@ -0,0 +1,88 @@ +package cli + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +// capsHandler serves /api/v1/server/capabilities with a swappable status/body +// and counts hits, so a test can assert BOTH the verdict and whether a probe +// was (re)issued. +type capsHandler struct { + status atomic.Int32 + body atomic.Value // string + hits atomic.Int32 +} + +func (h *capsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/server/capabilities" { + w.WriteHeader(http.StatusNotFound) + return + } + h.hits.Add(1) + w.WriteHeader(int(h.status.Load())) + if b, _ := h.body.Load().(string); b != "" { + _, _ = w.Write([]byte(b)) + } +} + +func TestCollectionNotFoundIsAuthoritative_Definitive_CachesVerdict(t *testing.T) { + for _, tc := range []struct { + name string + status int + body string + want bool + }{ + {"resolves", http.StatusOK, `{"collection_resolution":true}`, true}, + {"advertises false", http.StatusOK, `{"collection_resolution":false}`, false}, + {"legacy 404", http.StatusNotFound, "not found", false}, + } { + t.Run(tc.name, func(t *testing.T) { + h := &capsHandler{} + h.status.Store(int32(tc.status)) + h.body.Store(tc.body) + ts := httptest.NewServer(h) + defer ts.Close() + c := NewClientFromURL(ts.URL) + + if got := c.CollectionNotFoundIsAuthoritative(); got != tc.want { + t.Fatalf("first call = %v, want %v", got, tc.want) + } + // A definitive verdict is cached: the second call must NOT re-probe. + if got := c.CollectionNotFoundIsAuthoritative(); got != tc.want { + t.Fatalf("second call = %v, want %v", got, tc.want) + } + if h.hits.Load() != 1 { + t.Fatalf("expected exactly one probe (definitive verdict cached), got %d", h.hits.Load()) + } + }) + } +} + +func TestCollectionNotFoundIsAuthoritative_Transient_FailsClosedAndReprobes(t *testing.T) { + // Codex r2 P1: a transient probe failure (here a 5xx) must NOT be cached as + // "legacy" — caching it would permanently re-enable the alias retry and let + // a write bypass the archived/hidden protection on a resolving server. + h := &capsHandler{} + h.status.Store(http.StatusInternalServerError) + h.body.Store("boom") + ts := httptest.NewServer(h) + defer ts.Close() + c := NewClientFromURL(ts.URL) + + // Fail CLOSED: an indeterminate probe trusts the not-found (no retry). + if got := c.CollectionNotFoundIsAuthoritative(); !got { + t.Fatalf("transient probe must fail closed (true), got false") + } + // And it must NOT be cached: once the blip clears, the real verdict wins. + h.status.Store(http.StatusOK) + h.body.Store(`{"collection_resolution":false}`) + if got := c.CollectionNotFoundIsAuthoritative(); got { + t.Fatalf("after the blip cleared the verdict must be re-probed (false), got true") + } + if h.hits.Load() != 2 { + t.Fatalf("expected a re-probe after the transient failure, got %d probes", h.hits.Load()) + } +} diff --git a/internal/cli/client.go b/internal/cli/client.go index 6e9d8e71..5f861845 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -14,6 +14,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "time" "unicode" @@ -32,6 +33,15 @@ type Client struct { streamClient *http.Client authToken string // session or API token, sent as Authorization: Bearer agentName string // optional agent name, sent as X-Pad-Agent header + + // capMu guards the lazy, cached probe of GET /server/capabilities behind + // CollectionNotFoundIsAuthoritative. capProbed is set only once a DEFINITIVE + // answer is cached (a 200 with the flag, or a clean 404); a transient probe + // failure leaves capProbed false so a later call re-probes rather than + // poisoning the cache. capResolves is the cached definitive verdict. + capMu sync.Mutex + capProbed bool + capResolves bool } func NewClient(host string, port int) *Client { @@ -238,6 +248,83 @@ func (c *Client) CreateItem(wsSlug, collSlug string, input models.ItemCreate) (* return &result, c.post("/workspaces/"+wsSlug+"/collections/"+collSlug+"/items", input, &result) } +// CollectionNotFoundIsAuthoritative reports whether a collection-not-found from +// this server should be TRUSTED — i.e. the alias retry in +// WithCollectionAliasFallback should be SKIPPED. It answers the question the +// helper actually needs, which is not quite "does the server resolve": it also +// has to say the safe thing when the answer is unknown. +// +// - Server advertises collection_resolution=true → true. It already tried the +// singular/alias fallback and enforced exact-match + the archived/hidden +// refusal (resolveItemCollectionSlug, BUG-2578/2630), so its not-found is +// final: do not retry. +// - Server DEFINITIVELY lacks the resolver — a clean 404 (no such endpoint) or +// an explicit collection_resolution=false → false. Retry the legacy alias; +// that old build never had the archived-claims protection a retry could +// defeat, so the retry is non-regressive there. +// - Probe is INDETERMINATE (a transport error, timeout, or 5xx) → true, and +// the result is NOT cached. Failing CLOSED here is a DELIBERATE asymmetry: +// the cost of failing closed on a blip is at worst one alias-shorthand +// failure the user can simply re-run, whereas failing OPEN (retrying) risks +// a wrong-write that bypasses the archived/hidden protection and cannot be +// un-done. A recoverable UX miss is always the safer side of that trade. +// Not caching matters for the same reason: a single transient blip must not +// permanently re-enable the retry for the rest of the session. The only +// case this could "cost" is an old server whose capabilities probe +// transiently errors instead of returning a clean 404 — but a missing route +// returns 404, not a transient error, so a genuine old build still retries. +// +// The definitive verdict is cached (static for the server's lifetime); an +// indeterminate probe is re-tried on the next call. +func (c *Client) CollectionNotFoundIsAuthoritative() bool { + c.capMu.Lock() + defer c.capMu.Unlock() + if c.capProbed { + return c.capResolves + } + resolves, definitive := c.probeCollectionResolution() + if !definitive { + // Fail closed without caching: trust the not-found for THIS call, but + // re-probe next time in case the blip clears. + return true + } + c.capResolves = resolves + c.capProbed = true + return resolves +} + +// probeCollectionResolution issues the one GET /server/capabilities probe and +// classifies the outcome. definitive is true only when the server gave a clear +// answer — HTTP 200 (resolves = the advertised flag) or HTTP 404 (resolves = +// false: a build with no capabilities endpoint has no resolver). A transport +// error, a 200 whose body will not decode, or any other status (e.g. a 5xx) is +// NOT definitive. +func (c *Client) probeCollectionResolution() (resolves, definitive bool) { + req, err := c.newRequest("GET", "/server/capabilities", nil) + if err != nil { + return false, false + } + resp, err := c.httpClient.Do(req) + if err != nil { + return false, false + } + defer resp.Body.Close() + switch resp.StatusCode { + case http.StatusOK: + var caps struct { + CollectionResolution bool `json:"collection_resolution"` + } + if err := json.NewDecoder(resp.Body).Decode(&caps); err != nil { + return false, false + } + return caps.CollectionResolution, true + case http.StatusNotFound: + return false, true + default: + return false, false + } +} + func (c *Client) GetItem(wsSlug, itemSlug string) (*models.Item, error) { var result models.Item if err := c.get("/workspaces/"+wsSlug+"/items/"+itemSlug, &result); err != nil { diff --git a/internal/cli/collection_alias.go b/internal/cli/collection_alias.go new file mode 100644 index 00000000..90f2c7eb --- /dev/null +++ b/internal/cli/collection_alias.go @@ -0,0 +1,152 @@ +package cli + +import ( + "errors" + "fmt" + + "github.com/PerpetualSoftware/pad/internal/collections" +) + +// withCollectionAliasFallback runs op against the RAW collection slug the user +// typed, and — only if that fails specifically because the collection was not +// found — retries once with the legacy singular→plural alias +// (collections.NormalizeSlug). The result of whichever attempt succeeds is +// returned; if both fail, the RAW attempt's error is returned. +// +// Why send raw first (BUG-2630). The old client behaviour normalized the slug +// BEFORE the request, so a workspace whose collection slug IS one of the seven +// hardcoded singulars ("plan", "task", …) had the user's exact name rewritten +// away and their write landed in a DIFFERENT collection, silently, with a +// success message naming the wrong one. Sending raw lets the server's +// exact-match-first resolution (BUG-2578) win, so an exact collection name can +// never be shadowed by its own alias. +// +// Why keep the alias at all. A server that predates the server-side resolver +// (BUG-2578) matches collections by exact slug only, so a raw "task" would 404 +// there for a workspace whose collection is "tasks". The retry preserves +// today's shorthand UX against those older servers, at the cost of one extra +// round trip on a path that was already going to fail. +// +// Why the retry is keyed on collection-not-found specifically (see +// isCollectionNotFound). If it fired on ANY failure, a request aimed at a +// collection that really exists ("plan") but failed for another reason (a +// validation error, an auth hiccup) would be retried against the alias +// ("plans") and could silently succeed there — recreating BUG-2630 in a new +// costume. Keying on the not-found error means a collection that exists is +// never retried away from. +// +// REMOVAL CONDITION: once the minimum supported server is guaranteed to carry +// the resolver (BUG-2578), the raw slug always resolves on the first attempt +// and this fallback is dead weight — drop it and call op(rawSlug) directly. +// That is the Option 1 cleanup tracked on BUG-2630's trail, gated on a +// min-server-version floor that includes the resolver. +// +// notFoundIsAuthoritative reports whether a collection-not-found should be +// TRUSTED — when it returns true the alias retry is skipped, because the server +// resolves slugs itself (exact-match-first + alias fallback + archived/hidden +// refusal) and has therefore already ruled the slug absent. It returns false +// only for a server definitively without the resolver, where the legacy retry +// is the right non-regressive fallback. Pass nil to FORCE the always-retry +// behaviour — the right choice for an endpoint that does NOT resolve slugs +// server-side (the CLI schema lookup hits exact-match-only GetCollection, so its +// not-found is never authoritative about an alias). See +// Client.CollectionNotFoundIsAuthoritative. +// +// Exported so the cmd/pad item commands can funnel their create / list / move +// calls through this ONE implementation instead of each re-deriving the +// send-raw-then-retry dance (BUG-2630 lead ruling: one shared helper, not +// copied at the call sites). +func WithCollectionAliasFallback[T any](rawSlug string, notFoundIsAuthoritative func() bool, op func(slug string) (T, error)) (T, error) { + result, err := op(rawSlug) + if err == nil { + return result, nil + } + // The failure was not a missing collection (auth, validation, a 5xx): return + // it untouched. Retrying such an error against the alias could silently + // succeed on a DIFFERENT collection that does exist — BUG-2630 in a new + // costume — so a collection that exists is never retried away from. + if !isCollectionNotFound(err) { + return result, err + } + normalized := collections.NormalizeSlug(rawSlug) + if normalized == rawSlug { + // No alias to try (the input is not one of the aliased singulars): the + // slug genuinely names no collection. Name it in the error the user + // sees, since it is their own word. + return result, wrapCollectionNotFound(err, rawSlug) + } + // When the server's not-found is authoritative — it resolves collections + // itself (BUG-2578/2630), so it already tried the alias and enforced + // exact-match-first + the archived/hidden refusal — retrying the alias would + // only defeat that protection (BUG-2630 #1). Only fall back to the + // client-side retry for an OLDER server that never had the protection. + if notFoundIsAuthoritative != nil && notFoundIsAuthoritative() { + return result, wrapCollectionNotFound(err, rawSlug) + } + result2, err2 := op(normalized) + if err2 != nil { + if isCollectionNotFound(err2) { + // Neither the raw slug nor its alias names a collection. Surface an + // error naming the RAW slug the user typed, not the alias the + // client tried on their behalf — their own words are the ones they + // can act on. + return result, wrapCollectionNotFound(err, rawSlug) + } + // The alias DOES name a real collection, but the operation failed there + // for a substantive reason (plan limit, open children, validation, + // forbidden). That error is about a real collection and is far more + // useful than a misleading "collection not found" — surface it verbatim. + return result2, err2 + } + return result2, nil +} + +// wrapCollectionNotFound rewrites the server's context-free "Collection not +// found" into one that names the slug the user actually typed, so a failed +// `pad item create widget …` reads `collection "widget" not found` instead of a +// bare "Collection not found". It returns a fresh *APIError with the same Code +// (so downstream type/Code inspection — plan-limit, open-children — keeps +// working) and an enriched Message. Non-collection errors pass through +// untouched. Mirrors wrapItemNotFound's shape. +func wrapCollectionNotFound(err error, rawSlug string) error { + var apiErr *APIError + if errors.As(err, &apiErr) && isCollectionNotFound(err) { + return &APIError{ + Code: apiErr.Code, + Message: fmt.Sprintf("collection %q not found", rawSlug), + Details: apiErr.Details, + } + } + return err +} + +// isCollectionNotFound reports whether err is the server's specific +// "collection not found" response for an item create / list / move request. It +// is deliberately narrow: any other failure (auth, validation, a 5xx, an +// item-not-found) must NOT trigger the alias retry, or a request aimed at a +// collection that really exists could be silently rerouted to a different one +// (BUG-2630). +// +// The wire contract it matches is stable across the BUG-2578 server change — +// both codes predate it, so the predicate works against old and new servers: +// - create / list: HTTP 404, code "not_found", message "Collection not found" +// - move: HTTP 400, code "invalid_collection" +// +// The message check on the "not_found" branch distinguishes a missing +// COLLECTION from a missing ITEM, which shares the "not_found" code on other +// endpoints — though the create/list endpoints this helper wraps only ever +// emit "not_found" for the collection, the message guard keeps the predicate +// honest if it is ever reused on a broader path. +func isCollectionNotFound(err error) bool { + var apiErr *APIError + if !errors.As(err, &apiErr) { + return false + } + switch apiErr.Code { + case "invalid_collection": + return true + case "not_found": + return apiErr.Message == "Collection not found" + } + return false +} diff --git a/internal/cli/collection_alias_test.go b/internal/cli/collection_alias_test.go new file mode 100644 index 00000000..e7ae7ff7 --- /dev/null +++ b/internal/cli/collection_alias_test.go @@ -0,0 +1,224 @@ +package cli + +import ( + "errors" + "fmt" + "testing" +) + +// notFoundCollErr is the create/list wire shape for a missing collection. +func notFoundCollErr() error { + return &APIError{Code: "not_found", Message: "Collection not found"} +} + +// invalidCollErr is the move wire shape for a missing target collection. +func invalidCollErr() error { + return &APIError{Code: "invalid_collection", Message: "Target collection not found"} +} + +func TestWithCollectionAliasFallback_RawSucceeds_NoRetry(t *testing.T) { + // The shadow case (BUG-2630): the user typed a slug that IS a real + // collection ("plan") whose alias ("plans") also exists. The raw attempt + // succeeds, so the fallback must NOT retry — retrying would route the user + // to the aliased collection they did not name. + var calls []string + got, err := WithCollectionAliasFallback("plan", nil, func(slug string) (string, error) { + calls = append(calls, slug) + return "landed:" + slug, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "landed:plan" { + t.Fatalf("got %q, want landed:plan", got) + } + if len(calls) != 1 || calls[0] != "plan" { + t.Fatalf("expected exactly one call with the raw slug, got %v", calls) + } +} + +func TestWithCollectionAliasFallback_CollectionNotFound_RetriesAlias(t *testing.T) { + // Old-server compat: raw "task" 404s because the collection is "tasks" and + // the server has no resolver. The fallback retries with the alias. + var calls []string + got, err := WithCollectionAliasFallback("task", nil, func(slug string) (string, error) { + calls = append(calls, slug) + if slug == "task" { + return "", notFoundCollErr() + } + return "landed:" + slug, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "landed:tasks" { + t.Fatalf("got %q, want landed:tasks", got) + } + if len(calls) != 2 || calls[0] != "task" || calls[1] != "tasks" { + t.Fatalf("expected raw then alias, got %v", calls) + } +} + +func TestWithCollectionAliasFallback_InvalidCollectionCode_RetriesAlias(t *testing.T) { + // The move path reports a missing target collection as invalid_collection. + var calls []string + _, err := WithCollectionAliasFallback("task", nil, func(slug string) (string, error) { + calls = append(calls, slug) + if slug == "task" { + return "", invalidCollErr() + } + return "ok", nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(calls) != 2 { + t.Fatalf("expected a retry on invalid_collection, got %v", calls) + } +} + +func TestWithCollectionAliasFallback_NonAliasedInput_NoRetry(t *testing.T) { + // A slug that NormalizeSlug leaves unchanged ("widgets") has no alias to + // try, so a not-found must return immediately without a second call. + var calls int + _, err := WithCollectionAliasFallback("widgets", nil, func(slug string) (string, error) { + calls++ + return "", notFoundCollErr() + }) + if calls != 1 { + t.Fatalf("expected exactly one call (no alias to retry), got %d", calls) + } + // The error the user sees names the slug they typed (constraint 2), with + // the collection-not-found Code preserved for downstream inspection. + var apiErr *APIError + if !errors.As(err, &apiErr) || apiErr.Code != "not_found" { + t.Fatalf("expected a *APIError with Code=not_found, got %v", err) + } + if apiErr.Message != `collection "widgets" not found` { + t.Fatalf("error should name the raw slug, got %q", apiErr.Message) + } +} + +func TestWithCollectionAliasFallback_NonCollectionError_NoRetry(t *testing.T) { + // Constraint 1 (lead): a request aimed at a collection that really exists + // ("plan") but that fails for a DIFFERENT reason (here a validation error) + // must NOT be retried against the alias ("plans"). Retrying could silently + // succeed against the wrong collection — BUG-2630 in a new costume. + var calls []string + sentinel := &APIError{Code: "bad_request", Message: "Title is required"} + _, err := WithCollectionAliasFallback("plan", nil, func(slug string) (string, error) { + calls = append(calls, slug) + return "", sentinel + }) + if !errors.Is(err, sentinel) { + t.Fatalf("expected the raw sentinel error back, got %v", err) + } + if len(calls) != 1 || calls[0] != "plan" { + t.Fatalf("a non-collection error must not trigger the alias retry, got %v", calls) + } +} + +func TestWithCollectionAliasFallback_BothFail_SurfacesRawError(t *testing.T) { + // Constraint 2 (lead): when both raw and alias fail, the error the user + // sees must name the slug they typed ("task"), not the alias tried on their + // behalf ("tasks"). + got := "sentinel-unset" + _, err := WithCollectionAliasFallback("task", nil, func(slug string) (string, error) { + got = slug + if slug == "task" { + return "", notFoundCollErr() + } + return "", &APIError{Code: "not_found", Message: "Collection not found"} + }) + if got != "tasks" { + t.Fatalf("expected the alias to have been tried, last slug was %q", got) + } + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected a *APIError, got %v", err) + } + if apiErr.Message != `collection "task" not found` { + t.Fatalf("double-fail error must name the RAW slug, got %q", apiErr.Message) + } +} + +func TestWithCollectionAliasFallback_AliasSubstantiveError_Surfaced(t *testing.T) { + // Codex P2: when the raw slug misses but the alias names a REAL collection + // that fails for a substantive reason (here a plan limit), the user must + // see THAT error, not a misleading "collection not found" — the alias + // collection exists, so "not found" would be a lie. + planLimit := &APIError{Code: "plan_limit_exceeded", Message: "item limit reached"} + _, err := WithCollectionAliasFallback("task", nil, func(slug string) (string, error) { + if slug == "task" { + return "", notFoundCollErr() + } + return "", planLimit // the "tasks" collection exists but is over limit + }) + if !errors.Is(err, planLimit) { + t.Fatalf("expected the substantive alias error surfaced, got %v", err) + } +} + +func TestWithCollectionAliasFallback_ServerResolves_SuppressesRetry(t *testing.T) { + // BUG-2630 #1: a server that resolves collections itself has already tried + // the alias AND enforced exact-match + the archived/hidden refusal, so its + // collection-not-found is AUTHORITATIVE. The client must NOT retry the alias + // (which would defeat that protection, e.g. redirect an archived `plan` into + // a live `plans`). + var calls []string + _, err := WithCollectionAliasFallback("plan", func() bool { return true }, func(slug string) (string, error) { + calls = append(calls, slug) + return "", notFoundCollErr() + }) + if len(calls) != 1 || calls[0] != "plan" { + t.Fatalf("a resolving server's not-found must not be retried, got %v", calls) + } + var apiErr *APIError + if !errors.As(err, &apiErr) || apiErr.Message != `collection "plan" not found` { + t.Fatalf("expected raw-named not-found, got %v", err) + } +} + +func TestWithCollectionAliasFallback_ServerLacksResolution_Retries(t *testing.T) { + // The other branch: an OLD server that does not advertise resolution never + // had the archived-claims protection, so the legacy alias retry runs and is + // non-regressive there. + var calls []string + got, err := WithCollectionAliasFallback("plan", func() bool { return false }, func(slug string) (string, error) { + calls = append(calls, slug) + if slug == "plan" { + return "", notFoundCollErr() + } + return "landed:" + slug, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "landed:plans" || len(calls) != 2 { + t.Fatalf("expected retry into plans, got %q calls=%v", got, calls) + } +} + +func TestIsCollectionNotFound(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"create/list not_found", notFoundCollErr(), true}, + {"move invalid_collection", invalidCollErr(), true}, + {"item not_found (different message)", &APIError{Code: "not_found", Message: "item TASK-9 not found"}, false}, + {"validation error", &APIError{Code: "bad_request", Message: "Title is required"}, false}, + {"plan limit", &APIError{Code: "plan_limit_exceeded", Message: "limit"}, false}, + {"wrapped not_found", fmt.Errorf("ctx: %w", notFoundCollErr()), true}, + {"non-APIError", errors.New("network down"), false}, + {"nil", nil, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isCollectionNotFound(tc.err); got != tc.want { + t.Fatalf("isCollectionNotFound(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} diff --git a/internal/mcp/dispatch_http.go b/internal/mcp/dispatch_http.go index 8ad1e43b..396efd57 100644 --- a/internal/mcp/dispatch_http.go +++ b/internal/mcp/dispatch_http.go @@ -15,7 +15,6 @@ import ( "github.com/go-chi/chi/v5" "github.com/mark3labs/mcp-go/mcp" - "github.com/PerpetualSoftware/pad/internal/collections" "github.com/PerpetualSoftware/pad/internal/models" "github.com/PerpetualSoftware/pad/internal/server" ) @@ -860,15 +859,13 @@ func mapItemCreate(input map[string]any) (method, path string, body []byte, err return "", "", nil, fmt.Errorf("encode body: %w", err) } - // Normalize singular/shorthand forms ("task" → "tasks", "doc" → - // "docs", etc.) the same way the CLI does. Without this, an MCP - // caller that mirrors a documented CLI command shape like - // `item.create(collection: "task", ...)` would 404 against the - // REST handler even though the same call works through - // ExecDispatcher (which goes through normalizeCollectionSlug in - // cmd/pad/main.go). - collection = collections.NormalizeSlug(collection) - + // Send the collection slug verbatim. This dispatcher runs in-process + // against the same binary, whose create handler resolves shorthand + // server-side with exact-match-first (BUG-2578), so the raw slug keeps + // `item.create(collection: "task", ...)` working AND stops the old + // client-side alias from shadowing a real collection whose slug IS a + // singular like "task" (BUG-2630). The ExecDispatcher path (local stdio) + // gets the same behaviour through the CLI's alias-fallback helper. urlPath := fmt.Sprintf("/api/v1/workspaces/%s/collections/%s/items", url.PathEscape(workspace), url.PathEscape(collection)) return http.MethodPost, urlPath, body, nil diff --git a/internal/mcp/dispatch_http_collection_alias_test.go b/internal/mcp/dispatch_http_collection_alias_test.go index f9b8f2cc..ec748247 100644 --- a/internal/mcp/dispatch_http_collection_alias_test.go +++ b/internal/mcp/dispatch_http_collection_alias_test.go @@ -101,6 +101,79 @@ func TestHTTPItemCreate_AcceptsSingularOfANonDefaultCollection(t *testing.T) { } } +// newShadowFixture builds a workspace holding BOTH a user collection whose slug +// IS a hardcoded singular ("plan") and its plural ("plans"), so the BUG-2630 +// shadow — the client alias rewriting "plan" to "plans" before the server sees +// it — has a real collision to expose. +func newShadowFixture(t *testing.T) *HTTPHandlerDispatcher { + t.Helper() + s := storetest.NewSQLite(t) + srv := server.New(s) + t.Cleanup(srv.Stop) + + owner, err := s.CreateUser(models.UserCreate{ + Email: "shadow-owner@example.com", Name: "Owner", Password: "correct-horse-battery-staple", + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Shadow WS", Slug: "shadow-ws", OwnerID: owner.ID}) + if err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil { + t.Fatalf("AddWorkspaceMember: %v", err) + } + schema := `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"}]}` + for _, c := range []models.CollectionCreate{ + {Name: "Plans", Slug: "plans", Prefix: "PLANS", Schema: schema}, + {Name: "Plan", Slug: "plan", Prefix: "PLAN", Schema: schema}, + } { + if _, err := s.CreateCollection(ws.ID, c); err != nil { + t.Fatalf("CreateCollection %s: %v", c.Slug, err) + } + } + return &HTTPHandlerDispatcher{Handler: srv, UserResolver: func(context.Context) *models.User { return owner }} +} + +// BUG-2630 over the MCP dispatcher: an exact collection name that happens to be +// a hardcoded singular must NOT be rewritten into its plural. The item has to +// land in — and be listable from — the collection the caller actually named. +func TestHTTPItemCreate_ExactSingularNotShadowedByAlias(t *testing.T) { + d := newShadowFixture(t) + + createCtx := WithDispatchInput(context.Background(), map[string]any{ + "workspace": "shadow-ws", "collection": "plan", "title": "belongs in plan", + }) + res, err := d.Dispatch(createCtx, []string{"item", "create"}, nil) + if err != nil { + t.Fatalf("Dispatch(item create): %v", err) + } + if res.IsError { + t.Fatalf("create into `plan` over MCP failed: %s", textOf(res)) + } + + // Present in `plan` (the named collection)... + planList := WithDispatchInput(context.Background(), map[string]any{"workspace": "shadow-ws", "collection": "plan"}) + planRes, err := d.Dispatch(planList, []string{"item", "list"}, nil) + if err != nil { + t.Fatalf("Dispatch(list plan): %v", err) + } + if !strings.Contains(textOf(planRes), "belongs in plan") { + t.Errorf("item did not land in `plan`: %s", textOf(planRes)) + } + + // ...and ABSENT from `plans` (the alias that used to shadow it). + plansList := WithDispatchInput(context.Background(), map[string]any{"workspace": "shadow-ws", "collection": "plans"}) + plansRes, err := d.Dispatch(plansList, []string{"item", "list"}, nil) + if err != nil { + t.Fatalf("Dispatch(list plans): %v", err) + } + if strings.Contains(textOf(plansRes), "belongs in plan") { + t.Errorf("item was shadowed into `plans` — BUG-2630: %s", textOf(plansRes)) + } +} + // A collection that does not exist under any spelling must still fail, so the // fallback cannot be mistaken for "any name works". func TestHTTPItemCreate_UnknownCollectionStillFails(t *testing.T) { diff --git a/internal/mcp/dispatch_http_routes.go b/internal/mcp/dispatch_http_routes.go index 8775c875..448720a8 100644 --- a/internal/mcp/dispatch_http_routes.go +++ b/internal/mcp/dispatch_http_routes.go @@ -28,9 +28,11 @@ import ( // as standalone RouteMapper functions instead. // // All input keys are MCP property names (snake_case per TASK-964). -// `collection` and `target_collection` placeholders are normalized -// via collections.NormalizeSlug so callers can pass shorthand -// ("task" → "tasks") without 404s. +// Collection slugs are sent verbatim: this dispatcher runs in-process +// against the same binary, whose item handlers resolve shorthand +// server-side (exact-match-first, BUG-2578), so the raw slug both keeps +// shorthand working and avoids the client-side alias shadowing a real +// collection (BUG-2630). type routeSpec struct { // method is the HTTP method (GET / POST / PATCH / DELETE). method string @@ -85,9 +87,15 @@ func (s routeSpec) toRouteMapper() RouteMapper { // reply names the missing input rather than the agent receiving a // confusing 404 from the handler tree). // -// The placeholders "collection" / "target_collection" are normalized -// via collections.NormalizeSlug so shorthand forms like "task" work -// the same way they do through the CLI. +// Placeholders are substituted verbatim (path-escaped). It does NOT +// normalize a collection slug: this dispatcher runs in-process against +// the same binary, whose item handlers resolve shorthand server-side +// with exact-match-first (BUG-2578), so sending the raw slug both keeps +// shorthand working AND stops an alias from shadowing a real collection +// of the same singular name (BUG-2630). No current routeSpec even uses a +// {collection}/{target_collection} path placeholder — the standalone +// item mappers build those paths directly — so this is also dead-code +// removal in the area BUG-2630 fixed. func expandPath(template string, input map[string]any) (string, error) { var out strings.Builder out.Grow(len(template)) @@ -113,9 +121,6 @@ func expandPath(template string, input map[string]any) (string, error) { if s == "" { return "", fmt.Errorf("input %q must be non-empty for path placeholder", name) } - if name == "collection" || name == "target_collection" { - s = collections.NormalizeSlug(s) - } out.WriteString(url.PathEscape(s)) i += end + 1 } @@ -1197,8 +1202,11 @@ func mapItemList(input map[string]any) (string, string, []byte, error) { // "unknown input keys are ignored" behaviour. pathBase := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/items" if coll, _ := input["collection"].(string); coll != "" { + // Raw slug: the in-process list handler resolves shorthand + // server-side (BUG-2578) so an exact collection name is never + // shadowed by its alias (BUG-2630). pathBase = "/api/v1/workspaces/" + url.PathEscape(workspace) + - "/collections/" + url.PathEscape(collections.NormalizeSlug(coll)) + "/items" + "/collections/" + url.PathEscape(coll) + "/items" } values := url.Values{} @@ -1496,7 +1504,10 @@ func mapItemMove(input map[string]any) (string, string, []byte, error) { } payload := map[string]any{ - "target_collection": collections.NormalizeSlug(target), + // Raw slug: the in-process move handler resolves shorthand + // server-side (BUG-2578) so an exact collection name is never + // shadowed by its alias (BUG-2630). + "target_collection": target, "actor": "user", "source": "cli", } diff --git a/internal/mcp/dispatch_http_routes_test.go b/internal/mcp/dispatch_http_routes_test.go index d7037d25..ebfe4cb2 100644 --- a/internal/mcp/dispatch_http_routes_test.go +++ b/internal/mcp/dispatch_http_routes_test.go @@ -66,18 +66,12 @@ func TestExpandPath_BasicSubstitution(t *testing.T) { } } -func TestExpandPath_NormalizesCollection(t *testing.T) { - got, err := expandPath( - "/api/v1/workspaces/{workspace}/collections/{collection}/items", - map[string]any{"workspace": "docapp", "collection": "task"}, - ) - if err != nil { - t.Fatalf("expandPath: %v", err) - } - if !strings.Contains(got, "/collections/tasks/") { - t.Errorf("expected `task` to normalize to `tasks`, got %q", got) - } -} +// TestExpandPath_NormalizesCollection was removed with the collection-slug +// normalization it guarded (BUG-2630). expandPath no longer rewrites a +// `{collection}` placeholder — the in-process handlers resolve shorthand +// server-side (BUG-2578), and rewriting client-side let an alias shadow a real +// collection of the same singular name. No routeSpec uses that placeholder +// anyway; the item mappers build collection paths directly. func TestExpandPath_PathEscapesSpecials(t *testing.T) { got, err := expandPath( @@ -302,15 +296,18 @@ func TestRoute_ItemList_ExplicitStatusOverridesDefault(t *testing.T) { func TestRoute_ItemList_CollectionScopedPath(t *testing.T) { _, p, _, err := mapItemList(map[string]any{ - "workspace": "docapp", "collection": "task", // shorthand + "workspace": "docapp", "collection": "task", // singular }) if err != nil { t.Fatalf("err: %v", err) } - // Path may carry the non_terminal=true query string from the - // no-explicit-status branch — we only assert the path prefix here. - if !strings.HasPrefix(p, "/api/v1/workspaces/docapp/collections/tasks/items") { - t.Errorf("expected collection-scoped + normalized path, got %q", p) + // The slug is sent VERBATIM (BUG-2630): the in-process list handler + // resolves shorthand server-side (BUG-2578) and exact-match-first stops an + // alias from shadowing a real `task` collection. Path may carry the + // non_terminal=true query from the no-explicit-status branch — assert the + // prefix only. + if !strings.HasPrefix(p, "/api/v1/workspaces/docapp/collections/task/items") { + t.Errorf("expected the raw slug in the path, got %q", p) } } @@ -447,7 +444,7 @@ func TestRoute_ItemMove(t *testing.T) { m, p, body, err := routeTable["item move"](map[string]any{ "workspace": "docapp", "ref": "BUG-3", - "target_collection": "task", // shorthand + "target_collection": "task", // singular, sent verbatim "field": []any{"priority=high"}, }) if err != nil { @@ -463,8 +460,10 @@ func TestRoute_ItemMove(t *testing.T) { if err := json.Unmarshal(body, &payload); err != nil { t.Fatalf("decode body: %v\n%s", err, body) } - if payload["target_collection"] != "tasks" { - t.Errorf("target_collection not normalized: %v", payload) + // Verbatim, not aliased (BUG-2630): the in-process move handler resolves + // shorthand server-side (BUG-2578) with exact-match-first. + if payload["target_collection"] != "task" { + t.Errorf("target_collection should be sent verbatim, got: %v", payload) } if payload["source"] != "cli" { t.Errorf("source not stamped: %v", payload) diff --git a/internal/mcp/dispatch_http_test.go b/internal/mcp/dispatch_http_test.go index 9796ccfb..ece7b24a 100644 --- a/internal/mcp/dispatch_http_test.go +++ b/internal/mcp/dispatch_http_test.go @@ -491,23 +491,16 @@ func TestMapItemCreate_ExplicitFieldOverridesNamedFlag(t *testing.T) { } } -func TestMapItemCreate_NormalizesCollectionAliases(t *testing.T) { - // MCP callers may mirror documented CLI shapes ("item create task X") - // using singular/short collection names. The CLI normalizes via - // collections.NormalizeSlug; the dispatcher does the same so the - // HTTP transport doesn't 404 on a call that works through - // ExecDispatcher. Locked into a test so the parity doesn't drift. - cases := map[string]string{ - "task": "tasks", - "idea": "ideas", - "doc": "docs", - "plan": "plans", - "bug": "bugs", - "convention": "conventions", - "tasks": "tasks", // already-canonical: no change - "my-custom": "my-custom", - } - for in, wantSlug := range cases { +func TestMapItemCreate_SendsRawCollectionSlug(t *testing.T) { + // The dispatcher sends the collection slug VERBATIM (BUG-2630). It runs + // in-process against the same binary, whose create handler resolves + // shorthand server-side with exact-match-first (BUG-2578), so the raw slug + // keeps `item.create(collection: "task")` working AND stops the old + // client-side alias from shadowing a real collection whose slug IS a + // singular like "task". A singular that used to be rewritten must now reach + // the server unchanged so the server can decide. + cases := []string{"task", "idea", "doc", "plan", "bug", "convention", "tasks", "my-custom"} + for _, in := range cases { t.Run(in, func(t *testing.T) { _, path, _, err := mapItemCreate(map[string]any{ "workspace": "ws", "collection": in, "title": "x", @@ -515,9 +508,9 @@ func TestMapItemCreate_NormalizesCollectionAliases(t *testing.T) { if err != nil { t.Fatalf("mapItemCreate: %v", err) } - wantPath := "/api/v1/workspaces/ws/collections/" + wantSlug + "/items" + wantPath := "/api/v1/workspaces/ws/collections/" + in + "/items" if path != wantPath { - t.Errorf("path = %q, want %q", path, wantPath) + t.Errorf("path = %q, want %q (slug must be sent verbatim, not aliased)", path, wantPath) } }) } diff --git a/internal/server/collection_resolve.go b/internal/server/collection_resolve.go index 934d6676..c159910c 100644 --- a/internal/server/collection_resolve.go +++ b/internal/server/collection_resolve.go @@ -1,8 +1,10 @@ package server import ( + "slices" "strings" + "github.com/PerpetualSoftware/pad/internal/collections" "github.com/PerpetualSoftware/pad/internal/models" ) @@ -83,12 +85,14 @@ func (s *Server) resolveItemCollectionSlug(workspaceID, input string) (*models.C // collectionSlugCandidates returns the alternative slugs to try when `input` // matched nothing, in priority order and never including `input` itself. // -// Only ASCII-`s` pluralization is attempted. That is not a limitation to fix -// by reaching for an inflector: collection slugs are generated by slugify and -// the plural convention is the house style, so `s` covers the real cases, -// while irregular-plural guessing ("person" -> "people") would start inventing -// mappings a user never wrote. Anything an inflector would catch, an exact -// slug still resolves. +// ASCII-`s` pluralization is tried first, then the fixed legacy alias map +// (t/i/p/d, phase/phases). Neither reaches for an inflector: collection slugs +// are generated by slugify and the plural convention is the house style, so +// `s` covers the structural cases, while irregular-plural guessing +// ("person" -> "people") would start inventing mappings a user never wrote. +// Anything an inflector would catch, an exact slug still resolves. The alias +// map is a CLOSED set carried over from the retired client-side normalizer, not +// an open-ended guesser. func collectionSlugCandidates(input string) []string { trimmed := strings.ToLower(strings.TrimSpace(input)) if trimmed == "" { @@ -111,5 +115,19 @@ func collectionSlugCandidates(input string) []string { if s := strings.TrimSuffix(trimmed, "s"); s != trimmed && s != "" { out = append(out, s) } + // The legacy SEMANTIC aliases the client-side map used to apply before + // resolution moved to the server (BUG-2630): t/i/p/d -> tasks/ideas/plans/ + // docs and phase/phases -> plans. Folded in LAST so the server owns the full + // alias vocabulary and every client can send the raw slug — including the + // remote MCP transport, which no longer normalizes (BUG-2630 #2). It is only + // ever a last resort: the exact-match and the archived-claims refusal in + // resolveItemCollectionSlug run for `input` and every candidate BEFORE this + // one is reached, so a real (or archived-claimed) collection of the input + // name still wins, and this never shadows or redirects around it. Aliases + // that duplicate a structural candidate above (e.g. plan -> plans) are + // dropped so the resolve loop does not query the same slug twice. + if alias := collections.NormalizeSlug(trimmed); alias != trimmed && !slices.Contains(out, alias) { + out = append(out, alias) + } return out } diff --git a/internal/server/collection_resolve_test.go b/internal/server/collection_resolve_test.go index bdfab283..3cfe1872 100644 --- a/internal/server/collection_resolve_test.go +++ b/internal/server/collection_resolve_test.go @@ -156,6 +156,42 @@ func TestResolveItemCollectionSlug_ExactMatchAlwaysWins(t *testing.T) { } } +// BUG-2630 #2 (Codex round 1). The legacy semantic aliases the client-side map +// used to apply (t/i/p/d, phase/phases -> plans) are now resolved SERVER-side, +// so dropping client normalization — including on the remote MCP transport, +// which cannot retry — loses nothing. `t` must reach `tasks`. +func TestResolveItemCollectionSlug_LegacyAbbreviationResolves(t *testing.T) { + srv := testServer(t) + ws := createTestWorkspaceViaAPI(t, srv) // startup template ships `tasks` + + if rr := createItemIn(t, srv, ws, "t", "abbreviated"); rr.Code != http.StatusCreated { + t.Fatalf("create via alias `t` = %d: %s — the server must resolve the "+ + "legacy abbreviation now that clients send it raw", rr.Code, rr.Body) + } + if got := itemsIn(t, srv, ws, "tasks"); len(got) != 1 || got[0] != "abbreviated" { + t.Errorf("items in `tasks` = %v, want [abbreviated]", got) + } +} + +// The alias must never OUTRANK an exact match: a workspace with a real `t` +// collection routes `t` to `t`, not to `tasks`. Same exact-match-first property +// the ±s fallback already honors, asserted for the alias path too. +func TestResolveItemCollectionSlug_ExactMatchBeatsAlias(t *testing.T) { + srv := testServer(t) + ws := createTestWorkspaceViaAPI(t, srv) // ships `tasks` + makeCollection(t, srv, ws, "T", "t", "TT") + + if rr := createItemIn(t, srv, ws, "t", "belongs in t"); rr.Code != http.StatusCreated { + t.Fatalf("create in `t` = %d: %s", rr.Code, rr.Body) + } + if got := itemsIn(t, srv, ws, "t"); len(got) != 1 || got[0] != "belongs in t" { + t.Errorf("items in `t` = %v, want [belongs in t] — the alias outranked an exact match", got) + } + if got := itemsIn(t, srv, ws, "tasks"); len(got) != 0 { + t.Errorf("items in `tasks` = %v, want none — `t` was misrouted to its alias", got) + } +} + // The reverse direction: a workspace whose collection is genuinely singular // still answers to the plural a user might type out of habit. func TestResolveItemCollectionSlug_PluralOfASingularCollection(t *testing.T) { @@ -228,6 +264,15 @@ func TestCollectionSlugCandidates(t *testing.T) { // first would misfile the write. {in: "Spec", want: []string{"spec", "specs"}}, {in: "Specs", want: []string{"specs", "specss", "spec"}}, + // Legacy semantic aliases (BUG-2630): the abbreviation lands LAST, after + // the structural ±s candidates, so it is only ever a last resort. + {in: "t", want: []string{"ts", "tasks"}}, + {in: "d", want: []string{"ds", "docs"}}, + {in: "phase", want: []string{"phases", "plans"}}, + {in: "phases", want: []string{"phasess", "phase", "plans"}}, + // An alias that duplicates a structural candidate (plan -> plans is both + // the +s form AND the NormalizeSlug alias) is not emitted twice. + {in: "plan", want: []string{"plans"}}, } { got := collectionSlugCandidates(tc.in) if len(got) != len(tc.want) { diff --git a/internal/server/handlers_attachments_thumbnails_test.go b/internal/server/handlers_attachments_thumbnails_test.go index 4b1a037c..0d1b7a3a 100644 --- a/internal/server/handlers_attachments_thumbnails_test.go +++ b/internal/server/handlers_attachments_thumbnails_test.go @@ -286,11 +286,18 @@ func TestServerCapabilities_Endpoint(t *testing.T) { } var resp struct { - Image attachments.Capabilities `json:"image"` + Image attachments.Capabilities `json:"image"` + CollectionResolution bool `json:"collection_resolution"` } if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode response: %v", err) } + // BUG-2630: the CLI reads this flag to decide whether a collection-not-found + // is authoritative (skip the alias retry) or whether it is talking to an old + // build (retry). This build has the resolver, so it must advertise true. + if !resp.CollectionResolution { + t.Error("collection_resolution = false; this build resolves collections server-side") + } wantFormats := []string{"png", "jpeg", "gif", "bmp", "tiff"} if got, want := strings.Join(resp.Image.ImageFormats, ","), strings.Join(wantFormats, ","); got != want { t.Errorf("ImageFormats = %q, want %q", got, want) diff --git a/internal/server/handlers_capabilities.go b/internal/server/handlers_capabilities.go index d7793d6b..6e3011dc 100644 --- a/internal/server/handlers_capabilities.go +++ b/internal/server/handlers_capabilities.go @@ -14,6 +14,18 @@ import ( // Static for the lifetime of the binary, so clients are free to cache. type serverCapabilities struct { Image attachments.Capabilities `json:"image"` + + // CollectionResolution is true when this build resolves a collection slug + // server-side with exact-match-first + the singular/alias fallback and the + // archived-claims refusal (resolveItemCollectionSlug, BUG-2578/2630). The + // CLI reads it to decide, on a collection-not-found, whether that answer is + // AUTHORITATIVE (this build already tried every alias, so the slug is truly + // absent/archived/hidden — do not retry an alias and defeat the protection) + // or whether it is talking to an older build with no resolver and should + // fall back to the legacy client-side alias retry. Always true here; its + // ABSENCE (an old build that 404s this endpoint or omits the field) is the + // signal to retry. + CollectionResolution bool `json:"collection_resolution"` } // handleServerCapabilities reports what this build can do to the editor. @@ -24,7 +36,7 @@ type serverCapabilities struct { // rather than 500-ing — that signals to the editor "uploads still work, // but disable transformation tools." func (s *Server) handleServerCapabilities(w http.ResponseWriter, r *http.Request) { - resp := serverCapabilities{} + resp := serverCapabilities{CollectionResolution: true} if s.imageProcessor != nil { resp.Image = s.imageProcessor.Capabilities() } else {