Skip to content
Merged
5 changes: 4 additions & 1 deletion packages/cli/src/nonInteractiveCli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import type { LoadedSettings } from './config/settings.js';
import { CommandKind, type ExecutionMode } from './ui/commands/types.js';
import { filterCommandsForMode } from './services/commandUtils.js';
import { _resetCleanupFunctionsForTest } from './utils/cleanup.js';
import { AlreadyReportedError, _resetExitLatchForTest } from './utils/errors.js';
import {
AlreadyReportedError,
_resetExitLatchForTest,
} from './utils/errors.js';

// Mock core modules
vi.mock('./ui/hooks/atCommandProcessor.js');
Expand Down
60 changes: 60 additions & 0 deletions packages/cli/src/ui/commands/clearCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe('clearCommand', () => {
}),
getBackgroundShellRegistry: vi.fn().mockReturnValue({
getAll: vi.fn().mockReturnValue([]),
hasRunningEntries: vi.fn().mockReturnValue(false),
reset: mockResetBackgroundShells,
abortAll: mockAbortBackgroundShells,
}),
Expand Down Expand Up @@ -307,6 +308,7 @@ describe('clearCommand', () => {
}),
getBackgroundShellRegistry: vi.fn().mockReturnValue({
getAll: vi.fn().mockReturnValue([]),
hasRunningEntries: vi.fn().mockReturnValue(false),
reset: mockResetBackgroundShells,
abortAll: mockAbortBackgroundShells,
}),
Expand Down Expand Up @@ -380,6 +382,7 @@ describe('clearCommand', () => {
}),
getBackgroundShellRegistry: vi.fn().mockReturnValue({
getAll: vi.fn().mockReturnValue([]),
hasRunningEntries: vi.fn().mockReturnValue(false),
reset: vi.fn(),
}),
getMonitorRegistry: vi.fn().mockReturnValue({
Expand Down Expand Up @@ -430,6 +433,7 @@ describe('clearCommand', () => {
}),
getBackgroundShellRegistry: vi.fn().mockReturnValue({
getAll: vi.fn().mockReturnValue([]),
hasRunningEntries: vi.fn().mockReturnValue(false),
reset: vi.fn(),
}),
getMonitorRegistry: vi.fn().mockReturnValue({
Expand Down Expand Up @@ -470,5 +474,61 @@ describe('clearCommand', () => {
expect(mockStartNewSession).not.toHaveBeenCalled();
expect(mockResetChat).not.toHaveBeenCalled();
});

it('blocks session clearing while a background shell is still running', async () => {
if (!clearCommand.action)
throw new Error('clearCommand must have an action.');

const blockedContext = createMockCommandContext({
executionMode: 'non_interactive',
services: {
config: {
getBackgroundTaskRegistry: vi.fn().mockReturnValue({
hasUnfinalizedTasks: vi.fn().mockReturnValue(false),
reset: vi.fn(),
}),
getBackgroundShellRegistry: vi.fn().mockReturnValue({
getAll: vi.fn().mockReturnValue([
{
shellId: 'shell_123',
status: 'running',
},
]),
hasRunningEntries: vi.fn().mockReturnValue(true),
reset: vi.fn(),
}),
getMonitorRegistry: vi.fn().mockReturnValue({
getRunning: vi.fn().mockReturnValue([]),
reset: vi.fn(),
}),
getHookSystem: mockGetHookSystem,
startNewSession: mockStartNewSession,
getGeminiClient: vi.fn().mockReturnValue({
resetChat: mockResetChat,
} as unknown as GeminiClient),
getModel: vi.fn().mockReturnValue('test-model'),
getApprovalMode: vi.fn().mockReturnValue('default'),
getToolRegistry: vi.fn().mockReturnValue({
getAllTools: vi.fn().mockReturnValue([]),
}),
getDebugLogger: vi.fn().mockReturnValue({ warn: vi.fn() }),
},
},
session: {
startNewSession: vi.fn(),
},
});

const result = await clearCommand.action(blockedContext, '');

expect(result).toEqual({
type: 'message',
messageType: 'error',
content:
"Stop the current session's running background tasks before starting a new session.",
});
expect(mockStartNewSession).not.toHaveBeenCalled();
expect(mockResetChat).not.toHaveBeenCalled();
});
});
});
22 changes: 4 additions & 18 deletions packages/cli/src/ui/commands/clearCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,12 @@ import {
SessionEndReason,
SessionStartSource,
ToolNames,
type Config,
type PermissionMode,
} from '@qwen-code/qwen-code-core';

function hasBlockingBackgroundWork(config: Config): boolean {
return (
config.getBackgroundTaskRegistry().hasUnfinalizedTasks() ||
config.getMonitorRegistry().getRunning().length > 0 ||
config
.getBackgroundShellRegistry()
.getAll()
.some((entry) => entry.status === 'running')
);
}

