Skip to content

Commit 9d6a39d

Browse files
yiliang114qwencoder
yiliang114
and
qwencoder
committed
fix(core): exempt media-policy targets from the bridge argument pre-check
resolveDeferredToolCall now pre-validates bridged arguments against the target schema (#12889), but omni media-policy targets do not hold their final arguments at that point: both frontends run evaluateMediaPolicyToolCall AFTER bridge resolution (coreToolScheduler before buildInvocation, ACP Session.runTool), and that gate is what resolves resourceId to inputPath and merges defaultArguments/lockedArguments, while BaseMediaPolicyTool's validateToolParams deliberately checks the NATIVE schema rather than the model-visible projection tool_search returned. So the pre-check refused calls the very next stage accepted. A bridged {resourceId, outputDir} call — the shape omni/media-guidance.ts tells the model to send — was refused with "provide exactly one of inputPath ... or resourceId" even though resourceId was provided, and for path-less media the model can never learn the real path, so it cannot repair the call. An operator-pinned modelAccess.lockedArguments.outputDir was unwinnable both ways: projectMediaPolicyToolDeclaration strips the locked key from the model-visible properties AND required, so omitting it failed the pre-check while sending it failed the gate. Each refusal fed recordBatchRetryableToolError and gained RETRY_LOOP_STOP_DIRECTIVE, ending the turn in LOOP_DETECTED / invalid_tool_params_stagnation on a call shape that previously worked. All 14 omniPolicyToolFactories tools inherit the first trigger; the 13 with a native required outputDir also reach the second. Key the exemption off mediaPolicyDescriptor: the code-level fact the gate itself keys off (it passes every non-policy tool through untouched), so the exemption covers exactly the targets whose arguments a downstream stage completes. Nothing fails open — the gate still emits named invalid_params refusals before build(), and build() re-validates the merged arguments against the native schema. Acceptance test added to the existing #12889 block: a target whose model-visible schema omits a key its validateToolParams requires (native required: ['inputPath','outputDir'], projected required: []) resolves with projection-satisfying arguments instead of being refused. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmul3pj9gc4
1 parent 1979183 commit 9d6a39d

2 files changed

Lines changed: 110 additions & 9 deletions

File tree

‎packages/core/src/tools/tool-call.test.ts‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ import {
1212
deferredDeclarationFingerprint,
1313
type ToolRegistry,
1414
} from './tool-registry.js';
15-
import type { AnyDeclarativeTool } from './tools.js';
15+
import type { AnyDeclarativeTool, MediaPolicyToolDescriptor } from './tools.js';
1616
import {
1717
DEFERRED_TOOL_CALL_REFUSAL_PREFIX,
1818
resolveDeferredToolCall,
1919
ToolCallTool,
2020
} from './tool-call.js';
21+
import { SchemaValidator } from '../utils/schemaValidator.js';
2122
import { ToolErrorType } from './tool-error.js';
2223
import { ToolNames } from './tool-names.js';
2324
import { DEFAULT_MAX_SUBAGENT_DEPTH } from '../config/config.js';
@@ -802,5 +803,85 @@ describe('ToolCallTool', () => {
802803

803804
expect(result).toMatchObject({ arguments: { count: '3' } });
804805
});
806+
807+
it('resolves a media-policy target whose arguments the policy gate completes', async () => {
808+
// The projection split a media-policy tool creates: `schema` is the
809+
// model-visible declaration (an operator `modelAccess.lockedArguments`
810+
// key stripped from BOTH properties and required), while
811+
// `validateToolParams` keeps checking the NATIVE schema
812+
// (omni/policy/tools/media-policy-tool.ts). The model is therefore
813+
// correct to omit `outputDir`, and the modelAccess gate — which both
814+
// frontends run AFTER bridge resolution — merges it back in. Running the
815+
// pre-check on these raw arguments refuses a call the next stage accepts,
816+
// and sending the locked key instead makes the gate refuse it: unwinnable
817+
// both ways. Mutation check: dropping the media-policy exemption in
818+
// resolveDeferredToolCall turns this red.
819+
const nativeSchema = {
820+
type: 'object',
821+
properties: {
822+
inputPath: { type: 'string' },
823+
outputDir: { type: 'string' },
824+
},
825+
required: ['inputPath', 'outputDir'],
826+
additionalProperties: false,
827+
};
828+
const projectedSchema = {
829+
type: 'object',
830+
properties: { inputPath: { type: 'string' } },
831+
required: [],
832+
additionalProperties: false,
833+
};
834+
835+
class MockLockedMediaPolicyTool extends MockTool {
836+
override get mediaPolicyDescriptor(): MediaPolicyToolDescriptor {
837+
return {
838+
kind: 'media_policy',
839+
inputMediaTypes: ['audio'],
840+
outputs: [{ kind: 'media', required: true }],
841+
};
842+
}
843+
844+
override get schema() {
845+
return {
846+
name: this.name,
847+
description: this.description,
848+
parametersJsonSchema: projectedSchema,
849+
};
850+
}
851+
852+
override validateToolParams(params: {
853+
[key: string]: unknown;
854+
}): string | null {
855+
return SchemaValidator.validate(nativeSchema, params);
856+
}
857+
}
858+
859+
const target = new MockLockedMediaPolicyTool({
860+
name: 'omni_transcribe_audio',
861+
shouldDefer: true,
862+
params: nativeSchema,
863+
});
864+
// The mock really carries the split the defect needs: the model-visible
865+
// schema omits `outputDir`, native validation still requires it.
866+
expect(target.schema.parametersJsonSchema).toEqual(projectedSchema);
867+
expect(target.validateToolParams({ inputPath: '/tmp/in.wav' })).toContain(
868+
"'outputDir'",
869+
);
870+
871+
const result = await resolveDeferredToolCall(
872+
makeRegistry([target], new Set([target.name])),
873+
{
874+
name: 'omni_transcribe_audio',
875+
arguments: { inputPath: '/tmp/in.wav' },
876+
},
877+
);
878+
879+
expect(result).not.toHaveProperty('error');
880+
expect(result).not.toHaveProperty('errorType');
881+
expect(result).toMatchObject({
882+
tool: expect.objectContaining({ name: 'omni_transcribe_audio' }),
883+
arguments: { inputPath: '/tmp/in.wav' },
884+
});
885+
});
805886
});
806887
});

