Skip to content

Commit 86a71d1

Browse files
os-zhuangclaude
andauthored
fix(spec,runtime,metadata-protocol): discovery's "install this to enable" names a package that exists (#4093 follow-up) (#4204)
Discovery tells a consumer two things about an absent capability: that it is absent, and what to do about it. The first has been carefully honest since #2462/#4000 — the whole D12 lineage is about it. The second was invented from the slot name, and nothing ever checked it. The dispatcher templated `Install a ${slot} plugin to enable` across twelve slots. metadata-protocol hand-wrote a table where TEN OF FIFTEEN entries named a package that does not exist: plugin-redis, plugin-bullmq, job-scheduler, plugin-notifications, plugin-storage, plugin-automation, ui-plugin, plus plugin-ai / plugin-search / plugin-workflow for slots nothing implements at all. The same value is surfaced as discovery's `provider`. That is the same `declared ≠ enforced` failure this lineage has been closing, one level over: not "does the capability exist" but "is the fix real". A package that cannot be installed is a dead end handed to someone at the exact moment they are trying to fix their stack, and an agent reading discovery cannot tell it apart from a package it should install. CORE_SERVICE_PROVIDER + serviceUnavailableMessage() in @objectstack/spec/system are now the one place that sentence is written, read by both builders — so the two hosts cannot name different packages for the same slot, the drift #4089 and #4130 closed for the `metadata` and `data` entries. A parity test asserts it. Entries were verified against what actually calls `registerService` for each slot, not against name similarity. That is how `notification` turned out to be filled by @objectstack/service-messaging — the one slot whose package shares no word with its name, and the proof that a name-derived guess cannot be right in general. Four slots (ai, search, workflow, graphql) have no implementation anywhere, so `null` is a first-class entry and the message says nothing ships rather than naming a plausible package. `ui` keeps the fuller sentence from #4146 — it is served by the `protocol` service and nothing registers the `ui` slot, so "install X" alone would read as "X fills this slot" and leave the reader staring at an empty slot afterwards. Moving it into the shared table is what the parity test surfaced: the two builders agreed on the package but not on the sentence, and the richer one deserved to win rather than be flattened. scripts/check-service-providers.mjs (wired into the lint workflow as check:service-providers) fails CI on either way this table rots: a name that is not a real workspace package, or a CoreServiceName slot with no entry at all. Both modes verified by temporarily breaking the table. Same spirit as check-route-envelope, which caught a stale exemption of mine earlier in this lineage — the guard has to outlive the fix, or the next rename re-introduces it. Also: spec's api-surface snapshot regenerated (0 breaking, 2 added — the guard reported exactly the two new exports); the two docs that printed the old remedy corrected (`protocol/kernel/http-protocol.mdx` showed `Install plugin-workflow` / `Install plugin-ai` for slots that have no implementation, and `kernel/services-checklist.mdx` carried the old template down to its `a auth` grammar bug); and objectql's discovery test follows `provider` to the scoped name — a `provider`/`message` assertion can live in any package that exercises a discovery builder, and that one was caught by Test Core rather than by filtered local runs. Verified: spec 7143, runtime 947, metadata-protocol 110, objectql 1238 pass; build 71/71; pnpm lint and all eleven check:* scripts clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent bb1ce2e commit 86a71d1

13 files changed

