diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a8500e74e..4a5808901 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -34,6 +34,9 @@ ### Fixed - Fixed high CPU during multi-subagent / workflowz / orchestrate sessions: each live tool block (streaming args, a running partial tool, or a `task` subagent) armed its own 80ms spinner `setInterval` driving `requestComponentRender`, so N concurrent live blocks created N unsynchronized repaint timers that kept the render scheduler awake near-continuously. The per-block timers are now consolidated into a single shared spinner ticker that repaints every live block in one coalesced frame per glyph step, independent of block count ([#8731](https://github.com/can1357/oh-my-pi/issues/8731)). +### Fixed + +- Fixed `omp update` writing to the PATH launcher instead of the running binary on binary-only releases (major bumps or `omp.dist: "binary"`): a foreign symlink — e.g. an admin symlink into a shared install — now resolves to its real binary in every distribution channel, avoiding an `EACCES` on a root-owned link directory or a split-brain copy that shadows the shared install. Package-manager launchers keep their deliberate in-place takeover. ([#8732](https://github.com/can1357/oh-my-pi/issues/8732)) ## [17.3.5] - 2026-08-16 diff --git a/packages/coding-agent/src/cli/update-cli.ts b/packages/coding-agent/src/cli/update-cli.ts index 081707fc6..c13200edf 100644 --- a/packages/coding-agent/src/cli/update-cli.ts +++ b/packages/coding-agent/src/cli/update-cli.ts @@ -441,6 +441,14 @@ function tryRealpath(p: string): string | undefined { } } +function isSymlinkPath(p: string): boolean { + try { + return fs.lstatSync(p).isSymbolicLink(); + } catch { + return false; + } +} + function isPathInDirectoryLexical(filePath: string, directoryPath: string): boolean { const normalizedPath = normalizePathForComparison(path.resolve(filePath)); const normalizedDirectory = normalizePathForComparison(path.resolve(directoryPath)); @@ -509,6 +517,14 @@ interface UpdateMethodResolutionOptions { * preserves a global package symlink instead of resolving into its checkout. */ ompLinkTarget?: string; + /** + * Whether package-manager routing (bun/npm) is permitted. Binary-only + * releases pass `false`: a manager launcher then resolves to `"binary"` and + * is taken over in place rather than reinstalled through its manager. Defaults + * to `true` in {@link resolveUpdateMethod} so callers that only classify need + * not set it. + */ + allowPackageManagers?: boolean; } type UpdateTarget = @@ -525,6 +541,7 @@ function resolveUpdateMethod( options: UpdateMethodResolutionOptions = {}, ): UpdateMethod { const { + allowPackageManagers = true, bunGlobalDir, homebrewPrefix, miseBinDirs = [], @@ -555,6 +572,7 @@ function resolveUpdateMethod( globalBinDir: bunBinDir, }); if ( + allowPackageManagers && bunBinDir && isPathInDirectory(ompPath, bunBinDir) && !isStandaloneRegularFile && @@ -564,6 +582,7 @@ function resolveUpdateMethod( } const npmNodeModulesDir = resolveNpmGlobalNodeModulesDir(npmBinDir); if ( + allowPackageManagers && npmBinDir && isPathInDirectory(ompPath, npmBinDir) && !isStandaloneRegularFile && @@ -611,10 +630,25 @@ export function resolveUpdateTargetFromPath( ompLinkTarget, }); if (method === "binary") { - // A package-manager-enabled update follows a foreign alias to replace - // its standalone binary. Binary-only releases intentionally replace the - // selected manager launcher in place. - const binaryPath = options.allowPackageManagers && ompIsSymlink ? (ompRealpath ?? ompPath) : ompPath; + // A symlinked launcher created by bun/npm is taken over in place on a + // binary-only release: routing through the manager is impossible, so the + // standalone binary replaces the launcher and keeps the PATH entry live. + // Every other symlink — a foreign alias, or an admin symlink into a + // shared install — is self-healing: update the real binary it resolves + // to and leave the launcher untouched, in every distribution channel. + // The old channel gate clobbered these foreign launchers on binary-only + // releases (EACCES on a root-owned link dir, or a stale split-brain copy + // of the binary shadowing the shared install). + const managerLauncher = + ompIsSymlink && + !options.allowPackageManagers && + resolveUpdateMethod(ompPath, bunBinDir, { + ...options, + allowPackageManagers: true, + ompIsRegularFile, + ompLinkTarget, + }) !== "binary"; + const binaryPath = ompIsSymlink && !managerLauncher ? (ompRealpath ?? ompPath) : ompPath; return { method, path: binaryPath, replacesSymlink: ompIsSymlink && binaryPath === ompPath }; } if (method === "bun" || method === "npm") return { method, path: ompPath }; @@ -630,18 +664,25 @@ export function resolveUpdateTargetFromPath( * binaries and stay valid regardless of how the release is distributed. */ async function resolveUpdateTarget(options: { allowPackageManagers: boolean }): Promise { - const bunBinDir = options.allowPackageManagers ? await getBunGlobalBinDir() : undefined; - const npmBinDir = options.allowPackageManagers ? await getNpmGlobalBinDir() : undefined; const homebrewPrefix = await getHomebrewFormulaPrefix(); const miseAvailable = $which("mise") !== undefined; const miseBinDirs = miseAvailable ? await getMiseBinDirs() : []; const miseDataDir = miseAvailable ? getMiseDataDir() : undefined; const ompPath = resolveOmpPath(); + // Binary-only releases skip package-manager routing, but a symlinked + // launcher still needs the manager bin dirs to tell a bun/npm launcher + // (taken over in place) from a foreign symlink (resolved to its real + // binary). A plain-file install never needs the distinction, so the common + // case stays probe-free. + const probeManagers = options.allowPackageManagers || (ompPath !== undefined && isSymlinkPath(ompPath)); + const bunBinDir = probeManagers ? await getBunGlobalBinDir() : undefined; + const npmBinDir = probeManagers ? await getNpmGlobalBinDir() : undefined; + if (ompPath) { return resolveUpdateTargetFromPath(ompPath, bunBinDir, { allowPackageManagers: options.allowPackageManagers, - bunGlobalDir: options.allowPackageManagers ? process.env.BUN_INSTALL_GLOBAL_DIR : undefined, + bunGlobalDir: probeManagers ? process.env.BUN_INSTALL_GLOBAL_DIR : undefined, homebrewPrefix, miseBinDirs, miseDataDir, diff --git a/packages/coding-agent/test/update-cli.test.ts b/packages/coding-agent/test/update-cli.test.ts index 4e0e5d70e..a479fd9f2 100644 --- a/packages/coding-agent/test/update-cli.test.ts +++ b/packages/coding-agent/test/update-cli.test.ts @@ -247,6 +247,52 @@ describe("update-cli install target detection", () => { expect(target).toEqual({ method: "binary", path: standalonePath, replacesSymlink: false }); }); + it("resolves a foreign symlink to its real binary on a binary-only release instead of clobbering the launcher", async () => { + // Admin shared-install layout: a non-manager symlink in PATH points into + // a shared install dir. On a binary-only release the target must still be + // the resolved binary, not the launcher — otherwise the update writes + // beside a root-owned symlink (EACCES) or replaces it with a split-brain + // copy that shadows the shared install (#8732). + const dir = await makeTempDir(); + const sharedBinDir = path.join(dir, "opt", "omp", "bin"); + const standalonePath = path.join(sharedBinDir, "omp"); + const launcherDir = path.join(dir, "usr", "local", "bin"); + const launcherPath = path.join(launcherDir, "omp"); + await fs.mkdir(sharedBinDir, { recursive: true }); + await fs.mkdir(launcherDir, { recursive: true }); + await Bun.write(standalonePath, "binary"); + await fs.symlink(standalonePath, launcherPath); + + const target = resolveUpdateTargetFromPath(launcherPath, undefined, { + allowPackageManagers: false, + }); + + expect(target).toEqual({ method: "binary", path: standalonePath, replacesSymlink: false }); + expect(await fs.readlink(launcherPath)).toBe(standalonePath); + }); + + it("takes over a package-manager launcher in place on a binary-only release", async () => { + // A bun/npm-managed launcher symlinks into the manager's node_modules. + // A forced binary release cannot route through the manager, so the + // launcher is deliberately replaced in place, keeping the PATH entry live. + const dir = await makeTempDir(); + const npmPrefix = path.join(dir, ".npm-global"); + const npmBinDir = path.join(npmPrefix, "bin"); + const managedBinary = path.join(npmPrefix, "lib", "node_modules", "@oh-my-pi", "pi-coding-agent", "omp"); + const aliasPath = path.join(npmBinDir, "omp"); + await fs.mkdir(npmBinDir, { recursive: true }); + await fs.mkdir(path.dirname(managedBinary), { recursive: true }); + await Bun.write(managedBinary, "binary"); + await fs.symlink(managedBinary, aliasPath); + + const target = resolveUpdateTargetFromPath(aliasPath, undefined, { + allowPackageManagers: false, + npmBinDir, + }); + + expect(target).toEqual({ method: "binary", path: aliasPath, replacesSymlink: true }); + }); + it("keeps a split-root Bun-linked checkout under Bun management instead of overwriting its script", async () => { const dir = await makeTempDir(); const bunBinDir = path.join(dir, "bun-bin");