mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-09 02:50:58 +00:00
test(sdk): align tool-control E2E with prior-read enforcement (#3898)
* test(sdk): align tool-control E2E with prior-read enforcement
Three tests in tool-control.test.ts broke deterministically after the
prior-read enforcement landed. Each seeded test.txt then prompted a
direct write — the new guard now rejects the write before canUseTool
fires, with "File X has not been fully read in this session...".
- updatedInput-application + allowedTools-bypass tests: drop the seed
so write_file takes the new-file path (exempt from enforcement).
- asyncGenerator-deny test: keep seed (assertion requires unchanged
content), rewrite prompt to "Read X then write Y" — the pattern
used by 27 passing tests in the same file. Also fix a latent bug
where canUseTool returned `updatedInput: {}` for non-write tools,
which would erase file_path on read_file (SDK `?? toolInput` only
catches nullish, not empty objects).
* test(sdk): address self-review on #3898
- Fix the same `updatedInput: {}` reverse-pattern at line 1545 in
the multi-turn asyncGenerator test. The CLI side at
permissionController.ts:444 does `if (updatedInput && typeof
updatedInput === 'object')` — an empty object is truthy, so it
silently replaces args. Mirror the pass-through fix from line ~1457.
- In the deny test, add `expect(toolNames).toContain('read_file')`
before the canUseTool-deny assertion. If the model skips the
read-first instruction, prior-read enforcement would surface
EDIT_REQUIRES_PRIOR_READ rather than the canUseTool deny message,
causing a confusing toContain mismatch. Fail fast with a clear
signal instead.
This commit is contained in:
parent
76765b5aa2
commit
d119b60bf0
1 changed files with 28 additions and 9 deletions
|
|
@ -1121,8 +1121,9 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
it(
|
||||
'should apply updatedInput from canUseTool callback',
|
||||
async () => {
|
||||
await helper.createFile('test.txt', 'original');
|
||||
|
||||
// Don't pre-create test.txt: prior-read enforcement requires
|
||||
// existing files to have been read via read_file first, but
|
||||
// this test restricts coreTools to write_file only.
|
||||
let capturedInput: Record<string, unknown> = {};
|
||||
|
||||
const q = query({
|
||||
|
|
@ -1171,8 +1172,9 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
it(
|
||||
'canUseTool should not be called for allowedTools even if it would modify input',
|
||||
async () => {
|
||||
await helper.createFile('test.txt', 'original');
|
||||
|
||||
// Don't pre-create test.txt: prior-read enforcement requires
|
||||
// existing files to have been read via read_file first, but
|
||||
// this test restricts coreTools to write_file only.
|
||||
let canUseToolCalled = false;
|
||||
|
||||
const q = query({
|
||||
|
|
@ -1433,7 +1435,10 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
session_id: crypto.randomUUID(),
|
||||
message: {
|
||||
role: 'user',
|
||||
content: 'Write "modified" to test.txt.',
|
||||
// Read-first instruction satisfies prior-read enforcement
|
||||
// so the deny path is exercised by canUseTool, not by the
|
||||
// write tool's pre-write guard.
|
||||
content: 'Read test.txt and then write "modified" to it.',
|
||||
},
|
||||
parent_tool_use_id: null,
|
||||
};
|
||||
|
|
@ -1447,14 +1452,16 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
cwd: testDir,
|
||||
permissionMode: 'default',
|
||||
coreTools: ['read_file', 'write_file'],
|
||||
canUseTool: async (toolName) => {
|
||||
canUseTool: async (toolName, input) => {
|
||||
if (toolName === 'write_file') {
|
||||
return {
|
||||
behavior: 'deny',
|
||||
message: 'Write operations are not allowed',
|
||||
};
|
||||
}
|
||||
return { behavior: 'allow', updatedInput: {} };
|
||||
// Pass-through: empty `updatedInput` would erase
|
||||
// file_path and break the read_file call.
|
||||
return { behavior: 'allow', updatedInput: input };
|
||||
},
|
||||
debug: false,
|
||||
},
|
||||
|
|
@ -1470,6 +1477,15 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
}
|
||||
}
|
||||
|
||||
// Make the read-first dependency explicit: if the model
|
||||
// skipped read_file, prior-read enforcement would surface
|
||||
// EDIT_REQUIRES_PRIOR_READ instead of the canUseTool deny
|
||||
// message we are asserting on below — fail fast with a
|
||||
// clear signal instead of a confusing toContain mismatch.
|
||||
const toolCalls = findToolCalls(messages);
|
||||
const toolNames = toolCalls.map((tc) => tc.toolUse.name);
|
||||
expect(toolNames).toContain('read_file');
|
||||
|
||||
// write_file should have been attempted but stream was closed
|
||||
const writeFileResults = findToolResults(messages, 'write_file');
|
||||
expect(writeFileResults.length).toBeGreaterThan(0);
|
||||
|
|
@ -1533,9 +1549,12 @@ describe('Tool Control Parameters (E2E)', () => {
|
|||
cwd: testDir,
|
||||
permissionMode: 'default',
|
||||
coreTools: ['read_file', 'write_file'],
|
||||
canUseTool: async (toolName) => {
|
||||
canUseTool: async (toolName, input) => {
|
||||
canUseToolCalls.push(toolName);
|
||||
return { behavior: 'allow', updatedInput: {} };
|
||||
// Pass-through: empty `updatedInput` would erase
|
||||
// file_path on the SDK→CLI boundary
|
||||
// (permissionController.ts:444 truthy-replaces args).
|
||||
return { behavior: 'allow', updatedInput: input };
|
||||
},
|
||||
debug: false,
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue