From 6847c1dfe917c6eb2c10edfb98dc541573b3e9a8 Mon Sep 17 00:00:00 2001 From: KamijoToma Date: Fri, 19 Jun 2026 00:35:48 +0800 Subject: [PATCH] feat(coding-agent): add directory tab completion for /move command - Add buildDirectoryArgumentCompletions() to suggest directories relative to project dir - Support relative paths, ./, ../, ~/ and absolute paths - Allow spaces in destination paths (e.g. My Project/src/) - Use expandTilde() so ~foo prefixes resolve correctly and completions are resolvable - Emit display values that preserve the user's prefix style - Expand ~ in /move handler so accepted completions move to the intended directory - Wire /move command to the new completion provider - Add unit tests covering empty prefix, filtering, subdirs, relative/parent/home/absolute paths, and space-containing directories - Mock os.homedir() in home-relative test for deterministic results - Update CHANGELOG --- packages/coding-agent/CHANGELOG.md | 4 + .../src/slash-commands/builtin-registry.ts | 116 ++++++++++++++- .../slash-commands/move-completion.test.ts | 140 ++++++++++++++++++ 3 files changed, 257 insertions(+), 3 deletions(-) create mode 100644 packages/coding-agent/test/slash-commands/move-completion.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7025ca189..f33be1d9f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added Tab completion for `/move` destination directories. Typing `/move ` now offers directory suggestions relative to the current working directory, including support for `~`, absolute, and relative paths. + ### Fixed - Fixed `omp update` reporting `EPERM: operation not permitted, unlink '.bak'` on Windows when self-replacing a standalone binary, even though the new binary had already been installed. The backed-up old executable is still the running process image and cannot be unlinked until the process exits, so the post-verify backup cleanup is now best-effort, backups use a unique per-attempt name, and stale backups are swept on the next update ([#845](https://github.com/can1357/oh-my-pi/issues/845)). diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 2a2f1aeab..7fc1e1caf 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -4,7 +4,7 @@ import * as path from "node:path"; import { getOAuthProviders } from "@oh-my-pi/pi-ai/oauth"; import { setNextRequestDebugPath } from "@oh-my-pi/pi-ai/utils/request-debug"; import type { AutocompleteItem } from "@oh-my-pi/pi-tui"; -import { APP_NAME, setProjectDir } from "@oh-my-pi/pi-utils"; +import { APP_NAME, getProjectDir, setProjectDir } from "@oh-my-pi/pi-utils"; import { COLLAB_GUEST_ALLOWED_COMMANDS, CollabGuestLink } from "../collab/guest"; import { CollabHost } from "../collab/host"; import type { SettingPath, SettingValue } from "../config/settings"; @@ -28,6 +28,7 @@ import { theme } from "../modes/theme/theme"; import type { InteractiveModeContext } from "../modes/types"; import type { AgentSession, FreshSessionResult } from "../session/agent-session"; import { formatShakeSummary, type ShakeMode } from "../session/shake-types"; +import { expandTilde } from "../tools/path-utils"; import { urlHyperlinkAlways } from "../tui"; import { getChangelogPath, parseChangelog } from "../utils/changelog"; import { buildContextReportText } from "./helpers/context-report"; @@ -1491,7 +1492,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray = [ handle: async (command, runtime) => { if (runtime.session.isStreaming) return usage("Cannot move while streaming.", runtime); if (!command.args) return usage("Usage: /move ", runtime); - const resolvedPath = path.resolve(runtime.cwd, command.args); + const resolvedPath = path.resolve(runtime.cwd, expandTilde(command.args)); let isDirectory: boolean; try { isDirectory = (await fs.stat(resolvedPath)).isDirectory(); @@ -2184,6 +2185,106 @@ function buildStaticInlineHint(hint: string): (argumentText: string) => string | return (argumentText: string) => (argumentText.trim().length === 0 ? hint : null); } +/** + * Build getArgumentCompletions that suggests directories relative to the + * current project directory. Used by /move so users can Tab-complete the + * destination directory. + */ +function buildDirectoryArgumentCompletions(): (prefix: string) => Promise { + return async (argumentPrefix: string) => { + const prefix = argumentPrefix.trim(); + + const cwd = getProjectDir(); + const expandedPrefix = expandTilde(prefix); + const isAbsolute = path.isAbsolute(expandedPrefix); + + let searchDir: string; + let searchPrefix: string; + if ( + prefix === "" || + prefix === "." || + prefix === "./" || + prefix === ".." || + prefix === "../" || + prefix === "~" || + prefix === "~/" || + prefix === "/" + ) { + searchDir = isAbsolute ? expandedPrefix : path.join(cwd, expandedPrefix); + searchPrefix = ""; + } else if (expandedPrefix.endsWith("/")) { + searchDir = isAbsolute ? expandedPrefix : path.join(cwd, expandedPrefix); + searchPrefix = ""; + } else { + const dir = path.dirname(expandedPrefix); + searchDir = isAbsolute ? dir : path.join(cwd, dir); + searchPrefix = path.basename(expandedPrefix); + } + + try { + const entries = await fs.readdir(searchDir, { withFileTypes: true }); + const suggestions: AutocompleteItem[] = []; + for (const entry of entries) { + if (!entry.name.toLowerCase().startsWith(searchPrefix.toLowerCase())) continue; + if (entry.name === ".git") continue; + + let isDirectory = entry.isDirectory(); + if (!isDirectory && entry.isSymbolicLink()) { + try { + isDirectory = (await fs.stat(path.join(searchDir, entry.name))).isDirectory(); + } catch { + continue; + } + } + if (!isDirectory) continue; + + const absoluteValue = path.join(searchDir, entry.name); + const displayValue = buildDirectoryCompletionDisplayValue(prefix, absoluteValue, cwd); + suggestions.push({ value: displayValue, label: `${entry.name}/` }); + } + suggestions.sort((a, b) => a.label.localeCompare(b.label)); + return suggestions.length > 0 ? suggestions : null; + } catch { + return null; + } + }; +} +function buildDirectoryCompletionDisplayValue(prefix: string, absoluteValue: string, cwd: string): string { + // Preserve the user's prefix style where possible, but always return a + // value that /move can resolve (absolute or relative) without escaping. + const normalized = path.normalize(absoluteValue); + + if (prefix.startsWith("~/")) { + const home = os.homedir(); + const homeRelative = path.relative(home, normalized); + return `~/${homeRelative.replaceAll("\\", "/")}/`; + } + if (prefix === "~") { + const home = os.homedir(); + const homeRelative = path.relative(home, normalized); + return `~/${homeRelative.replaceAll("\\", "/")}/`; + } + if (prefix.startsWith("/")) { + return `${normalized.replaceAll("\\", "/")}/`; + } + if (prefix.startsWith("./")) { + const relative = path.relative(cwd, normalized); + return `./${relative.replaceAll("\\", "/")}/`; + } + if (prefix.startsWith("../")) { + const relative = path.relative(cwd, normalized); + return `${relative.replaceAll("\\", "/")}/`; + } + if (prefix === "..") { + const relative = path.relative(cwd, normalized); + return `${relative.replaceAll("\\", "/")}/`; + } + + // Default: relative to cwd. + const relative = path.relative(cwd, normalized); + return `${relative.replaceAll("\\", "/")}/`; +} + /** Builtin command metadata used for slash-command autocomplete and help text. */ export const BUILTIN_SLASH_COMMAND_DEFS: ReadonlyArray = BUILTIN_SLASH_COMMAND_REGISTRY.map( command => ({ @@ -2201,7 +2302,9 @@ export const BUILTIN_SLASH_COMMAND_DEFS: ReadonlyArray = BU */ export const BUILTIN_SLASH_COMMANDS: ReadonlyArray< BuiltinSlashCommand & { - getArgumentCompletions?: (prefix: string) => AutocompleteItem[] | null; + getArgumentCompletions?: + | ((prefix: string) => AutocompleteItem[] | null) + | ((prefix: string) => Promise); getInlineHint?: (argumentText: string) => string | null; } > = BUILTIN_SLASH_COMMAND_DEFS.map(cmd => { @@ -2212,6 +2315,13 @@ export const BUILTIN_SLASH_COMMANDS: ReadonlyArray< getInlineHint: buildSubcommandInlineHint(cmd.subcommands), }; } + if (cmd.name === "move") { + return { + ...cmd, + getArgumentCompletions: buildDirectoryArgumentCompletions(), + ...(cmd.inlineHint ? { getInlineHint: buildStaticInlineHint(cmd.inlineHint) } : {}), + }; + } if (cmd.inlineHint) { return { ...cmd, diff --git a/packages/coding-agent/test/slash-commands/move-completion.test.ts b/packages/coding-agent/test/slash-commands/move-completion.test.ts new file mode 100644 index 000000000..a9eea4071 --- /dev/null +++ b/packages/coding-agent/test/slash-commands/move-completion.test.ts @@ -0,0 +1,140 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { BUILTIN_SLASH_COMMANDS } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry"; +import * as piUtils from "@oh-my-pi/pi-utils"; + +describe("/move directory completion", () => { + let tempDir: string; + const move = BUILTIN_SLASH_COMMANDS.find(c => c.name === "move"); + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-move-completion-")); + vi.spyOn(piUtils, "getProjectDir").mockReturnValue(tempDir); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it("is wired to the /move command", () => { + expect(move).toBeDefined(); + expect(move!.getArgumentCompletions).toBeDefined(); + }); + + it("lists directories in the current project dir when no prefix is given", async () => { + await fs.mkdir(path.join(tempDir, "src")); + await fs.mkdir(path.join(tempDir, "tests")); + await fs.writeFile(path.join(tempDir, "README.md"), ""); + + const result = await move!.getArgumentCompletions!(""); + expect(result).not.toBeNull(); + const values = result!.map(i => i.value); + expect(values).toContain("src/"); + expect(values).toContain("tests/"); + expect(values).not.toContain("README.md"); + }); + + it("filters directories by prefix", async () => { + await fs.mkdir(path.join(tempDir, "src")); + await fs.mkdir(path.join(tempDir, "scripts")); + await fs.mkdir(path.join(tempDir, "tests")); + + const result = await move!.getArgumentCompletions!("sr"); + expect(result).not.toBeNull(); + const values = result!.map(i => i.value); + expect(values).toContain("src/"); + expect(values).not.toContain("scripts/"); + expect(values).not.toContain("tests/"); + }); + + it("completes inside a subdirectory", async () => { + const subDir = path.join(tempDir, "packages"); + await fs.mkdir(subDir); + await fs.mkdir(path.join(subDir, "coding-agent")); + await fs.mkdir(path.join(subDir, "tui")); + + const result = await move!.getArgumentCompletions!("packages/"); + expect(result).not.toBeNull(); + const values = result!.map(i => i.value); + expect(values).toContain("packages/coding-agent/"); + expect(values).toContain("packages/tui/"); + }); + + it("completes relative paths", async () => { + await fs.mkdir(path.join(tempDir, "src")); + + const result = await move!.getArgumentCompletions!("./sr"); + expect(result).not.toBeNull(); + expect(result!.map(i => i.value)).toContain("./src/"); + }); + + it("completes parent directory paths", async () => { + const parentDir = path.dirname(tempDir); + const siblingName = `omp-move-sibling-${path.basename(tempDir)}`; + const siblingDir = path.join(parentDir, siblingName); + await fs.mkdir(siblingDir); + try { + const result = await move!.getArgumentCompletions!(".."); + expect(result).not.toBeNull(); + expect(result!.map(i => i.value)).toContain(`../${siblingName}/`); + } finally { + await fs.rm(siblingDir, { recursive: true, force: true }); + } + }); + + it("completes directories with spaces in names", async () => { + const spacedDir = path.join(tempDir, "My Project"); + await fs.mkdir(spacedDir); + await fs.mkdir(path.join(spacedDir, "src")); + + const result = await move!.getArgumentCompletions!("My Project/"); + expect(result).not.toBeNull(); + expect(result!.map(i => i.value)).toContain("My Project/src/"); + }); + + it("filters inside a space-containing directory", async () => { + const spacedDir = path.join(tempDir, "My Project"); + await fs.mkdir(spacedDir); + await fs.mkdir(path.join(spacedDir, "src")); + await fs.mkdir(path.join(spacedDir, "tests")); + + const result = await move!.getArgumentCompletions!("My Project/sr"); + expect(result).not.toBeNull(); + const values = result!.map(i => i.value); + expect(values).toContain("My Project/src/"); + expect(values).not.toContain("My Project/tests/"); + }); + + it("returns null for non-matching prefixes", async () => { + await fs.mkdir(path.join(tempDir, "src")); + + const result = await move!.getArgumentCompletions!("xyz"); + expect(result).toBeNull(); + }); + + it("completes home-relative paths", async () => { + const homeDir = path.join(tempDir, "fake-home"); + await fs.mkdir(homeDir); + await fs.mkdir(path.join(homeDir, "project-a")); + await fs.mkdir(path.join(homeDir, "project-b")); + vi.spyOn(os, "homedir").mockReturnValue(homeDir); + + const result = await move!.getArgumentCompletions!("~/"); + expect(result).not.toBeNull(); + const values = result!.map(i => i.value); + expect(values).toContain("~/project-a/"); + expect(values).toContain("~/project-b/"); + }); + + it("completes absolute paths", async () => { + const targetDir = path.join(tempDir, "absolute-target"); + await fs.mkdir(targetDir); + + const result = await move!.getArgumentCompletions!(path.join(tempDir, "absolute-")); + expect(result).not.toBeNull(); + expect(result!.map(i => i.label)).toContain("absolute-target/"); + }); +});