fix(metadata-protocol,rest,spec): one predicate for capabilities.search and the /search route (#7541) - #7577
Merged
Conversation
…rch` and the /search route (#7541) `/discovery` reported `capabilities.search = {enabled:false}` while `GET /api/v1/search?q=audit` answered 200 with real hits. Prime Directive #10 inverted: 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 bit came from a registered `search` service slot; the route 501s on `typeof protocol.searchAll !== 'function'`. Nothing registers that slot in either repository and the protocol implements `searchAll` unconditionally, so the two did not merely risk disagreeing — they disagreed on every host that exists. `search` was the last well-known capability still on bare slot presence. Its neighbours already carry the rule stated in the builder ("the predicate is deliberately the SAME one that decides whether the route is advertised"), most recently `chunkedUpload` in #5672. This puts `search` on that footing: - metadata-protocol: the bit is now `typeof this.searchAll === 'function'` — the route's own refusal predicate. - rest: `/discovery` ANDs it with `api.enableSearch`, the flag that decides whether this server mounts the route at all. Same two-layer conjunction `transactionalBatch` uses with `api.enableBatch`. No fallback was added to the route. `services.search` keeps its own answer: the slot is a search ENGINE (Elastic/Meili, `ISearchService`), so it reports which implementation occupies it while the capability reports whether the surface is served. Those now legitimately differ, so spec gives the slot a REMEDY_DETAIL sentence — the treatment `ui` already carries for the same shape (#4146) — keeping the unchanged "no implementation ships" fact and adding which question it answers. The pin drives the real capability builder and the real route in one test and asserts they agree, over three hosts that genuinely differ (served 200, unmounted 404, unimplemented 501) — never `enabled === true`, which would pass again the day someone hardcodes the bit. Fixes #7541 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GHTC2SvMXprxV9eEHqfbCP
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
…rtial literal
The new test drove both handlers with `{ params, query }` object literals, which
do not satisfy `IHttpRequest` — 2 x TS2345, taking @objectstack/rest's TEST_DEBT
from its recorded 155 to 157. That ledger is a ratchet: it may only shrink.
Fixed at the source rather than by raising the entry, and the fix is not
bookkeeping. `enforceAuth` runs BEFORE either predicate under test and reads
`req.method` and `req.path` — both `undefined` in the old literals — so the pin
was measuring statuses a real caller would not necessarily get. The helper now
returns a complete `IHttpRequest` typed against the contract, so the gate reads
zero errors from this file AND the measured 200/404/501 are the statuses the
real request shape produces.
Not cast to `any`: the neighbouring conformance test casts its handler that way,
but a cast here would have hidden exactly the missing members that make the
measurement faithful.
Measured with the gate's own command (packages/rest/tsconfig.json with the test
globs dropped from `exclude`): 157 before, 155 after — equal to the recorded
ledger entry, which is left untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GHTC2SvMXprxV9eEHqfbCP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7541
/discoveryreportedcapabilities.search = {enabled:false}andservices.search = {enabled:false, status:"unavailable"}whileGET /api/v1/search?q=auditanswered 200 with 5 real hits. 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.Root cause: two producers, two unrelated predicates
registeredServices.has('search')getDiscovery(),packages/metadata-protocol/src/protocol.tstypeof protocol.searchAll !== 'function'→501registerSearchEndpoints,packages/rest/src/rest-server.tsNothing in either repository registers the
searchslot — the repo's ownCORE_SERVICE_PROVIDERtable records that as a verified fact — andObjectStackProtocolImplementationimplementssearchAllunconditionally. So the two producers did not merely risk disagreeing: they disagreed on every host that exists.searchwas the last well-known capability still on bare slot presence. Its neighbours already carry the rule stated in that builder — "the predicate is deliberately the SAME one that decides whether the route is advertised — what we advertise and what we claim cannot disagree" — withchunkedUploadmoved onto it in #5672. This bringssearchonto the same footing: one predicate feeding both the advertised bit and the route's own refusal. No fallback was added to the route.The two open determinations
1. Does the
searchservice slot carry independent meaning? — Yes. Not collapsed.Measured before touching it:
registerService('search'), andCORE_SERVICE_PROVIDERrecords'search': nullwith the note "Nothing provides the slot at all… Verified across BOTH repositories: nothing inobjectstack-ai/cloudregisters it" (ADR-0115 Evidence 5).http-dispatcher.ts:1080, which resolves the slot purely to fill its ownservices.searchentry. The runtime dispatcher serves no/searchdomain at all — so no code path anywhere routes a search request through the slot.CoreServiceNamedeclares it "Search Engine (Elastic/Meili)" andISearchServiceis a real index/query contract. A deployment that registers a Meilisearch service to get better search thansearchAll's LIKE-scan is exactly what the slot is for.So the two keys answer different questions, and the schema already says so:
WellKnownCapabilitiesSchema.searchis "whether the backend supports full-text search" (serveability), whileservices.searchreports which implementation occupies the slot.services.searchis therefore left exactly as it was — presence-derived, route/api/v1/searchwhen filled. Collapsing them would have destroyed the slot's only remaining signal.2. What should
services.search.statussay? — Stillunavailable. The message changed instead.After the fix an ordinary host reports
capabilities.search.enabled: truebesideservices.search.status: 'unavailable'. Both statements are true — but the old remedy line (No implementation ships for the 'search' slot — register a service under it to enable) reads as "search is dead here", which is exactly how two halves of one document come to look like they contradict each other.Flipping
statustoavailablewas rejected: no engine is registered, and saying otherwise is the original defect pointed the other way. Instead the slot gets aREMEDY_DETAILsentence in@objectstack/spec— the treatmentuialready carries for the identical shape (#4146: vestigial slot, surface served by the protocol). It keeps the unchanged "no implementation ships" opening (still pinned bycore-service-provider.test.ts) and adds which question the entry answers:Because that sentence lives in the one shared helper, the dispatcher and the metadata-protocol builder cannot drift on it.
File surface
packages/metadata-protocol/src/protocol.ts(@@ -3477)search: registeredServices.has('search')→typeof this.searchAll === 'function'— the route's own 501 predicate.packages/rest/src/rest-server.ts(@@ -3790)/discoveryANDs that withapi.enableSearch, the flag deciding whether this server mounts the route. Same two-layer conjunctiontransactionalBatchuses withapi.enableBatch; the mount's own?? truespelling is copied verbatim.packages/spec/src/system/core-services.zod.ts(@@ -97, @@ -131)REMEDY_DETAIL['search'](determination 2) + theCORE_SERVICE_PROVIDERcomment that describednull's two cases.packages/rest/src/discovery-search-capability-agreement.test.tspackages/objectql/src/protocol-discovery.test.tssearch: {enabled:false}— i.e. asserted the defect. Updated and re-commented; one now also pins that the two halves stay independent..changeset/search-capability-serveability-predicate.md@objectstack/metadata-protocol,@objectstack/rest,@objectstack/spec.The pin asserts agreement, not a value
A test that asserts
enabled === truepasses again the day someone hardcodes the bit. So the new file measures both sides from the real producers in one test —capabilities.search.enabledoff the realgetDiscovery()through the real/discoveryhandler, and the status the realregisterSearchEndpointshandler actually answers — and assertsdeclared === servedacross three hosts that genuinely differ:200, real hitstrueapi.enableSearch: false(route not mounted)404falsesearchAll(instance override)501falseA fourth test pins that the three are genuinely discriminated (
[true,false,false]/[200,404,501]), so the agreement cannot hold vacuously.Reverse verification
Predictions were written before any test run.
expected false to be trueenableSearch:falseagreesexpected true to be false; ✅ GREEN at full baselinesearchAllagrees[false,false,false]vs[true,false,false]protocol-discovery.test.ts› "capabilities field populated from registered services"protocol-discovery.test.ts› "all capabilities false when no services registered"protocol-discovery.test.ts› "dynamically set capabilities" (search: true)protocol-discovery.test.ts› "non-dispatcher-owned routes presence-gated" (services.search.route)services.searchuntouchedruntime/discovery-schema-conformance.test.ts:245(capabilities.search === false)spec/core-service-provider.test.ts(search remedy invariants)/No implementation ships/, noInstall, contains'search'rest/discovery-schema-conformance.test.ts(vocabulary + boolean-shape gates)scripts/check-service-providers.mjsMissed predictions: none. Two harness corrections were needed before the pin could measure anything, neither of which changed a prediction: sibling packages must be built for
@objectstack/metadata-protocolto resolve, andenforceAuth401s an anonymous request before thesearchAllprobe — so the pin authenticates, matching step 1 of the issue's reproduction.Consumption radius
Grepped for every consumer of the capability bit and of the
searchslot:capabilities.search— no production consumer in this repository. The SDK reads it only through the genericclient.capabilitiesgetter (packages/client/src/index.ts); the real consumer is objectui, out of this repo. Test readers:client/client.test.ts(fixture-fed, unaffected),runtime/discovery-schema-conformance.test.ts(dispatcher producer, untouched).searchslot — registrants: none. Consumers:http-dispatcher.ts:1080(its ownservices.searchentry only). Untouched by this PR.Two things deliberately left alone, both pre-existing and out of this card's lane:
capabilities.searchstays!!searchSvc. It serves no/searchdomain, so its honest answer isfalse— which is what it reports today, since nothing fills the slot. Its predicate is still presence-based and would over-promise if a slot were ever registered on a dispatcher host; that is a separate defect in a third producer, not this seam.routes.searchis still never advertised by either producer (serviceToRouteKeyhas nosearchentry), even thoughSERVICE_CONFIG.searchdeclares the path. The SDK'sclient.search()uses a fixed path, so nothing is broken by it today.Gates run
Full package suites, all green (
pnpm buildfirst — the earlier run's suite-level failures were unresolved workspace deps, not assertions):@objectstack/spec@objectstack/metadata-protocol@objectstack/objectql@objectstack/rest@objectstack/runtime@objectstack/client17 609 tests, 0 failures.
turbo run typecheckover the four touched packages — 20/20 ✅pnpm build— 71/71 ✅node scripts/check-service-providers.mjs✅node scripts/check-empty-changeset.mjs✅ ·node scripts/check-adr-0087-registration.mjs✅node scripts/docs-audit/check-audit-scope.mjs✅ (0 docs affected;content/docs/releases/untouched)eslint --no-inline-configover every changed file ✅🤖 Generated with Claude Code
https://claude.ai/code/session_01GHTC2SvMXprxV9eEHqfbCP
Generated by Claude Code