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

Merge PR #8734: fix(coding-agent): resolve update target from the running binary, not the PATH launcher (@roboomp)

Browse Source
This commit is contained in:
can1357
2026-08-19 01:36:04 +02:00
parent b0bfe7e457 ead0a4ceb4
commit b4db9c84e7
3 changed files with 97 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+3
View File
@@ -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
packages/coding-agent/src/cli/update-cli.ts
+48 -7
View File
@@ -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<UpdateTarget> {
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,
packages/coding-agent/test/update-cli.test.ts
+46
View File
@@ -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");