refactor(core): canonicalize pty service (#32182)

packages/core/test/pty/info-schema.test.ts

@@ -24,4 +24,9 @@ describe("Pty.Info", () => {
   test("rejects a negative pid", () => {
     expect(() => Schema.decodeUnknownSync(Pty.Info)(sample(-1))).toThrow()
   })
+
+  test("accepts an exit code for retained exited sessions", () => {
+    const info = Schema.decodeUnknownSync(Pty.Info)({ ...sample(48012), status: "exited", exitCode: 4 })
+    expect(info.exitCode).toBe(4)
+  })
 })

packages/core/test/pty/input.test.ts

@@ -1,19 +0,0 @@
-import { describe, expect } from "bun:test"
-import { Effect } from "effect"
-import { handlePtyInput } from "@opencode-ai/core/pty/input"
-import { it } from "../lib/effect"
-
-describe("pty websocket input", () => {
-  it.effect("does not forward invalid binary frames to the PTY handler", () =>
-    Effect.gen(function* () {
-      const messages: Array<string | ArrayBuffer> = []
-      const handler = { onMessage: (message: string | ArrayBuffer) => messages.push(message) }
-
-      yield* handlePtyInput(handler, "ready")
-      yield* handlePtyInput(handler, new Uint8Array([0xff, 0xfe, 0xfd]))
-      yield* handlePtyInput(handler, new TextEncoder().encode("hello"))
-
-      expect(messages).toEqual(["ready", "hello"])
-    }),
-  )
-})

packages/core/test/pty/protocol.test.ts

@@ -0,0 +1,27 @@
+import { describe, expect, test } from "bun:test"
+import { PtyProtocol } from "@opencode-ai/core/pty/protocol"
+
+describe("pty protocol", () => {
+  test("drops invalid binary input frames and decodes valid ones", () => {
+    expect(PtyProtocol.decodeInput("ready")).toBe("ready")
+    expect(PtyProtocol.decodeInput(new Uint8Array([0xff, 0xfe, 0xfd]))).toBeUndefined()
+    expect(PtyProtocol.decodeInput(new TextEncoder().encode("hello"))).toBe("hello")
+    expect(PtyProtocol.decodeInput(new TextEncoder().encode("hello").buffer)).toBe("hello")
+  })
+
+  test("encodes the cursor as a 0x00-prefixed JSON control frame", () => {
+    const frame = PtyProtocol.metaFrame(42)
+    expect(frame[0]).toBe(0)
+    expect(JSON.parse(new TextDecoder().decode(frame.subarray(1)))).toEqual({ cursor: 42 })
+  })
+
+  test("splits replay into bounded frames", () => {
+    expect(PtyProtocol.chunks("")).toEqual([])
+    expect(PtyProtocol.chunks("abc")).toEqual(["abc"])
+    const big = "x".repeat(PtyProtocol.REPLAY_CHUNK + 1)
+    const frames = PtyProtocol.chunks(big)
+    expect(frames.length).toBe(2)
+    expect(frames[0].length).toBe(PtyProtocol.REPLAY_CHUNK)
+    expect(frames.join("")).toBe(big)
+  })
+})

packages/core/test/pty/pty-output-isolation.test.ts

@@ -1,110 +0,0 @@
-import { describe, expect } from "bun:test"
-import { Duration, Effect, Layer, Queue } from "effect"
-import { EventV2 } from "@opencode-ai/core/event"
-import { Location } from "@opencode-ai/core/location"
-import { Pty } from "@opencode-ai/core/pty"
-import { AbsolutePath } from "@opencode-ai/core/schema"
-import { location } from "../fixture/location"
-import { testEffect } from "../lib/effect"
-
-type Socket = Parameters<Pty.Interface["connect"]>[1]
-
-const locationLayer = Layer.succeed(
-  Location.Service,
-  Location.Service.of(location({ directory: AbsolutePath.make("/tmp") })),
-)
-const it = testEffect(Pty.layer.pipe(Layer.provideMerge(EventV2.defaultLayer), Layer.provideMerge(locationLayer)))
-const ptyTest = process.platform === "win32" ? it.live.skip : it.live
-
-const createPty = Effect.fn("PtyOutputIsolationTest.createPty")(function* (command: string) {
-  const pty = yield* Pty.Service
-  return yield* Effect.acquireRelease(
-    pty.create({ command, args: [], cwd: "/tmp", env: { TERM: "xterm-256color", OPENCODE_TERMINAL: "1" } }),
-    (info) => pty.remove(info.id).pipe(Effect.ignore),
-  )
-})
-
-const decodeOutput = (data: string | Uint8Array | ArrayBuffer) =>
-  typeof data === "string"
-    ? data
-    : Buffer.from(data instanceof Uint8Array ? data : new Uint8Array(data)).toString("utf8")
-
-const makeSocket = Effect.fn("PtyOutputIsolationTest.makeSocket")(function* (data: unknown) {
-  const output = yield* Queue.unbounded<string>()
-  const socket: Socket = {
-    readyState: 1,
-    data,
-    send: (data) => Queue.offerUnsafe(output, decodeOutput(data)),
-    close: () => {},
-  }
-  return { socket, output }
-})
-
-const waitForOutput = (output: Queue.Queue<string>, text: string, duration: Duration.Input = "5 seconds") =>
-  Effect.gen(function* () {
-    let received = ""
-    while (!received.includes(text)) received += yield* Queue.take(output)
-    return received
-  }).pipe(
-    Effect.timeoutOrElse({
-      duration,
-      orElse: () => Effect.fail(new Error(`timeout waiting for output containing ${JSON.stringify(text)}`)),
-    }),
-  )
-
-describe("pty output isolation", () => {
-  ptyTest("does not leak output when websocket objects are reused", () =>
-    Effect.gen(function* () {
-      const pty = yield* Pty.Service
-      const a = yield* createPty("cat")
-      const b = yield* createPty("cat")
-      const shared = yield* makeSocket({ events: { connection: "a" } })
-      const outB = yield* Queue.unbounded<string>()
-
-      yield* pty.connect(a.id, shared.socket)
-      shared.socket.data = { events: { connection: "b" } }
-      shared.socket.send = (data) => Queue.offerUnsafe(outB, decodeOutput(data))
-      yield* pty.connect(b.id, shared.socket)
-      yield* pty.write(a.id, "AAA\n")
-
-      const verify = yield* makeSocket({ events: { connection: "verify-a" } })
-      yield* pty.connect(a.id, verify.socket)
-      expect(yield* waitForOutput(verify.output, "AAA")).toContain("AAA")
-      expect(yield* waitForOutput(outB, "AAA", "100 millis").pipe(Effect.option)).toMatchObject({ _tag: "None" })
-    }),
-  )
-
-  ptyTest("does not leak output when Bun recycles websocket objects before re-connect", () =>
-    Effect.gen(function* () {
-      const pty = yield* Pty.Service
-      const info = yield* createPty("cat")
-      const first = yield* makeSocket({ events: { connection: "a" } })
-      const recycled = yield* Queue.unbounded<string>()
-
-      yield* pty.connect(info.id, first.socket)
-      first.socket.data = { events: { connection: "b" } }
-      first.socket.send = (data) => Queue.offerUnsafe(recycled, decodeOutput(data))
-      yield* pty.write(info.id, "AAA\n")
-
-      const verify = yield* makeSocket({ events: { connection: "verify" } })
-      yield* pty.connect(info.id, verify.socket)
-      expect(yield* waitForOutput(verify.output, "AAA")).toContain("AAA")
-      expect(yield* waitForOutput(recycled, "AAA", "100 millis").pipe(Effect.option)).toMatchObject({ _tag: "None" })
-    }),
-  )
-
-  ptyTest("treats in-place socket data mutation as the same connection", () =>
-    Effect.gen(function* () {
-      const pty = yield* Pty.Service
-      const info = yield* createPty("cat")
-      const data = { connId: 1 }
-      const socket = yield* makeSocket(data)
-
-      yield* pty.connect(info.id, socket.socket)
-      data.connId = 2
-      yield* pty.write(info.id, "AAA\n")
-
-      expect(yield* waitForOutput(socket.output, "AAA")).toContain("AAA")
-    }),
-  )
-})

packages/core/test/pty/pty-session.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect } from "bun:test"
-import { Cause, Effect, Exit, Layer, Queue } from "effect"
+import { Cause, Deferred, Effect, Exit, Layer, Queue } from "effect"
+import { Config } from "@opencode-ai/core/config"
 import { EventV2 } from "@opencode-ai/core/event"
 import { Location } from "@opencode-ai/core/location"
 import { Pty } from "@opencode-ai/core/pty"
