Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
98 changes: 97 additions & 1 deletion packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -2041,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([
Expand Down Expand Up @@ -2075,6 +2078,99 @@ 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.',
},
},
});
});

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', () => {
Expand Down
29 changes: 19 additions & 10 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 ||
Expand Down Expand Up @@ -761,7 +763,21 @@ export class Session implements SessionContext {
stopHookIterationCount++;
stopHookReasons = [...stopHookReasons, continueReason];

// Emit StopHookLoop event for iterations after the first one
if (stopHookIterationCount >= stopHookBlockingCap) {

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.

[Critical] #handleStopHookLoop 的 cap-enforcement 路径未测试。config mock 已添加 getStopHookBlockingCap: vi.fn().mockReturnValue(8),但没有测试将 cap 设为低值(如 1 或 2)来触发 stopHookIterationCount >= stopHookBlockingCap 分支。当前的两个 Stop hook 测试在 cap=8 下分别仅运行 1-2 次迭代,远未达到上限。

Session.ts 的 cap 逻辑独立处理 ACP 会话(VSCode companion)的 stop hook 循环。此路径的 bug 仅会在 ACP 会话中暴露,现有测试无法捕获。

Suggested change
if (stopHookIterationCount >= stopHookBlockingCap) {
// Add a test that sets up a blocking Stop hook and getStopHookBlockingCap.mockReturnValue(1),
// then asserts the prompt completes with stopReason: 'end_turn' on cap hit
// and that the warning message is emitted via messageEmitter.emitAgentMessage.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

const warning = formatStopHookBlockingCapWarning(
'Stop',
stopHookBlockingCap,
);
abortGoalForStopHookCap(
this.config,
this.config.getSessionId(),
warning,
);
await this.messageEmitter.emitAgentMessage(warning);
debugLogger.warn(warning);
return { stopReason: 'end_turn' };
}

if (stopHookIterationCount > 1) {
await this.messageEmitter.emitStopHookLoop(
stopHookIterationCount,
Expand Down Expand Up @@ -904,13 +920,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' };
}

Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1957,6 +1958,19 @@ 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.',
// This is an advanced safety valve for runaway hook loops, not a common
// interactive preference.
showInDialog: false,
},

hooks: {
type: 'object',
label: 'Hooks',
Expand Down
11 changes: 4 additions & 7 deletions packages/cli/src/ui/commands/goalCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
});
Expand Down
19 changes: 7 additions & 12 deletions packages/cli/src/ui/commands/goalCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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) {
Expand Down
28 changes: 28 additions & 0 deletions packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx
Original file line number Diff line number Diff line change
@@ -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('<GoalStatusMessage />', () => {
it('shows the goal and judge reason on checking cards', () => {
const { lastFrame } = render(
<GoalStatusMessage
kind="checking"
condition="finish the refactor"
iterations={2}
lastReason="tests are still failing"
/>,
);

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');
});
});
18 changes: 12 additions & 6 deletions packages/cli/src/ui/components/messages/GoalStatusMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,31 @@ export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
}) => {
// 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 (
<Box flexDirection="row">
<Box width={2} flexShrink={0}>
<Text color={theme.text.secondary}>○</Text>
</Box>
<Box flexGrow={1}>
<Box flexGrow={1} flexDirection="column">
<Text color={theme.text.secondary}>
Goal check
{typeof iterations === 'number' && iterations > 0
? ` · turn ${iterations}`
: ''}{' '}
· not yet met
</Text>
<Text color={theme.text.secondary} wrap="wrap">
Goal: {condition}
</Text>
{reason ? (
<Text color={theme.text.secondary} wrap="wrap">
Judge: {reason}
</Text>
) : null}
</Box>
</Box>
);
Expand Down
17 changes: 6 additions & 11 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1308,12 +1308,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(
Expand Down Expand Up @@ -1454,8 +1451,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;
Expand Down Expand Up @@ -2133,9 +2129,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()) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,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;
Expand Down
Loading
Loading