Skip to content

fix(core): require integer stop hook cap #5667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Jun 23, 2026
Merged

fix(core): require integer stop hook cap #5667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/cli/src/config/settingsSchema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,16 @@ describe('SettingsSchema', () => {
expect(voiceModel.showInDialog).toBe(false);
});

it('should define stopHookBlockingCap schema override as a positive integer', () => {
expect(
getSettingsSchema().stopHookBlockingCap.jsonSchemaOverride,
).toEqual({
type: 'integer',
minimum: 1,
default: 8,
});
});

it('should have voice dictation settings under general', () => {
const voice =
getSettingsSchema().general.properties.voice.properties ?? {};
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2589,6 +2589,11 @@ const SETTINGS_SCHEMA = {
// This is an advanced safety valve for runaway hook loops, not a common
// interactive preference.
showInDialog: false,
jsonSchemaOverride: {
type: 'integer',
minimum: 1,
default: DEFAULT_STOP_HOOK_BLOCK_CAP,
},
},

hooks: {
Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/hooks/stopHookCap.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ describe('stop hook blocking cap', () => {
);
});

it('normalizes finite fractional values down to whole iterations', () => {
it('normalizes finite fractional config values down to whole iterations', () => {
expect(normalizeStopHookBlockingCap(3.7)).toBe(3);
expect(normalizeStopHookBlockingCap(100.9)).toBe(MAX_STOP_HOOK_BLOCK_CAP);
});

it('caps large finite values to avoid unbounded recursive Stop loops', () => {
it('caps large integer values to avoid unbounded recursive Stop loops', () => {
expect(normalizeStopHookBlockingCap(99999)).toBe(MAX_STOP_HOOK_BLOCK_CAP);
});

Expand All @@ -46,6 +46,16 @@ describe('stop hook blocking cap', () => {
expect(resolveStopHookBlockingCap(12)).toBe(3);
});

it('rejects fractional environment overrides', () => {
process.env[STOP_HOOK_BLOCK_CAP_ENV] = '1.5';

expect(resolveStopHookBlockingCap(12)).toBe(DEFAULT_STOP_HOOK_BLOCK_CAP);
});

it('preserves legacy fractional config values when no environment override is set', () => {
expect(resolveStopHookBlockingCap(3.7)).toBe(3);
});

it('ignores an empty environment override', () => {
process.env[STOP_HOOK_BLOCK_CAP_ENV] = '';

Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/hooks/stopHookCap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ export function normalizeStopHookBlockingCap(value: unknown): number {
: DEFAULT_STOP_HOOK_BLOCK_CAP;
}

function parseStopHookBlockingCapEnv(value: string): number {
const parsed = Number(value);

@doudouOUC doudouOUC Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Number() accepts hex ("0x10"), scientific notation ("1e2"), and binary ("0b10") as valid integers — inconsistent with every other integer env-var parser in the codebase.

Consider using the shared parsePositiveIntegerEnv helper from packages/core/src/utils/env.ts (used by 7+ other parsers: workflow-orchestrator.ts, workflow-budget.ts, coreToolScheduler.ts, background-tasks.ts, serve.ts, Session.ts, modelConfigResolver.ts). It enforces plain decimal integers via ^\d+$ regex + Number.isSafeInteger and removes the need for the private parseStopHookBlockingCapEnv function entirely:

import { parsePositiveIntegerEnv } from '../utils/env.js';

export function resolveStopHookBlockingCap(configValue?: number): number {
  const envValue = process.env[STOP_HOOK_BLOCK_CAP_ENV];
  if (envValue !== undefined && envValue.trim() !== '') {
    const parsed = parsePositiveIntegerEnv(envValue, 0);
    return normalizeStopHookBlockingCap(parsed);
  }
  return normalizeStopHookBlockingCap(configValue);
}

The fallback 0 works correctly because normalizeStopHookBlockingCap(0) maps to DEFAULT_STOP_HOOK_BLOCK_CAP (since 0 < 1).

— qwen3.7-max via Qwen Code /review

return Number.isInteger(parsed)
? normalizeStopHookBlockingCap(parsed)
: DEFAULT_STOP_HOOK_BLOCK_CAP;
}

export function resolveStopHookBlockingCap(configValue?: number): number {
const envValue = process.env[STOP_HOOK_BLOCK_CAP_ENV];
if (envValue !== undefined && envValue.trim() !== '') {
const parsed = Number(envValue);
return normalizeStopHookBlockingCap(parsed);
return parseStopHookBlockingCapEnv(envValue);
}

return normalizeStopHookBlockingCap(configValue);
Expand Down
7 changes: 4 additions & 3 deletions packages/vscode-ide-companion/schemas/settings.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,9 +1217,10 @@
"default": false
},
"stopHookBlockingCap": {
"description": "Maximum consecutive blocking Stop/SubagentStop hook decisions before Qwen Code overrides the hook loop and ends the turn. Can be overridden by QWEN_CODE_STOP_HOOK_BLOCK_CAP.",
"type": "number",
"default": 8
"type": "integer",
"minimum": 1,
"default": 8,
"description": "Maximum consecutive blocking Stop/SubagentStop hook decisions before Qwen Code overrides the hook loop and ends the turn. Can be overridden by QWEN_CODE_STOP_HOOK_BLOCK_CAP."
},
"hooks": {
"description": "Hook event configurations for extending CLI behavior at various lifecycle points.",
Expand Down
Loading