Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions bin/ocr.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,20 @@ const hintFile = path.join(os.homedir(), ".opencodereview", "update-available");
try {
const hint = JSON.parse(fs.readFileSync(hintFile, "utf8"));
if (hint.version && hint.pkg) {
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`
);
// Normalize both sides: strip leading 'v' so "v1.8.6" == "1.8.6".
const pkgVersion = (() => {
try {
return require(path.join(__dirname, "..", "package.json")).version || "";
} catch (_) { return ""; }
})();
const hintNorm = hint.version.startsWith("v") ? hint.version.slice(1) : hint.version;
const pkgNorm = pkgVersion.startsWith("v") ? pkgVersion.slice(1) : pkgVersion;
if (!pkgNorm || hintNorm !== pkgNorm) {
Comment on lines +25 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The wrapper package version is not necessarily the installed native binary version: OCR_VERSION can pin the binary to an older release while package.json retains the newer wrapper version. In that case this comparison suppresses the hint even though the binary is outdated. Compare against the resolved binary's actual version, or persist and use the version selected during installation. [api mismatch]

Severity Level: Major ⚠️
- ⚠️ Pinned native binaries can be mistaken for current versions.

Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** bin/ocr.js
**Line:** 25:32
**Comment:**
	*Api Mismatch: The wrapper package version is not necessarily the installed native binary version: `OCR_VERSION` can pin the binary to an older release while `package.json` retains the newer wrapper version. In that case this comparison suppresses the hint even though the binary is outdated. Compare against the resolved binary's actual version, or persist and use the version selected during installation.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

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`
);
}
}
} catch (_) {}

Expand Down
7 changes: 5 additions & 2 deletions scripts/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,11 @@ async function main() {
if (!installedVersion) return;

const pkg = loadPackageJson();
const latestVersion = await fetchLatestVersion(pkg);
if (!latestVersion) return;
const rawLatest = await fetchLatestVersion(pkg);
if (!rawLatest) return;

// Normalize: strip leading 'v' so "v1.8.6" and "1.8.6" are treated equally.
const latestVersion = rawLatest.startsWith("v") ? rawLatest.slice(1) : rawLatest;

if (!SEMVER_RE.test(latestVersion)) return;

Expand Down
70 changes: 70 additions & 0 deletions scripts/update.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/usr/bin/env node
"use strict";

// Inline the two pure functions under test so we don't need a test framework.
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 normalizeVersion(v) {
return v && v.startsWith("v") ? v.slice(1) : v;
}

let passed = 0, failed = 0;
function assert(cond, msg) {
if (cond) { console.log(` ok: ${msg}`); passed++; }
else { console.error(`FAIL: ${msg}`); failed++; }
}

// --- semverGt ---
assert(!semverGt("1.8.6", "1.8.6"), "equal versions: not gt");
assert( semverGt("1.8.7", "1.8.6"), "patch bump: gt");
assert(!semverGt("1.8.5", "1.8.6"), "older: not gt");
assert( semverGt("2.0.0", "1.9.9"), "major bump: gt");
assert( semverGt("1.8.6", "1.8.6-beta"), "stable > pre-release: gt");
assert( semverGt("1.8.6", "1.8.6-rc1"), "stable > rc: gt");

// --- normalizeVersion (the v-prefix fix) ---
assert(normalizeVersion("v1.8.6") === "1.8.6", "strips leading v");
assert(normalizeVersion("1.8.6") === "1.8.6", "leaves bare version alone");
assert(normalizeVersion("") === "", "empty string unchanged");

// --- equal after normalize => no nudge ---
const latestRaw = "1.8.6"; // npm registry style
const installed = "1.8.6"; // from binary
const latest = normalizeVersion(latestRaw);
assert(SEMVER_RE.test(latest), "normalized version passes SEMVER_RE");
assert(!semverGt(latest, installed), "equal versions: no nudge");

// --- v-prefixed registry response handled correctly ---
const latestRawV = "v1.8.6";
const latestNorm = normalizeVersion(latestRawV);
assert(SEMVER_RE.test(latestNorm), "v-prefixed from registry normalizes and passes SEMVER_RE");
assert(!semverGt(latestNorm, "1.8.6"), "v-prefixed equal: no nudge");
assert( semverGt(latestNorm, "1.8.5"), "v-prefixed newer: nudge shown");

// --- hint display guard (ocr.js logic) ---
function shouldShowNudge(hintVersion, pkgVersion) {
const hintNorm = normalizeVersion(hintVersion);
const pkgNorm = normalizeVersion(pkgVersion);
return !pkgNorm || hintNorm !== pkgNorm;
}
assert(!shouldShowNudge("1.8.6", "1.8.6"), "same bare: no nudge");
assert(!shouldShowNudge("v1.8.6", "1.8.6"), "v-hint vs bare: no nudge");
assert(!shouldShowNudge("1.8.6", "v1.8.6"), "bare hint vs v-pkg: no nudge");
assert( shouldShowNudge("1.8.7", "1.8.6"), "newer hint: show nudge");
assert( shouldShowNudge("1.8.6", ""), "unknown pkg version: show nudge (safe default)");

console.log(`\n${passed} passed, ${failed} failed`);
if (failed > 0) process.exit(1);
Loading