From b469493b2a6adea650209f7fc83e6c8427c53dfa Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 16 May 2026 20:44:06 +0800 Subject: [PATCH 1/8] feat(core): add stop hook blocking cap --- .../src/acp-integration/session/Session.ts | 37 +++++++----- packages/cli/src/config/config.ts | 1 + packages/cli/src/config/settingsSchema.ts | 12 ++++ .../agents/background-agent-resume.test.ts | 1 + .../src/agents/background-agent-resume.ts | 8 ++- packages/core/src/config/config.ts | 14 +++++ packages/core/src/core/client.test.ts | 60 +++++++++++++++++++ packages/core/src/core/client.ts | 22 +++++++ packages/core/src/goals/goalHook.ts | 19 ++++++ packages/core/src/goals/index.ts | 1 + packages/core/src/hooks/stopHookCap.test.ts | 45 ++++++++++++++ packages/core/src/hooks/stopHookCap.ts | 35 +++++++++++ packages/core/src/index.ts | 7 +++ packages/core/src/tools/agent/agent.test.ts | 32 ++++++++++ packages/core/src/tools/agent/agent.ts | 8 ++- .../schemas/settings.schema.json | 5 ++ 16 files changed, 288 insertions(+), 19 deletions(-) create mode 100644 packages/core/src/hooks/stopHookCap.test.ts create mode 100644 packages/core/src/hooks/stopHookCap.ts diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index e02d5a73568..4441ca27fcb 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -58,6 +58,8 @@ import { evaluatePermissionFlow, needsConfirmation, isPlanModeBlocked, + abortGoalForStopHookCap, + formatStopHookBlockingCapWarning, } from '@qwen-code/qwen-code-core'; import { getCommandSubcommandNames } from '../../services/commandMetadata.js'; import { getEffectiveSupportedModes } from '../../services/commandUtils.js'; @@ -693,11 +695,11 @@ export class Session implements SessionContext { hooksEnabled: boolean, messageBus: MessageBus | undefined, ): Promise<{ stopReason: PromptResponse['stopReason'] }> { - const MAX_STOP_HOOK_ITERATIONS = 100; + const stopHookBlockingCap = this.config.getStopHookBlockingCap(); let stopHookIterationCount = 0; let stopHookReasons: string[] = []; - while (stopHookIterationCount < MAX_STOP_HOOK_ITERATIONS) { + while (stopHookIterationCount < stopHookBlockingCap) { if ( !hooksEnabled || !messageBus || @@ -761,13 +763,25 @@ export class Session implements SessionContext { stopHookIterationCount++; stopHookReasons = [...stopHookReasons, continueReason]; - // Emit StopHookLoop event for iterations after the first one - if (stopHookIterationCount > 1) { - await this.messageEmitter.emitStopHookLoop( - stopHookIterationCount, - stopHookReasons, - response.stopHookCount ?? 1, + await this.messageEmitter.emitStopHookLoop( + stopHookIterationCount, + stopHookReasons, + response.stopHookCount ?? 1, + ); + + if (stopHookIterationCount >= stopHookBlockingCap) { + const warning = formatStopHookBlockingCapWarning( + 'Stop', + stopHookBlockingCap, + ); + abortGoalForStopHookCap( + this.config, + this.config.getSessionId(), + warning, ); + await this.messageEmitter.emitAgentMessage(warning); + debugLogger.warn(warning); + return { stopReason: 'end_turn' }; } // Continue the conversation with the hook's reason @@ -904,13 +918,6 @@ export class Session implements SessionContext { break; } - // If we exceeded max iterations, log a warning but still end gracefully - if (stopHookIterationCount >= MAX_STOP_HOOK_ITERATIONS) { - debugLogger.warn( - `Stop hook loop reached maximum iterations (${MAX_STOP_HOOK_ITERATIONS}), forcing stop`, - ); - } - return { stopReason: 'end_turn' }; } diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 1a56c3edceb..5c6ecdf4051 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1708,6 +1708,7 @@ export async function loadCliConfig( projectHooks: bareMode ? undefined : hooksConfig?.projectHooks, hooks: bareMode ? undefined : settings.hooks, // Keep for backward compatibility disableAllHooks: bareMode ? true : (settings.disableAllHooks ?? false), + stopHookBlockingCap: bareMode ? undefined : settings.stopHookBlockingCap, channel: argv.channel, // CLI flag wins over settings.json. `--json-fd` is fd-only (no settings // equivalent — fd passing is a spawn-time concern). `--json-file` and diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index e44b720c124..c2837b9a38e 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -14,6 +14,7 @@ import type { } from '@qwen-code/qwen-code-core'; import { ApprovalMode, + DEFAULT_STOP_HOOK_BLOCK_CAP, DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES, DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD, } from '@qwen-code/qwen-code-core'; @@ -1950,6 +1951,17 @@ const SETTINGS_SCHEMA = { showInDialog: false, }, + stopHookBlockingCap: { + type: 'number', + label: 'Stop Hook Blocking Cap', + category: 'Advanced', + requiresRestart: true, + default: DEFAULT_STOP_HOOK_BLOCK_CAP, + 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.', + showInDialog: false, + }, + hooks: { type: 'object', label: 'Hooks', diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index a0020c616e5..fb8e9af18b5 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -82,6 +82,7 @@ describe('BackgroundAgentResumeService', () => { getMonitorRegistry: () => monitorRegistry, getSubagentManager: () => subagentManager, getHookSystem: () => hookSystem, + getStopHookBlockingCap: () => 8, getApprovalMode: () => 'default', isTrustedFolder: () => true, getProjectRoot: () => tempDir, diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index a4906f24013..d2a244dd8ec 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -27,6 +27,7 @@ import type { ChatRecord } from '../services/chatRecordingService.js'; import { getInitialChatHistory } from '../utils/environmentContext.js'; import { getGitBranch } from '../utils/gitUtils.js'; import { PermissionMode, type StopHookOutput } from '../hooks/types.js'; +import { formatStopHookBlockingCapWarning } from '../hooks/stopHookCap.js'; import { runWithAgentContext } from './runtime/agent-context.js'; import { createApprovalModeOverride } from '../tools/agent/agent.js'; import type { ApprovalMode } from '../config/config.js'; @@ -982,7 +983,7 @@ export class BackgroundAgentResumeService { const hookSystem = this.config.getHookSystem(); if (!hookSystem) return; let stopHookActive = false; - const maxIterations = 5; + const maxIterations = this.config.getStopHookBlockingCap(); for (let i = 0; i < maxIterations; i++) { try { @@ -1022,7 +1023,10 @@ export class BackgroundAgentResumeService { } debugLogger.warn( - `[BackgroundAgentResume] SubagentStop hook reached maximum iterations (${maxIterations}), forcing stop`, + `[BackgroundAgentResume] ${formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, + )}`, ); } } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index acef4a9dfd7..a34cd85873c 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -74,6 +74,7 @@ import { MonitorRegistry } from '../services/monitorRegistry.js'; import { BackgroundAgentResumeService } from '../agents/background-agent-resume.js'; import { BackgroundShellRegistry } from '../services/backgroundShellRegistry.js'; import { FileReadCache } from '../services/fileReadCache.js'; +import { resolveStopHookBlockingCap } from '../hooks/stopHookCap.js'; import { DEFAULT_OTLP_ENDPOINT, DEFAULT_TELEMETRY_TARGET, @@ -592,6 +593,11 @@ export interface ConfigParameters { * to use disableAllHooks instead (note: inverted logic - enabled:true → disableAllHooks:false). */ disableAllHooks?: boolean; + /** + * Maximum consecutive blocking Stop/SubagentStop hook decisions before the + * runtime overrides the hook loop and allows the turn to end. + */ + stopHookBlockingCap?: number; /** * User-level hooks configuration (from user settings). * These hooks are always loaded regardless of folder trust status. @@ -818,6 +824,7 @@ export class Config { private readonly enableAutoSkill: boolean; private fastModel?: string; private readonly disableAllHooks: boolean; + private readonly stopHookBlockingCap: number; /** User-level hooks (always loaded regardless of trust) */ private readonly userHooks?: Record; /** Project-level hooks (only loaded in trusted folders) */ @@ -1020,6 +1027,9 @@ export class Config { this.enableAutoSkill = params.enableAutoSkill ?? false; this.fastModel = params.fastModel || undefined; this.disableAllHooks = params.disableAllHooks ?? false; + this.stopHookBlockingCap = resolveStopHookBlockingCap( + params.stopHookBlockingCap, + ); // Store user and project hooks separately for proper source attribution this.userHooks = params.userHooks; this.projectHooks = params.projectHooks; @@ -2653,6 +2663,10 @@ export class Config { return this.disableAllHooks || this.getBareMode(); } + getStopHookBlockingCap(): number { + return this.stopHookBlockingCap; + } + getManagedAutoMemoryEnabled(): boolean { return this.enableManagedAutoMemory && !this.getBareMode(); } diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 044625c51ec..e728eb214f1 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -510,6 +510,7 @@ describe('Gemini Client (client.ts)', () => { getResolvedModel: vi.fn().mockReturnValue(undefined), }), getDisableAllHooks: vi.fn().mockReturnValue(true), + getStopHookBlockingCap: vi.fn().mockReturnValue(8), getArenaManager: vi.fn().mockReturnValue(null), getMessageBus: vi.fn().mockReturnValue(undefined), hasHooksForEvent: vi.fn().mockReturnValue(false), @@ -4332,6 +4333,65 @@ Other open files: expect(mockMessageBus.request).not.toHaveBeenCalled(); }); + it('ends the Stop hook loop when the blocking cap is reached', async () => { + const mockMessageBus = { + request: vi.fn().mockResolvedValue({ + output: { + decision: 'block', + reason: 'Keep working', + }, + stopHookCount: 1, + }), + response: vi.fn(), + }; + vi.mocked(mockConfig.getDisableAllHooks).mockReturnValue(false); + vi.mocked(mockConfig.getMessageBus).mockReturnValue( + mockMessageBus as unknown as ReturnType, + ); + vi.mocked(mockConfig.hasHooksForEvent).mockImplementation( + (event: string) => event === 'Stop', + ); + vi.mocked(mockConfig.getStopHookBlockingCap).mockReturnValue(1); + + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([ + { + role: 'model', + parts: [{ text: 'not done' }], + }, + ]), + } as unknown as GeminiChat; + mockTurnRunFn.mockReturnValue( + (async function* () { + yield { type: GeminiEventType.Content, value: 'not done' }; + })(), + ); + + const events = await fromAsync( + client.sendMessageStream( + [{ text: 'Hi' }], + new AbortController().signal, + 'prompt-stop-cap', + ), + ); + + expect(mockTurnRunFn).toHaveBeenCalledTimes(1); + expect(events).toContainEqual({ + type: GeminiEventType.StopHookLoop, + value: { + iterationCount: 1, + reasons: ['Keep working'], + stopHookCount: 1, + }, + }); + expect(events).toContainEqual({ + type: GeminiEventType.HookSystemMessage, + value: + 'Stop hook blocked continuation 1 consecutive times; overriding and ending the turn.', + }); + }); + it('should not skip hooks when hasHooksForEvent returns true', async () => { const mockMessageBus = { request: vi.fn().mockResolvedValue({ modifiedPrompt: undefined }), diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 36efecd088b..548a824bd23 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -20,6 +20,8 @@ import { createDebugLogger } from '../utils/debugLogger.js'; import { recordStartupEvent } from '../utils/startupEventSink.js'; import { microcompactHistory } from '../services/microcompaction/microcompact.js'; import { getActiveGoal } from '../goals/activeGoalStore.js'; +import { abortGoalForStopHookCap } from '../goals/goalHook.js'; +import { formatStopHookBlockingCapWarning } from '../hooks/stopHookCap.js'; const debugLogger = createDebugLogger('CLIENT'); @@ -1577,6 +1579,26 @@ export class GeminiClient { }, }; + const stopHookBlockingCap = this.config.getStopHookBlockingCap(); + if (currentIterationCount >= stopHookBlockingCap) { + const warning = formatStopHookBlockingCapWarning( + 'Stop', + stopHookBlockingCap, + ); + abortGoalForStopHookCap( + this.config, + this.config.getSessionId(), + warning, + ); + yield { + type: GeminiEventType.HookSystemMessage, + value: warning, + }; + debugLogger.warn(warning); + if (isTopLevelInteraction) endInteractionSpan('ok'); + return turn; + } + const continueRequest = [{ text: continueReason }]; const activeGoal = getActiveGoal(this.config.getSessionId()); const hookTurnBudget = activeGoal ? boundedTurns : boundedTurns - 1; diff --git a/packages/core/src/goals/goalHook.ts b/packages/core/src/goals/goalHook.ts index 0e452249e44..d64ac74b8fa 100644 --- a/packages/core/src/goals/goalHook.ts +++ b/packages/core/src/goals/goalHook.ts @@ -120,6 +120,25 @@ function finishGoal( clearGoalTerminalObserver(sessionId); } +export function abortGoalForStopHookCap( + config: Config, + sessionId: string, + systemMessage: string, +): boolean { + const goal = getActiveGoal(sessionId); + if (!goal) return false; + + finishGoal(config, sessionId, goal, { + kind: 'aborted', + condition: goal.condition, + iterations: goal.iterations, + durationMs: Date.now() - goal.setAt, + lastReason: goal.lastReason, + systemMessage, + }); + return true; +} + /** * Builds the Function hook callback that, on every Stop event, asks a fast * model whether the goal condition holds. diff --git a/packages/core/src/goals/index.ts b/packages/core/src/goals/index.ts index af0930ad75c..eb382b58b95 100644 --- a/packages/core/src/goals/index.ts +++ b/packages/core/src/goals/index.ts @@ -27,6 +27,7 @@ export { GOAL_HOOK_TIMEOUT_MS, GOAL_HOOK_TIMEOUT_SECONDS, createGoalStopHookCallback, + abortGoalForStopHookCap, registerGoalHook, unregisterGoalHook, } from './goalHook.js'; diff --git a/packages/core/src/hooks/stopHookCap.test.ts b/packages/core/src/hooks/stopHookCap.test.ts new file mode 100644 index 00000000000..0bdaf0afefc --- /dev/null +++ b/packages/core/src/hooks/stopHookCap.test.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import { + DEFAULT_STOP_HOOK_BLOCK_CAP, + STOP_HOOK_BLOCK_CAP_ENV, + formatStopHookBlockingCapWarning, + normalizeStopHookBlockingCap, + resolveStopHookBlockingCap, +} from './stopHookCap.js'; + +describe('stop hook blocking cap', () => { + afterEach(() => { + delete process.env[STOP_HOOK_BLOCK_CAP_ENV]; + }); + + it('normalizes invalid values to the default cap', () => { + expect(normalizeStopHookBlockingCap(undefined)).toBe( + DEFAULT_STOP_HOOK_BLOCK_CAP, + ); + expect(normalizeStopHookBlockingCap(0)).toBe(DEFAULT_STOP_HOOK_BLOCK_CAP); + expect(normalizeStopHookBlockingCap(Number.NaN)).toBe( + DEFAULT_STOP_HOOK_BLOCK_CAP, + ); + }); + + it('prefers the environment override over config', () => { + process.env[STOP_HOOK_BLOCK_CAP_ENV] = '3'; + + expect(resolveStopHookBlockingCap(12)).toBe(3); + }); + + it('formats warnings for the relevant hook event', () => { + expect(formatStopHookBlockingCapWarning('Stop', 8)).toBe( + 'Stop hook blocked continuation 8 consecutive times; overriding and ending the turn.', + ); + expect(formatStopHookBlockingCapWarning('SubagentStop', 2)).toContain( + 'SubagentStop hook blocked continuation 2 consecutive times', + ); + }); +}); diff --git a/packages/core/src/hooks/stopHookCap.ts b/packages/core/src/hooks/stopHookCap.ts new file mode 100644 index 00000000000..de2158a9560 --- /dev/null +++ b/packages/core/src/hooks/stopHookCap.ts @@ -0,0 +1,35 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export const DEFAULT_STOP_HOOK_BLOCK_CAP = 8; +export const STOP_HOOK_BLOCK_CAP_ENV = 'QWEN_CODE_STOP_HOOK_BLOCK_CAP'; + +export function normalizeStopHookBlockingCap(value: unknown): number { + if (typeof value !== 'number' || !Number.isFinite(value)) { + return DEFAULT_STOP_HOOK_BLOCK_CAP; + } + + const normalized = Math.floor(value); + return normalized >= 1 ? normalized : DEFAULT_STOP_HOOK_BLOCK_CAP; +} + +export function resolveStopHookBlockingCap(configValue?: number): number { + const envValue = process.env[STOP_HOOK_BLOCK_CAP_ENV]; + if (envValue !== undefined) { + const parsed = Number(envValue); + return normalizeStopHookBlockingCap(parsed); + } + + return normalizeStopHookBlockingCap(configValue); +} + +export function formatStopHookBlockingCapWarning( + hookLabel: 'Stop' | 'SubagentStop', + cap: number, +): string { + const hookName = hookLabel === 'Stop' ? 'Stop hook' : 'SubagentStop hook'; + return `${hookName} blocked continuation ${cap} consecutive times; overriding and ending the turn.`; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7e7a29afca0..e068a589f8a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -357,6 +357,13 @@ export * from './test-utils/index.js'; export * from './hooks/types.js'; export { HookSystem, HookRegistry } from './hooks/index.js'; export type { HookRegistryEntry, SessionHookEntry } from './hooks/index.js'; +export { + DEFAULT_STOP_HOOK_BLOCK_CAP, + STOP_HOOK_BLOCK_CAP_ENV, + normalizeStopHookBlockingCap, + resolveStopHookBlockingCap, + formatStopHookBlockingCapWarning, +} from './hooks/stopHookCap.js'; export { type StopFailureErrorType } from './hooks/types.js'; // ============================================================================ diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index f5a7396ee1b..eb694e474a5 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -138,6 +138,7 @@ describe('AgentTool', () => { getSubagentManager: vi.fn(), getGeminiClient: vi.fn().mockReturnValue(undefined), getHookSystem: vi.fn().mockReturnValue(undefined), + getStopHookBlockingCap: vi.fn().mockReturnValue(8), getTranscriptPath: vi.fn().mockReturnValue('/test/transcript'), getApprovalMode: vi.fn().mockReturnValue('default'), isTrustedFolder: vi.fn().mockReturnValue(true), @@ -1295,6 +1296,37 @@ describe('AgentTool', () => { expect(mockAgent.execute).toHaveBeenCalledTimes(2); }); + it('uses the configured SubagentStop blocking cap', async () => { + ( + config as unknown as { + getStopHookBlockingCap: ReturnType; + } + ).getStopHookBlockingCap.mockReturnValue(2); + const mockBlockOutput = { + isBlockingDecision: vi.fn().mockReturnValue(true), + shouldStopExecution: vi.fn().mockReturnValue(false), + getEffectiveReason: vi.fn().mockReturnValue('Keep working'), + }; + + vi.mocked(mockHookSystem.fireSubagentStopEvent).mockResolvedValue( + mockBlockOutput as never, + ); + + const params: AgentParams = { + description: 'Search files', + prompt: 'Find all TypeScript files', + subagent_type: 'file-search', + }; + + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation(params); + await invocation.execute(); + + expect(mockHookSystem.fireSubagentStopEvent).toHaveBeenCalledTimes(2); + expect(mockAgent.execute).toHaveBeenCalledTimes(3); + }); + it('should allow stop when SubagentStop hook fails', async () => { vi.mocked(mockHookSystem.fireSubagentStopEvent).mockRejectedValue( new Error('Stop hook failed'), diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 77453fc5a58..39d00f08ad4 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -69,6 +69,7 @@ import { BuiltinAgentRegistry } from '../../subagents/builtin-agents.js'; import { createDebugLogger } from '../../utils/debugLogger.js'; import { PermissionMode } from '../../hooks/types.js'; import type { StopHookOutput } from '../../hooks/types.js'; +import { formatStopHookBlockingCapWarning } from '../../hooks/stopHookCap.js'; import { ApprovalMode } from '../../config/config.js'; import { getAgentJsonlPath, @@ -973,7 +974,7 @@ class AgentToolInvocation extends BaseToolInvocation { const effectiveTranscriptPath = transcriptPath ?? this.config.getTranscriptPath(); let stopHookActive = false; - const maxIterations = 5; + const maxIterations = this.config.getStopHookBlockingCap(); for (let i = 0; i < maxIterations; i++) { try { @@ -1014,7 +1015,10 @@ class AgentToolInvocation extends BaseToolInvocation { } debugLogger.warn( - `[Agent] SubagentStop hook reached maximum iterations (${maxIterations}), forcing stop`, + `[Agent] ${formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, + )}`, ); } diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index c21de8b9aa3..6b93c16146f 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -896,6 +896,11 @@ "type": "boolean", "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 + }, "hooks": { "description": "Hook event configurations for extending CLI behavior at various lifecycle points.", "type": "object", From 3aec63195e8ab946e8104e7da84c9313f25c78f0 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 16 May 2026 21:38:49 +0800 Subject: [PATCH 2/8] fix(core): tighten stop hook cap behavior --- .../acp-integration/session/Session.test.ts | 1 + .../src/acp-integration/session/Session.ts | 14 +++++----- packages/cli/src/config/settingsSchema.ts | 2 ++ packages/core/src/core/client.test.ts | 13 ++++----- packages/core/src/core/client.ts | 27 +++++++------------ packages/core/src/hooks/stopHookCap.test.ts | 5 ++++ packages/core/src/hooks/stopHookCap.ts | 3 +++ 7 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index e262db31fe7..1f3a8c8cd64 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -163,6 +163,7 @@ describe('Session', () => { getAuthType: vi.fn().mockImplementation(() => currentAuthType), isCronEnabled: vi.fn().mockReturnValue(false), getSessionTokenLimit: vi.fn().mockReturnValue(0), + getStopHookBlockingCap: vi.fn().mockReturnValue(8), getGeminiClient: vi.fn().mockReturnValue(mockGeminiClient), } as unknown as Config; diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 4441ca27fcb..de68a101366 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -763,12 +763,6 @@ export class Session implements SessionContext { stopHookIterationCount++; stopHookReasons = [...stopHookReasons, continueReason]; - await this.messageEmitter.emitStopHookLoop( - stopHookIterationCount, - stopHookReasons, - response.stopHookCount ?? 1, - ); - if (stopHookIterationCount >= stopHookBlockingCap) { const warning = formatStopHookBlockingCapWarning( 'Stop', @@ -784,6 +778,14 @@ export class Session implements SessionContext { return { stopReason: 'end_turn' }; } + if (stopHookIterationCount > 1) { + await this.messageEmitter.emitStopHookLoop( + stopHookIterationCount, + stopHookReasons, + response.stopHookCount ?? 1, + ); + } + // Continue the conversation with the hook's reason const continueParts: Part[] = [{ text: continueReason }]; let nextMessage: Content | null = { diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index c2837b9a38e..de10f15094c 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1959,6 +1959,8 @@ const SETTINGS_SCHEMA = { default: DEFAULT_STOP_HOOK_BLOCK_CAP, 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.', + // This is an advanced safety valve for runaway hook loops, not a common + // interactive preference. showInDialog: false, }, diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index e728eb214f1..22642a4beeb 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -4377,14 +4377,11 @@ Other open files: ); expect(mockTurnRunFn).toHaveBeenCalledTimes(1); - expect(events).toContainEqual({ - type: GeminiEventType.StopHookLoop, - value: { - iterationCount: 1, - reasons: ['Keep working'], - stopHookCount: 1, - }, - }); + expect(events).not.toContainEqual( + expect.objectContaining({ + type: GeminiEventType.StopHookLoop, + }), + ); expect(events).toContainEqual({ type: GeminiEventType.HookSystemMessage, value: diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 548a824bd23..175f40f5226 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -1561,24 +1561,6 @@ export class GeminiClient { continueReason, ]; - // Emit StopHookLoop on EVERY blocking Stop hook execution, including - // the first. `currentIterationCount === 1` means a Stop hook just - // fired once and produced a blocking decision — the user benefits - // from seeing that immediately (e.g. `/goal` rendering "Goal check: - // not yet met" on the very first not-met turn, or a configured Stop - // hook surfacing its blocking reason before the agent attempts a - // retry). The previous `>1` guard hid the first reason until the - // hook fired twice, which made /goal's first iteration invisible - // and delayed visibility for regular hooks that only fire once. - yield { - type: GeminiEventType.StopHookLoop, - value: { - iterationCount: currentIterationCount, - reasons: currentReasons, - stopHookCount: response.stopHookCount ?? 1, - }, - }; - const stopHookBlockingCap = this.config.getStopHookBlockingCap(); if (currentIterationCount >= stopHookBlockingCap) { const warning = formatStopHookBlockingCapWarning( @@ -1599,6 +1581,15 @@ export class GeminiClient { return turn; } + yield { + type: GeminiEventType.StopHookLoop, + value: { + iterationCount: currentIterationCount, + reasons: currentReasons, + stopHookCount: response.stopHookCount ?? 1, + }, + }; + const continueRequest = [{ text: continueReason }]; const activeGoal = getActiveGoal(this.config.getSessionId()); const hookTurnBudget = activeGoal ? boundedTurns : boundedTurns - 1; diff --git a/packages/core/src/hooks/stopHookCap.test.ts b/packages/core/src/hooks/stopHookCap.test.ts index 0bdaf0afefc..6f2e96162a3 100644 --- a/packages/core/src/hooks/stopHookCap.test.ts +++ b/packages/core/src/hooks/stopHookCap.test.ts @@ -23,11 +23,16 @@ describe('stop hook blocking cap', () => { DEFAULT_STOP_HOOK_BLOCK_CAP, ); expect(normalizeStopHookBlockingCap(0)).toBe(DEFAULT_STOP_HOOK_BLOCK_CAP); + expect(normalizeStopHookBlockingCap(-1)).toBe(DEFAULT_STOP_HOOK_BLOCK_CAP); expect(normalizeStopHookBlockingCap(Number.NaN)).toBe( DEFAULT_STOP_HOOK_BLOCK_CAP, ); }); + it('normalizes finite fractional values down to whole iterations', () => { + expect(normalizeStopHookBlockingCap(3.7)).toBe(3); + }); + it('prefers the environment override over config', () => { process.env[STOP_HOOK_BLOCK_CAP_ENV] = '3'; diff --git a/packages/core/src/hooks/stopHookCap.ts b/packages/core/src/hooks/stopHookCap.ts index de2158a9560..00ec2daa2a2 100644 --- a/packages/core/src/hooks/stopHookCap.ts +++ b/packages/core/src/hooks/stopHookCap.ts @@ -30,6 +30,9 @@ export function formatStopHookBlockingCapWarning( hookLabel: 'Stop' | 'SubagentStop', cap: number, ): string { + // Only Stop and SubagentStop hooks can request continuation after the + // model or subagent would otherwise finish, so keep user-facing labels + // explicit instead of accepting arbitrary hook names. const hookName = hookLabel === 'Stop' ? 'Stop hook' : 'SubagentStop hook'; return `${hookName} blocked continuation ${cap} consecutive times; overriding and ending the turn.`; } From a7ea5148898097247a23cab5225e0b150158cf00 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 16 May 2026 21:52:15 +0800 Subject: [PATCH 3/8] fix(cli): show goal judge details --- .../cli/src/ui/commands/goalCommand.test.ts | 11 +++----- packages/cli/src/ui/commands/goalCommand.ts | 19 +++++-------- .../messages/GoalStatusMessage.test.tsx | 28 +++++++++++++++++++ .../components/messages/GoalStatusMessage.tsx | 18 ++++++++---- packages/cli/src/ui/hooks/useGeminiStream.ts | 17 ++++------- packages/cli/src/ui/types.ts | 2 +- packages/core/src/goals/activeGoalStore.ts | 9 ++---- packages/core/src/goals/goalHook.ts | 12 +------- packages/core/src/goals/goalJudge.ts | 22 ++++++--------- 9 files changed, 71 insertions(+), 67 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx diff --git a/packages/cli/src/ui/commands/goalCommand.test.ts b/packages/cli/src/ui/commands/goalCommand.test.ts index d92cbc1d66d..0b44f2d0116 100644 --- a/packages/cli/src/ui/commands/goalCommand.test.ts +++ b/packages/cli/src/ui/commands/goalCommand.test.ts @@ -247,13 +247,10 @@ describe('goalCommand', () => { expect(content).toMatch(/Last check: transcript shows completion/); }); - it('strict claude alignment: `/goal clear` with no active goal does NOT dismiss the achievement summary', async () => { - // Claude Code's `woH` bails (`q.length===0 → return null`) when no active - // goal exists — it does NOT write a dismissal sentinel and does NOT wipe - // the cache. Subsequent empty `/goal` still surfaces the previous - // achievement via `findLastTerminalGoal`. We pin this behavior to prevent - // accidental divergence; users who want a true "forget" will need a - // separate dedicated keyword (out of scope for this alignment). + it('keeps the latest terminal summary when `/goal clear` has no active goal', async () => { + // A no-op clear should not write a dismissal sentinel or wipe the cache. + // Subsequent empty `/goal` still surfaces the previous achievement + // summary. const ctx = createMockCommandContext({ services: { config: makeConfig() as unknown as Config }, }); diff --git a/packages/cli/src/ui/commands/goalCommand.ts b/packages/cli/src/ui/commands/goalCommand.ts index 60a55513209..cf542036c34 100644 --- a/packages/cli/src/ui/commands/goalCommand.ts +++ b/packages/cli/src/ui/commands/goalCommand.ts @@ -111,10 +111,9 @@ export const goalCommand: SlashCommand = { ); } // No active goal — surface a summary of the most recent terminal goal - // for this session, matching Claude Code's behavior of rendering the - // "Goal achieved" card on empty /goal after completion. Only achieved / - // aborted entries flow through `getLastGoalTerminal`; user-initiated - // `/goal clear` does not populate it. + // for this session. Only achieved / aborted entries flow through + // `getLastGoalTerminal`; user-initiated `/goal clear` does not + // populate it. const last = getLastGoalTerminal(sessionId); if (last) { return infoMessage(formatTerminalSummary(last)); @@ -126,14 +125,10 @@ export const goalCommand: SlashCommand = { // ── Branch 2: clear keyword ────────────────────────────────────────── // - // Strict alignment with Claude Code 2.1.140 `woH`: when an active goal - // exists, drop the Stop hook + emit a `cleared` history sentinel; when - // no active goal exists, this is a no-op that just returns "No goal - // set". Claude does NOT wipe the cached "Goal achieved" summary on - // clear — subsequent empty `/goal` may still surface the most recent - // achievement via `findLastTerminalGoal`. That's intentional: the - // `cleared` history item is a sentinel `findLastTerminalGoal` skips, - // and the previous non-sentinel achievement remains visible. + // When an active goal exists, drop the Stop hook and emit a `cleared` + // history sentinel. When no active goal exists, this is a no-op that just + // returns "No goal set." The cached terminal summary is left intact so a + // later empty `/goal` can still show the latest achieved/aborted state. if (CLEAR_KEYWORDS.has(q.toLowerCase())) { const cleared = unregisterGoalHook(config, sessionId); if (!cleared) { diff --git a/packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx b/packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx new file mode 100644 index 00000000000..50e99ae55a7 --- /dev/null +++ b/packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { render } from 'ink-testing-library'; +import { describe, expect, it } from 'vitest'; +import { GoalStatusMessage } from './GoalStatusMessage.js'; + +describe('', () => { + it('shows the goal and judge reason on checking cards', () => { + const { lastFrame } = render( + , + ); + + const output = lastFrame(); + expect(output).toContain('Goal check'); + expect(output).toContain('turn 2'); + expect(output).toContain('Goal: finish the refactor'); + expect(output).toContain('Judge: tests are still failing'); + }); +}); diff --git a/packages/cli/src/ui/components/messages/GoalStatusMessage.tsx b/packages/cli/src/ui/components/messages/GoalStatusMessage.tsx index f80ebad9ccc..85996df7aab 100644 --- a/packages/cli/src/ui/components/messages/GoalStatusMessage.tsx +++ b/packages/cli/src/ui/components/messages/GoalStatusMessage.tsx @@ -29,18 +29,16 @@ export const GoalStatusMessage: React.FC = ({ }) => { // The "checking" kind is the per-iteration "judge said not met, continuing" // marker that replaces the generic `stop_hook_loop` rendering for /goal. - // Slim one-liner with a hollow circle to signal "pending" without the - // alarming `Stop hook error:` framing. The judge's reason is intentionally - // NOT shown here — it would clutter the per-turn chip and the same reason - // surfaces as the model's next user prompt anyway. The eventual "Last - // check: …" line appears once in the final achieved/aborted card. + // Show the active condition and latest judge reason on every iteration so + // the user can see why the loop is continuing. if (kind === 'checking') { + const reason = lastReason?.trim(); return ( - + Goal check {typeof iterations === 'number' && iterations > 0 @@ -48,6 +46,14 @@ export const GoalStatusMessage: React.FC = ({ : ''}{' '} · not yet met + + Goal: {condition} + + {reason ? ( + + Judge: {reason} + + ) : null} ); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 83be7125e89..ef9721255a3 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -1304,12 +1304,9 @@ export const useGeminiStream = ( setPendingHistoryItem(null); } // When the active loop is driven by `/goal`, replace the generic - // "Ran N stop hooks ⎿ Stop hook error: ..." chip with a goal-aware - // `goal_status` `kind:'checking'` item. Claude Code surfaces this - // mid-state through a single updating "running" card; qwen-code keeps - // a per-iteration history trail (familiar to its other progress - // indicators) but drops the `error:` framing — a not-met judge is the - // *expected* outcome of every continuation, not a failure. + // "Ran N stop hooks" chip with a goal-aware `goal_status` + // `kind:'checking'` item. A not-met judge is the expected outcome of a + // continuation, not a hook failure. const activeGoal = getActiveGoal(config.getSessionId()); if (activeGoal && activeGoal.condition) { addItem( @@ -1450,8 +1447,7 @@ export const useGeminiStream = ( case ServerGeminiEventType.ToolCallRequest: flushBufferedStreamEvents(); toolCallRequests.push(event.value); - // Count tool call args JSON toward token estimation (matches - // Claude Code's input_json_delta handling). + // Count tool call args JSON toward token estimation. try { const argsJson = JSON.stringify(event.value.args); streamingResponseLengthRef.current += argsJson.length; @@ -2129,9 +2125,8 @@ export const useGeminiStream = ( markToolsAsSubmitted(callIdsToMarkAsSubmitted); // Fire tool-use summary generation in parallel with the next API call. - // The fast-model Haiku-equivalent latency (~1s) is hidden behind the - // main-model streaming (5-30s). Mirrors Claude Code's query.ts:1411-1482 - // behavior. Fire-and-forget: failures are silent and never block the turn. + // The fast-model latency is hidden behind the main-model streaming. + // Fire-and-forget: failures are silent and never block the turn. // Subagent exclusion is implicit — useGeminiStream only drives the // main session; subagents run through agents/runtime/ with their own loop. if (config.getEmitToolUseSummaries()) { diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 6d267c73e35..1c6b15fe4f7 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -510,7 +510,7 @@ export type HistoryItemGoalStatus = HistoryItemBase & { type: 'goal_status'; kind: GoalStatusKind; condition: string; - /** Set when kind === 'achieved'. */ + /** Set for progress and terminal goal states. */ iterations?: number; durationMs?: number; lastReason?: string; diff --git a/packages/core/src/goals/activeGoalStore.ts b/packages/core/src/goals/activeGoalStore.ts index 088bbc6d82d..593e45efacc 100644 --- a/packages/core/src/goals/activeGoalStore.ts +++ b/packages/core/src/goals/activeGoalStore.ts @@ -102,10 +102,8 @@ export function notifyGoalTerminal( event: GoalTerminalEvent, ): void { // Stash the last terminal event so an empty `/goal` after the loop ends - // can surface a summary of what just happened (matches Claude Code 2.1.140 - // empty-/goal-after-achievement UX: scans transcript for last met:true - // goal_status and renders an achievement card). We keep the cache in core - // so the CLI command can read it without having access to UI history. + // can surface a summary of what just happened. We keep the cache in core so + // the CLI command can read it without having access to UI history. recordLastTerminalEvent(sessionId, event); const observer = observers.get(sessionId); if (!observer) return; @@ -120,8 +118,7 @@ export function notifyGoalTerminal( // ─────────────────────────────────────────────────────────────────────────── // Last-completed-goal cache // -// Mirrors `yjK` in Claude Code's binary: empty `/goal` after the active goal -// is gone should show "Goal achieved · X turns · Ys" for the most recent +// Empty `/goal` after the active goal is gone should show the most recent // actually-finished goal. Only `achieved` and `aborted` qualify (those are // the `GoalTerminalKind`s); the user-driven `/goal clear` path emits a // `cleared` history card directly and never flows through this notifier. diff --git a/packages/core/src/goals/goalHook.ts b/packages/core/src/goals/goalHook.ts index d64ac74b8fa..88e497ba02c 100644 --- a/packages/core/src/goals/goalHook.ts +++ b/packages/core/src/goals/goalHook.ts @@ -33,17 +33,7 @@ const debugLogger = createDebugLogger('GOAL_HOOK'); */ export const MAX_GOAL_ITERATIONS = 50; -/** - * Default budget (seconds) for a single goal-judge LLM call. Mirrors Claude - * Code 2.1.140's prompt-hook default of 30s (see `cRK` in the binary, which - * reads `H.timeout ? H.timeout * 1000 : 30000`). - * - * Why this matters in qwen-code specifically: the `FunctionHookRunner` default - * is 5s, and a real-world session log showed the judge call against a 5K-token - * context taking ~9.9s — well past 5s but comfortably under 30s. Without - * passing this through, the hook is killed mid-flight, no `continue:false` is - * emitted, and the `/goal` loop silently dies after the second turn. - */ +/** Default budget (seconds) for a single goal-judge LLM call. */ export const GOAL_JUDGE_TIMEOUT_MS = 25_000; export const GOAL_HOOK_TIMEOUT_SECONDS = 30; export const GOAL_HOOK_TIMEOUT_MS = GOAL_HOOK_TIMEOUT_SECONDS * 1000; diff --git a/packages/core/src/goals/goalJudge.ts b/packages/core/src/goals/goalJudge.ts index f6324b17c35..bf3e1f970aa 100644 --- a/packages/core/src/goals/goalJudge.ts +++ b/packages/core/src/goals/goalJudge.ts @@ -14,11 +14,9 @@ const debugLogger = createDebugLogger('GOAL_JUDGE'); /** * System prompt for the goal-completion judge. * - * Wording is aligned with Claude Code 2.1.140's `Stop` prompt-hook evaluator - * (function `cRK` in the compiled binary): it forces the judge to ground its - * verdict on transcript evidence and to default to "not met" whenever the - * evidence is ambiguous. The strict JSON shape lets us pair this with the - * model's structured-output mode below. + * The judge grounds its verdict on transcript evidence and defaults to "not + * met" whenever the evidence is ambiguous. The strict JSON shape lets us pair + * this with the model's structured-output mode below. */ const JUDGE_SYSTEM_PROMPT = `You are evaluating a stop-condition hook in an autonomous coding agent. Read the conversation transcript above carefully, then judge whether the @@ -33,8 +31,7 @@ whenever possible. If the transcript does not contain clear evidence that the condition is satisfied, return {"ok": false, "reason": "insufficient evidence in transcript"}.`; /** - * Wraps the raw user condition into a transcript-grounded question, matching - * Claude Code's `Based on the conversation transcript above...` framing so the + * Wraps the raw user condition into a transcript-grounded question so the * model sees the condition as a binary judgement task, not a new directive. */ const userJudgementPrompt = (condition: string): string => @@ -110,9 +107,9 @@ export async function judgeGoal( if (args.signal.aborted) return { ok: false, reason: JUDGE_REASON_FALLBACK }; // Feed the conversation transcript (trailing N messages) plus the framed - // judgement prompt — Claude Code's design. The hook input's - // `last_assistant_message` is appended only when the live history doesn't - // yet contain it (e.g. before the model turn is committed to chat). + // judgement prompt. The hook input's `last_assistant_message` is appended + // only when the live history doesn't yet contain it (e.g. before the model + // turn is committed to chat). const transcript = collectTranscript(config, args.lastAssistantText); transcript.push({ role: 'user', @@ -130,9 +127,8 @@ export async function judgeGoal( temperature: 0, responseMimeType: 'application/json', responseSchema: RESPONSE_SCHEMA, - // Disable extended thinking — the judge is a binary check; thinking - // burns latency and tokens for no quality gain. Matches Claude Code's - // `thinkingConfig: { type: "disabled" }` for the same call. + // Disable extended thinking: the judge is a binary check, and + // thinking burns latency and tokens for no quality gain. thinkingConfig: { thinkingBudget: 0 }, }, args.signal, From cc6fae9da5898d697503129f3dc5729f18756e0a Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 16 May 2026 22:08:19 +0800 Subject: [PATCH 4/8] fix(core): bound stop hook blocking cap --- packages/core/src/agents/background-agent-resume.ts | 11 +++++++++++ packages/core/src/hooks/stopHookCap.test.ts | 12 ++++++++++++ packages/core/src/hooks/stopHookCap.ts | 7 +++++-- packages/core/src/tools/agent/agent.test.ts | 2 +- packages/core/src/tools/agent/agent.ts | 11 +++++++++++ 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index d2a244dd8ec..5dc3eee0314 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -1006,6 +1006,17 @@ export class BackgroundAgentResumeService { } stopHookActive = true; + const currentIterationCount = i + 1; + if (currentIterationCount >= maxIterations) { + debugLogger.warn( + `[BackgroundAgentResume] ${formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, + )}`, + ); + return; + } + const continueContext = new ContextState(); continueContext.set( 'task_prompt', diff --git a/packages/core/src/hooks/stopHookCap.test.ts b/packages/core/src/hooks/stopHookCap.test.ts index 6f2e96162a3..08b9a386a46 100644 --- a/packages/core/src/hooks/stopHookCap.test.ts +++ b/packages/core/src/hooks/stopHookCap.test.ts @@ -7,6 +7,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import { DEFAULT_STOP_HOOK_BLOCK_CAP, + MAX_STOP_HOOK_BLOCK_CAP, STOP_HOOK_BLOCK_CAP_ENV, formatStopHookBlockingCapWarning, normalizeStopHookBlockingCap, @@ -31,6 +32,11 @@ describe('stop hook blocking cap', () => { it('normalizes finite fractional 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', () => { + expect(normalizeStopHookBlockingCap(99999)).toBe(MAX_STOP_HOOK_BLOCK_CAP); }); it('prefers the environment override over config', () => { @@ -39,6 +45,12 @@ describe('stop hook blocking cap', () => { expect(resolveStopHookBlockingCap(12)).toBe(3); }); + it('ignores an empty environment override', () => { + process.env[STOP_HOOK_BLOCK_CAP_ENV] = ''; + + expect(resolveStopHookBlockingCap(12)).toBe(12); + }); + it('formats warnings for the relevant hook event', () => { expect(formatStopHookBlockingCapWarning('Stop', 8)).toBe( 'Stop hook blocked continuation 8 consecutive times; overriding and ending the turn.', diff --git a/packages/core/src/hooks/stopHookCap.ts b/packages/core/src/hooks/stopHookCap.ts index 00ec2daa2a2..92edb404817 100644 --- a/packages/core/src/hooks/stopHookCap.ts +++ b/packages/core/src/hooks/stopHookCap.ts @@ -5,6 +5,7 @@ */ export const DEFAULT_STOP_HOOK_BLOCK_CAP = 8; +export const MAX_STOP_HOOK_BLOCK_CAP = 100; export const STOP_HOOK_BLOCK_CAP_ENV = 'QWEN_CODE_STOP_HOOK_BLOCK_CAP'; export function normalizeStopHookBlockingCap(value: unknown): number { @@ -13,12 +14,14 @@ export function normalizeStopHookBlockingCap(value: unknown): number { } const normalized = Math.floor(value); - return normalized >= 1 ? normalized : DEFAULT_STOP_HOOK_BLOCK_CAP; + return normalized >= 1 + ? Math.min(normalized, MAX_STOP_HOOK_BLOCK_CAP) + : DEFAULT_STOP_HOOK_BLOCK_CAP; } export function resolveStopHookBlockingCap(configValue?: number): number { const envValue = process.env[STOP_HOOK_BLOCK_CAP_ENV]; - if (envValue !== undefined) { + if (envValue !== undefined && envValue.trim() !== '') { const parsed = Number(envValue); return normalizeStopHookBlockingCap(parsed); } diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index eb694e474a5..1677389721b 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -1324,7 +1324,7 @@ describe('AgentTool', () => { await invocation.execute(); expect(mockHookSystem.fireSubagentStopEvent).toHaveBeenCalledTimes(2); - expect(mockAgent.execute).toHaveBeenCalledTimes(3); + expect(mockAgent.execute).toHaveBeenCalledTimes(2); }); it('should allow stop when SubagentStop hook fails', async () => { diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 39d00f08ad4..7052f360929 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -998,6 +998,17 @@ class AgentToolInvocation extends BaseToolInvocation { } stopHookActive = true; + const currentIterationCount = i + 1; + if (currentIterationCount >= maxIterations) { + debugLogger.warn( + `[Agent] ${formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, + )}`, + ); + return; + } + const continueContext = new ContextState(); continueContext.set( 'task_prompt', From 05fa9faa37eb18af846dc9d944d8a694b26c9cb2 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 16 May 2026 23:10:56 +0800 Subject: [PATCH 5/8] fix(core): surface subagent stop cap warnings --- .../src/agents/background-agent-resume.ts | 47 ++++++++----- packages/core/src/tools/agent/agent.test.ts | 5 +- packages/core/src/tools/agent/agent.ts | 67 ++++++++++++------- 3 files changed, 77 insertions(+), 42 deletions(-) diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 5dc3eee0314..f5bb851aa75 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -55,6 +55,14 @@ const debugLogger = createDebugLogger('BACKGROUND_AGENT_RESUME'); const META_FILE_SUFFIX = '.meta.json'; +function appendSubagentStopWarning( + text: string, + warning: string | undefined, +): string { + if (!warning) return text; + return text ? `${text}\n\n${warning}` : warning; +} + export const DEFAULT_BACKGROUND_AGENT_CONTINUATION_MESSAGE = 'Continue working on the current task from the last completed step.'; @@ -729,8 +737,9 @@ export class BackgroundAgentResumeService { try { await subagent.execute(contextState, bgAbortController.signal); + let stopHookWarning: string | undefined; if (hookSystem && !bgAbortController.signal.aborted) { - await this.runSubagentStopHookLoop(subagent, { + stopHookWarning = await this.runSubagentStopHookLoop(subagent, { agentId: meta.agentId, agentType: meta.agentType, transcriptPath: outputFile, @@ -740,7 +749,10 @@ export class BackgroundAgentResumeService { } const terminateMode = subagent.getTerminateMode(); - const finalText = subagent.getFinalText(); + const finalText = appendSubagentStopWarning( + subagent.getFinalText(), + stopHookWarning, + ); const stats = getCompletionStats(subagent, liveToolCallCount); if (terminateMode === AgentTerminateMode.GOAL) { registry.complete(meta.agentId, finalText, stats); @@ -978,10 +990,10 @@ export class BackgroundAgentResumeService { resolvedMode: PermissionMode; signal?: AbortSignal; }, - ): Promise { + ): Promise { const { agentId, agentType, transcriptPath, resolvedMode, signal } = opts; const hookSystem = this.config.getHookSystem(); - if (!hookSystem) return; + if (!hookSystem) return undefined; let stopHookActive = false; const maxIterations = this.config.getStopHookBlockingCap(); @@ -1002,19 +1014,18 @@ export class BackgroundAgentResumeService { !typedStopOutput?.isBlockingDecision() && !typedStopOutput?.shouldStopExecution() ) { - return; + return undefined; } stopHookActive = true; const currentIterationCount = i + 1; if (currentIterationCount >= maxIterations) { - debugLogger.warn( - `[BackgroundAgentResume] ${formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - )}`, + const warning = formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, ); - return; + debugLogger.warn(`[BackgroundAgentResume] ${warning}`); + return warning; } const continueContext = new ContextState(); @@ -1024,20 +1035,20 @@ export class BackgroundAgentResumeService { ); await subagent.execute(continueContext, signal); - if (signal?.aborted) return; + if (signal?.aborted) return undefined; } catch (hookError) { debugLogger.warn( `[BackgroundAgentResume] SubagentStop hook failed, allowing stop: ${hookError}`, ); - return; + return undefined; } } - debugLogger.warn( - `[BackgroundAgentResume] ${formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - )}`, + const warning = formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, ); + debugLogger.warn(`[BackgroundAgentResume] ${warning}`); + return warning; } } diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 1677389721b..81e949b86fc 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -1321,10 +1321,13 @@ describe('AgentTool', () => { const invocation = ( agentTool as AgentToolWithProtectedMethods ).createInvocation(params); - await invocation.execute(); + const result = await invocation.execute(); expect(mockHookSystem.fireSubagentStopEvent).toHaveBeenCalledTimes(2); expect(mockAgent.execute).toHaveBeenCalledTimes(2); + expect(partToString(result.llmContent)).toContain( + 'SubagentStop hook blocked continuation 2 consecutive times; overriding and ending the turn.', + ); }); it('should allow stop when SubagentStop hook fails', async () => { diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 7052f360929..a588766fa08 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -167,6 +167,14 @@ export interface AgentParams { const debugLogger = createDebugLogger('AGENT'); +function appendSubagentStopWarning( + text: string, + warning: string | undefined, +): string { + if (!warning) return text; + return text ? `${text}\n\n${warning}` : warning; +} + /** * Maps ApprovalMode to PermissionMode for hook events. */ @@ -966,10 +974,10 @@ class AgentToolInvocation extends BaseToolInvocation { resolvedMode: PermissionMode; signal?: AbortSignal; }, - ): Promise { + ): Promise { const { agentId, agentType, transcriptPath, resolvedMode, signal } = opts; const hookSystem = this.config.getHookSystem(); - if (!hookSystem) return; + if (!hookSystem) return undefined; const effectiveTranscriptPath = transcriptPath ?? this.config.getTranscriptPath(); @@ -994,19 +1002,18 @@ class AgentToolInvocation extends BaseToolInvocation { !typedStopOutput?.isBlockingDecision() && !typedStopOutput?.shouldStopExecution() ) { - return; + return undefined; } stopHookActive = true; const currentIterationCount = i + 1; if (currentIterationCount >= maxIterations) { - debugLogger.warn( - `[Agent] ${formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - )}`, + const warning = formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, ); - return; + debugLogger.warn(`[Agent] ${warning}`); + return warning; } const continueContext = new ContextState(); @@ -1016,21 +1023,21 @@ class AgentToolInvocation extends BaseToolInvocation { ); await subagent.execute(continueContext, signal); - if (signal?.aborted) return; + if (signal?.aborted) return undefined; } catch (hookError) { debugLogger.warn( `[Agent] SubagentStop hook failed, allowing stop: ${hookError}`, ); - return; + return undefined; } } - debugLogger.warn( - `[Agent] ${formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - )}`, + const warning = formatStopHookBlockingCapWarning( + 'SubagentStop', + maxIterations, ); + debugLogger.warn(`[Agent] ${warning}`); + return warning; } /** @@ -1047,7 +1054,7 @@ class AgentToolInvocation extends BaseToolInvocation { signal?: AbortSignal; updateOutput?: (output: ToolResultDisplay) => void; }, - ): Promise { + ): Promise { const { agentId, agentType, resolvedMode, signal, updateOutput } = opts; const hookSystem = this.config.getHookSystem(); @@ -1076,8 +1083,9 @@ class AgentToolInvocation extends BaseToolInvocation { // Execute the subagent (blocking) await subagent.execute(contextState, signal); + let stopHookWarning: string | undefined; if (hookSystem && !signal?.aborted) { - await this.runSubagentStopHookLoop(subagent, { + stopHookWarning = await this.runSubagentStopHookLoop(subagent, { agentId, agentType, resolvedMode, @@ -1086,7 +1094,10 @@ class AgentToolInvocation extends BaseToolInvocation { } // Get the results - const finalText = subagent.getFinalText(); + const finalText = appendSubagentStopWarning( + subagent.getFinalText(), + stopHookWarning, + ); const terminateMode = subagent.getTerminateMode(); const success = terminateMode === AgentTerminateMode.GOAL; const executionSummary = subagent.getExecutionSummary(); @@ -1111,6 +1122,7 @@ class AgentToolInvocation extends BaseToolInvocation { updateOutput, ); } + return stopHookWarning; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); @@ -1124,6 +1136,7 @@ class AgentToolInvocation extends BaseToolInvocation { }, updateOutput, ); + return undefined; } } @@ -1786,8 +1799,9 @@ class AgentToolInvocation extends BaseToolInvocation { try { await bgSubagent.execute(contextState, bgAbortController.signal); + let stopHookWarning: string | undefined; if (hookSystem && !bgAbortController.signal.aborted) { - await this.runSubagentStopHookLoop(bgSubagent, { + stopHookWarning = await this.runSubagentStopHookLoop(bgSubagent, { agentId: hookOpts.agentId, agentType: hookOpts.agentType, transcriptPath: jsonlPath, @@ -1806,7 +1820,11 @@ class AgentToolInvocation extends BaseToolInvocation { const wtSuffix = formatWorktreeSuffix( await cleanupWorktreeIsolation(), ); - const finalText = bgSubagent.getFinalText() + wtSuffix; + const finalText = + appendSubagentStopWarning( + bgSubagent.getFinalText(), + stopHookWarning, + ) + wtSuffix; const completionStats = getCompletionStats(); if (terminateMode === AgentTerminateMode.GOAL) { registry.complete(hookOpts.agentId, finalText, completionStats); @@ -2061,8 +2079,11 @@ class AgentToolInvocation extends BaseToolInvocation { this.eventEmitter.on(AgentEventType.USAGE_METADATA, onFgUsageMetadata); try { - await runFramed(); - const finalText = subagent.getFinalText(); + const stopHookWarning = await runFramed(); + const finalText = appendSubagentStopWarning( + subagent.getFinalText(), + stopHookWarning, + ); const terminateMode = subagent.getTerminateMode(); const wtSuffix = formatWorktreeSuffix(await cleanupWorktreeIsolation()); if (terminateMode === AgentTerminateMode.ERROR) { From e34364c8efd749db878a5af8a5a86344750415b2 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sun, 17 May 2026 00:14:08 +0800 Subject: [PATCH 6/8] fix(core): clean up stop hook cap loop --- packages/core/src/agents/background-agent-resume.ts | 7 +------ packages/core/src/core/client.ts | 4 ++++ packages/core/src/tools/agent/agent.ts | 13 ++++--------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index f5bb851aa75..83de2a32539 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -1044,11 +1044,6 @@ export class BackgroundAgentResumeService { } } - const warning = formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - ); - debugLogger.warn(`[BackgroundAgentResume] ${warning}`); - return warning; + return undefined; } } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 175f40f5226..5ac09e6856d 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -1561,6 +1561,10 @@ export class GeminiClient { continueReason, ]; + // Emit StopHookLoop starting with the first blocking decision so + // /goal and configured Stop hooks both surface their reason before + // the follow-up turn is generated. The cap check stays before the + // yield because a cap of 1 means no follow-up turn should run. const stopHookBlockingCap = this.config.getStopHookBlockingCap(); if (currentIterationCount >= stopHookBlockingCap) { const warning = formatStopHookBlockingCapWarning( diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index a588766fa08..5b528af130c 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -962,9 +962,9 @@ class AgentToolInvocation extends BaseToolInvocation { return { subagent, initialMessages, taskPrompt, promptConfig, toolConfig }; } - // Runs the SubagentStop hook after execution. On a blocking decision, feeds the - // reason back and re-executes — up to 5 iterations to defend against a - // misconfigured hook looping forever. + // Runs the SubagentStop hook after execution. On a blocking decision, feeds + // the reason back and re-executes until the configured cap prevents a + // misconfigured hook from looping forever. private async runSubagentStopHookLoop( subagent: AgentHeadless, opts: { @@ -1032,12 +1032,7 @@ class AgentToolInvocation extends BaseToolInvocation { } } - const warning = formatStopHookBlockingCapWarning( - 'SubagentStop', - maxIterations, - ); - debugLogger.warn(`[Agent] ${warning}`); - return warning; + return undefined; } /** From a449a9ebbc91e4f91517f0b3b473da5d1e8f85fd Mon Sep 17 00:00:00 2001 From: qqqys Date: Sun, 17 May 2026 01:11:51 +0800 Subject: [PATCH 7/8] test(core): cover stop hook cap integrations --- .../acp-integration/session/Session.test.ts | 52 +++++++++++- .../agents/background-agent-resume.test.ts | 83 ++++++++++++++++++- packages/core/src/goals/goalHook.test.ts | 57 +++++++++++++ 3 files changed, 189 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 1f3a8c8cd64..93149f5d45b 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -2042,7 +2042,9 @@ describe('Session', () => { }; mockConfig.getMessageBus = vi.fn().mockReturnValue(messageBus); mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); - mockConfig.hasHooksForEvent = vi.fn().mockReturnValue(true); + mockConfig.hasHooksForEvent = vi + .fn() + .mockImplementation((eventName: string) => eventName === 'Stop'); mockChat.getHistory = vi .fn() .mockReturnValue([ @@ -2076,6 +2078,54 @@ describe('Session', () => { expect.anything(), ); }); + + it('ends Stop hook continuation when the blocking cap is reached', async () => { + const messageBus = { + request: vi.fn().mockImplementation(async (request) => ({ + success: true, + output: + request.eventName === 'Stop' + ? { + decision: 'block', + reason: 'Continue after Stop hook', + } + : {}, + })), + }; + mockConfig.getMessageBus = vi.fn().mockReturnValue(messageBus); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); + mockConfig.hasHooksForEvent = vi + .fn() + .mockImplementation((eventName: string) => eventName === 'Stop'); + mockConfig.getStopHookBlockingCap = vi.fn().mockReturnValue(2); + mockChat.getHistory = vi + .fn() + .mockReturnValue([ + { role: 'model', parts: [{ text: 'response text' }] }, + ]); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + const result = await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + expect(result).toEqual({ stopReason: 'end_turn' }); + expect(messageBus.request).toHaveBeenCalledTimes(2); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(2); + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'agent_message_chunk', + content: { + type: 'text', + text: 'Stop hook blocked continuation 2 consecutive times; overriding and ending the turn.', + }, + }, + }); + }); }); describe('PreToolUse hook', () => { diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index 05d3cba8868..73dfe9c87c7 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -42,7 +42,7 @@ describe('BackgroundAgentResumeService', () => { }); }); - function createService() { + function createService(options: { stopHookBlockingCap?: number } = {}) { const subagentManager = { loadSubagent: vi.fn(async (name: string) => name === 'researcher' @@ -82,7 +82,7 @@ describe('BackgroundAgentResumeService', () => { getMonitorRegistry: () => monitorRegistry, getSubagentManager: () => subagentManager, getHookSystem: () => hookSystem, - getStopHookBlockingCap: () => 8, + getStopHookBlockingCap: () => options.stopHookBlockingCap ?? 8, getApprovalMode: () => 'default', isTrustedFolder: () => true, getProjectRoot: () => tempDir, @@ -529,6 +529,85 @@ describe('BackgroundAgentResumeService', () => { }); }); + it('appends a warning when resumed SubagentStop hooks reach the blocking cap', async () => { + const sessionId = 'session-stop-hook-cap'; + const agentId = 'agent-stop-hook-cap'; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume cap path', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + resolvedApprovalMode: 'default', + }); + fs.writeFileSync( + outputFile, + JSON.stringify({ + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-20T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Resume cap path' }] }, + }) + '\n', + 'utf8', + ); + + registry.register({ + agentId, + description: 'Resume cap path', + subagentType: 'researcher', + isBackgrounded: true, + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume cap path', + outputFile, + metaPath, + }); + + const subagent = { + execute: vi.fn(async () => undefined), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'final output', + }; + const stopOutput = { + isBlockingDecision: vi.fn().mockReturnValue(true), + shouldStopExecution: vi.fn().mockReturnValue(false), + getEffectiveReason: vi.fn().mockReturnValue('Keep going'), + }; + + const { service, subagentManager, hookSystem } = createService({ + stopHookBlockingCap: 2, + }); + subagentManager.createAgentHeadless.mockResolvedValue(subagent); + hookSystem.fireSubagentStopEvent.mockResolvedValue(stopOutput); + + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + + expect(resumed).toBeDefined(); + await vi.waitFor(() => { + expect(registry.get(agentId)?.status).toBe('completed'); + }); + expect(hookSystem.fireSubagentStopEvent).toHaveBeenCalledTimes(2); + expect(subagent.execute).toHaveBeenCalledTimes(2); + expect(registry.get(agentId)?.result).toContain( + 'SubagentStop hook blocked continuation 2 consecutive times; overriding and ending the turn.', + ); + }); + // Windows-24 GitHub Actions runners can take 10s+ on this fs-heavy // setup (writeAgentMeta + fs.writeFileSync + Promise resolution chain), // exceeding vitest's 5s default. Raise the per-test timeout so the diff --git a/packages/core/src/goals/goalHook.test.ts b/packages/core/src/goals/goalHook.test.ts index 3c2d5d999aa..49cba6d0435 100644 --- a/packages/core/src/goals/goalHook.test.ts +++ b/packages/core/src/goals/goalHook.test.ts @@ -19,6 +19,7 @@ import { type GoalTerminalEvent, } from './activeGoalStore.js'; import { + abortGoalForStopHookCap, createGoalStopHookCallback, GOAL_HOOK_TIMEOUT_MS, GOAL_JUDGE_TIMEOUT_MS, @@ -353,6 +354,62 @@ describe('createGoalStopHookCallback', () => { }); }); +describe('abortGoalForStopHookCap', () => { + beforeEach(() => { + __resetActiveGoalStoreForTests(); + }); + + afterEach(() => __resetActiveGoalStoreForTests()); + + it('returns false when no active goal exists', () => { + const removeFunctionHook = vi.fn(); + const config = { + getHookSystem: () => ({ removeFunctionHook }), + } as unknown as Config; + + expect(abortGoalForStopHookCap(config, 'missing-session', 'cap hit')).toBe( + false, + ); + expect(removeFunctionHook).not.toHaveBeenCalled(); + }); + + it('clears the active goal and notifies observers when the cap is reached', () => { + const removeFunctionHook = vi.fn(); + const config = { + getHookSystem: () => ({ removeFunctionHook }), + } as unknown as Config; + const events: GoalTerminalEvent[] = []; + setActiveGoal('sess-1', { + condition: 'finish tests', + iterations: 3, + setAt: Date.now() - 100, + tokensAtStart: 0, + lastReason: 'still incomplete', + hookId: 'goal-hook-id', + }); + setGoalTerminalObserver('sess-1', (event) => events.push(event)); + + expect( + abortGoalForStopHookCap(config, 'sess-1', 'Stop hook cap reached'), + ).toBe(true); + + expect(getActiveGoal('sess-1')).toBeUndefined(); + expect(removeFunctionHook).toHaveBeenCalledWith( + 'sess-1', + HookEventName.Stop, + 'goal-hook-id', + ); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ + kind: 'aborted', + condition: 'finish tests', + iterations: 3, + lastReason: 'still incomplete', + systemMessage: 'Stop hook cap reached', + }); + }); +}); + describe('registerGoalHook / unregisterGoalHook', () => { let addFunctionHook: ReturnType; let removeFunctionHook: ReturnType; From 3d8a74646e0e4d5e2d5f355c53cfcaef7d3426d7 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sun, 17 May 2026 02:14:31 +0800 Subject: [PATCH 8/8] test(core): strengthen stop hook cap coverage --- .../acp-integration/session/Session.test.ts | 45 +++++++++++++++++++ .../src/agents/background-agent-resume.ts | 15 +++---- packages/core/src/core/client.test.ts | 2 +- packages/core/src/hooks/stopHookCap.test.ts | 12 +++++ packages/core/src/hooks/stopHookCap.ts | 11 ++++- packages/core/src/tools/agent/agent.ts | 19 +++----- 6 files changed, 80 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 93149f5d45b..e11f4ac4357 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -2126,6 +2126,51 @@ describe('Session', () => { }, }); }); + + it('emits the cap warning without retrying when the blocking cap is one', async () => { + const messageBus = { + request: vi.fn().mockResolvedValue({ + success: true, + output: { + decision: 'block', + reason: 'Continue after Stop hook', + }, + }), + }; + mockConfig.getMessageBus = vi.fn().mockReturnValue(messageBus); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); + mockConfig.hasHooksForEvent = vi + .fn() + .mockImplementation((eventName: string) => eventName === 'Stop'); + mockConfig.getStopHookBlockingCap = vi.fn().mockReturnValue(1); + mockChat.getHistory = vi + .fn() + .mockReturnValue([ + { role: 'model', parts: [{ text: 'response text' }] }, + ]); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + const result = await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + expect(result).toEqual({ stopReason: 'end_turn' }); + expect(messageBus.request).toHaveBeenCalledTimes(1); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'agent_message_chunk', + content: { + type: 'text', + text: 'Stop hook blocked continuation 1 consecutive time; overriding and ending the turn.', + }, + }, + }); + }); }); describe('PreToolUse hook', () => { diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index d6cf0848da1..640ce02428f 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -27,7 +27,10 @@ import type { ChatRecord } from '../services/chatRecordingService.js'; import { getInitialChatHistory } from '../utils/environmentContext.js'; import { getGitBranch } from '../utils/gitUtils.js'; import { PermissionMode, type StopHookOutput } from '../hooks/types.js'; -import { formatStopHookBlockingCapWarning } from '../hooks/stopHookCap.js'; +import { + appendStopHookBlockingCapWarning, + formatStopHookBlockingCapWarning, +} from '../hooks/stopHookCap.js'; import { runWithAgentContext } from './runtime/agent-context.js'; import { createApprovalModeOverride } from '../tools/agent/agent.js'; import type { ApprovalMode } from '../config/config.js'; @@ -56,14 +59,6 @@ const debugLogger = createDebugLogger('BACKGROUND_AGENT_RESUME'); const META_FILE_SUFFIX = '.meta.json'; -function appendSubagentStopWarning( - text: string, - warning: string | undefined, -): string { - if (!warning) return text; - return text ? `${text}\n\n${warning}` : warning; -} - export const DEFAULT_BACKGROUND_AGENT_CONTINUATION_MESSAGE = 'Continue working on the current task from the last completed step.'; @@ -750,7 +745,7 @@ export class BackgroundAgentResumeService { } const terminateMode = subagent.getTerminateMode(); - const finalText = appendSubagentStopWarning( + const finalText = appendStopHookBlockingCapWarning( subagent.getFinalText(), stopHookWarning, ); diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index ba69aebada3..57b268ce7ca 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -4338,7 +4338,7 @@ Other open files: expect(events).toContainEqual({ type: GeminiEventType.HookSystemMessage, value: - 'Stop hook blocked continuation 1 consecutive times; overriding and ending the turn.', + 'Stop hook blocked continuation 1 consecutive time; overriding and ending the turn.', }); }); diff --git a/packages/core/src/hooks/stopHookCap.test.ts b/packages/core/src/hooks/stopHookCap.test.ts index 08b9a386a46..becb10931ac 100644 --- a/packages/core/src/hooks/stopHookCap.test.ts +++ b/packages/core/src/hooks/stopHookCap.test.ts @@ -9,6 +9,7 @@ import { DEFAULT_STOP_HOOK_BLOCK_CAP, MAX_STOP_HOOK_BLOCK_CAP, STOP_HOOK_BLOCK_CAP_ENV, + appendStopHookBlockingCapWarning, formatStopHookBlockingCapWarning, normalizeStopHookBlockingCap, resolveStopHookBlockingCap, @@ -58,5 +59,16 @@ describe('stop hook blocking cap', () => { expect(formatStopHookBlockingCapWarning('SubagentStop', 2)).toContain( 'SubagentStop hook blocked continuation 2 consecutive times', ); + expect(formatStopHookBlockingCapWarning('Stop', 1)).toBe( + 'Stop hook blocked continuation 1 consecutive time; overriding and ending the turn.', + ); + }); + + it('appends cap warnings to visible subagent output', () => { + expect(appendStopHookBlockingCapWarning('done', undefined)).toBe('done'); + expect(appendStopHookBlockingCapWarning('', 'warning')).toBe('warning'); + expect(appendStopHookBlockingCapWarning('done', 'warning')).toBe( + 'done\n\nwarning', + ); }); }); diff --git a/packages/core/src/hooks/stopHookCap.ts b/packages/core/src/hooks/stopHookCap.ts index 92edb404817..94924a1d7f8 100644 --- a/packages/core/src/hooks/stopHookCap.ts +++ b/packages/core/src/hooks/stopHookCap.ts @@ -37,5 +37,14 @@ export function formatStopHookBlockingCapWarning( // model or subagent would otherwise finish, so keep user-facing labels // explicit instead of accepting arbitrary hook names. const hookName = hookLabel === 'Stop' ? 'Stop hook' : 'SubagentStop hook'; - return `${hookName} blocked continuation ${cap} consecutive times; overriding and ending the turn.`; + const timesWord = cap === 1 ? 'time' : 'times'; + return `${hookName} blocked continuation ${cap} consecutive ${timesWord}; overriding and ending the turn.`; +} + +export function appendStopHookBlockingCapWarning( + text: string, + warning: string | undefined, +): string { + if (!warning) return text; + return text ? `${text}\n\n${warning}` : warning; } diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index dbc8546ff88..f7cd9114fd1 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -69,7 +69,10 @@ import { BuiltinAgentRegistry } from '../../subagents/builtin-agents.js'; import { createDebugLogger } from '../../utils/debugLogger.js'; import { PermissionMode } from '../../hooks/types.js'; import type { StopHookOutput } from '../../hooks/types.js'; -import { formatStopHookBlockingCapWarning } from '../../hooks/stopHookCap.js'; +import { + appendStopHookBlockingCapWarning, + formatStopHookBlockingCapWarning, +} from '../../hooks/stopHookCap.js'; import { ApprovalMode } from '../../config/config.js'; import { getAgentJsonlPath, @@ -181,14 +184,6 @@ export interface AgentParams { const debugLogger = createDebugLogger('AGENT'); -function appendSubagentStopWarning( - text: string, - warning: string | undefined, -): string { - if (!warning) return text; - return text ? `${text}\n\n${warning}` : warning; -} - /** * Maps ApprovalMode to PermissionMode for hook events. */ @@ -1103,7 +1098,7 @@ class AgentToolInvocation extends BaseToolInvocation { } // Get the results - const finalText = appendSubagentStopWarning( + const finalText = appendStopHookBlockingCapWarning( subagent.getFinalText(), stopHookWarning, ); @@ -1834,7 +1829,7 @@ class AgentToolInvocation extends BaseToolInvocation { await cleanupWorktreeIsolation(), ); const finalText = - appendSubagentStopWarning( + appendStopHookBlockingCapWarning( bgSubagent.getFinalText(), stopHookWarning, ) + wtSuffix; @@ -2157,7 +2152,7 @@ class AgentToolInvocation extends BaseToolInvocation { }); const stopHookWarning = await runFramed(); - const finalText = appendSubagentStopWarning( + const finalText = appendStopHookBlockingCapWarning( subagent.getFinalText(), stopHookWarning, );