Pārlūkot izejas kodu

fix(installer): opencode global config goes to ~/.config/opencode on every platform (#535) (#836)

* fix(installer): opencode global config goes to ~/.config/opencode on every platform (#535)

opencode resolves its config dir with xdg-basedir (XDG_CONFIG_HOME ??
~/.config) unconditionally — it never reads %APPDATA%; that layout
belonged to the discontinued Go fork. Writing there on Windows meant
opencode never saw the MCP entry.

- globalConfigDir(): drop the win32 APPDATA branch; XDG resolution everywhere
- install/uninstall (global): sweep a stale codegraph entry + AGENTS.md block
  out of the legacy %APPDATA%/opencode location (siblings/comments untouched)
- detect(global): a legacy-only dir still counts as installed so the sweep
  is reachable
- tests are env-gated, not platform-gated, so the whole matrix runs on any
  OS; the suite previously pointed APPDATA and XDG_CONFIG_HOME at the same
  dir, which is exactly how the divergence stayed invisible

Supersedes the prefer-if-exists approach of #670 (greenfield installs --
before opencode's first run -- would still have fallen back to APPDATA).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(installer): match legacy sweep paths by dir prefix, not 'AppData' substring

On Windows os.tmpdir() lives under AppData\Local\Temp, so every harness
path contains 'AppData' and the substring assertions false-positive.
Caught on the real Windows VM.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Colby Mchenry 1 nedēļu atpakaļ
vecāks
revīzija
13b2575dfe
3 mainītis faili ar 236 papildinājumiem un 37 dzēšanām
  1. 1 0
      CHANGELOG.md
  2. 149 0
      __tests__/installer-targets.test.ts
  3. 86 37
      src/installer/targets/opencode.ts

+ 1 - 0
CHANGELOG.md

@@ -45,6 +45,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- **opencode on Windows now finds CodeGraph.** The installer wrote opencode's global MCP entry to `%APPDATA%\opencode\`, but opencode reads its config from `~/.config/opencode/` on every platform (honoring `XDG_CONFIG_HOME`), so the entry was invisible to it. Installs now write where opencode actually looks, and `codegraph install` / `codegraph uninstall` both clean a stale CodeGraph entry out of the old `%APPDATA%` location — other servers and comments in that file are left untouched. Thanks @fucknoobhanzo for the report and @WodenJay for the first patch. (#535)
 - The `codegraph_search` tool's `kind: "type"` filter — a value its own schema advertises — silently matched nothing; it now correctly finds type aliases. The `codegraph_explore` tool's parameter guidance also no longer suggests running `codegraph_search` first, which contradicted explore's call-it-first design and cost agents an extra round-trip.
 - Symbols defined in Svelte and Vue `<script>` blocks were reported one line below where they actually are — a function on line 3 was reported at line 4 — which offset every script-block symbol's location in search, `codegraph_node`, and explore output. Line numbers now match the file exactly. Re-index a project to benefit. (Svelte, Vue)
 - Doc comments are now captured for exported, `const`-assigned, and decorated declarations, and the documentation a symbol carries is now clean across every supported language. Previously a comment above `export class X`, `export const fn = () => …`, a plain `const fn = () => …`, or a decorated Python `def`/`class` (`@app.route(...)`, `@dataclass`) was dropped entirely — only comments directly above a plain declaration were kept. CodeGraph now finds the comment through the `export` / `const` / decorator wrapper. Comment-marker cleanup was also rounded out for every language CodeGraph supports: Rust/Swift/Kotlin doc lines (`///`, `//!`), Python/Ruby/shell `#`, Lua/Luau (`--` and `--[[ ]]`), and Pascal (`{ }` and `(* *)`) no longer leave stray markers in the stored text — validated end-to-end across all 19 code languages plus Svelte/Vue `<script>` blocks. (#780). Thanks @caleb-kaiser.

+ 149 - 0
__tests__/installer-targets.test.ts

@@ -1399,3 +1399,152 @@ function listAllFiles(dir: string): string[] {
   }
   return out;
 }
+
+// ---------------------------------------------------------------------------
+// opencode global config path — XDG on every platform (#535)
+//
+// opencode resolves its config dir with `xdg-basedir`: XDG_CONFIG_HOME if
+// set, else ~/.config — on ALL platforms, Windows included. It never reads
+// %APPDATA%; we used to write there on Windows, so opencode never saw the
+// entry. The suite-wide setHome() points APPDATA and XDG_CONFIG_HOME at the
+// SAME directory (which is exactly how this bug stayed invisible), so these
+// tests deliberately split them.
+// ---------------------------------------------------------------------------
+describe('Installer targets — opencode XDG config path (#535)', () => {
+  let tmpHome: string;
+  let tmpCwd: string;
+  let origCwd: string;
+  let homeRestore: { restore: () => void };
+  let appDataDir: string; // distinct from ~/.config, like real Windows
+
+  beforeEach(() => {
+    tmpHome = mkTmpDir('home');
+    tmpCwd = mkTmpDir('cwd');
+    origCwd = process.cwd();
+    process.chdir(tmpCwd);
+    homeRestore = setHome(tmpHome);
+    appDataDir = path.join(tmpHome, 'AppData', 'Roaming');
+    process.env.APPDATA = appDataDir; // realistic split: APPDATA ≠ ~/.config
+    delete process.env.XDG_CONFIG_HOME; // default resolution: ~/.config
+  });
+
+  afterEach(() => {
+    homeRestore.restore();
+    process.chdir(origCwd);
+    fs.rmSync(tmpHome, { recursive: true, force: true });
+    fs.rmSync(tmpCwd, { recursive: true, force: true });
+  });
+
+  const xdgConfigFile = () => path.join(tmpHome, '.config', 'opencode', 'opencode.jsonc');
+  const legacyDir = () => path.join(appDataDir, 'opencode');
+  // NOTE: never match on an 'AppData' substring — on Windows os.tmpdir()
+  // itself lives under AppData\Local\Temp, so EVERY harness path contains
+  // it. Match on the legacy dir prefix instead.
+  const inLegacyDir = (p: string) => path.resolve(p).startsWith(path.resolve(legacyDir()) + path.sep);
+
+  it('global install writes to ~/.config/opencode, never %APPDATA% (#535)', () => {
+    const opencode = getTarget('opencode')!;
+    const result = opencode.install('global', { autoAllow: true });
+
+    const written = result.files.find((f) => f.path.endsWith('opencode.jsonc'))!;
+    expect(written.action).toBe('created');
+    expect(path.resolve(written.path)).toBe(path.resolve(xdgConfigFile()));
+    expect(fs.existsSync(xdgConfigFile())).toBe(true);
+    // Nothing of ours may land in the legacy location.
+    expect(fs.existsSync(legacyDir())).toBe(false);
+  });
+
+  it('greenfield: targets ~/.config/opencode even when the dir does not exist yet (#535)', () => {
+    // The rejected fallback design (#670) would send this install to
+    // %APPDATA% — where opencode would never find it. opencode creates
+    // ~/.config/opencode itself on first run; installing codegraph FIRST
+    // must land where opencode will look.
+    expect(fs.existsSync(path.join(tmpHome, '.config', 'opencode'))).toBe(false);
+    const opencode = getTarget('opencode')!;
+    const result = opencode.install('global', { autoAllow: true });
+    expect(path.resolve(result.files[0]!.path)).toBe(path.resolve(xdgConfigFile()));
+    expect(fs.existsSync(xdgConfigFile())).toBe(true);
+    expect(fs.existsSync(legacyDir())).toBe(false);
+  });
+
+  it('honors XDG_CONFIG_HOME for the global path, like opencode does', () => {
+    const custom = path.join(tmpHome, 'xdg-custom');
+    process.env.XDG_CONFIG_HOME = custom;
+    const opencode = getTarget('opencode')!;
+    const result = opencode.install('global', { autoAllow: true });
+    expect(path.resolve(result.files[0]!.path))
+      .toBe(path.resolve(path.join(custom, 'opencode', 'opencode.jsonc')));
+  });
+
+  it('install self-heals a pre-#535 %APPDATA% entry, preserving siblings and comments', () => {
+    // A previous codegraph version wrote into %APPDATA%/opencode. The user
+    // also has another MCP server and a comment there — those must survive.
+    fs.mkdirSync(legacyDir(), { recursive: true });
+    fs.writeFileSync(path.join(legacyDir(), 'opencode.jsonc'), [
+      '{',
+      '  // my servers',
+      '  "$schema": "https://opencode.ai/config.json",',
+      '  "mcp": {',
+      '    "codegraph": { "type": "local", "command": ["codegraph", "serve", "--mcp"], "enabled": true },',
+      '    "other": { "type": "local", "command": ["other"], "enabled": true }',
+      '  }',
+      '}',
+      '',
+    ].join('\n'));
+    fs.writeFileSync(path.join(legacyDir(), 'AGENTS.md'), LEGACY_BLOCK + '\n');
+
+    const opencode = getTarget('opencode')!;
+    const result = opencode.install('global', { autoAllow: true });
+
+    // New entry in the right place…
+    expect(fs.existsSync(xdgConfigFile())).toBe(true);
+    // …stale entry swept out of the legacy file, siblings + comment intact.
+    const legacyText = fs.readFileSync(path.join(legacyDir(), 'opencode.jsonc'), 'utf-8');
+    expect(legacyText).not.toContain('codegraph');
+    expect(legacyText).toContain('"other"');
+    expect(legacyText).toContain('// my servers');
+    // …and the legacy AGENTS.md — block-only, so emptied — removed outright
+    // (removeMarkedSection unlinks a file it leaves empty).
+    expect(fs.existsSync(path.join(legacyDir(), 'AGENTS.md'))).toBe(false);
+    // Both cleanups are reported.
+    const removed = result.files.filter((f) => f.action === 'removed').map((f) => f.path);
+    expect(removed.some((p) => inLegacyDir(p) && p.endsWith('opencode.jsonc'))).toBe(true);
+    expect(removed.some((p) => inLegacyDir(p) && p.endsWith('AGENTS.md'))).toBe(true);
+  });
+
+  it('uninstall sweeps the legacy %APPDATA% entry too (no prior re-install needed)', () => {
+    // A user on the broken version goes straight to `codegraph uninstall`:
+    // the only entry that exists is the stale %APPDATA% one.
+    fs.mkdirSync(legacyDir(), { recursive: true });
+    fs.writeFileSync(path.join(legacyDir(), 'opencode.json'),
+      '{\n  "mcp": {\n    "codegraph": { "type": "local", "command": ["codegraph", "serve", "--mcp"], "enabled": true }\n  }\n}\n');
+
+    const opencode = getTarget('opencode')!;
+    const result = opencode.uninstall('global');
+
+    expect(fs.readFileSync(path.join(legacyDir(), 'opencode.json'), 'utf-8')).not.toContain('codegraph');
+    expect(result.files.some((f) => f.action === 'removed' && inLegacyDir(f.path))).toBe(true);
+  });
+
+  it('install after install sweeps only once — second run reports no legacy changes', () => {
+    fs.mkdirSync(legacyDir(), { recursive: true });
+    fs.writeFileSync(path.join(legacyDir(), 'opencode.json'),
+      '{\n  "mcp": {\n    "codegraph": { "type": "local", "command": ["codegraph", "serve", "--mcp"], "enabled": true }\n  }\n}\n');
+
+    const opencode = getTarget('opencode')!;
+    const first = opencode.install('global', { autoAllow: true });
+    expect(first.files.some((f) => f.action === 'removed' && inLegacyDir(f.path))).toBe(true);
+
+    const second = opencode.install('global', { autoAllow: true });
+    expect(second.files.some((f) => inLegacyDir(f.path))).toBe(false);
+    expect(second.files.find((f) => f.path.endsWith('opencode.jsonc'))!.action).toBe('unchanged');
+  });
+
+  it('detects opencode as installed from a legacy-only %APPDATA% dir (so install can heal it)', () => {
+    fs.mkdirSync(legacyDir(), { recursive: true });
+    const opencode = getTarget('opencode')!;
+    expect(opencode.detect('global').installed).toBe(true);
+    // But configuration state is read from the REAL path only.
+    expect(opencode.detect('global').alreadyConfigured).toBe(false);
+  });
+});

+ 86 - 37
src/installer/targets/opencode.ts

@@ -2,10 +2,18 @@
  * opencode target.
  *
  *   - MCP server entry to `~/.config/opencode/opencode.jsonc` (global,
- *     XDG-style; `%APPDATA%/opencode/opencode.jsonc` on Windows) or
+ *     XDG-style on EVERY platform, Windows included — see below) or
  *     `./opencode.jsonc` (local). Falls back to `opencode.json` when a
  *     `.json` file already exists; defaults new installs to `.jsonc`
  *     because that's what opencode itself creates on first run.
+ *
+ *     opencode resolves its config dir with the `xdg-basedir` package
+ *     (sst/opencode `packages/core/src/global.ts`): `XDG_CONFIG_HOME`
+ *     if set, else `~/.config` — unconditionally, on all platforms. It
+ *     never reads `%APPDATA%`; that layout belonged to the discontinued
+ *     Go fork. We previously wrote there on Windows, so opencode never
+ *     saw the entry (#535) — install/uninstall now also sweep a stale
+ *     codegraph entry out of the legacy `%APPDATA%/opencode` location.
  *   - Instructions to `~/.config/opencode/AGENTS.md` (global) or
  *     `./AGENTS.md` (local). opencode reads AGENTS.md for agent
  *     instructions — same convention Codex CLI uses.
@@ -49,17 +57,29 @@ import {
 } from '../instructions-template';
 
 function globalConfigDir(): string {
-  if (process.platform === 'win32') {
-    const appData = process.env.APPDATA ?? path.join(os.homedir(), 'AppData', 'Roaming');
-    return path.join(appData, 'opencode');
-  }
-  // XDG_CONFIG_HOME if set, else ~/.config — matches opencode's docs.
+  // XDG_CONFIG_HOME if set, else ~/.config — on every platform, matching
+  // opencode's own `xdg-basedir` resolution (no Windows special case; #535).
   const xdg = process.env.XDG_CONFIG_HOME && process.env.XDG_CONFIG_HOME.trim().length > 0
     ? process.env.XDG_CONFIG_HOME
     : path.join(os.homedir(), '.config');
   return path.join(xdg, 'opencode');
 }
 
+/**
+ * Pre-#535 installs wrote the global entry to `%APPDATA%/opencode` — a dir
+ * today's opencode never reads. Returns that legacy dir when it could hold
+ * stale state (APPDATA set and resolving somewhere other than the real config
+ * dir). Gated on the env var rather than `process.platform` so the cleanup
+ * logic runs under the cross-platform test suite; on POSIX, APPDATA is unset
+ * in real life and this is a no-op.
+ */
+function legacyWindowsConfigDir(): string | null {
+  const appData = process.env.APPDATA;
+  if (!appData || !appData.trim()) return null;
+  const legacy = path.join(appData, 'opencode');
+  return path.resolve(legacy) === path.resolve(globalConfigDir()) ? null : legacy;
+}
+
 function configBaseDir(loc: Location): string {
   return loc === 'global' ? globalConfigDir() : process.cwd();
 }
@@ -118,8 +138,12 @@ class OpencodeTarget implements AgentTarget {
     const file = configPath(loc);
     const config = parseConfig(readConfigText(file));
     const alreadyConfigured = !!config.mcp?.codegraph;
+    // Global: the XDG dir is what current opencode creates on first run; the
+    // legacy %APPDATA% dir still counts as "opencode present" so a re-install
+    // can sweep the stale pre-#535 entry out of it.
+    const legacy = legacyWindowsConfigDir();
     const installed = loc === 'global'
-      ? fs.existsSync(globalConfigDir())
+      ? fs.existsSync(globalConfigDir()) || (!!legacy && fs.existsSync(legacy))
       : fs.existsSync(file);
     return { installed, alreadyConfigured, configPath: file };
   }
@@ -133,42 +157,18 @@ class OpencodeTarget implements AgentTarget {
     // initialize instructions. Upsert self-heals a stale pre-#529 block.
     files.push(upsertInstructionsEntry(instructionsPath(loc)));
 
+    // Self-heal a pre-#535 install that wrote to %APPDATA%/opencode —
+    // opencode never reads it, so anything of ours there is stale.
+    if (loc === 'global') files.push(...cleanupLegacyWindowsState());
+
     return { files };
   }
 
   uninstall(loc: Location): WriteResult {
     const files: WriteResult['files'] = [];
-    const file = configPath(loc);
-
-    if (!fs.existsSync(file)) {
-      files.push({ path: file, action: 'not-found' });
-    } else {
-      const text = readConfigText(file);
-      const config = parseConfig(text);
-      if (!config.mcp?.codegraph) {
-        files.push({ path: file, action: 'not-found' });
-      } else {
-        // Drop our key surgically. Leaves siblings + comments untouched.
-        let edits = modify(text, ['mcp', 'codegraph'], undefined, {
-          formattingOptions: FORMATTING,
-        });
-        let updated = applyEdits(text, edits);
-
-        // If `mcp` is now an empty object, drop the wrapper too.
-        const afterParsed = parseConfig(updated);
-        if (afterParsed.mcp && typeof afterParsed.mcp === 'object' &&
-            Object.keys(afterParsed.mcp).length === 0) {
-          edits = modify(updated, ['mcp'], undefined, { formattingOptions: FORMATTING });
-          updated = applyEdits(updated, edits);
-        }
-
-        atomicWriteFileSync(file, updated);
-        files.push({ path: file, action: 'removed' });
-      }
-    }
-
+    files.push(removeMcpEntryAt(configPath(loc)));
     files.push(removeInstructionsEntry(loc));
-
+    if (loc === 'global') files.push(...cleanupLegacyWindowsState());
     return { files };
   }
 
@@ -225,6 +225,55 @@ function writeMcpEntry(loc: Location): WriteResult['files'][number] {
   return { path: file, action: existed ? 'updated' : 'created' };
 }
 
+/**
+ * Surgically drop `mcp.codegraph` from one config file. Leaves sibling
+ * servers, comments, and formatting untouched; drops an emptied `mcp`
+ * wrapper too. Shared by uninstall and the legacy-%APPDATA% sweep.
+ */
+function removeMcpEntryAt(file: string): WriteResult['files'][number] {
+  if (!fs.existsSync(file)) return { path: file, action: 'not-found' };
+  const text = readConfigText(file);
+  const config = parseConfig(text);
+  if (!config.mcp?.codegraph) return { path: file, action: 'not-found' };
+
+  let edits = modify(text, ['mcp', 'codegraph'], undefined, {
+    formattingOptions: FORMATTING,
+  });
+  let updated = applyEdits(text, edits);
+
+  // If `mcp` is now an empty object, drop the wrapper too.
+  const afterParsed = parseConfig(updated);
+  if (afterParsed.mcp && typeof afterParsed.mcp === 'object' &&
+      Object.keys(afterParsed.mcp).length === 0) {
+    edits = modify(updated, ['mcp'], undefined, { formattingOptions: FORMATTING });
+    updated = applyEdits(updated, edits);
+  }
+
+  atomicWriteFileSync(file, updated);
+  return { path: file, action: 'removed' };
+}
+
+/**
+ * Remove whatever a pre-#535 install left in `%APPDATA%/opencode` — an MCP
+ * entry opencode never reads, plus our marker-fenced AGENTS.md block. Returns
+ * only files actually changed, so install output stays quiet when there is
+ * nothing to heal. Never touches anything else in the legacy dir: a user may
+ * genuinely keep other tools' state under %APPDATA%.
+ */
+function cleanupLegacyWindowsState(): WriteResult['files'] {
+  const dir = legacyWindowsConfigDir();
+  if (!dir || !fs.existsSync(dir)) return [];
+  const out: WriteResult['files'] = [];
+  for (const name of ['opencode.jsonc', 'opencode.json']) {
+    const res = removeMcpEntryAt(path.join(dir, name));
+    if (res.action === 'removed') out.push(res);
+  }
+  const agents = path.join(dir, 'AGENTS.md');
+  const action = removeMarkedSection(agents, CODEGRAPH_SECTION_START, CODEGRAPH_SECTION_END);
+  if (action === 'removed') out.push({ path: agents, action });
+  return out;
+}
+
 /**
  * Strip the marker-delimited CodeGraph block from AGENTS.md if a prior
  * install wrote one. Used by both install (self-heal on upgrade) and