mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-04 22:51:08 +00:00
refactor(cli): replace slash command whitelist with capability-based filtering (Phase 1) (#3283)
* refactor(cli): replace slash command whitelist with capability-based filtering (Phase 1)
## Summary
Replace the hardcoded ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE whitelist with a
unified, capability-based command metadata model. This is Phase 1 of the slash
command architecture refactor described in docs/design/slash-command/.
## Key changes
### New types (types.ts)
- Add ExecutionMode ('interactive' | 'non_interactive' | 'acp')
- Add CommandSource ('builtin-command' | 'bundled-skill' | 'skill-dir-command' |
'plugin-command' | 'mcp-prompt')
- Add CommandType ('prompt' | 'local' | 'local-jsx')
- Extend SlashCommand interface with: source, sourceLabel, commandType,
supportedModes, userInvocable, modelInvocable, argumentHint, whenToUse,
examples (all optional, backward-compatible)
### New module (commandUtils.ts + commandUtils.test.ts)
- getEffectiveSupportedModes(): 3-priority inference
(explicit supportedModes > commandType > CommandKind fallback)
- filterCommandsForMode(): replaces filterCommandsForNonInteractive()
- 18 unit tests
### Whitelist removal (nonInteractiveCliCommands.ts)
- Remove ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE constant
- Remove filterCommandsForNonInteractive() function
- Replace with CommandService.getCommandsForMode(mode)
### CommandService enhancements (CommandService.ts)
- Add getCommandsForMode(mode: ExecutionMode): filters by mode, excludes hidden
- Add getModelInvocableCommands(): reserved for Phase 3 model tool-call use
### Built-in command annotations (41 files)
Annotate every built-in command with commandType:
- commandType='local' + supportedModes all-modes: btw, bug, compress, context,
init, summary (replaces the 6-command whitelist)
- commandType='local' interactive-only: export, memory, plan, insight
- commandType='local-jsx' interactive-only: all remaining ~31 commands
### Loader metadata injection (4 files)
Each loader stamps source/sourceLabel/commandType/modelInvocable on every
command it emits:
- BuiltinCommandLoader: source='builtin-command', modelInvocable=false
- BundledSkillLoader: source='bundled-skill', commandType='prompt',
modelInvocable=true
- command-factory (FileCommandLoader): source per extension/user origin,
commandType='prompt', modelInvocable=!extensionName
- McpPromptLoader: source='mcp-prompt', commandType='prompt', modelInvocable=true
### Bug fix
MCP_PROMPT commands were incorrectly excluded from non-interactive/ACP modes by
the old whitelist logic. commandType='prompt' now correctly allows them in all
modes.
### Session.ts / nonInteractiveHelpers.ts
- ACP session calls getAvailableCommands with explicit 'acp' mode
- Remove allowedBuiltinCommandNames parameter from buildSystemMessage() —
capability filtering is now self-contained in CommandService
* fix test ci
* fix memory command
* fix: pass 'non_interactive' mode explicitly to getAvailableCommands
- Fix critical bug in nonInteractiveHelpers.ts: loadSlashCommandNames was
calling getAvailableCommands without specifying mode, causing it to default
to 'acp' instead of 'non_interactive'. Commands with supportedModes that
include 'non_interactive' but not 'acp' would be silently excluded.
- Apply the same fix in systemController.ts for the same reason.
- Update test mock to delegate filtering to production filterCommandsForMode()
instead of duplicating the logic inline, preventing divergence.
Fixes review comments by wenshao and tanzhenxin on PR #3283.
* fix: resolve TypeScript type error in nonInteractiveHelpers.test.ts
* fix test ci
This commit is contained in:
parent
6c999fe29f
commit
a82d766727
62 changed files with 2350 additions and 307 deletions
|
|
@ -17,10 +17,10 @@ import { BuiltinCommandLoader } from './services/BuiltinCommandLoader.js';
|
|||
import { BundledSkillLoader } from './services/BundledSkillLoader.js';
|
||||
import { FileCommandLoader } from './services/FileCommandLoader.js';
|
||||
import {
|
||||
CommandKind,
|
||||
type CommandContext,
|
||||
type SlashCommand,
|
||||
type SlashCommandActionReturn,
|
||||
type ExecutionMode,
|
||||
} from './ui/commands/types.js';
|
||||
import { createNonInteractiveUI } from './ui/noninteractive/nonInteractiveUi.js';
|
||||
import type { LoadedSettings } from './config/settings.js';
|
||||
|
|
@ -29,27 +29,6 @@ import { t } from './i18n/index.js';
|
|||
|
||||
const debugLogger = createDebugLogger('NON_INTERACTIVE_COMMANDS');
|
||||
|
||||
/**
|
||||
* Built-in commands that are allowed in non-interactive modes (CLI and ACP).
|
||||
* Only safe, read-only commands that don't require interactive UI.
|
||||
*
|
||||
* These commands are:
|
||||
* - init: Initialize project configuration
|
||||
* - summary: Generate session summary
|
||||
* - compress: Compress conversation history
|
||||
* - context: Show context window usage (read-only diagnostic)
|
||||
* - doctor: Run installation and environment diagnostics (read-only diagnostic)
|
||||
*/
|
||||
export const ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE = [
|
||||
'init',
|
||||
'summary',
|
||||
'compress',
|
||||
'btw',
|
||||
'bug',
|
||||
'context',
|
||||
'doctor',
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* Result of handling a slash command in non-interactive mode.
|
||||
*
|
||||
|
|
@ -187,36 +166,6 @@ function handleCommandResult(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters commands based on the allowed built-in command names.
|
||||
*
|
||||
* - Always includes FILE commands
|
||||
* - Only includes BUILT_IN commands if their name is in the allowed set
|
||||
* - Excludes other command types (e.g., MCP_PROMPT) in non-interactive mode
|
||||
*
|
||||
* @param commands All loaded commands
|
||||
* @param allowedBuiltinCommandNames Set of allowed built-in command names (empty = none allowed)
|
||||
* @returns Filtered commands
|
||||
*/
|
||||
function filterCommandsForNonInteractive(
|
||||
commands: readonly SlashCommand[],
|
||||
allowedBuiltinCommandNames: Set<string>,
|
||||
): SlashCommand[] {
|
||||
return commands.filter((cmd) => {
|
||||
if (cmd.kind === CommandKind.FILE || cmd.kind === CommandKind.SKILL) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Built-in commands: only include if in the allowed list
|
||||
if (cmd.kind === CommandKind.BUILT_IN) {
|
||||
return allowedBuiltinCommandNames.has(cmd.name);
|
||||
}
|
||||
|
||||
// Exclude other types (e.g., MCP_PROMPT) in non-interactive mode
|
||||
return false;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Processes a slash command in a non-interactive environment.
|
||||
*
|
||||
|
|
@ -224,9 +173,6 @@ function filterCommandsForNonInteractive(
|
|||
* @param abortController Controller to cancel the operation
|
||||
* @param config The configuration object
|
||||
* @param settings The loaded settings
|
||||
* @param allowedBuiltinCommandNames Optional array of built-in command names that are
|
||||
* allowed. Defaults to ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE (init, summary, compress).
|
||||
* Pass an empty array to only allow file commands.
|
||||
* @returns A Promise that resolves to a `NonInteractiveSlashCommandResult` describing
|
||||
* the outcome of the command execution.
|
||||
*/
|
||||
|
|
@ -235,9 +181,6 @@ export const handleSlashCommand = async (
|
|||
abortController: AbortController,
|
||||
config: Config,
|
||||
settings: LoadedSettings,
|
||||
allowedBuiltinCommandNames: string[] = [
|
||||
...ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE,
|
||||
],
|
||||
): Promise<NonInteractiveSlashCommandResult> => {
|
||||
const trimmed = rawQuery.trim();
|
||||
if (!trimmed.startsWith('/')) {
|
||||
|
|
@ -247,26 +190,13 @@ export const handleSlashCommand = async (
|
|||
const isAcpMode = config.getExperimentalZedIntegration();
|
||||
const isInteractive = config.isInteractive();
|
||||
|
||||
const executionMode = isAcpMode
|
||||
const executionMode: ExecutionMode = isAcpMode
|
||||
? 'acp'
|
||||
: isInteractive
|
||||
? 'interactive'
|
||||
: 'non_interactive';
|
||||
|
||||
const allowedBuiltinSet = new Set(allowedBuiltinCommandNames ?? []);
|
||||
const disabledSlashCommandsRaw = config.getDisabledSlashCommands();
|
||||
const disabledNameSet = new Set<string>();
|
||||
for (const name of disabledSlashCommandsRaw) {
|
||||
const trimmed = name.trim();
|
||||
if (trimmed) disabledNameSet.add(trimmed.toLowerCase());
|
||||
}
|
||||
const isDisabled = (cmd: { name: string }) =>
|
||||
disabledNameSet.has(cmd.name.toLowerCase());
|
||||
|
||||
// Load the full command set (unfiltered by the denylist) so that the
|
||||
// fallback existence check below can distinguish a disabled command from a
|
||||
// truly unknown one. Without this, a disabled command would fall through to
|
||||
// `no_command` and be forwarded to the model as plain prompt text.
|
||||
// Load all commands to check if the command exists but is not allowed
|
||||
const allLoaders = [
|
||||
new BuiltinCommandLoader(config),
|
||||
new BundledSkillLoader(config),
|
||||
|
|
@ -278,10 +208,7 @@ export const handleSlashCommand = async (
|
|||
abortController.signal,
|
||||
);
|
||||
const allCommands = commandService.getCommands();
|
||||
const filteredCommands = filterCommandsForNonInteractive(
|
||||
allCommands,
|
||||
allowedBuiltinSet,
|
||||
).filter((cmd) => !isDisabled(cmd));
|
||||
const filteredCommands = commandService.getCommandsForMode(executionMode);
|
||||
|
||||
// First, try to parse with filtered commands
|
||||
const { commandToExecute, args } = parseSlashCommand(
|
||||
|
|
@ -297,23 +224,12 @@ export const handleSlashCommand = async (
|
|||
);
|
||||
|
||||
if (knownCommand) {
|
||||
if (isDisabled(knownCommand)) {
|
||||
return {
|
||||
type: 'unsupported',
|
||||
reason: t(
|
||||
'The command "/{{command}}" is disabled by the current configuration.',
|
||||
{ command: knownCommand.name },
|
||||
),
|
||||
originalType: 'filtered_command',
|
||||
};
|
||||
}
|
||||
// Command exists but is not allowed in non-interactive mode
|
||||
// Command exists but is not allowed in this mode
|
||||
return {
|
||||
type: 'unsupported',
|
||||
reason: t(
|
||||
'The command "/{{command}}" is not supported in non-interactive mode.',
|
||||
{ command: knownCommand.name },
|
||||
),
|
||||
reason: t('The command "/{{command}}" is not supported in this mode.', {
|
||||
command: knownCommand.name,
|
||||
}),
|
||||
originalType: 'filtered_command',
|
||||
};
|
||||
}
|
||||
|
|
@ -372,51 +288,27 @@ export const handleSlashCommand = async (
|
|||
};
|
||||
|
||||
/**
|
||||
* Retrieves all available slash commands for the current configuration.
|
||||
* Retrieves all available slash commands for the given execution mode.
|
||||
*
|
||||
* @param config The configuration object
|
||||
* @param abortSignal Signal to cancel the loading process
|
||||
* @param allowedBuiltinCommandNames Optional array of built-in command names that are
|
||||
* allowed. Defaults to ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE (init, summary, compress).
|
||||
* Pass an empty array to only include file commands.
|
||||
* @param mode The execution mode to filter commands for. Defaults to 'acp'.
|
||||
* @returns A Promise that resolves to an array of SlashCommand objects
|
||||
*/
|
||||
export const getAvailableCommands = async (
|
||||
config: Config,
|
||||
abortSignal: AbortSignal,
|
||||
allowedBuiltinCommandNames: string[] = [
|
||||
...ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE,
|
||||
],
|
||||
mode: ExecutionMode = 'acp',
|
||||
): Promise<SlashCommand[]> => {
|
||||
try {
|
||||
const allowedBuiltinSet = new Set(allowedBuiltinCommandNames ?? []);
|
||||
const loaders = [
|
||||
new BuiltinCommandLoader(config),
|
||||
new BundledSkillLoader(config),
|
||||
new FileCommandLoader(config),
|
||||
];
|
||||
|
||||
// Only load BuiltinCommandLoader if there are allowed built-in commands
|
||||
const loaders =
|
||||
allowedBuiltinSet.size > 0
|
||||
? [
|
||||
new BuiltinCommandLoader(config),
|
||||
new BundledSkillLoader(config),
|
||||
new FileCommandLoader(config),
|
||||
]
|
||||
: [new BundledSkillLoader(config), new FileCommandLoader(config)];
|
||||
|
||||
const disabledSlashCommands = config.getDisabledSlashCommands();
|
||||
const commandService = await CommandService.create(
|
||||
loaders,
|
||||
abortSignal,
|
||||
disabledSlashCommands.length > 0
|
||||
? new Set(disabledSlashCommands)
|
||||
: undefined,
|
||||
);
|
||||
const commands = commandService.getCommands();
|
||||
const filteredCommands = filterCommandsForNonInteractive(
|
||||
commands,
|
||||
allowedBuiltinSet,
|
||||
);
|
||||
|
||||
// Filter out hidden commands
|
||||
return filteredCommands.filter((cmd) => !cmd.hidden);
|
||||
const commandService = await CommandService.create(loaders, abortSignal);
|
||||
return commandService.getCommandsForMode(mode) as SlashCommand[];
|
||||
} catch (error) {
|
||||
// Handle errors gracefully - log and return empty array
|
||||
debugLogger.error('Error loading available commands:', error);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue