fix(cli): restored binary update rollback
Rolled back binary updater replacements when post-install version verification fails instead of deleting the previous working binary first. Added a release workflow gate that downloads the published macOS arm64 asset and verifies codesign plus --version before npm publishing. Fixes #1240
This commit is contained in:
@@ -334,10 +334,31 @@ jobs:
|
||||
packages/coding-agent/binaries/omp-*
|
||||
generate_release_notes: true
|
||||
|
||||
|
||||
release_github_verify:
|
||||
if: ${{ startsWith(github.ref, 'refs/tags/v') && !cancelled() &&
|
||||
needs['release-github'].result == 'success' }}
|
||||
needs: [release-github]
|
||||
runs-on: macos-14
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- name: Download published macOS arm64 binary
|
||||
run: |
|
||||
curl -fsSL -o omp-darwin-arm64 "https://github.com/${{ github.repository }}/releases/download/${{ github.ref_name }}/omp-darwin-arm64"
|
||||
chmod +x omp-darwin-arm64
|
||||
- name: Verify published macOS arm64 binary
|
||||
run: |
|
||||
codesign -dv ./omp-darwin-arm64
|
||||
runtime_dir="$(mktemp -d)"
|
||||
HOME="$runtime_dir/home" XDG_DATA_HOME="$runtime_dir/xdg" ./omp-darwin-arm64 --version
|
||||
|
||||
release-npm:
|
||||
if: ${{ startsWith(github.ref, 'refs/tags/v') && !cancelled() &&
|
||||
needs.release_binary.result == 'success' && !inputs.skip_npm }}
|
||||
needs: [release_binary, rust-hash]
|
||||
needs.release_binary.result == 'success' &&
|
||||
needs.release_github_verify.result == 'success' &&
|
||||
!inputs.skip_npm }}
|
||||
needs: [release_binary, release_github_verify, rust-hash]
|
||||
runs-on: ubuntu-22.04
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed binary `omp update` rollbacks so a downloaded replacement that fails post-install version verification no longer remains installed over the previous working binary. ([#1240](https://github.com/can1357/oh-my-pi/issues/1240))
|
||||
|
||||
## [15.1.8] - 2026-05-20
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -20,6 +20,22 @@ interface ReleaseInfo {
|
||||
version: string;
|
||||
}
|
||||
|
||||
/** Result from running the installed binary and parsing its reported version. */
|
||||
export interface InstalledVersionVerification {
|
||||
ok: boolean;
|
||||
actual?: string;
|
||||
path?: string;
|
||||
}
|
||||
|
||||
/** Paths and verifier used while replacing a downloaded binary update. */
|
||||
export interface BinaryReplacementOptions {
|
||||
targetPath: string;
|
||||
tempPath: string;
|
||||
backupPath: string;
|
||||
expectedVersion: string;
|
||||
verifyInstalledVersion: (expectedVersion: string) => Promise<InstalledVersionVerification>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse update subcommand arguments.
|
||||
* Returns undefined if not an update command.
|
||||
@@ -197,9 +213,7 @@ function resolveOmpPath(): string | undefined {
|
||||
/**
|
||||
* Run the resolved omp binary and check if it reports the expected version.
|
||||
*/
|
||||
async function verifyInstalledVersion(
|
||||
expectedVersion: string,
|
||||
): Promise<{ ok: boolean; actual?: string; path?: string }> {
|
||||
async function verifyInstalledVersion(expectedVersion: string): Promise<InstalledVersionVerification> {
|
||||
const ompPath = resolveOmpPath();
|
||||
if (!ompPath) return { ok: false };
|
||||
try {
|
||||
@@ -215,29 +229,69 @@ async function verifyInstalledVersion(
|
||||
}
|
||||
}
|
||||
|
||||
function printVerifiedVersion(expectedVersion: string): void {
|
||||
console.log(chalk.green(`\n${theme.status.success} Updated to ${expectedVersion}`));
|
||||
}
|
||||
|
||||
function formatVerificationFailure(result: InstalledVersionVerification, expectedVersion: string): string {
|
||||
if (result.actual) {
|
||||
return `${APP_NAME} at ${result.path} still reports ${result.actual} (expected ${expectedVersion})`;
|
||||
}
|
||||
return `could not verify updated version${result.path ? ` at ${result.path}` : ""}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Print post-update verification result.
|
||||
*/
|
||||
async function printVerification(expectedVersion: string): Promise<void> {
|
||||
const result = await verifyInstalledVersion(expectedVersion);
|
||||
if (result.ok) {
|
||||
console.log(chalk.green(`\n${theme.status.success} Updated to ${expectedVersion}`));
|
||||
printVerifiedVersion(expectedVersion);
|
||||
return;
|
||||
}
|
||||
if (result.actual) {
|
||||
console.log(
|
||||
chalk.yellow(
|
||||
`\nWarning: ${APP_NAME} at ${result.path} still reports ${result.actual} (expected ${expectedVersion})`,
|
||||
),
|
||||
);
|
||||
} else {
|
||||
console.log(
|
||||
chalk.yellow(`\nWarning: could not verify updated version${result.path ? ` at ${result.path}` : ""}`),
|
||||
);
|
||||
}
|
||||
console.log(chalk.yellow(`\nWarning: ${formatVerificationFailure(result, expectedVersion)}`));
|
||||
console.log(chalk.yellow(`You may need to reinstall: curl -fsSL https://omp.sh/install | sh`));
|
||||
}
|
||||
|
||||
async function unlinkIfExists(filePath: string): Promise<void> {
|
||||
try {
|
||||
await fs.promises.unlink(filePath);
|
||||
} catch (err) {
|
||||
if (!isEnoent(err)) throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomically replace the installed binary and roll back if version verification fails.
|
||||
*/
|
||||
export async function replaceBinaryForUpdate(options: BinaryReplacementOptions): Promise<InstalledVersionVerification> {
|
||||
let backupReady = false;
|
||||
try {
|
||||
await unlinkIfExists(options.backupPath);
|
||||
await fs.promises.rename(options.targetPath, options.backupPath);
|
||||
backupReady = true;
|
||||
await fs.promises.rename(options.tempPath, options.targetPath);
|
||||
|
||||
const verification = await options.verifyInstalledVersion(options.expectedVersion);
|
||||
if (!verification.ok) {
|
||||
throw new Error(
|
||||
`${formatVerificationFailure(verification, options.expectedVersion)}; restored previous ${APP_NAME} binary`,
|
||||
);
|
||||
}
|
||||
|
||||
backupReady = false;
|
||||
await unlinkIfExists(options.backupPath);
|
||||
return verification;
|
||||
} catch (err) {
|
||||
if (backupReady) {
|
||||
await unlinkIfExists(options.targetPath);
|
||||
await fs.promises.rename(options.backupPath, options.targetPath);
|
||||
}
|
||||
await unlinkIfExists(options.tempPath);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Update via bun package manager.
|
||||
*/
|
||||
@@ -271,27 +325,15 @@ async function updateViaBinaryAt(targetPath: string, expectedVersion: string): P
|
||||
await pipeline(response.body, fileStream);
|
||||
|
||||
console.log(chalk.dim("Installing update..."));
|
||||
try {
|
||||
try {
|
||||
await fs.promises.unlink(backupPath);
|
||||
} catch (err) {
|
||||
if (!isEnoent(err)) throw err;
|
||||
}
|
||||
await fs.promises.rename(targetPath, backupPath);
|
||||
await fs.promises.rename(tempPath, targetPath);
|
||||
await fs.promises.unlink(backupPath);
|
||||
|
||||
await printVerification(expectedVersion);
|
||||
console.log(chalk.dim(`Restart ${APP_NAME} to use the new version`));
|
||||
} catch (err) {
|
||||
if (fs.existsSync(backupPath) && !fs.existsSync(targetPath)) {
|
||||
await fs.promises.rename(backupPath, targetPath);
|
||||
}
|
||||
if (fs.existsSync(tempPath)) {
|
||||
await fs.promises.unlink(tempPath);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
await replaceBinaryForUpdate({
|
||||
targetPath,
|
||||
tempPath,
|
||||
backupPath,
|
||||
expectedVersion,
|
||||
verifyInstalledVersion,
|
||||
});
|
||||
printVerifiedVersion(expectedVersion);
|
||||
console.log(chalk.dim(`Restart ${APP_NAME} to use the new version`));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,6 +1,20 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { resolveUpdateMethodForTest } from "../src/cli/update-cli";
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { replaceBinaryForUpdate, resolveUpdateMethodForTest } from "../src/cli/update-cli";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function makeTempDir(): Promise<string> {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-update-test-"));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
describe("update-cli install target detection", () => {
|
||||
it("uses bun update when prioritized omp is inside bun global bin", () => {
|
||||
const method = resolveUpdateMethodForTest("/Users/test/.bun/bin/omp", "/Users/test/.bun/bin");
|
||||
@@ -20,3 +34,49 @@ describe("update-cli install target detection", () => {
|
||||
expect(method).toBe("binary");
|
||||
});
|
||||
});
|
||||
|
||||
describe("update-cli binary replacement", () => {
|
||||
it("restores the previous binary when the replacement fails verification", async () => {
|
||||
const dir = await makeTempDir();
|
||||
const targetPath = path.join(dir, "omp");
|
||||
const tempPath = `${targetPath}.new`;
|
||||
const backupPath = `${targetPath}.bak`;
|
||||
await Bun.write(targetPath, "old binary");
|
||||
await Bun.write(tempPath, "broken binary");
|
||||
|
||||
await expect(
|
||||
replaceBinaryForUpdate({
|
||||
targetPath,
|
||||
tempPath,
|
||||
backupPath,
|
||||
expectedVersion: "15.1.8",
|
||||
verifyInstalledVersion: async () => ({ ok: false, path: targetPath }),
|
||||
}),
|
||||
).rejects.toThrow("restored previous omp binary");
|
||||
|
||||
expect(await Bun.file(targetPath).text()).toBe("old binary");
|
||||
expect(await Bun.file(tempPath).exists()).toBe(false);
|
||||
expect(await Bun.file(backupPath).exists()).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the replacement only after it reports the expected version", async () => {
|
||||
const dir = await makeTempDir();
|
||||
const targetPath = path.join(dir, "omp");
|
||||
const tempPath = `${targetPath}.new`;
|
||||
const backupPath = `${targetPath}.bak`;
|
||||
await Bun.write(targetPath, "old binary");
|
||||
await Bun.write(tempPath, "new binary");
|
||||
|
||||
await replaceBinaryForUpdate({
|
||||
targetPath,
|
||||
tempPath,
|
||||
backupPath,
|
||||
expectedVersion: "15.1.8",
|
||||
verifyInstalledVersion: async () => ({ ok: true, actual: "15.1.8", path: targetPath }),
|
||||
});
|
||||
|
||||
expect(await Bun.file(targetPath).text()).toBe("new binary");
|
||||
expect(await Bun.file(tempPath).exists()).toBe(false);
|
||||
expect(await Bun.file(backupPath).exists()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user