sapient/oh-my-openagent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
fix: prevent terminal corruption during background bun install
This commit is contained in:
YeonGyu-Kim
2026-03-11 21:04:43 +09:00
committed by GitHub
parent 4b5c47172d de2b073fce
commit 3f364cc8df
4 changed files with 277 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

155
src/cli/config-manager/bun-install.test.ts
View File

@@ -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({
exited: Promise.resolve(0),
exitCode: 0,
kill: () => {},
} as ReturnType<typeof spawnHelpers.spawnWithWindowsHide>)
spawnWithWindowsHideSpy = spyOn(spawnHelpers, "spawnWithWindowsHide").mockReturnValue(createProc())
existsSyncSpy = spyOn(fs, "existsSync").mockReturnValue(true)
})
@@ -29,15 +52,119 @@ describe("runBunInstallWithDetails", () => {
existsSyncSpy.mockRestore()
})
it("runs bun install in the OpenCode cache directory", async () => {
const result = await runBunInstallWithDetails()
describe("#given the cache workspace exists", () => {
describe("#when bun install uses inherited output", () => {
it("#then runs bun install in the cache directory", async () => {
// given
expect(result).toEqual({ success: true })
expect(getOpenCodeCacheDirSpy).toHaveBeenCalledTimes(1)
expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], {
cwd: "/tmp/opencode-cache",
stdout: "inherit",
stderr: "inherit",
// when
const result = await runBunInstallWithDetails()
// then
expect(result).toEqual({ success: true })
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()
}
})
})
})
})

78
src/cli/config-manager/bun-install.ts
View File

@@ -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",
stderr: "inherit",
stdout: outputMode,
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}`,

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

@@ -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 mockRunBunInstall = mock(async () => true)
const operationOrder: string[] = []
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 getToastMessage: ToastMessageGetter = (isUpdate, version) =>
isUpdate ? `Update to ${version}` : "Up to date"
const mockContext = { directory: "/test" } as PluginInput
const getToastMessage: ToastMessageGetter = (isUpdate, version) =>
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.objectContaining({ directory: "/test" }),
"3.5.0",
expect.any(Function)
)
expect(mockRunBunInstall).not.toHaveBeenCalled()
expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage)
expect(mockRunBunInstallWithDetails).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(mockShowAutoUpdatedToast).toHaveBeenCalledWith(
expect.objectContaining({ directory: "/test" }),
"3.4.0",
"3.5.0"
)
expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1)
expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" })
expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(mockContext, "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(mockShowUpdateAvailableToast).toHaveBeenCalledWith(
expect.objectContaining({ directory: "/test" }),
"3.5.0",
expect.any(Function)
)
expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1)
expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" })
expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledWith(pluginEntry)
expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage)
expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled()
expect(operationOrder).toEqual(["sync", "invalidate"])
})
})
})

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

@@ -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)