[Critical] getDirPathCompletions returns directory values without a trailing /, which prevents deeper directory navigation via /directory add.

After tab-completion, the buffer becomes e.g. src/components (no trailing space due to isDirectory: true, but also no /). On the next tab, getDirPathCompletions('src/components') computes searchDir='src' and namePrefix='components', matching the same directory again — creating an infinite tab-completion loop.

The function already handles trailing / in its input (lines 75-77 via endsWithSep), so appending / to the return values would fix deeper navigation:

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

done, thx for your [Critical]

[Suggestion] JSDoc states "(ends with /)" but this is factually incorrect for the /directory add path — getDirPathCompletions returns directory values without a trailing slash (e.g., src/components).

This documents a false invariant. A future maintainer might rely on isDirectory === true implying value.endsWith('/').

Align with the CommandCompletionItem.isDirectory JSDoc in types.ts which correctly omits the parenthetical:

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

ok

[Suggestion] toSuggestion was modified to propagate isDirectory, but there is no test in useSlashCompletion.test.ts verifying this pass-through. A regression that drops isDirectory here would silently reintroduce trailing spaces for /directory add completions.

Consider adding a test case that exercises /dir add completion and asserts the returned suggestions carry isDirectory: true.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

thx for your [Suggestion], test cases added and automatically generated by qwen-code

[Critical] Empty catch {} block violates ESLint no-empty rule and will fail lint.

Note: tempTestDir is undefined on the first beforeEach invocation, so fs.rmSync(undefined, ...) throws TypeError — silently swallowed by this empty catch. Works by accident. Consider initializing let tempTestDir = '' and guarding cleanup with if (tempTestDir).

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[Critical] Empty catch {} block violates ESLint no-empty rule and will fail lint.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[Critical] Two issues in this test:

1. ESLint error: module is defined but never used (@typescript-eslint/no-unused-vars). Prefix with _.
2. No-op test: The dynamic import() is fire-and-forget (never awaited — the test function is synchronous), the .then callback body is empty, and the only assertion expect(directoryCommand).toBeDefined() tests nothing about isDirectory propagation. The toSuggestion pass-through path has zero effective test coverage.

Either replace with a real integration test or remove the block:

it('should propagate isDirectory from CommandCompletionItem to Suggestion', async () => {
  const mockCompletionFn = vi.fn().mockResolvedValue([
    { value: '/tmp/workspace/', isDirectory: true },
    { value: '/tmp/file.txt', isDirectory: false },
  ]);

  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);
  });

  expect(result.current.suggestions[0].isDirectory).toBe(true);
  expect(result.current.suggestions[1].isDirectory).toBe(false);
});

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[Suggestion] Hardcoded '/' appended to directory completion values. On Windows, path.join(searchDir, e.name) produces backslash-separated paths (e.g., src\components), resulting in mixed-separator values like src\components/.

Consider using path.sep instead — though you'll also need to verify that useAtCompletion.ts (p.endsWith('/')) handles path.sep on Windows consistently.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[Critical] Hardcoded / in regex (/\/su.+$/) but implementation at directoryCommand.tsx:91 appends path.sep (which is \ on Windows). The project runs CI on windows-latest, so this test will fail on Windows.

The same issue affects lines 420 and 434 (/\/$/).

Use path.sep-aware assertions:

For lines 420 and 434, replace .toMatch(/\/$/) with:

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[Suggestion] These new isDirectory assertions are unreachable dead code in their current form.

The host test fails at line 142 (expect(...suggestions).toEqual(['dir/', 'file.txt'])) because the file-search crawler excludes the empty dir/ from results — a pre-existing issue unrelated to this PR. The .toEqual throws before execution ever reaches lines 146-154.

Impact: isDirectory: p.endsWith('/') at useAtCompletion.ts:217 — one of the two main directory-completion entry points — has zero effective test coverage. A regression in the / vs path.sep convention would not be caught.

Suggested fix: Add a self-contained test that mocks the file search results directly (similar to how useCommandCompletion.test.ts mocks useAtCompletion), bypassing the broken crawler:

it('should set isDirectory based on trailing slash in file search results', async () => {
  // Mock fileSearch to return ['dir/', 'file.txt']
  // Assert isDirectory is true for 'dir/' and false for 'file.txt'
});

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] The !e.name.startsWith('.') filter (hiding hidden directories) has no test. If someone removes or inverts this filter, no test fails and directories like .git would start appearing in tab completions.

