Uh oh!
There was an error while loading. Please reload this page.
fix(mcp): scope the tool bridge's openWorldHint: false to platform-registered names - #13485
Conversation
…tered names `registerToolFromDefinition` asserted `openWorldHint: false` in every bridged tool's annotations, from no declared source — over every tool an app registers under its own name as well as the platform's. `AIToolDefinition` has no member expressing the hint, so the `false` was a property of this file served to every MCP client as a property of the tool: an app tool reaching a weather API, an LLM or any outbound service was announced as closed-world. The hint is now derived in `worldAnnotation()` from `PLATFORM_PROVIDED_TOOL_NAMES` (`@objectstack/spec/system`) — the same registry the bridge's `readOnlyHint` name fallback is already pinned to as a subset, so the two hints read one registry between them. Platform names keep the known-correct `false`; every other bridged tool is served no `openWorldHint` key at all. The asymmetry with the safety hints is deliberate and written down at the derivation site: SDK 1.30.0 documents `openWorldHint` as `Default: true`, so omission here is the honest direction rather than the conservative one, unlike `readOnlyHint` (`Default: false`) and `destructiveHint` (`Default: true`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
…enworldhint-platform-scope
📓 Docs Drift Check5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f60eb27d5f2f4a36b1f8fd0b67dffc823825ce49 && git checkout f60eb27d5f2f4a36b1f8fd0b67dffc823825ce49
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9cfc1f7e935aa62e46ec9a03ecd49194fd024754 7280f39e836ea9d0472f15c77169a5ace4fe5ffc && git checkout -B drift-repro 9cfc1f7e935aa62e46ec9a03ecd49194fd024754 && git merge --no-ff 7280f39e836ea9d0472f15c77169a5ace4fe5ffc
node scripts/docs-audit/affected-docs.mjs --json 9cfc1f7e935aa62e46ec9a03ecd49194fd024754 |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13350
Implements the ruled repair: option 2 now, option 1 as follow-up (maintainer, 2026-08-30 12:21:51Z, 第 5 場總監席決裁批 #1, verbatim「第一批其餘同意」). The bridge asserts
openWorldHint: falsefor platform-registered names only; everything else — app-registered tools included — omits the annotation entirely. ⛔packages/spec/**is untouched: no member added toAIToolDefinition, no clause ② surface moved.Measured at
7280f39e83— the commit this branch carries and the commit every verdict below was produced at.The defect
registerToolFromDefinitioninpackages/mcp/src/mcp-server-runtime.tsput a bareopenWorldHint: falsein every bridged tool'sannotations, from no source at all.AIToolDefinitionhas no member expressing the hint, so thefalsewas a property of that file served to every MCP client as a property of the tool. An app tool reaching a weather API, an LLM or any outbound service was announced as having a closed, well-defined domain of interaction.The mechanism reused, and where it came from
The crux was to find how the
readOnlyHintfallback already identifies platform-owned names in this same function and reuse it, rather than mint a fresh allowlist. Measured onorigin/main:Both are membership tests against hand-held subsets of one canonical registry, and the file says so at line 63: "Every name here is a tool the cloud AI runtime registers statically (
PLATFORM_TOOLS_BY_PACKAGEin@objectstack/spec/system)". The pin that holds them there ismcp-tool-bridge-safety-annotations.test.ts, which already importsPLATFORM_PROVIDED_TOOL_NAMESfrom@objectstack/spec/systemand asserts "no tool outsidePLATFORM_PROVIDED_TOOL_NAMESreceives a hint it did not declare".So the mechanism exists and it can carry this hint. The new
worldAnnotation()is the same shape, reading the registry the safety lists are pinned to:Why the registry and not the two local safety sets. They answer different questions. The safety sets classify a safety class and are deliberately partial — 6 names of the registry's 30. Ownership is the question the world hint needs, and keying it off the safety sets would have dropped
create_object,add_field,list_metadata,describe_metadataand twenty more — platform tools whose world is just as closed — to the protocol default. That is option 3's accuracy loss under a narrower name, and option 3 was rejected precisely for it. Registry size measured live at 30, against the same specifier the test resolves.⛔ Not the dynamic families.
PLATFORM_TOOL_FAMILY_PREFIXESmaterialisesaction_NAMEwrappers around an app's own declarative actions — the platform registers the wrapper, the app defines the behaviour.mcp-http-tools.tsassertsopenWorldHint: trueforrun_actionon exactly that reasoning. A pin drivesaction_close_dealand asserts it receives no key, so widening the membership test to the prefixes goes red.The trap, written into the code
Verified in the pinned SDK rather than taken on faith —
@modelcontextprotocol/sdk1.30.0,ToolAnnotationsSchemaindist/esm/types.js:readOnlyHintDefault: falsedestructiveHintDefault: trueopenWorldHintDefault: trueSo the two functions are structurally one rule (assert what the platform can source, omit what it cannot, because MCP has no spelling for "unknown" other than absence) while the price of the omission differs. That asymmetry is stated at the derivation site and again in the test file header, explicitly as a thing not to tidy — because a reader who makes the three hints behave alike reintroduces the defect, and the reader who tries has to turn four pins red first.
Positive controls
All in
packages/mcp/src/mcp-tool-bridge-safety-annotations.test.ts, driven through a realStdioServerTransportand read from an actualtools/listanswer — what a desktop MCP host receives, not the in-process JS object.false—query_records,list_objects, pluscreate_object,list_metadata,describe_metadata, the three that a safety-set-keyed fallback would have silently dropped.check_weather,ask_llm,delete_opportunityasserted with own-property presence on the parsed wire object, nottoBeUndefined(), which a spread ofopenWorldHint: undefinedwould also satisfy. Each such case carries a same-object positive control:query_recordsis listed in the same call and its own-property check must answer true, so afalseis a reading rather than a misspelled key.tools/list; the set carrying the hint must equal the registry exactly, and non-vacuity is asserted first (sizegreater than 0, and the carrier count equal to it) becausetoEqualbetween two empty arrays passes.delete_opportunitydeclaringrequiresConfirmation: truestill getsdestructiveHint: trueand no world hint: declaring confirmation buys the safety hints and buys nothing about the world.pnpm --filter @objectstack/mcp test→Test Files 25 passed (25)·Tests 269 passed (269).The card's own re-check, and one place I read it differently
git grep -n openWorldHint -- 'packages/**'on this branch: 11 hits at themcp-http-tools.tsregistration sites (unchanged, includingrun_action's deliberatetrue), 2 inmcp-server-runtime.ts, 20 in the pin file.Reverse verification (ablation)
Predicted before running: replacing the platform-scoping predicate with the unconditional
return { openWorldHint: false };restores the pre-fix behaviour, so the four absence cases go red while the three platform controls and the four safety-only cases stay green — an asymmetric prediction, because "everything goes red" would not show that the pins discriminate the app branch.No rebuild is performed on purpose: the pin imports the bridge by a relative specifier, so vitest resolves TypeScript source. Had it been
dist-resolved, an un-rebuilt ablation would have stayed green — so the red is itself the proof that the mutated source is what ran.Prediction matched exactly: 4 red / 7 green, and the red set is precisely the four absence cases. The script restores with
git checkout HEAD -- ABSOLUTE_PATHundertrap … EXIT INT TERM, and judges by observed bytes and blob hashes, never by an exit code; an empty hash is treated as failure, not as "nothing to compare".Gate union
Derived after merging
origin/main. Both known defects in the derivation script were read around rather than trusted:The behind-count is the reading; the banner is not. It came back
0, and no STALE TREE banner appeared — the two agree, which is what the two known derivation defects each break in one direction.GREEN — 29 path-matched plus the convention-triggered families:
check:changeset-gate-self-tests·check:cross-package-test-inputs·check:doc-authoring·check:logger-receiver-detach·check:objectql-double-limit·check:objectui-changeset·check:page-declaration-shape·check:pm-half-states·check:published-files·check:slot-lookup·check:test-source-alias·check:type-source-resolution·check:where-matcher— all exit 0check-adr-0087-registration·check-changeset-no-major·check-ci-filter-parity·check-comment-mask-adoption·check-cross-package-test-inputs·check-empty-changeset·check-keyed-text-bounds·check-plugin-teardown-shape·check-shard-attestation·check-undeclared-dep-imports·docs-audit/check-affected-docs·docs-audit/check-drift-comment·pm/release-rehearsal-clone --self-test— all exit 0check:query-options-erasure·check:type-check-coverage·check:engine-double-contract·check:dispatcher-error-vocabulary— all exit 0check:nul-bytes—check-nul-bytes: OK (scanned 7448 text file(s) … no raw ASCII control bytes).check:dual-build-cjs-loads— refused on the unbuilt worktree, then green after building the closure:✓ check:dual-build-cjs-loads — 102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parse; 1 cross-format behaviour probe(s) agree.check:type-check-debt—check-type-check-coverage --re-measure: OK — 29 ledger entr(ies) re-measured in 442.5s, 1547 raw tsc error(s) total, none above its recorded number./surplus: none — every entry sits exactly at its measurement, so any new error is red.REFUSALS — recorded separately, NOT folded into the green list.
NOT MEASUREDis not a pass:node scripts/check-test-completeness.mjs—PREREQUISITE NOT MET — this gate grades a saved 'turbo run test' log, and no log was named.Exit 3. The gate's own text says the family list names it with no argument and that the local reading is NOT MEASURED. CI tees the log and passes the path, so CI is unaffected.node scripts/pm/check-half-states.mjs—PREREQUISITE NOT MET, exit 3: the container'sGITHUB_TOKENis the proxy placeholder andGET /rate_limitanswers 401 with it.Nothing was swept: no issue was listed, no predicate (H1–H16) ran. Not a clean board and not a dirty one.⭐ The ratchet nearly caught something, and this is why the debt entry matters. The first draft of the pins used
Object.hasOwn, which runs fine on Node 22 and passed both the targeted run and the full suite. It is ES2022; this repo compiles atlib: ["ES2020"].pnpm --filter @objectstack/mcp typecheckcould not see it either —packages/mcp/tsconfig.jsonexcludes test files, andtsc --listFilesconfirmed the edited test file has 0 hits in that program whilemcp-server-runtime.tshas 1. Measured explicitly with a throwaway tests-including tsconfig: 6 errors attributable to the new pins, on top of 53 pre-existing (reverse control: 14 inmcp-action-tools.test.ts, so the grep can find errors that are there). The TEST_DEBT ledger records@objectstack/mcpat exactlyerrors: 53, frozen — so shipping it would have put the count at 59 and turned the ratchet red in the merge queue, which that entry's own note says has happened to it before. Replaced with an ES2020-safe own-property helper; the tests-including typecheck now reports 0 errors in the edited file and the ratchet is green. The temp tsconfig was deleted andgit status --porcelainverified clean afterwards.Repo-wide
pnpm lintwas not run locally — it is a repo-scale scan CI owns and runs exactly once regardless. Declared, not silently skipped.Changeset judgement, by measurement
.changeset/mcp-openworldhint-platform-scope.md,@objectstack/mcp: patch. Judged by building the artifact and grepping it, not by analogy:The runtime artifact moves.
packages/mcp/dist/index.jsafterpnpm --filter @objectstack/mcp buildcarries the derivation verbatim —— plus a new
from "@objectstack/spec/system"import edge (@objectstack/specwas already a runtime dependency, andmcp-http-tools.ts/stdio-data-bridge.tsalready take value imports from sibling spec subpaths, so no dependency is added). What every consumer's MCP clients receive changes.The type artifact does not.
dist/index.d.tscontainsopenWorldHintandworldAnnotationonce each, both inside the JSDoc of a private method; grepping for a declared or exportedworldAnnotationreturns 0. Positive control on the same file so the zero is a reading:MCPServerRuntime7 hits,bridgeTools2 hits.⇒ behaviour of a published package moves, no exported declaration moves ⇒ patch, and a changeset is owed because it is user-visible. The changeset states the consumer-facing consequence plainly: a host that keyed off a bridged app tool's
openWorldHint: falsewill now find the annotation absent and fall to the protocol'strue.Follow-up, deliberately not done here
Option 1 — a declared member on
AIToolDefinition, with action-backed tools inheritingrun_action'sopenWorldHint: true— is the shape that makes the hint true rather than defensible. It is apackages/spec/**public contract extension (clause ②) and was ruled a follow-up. Nothing here anticipates it.One observation for whoever picks it up: this repair asserts
falseacross all 30 registry names, which is the ruled predicate, but a handful of them (propose_blueprint,suggest_builder) are AI-runtime tools where "closed domain of interaction" is a judgement about the domain rather than about the implementation. A per-tool declared member is where that gets settled individually; a name-membership test cannot.Generated by Claude Code