fix(updater): discard stale version hints

bin/ocr.js

@@ -7,6 +7,8 @@ const fs = require("fs");
 const os = require("os");
 
 const { resolveNativeBinary } = require("../scripts/platform");
+const { version: packageVersion } = require("../package.json");
+const { shouldShowUpdateHint } = require("../scripts/version");
 
 const resolved = resolveNativeBinary();
 if (!resolved) {
@@ -20,11 +22,13 @@ const binaryPath = resolved.path;
 const hintFile = path.join(os.homedir(), ".opencodereview", "update-available");
 try {
   const hint = JSON.parse(fs.readFileSync(hintFile, "utf8"));
-  if (hint.version && hint.pkg) {
+  if (hint.pkg && shouldShowUpdateHint(hint.version, packageVersion)) {
     console.error(
       `\x1b[33m[ocr] A new version (v${hint.version}) is available. Run to update:\x1b[0m\n` +
       `\x1b[33m  npm i -g ${hint.pkg}@${hint.version}\x1b[0m\n`
     );
+  } else {
+    fs.unlinkSync(hintFile);
   }
 } catch (_) {}
 

package.json

@@ -9,12 +9,14 @@
     "bin/ocr.js",
     "scripts/install.js",
     "scripts/update.js",
+    "scripts/version.js",
     "scripts/platform.js",
     "imgs/"
   ],
   "scripts": {
     "postinstall": "node scripts/install.js",
-    "test:github-actions": "node scripts/github-actions/post-review-comments.test.js && node scripts/github-actions/check-translation-sync.test.js"
+    "test:github-actions": "node scripts/github-actions/post-review-comments.test.js && node scripts/github-actions/check-translation-sync.test.js",
+    "test:update": "node scripts/version.test.js"
   },
   "repository": {
     "type": "git",

scripts/update.js

@@ -9,6 +9,7 @@ const { spawnSync } = require("child_process");
 
 const { resolveNativeBinary } = require("./platform");
 const { loadPackageJson } = require("./install.js");
+const { SEMVER_RE, parseVersionOutput, semverGt } = require("./version");
 
 const stateDir = path.join(os.homedir(), ".opencodereview");
 const tsFile = path.join(stateDir, "last-update-check");
@@ -62,8 +63,7 @@ function getInstalledVersion(binPath) {
       encoding: "utf8",
       timeout: 3000,
     });
-    const match = (result.stdout || "").match(/v(\d+\.\d+(?:\.\d+)?)/);
-    return match ? match[1] : null;
+    return parseVersionOutput(result.stdout);
   } catch (_) {
     return null;
   }
@@ -109,21 +109,6 @@ function fetchLatestVersion(pkg) {
   });
 }
 
-const SEMVER_RE = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/;
-
-function semverGt(a, b) {
-  const pa = a.replace(/-.*$/, "").split(".").map(Number);
-  const pb = b.replace(/-.*$/, "").split(".").map(Number);
-  for (let i = 0; i < 3; i++) {
-    if ((pa[i] || 0) > (pb[i] || 0)) return true;
-    if ((pa[i] || 0) < (pb[i] || 0)) return false;
-  }
-  const aPre = a.includes("-");
-  const bPre = b.includes("-");
-  if (bPre && !aPre) return true;
-  return false;
-}
-
 function writeHint(latestVersion, pkgName) {
   try {
     fs.writeFileSync(hintFile, JSON.stringify({ version: latestVersion, pkg: pkgName }));

scripts/version.js

@@ -0,0 +1,42 @@
+"use strict";
+
+const SEMVER_RE =
+  /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
+
+function parseVersionOutput(output) {
+  const match = String(output || "").match(
+    /v(\d+\.\d+(?:\.\d+)?(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)/
+  );
+  return match ? match[1] : null;
+}
+
+function semverGt(a, b) {
+  if (!SEMVER_RE.test(a) || !SEMVER_RE.test(b)) return false;
+
+  const aWithoutBuild = a.replace(/\+.*$/, "");
+  const bWithoutBuild = b.replace(/\+.*$/, "");
+  const pa = aWithoutBuild.replace(/-.*$/, "").split(".").map(Number);
+  const pb = bWithoutBuild.replace(/-.*$/, "").split(".").map(Number);
+  for (let i = 0; i < 3; i++) {
+    if (pa[i] > pb[i]) return true;
+    if (pa[i] < pb[i]) return false;
+  }
+  const aPre = aWithoutBuild.includes("-");
+  const bPre = bWithoutBuild.includes("-");
+  if (bPre && !aPre) return true;
+  return false;
+}
+
+function shouldShowUpdateHint(hintVersion, installedVersion) {
+  if (!SEMVER_RE.test(hintVersion) || !SEMVER_RE.test(installedVersion)) {
+    return false;
+  }
+  return semverGt(hintVersion, installedVersion);
+}
+
+module.exports = {
+  SEMVER_RE,
+  parseVersionOutput,
+  semverGt,
+  shouldShowUpdateHint,
+};

scripts/version.test.js

@@ -0,0 +1,37 @@
+"use strict";
+
+const assert = require("assert");
+
+const {
+  parseVersionOutput,
+  semverGt,
+  shouldShowUpdateHint,
+} = require("./version");
+
+assert.strictEqual(
+  parseVersionOutput("open-code-review v1.8.6 (1b193db35) darwin/arm64"),
+  "1.8.6"
+);
+assert.strictEqual(parseVersionOutput("version unavailable"), null);
+assert.strictEqual(
+  parseVersionOutput("open-code-review v1.8.6-beta.1+darwin.arm64"),
+  "1.8.6-beta.1+darwin.arm64"
+);
+
+assert.strictEqual(semverGt("1.8.7", "1.8.6"), true);
+assert.strictEqual(semverGt("1.8.6", "1.8.6"), false);
+assert.strictEqual(semverGt("1.8.5", "1.8.6"), false);
+assert.strictEqual(semverGt("1.8.6", "1.8.6-beta.1"), true);
+assert.strictEqual(semverGt("1.8.6+build.2", "1.8.6+build.1"), false);
+assert.strictEqual(semverGt("1.8.6", "1.8.6-beta.1+build.1"), true);
+assert.strictEqual(semverGt("not-a-version", "1.8.6"), false);
+
+assert.strictEqual(shouldShowUpdateHint("1.8.7", "1.8.6"), true);
+assert.strictEqual(shouldShowUpdateHint("1.8.6", "1.8.6"), false);
+assert.strictEqual(shouldShowUpdateHint("1.8.5", "1.8.6"), false);
+assert.strictEqual(shouldShowUpdateHint("not-a-version", "1.8.6"), false);
+assert.strictEqual(shouldShowUpdateHint("1.8.7", null), false);
+assert.strictEqual(shouldShowUpdateHint("1.8.7", "not-a-version"), false);
+assert.strictEqual(shouldShowUpdateHint("1.8.6+new", "1.8.6+old"), false);
+
+console.log("version tests passed");