Lines changed: 416 additions & 27 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/runtime': patch
4+
'@objectstack/metadata-protocol': patch
5+
---
6+
7+
Discovery's "install this to enable" now names a package that exists (#4093 follow-up).
8+
9+
Discovery tells a consumer two things about an absent capability: that it is absent, and what to do about it. The first has been carefully honest since #2462/#4000. The second was invented from the slot name.
10+
11+
The dispatcher templated `Install a ${slot} plugin to enable` across twelve slots, and `metadata-protocol` carried a hand-written table in which **ten of fifteen entries named a package that does not exist**`plugin-redis`, `plugin-bullmq`, `job-scheduler`, `plugin-notifications`, `plugin-storage`, `plugin-automation`, `ui-plugin`, plus `plugin-ai`, `plugin-search` and `plugin-workflow` for slots nothing implements at all. That value is also surfaced as discovery's `provider`.
12+
13+
A remedy naming a package that cannot be installed is a dead end handed to someone at the exact moment they are trying to fix their stack — and an agent reading discovery cannot tell it apart from a package it should install. It is the same `declared ≠ enforced` failure this lineage has been closing, one level over: not "does the capability exist" but "is the fix real".
14+
15+
`CORE_SERVICE_PROVIDER` and `serviceUnavailableMessage()` in `@objectstack/spec/system` are now the one place that sentence is written, and both discovery builders read them, so the two hosts cannot tell a consumer to install different things (the drift #4089 and #4130 closed for the `metadata` and `data` entries). Entries were verified against what actually calls `registerService` for each slot rather than against name similarity — which is how `notification` turned out to be filled by `@objectstack/service-messaging`, the one slot whose package shares no word with its name.
16+
17+
Four slots — `ai`, `search`, `workflow`, `graphql` — have no implementation anywhere, so they now say so instead of naming a plausible package. `ui` keeps the fuller sentence it got in #4146 (`/ui` is served by the `protocol` service; nothing registers the `ui` slot), and that sentence now reaches both builders instead of one.
18+
19+
`scripts/check-service-providers.mjs` (wired into the lint workflow as `check:service-providers`) fails CI when a named package is not a real workspace package, or when a `CoreServiceName` slot has no entry — so a rename or a deletion cannot leave a stale instruction behind.
20+
21+
FROM → TO: `services.<slot>.message` and `services.<slot>.provider` change text for most unavailable slots. Anything matching on the old `Install a <slot> plugin to enable` wording should match on `status: 'unavailable'` instead — the status field is the contract; the message is prose for humans and agents.

.github/workflows/lint.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ jobs:
9999
- name: Single authz resolver guard
100100
run: pnpm check:authz-resolver
101101

102+
# #4093 follow-up. Discovery tells a consumer an absent capability is
103+
# absent AND what to install. The first half has been carefully honest
104+
# since #2462/#4000; the second was invented from the slot name, so ten
105+
# of fifteen entries named packages that do not exist — a dead end handed
106+
# to whoever is trying to fix their stack. Asserts every package named in
107+
# CORE_SERVICE_PROVIDER is a real workspace package, and that no core
108+
# slot is missing an entry.
109+
- name: Service-provider remedy guard
110+
run: pnpm check:service-providers
111+
102112
# #3843 response-envelope guard. The route ledgers audit which routes EXIST;
103113
# nothing audited what comes back, so six route modules emitted bodies
104114
# outside BaseResponseSchema while carrying green `sdk` rows. Counts the

content/docs/kernel/services-checklist.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ no message. Below is the former — the fallback case:
134134
},
135135
"auth": {
136136
"enabled": false, "status": "unavailable",
137-
"message": "Install a auth plugin to enable"
137+
"message": "Install @objectstack/plugin-auth to enable"
138138
}
139139
}
140140
}

