Merge pull request #2451 from code-yeongyu/fix/issue-2238-v2

fix: prevent terminal corruption during background bun install

src/cli/config-manager/bun-install.test.ts

@@ -1,10 +1,37 @@
-import { beforeEach, afterEach, describe, expect, it, spyOn } from "bun:test"
+/// <reference types="bun-types" />
+
 import * as fs from "node:fs"
+
+import { afterEach, beforeEach, describe, expect, it, jest, spyOn } from "bun:test"
+
 import * as dataPath from "../../shared/data-path"
 import * as logger from "../../shared/logger"
 import * as spawnHelpers from "../../shared/spawn-with-windows-hide"
+import type { BunInstallResult } from "./bun-install"
 import { runBunInstallWithDetails } from "./bun-install"
 
+type CreateProcOptions = {
+  exitCode?: number | null
+  exited?: Promise<number>
+  kill?: () => void
+  output?: {
+    stdout?: string
+    stderr?: string
+  }
+}
+
+function createProc(options: CreateProcOptions = {}): ReturnType<typeof spawnHelpers.spawnWithWindowsHide> {
+  const exitCode = options.exitCode ?? 0
+
+  return {
+    exited: options.exited ?? Promise.resolve(exitCode),
+    exitCode,
+    stdout: options.output?.stdout !== undefined ? new Blob([options.output.stdout]).stream() : undefined,
+    stderr: options.output?.stderr !== undefined ? new Blob([options.output.stderr]).stream() : undefined,
+    kill: options.kill ?? (() => {}),
+  } satisfies ReturnType<typeof spawnHelpers.spawnWithWindowsHide>
+}
+
 describe("runBunInstallWithDetails", () => {
   let getOpenCodeCacheDirSpy: ReturnType<typeof spyOn>
   let logSpy: ReturnType<typeof spyOn>
@@ -14,11 +41,7 @@ describe("runBunInstallWithDetails", () => {
   beforeEach(() => {
     getOpenCodeCacheDirSpy = spyOn(dataPath, "getOpenCodeCacheDir").mockReturnValue("/tmp/opencode-cache")
     logSpy = spyOn(logger, "log").mockImplementation(() => {})
-    spawnWithWindowsHideSpy = spyOn(spawnHelpers, "spawnWithWindowsHide").mockReturnValue({
+    spawnWithWindowsHideSpy = spyOn(spawnHelpers, "spawnWithWindowsHide").mockReturnValue(createProc())
-      exited: Promise.resolve(0),
-      exitCode: 0,
-      kill: () => {},
-    } as ReturnType<typeof spawnHelpers.spawnWithWindowsHide>)
     existsSyncSpy = spyOn(fs, "existsSync").mockReturnValue(true)
   })
 
@@ -29,15 +52,119 @@ describe("runBunInstallWithDetails", () => {
     existsSyncSpy.mockRestore()
   })
 
-  it("runs bun install in the OpenCode cache directory", async () => {
+  describe("#given the cache workspace exists", () => {
-    const result = await runBunInstallWithDetails()
+    describe("#when bun install uses inherited output", () => {
+      it("#then runs bun install in the cache directory", async () => {
+        // given
 
-    expect(result).toEqual({ success: true })
+        // when
-    expect(getOpenCodeCacheDirSpy).toHaveBeenCalledTimes(1)
+        const result = await runBunInstallWithDetails()
-    expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], {
+
-      cwd: "/tmp/opencode-cache",
+        // then
-      stdout: "inherit",
+        expect(result).toEqual({ success: true })
-      stderr: "inherit",
+        expect(getOpenCodeCacheDirSpy).toHaveBeenCalledTimes(1)
+        expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], {
+          cwd: "/tmp/opencode-cache",
+          stdout: "inherit",
+          stderr: "inherit",
+        })
+      })
+    })
+
+    describe("#when bun install uses piped output", () => {
+      it("#then passes pipe mode to the spawned process", async () => {
+        // given
+
+        // when
+        const result = await runBunInstallWithDetails({ outputMode: "pipe" })
+
+        // then
+        expect(result).toEqual({ success: true })
+        expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], {
+          cwd: "/tmp/opencode-cache",
+          stdout: "pipe",
+          stderr: "pipe",
+        })
+      })
+    })
+
+    describe("#when piped bun install fails", () => {
+      it("#then logs captured stdout and stderr", async () => {
+        // given
+        spawnWithWindowsHideSpy.mockReturnValue(
+          createProc({
+            exitCode: 1,
+            output: {
+              stdout: "resolved 10 packages",
+              stderr: "network error",
+            },
+          })
+        )
+
+        // when
+        const result = await runBunInstallWithDetails({ outputMode: "pipe" })
+
+        // then
+        expect(result).toEqual({
+          success: false,
+          error: "bun install failed with exit code 1",
+        })
+        expect(logSpy).toHaveBeenCalledWith("[bun-install] Captured output from failed bun install", {
+          stdout: "resolved 10 packages",
+          stderr: "network error",
+        })
+      })
+    })
+
+    describe("#when the install times out and proc.exited never resolves", () => {
+      it("#then returns timedOut true without hanging", async () => {
+        // given
+        jest.useFakeTimers()
+
+        let killCallCount = 0
+        spawnWithWindowsHideSpy.mockReturnValue(
+          createProc({
+            exitCode: null,
+            exited: new Promise<number>(() => {}),
+            kill: () => {
+              killCallCount += 1
+            },
+          })
+        )
+
+        try {
+          // when
+          const resultPromise = runBunInstallWithDetails({ outputMode: "pipe" })
+          jest.advanceTimersByTime(60_000)
+          jest.runOnlyPendingTimers()
+          await Promise.resolve()
+
+          const outcome = await Promise.race([
+            resultPromise.then((result) => ({
+              status: "resolved" as const,
+              result,
+            })),
+            new Promise<{ status: "pending" }>((resolve) => {
+              queueMicrotask(() => resolve({ status: "pending" }))
+            }),
+          ])
+
+          // then
+          if (outcome.status === "pending") {
+            throw new Error("runBunInstallWithDetails did not resolve after timing out")
+          }
+
+          expect(outcome.result).toEqual({
+            success: false,
+            timedOut: true,
+            error: 'bun install timed out after 60 seconds. Try running manually: cd "/tmp/opencode-cache" && bun i',
+          } satisfies BunInstallResult)
+          expect(killCallCount).toBe(1)
+        } finally {
+          jest.clearAllTimers()
+          jest.useRealTimers()
+        }
+      })
     })
   })
 })

