diff --git a/docs/QWEN-GUIDE.md b/docs/QWEN-GUIDE.md index 424a8d722..8e9e17122 100644 --- a/docs/QWEN-GUIDE.md +++ b/docs/QWEN-GUIDE.md @@ -61,3 +61,15 @@ Canonical agent files stay unchanged. The standalone `scripts/qwen-adapt-agents. can adapt an explicitly selected, user-owned copy; use the managed installer for ECC-owned files to keep repair and uninstall receipts consistent. Native subagent dispatch still requires verification in the user's configured Qwen runtime. + +Qwen Code 0.25.0 treats omitted, null, and empty `tools` fields in named-agent +files as inheriting the parent tool registry. An empty definition list differs +from the internal runtime `ToolConfig` and `fork_tools` deny-all contracts. +See the [official named-agent conversion](https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/subagent-manager.ts#L1661-L1685) +and [empty-list validation](https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/validation.ts#L216-L226). +ECC rejects an explicitly empty or null source tools field before managed +installation writes or standalone batch rewrites, because this adapter cannot +represent a deny-all agent through that native field. Use a nonempty explicit +allowlist, or omit the field only when parent-tool inheritance is intended. +These checks verify adaptation and write prevention; they do not certify native +provider execution or sandbox containment. diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 2632265ff..818035358 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -676,7 +676,20 @@ function prepareHookConsentMigration(plan, migration) { }; } +function preflightQwenAgentOperations(plan) { + for (const operation of plan.operations || []) { + if (operation.kind === 'copy-file' && operation.contentTransform === 'qwen-agent-frontmatter') { + assertSafeInstallOperation(plan, operation); + if (!fs.lstatSync(operation.sourcePath).isFile()) { + throw new Error('Refusing Qwen agent adaptation from a non-regular source file'); + } + transformInstallContent(operation, fs.readFileSync(operation.sourcePath, 'utf8')); + } + } +} + function previewInstallPlan(plan) { + preflightQwenAgentOperations(plan); assertOpenCodeHookDeactivationReady(plan); const migration = prepareStaleOperationsReconciliation( plan, @@ -736,6 +749,7 @@ function applyInstallPlan(plan, dependencies = {}) { } function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) { + preflightQwenAgentOperations(plan); const persistInstallState = dependencies.writeInstallState || writeInstallState; const beforeInstallStateRead = dependencies.beforeInstallStateRead; const beforeOperationWrite = dependencies.beforeOperationWrite; diff --git a/scripts/qwen-adapt-agents.js b/scripts/qwen-adapt-agents.js index 55c972b06..951a1e54f 100644 --- a/scripts/qwen-adapt-agents.js +++ b/scripts/qwen-adapt-agents.js @@ -3,6 +3,7 @@ const fs = require('fs'); const path = require('path'); +const yaml = require('js-yaml'); const { normalizeAgentTools } = require('./lib/agent-tools'); // Qwen Code is a Gemini CLI descendant, so most tool ids match Gemini's. Three @@ -90,15 +91,6 @@ function ensureDirectory(dirPath) { } } -function parseToolList(line) { - const match = line.match(/^\s*tools\s*:\s*(.*)$/); - if (!match) { - return null; - } - - return normalizeAgentTools(match[1]); -} - function adaptToolName(toolName) { const mapped = TOOL_NAME_MAP.get(toolName); if (mapped) { @@ -113,9 +105,7 @@ function formatToolLine(tools) { // Comma-separated scalar rather than the JSON flow sequence // gemini-adapt-agents.js emits: Qwen Code parses both, and the scalar form is // what its own agent examples use. - // YAML null can discard an optional allowlist and inherit parent tools. - // Keep an explicitly empty source list as an empty native list. - return tools.length ? `tools: ${tools.join(', ')}` : 'tools: []'; + return `tools: ${tools.join(', ')}`; } function adaptColor(line) { @@ -149,10 +139,31 @@ function adaptFrontmatter(text) { return { text, changed: false }; } + const firstMappingLine = match[1].split('\n').find(line => line.trim() && !line.trimStart().startsWith('#')); + if (firstMappingLine && /^\s/.test(firstMappingLine)) { + throw new Error('Unsupported indented Qwen agent frontmatter: use unindented root mapping keys'); + } + + const frontmatter = yaml.load(match[1]); + const hasTools = frontmatter && Object.hasOwn(frontmatter, 'tools'); + const sourceTools = hasTools ? normalizeAgentTools(frontmatter.tools) : null; + // Qwen 0.25.0 definition files interpret [] as inherit-all, unlike its + // internal runtime ToolConfig. Reject rather than broaden source authority. + // https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/subagent-manager.ts#L1661-L1685 + if (hasTools && sourceTools.length === 0) { + throw new Error('Unsupported empty tools allowlist: Qwen Code inherits all tools for an empty agent list; omit tools only when inheritance is intended'); + } + + if (hasTools && !/^tools\s*:/m.test(match[1])) { + throw new Error('Unsupported Qwen tools key format: use an unquoted root tools key'); + } + let changed = false; const updatedLines = []; - for (const line of match[1].split('\n')) { + const lines = match[1].split('\n'); + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; // Claude tiers are not Qwen provider IDs; inherit the configured session. if (/^model:\s*["']?(haiku|sonnet|opus)["']?\s*$/.test(line)) { updatedLines.push('model: inherit'); @@ -170,12 +181,16 @@ function adaptFrontmatter(text) { continue; } - const tools = parseToolList(line); - if (tools) { + if (/^tools\s*:/.test(line) && hasTools) { + // Consume indented YAML continuations before writing the native scalar. + while (index + 1 < lines.length && /^(?:\s|#|$)/.test(lines[index + 1])) { + index += 1; + changed = true; + } const adaptedTools = []; const seen = new Set(); - for (const tool of tools.map(adaptToolName)) { + for (const tool of sourceTools.map(adaptToolName)) { if (!tool || seen.has(tool)) { continue; } @@ -212,15 +227,25 @@ function adaptAgents(dirPath) { let updated = 0; let unchanged = 0; - for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) { + const prepared = []; + // Validate the entire batch before rewriting any installed agent. + for (const entry of fs.readdirSync(dirPath, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { if (!entry.isFile() || !entry.name.endsWith('.md')) { continue; } const filePath = path.join(dirPath, entry.name); const original = fs.readFileSync(filePath, 'utf8'); - const adapted = adaptFrontmatter(original); + let adapted; + try { + adapted = adaptFrontmatter(original); + } catch (error) { + throw new Error(`${entry.name}: ${error.message}`); + } + prepared.push({ filePath, adapted }); + } + for (const { filePath, adapted } of prepared) { if (adapted.changed) { fs.writeFileSync(filePath, adapted.text); updated += 1; diff --git a/tests/scripts/qwen-adapt-agents.test.js b/tests/scripts/qwen-adapt-agents.test.js index 92ab1605d..6acfa219a 100644 --- a/tests/scripts/qwen-adapt-agents.test.js +++ b/tests/scripts/qwen-adapt-agents.test.js @@ -366,28 +366,88 @@ function runTests() { } })) passed++; else failed++; - if (test('preserves an explicit empty allowlist through actual CLI adaptation and rerun', () => { + if (test('rejects an empty allowlist before writing any agent in the CLI batch', () => { const tempDir = createTempDir(); const agentsDir = path.join(tempDir, '.qwen', 'agents'); - const yaml = require('js-yaml'); - const frontmatter = text => yaml.load(text.match(/^---\n([\s\S]*?)\n---/)[1]); try { - writeAgent(agentsDir, 'no-tools.md', [ - '---', 'name: no-tools', 'description: No tools permitted', - 'tools: []', 'model: sonnet', '---', '', 'BODY_SENTINEL' - ].join('\n')); - const first = run([agentsDir]); - assert.strictEqual(first.code, 0, first.stderr); - assert.ok(first.stdout.includes('Updated 1 agent file(s)')); - const adapted = readAgent(agentsDir, 'no-tools.md'); - assert.deepStrictEqual(frontmatter(adapted).tools, []); - assert.strictEqual(frontmatter(adapted).model, 'inherit'); - assert.ok(adapted.endsWith('BODY_SENTINEL')); - const second = run([agentsDir]); - assert.strictEqual(second.code, 0, second.stderr); - assert.ok(second.stdout.includes('Updated 0 agent file(s)')); - assert.strictEqual(readAgent(agentsDir, 'no-tools.md'), adapted); - assert.deepStrictEqual(frontmatter(readAgent(agentsDir, 'no-tools.md')).tools, []); + for (const emptyTools of ['[]', '[] # no tools', '', 'null', '""']) { + writeAgent(agentsDir, 'a-valid.md', ['---', 'name: valid', 'tools: Read', 'model: sonnet', '---', 'VALID_BODY'].join('\n')); + writeAgent(agentsDir, 'z-no-tools.md', ['---', 'name: no-tools', `tools: ${emptyTools}`, 'model: sonnet', '---', 'NO_TOOLS_BODY'].join('\n')); + writeAgent(agentsDir, 'ignored.txt', 'UNCHANGED'); + const snapshot = () => fs.readdirSync(agentsDir).sort().map(name => [name, fs.readFileSync(path.join(agentsDir, name)).toString('base64')]); + const before = snapshot(); + for (let attempt = 0; attempt < 2; attempt++) { + const result = run([agentsDir]); + assert.strictEqual(result.code, 1, result.stdout); + assert.match(result.stderr, /z-no-tools.md: Unsupported empty tools allowlist/); + assert.match(result.stderr, /inherits all tools/); + assert.deepStrictEqual(snapshot(), before); + } + } + } finally { cleanupTempDir(tempDir); } + })) passed++; else failed++; + + if (test('adapts a nonempty YAML block list without leftover tool entries', () => { + const { adaptFrontmatter } = require(SCRIPT); + const yaml = require('js-yaml'); + const result = adaptFrontmatter(['---', 'name: block-tools', 'description: Neighbor metadata', 'tools:', ' - Read', '', '# Tool list comment', ' - Grep', 'model: sonnet', 'metadata:', ' owner: operator', '---', 'BODY'].join('\n')); + const parsed = yaml.load(result.text.match(/^---\n([\s\S]*?)\n---/)[1]); + assert.strictEqual(parsed.tools, 'read_file, grep_search'); + assert.strictEqual(parsed.model, 'inherit'); + assert.strictEqual(parsed.description, 'Neighbor metadata'); + assert.deepStrictEqual(parsed.metadata, { owner: 'operator' }); + assert.strictEqual(adaptFrontmatter(result.text).changed, false); + })) passed++; else failed++; + + if (test('rejects uniformly indented mappings without changing neighboring metadata', () => { + const tempDir = createTempDir(); + try { + const source = ['---', ' name: indented', ' description: Preserve me', ' tools:', ' - Read', ' - Grep', ' model: sonnet', '---', 'BODY'].join('\n'); + writeAgent(tempDir, 'indented.md', source); + for (let attempt = 0; attempt < 2; attempt++) { + const result = run([tempDir]); + assert.strictEqual(result.code, 1); + assert.match(result.stderr, /Unsupported indented Qwen agent frontmatter/); + assert.strictEqual(readAgent(tempDir, 'indented.md'), source); + } + } finally { cleanupTempDir(tempDir); } + })) passed++; else failed++; + + if (test('preflights every managed Qwen transform before file or receipt writes', () => { + const { applyInstallPlan, previewInstallPlan } = require('../../scripts/lib/install/apply'); + const tempDir = createTempDir(); + const targetRoot = path.join(tempDir, 'installed'); + const sourceRoot = path.join(tempDir, 'source'); + try { + fs.mkdirSync(targetRoot); + fs.mkdirSync(sourceRoot); + const valid = ['---', 'name: valid', 'tools: Read', '---', 'BODY'].join('\n'); + const invalid = ['---', 'name: invalid', 'tools: []', '---', 'BODY'].join('\n'); + writeAgent(sourceRoot, 'a-valid.md', valid); + writeAgent(sourceRoot, 'z-invalid.md', invalid); + writeAgent(targetRoot, 'z-invalid.md', 'OPERATOR_OWNED_BYTES'); + writeAgent(targetRoot, 'ecc-install-state.json', '{"preserve":"receipt"}'); + const plan = { + adapter: { id: 'qwen-home', target: 'qwen' }, + targetRoot, + installStatePath: path.join(targetRoot, 'ecc-install-state.json'), + operations: ['a-valid.md', 'z-invalid.md'].map(name => ({ + kind: 'copy-file', sourcePath: path.join(sourceRoot, name), + sourceRelativePath: `agents/${name}`, + destinationPath: path.join(targetRoot, name), + contentTransform: 'qwen-agent-frontmatter', + })), + }; + const snapshot = () => fs.readdirSync(targetRoot).sort().map(name => [name, fs.readFileSync(path.join(targetRoot, name)).toString('base64')]); + const before = snapshot(); + for (const operation of [previewInstallPlan, applyInstallPlan]) { + assert.throws(() => operation(plan), /Unsupported empty tools allowlist/); + assert.deepStrictEqual(snapshot(), before); + } + fs.unlinkSync(path.join(sourceRoot, 'z-invalid.md')); + fs.mkdirSync(path.join(sourceRoot, 'z-invalid.md')); + assert.throws(() => applyInstallPlan(plan), /non-regular source file/); + assert.deepStrictEqual(snapshot(), before); } finally { cleanupTempDir(tempDir); } })) passed++; else failed++;