From f9161c4d4e0d001ebfbd3ba98f0b89adb253260a Mon Sep 17 00:00:00 2001 From: Fero Date: Sat, 7 Feb 2026 03:30:28 +0100 Subject: [PATCH] fix(coding-agent): add package subcommand help and friendly errors (#1347) * fix(coding-agent): add package subcommand help and friendly errors * refactor(coding-agent): simplify package command parsing and dispatch * fix(coding-agent): add plain git URL examples to install help --- packages/coding-agent/src/cli/args.ts | 1 + packages/coding-agent/src/main.ts | 266 ++++++++++++------ .../test/package-command-paths.test.ts | 55 +++- 3 files changed, 241 insertions(+), 81 deletions(-) diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 904a25b4..92b6a3dc 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -185,6 +185,7 @@ ${chalk.bold("Commands:")} ${APP_NAME} update [source] Update installed extensions (skips pinned sources) ${APP_NAME} list List installed extensions from settings ${APP_NAME} config Open TUI to enable/disable package resources + ${APP_NAME} --help Show help for install/remove/update/list ${chalk.bold("Options:")} --provider Provider name (default: google) diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 99446f54..fa97e7a6 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -13,7 +13,7 @@ import { selectConfig } from "./cli/config-selector.js"; import { processFileArguments } from "./cli/file-processor.js"; import { listModels } from "./cli/list-models.js"; import { selectSession } from "./cli/session-picker.js"; -import { getAgentDir, getModelsPath, VERSION } from "./config.js"; +import { APP_NAME, getAgentDir, getModelsPath, VERSION } from "./config.js"; import { AuthStorage } from "./core/auth-storage.js"; import { DEFAULT_THINKING_LEVEL } from "./core/defaults.js"; import { exportFromFile } from "./core/export-html/index.js"; @@ -61,6 +61,74 @@ interface PackageCommandOptions { command: PackageCommand; source?: string; local: boolean; + help: boolean; + invalidOption?: string; +} + +function getPackageCommandUsage(command: PackageCommand): string { + switch (command) { + case "install": + return `${APP_NAME} install [-l]`; + case "remove": + return `${APP_NAME} remove [-l]`; + case "update": + return `${APP_NAME} update [source]`; + case "list": + return `${APP_NAME} list`; + } +} + +function printPackageCommandHelp(command: PackageCommand): void { + switch (command) { + case "install": + console.log(`${chalk.bold("Usage:")} + ${getPackageCommandUsage("install")} + +Install a package and add it to settings. + +Options: + -l, --local Install project-locally (.pi/settings.json) + +Examples: + ${APP_NAME} install npm:@foo/bar + ${APP_NAME} install git:github.com/user/repo + ${APP_NAME} install https://github.com/user/repo + ${APP_NAME} install git@github.com:user/repo + ${APP_NAME} install ./local/path +`); + return; + + case "remove": + console.log(`${chalk.bold("Usage:")} + ${getPackageCommandUsage("remove")} + +Remove a package and its source from settings. + +Options: + -l, --local Remove from project settings (.pi/settings.json) + +Example: + ${APP_NAME} remove npm:@foo/bar +`); + return; + + case "update": + console.log(`${chalk.bold("Usage:")} + ${getPackageCommandUsage("update")} + +Update installed packages. +If is provided, only that package is updated. +`); + return; + + case "list": + console.log(`${chalk.bold("Usage:")} + ${getPackageCommandUsage("list")} + +List installed packages from user and project settings. +`); + return; + } } function parsePackageCommand(args: string[]): PackageCommandOptions | undefined { @@ -70,16 +138,36 @@ function parsePackageCommand(args: string[]): PackageCommandOptions | undefined } let local = false; - const sources: string[] = []; + let help = false; + let invalidOption: string | undefined; + let source: string | undefined; + for (const arg of rest) { - if (arg === "-l" || arg === "--local") { - local = true; + if (arg === "-h" || arg === "--help") { + help = true; continue; } - sources.push(arg); + + if (arg === "-l" || arg === "--local") { + if (command === "install" || command === "remove") { + local = true; + } else { + invalidOption = invalidOption ?? arg; + } + continue; + } + + if (arg.startsWith("-")) { + invalidOption = invalidOption ?? arg; + continue; + } + + if (!source) { + source = arg; + } } - return { command, source: sources[0], local }; + return { command, source, local, help, invalidOption }; } async function handlePackageCommand(args: string[]): Promise { @@ -88,94 +176,112 @@ async function handlePackageCommand(args: string[]): Promise { return false; } + if (options.help) { + printPackageCommandHelp(options.command); + return true; + } + + if (options.invalidOption) { + console.error(chalk.red(`Unknown option ${options.invalidOption} for "${options.command}".`)); + console.error(chalk.dim(`Use "${APP_NAME} --help" or "${getPackageCommandUsage(options.command)}".`)); + process.exitCode = 1; + return true; + } + + const source = options.source; + if ((options.command === "install" || options.command === "remove") && !source) { + console.error(chalk.red(`Missing ${options.command} source.`)); + console.error(chalk.dim(`Usage: ${getPackageCommandUsage(options.command)}`)); + process.exitCode = 1; + return true; + } + const cwd = process.cwd(); const agentDir = getAgentDir(); const settingsManager = SettingsManager.create(cwd, agentDir); const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager }); - // Set up progress callback for CLI feedback packageManager.setProgressCallback((event) => { if (event.type === "start") { process.stdout.write(chalk.dim(`${event.message}\n`)); - } else if (event.type === "error") { - console.error(chalk.red(`Error: ${event.message}`)); } }); - if (options.command === "install") { - if (!options.source) { - console.error(chalk.red("Missing install source.")); - process.exit(1); + try { + switch (options.command) { + case "install": + await packageManager.install(source!, { local: options.local }); + packageManager.addSourceToSettings(source!, { local: options.local }); + console.log(chalk.green(`Installed ${source}`)); + return true; + + case "remove": { + await packageManager.remove(source!, { local: options.local }); + const removed = packageManager.removeSourceFromSettings(source!, { local: options.local }); + if (!removed) { + console.error(chalk.red(`No matching package found for ${source}`)); + process.exitCode = 1; + return true; + } + console.log(chalk.green(`Removed ${source}`)); + return true; + } + + case "list": { + const globalSettings = settingsManager.getGlobalSettings(); + const projectSettings = settingsManager.getProjectSettings(); + const globalPackages = globalSettings.packages ?? []; + const projectPackages = projectSettings.packages ?? []; + + if (globalPackages.length === 0 && projectPackages.length === 0) { + console.log(chalk.dim("No packages installed.")); + return true; + } + + const formatPackage = (pkg: (typeof globalPackages)[number], scope: "user" | "project") => { + const source = typeof pkg === "string" ? pkg : pkg.source; + const filtered = typeof pkg === "object"; + const display = filtered ? `${source} (filtered)` : source; + console.log(` ${display}`); + const path = packageManager.getInstalledPath(source, scope); + if (path) { + console.log(chalk.dim(` ${path}`)); + } + }; + + if (globalPackages.length > 0) { + console.log(chalk.bold("User packages:")); + for (const pkg of globalPackages) { + formatPackage(pkg, "user"); + } + } + + if (projectPackages.length > 0) { + if (globalPackages.length > 0) console.log(); + console.log(chalk.bold("Project packages:")); + for (const pkg of projectPackages) { + formatPackage(pkg, "project"); + } + } + + return true; + } + + case "update": + await packageManager.update(source); + if (source) { + console.log(chalk.green(`Updated ${source}`)); + } else { + console.log(chalk.green("Updated packages")); + } + return true; } - await packageManager.install(options.source, { local: options.local }); - packageManager.addSourceToSettings(options.source, { local: options.local }); - console.log(chalk.green(`Installed ${options.source}`)); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : "Unknown package command error"; + console.error(chalk.red(`Error: ${message}`)); + process.exitCode = 1; return true; } - - if (options.command === "remove") { - if (!options.source) { - console.error(chalk.red("Missing remove source.")); - process.exit(1); - } - await packageManager.remove(options.source, { local: options.local }); - const removed = packageManager.removeSourceFromSettings(options.source, { local: options.local }); - if (!removed) { - console.error(chalk.red(`No matching package found for ${options.source}`)); - process.exit(1); - } - console.log(chalk.green(`Removed ${options.source}`)); - return true; - } - - if (options.command === "list") { - const globalSettings = settingsManager.getGlobalSettings(); - const projectSettings = settingsManager.getProjectSettings(); - const globalPackages = globalSettings.packages ?? []; - const projectPackages = projectSettings.packages ?? []; - - if (globalPackages.length === 0 && projectPackages.length === 0) { - console.log(chalk.dim("No packages installed.")); - return true; - } - - const formatPackage = (pkg: (typeof globalPackages)[number], scope: "user" | "project") => { - const source = typeof pkg === "string" ? pkg : pkg.source; - const filtered = typeof pkg === "object"; - const display = filtered ? `${source} (filtered)` : source; - console.log(` ${display}`); - // Show resolved path - const path = packageManager.getInstalledPath(source, scope); - if (path) { - console.log(chalk.dim(` ${path}`)); - } - }; - - if (globalPackages.length > 0) { - console.log(chalk.bold("User packages:")); - for (const pkg of globalPackages) { - formatPackage(pkg, "user"); - } - } - - if (projectPackages.length > 0) { - if (globalPackages.length > 0) console.log(); - console.log(chalk.bold("Project packages:")); - for (const pkg of projectPackages) { - formatPackage(pkg, "project"); - } - } - - return true; - } - - await packageManager.update(options.source); - if (options.source) { - console.log(chalk.green(`Updated ${options.source}`)); - } else { - console.log(chalk.green("Updated packages")); - } - return true; } async function prepareInitialMessage( diff --git a/packages/coding-agent/test/package-command-paths.test.ts b/packages/coding-agent/test/package-command-paths.test.ts index 451843b9..95270396 100644 --- a/packages/coding-agent/test/package-command-paths.test.ts +++ b/packages/coding-agent/test/package-command-paths.test.ts @@ -1,7 +1,7 @@ import { mkdirSync, readFileSync, realpathSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ENV_AGENT_DIR } from "../src/config.js"; import { main } from "../src/main.js"; @@ -12,6 +12,7 @@ describe("package commands", () => { let packageDir: string; let originalCwd: string; let originalAgentDir: string | undefined; + let originalExitCode: typeof process.exitCode; beforeEach(() => { tempDir = join(tmpdir(), `pi-package-commands-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -24,12 +25,15 @@ describe("package commands", () => { originalCwd = process.cwd(); originalAgentDir = process.env[ENV_AGENT_DIR]; + originalExitCode = process.exitCode; + process.exitCode = undefined; process.env[ENV_AGENT_DIR] = agentDir; process.chdir(projectDir); }); afterEach(() => { process.chdir(originalCwd); + process.exitCode = originalExitCode; if (originalAgentDir === undefined) { delete process.env[ENV_AGENT_DIR]; } else { @@ -64,4 +68,53 @@ describe("package commands", () => { const removedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")) as { packages?: string[] }; expect(removedSettings.packages ?? []).toHaveLength(0); }); + + it("shows install subcommand help", async () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await expect(main(["install", "--help"])).resolves.toBeUndefined(); + + const stdout = logSpy.mock.calls.map(([message]) => String(message)).join("\n"); + expect(stdout).toContain("Usage:"); + expect(stdout).toContain("pi install [-l]"); + expect(errorSpy).not.toHaveBeenCalled(); + expect(process.exitCode).toBeUndefined(); + } finally { + logSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); + + it("shows a friendly error for unknown install options", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await expect(main(["install", "--unknown"])).resolves.toBeUndefined(); + + const stderr = errorSpy.mock.calls.map(([message]) => String(message)).join("\n"); + expect(stderr).toContain('Unknown option --unknown for "install".'); + expect(stderr).toContain('Use "pi --help" or "pi install [-l]".'); + expect(process.exitCode).toBe(1); + } finally { + errorSpy.mockRestore(); + } + }); + + it("shows a friendly error for missing install source", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await expect(main(["install"])).resolves.toBeUndefined(); + + const stderr = errorSpy.mock.calls.map(([message]) => String(message)).join("\n"); + expect(stderr).toContain("Missing install source."); + expect(stderr).toContain("Usage: pi install [-l]"); + expect(stderr).not.toContain("at "); + expect(process.exitCode).toBe(1); + } finally { + errorSpy.mockRestore(); + } + }); });