diff --git a/CLAUDE.md b/CLAUDE.md index 288edf93..aec50399 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,4 +116,4 @@ User-scope editors (e.g., opencode) write to a path resolved in `paths.ts` rathe **opencode ships two distributions — CLI detection alone misses the Desktop app.** The Electron-based Desktop app (`@opencode-aidesktop` on Windows) ships **no CLI binary**, so `isOpencodeCliAvailable()` (PATH check) can't detect it. Both distributions share `opencodeConfigDir()` (`~/.config/opencode/`), so `isOpencodeAvailable()` in `plugin-install.ts` also treats that directory's existence as installed. All three call sites (`editor-detect.ts`, `init-project.ts`, `commands/plugin/install.ts`) use the broader `isOpencodeAvailable()` — use it for any new opencode-gated behavior too. -**GitHub Copilot has the same two-distribution shape — and a declarative install path.** The Copilot desktop app ships no CLI binary but shares `copilotConfigDir()` (`~/.copilot/`) with the `copilot` CLI, so copilot-gated behavior must use `isCopilotAvailable()` (PATH check OR config dir exists), never `isCopilotCliAvailable()` alone. The plugin install is declarative-first: `installCopilotPlugin()` always writes the marketplace + plugin declaration into `~/.copilot/settings.json` (`extraKnownMarketplaces` + `enabledPlugins` — Copilot's marketplace registry and auto-install list, read by both distributions on startup; an existing `archgate` entry is overwritten to correct stale URLs), then additionally runs `copilot plugin install archgate@archgate` when the CLI is on PATH. `mode: "declarative"` in the result means the user must restart the Copilot app for the install to take effect. The settings merge lives in `copilot-user-settings.ts` (user-scope); `copilot-settings.ts` is project-scope (`.github/copilot/`). +**GitHub Copilot has the same two-distribution shape — and a declarative install path.** The Copilot desktop app ships no CLI binary but shares `copilotConfigDir()` (`~/.copilot/`) with the `copilot` CLI, so copilot-gated behavior must use `isCopilotAvailable()` (PATH check OR config dir exists), never `isCopilotCliAvailable()` alone. The plugin install is declarative-first: `installCopilotPlugin()` always writes the marketplace + plugin declaration into `~/.copilot/settings.json` (`extraKnownMarketplaces` + `enabledPlugins` — Copilot's marketplace registry and auto-install list, read by both distributions on startup; an existing `archgate` entry is overwritten to correct stale URLs), then additionally runs `copilot plugin install archgate@archgate` when the CLI is on PATH. `mode: "declarative"` in the result means the user must restart the Copilot app for the install to take effect. `mode: "existing"` means the CLI install failed but `copilot plugin list` already shows `archgate@archgate`: on Windows the running Copilot app locks the installed plugin's files, so a reinstall fails with os error 5 (access denied) and the existing install is kept. The settings merge lives in `copilot-user-settings.ts` (user-scope); `copilot-settings.ts` is project-scope (`.github/copilot/`). diff --git a/src/commands/plugin/install.ts b/src/commands/plugin/install.ts index 185614ab..573a6f60 100644 --- a/src/commands/plugin/install.ts +++ b/src/commands/plugin/install.ts @@ -75,6 +75,11 @@ export async function installForEditor( const { mode } = await installCopilotPlugin(); if (mode === "cli") { logInfo(`Archgate plugin installed for ${label}.`); + } else if (mode === "existing") { + logInfo( + `Archgate plugin already installed for ${label}.`, + "To refresh it, close the GitHub Copilot app and run: copilot plugin update archgate@archgate" + ); } else { logInfo( `Archgate plugin configured for ${label}.`, diff --git a/src/helpers/init-project.ts b/src/helpers/init-project.ts index 23f5d20e..59e87b57 100644 --- a/src/helpers/init-project.ts +++ b/src/helpers/init-project.ts @@ -386,7 +386,8 @@ async function tryInstallPlugin(editor: EditorTarget): Promise { try { const { mode } = await installCopilotPlugin(); - return mode === "cli" + // `existing` leaves a working install in place — report it as done. + return mode === "cli" || mode === "existing" ? { installed: true, autoInstalled: true } : { installed: true, diff --git a/src/helpers/plugin-install.ts b/src/helpers/plugin-install.ts index aff1d4eb..55d6ba8c 100644 --- a/src/helpers/plugin-install.ts +++ b/src/helpers/plugin-install.ts @@ -409,8 +409,10 @@ export interface CopilotInstallResult { * `cli` — installed immediately via `copilot plugin install`. * `declarative` — declared in `~/.copilot/settings.json`; Copilot * auto-installs it on next launch (CLI not on PATH, e.g. desktop app). + * `existing` — `copilot plugin install` failed but the plugin is already + * installed (typically its files are locked by the running Copilot app). */ - mode: "cli" | "declarative"; + mode: "cli" | "declarative" | "existing"; } /** @@ -440,6 +442,15 @@ export async function installCopilotPlugin(): Promise { "archgate@archgate", ]); if (installResult.exitCode !== 0) { + // Reinstalling over an existing install fails on Windows while the + // Copilot app holds the plugin's files open (os error 5, access denied). + // The plugin is already in place and declared, so that is not a failure. + if (await isCopilotPluginInstalled(cmd)) { + logDebug( + `copilot plugin install exited ${installResult.exitCode}; keeping the existing install` + ); + return { mode: "existing" }; + } // Exit code only: this message reaches logError, which feeds Sentry // breadcrumbs, and the CLI's output can carry auth state and local paths. throw new UserError( @@ -449,6 +460,36 @@ export async function installCopilotPlugin(): Promise { return { mode: "cli" }; } +const CopilotPluginListSchema = z.array( + z.object({ + name: z.string(), + marketplace: z.string().optional(), + enabled: z.boolean().optional(), + }) +); + +/** Whether `copilot plugin list --json` reports archgate@archgate as installed and enabled. */ +async function isCopilotPluginInstalled(cmd: string): Promise { + const listResult = await run([cmd, "plugin", "list", "--json"]); + if (listResult.exitCode !== 0) return false; + let raw: unknown; + try { + raw = JSON.parse(listResult.stdout); + } catch { + return false; + } + const parsed = CopilotPluginListSchema.safeParse(raw); + return ( + parsed.success && + parsed.data.some( + (p) => + p.name === "archgate" && + p.marketplace === "archgate" && + p.enabled === true + ) + ); +} + // --------------------------------------------------------------------------- // VS Code — download .vsix and install via `code` CLI // --------------------------------------------------------------------------- diff --git a/tests/commands/plugin/install.test.ts b/tests/commands/plugin/install.test.ts index 0ca34869..cb38270f 100644 --- a/tests/commands/plugin/install.test.ts +++ b/tests/commands/plugin/install.test.ts @@ -270,6 +270,23 @@ describe("plugin install action", () => { expect(logged).toContain("Restart the GitHub Copilot app"); }); + test("reports an existing copilot install as success with a refresh hint", async () => { + mockLoadCredentials.mockImplementation(async () => ({ + token: "tok", + github_user: "user", + })); + mockIsCopilotAvailable.mockImplementation(async () => true); + mockInstallCopilotPlugin.mockImplementation(async () => ({ + mode: "existing" as const, + })); + + await runInstall(["--editor", "copilot"]); + + const logged = logSpy.mock.calls.map((c) => c.join(" ")).join("\n"); + expect(logged).toContain("already installed"); + expect(logged).toContain("copilot plugin update archgate@archgate"); + }); + test("prints manual instructions when Copilot is not installed", async () => { mockLoadCredentials.mockImplementation(async () => ({ token: "tok", diff --git a/tests/helpers/init-project-copilot.test.ts b/tests/helpers/init-project-copilot.test.ts index 3c63669e..c58501b0 100644 --- a/tests/helpers/init-project-copilot.test.ts +++ b/tests/helpers/init-project-copilot.test.ts @@ -85,6 +85,31 @@ describe("tryInstallPlugin via initProject — copilot", () => { } }); + test("copilot existing install reports success with no restart note", async () => { + credSpy.mockResolvedValue({ token: "tok", github_user: "user" }); + const availableSpy = spyOn( + pluginInstall, + "isCopilotAvailable" + ).mockResolvedValue(true); + const installSpy = spyOn( + pluginInstall, + "installCopilotPlugin" + ).mockResolvedValue({ mode: "existing" }); + try { + const result = await initProject(tempDir, { + installPlugin: true, + editor: "copilot", + }); + expect(result.plugin!.installed).toBe(true); + expect(result.plugin!.autoInstalled).toBe(true); + expect(result.plugin!.deferred).toBeUndefined(); + expect(result.plugin!.detail).toBeUndefined(); + } finally { + availableSpy.mockRestore(); + installSpy.mockRestore(); + } + }); + test("copilot not installed returns the not-found sentinel", async () => { credSpy.mockResolvedValue({ token: "tok", github_user: "user" }); const availableSpy = spyOn( diff --git a/tests/helpers/plugin-install-copilot.test.ts b/tests/helpers/plugin-install-copilot.test.ts index 5b703cc1..8e86336d 100644 --- a/tests/helpers/plugin-install-copilot.test.ts +++ b/tests/helpers/plugin-install-copilot.test.ts @@ -26,6 +26,7 @@ import { installCopilotPlugin, isCopilotAvailable, } from "../../src/helpers/plugin-install"; +import { UserError } from "../../src/helpers/user-error"; import { restoreEnv } from "../test-utils"; /** @@ -212,6 +213,80 @@ describe("installCopilotPlugin", () => { expect(message).not.toContain("ghp_stderrsecret"); }); + test("existing mode: a failed reinstall over an installed plugin is not an error", async () => { + mockResolveCommand.mockImplementation(async () => "copilot"); + mkdirSync(copilotConfigDir(), { recursive: true }); + // Install fails (files locked by the running app); list shows the plugin. + spawnSpy.mockImplementation((argv) => + Array.isArray(argv) && argv.includes("list") + ? fakeSpawnResult( + 0, + JSON.stringify([ + // A direct install carries no marketplace and must not reject the list. + { name: "local-tool", enabled: true }, + { + name: "archgate", + marketplace: "archgate", + version: "0.18.0", + enabled: true, + }, + ]) + ) + : fakeSpawnResult(1, "", "Failed to install plugin: (os error 5)") + ); + + const result = await installCopilotPlugin(); + + expect(result).toEqual({ mode: "existing" }); + expect(spawnSpy).toHaveBeenCalledTimes(2); + expect(spawnSpy.mock.calls[1][0]).toEqual([ + "copilot", + "plugin", + "list", + "--json", + ]); + }); + + test.each([ + ["the plugin is not listed", 0, "[]"], + [ + "only a different marketplace's archgate is listed", + 0, + JSON.stringify([ + { name: "archgate", marketplace: "archgate-dev", enabled: true }, + ]), + ], + [ + "the listed plugin is disabled", + 0, + JSON.stringify([ + { name: "archgate", marketplace: "archgate", enabled: false }, + ]), + ], + ["the list output is not JSON", 0, "No plugins installed."], + ["the list command fails", 1, ""], + ])( + "still throws when the install fails and %s", + async (_case, listExit, listOut) => { + mockResolveCommand.mockImplementation(async () => "copilot"); + mkdirSync(copilotConfigDir(), { recursive: true }); + spawnSpy.mockImplementation((argv) => + Array.isArray(argv) && argv.includes("list") + ? fakeSpawnResult(listExit, listOut) + : fakeSpawnResult(1) + ); + + const err = await installCopilotPlugin().then( + () => null, + (e: unknown) => e + ); + expect(err).toBeInstanceOf(UserError); + expect(err instanceof Error ? err.message : "").toMatch( + /plugin install failed \(exit 1\)/u + ); + } + ); + test("resolves the settings path via COPILOT_HOME when set", async () => { const copilotHome = join(tempHome, "custom-copilot-home"); Bun.env.COPILOT_HOME = copilotHome;