diff --git a/packages/client/src/effect/api/api.ts b/packages/client/src/effect/api/api.ts index dac3496c459..5a6b3f681fc 100644 --- a/packages/client/src/effect/api/api.ts +++ b/packages/client/src/effect/api/api.ts @@ -1658,9 +1658,9 @@ export type ExperimentalPersistentPtyListOperation = ( export type ExperimentalPersistentPtyCreateInput = { readonly sessionID: Session.ID - readonly command: string + readonly command?: string | undefined readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string | undefined readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } | undefined diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index 372872e0ffd..0af1a23ca47 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -5753,50 +5753,50 @@ export type ExperimentalPersistentPtyListOutput = { data: Array - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } }["command"] readonly args: { - readonly command: string + readonly command?: string readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } }["args"] - readonly cwd: { - readonly command: string + readonly cwd?: { + readonly command?: string readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } }["cwd"] readonly title: { - readonly command: string + readonly command?: string readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } }["title"] readonly env: { - readonly command: string + readonly command?: string readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } }["env"] readonly size?: { - readonly command: string + readonly command?: string readonly args: ReadonlyArray - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: { readonly [x: string]: string } readonly size?: { readonly cols: number; readonly rows: number } diff --git a/packages/core/src/persistent-pty/index.ts b/packages/core/src/persistent-pty/index.ts index 68240706a19..440e14b81ea 100644 --- a/packages/core/src/persistent-pty/index.ts +++ b/packages/core/src/persistent-pty/index.ts @@ -11,6 +11,7 @@ import { Bus } from "../bus.js" import { Database } from "../database/database.js" import { Pty } from "@opencode-ai/schema/pty" import { Global } from "@opencode-ai/util/global" +import { ShellSelect } from "../shell/select.js" import { makeDaemonTransport, type DaemonTransport, @@ -66,9 +67,9 @@ export interface Interface { readonly create: ( sessionID: Session.ID, input: { - readonly command: string + readonly command?: string readonly args: readonly string[] - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: Readonly> readonly cols?: number @@ -154,9 +155,9 @@ export const layer = Layer.effect( const create = Effect.fn("PersistentPty.create")(function* ( sessionID: Session.ID, input: { - readonly command: string + readonly command?: string readonly args: readonly string[] - readonly cwd: string + readonly cwd?: string readonly title: string readonly env: Readonly> readonly cols?: number @@ -167,9 +168,9 @@ export const layer = Layer.effect( daemon, { op: "create", - program: input.command, + program: input.command ?? ShellSelect.environment(global.bin), args: input.args, - cwd: input.cwd, + cwd: input.cwd ?? path.resolve("/"), title: input.title, group_id: sessionID, env: input.env, diff --git a/packages/core/src/shell/select.ts b/packages/core/src/shell/select.ts index bbe538638b6..0eb7b3d2ac8 100644 --- a/packages/core/src/shell/select.ts +++ b/packages/core/src/shell/select.ts @@ -167,6 +167,11 @@ export function args(file: string, command: string) { return ["-c", command] } +// Resolve afresh so removing a shell does not leave terminals using a stale cached path. +export function environment(bin?: string, filter?: { compatible?: boolean }) { + return select(process.env.SHELL, undefined, filter, bin) ?? fallback(bin) +} + let defaultConfigured: { bin?: string; value: string } | undefined let defaultCompatible: { bin?: string; value: string } | undefined @@ -176,7 +181,7 @@ export function resolve(input: ResolveInput, configShell?: string, options?: Opt if (options?.gitbash) return select(process.env.SHELL, options, filter, bin) const cached = input.priority === "compat" ? defaultCompatible : defaultConfigured if (cached && cached.bin === bin) return cached.value - const value = select(process.env.SHELL, undefined, filter, bin) ?? fallback(bin) + const value = environment(bin, filter) if (input.priority === "compat") defaultCompatible = { bin, value } if (input.priority === "config") defaultConfigured = { bin, value } return value diff --git a/packages/core/test/shell.test.ts b/packages/core/test/shell.test.ts index a9b3f17fff3..11fb974c8e8 100644 --- a/packages/core/test/shell.test.ts +++ b/packages/core/test/shell.test.ts @@ -3,6 +3,8 @@ import path from "path" import { ShellSelect } from "@opencode-ai/core/shell/select" import { FSUtil } from "@opencode-ai/util/fs-util" import { which } from "@opencode-ai/core/util/which" +import fs from "node:fs/promises" +import { tmpdir } from "./fixture/tmpdir" const withShell = async (shell: string | undefined, fn: () => void | Promise) => { const prev = process.env.SHELL @@ -60,6 +62,22 @@ describe("shell", () => { ]) }) + if (process.platform !== "win32") { + test("resolves the environment shell without retaining a removed executable", async () => { + await withShell(undefined, async () => { + await using directory = await tmpdir() + const fallback = ShellSelect.environment() + const shell = path.join(directory.path, "preferred-shell") + await fs.symlink("/bin/sh", shell) + process.env.SHELL = shell + expect(ShellSelect.environment()).toBe(shell) + expect(ShellSelect.resolve({ priority: "config" })).toBe(shell) + await fs.unlink(shell) + expect(ShellSelect.environment()).toBe(fallback) + }) + }) + } + if (process.platform === "win32") { test("rejects blacklisted shells case-insensitively", async () => { await withShell("NU.EXE", async () => { diff --git a/packages/schema/src/persistent-pty.ts b/packages/schema/src/persistent-pty.ts index c791f357cb4..ad5e4a364dc 100644 --- a/packages/schema/src/persistent-pty.ts +++ b/packages/schema/src/persistent-pty.ts @@ -16,9 +16,9 @@ export const Info = Schema.Struct({ export interface Info extends Schema.Schema.Type {} export const CreateInput = Schema.Struct({ - command: Schema.String, + command: optional(Schema.String), args: Schema.Array(Schema.String), - cwd: Schema.String, + cwd: optional(Schema.String), title: Schema.String, env: Schema.Record(Schema.String, Schema.String), size: optional(Schema.Struct({ cols: PositiveInt, rows: PositiveInt })), diff --git a/packages/server/test/persistent-pty.test.ts b/packages/server/test/persistent-pty.test.ts index cd2a66e9d99..8046f0cc669 100644 --- a/packages/server/test/persistent-pty.test.ts +++ b/packages/server/test/persistent-pty.test.ts @@ -23,6 +23,7 @@ smoke( binary: process.env.OPENCODE_PTY_BIN, runtime: process.env.OPENCODE_PTY_RUNTIME_DIR, xdg: process.env.XDG_RUNTIME_DIR, + shell: process.env.SHELL, } const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-pty-server-test-")) const database = path.join(root, "opencode.db") @@ -30,6 +31,7 @@ smoke( process.env.OPENCODE_PTY_BIN = binary delete process.env.OPENCODE_PTY_RUNTIME_DIR process.env.XDG_RUNTIME_DIR = runtime + process.env.SHELL = "/bin/sh" return { database, directory: path.join( @@ -53,19 +55,47 @@ smoke( }) const base = HttpServer.formatAddress(server.address) const sessionID = Session.ID.make("ses_persistent_pty_test") - const events = yield* Effect.promise(() => openEventStream(base)) expect(existsSync(path.join(fixture.directory, "service.json"))).toBeFalse() expect((yield* request(base, "GET", `/api/experimental/session/${sessionID}/terminal`)).data).toEqual([]) expect(existsSync(path.join(fixture.directory, "service.json"))).toBeFalse() + const defaults = { + args: [], + cwd: fixture.root, + title: "default shell", + env: { SHELL: "/missing/client/zsh" }, + } + const createPath = `/api/experimental/session/${sessionID}/terminal` + const terminal = Schema.decodeUnknownSync(PersistentPty.Info)( + (yield* request(base, "POST", createPath, defaults)).data, + ) + expect(terminal.command).toBe("/bin/sh") + expect(terminal.cwd).toBe(fixture.root) + expect(terminal.cwd).not.toBe(process.cwd()) + yield* request(base, "DELETE", `/api/experimental/persistent-pty/${terminal.id}`) + const root = Schema.decodeUnknownSync(PersistentPty.Info)( + (yield* request(base, "POST", createPath, { + args: ["-c", "printf 'root-cwd:%s\\n' \"$PWD\"; cat"], + title: "root directory", + env: {}, + })).data, + ) + expect(root.cwd).toBe(path.parse(fixture.root).root) + expect(root.cwd).not.toBe(process.cwd()) + expect(yield* waitForText(base, root.id, `root-cwd:${root.cwd}`)).toContain(`root-cwd:${root.cwd}`) + yield* request(base, "DELETE", `/api/experimental/persistent-pty/${root.id}`) + const events = yield* Effect.promise(() => openEventStream(base)) const first = Schema.decodeUnknownSync(PersistentPty.Info)( - (yield* request(base, "POST", `/api/experimental/session/${sessionID}/terminal`, { - command: "/bin/sh", - args: ["-c", "stty -echo; printf terminal-one; cat"], + (yield* request(base, "POST", createPath, { + command: "/usr/bin/env", + args: ["/bin/sh", "-c", "stty -echo; printf terminal-one; cat"], cwd: process.cwd(), title: "first", env: {}, })).data, ) + expect(first.command).toBe("/usr/bin/env") + expect(first.args).toEqual(["/bin/sh", "-c", "stty -echo; printf terminal-one; cat"]) + expect(first.cwd).toBe(process.cwd()) expect(yield* Effect.promise(() => events.next("persistent-pty.added"))).toMatchObject({ data: { sessionID, terminal: { id: first.id } }, }) @@ -185,6 +215,7 @@ smoke( restore("OPENCODE_PTY_BIN", fixture.environment.binary) restore("OPENCODE_PTY_RUNTIME_DIR", fixture.environment.runtime) restore("XDG_RUNTIME_DIR", fixture.environment.xdg) + restore("SHELL", fixture.environment.shell) }), ), 20_000, diff --git a/packages/tui/src/context/session-terminals.tsx b/packages/tui/src/context/session-terminals.tsx index 7df9f139b18..3c2681e2c3b 100644 --- a/packages/tui/src/context/session-terminals.tsx +++ b/packages/tui/src/context/session-terminals.tsx @@ -79,9 +79,8 @@ export const { use: useSessionTerminals, provider: SessionTerminalsProvider } = const session = data.session.get(sessionID) const terminal = await client.api.experimental.persistentPty.create({ sessionID, - command: process.env.SHELL || "/bin/sh", args: [], - cwd: session?.location.directory ?? process.cwd(), + cwd: session?.location.directory, title: "Terminal", env: {}, })