diff --git a/src/hooks/auto-update-checker/checker.ts b/src/hooks/auto-update-checker/checker.ts index 0d3fd310d..e396ca771 100644 --- a/src/hooks/auto-update-checker/checker.ts +++ b/src/hooks/auto-update-checker/checker.ts @@ -3,7 +3,8 @@ export { getLocalDevVersion } from "./checker/local-dev-version" export { findPluginEntry } from "./checker/plugin-entry" export type { PluginEntryInfo } from "./checker/plugin-entry" export { getCachedVersion } from "./checker/cached-version" -export { updatePinnedVersion, revertPinnedVersion } from "./checker/pinned-version-updater" +export { updatePinnedVersion } from "./checker/pinned-version-updater" export { getLatestVersion } from "./checker/latest-version" export { checkForUpdate } from "./checker/check-for-update" export { syncCachePackageJsonToIntent } from "./checker/sync-package-json" +export type { SyncResult } from "./checker/sync-package-json" diff --git a/src/hooks/auto-update-checker/checker/plugin-entry.ts b/src/hooks/auto-update-checker/checker/plugin-entry.ts index 7aa79cc1e..f204d61f1 100644 --- a/src/hooks/auto-update-checker/checker/plugin-entry.ts +++ b/src/hooks/auto-update-checker/checker/plugin-entry.ts @@ -11,9 +11,7 @@ export interface PluginEntryInfo { configPath: string } -function isExplicitVersionPin(pinnedVersion: string): boolean { - return /^\d+\.\d+\.\d+/.test(pinnedVersion) -} +const EXACT_SEMVER_REGEX = /^\d+\.\d+\.\d+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$/ export function findPluginEntry(directory: string): PluginEntryInfo | null { for (const configPath of getConfigPaths(directory)) { @@ -29,7 +27,7 @@ export function findPluginEntry(directory: string): PluginEntryInfo | null { } if (entry.startsWith(`${PACKAGE_NAME}@`)) { const pinnedVersion = entry.slice(PACKAGE_NAME.length + 1) - const isPinned = isExplicitVersionPin(pinnedVersion) + const isPinned = EXACT_SEMVER_REGEX.test(pinnedVersion.trim()) return { entry, isPinned, pinnedVersion, configPath } } } diff --git a/src/hooks/auto-update-checker/checker/sync-package-json.test.ts b/src/hooks/auto-update-checker/checker/sync-package-json.test.ts index 99de183f1..c83774810 100644 --- a/src/hooks/auto-update-checker/checker/sync-package-json.test.ts +++ b/src/hooks/auto-update-checker/checker/sync-package-json.test.ts @@ -8,6 +8,14 @@ const TEST_CACHE_DIR = join(import.meta.dir, "__test-sync-cache__") mock.module("../constants", () => ({ CACHE_DIR: TEST_CACHE_DIR, PACKAGE_NAME: "oh-my-opencode", + NPM_REGISTRY_URL: "https://registry.npmjs.org/-/package/oh-my-opencode/dist-tags", + NPM_FETCH_TIMEOUT: 5000, + VERSION_FILE: join(TEST_CACHE_DIR, "version"), + USER_CONFIG_DIR: "/tmp/opencode-config", + USER_OPENCODE_CONFIG: "/tmp/opencode-config/opencode.json", + USER_OPENCODE_CONFIG_JSONC: "/tmp/opencode-config/opencode.jsonc", + INSTALLED_PACKAGE_JSON: join(TEST_CACHE_DIR, "node_modules", "oh-my-opencode", "package.json"), + getWindowsAppdataDir: () => null, })) mock.module("../../../shared/logger", () => ({ @@ -59,11 +67,10 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(true) + expect(result.synced).toBe(true) + expect(result.error).toBeNull() expect(readCachePackageJsonVersion()).toBe("latest") }) }) @@ -79,11 +86,10 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(true) + expect(result.synced).toBe(true) + expect(result.error).toBeNull() expect(readCachePackageJsonVersion()).toBe("next") }) }) @@ -99,19 +105,17 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(true) + expect(result.synced).toBe(true) + expect(result.error).toBeNull() expect(readCachePackageJsonVersion()).toBe("latest") }) }) }) describe("#given cache package.json already matches intent", () => { - it("#then returns false without modifying package.json", async () => { - //#given + it("#then returns synced false with no error", async () => { resetTestCache("latest") const { syncCachePackageJsonToIntent } = await import("./sync-package-json") @@ -122,18 +126,16 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(false) + expect(result.synced).toBe(false) + expect(result.error).toBeNull() expect(readCachePackageJsonVersion()).toBe("latest") }) }) describe("#given cache package.json does not exist", () => { - it("#then returns false", async () => { - //#given + it("#then returns file_not_found error", async () => { cleanupTestCache() const { syncCachePackageJsonToIntent } = await import("./sync-package-json") @@ -144,17 +146,15 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(false) + expect(result.synced).toBe(false) + expect(result.error).toBe("file_not_found") }) }) describe("#given plugin not in cache package.json dependencies", () => { - it("#then returns false", async () => { - //#given + it("#then returns plugin_not_in_deps error", async () => { cleanupTestCache() mkdirSync(TEST_CACHE_DIR, { recursive: true }) writeFileSync( @@ -171,17 +171,15 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(false) + expect(result.synced).toBe(false) + expect(result.error).toBe("plugin_not_in_deps") }) }) - describe("#given user explicitly pinned a different semver", () => { + describe("#given user explicitly changed from one semver to another", () => { it("#then updates package.json to new version", async () => { - //#given resetTestCache("3.9.0") const { syncCachePackageJsonToIntent } = await import("./sync-package-json") @@ -192,18 +190,16 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when const result = syncCachePackageJsonToIntent(pluginInfo) - //#then - expect(result).toBe(true) + expect(result.synced).toBe(true) + expect(result.error).toBeNull() expect(readCachePackageJsonVersion()).toBe("3.10.0") }) }) - describe("#given other dependencies exist in cache package.json", () => { - it("#then preserves other dependencies while updating the plugin", async () => { - //#given + describe("#given cache package.json with other dependencies", () => { + it("#then other dependencies are preserved when updating plugin version", async () => { const { syncCachePackageJsonToIntent } = await import("./sync-package-json") const pluginInfo: PluginEntryInfo = { @@ -213,14 +209,133 @@ describe("syncCachePackageJsonToIntent", () => { configPath: "/tmp/opencode.json", } - //#when - syncCachePackageJsonToIntent(pluginInfo) + const result = syncCachePackageJsonToIntent(pluginInfo) + + expect(result.synced).toBe(true) + expect(result.error).toBeNull() - //#then const content = readFileSync(join(TEST_CACHE_DIR, "package.json"), "utf-8") const pkg = JSON.parse(content) as { dependencies?: Record } - expect(pkg.dependencies?.other).toBe("1.0.0") - expect(pkg.dependencies?.["oh-my-opencode"]).toBe("latest") + expect(pkg.dependencies?.["other"]).toBe("1.0.0") + }) + }) + + describe("#given malformed JSON in cache package.json", () => { + it("#then returns parse_error", async () => { + cleanupTestCache() + mkdirSync(TEST_CACHE_DIR, { recursive: true }) + writeFileSync(join(TEST_CACHE_DIR, "package.json"), "{ invalid json }") + + const { syncCachePackageJsonToIntent } = await import("./sync-package-json") + + const pluginInfo: PluginEntryInfo = { + entry: "oh-my-opencode@latest", + isPinned: false, + pinnedVersion: "latest", + configPath: "/tmp/opencode.json", + } + + const result = syncCachePackageJsonToIntent(pluginInfo) + + expect(result.synced).toBe(false) + expect(result.error).toBe("parse_error") + }) + }) + + describe("#given write permission denied", () => { + it("#then returns write_error", async () => { + cleanupTestCache() + mkdirSync(TEST_CACHE_DIR, { recursive: true }) + writeFileSync( + join(TEST_CACHE_DIR, "package.json"), + JSON.stringify({ dependencies: { "oh-my-opencode": "3.10.0" } }, null, 2) + ) + + const fs = await import("node:fs") + const originalWriteFileSync = fs.writeFileSync + const originalRenameSync = fs.renameSync + + mock.module("node:fs", () => ({ + ...fs, + writeFileSync: mock(() => { + throw new Error("EACCES: permission denied") + }), + renameSync: fs.renameSync, + })) + + try { + const { syncCachePackageJsonToIntent } = await import("./sync-package-json") + + const pluginInfo: PluginEntryInfo = { + entry: "oh-my-opencode@latest", + isPinned: false, + pinnedVersion: "latest", + configPath: "/tmp/opencode.json", + } + + const result = syncCachePackageJsonToIntent(pluginInfo) + + expect(result.synced).toBe(false) + expect(result.error).toBe("write_error") + } finally { + mock.module("node:fs", () => ({ + ...fs, + writeFileSync: originalWriteFileSync, + renameSync: originalRenameSync, + })) + } + }) + }) + + describe("#given rename fails after successful write", () => { + it("#then returns write_error and cleans up temp file", async () => { + cleanupTestCache() + mkdirSync(TEST_CACHE_DIR, { recursive: true }) + writeFileSync( + join(TEST_CACHE_DIR, "package.json"), + JSON.stringify({ dependencies: { "oh-my-opencode": "3.10.0" } }, null, 2) + ) + + const fs = await import("node:fs") + const originalWriteFileSync = fs.writeFileSync + const originalRenameSync = fs.renameSync + + let tempFilePath: string | null = null + + mock.module("node:fs", () => ({ + ...fs, + writeFileSync: mock((path: string, data: string) => { + tempFilePath = path + return originalWriteFileSync(path, data) + }), + renameSync: mock(() => { + throw new Error("EXDEV: cross-device link not permitted") + }), + })) + + try { + const { syncCachePackageJsonToIntent } = await import("./sync-package-json") + + const pluginInfo: PluginEntryInfo = { + entry: "oh-my-opencode@latest", + isPinned: false, + pinnedVersion: "latest", + configPath: "/tmp/opencode.json", + } + + const result = syncCachePackageJsonToIntent(pluginInfo) + + expect(result.synced).toBe(false) + expect(result.error).toBe("write_error") + expect(tempFilePath).not.toBeNull() + expect(existsSync(tempFilePath!)).toBe(false) + } finally { + mock.module("node:fs", () => ({ + ...fs, + writeFileSync: originalWriteFileSync, + renameSync: originalRenameSync, + })) + } }) }) }) diff --git a/src/hooks/auto-update-checker/checker/sync-package-json.ts b/src/hooks/auto-update-checker/checker/sync-package-json.ts index 1e384b322..443cbc97e 100644 --- a/src/hooks/auto-update-checker/checker/sync-package-json.ts +++ b/src/hooks/auto-update-checker/checker/sync-package-json.ts @@ -1,3 +1,4 @@ +import * as crypto from "node:crypto" import * as fs from "node:fs" import * as path from "node:path" import { CACHE_DIR, PACKAGE_NAME } from "../constants" @@ -8,6 +9,22 @@ interface CachePackageJson { dependencies?: Record } +export interface SyncResult { + synced: boolean + error: "file_not_found" | "plugin_not_in_deps" | "parse_error" | "write_error" | null + message?: string +} + +const EXACT_SEMVER_REGEX = /^\d+\.\d+\.\d+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$/ + +function safeUnlink(filePath: string): void { + try { + fs.unlinkSync(filePath) + } catch (err) { + log(`[auto-update-checker] Failed to cleanup temp file: ${filePath}`, err) + } +} + function getIntentVersion(pluginInfo: PluginEntryInfo): string { if (!pluginInfo.pinnedVersion) { return "latest" @@ -15,49 +32,67 @@ function getIntentVersion(pluginInfo: PluginEntryInfo): string { return pluginInfo.pinnedVersion } -/** - * Sync cache package.json to match opencode.json plugin intent before bun install. - * - * OpenCode pins resolved versions in cache package.json (e.g., "3.11.0" instead of "latest"). - * When auto-update detects a newer version and runs `bun install`, it re-resolves the pinned - * version instead of the user's declared tag, causing updates to silently fail. - * - * @returns true if package.json was updated, false otherwise - */ -export function syncCachePackageJsonToIntent(pluginInfo: PluginEntryInfo): boolean { +export function syncCachePackageJsonToIntent(pluginInfo: PluginEntryInfo): SyncResult { const cachePackageJsonPath = path.join(CACHE_DIR, "package.json") if (!fs.existsSync(cachePackageJsonPath)) { log("[auto-update-checker] Cache package.json not found, nothing to sync") - return false + return { synced: false, error: "file_not_found", message: "Cache package.json not found" } + } + + let content: string + let pkgJson: CachePackageJson + + try { + content = fs.readFileSync(cachePackageJsonPath, "utf-8") + } catch (err) { + log("[auto-update-checker] Failed to read cache package.json:", err) + return { synced: false, error: "parse_error", message: "Failed to read cache package.json" } } try { - const content = fs.readFileSync(cachePackageJsonPath, "utf-8") - const pkgJson = JSON.parse(content) as CachePackageJson - - if (!pkgJson.dependencies?.[PACKAGE_NAME]) { - log("[auto-update-checker] Plugin not in cache package.json dependencies, nothing to sync") - return false - } - - const currentVersion = pkgJson.dependencies[PACKAGE_NAME] - const intentVersion = getIntentVersion(pluginInfo) - - if (currentVersion === intentVersion) { - log("[auto-update-checker] Cache package.json already matches intent:", intentVersion) - return false - } - - log( - `[auto-update-checker] Syncing cache package.json: "${currentVersion}" → "${intentVersion}"` - ) - - pkgJson.dependencies[PACKAGE_NAME] = intentVersion - fs.writeFileSync(cachePackageJsonPath, JSON.stringify(pkgJson, null, 2)) - return true + pkgJson = JSON.parse(content) as CachePackageJson } catch (err) { - log("[auto-update-checker] Failed to sync cache package.json:", err) - return false + log("[auto-update-checker] Failed to parse cache package.json:", err) + return { synced: false, error: "parse_error", message: "Failed to parse cache package.json (malformed JSON)" } + } + + if (!pkgJson || !pkgJson.dependencies?.[PACKAGE_NAME]) { + log("[auto-update-checker] Plugin not in cache package.json dependencies, nothing to sync") + return { synced: false, error: "plugin_not_in_deps", message: "Plugin not in cache package.json dependencies" } + } + + const currentVersion = pkgJson.dependencies[PACKAGE_NAME] + const intentVersion = getIntentVersion(pluginInfo) + + if (currentVersion === intentVersion) { + log("[auto-update-checker] Cache package.json already matches intent:", intentVersion) + return { synced: false, error: null, message: `Already matches intent: ${intentVersion}` } + } + + const intentIsTag = !EXACT_SEMVER_REGEX.test(intentVersion.trim()) + const currentIsSemver = EXACT_SEMVER_REGEX.test(String(currentVersion).trim()) + + if (intentIsTag && currentIsSemver) { + log( + `[auto-update-checker] Syncing cache package.json: "${currentVersion}" → "${intentVersion}" (opencode.json intent)` + ) + } else { + log( + `[auto-update-checker] Updating cache package.json: "${currentVersion}" → "${intentVersion}"` + ) + } + + pkgJson.dependencies[PACKAGE_NAME] = intentVersion + + const tmpPath = `${cachePackageJsonPath}.${crypto.randomUUID()}` + try { + fs.writeFileSync(tmpPath, JSON.stringify(pkgJson, null, 2)) + fs.renameSync(tmpPath, cachePackageJsonPath) + return { synced: true, error: null, message: `Updated: "${currentVersion}" → "${intentVersion}"` } + } catch (err) { + log("[auto-update-checker] Failed to write cache package.json:", err) + safeUnlink(tmpPath) + return { synced: false, error: "write_error", message: "Failed to write cache 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 b229d3547..7ad291a81 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 @@ -1,12 +1,6 @@ -/// - -import type { BunInstallResult } from "../../../cli/config-manager" +import type { PluginInput } from "@opencode-ai/plugin" import { beforeEach, describe, expect, it, mock } from "bun:test" -type PluginInput = { - directory: string -} - type PluginEntry = { entry: string isPinned: boolean @@ -30,14 +24,8 @@ const mockFindPluginEntry = mock((_directory: string): PluginEntry | null => cre const mockGetCachedVersion = mock((): string | null => "3.4.0") const mockGetLatestVersion = mock(async (): Promise => "3.5.0") const mockExtractChannel = mock(() => "latest") -const operationOrder: string[] = [] -const mockSyncCachePackageJsonToIntent = mock((_pluginEntry: PluginEntry) => { - operationOrder.push("sync") -}) -const mockInvalidatePackage = mock((_packageName: string) => { - operationOrder.push("invalidate") -}) -const mockRunBunInstallWithDetails = mock(async (): Promise => ({ success: true })) +const mockInvalidatePackage = mock(() => {}) +const mockRunBunInstall = mock(async () => true) const mockShowUpdateAvailableToast = mock( async (_ctx: PluginInput, _latestVersion: string, _getToastMessage: ToastMessageGetter): Promise => {} ) @@ -45,6 +33,8 @@ const mockShowAutoUpdatedToast = mock( async (_ctx: PluginInput, _fromVersion: string, _toVersion: string): Promise => {} ) +const mockSyncCachePackageJsonToIntent = mock(() => false) + mock.module("../checker", () => ({ findPluginEntry: mockFindPluginEntry, getCachedVersion: mockGetCachedVersion, @@ -54,7 +44,7 @@ mock.module("../checker", () => ({ })) mock.module("../version-channel", () => ({ extractChannel: mockExtractChannel })) mock.module("../cache", () => ({ invalidatePackage: mockInvalidatePackage })) -mock.module("../../../cli/config-manager", () => ({ runBunInstallWithDetails: mockRunBunInstallWithDetails })) +mock.module("../../../cli/config-manager", () => ({ runBunInstall: mockRunBunInstall })) mock.module("./update-toasts", () => ({ showUpdateAvailableToast: mockShowUpdateAvailableToast, showAutoUpdatedToast: mockShowAutoUpdatedToast, @@ -64,89 +54,85 @@ mock.module("../../../shared/logger", () => ({ log: () => {} })) const modulePath = "./background-update-check?test" const { runBackgroundUpdateCheck } = await import(modulePath) -const mockContext = { directory: "/test" } as PluginInput -const getToastMessage: ToastMessageGetter = (isUpdate, version) => - isUpdate ? `Update to ${version}` : "Up to date" - -async function runCheck(autoUpdate = true): Promise { - await runBackgroundUpdateCheck(mockContext, autoUpdate, getToastMessage) -} - -function expectNoUpdateEffects(): void { - expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() - expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() - expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() - expect(mockSyncCachePackageJsonToIntent).not.toHaveBeenCalled() - expect(mockInvalidatePackage).not.toHaveBeenCalled() -} - describe("runBackgroundUpdateCheck", () => { - let pluginEntry: PluginEntry + const mockCtx = { directory: "/test" } as PluginInput + const getToastMessage: ToastMessageGetter = (isUpdate, version) => + isUpdate ? `Update to ${version}` : "Up to date" beforeEach(() => { mockFindPluginEntry.mockReset() mockGetCachedVersion.mockReset() mockGetLatestVersion.mockReset() mockExtractChannel.mockReset() - mockSyncCachePackageJsonToIntent.mockReset() mockInvalidatePackage.mockReset() - mockRunBunInstallWithDetails.mockReset() + mockRunBunInstall.mockReset() mockShowUpdateAvailableToast.mockReset() mockShowAutoUpdatedToast.mockReset() + mockSyncCachePackageJsonToIntent.mockReset() - operationOrder.length = 0 - - mockSyncCachePackageJsonToIntent.mockImplementation((_pluginEntry: PluginEntry) => { - operationOrder.push("sync") - }) - mockInvalidatePackage.mockImplementation((_packageName: string) => { - operationOrder.push("invalidate") - }) - - pluginEntry = createPluginEntry() - mockFindPluginEntry.mockReturnValue(pluginEntry) + mockFindPluginEntry.mockReturnValue(createPluginEntry()) mockGetCachedVersion.mockReturnValue("3.4.0") mockGetLatestVersion.mockResolvedValue("3.5.0") mockExtractChannel.mockReturnValue("latest") - mockRunBunInstallWithDetails.mockResolvedValue({ success: true }) + mockRunBunInstall.mockResolvedValue(true) + mockSyncCachePackageJsonToIntent.mockReturnValue({ synced: true, error: null }) }) - describe("#given no-op scenarios", () => { - it.each([ - { - name: "plugin entry is missing", - setup: () => { - mockFindPluginEntry.mockReturnValue(null) - }, - }, - { - name: "no cached or pinned version exists", - setup: () => { - mockFindPluginEntry.mockReturnValue(createPluginEntry({ entry: "oh-my-opencode" })) - mockGetCachedVersion.mockReturnValue(null) - }, - }, - { - name: "latest version lookup fails", - setup: () => { - mockGetLatestVersion.mockResolvedValue(null) - }, - }, - { - name: "current version is already latest", - setup: () => { - mockGetLatestVersion.mockResolvedValue("3.4.0") - }, - }, - ])("returns without user-visible update effects when $name", async ({ setup }) => { + describe("#given no plugin entry found", () => { + it("returns early without showing any toast", async () => { //#given - setup() - + mockFindPluginEntry.mockReturnValue(null) //#when - await runCheck() - + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then - expectNoUpdateEffects() + expect(mockFindPluginEntry).toHaveBeenCalledTimes(1) + expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() + }) + }) + + describe("#given no version available", () => { + it("returns early when neither cached nor pinned version exists", async () => { + //#given + mockFindPluginEntry.mockReturnValue(createPluginEntry({ entry: "oh-my-opencode" })) + mockGetCachedVersion.mockReturnValue(null) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockGetCachedVersion).toHaveBeenCalledTimes(1) + expect(mockGetLatestVersion).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given latest version fetch fails", () => { + it("returns early without toasts", async () => { + //#given + mockGetLatestVersion.mockResolvedValue(null) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockGetLatestVersion).toHaveBeenCalledWith("latest") + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given already on latest version", () => { + it("returns early without any action", async () => { + //#given + mockGetCachedVersion.mockReturnValue("3.4.0") + mockGetLatestVersion.mockResolvedValue("3.4.0") + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockGetLatestVersion).toHaveBeenCalledTimes(1) + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) }) @@ -155,12 +141,11 @@ describe("runBackgroundUpdateCheck", () => { //#given const autoUpdate = false //#when - await runCheck(autoUpdate) + await runBackgroundUpdateCheck(mockCtx, autoUpdate, getToastMessage) //#then - expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage) - expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) + expect(mockRunBunInstall).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() - expect(operationOrder).toEqual([]) }) }) @@ -169,10 +154,10 @@ describe("runBackgroundUpdateCheck", () => { //#given mockFindPluginEntry.mockReturnValue(createPluginEntry({ isPinned: true, pinnedVersion: "3.4.0" })) //#when - await runCheck() + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockShowUpdateAvailableToast).toHaveBeenCalledTimes(1) - expect(mockRunBunInstallWithDetails).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() }) @@ -186,7 +171,7 @@ describe("runBackgroundUpdateCheck", () => { } ) //#when - await runCheck() + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then expect(mockShowUpdateAvailableToast).toHaveBeenCalledTimes(1) expect(capturedToastMessage).toBeDefined() @@ -200,35 +185,126 @@ describe("runBackgroundUpdateCheck", () => { }) describe("#given unpinned with auto-update and install succeeds", () => { - it("invalidates cache, installs, and shows auto-updated toast", async () => { + it("syncs cache, invalidates, installs, and shows auto-updated toast", async () => { //#given - mockRunBunInstallWithDetails.mockResolvedValue({ success: true }) + mockRunBunInstall.mockResolvedValue(true) //#when - await runCheck() + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then - expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledWith(pluginEntry) + expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) expect(mockInvalidatePackage).toHaveBeenCalledTimes(1) - expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1) - expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" }) - expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(mockContext, "3.4.0", "3.5.0") + expect(mockRunBunInstall).toHaveBeenCalledTimes(1) + expect(mockShowAutoUpdatedToast).toHaveBeenCalledWith(mockCtx, "3.4.0", "3.5.0") expect(mockShowUpdateAvailableToast).not.toHaveBeenCalled() - expect(operationOrder).toEqual(["sync", "invalidate"]) + }) + + it("syncs before invalidate and install (correct order)", async () => { + //#given + const callOrder: string[] = [] + mockSyncCachePackageJsonToIntent.mockImplementation(() => { + callOrder.push("sync") + return { synced: true, error: null } + }) + mockInvalidatePackage.mockImplementation(() => { + callOrder.push("invalidate") + }) + mockRunBunInstall.mockImplementation(async () => { + callOrder.push("install") + return true + }) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(callOrder).toEqual(["sync", "invalidate", "install"]) }) }) describe("#given unpinned with auto-update and install fails", () => { it("falls back to notification-only toast", async () => { //#given - mockRunBunInstallWithDetails.mockResolvedValue({ success: false, error: "install failed" }) + mockRunBunInstall.mockResolvedValue(false) //#when - await runCheck() + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) //#then - expect(mockRunBunInstallWithDetails).toHaveBeenCalledTimes(1) - expect(mockRunBunInstallWithDetails).toHaveBeenCalledWith({ outputMode: "pipe" }) - expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledWith(pluginEntry) - expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockContext, "3.5.0", getToastMessage) + expect(mockRunBunInstall).toHaveBeenCalledTimes(1) + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given sync fails with file_not_found", () => { + it("aborts update and shows notification-only toast", async () => { + //#given + mockSyncCachePackageJsonToIntent.mockReturnValue({ + synced: false, + error: "file_not_found", + message: "Cache package.json not found", + }) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) + expect(mockInvalidatePackage).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given sync fails with plugin_not_in_deps", () => { + it("aborts update and shows notification-only toast", async () => { + //#given + mockSyncCachePackageJsonToIntent.mockReturnValue({ + synced: false, + error: "plugin_not_in_deps", + message: "Plugin not in cache package.json dependencies", + }) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) + expect(mockInvalidatePackage).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given sync fails with parse_error", () => { + it("aborts update and shows notification-only toast", async () => { + //#given + mockSyncCachePackageJsonToIntent.mockReturnValue({ + synced: false, + error: "parse_error", + message: "Failed to parse cache package.json (malformed JSON)", + }) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) + expect(mockInvalidatePackage).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) + expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() + }) + }) + + describe("#given sync fails with write_error", () => { + it("aborts update and shows notification-only toast", async () => { + //#given + mockSyncCachePackageJsonToIntent.mockReturnValue({ + synced: false, + error: "write_error", + message: "Failed to write cache package.json", + }) + //#when + await runBackgroundUpdateCheck(mockCtx, true, getToastMessage) + //#then + expect(mockSyncCachePackageJsonToIntent).toHaveBeenCalledTimes(1) + expect(mockInvalidatePackage).not.toHaveBeenCalled() + expect(mockRunBunInstall).not.toHaveBeenCalled() + expect(mockShowUpdateAvailableToast).toHaveBeenCalledWith(mockCtx, "3.5.0", getToastMessage) expect(mockShowAutoUpdatedToast).not.toHaveBeenCalled() - expect(operationOrder).toEqual(["sync", "invalidate"]) }) }) }) diff --git a/src/hooks/auto-update-checker/hook/background-update-check.ts b/src/hooks/auto-update-checker/hook/background-update-check.ts index 4b40a7c04..0875c23a5 100644 --- a/src/hooks/auto-update-checker/hook/background-update-check.ts +++ b/src/hooks/auto-update-checker/hook/background-update-check.ts @@ -4,7 +4,7 @@ import { log } from "../../../shared/logger" import { invalidatePackage } from "../cache" import { PACKAGE_NAME } from "../constants" import { extractChannel } from "../version-channel" -import { findPluginEntry, getCachedVersion, getLatestVersion, revertPinnedVersion, syncCachePackageJsonToIntent } from "../checker" +import { findPluginEntry, getCachedVersion, getLatestVersion, syncCachePackageJsonToIntent } from "../checker" import { showAutoUpdatedToast, showUpdateAvailableToast } from "./update-toasts" function getPinnedVersionToastMessage(latestVersion: string): string { @@ -15,9 +15,8 @@ async function runBunInstallSafe(): Promise { try { const result = await runBunInstallWithDetails({ outputMode: "pipe" }) if (!result.success && result.error) { - log("[auto-update-checker] bun install failed:", result.error) + log("[auto-update-checker] bun install error:", result.error) } - return result.success } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err) @@ -70,7 +69,17 @@ export async function runBackgroundUpdateCheck( return } - syncCachePackageJsonToIntent(pluginInfo) + // Sync cache package.json to match opencode.json intent before updating + // This handles the case where user switched from pinned version to tag (e.g., 3.10.0 -> @latest) + const syncResult = syncCachePackageJsonToIntent(pluginInfo) + + // Abort on ANY sync error to prevent corrupting a bad state further + if (syncResult.error) { + log(`[auto-update-checker] Sync failed with error: ${syncResult.error}`, syncResult.message) + await showUpdateAvailableToast(ctx, latestVersion, getToastMessage) + return + } + invalidatePackage(PACKAGE_NAME) const installSuccess = await runBunInstallSafe() @@ -81,11 +90,6 @@ export async function runBackgroundUpdateCheck( return } - if (pluginInfo.isPinned) { - revertPinnedVersion(pluginInfo.configPath, latestVersion, pluginInfo.entry) - log("[auto-update-checker] Config reverted due to install failure") - } - await showUpdateAvailableToast(ctx, latestVersion, getToastMessage) log("[auto-update-checker] bun install failed; update not installed (falling back to notification-only)") }