function resetBackgroundStateForSessionSwitch(config: Config): void {
config.getBackgroundTaskRegistry().reset();
config.getMonitorRegistry().reset();
config.getBackgroundShellRegistry().reset();
}
import {
hasBlockingBackgroundWork,
resetBackgroundStateForSessionSwitch,
} from '../utils/backgroundWorkUtils.js';

export const clearCommand: SlashCommand = {
name: 'clear',
Expand Down
21 changes: 4 additions & 17 deletions packages/cli/src/ui/hooks/useResumeCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import {
import { buildResumedHistoryItems } from '../utils/resumeHistoryUtils.js';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
import { MessageType, type HistoryItem } from '../types.js';
import {
hasBlockingBackgroundWork,
resetBackgroundStateForSessionSwitch,
} from '../utils/backgroundWorkUtils.js';

export interface UseResumeCommandOptions {
config: Config | null;
Expand Down Expand Up @@ -45,23 +49,6 @@ export interface UseResumeCommandResult {
const BACKGROUND_WORK_SWITCH_BLOCKED_MESSAGE =
"Stop the current session's running background tasks before resuming another session.";

function hasBlockingBackgroundWork(config: Config): boolean {
return (
config.getBackgroundTaskRegistry().hasUnfinalizedTasks() ||
config.getMonitorRegistry().getRunning().length > 0 ||
config
.getBackgroundShellRegistry()
.getAll()
.some((entry) => entry.status === 'running')
);
}

function resetBackgroundStateForSessionSwitch(config: Config): void {
config.getBackgroundTaskRegistry().reset();
config.getMonitorRegistry().reset();
config.getBackgroundShellRegistry().reset();
}

export function useResumeCommand(
options?: UseResumeCommandOptions,
): UseResumeCommandResult {
Expand Down
21 changes: 21 additions & 0 deletions packages/cli/src/ui/utils/backgroundWorkUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @license
* Copyright 2025 Qwen Code
* SPDX-License-Identifier: Apache-2.0
*/

import type { Config } from '@qwen-code/qwen-code-core';

export function hasBlockingBackgroundWork(config: Config): boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] 提取到共享模块的 hasBlockingBackgroundWork 缺少 JSDoc。建议添加说明其检查顺序和阻塞语义:

Suggested change
export function hasBlockingBackgroundWork(config: Config): boolean {
/**
* Returns true when any background work is still in progress that would
* conflict with starting or resuming a session.
*
* Checks in short-circuit order: unfinalized tasks, running monitors,
* then running background shells.
*/
export function hasBlockingBackgroundWork(config: Config): boolean {

— deepseek-v4-pro via Qwen Code /review

return (
config.getBackgroundTaskRegistry().hasUnfinalizedTasks() ||
config.getMonitorRegistry().getRunning().length > 0 ||
config.getBackgroundShellRegistry().hasRunningEntries()
);
}
Comment thread
doudouOUC marked this conversation as resolved.
Resolved

export function resetBackgroundStateForSessionSwitch(config: Config): void {
config.getBackgroundTaskRegistry().reset();
config.getMonitorRegistry().reset();
config.getBackgroundShellRegistry().reset();
}
3 changes: 2 additions & 1 deletion packages/cli/src/utils/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ describe('errors', () => {
{
error: {
type: 'AlreadyReportedError',
message: '[API Error: 402 Model X is not available for billing.]',
message:
'[API Error: 402 Model X is not available for billing.]',
code: 42,
},
},
Expand Down
20 changes: 7 additions & 13 deletions packages/core/src/permissions/permission-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
matchesRule,
resolveToolName,
splitCompoundCommand,
SHELL_TOOL_NAMES,
} from './rule-parser.js';
import type { PathMatchContext } from './rule-parser.js';
import { extractShellOperations } from './shell-semantics.js';
Expand All @@ -32,13 +33,6 @@ import type {

const debugLogger = createDebugLogger('PERMISSIONS');

/**
* Tools that spawn shell commands and share the same permission evaluation
* semantics: compound-command splitting, AST read-only analysis, and
* virtual file/network operation matching.
*/
const SHELL_LIKE_TOOLS = new Set(['run_shell_command', 'monitor']);

/**
* Numeric priority for each PermissionDecision.
* Higher number = more restrictive. Used to combine decisions by taking
Expand Down Expand Up @@ -189,7 +183,7 @@ export class PermissionManager {
// a concrete permission (deny/ask/allow) based on the command's readonly status.
if (
decision === 'default' &&
SHELL_LIKE_TOOLS.has(toolName) &&
SHELL_TOOL_NAMES.has(toolName) &&
command !== undefined
) {
return this.resolveDefaultPermission(command);
Expand Down Expand Up @@ -267,7 +261,7 @@ export class PermissionManager {
// must never downgrade an explicit 'allow' decision from a Bash rule.
// Example: `git status` has no file ops; an allow rule for `Bash(git *)`
// should return 'allow', not be downgraded to 'default'.
if (SHELL_LIKE_TOOLS.has(toolName) && command !== undefined) {
if (SHELL_TOOL_NAMES.has(toolName) && command !== undefined) {
const cwd = pathCtx?.cwd ?? process.cwd();
const virtualDecision = this.evaluateShellVirtualOps(
extractShellOperations(command, cwd),
Expand Down Expand Up @@ -584,7 +578,7 @@ export class PermissionManager {
ctx = this.normalizePermissionContext(ctx);
const { toolName, command, cwd, filePath, domain, specifier } = ctx;

if (SHELL_LIKE_TOOLS.has(ctx.toolName) && command !== undefined) {
if (SHELL_TOOL_NAMES.has(ctx.toolName) && command !== undefined) {
const subCommands = splitCompoundCommand(command);
if (subCommands.length > 1) {
return subCommands.some((subCmd) =>
Expand Down Expand Up @@ -625,7 +619,7 @@ export class PermissionManager {
// extracted from the command has a relevant rule. This ensures the PM is
// consulted (and the confirmation dialog shown) when Read/Edit/etc. rules
// would match equivalent shell commands.
if (SHELL_LIKE_TOOLS.has(ctx.toolName) && ctx.command !== undefined) {
if (SHELL_TOOL_NAMES.has(ctx.toolName) && ctx.command !== undefined) {
const cwd = pathCtx?.cwd ?? process.cwd();
const ops = extractShellOperations(ctx.command, cwd);
if (
Expand Down Expand Up @@ -661,7 +655,7 @@ export class PermissionManager {
ctx = this.normalizePermissionContext(ctx);
const { toolName, command, cwd, filePath, domain, specifier } = ctx;

if (SHELL_LIKE_TOOLS.has(ctx.toolName) && command !== undefined) {
if (SHELL_TOOL_NAMES.has(ctx.toolName) && command !== undefined) {
const subCommands = splitCompoundCommand(command);
if (subCommands.length > 1) {
return subCommands.some((subCmd) =>
Expand Down Expand Up @@ -693,7 +687,7 @@ export class PermissionManager {
return true;
}

if (SHELL_LIKE_TOOLS.has(ctx.toolName) && ctx.command !== undefined) {
if (SHELL_TOOL_NAMES.has(ctx.toolName) && ctx.command !== undefined) {
const cwd = pathCtx?.cwd ?? process.cwd();
const ops = extractShellOperations(ctx.command, cwd);
return ops.some((op) => {
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/permissions/rule-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ export const TOOL_NAME_ALIASES: Readonly<Record<string, string>> = {
/**
* Shell tool canonical names. These use command-style rule specifiers.
*/
const SHELL_TOOL_NAMES = new Set(['run_shell_command', 'monitor']);
export const SHELL_TOOL_NAMES: ReadonlySet<string> = new Set([

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] SHELL_TOOL_NAMES 导出时 JSDoc 过于简略,丢失了原来 permission-manager.ts 中 SHELL_LIKE_TOOLS 的详细注释(复合命令拆分、AST 只读分析等)。建议补充:

Suggested change
export const SHELL_TOOL_NAMES: ReadonlySet<string> = new Set([
/**
* Shell tool canonical names. These tools spawn shell commands and share
* the same permission evaluation semantics: compound-command splitting,
* AST read-only analysis, and virtual file/network operation matching.
*/
export const SHELL_TOOL_NAMES: ReadonlySet<string> = new Set([
'run_shell_command',
'monitor',
]);

— deepseek-v4-pro via Qwen Code /review

'run_shell_command',
'monitor',
]);

Comment thread
doudouOUC marked this conversation as resolved.
Resolved
/**
* File-reading tools — "Read" rules apply to all of these (best-effort).
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/services/monitorRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,21 @@ describe('MonitorRegistry', () => {
).not.toThrow();
});

it('includes droppedLines count in terminal notification text', () => {
const callback = vi.fn();
registry.setNotificationCallback(callback);
const entry = createEntry();
registry.register(entry);

// Simulate throttle drops (droppedLines is incremented by Monitor tool)
entry.droppedLines = 5;
registry.complete('mon-1', 0);

const [displayText, modelText] = callback.mock.calls[0] as [string, string];
expect(displayText).toContain('5 lines dropped due to throttling');
expect(modelText).toContain('5 lines dropped due to throttling');
});

describe('setStatusChangeCallback', () => {
it('fires once on register (nothing → running)', () => {
const cb = vi.fn();
Expand Down
Loading
Loading