Skip to content

Commit 1abccdb

Browse files
yiliang114qwencoderyiliang114
authored
fix(core): stop misdiagnosing malformed tool-call args as max_tokens truncation (#12982)
* fix(core): stop misdiagnosing malformed tool-call args as max_tokens truncation When a provider streams a fused/malformed tool-call argument bag, the streaming parser flags the JSON as incomplete (brace depth > 0) and the OpenAI converter unconditionally rewrites finish_reason to "length", which turn.ts maps to wasOutputTruncated and the scheduler then appends the max_tokens truncation note to the schema-validation error. The note is asserted with no evidence: neither the provider's real finish_reason nor usageMetadata is consulted, so a response that ended at 185 completion tokens is reported as token-limit truncation and the model retries the identical call. Guard the finish_reason override with the usage evidence: the pipeline records the wire output budget (max_tokens or a provider-specific budget key) on the request context, and the converter skips the override when reported completion tokens fall well below that budget. Genuine truncation (completion tokens at the cap, or usage/ceiling unavailable) keeps the legacy inference, preserving the #4964 recovery path, and an explicit provider-reported "length" is still trusted verbatim. Refs #12970 Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-issue-patrol/jmum5kp9p2z * style(core): apply prettier formatting * fix(core): settle the truncation override where delayed usage lands Two defects in the #12970 corroboration guard, both reported on the review thread at head 8b7848a: R1-1 — the guard read `chunk.usage` off the chunk carrying `finish_reason`, but this pipeline requests `stream_options.include_usage` (pipeline.ts:1079) and under that convention the finish chunk reports no usage: the totals arrive on a later `choices: []` chunk that `handleChunkMerging` folds into the parked finish response. For that whole provider class the verdict was always "inconclusive", the `stop` -> `length` rewrite fired exactly as before, and the misdiagnosis this PR exists to remove still shipped. The converter now parks the provider's own finish reason on the request context, and the pipeline settles the rewrite on the parked response — before every one of its three delivery points (post-merge yield, Stage 2d flush, error-path flush), so the consumer only ever observes the settled reason. The park is reset when a stream starts, because a retry can reuse the same context object. R1-2 — only `undefined` counted as "no evidence", so a zero-filled usage object (ModelScope sends exactly that on the finish chunk) or an explicit `null` fell through to the ratio comparison and was read as proof *against* truncation. That suppressed the #4964 override and, since `wasOutputTruncated` is the sole key for the scheduler's reject-file-writes-while-truncated guard, disarmed it on the responses it exists for — a truncated `write_file` repaired by jsonrepair could then write a partial file. A missing, non-numeric or non-positive count is now inconclusive rather than a disproof; it cannot mean "no output" here, because this branch is only reached once the parser found incomplete JSON. The corroboration helper becomes an exported three-state verdict so the converter (chunk usage) and the pipeline (merged usageMetadata) share one threshold and one notion of "no evidence". Tests: 6 new converter cases — zero-filled usage, null completion_tokens, the 50% threshold on both sides, and park / no-park. 252/252 pass. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmumj5dw0er * test(core): pin the wire-budget capture and the parked-override settlement Co-authored-by: Qwen-Coder <[email protected]> R1-7: nothing proved the pipeline *sets* RequestContext.maxOutputTokens — all the corroboration tests injected it themselves through the converter harness, and getWireOutputBudget has exactly one caller. Three cases now assert the context handed to the converter carries the budget as max_tokens, as either provider key (max_completion_tokens and max_new_tokens, since the clamp path treats both as budget keys), or as undefined when the request caps by neither. R1-1: the reference stream shape — argument chunks, a finish_reason chunk with no usage, then a trailing choices:[] chunk carrying the totals — now has an end-to-end case asserting the yielded finish response reads STOP, plus the two invariants that keep it honest: trailing usage at the cap still yields MAX_TOKENS (#4964), and no trailing usage at all still yields MAX_TOKENS rather than clearing the inference on a guess. The converter module is stubbed wholesale in this file, so the mock mirrors the parking contract that converter.test.ts pins on the real implementation, and the mock factory now re-exports the real verdict helper the pipeline settles with. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmumj5dw0er * test(core): access the wire max_tokens key via index signature tsc --noEmit flags TS4111 on dot access of an index-signature property; the captured wire request is typed Record<string, unknown>. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmumj5dw0er * fix(core): keep the incomplete-file-write guard when the max_tokens diagnosis is withdrawn `wasOutputTruncated` is derived solely from `finishReason === MAX_TOKENS` (turn.ts), and it feeds two consumers with different needs: - the user-visible note appended to a validation failure, and the max_tokens recovery loop — both of which must NOT fire when the response's own usage disproves a token-limit cut (#12970); - the scheduler's rejection of file-modifying calls, whose real precondition is that the arguments arrived incomplete, whatever cut them off. Suppressing the `stop` -> `length` override on a disproof fixed the first and silently disarmed the second. A model emitting `write_file` and stopping partway through `content` at low usage — the same malformed-generation class #12970 reports, on a `Kind.Edit` tool — kept `finish_reason: 'stop'`, so `getCompletedToolCalls()` repaired the unterminated string and brace into schema-valid arguments and the call executed, overwriting the target with half its intended content. The merge base rejected it ahead of `buildInvocation`. Carry the fact separately from the diagnosis: a non-enumerable symbol on the `FunctionCall` (the `PROVIDER_TOOL_CALL_ID` precedent in `toolCallIdUtils.ts`), set at both points where the token-limit diagnosis is withdrawn — the converter's immediate `disproved` branch and the pipeline's delayed settle — surfaced as `hadIncompleteArguments`, and honoured by the guard alongside `wasOutputTruncated`. The guard now keys on what it always should have. The refusal wording follows the cause, which is the second half of #12970's expected behavior 1: a malformed-generation rejection says so and asks for one tool call per turn with schema-valid parameters, rather than blaming max_tokens and advising the model to split content it never ran out of room for. Its error type is `invalid_tool_params`, matching the issue report, not `output_truncated`. Also log both rewrite directions with the two numbers they were decided from. The heuristic has now been wrong in both directions once (#4964 missed a real cut, #12970 invented one), so the next field report should not have to re-derive the wire budget from a request capture. Regression pins: the guard still rejects on `hadIncompleteArguments` alone, with the cause-matched wording and error type; both marking points are asserted on the paths production actually takes. All three were mutation-checked — reverting the guard condition makes the write execute (`expected 'success' to be 'error'`), and removing either marking point fails its own assertion. Co-authored-by: Qwen-Coder <[email protected]> * test(core): keep the real truncation verdict in the omni-cache converter mock `pipeline.omniCacheInvalidation.test.ts` stubbed `./converter.js` with a plain factory, so `corroborateTruncationFromCompletionTokens` was absent from the module namespace. It passes today only because that suite's converter stub never parks an override, so `settleParkedTruncationOverride` returns at `if (!parked)` before touching the missing export. The first parked-override case added there would fail with a mock-plumbing error naming the mock rather than the missing wiring, which reads as a converter bug. Keep the pure verdict real, as `pipeline.test.ts` already does. Co-authored-by: Qwen-Coder <[email protected]> * test(core): drive the real converter and the real pipeline through one stream The park/settle handshake has two ends that each own half of one contract: the converter suspects a rewrite and parks the provider's own reason, the pipeline settles it once the delayed usage totals land. `converter.test.ts` drives the real converter against a hand-written expectation of what the pipeline will do; `pipeline.test.ts` drives the real pipeline against a stubbed converter that hand-writes the park. Either side can stay green while the seam between them breaks — which is how the R1-1 guard shipped inert. These two cases run both ends for real, on the reference protocol's stream shape rather than the convenient one: under `stream_options.include_usage` the chunk carrying `finish_reason` reports no usage, and the totals arrive on a later `choices: []` chunk. They pin both outcomes of the same fused `write_file` stream — 185 of 8192 withdraws the max_tokens diagnosis while keeping the incomplete-write guard armed, 8192 of 8192 keeps the override so the #4964 recovery still fires. Mutation-checked: removing the settle-path marking fails the disproof case and leaves the corroboration case green. Co-authored-by: Qwen-Coder <[email protected]> * fix(core,cli): carry the incomplete-arguments marker to every consumer R2-1: normalizeModelToolCallIds rebuilds each FunctionCall with an object spread, which copies enumerable own properties only, so the non-enumerable incomplete-arguments marker was dropped at the llm-chat hop and never reached turn.ts. Both scheduler consumers of hadIncompleteArguments were therefore unreachable. Re-attach it the same way PROVIDER_TOOL_CALL_ID already is. R3-1: the subagent runtime builds its own ToolCallRequestInfo and derived wasOutputTruncated solely from finishReason === MAX_TOKENS, so the data-loss guard stayed disarmed for every subagent even with the marker preserved. Mirror turn.ts and read the marker there too. R3-2: the hosted-workspace tool turn refused incomplete-argument calls on wasOutputTruncated alone and executes them itself rather than through CoreToolScheduler. That profile commits to a remote workspace with no undo backup, so it has to consult the new field as well. R3-6: drop the pendingTruncationOverride reset at the top of processStreamWithLogging. No RequestContext can carry a stale override into it - the streaming executor returns a lazy generator, so an executeAttempt retry can only be entered before the generator body runs - and the comment above it described a retry model the code does not have. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmuo2vk3phn * test(core): pin the incomplete-args marker producers and fix the parked-override contract Round-4 review triage (#12970): - turn.test.ts: pin the main-session producer of hadIncompleteArguments — a marked call under a STOP finish must surface the flag on the emitted ToolCallRequest (with wasOutputTruncated untouched), and an unmarked control call must surface without it. Deleting the producer spread or making it unconditional now reddens the suite. - agent-core.test.ts: extend the subagent marker test with an unmarked sibling call so over-application of the marker is detectable. - coreToolScheduler.test.ts: pin INCOMPLETE_ARGS_PARAM_GUIDANCE with a non-Edit tool failing schema validation — the branch the Edit-guard tests can never reach — asserting the guidance names malformed generation and not a max_tokens cut. - types.ts: the stream-start clear of pendingTruncationOverride was deliberately removed earlier in this PR; restate the field contract to the guarantee the code actually provides (lazy generator + fresh RequestContext per attempt make a stale park unreachable). Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmuodld0708 * style(core): let Prettier join the wrapped assertion in turn.test.ts The previous commit left one expectation in turn.test.ts wrapped narrower than Prettier 3.6.1 wants it, so Lint & Static failed at its Run Prettier step. Reformatted with the pinned Prettier; formatting only, no behaviour change. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-conflict/jmuod8i2ti5 * fix(core): use an incomplete-args retry loop directive for pre-validation Edit rejections The incomplete-args arm of the Edit guard rejects before buildInvocation, so schema validation never runs on those calls. Attaching the validation retry-loop directive there misdiagnosed the cause ('failed validation ... re-examine the tool schema') and would drive the wrong recovery. Add a third directive for repeated incomplete file writes, keep the directive appended after the rejectionMessage counter key is recorded, and cover the branch with a threshold test that goes red if the arm falls back to either existing directive. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmuofqisz0a * test(core): rebuild the truncation witnesses on the compressed helpers main's #13007 test-suite compression reshaped two helpers these witnesses depend on, and this branch was merged from a pre-compression main, so `tsc --build` broke in the CI prepare step: src/core/coreToolScheduler.test.ts(7207,7): error TS2554: Expected 1 arguments, but got 2. src/core/coreToolScheduler.test.ts(7266,7): error TS2554: Expected 1 arguments, but got 2. src/core/coreToolScheduler.test.ts(7391,7): error TS2554: Expected 1 arguments, but got 2. src/core/turn.test.ts(951,27): error TS2304: Cannot find name 'GenerateContentResponse'. (and three more GenerateContentResponse sites in turn.test.ts) `createTruncationTestScheduler` now takes only the tool and derives `getAllToolNames` from `tool.name`, which equals the explicit name array each call site passed, so dropping the second argument preserves the registry shape the witness asserts against. `turn.test.ts` kept casting stream chunks to `GenerateContentResponse` after the compression dropped that type-only import; restore it alongside the other `@google/genai` types. `tsc --build` on packages/core is now clean. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-conflict/jmup4109ajg * test(core): pin the per-call marker quantifier and the truncated errorType arm R5-1: the truncated arm of the scheduler's errorType ternary had no test naming OUTPUT_TRUNCATED, so collapsing the ternary to INVALID_TOOL_PARAMS left the suite green. The shared truncation-rejection helper now asserts errorType, and the collapse mutant turns the truncation cases red. R5-2: the main-session producer test carried a single-call response, so a response-wide read of the incomplete-arguments marker was indistinguishable from the per-call read. The positive case now carries a marked call plus a clean sibling and asserts both directions; hoisting the read turns it red. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: yiliang114 <[email protected]> Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: yiliang114 <[email protected]>
1 parent 623cfc6 commit 1abccdb

18 files changed

Lines changed: 1766 additions & 37 deletions

‎packages/cli/src/serve/hosted-workspace-tool-turn.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,10 @@ it('refuses unsupported profile calls before acquiring or reserving work', async
827827
for (const call of [
828828
{ ...calls[0], name: 'run_shell_command' },
829829
{ ...calls[0], wasOutputTruncated: true },
830+
// Arguments that arrived unterminated are refused even when the output
831+
// token limit was not what cut them: this profile writes to a remote
832+
// Workspace with no undo backup (#12970).
833+
{ ...calls[0], hadIncompleteArguments: true },
830834
]) {
831835
await expect(
832836
turn.execute([call], parts, 'model', new AbortController().signal),

‎packages/cli/src/serve/hosted-workspace-tool-turn.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,12 @@ export class HostedWorkspaceToolTurn {
716716
if (
717717
!declarations.some((tool) => tool.name === call.name) ||
718718
ids.has(call.callId) ||
719-
call.wasOutputTruncated === true
719+
// Refuse a call whose arguments arrived unterminated even when the
720+
// output token limit was not what cut them: this profile commits to a
721+
// remote Workspace with no undo backup, so a repaired partial
722+
// `content` or half-streamed command line is unrecoverable.
723+
call.wasOutputTruncated === true ||
724+
call.hadIncompleteArguments === true
720725
)
721726
throw new Error('Hosted Workspace profile refused a tool call.');
722727
ids.add(call.callId);

‎packages/core/src/agents/runtime/agent-core.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ import {
8282
type ToolCall,
8383
type WaitingToolCall,
8484
} from '../../core/coreToolScheduler.js';
85+
import { markToolCallArgumentsIncomplete } from '../../core/incomplete-tool-call-args.js';
8586
import { ToolConfirmationOutcome } from '../../tools/tools.js';
8687
import {
8788
AgentEventType,
@@ -1439,6 +1440,88 @@ describe('AgentCore approval response deduplication', () => {
14391440
});
14401441
});
14411442

1443+
describe('AgentCore.processFunctionCalls incomplete-argument marker', () => {
1444+
it('forwards hadIncompleteArguments when the turn was not token-truncated', async () => {
1445+
const config = {
1446+
getToolRegistry: vi.fn().mockReturnValue({ getTool: vi.fn() }),
1447+
getDebugLogger: vi
1448+
.fn()
1449+
.mockReturnValue({ debug: vi.fn(), error: vi.fn() }),
1450+
getToolOutputBatchBudget: vi
1451+
.fn()
1452+
.mockReturnValue(Number.POSITIVE_INFINITY),
1453+
getToolResultBytesWritten: vi.fn().mockReturnValue(0),
1454+
getSessionId: vi.fn().mockReturnValue('incomplete-args-session'),
1455+
} as unknown as Config;
1456+
const core = new AgentCore(
1457+
'incomplete-args-agent',
1458+
config,
1459+
{ systemPrompt: '' },
1460+
{ model: 'test-model' },
1461+
{ max_turns: 1 },
1462+
);
1463+
1464+
const functionCall = {
1465+
id: 'call-incomplete',
1466+
name: 'write_file',
1467+
args: { file_path: 'a.txt', content: 'half-written' },
1468+
};
1469+
markToolCallArgumentsIncomplete([{ functionCall }]);
1470+
// Negative control: an unmarked sibling must reach the scheduler WITHOUT
1471+
// hadIncompleteArguments, or over-application of the marker (which would
1472+
// reject every subagent Edit call) is undetectable.
1473+
const cleanFunctionCall = {
1474+
id: 'call-clean',
1475+
name: 'write_file',
1476+
args: { file_path: 'b.txt', content: 'complete' },
1477+
};
1478+
1479+
const scheduleSpy = vi
1480+
.spyOn(CoreToolScheduler.prototype, 'schedule')
1481+
.mockResolvedValue(undefined);
1482+
const abortController = new AbortController();
1483+
1484+
const processing = core.processFunctionCalls(
1485+
[functionCall, cleanFunctionCall],
1486+
abortController,
1487+
'prompt-incomplete',
1488+
1,
1489+
[{ name: 'write_file' } as FunctionDeclaration],
1490+
undefined,
1491+
// The subagent turn saw finishReason STOP (delayed usage disproved a
1492+
// token cut), so the marker is the only thing left that can arm the
1493+
// scheduler's data-loss guard here — a subagent has no approval prompt
1494+
// between the response and the write (#12970).
1495+
false,
1496+
);
1497+
try {
1498+
await vi.waitFor(() => expect(scheduleSpy).toHaveBeenCalledOnce());
1499+
const scheduledRequests = scheduleSpy.mock.calls[0]?.[0];
1500+
expect(scheduledRequests).toEqual([
1501+
expect.objectContaining({
1502+
callId: 'call-incomplete',
1503+
name: 'write_file',
1504+
wasOutputTruncated: false,
1505+
hadIncompleteArguments: true,
1506+
}),
1507+
expect.objectContaining({
1508+
callId: 'call-clean',
1509+
name: 'write_file',
1510+
wasOutputTruncated: false,
1511+
}),
1512+
]);
1513+
expect(Array.isArray(scheduledRequests)).toBe(true);
1514+
expect((scheduledRequests as unknown[])[1]).not.toHaveProperty(
1515+
'hadIncompleteArguments',
1516+
);
1517+
} finally {
1518+
abortController.abort();
1519+
await processing;
1520+
scheduleSpy.mockRestore();
1521+
}
1522+
});
1523+
});
1524+
14421525
describe('AgentCore.prepareTools', () => {
14431526
// Subagents that opt into the wildcard (`tools: ['*']`), or omit
14441527
// toolConfig entirely, must inherit DEFERRED tools too. Otherwise a

‎packages/core/src/agents/runtime/agent-core.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ import type {
8686
GenerateContentResponseUsageMetadata,
8787
} from '@google/genai';
8888
import { LlmChat } from '../../core/llm-chat.js';
89+
import { toolCallArgumentsWereIncomplete } from '../../core/incomplete-tool-call-args.js';
8990
import { assembleSystemPrompt } from '../../core/prompts.js';
9091
import {
9192
dedupeToolCallsById,
@@ -2418,6 +2419,12 @@ export class AgentCore {
24182419
prompt_id: promptId,
24192420
response_id: responseId,
24202421
wasOutputTruncated,
2422+
// Mirror `turn.ts`: the data-loss guard keys on the fact that the
2423+
// arguments arrived unterminated, which is independent of whether the
2424+
// output token limit was what cut them (QwenLM/qwen-code#12970).
2425+
...(toolCallArgumentsWereIncomplete(fc)
2426+
? { hadIncompleteArguments: true }
2427+
: {}),
24212428
...((toolName === ToolNames.EXEC ||
24222429
toolName === ToolNames.TOOL_SEARCH) &&
24232430
this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly

‎packages/core/src/core/coreToolScheduler.test.ts‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7181,6 +7181,10 @@ describe('CoreToolScheduler truncated output protection', () => {
71817181
expect(errorMessage).toContain(
71827182
'rejected to prevent writing truncated content',
71837183
);
7184+
// The telemetry arm of the cause-versus-diagnosis distinction (#12970):
7185+
// a genuine max_tokens cut must keep reporting OUTPUT_TRUNCATED, never
7186+
// collapse into the malformed-generation INVALID_TOOL_PARAMS.
7187+
expect(call.response.errorType).toBe(ToolErrorType.OUTPUT_TRUNCATED);
71847188
return errorMessage;
71857189
}
71867190

@@ -7195,6 +7199,110 @@ describe('CoreToolScheduler truncated output protection', () => {
71957199
);
71967200
});
71977201

7202+
// The token-limit diagnosis being withdrawn must not withdraw the data-loss
7203+
// guard with it: incomplete arguments mean incomplete file content either
7204+
// way (QwenLM/qwen-code#12970).
7205+
it('rejects Kind.Edit calls whose arguments were incomplete without a max_tokens cut', async () => {
7206+
const declarativeTool = new TestApprovalTool({
7207+
getApprovalMode: () => ApprovalMode.AUTO_EDIT,
7208+
} as unknown as Config);
7209+
const { scheduler, onAllToolCallsComplete } =
7210+
createTruncationTestScheduler(declarativeTool);
7211+
7212+
await scheduler.schedule(
7213+
[
7214+
{
7215+
callId: '1',
7216+
name: TestApprovalTool.Name,
7217+
args: { id: 'test-malformed' },
7218+
isClientInitiated: false,
7219+
prompt_id: 'prompt-id-malformed',
7220+
hadIncompleteArguments: true,
7221+
},
7222+
],
7223+
new AbortController().signal,
7224+
);
7225+
7226+
await vi.waitFor(() => {
7227+
expect(onAllToolCallsComplete).toHaveBeenCalled();
7228+
});
7229+
7230+
const completedCalls = onAllToolCallsComplete.mock
7231+
.calls[0][0] as ToolCall[];
7232+
expect(completedCalls).toHaveLength(1);
7233+
const completedCall = completedCalls[0];
7234+
expect(completedCall.status).toBe('error');
7235+
7236+
if (completedCall.status === 'error') {
7237+
const errorMessage = completedCall.response.error?.message ?? '';
7238+
// Still rejected, and for the real reason.
7239+
expect(errorMessage).toContain(
7240+
'rejected to prevent writing incomplete content',
7241+
);
7242+
expect(errorMessage).toContain('malformed generation');
7243+
expect(errorMessage).not.toContain('was truncated due to max_tokens');
7244+
expect(completedCall.response.errorType).toBe(
7245+
ToolErrorType.INVALID_TOOL_PARAMS,
7246+
);
7247+
}
7248+
});
7249+
7250+
// The non-Edit half of #12970 — and the wording the issue actually asks
7251+
// for: a non-Edit tool whose schema validation fails lands past the Edit
7252+
// guard, so its guidance must name malformed generation rather than a
7253+
// max_tokens cut the response's own usage disproved. The witness tool must
7254+
// not be Kind.Edit: Edit calls are rejected before validation and can never
7255+
// reach the paramGuidance branch.
7256+
it('attaches malformed-generation guidance to validation errors of incomplete non-Edit calls', async () => {
7257+
const readTool = new MockTool({
7258+
name: 'mockReadWithRequiredParam',
7259+
kind: Kind.Read,
7260+
params: {
7261+
type: 'object',
7262+
properties: { path: { type: 'string' } },
7263+
required: ['path'],
7264+
},
7265+
});
7266+
const { scheduler, onAllToolCallsComplete } =
7267+
createTruncationTestScheduler(readTool);
7268+
7269+
await scheduler.schedule(
7270+
[
7271+
{
7272+
callId: '1',
7273+
name: 'mockReadWithRequiredParam',
7274+
args: {},
7275+
isClientInitiated: false,
7276+
prompt_id: 'prompt-id-malformed-nonedit',
7277+
hadIncompleteArguments: true,
7278+
},
7279+
],
7280+
new AbortController().signal,
7281+
);
7282+
7283+
await vi.waitFor(() => {
7284+
expect(onAllToolCallsComplete).toHaveBeenCalled();
7285+
});
7286+
7287+
const completedCalls = onAllToolCallsComplete.mock
7288+
.calls[0][0] as ToolCall[];
7289+
expect(completedCalls).toHaveLength(1);
7290+
const completedCall = completedCalls[0];
7291+
expect(completedCall.status).toBe('error');
7292+
7293+
if (completedCall.status === 'error') {
7294+
const errorMessage = completedCall.response.error?.message ?? '';
7295+
// Reached validation (not the pre-validation Edit rejection)...
7296+
expect(errorMessage).toContain("required property 'path'");
7297+
// ...and the attached guidance matches the actual cause.
7298+
expect(errorMessage).toContain('malformed generation');
7299+
expect(errorMessage).not.toContain('truncated due to max_tokens limit');
7300+
expect(completedCall.response.errorType).toBe(
7301+
ToolErrorType.INVALID_TOOL_PARAMS,
7302+
);
7303+
}
7304+
});
7305+
71987306
it('should allow Kind.Edit tool calls when wasOutputTruncated is false', async () => {
71997307
const completedCall = await completeTruncationCall(
72007308
approvalTool(),
@@ -7261,6 +7369,70 @@ describe('CoreToolScheduler truncated output protection', () => {
72617369
expect(messages[1]).not.toContain('RETRY LOOP DETECTED');
72627370
expect(messages[2]).toContain('RETRY LOOP DETECTED');
72637371
});
7372+
7373+
// The Edit guard rejects before buildInvocation, so schema validation never
7374+
// runs on these calls: at the retry-loop threshold the directive must match
7375+
// the actual cause (repeated incomplete writes), not the validation-failure
7376+
// wording that would send the model re-examining a schema it never violated.
7377+
it('should inject the incomplete-args retry loop directive after repeated incomplete write_file rejections', async () => {
7378+
const writeFileConfig = {
7379+
getProjectRoot: () => '/tmp',
7380+
getTargetDir: () => '/tmp',
7381+
getFileSystemService: () => ({
7382+
readTextFile: vi.fn(),
7383+
writeTextFile: vi.fn(),
7384+
}),
7385+
getDefaultFileEncoding: () => undefined,
7386+
setApprovalMode: vi.fn(),
7387+
} as unknown as Config;
7388+
const writeFileTool = new WriteFileTool(writeFileConfig);
7389+
const { scheduler, onAllToolCallsComplete } =
7390+
createTruncationTestScheduler(writeFileTool);
7391+
7392+
const messages: string[] = [];
7393+
7394+
for (let i = 1; i <= 3; i++) {
7395+
await scheduler.schedule(
7396+
[
7397+
{
7398+
callId: `incomplete-write-file-${i}`,
7399+
name: WriteFileTool.Name,
7400+
args: { file_path: '/tmp/test.txt', content: 'partial' },
7401+
isClientInitiated: false,
7402+
prompt_id: `prompt-id-write-file-incomplete-${i}`,
7403+
hadIncompleteArguments: true,
7404+
},
7405+
],
7406+
new AbortController().signal,
7407+
);
7408+
7409+
await vi.waitFor(() => {
7410+
expect(onAllToolCallsComplete).toHaveBeenCalledTimes(i);
7411+
});
7412+
7413+
const completedCalls = onAllToolCallsComplete.mock.calls.at(-1)?.[0] as
7414+
| ToolCall[]
7415+
| undefined;
7416+
const completedCall = completedCalls?.[0];
7417+
expect(completedCall?.status).toBe('error');
7418+
if (completedCall?.status === 'error') {
7419+
messages.push(completedCall.response.error?.message ?? '');
7420+
}
7421+
}
7422+
7423+
expect(messages[0]).toContain(
7424+
'rejected to prevent writing incomplete content',
7425+
);
7426+
expect(messages[0]).not.toContain('RETRY LOOP DETECTED');
7427+
expect(messages[1]).not.toContain('RETRY LOOP DETECTED');
7428+
// At the threshold, the directive must be the incomplete-args one: the
7429+
// validation wording would misdiagnose the cause, and the truncation
7430+
// wording would re-introduce the max_tokens blame #12970 removed.
7431+
expect(messages[2]).toContain('RETRY LOOP DETECTED');
7432+
expect(messages[2]).toContain('same incomplete file write');
7433+
expect(messages[2]).not.toContain('failed validation');
7434+
expect(messages[2]).not.toContain('truncated');
7435+
});
72647436
});
72657437

72667438
describe('CoreToolScheduler Sequential Execution', () => {

0 commit comments

Comments
 (0)