diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index 5a913a857c7..a34d798ecdf 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -422,6 +422,19 @@ describe('BackgroundTaskRegistry', () => { expect(MAX_CONCURRENT_BACKGROUND_AGENTS).toBeGreaterThanOrEqual(1); }); + it('rejects hex / scientific / non-decimal-integer overrides and falls back', () => { + // Number('0x10')=16, Number('1e2')=100 and Number('1.0')=1 all pass + // Number.isInteger, so the loose parse silently accepted them. The cap + // should only honor plain decimal integers, like the rest of the codebase. + for (const raw of ['0x10', '1e2', '1.0']) { + expect( + resolveMaxConcurrentBackgroundAgents({ + [BACKGROUND_AGENT_CONCURRENCY_ENV]: raw, + }), + ).toBe(DEFAULT_MAX_CONCURRENT_BACKGROUND_AGENTS); + } + }); + it('rejects new running background agents once the cap is reached', () => { registry = new BackgroundTaskRegistry({ maxConcurrentBackgroundAgents: 2, diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index f2a234b8f02..dd987ab826d 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -23,6 +23,7 @@ import { ToolConfirmationOutcome } from '../tools/tools.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { parsePositiveIntegerEnv } from '../utils/env.js'; import { escapeXml } from '../utils/xml.js'; import { patchAgentMeta } from './agent-transcript.js'; import { @@ -65,8 +66,10 @@ export function resolveMaxConcurrentBackgroundAgents( return DEFAULT_MAX_CONCURRENT_BACKGROUND_AGENTS; } - const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed < 1) { + // Parse through the shared helper so only plain decimal integers are + // accepted; Number() alone would let "0x10"/"1e2"/"1.0" slip through. + const parsed = parsePositiveIntegerEnv(raw, 0); + if (parsed < 1) { debugLogger.warn( `Invalid ${BACKGROUND_AGENT_CONCURRENCY_ENV}=${JSON.stringify(raw)}, ` + `using default (${DEFAULT_MAX_CONCURRENT_BACKGROUND_AGENTS})`, diff --git a/packages/core/src/agents/runtime/workflow-budget.test.ts b/packages/core/src/agents/runtime/workflow-budget.test.ts index f928a0f9507..74f78596086 100644 --- a/packages/core/src/agents/runtime/workflow-budget.test.ts +++ b/packages/core/src/agents/runtime/workflow-budget.test.ts @@ -48,6 +48,16 @@ describe('resolveMaxTokensPerWorkflow', () => { ).toBeNull(); }); + it('returns null on hex / scientific / non-decimal-integer overrides', () => { + // Number('0x2BF20')=180000, Number('1e6')=1000000, Number('5.0')=5 all + // pass Number.isInteger; only plain decimal integers should set a cap. + for (const raw of ['0x2BF20', '1e6', '5.0']) { + expect( + resolveMaxTokensPerWorkflow({ [MAX_TOKENS_PER_WORKFLOW_ENV]: raw }), + ).toBeNull(); + } + }); + it('returns null on zero / negative override', () => { expect( resolveMaxTokensPerWorkflow({ diff --git a/packages/core/src/agents/runtime/workflow-budget.ts b/packages/core/src/agents/runtime/workflow-budget.ts index 9d05cdf7dfb..d10ac4f3972 100644 --- a/packages/core/src/agents/runtime/workflow-budget.ts +++ b/packages/core/src/agents/runtime/workflow-budget.ts @@ -36,6 +36,7 @@ */ import { createDebugLogger } from '../../utils/debugLogger.js'; +import { parsePositiveIntegerEnv } from '../../utils/env.js'; import type { WorkflowBudget } from './workflow-sandbox.js'; const debugLogger = createDebugLogger('WORKFLOW_BUDGET'); @@ -74,8 +75,10 @@ export function resolveMaxTokensPerWorkflow( if (raw === undefined || raw.trim() === '') { return null; } - const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed < 1) { + // Parse through the shared helper so only plain decimal integers are + // accepted; Number() alone would let "0x2BF20"/"1e6"/"5.0" slip through. + const parsed = parsePositiveIntegerEnv(raw, 0); + if (parsed < 1) { debugLogger.warn( `Invalid ${MAX_TOKENS_PER_WORKFLOW_ENV}=${JSON.stringify(raw)}, ` + `treating as unset (no cap)`, diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index 169c00f19ae..629366ba618 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -756,7 +756,10 @@ describe('WorkflowOrchestrator', () => { ref: string | { scriptPath: string }, ) => { expect(ref).toBe('child'); - return { script: `return 'nested-' + (await agent('inner'));`, name: 'child' }; + return { + script: `return 'nested-' + (await agent('inner'));`, + name: 'child', + }; }; const outcome = await orchestrator.run({ script: `const r = await workflow('child'); return 'parent:' + r;`, @@ -808,7 +811,8 @@ describe('WorkflowOrchestrator', () => { expect(dispatchCalls).toBe(2); expect(String(caught)).toMatch(/exceeded the maximum of 2 agent/); } finally { - if (prev === undefined) delete process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS']; + if (prev === undefined) + delete process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS']; else process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS'] = prev; } }); @@ -1058,7 +1062,8 @@ describe('WorkflowOrchestrator', () => { }); expect(outcome.result).toBe('ok'); // no cap error despite 3 > 2 } finally { - if (prev === undefined) delete process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS']; + if (prev === undefined) + delete process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS']; else process.env['QWEN_CODE_MAX_WORKFLOW_AGENTS'] = prev; } }); @@ -1634,6 +1639,29 @@ describe('WorkflowOrchestrator P2 — parallel() / pipeline() / caps', () => { ).toBe(DEFAULT_MAX_AGENTS_PER_RUN); }); + it('resolveMaxAgentsPerRun rejects hex / scientific / non-decimal-integer overrides', () => { + // Number('0x10')=16, Number('1e3')=1000, Number('1.0')=1 pass + // Number.isInteger; only plain decimal integers should override the cap. + for (const raw of ['0x10', '1e3', '1.0']) { + expect( + resolveMaxAgentsPerRun({ QWEN_CODE_MAX_WORKFLOW_AGENTS: raw }), + ).toBe(DEFAULT_MAX_AGENTS_PER_RUN); + } + }); + + it('resolveConcurrencyLimit treats hex / scientific overrides as invalid (cpu default)', () => { + // An invalid override falls back to the cpu-derived default in [1,16]; + // 0x10/1e2 must be rejected too, not parsed as 16/100. + const cpuDefault = resolveConcurrencyLimit({ + QWEN_CODE_MAX_WORKFLOW_CONCURRENCY: '-1', + }); + for (const raw of ['0x10', '1e2']) { + expect( + resolveConcurrencyLimit({ QWEN_CODE_MAX_WORKFLOW_CONCURRENCY: raw }), + ).toBe(cpuDefault); + } + }); + // PR #4947 R1 T4 (wenshao): an env override above the hard ceiling must // be clamped, not honored — protects operators from a fat-finger // QWEN_CODE_MAX_WORKFLOW_AGENTS=999999999 silently uncapping the run. diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 0ed9b669054..6cbd33d1fca 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -32,6 +32,7 @@ import type { } from './agent-events.js'; import { ToolNames } from '../../tools/tool-names.js'; import { createConcurrencyLimiter } from '../../utils/concurrencyLimiter.js'; +import { parsePositiveIntegerEnv } from '../../utils/env.js'; import { stripAnsiAndControl } from '../../utils/textUtils.js'; import type { SubagentConfig } from '../../subagents/types.js'; import { @@ -76,8 +77,10 @@ export function resolveMaxAgentsPerRun( if (raw === undefined || raw.trim() === '') { return DEFAULT_MAX_AGENTS_PER_RUN; } - const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed < 1) { + // Parse through the shared helper so only plain decimal integers are + // accepted; Number() alone would let "0x10"/"1e3"/"1.0" slip through. + const parsed = parsePositiveIntegerEnv(raw, 0); + if (parsed < 1) { debugLogger.warn( `Invalid ${MAX_WORKFLOW_AGENTS_ENV}=${JSON.stringify(raw)}, ` + `using default (${DEFAULT_MAX_AGENTS_PER_RUN})`, @@ -117,8 +120,10 @@ export function resolveConcurrencyLimit( ): number { const raw = env[MAX_WORKFLOW_CONCURRENCY_ENV]; if (raw !== undefined && raw.trim() !== '') { - const parsed = Number(raw); - if (Number.isInteger(parsed) && parsed >= 1) { + // Parse through the shared helper so only plain decimal integers are + // accepted; Number() alone would let "0x10"/"1e2"/"1.0" slip through. + const parsed = parsePositiveIntegerEnv(raw, 0); + if (parsed >= 1) { if (parsed > HARD_MAX_CONCURRENCY_CEILING) { debugLogger.warn( `${MAX_WORKFLOW_CONCURRENCY_ENV}=${parsed} exceeds hard ceiling ` + @@ -369,7 +374,14 @@ export function createProductionDispatch( ); return runStallResilient( (attemptSignal, emitter) => - runSingleDispatch(config, prompt, opts, attemptSignal, emitter, onTokens), + runSingleDispatch( + config, + prompt, + opts, + attemptSignal, + emitter, + onTokens, + ), { stallMs, signal, @@ -711,7 +723,8 @@ async function runOverridePath( // no emitter was passed (legacy / direct callers), fall back to a fresh // emitter only in schema mode (the watchdog is then absent, matching the // pre-P-stall behaviour for direct override-path callers). - const eventEmitter = emitter ?? (schemaState ? new AgentEventEmitter() : undefined); + const eventEmitter = + emitter ?? (schemaState ? new AgentEventEmitter() : undefined); if (schemaState && eventEmitter) { attachSchemaListeners(eventEmitter, schemaState); } @@ -1336,7 +1349,8 @@ export class WorkflowOrchestrator { `respawn(s) for runId=${runId}.`, ); } - const label = typeof opts.label === 'string' ? opts.label : undefined; + const label = + typeof opts.label === 'string' ? opts.label : undefined; try { emitter?.agentDispatched?.(label); } catch (e) {