fix(metadata-protocol,rest,spec): one predicate for capabilities.search and the /search route (#7541)

.changeset/search-capability-serveability-predicate.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/metadata-protocol": patch
+"@objectstack/rest": patch
+"@objectstack/spec": patch
+---
+
+fix(metadata-protocol,rest,spec): derive `capabilities.search` from what serves `/search`, not from an empty service slot (#7541)
+
+Every REST host advertised `capabilities.search = { enabled: false }` in
+`/discovery` while `GET /api/v1/search?q=…` answered `200` with real hits. This
+is Prime Directive #10 inverted: not an advertised endpoint that 404s, but a
+live endpoint **no conforming client will ever call**, because the document
+whose only job is to say what is available said it was not.
+
+**Two producers, two unrelated predicates.** The capability bit came from a
+registered `search` service slot (`registeredServices.has('search')`), while the
+route refused on something else entirely — `registerSearchEndpoints` returns
+`501 NOT_IMPLEMENTED` exactly when `typeof protocol.searchAll !== 'function'`.
+Nothing in either repository registers that slot (`CORE_SERVICE_PROVIDER`
+records this, verified), and the protocol implements `searchAll`
+unconditionally, so the two answers were not merely capable of disagreeing —
+they disagreed on every host that exists.
+
+`search` was the last well-known capability still on bare slot presence. Its
+neighbours were moved onto serveability with the rule stated in the builder —
+*"the predicate is deliberately the SAME one that decides whether the route is
+advertised — what we advertise and what we claim cannot disagree"* — most
+recently `chunkedUpload` in #5672. This brings `search` onto that footing: **one
+predicate, both ends.**
+
+- `@objectstack/metadata-protocol` — `capabilities.search` is now
+  `typeof this.searchAll === 'function'`, the route's own refusal predicate.
+- `@objectstack/rest` — the `/discovery` producer ANDs that with
+  `api.enableSearch`, the flag that decides whether this server mounts the route
+  at all. Exactly the two-layer conjunction `transactionalBatch` already uses
+  with `api.enableBatch`: the protocol states what it can serve, the server
+  states what it mounted, and a deployment that opts out reports `false` rather
+  than promising a 404. Nothing was added to the route itself.
+
+**`services.search` is unchanged, and deliberately so.** The slot answers a
+different question — `CoreServiceName` declares it "Search Engine
+(Elastic/Meili)" and `ISearchService` is an index/query contract — so it still
+reports *which engine occupies the slot*, while the capability reports *whether
+the surface is served*. On an ordinary host those now differ
+(`capabilities.search.enabled: true` beside `services.search.status:
+'unavailable'`), and both statements are true. So that the two halves of one
+document do not read as contradicting each other, `@objectstack/spec` gives the
+slot a `REMEDY_DETAIL` sentence — the same treatment `ui` carries for the same
+shape (#4146) — which keeps the unchanged "no implementation ships" fact and
+adds which question the entry answers. The `status` itself stays
+`unavailable`: no engine is registered, and saying otherwise would be the
+original defect pointed the other way.
+
+**Client impact.** A client that gated its search UI on
+`capabilities.search.enabled` was hiding a working feature on every deployment;
+it now sees `true` wherever the endpoint really serves, and `false` when the
+protocol cannot search (route `501`) or the server did not mount it (`404`).

packages/metadata-protocol/src/protocol.ts

@@ -3477,7 +3477,34 @@ export class ObjectStackProtocolImplementation implements
             comments: !!this.engine.registry?.getObject?.('sys_comment'),
             automation: registeredServices.has('automation'),
             cron: registeredServices.has('job'),
-            search: registeredServices.has('search'),
+            // [#7541] Serveability-gated on the protocol's OWN search
+            // implementation, was slot presence. This is the same predicate the
+            // route refuses on: `registerSearchEndpoints`
+            // (packages/rest/src/rest-server.ts) 501s exactly when
+            // `typeof protocol.searchAll !== 'function'`, so the two ends can no
+            // longer answer the same question differently — the rule stated at
+            // the top of this block, applied to the key that was still exempt.
+            //
+            // The old predicate was wrong in the direction discovery exists to
+            // prevent: `searchAll` is implemented by this class unconditionally,
+            // nothing in either repository registers the `search` slot
+            // (CORE_SERVICE_PROVIDER records that, verified), so every REST host
+            // served `GET /api/v1/search` 200 while advertising
+            // `capabilities.search = false`. A conforming client — one that
+            // trusts the document instead of probing — skipped a working
+            // surface. Prime Directive #10 inverted.
+            //
+            // `services.search` is deliberately NOT collapsed into this. The
+            // slot is a distinct question with its own answer: `CoreServiceName`
+            // declares it "Search Engine (Elastic/Meili)" and `ISearchService`
+            // is an index/query contract, so `services.search` reports WHICH
+            // ENGINE occupies the slot while this bit reports WHETHER THE
+            // SURFACE IS SERVED (`WellKnownCapabilitiesSchema.search`: "whether
+            // the backend supports full-text search"). They may legitimately
+            // differ — an empty slot with a served endpoint is today's normal
+            // host — and `serviceUnavailableMessage('search')` now says so in
+            // the same document, the way `ui` does for the same shape (#4146).
+            search: typeof this.searchAll === 'function',
             export: registeredServices.has('automation') || registeredServices.has('queue'),
             // [#5672] Serveability-gated, was presence-only. Two reasons, and
             // the second is the binding one:

packages/objectql/src/protocol-discovery.test.ts

@@ -448,27 +448,43 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () =>
     expect(discovery.capabilities).toBeDefined();
     // ui is registered but doesn't map to a well-known capability directly
     expect(discovery.services.ui.enabled).toBe(true);
-    // All well-known capabilities should be disabled since ui doesn't map to any
-    // (comments derives from the sys_comment object, which is not registered here).
+    // The SLOT-DERIVED well-known capabilities should be disabled since ui maps
+    // to none of them (comments derives from the sys_comment object, which is
+    // not registered here).
     expect(discovery.capabilities!.comments).toEqual({ enabled: false });
     expect(discovery.capabilities!.automation).toEqual({ enabled: false });
     expect(discovery.capabilities!.cron).toEqual({ enabled: false });
-    expect(discovery.capabilities!.search).toEqual({ enabled: false });
     expect(discovery.capabilities!.export).toEqual({ enabled: false });
     expect(discovery.capabilities!.chunkedUpload).toEqual({ enabled: false });
+    // [#7541] `search` is NOT in that list any more. It is no longer derived
+    // from a service slot at all — it reports whether this protocol can serve
+    // `/search` (`typeof searchAll === 'function'`, the predicate the route's
+    // own 501 uses), and this class always can. Asserting `false` here was
+    // asserting the defect: the endpoint served 200s while the document said
+    // the capability was off.
+    expect(discovery.capabilities!.search).toEqual({ enabled: true });
   });
 
-  it('should set all capabilities to false when no services are registered', async () => {
+  it('should set all slot-derived capabilities to false when no services are registered', async () => {
     protocol = new ObjectStackProtocolImplementation(engine);
     const discovery = await protocol.getDiscovery();
 
     expect(discovery.capabilities).toBeDefined();
     expect(discovery.capabilities!.comments).toEqual({ enabled: false });
     expect(discovery.capabilities!.automation).toEqual({ enabled: false });
     expect(discovery.capabilities!.cron).toEqual({ enabled: false });
-    expect(discovery.capabilities!.search).toEqual({ enabled: false });
     expect(discovery.capabilities!.export).toEqual({ enabled: false });
     expect(discovery.capabilities!.chunkedUpload).toEqual({ enabled: false });
+    // [#7541] Same reason as above — and this is the exact host the issue was
+    // reported against: an empty registry, a live `/search`. The two halves of
+    // the document stay independent and both stay honest: the SLOT is still
+    // empty here...
+    expect(discovery.capabilities!.search).toEqual({ enabled: true });
+    expect(discovery.services.search.enabled).toBe(false);
+    expect(discovery.services.search.status).toBe('unavailable');
+    // ...and its message now says which question that answers, instead of
+    // reading as "search is dead on this host".
+    expect(discovery.services.search.message).toMatch(/capabilities\.search/);
   });
 
   it('should dynamically set capabilities based on registered services', async () => {
@@ -482,11 +498,18 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () =>
 
     expect(discovery.capabilities!.automation).toEqual({ enabled: true });
     expect(discovery.capabilities!.cron).toEqual({ enabled: false });
-    expect(discovery.capabilities!.search).toEqual({ enabled: true });
     expect(discovery.capabilities!.export).toEqual({ enabled: true });
     expect(discovery.capabilities!.chunkedUpload).toEqual({ enabled: true });
     // comments is independent of services — it tracks the sys_comment object (#3180).
     expect(discovery.capabilities!.comments).toEqual({ enabled: false });
+    // [#7541] `search` is true here too, but NOT because the slot above is
+    // filled — this line proves nothing about the slot and is kept only so the
+    // reader is not left thinking it does. The discriminating cases live in
+    // `packages/rest/src/discovery-search-capability-agreement.test.ts`, which
+    // drives the capability builder and the route together.
+    expect(discovery.capabilities!.search).toEqual({ enabled: true });
+    // What the slot DOES still decide, unchanged: the `services` half.
+    expect(discovery.services.search.enabled).toBe(true);
   });
 
   // ── Atomic cross-object batch capability (#3298 / #1604 / ADR-0034) ─────────

packages/rest/src/discovery-search-capability-agreement.test.ts

@@ -0,0 +1,188 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#7541] The two producers of "can this host search" must answer the same
+// question the same way.
+//
+// The defect: `capabilities.search` came from a registered `search` SERVICE
+// SLOT, while `GET {basePath}/search` refused on something else entirely —
+// `typeof protocol.searchAll !== 'function'`. Nothing in either repository
+// registers that slot and the protocol implements `searchAll` unconditionally,
+// so every REST host advertised `capabilities.search = {enabled:false}` while
+// serving 200s with real hits. A client that trusts the discovery document —
+// which is the document's only purpose — skipped a working surface. Prime
+// Directive #10 inverted: not an advertised endpoint that 404s, but a live
+// endpoint no conforming client will ever call.
+//
+// WHAT THIS FILE ASSERTS, and why it is shaped this way: it does NOT assert
+// `enabled === true`. That assertion passes again the day someone hardcodes the
+// bit, which is the same class of defect one layer over. It asserts AGREEMENT —
+// `declared === served` — with both sides MEASURED from the real producers in
+// the same test: `capabilities.search` off the real `getDiscovery()` through
+// the real `/discovery` handler, and the served status off the real
+// `registerSearchEndpoints` handler. Three hosts that genuinely differ (below)
+// keep the agreement from holding vacuously.
+
+import { describe, it, expect, vi } from 'vitest';
+import type { IHttpRequest } from '@objectstack/spec/contracts';
+import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
+import { RestServer } from './rest-server.js';
+
+/**
+ * A complete `IHttpRequest`, typed against the contract rather than cast to
+ * `any`. Not ceremony: `enforceAuth` — which runs before either predicate under
+ * test — reads `method` and `path`, so a partial literal would exercise the
+ * gate with `undefined` on both. Building the real shape is what makes the
+ * measured statuses below the statuses a real caller gets.
+ */
+function request(path: string, query: Record<string, string> = {}): IHttpRequest {
+  return { params: {}, query, headers: {}, method: 'GET', path };
+}
+
+function createMockServer() {
+  return {
+    get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
+    use: vi.fn(),
+    listen: vi.fn().mockResolvedValue(undefined),
+    close: vi.fn().mockResolvedValue(undefined),
+  };
+}
+
+/**
+ * An engine carrying ONE searchable object with ONE matching row, so the
+ * served path is a real 200-with-hits rather than a 200-with-nothing — the
+ * exact reproduction in the issue (`?q=audit` → 200, real hits).
+ */
+function createEngine() {
+  const widget = {
+    name: 'widget',
+    fields: [{ name: 'title', type: 'text', searchable: true }],
+  };
+  return {
+    registry: {
+      getObject: (n: string) => (n === 'widget' ? widget : undefined),
+      getAllObjects: () => [widget],
+      getRegisteredTypes: () => [],
+    },
+    find: async () => [{ id: 'w1', title: 'audit trail' }],
+  };
+}
+
+type Host = {
+  /** `capabilities.search.enabled` as the composed `/discovery` body reports it. */
+  declared: boolean;
+  /** HTTP status `GET {basePath}/search?q=audit` actually answers. */
+  status: number;
+  /** Hit count when it answered 200. */
+  hits: number;
+};
+
+/**
+ * Boot a REST server over the REAL protocol and read BOTH producers off it.
+ *
+ * `enableSearch` selects whether this server mounts the route at all;
+ * `withSearchAll: false` removes the protocol's own implementation, which is
+ * the input the route's 501 branch exists for.
+ */
+async function measure(opts: {
+  enableSearch?: boolean;
+  withSearchAll?: boolean;
+} = {}): Promise<Host> {
+  const protocol: any = new ObjectStackProtocolImplementation(
+    createEngine() as any,
+    () => new Map(),
+  );
+  if (opts.withSearchAll === false) {
+    // Shadow the prototype method on the instance. Both predicates read the
+    // same property off the same object, so this single override is what makes
+    // the "protocol cannot search" host measurable at all — and it is why the
+    // test cannot pass by two independent predicates coincidentally agreeing.
+    Object.defineProperty(protocol, 'searchAll', { value: undefined, configurable: true });
+  }
+
+  const config: any = {
+    api: {
+      requireAuth: false,
+      ...(opts.enableSearch === false ? { enableSearch: false } : {}),
+    },
+  };
+  const rest = new RestServer(createMockServer() as any, protocol as any, config);
+  // Authenticated caller — step 1 of the issue's reproduction. `enforceAuth`
+  // runs BEFORE the `searchAll` probe, so an anonymous request 401s and never
+  // reaches either predicate; this is the house stub the other rest tests use
+  // for authed handlers, and it is upstream of everything under test here.
+  (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' });
+  rest.registerRoutes();
+  const routes = rest.getRouteManager();
+
+  const discoveryEntry = routes.get('GET', '/api/v1/discovery');
+  if (!discoveryEntry) throw new Error('discovery route not registered');
+  let discoveryBody: any;
+  const discoveryRes: any = {
+    json: (b: any) => { discoveryBody = b; },
+    status: () => discoveryRes,
+  };
+  await discoveryEntry.handler(request('/api/v1/discovery'), discoveryRes);
+  const declared = discoveryBody?.capabilities?.search?.enabled;
+
+  const searchEntry = routes.get('GET', '/api/v1/search');
+  if (!searchEntry) {
+    // Not mounted — a client calling it gets the router's 404. That IS the
+    // served answer for this host.
+    return { declared, status: 404, hits: 0 };
+  }
+  let status = 200;
+  let searchBody: any;
+  const searchRes: any = {
+    status: (s: number) => { status = s; return searchRes; },
+    json: (b: any) => { searchBody = b; },
+  };
+  await searchEntry.handler(request('/api/v1/search', { q: 'audit' }), searchRes);
+  return { declared, status, hits: searchBody?.hits?.length ?? 0 };
+}
+
+/** Served ⇔ a caller can get search results out of this host. */
+const isServed = (h: Host) => h.status !== 404 && h.status !== 501;
+
+describe('[#7541] `capabilities.search` and the /search route answer one question', () => {
+  it('agrees on the ordinary host — where the document used to contradict the endpoint', async () => {
+    const host = await measure();
+
+    // The symptom, measured: the endpoint really does serve real hits here.
+    expect(host.status).toBe(200);
+    expect(host.hits).toBeGreaterThan(0);
+
+    // The pin: whatever the endpoint does, the document says the same thing.
+    // Before the fix `declared` was false against a 200 — the inversion.
+    expect(host.declared).toBe(isServed(host));
+  });
+
+  it('agrees on a host that does not mount the route (`api.enableSearch: false`)', async () => {
+    const host = await measure({ enableSearch: false });
+
+    expect(host.status).toBe(404);
+    expect(host.declared).toBe(isServed(host));
+  });
+
+  it('agrees on a protocol that cannot search — both ends refuse on the same predicate', async () => {
+    const host = await measure({ withSearchAll: false });
+
+    // The route's own 501 branch, reached through the real handler.
+    expect(host.status).toBe(501);
+    expect(host.declared).toBe(isServed(host));
+  });
+
+  it('anti-vacuity: the three hosts are genuinely discriminated, in both directions', async () => {
+    const [served, unmounted, unimplemented] = await Promise.all([
+      measure(),
+      measure({ enableSearch: false }),
+      measure({ withSearchAll: false }),
+    ]);
+
+    // Without this, `declared === served` would hold for the empty reason if
+    // some future edit pinned the bit — or the route — to one constant.
+    expect([served.declared, unmounted.declared, unimplemented.declared])
+      .toEqual([true, false, false]);
+    expect([served.status, unmounted.status, unimplemented.status])
+      .toEqual([200, 404, 501]);
+  });
+});

packages/rest/src/rest-server.ts

@@ -3790,6 +3790,25 @@ export class RestServer {
                             + '{ $ref: <opIndex> } parent references (#1604 / ADR-0034).',
                     };
 
+                    // [#7541] Global search — the same two-layer AND, for the
+                    // same reason. The protocol answered whether IT can serve a
+                    // search (`typeof searchAll === 'function'`, the predicate
+                    // `registerSearchEndpoints` 501s on); this server answers
+                    // whether it MOUNTED the route at all (`api.enableSearch`,
+                    // the flag gated in registerRoutes). A deployment that opts
+                    // out gets a 404, so advertising the protocol's `true`
+                    // unqualified would re-open the declared ≠ enforced gap one
+                    // layer up from the one this issue closed. Neither half is a
+                    // fallback for a wrong bit: each layer states the fact only
+                    // it knows, and `enabled` is their conjunction.
+                    //
+                    // The flag is read with the mount's own `?? true` spelling
+                    // rather than the equivalent `!== false` — same predicate,
+                    // same characters, so the two cannot be edited apart.
+                    caps.search = {
+                        enabled: !!caps.search?.enabled && (this.config.api.enableSearch ?? true),
+                    };
+
                     // Attach scoping metadata so clients can detect dual-mode routing.
                     (discovery as any).scoping = {
                         enabled: this.config.api.enableProjectScoping,