You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
core: the deferred tool_call bridge enforces a declaration-schema layer that 8 tool families never enforce themselves #12999
resolveDeferredToolCall (the deferred tool_call bridge added by PR #12901) pre-validates bridged arguments against the target's model-visible declaration schema, target.schema.parametersJsonSchema.
For any tool that does not override BaseDeclarativeTool.validateToolParams, that pre-check duplicates build()-time validation exactly — packages/core/src/tools/tools.ts:510-513 runs SchemaValidator.validate(this.schema.parametersJsonSchema, params) and nothing else before validateToolParamValues. So the pre-check is a no-op difference for those tools.
But eight tool families override validateToolParams and never run that schema layer at all — no super.validateToolParams call and no SchemaValidator.validate call:
file
line
packages/core/src/tools/enter-worktree.ts
275
packages/core/src/tools/skill.ts
257
packages/core/src/tools/agent/agent.ts
1113
packages/core/src/tools/exitPlanMode.ts
466
packages/core/src/tools/exit-worktree.ts
611
packages/core/src/tools/team-plan-approval.ts
153
packages/core/src/tools/askUserQuestion.ts
341
packages/core/src/tools/todoWrite.ts
677
For those eight, the bridge becomes the first and only place the declaration schema is enforced. A bridged call can therefore be refused for a parameter the target itself accepts, which is exactly the outcome the pre-check's own rationale comment says to avoid (tool-call.ts: "running it here would refuse calls the very next stage accepts").
Reproduce with:
grep -rn "override validateToolParams" packages/*/src --include=*.ts | grep -v "\.test\.ts"
# then, for each hit, check whether the body calls super.validateToolParams or SchemaValidator.validate
Concrete failure: bridged agent fork refused in a non-team session
Observed as an A/B against the merge base during review of PR #12901 (head 4c7ebf961c), identical input on both arms:
BASE: declaration has `name` property : false
BASE: description tells model to pass : true
BASE: tool.validateToolParams(args) : null
BASE: bridged call -> RESOLVED -> agent
PR : declaration has `name` property : false
PR : description tells model to pass : true
PR : validateToolParams(args) : null
PR : bridged call -> REFUSED [invalid_tool_params]: ... params must NOT have additional properties.
The mechanism, all in packages/core/src/tools/agent/agent.ts:
AgentTool's parameter schema is additionalProperties: false (:873).
updateDescriptionAndSchema() deletes properties.name whenever isAgentTeamEnabled() is false (:1091-1095).
The same method builds the ## When to fork instruction "Pass a short `name` (one or two words, lowercase) so the user can track the fork." unconditionally (:1056), while a sibling name sentence in the same method is gated on teamEnabled (:997-1002, with a comment recording that sending it ungated was itself a prior bug).
AgentTool.validateToolParams (:1113+) has no super call and no SchemaValidator call, so it tolerates name; execution merely logs "Ignoring teammate name ... because no team is active" (:2568-2571), and the comment above it states this tolerance is deliberate ("prompts may still pass it without a team").
So in a non-team session where settings.tools.eager demotes agent to deferred+hidden (making the bridge its only invocation path), the model reads a description telling it to pass name beside a schema that has no name property, bridges {description, prompt, subagent_type:'fork', name:'docs'}, and the call is refused. Because an additionalProperties refusal names no key, the model has nothing to remove and no way to repair the call; each refusal also accrues a strike toward VALIDATION_RETRY_LOOP_THRESHOLD. The identical direct call succeeds.
The description/schema contradiction in step 3 exists on both arms, so the divergence is introduced by the pre-check alone.
Two candidate shapes were reviewed there and both are design decisions rather than mechanical fixes:
An opt-out hook on BaseDeclarativeTool (default "my declaration is enforced", overridden where validateToolParams bypasses the schema layer), consulted by the bridge. This extends the tool base-class API, and for AgentTool it also gives back the target-named refusal that issue Deferred tool_call schema allows empty arguments for tools with required fields #12889 asked for.
The deeper problem is the one review entry R3-11 on that PR named: the bridge has no way to learn a target's real contract except by enumerating families at the pre-check site. Every future target that completes, projects or relaxes its arguments between resolution and build() needs a new exemption there, discovered by whoever ships the breakage. Two review rounds produced three such cases (omni media-policy's lockedArguments projection, truncation-completed arguments, and AgentTool's unenforced declaration), and the third shipped as a fails-closed regression.
Proposed direction
Consolidate at the build path, as the PR author proposed during round 3: let the stage that owns the target's contract produce the refusal, and let a target declare its own enforcement rather than having the pre-check model it per family. Whichever shape is chosen, the acceptance bar is:
a bridged call must not be refused for a parameter the target's own validateToolParams accepts;
Deferred tool_call schema allows empty arguments for tools with required fields #12889's behaviour must be preserved — an empty arguments object that misses required target fields is still refused at the bridge with a message naming the target and the missing field (pinned by refuses an empty arguments object that misses required target fields in packages/core/src/tools/tool-call.test.ts);
the pre-check must stay at the schema layer only, leaving value-level rules to build() (pinned by pre-checks only the schema layer, leaving value-level rules to build(), which asserts expect(valueRuleSpy).not.toHaveBeenCalled()).
Source
Review threads R4-1 (Critical, fails-closed regression) and R3-11 (Suggestion, re-asserted after a round-3 decline) on PR #12901. Filed so the contract-ownership mechanism has a home outside that PR's scope; that PR keeps the schema-layer pre-check as shipped.
Summary
resolveDeferredToolCall(the deferredtool_callbridge added by PR #12901) pre-validates bridgedargumentsagainst the target's model-visible declaration schema,target.schema.parametersJsonSchema.For any tool that does not override
BaseDeclarativeTool.validateToolParams, that pre-check duplicatesbuild()-time validation exactly —packages/core/src/tools/tools.ts:510-513runsSchemaValidator.validate(this.schema.parametersJsonSchema, params)and nothing else beforevalidateToolParamValues. So the pre-check is a no-op difference for those tools.But eight tool families override
validateToolParamsand never run that schema layer at all — nosuper.validateToolParamscall and noSchemaValidator.validatecall:packages/core/src/tools/enter-worktree.tspackages/core/src/tools/skill.tspackages/core/src/tools/agent/agent.tspackages/core/src/tools/exitPlanMode.tspackages/core/src/tools/exit-worktree.tspackages/core/src/tools/team-plan-approval.tspackages/core/src/tools/askUserQuestion.tspackages/core/src/tools/todoWrite.tsFor those eight, the bridge becomes the first and only place the declaration schema is enforced. A bridged call can therefore be refused for a parameter the target itself accepts, which is exactly the outcome the pre-check's own rationale comment says to avoid (
tool-call.ts: "running it here would refuse calls the very next stage accepts").Reproduce with:
Concrete failure: bridged
agentfork refused in a non-team sessionObserved as an A/B against the merge base during review of PR #12901 (head
4c7ebf961c), identical input on both arms:The mechanism, all in
packages/core/src/tools/agent/agent.ts:AgentTool's parameter schema isadditionalProperties: false(:873).updateDescriptionAndSchema()deletesproperties.namewheneverisAgentTeamEnabled()is false (:1091-1095).## When to forkinstruction "Pass a short `name` (one or two words, lowercase) so the user can track the fork." unconditionally (:1056), while a siblingnamesentence in the same method is gated onteamEnabled(:997-1002, with a comment recording that sending it ungated was itself a prior bug).AgentTool.validateToolParams(:1113+) has nosupercall and noSchemaValidatorcall, so it toleratesname; execution merely logs "Ignoring teammate name ... because no team is active" (:2568-2571), and the comment above it states this tolerance is deliberate ("prompts may still pass it without a team").So in a non-team session where
settings.tools.eagerdemotesagentto deferred+hidden (making the bridge its only invocation path), the model reads a description telling it to passnamebeside a schema that has nonameproperty, bridges{description, prompt, subagent_type:'fork', name:'docs'}, and the call is refused. Because anadditionalPropertiesrefusal names no key, the model has nothing to remove and no way to repair the call; each refusal also accrues a strike towardVALIDATION_RETRY_LOOP_THRESHOLD. The identical direct call succeeds.The description/schema contradiction in step 3 exists on both arms, so the divergence is introduced by the pre-check alone.
Why this is not fixed inside PR #12901
Two candidate shapes were reviewed there and both are design decisions rather than mechanical fixes:
BaseDeclarativeTool(default "my declaration is enforced", overridden wherevalidateToolParamsbypasses the schema layer), consulted by the bridge. This extends the tool base-class API, and forAgentToolit also gives back the target-named refusal that issue Deferredtool_callschema allows empty arguments for tools with required fields #12889 asked for.namesentence onteamEnabled. This collides with two pinned contracts —packages/core/src/skills/bundled/agent-delegation/SKILL.test.ts:215(the perf(core): the built-in tool descriptions and schemas are the largest block of non-conversation context, and have no size tracking #12054 description-vs-skill split, which asserts that anchor stays in the tool description) andpackages/core/src/tools/agent/agent.test.ts:982— and with the deliberate execution-time tolerance atagent.ts:2560-2571.The deeper problem is the one review entry R3-11 on that PR named: the bridge has no way to learn a target's real contract except by enumerating families at the pre-check site. Every future target that completes, projects or relaxes its arguments between resolution and
build()needs a new exemption there, discovered by whoever ships the breakage. Two review rounds produced three such cases (omni media-policy'slockedArgumentsprojection, truncation-completed arguments, andAgentTool's unenforced declaration), and the third shipped as a fails-closed regression.Proposed direction
Consolidate at the build path, as the PR author proposed during round 3: let the stage that owns the target's contract produce the refusal, and let a target declare its own enforcement rather than having the pre-check model it per family. Whichever shape is chosen, the acceptance bar is:
validateToolParamsaccepts;tool_callschema allows empty arguments for tools with required fields #12889's behaviour must be preserved — an emptyargumentsobject that misses required target fields is still refused at the bridge with a message naming the target and the missing field (pinned byrefuses an empty arguments object that misses required target fieldsinpackages/core/src/tools/tool-call.test.ts);build()(pinned bypre-checks only the schema layer, leaving value-level rules to build(), which assertsexpect(valueRuleSpy).not.toHaveBeenCalled()).Source
Review threads
R4-1(Critical, fails-closed regression) andR3-11(Suggestion, re-asserted after a round-3 decline) on PR #12901. Filed so the contract-ownership mechanism has a home outside that PR's scope; that PR keeps the schema-layer pre-check as shipped.