feat(cli): do not append trailing space for directory completions (#4092) (#4288)

* feat(cli): do not append trailing space for directory completions (#4092)

## What

在 @路径补全和 /dir add 命令的目录补全中不再追加尾部空格。这样可以允许用户在补全目录后直接按 Tab 继续深入下一级子目录，无需先删除空格。

## Examples

- Input: `@src/com` + Tab → Output: `@src/components/` (no trailing space)

- Input: `/dir add ./pac` + Tab → Output: `/dir add ./packages/` (no trailing space)

- File completions still append a space (e.g., `@src/file.txt `)

## Changes

- Added `isDirectory` flag to `Suggestion` and `CommandCompletionItem` interfaces

- Updated `handleAutocomplete` to skip trailing space when `isDirectory === true`

- Modified `getDirPathCompletions` to return `CommandCompletionItem[]` with `isDirectory: true`

- Added test case for directory completion behavior

* fix(cli): append trailing / to directory completions for deeper navigation

* fix(cli): propagate isDirectory and fix JSDoc comment

## Comment 2: Fix JSDoc in SuggestionsDisplay

Removed "(ends with /)" from isDirectory description since it was factually incorrect.

## Comment 3: Add test for isDirectory propagation

- Added test suite in useSlashCompletion.test.ts to verify directory command structure

- Real filesystem testing is done in directoryCommand.test.tsx

* fix(cli): add comprehensive isDirectory propagation tests

Added getDirPathCompletions unit tests that verify:
- Directory suggestions include isDirectory: true
- Directory values end with / for continued navigation
- Prefix filtering preserves isDirectory flag
- Comma-separated path completion works correctly
- Deeply nested directories maintain isDirectory flag

This closes the testing gap identified in review comment 3.

* fix(cli): address wenshao feedback - lint rules, real test, cross-platform

Fixes 4 new review comments from wenshao:

- [Critical] Empty catch {} blocks: guarded with if (tempTestDir) + void err

- [Critical] useSlashCompletion.no-op test: replaced with real integration test that

  verifies isDirectory propagation through toSuggestion pass-through

- [Suggestion] Windows path separator: using path.sep instead of hardcoded /

  in both directoryCommand.tsx and related test assertions

* fix(cli): remove unused import and fix Windows path separator in tests

- Remove unused directoryCommand import in useSlashCompletion.test.ts (TS6133)
- Replace hardcoded / regex with path.sep-aware assertions in
  directoryCommand.test.tsx to fix Windows CI failures

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Apply suggestion from @wenshao

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.test.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(cli): normalize isDirectory to explicit boolean in toSuggestion

Normalize isDirectory from three-state (true/false/undefined) to explicit
boolean (true/false) to prevent latent bugs in future code that might
distinguish between false and undefined.

Fixes review comment: isDirectory normalization is inconsistent across
completion paths.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/cli/src/ui/hooks/useSlashCompletion.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* chore: remove accidentally committed pr_body.md

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore: add pr_body.md to .gitignore

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): remove duplicate .slice and orphaned test code from directoryCommand.tsx

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): only suppress trailing space for dir completions at end-of-line

When isDirectory is true, the trailing space was suppressed unconditionally,
even when the cursor is mid-line. This caused directory completions to merge
directly with following text (e.g. '@src/components/something').

Now only suppress the space when the cursor is at end-of-line, allowing
continued Tab navigation into subdirectories.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(cli): document crawler path separator dependency for isDirectory check

The isDirectory detection uses p.endsWith('/') which depends on the
crawler in @qwen-code/qwen-code-core normalizing paths with posix '/'
(fdir.withPathSeparator('/') in crawler.ts). Add a comment to make this
implicit coupling explicit.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): add mid-line directory completion test

Verify that directory completions append a trailing space when the cursor
is mid-line, preventing the completed path from merging with following text.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/cli/src/ui/hooks/useCommandCompletion.test.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

---------

Co-authored-by: 方磊 <fanglei@192.168.1.11>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

.gitignore

@@ -56,6 +56,9 @@ bundle
 junit.xml
 packages/*/coverage/
 
+# PR body draft
+pr_body.md
+
 # Generated files
 packages/cli/src/generated/
 packages/core/src/generated/

packages/cli/src/ui/commands/directoryCommand.test.tsx

@@ -5,13 +5,18 @@
  */
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
-import { directoryCommand, expandHomeDir } from './directoryCommand.js';
+import {
+  directoryCommand,
+  expandHomeDir,
+  getDirPathCompletions,
+} from './directoryCommand.js';
 import type { Config, WorkspaceContext } from '@qwen-code/qwen-code-core';
 import type { CommandContext } from './types.js';
 import { MessageType } from '../types.js';
 import { SettingScope } from '../../config/settings.js';
 import * as os from 'node:os';
 import * as path from 'node:path';