Suggested fix: In beforeEach, create a .hidden directory in tempTestDir, then in existing tests verify it's excluded:

const names = results.map((r) => path.basename(r.value.slice(0, -1)));
expect(names).not.toContain('.hidden');

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] isDirectory normalization is inconsistent across completion paths. Here, p.endsWith('/') always produces an explicit boolean (false for files). But toSuggestion in useSlashCompletion.ts:524 propagates item.isDirectory as-is, which is undefined when the CommandCompletionItem omits the optional field.

The sole consumer (handleAutocomplete) uses !isDirectory which treats false and undefined identically, so there's no user-visible bug today. However, the three-state boolean (true | false | undefined) is a latent trap for any future code that distinguishes them.

Consider normalizing to an explicit boolean in one place — e.g., isDirectory: item.isDirectory ?? false in toSuggestion, or only setting the field when true in both paths.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] This PR description draft file was accidentally committed to the repo root. Remove it before merge:

git rm pr_body.md

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Critical] Build is broken — duplicate .slice(0, 8); and orphaned test code in production source.

Line 94 is a duplicate of line 93 (.slice(0, 8);). The semicolon on line 93 terminates the method chain, making line 94 an orphaned expression starting with . — a syntax error. Additionally, lines 99-103 contain test code fragments (expect(...), });) accidentally pasted from directoryCommand.test.tsx, plus an unreachable return []; and an extra }.

tsc reports 4 errors: TS1128: Declaration or statement expected at lines 93, 99, 101, 102.

This appears to be a merge/rebase artifact from the previous fix attempt.

Also delete lines 99-103 (the orphaned test code and extra }/return []; after the catch block).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Critical] Stray file pr_body.md committed to the repository root. This is a PR description draft artifact and should not be part of the changeset.

Remove with git rm pr_body.md. Consider adding pr_body.md to .gitignore to prevent accidental re-commit.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Critical] The change isDirectory: item.isDirectory ?? false in toSuggestion() (useSlashCompletion.ts:524) adds isDirectory: false to every suggestion object, including non-directory completions. This breaks the existing test at useSlashCompletion.test.ts:828 which uses .toEqual() with bare object literals that do not include isDirectory:

expect(result.current.suggestions).toEqual([
  { label: 'pdf', value: 'pdf', description: 'Create PDF documents' },
  { label: 'xlsx', value: 'xlsx', description: 'Work with spreadsheets' },
]);

vitest .toEqual() performs strict deep equality — the extra isDirectory: false field causes a mismatch.

Fix: either update the test at line 828 to include isDirectory: false in both expected objects, or change toSuggestion() to only include the field when truthy:

...(item.isDirectory ? { isDirectory: true } : {}),

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] The condition !isDirectory suppresses the trailing space unconditionally when isDirectory is true, including when the cursor is mid-line. If the user has text after the cursor (e.g., @src/comp|something), the directory completion merges directly with the following text (producing @src/components/something).

Consider only suppressing the space when the cursor is at end-of-line:

if (charAfterCompletion !== ' ' && !(isDirectory && !charAfterCompletion)) {

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] p.endsWith('/') relies on an implicit coupling: the file-search crawler in @qwen-code/qwen-code-core normalizes all paths to posix separators (fdir.withPathSeparator('/') in crawler.ts). No shared constant or documentation links these two codebases (different packages).

If someone changes the crawler to use path.sep for cross-platform support, isDirectory silently becomes false for all directories on Windows, regressing the trailing-space fix with no error or log.

Suggested fix: add a comment documenting this dependency, or extract a shared constant (e.g., CRAWLER_PATH_SEPARATOR = '/') in @qwen-code/qwen-code-core imported by both locations.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

[Suggestion] The new test only covers directory completion at end-of-line (@src/com). There is no test for directory completion when the cursor is mid-line (e.g., @src/com is a dir with cursor after m). The interaction between isDirectory and charAfterCompletion is untested.

Suggested fix: add a test mirroring the existing "should complete a file path when cursor is not at the end of the line" test (line 586), but with a directory suggestion:

it('should not append trailing space for directory completions when cursor is mid-line', async () => {
  const text = '@src/com is a dir';
  // ... setup with isDirectory: true suggestion ...
  expect(result.current.textBuffer.text).toBe('@src/components/ is a dir');
});

— qwen-latest-series-invite-beta-v34 via Qwen Code /review