src/cli/config-manager/bun-install.ts

@@ -1,4 +1,5 @@
 import { existsSync } from "node:fs"
+
 import { getOpenCodeCacheDir } from "../../shared/data-path"
 import { log } from "../../shared/logger"
 import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide"
@@ -6,6 +7,26 @@ import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide"
 const BUN_INSTALL_TIMEOUT_SECONDS = 60
 const BUN_INSTALL_TIMEOUT_MS = BUN_INSTALL_TIMEOUT_SECONDS * 1000
 
+type BunInstallOutputMode = "inherit" | "pipe"
+
+interface RunBunInstallOptions {
+  outputMode?: BunInstallOutputMode
+}
+
+interface BunInstallOutput {
+  stdout: string
+  stderr: string
+}
+
+declare function setTimeout(callback: () => void, delay?: number): number
+declare function clearTimeout(timeout: number): void
+
+type ProcessOutputStream = ReturnType<typeof spawnWithWindowsHide>["stdout"]
+
+declare const Bun: {
+  readableStreamToText(stream: NonNullable<ProcessOutputStream>): Promise<string>
+}
+
 export interface BunInstallResult {
   success: boolean
   timedOut?: boolean
@@ -17,7 +38,33 @@ export async function runBunInstall(): Promise<boolean> {
   return result.success
 }
 
-export async function runBunInstallWithDetails(): Promise<BunInstallResult> {
+function readProcessOutput(stream: ProcessOutputStream): Promise<string> {
+  if (!stream) {
+    return Promise.resolve("")
+  }
+
+  return Bun.readableStreamToText(stream)
+}
+
+function logCapturedOutputOnFailure(outputMode: BunInstallOutputMode, output: BunInstallOutput): void {
+  if (outputMode !== "pipe") {
+    return
+  }
+
+  const stdout = output.stdout.trim()
+  const stderr = output.stderr.trim()
+  if (!stdout && !stderr) {
+    return
+  }
+
+  log("[bun-install] Captured output from failed bun install", {
+    stdout,
+    stderr,
+  })
+}
+
+export async function runBunInstallWithDetails(options?: RunBunInstallOptions): Promise<BunInstallResult> {
+  const outputMode = options?.outputMode ?? "inherit"
   const cacheDir = getOpenCodeCacheDir()
   const packageJsonPath = `${cacheDir}/package.json`
 
@@ -31,17 +78,23 @@ export async function runBunInstallWithDetails(): Promise<BunInstallResult> {
   try {
     const proc = spawnWithWindowsHide(["bun", "install"], {
       cwd: cacheDir,
-      stdout: "inherit",
+      stdout: outputMode,
-      stderr: "inherit",
+      stderr: outputMode,
     })
 
-    let timeoutId: ReturnType<typeof setTimeout>
+    const outputPromise = Promise.all([readProcessOutput(proc.stdout), readProcessOutput(proc.stderr)]).then(
+      ([stdout, stderr]) => ({ stdout, stderr })
+    )
+
+    let timeoutId: ReturnType<typeof setTimeout> | undefined
     const timeoutPromise = new Promise<"timeout">((resolve) => {
       timeoutId = setTimeout(() => resolve("timeout"), BUN_INSTALL_TIMEOUT_MS)
     })
     const exitPromise = proc.exited.then(() => "completed" as const)
     const result = await Promise.race([exitPromise, timeoutPromise])
-    clearTimeout(timeoutId!)
+    if (timeoutId) {
+      clearTimeout(timeoutId)
+    }
 
     if (result === "timeout") {
       try {
@@ -49,6 +102,17 @@ export async function runBunInstallWithDetails(): Promise<BunInstallResult> {
       } catch (err) {
         log("[cli/install] Failed to kill timed out bun install process:", err)
       }
+
+      if (outputMode === "pipe") {
+        void outputPromise
+          .then((output) => {
+            logCapturedOutputOnFailure(outputMode, output)
+          })
+          .catch((err) => {
+            log("[bun-install] Failed to read captured output after timeout:", err)
+          })
+      }
+
       return {
         success: false,
         timedOut: true,
@@ -56,7 +120,11 @@ export async function runBunInstallWithDetails(): Promise<BunInstallResult> {
       }
     }
 
+    const output = await outputPromise
+
     if (proc.exitCode !== 0) {
+      logCapturedOutputOnFailure(outputMode, output)
+
       return {
         success: false,
         error: `bun install failed with exit code ${proc.exitCode}`,

src/hooks/auto-update-checker/hook/background-update-check.test.ts

@@ -1,6 +1,12 @@
-import type { PluginInput } from "@opencode-ai/plugin"
+/// <reference types="bun-types" />
+
+import type { BunInstallResult } from "../../../cli/config-manager"
 import { beforeEach, describe, expect, it, mock } from "bun:test"
 
+type PluginInput = {
+  directory: string
+}
+
 type PluginEntry = {
   entry: string
   isPinned: boolean
@@ -24,8 +30,14 @@ const mockFindPluginEntry = mock((_directory: string): PluginEntry | null => cre
 const mockGetCachedVersion = mock((): string | null => "3.4.0")
 const mockGetLatestVersion = mock(async (): Promise<string | null> => "3.5.0")
 const mockExtractChannel = mock(() => "latest")
-const mockInvalidatePackage = mock(() => {})
+const operationOrder: string[] = []
-const mockRunBunInstall = mock(async () => true)
+const mockSyncCachePackageJsonToIntent = mock((_pluginEntry: PluginEntry) => {
+  operationOrder.push("sync")
+})
+const mockInvalidatePackage = mock((_packageName: string) => {
+  operationOrder.push("invalidate")
+})
+const mockRunBunInstallWithDetails = mock(async (): Promise<BunInstallResult> => ({ success: true }))
 const mockShowUpdateAvailableToast = mock(
   async (_ctx: PluginInput, _latestVersion: string, _getToastMessage: ToastMessageGetter): Promise<void> => {}
 )
@@ -38,10 +50,11 @@ mock.module("../checker", () => ({
   getCachedVersion: mockGetCachedVersion,
   getLatestVersion: mockGetLatestVersion,
   revertPinnedVersion: mock(() => false),
+  syncCachePackageJsonToIntent: mockSyncCachePackageJsonToIntent,
 }))
 mock.module("../version-channel", () => ({ extractChannel: mockExtractChannel }))
 mock.module("../cache", () => ({ invalidatePackage: mockInvalidatePackage }))
-mock.module("../../../cli/config-manager", () => ({ runBunInstall: mockRunBunInstall }))
+mock.module("../../../cli/config-manager", () => ({ runBunInstallWithDetails: mockRunBunInstallWithDetails }))
 mock.module("./update-toasts", () => ({
   showUpdateAvailableToast: mockShowUpdateAvailableToast,
   showAutoUpdatedToast: mockShowAutoUpdatedToast,
@@ -51,36 +64,51 @@ mock.module("../../../shared/logger", () => ({ log: () => {} }))
 const modulePath = "./background-update-check?test"
 const { runBackgroundUpdateCheck } = await import(modulePath)
 
-async function runCheck(autoUpdate = true): Promise<void> {
+const mockContext = { directory: "/test" } as PluginInput
-  const mockContext = { directory: "/test" } as PluginInput
+const getToastMessage: ToastMessageGetter = (isUpdate, version) =>
-  const getToastMessage: ToastMessageGetter = (isUpdate, version) =>
+  isUpdate ? `Update to ${version}` : "Up to date"
-    isUpdate ? `Update to ${version}` : "Up to date"
 
+async function runCheck(autoUpdate = true): Promise<void> {
   await runBackgroundUpdateCheck(mockContext, autoUpdate, getToastMessage)
 }
 
 function expectNoUpdateEffects(): void {
   expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled()
   expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled()
-  expect(mockRunBunInstall).not.toHaveBeenCalled()
+  expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled()
+  expect(mockSyncCachePackageJsonToIntent).not.toHaveBeenCalled()
+  expect(mockInvalidatePackage).not.toHaveBeenCalled()
 }
 
 describe("runBackgroundUpdateCheck", () => {
+  let pluginEntry: PluginEntry
+
   beforeEach(() => {
     mockFindPluginEntry.mockReset()
     mockGetCachedVersion.mockReset()
     mockGetLatestVersion.mockReset()
     mockExtractChannel.mockReset()
+    mockSyncCachePackageJsonToIntent.mockReset()
     mockInvalidatePackage.mockReset()
-    mockRunBunInstall.mockReset()
+    mockRunBunInstallWithDetails.mockReset()
     mockShowUpdateAvailableToast.mockReset()
     mockShowAutoUpdatedToast.mockReset()
 
-    mockFindPluginEntry.mockReturnValue(createPluginEntry())
+    operationOrder.length = 0
+
+    mockSyncCachePackageJsonToIntent.mockImplementation((_pluginEntry: PluginEntry) => {
+      operationOrder.push("sync")
+    })
+    mockInvalidatePackage.mockImplementation((_packageName: string) => {
+      operationOrder.push("invalidate")
+    })
+
+    pluginEntry = createPluginEntry()
+    mockFindPluginEntry.mockReturnValue(pluginEntry)
     mockGetCachedVersion.mockReturnValue("3.4.0")
     mockGetLatestVersion.mockResolvedValue("3.5.0")
     mockExtractChannel.mockReturnValue("latest")
-    mockRunBunInstall.mockResolvedValue(true)
+    mockRunBunInstallWithDetails.mockResolvedValue({ success: true })
   })
 
   describe("#given no-op scenarios", () => {
@@ -129,13 +157,10 @@ describe("runBackgroundUpdateCheck", () => {
       //#when
       await runCheck(autoUpdate)
       //#then
-      expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(
+      expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage)
-        expect.objectContaining({ directory: "/test" }),
+      expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled()
-        "3.5.0",
-        expect.any(Function)
-      )
-      expect(mockRunBunInstall).not.toHaveBeenCalled()
       expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled()
+      expect(operationOrder).toEqual([])
     })
   })
 
@@ -147,7 +172,7 @@ describe("runBackgroundUpdateCheck", () => {
       await runCheck()
       //#then
       expect(mockShowUpdateAvailableToast).toHaveBeenCalledTimes(1)
-      expect(mockRunBunInstall).not.toHaveBeenCalled()
+      expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled()
       expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled()
     })
 
@@ -177,35 +202,33 @@ describe("runBackgroundUpdateCheck", () => {
   describe("#given unpinned with auto-update and install succeeds", () => {
     it("invalidates cache, installs, and shows auto-updated toast", async () => {
       //#given
-      mockRunBunInstall.mockResolvedValue(true)
+      mockRunBunInstallWithDetails.mockResolvedValue({ success: true })
       //#when
       await runCheck()
       //#then
+      expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledWith(pluginEntry)
       expect(mockInvalidatePackage).toHaveBeenCalledTimes(1)
-      expect(mockRunBunInstall).toHaveBeenCalledTimes(1)
+      expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1)
-      expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(
+      expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" })
-        expect.objectContaining({ directory: "/test" }),
+      expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(mockContext, "3.4.0", "3.5.0")
-        "3.4.0",
-        "3.5.0"
-      )
       expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled()
+      expect(operationOrder).toEqual(["sync", "invalidate"])
     })
   })
 
   describe("#given unpinned with auto-update and install fails", () => {
     it("falls back to notification-only toast", async () => {
       //#given
-      mockRunBunInstall.mockResolvedValue(false)
+      mockRunBunInstallWithDetails.mockResolvedValue({ success: false, error: "install failed" })
       //#when
       await runCheck()
       //#then
-      expect(mockRunBunInstall).toHaveBeenCalledTimes(1)
+      expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1)
-      expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(
+      expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" })
-        expect.objectContaining({ directory: "/test" }),
+      expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledWith(pluginEntry)
-        "3.5.0",
+      expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage)
-        expect.any(Function)
-      )
       expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled()
+      expect(operationOrder).toEqual(["sync", "invalidate"])
     })
   })
 })

src/hooks/auto-update-checker/hook/background-update-check.ts

@@ -1,5 +1,5 @@
 import type { PluginInput } from "@opencode-ai/plugin"
-import { runBunInstall } from "../../../cli/config-manager"
+import { runBunInstallWithDetails } from "../../../cli/config-manager"
 import { log } from "../../../shared/logger"
 import { invalidatePackage } from "../cache"
 import { PACKAGE_NAME } from "../constants"
@@ -13,7 +13,12 @@ function getPinnedVersionToastMessage(latestVersion: string): string {
 
 async function runBunInstallSafe(): Promise<boolean> {
   try {
-    return await runBunInstall()
+    const result = await runBunInstallWithDetails({ outputMode: "pipe" })
+    if (!result.success && result.error) {
+      log("[auto-update-checker] bun install failed:", result.error)
+    }
+
+    return result.success
   } catch (err) {
     const errorMessage = err instanceof Error ? err.message : String(err)
     log("[auto-update-checker] bun install error:", errorMessage)