From ca7c0e391e760c24b0803fa3d250dd8df7e62b57 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 12 Mar 2026 17:28:51 +0900 Subject: [PATCH] fix(bun-install): default outputMode to "pipe" to prevent TUI stdout leak runBunInstallWithDetails() defaulted to outputMode:"inherit", causing bun install stdout/stderr to leak into the TUI when callers omitted the option. Changed default to "pipe" so output is captured silently. Also fixed stale mock in background-update-check.test.ts: the test was mocking runBunInstall (unused) instead of runBunInstallWithDetails, and returning boolean instead of BunInstallResult. --- src/cli/config-manager/bun-install.test.ts | 25 ++++++++++-- src/cli/config-manager/bun-install.ts | 2 +- .../hook/background-update-check.test.ts | 38 +++++++++---------- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/cli/config-manager/bun-install.test.ts b/src/cli/config-manager/bun-install.test.ts index 02fd2174f..4f739612b 100644 --- a/src/cli/config-manager/bun-install.test.ts +++ b/src/cli/config-manager/bun-install.test.ts @@ -53,8 +53,8 @@ describe("runBunInstallWithDetails", () => { }) describe("#given the cache workspace exists", () => { - describe("#when bun install uses inherited output", () => { - it("#then runs bun install in the cache directory", async () => { + describe("#when bun install uses default piped output", () => { + it("#then pipes stdout and stderr by default", async () => { // given // when @@ -65,8 +65,8 @@ describe("runBunInstallWithDetails", () => { expect(getOpenCodeCacheDirSpy).toHaveBeenCalledTimes(1) expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], { cwd: "/tmp/opencode-cache", - stdout: "inherit", - stderr: "inherit", + stdout: "pipe", + stderr: "pipe", }) }) }) @@ -88,6 +88,23 @@ describe("runBunInstallWithDetails", () => { }) }) + describe("#when bun install uses explicit inherited output", () => { + it("#then passes inherit mode to the spawned process", async () => { + // given + + // when + const result = await runBunInstallWithDetails({ outputMode: "inherit" }) + + // then + expect(result).toEqual({ success: true }) + expect(spawnWithWindowsHideSpy).toHaveBeenCalledWith(["bun", "install"], { + cwd: "/tmp/opencode-cache", + stdout: "inherit", + stderr: "inherit", + }) + }) + }) + describe("#when piped bun install fails", () => { it("#then logs captured stdout and stderr", async () => { // given diff --git a/src/cli/config-manager/bun-install.ts b/src/cli/config-manager/bun-install.ts index ebf4846dd..32b9d033f 100644 --- a/src/cli/config-manager/bun-install.ts +++ b/src/cli/config-manager/bun-install.ts @@ -64,7 +64,7 @@ function logCapturedOutputOnFailure(outputMode: BunInstallOutputMode, output: Bu } export async function runBunInstallWithDetails(options?: RunBunInstallOptions): Promise { - const outputMode = options?.outputMode ?? "inherit" + const outputMode = options?.outputMode ?? "pipe" const cacheDir = getOpenCodeCacheDir() const packageJsonPath = `${cacheDir}/package.json` diff --git a/src/hooks/auto-update-checker/hook/background-update-check.test.ts b/src/hooks/auto-update-checker/hook/background-update-check.test.ts index 7ad291a81..1033d7854 100644 --- a/src/hooks/auto-update-checker/hook/background-update-check.test.ts +++ b/src/hooks/auto-update-checker/hook/background-update-check.test.ts @@ -25,7 +25,7 @@ const mockGetCachedVersion = mock((): string | null => "3.4.0") const mockGetLatestVersion = mock(async (): Promise => "3.5.0") const mockExtractChannel = mock(() => "latest") const mockInvalidatePackage = mock(() => {}) -const mockRunBunInstall = mock(async () => true) +const mockRunBunInstallWithDetails = mock(async () => ({ success: true })) const mockShowUpdateAvailableToast = mock( async (_ctx: PluginInput, _latestVersion: string, _getToastMessage: ToastMessageGetter): Promise => {} ) @@ -44,7 +44,7 @@ mock.module("../checker", () => ({ })) 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, @@ -65,7 +65,7 @@ describe("runBackgroundUpdateCheck", () => { mockGetLatestVersion.mockReset() mockExtractChannel.mockReset() mockInvalidatePackage.mockReset() - mockRunBunInstall.mockReset() + mockRunBunInstallWithDetails.mockReset() mockShowUpdateAvailableToast.mockReset() mockShowAutoUpdatedToast.mockReset() mockSyncCachePackageJsonToIntent.mockReset() @@ -74,7 +74,7 @@ describe("runBackgroundUpdateCheck", () => { mockGetCachedVersion.mockReturnValue("3.4.0") mockGetLatestVersion.mockResolvedValue("3.5.0") mockExtractChannel.mockReturnValue("latest") - mockRunBunInstall.mockResolvedValue(true) + mockRunBunInstallWithDetails.mockResolvedValue({ success: true }) mockSyncCachePackageJsonToIntent.mockReturnValue({ synced: true, error: null }) }) @@ -88,7 +88,7 @@ describe("runBackgroundUpdateCheck", () => { expect(mockFindPluginEntry).toHaveBeenCalledTimes(1) expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() }) }) @@ -115,7 +115,7 @@ describe("runBackgroundUpdateCheck", () => { await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockGetLatestVersion).toHaveBeenCalledWith("latest") - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -130,7 +130,7 @@ describe("runBackgroundUpdateCheck", () => { await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockGetLatestVersion).toHaveBeenCalledTimes(1) - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -144,7 +144,7 @@ describe("runBackgroundUpdateCheck", () => { await runBackgroundUpdateCheck(mockCtx, autoUpdate, getToastMessage) //#then expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) }) @@ -157,7 +157,7 @@ describe("runBackgroundUpdateCheck", () => { await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockShowUpdateAvailableToast).toHaveBeenCalledTimes(1) - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -187,13 +187,13 @@ describe("runBackgroundUpdateCheck", () => { describe("#given unpinned with auto-update and install succeeds", () => { it("syncs cache, invalidates, installs, and shows auto-updated toast", async () => { //#given - mockRunBunInstall.mockResolvedValue(true) + mockRunBunInstallWithDetails.mockResolvedValue({ success: true }) //#when await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).toHaveBeenCalledTimes(1) - expect(mockRunBunInstall).toHaveBeenCalledTimes(1) + expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1) expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(mockCtx, "3.4.0", "3.5.0") expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() }) @@ -208,9 +208,9 @@ describe("runBackgroundUpdateCheck", () => { mockInvalidatePackage.mockImplementation(() => { callOrder.push("invalidate") }) - mockRunBunInstall.mockImplementation(async () => { + mockRunBunInstallWithDetails.mockImplementation(async () => { callOrder.push("install") - return true + return { success: true } }) //#when await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) @@ -222,11 +222,11 @@ describe("runBackgroundUpdateCheck", () => { 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 }) //#when await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then - expect(mockRunBunInstall).toHaveBeenCalledTimes(1) + expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1) expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -245,7 +245,7 @@ describe("runBackgroundUpdateCheck", () => { //#then expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).not.toHaveBeenCalled() - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -264,7 +264,7 @@ describe("runBackgroundUpdateCheck", () => { //#then expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).not.toHaveBeenCalled() - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -283,7 +283,7 @@ describe("runBackgroundUpdateCheck", () => { //#then expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).not.toHaveBeenCalled() - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -302,7 +302,7 @@ describe("runBackgroundUpdateCheck", () => { //#then expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).not.toHaveBeenCalled() - expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() })