Skip to content

Commit 8a341a4

Browse files
os-zhuangclaude
andauthored
fix(runtime)!: a mounted domain with no implementation answers 501, not a 404 that blames the route (#4093 follow-up) (#4222)
Two different facts were being answered with whatever each domain happened to reach for, and only `mcp` told them apart: - The route is not there. `/mcp` when the server is disabled for the environment; `/analytics` when the service is unserveable, because dispatcher-plugin gates the MOUNT and never registers those paths (#4000). A path the server does not expose is a 404 from the host's own router. Unchanged — that half was already right. - The route is there; the implementation is not. Every unconditionally mounted domain. The request reached a handler with nothing to delegate to. That is 501, and it is what changes here. `/automation` and `/notifications` returned `{ handled: false }`. That looks neutral, but it is not a fall-through: the dispatcher plugin's single exit turns it into `404 ROUTE_NOT_FOUND` with the hint "No handler matched this request. Check the API discovery endpoint for available routes." Both halves are false. A handler DID match — it just had nothing behind it — and discovery correctly does not list the route, so the hint points at a page that will never mention it. An operator reads that as a routing bug and goes looking for one that does not exist. `/ui` answered 503, which claims the condition is temporary. An uninstalled MetadataPlugin does not become installed by retrying. `/ai` answered 404 for the same mounted-but-unimplemented case. The refusal is written once (`domains/unavailable.ts`) and carries the same remedy sentence discovery reports for the slot, via `serviceUnavailableMessage` from the table #4204 added — so the wall and the discovery entry cannot drift into naming different fixes. `POST /api/v1/automation` on a stack without the service now answers `501 Install @objectstack/service-automation to enable`. `/ai` keeps a local message deliberately: its real provider ships outside this workspace as a Cloud/EE package, so CORE_SERVICE_PROVIDER — verified against workspace packages by check:service-providers — records `null` for that slot and the shared sentence would say "nothing ships", which is wrong. That gap between "no workspace package" and "no package" is worth knowing about; the override keeps this change honest without widening the guard's contract here. Deliberately unchanged: analytics's route-mount gate (a genuinely absent path); mcp's 404-vs-501 pair, which is the model this generalizes; the `handled: false` at the END of each domain, which means "no sub-route matched" and is a true 404; `GET /ai/agents`'s empty-list 200 (a deliberate courtesy for the console's per-navigation poll); and `/ai`'s `503 routes not yet initialized`, a different condition — service present, internal state unready — that may genuinely be transient. This was already the house convention for one surface: the API docs described `/graphql` as "route is wired but bring-your-own service — returns 501 unless an implementation is registered". The rule now holds everywhere, and those docs state it as a rule rather than a GraphQL quirk, including that nothing answers 503 for a missing capability (that status is for transient states like a still-booting kernel on `/ready`). Tests: nine pinned the old answers and are updated to the new one, with the part that actually mattered preserved in each — the automation stub is still asserted NEVER CALLED, the notifications stub's `listInbox` likewise, and `/ai/agents` still returns its empty list. route-parity's integration assertion keeps its invariant (not advertised ⟹ not served, #3369) and now states why 501 expresses it better than the 404 it used to assert. Verified: full `pnpm test` 132/132 tasks; runtime 955; build 71/71; pnpm lint and every check:* gate clean (route-envelope needed the new domains/ file declared as handBuilt: 0 — it answers only through deps.error, which is that check's own thesis applied to the refusal itself). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e4c2dc8 commit 8a341a4

11 files changed

Lines changed: 188 additions & 28 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/runtime': major
3+
---
4+
5+
A dispatcher domain whose route is mounted but whose implementation is absent answers **501**, not a 404 that blames the route (#4093 follow-up).
6+
7+
Two different facts were being answered with whatever each domain happened to reach for, and only `mcp` told them apart:
8+
9+
- **The route is not there.** `/mcp` when the server is disabled for the environment; `/analytics` when the service is unserveable, because `dispatcher-plugin` gates the *mount* and never registers those paths (#4000). A path the server does not expose is a **404** from the host's own router. **Unchanged** — that half was already right.
10+
- **The route is there; the implementation is not.** Every unconditionally-mounted domain. The request reached a handler that had nothing to delegate to. That is **501**, and it is what changes here.
11+
12+
`/automation` and `/notifications` returned `{ handled: false }`, which looks neutral but lands on the dispatcher plugin's single exit: `404 ROUTE_NOT_FOUND` with the hint *"No handler matched this request. Check the API discovery endpoint for available routes."* Both halves were false — a handler did match, and discovery correctly does not list the route, so the hint pointed at a page that would never mention it. An operator reads that as a routing bug and goes looking for one that does not exist.
13+
14+
`/ui` answered **503**, which claims the condition is temporary. An uninstalled MetadataPlugin does not become installed by retrying.
15+
16+
`/ai` answered **404** for the same mounted-but-unimplemented case.
17+
18+
The refusal now carries the same remedy sentence discovery reports for that slot (`serviceUnavailableMessage`, shared via `@objectstack/spec/system`), so the wall and the discovery entry cannot drift into naming different fixes — `POST /api/v1/automation` on a stack without the service answers `501 Install @objectstack/service-automation to enable`. `/ai` keeps a local message: its real provider ships outside this workspace as a Cloud/EE package, so the shared table — verified against workspace packages — records no entry and would otherwise describe it as "nothing ships".
19+
20+
Deliberately unchanged: `analytics`'s route-mount gate (a genuinely absent path); `mcp`'s 404-vs-501 pair, which is the model; the `handled: false` at the END of each domain, which means "no sub-route matched" and is a true 404; `GET /ai/agents`'s empty-list 200, a deliberate courtesy for the console's per-navigation poll; and `/ai`'s `503 routes not yet initialized`, which is a different condition (service present, internal state unready) and may genuinely be transient.
21+
22+
FROM → TO: requests to `/automation`, `/notifications`, `/ui/*` and `/ai/*` on a deployment lacking the backing service now get `501` (with the package to install) instead of `404`/`503`. Anything branching on the old status should branch on `status >= 400` or read the error code; the capability was equally unavailable before, so no working flow changes. Discovery is unaffected — it already reported these slots `unavailable` and advertised no route for them.

content/docs/api/index.mdx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,19 @@ Served by the runtime dispatcher (`@objectstack/runtime`), not `@objectstack/res
142142
**Service Status Values**: `available` (fully operational), `registered` (route declared but handler unverified — may return 501), `degraded` (partial functionality), `unavailable` (not installed), `stub` (placeholder that throws errors)
143143
</Callout>
144144

145+
### What an absent capability answers
146+
147+
Discovery never advertises a route for a service it reports `unavailable`. If you call one anyway, the status tells you **which kind of absence** you hit:
148+
149+
| You get | Meaning | Example |
150+
| :--- | :--- | :--- |
151+
| **404** | The route is not mounted. The server does not expose this path at all. | `/analytics/*` without an analytics service — the mount itself is gated; `/mcp` when the MCP server is disabled for the environment |
152+
| **501** | The route is mounted; nothing implements it. The request reached a handler that had nothing to delegate to. | `/automation`, `/notifications`, `/ui/*`, `/ai/*`, `/auth/*`, `/i18n/*`, `/graphql` without their backing service |
153+
154+
A 501 body names the package that would provide the capability — the same sentence `services.<slot>.message` carries in discovery, so the wall and the discovery entry always agree. A 404 here means what 404 always means: check the path.
155+
156+
Neither is retryable. Nothing answers **503** for a missing capability; that status is reserved for genuinely transient states (the kernel still booting, on `GET /ready`).
157+
145158
---
146159

147160
## Error Handling

packages/runtime/src/domain-handler-registry.test.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,14 +351,18 @@ describe('HttpDispatcher extracted domains (PR-3: keys/storage/ui)', () => {
351351
expect(download).not.toHaveBeenCalled();
352352
});
353353

354-
it('/ui/view/:object serves the protocol getUiView result; 503 without a protocol service', async () => {
354+
it('/ui/view/:object serves the protocol getUiView result; 501 without a protocol service', async () => {
355355
const getUiView = vi.fn().mockResolvedValue({ view: 'list-def' });
356356
const ok = await makeDispatcher({ protocol: { getUiView } }).dispatch('GET', '/ui/view/account/list', undefined, {}, {} as any);
357357
expect(ok.response?.status).toBe(200);
358358
expect(getUiView).toHaveBeenCalledWith({ object: 'account', type: 'list' });
359359

360+
// [#4093 follow-up] Was 503. The route is mounted and the
361+
// implementation is absent — 501. 503 claimed the condition was
362+
// temporary; an uninstalled MetadataPlugin does not install itself.
360363
const missing = await makeDispatcher().dispatch('GET', '/ui/view/account', undefined, {}, {} as any);
361-
expect(missing.response?.status).toBe(503);
364+
expect(missing.response?.status).toBe(501);
365+
expect(missing.response?.body?.error?.message ?? '').toContain('MetadataPlugin');
362366
});
363367
});
364368

@@ -702,9 +706,13 @@ describe('HttpDispatcher extracted domains (PR-7: auth/ai)', () => {
702706
expect(result.response?.body?.agents).toBeUndefined();
703707
});
704708

705-
it('/ai routes 404 (service missing) for non-agents paths', async () => {
709+
// [#4093 follow-up] Was 404. `/ai/*` is mounted unconditionally, so a
710+
// request with no AI service reached a handler that had nothing to
711+
// delegate to — 501. (`GET /ai/agents` keeps its deliberate empty-list
712+
// 200, asserted separately: the console polls it on every navigation.)
713+
it('/ai routes 501 (service missing) for non-agents paths', async () => {
706714
const result = await makeDispatcher().dispatch('POST', '/ai/chat', { q: 'hi' }, {}, {} as any);
707-
expect(result.response?.status).toBe(404);
715+
expect(result.response?.status).toBe(501);
708716
});
709717

710718
it('/ai dispatches to a matching cached kernel route with params + user threading', async () => {

packages/runtime/src/domains/ai.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE,
1515
} from '@objectstack/core';
1616
import { isServiceServeable } from '../service-serveable.js';
17+
import { capabilityUnavailable } from './unavailable.js';
1718
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
1819
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
1920

@@ -66,7 +67,14 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string,
6667
}
6768
// [#3842] Was a hand-rolled envelope with the status in `code`. It has
6869
// no header or shape of its own, so it is simply the shared exit now.
69-
return { handled: true, response: deps.error('AI service is not configured', 404) };
70+
// 501, not 404: `/ai/*` IS mounted, so the request reached a handler
71+
// with nothing behind it — see ./unavailable.ts. The message stays
72+
// local rather than using the shared sentence: the real provider
73+
// (`@objectstack/service-ai`) ships outside this workspace as a
74+
// Cloud/EE package, so CORE_SERVICE_PROVIDER — verified against
75+
// workspace packages by check:service-providers — records `null` for
76+
// this slot and would describe it as "nothing ships", which is wrong.
77+
return capabilityUnavailable(deps, 'ai', 'AI service is not configured');
7078
}
7179

7280
// The AI service exposes route definitions via buildAIRoutes.

packages/runtime/src/domains/automation.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import { CoreServiceName } from '@objectstack/spec/system';
1313
import type { IAutomationService } from '@objectstack/spec/contracts';
1414
import { isServiceServeable } from '../service-serveable.js';
15+
import { capabilityUnavailable } from './unavailable.js';
1516
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
1617
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
1718

@@ -119,8 +120,12 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
119120
// automation capability. This domain is the sharpest case for the rule: a
120121
// stub whose `execute` returns `{ success: true }` without running anything
121122
// answered 200, so a caller (or an agent) read "flow executed" off a flow
122-
// that never ran. 404 handled by caller.
123-
if (!isServiceServeable(automationService)) return { handled: false };
123+
// that never ran.
124+
//
125+
// 501, not the `handled: false` this used to return: `/automation` IS
126+
// mounted, so the dispatcher's ROUTE_NOT_FOUND exit ("No handler matched
127+
// this request") described neither half truthfully. See ./unavailable.ts.
128+
if (!isServiceServeable(automationService)) return capabilityUnavailable(deps, 'automation');
124129

125130
const m = method.toUpperCase();
126131
const parts = path.replace(/^\/+/, '').split('/').filter(Boolean);

packages/runtime/src/domains/notifications.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import { CoreServiceName } from '@objectstack/spec/system';
2323
import type { INotificationService } from '@objectstack/spec/contracts';
2424
import { isServiceServeable } from '../service-serveable.js';
25+
import { capabilityUnavailable } from './unavailable.js';
2526
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
2627
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
2728

@@ -67,8 +68,13 @@ export async function handleNotificationRequest(
6768
// The dev stub implementing exactly `send`/`sendBatch` was never a bug on
6869
// its part: it followed the contract, and the contract was the incomplete
6970
// thing.
71+
//
72+
// 501 rather than the `handled: false` this used to return — `/notifications`
73+
// is mounted, so that produced a ROUTE_NOT_FOUND whose hint ("check the API
74+
// discovery endpoint") pointed at a page that correctly does not list the
75+
// route. See ./unavailable.ts. The slot key is `notification`, singular.
7076
if (!service || !isServiceServeable(service) || typeof service.listInbox !== 'function') {
71-
return { handled: false };
77+
return capabilityUnavailable(deps, 'notification');
7278
}
7379
// Narrowed for the routes below: the entry probe established `listInbox`.
7480
const inbox = service as INotificationService & Required<Pick<INotificationService, 'listInbox'>>;
@@ -105,15 +111,15 @@ export async function handleNotificationRequest(
105111

106112
// POST /notifications/read — mark specific notifications read.
107113
if (subPath === 'read' && m === 'POST') {
108-
if (typeof inbox.markRead !== 'function') return { handled: false };
114+
if (typeof inbox.markRead !== 'function') return capabilityUnavailable(deps, 'notification');
109115
const ids: string[] = Array.isArray(body?.ids) ? body.ids.map((x: unknown) => String(x)) : [];
110116
const result = await inbox.markRead(userId, ids);
111117
return { handled: true, response: deps.success(result) };
112118
}
113119

114120
// POST /notifications/read/all — mark all of the user's inbox read.
115121
if (subPath === 'read/all' && m === 'POST') {
116-
if (typeof inbox.markAllRead !== 'function') return { handled: false };
122+
if (typeof inbox.markAllRead !== 'function') return capabilityUnavailable(deps, 'notification');
117123
const result = await inbox.markAllRead(userId);
118124
return { handled: true, response: deps.success(result) };
119125
}

packages/runtime/src/domains/ui.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
1212
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
13+
import { capabilityUnavailable } from './unavailable.js';
1314

1415
export function createUiDomain(deps: DomainHandlerDeps): DomainRoute {
1516
return {
@@ -44,7 +45,11 @@ export async function handleUiRequest(
4445
return { handled: true, response: deps.errorFromThrown(e, 500) };
4546
}
4647
} else {
47-
return { handled: true, response: deps.error('Protocol service not available', 503) };
48+
// 501, not the 503 this used to answer: 503 claims the condition
49+
// is temporary, but an uninstalled MetadataPlugin does not become
50+
// installed by retrying. The message now names that remedy, and is
51+
// the same sentence discovery reports for the slot (#4146).
52+
return capabilityUnavailable(deps, 'ui');
4853
}
4954
}
5055

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { serviceUnavailableMessage } from '@objectstack/spec/system';
4+
import type { HttpDispatcherResult } from '../http-dispatcher.js';
5+
import type { DomainHandlerDeps } from '../domain-handler-registry.js';
6+
7+
/**
8+
* The one answer a dispatcher domain gives when its route is mounted but
9+
* nothing implements the capability behind it — **501**, carrying the same
10+
* remedy sentence discovery reports for that slot.
11+
*
12+
* ## The distinction this encodes
13+
*
14+
* Two different facts were being answered with whatever each domain happened
15+
* to reach for. They are not the same fact, and `mcp` is the one domain that
16+
* already told them apart:
17+
*
18+
* - **The route is not there.** `/mcp` when the server is disabled for this
19+
* environment; `/analytics` when the service is unserveable, because
20+
* `dispatcher-plugin` gates the *mount* and never registers those paths
21+
* (#4000). Asking for a path the server does not expose is a **404**, and
22+
* the host's own router says so. Nothing here overrides that.
23+
*
24+
* - **The route is there; the implementation is not.** Every domain mounted
25+
* unconditionally. The request reached a handler — it simply has nothing to
26+
* delegate to. That is **501 Not Implemented**, and it is what this helper
27+
* is for.
28+
*
29+
* ## What it replaces
30+
*
31+
* `return { handled: false }` looked like a neutral "not mine", but the
32+
* dispatcher plugin's single exit turns it into `404 ROUTE_NOT_FOUND` with the
33+
* hint *"No handler matched this request. Check the API discovery endpoint for
34+
* available routes."* Both halves are false: a handler did match, and
35+
* discovery — correctly — does not list the route, so the hint sends the
36+
* caller to a page that will not mention it. An operator reads that as a
37+
* routing bug and goes looking for one that does not exist.
38+
*
39+
* `/ui` said **503**, which claims the condition is temporary. An uninstalled
40+
* MetadataPlugin does not become installed by retrying.
41+
*
42+
* ## Why the message comes from `spec`
43+
*
44+
* `serviceUnavailableMessage` is the same sentence `services.<slot>.message`
45+
* carries in discovery (#4093 follow-up), so the 501 body and the discovery
46+
* entry cannot drift into naming different remedies — and a caller who hits
47+
* the wall gets the fix without a second round trip.
48+
*
49+
* @param slot the `CoreServiceName` key, NOT the route segment — `/notifications`
50+
* is served by the `notification` slot, and the remedy is looked up
51+
* by slot.
52+
*/
53+
export function capabilityUnavailable(
54+
deps: DomainHandlerDeps,
55+
slot: string,
56+
/**
57+
* Overrides the shared sentence. Only for slots whose provider cannot be
58+
* named by `CORE_SERVICE_PROVIDER` — it is verified against workspace
59+
* packages, so a real provider that ships outside this repo (`ai`) has no
60+
* entry there and would otherwise be described as "nothing ships".
61+
*/
62+
message?: string,
63+
): HttpDispatcherResult {
64+
return { handled: true, response: deps.error(message ?? serviceUnavailableMessage(slot), 501) };
65+
}

0 commit comments

Comments
 (0)