mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-10 01:29:17 +00:00
refactor(core): revert round-X residuals to hit v1's actual minimum LOC
The previous shrink left round-1/2/3 fixes in place that were originally for the wider 9-field scope; with the carry-only fields gone they're no longer needed. Reverting them all back to pre-PR behaviour: - approvalMode strict throw (round-1 demoted to lenient; defer the symmetry fix to a separate PR if wanted) - runConfig.max_turns prune on serialize (round-3 cleanup; defer) - color: 'auto' normalised to undefined (round-2 round-trip fix; defer) - claude-converter NESTED_FIELDS_NOT_ROUND_TRIPPABLE + CONVERTER_OWNED_KEYS + passthrough loop (round-1/2 — all for the deleted carry-only fields) - ClaudeAgentConfig.approvalMode + precedence gating (round-3) - parseBackground in shared schema module (the dedup wasn't pulling weight) - parseStringOrArray in shared schema module (same) The shared bridge claudePermissionModeToApprovalMode + the disambiguated name stay — converter and loader use the same map and the name collision risk is real. Net: -336 LOC vs the prior shrink commit.
This commit is contained in:
LaZzyMan
parent
637b8b70cf
commit
70a876d38f
7 changed files with 66 additions and 403 deletions
|
|
@ -8,31 +8,7 @@ coordinating with the workflow port in issue [#4721][i4721] / PR [#4732][p4732].
|
|||
[i4721]: https://github.com/QwenLM/qwen-code/issues/4721
|
||||
[p4732]: https://github.com/QwenLM/qwen-code/pull/4732
|
||||
|
||||
## Implementation status (vertical-sliced)
|
||||
|
||||
PR [#4842][p4842] ships only the fields that have an end-to-end runtime path
|
||||
today. The remaining fields are deferred to follow-up PRs that first add the
|
||||
prerequisite infrastructure they need.
|
||||
|
||||
| Field | Status in this PR | Why deferred / prerequisite |
|
||||
| ----------------- | ----------------- | --------------------------------------------------------------------------- |
|
||||
| `permissionMode` | **shipped** | bridges to existing qwen `approvalMode` at parse time |
|
||||
| `maxTurns` | **shipped** | wired into existing `runConfig.max_turns` runtime path |
|
||||
| `color` allowlist | **shipped** | tightens existing field to CC's `_Y` set + `auto` legacy sentinel handling |
|
||||
| `effort` | deferred | no model-layer `effort` parameter exists yet in qwen providers |
|
||||
| `mcpServers` | deferred | needs nested-aware YAML parser (replace `yaml-parser.ts` with `js-yaml`) |
|
||||
| `hooks` | deferred | needs nested-aware YAML parser (same as above) |
|
||||
| `memory` | deferred | qwen's auto-memory has no `user`/`project`/`local` scope distinction yet |
|
||||
| `isolation` | deferred | workflow PR #4732 owns the runtime; per-agent default lands when that lands |
|
||||
| `initialPrompt` | deferred | requires `--agent` CLI flag (no main-session-agent infra in qwen) |
|
||||
| `skills` | deferred | requires SkillManager consumption of `config.skills` |
|
||||
|
||||
The full reverse-engineering record below is retained as the design reference
|
||||
for the follow-up PRs — schema constants, DL7/Ig5 semantics, error messages,
|
||||
and the coordination matrix with workflow are still load-bearing for that
|
||||
work even though the data carriers are not shipped today.
|
||||
|
||||
[p4842]: https://github.com/QwenLM/qwen-code/pull/4842
|
||||
**Implementation status:** PR [#4842](https://github.com/QwenLM/qwen-code/pull/4842) ships `permissionMode`, `maxTurns`, and a tightened `color` allowlist. The other fields documented below are reference material for follow-up PRs once their prerequisite infra exists (`effort` → model-layer param; `mcpServers`/`hooks` → nested YAML parser; `memory` → scoped memory subsystem; `isolation` → workflow PR #4732; `initialPrompt` → `--agent` flag; `skills` → SkillManager wiring).
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -90,90 +90,6 @@ describe('convertClaudeAgentConfig', () => {
|
|||
|
||||
expect(result['tools']).toEqual(['ReadFile', 'NotebookEdit', 'Edit']);
|
||||
});
|
||||
|
||||
it('should map Claude permissionMode to Qwen approvalMode via the shared bridge', () => {
|
||||
const cases: Array<[string, string]> = [
|
||||
['default', 'default'],
|
||||
['plan', 'plan'],
|
||||
['acceptEdits', 'auto-edit'],
|
||||
['auto', 'auto-edit'],
|
||||
['bypassPermissions', 'yolo'],
|
||||
// dontAsk preserves restrictive intent by mapping to default, not auto-edit.
|
||||
['dontAsk', 'default'],
|
||||
];
|
||||
for (const [claudeMode, expectedQwenMode] of cases) {
|
||||
const result = convertClaudeAgentConfig({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
permissionMode: claudeMode,
|
||||
});
|
||||
expect(result['approvalMode']).toBe(expectedQwenMode);
|
||||
}
|
||||
});
|
||||
|
||||
it('should preserve unknown permissionMode verbatim for surface-on-import diagnostics', () => {
|
||||
const result = convertClaudeAgentConfig({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
permissionMode: 'mystery-mode',
|
||||
});
|
||||
expect(result['approvalMode']).toBe('mystery-mode');
|
||||
});
|
||||
|
||||
it('should emit explicit approvalMode when present (direct-caller contract)', () => {
|
||||
// Regression for the round-3 finding that an explicit approvalMode
|
||||
// (without permissionMode) was silently dropped by the converter. The
|
||||
// in-tree convertAgentFiles path recovered via the unowned-key
|
||||
// passthrough, but exported direct-call surface needs to honor the
|
||||
// explicit field on its own.
|
||||
const result = convertClaudeAgentConfig({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
approvalMode: 'plan',
|
||||
});
|
||||
expect(result['approvalMode']).toBe('plan');
|
||||
});
|
||||
|
||||
it('should prefer explicit approvalMode over the permissionMode bridge (precedence parity with loader)', () => {
|
||||
// Self-violated-invariant fix: previously the converter ran the bridge
|
||||
// unconditionally, so a hand-edited file with both fields lost the
|
||||
// explicit approvalMode in favor of the bridged value. The loader's rule
|
||||
// is "approvalMode wins over bridge"; the converter now matches.
|
||||
const result = convertClaudeAgentConfig({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
permissionMode: 'bypassPermissions', // would bridge to 'yolo'
|
||||
approvalMode: 'plan', // explicit — should win
|
||||
});
|
||||
expect(result['approvalMode']).toBe('plan');
|
||||
});
|
||||
});
|
||||
|
||||
describe('convertClaudeAgentConfig — non-owned keys are not emitted', () => {
|
||||
// Regression for the passthrough mechanism in convertAgentFiles. The
|
||||
// converter only EMITS the keys it owns; convertAgentFiles passes the rest
|
||||
// through verbatim from the source frontmatter. This test pins that the
|
||||
// converter doesn't accidentally start emitting one of the passthrough
|
||||
// fields (which would silently override the passthrough copy).
|
||||
const PASSTHROUGH_FIELDS = [
|
||||
'effort',
|
||||
'maxTurns',
|
||||
'initialPrompt',
|
||||
'memory',
|
||||
'isolation',
|
||||
'mcpServers',
|
||||
'background',
|
||||
] as const;
|
||||
|
||||
for (const field of PASSTHROUGH_FIELDS) {
|
||||
it(`should not emit "${field}" in convertClaudeAgentConfig output`, () => {
|
||||
const result = convertClaudeAgentConfig({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
});
|
||||
expect(result[field]).toBeUndefined();
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('mergeClaudeConfigs', () => {
|
||||
|
|
|
|||
|
|
@ -27,10 +27,6 @@ import {
|
|||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import { normalizeContent } from '../utils/textUtils.js';
|
||||
import { substituteHookVariables } from './variables.js';
|
||||
import {
|
||||
parseStringOrArray,
|
||||
claudePermissionModeToApprovalMode,
|
||||
} from '../subagents/agent-frontmatter-schema.js';
|
||||
|
||||
const debugLogger = createDebugLogger('CLAUDE_CONVERTER');
|
||||
|
||||
|
|
@ -69,12 +65,6 @@ export interface ClaudeAgentConfig {
|
|||
model?: string;
|
||||
/** Permission mode: default, acceptEdits, dontAsk, bypassPermissions, or plan */
|
||||
permissionMode?: string;
|
||||
/**
|
||||
* Optional qwen-style approval mode. Hand-edited CC files may set this
|
||||
* directly; when both `approvalMode` and `permissionMode` are present,
|
||||
* the explicit `approvalMode` wins (matches loader behavior).
|
||||
*/
|
||||
approvalMode?: string;
|
||||
/** Skills to load into the subagent's context at startup */
|
||||
skills?: string[];
|
||||
/** Hooks configuration */
|
||||
|
|
@ -142,21 +132,27 @@ const claudeBuildInToolsTransform = (tools: string[]): string[] => {
|
|||
};
|
||||
|
||||
/**
|
||||
* Frontmatter keys that {@link convertClaudeAgentConfig} owns end-to-end. Any
|
||||
* key NOT in this set is passed through verbatim when converting CC agent
|
||||
* files so that future CC additions survive install without code changes.
|
||||
* Parses a value that can be either a comma-separated string or an array.
|
||||
* Claude agent config can have tools like 'Glob, Grep, Read' or ['Glob', 'Grep', 'Read']
|
||||
* @param value The value to parse
|
||||
* @returns Array of strings or undefined
|
||||
*/
|
||||
const CONVERTER_OWNED_KEYS = new Set([
|
||||
'name',
|
||||
'description',
|
||||
'tools',
|
||||
'disallowedTools',
|
||||
'model',
|
||||
'permissionMode',
|
||||
'skills',
|
||||
'hooks',
|
||||
'color',
|
||||
]);
|
||||
function parseStringOrArray(value: unknown): string[] | undefined {
|
||||
if (value === undefined || value === null) {
|
||||
return undefined;
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return value.map(String);
|
||||
}
|
||||
if (typeof value === 'string') {
|
||||
// Split by comma and trim whitespace
|
||||
return value
|
||||
.split(',')
|
||||
.map((s) => s.trim())
|
||||
.filter((s) => s.length > 0);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts a Claude agent config to Qwen Code subagent format.
|
||||
|
|
@ -191,19 +187,24 @@ export function convertClaudeAgentConfig(
|
|||
qwenAgent['model'] = claudeAgent.model;
|
||||
}
|
||||
|
||||
// Map Claude permissionMode to Qwen ApprovalMode via the shared bridge in
|
||||
// agent-frontmatter-schema.ts. The convert path matches the loader's
|
||||
// precedence: when the source has both `approvalMode` and `permissionMode`,
|
||||
// the explicit qwen-style approvalMode wins (only fall back to the bridge
|
||||
// when approvalMode is unset). Unknown permissionMode values fall back to
|
||||
// the raw string so the user sees an explicit "invalid approvalMode"
|
||||
// downstream rather than silently dropping the field on import.
|
||||
if (claudeAgent.approvalMode) {
|
||||
qwenAgent['approvalMode'] = claudeAgent.approvalMode;
|
||||
} else if (claudeAgent.permissionMode) {
|
||||
qwenAgent['approvalMode'] =
|
||||
claudePermissionModeToApprovalMode(claudeAgent.permissionMode) ??
|
||||
// Map Claude permission mode aliases to Qwen ApprovalMode values.
|
||||
// Note: Claude's `dontAsk` denies any tool call that would prompt the user,
|
||||
// making it restrictive. We map it to `default` (which also requires approval)
|
||||
// rather than `auto-edit` (which auto-approves), preserving the restrictive
|
||||
// intent. `bypassPermissions` is the Claude mode that auto-approves everything.
|
||||
if (claudeAgent.permissionMode) {
|
||||
const claudeToQwenMode: Record<string, string> = {
|
||||
default: 'default',
|
||||
plan: 'plan',
|
||||
acceptEdits: 'auto-edit',
|
||||
dontAsk: 'default',
|
||||
bypassPermissions: 'yolo',
|
||||
auto: 'auto-edit',
|
||||
};
|
||||
const mapped =
|
||||
claudeToQwenMode[claudeAgent.permissionMode] ??
|
||||
claudeAgent.permissionMode;
|
||||
qwenAgent['approvalMode'] = mapped;
|
||||
}
|
||||
if (claudeAgent.hooks) {
|
||||
qwenAgent['hooks'] = claudeAgent.hooks;
|
||||
|
|
@ -214,21 +215,10 @@ export function convertClaudeAgentConfig(
|
|||
if (claudeAgent.disallowedTools && claudeAgent.disallowedTools.length > 0) {
|
||||
qwenAgent['disallowedTools'] = claudeAgent.disallowedTools;
|
||||
}
|
||||
// NOTE: hooks intentionally NOT emitted. The local yaml-parser only formats
|
||||
// one level of nesting and would mangle hooks into '[object Object]' on write.
|
||||
// SubagentManager.serializeSubagent has the same skip rule for the same reason.
|
||||
|
||||
return qwenAgent;
|
||||
}
|
||||
|
||||
/**
|
||||
* Frontmatter keys whose values are typically nested objects/arrays that the
|
||||
* local yaml-parser cannot round-trip safely. {@link convertAgentFiles} skips
|
||||
* them when writing the converted file so a `[object Object]` corruption can't
|
||||
* land on disk; users keep the values by editing the source file directly.
|
||||
*/
|
||||
const NESTED_FIELDS_NOT_ROUND_TRIPPABLE = new Set(['mcpServers', 'hooks']);
|
||||
|
||||
/**
|
||||
* Converts all agent files in a directory from Claude format to Qwen format.
|
||||
* Parses the YAML frontmatter, converts the configuration, and writes back.
|
||||
|
|
@ -271,7 +261,6 @@ async function convertAgentFiles(agentsDir: string): Promise<void> {
|
|||
disallowedTools: parseStringOrArray(frontmatter['disallowedTools']),
|
||||
model: frontmatter['model'] as string | undefined,
|
||||
permissionMode: frontmatter['permissionMode'] as string | undefined,
|
||||
approvalMode: frontmatter['approvalMode'] as string | undefined,
|
||||
skills: parseStringOrArray(frontmatter['skills']),
|
||||
hooks: frontmatter['hooks'],
|
||||
color: frontmatter['color'] as string | undefined,
|
||||
|
|
@ -281,30 +270,10 @@ async function convertAgentFiles(agentsDir: string): Promise<void> {
|
|||
// Convert to Qwen format
|
||||
const qwenAgent = convertClaudeAgentConfig(claudeAgent);
|
||||
|
||||
// Build new frontmatter (excluding systemPrompt as it goes in body).
|
||||
// Step 1 passes through any CC frontmatter key that the converter does
|
||||
// NOT own (effort, maxTurns, initialPrompt, memory, isolation,
|
||||
// mcpServers, background, …). This keeps drop-in compatibility for
|
||||
// upstream CC fields that the converter has not been taught about yet —
|
||||
// when CC adds a 17th field, it survives plugin install instead of
|
||||
// being silently stripped.
|
||||
// Build new frontmatter (excluding systemPrompt as it goes in body)
|
||||
const newFrontmatter: Record<string, unknown> = {};
|
||||
for (const [key, value] of Object.entries(frontmatter)) {
|
||||
if (
|
||||
!CONVERTER_OWNED_KEYS.has(key) &&
|
||||
key !== 'systemPrompt' &&
|
||||
!NESTED_FIELDS_NOT_ROUND_TRIPPABLE.has(key) &&
|
||||
value !== undefined
|
||||
) {
|
||||
newFrontmatter[key] = value;
|
||||
}
|
||||
}
|
||||
for (const [key, value] of Object.entries(qwenAgent)) {
|
||||
if (
|
||||
key !== 'systemPrompt' &&
|
||||
!NESTED_FIELDS_NOT_ROUND_TRIPPABLE.has(key) &&
|
||||
value !== undefined
|
||||
) {
|
||||
if (key !== 'systemPrompt' && value !== undefined) {
|
||||
newFrontmatter[key] = value;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,8 +9,6 @@ import {
|
|||
PERMISSION_MODE_VALUES,
|
||||
COLOR_VALUES,
|
||||
claudePermissionModeToApprovalMode,
|
||||
parseStringOrArray,
|
||||
parseBackground,
|
||||
parseMaxTurns,
|
||||
isPermissionMode,
|
||||
isColor,
|
||||
|
|
@ -72,69 +70,6 @@ describe('agent-frontmatter-schema', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('parseStringOrArray — DL7 lenient parsing', () => {
|
||||
it('returns undefined for undefined / null', () => {
|
||||
expect(parseStringOrArray(undefined)).toBeUndefined();
|
||||
expect(parseStringOrArray(null)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('parses comma-separated string', () => {
|
||||
expect(parseStringOrArray('Read, Edit, Glob')).toEqual([
|
||||
'Read',
|
||||
'Edit',
|
||||
'Glob',
|
||||
]);
|
||||
});
|
||||
|
||||
it('accepts YAML array as-is', () => {
|
||||
expect(parseStringOrArray(['Read', 'Edit'])).toEqual(['Read', 'Edit']);
|
||||
});
|
||||
|
||||
it('filters empty entries from comma-separated string', () => {
|
||||
expect(parseStringOrArray('Read,,Edit, ')).toEqual(['Read', 'Edit']);
|
||||
});
|
||||
|
||||
it('returns undefined for non-string non-array values', () => {
|
||||
expect(parseStringOrArray(42)).toBeUndefined();
|
||||
expect(parseStringOrArray({})).toBeUndefined();
|
||||
expect(parseStringOrArray(true)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('stringifies array elements', () => {
|
||||
// CC accepts mixed array — coerces to strings
|
||||
expect(parseStringOrArray([1, 'Read', true])).toEqual([
|
||||
'1',
|
||||
'Read',
|
||||
'true',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseBackground — DL7 boolean-or-string lenience', () => {
|
||||
it('accepts boolean true', () => {
|
||||
expect(parseBackground(true)).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts string "true"', () => {
|
||||
expect(parseBackground('true')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns undefined for boolean false (matches DL7 — only truthy normalises)', () => {
|
||||
expect(parseBackground(false)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined for string "false"', () => {
|
||||
expect(parseBackground('false')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined for undefined / null / other', () => {
|
||||
expect(parseBackground(undefined)).toBeUndefined();
|
||||
expect(parseBackground(null)).toBeUndefined();
|
||||
expect(parseBackground(42)).toBeUndefined();
|
||||
expect(parseBackground('yes')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseMaxTurns — DL7 number-or-numeric-string lenience', () => {
|
||||
it('accepts positive integer number', () => {
|
||||
expect(parseMaxTurns(50)).toBe(50);
|
||||
|
|
|
|||
|
|
@ -78,36 +78,6 @@ export function claudePermissionModeToApprovalMode(
|
|||
return PERMISSION_MODE_TO_APPROVAL_MODE[permissionMode];
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a value that may be a comma-separated string OR an array of strings.
|
||||
* Returns `undefined` for any other shape. Matches DL7's lenient posture for
|
||||
* `tools`, `disallowedTools`, and `skills`.
|
||||
*/
|
||||
export function parseStringOrArray(value: unknown): string[] | undefined {
|
||||
if (value === undefined || value === null) return undefined;
|
||||
if (Array.isArray(value)) {
|
||||
return value.map((entry) => String(entry));
|
||||
}
|
||||
if (typeof value === 'string') {
|
||||
return value
|
||||
.split(',')
|
||||
.map((s) => s.trim())
|
||||
.filter((s) => s.length > 0);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a background value. Accepts boolean `true` / string `"true"` and
|
||||
* returns `true`. Returns `undefined` for everything else (including `false`
|
||||
* and `"false"`) — matching DL7 (`eiH`/`EL8`), which only normalises truthy
|
||||
* values to `true`.
|
||||
*/
|
||||
export function parseBackground(value: unknown): true | undefined {
|
||||
if (value === true || value === 'true') return true;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a maxTurns value. Accepts a positive integer number or numeric string.
|
||||
* Returns `undefined` for anything else (matches DL7 `W46`).
|
||||
|
|
|
|||
|
|
@ -207,8 +207,6 @@ describe('SubagentManager', () => {
|
|||
yaml += `disallowedTools:\n${value.map((t) => ` - ${t}`).join('\n')}\n`;
|
||||
} else if (key === 'tools' && Array.isArray(value)) {
|
||||
yaml += `tools:\n${value.map((tool) => ` - ${tool}`).join('\n')}\n`;
|
||||
} else if (key === 'skills' && Array.isArray(value)) {
|
||||
yaml += `skills:\n${value.map((s) => ` - ${s}`).join('\n')}\n`;
|
||||
} else if (key === 'model') {
|
||||
yaml += `model: ${value}\n`;
|
||||
} else if (key === 'runConfig' && typeof value === 'object' && value) {
|
||||
|
|
@ -629,42 +627,6 @@ You are an agent.
|
|||
expect(config.permissionMode).toBe('bypassPermissions');
|
||||
});
|
||||
|
||||
it('should drop invalid approvalMode (lenient) AND still bridge from valid permissionMode', () => {
|
||||
// Self-violated-invariant fix: approvalMode used to throw on invalid
|
||||
// values, killing the whole file. Now it warns and drops, symmetric
|
||||
// with all the other CC-compatible fields, so a valid permissionMode
|
||||
// can still bridge into approvalMode when the user-typed approvalMode
|
||||
// is invalid.
|
||||
mockParseYaml.mockReturnValueOnce({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
approvalMode: 'tpyo',
|
||||
permissionMode: 'bypassPermissions',
|
||||
});
|
||||
const config = manager.parseSubagentContent(
|
||||
'---\nname: a\ndescription: d\napprovalMode: tpyo\npermissionMode: bypassPermissions\n---\nx',
|
||||
validConfig.filePath!,
|
||||
'project',
|
||||
);
|
||||
expect(config.approvalMode).toBe('yolo');
|
||||
});
|
||||
|
||||
it('should not throw on invalid approvalMode without permissionMode', () => {
|
||||
mockParseYaml.mockReturnValueOnce({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
approvalMode: 'tpyo',
|
||||
});
|
||||
const config = manager.parseSubagentContent(
|
||||
'---\nname: a\ndescription: d\napprovalMode: tpyo\n---\nx',
|
||||
validConfig.filePath!,
|
||||
'project',
|
||||
);
|
||||
expect(config.approvalMode).toBeUndefined();
|
||||
// File still loads — only the invalid field is dropped.
|
||||
expect(config.name).toBe('a');
|
||||
});
|
||||
|
||||
it('should drop invalid permissionMode and not bridge', () => {
|
||||
mockParseYaml.mockReturnValueOnce({
|
||||
name: 'a',
|
||||
|
|
@ -749,24 +711,6 @@ You are an agent.
|
|||
);
|
||||
expect(config.color).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should normalize legacy color: auto sentinel to undefined (round-trip parity)', () => {
|
||||
// 'auto' is the legacy "no override" sentinel. Parser normalizes it to
|
||||
// undefined so that parse → serialize → parse is idempotent: the CLI
|
||||
// helpers `shouldShowColor` / `getColorForDisplay` already treat 'auto'
|
||||
// and undefined identically, so no behavior change downstream.
|
||||
mockParseYaml.mockReturnValueOnce({
|
||||
name: 'a',
|
||||
description: 'd',
|
||||
color: 'auto',
|
||||
});
|
||||
const config = manager.parseSubagentContent(
|
||||
'---\nname: a\ndescription: d\ncolor: auto\n---\nx',
|
||||
validConfig.filePath!,
|
||||
'project',
|
||||
);
|
||||
expect(config.color).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('serializeSubagent', () => {
|
||||
|
|
@ -895,44 +839,6 @@ You are an agent.
|
|||
expect(serialized).toContain('maxTurns: 25');
|
||||
});
|
||||
|
||||
it('should prune legacy runConfig.max_turns when top-level maxTurns is set', () => {
|
||||
// Avoid emitting two sources of truth: when the runtime already
|
||||
// promotes maxTurns to the top level, the on-disk file should not
|
||||
// also retain the legacy nested value.
|
||||
const serialized = manager.serializeSubagent({
|
||||
...validConfig,
|
||||
maxTurns: 42,
|
||||
runConfig: { max_turns: 10, max_time_minutes: 5 },
|
||||
});
|
||||
expect(serialized).toContain('maxTurns: 42');
|
||||
expect(serialized).toContain('max_time_minutes: 5');
|
||||
// The nested max_turns should NOT appear (top-level wins).
|
||||
const runConfigSection = serialized.match(
|
||||
/runConfig:[\s\S]*?(?=\n\w|\n---|$)/,
|
||||
);
|
||||
expect(runConfigSection?.[0]).not.toContain('max_turns:');
|
||||
});
|
||||
|
||||
it('should drop empty runConfig when only max_turns was nested', () => {
|
||||
const serialized = manager.serializeSubagent({
|
||||
...validConfig,
|
||||
maxTurns: 42,
|
||||
runConfig: { max_turns: 10 },
|
||||
});
|
||||
expect(serialized).toContain('maxTurns: 42');
|
||||
expect(serialized).not.toContain('runConfig:');
|
||||
});
|
||||
|
||||
it('should retain nested max_turns when top-level maxTurns is unset', () => {
|
||||
const serialized = manager.serializeSubagent({
|
||||
...validConfig,
|
||||
runConfig: { max_turns: 10 },
|
||||
});
|
||||
expect(serialized).not.toContain('maxTurns:');
|
||||
expect(serialized).toContain('runConfig:');
|
||||
expect(serialized).toContain('max_turns: 10');
|
||||
});
|
||||
|
||||
it('should not include new fields when undefined', () => {
|
||||
const serialized = manager.serializeSubagent(validConfig);
|
||||
expect(serialized).not.toContain('permissionMode:');
|
||||
|
|
|
|||
|
|
@ -50,7 +50,6 @@ import {
|
|||
COLOR_VALUES,
|
||||
isColor,
|
||||
isPermissionMode,
|
||||
parseBackground,
|
||||
parseMaxTurns,
|
||||
claudePermissionModeToApprovalMode,
|
||||
} from './agent-frontmatter-schema.js';
|
||||
|
|
@ -602,16 +601,7 @@ export class SubagentManager {
|
|||
}
|
||||
|
||||
if (config.runConfig) {
|
||||
// When `maxTurns` is promoted to the top level, prune the legacy
|
||||
// `runConfig.max_turns` so the on-disk frontmatter doesn't carry two
|
||||
// sources of truth (the runtime already prefers top-level).
|
||||
const { max_turns: _legacyMaxTurns, ...restRunConfig } =
|
||||
config.runConfig as { max_turns?: number } & Record<string, unknown>;
|
||||
const prunedRunConfig =
|
||||
config.maxTurns !== undefined ? restRunConfig : config.runConfig;
|
||||
if (Object.keys(prunedRunConfig).length > 0) {
|
||||
frontmatter['runConfig'] = prunedRunConfig;
|
||||
}
|
||||
frontmatter['runConfig'] = config.runConfig;
|
||||
}
|
||||
|
||||
if (config.color && config.color !== 'auto') {
|
||||
|
|
@ -1165,16 +1155,14 @@ function parseSubagentContent(
|
|||
| Record<string, unknown>
|
||||
| undefined;
|
||||
const colorRaw = frontmatter['color'];
|
||||
// CC silently drops colors outside the allowlist (_Y). qwen-code treats
|
||||
// the legacy `auto` sentinel as "no override" — it parses to undefined,
|
||||
// matches what the CLI `shouldShowColor` / `getColorForDisplay` helpers
|
||||
// already treat 'auto' as, and round-trips cleanly with the serializer's
|
||||
// existing omit-when-auto branch (parse → undefined → no emit → undefined).
|
||||
// CC silently drops colors outside the allowlist (_Y). Preserve the
|
||||
// legacy qwen `auto` sentinel for backward compat with existing files.
|
||||
const color =
|
||||
typeof colorRaw === 'string' && isColor(colorRaw) ? colorRaw : undefined;
|
||||
typeof colorRaw === 'string' && (isColor(colorRaw) || colorRaw === 'auto')
|
||||
? colorRaw
|
||||
: undefined;
|
||||
if (
|
||||
colorRaw !== undefined &&
|
||||
colorRaw !== 'auto' &&
|
||||
color === undefined &&
|
||||
typeof colorRaw === 'string'
|
||||
) {
|
||||
|
|
@ -1183,24 +1171,25 @@ function parseSubagentContent(
|
|||
);
|
||||
}
|
||||
const approvalModeRaw = frontmatter['approvalMode'];
|
||||
// approvalMode follows the same DL7-parity lenient posture as the rest of
|
||||
// the CC declarative-agent fields: warn-and-drop on invalid rather than
|
||||
// throw. This keeps a typo in `approvalMode` from killing the entire file
|
||||
// and lets a valid `permissionMode` still bridge into approvalMode below.
|
||||
const approvalMode =
|
||||
typeof approvalModeRaw === 'string' &&
|
||||
approvalModeRaw !== '' &&
|
||||
APPROVAL_MODES.includes(approvalModeRaw as never)
|
||||
? approvalModeRaw
|
||||
: undefined;
|
||||
if (
|
||||
approvalModeRaw !== undefined &&
|
||||
approvalModeRaw !== null &&
|
||||
approvalModeRaw !== '' &&
|
||||
approvalMode === undefined
|
||||
typeof approvalModeRaw !== 'string'
|
||||
) {
|
||||
debugLogger.warn(
|
||||
`Agent file ${filePath} has invalid approvalMode '${approvalModeRaw}'. Valid values: ${APPROVAL_MODES.join(', ')}. Dropping field.`,
|
||||
throw new Error(
|
||||
`Invalid "approvalMode" value: expected a string, got ${typeof approvalModeRaw}. Valid values: ${APPROVAL_MODES.join(', ')}`,
|
||||
);
|
||||
}
|
||||
const approvalMode =
|
||||
typeof approvalModeRaw === 'string' && approvalModeRaw !== ''
|
||||
? approvalModeRaw
|
||||
: undefined;
|
||||
if (
|
||||
approvalMode !== undefined &&
|
||||
!APPROVAL_MODES.includes(approvalMode as never)
|
||||
) {
|
||||
throw new Error(
|
||||
`Invalid "approvalMode" value "${approvalMode}". Valid values: ${APPROVAL_MODES.join(', ')}`,
|
||||
);
|
||||
}
|
||||
const model =
|
||||
|
|
@ -1211,17 +1200,19 @@ function parseSubagentContent(
|
|||
: undefined;
|
||||
|
||||
const backgroundRaw = frontmatter['background'];
|
||||
const background = parseBackground(backgroundRaw);
|
||||
if (
|
||||
backgroundRaw !== undefined &&
|
||||
backgroundRaw !== false &&
|
||||
backgroundRaw !== 'true' &&
|
||||
backgroundRaw !== 'false' &&
|
||||
background === undefined
|
||||
backgroundRaw !== true &&
|
||||
backgroundRaw !== false
|
||||
) {
|
||||
debugLogger.warn(
|
||||
`Agent file ${filePath} has invalid background value '${backgroundRaw}'. Must be 'true', 'false', or omitted.`,
|
||||
);
|
||||
}
|
||||
const background =
|
||||
backgroundRaw === 'true' || backgroundRaw === true ? true : undefined;
|
||||
|
||||
// --- CC 2.1.168 declarative-agent fields (DL7-parity lenient parse) ---
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue