fix(mcp): scope the tool bridge's openWorldHint: false to platform-registered names

.changeset/mcp-openworldhint-platform-scope.md

@@ -0,0 +1,44 @@
+---
+"@objectstack/mcp": patch
+---
+
+fix(mcp): assert `openWorldHint: false` only for platform-registered tool names (#13350)
+
+The MCP tool bridge (`registerToolFromDefinition` in `mcp-server-runtime.ts`)
+put `openWorldHint: false` in every bridged tool's `annotations` — for every
+tool an app registers under its own name as well as the platform's own. No
+source existed for that claim: `AIToolDefinition` has no member expressing it,
+so the `false` was a property of the bridge file presented to every MCP client
+as a property of the tool. An app tool that calls a weather API, an LLM or any
+other outbound service was announced as having a closed, well-defined domain of
+interaction. Same defect class as the `readOnlyHint` / `destructiveHint` repair
+that preceded it.
+
+The hint is now derived from `PLATFORM_PROVIDED_TOOL_NAMES`
+(`@objectstack/spec/system`) — the canonical registry of the statically named
+tools the cloud AI runtime registers, and the same registry the bridge's
+existing `readOnlyHint` name fallback is pinned to as a subset. A platform name
+keeps `openWorldHint: false`, which is known-correct: those tools act on this
+stack's own records and metadata. Every other bridged tool is served **no
+`openWorldHint` key at all**.
+
+⚠️ **What omission means here, and why it differs from the sibling hints.**
+`@modelcontextprotocol/sdk` 1.30.0 documents `openWorldHint` as `Default: true`
+(`ToolAnnotationsSchema`), so an app tool that declares nothing is now read by
+a conforming host as reaching an **open** world — where before it was told the
+world was closed. That is the intended direction: the bridge has no source for
+an app tool, and the protocol's own default is a better answer than a
+fabricated one. It is the opposite of the `readOnlyHint` (`Default: false`) and
+`destructiveHint` (`Default: true`) cases, where omission lands on the cautious
+reading; the asymmetry is documented at the derivation site so it is not
+"tidied" back into the defect.
+
+Hosts that keyed behaviour off a bridged app tool's `openWorldHint: false` will
+now see the annotation absent. The platform tools' `false` is unchanged, and
+the object-CRUD and action bridges in `mcp-http-tools.ts` — including
+`run_action`'s deliberate `openWorldHint: true` — are untouched.
+
+Making the hint a property of the tool (a declared member on
+`AIToolDefinition`, with action-backed tools inheriting `run_action`'s
+`openWorldHint: true`) is a public contract extension and was ruled a
+follow-up; this is the zero-contract-change half.

packages/mcp/src/mcp-server-runtime.ts

@@ -5,6 +5,7 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
 import { WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js';
 import type { Logger, IMetadataService, AIToolDefinition } from '@objectstack/spec/contracts';
 import type { Agent } from '@objectstack/spec/ai';
+import { PLATFORM_PROVIDED_TOOL_NAMES } from '@objectstack/spec/system';
 import type { ToolRegistry, ToolExecutionResult } from './types.js';
 import { wireBridgeTools } from './mcp-http-tools.js';
 import type {
@@ -184,6 +185,96 @@ function safetyAnnotations(tool: AIToolDefinition): ToolSafetyHints {
  return {};
 }
 
+/** The world-domain hint this bridge can source. */
+interface ToolWorldHint {
+ openWorldHint?: boolean;
+}
+
+/**
+ * The `openWorldHint` this bridge can actually SOURCE — for the names the
+ * PLATFORM registers, and for nobody else.
+ *
+ * THE DEFECT. This hint used to be a bare `openWorldHint: false` sitting in
+ * the annotations literal below, asserted over EVERY bridged tool from
+ * nothing at all — every tool an app registers under its own name included.
+ * `AIToolDefinition` has no member expressing it, so that `false` was never a
+ * property of the tool; it was a property of this file. An app can register a
+ * tool that calls a weather API, an LLM, or any outbound service, and this
+ * bridge told every MCP client its domain of interaction was closed. Same
+ * class as the `readOnlyHint` / `destructiveHint` defect described above.
+ *
+ * THE SOURCE is {@link PLATFORM_PROVIDED_TOOL_NAMES}
+ * (`@objectstack/spec/system`) — the canonical registry of the statically
+ * named tools the cloud AI runtime registers (`PLATFORM_TOOLS_BY_PACKAGE`).
+ * Every name in it acts on the ObjectStack environment itself, this stack's
+ * records and its own metadata, which is a closed and well-defined domain in
+ * exactly the sense the SDK gives the word. So `false` for those names is
+ * known rather than assumed, and it is the information option 3 (drop the
+ * hint outright) would have thrown away.
+ *
+ * WHY THE REGISTRY AND NOT THE TWO NAME SETS ABOVE. Same shape — a membership
+ * test against platform-registered names — but a different question.
+ * {@link PLATFORM_READ_ONLY_TOOL_NAMES} and
+ * {@link PLATFORM_DESTRUCTIVE_TOOL_NAMES} answer "what SAFETY class does the
+ * platform know for this name", and they are deliberately partial: they carry
+ * only the platform names whose safety class this bridge knows. The question
+ * here is OWNERSHIP, and the registry is what answers it. Keying the world
+ * hint off the safety lists instead would drop `create_object`, `add_field`,
+ * `list_metadata`, `describe_metadata` and twenty more — platform tools whose
+ * world is just as closed — to the protocol default, reintroducing option 3's
+ * accuracy loss under a narrower name. A sibling pin already holds the two
+ * safety lists to this same registry as a SUBSET, so the hints read one
+ * registry between them rather than three hand lists that can drift apart.
+ *
+ * ⚠️ WHY OMISSION IS NOT THE CONSERVATIVE DIRECTION HERE — AND WHY THE
+ * ASYMMETRY WITH {@link safetyAnnotations} IS DELIBERATE, NOT AN OVERSIGHT
+ * WAITING TO BE TIDIED. Structurally the two functions are one rule: assert
+ * what the platform can source, omit what it cannot, because MCP has no
+ * spelling for "unknown" other than absence. What DIFFERS is the price of
+ * that absence, and the difference is the SDK's own. Measured in the pinned
+ * `@modelcontextprotocol/sdk` 1.30.0 (`ToolAnnotationsSchema`, `dist/esm/types.js`):
+ *
+ * ```
+ * readOnlyHint Default: false ← omission reads "not read-only"
+ * destructiveHint Default: true ← omission reads "may be destructive"
+ * openWorldHint Default: true ← omission reads "OPEN world"
+ * ```
+ *
+ * For the two safety hints, omission lands on the cautious answer and costs
+ * only information — which is why #13318 could move them to omit-when-unsourced
+ * and call it conservative. For this one, omission lands on the LESS cautious
+ * reading: a tool that sources nothing is understood by every conforming host
+ * to reach an open world. That trade is accepted on purpose. For an
+ * app-registered tool this bridge genuinely has no source, and falling to the
+ * protocol's documented default is honest where asserting `false` was a lie.
+ * ⛔ So do not "repair" the asymmetry by re-asserting `false` for everyone:
+ * that IS the defect. The identical STRUCTURE of the two functions is what
+ * keeps the one rule legible; identical consequences were never the point.
+ *
+ * ⛔ NOT the dynamic tool families. `PLATFORM_TOOL_FAMILY_PREFIXES`
+ * (`action_<name>`) names tools the runtime materialises from an app's OWN
+ * declarative actions: the platform registers the wrapper, the app defines the
+ * behaviour, outbound calls included. `mcp-http-tools.ts` asserts
+ * `openWorldHint: true` for `run_action` on exactly that reasoning. Widening
+ * this membership test to the prefixes would re-import the defect under a new
+ * name.
+ *
+ * ⛔ NOT `{ openWorldHint: undefined }`. The no-source answer is an absent
+ * KEY, which is why this returns `{}`: an undefined-valued property is dropped
+ * by JSON serialization but survives a spread, so "omitted" has to be true of
+ * the object as well as of the wire.
+ *
+ * THE FOLLOW-UP THIS IS NOT. Making the hint a property of the TOOL — a
+ * declared member on `AIToolDefinition`, with an action-backed tool inheriting
+ * `run_action`'s `openWorldHint: true` — is the shape that would make it true
+ * rather than merely defensible. That is a public contract extension in
+ * `packages/spec/**` and was ruled a follow-up; this is the zero-contract-change
+ * half, which stops the unsourced assertion now.
+ */
+function worldAnnotation(tool: AIToolDefinition): ToolWorldHint {
+ return PLATFORM_PROVIDED_TOOL_NAMES.has(tool.name) ? { openWorldHint: false } : {};
+}
+
 // ── AIToolDefinition.parameters → MCP inputSchema ────────────────────────────
 
 /**
@@ -992,8 +1083,11 @@ export class MCPServerRuntime {
  * The safety annotations come from {@link safetyAnnotations}, which reads
  * what the definition DECLARES and omits the hints it cannot source; the
  * name-derived `readOnlyHint: false, destructiveHint: false` this call used
- * to assert over every unlisted tool is gone. `openWorldHint` is untouched
- * by that change and still asserted for every bridged tool.
+ * to assert over every unlisted tool is gone. `openWorldHint` now goes
+ * through {@link worldAnnotation} on the same principle — asserted `false`
+ * for platform-registered names, omitted for everyone else — but read that
+ * function before assuming the two omissions cost the same thing: the SDK
+ * defaults them in opposite directions.
  */
  private registerToolFromDefinition(tool: AIToolDefinition, toolRegistry: ToolRegistry): void {
  const logger = this.config.logger;
@@ -1004,10 +1098,12 @@ export class MCPServerRuntime {
  description: tool.description,
  inputSchema: toolInputSchema(tool, logger),
  annotations: {
- // Only the hints {@link safetyAnnotations} can source — a tool that
- // declares nothing is served neither, so the MCP defaults apply.
+ // Only the hints these two can source — a tool that declares nothing
+ // and is not a platform name is served neither set, so the MCP
+ // defaults apply. Those defaults are NOT symmetrical between them;
+ // {@link worldAnnotation} is where that is written down.
  ...safetyAnnotations(tool),
-openWorldHint: false,
+...worldAnnotation(tool),
  },
  },
  async (args) => {

packages/mcp/src/mcp-tool-bridge-safety-annotations.test.ts

@@ -36,6 +36,26 @@
  * omission, and the SDK's own `ToolAnnotationsSchema` documents the defaults
  * that then apply (`readOnlyHint` false, `destructiveHint` **true**), which is
  * the conservative reading the old `false` inverted.
+ *
+ * THE SECOND DEFECT, PINNED HERE TOO. `openWorldHint: false` was asserted for
+ * every bridged tool from no source at all — the sibling of the above, one
+ * hint later. It is now sourced the same way the `readOnlyHint` fallback is,
+ * from platform-registered names (`PLATFORM_PROVIDED_TOOL_NAMES`), and
+ * omitted for everyone else.
+ *
+ * ⚠️ AND ITS OMISSION IS PINNED DIFFERENTLY, ON PURPOSE. The same SDK schema
+ * documents `openWorldHint` as **`Default: true`**, so an omitted world hint
+ * is read as an OPEN world — the one place in this file where absence is not
+ * the cautious answer, only the honest one. A future reader tempted to make
+ * the three hints behave alike has to make these pins red first, which is the
+ * point of stating it here as well as at the call site.
+ *
+ * ⚠️ ABSENCE MEANS AN ABSENT KEY, and these cases say so with
+ * `Object.hasOwn`, not with `toBeUndefined()`. A spread of
+ * `{ openWorldHint: undefined }` would satisfy `toBeUndefined()` while still
+ * putting the property on the object; every such case below carries a
+ * same-object positive control (a platform tool listed in the same call) so a
+ * `false` from {@link hasHint} is a reading rather than a typo'd key name.
  */
 
 import { describe, it, expect, afterEach } from 'vitest';
@@ -155,6 +175,19 @@ async function annotationsOf(
  return { session, byName: Object.fromEntries(listed.map((t: any) => [t.name, t])) };
 }
 
+/**
+ * Own-property presence on the PARSED WIRE object.
+ *
+ * ⚠️ Not `toBeUndefined()`, which a spread of `{ openWorldHint: undefined }`
+ * would also satisfy while still putting the property on the object — the
+ * distinction these cases exist to make. ⛔ And not `Object`.`hasOwn` either:
+ * that is ES2022 and this repo compiles at `lib: ["ES2020"]`, so it type-errors
+ * where the runtime (Node 22) would have run it happily — a gap this package's
+ * own `typecheck` cannot report, because its tsconfig excludes test files.
+ */
+const hasHint = (annotations: Record<string, unknown> | undefined, hint: string): boolean =>
+ Object.prototype.hasOwnProperty.call(annotations ?? {}, hint);
+
 const tool = (name: string, extra: Partial<AIToolDefinition> = {}): AIToolDefinition => ({
  name,
  description: `the ${name} tool`,
@@ -215,11 +248,13 @@ describe('bridgeTools — the safety annotations a client receives', () => {
  const s = await annotationsOf([tool('send_invoice_email')]);
  openSession = s.session;
 
- const annotations = s.byName.send_invoice_email.annotations;
+ const annotations = s.byName.send_invoice_email.annotations ?? {};
  expect(annotations.destructiveHint).toBeUndefined();
  expect(annotations.readOnlyHint).toBeUndefined();
- // The hint the bridge does still assert for every tool, unchanged here.
- expect(annotations.openWorldHint).toBe(false);
+ // ...and no world hint either. `send_invoice_email` is the case in the
+ // name: an app tool that reaches an outbound service was being told to
+ // every client as closed-world.
+ expect(hasHint(annotations, 'openWorldHint')).toBe(false);
  });
 
  it('what the definition declares outranks what its name suggests', async () => {
@@ -237,6 +272,9 @@ describe('bridgeTools — the safety annotations a client receives', () => {
  expect(PLATFORM_PROVIDED_TOOL_NAMES.has('aggregate_records')).toBe(false);
  expect(s.byName.aggregate_records.annotations.readOnlyHint).toBeUndefined();
  expect(s.byName.aggregate_records.annotations.destructiveHint).toBeUndefined();
+ // Nor a world hint from this bridge. It gets one at its OWN registration
+ // site in `mcp-http-tools.ts`, which is where that fact is known.
+ expect(hasHint(s.byName.aggregate_records.annotations, 'openWorldHint')).toBe(false);
  });
 
  /**
@@ -274,4 +312,95 @@ describe('bridgeTools — the safety annotations a client receives', () => {
  expect(s.byName[stranger.name].annotations.destructiveHint).toBeUndefined();
  }
  });
+
+ // ── openWorldHint ────────────────────────────────────────────────────────
+
+ it('CONTROL: a platform-registered name still receives `openWorldHint: false`', async () => {
+ const s = await annotationsOf([
+ tool('query_records'),
+ tool('list_objects'),
+ // Platform names OUTSIDE the two safety-class sets. These are the tools
+ // a fallback keyed on those sets instead of the registry would have
+ // silently flipped to the protocol's open-world default.
+ tool('create_object'),
+ tool('list_metadata'),
+ tool('describe_metadata'),
+ ]);
+ openSession = s.session;
+
+ for (const name of ['query_records', 'list_objects', 'create_object', 'list_metadata', 'describe_metadata']) {
+ expect(PLATFORM_PROVIDED_TOOL_NAMES.has(name)).toBe(true);
+ expect(s.byName[name].annotations.openWorldHint).toBe(false);
+ }
+ // The safety hints are sourced separately: `create_object` is a platform
+ // name in NEITHER safety set, so it keeps the world hint and no other.
+ expect(s.byName.create_object.annotations.readOnlyHint).toBeUndefined();
+ expect(s.byName.create_object.annotations.destructiveHint).toBeUndefined();
+ });
+
+ it('an app-registered tool receives NO `openWorldHint` KEY — absence, not `undefined`', async () => {
+ const s = await annotationsOf([
+ tool('check_weather'),
+ tool('ask_llm', { requiresConfirmation: false }),
+ tool('delete_opportunity', { requiresConfirmation: true }),
+ // Positive control in the same `tools/list` answer: `hasOwn` must be
+ // able to say `true` about this exact wire object, or the `false`s
+ // below would be indistinguishable from a misspelled key.
+ tool('query_records'),
+ ]);
+ openSession = s.session;
+
+ expect(hasHint(s.byName.query_records.annotations, 'openWorldHint')).toBe(true);
+
+ for (const name of ['check_weather', 'ask_llm', 'delete_opportunity']) {
+ const annotations = s.byName[name].annotations ?? {};
+ expect(hasHint(annotations, 'openWorldHint')).toBe(false);
+ expect(annotations.openWorldHint).toBeUndefined();
+ }
+
+ // Independently sourced: declaring `requiresConfirmation` buys the SAFETY
+ // hints and buys nothing about the world, which is the whole point of the
+ // two derivations being separate.
+ expect(s.byName.delete_opportunity.annotations.destructiveHint).toBe(true);
+ expect(s.byName.ask_llm.annotations.destructiveHint).toBe(false);
+ });
+
+ /**
+ * The world-hint counterpart of the fallback invariant above, driven across
+ * the whole registry at once: `openWorldHint: false` is a claim the platform
+ * can source about the tools it registers, and about nothing else.
+ */
+ it('exactly the platform-registered names carry `openWorldHint`, and every one of them carries `false`', async () => {
+ const platform = [...PLATFORM_PROVIDED_TOOL_NAMES].map((name) => tool(name));
+ const strangers = [
+ 'aggregate_records',
+ 'action_close_deal',
+ 'check_weather',
+ 'send_invoice_email',
+ 'void_invoice',
+ ].map((name) => tool(name));
+
+ const s = await annotationsOf([...platform, ...strangers]);
+ openSession = s.session;
+
+ const withWorldHint = Object.values(s.byName)
+ .filter((t: any) => hasHint(t.annotations, 'openWorldHint'))
+ .map((t: any) => t.name)
+ .sort();
+
+ // ⚠️ Non-vacuity first: `toEqual` between two empty arrays passes, so an
+ // unbuilt or empty registry would make every assertion below say nothing.
+ expect(PLATFORM_PROVIDED_TOOL_NAMES.size).toBeGreaterThan(0);
+ expect(withWorldHint.length).toBe(PLATFORM_PROVIDED_TOOL_NAMES.size);
+ expect(withWorldHint).toEqual([...PLATFORM_PROVIDED_TOOL_NAMES].sort());
+ for (const name of withWorldHint) {
+ expect(s.byName[name].annotations.openWorldHint).toBe(false);
+ }
+
+ // `action_close_deal` is the family case stated explicitly: the runtime
+ // materialises `action_<name>` wrappers around an app's OWN actions, so a
+ // membership test widened to `PLATFORM_TOOL_FAMILY_PREFIXES` would put
+ // this bridge back to claiming a closed world over app-defined behaviour.
+ expect(hasHint(s.byName.action_close_deal.annotations, 'openWorldHint')).toBe(false);
+ });
 });