‎packages/core/src/tools/tool-call.ts‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,15 +239,35 @@ export async function resolveDeferredToolCall(
239239
// unwrapped (#12889). Validate a clone: SchemaValidator coerces values in
240240
// place, and the scheduler re-validates the returned arguments at build
241241
// time.
242+
//
243+
// Omni media-policy targets are exempt: their arguments are not final here.
244+
// Both frontends run the modelAccess gate AFTER bridge resolution
245+
// (coreToolScheduler's `evaluateMediaPolicyToolCall`, before buildInvocation;
246+
// ACP Session.runTool), and that gate resolves `resourceId` → `inputPath`
247+
// and merges `defaultArguments`/`lockedArguments`. Their `validateToolParams`
248+
// deliberately checks the NATIVE schema rather than the model-visible
249+
// projection `tool_search` returned, so pre-checking the raw bridged
250+
// arguments refuses calls the very next stage accepts — and for an operator
251+
// locked key the refusal is unwinnable both ways (omitting it fails here,
252+
// sending it fails the gate). `mediaPolicyDescriptor` is the code-level fact
253+
// the gate itself keys off (it passes every non-policy tool through
254+
// untouched), so the exemption covers exactly the tools whose arguments a
255+
// downstream stage completes. Nothing fails open: the gate still emits named
256+
// `invalid_params` refusals, and build() re-validates the merged arguments.
257+
const argsCompletedByPolicyGate =
258+
(target as { mediaPolicyDescriptor?: unknown }).mediaPolicyDescriptor !==
259+
undefined;
242260
let paramsError: string | null = null;
243-
try {
244-
paramsError = target.validateToolParams(
245-
structuredClone(invocation.params.arguments),
246-
);
247-
} catch {
248-
// A target whose validation throws under this pre-check must not become
249-
// a new bridge failure mode: the scheduler's build() reports the same
250-
// throw as before.
261+
if (!argsCompletedByPolicyGate) {
262+
try {
263+
paramsError = target.validateToolParams(
264+
structuredClone(invocation.params.arguments),
265+
);
266+
} catch {
267+
// A target whose validation throws under this pre-check must not become
268+
// a new bridge failure mode: the scheduler's build() reports the same
269+
// throw as before.
270+
}
251271
}
252272
if (paramsError) {
253273
return {

0 commit comments

Comments
 (0)