+import * as fs from 'node:fs';
 
 describe('directoryCommand', () => {
   let mockContext: CommandContext;
@@ -323,3 +328,116 @@ describe('directoryCommand', () => {
     );
   });
 });
+
+describe('getDirPathCompletions', () => {
+  let tempTestDir = '';
+
+  beforeEach(() => {
+    // Clean up any previous test runs
+    if (tempTestDir) {
+      try {
+        fs.rmSync(tempTestDir, { recursive: true, force: true });
+      } catch (err) {
+        // ignore cleanup errors
+        void err;
+      }
+    }
+
+    tempTestDir = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-dir-test-'));
+    // Create a nested directory structure: root/sub1, root/sub2, root/sub1/deep
+    fs.mkdirSync(tempTestDir, { recursive: true });
+    fs.mkdirSync(path.join(tempTestDir, 'sub1'), { recursive: true });
+    fs.mkdirSync(path.join(tempTestDir, 'sub2'), { recursive: true });
+    fs.mkdirSync(path.join(tempTestDir, 'sub1', 'deep'), { recursive: true });
+    // Add some non-directory files (should be filtered out)
+    fs.writeFileSync(path.join(tempTestDir, 'file.txt'), '');
+    fs.writeFileSync(
+      path.join(tempTestDir, 'sub1', 'nested.txt'),
+      '',
+    );
+  });
+
+  afterAll(() => {
+    // Cleanup after all tests
+    if (tempTestDir) {
+      try {
+        fs.rmSync(tempTestDir, { recursive: true, force: true });
+      } catch (err) {
+        // ignore cleanup errors
+        void err;
+      }
+    }
+  });
+
+  describe('directory completions should include isDirectory flag', () => {
+    it('should return suggestions with isDirectory: true and trailing /', () => {
+      // Use "/" suffix so getDirPathCompletions searches INSIDE the directory
+      const results = getDirPathCompletions(`${tempTestDir}/`);
+
+      expect(results.length).toBeGreaterThan(0);
+
+      // Each suggestion should be a CommandCompletionItem with isDirectory: true
+      results.forEach((suggestion) => {
+        expect(suggestion.value).toBeDefined();
+        expect(suggestion.isDirectory).toBe(true);
+
+        // Directory values should end with path separator for continued navigation
+        expect(suggestion.value.endsWith(path.sep)).toBe(true);
+
+        // Should match one of our created directories
+        const dirNameWithoutSlash = suggestion.value.slice(0, -1);
+        const basename = path.basename(dirNameWithoutSlash);
+        expect(['sub1', 'sub2'].includes(basename)).toBe(true);
+      });
+    });
+
+    it('should filter by prefix while preserving isDirectory flag', () => {
+      const results = getDirPathCompletions(`${tempTestDir}/su`);
+
+      expect(results.length).toBeGreaterThan(0);
+
+      // Only directories starting with "su" should be returned
+      results.forEach((suggestion) => {
+        expect(suggestion.isDirectory).toBe(true);
+        const sepRe = path.sep === '\\' ? '\\\\' : path.sep;
+        expect(suggestion.value).toMatch(new RegExp(`${sepRe}su.+$`));
+        // Only top-level directories matching the prefix are returned
+        const basename = path.basename(suggestion.value.slice(0, -1));
+        expect(basename).toMatch(/^su/);
+        const dirname = path.dirname(suggestion.value);
+        expect(dirname).toContain(tempTestDir);
+      });
+    });
+
+    it('should support comma-separated paths with isDirectory flag on last segment', () => {
+      const multiPath = `${tempTestDir}, ${tempTestDir}/`;
+      const results = getDirPathCompletions(multiPath);
+
+      expect(results.length).toBeGreaterThan(0);
+
+      // Results should start with the prefix from first part
+      results.forEach((suggestion) => {
+        expect(suggestion.isDirectory).toBe(true);
+        expect(suggestion.value.startsWith(`${tempTestDir}`)).toBe(true);
+        expect(suggestion.value.endsWith(path.sep)).toBe(true);
+      });
+    });
+
+    it('should handle deeply nested directories with isDirectory flag', () => {
+      // Navigate into sub1
+      const deepResults = getDirPathCompletions(`${tempTestDir}/sub1/`);
+
+      expect(deepResults.length).toBeGreaterThan(0);
+
+      // Only directories inside sub1 should be returned
+      deepResults.forEach((suggestion) => {
+        expect(suggestion.isDirectory).toBe(true);
+        expect(suggestion.value).toContain('sub1');
+        expect(suggestion.value.endsWith(path.sep)).toBe(true);
+        // The nested 'deep' directory should be in the results
+        const basename = path.basename(suggestion.value.slice(0, -1));
+        expect(basename).toBe('deep');
+      });
+    });
+  });
+});

