Browse Source

fix(cli): store stable plugin paths during install (#296)

* fix(cli): store stable plugin paths during install

* fix(cli): detect local plugin path installs

* fix(cli): detect opencode from PATH and known locations

* fix(skills): tighten cartography skill description to prevent overuse
Alvin 3 months ago
parent
commit
194aae741b
5 changed files with 250 additions and 42 deletions
  1. 95 0
      src/cli/config-io.test.ts
  2. 65 12
      src/cli/config-io.ts
  3. 31 0
      src/cli/system.test.ts
  4. 58 29
      src/cli/system.ts
  5. 1 1
      src/skills/cartography/SKILL.md

+ 95 - 0
src/cli/config-io.test.ts

@@ -3,6 +3,7 @@
 import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test';
 import {
   existsSync,
+  mkdirSync,
   mkdtempSync,
   readFileSync,
   rmSync,
@@ -25,6 +26,7 @@ import * as paths from './paths';
 describe('config-io', () => {
   let tmpDir: string;
   const originalEnv = { ...process.env };
+  const originalArgv = [...process.argv];
 
   beforeEach(() => {
     tmpDir = mkdtempSync(join(tmpdir(), 'opencode-io-test-'));
@@ -34,12 +36,21 @@ describe('config-io', () => {
 
   afterEach(() => {
     process.env = { ...originalEnv };
+    process.argv = [...originalArgv];
     if (tmpDir && existsSync(tmpDir)) {
       rmSync(tmpDir, { recursive: true, force: true });
     }
     mock.restore();
   });
 
+  function writePackageJson(dir: string): void {
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(
+      join(dir, 'package.json'),
+      JSON.stringify({ name: 'oh-my-opencode-slim' }),
+    );
+  }
+
   test('stripJsonComments strips comments and trailing commas', () => {
     const jsonc = `{
       // comment
@@ -110,6 +121,7 @@ describe('config-io', () => {
       configPath,
       JSON.stringify({ plugin: ['other', 'oh-my-opencode-slim@1.0.0'] }),
     );
+    process.argv[1] = '';
 
     const result = await addPluginToOpenCodeConfig();
     expect(result.success).toBe(true);
@@ -120,6 +132,77 @@ describe('config-io', () => {
     expect(saved.plugin.length).toBe(2);
   });
 
+  test('addPluginToOpenCodeConfig stores package name for bunx temp paths', async () => {
+    const configPath = join(tmpDir, 'opencode', 'opencode.json');
+    const packageRoot = join(
+      tmpDir,
+      'bunx-1000-oh-my-opencode-slim@latest',
+      'node_modules',
+      'oh-my-opencode-slim',
+    );
+    paths.ensureConfigDir();
+    writeFileSync(configPath, JSON.stringify({ plugin: [] }));
+    writePackageJson(packageRoot);
+    process.argv[1] = join(packageRoot, 'dist', 'cli', 'index.js');
+
+    const result = await addPluginToOpenCodeConfig();
+
+    expect(result.success).toBe(true);
+    const saved = JSON.parse(readFileSync(configPath, 'utf-8'));
+    expect(saved.plugin).toEqual(['oh-my-opencode-slim']);
+  });
+
+  test('addPluginToOpenCodeConfig stores local repo path for local dev paths', async () => {
+    const configPath = join(tmpDir, 'opencode', 'opencode.json');
+    const packageRoot = join(tmpDir, 'repo');
+    const localCliPath = join(packageRoot, 'dist', 'cli', 'index.js');
+    paths.ensureConfigDir();
+    writeFileSync(configPath, JSON.stringify({ plugin: [] }));
+    writePackageJson(packageRoot);
+    process.argv[1] = localCliPath;
+
+    const result = await addPluginToOpenCodeConfig();
+
+    expect(result.success).toBe(true);
+    const saved = JSON.parse(readFileSync(configPath, 'utf-8'));
+    expect(saved.plugin).toEqual([packageRoot]);
+  });
+
+  test('addPluginToOpenCodeConfig stores local repo path for local paths containing bunx-', async () => {
+    const configPath = join(tmpDir, 'opencode', 'opencode.json');
+    const packageRoot = join(tmpDir, 'repo', 'bunx-tools');
+    const localCliPath = join(packageRoot, 'dist', 'cli', 'index.js');
+    paths.ensureConfigDir();
+    writeFileSync(configPath, JSON.stringify({ plugin: [] }));
+    writePackageJson(packageRoot);
+    process.argv[1] = localCliPath;
+
+    const result = await addPluginToOpenCodeConfig();
+
+    expect(result.success).toBe(true);
+    const saved = JSON.parse(readFileSync(configPath, 'utf-8'));
+    expect(saved.plugin).toEqual([packageRoot]);
+  });
+
+  test('addPluginToOpenCodeConfig deduplicates existing local repo path entries', async () => {
+    const configPath = join(tmpDir, 'opencode', 'opencode.json');
+    const packageRoot = join(tmpDir, 'repo');
+    const localCliPath = join(packageRoot, 'dist', 'cli', 'index.js');
+    paths.ensureConfigDir();
+    writePackageJson(packageRoot);
+    writeFileSync(
+      configPath,
+      JSON.stringify({ plugin: ['other', packageRoot] }),
+    );
+    process.argv[1] = localCliPath;
+
+    const result = await addPluginToOpenCodeConfig();
+
+    expect(result.success).toBe(true);
+    const saved = JSON.parse(readFileSync(configPath, 'utf-8'));
+    expect(saved.plugin).toEqual(['other', packageRoot]);
+  });
+
   test('writeLiteConfig writes lite config with OpenAI preset', () => {
     const litePath = join(tmpDir, 'opencode', 'oh-my-opencode-slim.json');
     paths.ensureConfigDir();
@@ -192,4 +275,16 @@ describe('config-io', () => {
     expect(detected.hasZaiPlan).toBe(true);
     expect(detected.hasTmux).toBe(true);
   });
+
+  test('detectCurrentConfig treats local repo path entries as installed', () => {
+    const configPath = join(tmpDir, 'opencode', 'opencode.json');
+    const packageRoot = join(tmpDir, 'repo');
+    paths.ensureConfigDir();
+    writePackageJson(packageRoot);
+    writeFileSync(configPath, JSON.stringify({ plugin: [packageRoot] }));
+
+    const detected = detectCurrentConfig();
+
+    expect(detected.isInstalled).toBe(true);
+  });
 });

+ 65 - 12
src/cli/config-io.ts

@@ -6,7 +6,7 @@ import {
   statSync,
   writeFileSync,
 } from 'node:fs';
-import { pathToFileURL } from 'node:url';
+import { dirname, join } from 'node:path';
 import {
   ensureConfigDir,
   ensureOpenCodeConfigDir,
@@ -23,11 +23,71 @@ import type {
 
 const PACKAGE_NAME = 'oh-my-opencode-slim';
 
+function normalizePathForMatch(path: string): string {
+  return path.replaceAll('\\', '/');
+}
+
+function findPackageRoot(startPath: string): string | null {
+  let currentPath = dirname(startPath);
+
+  while (true) {
+    const packageJsonPath = join(currentPath, 'package.json');
+
+    if (existsSync(packageJsonPath)) {
+      try {
+        const packageJson = JSON.parse(
+          readFileSync(packageJsonPath, 'utf-8'),
+        ) as {
+          name?: string;
+        };
+
+        if (packageJson.name === PACKAGE_NAME) {
+          return currentPath;
+        }
+      } catch {
+        // Ignore invalid package.json while walking upward.
+      }
+    }
+
+    const parentPath = dirname(currentPath);
+    if (parentPath === currentPath) {
+      return null;
+    }
+    currentPath = parentPath;
+  }
+}
+
+function isPackageManagerInstall(path: string): boolean {
+  const normalizedPath = normalizePathForMatch(path);
+  return normalizedPath.includes(`/node_modules/${PACKAGE_NAME}`);
+}
+
+function isLocalPackageRootEntry(entry: string): boolean {
+  if (!entry || entry.startsWith('file://')) {
+    return false;
+  }
+
+  const packageJsonPath = join(entry, 'package.json');
+  if (!existsSync(packageJsonPath)) {
+    return false;
+  }
+
+  try {
+    const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as {
+      name?: string;
+    };
+    return packageJson.name === PACKAGE_NAME;
+  } catch {
+    return false;
+  }
+}
+
 function isPluginEntry(entry: string): boolean {
   return (
     entry === PACKAGE_NAME ||
     entry.startsWith(`${PACKAGE_NAME}@`) ||
-    (entry.startsWith('file://') && entry.includes(PACKAGE_NAME))
+    (entry.startsWith('file://') && entry.includes(PACKAGE_NAME)) ||
+    isLocalPackageRootEntry(entry)
   );
 }
 
@@ -39,20 +99,13 @@ function getPluginEntry(): string {
   }
 
   try {
-    const pluginEntryPath = cliEntryPath.match(
-      /[\\/]dist[\\/]cli[\\/]index\.js$/,
-    )
-      ? cliEntryPath.replace(
-          /[\\/]dist[\\/]cli[\\/]index\.js$/,
-          '/dist/index.js',
-        )
-      : null;
+    const packageRoot = findPackageRoot(cliEntryPath);
 
-    if (!pluginEntryPath) {
+    if (!packageRoot || isPackageManagerInstall(packageRoot)) {
       return PACKAGE_NAME;
     }
 
-    return pathToFileURL(pluginEntryPath).href;
+    return packageRoot;
   } catch {
     return PACKAGE_NAME;
   }

+ 31 - 0
src/cli/system.test.ts

@@ -1,6 +1,15 @@
 /// <reference types="bun-types" />
 
 import { describe, expect, mock, test } from 'bun:test';
+import {
+  chmodSync,
+  mkdirSync,
+  mkdtempSync,
+  rmSync,
+  writeFileSync,
+} from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
 import {
   fetchLatestVersion,
   getOpenCodeVersion,
@@ -9,6 +18,28 @@ import {
 } from './system';
 
 describe('system', () => {
+  test('isOpenCodeInstalled detects opencode in ~/.opencode/bin', async () => {
+    const dir = mkdtempSync(join(tmpdir(), 'opencode-system-test-'));
+    const originalPath = process.env.PATH;
+    const originalHome = process.env.HOME;
+
+    try {
+      const opencodePath = join(dir, '.opencode', 'bin', 'opencode');
+      mkdirSync(join(dir, '.opencode', 'bin'), { recursive: true });
+      writeFileSync(opencodePath, '#!/bin/sh\necho 1.2.3\n');
+      chmodSync(opencodePath, 0o755);
+      process.env.HOME = dir;
+      process.env.PATH = '/usr/bin:/bin:/usr/sbin:/sbin';
+
+      const system = await import(`./system?test=home-detect-${Date.now()}`);
+      expect(await system.isOpenCodeInstalled()).toBe(true);
+    } finally {
+      process.env.PATH = originalPath;
+      process.env.HOME = originalHome;
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
   test('isOpenCodeInstalled returns boolean', async () => {
     // We don't necessarily want to depend on the host system
     // but for a basic test we can just check it returns a boolean

+ 58 - 29
src/cli/system.ts

@@ -1,7 +1,42 @@
+import { spawnSync } from 'node:child_process';
 import { statSync } from 'node:fs';
 
 let cachedOpenCodePath: string | null = null;
 
+function resolvePathCommand(command: string): string | null {
+  try {
+    const resolver = process.platform === 'win32' ? 'where' : 'which';
+    const result = spawnSync(resolver, [command], {
+      encoding: 'utf-8',
+      stdio: ['ignore', 'pipe', 'ignore'],
+    });
+
+    if (result.status !== 0) {
+      return null;
+    }
+
+    const resolved = result.stdout
+      .split(/\r?\n/)
+      .map((line) => line.trim())
+      .find(Boolean);
+
+    return resolved ?? null;
+  } catch {
+    return null;
+  }
+}
+
+function canExecute(command: string, args: string[]): boolean {
+  try {
+    const result = spawnSync(command, args, {
+      stdio: 'ignore',
+    });
+    return result.status === 0;
+  } catch {
+    return false;
+  }
+}
+
 function getOpenCodePaths(): string[] {
   const home = process.env.HOME || process.env.USERPROFILE || '';
 
@@ -53,6 +88,12 @@ export function resolveOpenCodePath(): string {
     return cachedOpenCodePath;
   }
 
+  const pathOpenCodePath = resolvePathCommand('opencode');
+  if (pathOpenCodePath) {
+    cachedOpenCodePath = pathOpenCodePath;
+    return pathOpenCodePath;
+  }
+
   const paths = getOpenCodePaths();
 
   for (const opencodePath of paths) {
@@ -73,50 +114,38 @@ export function resolveOpenCodePath(): string {
 }
 
 export async function isOpenCodeInstalled(): Promise<boolean> {
+  const pathOpenCodePath = resolvePathCommand('opencode');
+
+  if (pathOpenCodePath && canExecute(pathOpenCodePath, ['--version'])) {
+    cachedOpenCodePath = pathOpenCodePath;
+    return true;
+  }
+
   const paths = getOpenCodePaths();
 
   for (const opencodePath of paths) {
-    try {
-      const proc = Bun.spawn([opencodePath, '--version'], {
-        stdout: 'pipe',
-        stderr: 'pipe',
-      });
-      await proc.exited;
-      if (proc.exitCode === 0) {
-        cachedOpenCodePath = opencodePath;
-        return true;
-      }
-    } catch {
-      // Try next path
+    if (opencodePath === 'opencode') continue;
+    if (canExecute(opencodePath, ['--version'])) {
+      cachedOpenCodePath = opencodePath;
+      return true;
     }
   }
   return false;
 }
 
 export async function isTmuxInstalled(): Promise<boolean> {
-  try {
-    const proc = Bun.spawn(['tmux', '-V'], {
-      stdout: 'pipe',
-      stderr: 'pipe',
-    });
-    await proc.exited;
-    return proc.exitCode === 0;
-  } catch {
-    return false;
-  }
+  return canExecute('tmux', ['-V']);
 }
 
 export async function getOpenCodeVersion(): Promise<string | null> {
   const opencodePath = resolveOpenCodePath();
   try {
-    const proc = Bun.spawn([opencodePath, '--version'], {
-      stdout: 'pipe',
-      stderr: 'pipe',
+    const result = spawnSync(opencodePath, ['--version'], {
+      encoding: 'utf-8',
+      stdio: ['ignore', 'pipe', 'ignore'],
     });
-    const output = await new Response(proc.stdout).text();
-    await proc.exited;
-    if (proc.exitCode === 0) {
-      return output.trim();
+    if (result.status === 0) {
+      return result.stdout.trim();
     }
   } catch {
     // Failed

+ 1 - 1
src/skills/cartography/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: cartography
-description: Repository understanding and hierarchical codemap generation
+description: Generate comprehensive hierarchical codemaps for UNFAMILIAR repositories. Expensive operation - only use when explicitly asked for codebase documentation or initial repository mapping
 ---
 
 # Cartography Skill