From 5ad086b1dec6677c9bc1ea279c0487a3d5b91091 Mon Sep 17 00:00:00 2001 From: ppe Date: Sat, 28 Feb 2026 21:44:56 +0200 Subject: [PATCH] Fix symlink path resolution causing duplicate instances (#15482) - Add canonical() function to resolve symlinks before caching instances - Prevents TUI freeze when working with symlinked directories - Add comprehensive tests for symlink path handling - Fixes #15482 --- SYMLINK_BUG_FIX.md | 124 ++++++++++++++++++ packages/opencode/src/project/instance.ts | 20 ++- packages/opencode/test/project/SYMLINK_FIX.md | 66 ++++++++++ .../test/project/instance-symlink.test.ts | 117 +++++++++++++++++ 4 files changed, 322 insertions(+), 5 deletions(-) create mode 100644 SYMLINK_BUG_FIX.md create mode 100644 packages/opencode/test/project/SYMLINK_FIX.md create mode 100644 packages/opencode/test/project/instance-symlink.test.ts diff --git a/SYMLINK_BUG_FIX.md b/SYMLINK_BUG_FIX.md new file mode 100644 index 00000000000..2338c0ee0af --- /dev/null +++ b/SYMLINK_BUG_FIX.md @@ -0,0 +1,124 @@ +# Fix for Issue #15482: Symlink Path Resolution + +## Problem + +When OpenCode was started from a directory that was a symlink, it would create two separate server instances: +- One for the symlink path (e.g., `/home/user/ssd/project`) +- One for the resolved real path (e.g., `/media/user/ssd_storage/project`) + +This caused the TUI to freeze when sending prompts because the UI was connected to one instance while session operations were executing on a different instance. + +## Root Cause + +The `Instance.provide()` function in `packages/opencode/src/project/instance.ts` used the raw input directory path as the cache key without resolving symlinks first. This meant: + +1. User opens OpenCode with `opencode ~/ssd/project` (symlink path) +2. Instance created with cache key: `/home/user/ssd/project` +3. Sessions created and stored with this directory +4. Later operations (git commands, etc.) might resolve to real path: `/media/user/ssd_storage/project` +5. When switching sessions, a second instance gets created with the real path +6. TUI remains connected to first instance, but operations happen on second instance +7. Messages never reach the UI → freeze + +Evidence from logs showed both instances being disposed: +``` +service=default directory=/media/user/ssd_storage/project disposing instance +service=default directory=/home/user/ssd/project disposing instance +``` + +## Visual Explanation + +### Before the fix: +``` +User runs: opencode ~/ssd/project + ↓ + ~/ssd → /media/user/ssd_storage + ↓ + Instance.provide({ directory: "~/ssd/project" }) + ↓ + cache["~/ssd/project"] = Instance #1 ← TUI connected here + ↓ + Session created with directory: "~/ssd/project" + ↓ + (Later, when switching sessions...) + ↓ + Instance.provide({ directory: "/media/user/ssd_storage/project" }) + ↓ + cache["/media/user/ssd_storage/project"] = Instance #2 ← Operations happen here + ↓ + TUI FREEZE! + (UI listening to Instance #1, but messages going to Instance #2) +``` + +### After the fix: +``` +User runs: opencode ~/ssd/project + ↓ + canonical("~/ssd/project") + ↓ + resolves symlinks + ↓ + "/media/user/ssd_storage/project" + ↓ + cache["/media/user/ssd_storage/project"] = Instance #1 + ↓ + (Later, when switching sessions...) + ↓ + canonical("/media/user/ssd_storage/project") + ↓ + "/media/user/ssd_storage/project" + ↓ + Uses same Instance #1 ✓ +``` + +## Solution + +Added symlink resolution in `Instance.provide()` before using the directory as a cache key: + +```typescript +async function canonical(input: string) { + const abs = path.resolve(input) + const real = await fs.realpath(abs).catch(() => abs) + const normalized = path.normalize(real) + return process.platform === "win32" ? normalized.toLowerCase() : normalized +} +``` + +This function: +1. Resolves to absolute path +2. Resolves symlinks using `fs.realpath()` +3. Normalizes path separators +4. Lowercases on Windows for case-insensitive matching + +The normalized directory is now used consistently as: +- The cache key for instance lookup +- The stored value in `Instance.directory` +- The directory passed to `Project.fromDirectory()` + +## Testing + +Added comprehensive tests in `packages/opencode/test/project/instance-symlink.test.ts`: + +1. **Same instance for symlink and real path**: Verifies that accessing via symlink or real path uses the same instance +2. **No duplicate instances when switching sessions**: Simulates the original bug scenario + +Both tests pass, confirming the fix works correctly. + +## Impact + +- Symlink and real paths now resolve to the same instance +- Session switching works correctly regardless of path representation +- No more TUI freezing when working with symlinked directories +- All existing tests continue to pass + +## Files Changed + +- `packages/opencode/src/project/instance.ts` - Added `canonical()` function and path normalization +- `packages/opencode/test/project/instance-symlink.test.ts` - New test file with regression tests +- `packages/opencode/test/project/SYMLINK_FIX.md` - Technical documentation + +## Related + +- GitHub Issue: https://github.com/anomalyco/opencode/issues/15482 +- Reported by: snowstorm0182 +- OpenCode version affected: 1.2.15 \ No newline at end of file diff --git a/packages/opencode/src/project/instance.ts b/packages/opencode/src/project/instance.ts index 98031f18d3f..077c30b57cd 100644 --- a/packages/opencode/src/project/instance.ts +++ b/packages/opencode/src/project/instance.ts @@ -5,6 +5,8 @@ import { State } from "./state" import { iife } from "@/util/iife" import { GlobalBus } from "@/bus/global" import { Filesystem } from "@/util/filesystem" +import * as fs from "fs/promises" +import * as path from "path" interface Context { directory: string @@ -18,15 +20,23 @@ const disposal = { all: undefined as Promise | undefined, } +async function canonical(input: string) { + const abs = path.resolve(input) + const real = await fs.realpath(abs).catch(() => abs) + const normalized = path.normalize(real) + return process.platform === "win32" ? normalized.toLowerCase() : normalized +} + export const Instance = { async provide(input: { directory: string; init?: () => Promise; fn: () => R }): Promise { - let existing = cache.get(input.directory) + const normalizedDirectory = await canonical(input.directory) + let existing = cache.get(normalizedDirectory) if (!existing) { - Log.Default.info("creating instance", { directory: input.directory }) + Log.Default.info("creating instance", { directory: normalizedDirectory }) existing = iife(async () => { - const { project, sandbox } = await Project.fromDirectory(input.directory) + const { project, sandbox } = await Project.fromDirectory(normalizedDirectory) const ctx = { - directory: input.directory, + directory: normalizedDirectory, worktree: sandbox, project, } @@ -35,7 +45,7 @@ export const Instance = { }) return ctx }) - cache.set(input.directory, existing) + cache.set(normalizedDirectory, existing) } const ctx = await existing return context.provide(ctx, async () => { diff --git a/packages/opencode/test/project/SYMLINK_FIX.md b/packages/opencode/test/project/SYMLINK_FIX.md new file mode 100644 index 00000000000..75c657c5ab0 --- /dev/null +++ b/packages/opencode/test/project/SYMLINK_FIX.md @@ -0,0 +1,66 @@ +# Symlink Path Resolution Fix + +## Issue + +When OpenCode was opened from a directory that was a symlink to another path, it would create two separate server instances: + +1. One instance for the symlink path (e.g., `/home/user/ssd/project`) +2. Another instance for the resolved real path (e.g., `/media/user/ssd_storage/project`) + +This caused the TUI to freeze when sending prompts because: +- The TUI event stream was connected to one instance +- But session operations were executing on a different instance +- Messages would never reach the UI since they were going to the wrong instance + +## Root Cause + +The `Instance.provide()` function used the raw input directory as the cache key without resolving symlinks first. This meant: + +1. User opens OpenCode with `opencode ~/ssd/project` (symlink path) +2. Instance created with key: `/home/user/ssd/project` +3. Session created and stored with `Instance.directory` (symlink path) +4. Later, git commands or other operations resolve to real path: `/media/user/ssd_storage/project` +5. When switching sessions, the stored directory might differ from cwd +6. Second instance created with key: `/media/user/ssd_storage/project` +7. TUI connected to first instance, but operations happening on second instance + +## Solution + +Added a `canonical()` function in `instance.ts` that resolves symlinks using `fs.realpath()` before using the directory as a cache key. This ensures: + +- Symlink paths and their real paths map to the same instance +- Only one server instance is created per physical directory +- Session switching works correctly even when paths contain symlinks +- All operations happen on the same instance the TUI is connected to + +## Implementation + +```typescript +async function canonical(input: string) { + const abs = path.resolve(input) + const real = await fs.realpath(abs).catch(() => abs) + const normalized = path.normalize(real) + return process.platform === "win32" ? normalized.toLowerCase() : normalized +} +``` + +This function: +1. Resolves the path to an absolute path +2. Resolves any symlinks to the real path using `fs.realpath()` +3. Normalizes the path (handles platform differences) +4. On Windows, lowercases for case-insensitive matching + +The normalized directory is then used consistently as both: +- The cache key for instance lookup +- The stored value in `Instance.directory` + +## Testing + +Two test cases were added in `instance-symlink.test.ts`: + +1. **Same instance for symlink and real path**: Verifies that accessing a directory via symlink and via real path uses the same instance +2. **No duplicate instances when switching sessions**: Simulates the original bug scenario where switching sessions could create duplicate instances + +## Related Issue + +GitHub Issue: #15482 \ No newline at end of file diff --git a/packages/opencode/test/project/instance-symlink.test.ts b/packages/opencode/test/project/instance-symlink.test.ts new file mode 100644 index 00000000000..69b641024ae --- /dev/null +++ b/packages/opencode/test/project/instance-symlink.test.ts @@ -0,0 +1,117 @@ +import { describe, test, expect } from "bun:test" +import { tmpdir } from "../fixture/fixture" +import path from "path" +import fs from "fs/promises" +import { existsSync } from "fs" + +describe("Instance with symlinks", () => { + test("should use same instance for symlink and real path", async () => { + await using tmp = await tmpdir({ git: true }) + + // Create a symlink to the temp directory + const symlinkPath = path.join(path.dirname(tmp.path), "symlink-test") + + // Clean up any existing symlink + if (existsSync(symlinkPath)) { + await fs.unlink(symlinkPath) + } + + await fs.symlink(tmp.path, symlinkPath, "dir") + + try { + const { Instance } = await import("../../src/project/instance") + const { InstanceBootstrap } = await import("../../src/project/bootstrap") + const { Session } = await import("../../src/session") + + // Track which instances were created + const instanceKeys = new Set() + + // Create session using symlink path + const session1 = await Instance.provide({ + directory: symlinkPath, + init: InstanceBootstrap, + fn: async () => { + instanceKeys.add(Instance.directory) + return Session.create({}) + }, + }) + + // Create session using real path + const session2 = await Instance.provide({ + directory: tmp.path, + init: InstanceBootstrap, + fn: async () => { + instanceKeys.add(Instance.directory) + return Session.create({}) + }, + }) + + // Both operations should use the same instance + expect(instanceKeys.size).toBe(1) + + // Both sessions should have the same directory (normalized) + expect(session1.directory).toBe(session2.directory) + + // Cleanup instances + await Instance.provide({ + directory: symlinkPath, + fn: async () => { + await Instance.dispose() + }, + }) + } finally { + await fs.unlink(symlinkPath).catch(() => {}) + } + }) + + test("should not create duplicate instances when switching sessions", async () => { + await using tmp = await tmpdir({ git: true }) + + const symlinkPath = path.join(path.dirname(tmp.path), "symlink-session-test") + + if (existsSync(symlinkPath)) { + await fs.unlink(symlinkPath) + } + + await fs.symlink(tmp.path, symlinkPath, "dir") + + try { + const { Instance } = await import("../../src/project/instance") + const { InstanceBootstrap } = await import("../../src/project/bootstrap") + const { Session } = await import("../../src/session") + + let firstInstanceDir: string | undefined + let secondInstanceDir: string | undefined + + // Create a session from symlink + const session = await Instance.provide({ + directory: symlinkPath, + init: InstanceBootstrap, + fn: async () => { + firstInstanceDir = Instance.directory + return Session.create({ title: "Test session" }) + }, + }) + + // Simulate switching to the session (which has a stored directory) + // The stored directory might be different from the current path + await Instance.provide({ + directory: session.directory, + init: InstanceBootstrap, + fn: async () => { + secondInstanceDir = Instance.directory + // Get the session + await Session.get(session.id) + }, + }) + + // Both should resolve to the same instance directory + expect(firstInstanceDir).toBe(secondInstanceDir) + + // Cleanup + await Instance.disposeAll() + } finally { + await fs.unlink(symlinkPath).catch(() => {}) + } + }) +})