packages/cli/src/ui/commands/directoryCommand.tsx

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import type { SlashCommand, CommandContext } from './types.js';
+import type { SlashCommand, CommandContext, CommandCompletionItem } from './types.js';
 import { CommandKind } from './types.js';
 import { MessageType } from '../types.js';
 import * as fs from 'node:fs';
@@ -58,7 +58,7 @@ function findExistingWorkspaceDirectory(
  * Returns directory path completions for the given partial argument.
  * Supports comma-separated paths by completing only the last segment.
  */
-export function getDirPathCompletions(partialArg: string): string[] {
+export function getDirPathCompletions(partialArg: string): CommandCompletionItem[] {
   const lastComma = partialArg.lastIndexOf(',');
   const prefix = lastComma >= 0 ? partialArg.substring(0, lastComma + 1) : '';
   const partial =
@@ -85,7 +85,10 @@ export function getDirPathCompletions(partialArg: string): string[] {
           e.name.startsWith(namePrefix) &&
           !e.name.startsWith('.'),
       )
-      .map((e) => prefix + path.join(searchDir, e.name))
+      .map((e) => ({
+        value: prefix + path.join(searchDir, e.name) + path.sep,
+        isDirectory: true,
+      }))
       .slice(0, 8);
   } catch {
     return [];

packages/cli/src/ui/commands/types.ts

@@ -292,6 +292,8 @@ export interface CommandCompletionItem {
   value: string;
   label?: string;
   description?: string;
+  /** Whether the completion represents a directory path. When true, handleAutocomplete should NOT append a trailing space so the user can continue tab-completing deeper into the directory tree. */
+  isDirectory?: boolean;
 }
 
 // The standardized contract for any command in the system.

packages/cli/src/ui/components/SuggestionsDisplay.tsx

@@ -28,6 +28,8 @@ export interface Suggestion {
   matchedAlias?: string;
   supportedModes?: ExecutionMode[];
   modelInvocable?: boolean;
+  /** Whether the suggestion represents a directory path. When true, handleAutocomplete should NOT append a trailing space so the user can continue tab-completing deeper into the directory tree. */
+  isDirectory?: boolean;
 }
 interface SuggestionsDisplayProps {
   suggestions: Suggestion[];

packages/cli/src/ui/hooks/useAtCompletion.test.ts

@@ -143,6 +143,15 @@ describe('useAtCompletion', () => {
         'dir/',
         'file.txt',
       ]);
+      // Verify isDirectory flag
+      const dirSuggestion = result.current.suggestions.find(
+        (s) => s.value === 'dir/',
+      );
+      const fileSuggestion = result.current.suggestions.find(
+        (s) => s.value === 'file.txt',
+      );
+      expect(dirSuggestion?.isDirectory).toBe(true);
+      expect(fileSuggestion?.isDirectory).toBe(false);
     });
   });
 

packages/cli/src/ui/hooks/useAtCompletion.ts

@@ -211,9 +211,13 @@ export function useAtCompletion(props: UseAtCompletionProps): void {
           return;
         }
 
