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

Revert "fix(coding-agent): routed foreign aliases to binary updates"

Browse Source 
This reverts commit 834002adc5.
This commit is contained in:
can1357
2026-08-14 00:13:06 +02:00
parent 0d6a7146a3
commit 6ecb1e3383
3 changed files with 19 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
-1
View File
@@ -5,7 +5,6 @@
### Fixed
- Fixed the parent TUI stalling after a subagent submits its result until terminal focus or resize wakes the event loop ([#8462](https://github.com/can1357/oh-my-pi/issues/8462)).
- Fixed `omp update` routing foreign npm/bun bin-directory alias symlinks through the package manager, causing npm EEXIST instead of updating the aliased standalone binary ([#8468](https://github.com/can1357/oh-my-pi/issues/8468)).
- Fixed `read` hashline headers collapsing nested in-workspace paths to the bare basename, which let a same-basename file at the session cwd capture a verbatim follow-up `edit` and deterministically reject it with `hash is not from this session`. Headers now retain the workspace-relative path (e.g. `[src/settings.json#0063]`) ([#8482](https://github.com/can1357/oh-my-pi/issues/8482)).
## [17.3.1] - 2026-08-13
packages/coding-agent/src/cli/update-cli.ts
+19 -59
View File
@@ -481,11 +481,6 @@ interface UpdateMethodResolutionOptions {
* target directory.
*/
ompIsRegularFile?: boolean;
/**
* Resolved symlink target. npm/bun own links into their manager root;
* targets elsewhere are foreign aliases that must update as binaries.
*/
ompRealpath?: string;
}
type UpdateTarget =
@@ -501,7 +496,7 @@ function resolveUpdateMethod(
bunBinDir: string | undefined,
options: UpdateMethodResolutionOptions = {},
): UpdateMethod {
const { homebrewPrefix, miseBinDirs = [], miseDataDir, npmBinDir, ompIsRegularFile = false, ompRealpath } = options;
const { homebrewPrefix, miseBinDirs = [], miseDataDir, npmBinDir, ompIsRegularFile = false } = options;
const launcherExtension = path.extname(ompPath).toLowerCase();
const isWindowsScriptLauncher =
launcherExtension === ".cmd" || launcherExtension === ".ps1" || launcherExtension === ".bat";
@@ -519,31 +514,9 @@ function resolveUpdateMethod(
// (bun's .exe launcher, npm's .cmd/.ps1), so a regular file is NOT evidence
// of a standalone install and the override would hijack managed installs.
const isStandaloneRegularFile = ompIsRegularFile && process.platform !== "win32";
if (
bunBinDir &&
isPathInDirectory(ompPath, bunBinDir) &&
!isStandaloneRegularFile &&
(!ompRealpath ||
isPathInDirectoryLexical(
ompRealpath,
tryRealpath(path.dirname(bunBinDir)) ?? path.dirname(path.resolve(bunBinDir)),
))
) {
return "bun";
}
if (
npmBinDir &&
isPathInDirectory(ompPath, npmBinDir) &&
!isStandaloneRegularFile &&
(!ompRealpath ||
isPathInDirectoryLexical(
ompRealpath,
tryRealpath(path.dirname(npmBinDir)) ?? path.dirname(path.resolve(npmBinDir)),
))
) {
if (bunBinDir && isPathInDirectory(ompPath, bunBinDir) && !isStandaloneRegularFile) return "bun";
if ((npmBinDir && isPathInDirectory(ompPath, npmBinDir) && !isStandaloneRegularFile) || isWindowsScriptLauncher)
return "npm";
}
if (isWindowsScriptLauncher) return "npm";
return "binary";
}
@@ -554,33 +527,6 @@ export function resolveUpdateMethodForTest(
): UpdateMethod {
return resolveUpdateMethod(ompPath, bunBinDir, options);
}
/** Resolve an install target from a concrete PATH entry without probing package managers. */
export function resolveUpdateTargetFromPathForTest(
ompPath: string,
bunBinDir: string | undefined,
options: UpdateMethodResolutionOptions & { allowPackageManagers: boolean },
): UpdateTarget {
let ompIsRegularFile = false;
let ompIsSymlink = false;
let ompRealpath: string | undefined;
try {
const stat = fs.lstatSync(ompPath);
ompIsRegularFile = stat.isFile() && !stat.isSymbolicLink();
ompIsSymlink = stat.isSymbolicLink();
if (ompIsSymlink) ompRealpath = tryRealpath(ompPath);
} catch {}
const method = resolveUpdateMethod(ompPath, bunBinDir, { ...options, ompIsRegularFile, ompRealpath });
if (method === "binary") {
// Preserve a foreign alias and replace the standalone binary it
// resolves to. Binary-only releases still replace manager launchers
// in place because package-manager detection is intentionally off.
const binaryPath = options.allowPackageManagers ? (ompRealpath ?? ompPath) : ompPath;
return { method, path: binaryPath, replacesSymlink: ompIsSymlink && binaryPath === ompPath };
}
if (method === "bun" || method === "npm") return { method, path: ompPath };
return { method };
}
/**
* Resolve how the running install should be updated.
*
@@ -600,13 +546,27 @@ async function resolveUpdateTarget(options: { allowPackageManagers: boolean }):
const ompPath = resolveOmpPath();
if (ompPath) {
return resolveUpdateTargetFromPathForTest(ompPath, bunBinDir, {
allowPackageManagers: options.allowPackageManagers,
// Package-manager installs symlink the bin entry into node_modules; the
// standalone installer writes a plain executable. When the global bin dir
// overlaps the installer's default (~/.local/bin), that file type — not
// directory containment — distinguishes a binary install from npm/bun.
let ompIsRegularFile = false;
let ompIsSymlink = false;
try {
const stat = fs.lstatSync(ompPath);
ompIsRegularFile = stat.isFile() && !stat.isSymbolicLink();
ompIsSymlink = stat.isSymbolicLink();
} catch {}
const method = resolveUpdateMethod(ompPath, bunBinDir, {
homebrewPrefix,
miseBinDirs,
miseDataDir,
npmBinDir,
ompIsRegularFile,
});
if (method === "binary") return { method, path: ompPath, replacesSymlink: ompIsSymlink };
if (method === "bun" || method === "npm") return { method, path: ompPath };
return { method };
}
if (bunBinDir) return { method: "bun" };
packages/coding-agent/test/update-cli.test.ts
-47
View File
@@ -26,7 +26,6 @@ import {
resolveReleaseDist,
resolveReleaseRename,
resolveUpdateMethodForTest,
resolveUpdateTargetFromPathForTest,
shouldForceBinaryUpdate,
sweepStaleUpdateArtifacts,
updateViaBinaryAt,
@@ -188,52 +187,6 @@ describe("update-cli install target detection", () => {
expect(method).toBe("npm");
});
it("updates the resolved standalone binary behind a foreign npm-bin alias", async () => {
// realpath: macOS tmpdir lives behind the /var -> /private/var symlink and
// the resolver returns realpathed targets.
const dir = await fs.realpath(await makeTempDir());
const npmBinDir = path.join(dir, ".npm-global", "bin");
const standalonePath = path.join(dir, ".local", "bin", "omp");
const aliasPath = path.join(npmBinDir, "omp");
await fs.mkdir(npmBinDir, { recursive: true });
await fs.mkdir(path.dirname(standalonePath), { recursive: true });
await Bun.write(standalonePath, "binary");
await fs.symlink(standalonePath, aliasPath);
const target = resolveUpdateTargetFromPathForTest(aliasPath, undefined, {
allowPackageManagers: true,
npmBinDir,
});
expect(target).toEqual({ method: "binary", path: standalonePath, replacesSymlink: false });
expect(await fs.readlink(aliasPath)).toBe(standalonePath);
});
it("uses npm update when the bin symlink resolves into npm's global install tree", () => {
const method = resolveUpdateMethodForTest("/home/u/.npm-global/bin/omp", undefined, {
npmBinDir: "/home/u/.npm-global/bin",
ompRealpath: "/home/u/.npm-global/lib/node_modules/@oh-my-pi/pi-coding-agent/dist/cli.js",
});
expect(method).toBe("npm");
});
it("uses binary update when the bun global bin entry is a foreign alias symlink", () => {
const method = resolveUpdateMethodForTest("/home/u/.bun/bin/omp", "/home/u/.bun/bin", {
ompRealpath: "/home/u/.local/bin/omp",
});
expect(method).toBe("binary");
});
it("uses bun update when the bin symlink resolves into bun's global install tree", () => {
const method = resolveUpdateMethodForTest("/home/u/.bun/bin/omp", "/home/u/.bun/bin", {
ompRealpath: "/home/u/.bun/install/global/node_modules/@oh-my-pi/pi-coding-agent/dist/cli.js",
});
expect(method).toBe("bun");
});
it("uses binary update when prioritized omp is outside bun global bin", () => {
const method = resolveUpdateMethodForTest("/Users/test/.local/bin/omp", "/Users/test/.bun/bin");