content/docs/protocol/kernel/http-protocol.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ GET /api/v1/discovery HTTP/1.1
5353
"services": {
5454
"data": { "enabled": true, "status": "available", "route": "/api/v1/data", "provider": "objectql" },
5555
"metadata": { "enabled": true, "status": "available", "route": "/api/v1/meta", "provider": "objectql" },
56-
"auth": { "enabled": true, "status": "available", "route": "/api/v1/auth", "provider": "plugin-auth" },
57-
"workflow": { "enabled": false, "status": "unavailable", "message": "Install plugin-workflow to enable" },
58-
"ai": { "enabled": false, "status": "unavailable", "message": "Install plugin-ai to enable" }
56+
"auth": { "enabled": true, "status": "available", "route": "/api/v1/auth", "provider": "@objectstack/plugin-auth" },
57+
"workflow": { "enabled": false, "status": "unavailable", "message": "No implementation ships for the 'workflow' slot — register a service under it to enable" },
58+
"ai": { "enabled": false, "status": "unavailable", "message": "No implementation ships for the 'ai' slot — register a service under it to enable" }
5959
},
6060
"locale": {
6161
"default": "en-US",

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
"check:role-word": "node scripts/check-role-word.mjs",
3535
"check:org-identifier": "node scripts/check-org-identifier.mjs",
3636
"check:authz-resolver": "node scripts/check-single-authz-resolver.mjs",
37+
"check:service-providers": "node scripts/check-service-providers.mjs",
3738
"check:route-envelope": "node scripts/check-route-envelope.mjs --self-test && node scripts/check-route-envelope.mjs",
3839
"check:error-code-casing": "node scripts/check-error-code-casing.mjs --self-test && node scripts/check-error-code-casing.mjs",
3940
"check:wildcard-fallthrough": "node scripts/check-wildcard-fallthrough.mjs --self-test && node scripts/check-wildcard-fallthrough.mjs",

packages/metadata-protocol/src/protocol.ts

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { readServiceSelfInfo } from '@objectstack/spec/api';
2121
import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data';
2222
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2323
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
24-
import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system';
24+
import { METADATA_FORM_REGISTRY, CORE_SERVICE_PROVIDER, serviceUnavailableMessage } from '@objectstack/spec/system';
2525
import { DEFAULT_METADATA_TYPE_REGISTRY, getMetadataTypeSchema, getMetadataTypeActions, getMetadataCreateSeed } from '@objectstack/spec/kernel';
2626
import {
2727
extractProtection,
@@ -916,28 +916,42 @@ function suggestQueryParam(param: string, knownFields: readonly string[]): strin
916916
* not advertise a route for it (ADR-0076 D12, #2462: an advertised route
917917
* with no mounted handler 404s and misleads consumers).
918918
*/
919-
const SERVICE_CONFIG: Record<string, { route?: string; plugin: string }> = {
919+
/**
920+
* [#4093 follow-up] `plugin` is no longer written here. It named the package a
921+
* consumer should install, and ten of the fifteen names did not exist —
922+
* `plugin-redis` / `plugin-bullmq` / `job-scheduler` / `plugin-notifications` /
923+
* `plugin-storage` / `plugin-automation` / `ui-plugin`, plus `plugin-ai`,
924+
* `plugin-search` and `plugin-workflow` for slots nothing implements at all.
925+
* The value is surfaced as discovery's `provider` and as its remedy line, so a
926+
* wrong name is a dead end handed to whoever is trying to fix their stack.
927+
*
928+
* It now comes from `CORE_SERVICE_PROVIDER` in `@objectstack/spec/system`, the
929+
* one table both discovery builders read, verified against what actually
930+
* registers each slot and guarded by `scripts/check-service-providers.mjs`.
931+
* Only the ROUTE stays local — that is this builder's own knowledge.
932+
*/
933+
const SERVICE_CONFIG: Record<string, { route?: string }> = {
920934
// Plugin-provided like every other optional service since the degraded
921935
// ObjectQL fallback was retired (#3891): advertised iff the real engine
922936
// is registered — never hardcoded 'available' (the pre-#2462 lie the
923937
// fallback existed to paper over).
924-
analytics: { route: '/api/v1/analytics', plugin: 'service-analytics' },
925-
auth: { route: '/api/v1/auth', plugin: 'plugin-auth' },
926-
automation: { route: '/api/v1/automation', plugin: 'plugin-automation' },
927-
cache: { route: '/api/v1/cache', plugin: 'plugin-redis' },
928-
queue: { route: '/api/v1/queue', plugin: 'plugin-bullmq' },
929-
job: { route: '/api/v1/jobs', plugin: 'job-scheduler' },
930-
ui: { route: '/api/v1/ui', plugin: 'ui-plugin' },
931-
workflow: { route: '/api/v1/workflow', plugin: 'plugin-workflow' },
938+
analytics: { route: '/api/v1/analytics' },
939+
auth: { route: '/api/v1/auth' },
940+
automation: { route: '/api/v1/automation' },
941+
cache: { route: '/api/v1/cache' },
942+
queue: { route: '/api/v1/queue' },
943+
job: { route: '/api/v1/jobs' },
944+
ui: { route: '/api/v1/ui' },
945+
workflow: { route: '/api/v1/workflow' },
932946
// service-realtime is an in-process pub/sub bus; nothing mounts
933947
// /api/v1/realtime, so no route is advertised (D12, #2462).
934-
realtime: { plugin: 'service-realtime' },
935-
notification: { route: '/api/v1/notifications', plugin: 'plugin-notifications' },
936-
ai: { route: '/api/v1/ai', plugin: 'plugin-ai' },
937-
i18n: { route: '/api/v1/i18n', plugin: 'service-i18n' },
938-
graphql: { route: '/graphql', plugin: 'plugin-graphql' }, // GraphQL uses /graphql by convention (not versioned REST)
939-
'file-storage': { route: '/api/v1/storage', plugin: 'plugin-storage' },
940-
search: { route: '/api/v1/search', plugin: 'plugin-search' },
948+
realtime: {},
949+
notification: { route: '/api/v1/notifications' },
950+
ai: { route: '/api/v1/ai' },
951+
i18n: { route: '/api/v1/i18n' },
952+
graphql: { route: '/graphql' }, // GraphQL uses /graphql by convention (not versioned REST)
953+
'file-storage': { route: '/api/v1/storage' },
954+
search: { route: '/api/v1/search' },
941955
};
942956

943957
/**
@@ -1747,7 +1761,7 @@ export class ObjectStackProtocolImplementation implements
17471761
enabled: true,
17481762
status: self?.status ?? (noHttpSurface ? ('degraded' as const) : ('available' as const)),
17491763
route: advertisedRoute(serviceName, config.route),
1750-
provider: config.plugin,
1764+
provider: CORE_SERVICE_PROVIDER[serviceName] ?? undefined,
17511765
...(noHttpSurface || self?.handlerReady !== undefined
17521766
? { handlerReady: noHttpSurface ? false : self?.handlerReady }
17531767
: {}),
@@ -1762,7 +1776,7 @@ export class ObjectStackProtocolImplementation implements
17621776
services[serviceName] = {
17631777
enabled: false,
17641778
status: 'unavailable' as const,
1765-
message: `Install ${config.plugin} to enable`,
1779+
message: serviceUnavailableMessage(serviceName),
17661780
};
17671781
}
17681782
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () =>
4141
expect(discovery.services.auth.enabled).toBe(true);
4242
expect(discovery.services.auth.status).toBe('available');
4343
expect(discovery.services.auth.route).toBe('/api/v1/auth');
44-
expect(discovery.services.auth.provider).toBe('plugin-auth');
44+
// [#4093 follow-up] Scoped now: `provider` comes from CORE_SERVICE_PROVIDER,
45+
// which names the package you can actually install rather than a bare label.
46+
expect(discovery.services.auth.provider).toBe('@objectstack/plugin-auth');
4547
expect(discovery.routes.auth).toBe('/api/v1/auth');
4648
});
4749

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,6 +2290,61 @@ describe('HttpDispatcher', () => {
22902290
expect(info.features.i18n).toBe(false);
22912291
});
22922292

2293+
// [#4093 follow-up] Discovery's remedy line must name a package that can
2294+
// actually be installed. It used to be templated from the slot name
2295+
// (`Install a ${slot} plugin to enable`), which named nothing real for
2296+
// `ai` / `search` / `workflow` and got it wrong wherever the package is
2297+
// not called after its slot. Both builders now read one table.
2298+
describe('unavailable slots name a real remedy (#4093 follow-up)', () => {
2299+
beforeEach(() => {
2300+
(kernel as any).getService = vi.fn().mockResolvedValue(null);
2301+
(kernel as any).services = new Map();
2302+
});
2303+
2304+
it('names the actual package, including where the name differs from the slot', async () => {
2305+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2306+
// service-messaging fills `notification` — the case a
2307+
// name-derived guess can never get right.
2308+
expect(info.services.notification.message).toBe('Install @objectstack/service-messaging to enable');
2309+
expect(info.services.auth.message).toBe('Install @objectstack/plugin-auth to enable');
2310+
expect(info.services['file-storage'].message).toBe('Install @objectstack/service-storage to enable');
2311+
});
2312+
2313+
it('says nothing ships rather than naming a package that does not exist', async () => {
2314+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2315+
for (const slot of ['ai', 'search', 'workflow'] as const) {
2316+
expect(info.services[slot].message, `services.${slot}.message`).not.toMatch(/Install/);
2317+
expect(info.services[slot].message, `services.${slot}.message`).toContain(slot);
2318+
}
2319+
});
2320+
2321+
it('never emits the old slot-name-derived template', async () => {
2322+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2323+
for (const [slot, entry] of Object.entries(info.services as Record<string, any>)) {
2324+
if (typeof entry?.message !== 'string') continue;
2325+
expect(entry.message, `services.${slot}.message`).not.toMatch(/Install a .+ plugin to enable/);
2326+
}
2327+
});
2328+
2329+
it('gives the same remedy as the metadata-protocol builder', async () => {
2330+
const { ObjectStackProtocolImplementation } = await import('@objectstack/metadata-protocol');
2331+
const fromProtocol = (await new ObjectStackProtocolImplementation(
2332+
mockObjectQL as any,
2333+
() => new Map<string, any>(),
2334+
).getDiscovery()).services;
2335+
const fromDispatcher = (await dispatcher.getDiscoveryInfo('/api/v1')).services as Record<string, any>;
2336+
2337+
// Every slot both builders report must carry the same remedy —
2338+
// two hosts telling a consumer to install different things is
2339+
// the drift #4089/#4130 closed for `metadata` and `data`.
2340+
for (const slot of Object.keys(fromProtocol)) {
2341+
const mine = fromDispatcher[slot];
2342+
if (!mine || mine.enabled !== false) continue;
2343+
expect(mine.message, `services.${slot}.message parity`).toBe((fromProtocol as any)[slot].message);
2344+
}
2345+
});
2346+
});
2347+
22932348
it('should detect i18n via getServiceAsync (async factory) in discovery', async () => {
22942349
const mockI18nService = {
22952350
getLocales: vi.fn().mockReturnValue(['en', 'fr']),

packages/runtime/src/http-dispatcher.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
} from '@objectstack/core';
66
import { isMcpServerEnabled, looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
77
import { measureServerTiming, allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability';
8-
import { CoreServiceName } from '@objectstack/spec/system';
8+
import { CoreServiceName, serviceUnavailableMessage } from '@objectstack/spec/system';
99
import { readServiceSelfInfo, DispatcherErrorCode } from '@objectstack/spec/api';
1010
import { apiErrorResponse } from './error-envelope.js';
1111
import type { ExecutionContext } from '@objectstack/spec/kernel';
@@ -995,9 +995,16 @@ export class HttpDispatcher {
995995
}
996996
return { enabled: true, status: 'available' as const, handlerReady: true, route, provider };
997997
};
998+
// [#4093 follow-up] The remedy comes from the shared provider table
999+
// (`CORE_SERVICE_PROVIDER`), not from the slot name. `Install a ${name}
1000+
// plugin to enable` named a package that does not exist for `ai`,
1001+
// `search` and `workflow` — nothing implements those slots — and got
1002+
// the name wrong wherever the package is not called after its slot
1003+
// (`notification` is filled by service-messaging). A fix a consumer
1004+
// cannot carry out is worse than no fix named.
9981005
const svcUnavailable = (name: string) => ({
9991006
enabled: false, status: 'unavailable' as const, handlerReady: false,
1000-
message: `Install a ${name} plugin to enable`,
1007+
message: serviceUnavailableMessage(name),
10011008
});
10021009

10031010
// Self-description of the registered realtime service, if any (D12).
@@ -1124,7 +1131,7 @@ export class HttpDispatcher {
11241131
? svcAvailable(routes.ui, 'metadata-protocol', protocolSvc)
11251132
: {
11261133
enabled: false, status: 'unavailable' as const, handlerReady: false,
1127-
message: 'Served by the protocol service — register MetadataPlugin (@objectstack/metadata-protocol) to enable',
1134+
message: serviceUnavailableMessage('ui'),
11281135
},
11291136
workflow: hasWorkflow ? svcAvailable(routes.workflow, undefined, workflowSvc) : svcUnavailable('workflow'),
11301137
// Honest entry (ADR-0076 D12, #2462): the registered realtime

packages/spec/api-surface.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@
701701
"BucketConfig (type)",
702702
"BucketConfigSchema (const)",
703703
"CLOUD_PROVIDED_OBJECT_NAMES (const)",
704+
"CORE_SERVICE_PROVIDER (const)",
704705
"CRDTMergeResult (type)",
705706
"CRDTMergeResultSchema (const)",
706707
"CRDTState (type)",
@@ -1374,6 +1375,7 @@
13741375
"resolveViewDescription (function)",
13751376
"resolveViewLabel (function)",
13761377
"s3StorageExample (const)",
1378+
"serviceUnavailableMessage (function)",
13771379
"toLocaleDescriptors (function)",
13781380
"translateAction (function)",
13791381
"translateApp (function)",

0 commit comments

Comments
 (0)