fix(core): close the additionalProperties relaxation at the $ref boun…

…dary The name-keyed exemption made composition non-lexical: $defs and definitions were walked by value, so a $ref-expressed oneOf had its branches relaxed inside the shared definition — an argument matching exactly one published branch then matched two in the pre-check's clone and the bridge refused a call the target's own schema accepts. $defs and definitions now stay byte-identical (they are reachable only through $ref, and each use site is covered directly). Two measured corrections ride along: allOf/anyOf/then/else come out of the verbatim list (relaxing under them only widens acceptance — measured 0 newly-rejected against oneOf's 1 and not's 8), and draft-07 dependencies joins the name-keyed walk so a constraint literally named additionalProperties keeps its own value instead of being inverted into always-pass. Three discriminating tests: the same tagged union written with $defs/$ref resolves and the target's own build() accepts it; allOf branches relax; a false-valued dependency named additionalProperties still refuses. Each is red under its revert (re-walking $defs, re-listing allOf, dropping dependencies from the name-keyed walk). Co-authored-by: Qwen-Coder <[email protected]>
yiliang114 · Oct 6, 2026 · Sep 28, 2026 · Sep 28, 2026 · Sep 28, 2026 · Sep 28, 2026
commit df8c325776e2e9f3fd6e1d1085913e0d23431b6f
@@ -937,6 +937,112 @@ describe('ToolCallTool', () => {
       });
     });
 
+    it('resolves the same tagged union written with $defs/$ref', async () => {
+      // The $ref form is what generated schemas actually use. Composition is
+      // non-lexical through it, so the relaxation must not descend into the
+      // shared definitions: flipping a branch's additionalProperties there
+      // makes {a,b} match both and oneOf (exactly one) fails. Mutation check:
+      // re-adding $defs to the name-keyed walk turns this red with "must match
+      // exactly one schema in oneOf".
+      const target = new MockTool({
+        name: 'mcp__srv__refunion',
+        shouldDefer: true,
+        params: {
+          type: 'object',
+          $defs: {
+            A: {
+              properties: { a: { type: 'string' } },
+              required: ['a'],
+              additionalProperties: false,
+            },
+            B: {
+              properties: { a: { type: 'string' }, b: { type: 'string' } },
+              required: ['a'],
+              additionalProperties: false,
+            },
+          },
+          oneOf: [{ $ref: '#/$defs/A' }, { $ref: '#/$defs/B' }],
+        },
+      });
+      const result = await resolveDeferredToolCall(
+        makeRegistry([target], new Set([target.name])),
+        { name: target.name, arguments: { a: 'x', b: 'y' } },
+      );
+
+      expect(result).not.toHaveProperty('error');
+      expect(result).toMatchObject({ arguments: { a: 'x', b: 'y' } });
+      if ('tool' in result) {
+        // The target's own validator (Ajv on the unmodified schema) accepts.
+        expect(() => result.tool.build(result.arguments)).not.toThrow();
+      }
+    });
+
+    it('relaxes additionalProperties inside allOf branches, which only widens', async () => {
+      // allOf branches do not discriminate — both must hold — so a per-branch
+      // additionalProperties: false is not load-bearing the way oneOf's is.
+      // args {a,b} fail each strict branch (each forbids the other key) and
+      // pass both relaxed ones. Mutation check: re-listing allOf as verbatim
+      // turns this red with a refusal.
+      class LenientTool extends MockTool {
+        override validateToolParams(): string | null {
+          return null;
+        }
+      }
+      const target = new LenientTool({
+        name: 'allof_target',
+        shouldDefer: true,
+        params: {
+          type: 'object',
+          allOf: [
+            {
+              properties: { a: { type: 'string' } },
+              required: ['a'],
+              additionalProperties: false,
+            },
+            {
+              properties: { b: { type: 'string' } },
+              required: ['b'],
+              additionalProperties: false,
+            },
+          ],
+        },
+      });
+      const result = await resolveDeferredToolCall(
+        makeRegistry([target], new Set([target.name])),
+        { name: target.name, arguments: { a: 'x', b: 'y' } },
+      );
+
+      expect(result).not.toHaveProperty('error');
+      expect(result).toMatchObject({ arguments: { a: 'x', b: 'y' } });
+    });
+
+    it('never reads a constraint literally named additionalProperties as the keyword', async () => {
+      // `dependencies` maps a NAME to a constraint; a dependency named
+      // additionalProperties with a false schema forbids that property, and
+      // flipping the false to true would invert it into always-pass — the
+      // pre-check would then resolve a call the target's own validator
+      // refuses. The constraint must stay false after the walk.
+      const target = new MockTool({
+        name: 'dep_target',
+        shouldDefer: true,
+        params: {
+          type: 'object',
+          properties: { mode: { type: 'string' } },
+          dependencies: { additionalProperties: false },
+        },
+      });
+      const result = await resolveDeferredToolCall(
+        makeRegistry([target], new Set([target.name])),
+        { name: target.name, arguments: { additionalProperties: 'x' } },
+      );
+
+      expect(result).toMatchObject({
+        errorType: ToolErrorType.INVALID_TOOL_PARAMS,
+        targetName: 'dep_target',
+      });
+      expect(result).not.toHaveProperty('tool');
+    });
+
     it('still attributes wrong field types when surplus keys are present', async () => {
       const target = makeWebFetchLike();
       const result = await resolveDeferredToolCall(

@@ -89,34 +89,37 @@ function bridgeRefusal(message: string): Error {
 
 /**
  * Schema keywords whose subtree the relaxation below must leave byte-identical.
- * Composition branches (`oneOf`/`anyOf`/`allOf`/`if`/`then`/`else`/`not`) use a
- * per-branch `additionalProperties: false` to tell the branches apart, so
- * relaxing it there inverts the schema's meaning instead of widening acceptance.
- * Annotation keywords hold data the schema compares against, not a subschema.
+ * `oneOf`/`not` discriminate: a per-branch `additionalProperties: false` tells
+ * branches apart, so relaxing it there inverts the schema's meaning instead of
+ * widening acceptance (`if` selects a branch by the same mechanism). Annotation
+ * keywords hold data the schema compares against, not a subschema. `$defs` and
+ * `definitions` are reached only through `$ref`: they are shared definitions,
+ * and relaxing inside one silently rewrites every branch that references it —
+ * each use site is already covered directly by the walk above.
+ * (`allOf`/`anyOf`/`then`/`else` are deliberately absent: relaxing under them
+ * only widens acceptance, so the walk descends.)
  */
 const VERBATIM_SCHEMA_KEYS: ReadonlySet<string> = new Set([
-  'allOf',
-  'anyOf',
   'oneOf',
   'not',
   'if',
-  'then',
-  'else',
   'const',
   'default',
   'enum',
   'example',
   'examples',
+  '$defs',
+  'definitions',
 ]);
 
 /**
- * Schema keywords whose value maps an arbitrary NAME to a subschema. The names
- * are data, so a property literally named `additionalProperties` keeps its own
- * schema rather than being read as the keyword: these are walked by value only.
+ * Schema keywords whose value maps an arbitrary NAME to a subschema or
+ * constraint. The names are data, so a property literally named
+ * `additionalProperties` keeps its own schema rather than being read as the
+ * keyword: these are walked by value only.
  */
 const NAME_TO_SCHEMA_KEYS: ReadonlySet<string> = new Set([
-  '$defs',
-  'definitions',
+  'dependencies',
   'dependentSchemas',
   'patternProperties',
   'properties',