@@ -14,7 +15,14 @@ const locationLayer = Layer.succeed(
   Location.Service,
   Location.Service.of(location({ directory: AbsolutePath.make("/tmp") })),
 )
-const it = testEffect(Pty.layer.pipe(Layer.provideMerge(EventV2.defaultLayer), Layer.provideMerge(locationLayer)))
+const configLayer = Layer.mock(Config.Service)({ entries: () => Effect.succeed([]) })
+const it = testEffect(
+  Pty.layer.pipe(
+    Layer.provide(configLayer),
+    Layer.provideMerge(EventV2.defaultLayer),
+    Layer.provideMerge(locationLayer),
+  ),
+)
 const ptyTest = process.platform === "win32" ? it.live.skip : it.live
 
 const subscribePtyEvents = Effect.fn("PtySessionTest.subscribePtyEvents")(function* () {
@@ -56,36 +64,176 @@ const waitForEvents = (events: Queue.Queue<PtyEvent>, id: PtyID, count: number)
     }),
   )
 
+const attachCollecting = Effect.fn("PtySessionTest.attachCollecting")(function* (id: PtyID, cursor?: number) {
+  const pty = yield* Pty.Service
+  const output = yield* Queue.unbounded<string>()
+  const ended = yield* Deferred.make<{ exitCode?: number }>()
+  const attachment = yield* pty.attach(id, {
+    cursor,
+    onData: (chunk) => Queue.offerUnsafe(output, chunk),
+    onEnd: (event) => Deferred.doneUnsafe(ended, Effect.succeed(event)),
+  })
+  attachment.activate()
+  return { attachment, output, ended }
+})
+
+const waitForOutput = (output: Queue.Queue<string>, text: string) =>
+  Effect.gen(function* () {
+    let received = ""
+    while (!received.includes(text)) received += yield* Queue.take(output)
+    return received
+  }).pipe(
+    Effect.timeoutOrElse({
+      duration: "5 seconds",
+      orElse: () => Effect.fail(new Error(`timeout waiting for output containing ${JSON.stringify(text)}`)),
+    }),
+  )
+
 describe("pty", () => {
   it.live("returns typed not found errors for missing sessions", () =>
     Effect.gen(function* () {
       const pty = yield* Pty.Service
       const id = "pty_missing" as PtyID
-      let closed = false
-      const socket = { readyState: 1, send: () => {}, close: () => void (closed = true) }
 
       for (const result of [
         yield* pty.get(id).pipe(Effect.asVoid, Effect.exit),
         yield* pty.update(id, { title: "missing" }).pipe(Effect.asVoid, Effect.exit),
         yield* pty.remove(id).pipe(Effect.exit),
-        yield* pty.resize(id, 80, 24).pipe(Effect.exit),
         yield* pty.write(id, "input").pipe(Effect.exit),
-        yield* pty.connect(id, socket).pipe(Effect.asVoid, Effect.exit),
+        yield* pty.attach(id, { onData: () => {}, onEnd: () => {} }).pipe(Effect.asVoid, Effect.exit),
       ]) {
         expect(Exit.isFailure(result)).toBe(true)
         if (Exit.isFailure(result))
           expect(Cause.squash(result.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id })
       }
-      expect(closed).toBe(true)
     }),
   )
 
-  ptyTest("publishes created, exited, deleted in order for a short-lived process", () =>
+  ptyTest("retains exited sessions until removed", () =>
     Effect.gen(function* () {
+      const pty = yield* Pty.Service
       const events = yield* subscribePtyEvents()
-      const info = yield* createPty("/usr/bin/env", ["sh", "-c", "sleep 0.1"])
+      const info = yield* createPty("/usr/bin/env", ["sh", "-c", "exit 3"])
 
-      expect(yield* waitForEvents(events, info.id, 3)).toEqual(["created", "exited", "deleted"])
+      expect(yield* waitForEvents(events, info.id, 2)).toEqual(["created", "exited"])
+      const exited = yield* pty.get(info.id)
+      expect(exited.status).toBe("exited")
+      expect(exited.exitCode).toBe(3)
+
+      yield* pty.remove(info.id)
+      expect(yield* waitForEvents(events, info.id, 1)).toEqual(["deleted"])
+      const missing = yield* pty.get(info.id).pipe(Effect.exit)
+      expect(Exit.isFailure(missing)).toBe(true)
+    }),
+  )
+
+  ptyTest("replays buffered output and streams live output to attachments", () =>
+    Effect.gen(function* () {
+      const pty = yield* Pty.Service
+      const info = yield* createPty("cat")
+      yield* pty.write(info.id, "AAA\n")
+
+      const first = yield* attachCollecting(info.id)
+      expect(yield* waitForOutput(first.output, "AAA")).toContain("AAA")
+
+      first.attachment.write("BBB\n")
+      yield* waitForOutput(first.output, "BBB")
+
+      // A later attachment replays everything already buffered.
+      const replayed = yield* attachCollecting(info.id)
+      expect(replayed.attachment.replay).toContain("AAA")
+      expect(replayed.attachment.replay).toContain("BBB")
+      expect(replayed.attachment.cursor).toBeGreaterThan(0)
+
+      // Tail attachments skip the buffer and only see subsequent output.
+      const tail = yield* attachCollecting(info.id, -1)
+      expect(tail.attachment.replay).toBe("")
+      expect(tail.attachment.cursor).toBe(replayed.attachment.cursor)
+    }),
+  )
+
+  ptyTest("stops delivering output after detach", () =>
+    Effect.gen(function* () {
+      const pty = yield* Pty.Service
+      const info = yield* createPty("cat")
+      const attached = yield* attachCollecting(info.id, -1)
+
+      attached.attachment.detach()
+      yield* pty.write(info.id, "AAA\n")
+
+      const verify = yield* attachCollecting(info.id)
+      yield* waitForOutput(verify.output, "AAA")
+      const leaked = yield* Queue.poll(attached.output)
+      expect(leaked._tag).toBe("None")
+    }),
+  )
+
+  ptyTest("isolates output between sessions", () =>
+    Effect.gen(function* () {
+      const pty = yield* Pty.Service
+      const a = yield* createPty("cat")
+      const b = yield* createPty("cat")
+      const attachedA = yield* attachCollecting(a.id)
+      const attachedB = yield* attachCollecting(b.id)
+
+      yield* pty.write(a.id, "AAA\n")
+      yield* waitForOutput(attachedA.output, "AAA")
+
+      const leaked = yield* Queue.poll(attachedB.output)
+      expect(leaked._tag).toBe("None")
+    }),
+  )
+
+  ptyTest("notifies attachments with the exit code and rejects attach after exit", () =>
+    Effect.gen(function* () {
+      const pty = yield* Pty.Service
+      const events = yield* subscribePtyEvents()
+      const info = yield* createPty("cat")
+      const attached = yield* attachCollecting(info.id)
+
+      yield* pty.write(info.id, "\u0004")
+      expect(yield* Deferred.await(attached.ended).pipe(Effect.timeout("5 seconds"))).toEqual({ exitCode: 0 })
+      yield* waitForEvents(events, info.id, 2)
+
+      const result = yield* pty.attach(info.id, { onData: () => {}, onEnd: () => {} }).pipe(Effect.exit)
+      expect(Exit.isFailure(result)).toBe(true)
+      if (Exit.isFailure(result))
+        expect(Cause.squash(result.cause)).toMatchObject({ _tag: "Pty.ExitedError", ptyID: info.id })
+    }),
+  )
+})
+
+const configuredShell = process.platform === "win32" ? undefined : Bun.which("bash")
+const configuredIt = testEffect(
+  Pty.layer.pipe(
+    Layer.provide(
+      Layer.mock(Config.Service)({
+        entries: () =>
+          Effect.succeed(
+            configuredShell
+              ? [new Config.Document({ type: "document", info: new Config.Info({ shell: configuredShell }) })]
+              : [],
+          ),
+      }),
+    ),
+    Layer.provideMerge(EventV2.defaultLayer),
+    Layer.provideMerge(locationLayer),
+  ),
+)
+const configuredTest = process.platform === "win32" ? configuredIt.live.skip : configuredIt.live
+
+describe("pty create defaults", () => {
+  configuredTest("defaults command, login args, and cwd from config and location", () =>
+    Effect.gen(function* () {
+      if (!configuredShell) return
+      const pty = yield* Pty.Service
+      const info = yield* Effect.acquireRelease(pty.create({ title: "configured" }), (created) =>
+        pty.remove(created.id).pipe(Effect.ignore),
+      )
+      expect(info.command).toBe(configuredShell)
+      expect(info.args).toEqual(["-l"])
+      expect(info.cwd).toBe("/tmp")
+      expect(info.title).toBe("configured")
     }),
   )
 })

packages/core/test/shell.test.ts

@@ -0,0 +1,108 @@
+import { describe, expect, test } from "bun:test"
+import path from "path"
+import { Shell } from "@opencode-ai/core/shell"
+import { FSUtil } from "@opencode-ai/core/fs-util"
+import { which } from "@opencode-ai/core/util/which"
+
+const withShell = async (shell: string | undefined, fn: () => void | Promise<void>) => {
+  const prev = process.env.SHELL
+  if (shell === undefined) delete process.env.SHELL
+  else process.env.SHELL = shell
+  Shell.acceptable.reset()
+  Shell.preferred.reset()
+  try {
+    await fn()
+  } finally {
+    if (prev === undefined) delete process.env.SHELL
+    else process.env.SHELL = prev
+    Shell.acceptable.reset()
+    Shell.preferred.reset()
+  }
+}
+
+describe("shell", () => {
+  test("normalizes shell names", () => {
+    expect(Shell.name("/bin/bash")).toBe("bash")
+    if (process.platform === "win32") {
+      expect(Shell.name("C:/tools/NU.EXE")).toBe("nu")
+      expect(Shell.name("C:/tools/PWSH.EXE")).toBe("pwsh")
+    }
+  })
+
+  test("detects login shells", () => {
+    expect(Shell.login("/bin/bash")).toBe(true)
+    expect(Shell.login("C:/tools/pwsh.exe")).toBe(false)
+  })
+
+  test("detects posix shells", () => {
+    expect(Shell.posix("/bin/bash")).toBe(true)
+    expect(Shell.posix("/bin/fish")).toBe(false)
+    expect(Shell.posix("C:/tools/pwsh.exe")).toBe(false)
+  })
+
+  test("falls back when configured shell cannot be resolved", async () => {
+    await withShell(undefined, async () => {
+      const preferred = Shell.preferred()
+      const acceptable = Shell.acceptable()
+      expect(Shell.preferred("opencode-missing-shell")).toBe(preferred)
+      expect(Shell.acceptable("opencode-missing-shell")).toBe(acceptable)
+    })
+  })
+
+  test("falls back for terminal-only acceptable shells", () => {
+    expect(Shell.name(Shell.acceptable("fish"))).not.toBe("fish")
+    expect(Shell.name(Shell.acceptable("nu"))).not.toBe("nu")
+  })
+
+  test("builds command args per shell family", () => {
+    expect(Shell.args("/bin/sh", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
+    expect(Shell.args("/usr/bin/fish", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
+    const zsh = Shell.args("/bin/zsh", "echo hi", "/tmp")
+    expect(zsh[0]).toBe("-l")
+    expect(zsh[1]).toBe("-c")
+    expect(zsh.at(-1)).toBe("/tmp")
+  })
+
+  if (process.platform === "win32") {
+    test("rejects blacklisted shells case-insensitively", async () => {
+      await withShell("NU.EXE", async () => {
+        expect(Shell.name(Shell.acceptable())).not.toBe("nu")
+      })
+    })
+
+    test("normalizes Git Bash shell paths from env", async () => {
+      const shell = "/cygdrive/c/Program Files/Git/bin/bash.exe"
+      await withShell(shell, async () => {
+        expect(Shell.preferred()).toBe(FSUtil.windowsPath(shell))
+      })
+    })
+
+    test("resolves /usr/bin/bash from env to Git Bash", async () => {
+      const bash = Shell.gitbash()
+      if (!bash) return
+      await withShell("/usr/bin/bash", async () => {
+        expect(Shell.acceptable()).toBe(bash)
+        expect(Shell.preferred()).toBe(bash)
+      })
+    })
+
+    test("resolves bare bash to Git Bash before PATH", async () => {
+      const bash = Shell.gitbash()
+      if (!bash) return
+      expect(Shell.acceptable("bash")).toBe(bash)
+      expect(Shell.preferred("bash")).toBe(bash)
+      await withShell("bash", async () => {
+        expect(Shell.acceptable()).toBe(bash)
+        expect(Shell.preferred()).toBe(bash)
+      })
+    })
+
+    test("resolves bare PowerShell shells", async () => {
+      const shell = which("pwsh") || which("powershell")
+      if (!shell) return
+      await withShell(path.win32.basename(shell), async () => {
+        expect(Shell.preferred()).toBe(shell)
+      })
+    })
+  }
+})