+        // isDirectory relies on crawler.ts in @qwen-code/qwen-code-core
+        // always normalizing paths with posix '/' via fdir.withPathSeparator('/').
+        // If the crawler ever switches to path.sep, this check must be updated.
         const suggestions = results.map((p) => ({
           label: p,
           value: escapePath(p),
+          isDirectory: p.endsWith('/'),
         }));
         dispatch({ type: 'SEARCH_SUCCESS', payload: suggestions });
       } catch (error) {

packages/cli/src/ui/hooks/useCommandCompletion.test.ts

@@ -552,6 +552,37 @@ describe('useCommandCompletion', () => {
       expect(result.current.textBuffer.text).toBe('@src/file1.txt ');
     });
 
+    it('should not append trailing space for directory completions', async () => {
+      setupMocks({
+        atSuggestions: [
+          { label: 'src/components/', value: 'src/components/', isDirectory: true },
+        ],
+      });
+
+      const { result } = renderHook(() => {
+        const textBuffer = useTextBufferForTest('@src/com');
+        const completion = useCommandCompletion(
+          textBuffer,
+          testRootDir,
+          [],
+          mockCommandContext,
+          false,
+          mockConfig,
+        );
+        return { ...completion, textBuffer };
+      });
+
+      await waitFor(() => {
+        expect(result.current.suggestions.length).toBe(1);
+      });
+
+      act(() => {
+        result.current.handleAutocomplete(0);
+      });
+
+      expect(result.current.textBuffer.text).toBe('@src/components/');
+    });
+
     it('should complete a file path when cursor is not at the end of the line', async () => {
       const text = '@src/fi is a good file';
       const cursorOffset = 7; // after "i"
@@ -585,6 +616,46 @@ describe('useCommandCompletion', () => {
         '@src/file1.txt is a good file',
       );
     });
+
+    it('should preserve existing space after directory completions at mid-line cursor', async () => {
+      const text = '@src/com is a dir';
+      const cursorOffset = 8; // after "m"
+
+      setupMocks({
+        atSuggestions: [
+          {
+            label: 'src/components/',
+            value: 'src/components/',
+            isDirectory: true,
+          },
+        ],
+      });
+
+      const { result } = renderHook(() => {
+        const textBuffer = useTextBufferForTest(text, cursorOffset);
+        const completion = useCommandCompletion(
+          textBuffer,
+          testRootDir,
+          [],
+          mockCommandContext,
+          false,
+          mockConfig,
+        );
+        return { ...completion, textBuffer };
+      });
+
+      await waitFor(() => {
+        expect(result.current.suggestions.length).toBe(1);
+      });
+
+      act(() => {
+        result.current.handleAutocomplete(0);
+      });
+
+      expect(result.current.textBuffer.text).toBe(
+        '@src/components/ is a dir',
+      );
+    });
   });
 
   describe('argument hint ghost text', () => {

packages/cli/src/ui/hooks/useCommandCompletion.tsx

@@ -228,7 +228,8 @@ export function useCommandCompletion(
 
       const lineCodePoints = toCodePoints(buffer.lines[cursorRow] || '');
       const charAfterCompletion = lineCodePoints[end];
-      if (charAfterCompletion !== ' ') {
+      const isDirectory = suggestions[indexToUse].isDirectory;
+      if (charAfterCompletion !== ' ' && !(isDirectory && !charAfterCompletion)) {
         suggestionText += ' ';
       }
 

packages/cli/src/ui/hooks/useSlashCompletion.test.ts

@@ -1122,4 +1122,39 @@ describe('useSlashCompletion', () => {
     expect(mockSetIsLoadingSuggestions).not.toHaveBeenCalled();
     expect(mockSetIsPerfectMatch).not.toHaveBeenCalled();
   });
+
+  describe('isDirectory propagation', () => {
+    it('should propagate isDirectory from CommandCompletionItem to Suggestion', async () => {
+      const mockCompletionFn = vi.fn().mockResolvedValue([
+        { value: '/tmp/workspace/', isDirectory: true },
+        { value: '/tmp/file.txt' },
+      ]);
+
+      const slashCommands = [
+        createTestCommand({
+          name: 'dir',
+          description: 'test',
+          completion: mockCompletionFn,
+        }),
+      ];
+
+      const { result } = renderHook(() =>
+        useTestHarnessForSlashCompletion(
+          true,
+          '/dir ',
+          slashCommands,
+          mockCommandContext,
+        ),
+      );
+
+      await waitFor(() => {
+        expect(result.current.suggestions.length).toBe(2);
+      });
+
+      // First suggestion (directory) should have isDirectory: true
+      expect(result.current.suggestions[0].isDirectory).toBe(true);
+      // Second suggestion (file) should NOT have isDirectory flag
+      expect(result.current.suggestions[1].isDirectory).toBeFalsy();
+    });
+  });
 });

packages/cli/src/ui/hooks/useSlashCompletion.ts

@@ -521,6 +521,7 @@ function toSuggestion(item: string | CommandCompletionItem): Suggestion | null {
     label: item.label ?? item.value,
     value: item.value,
     description: item.description,
+    ...(item.isDirectory !== undefined && { isDirectory: item.isDirectory }),
   };
 }