From 40c206c2f9999689572926a758c3661297e99a95 Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Wed, 17 Sep 2025 03:29:33 -0400 Subject: [PATCH] add `opencode attach` command to connect to a remote opencode server --- packages/opencode/src/cli/cmd/attach.ts | 56 ++++ packages/opencode/src/cli/cmd/tui.ts | 2 - packages/opencode/src/index.ts | 2 + packages/opencode/src/plugin/index.ts | 1 - packages/opencode/test/snapshot/bugs.test.ts | 310 ------------------ .../test/snapshot/snapshot-path-bug.test.ts | 96 ------ 6 files changed, 58 insertions(+), 409 deletions(-) create mode 100644 packages/opencode/src/cli/cmd/attach.ts delete mode 100644 packages/opencode/test/snapshot/bugs.test.ts delete mode 100644 packages/opencode/test/snapshot/snapshot-path-bug.test.ts diff --git a/packages/opencode/src/cli/cmd/attach.ts b/packages/opencode/src/cli/cmd/attach.ts new file mode 100644 index 00000000..5a0c23ea --- /dev/null +++ b/packages/opencode/src/cli/cmd/attach.ts @@ -0,0 +1,56 @@ +import { Global } from "../../global" +import { cmd } from "./cmd" +import path from "path" +import fs from "fs/promises" +import { Log } from "../../util/log" + +import { $ } from "bun" + +export const AttachCommand = cmd({ + command: "attach ", + describe: "attach to a running opencode server", + builder: (yargs) => + yargs.positional("server", { + type: "string", + describe: "http://localhost:4096", + }), + handler: async (args) => { + let cmd = [] as string[] + const tui = Bun.embeddedFiles.find((item) => (item as File).name.includes("tui")) as File + if (tui) { + let binaryName = tui.name + if (process.platform === "win32" && !binaryName.endsWith(".exe")) { + binaryName += ".exe" + } + const binary = path.join(Global.Path.cache, "tui", binaryName) + const file = Bun.file(binary) + if (!(await file.exists())) { + await Bun.write(file, tui, { mode: 0o755 }) + if (process.platform !== "win32") await fs.chmod(binary, 0o755) + } + cmd = [binary] + } + if (!tui) { + const dir = Bun.fileURLToPath(new URL("../../../../tui/cmd/opencode", import.meta.url)) + let binaryName = `./dist/tui${process.platform === "win32" ? ".exe" : ""}` + await $`go build -o ${binaryName} ./main.go`.cwd(dir) + cmd = [path.join(dir, binaryName)] + } + Log.Default.info("tui", { + cmd, + }) + const proc = Bun.spawn({ + cmd, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + env: { + ...process.env, + CGO_ENABLED: "0", + OPENCODE_SERVER: args.server, + }, + }) + + await proc.exited + }, +}) diff --git a/packages/opencode/src/cli/cmd/tui.ts b/packages/opencode/src/cli/cmd/tui.ts index 7281f6e4..bbc229e6 100644 --- a/packages/opencode/src/cli/cmd/tui.ts +++ b/packages/opencode/src/cli/cmd/tui.ts @@ -15,7 +15,6 @@ import { Ide } from "../../ide" import { Flag } from "../../flag/flag" import { Session } from "../../session" -import { Instance } from "../../project/instance" import { $ } from "bun" declare global { @@ -151,7 +150,6 @@ export const TuiCommand = cmd({ ...process.env, CGO_ENABLED: "0", OPENCODE_SERVER: server.url.toString(), - OPENCODE_PROJECT: JSON.stringify(Instance.project), }, onExit: () => { server.stop() diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index a45ec3ac..6f03e23b 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -18,6 +18,7 @@ import { StatsCommand } from "./cli/cmd/stats" import { McpCommand } from "./cli/cmd/mcp" import { GithubCommand } from "./cli/cmd/github" import { ExportCommand } from "./cli/cmd/export" +import { AttachCommand } from "./cli/cmd/attach" const cancel = new AbortController() @@ -71,6 +72,7 @@ const cli = yargs(hideBin(process.argv)) .usage("\n" + UI.logo()) .command(McpCommand) .command(TuiCommand) + .command(AttachCommand) .command(RunCommand) .command(GenerateCommand) .command(DebugCommand) diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index ab36087b..272d66a7 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -15,7 +15,6 @@ export namespace Plugin { const state = Instance.state(async () => { const client = createOpencodeClient({ baseUrl: "http://localhost:4096", - // @ts-expect-error fetch: async (...args) => Server.App().fetch(...args), }) const config = await Config.get() diff --git a/packages/opencode/test/snapshot/bugs.test.ts b/packages/opencode/test/snapshot/bugs.test.ts deleted file mode 100644 index 0984b2c0..00000000 --- a/packages/opencode/test/snapshot/bugs.test.ts +++ /dev/null @@ -1,310 +0,0 @@ -import { test, expect } from "bun:test" -import { $ } from "bun" -import { Snapshot } from "../../src/snapshot" -import { Instance } from "../../src/project/instance" -import path from "path" - -async function bootstrap() { - const dir = await $`mktemp -d`.text().then((t) => t.trim()) - const unique = Math.random().toString(36).slice(2) - const aContent = `A${unique}` - const bContent = `B${unique}` - await Bun.write(`${dir}/a.txt`, aContent) - await Bun.write(`${dir}/b.txt`, bContent) - await $`git init`.cwd(dir).quiet() - await $`git add .`.cwd(dir).quiet() - await $`git commit -m init`.cwd(dir).quiet() - - return { - [Symbol.asyncDispose]: async () => { - await $`rm -rf ${dir}`.quiet() - }, - dir, - aContent, - bContent, - } -} - -test("BUG: revert fails with absolute paths outside worktree", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - await Bun.write(`${tmp.dir}/new.txt`, "NEW") - - const patch = await Snapshot.patch(before!) - - // Bug: The revert function tries to checkout files using absolute paths - // but git checkout expects relative paths from the worktree - // This will fail when the file path contains the full absolute path - await expect(Snapshot.revert([patch])).resolves.toBeUndefined() - - // The file should be deleted but won't be due to git checkout failure - expect(await Bun.file(`${tmp.dir}/new.txt`).exists()).toBe(false) - }) -}) - -test("BUG: filenames with special git characters break operations", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Create files with characters that need escaping in git - const problematicFiles = [ - `${tmp.dir}/"quotes".txt`, - `${tmp.dir}/'apostrophe'.txt`, - `${tmp.dir}/file\nwith\nnewline.txt`, - `${tmp.dir}/file\twith\ttab.txt`, - `${tmp.dir}/file with $ dollar.txt`, - `${tmp.dir}/file with \` backtick.txt`, - ] - - for (const file of problematicFiles) { - try { - await Bun.write(file, "content") - } catch (e) { - // Some filenames might not be valid on the filesystem - } - } - - const patch = await Snapshot.patch(before!) - - // The patch should handle these special characters correctly - // but git commands may fail or produce unexpected results - for (const file of patch.files) { - if (problematicFiles.some((pf) => file.includes(path.basename(pf)))) { - // These files with special characters may not be handled correctly - console.log("Found problematic file in patch:", file) - } - } - - // Reverting these files will likely fail - await Snapshot.revert([patch]) - - // Check if files were actually removed (they likely won't be) - for (const file of problematicFiles) { - try { - const exists = await Bun.file(file).exists() - if (exists) { - console.log("File with special chars still exists after revert:", file) - } - } catch {} - } - }) -}) - -test("BUG: race condition in concurrent track calls", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - // Create initial state - await Bun.write(`${tmp.dir}/file1.txt`, "initial1") - const hash1 = await Snapshot.track() - - // Start multiple concurrent modifications and tracks - const promises = [] - for (let i = 0; i < 10; i++) { - promises.push( - (async () => { - await Bun.write(`${tmp.dir}/file${i}.txt`, `content${i}`) - const hash = await Snapshot.track() - return hash - })(), - ) - } - - const hashes = await Promise.all(promises) - - // Bug: Multiple concurrent track() calls may interfere with each other - // because they all run `git add .` and `git write-tree` without locking - // This can lead to inconsistent state - - // All hashes should be different (since files are different) - // but due to race conditions, some might be the same - const uniqueHashes = new Set(hashes) - console.log(`Got ${uniqueHashes.size} unique hashes out of ${hashes.length} operations`) - - // This assertion might fail due to race conditions - expect(uniqueHashes.size).toBe(hashes.length) - }) -}) - -test("BUG: restore doesn't handle modified files correctly", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Modify existing file - await Bun.write(`${tmp.dir}/a.txt`, "MODIFIED") - - // Add new file - await Bun.write(`${tmp.dir}/new.txt`, "NEW") - - // Delete existing file - await $`rm ${tmp.dir}/b.txt`.quiet() - - // Restore to original state - await Snapshot.restore(before!) - - // Check restoration - expect(await Bun.file(`${tmp.dir}/a.txt`).text()).toBe(tmp.aContent) - expect(await Bun.file(`${tmp.dir}/b.txt`).text()).toBe(tmp.bContent) - - // Bug: restore uses checkout-index -a which only restores tracked files - // It doesn't remove untracked files that were added after the snapshot - expect(await Bun.file(`${tmp.dir}/new.txt`).exists()).toBe(false) // This will fail - }) -}) - -test("BUG: patch with spaces in filenames not properly escaped", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Create file with spaces - const fileWithSpaces = `${tmp.dir}/file with many spaces.txt` - await Bun.write(fileWithSpaces, "content") - - const patch = await Snapshot.patch(before!) - expect(patch.files).toContain(fileWithSpaces) - - // Try to revert - this might fail due to improper escaping - await Snapshot.revert([patch]) - - // File should be removed but might not be due to escaping issues - expect(await Bun.file(fileWithSpaces).exists()).toBe(false) - }) -}) - -test("BUG: init() recursive directory removal uses wrong method", async () => { - // The init() function uses fs.rmdir() which is deprecated - // and might not work correctly on all systems - // It should use fs.rm() with recursive: true instead - // This is more of a code quality issue than a functional bug - // but could fail on certain node versions or systems -}) - -test("BUG: diff and patch don't handle binary files correctly", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Create a binary file - const binaryData = Buffer.from([ - 0x89, - 0x50, - 0x4e, - 0x47, - 0x0d, - 0x0a, - 0x1a, - 0x0a, // PNG header - 0x00, - 0x00, - 0x00, - 0x0d, - 0x49, - 0x48, - 0x44, - 0x52, - ]) - await Bun.write(`${tmp.dir}/image.png`, binaryData) - - // diff() returns text which won't handle binary files correctly - const diff = await Snapshot.diff(before!) - - // Binary files should be indicated differently in diff - // but the current implementation just returns text() - console.log("Diff output for binary file:", diff) - - // The diff might contain binary data as text, which could cause issues - expect(diff).toContain("image.png") - }) -}) - -test("BUG: revert with relative path from different cwd fails", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - await $`mkdir -p ${tmp.dir}/subdir`.quiet() - await Bun.write(`${tmp.dir}/subdir/file.txt`, "content") - - const patch = await Snapshot.patch(before!) - - // Change cwd to a different directory - const originalCwd = process.cwd() - process.chdir(tmp.dir) - - try { - // The revert function uses Instance.worktree as cwd for git checkout - // but the file paths in the patch are absolute - // This mismatch can cause issues - await Snapshot.revert([patch]) - - expect(await Bun.file(`${tmp.dir}/subdir/file.txt`).exists()).toBe(false) - } finally { - process.chdir(originalCwd) - } - }) -}) - -test("BUG: track without git init in Instance.worktree creates orphaned git dir", async () => { - // Create a directory without git initialization - const dir = await $`mktemp -d`.text().then((t) => t.trim()) - - try { - await Instance.provide(dir, async () => { - // Track will create a git directory in Global.Path.data - // but if the worktree doesn't have git, operations might fail - const hash = await Snapshot.track() - - // This might return a hash even though the worktree isn't properly tracked - console.log("Hash from non-git directory:", hash) - - if (hash) { - // Try to use the hash - this might fail or produce unexpected results - const patch = await Snapshot.patch(hash) - console.log("Patch from non-git directory:", patch) - } - }) - } finally { - await $`rm -rf ${dir}`.quiet() - } -}) - -test("BUG: patch doesn't handle deleted files in snapshot correctly", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - // Track initial state - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Delete a file - await $`rm ${tmp.dir}/a.txt`.quiet() - - // Track after deletion - const after = await Snapshot.track() - expect(after).toBeTruthy() - - // Now create a new file - await Bun.write(`${tmp.dir}/new.txt`, "NEW") - - // Get patch from the state where a.txt was deleted - // This should show that new.txt was added and a.txt is still missing - const patch = await Snapshot.patch(after!) - - // But the patch might incorrectly include a.txt as a changed file - // because git diff compares against the snapshot tree, not working directory - console.log("Patch files:", patch.files) - - // The patch should only contain new.txt - expect(patch.files).toContain(`${tmp.dir}/new.txt`) - expect(patch.files).not.toContain(`${tmp.dir}/a.txt`) - }) -}) diff --git a/packages/opencode/test/snapshot/snapshot-path-bug.test.ts b/packages/opencode/test/snapshot/snapshot-path-bug.test.ts deleted file mode 100644 index 364cb4ef..00000000 --- a/packages/opencode/test/snapshot/snapshot-path-bug.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { test, expect } from "bun:test" -import { $ } from "bun" -import path from "path" -import { Snapshot } from "../../src/snapshot" -import { Instance } from "../../src/project/instance" - -async function bootstrap() { - const dir = await $`mktemp -d`.text().then((t) => t.trim()) - // Randomize file contents to ensure unique git repos - const unique = Math.random().toString(36).slice(2) - const aContent = `A${unique}` - const bContent = `B${unique}` - await Bun.write(`${dir}/a.txt`, aContent) - await Bun.write(`${dir}/b.txt`, bContent) - await $`git init`.cwd(dir).quiet() - await $`git add .`.cwd(dir).quiet() - await $`git commit -m init`.cwd(dir).quiet() - - return { - [Symbol.asyncDispose]: async () => { - await $`rm -rf ${dir}`.quiet() - }, - dir, - aContent, - bContent, - } -} - -test("file path bug - git returns paths with worktree prefix", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Create a file in subdirectory - await $`mkdir -p ${tmp.dir}/sub`.quiet() - await Bun.write(`${tmp.dir}/sub/file.txt`, "SUB") - - // Get the patch - this will demonstrate the path bug - const patch = await Snapshot.patch(before!) - - // Log what we get to see the actual paths - console.log("Worktree path:", Instance.worktree) - console.log("Patch files:", patch.files) - - // The bug: if git returns paths that already include the worktree directory, - // path.join(Instance.worktree, x) will create double paths - // For example: if git returns "tmpDir/sub/file.txt" and worktree is "tmpDir", - // we get "tmpDir/tmpDir/sub/file.txt" which is wrong - - // Check if any paths are duplicated - const hasDoublePaths = patch.files.some((filePath) => { - const worktreeParts = Instance.worktree.split("/").filter(Boolean) - const fileParts = filePath.split("/").filter(Boolean) - - // Check if worktree appears twice at the start - if (worktreeParts.length > 0 && fileParts.length >= worktreeParts.length * 2) { - const firstWorktree = fileParts.slice(0, worktreeParts.length).join("/") - const secondWorktree = fileParts.slice(worktreeParts.length, worktreeParts.length * 2).join("/") - return firstWorktree === secondWorktree - } - return false - }) - - expect(hasDoublePaths).toBe(false) // This test will fail if the bug exists - }) -}) - -test("file path bug - manual demonstration", async () => { - await using tmp = await bootstrap() - await Instance.provide(tmp.dir, async () => { - const before = await Snapshot.track() - expect(before).toBeTruthy() - - // Create a file - await Bun.write(`${tmp.dir}/test.txt`, "TEST") - - // Simulate what happens in the patch function - // Mock git diff returning a path that already includes worktree - const mockGitOutput = `${Instance.worktree}/test.txt\n` - - // This is what the current code does: - const files = mockGitOutput - .trim() - .split("\n") - .map((x) => x.trim()) - .filter(Boolean) - .map((x) => path.join(Instance.worktree, x)) // This is the bug! - - console.log("Mock git output:", mockGitOutput) - console.log("Result after path.join:", files) - - // This will show the double path: /tmp/dir/tmp/dir/test.txt - expect(files[0]).toBe(`${Instance.worktree}/test.txt`) // This should pass but won't due to the bug - }) -})